ansible-collections / cisco.iosxr

Ansible Network Collection for Cisco IOSXR
GNU General Public License v3.0
69 stars 48 forks source link

cisco.iosxr.iosxr_acls module will not overwrite an ACL remark with a non-remark entry, in either `state: merged` or `state: replaced` #321

Closed digitalfiend64 closed 1 year ago

digitalfiend64 commented 1 year ago
SUMMARY

cisco.iosxr.iosxr_acls module will not overwrite an ACL remark with a non-remark entry, in either state: merged or state: replaced. It will successfully overwrite a remark with another remark, or remove the remark completely in the case of state: replaced when that sequence number doesn’t exist in the requested config.

The issue seems to be caused at https://github.com/ansible-collections/cisco.iosxr/blob/31a8aaecbb4eb8ba042e04acd22c77fdd8c10818/plugins/module_utils/network/iosxr/config/acls/acls.py#L505

        if delta or protocol_opt_delta:
            want_ace = self._dict_merge(have_ace, want_ace)
            return self._compute_commands(want_ace)

This block, for some reason is merging have_ace and want_ace, which doesn’t make sense in this case. The method _compute_commands should be called on want_ace alone. I’m not sure what case this _dict_merge was added for, but it appears to be causing problems. By simply deleting/commenting line 505, the module works as expected in both state: merged and state: replaced.

ISSUE TYPE
COMPONENT NAME

cisco.iosxr.iosxr_acls ansible_collections/cisco/iosxr/plugins/module_utils/network/iosxr/config/acls/acls.py, line 505

