search

PaloAltoNetworks / pan-os-python

The PAN-OS SDK for Python is a package to help interact with Palo Alto Networks devices (including physical and virtualized Next-generation Firewalls and Panorama). The pan-os-python SDK is object oriented and mimics the traditional interaction with the device via the GUI or CLI/API.
https://pan-os-python.readthedocs.io
ISC License
339 stars 166 forks source link

Fix action_community_argument vartype #535

Closed adambaumeister closed 5 months ago

adambaumeister commented 8 months ago

Description

Fixes #534

In at least as early as 9.1, the 'append community' action within BGP is a member list which was causing BGP updates to fail when attempting to update an existing BGP protocol element with this configured.

All that has changed is the vartype of "action_community_argument" to be "member".

Example XML from 9.1 for this path;

<response status="success" code="19">
    <result total-count="1" count="1">
        <append admin="panadmin" dirtyId="12" time="2023/11/21 11:36:45">
            <member admin="panadmin" dirtyId="12" time="2023/11/21 11:36:45">local-as</member>
        </append>
    </result>
</response>
shinmog commented 7 months ago

the spec is wrong for this, and the issue is that while "remove-regex" takes one param, if the user does either "append" or "overwrite" those are actually arrays. so there can't be one single value param that works for everything when the vartypes of stuff is different.

the fix for this is that there needs to be a new param for when the user does action=allow / action_community_type of "append" or "overwrite", would be vartype="member". at the same time, i'd recommend changing the condition of "action_community_argument" to just be action=allow / action_community_type="remove-regex".

looking at the schemas, this doesn't seem like it could have ever worked, so even tho we're modifying a pre-existing param's definition, it is not a breaking change for anyone.

adambaumeister commented 6 months ago

@shinmog customer was asking about this one

shinmog commented 6 months ago

The code is fine, but the CI is still failing on the docs task. There's another PR open for that.

adambaumeister commented 5 months ago

Now that the other PR is closed do we have to do anything with this one or can it be merged? @shinmog

paulmnguyen commented 5 months ago

The PR to fix the docs was merged into develop. @adambaumeister could you rebase develop and push your branch.

adambaumeister commented 5 months ago

@paulmnguyen done. I had to reset to origin/develop then cherry-pick, rebase didn't work for some reason.