Refactor(eos_cli_config_gen): Improve the aaa accounting j2 template #4636

MaheshGSLAB · 2024-10-23T12:01:53Z

Change Summary

Improve the aaa accounting j2 template.

Fix the variable error incase command type is not define.
Improve the j2 template for default/console commands and exec.

Related Issue(s)

Fixes #4630

Component(s) name

arista.avd.eos_cli_config_gen

Proposed changes

Fix the variable error incase command type is not define.
Improve the j2 template for default/console commands and exec.

How to test

Run molecule with different type of inputs in aaa_accounting

Checklist

User Checklist

N/A

Repository Checklist

My code has been rebased from devel before I start
I have read the CONTRIBUTING document.
My change requires a change to the documentation and documentation have been updated accordingly.
I have updated molecule CI testing accordingly. (check the box if not applicable)

github-actions · 2024-10-23T12:02:07Z

Review docs on Read the Docs

To test this pull request:

# Create virtual environment for this testing below the current directory
python -m venv test-avd-pr-4636
# Activate the virtual environment
source test-avd-pr-4636/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/MaheshGSLAB/ansible-avd.git@issue-4630#subdirectory=python-avd" --force
# Point Ansible collections path to the Python virtual environment
export ANSIBLE_COLLECTIONS_PATH=$VIRTUAL_ENV/ansible_collections
# Install Ansible collection
ansible-galaxy collection install git+https://github.com/MaheshGSLAB/ansible-avd.git#/ansible_collections/arista/avd/,issue-4630 --force
# Optional: Install AVD examples
cd test-avd-pr-4636
ansible-playbook arista.avd.install_examples

ansible_collections/arista/avd/molecule/eos_cli_config_gen/intended/configs/aaa.cfg

github-actions · 2024-10-30T06:34:09Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions · 2024-10-30T10:16:41Z

Conflicts have been resolved. A maintainer will review the pull request shortly.

...lections/arista/avd/molecule/eos_cli_config_gen/inventory/host_vars/host1/aaa-accounting.yml

python-avd/pyavd/_eos_cli_config_gen/j2templates/eos/aaa-accounting.j2

sonarcloud · 2024-11-07T07:49:48Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

python-avd/pyavd/_eos_cli_config_gen/schema/schema_fragments/aaa_accounting.schema.yml

ClausHolbechArista · 2024-11-19T06:50:21Z

Moving to draft until comments have been addressed

gmuloc · 2024-11-28T09:57:20Z

python-avd/pyavd/_eos_cli_config_gen/schema/schema_fragments/aaa_accounting.schema.yml

+            unique_keys:
+              - commands


this help but will not cover this case (as 2 is included in the range and the unique won't be able to catch it)

command: - commands: 2-4 - commands: 2

This is not fixable easily in the schema anyways

site3-wan1(config)#aaa accounting commands 2-4 console none site3-wan1(config)#show run se aaa no aaa root aaa authorization exec default local aaa authorization commands all default local aaa accounting commands 2-4 console none site3-wan1(config)#aaa accounting commands 3 console start-stop ? group Specify server group logging Use syslog for accounting site3-wan1(config)#aaa accounting commands 3 console start-stop logging site3-wan1(config)#show run se aaa no aaa root aaa authorization exec default local aaa authorization commands all default local aaa accounting commands 2,4 console none aaa accounting commands 3 console start-stop logging

we could add something in the description to state that if using ranges then the user must make sure they don't overlap

added description.

gmuloc · 2024-11-28T10:02:30Z

python-avd/pyavd/_eos_cli_config_gen/schema/schema_fragments/aaa_accounting.schema.yml

            items:
              type: dict
              keys:
                commands:
                  type: str
                  convert_types:
-                  - int
+                    - int


not for this PR either but seems that you can have multiple groups in one CLI (we shall implement this if it is valid and someone needs it, until then the model is fine)

aaa accounting commands 3 console start-stop logging group radius group tacacs+

(and multiple time logging but it feels like a CLI issue)

yes, we may need to update the schema.

gmuloc · 2024-11-28T10:07:04Z

python-avd/pyavd/_eos_cli_config_gen/j2templates/eos/aaa-accounting.j2

 {%     if aaa_accounting.exec.default.type is arista.avd.defined %}
-{%         set aaa_accounting_exec_default_cli = "aaa accounting exec default" %}
 {%         if aaa_accounting.exec.default.type is arista.avd.defined('none') %}
-{%             set aaa_accounting_exec_default_cli = aaa_accounting_exec_default_cli ~ " none" %}
-{%         else %}
-{%             set aaa_accounting_exec_default_cli = aaa_accounting_exec_default_cli ~ " " ~ aaa_accounting.exec.default.type %}
+aaa accounting exec default none
+{%         elif aaa_accounting.exec.default.group is arista.avd.defined or aaa_accounting.exec.default.logging is arista.avd.defined(true) %}
+{%             set exec_default_cli = "aaa accounting exec default " ~ aaa_accounting.exec.default.type %}
 {%             if aaa_accounting.exec.default.group is arista.avd.defined %}
-{%                 set aaa_accounting_exec_default_cli = aaa_accounting_exec_default_cli ~ " group " ~ aaa_accounting.exec.default.group %}
+{%                 set exec_default_cli = exec_default_cli ~ " group " ~ aaa_accounting.exec.default.group %}
 {%             endif %}
 {%             if aaa_accounting.exec.default.logging is arista.avd.defined(true) %}
-{%                 set aaa_accounting_exec_default_cli = aaa_accounting_exec_default_cli ~ " logging" %}
+{%                 set exec_default_cli = exec_default_cli ~ " logging" %}
 {%             endif %}
+{{ exec_default_cli }}


I feel this could be optimised by setting the value first and then checking for not none and (group or logging) (not 100% sure for the is not syntax inside J2 right now)

Suggested change

{% if aaa_accounting.exec.default.type is arista.avd.defined %}

{% set aaa_accounting_exec_default_cli = "aaa accounting exec default" %}

{% if aaa_accounting.exec.default.type is arista.avd.defined('none') %}

{% set aaa_accounting_exec_default_cli = aaa_accounting_exec_default_cli ~ " none" %}

{% else %}

{% set aaa_accounting_exec_default_cli = aaa_accounting_exec_default_cli ~ " " ~ aaa_accounting.exec.default.type %}

aaa accounting exec default none

{% elif aaa_accounting.exec.default.group is arista.avd.defined or aaa_accounting.exec.default.logging is arista.avd.defined(true) %}

{% set exec_default_cli = "aaa accounting exec default " ~ aaa_accounting.exec.default.type %}

{% if aaa_accounting.exec.default.group is arista.avd.defined %}

{% set aaa_accounting_exec_default_cli = aaa_accounting_exec_default_cli ~ " group " ~ aaa_accounting.exec.default.group %}

{% set exec_default_cli = exec_default_cli ~ " group " ~ aaa_accounting.exec.default.group %}

{% endif %}

{% if aaa_accounting.exec.default.logging is arista.avd.defined(true) %}

{% set aaa_accounting_exec_default_cli = aaa_accounting_exec_default_cli ~ " logging" %}

{% set exec_default_cli = exec_default_cli ~ " logging" %}

{% endif %}

{{ exec_default_cli }}

{% if aaa_accounting.exec.default.type is arista.avd.defined %}

{% set aaa_accounting_exec_default_cli = "aaa accounting exec default " ~ aaa_accounting.exec.default.type %}

{% if aaa_accounting.exec.default.type is not arista.avd.defined('none') and (aaa_accounting.exec.default.group is arista.avd.defined or aaa_accounting.exec.default.logging is arista.avd.defined(true)) %}

{% if aaa_accounting.exec.default.group is arista.avd.defined %}

{% set exec_default_cli = exec_default_cli ~ " group " ~ aaa_accounting.exec.default.group %}

{% endif %}

{% if aaa_accounting.exec.default.logging is arista.avd.defined(true) %}

{% set exec_default_cli = exec_default_cli ~ " logging" %}

{% endif %}

{{ exec_default_cli }}

if type is start-stop but did not defined any group or logging then rendering below invalid command by this logic.
aaa accounting commands 3 default start-stop

fix the template for aaa accounting

9850664

MaheshGSLAB self-assigned this Oct 23, 2024

github-actions bot added the state: CI Updated CI scenario have been updated in the PR label Oct 23, 2024

added the test

e796993

Shivani-gslab reviewed Oct 28, 2024

View reviewed changes

ansible_collections/arista/avd/molecule/eos_cli_config_gen/intended/configs/aaa.cfg Outdated Show resolved Hide resolved

ansible_collections/arista/avd/molecule/eos_cli_config_gen/intended/configs/aaa.cfg Outdated Show resolved Hide resolved

github-actions bot added the state: conflict PR with conflict label Oct 30, 2024

resolved the conflicts

a6541d9

github-actions bot removed the state: conflict PR with conflict label Oct 30, 2024

MaheshGSLAB marked this pull request as ready for review November 4, 2024 06:02

MaheshGSLAB requested review from a team as code owners November 4, 2024 06:02

Merge branch 'devel' into issue-4630

c7079bc

MaheshGSLAB requested a review from Shivani-gslab November 4, 2024 13:30

ClausHolbechArista requested changes Nov 6, 2024

View reviewed changes

...lections/arista/avd/molecule/eos_cli_config_gen/inventory/host_vars/host1/aaa-accounting.yml Outdated Show resolved Hide resolved

Shivani-gslab reviewed Nov 6, 2024

View reviewed changes

python-avd/pyavd/_eos_cli_config_gen/j2templates/eos/aaa-accounting.j2 Show resolved Hide resolved

updated the schema as type required key

17ac1fd

github-actions bot added role: eos_cli_config_gen issue related to eos_cli_config_gen role state: Documentation role Updated labels Nov 7, 2024

ClausHolbechArista requested changes Nov 8, 2024

View reviewed changes

python-avd/pyavd/_eos_cli_config_gen/schema/schema_fragments/aaa_accounting.schema.yml Outdated Show resolved Hide resolved

ClausHolbechArista marked this pull request as draft November 19, 2024 06:50

MaheshGSLAB and others added 2 commits November 22, 2024 17:25

Merge branch 'devel' into issue-4630

911a81f

removed required key

a9c42c2

github-actions bot removed role: eos_cli_config_gen issue related to eos_cli_config_gen role state: Documentation role Updated labels Nov 22, 2024

MaheshGSLAB requested a review from Shivani-gslab November 25, 2024 08:53

MaheshGSLAB requested a review from ClausHolbechArista November 25, 2024 08:53

MaheshGSLAB marked this pull request as ready for review November 25, 2024 08:54

MaheshGSLAB requested a review from gmuloc November 28, 2024 09:08

Merge branch 'devel' into issue-4630

be157c7

gmuloc reviewed Nov 28, 2024

View reviewed changes

MaheshGSLAB marked this pull request as draft November 29, 2024 11:44

MaheshGSLAB and others added 2 commits November 29, 2024 18:04

Merge branch 'devel' into issue-4630

674b72c

updated description in schema

096db31

github-actions bot added role: eos_cli_config_gen issue related to eos_cli_config_gen role state: Documentation role Updated labels Nov 29, 2024

MaheshGSLAB marked this pull request as ready for review November 29, 2024 13:29

MaheshGSLAB requested a review from gmuloc November 29, 2024 13:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor(eos_cli_config_gen): Improve the aaa accounting j2 template #4636

Refactor(eos_cli_config_gen): Improve the aaa accounting j2 template #4636

MaheshGSLAB commented Oct 23, 2024 •

edited

Loading

github-actions bot commented Oct 23, 2024

github-actions bot commented Oct 30, 2024

github-actions bot commented Oct 30, 2024

sonarcloud bot commented Nov 7, 2024

ClausHolbechArista commented Nov 19, 2024

gmuloc Nov 28, 2024

MaheshGSLAB Nov 29, 2024

gmuloc Nov 28, 2024

MaheshGSLAB Nov 29, 2024

gmuloc Nov 28, 2024

MaheshGSLAB Nov 29, 2024

Refactor(eos_cli_config_gen): Improve the aaa accounting j2 template #4636

Are you sure you want to change the base?

Refactor(eos_cli_config_gen): Improve the aaa accounting j2 template #4636

Conversation

MaheshGSLAB commented Oct 23, 2024 • edited Loading

Change Summary

Related Issue(s)

Component(s) name

Proposed changes

How to test

Checklist

User Checklist

Repository Checklist

github-actions bot commented Oct 23, 2024

github-actions bot commented Oct 30, 2024

github-actions bot commented Oct 30, 2024

sonarcloud bot commented Nov 7, 2024

Quality Gate passed

ClausHolbechArista commented Nov 19, 2024

gmuloc Nov 28, 2024

Choose a reason for hiding this comment

MaheshGSLAB Nov 29, 2024

Choose a reason for hiding this comment

gmuloc Nov 28, 2024

Choose a reason for hiding this comment

MaheshGSLAB Nov 29, 2024

Choose a reason for hiding this comment

gmuloc Nov 28, 2024

Choose a reason for hiding this comment

MaheshGSLAB Nov 29, 2024

Choose a reason for hiding this comment

MaheshGSLAB commented Oct 23, 2024 •

edited

Loading