-
Notifications
You must be signed in to change notification settings - Fork 656
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
Use reference of servers in /system/aaa/server-groups. Move the actual server list to top level of /system/aaa. #1105
base: master
Are you sure you want to change the base?
Conversation
/gcbrun |
No major YANG version changes in commit d0533c4 |
It looks like that some checks are complaining about the deref I use in path. Is deref not support in path? Pyang doesn't give me any error on that. If it's not supported, any suggestion on how to use leafref on a node with multiple keys and make sure all keys are from the same element? Below is the code using deref in the PR:
|
https://datatracker.ietf.org/doc/html/rfc7950#section-9.9.2 |
Wendyi who did you request review this commit? |
Removed deref from the path. Use name as the single key for server and as the leafref in server group member list. |
/gcbrun |
/gcbrun |
/gcbrun |
description "Network-instance of the authentication server"; | ||
} | ||
|
||
leaf type { |
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.
This aaa server type overlaps/conflicts with the type defined at the server-group level. Do we need this defined per server?
Also, I observe that there is only one base type, so maybe we don't really even need type at all?
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.
Thanks for reviewing Darren.
This aaa server type overlaps/conflicts with the type defined at the server-group level. Do we need this defined per server?
An AAA server can be used either directly by itself or within a server group, so we need this type defined per server as well. Vendors can decide what to do if they see a conflict between the type of the group and the type of the server.
Also, I observe that there is only one base type, so maybe we don't really even need type at all?
leaf type
here is an identityref which has existing identity RADIUS and TACACS added to it from their own files:
identity RADIUS { |
identity TACACS { |
so it seems to have two types with it. Am I using this wrong?
Placeholder note for myself, this change looks like it is needed for supporting aaa servers that have the same IP address but are in different network instances, is that right @WenyiChengANET ? |
This is correct. It also moves the |
/gcbrun |
/gcbrun |
/gcbrun |
This change is needed for configuring AAA servers in network-instances. I also observe 3 implementations which already support single and server group based configuration. Quoting more implementation references: Arista EOS documentation shows server and server group configuration. Cisco IOS XR supports single aaa server and grouped aaa server configuration. JunOS supports group and single server configuration (the example uses a group, but it is mentioned that the group is optional). SRLinux appears to only support aaa configuration in a group |
Setting last call to Nov 14, 2024 |
Should this one maybe get a bit more visibility and discussion? I don't think it has come up on the monthly vendor meeting. Although it isn't NBC right now (since the old model is still available but deprecated) it will be a breaking change later. The references above do show implementations with servers defined outside of server-groups, but all those implementations seem to use address as the server key today. Do any OpenConfig models actually point to an individual server rather than a server-group? Is there a plan to do so? I can see how some implementations may support that but if OpenConfig models don't refer to an individual server then is there a need to make that change in the model? Adding the NI leafref does seem like a needed addition though. |
(M) system/openconfig-aaa-radius.yang
(M) system/openconfig-aaa-tacacs.yang
(M) system/openconfig-aaa.yang
Change Scope:
Use name instead of only address as the key for a server. A server should be defined with address, port, network-instance and type. Using name as the single key because this need to be a leafref in the server group member list and multi-key ref is not well supported currently.
Deprecate port/authen-port from aaa-tacacs and aaa-radius config. Move it to server config.
Deprecate the server list in the “server-groups” container and move “servers” up to the top level for aaa.
Add a list of server ref matching the top level server list in the “server-groups” container.
This is a slow change which keeps the original nodes and marks them as deprecated.
Platform Implementations:
Arista AAA configuration:
Servers with the same host/port can be configured in different VRFs(network-instance).
One server can be added to multiple server groups.
Arista documentation
Pyang output for the new model:
*please note that the old nodes has been removed from this output for easy review. In the actual change they are marked as deprecated.