Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feat(eos_cli_config_gen): Add support for ingress in system.control_plane.ipv4/6_access_group #4481

Closed
wants to merge 17 commits into from
Closed

Feat(eos_cli_config_gen): Add support for ingress in system.control_plane.ipv4/6_access_group #4481

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
6d95a59
Refactor(eos_cli_config_gen):Add support for ingress in system.contro…
Vibhu-gslab Sep 17, 2024
9ed0ed1
updating description
Vibhu-gslab Sep 19, 2024
3e124d8
addressing comments
Vibhu-gslab Oct 17, 2024
3214096
Merge branch 'devel' into ingress
Vibhu-gslab Oct 21, 2024
80e6eab
Merge branch 'devel' into ingress
Vibhu-gslab Oct 22, 2024
80590c8
Merge branch 'devel' into ingress
Vibhu-gslab Oct 23, 2024
d0d53fc
Merge branch 'devel' into ingress
Vibhu-gslab Oct 25, 2024
22a54e5
Merge branch 'devel' into ingress
Vibhu-gslab Oct 30, 2024
1457838
Merge branch 'devel' into ingress
Vibhu-gslab Nov 4, 2024
53af345
Merge branch 'devel' into ingress
Vibhu-gslab Nov 4, 2024
0a6995d
Merge branch 'devel' into ingress
Vibhu-gslab Nov 5, 2024
932b032
Merge branch 'devel' into ingress
Vibhu-gslab Nov 5, 2024
7a47340
fixing comments
Vibhu-gslab Nov 7, 2024
c92077c
Merge branch 'devel' into ingress
Vibhu-gslab Nov 7, 2024
2464b30
Merge branch 'devel' into ingress
Vibhu-gslab Nov 7, 2024
805308a
adding files to hosts
Vibhu-gslab Nov 7, 2024
161e7b2
Merge branch 'devel' into ingress
Vibhu-gslab Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 22 additions & 12 deletions ...lections/arista/avd/molecule/eos_cli_config_gen/documentation/devices/system.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,21 @@ interface Management1

#### Control-Plane Access-Groups

| Protocol | VRF | Access-list |
| -------- | --- | ------------|
| IPv4 | default | acl4_1 |
| IPv4 | red | acl4_2 |
| IPv4 | red_1 | acl4_2 |
| IPv4 | default | acl4_3 |
| IPv6 | default | acl6_1 |
| IPv6 | blue | acl6_2 |
| IPv6 | blue_1 | acl6_2 |
| IPv6 | default | acl6_3 |
| Protocol | VRF | Access-list | Ingress-default |
| -------- | --- | ------------| --------------- |
| IPv4 | default | acl4_1 | - |
| IPv4 | red | acl4_2 | - |
| IPv4 | red_1 | acl4_2 | - |
| IPv4 | default | acl4_3 | - |
| IPv4 | red_2 | acl4_4 | True |
alexeygorbunov marked this conversation as resolved.
Show resolved Hide resolved
| IPv4 | red_3 | acl4_5 | False |
| IPv4 | red_4 | ingress | - |
| IPv6 | default | acl6_1 | - |
| IPv6 | blue | acl6_2 | - |
| IPv6 | blue_1 | acl6_2 | - |
| IPv6 | default | acl6_3 | - |
| IPv6 | default | acl6_4 | True |
| IPv6 | blue_2 | ingress | - |

#### System Control-Plane Device Configuration

