PaloAltoNetworks / pan-os-ansible

Ansible collection for easy automation of Palo Alto Networks next generation firewalls and Panorama, in both physical and virtual form factors.
https://pan.dev/ansible/docs/panos
Apache License 2.0
209 stars 97 forks source link

panos_commit_push for log collector group gives NoneType exception #369

Open ShreyasNBS opened 1 year ago

ShreyasNBS commented 1 year ago

Describe the bug

When I run the following code

- name: push log collector config to EW firewalls
  paloaltonetworks.panos.panos_commit_push:
    provider: '{{ provider }}'
    description: 'push log collector config to firewalls'
    style: 'log collector group'
    name: "LogCollectorGroup"

- name: wait for commits to finish
  paloaltonetworks.panos.panos_check:
    provider: '{{ rovider }}'
    initial_delay: 5
    interval: 5
    timeout: 100

I get the exception

TASK [push log collector config to firewalls] *******************************
--
  | An exception occurred during task execution. To see the full traceback, use -vvv. The error was: TypeError: 'NoneType' object is not subscriptable
  | fatal: [localhost]: FAILED! => {"changed": false, "module_stderr": "Traceback (most recent call last):\n  File \"/opt/app/.ansible/tmp/ansible-tmp-1672958494.5937903-242-47407665068504/AnsiballZ_panos_commit_push.py\", line 107, in <module>\n    _ansiballz_main()\n  File \"/opt/app/.ansible/tmp/ansible-tmp-1672958494.5937903-242-47407665068504/AnsiballZ_panos_commit_push.py\", line 99, in _ansiballz_main\n    invoke_module(zipped_mod, temp_path, ANSIBALLZ_PARAMS)\n  File \"/opt/app/.ansible/tmp/ansible-tmp-1672958494.5937903-242-47407665068504/AnsiballZ_panos_commit_push.py\", line 47, in invoke_module\n    runpy.run_module(mod_name='ansible_collections.paloaltonetworks.panos.plugins.modules.panos_commit_push', init_globals=dict(_module_fqn='ansible_collections.paloaltonetworks.panos.plugins.modules.panos_commit_push', _modlib_path=modlib_path),\n  File \"/usr/lib/python3.10/runpy.py\", line 224, in run_module\n    return _run_module_code(code, init_globals, run_name, mod_spec)\n  File \"/usr/lib/python3.10/runpy.py\", line 96, in _run_module_code\n    _run_code(code, mod_globals, init_globals,\n  File \"/usr/lib/python3.10/runpy.py\", line 86, in _run_code\n    exec(code, run_globals)\n  File \"/tmp/ansible_paloaltonetworks.panos.panos_commit_push_payload_xwl_us8v/ansible_paloaltonetworks.panos.panos_commit_push_payload.zip/ansible_collections/paloaltonetworks/panos/plugins/modules/panos_commit_push.py\", line 206, in <module>\n  File \"/tmp/ansible_paloaltonetworks.panos.panos_commit_push_payload_xwl_us8v/ansible_paloaltonetworks.panos.panos_commit_push_payload.zip/ansible_collections/paloaltonetworks/panos/plugins/modules/panos_commit_push.py\", line 194, in main\nTypeError: 'NoneType' object is not subscriptable\n", "module_stdout": "", "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", "rc": 1}

I have a device group and template stack configured via ansible and they work fine when I commit. The only "gotcha" could be that I do not have any managed firewalls added yet (but if the behaviour was consistent, then commit to device group and template stack should also fail?)

Another interesting thing to note is that the commit actually goes through on the panorama side, even though ansible errors. So I am presuming the issue is on the panos collection side.

I am using the latest panos ansible collection, and I am on panorama 10.2.2-h2.

jamesholland-uk commented 1 year ago

FYI: This is a pan-os-python issue, it is reproducible outside of Ansible. We will need to progress a fix in that SDK to fix this issue.

ShreyasNBS commented 1 year ago

@jamesholland-uk Can we add the ignore_disconnect flag like we have on panos_op? We had a similar issue in the past when you set serial number (empty response).

- name: set panorama serial number if it does not match the current one
  no_log: true
  paloaltonetworks.panos.panos_op:
    provider: '{{ primary_provider }}'
    cmd: "<set><serial-number>{{ primary_panos_data['serial_number'] }}</serial-number></set>"
    cmd_is_xml: true
    ignore_disconnect: true
  when: "serial != primary_panos_data['serial_number']"
jamesholland-uk commented 1 year ago

Hi @ShreyasNBS, that could be one of probably multiple solutions, let's see which is the best solution though

ShreyasNBS commented 1 year ago

Thanks @jamesholland-uk . Another question based on the same issue above. We are attempting a commit and push for log collector group to managed devices via lambda function. This is done when a new firewall spins up, grabs a license from CSP, has the device group and template stack pushed to it (via commit-all). Code below

def panorama_push_log_collector_group(panoramaIpAddress, panoramaApiKey, ewLogCollectorGroupName, instanceId, lifeCycleHook, autoScalingGroup):
    # Panorama push (or commit-all in Pano land) log collector group to firewall.
    try:
        request = "https://" + panoramaIpAddress + "/api/?type=commit&action=all&cmd=<commit-all><log-collector-config><log-collector-group>" + ewLogCollectorGroupName + "</log-collector-group></log-collector-config></commit-all>&key="+ panoramaApiKey
        response_panorama_commit_all = http.request('POST', request).data.decode('utf-8')
        logger.info("Launch: [Panorama Push Log Collector Group Response]: %s", response_panorama_commit_all) 
        resp_panorama_commit_all = et.fromstring(response_panorama_commit_all) 
        messageElement = resp_panorama_commit_all.findall(".//msg")
        message = messageElement[0].text       
        if message is not None and message.find("There are no changes to commit") != -1:
            logger.info("Launch: No panorama pending commits")
            return True
        else:
            jobIdElement = resp_panorama_commit_all.findall(".//job")
            jobId = jobIdElement[0].text
            return check_job_status(jobId, panoramaIpAddress, panoramaApiKey, instanceId, lifeCycleHook, autoScalingGroup)
    except Exception as e: 
        logger.error('Launch: Panorama Push Log Collector Group failed')     
        logger.error(e)
        abandon_lifecycle_hook(instanceId, lifeCycleHook, autoScalingGroup)
        return False

The same code logic works for DG and TS (different api call of course), but fails for LCG. The error we get is

image

If we try to execute the command in a browser via panorama API, it works

MicrosoftTeams-image (12)

The next thing I am going to try is adding "merge with candidate config" if it's allowed for LCG, but that's the only thing remaining tbh.

ShreyasNBS commented 1 year ago

Ok another quick update. We changed the timeout for the LCG commit-all request to 120 seconds, and it returned success response. However this does not seem to fit in the generic "commit-all" response structure which returns a Job ID that we have to poll. In this case, the success response was similar to one we tried via browser. Honestly, I feel stupid, because the answer was staring right in my face. For DG and TS commit-all, the response is "Job ID xxx enqueued". So we are going to change the logic to just check the success status and move on. Still not sure why the pan-os-sdk returns an empty response though.

MicrosoftTeams-image (13)