Skip to content

eos_acls: Fix missing 'standard' keyword in replaced state#613

Open
PcInfamy wants to merge 3 commits intoansible-collections:mainfrom
PcInfamy:fix-acl-standard-replaced
Open

eos_acls: Fix missing 'standard' keyword in replaced state#613
PcInfamy wants to merge 3 commits intoansible-collections:mainfrom
PcInfamy:fix-acl-standard-replaced

Conversation

@PcInfamy
Copy link
Copy Markdown

SUMMARY

Fixes issue where eos_acls module with state: replaced did not generate the standard keyword for standard ACLs.

When using state: "replaced" on a standard ACL, the generated commands were missing the standard keyword (e.g., ip access-list test-acl instead of ip access-list standard test-acl). This caused the ACL to be created as an extended ACL instead of a standard one.

The fix ensures that the standard key from the ACL configuration is properly passed to the command generation logic for new ACLs in replaced state.

Fixes #608

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

eos_acls

ADDITIONAL INFORMATION

Before the fix:

- name: "Deploy test ACL"
  arista.eos.eos_acls:
    config:
      - afi: "ipv4"
        acls:
          - name: "test-acl"
            standard: true
            aces:
              - grant: "permit"
                sequence: 10
                source:
                  host: "192.168.1.1"
    state: "replaced"

Generated commands (incorrect):

ip access-list test-acl
10 permit host 192.168.1.1

After the fix:
Generated commands (correct):

ip access-list standard test-acl
10 permit host 192.168.1.1

Changes made:

  • Modified _state_replaced in plugins/module_utils/network/eos/config/acls/acls.py to pass the full ACL configuration (including standard key) when creating new ACLs
  • Added unit tests test_eos_acls_replaced_standard and test_eos_acls_replaced_extended
  • Added changelog fragment documenting the fix

**Test Results:**
The new test `test_eos_acls_replaced_standard` passes, confirming that `state: replaced` now correctly generates the `standard` keyword for standard ACLs. The fix ensures consistency with `state: merged` behavior.

- Fix issue where state: replaced did not generate the standard keyword for standard ACLs
- Refactor ACL command generation to pass full ACL object instead of individual ACEs
- Add unit tests for both standard and extended ACL replacement
- Resolves issue ansible-collections#608
@PcInfamy PcInfamy force-pushed the fix-acl-standard-replaced branch from 86d9a5d to 319ee9c Compare November 24, 2025 15:29
@PcInfamy PcInfamy marked this pull request as ready for review November 24, 2025 15:30
@sonarqubecloud
Copy link
Copy Markdown

@PcInfamy
Copy link
Copy Markdown
Author

@Ruchip16
@komaldesai13
@rohitthakur2590

Sorry for the headache - I messed up my previous PR. This one should be good to go.

@komaldesai13 komaldesai13 self-requested a review December 3, 2025 09:54
@rohitthakur2590
Copy link
Copy Markdown
Contributor

@PcInfamy thank you for the contribution , this looks good could you please ensure its rebased/updated

@rohitthakur2590 rohitthakur2590 added the safe to test For network-integration jobs label Jan 13, 2026
@PcInfamy
Copy link
Copy Markdown
Author

@PcInfamy thank you for the contribution , this looks good could you please ensure its rebased/updated

@rohitthakur2590 ready!

@PcInfamy PcInfamy temporarily deployed to integration_destroy February 25, 2026 18:37 — with GitHub Actions Inactive
@rohitthakur2590
Copy link
Copy Markdown
Contributor

@PcInfamy thanks for the contribution!
the branch is behind main. Could you please update it so we can merge.

@PcInfamy
Copy link
Copy Markdown
Author

@PcInfamy thanks for the contribution! the branch is behind main. Could you please update it so we can merge.

@rohitthakur2590 done!

@sonarqubecloud
Copy link
Copy Markdown

@PcInfamy PcInfamy temporarily deployed to integration_destroy March 5, 2026 06:43 — with GitHub Actions Inactive
@rohitthakur2590
Copy link
Copy Markdown
Contributor

@PcInfamy thanks for the contribution! the branch is behind main. Could you please update it so we can merge.

@rohitthakur2590 done!

thank you , all tests passed successfully , but as other PR got merged before this one , you will have to again rebase this .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

acls acls resource module safe to test For network-integration jobs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

eos_acls | state: "replaced" breaks standard ACLs

3 participants