Expand All @@ -66,13 +71,18 @@ interface Management1
system control-plane
tcp mss ceiling ipv4 1344 ipv6 1366
ip access-group acl4_1 in
ip access-group acl4_3 vrf default in
ip access-group acl4_2 vrf red in
ip access-group acl4_2 vrf red_1 in
ip access-group acl4_3 vrf default in
ip access-group ingress default acl4_4
ip access-group acl4_5 vrf red_3 in
ip access-group ingress vrf red_4 in
ipv6 access-group acl6_1 in
ipv6 access-group acl6_3 vrf default in
ipv6 access-group acl6_2 vrf blue in
ipv6 access-group acl6_2 vrf blue_1 in
ipv6 access-group acl6_3 vrf default in
ipv6 access-group ingress default acl6_4
ipv6 access-group ingress vrf blue_2 in
```

## System L1
Expand Down
9 changes: 7 additions & 2 deletions ansible_collections/arista/avd/molecule/eos_cli_config_gen/intended/configs/system.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,15 @@ interface Management1
system control-plane
tcp mss ceiling ipv4 1344 ipv6 1366
ip access-group acl4_1 in
ip access-group acl4_3 vrf default in
ip access-group acl4_2 vrf red in
ip access-group acl4_2 vrf red_1 in
ip access-group acl4_3 vrf default in
ip access-group ingress default acl4_4
ip access-group acl4_5 vrf red_3 in
ip access-group ingress vrf red_4 in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

order is not correct

s1-leaf1(config-system-cp)#sh ac
system control-plane
   tcp mss ceiling ipv4 1344 ipv6 1366
   ip access-group ingress default acl4_4
   ip access-group acl4_3 in
   ip access-group acl4_2 vrf red in
   ip access-group acl4_2 vrf red_1 in
   ip access-group acl4_5 vrf red_3 in
   ip access-group ingress vrf red_4 in
   ipv6 access-group ingress default acl6_4
   ipv6 access-group acl6_3 in
   ipv6 access-group acl6_2 vrf blue in
   ipv6 access-group acl6_2 vrf blue_1 in
   ipv6 access-group ingress vrf blue_2 in
s1-leaf1(config-system-cp)#

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to add to this comment:

  • EoS treats missing VRF and VRF default as the same.
   ip access-group acl4_1 in
   ip access-group acl4_3 vrf default in

will end up in running-config as ip access-group acl4_3 in (last command overwrites previous ones)

  • Order seems to be the following:
  1. tcp mss ceiling.*
  2. ip access-group ingress default.*
  3. ip access-group <IPv4_ACL_NAME> in # IPv4 ACL for default` VRF. Can be only one
  4. ip access-group <IPv4_ACL_NAME> vrf <VRF_NAME> in # Sorted by VRF name

Same ordering applies to IPv6.

Example:

avd-ci-leaf2(config-s-s19)#system control-plane
avd-ci-leaf2(config-s-s19-system-cp)#   tcp mss ceiling ipv4 1344 ipv6 1366
avd-ci-leaf2(config-s-s19-system-cp)#   ip access-group acl4_1 in
avd-ci-leaf2(config-s-s19-system-cp)#   ip access-group acl4_2 vrf red in
avd-ci-leaf2(config-s-s19-system-cp)#   ip access-group acl4_2 vrf red_1 in
avd-ci-leaf2(config-s-s19-system-cp)#   ip access-group acl4_3 vrf default in
avd-ci-leaf2(config-s-s19-system-cp)#   ip access-group ingress default acl4_4
avd-ci-leaf2(config-s-s19-system-cp)#   ip access-group acl4_5 vrf red_3 in
avd-ci-leaf2(config-s-s19-system-cp)#   ip access-group acl4_6 vrf red_5 in
avd-ci-leaf2(config-s-s19-system-cp)#   ip access-group ingress vrf red_4 in
avd-ci-leaf2(config-s-s19-system-cp)#   ipv6 access-group acl6_1 in
avd-ci-leaf2(config-s-s19-system-cp)#   ipv6 access-group acl6_2 vrf blue in
avd-ci-leaf2(config-s-s19-system-cp)#   ipv6 access-group acl6_2 vrf blue_1 in
avd-ci-leaf2(config-s-s19-system-cp)#   ipv6 access-group acl6_3 vrf default in
avd-ci-leaf2(config-s-s19-system-cp)#   ipv6 access-group ingress default acl6_4
avd-ci-leaf2(config-s-s19-system-cp)#   ipv6 access-group acl6_6 in
avd-ci-leaf2(config-s-s19-system-cp)#   ipv6 access-group ingress vrf blue_2 in
avd-ci-leaf2(config-s-s19-system-cp)#show session-config diffs 
--- system:/running-config
+++ session:/s19-session-config
+system control-plane
+   tcp mss ceiling ipv4 1344 ipv6 1366
+   ip access-group ingress default acl4_4
+   ip access-group acl4_3 in
+   ip access-group acl4_2 vrf red in
+   ip access-group acl4_2 vrf red_1 in
+   ip access-group acl4_5 vrf red_3 in
+   ip access-group ingress vrf red_4 in
+   ip access-group acl4_6 vrf red_5 in
+   ipv6 access-group ingress default acl6_4
+   ipv6 access-group acl6_6 in
+   ipv6 access-group acl6_2 vrf blue in
+   ipv6 access-group acl6_2 vrf blue_1 in
+   ipv6 access-group ingress vrf blue_2 in
avd-ci-leaf2(config-s-s19-system-cp)#

