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

CLI: New commands rill sudo org join and rill sudo org list-admins #5620

Merged
merged 3 commits into from
Sep 9, 2024

Conversation

begelundmuller
Copy link
Contributor

@begelundmuller begelundmuller commented Sep 5, 2024

This PR adds two commands to make it easier to manage orgs as a superuser:

  • rill sudo org list-admins <org>: lists all the admins of an organization. This can be used together with rill sudo user assume to temporarily assume an admin role on an organization that allows access from superusers.
  • rill sudo org join <org>: adds the current user as an admin of an organization. This action is not temporary and the user will show up in member listings.

Closes https://github.com/rilldata/rill-private-issues/issues/624

@begelundmuller begelundmuller self-assigned this Sep 5, 2024
@begelundmuller begelundmuller marked this pull request as ready for review September 5, 2024 17:00
Copy link
Contributor

@Kavinjsir Kavinjsir left a comment

Choose a reason for hiding this comment

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

LGTM!
Just left a small question to understand the coding pattern we are using :)

admin/server/organizations.go Show resolved Hide resolved
admin/server/organizations.go Show resolved Hide resolved
Copy link
Member

@pjain1 pjain1 left a comment

Choose a reason for hiding this comment

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

Should we just error in case of role change of already added user and indicate to use set-role cmd. Earlier behaviour was incorrect in case of role change that it would do nothing and still send out an email with wrong role name. I am fine with this approach as well.

@begelundmuller begelundmuller merged commit f849225 into main Sep 9, 2024
10 checks passed
@begelundmuller begelundmuller deleted the begelundmuller/sudo-admin-member-utils branch September 9, 2024 10:20
@begelundmuller
Copy link
Contributor Author

Should we just error in case of role change of already added user and indicate to use set-role cmd. Earlier behaviour was incorrect in case of role change that it would do nothing and still send out an email with wrong role name. I am fine with this approach as well.

It was a product wish to not error in rill user add even if they were already added, so given that constraint I think this behavior is closer to what's expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants