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.

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

Feat(eos_cli_config_gen): Add TLS options for radius_server #4194

Merged
merged 8 commits into from
Jul 11, 2024

Conversation

nnbruce
Copy link
Contributor

@nnbruce nnbruce commented Jul 9, 2024

Change Summary

Enhance radius_server functionality by implementing TLS (RadSec) support.

Related Issue(s)

Fixes #4175

Component(s) name

arista.avd.eos_cli_config_gen

Proposed changes

Adhering to EOS syntax, its possible to only enable TLS for a radius-server, in that case the global ssl-profile is used. In addition it should also be possible to specify a ssl-profile per radius-server. Also added the option to specify TCP Port if using TLS, might be useful if not using AGNI.

If TLS is enabled, we should no longer render key for that specific radius-server.

Any suggestions to make the data model better is appreciated.

How to test

Tested with DevContainer and Molecule. Molecule generated config tested on EOS.

Checklist

User Checklist

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 github-actions bot added role: eos_cli_config_gen issue related to eos_cli_config_gen role state: CI Updated CI scenario have been updated in the PR state: Documentation role Updated labels Jul 9, 2024
Copy link

github-actions bot commented Jul 9, 2024

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-4194
# Activate the virtual environment
source test-avd-pr-4194/bin/activate
# Install all requirements including PyAVD
pip install "pyavd[ansible] @ git+https://github.com/nnbruce/avd.git@fix_issue_4175#subdirectory=python-avd" --force
# Install Ansible collection
ansible-galaxy collection install git+https://github.com/nnbruce/avd.git#/ansible_collections/arista/avd/,fix_issue_4175 --force
# Optional: Install AVD examples
cd test-avd-pr-4194
ansible-playbook arista.avd.install_examples

@nnbruce nnbruce marked this pull request as ready for review July 9, 2024 09:51
@nnbruce nnbruce requested review from a team as code owners July 9, 2024 09:51
Copy link
Contributor

@gmuloc gmuloc left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR - and for implementing the molecule tests. I have reviewed and required a couple of changes but the PR is almost ready. Please reach out if you need help

@nnbruce
Copy link
Contributor Author

nnbruce commented Jul 9, 2024

Thank you for the great feedback. I think I've resolved everything. Let me know if you think we should change something else.

@nnbruce nnbruce requested a review from gmuloc July 10, 2024 08:59
@gmuloc gmuloc requested a review from a team July 10, 2024 11:21
Copy link
Contributor

@gmuloc gmuloc left a comment

Choose a reason for hiding this comment

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

Thanks @nnbruce - I have taken the opportunity to add the Global TLS profile to the PR.

Also you had me discover a potential bug #4198 for dynamic-authorization.

LGTM once CI passes.

@laxmikantchintakindi
Copy link
Contributor

LGTM.

@gmuloc gmuloc merged commit b64b80c into aristanetworks:devel Jul 11, 2024
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rn: Feat(eos_cli_config_gen) role: eos_cli_config_gen issue related to eos_cli_config_gen role state: CI Updated CI scenario have been updated in the PR state: Documentation role Updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

eos_cli_config_gen:radius_server/host/ssl_profile
5 participants