CNTRLPLANE-71: update cao to manage rolebindingrestriction crd #748
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: everettraven The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-robot
added
the
needs-rebase
everettraven
force-pushed
the
feature/manage-rbrs
branch
from
8e0b413 to
78efc35
Compare
everettraven
force-pushed
the
feature/manage-rbrs
branch
from
2733a85 to
7727139
Compare
Signed-off-by: Bryce Palmer <[email protected]>
everettraven
force-pushed
the
feature/manage-rbrs
branch
from
7727139 to
990efd5
Compare
openshift-merge-robot
removed
the
needs-rebase
everettraven
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@everettraven: This pull request references CNTRLPLANE-71 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
Signed-off-by: Bryce Palmer <[email protected]>
Signed-off-by: Bryce Palmer <[email protected]>
|
@everettraven: The following tests failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
| @@ -142,8 +141,9 @@ func prepareOauthOperator( | |||
| "oauth-openshift/oauth-service.yaml", | |||
| "oauth-openshift/trust_distribution_role.yaml", | |||
| "oauth-openshift/trust_distribution_rolebinding.yaml", | |||
| "oauth-openshift/authorization.openshift.io_rolebindingrestrictions.yaml", | |||
There was a problem hiding this comment.
Use a conditional resource (via WithConditionalResources; WithPrecondition might be useful too), to manage when this should be created/deleted and avoid contention.
There was a problem hiding this comment.
Launching a cluster with this PR as-is seemed to work just fine and didn't appear to have any contention between this and CVO.
I'm happy to add the conditional resources, but it would essentially be:
- Create only when the resource is not found or requires updates
- Delete never
Walking through the underlying logic, this seems to be the exact flow the static resource controller already takes:
- Calls
ApplyDirectly(): https://github.com/openshift/library-go/blob/4ea50293b28af3d28f547c670f52c175af9e4427/pkg/operator/staticresourcecontroller/static_resource_controller.go#L334 - For CustomResourceDefinitions,
ApplyDirectly()callsApplyCustomResourceDefinitionV1(): https://github.com/openshift/library-go/blob/4ea50293b28af3d28f547c670f52c175af9e4427/pkg/operator/resource/resourceapply/generic.go#L179 ApplyCustomResourceDefinitionV1()checks if the resource exists or has been modified to determine if it needs to sync it: https://github.com/openshift/library-go/blob/4ea50293b28af3d28f547c670f52c175af9e4427/pkg/operator/resource/resourceapply/apiextensions.go#L19-L35
I'm not sure it makes much sense to gate this resource on the same conditions that would be done by default from the static resource controller. We will add conditional logic for create/delete in the future based on desired state of the OAuth stack (i.e if the OAuth stack should be removed, this resource should also be removed and vice versa)
| @@ -38,6 +39,8 @@ func runOutputResources(ctx context.Context) (*libraryoutputresources.OutputReso | |||
| libraryoutputresources.ExactRoleBinding("openshift-config-managed", "system:openshift:oauth-servercert-trust"), | |||
|
|
|||
| libraryoutputresources.ExactPDB("openshift-oauth-apiserver", "oauth-apiserver-pdb"), | |||
|
|
|||
| libraryoutputresources.ExactResource(apiextensionsv1.SchemeGroupVersion.Group, apiextensionsv1.SchemeGroupVersion.Version, "customresourcedefinitions", "", "rolebindingrestrictions.authorization.openshift.io"), | |||
Description
Updates the cluster-authentication-operator to manage the
RoleBindingRestrictionCRD.More specifically, this PR:
authorization.openshift.io/RoleBindingRestrictionCRD manifest from https://github.com/openshift/apiRoleBindingRestrictionCRD manifests to thebindata/directory so the manifest can be embedded into the binary using the embed FS.bindata/directory is up to dateRoleBindingRestrictionCRD manifest inbindata/is up to date based on the latest vendored manifest.oauth-openshift/authorization.openshift.io_rolebindingrestrictions.yamlfile to the list of manifest files managed by the static resource controllerMotivation
Update the cluster-authentication-operator to manage the
RoleBindingRestrictionCRD, as outlined in openshift/enhancements#1726, to allow for this CRD to be removed from the cluster when the oauth stack is no longer desired.