aristanetworks / avd

Arista Validated Designs
https://avd.arista.com
Apache License 2.0
279 stars 201 forks source link

"system control-plane" overwritten when same ACL is used for different VRFs #4446

Closed PieterL75 closed 1 day ago

PieterL75 commented 1 week ago

Issue Summary

When I apply a structured config to a "control_plane", using the same ACL name, but a different VRF, then only one instance is applied

According to the documentation,

system:
  control_plane:
    ipv4_access_groups:
      - acl_name: <str; required; unique>
        vrf: <str>

the acl_name has to be unique, but that is not the case. it is the VRF that has to be unique, and the same ACL can be used on different VRFs

Which component(s) of AVD impacted

eos_designs

How do you run AVD ?

Ansible CLI (with virtual-env or native python)

Steps to reproduce

Create an access list
apply that to two vrf's using the "structured_config" in a vrf

    vrfs:
      - name: VRF1
        structured_config:
          system:
            control_plane:
              ipv4_access_groups:
                  - acl_name: SEC-COPP_STRICT-V4-IN
                    vrf: VRF1
      - name: VRF2
        structured_config:
          system:
            control_plane:
              ipv4_access_groups:
                  - acl_name: SEC-COPP_STRICT-V4-IN
                    vrf: VRF2

This compiles to intended config
system:
  control_plane:
    ipv4_access_groups:
    - acl_name: SEC-COPP_STRICT-V4-IN
      vrf: VRF1
and is missing the 'VRF2'

When a different ACL is used, then both VRFs are visible

Relevant log output

No response

Contributing Guide

ClausHolbechArista commented 1 week ago

The issue here is how we merge multiple snips of structured_config. When we have a list of dicts, we identify overlapping entries by a "primary_key". In this case the "primary_key" is set to the acl_name for historic reasons (the <4.x data model was a dict keyed by the acl name).

This means we merge the two snips of structured_config to a single entry. Last one wins. Fixing this in AVD 4.x will cause breaking changes for people still using the old data models. Support for the old data models will be removed in 5.0, so it will be possible to change the primary_key to be the vrf field instead, since you should only have one ACL entry per VRF.

For now you will have to maintain a custom_structured_configuration_system key where you set the complete list and maintain your VRFs there too.

ClausHolbechArista commented 1 week ago

This issues tracks ~changing~ removing the primary key of system.control_plane.ipv4_access_groups ~from acl_name to vrf~ and will be implemented in 5.0.