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 action_community_argument vartype #535

Merged

Conversation

adambaumeister
Copy link
Collaborator

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
Copy link
Collaborator

shinmog commented Dec 7, 2023

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
Copy link
Collaborator Author

@shinmog customer was asking about this one

Copy link
Collaborator

@shinmog shinmog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@shinmog
Copy link
Collaborator

shinmog commented Jan 12, 2024

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

@adambaumeister
Copy link
Collaborator Author

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

@paulmnguyen paulmnguyen requested a review from shinmog February 5, 2024 21:57
@paulmnguyen paulmnguyen self-assigned this Feb 5, 2024
@paulmnguyen
Copy link
Contributor

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

@adambaumeister adambaumeister force-pushed the bugfix/bgp-append-community branch from 586f058 to 1b74186 Compare February 6, 2024 03:45
@adambaumeister
Copy link
Collaborator Author

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

@shinmog shinmog merged commit 58ce888 into PaloAltoNetworks:develop Feb 6, 2024
7 checks passed
github-actions bot pushed a commit that referenced this pull request Apr 11, 2024
## [1.12.0](v1.11.0...v1.12.0) (2024-04-11)

### Features

* **ApplicationFilter:** Add new_appid parameter ([#547](#547)) ([bc96108](bc96108))
* **Firewall:** New fields for show_system_resources ([#544](#544)) ([9e8cc2a](9e8cc2a))
* Handling Log Collector Group (LCG) pushes ([#493](#493)) ([7e87952](7e87952))
* **panos/network:** Advanced routing engine ([#539](#539)) ([173bb8a](173bb8a))
* Enhance updater logic ([#548](#548)) ([23ed1ad](23ed1ad))
* **network.BgpPolicyRule:** Add action_community_modifier param ([#535](#535)) ([58ce888](58ce888)), closes [#534](#534)
* **PanDevice:** add `is_ready()` ([#532](#532)) ([a6b018e](a6b018e))

### Bug Fixes

* **_parse_job_results:** Catch None details in response ([#471](#471)) ([f01ae25](f01ae25)), closes [#470](#470)
github-actions bot pushed a commit that referenced this pull request Apr 11, 2024
## [1.12.0](v1.11.0...v1.12.0) (2024-04-11)

### Features

* **ApplicationFilter:** Add new_appid parameter ([#547](#547)) ([bc96108](bc96108))
* **Firewall:** New fields for show_system_resources ([#544](#544)) ([9e8cc2a](9e8cc2a))
* Handling Log Collector Group (LCG) pushes ([#493](#493)) ([7e87952](7e87952))
* **panos/network:** Advanced routing engine ([#539](#539)) ([173bb8a](173bb8a))
* Enhance updater logic ([#548](#548)) ([23ed1ad](23ed1ad))
* **network.BgpPolicyRule:** Add action_community_modifier param ([#535](#535)) ([58ce888](58ce888)), closes [#534](#534)
* **PanDevice:** add `is_ready()` ([#532](#532)) ([a6b018e](a6b018e))

### Bug Fixes

* **_parse_job_results:** Catch None details in response ([#471](#471)) ([f01ae25](f01ae25)), closes [#470](#470)
github-actions bot pushed a commit that referenced this pull request Apr 11, 2024
## [1.12.0](v1.11.0...v1.12.0) (2024-04-11)

### Features

* **ApplicationFilter:** Add new_appid parameter ([#547](#547)) ([bc96108](bc96108))
* **Firewall:** New fields for show_system_resources ([#544](#544)) ([9e8cc2a](9e8cc2a))
* Handling Log Collector Group (LCG) pushes ([#493](#493)) ([7e87952](7e87952))
* **panos/network:** Advanced routing engine ([#539](#539)) ([173bb8a](173bb8a))
* Enhance updater logic ([#548](#548)) ([23ed1ad](23ed1ad))
* **network.BgpPolicyRule:** Add action_community_modifier param ([#535](#535)) ([58ce888](58ce888)), closes [#534](#534)
* **PanDevice:** add `is_ready()` ([#532](#532)) ([a6b018e](a6b018e))

### Bug Fixes

* **_parse_job_results:** Catch None details in response ([#471](#471)) ([f01ae25](f01ae25)), closes [#470](#470)
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.

BGP Configuration fails if 'append community' rule configured
3 participants