ipv6 access-group acl6_1 in
ipv6 access-group acl6_3 vrf default in
ipv6 access-group acl6_2 vrf blue in
ipv6 access-group acl6_2 vrf blue_1 in
ipv6 access-group acl6_3 vrf default in
ipv6 access-group ingress default acl6_4
ipv6 access-group ingress vrf blue_2 in
12 changes: 12 additions & 0 deletions ansible_collections/arista/avd/molecule/eos_cli_config_gen/inventory/host_vars/system.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ system:
vrf: red_1
- acl_name: "acl4_3"
vrf: default
- acl_name: "acl4_4"
vrf: red_2
ingress_default: true
- acl_name: "acl4_5"
vrf: red_3
ingress_default: false
- acl_name: "ingress"
vrf: red_4
ipv6_access_groups:
- acl_name: "acl6_1"
- acl_name: "acl6_2"
Expand All @@ -19,6 +27,10 @@ system:
vrf: blue_1
- acl_name: "acl6_3"
vrf: default
- acl_name: "acl6_4"
ingress_default: true
- acl_name: "ingress"
vrf: blue_2
l1:
unsupported_speed_action: warn
unsupported_error_correction_action: error
21 changes: 18 additions & 3 deletions ansible_collections/arista/avd/roles/eos_cli_config_gen/docs/tables/system.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions python-avd/pyavd/_eos_cli_config_gen/j2templates/documentation/system.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@

#### Control-Plane Access-Groups