ANSIBLE VERSION
ansible [core 2.12.6]
  config file = /home/REDACTED/.ansible.cfg
  configured module search path = ['/home/REDACTED/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.8/site-packages/ansible
  ansible collection location = /home/REDACTED/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/local/bin/ansible
  python version = 3.8.12 (default, Sep 16 2021, 10:46:05) [GCC 8.5.0 20210514 (Red Hat 8.5.0-3)]
  jinja version = 3.0.2
  libyaml = True
COLLECTION VERSION

# /home/REDACTED/.ansible/collections/ansible_collections
Collection  Version
----------- -------
cisco.iosxr 4.0.3

# /usr/local/lib/python3.8/site-packages/ansible_collections
Collection  Version
----------- -------
cisco.iosxr 2.9.0
CONFIGURATION
ANSIBLE_FORCE_COLOR(env: ANSIBLE_FORCE_COLOR) = True
DEFAULT_LOAD_CALLBACK_PLUGINS(/home/REDACTED/.ansible.cfg) = True
DEFAULT_STDOUT_CALLBACK(/home/REDACTED/.ansible.cfg) = yaml
DEFAULT_TIMEOUT(/home/REDACTED/.ansible.cfg) = 120
DEPRECATION_WARNINGS(/home/REDACTED/.ansible.cfg) = False
GALAXY_IGNORE_CERTS(/home/REDACTED/.ansible.cfg) = True
GALAXY_SERVER_LIST(/home/REDACTED/.ansible.cfg) = ['inbound_yeti_repo', 'published_repo', 'rh-certified_repo']
HOST_KEY_CHECKING(/home/REDACTED/.ansible.cfg) = False
PARAMIKO_HOST_KEY_AUTO_ADD(/home/REDACTED/.ansible.cfg) = True
PERSISTENT_COMMAND_TIMEOUT(/home/REDACTED/.ansible.cfg) = 300
PERSISTENT_CONNECT_TIMEOUT(/home/REDACTED/.ansible.cfg) = 120
RETRY_FILES_ENABLED(/home/REDACTED/.ansible.cfg) = False
OS / ENVIRONMENT

cisco 8201-32FH, firmware 7.7.2 cisco ASR9K Series ASR-9010, firmware 6.5.3 cisco NCS-5500, firmware 7.2.2

STEPS TO REPRODUCE

Attempted to use cisco.iosxr.iosxr_acls module to configure an access-list, replacing a remark with a permit or deny statement.

Example before configuration: ipv4 access-list ACL-TEST 10 remark THIS IS A REMARK

      cisco.iosxr.iosxr_acls:
        state: merged
        config:
      - afi: ipv4
        acls:
          - name: ACL-TEST
            aces:
            -   destination:
                    host: 1.1.1.1
                grant: permit
                protocol: ipv4
                sequence: 10
                source:
                    any: true

      cisco.iosxr.iosxr_acls:
        state: replaced
        config:
      - afi: ipv4
        acls:
          - name: ACL-TEST
            aces:
            -   destination:
                    host: 1.1.1.1
                grant: permit
                protocol: ipv4
                sequence: 10
                source:
                    any: true
EXPECTED RESULTS

Expected configuration: ipv4 access-list ACL-TEST 10 permit ip any host 1.1.1.1

ACTUAL RESULTS

Example after configuration (no change): ipv4 access-list ACL-TEST 10 remark THIS IS A REMARK


  commands:
  - ipv4 access-list ACL-TEST
  - 10 remark THIS IS A REMARK
  invocation:
    module_args:
      config:
      - acls:
        - aces:
          - authen: null
            capture: null
            destination:
              address: null
              any: null
              host: 1.1.1.1
              net_group: null
              port_group: null
              port_protocol: null
              prefix: null
              wildcard_bits: null
            destopts: null
            dscp: null
            fragments: null
            grant: permit
            hop_by_hop: null
            icmp_off: null
            line: null
            log: null
            log_input: null
            packet_length: null
            precedence: null
            protocol: ipv4
            protocol_options: null
            remark: null
            routing: null
            sequence: 10
            source:
              address: null
              any: true
              host: null
              net_group: null
              port_group: null
              port_protocol: null
              prefix: null
              wildcard_bits: null
            ttl: null
          name: ACL-TEST
        afi: ipv4
      running_config: null
      state: merged

…

  commands:
  - ipv4 access-list ACL-TEST
  - 10 remark THIS IS A REMARK
  invocation:
    module_args:
      config:
      - acls:
        - aces:
          - authen: null
            capture: null
            destination:
              address: null
              any: null
              host: 1.1.1.1
              net_group: null
              port_group: null
              port_protocol: null
              prefix: null
              wildcard_bits: null
            destopts: null
            dscp: null
            fragments: null
            grant: permit
            hop_by_hop: null
            icmp_off: null
            line: null
            log: null
            log_input: null
            packet_length: null
            precedence: null
            protocol: ipv4
            protocol_options: null
            remark: null
            routing: null
            sequence: 10
            source:
              address: null
              any: true
              host: null
              net_group: null
              port_group: null
              port_protocol: null
              prefix: null
              wildcard_bits: null
            ttl: null
          name: ACL-TEST
        afi: ipv4
      running_config: null
      state: replaced
ashwini-mhatre commented 1 year ago

@digitalfiend64 I am not able to reproduce this on my setup.Please use replace state for replacing config. Please share your playbook. In my case its working fine

- name: IOS XR test ACL
  hosts: iosxr
  gather_facts: no
  connection: network_cli
  tasks:
    - name: Configure ACL

      cisco.iosxr.iosxr_acls:
        state: replaced
        config:
          - afi: ipv4
            acls:
              - name: ACL-TEST
                aces:
                  - destination:
                      host: 1.1.1.1
                    grant: permit
                    protocol: ipv4
                    sequence: 10
                    source:
                      any: true`
ashwini-mhatre commented 1 year ago

@digitalfiend64 Just to add more information. You can use overridden state also. but not merged

ashwini-mhatre commented 1 year ago

@digitalfiend64 any update on this?

digitalfiend64 commented 1 year ago

Hello @ashwini-mhatre , I haven't received the logs yet from my customer, they were going to test again with the latest version of collection and send logs to me if the issue persists.

A question from the customer about your comment on overrideent state versus meged: Are you saying that the merged state is not supported in this module, or just not for the use-case presented here?

ashwini-mhatre commented 1 year ago

@digitalfiend64 Merged state is supported in module but not suitable for Usecase. Overridden or replaced is design to replace config. so it is recomnded state.

sean-m-sullivan commented 1 year ago

updated sample playbook to recreate

---
- name: IOSXR ACCESS LIST - BUG TESTING
  hosts: all
  gather_facts: no

  vars:
    new_acl_2:
      - afi: ipv4
        acls:
          - name: ACL-TEST
            aces:
              - destination:
                    host: 1.1.1.30
                grant: permit
                protocol: ipv4
                sequence: 30
                source:
                    any: true

  tasks:
    - name: GET ORIGINAL access list configuration
      cisco.iosxr.iosxr_command:
        commands:
          - show ip access-list
      register: preconfig_acls

    - name: APPLY access list configuration
      cisco.iosxr.iosxr_acls:
        state: replaced
        config: "{{ new_acl_2 }}"

    - name: GET NEW access list configuration
      cisco.iosxr.iosxr_command:
        commands:
          - show ip access-list
      register: postconfig_acls

    - name: DISPLAY PRECONFIG
      debug: var=preconfig_acls.stdout

    - name: DISPLAY POSTCONFIG
      debug: var=postconfig_acls.stdout

And the inventory to invoke

---
all:
  children:
    iosxr:
      vars:
        ansible_network_os: "cisco.iosxr.iosxr"
        ansible_connection: "ansible.netcommon.network_cli"
      hosts:
        # ASR9K-01:
        #   ansible_host: "192.168.1.21"
        # NCS55A1-01:
        #   ansible_host: "192.168.1.104"
        # ASR9K-03:
        #   ansible_host: "192.168.1.27"
        NCS55A1-04:
          ansible_host: "192.168.1.107"