aristanetworks / avd

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

Feat(eos_cli_config_gen): Avoid rendering duplicated CoPP ACLs for VRF `default` #4717

Open alexeygorbunov opened 2 days ago

alexeygorbunov commented 2 days ago

Enhancement summary

system:
  control_plane:
    ipv4_access_groups:
      - acl_name: "acl4_1"
      - acl_name: "acl4_11"
      - acl_name: "acl4_3"
        vrf: default
    ipv6_access_groups:
      - acl_name: "acl6_1"
      - acl_name: "acl6_11"
      - acl_name: "acl6_3"
        vrf: default
system control-plane
   ip access-group acl4_1 in
   ip access-group acl4_11 in
   ip access-group acl4_3 vrf default in
   ipv6 access-group acl6_1 in
   ipv6 access-group acl6_11 in
   ipv6 access-group acl6_3 vrf default in

Example:

avd-ci-leaf2(config-s-s17)#system control-plane 
avd-ci-leaf2(config-s-s17-system-cp)#ip access-group acl4_1 in
avd-ci-leaf2(config-s-s17-system-cp)#show session-config diffs
--- system:/running-config
+++ session:/s17-session-config
+system control-plane
+   ip access-group acl4_1 in
avd-ci-leaf2(config-s-s17-system-cp)#ip access-group acl4_3 vrf default in
avd-ci-leaf2(config-s-s17-system-cp)#show session-config diffs
--- system:/running-config
+++ session:/s17-session-config
+system control-plane
+   ip access-group acl4_3 in
avd-ci-leaf2(config-s-s17-system-cp)#ip access-group acl4_1 in
avd-ci-leaf2(config-s-s17-system-cp)#show session-config diffs
--- system:/running-config
+++ session:/s17-session-config
+system control-plane
+   ip access-group acl4_1 in
avd-ci-leaf2(config-s-s17-system-cp)#
avd-ci-leaf2(config-s-s17-system-cp)#abort

avd-ci-leaf2(config-s-s18)#system control-plane 
avd-ci-leaf2(config-s-s18-system-cp)#ipv6 access-group acl6_1 in
avd-ci-leaf2(config-s-s18-system-cp)#show session-config diffs
--- system:/running-config
+++ session:/s18-session-config
+system control-plane
+   ipv6 access-group acl6_1 in
avd-ci-leaf2(config-s-s18-system-cp)#ipv6 access-group acl6_3 vrf default in
avd-ci-leaf2(config-s-s18-system-cp)#show session-config diffs
--- system:/running-config
+++ session:/s18-session-config
+system control-plane
+   ipv6 access-group acl6_3 in
avd-ci-leaf2(config-s-s18-system-cp)#ipv6 access-group acl6_1 in
avd-ci-leaf2(config-s-s18-system-cp)#show session-config diffs
--- system:/running-config
+++ session:/s18-session-config
+system control-plane
+   ipv6 access-group acl6_1 in
avd-ci-leaf2(config-s-s18-system-cp)#

We should only render one CoPP ACL per IP version for default VRF

Which component of AVD is impacted

eos_cli_config_gen

Use case example

N/A

Describe the solution you would like

One of the solutions may be to make vrf attribute of the ipv4_access_groups[] or ipv6_access_groups[] a mandatory requirement. If vrf is not set - J2 should skip this item. If it's set to default - schema is already enforcing that only one item per IPv4 and IPv6 can have this value (effectively enforcing single ACL per IP version in Designed Config)

Example:

{#     control_plane access_groups ipv4 #}
{%     if system.control_plane.ipv4_access_groups is arista.avd.defined %}
{%         set with_vrf_non_default = system.control_plane.ipv4_access_groups | selectattr('vrf', 'arista.avd.defined') | rejectattr('vrf', 'equalto', 'default') | arista.avd.natural_sort | arista.avd.natural_sort('vrf') %}
{%         set with_vrf_default = system.control_plane.ipv4_access_groups | selectattr('vrf', 'arista.avd.defined') | selectattr('vrf', 'equalto', 'default') | arista.avd.natural_sort %}
{%         set sorted_ipv4_access_groups =  with_vrf_default | list + with_vrf_non_default | list %}
{%     endif %}
{%     for acl_set in sorted_ipv4_access_groups | arista.avd.default([]) %}
{%         set cp_ipv4_access_grp = "ip access-group " ~ acl_set.acl_name ~ " vrf " ~ acl_set.vrf ~ " in" %}
   {{ cp_ipv4_access_grp }}
{%     endfor %}
{#     control_plane access_groups ipv6 #}
{%     if system.control_plane.ipv6_access_groups is arista.avd.defined %}
{%         set with_vrf_non_default = system.control_plane.ipv6_access_groups | selectattr('vrf', 'arista.avd.defined') | rejectattr('vrf', 'equalto', 'default') | arista.avd.natural_sort | arista.avd.natural_sort('vrf') %}
{%         set with_vrf_default = system.control_plane.ipv6_access_groups | selectattr('vrf', 'arista.avd.defined') | selectattr('vrf', 'equalto', 'default') | arista.avd.natural_sort %}
{%         set sorted_ipv6_access_groups =  with_vrf_default | list + with_vrf_non_default | list %}
{%     endif %}
{%     for acl_set in sorted_ipv6_access_groups | arista.avd.default([]) %}
{%         set cp_ipv6_access_grp = "ipv6 access-group " ~ acl_set.acl_name ~ " vrf " ~ acl_set.vrf ~ " in" %}
   {{ cp_ipv6_access_grp }}
{%     endfor %}

Describe alternatives you have considered

N/A

Additional context

N/A

Contributing Guide

ClausHolbechArista commented 2 days ago

This is a general thing in most areas of eos_cli_config_gen. The EOS CLI is a bit inconsistent, but if EOS prints out without vrf default even when entering vrf default we want to accept inputs without the VRF. A way could be to just disallow default as a value in the vrf field, but the simpler way we often turn to is to just not output the vrf default even if set (if vrf != "default" add "vrf "). That indeed means it is possible to create two lines of config if someone gave one line without vrf and one with vrf:default, but it is simple for now. Once we have schema "formats" we could add schema enforcement of valid vrf_name_except_default (a breaking change).