-
Notifications
You must be signed in to change notification settings - Fork 985
create admin group with server admin permission on install #15215
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
Conversation
6fb4ce4 to
7d0f89b
Compare
4abc9ab to
a1430a8
Compare
0b6a1d6 to
3b901b2
Compare
3b901b2 to
0e65343
Compare
7529182 to
7707522
Compare
|
I'm not opposed to creating an admin group by default but just want to clarify a little more. Why did we decide to just add an admin group by default rather than making this a Also, can the permissions of this group be modified? Can the group be deleted? Who relies on the admin group being present? |
I think it is simpler to create the group with permissions, we can avoid adding another question on the
Currently, yes, I don't think we need to protect it, see below.
The UI onboarding will reference the group. Currently, we suggest to 1. create the group 2. add the permission and 3. create the user with the group. We hope to simplify the onboarding to be a single step with creation of the user and referencing the group in a single command. The UI will show a note that if the group is not existing, this is how to create it. The note might be a solution for the question above to let users delete or change the group as they like and still have reliable onboarding instructions. |
7707522 to
2262b90
Compare
markylaing
left a comment
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.
Just the one comment and then LGTM :)
2262b90 to
bbefd3f
Compare
|
Comments should be resolved. Please have another pass at this one @markylaing |
markylaing
left a comment
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.
Ok last thing then as per #15215 (comment)
bbefd3f to
710cf34
Compare
710cf34 to
dab8b84
Compare
5ef058e to
649353a
Compare
…stall WD-20464 Signed-off-by: David Edler <[email protected]>
649353a to
2e18e59
Compare
|
I misunderstood what you meant previously @markylaing Please have another pass at this one now. |
markylaing
left a comment
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!
tomponline
left a comment
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.
Ta!
Done
and updateThis simplifies the UI onboarding, because we can rely on the presence of this group.