Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(panos/base): Added check for commit_response text validation #557

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

horiagunica
Copy link
Collaborator

Description

With latest develop/master branch commit (not yet released) - there was an issue committing to Panorama which has no changes to commit (discovered when running the panos_commit_panorama ansible module).

Motivation and Context

Simple python script to test the issue:

from panos.panorama import Panorama, PanoramaCommitAll, PanoramaCommit

panorama = Panorama("<YOUR_IP>", "<YOUR_USERNAME>", "<YOUR_PASSWORD>")

cmd = PanoramaCommit()
sync = True

result = panorama.commit(cmd=cmd, sync=sync)

print(result)

Result:

python3 python_test_panorama.py
Traceback (most recent call last):
  File "/Users/hgunica/Documents/PROJECTS/kyndryl/pso-azure-kyndryl-1089022/kx-panw-cac/ansible/panorama-cac/.venv/lib/python3.11/site-packages/panos/base.py", line 4899, in _commit
    jobid = commit_response.find("./result/job").text
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'text'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/hgunica/Documents/PROJECTS/kyndryl/pso-azure-kyndryl-1089022/kx-panw-cac/ansible/panorama-cac/python_test_panorama.py", line 17, in <module>
    result = panorama.commit(cmd=cmd, sync=sync)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/hgunica/Documents/PROJECTS/kyndryl/pso-azure-kyndryl-1089022/kx-panw-cac/ansible/panorama-cac/.venv/lib/python3.11/site-packages/panos/base.py", line 4803, in commit
    return self._commit(
           ^^^^^^^^^^^^^
  File "/Users/hgunica/Documents/PROJECTS/kyndryl/pso-azure-kyndryl-1089022/kx-panw-cac/ansible/panorama-cac/.venv/lib/python3.11/site-packages/panos/base.py", line 4906, in _commit
    commit_response_msg = commit_response.find("./msg/line").text
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'text'

How Has This Been Tested?

Tested with above mentioned script. After validation was added:

python3 python_test_panorama.py
None

Ansible task also passes:

TASK [bootstrap_panorama : Commit to Panorama] *************************************************************************************************
ok: [panorama-mgmt-secondary]

Test with an actual commit is still working properly:

python3 python_test_panorama.py
{'success': True, 'result': 'OK', 'jobid': '24', 'user': 'panadmin', 'warnings': {'line': 'The latest API KeyGen was executed on Sat Mar 23 08:31:58 2024 with the deprecated algorithm. You are advised to configure the more secure API key infrastructure by web interface: Setup -> Management -> Authentiation Settings -> API Key Certificate, or by CLI: set deviceconfig setting management api key certificate\n'}, 'starttime': '2024/03/23 08:31:59', 'endtime': '2024/03/23 08:32:17', 'messages': ['Configuration committed successfully', 'Local configuration size: 9 KB', 'Predefined configuration size: 16 MB', 'Total configuration size(local, predefined): 17 MB', 'Maximum recommended configuration size: 120 MB (14% configured)'], 'devices': {}, 'xml': <Element 'response' at 0x10251ffb0>}

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes if appropriate.
  • All new and existing tests passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant