-
Notifications
You must be signed in to change notification settings - Fork 357
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
chore: add configurable posix claims fields to master config [RM-398] #9690
Conversation
✅ Deploy Preview for determined-ui canceled.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9690 +/- ##
==========================================
- Coverage 53.54% 53.53% -0.01%
==========================================
Files 1256 1256
Lines 152910 152914 +4
Branches 3297 3296 -1
==========================================
- Hits 81873 81870 -3
- Misses 70887 70894 +7
Partials 150 150
Flags with carried forward coverage won't be shown. Click here to find out more.
|
AgentUID int `json:"agent_uid"` | ||
AgentGID int `json:"agent_gid"` | ||
AgentUserName string `json:"agent_user_name"` | ||
AgentGroupName string `json:"agent_group_name"` |
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.
nit: Could we instead make these match GroupAttributeName
& DisplayNameAttributeName
, that way it's clear that these aren't the values for these fields, but instead the name of the attribute that can be passed in. Maybe something along the lines of AgentUIDAttributeName
?
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.
Absolutely! I was wondering about the naming; what is the meaning of AttributeName
? Is it kinda like a "key" in a key/value pair?
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.
Yep exactly!
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.
One small naming nit, but besides that looks solid! Thank you so much for your work on this!
Ticket
RM-398
Description
Add the following fields to the master config, for saml and oidc:
For now, these fields are unused. Docs will be updated once the fields have an impact.
Test Plan
Checklist
docs/release-notes/
See Release Note for details.