Deleteting remarks ACEs in ACL will destroy whole ACL

[TheRealBecks]

SUMMARY

When deleting a port_protocol from an ACL ACE the no <sequence number> command is missing and an error-message will be provided as the to be changed/configured sequence number already exists.

ISSUE TYPE

• Bug Report

COMPONENT NAME

eos_acls

ANSIBLE VERSION

ansible [core 2.15.6]
  config file = /home/mbeckert/Entwicklung/Strato/lightning/lightning/ansible.cfg
  configured module search path = ['/home/mbeckert/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/mbeckert/.local/share/virtualenvs/lightning-vg2lOQBb/lib/python3.11/site-packages/ansible
  ansible collection location = /home/mbeckert/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/mbeckert/.local/share/virtualenvs/lightning-vg2lOQBb/bin/ansible
  python version = 3.11.6 (main, Nov 15 2023, 09:22:27) [GCC] (/home/mbeckert/.local/share/virtualenvs/lightning-vg2lOQBb/bin/python)
  jinja version = 3.1.2
  libyaml = True

COLLECTION VERSION

Collection        Version
----------------- -------
community.general 7.5.1

CONFIGURATION

CONFIG_FILE() = /home/mbeckert/Entwicklung/Strato/lightning/lightning/ansible.cfg
DEFAULT_FORKS(/home/mbeckert/Entwicklung/Strato/lightning/lightning/ansible.cfg) = 10
DEFAULT_HASH_BEHAVIOUR(/home/mbeckert/Entwicklung/Strato/lightning/lightning/ansible.cfg) = merge
DEFAULT_HOST_LIST(/home/mbeckert/Entwicklung/Strato/lightning/lightning/ansible.cfg) = ['/home/mbeckert/Entwicklung/Strato/lightning/lightning/inventory.yml']
DEFAULT_ROLES_PATH(/home/mbeckert/Entwicklung/Strato/lightning/lightning/ansible.cfg) = ['/home/mbeckert/Entwicklung/Strato/lightning/lightning/roles']
DEFAULT_VAULT_PASSWORD_FILE(/home/mbeckert/Entwicklung/Strato/lightning/lightning/ansible.cfg) = /home/mbeckert/Entwicklung/Strato/lightning/lightning/.vault_passphrase/open_vault.sh
EDITOR(env: EDITOR) = nano
PAGER(env: PAGER) = less

OS / ENVIRONMENT

Arista EOS 4.28.4M

STEPS TO REPRODUCE

Before task.yml:

---
- name: Remark test
  arista.eos.eos_acls:
    state: "replaced"
    config:
      - afi: ipv4
        acls:
          - name: test-acl3
            aces:
              - sequence: 10
                remark: test foo
              - sequence: 20
                grant: permit
                protocol: udp
                source:
                  any: true
                destination:
                  any: true

After task.yml:

---
- name: Remark test
  arista.eos.eos_acls:
    state: "replaced"
    config:
      - afi: ipv4
        acls:
          - name: test-acl3
            aces:
              # - sequence: 10
              #   remark: test foo
              - sequence: 10
                grant: permit
                protocol: udp
                source:
                  any: true
                destination:
                  any: true

-> Sequence 10 will be deleted and 20 will be the new 10

EXPECTED RESULTS

On the second run the sequence number 10 will be deleted before a new 10 will be configured.

To be generated commands:

[
  'ip access-list test-acl2',
  'no 10',
  'no 20',
  '10 permit udp any any'
]

ACTUAL RESULTS

On the second run on the configuration will fail on the device:

Generated commands:

[
  'ip access-list test-acl3',
  'no 10',
  'no 20',
  '10 remark test foo permit udp any any'
]

ROOT CAUSE

get_updated_ace(w, h) will be called:

arista.eos/plugins/module_utils/network/eos/config/acls/acls.py

Lines 456 to 466 in 2342353

	def get_updated_ace(w, h):
	# gives the ace to be updated in case of merge update.
	if not dict_diff(w, h):
	return
	w_updated = w.copy()
	for hkey in h.keys():
	if hkey not in w.keys():
	w_updated.update({hkey: h[hkey]})
	else:
	w_updated.update({hkey: w[hkey]})
	return w_updated

In this example the content of the variables are:
w:

{
    'destination': {
        'any': True
    },
    'grant': 'permit',
    'protocol': 'udp',
    'sequence': 10,
    'source': {
        'any': True
    }
}

h:

{
    'remark': 'test foo',
    'sequence': 10
}

w_updated (return value):

{
    'destination': {
        'any': True
    },
    'grant': 'permit',
    'protocol': 'udp',
    'remark': 'test foo',
    'sequence': 10,
    'source': {
        'any': True
    }
}

-> Both ACEs have been merged, but WHY? Merging and also overriding ACEs is not possible at all. An existing ACE needs to be deleted with a no <sequence number> first before a new one gets added.

The only call of get_updated_ace(w, h) happens here:

arista.eos/plugins/module_utils/network/eos/config/acls/acls.py

Lines 441 to 450 in 2342353

	if wace["sequence"] == hace["sequence"]:
	wace_updated = get_updated_ace(
	wace,
	hace,
	)
	if wace_updated:
	want_aces.pop(
	want_aces.index(wace),
	)
	want_aces.append(wace_updated)

My suggestion:

if wace["sequence"] == hace["sequence"]:
   if wace == hace:
       want_aces.pop(want_aces.index(wace))

And also the deletion of get_updated_ace(w, h)

[TheRealBecks]

And to be clear: If there are multiple remark entries in an ACL every element following after a remark entry will be rewritten as a remark. That could render an ACL to garbage leading the last remaining entry in an ACL to be the default deny ip any any entry hidden in the system. This could cause a serious production outage.