Add support for RoleDefinition resource #4067
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
matthchr
requested review from
davefellows,
theunrepentantgeek,
babbageclunk and
super-harsh
as code owners
matthchr
commented
Jun 7, 2024
| // TODO: Fix name | ||
| // MakeUniqueResourceString generates a string that uniquely identifies a cluster resource. | ||
| func MakeUniqueResourceString2(ownerGK schema.GroupKind, ownerName string, gk schema.GroupKind, namespace string, name string) string { | ||
| // TODO: This method has a bug where it is called with an empty owner gk when the owner is an ARM ID. |
There was a problem hiding this comment.
I'm not 100% sure what we should do about this bug. We can fix it (which could cause people to leak resources? Maybe not because our validation will protect us possibly? I need to do some testing...)
There was a problem hiding this comment.
I filed #4079 to track this in 2.9.0 and fixed this bug but only for the new resource RoleDefinition. We can track fixing it for existing resources as part of 2.9.0 with the above bug.
matthchr
commented
Jun 7, 2024
theunrepentantgeek
approved these changes
Jun 9, 2024
matthchr
force-pushed
the
feature/role-defs
branch
from
e9d784d
to
197c54d
Compare
theunrepentantgeek
approved these changes
Jun 11, 2024
|
/ok-to-test sha=197c54d |
matthchr
force-pushed
the
feature/role-defs
branch
from
197c54d
to
e6ace07
Compare
|
/ok-to-test sha=e6ace07 |
* Re-record tests using the newer API version.
Split Taskfile targets more to make it easier to run manual upgrade tests where the flow is: 1. Install GA ASO. 2. Perform manual testing step. 3. Upgrade to vNext ASO. 4. Perform manual testing step.
matthchr
force-pushed
the
feature/role-defs
branch
from
e6ace07
to
3af4937
Compare
|
/ok-to-test sha=3af4937 |
matthchr
force-pushed
the
feature/role-defs
branch
from
3af4937
to
22738a3
Compare
|
/ok-to-test sha=22738a3 |
This fixes Azure#2570.
Fix bug where RoleAssignment owned by ARM ID doesn't account for the ARM ID in the seed of the random UUID generate. This bugfix is BREAKING if the owner is using ARM ID and in the following cases: * User migrates RoleAssignment from one cluster to another. * User sets reconcile-policy: skip, deletes the RoleAssignment and then recreates it. In the above two cases, the new correct algorithm will consider the ARM ID of the owner and generate a different UUID than before. Other cases such as standard updates will not be impacted as Kubernetes sends the WHOLE object to the mutating webhook and for updates the object contains the (old) generated UUID.
matthchr
force-pushed
the
feature/role-defs
branch
from
22738a3
to
76e168b
Compare
|
/ok-to-test sha=197c54d |
|
/ok-to-test sha=76e168b |
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes #2570.
Closes #[issue number]
What this PR does / why we need it:
Special notes for your reviewer:
How does this PR make you feel:
If applicable: