Skip to content

Conversation

jmpesp
Copy link
Contributor

@jmpesp jmpesp commented Aug 18, 2025

The silo admin group is a special group that is automatically created during silo creation where members are granted the silo admin role. This name however is not stored, but this is currently ok: any existing silo won't have the ability to delete groups, meaning that the automatically created admin group is there to stay and a group with a duplicate name cannot be created.

Very soon there will be silos with provision types that can delete groups, meaning this name has to be known in order for Nexus to create the appropriate policy when a group with a matching name is created again. It was a mistake not to store this parameter with the silo, so rectify that with this PR.

The silo admin group is a special group that is automatically created
during silo creation where members are granted the silo admin role. This
name however is not stored, but this is currently ok: any existing silo
won't have the ability to delete groups, meaning that the automatically
created admin group is there to stay and a group with a duplicate name
cannot be created.

Very soon there will be silos with provision types that _can_ delete
groups, meaning this name has to be known in order for Nexus to create
the appropriate policy when a group with a matching name is created
again. It was a mistake not to store this parameter with the silo, so
rectify that with this PR.
@jmpesp jmpesp requested a review from papertigers August 18, 2025 18:47
/// at silo create time.
/// 2) assigned a policy granting members the silo admin role
///
/// Prior to this column existing, for api_only and jit provision types,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do I understand correctly that existing api/JIT based silos will continue to function as they did before? This change makes it so that we have the potential for storing the admin group name, and will be doing so for SCIM based silos?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct yeah - existing silos didn't need this to function, though note that going forward any new silo created will record this field, including SCIM (which is the real type that needs it).

@jmpesp jmpesp enabled auto-merge (squash) August 28, 2025 17:01
@jmpesp jmpesp merged commit c2862a0 into oxidecomputer:main Aug 28, 2025
17 checks passed
@jmpesp jmpesp deleted the store_silo_admin_group_name branch August 29, 2025 15:01
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