| Protocol | VRF | Access-list |
| -------- | --- | ------------|
| Protocol | VRF | Access-list | Ingress-default |
| -------- | --- | ------------| --------------- |
{# IPv4 Access-groups #}
{% for acl_set in system.control_plane.ipv4_access_groups | arista.avd.natural_sort %}
| IPv4 | {{ acl_set.vrf | arista.avd.default('default') }} | {{ acl_set.acl_name }} |
| IPv4 | {{ acl_set.vrf | arista.avd.default('default') }} | {{ acl_set.acl_name }} | {{ acl_set.ingress_default | arista.avd.default('-') }} |
{% endfor %}
{# IPv6 Access-groups #}
{% for acl_set in system.control_plane.ipv6_access_groups | arista.avd.natural_sort %}
| IPv6 | {{ acl_set.vrf | arista.avd.default('default') }} | {{ acl_set.acl_name }} |
| IPv6 | {{ acl_set.vrf | arista.avd.default('default') }} | {{ acl_set.acl_name }} | {{ acl_set.ingress_default | arista.avd.default('-') }} |
{% endfor %}
{% endif %}

Expand Down
28 changes: 18 additions & 10 deletions python-avd/pyavd/_eos_cli_config_gen/j2templates/eos/system.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,16 @@ system control-plane
{% 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 = without_vrf | list + 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 %}
{% if acl_set.vrf is arista.avd.defined %}
{% set cp_ipv4_access_grp = cp_ipv4_access_grp ~ " vrf " ~ acl_set.vrf %}
{% for acl_set in system.control_plane.ipv4_access_groups | arista.avd.natural_sort %}
Vibhu-gslab marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you fix the j2 template for vrf default and none case. So if vrf is default or none then we have to render the only last command.
Like from your example
ipv4_access_groups:
- acl_name: "acl4_1"
- acl_name: "acl4_3"
vrf: default
it should render the only below config:
ip access-group acl4_3 in

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can as well optionally simply pick first IPv4 and IPv6 out of all with either "missing" VRF or those where it is set to default. EoS takes last line from overlapping commands pushed to conf or conf session because it simply overwrites previous ones. As we generate designed configuration state for the device - we should probably avoid generating commands which would be overwriting each other (although EoS would be OK with it).

{% if acl_set.ingress_default is arista.avd.defined(true) %}
{% set cp_ipv4_access_grp = "ip access-group ingress default " ~ acl_set.acl_name %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Right now neither schema nor J2 enforces only one default ingress ACL for IPv4 and IPv6 (although EoS will only allow one, because there can be only one implicit per ipv4/iv6).

Defining multiple ACLs as ingress_default: true renders all lines:

    ipv4_access_groups:
      - acl_name: "acl4_4"
        vrf: red_2
        ingress_default: true
      - acl_name: "acl4_6"
        vrf: red_5
        ingress_default: true
    ipv6_access_groups:
      - acl_name: "acl6_4"
        ingress_default: true
      - acl_name: "acl6_6"
        ingress_default: true
   ip access-group ingress default acl4_4
   ip access-group ingress default acl4_6

   ipv6 access-group ingress default acl6_4
   ipv6 access-group ingress default acl6_6

Maybe we could at least only render first "ingress default" per address family, and treat all others as regular (per-vrf ACLs)

# Line 8
+ {% set ns = namespace(ipv4_ingress_default_set=false, ipv6_ingress_default_set=false) %}

# Line [29:]
-{%     for acl_set in system.control_plane.ipv4_access_groups | arista.avd.natural_sort %}
+{%     for acl_set in sorted_ipv4_access_groups | arista.avd.natural_sort %}
-{%         if acl_set.ingress_default is arista.avd.defined(true) %}
+{%         if acl_set.ingress_default is arista.avd.defined(true) and ns.ipv4_ingress_default_set is not arista.avd.defined(true) %}
{%             set cp_ipv4_access_grp = "ip access-group ingress default " ~ acl_set.acl_name %}
+{%             set ns.ipv4_ingress_default_set = true %}
{%         else %}
{%             set cp_ipv4_access_grp = "ip access-group " ~ acl_set.acl_name %}
{%             if acl_set.vrf is arista.avd.defined %}
{%                 set cp_ipv4_access_grp = cp_ipv4_access_grp ~ " vrf " ~ acl_set.vrf %}
{%             endif %}
{%             set cp_ipv4_access_grp = cp_ipv4_access_grp ~ " in" %}
{%         endif %}
   {{ 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 without_vrf = system.control_plane.ipv6_access_groups | rejectattr('vrf', 'arista.avd.defined') | arista.avd.natural_sort %}
{%         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 =  without_vrf | list + with_vrf_default | list + with_vrf_non_default | list %}
{%     endif %}
-{%     for acl_set in system.control_plane.ipv6_access_groups | arista.avd.natural_sort %}
+{%     for acl_set in sorted_ipv6_access_groups | arista.avd.natural_sort %}
-{%         if acl_set.ingress_default is arista.avd.defined(true) %}
+{%         if acl_set.ingress_default is arista.avd.defined(true) and ns.ipv6_ingress_default_set is not arista.avd.defined(true) %}
{%             set cp_ipv6_access_grp = "ipv6 access-group ingress default " ~ acl_set.acl_name %}
+{%             set ns.ipv6_ingress_default_set = true %}
{%         else %}
{%             set cp_ipv6_access_grp = "ipv6 access-group " ~ acl_set.acl_name %}
{%             if acl_set.vrf is arista.avd.defined %}
{%                 set cp_ipv6_access_grp = cp_ipv6_access_grp ~ " vrf " ~ acl_set.vrf %}
{%             endif %}
{%             set cp_ipv6_access_grp = cp_ipv6_access_grp ~ " in" %}
{%         endif %}
   {{ cp_ipv6_access_grp }}
{%     endfor %}

Copy link
Contributor

@ClausHolbechArista ClausHolbechArista Nov 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default here is the default VRF, so I think there are some misunderstandings about the data model and EOS CLI.

EDIT:

So there are

[no|default] ip access-group ACL_NAME [ ( VRF_VRF_KW ( VRF_VRF_DYNAMIC | VRF_VRF_DEFAULT ) ) ] in
[no|default] ip access-group ingress default ACL_NAME
[no|default] ipv6 access-group ACL_NAME [ ( VRF_VRF_KW ( VRF_VRF_DYNAMIC | VRF_VRF_DEFAULT ) ) ] in
[no|default] ipv6 access-group ingress default ACL_NAME

We should have ipv4_access_group_ingress_default: <str> and ipv6_access_group_ingress_default: <str>.
And then keep the vrf access groups as they are Today.

{% else %}
{% set cp_ipv4_access_grp = "ip access-group " ~ acl_set.acl_name %}
{% if acl_set.vrf is arista.avd.defined %}
{% set cp_ipv4_access_grp = cp_ipv4_access_grp ~ " vrf " ~ acl_set.vrf %}
{% endif %}
{% set cp_ipv4_access_grp = cp_ipv4_access_grp ~ " in" %}
{% endif %}
{% set cp_ipv4_access_grp = cp_ipv4_access_grp ~ " in" %}
{{ cp_ipv4_access_grp }}
{% endfor %}
{# control_plane access_groups ipv6 #}
Expand All @@ -40,12 +44,16 @@ system control-plane
{% 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 = without_vrf | list + 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 %}
{% if acl_set.vrf is arista.avd.defined %}
{% set cp_ipv6_access_grp = cp_ipv6_access_grp ~ " vrf " ~ acl_set.vrf %}
{% for acl_set in system.control_plane.ipv6_access_groups | arista.avd.natural_sort %}
{% if acl_set.ingress_default is arista.avd.defined(true) %}
{% set cp_ipv6_access_grp = "ipv6 access-group ingress default " ~ acl_set.acl_name %}
{% else %}
{% set cp_ipv6_access_grp = "ipv6 access-group " ~ acl_set.acl_name %}
{% if acl_set.vrf is arista.avd.defined %}
{% set cp_ipv6_access_grp = cp_ipv6_access_grp ~ " vrf " ~ acl_set.vrf %}
{% endif %}
{% set cp_ipv6_access_grp = cp_ipv6_access_grp ~ " in" %}
{% endif %}
{% set cp_ipv6_access_grp = cp_ipv6_access_grp ~ " in" %}
{{ cp_ipv6_access_grp }}
{% endfor %}
{% endif %}
11 changes: 11 additions & 0 deletions python-avd/pyavd/_eos_cli_config_gen/schema/eos_cli_config_gen.schema.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions python-avd/pyavd/_eos_cli_config_gen/schema/schema_fragments/system.schema.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,17 @@ keys:
acl_name:
type: str
required: true
description: |-
Access list name.
String "ingress" could be access list name as well.
vrf:
type: str
convert_types:
- int
description: Keys `vrf` and `ingress_default` are mutually exclusive.
ingress_default:
Vibhu-gslab marked this conversation as resolved.
Show resolved Hide resolved
type: bool
description: Keys `vrf` and `ingress_default` are mutually exclusive.
ipv6_access_groups:
type: list
unique_keys:
Expand All @@ -50,6 +57,10 @@ keys:
type: str
convert_types:
- int
description: "'vrf' and 'ingress_default' are mutually exclusive."
ingress_default:
type: bool
description: "'vrf' and 'ingress_default' are mutually exclusive."
l1:
type: dict
keys:
Expand Down
2 changes: 2 additions & 0 deletions python-avd/pyavd/_eos_designs/structured_config/core_interfaces_and_l3_edge/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,8 @@ def _get_port_channel_member_cfg(self: AvdStructuredConfigCoreInterfacesAndL3Edg
Return partial structured_config for one p2p_link.

Covers config for ethernet interfaces that are port-channel members.

TODO: Change description for members to be the physical peer interface instead of port-channel
"""
return {
"name": member["interface"],
Expand Down
Loading