Add RoleAssignment UUID Generation ADR #3935
Conversation
super-harsh
requested review from
davefellows,
theunrepentantgeek,
matthchr and
babbageclunk
as code owners
docs/hugo/content/design/ADR-2024-04-RoleAssignment-UUID-Generation.md
Outdated
Show resolved
Hide resolved
docs/hugo/content/design/ADR-2024-04-RoleAssignment-UUID-Generation.md
Outdated
Show resolved
Hide resolved
|
|
||
| **Pro**: User does not have to do anything | ||
|
|
||
| **Con**: Moving resource with same name between multiple ASO instances would require a workaround |
There was a problem hiding this comment.
Including subid in the seed doesn't imply this, does it?
There was a problem hiding this comment.
True, will update.
The case is more when Moving resource with same name between older to newer ASO version would require a workaround
…ation.md Co-authored-by: Matthew Christopher <matthchr@users.noreply.github.com>
…ation.md Co-authored-by: Matthew Christopher <matthchr@users.noreply.github.com>
|
|
||
| To use deterministic uuid(default) | ||
| ```yaml | ||
| serviceoperator.azure.com/uuid-generation: Default |
There was a problem hiding this comment.
I don't love Default as a value because it doesn't actually say what the default is. IMO we should call this something like Stable
There was a problem hiding this comment.
This applies to the above option 1 as well.
| To use deterministic uuid(default) | ||
| ```yaml | ||
| serviceoperator.azure.com/uuid-generation: Default | ||
| ``` |
There was a problem hiding this comment.
I think you should talk here about how we actually do defaulting too:
There are two options:
- If not specified, automatically set
serviceoperator.azure.com/uuid-generation: Defaultwhen a resource is created when defaulting its name. - If not specified, don't set any annotation (but still generate a UUID as per the
Defaultpattern.
I think it's OK to default-in the uuid-generation annotation if the user hasn't set it (and also hasn't set the name?). I guess if the name is set by the user, then we wouldn't add the annotation at all because we actualyl didn't do any generation?
and if the user sets it, we'll honor what they set (though it'll not do anything if they set it and the name)?
|
Approved but had a few comments |
docs/hugo/content/design/ADR-2024-04-RoleAssignment-UUID-Generation.md
Outdated
Show resolved
Hide resolved
| - include the group and kind to ensure that different kinds of resources get different UUIDs. This isn't entirely required by Azure, but it makes sense to avoid collisions between two resources of different types even if they have the same namespace and name. | ||
| - include the owner group, kind, and name to avoid collisions between resources with the same name in different clusters that actually point to different Azure resources. | ||
|
|
||
| However, the case where users have multiple ASO instances in multiple clusters with resources in the same namespace, in each cluster having the same name and pointing to different Azure resource is not supported without the user manually giving each RoleAssignment its own UUID. |
There was a problem hiding this comment.
I don't understand this sentence.
|
|
||
| **Pro**: User does not have to do anything | ||
|
|
||
| **Con**: Moving resource with same name between older to newer ASO version would require a workaround |
There was a problem hiding this comment.
This is a breaking change when someone upgrades the version of ASO in their cluster, as any automated deployments will suddenly experience different behaviour.
| **Pro**: User does not have to do anything | ||
|
|
||
| **Con**: Moving resource with same name between older to newer ASO version would require a workaround | ||
| **Con**: Webhooks don't have information about subscriptionID |
There was a problem hiding this comment.
Doesn't this automatically disqualify Option 3 from consideration?
There was a problem hiding this comment.
Kinda, however worth mentioning it here so we know why we didn't consider this option. Just in case if someone stumbles upon this in future
| Recommendation: Option 2 - Using annotations | ||
|
|
||
| We retain how the resource shape looks like and suggest using annotation for the users running into the edge case. |
…ation.md Co-authored-by: Bevan Arps <bevan.arps@microsoft.com>
…ation.md Co-authored-by: Bevan Arps <bevan.arps@microsoft.com>
What this PR does / why we need it:
This PR adds ADR for RoleAssignment UUID clashes. ADR discusses about the issue with UUID clashes for RoleAssignment mentioned in #3637.
If applicable: