-
Notifications
You must be signed in to change notification settings - Fork 220
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.
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
Fix(eos_cli_config_gen): Prevent empty source and dest ports list for ip access lists #4660
Conversation
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-4660
# Activate the virtual environment
source test-avd-pr-4660/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/gmuloc/avd.git@issue/4642#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/gmuloc/avd.git#/ansible_collections/arista/avd/,issue/4642 --force
# Optional: Install AVD examples
cd test-avd-pr-4660
ansible-playbook arista.avd.install_examples |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same behavior is observed here:
# Results in empty line (OK)
field-set l4-port src_port_set1
!
Line 126 in da8a39d
type: list
# Producing incorrect EOS CLI commands
sample policy samplepo1
match rule1 ipv4
source prefix 3.4.5.0/24
destination prefix 10.3.3.0/24
protocol tcp destination port
protocol udp source port destination port
avd-ci-leaf2(config-s-sc-postcard-sample-policy-match-samplepo1-rule1-ipv4)# protocol tcp destination port
% Incomplete command
avd-ci-leaf2(config-s-sc-postcard-sample-policy-match-samplepo1-rule1-ipv4)#protocol udp source port destination port
% Invalid input
avd-ci-leaf2(config-s-sc-postcard-sample-policy-match-samplepo1-rule1-ipv4)#
Should we tackle it here as well?
not really the scope of this PR to only fix the issue but that should be easy enough |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
All invalid use cases with empty lists are now raising an error
ERROR! [application-traffic-recognition]: 'Validation Error: application_traffic_recognition.field_sets.l4_ports[0].port_values': The value is shorter (0) than the allowed minimum of 1.
ERROR! [application-traffic-recognition]: 'Validation Error: application_traffic_recognition.field_sets.ipv4_prefixes[0].prefix_values': The value is shorter (0) than the allowed minimum of 1.
ERROR! [ip-access-lists]: 'Validation Error: ip_access_lists[0].entries[4].source_ports': The value is shorter (0) than the allowed minimum of 1.
ERROR! [ip-access-lists]: 'Validation Error: ip_access_lists[0].entries[4].destination_ports': The value is shorter (0) than the allowed minimum of 1.
ERROR! [monitor-telemetry-postcard-policy]: 'Validation Error: monitor_telemetry_postcard_policy.sample_policies[0].match_rules[0].protocols[0].destination_ports': The value is shorter (0) than the allowed minimum of 1.
ERROR! [monitor-telemetry-postcard-policy]: 'Validation Error: monitor_telemetry_postcard_policy.sample_policies[0].match_rules[0].protocols[1].source_ports': The value is shorter (0) than the allowed minimum of 1.
Quality Gate passedIssues Measures |
Change Summary
1/ cf issue description - without the limitation the key accepts empty list and render
eq <empty>
which is wrong CLI2/ PR was then grown for monitor postcard telemetry and application-traffic-recognition
Related Issue(s)
Fixes #4642
Component(s) name
arista.avd.eos_cli_config_gen
Proposed changes
min_length: 1
for dest and source port lists and other places from reviewHow to test
Verified schema in molecule by adding empty lists in the molecule test for ACL (not committed as this is only schema change)
Checklist
Repository Checklist