-
Notifications
You must be signed in to change notification settings - Fork 39.8k
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
Correct APIGroup for RoleBindingBuilder Subjects #53239
Correct APIGroup for RoleBindingBuilder Subjects #53239
Conversation
@enj: GitHub didn't allow me to assign the following users: liggit. Note that only kubernetes members can be assigned. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
needs a test to make sure we don't drift in the future:
|
lgtm otherwise. we'll want to pick to 1.8 and 1.7 |
needs a kube issue and release note |
e4dd418
to
256d258
Compare
256d258
to
04cc6f4
Compare
Need to update the linting file.
|
This change corrects RoleBindingBuilder to use the RBAC API group with users and groups as subjects (service accounts use the empty string since they are in the legacy core group). This is based on the defaulting in pkg/apis/rbac/v1/defaults.go#SetDefaults_Subject. This is required because the bootstrap RBAC data is built with these helpers and does not go through defaulting, whereas the data retrieved from the server has already gone through defaulting. This can lead to the reconciliation code incorrectly adding duplicate subjects because it believes that they are missing (since the API groups do not match). Signed-off-by: Monis Khan <[email protected]>
04cc6f4
to
5eb5b3e
Compare
@ericchiang @liggitt comments addressed. |
/lgtm |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: enj, liggitt Assign the PR to them by writing Associated issue: 53296 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
Automatic merge from submit-queue (batch tested with PRs 51034, 53239). If you want to cherry-pick this change to another branch, please follow the instructions here. |
Commit found in the "release-1.8" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked. |
This change corrects
RoleBindingBuilder
to use the RBAC API group with users and groups as subjects (service accounts use the empty string since they are in the legacy core group). This is based on the defaulting inpkg/apis/rbac/v1/defaults.go#SetDefaults_Subject
. This is required because the bootstrap RBAC data is built with these helpers and does not go through defaulting, whereas the data retrieved from the server has already gone through defaulting. This can lead to the reconciliation code incorrectly adding duplicate subjects because it believes that they are missing (since the API groups do not match).Signed-off-by: Monis Khan [email protected]
/assign @liggitt
/sig auth
Fixes #53296
Fixes openshift/origin/issues/16611