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

Bug453 port group #455

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

ntl-afzaalarif
Copy link

SUMMARY

The iosxr acls module was wrongly setting port-group param to be mutually exclusive with net-group,host,address,any and prefix in source and destination of ACE. The following ACE was correct and was a part of ACL on cisco ios xr9k series router. 1390 permit udp any 1.1.1.1 0.0.0.0 port-group ABC

As can be seen the [address(1.1.1.1), wildcard(0.0.0.0)] and port-group(ABC) are placed together, which as per previously implemented logic of iosxr.acls ansible module was inconsistent and should be mutually exclusive.

I have modified the iosxr.acls module to now fix this problem, moreover removed the port-group as direct option to the source/destination and moved it under the port_protocol options. Also updated the ansible-doc for it.

This fixes #453

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

cisco.iosxr.iosxr_acls

ADDITIONAL INFORMATION

…otocol section instead of source/destination options.
…tination so that port-group is not mutually exclusive to [host,any,address,prefix,net-group] and also removed it as direct opton of src/dest instead added port-group to argument of port_protocol
…e/destination so that port-group is not direct opton of src/dest instead added port-group to argument of port_protocol
Copy link

@ashwini-mhatre
Copy link
Contributor

@ntl-afzaalarif please add uts and integration tests

@ntl-afzaalarif
Copy link
Author

Hi @ashwini-mhatre, this is my first PR. Just noticed a bug and wanted to highlight. Can you please guide on how to add uts and integration tests?

@NilashishC NilashishC requested a review from roverflow November 22, 2023 13:11
Copy link
Member

@roverflow roverflow left a comment

Choose a reason for hiding this comment

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

Hey @ntl-afzaalarif, i can see there are a few changes present, thanks for contributing, it would be amazing if you could make changes according to the CONTRIBUTION guidelines that we follow

Copy link

@ashwini-mhatre
Copy link
Contributor

@ashwini-mhatre
Copy link
Contributor

Copy link

Copy link

Copy link

Copy link

@ashwini-mhatre ashwini-mhatre linked an issue Dec 15, 2023 that may be closed by this pull request
@ashwini-mhatre ashwini-mhatre self-requested a review December 29, 2023 13:07
Copy link

Label error. Requires exactly 1 of: bug, enhancement, major, minor, patch, skip-changelog. Found:

Copy link

Copy link

@Ruchip16 Ruchip16 requested a review from roverflow April 10, 2024 10:15
@NilashishC NilashishC added the bug This issue/PR relates to a bug. label Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug.
Projects
None yet
5 participants