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): Support custom User Roles #1316

Merged
merged 14 commits into from
Nov 23, 2021
Merged

Feat(eos_cli_config_gen): Support custom User Roles #1316

merged 14 commits into from
Nov 23, 2021

Conversation

ccsnw
Copy link
Contributor

@ccsnw ccsnw commented Nov 11, 2021

Change Summary

Add possibility to create custom User Roles

Related Issue(s)

N/A

Component(s) name

arista.avd.eos_cli_config_gen

Proposed changes

Support for adding custom user roles

How to test

N/A

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 github-actions bot requested a review from onurgashi November 11, 2021 05:21
@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 Nov 11, 2021
ClausHolbechArista
ClausHolbechArista requested changes Nov 11, 2021
View reviewed changes
Copy link
Contributor

@ClausHolbechArista ClausHolbechArista left a comment

Choose a reason for hiding this comment

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

Great work. I have a suggestion on changing the data model to align closer to EOS CLI as well as following our future preferences.

ansible_collections/arista/avd/molecule/eos_cli_config_gen/inventory/host_vars/user-roles.yml Outdated Show resolved Hide resolved
ansible_collections/arista/avd/roles/eos_cli_config_gen/README.md Outdated Show resolved Hide resolved
ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/eos/user-roles.j2 Outdated Show resolved Hide resolved
@ClausHolbechArista ClausHolbechArista added this to the v3.2.0 milestone Nov 11, 2021
@ccsnw
Copy link
Contributor Author

ccsnw commented Nov 11, 2021

thanks for your feedback Claus. In oder to make your suggested changes, I'll have to close this PR, do the changes and then add a new PR, right?

According to the Datamodel: Sounds good: I can also rework the ACL part in a similar way as next step

@ClausHolbechArista
Copy link
Contributor

thanks for your feedback Claus. In oder to make your suggested changes, I'll have to close this PR, do the changes and then add a new PR, right?

If you just continue pushing to your own fork (same branch), it will update the PR automatically.

According to the Datamodel: Sounds good: I can also rework the ACL part in a similar way as next step

Re ACL, we will not merge changes to existing data models until 4.0 since they would break existing deployments. We are already planning to rework all the dicts with variable keys for 4.0. If you are up for it when we get closer, I can assign you some of them.

Btw please drop me a mail on holbech at arista.com, so we know who contributes. Thanks

ClausHolbechArista
ClausHolbechArista requested changes Nov 12, 2021
View reviewed changes
ansible_collections/arista/avd/molecule/eos_cli_config_gen/documentation/devices/user-roles.md Outdated Show resolved Hide resolved
ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/documentation/user-roles.j2 Outdated Show resolved Hide resolved
ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/documentation/user-roles.j2 Outdated Show resolved Hide resolved
ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/documentation/user-roles.j2 Outdated Show resolved Hide resolved
ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/documentation/user-roles.j2 Outdated Show resolved Hide resolved
ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/eos/user-roles.j2 Outdated Show resolved Hide resolved
ansible_collections/arista/avd/roles/eos_cli_config_gen/templates/eos/user-roles.j2 Outdated Show resolved Hide resolved
@github-actions github-actions bot added the state: conflict PR with conflict label Nov 22, 2021
@github-actions
Copy link

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

ClausHolbechArista
ClausHolbechArista approved these changes Nov 22, 2021
View reviewed changes
Copy link
Contributor

@ClausHolbechArista ClausHolbechArista left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot removed the state: conflict PR with conflict label Nov 22, 2021
@github-actions
Copy link

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

onurgashi
onurgashi approved these changes Nov 23, 2021
View reviewed changes
Copy link
Contributor

@onurgashi onurgashi left a comment

Choose a reason for hiding this comment

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

LGTM!

@ClausHolbechArista ClausHolbechArista merged commit 117002b into aristanetworks:devel Nov 23, 2021
@ccsnw ccsnw deleted the user-roles branch November 25, 2021 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ClausHolbechArista ClausHolbechArista ClausHolbechArista approved these changes

@onurgashi onurgashi onurgashi approved these changes

@titom73 titom73 Awaiting requested review from titom73

@aphillipps aphillipps Awaiting requested review from aphillipps

@carlbuchmann carlbuchmann Awaiting requested review from carlbuchmann

Assignees

@ccsnw ccsnw

Labels
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
Milestone
v3.2.0
Development

Successfully merging this pull request may close these issues.
4 participants
@ccsnw @ClausHolbechArista @onurgashi @emilarista