From 171b08c000b91306966a181201429acbfc7b34af Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Thu, 8 Jun 2017 14:38:27 -0400 Subject: [PATCH] Collapse code between authorizationsync and migrate This change adds functions that handle all normalization, conversion and comparison for the authorization objects. These are now shared between authorizationsync and `oadm migrate authorization` to prevent any logic drift. Signed-off-by: Monis Khan --- .../controller/authorizationsync/normalize.go | 152 +++++++++++++++++- .../origin_to_rbac_clusterrole_controller.go | 52 ++---- ...n_to_rbac_clusterrolebinding_controller.go | 43 ++--- .../origin_to_rbac_role_controller.go | 46 ++---- .../origin_to_rbac_rolebinding_controller.go | 43 ++--- .../migrate/authorization/authorization.go | 62 ++----- 6 files changed, 220 insertions(+), 178 deletions(-) diff --git a/pkg/authorization/controller/authorizationsync/normalize.go b/pkg/authorization/controller/authorizationsync/normalize.go index 20772b6cf67a..ee508c4afef5 100644 --- a/pkg/authorization/controller/authorizationsync/normalize.go +++ b/pkg/authorization/controller/authorizationsync/normalize.go @@ -3,16 +3,160 @@ package authorizationsync import ( "strings" - authorizationapi "github.com/openshift/origin/pkg/authorization/api" - + apiequality "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kubernetes/pkg/apis/rbac" + + authorizationapi "github.com/openshift/origin/pkg/authorization/api" ) -// NormalizePolicyRules mutates the given rules and lowercases verbs, resources and API groups. +// ConvertToRBACClusterRole performs the conversion and guarantees the returned object is safe to mutate. +func ConvertToRBACClusterRole(originClusterRole *authorizationapi.ClusterRole) (*rbac.ClusterRole, error) { + // convert the origin role to an rbac role + convertedClusterRole := &rbac.ClusterRole{} + if err := authorizationapi.Convert_api_ClusterRole_To_rbac_ClusterRole(originClusterRole, convertedClusterRole, nil); err != nil { + return nil, err + } + // do a deep copy here since conversion does not guarantee a new object. + equivalentClusterRole := &rbac.ClusterRole{} + if err := rbac.DeepCopy_rbac_ClusterRole(convertedClusterRole, equivalentClusterRole, cloner); err != nil { + return nil, err + } + + // normalize rules before persisting so RBAC's case sensitive authorizer will work + normalizePolicyRules(equivalentClusterRole.Rules) + + // there's one wrinkle. If `openshift.io/reconcile-protect` is to true, then we must set rbac.authorization.kubernetes.io/autoupdate to false to + if equivalentClusterRole.Annotations["openshift.io/reconcile-protect"] == "true" { + equivalentClusterRole.Annotations["rbac.authorization.kubernetes.io/autoupdate"] = "false" + delete(equivalentClusterRole.Annotations, "openshift.io/reconcile-protect") + } + + // resource version cannot be set during creation + equivalentClusterRole.ResourceVersion = "" + + return equivalentClusterRole, nil +} + +// PrepareForUpdateClusterRole compares newClusterRole with existingClusterRole to determine if an update is required. +// newClusterRole must be safe to modify as it is mutated during the comparison which must ignore fields that will never match. +// Returns true if an update is required, in which case newClusterRole should be passed to Update. +func PrepareForUpdateClusterRole(newClusterRole, existingClusterRole *rbac.ClusterRole) bool { + // if we might need to update, we need to stomp fields that are never going to match like uid and creation time + newClusterRole.ObjectMeta = prepareForUpdateObjectMeta(newClusterRole.ObjectMeta, existingClusterRole.ObjectMeta) + + // determine if they are not equal + return !apiequality.Semantic.DeepEqual(newClusterRole, existingClusterRole) +} + +// ConvertToRBACClusterRoleBinding performs the conversion and guarantees the returned object is safe to mutate. +func ConvertToRBACClusterRoleBinding(originClusterRoleBinding *authorizationapi.ClusterRoleBinding) (*rbac.ClusterRoleBinding, error) { + // convert the origin roleBinding to an rbac roleBinding + convertedClusterRoleBinding := &rbac.ClusterRoleBinding{} + if err := authorizationapi.Convert_api_ClusterRoleBinding_To_rbac_ClusterRoleBinding(originClusterRoleBinding, convertedClusterRoleBinding, nil); err != nil { + return nil, err + } + // do a deep copy here since conversion does not guarantee a new object. + equivalentClusterRoleBinding := &rbac.ClusterRoleBinding{} + if err := rbac.DeepCopy_rbac_ClusterRoleBinding(convertedClusterRoleBinding, equivalentClusterRoleBinding, cloner); err != nil { + return nil, err + } + + // resource version cannot be set during creation + equivalentClusterRoleBinding.ResourceVersion = "" + + return equivalentClusterRoleBinding, nil +} + +// PrepareForUpdateClusterRoleBinding compares newClusterRoleBinding with existingClusterRoleBinding to determine if an update is required. +// newClusterRoleBinding must be safe to modify as it is mutated during the comparison which must ignore fields that will never match. +// Returns true if an update is required, in which case newClusterRoleBinding should be passed to Update. +func PrepareForUpdateClusterRoleBinding(newClusterRoleBinding, existingClusterRoleBinding *rbac.ClusterRoleBinding) bool { + // if we might need to update, we need to stomp fields that are never going to match like uid and creation time + newClusterRoleBinding.ObjectMeta = prepareForUpdateObjectMeta(newClusterRoleBinding.ObjectMeta, existingClusterRoleBinding.ObjectMeta) + + // determine if they are not equal + return !apiequality.Semantic.DeepEqual(newClusterRoleBinding, existingClusterRoleBinding) +} + +// ConvertToRBACRole performs the conversion and guarantees the returned object is safe to mutate. +func ConvertToRBACRole(originRole *authorizationapi.Role) (*rbac.Role, error) { + // convert the origin role to an rbac role + convertedRole := &rbac.Role{} + if err := authorizationapi.Convert_api_Role_To_rbac_Role(originRole, convertedRole, nil); err != nil { + return nil, err + } + // do a deep copy here since conversion does not guarantee a new object. + equivalentRole := &rbac.Role{} + if err := rbac.DeepCopy_rbac_Role(convertedRole, equivalentRole, cloner); err != nil { + return nil, err + } + + // normalize rules before persisting so RBAC's case sensitive authorizer will work + normalizePolicyRules(equivalentRole.Rules) + + // resource version cannot be set during creation + equivalentRole.ResourceVersion = "" + + return equivalentRole, nil +} + +// PrepareForUpdateRole compares newRole with existingRole to determine if an update is required. +// newRole must be safe to modify as it is mutated during the comparison which must ignore fields that will never match. +// Returns true if an update is required, in which case newRole should be passed to Update. +func PrepareForUpdateRole(newRole, existingRole *rbac.Role) bool { + // if we might need to update, we need to stomp fields that are never going to match like uid and creation time + newRole.ObjectMeta = prepareForUpdateObjectMeta(newRole.ObjectMeta, existingRole.ObjectMeta) + + // determine if they are not equal + return !apiequality.Semantic.DeepEqual(newRole, existingRole) +} + +// ConvertToRBACRoleBinding performs the conversion and guarantees the returned object is safe to mutate. +func ConvertToRBACRoleBinding(originRoleBinding *authorizationapi.RoleBinding) (*rbac.RoleBinding, error) { + // convert the origin roleBinding to an rbac roleBinding + convertedRoleBinding := &rbac.RoleBinding{} + if err := authorizationapi.Convert_api_RoleBinding_To_rbac_RoleBinding(originRoleBinding, convertedRoleBinding, nil); err != nil { + return nil, err + } + // do a deep copy here since conversion does not guarantee a new object. + equivalentRoleBinding := &rbac.RoleBinding{} + if err := rbac.DeepCopy_rbac_RoleBinding(convertedRoleBinding, equivalentRoleBinding, cloner); err != nil { + return nil, err + } + + // resource version cannot be set during creation + equivalentRoleBinding.ResourceVersion = "" + + return equivalentRoleBinding, nil +} + +// PrepareForUpdateRoleBinding compares newRoleBinding with existingRoleBinding to determine if an update is required. +// newRoleBinding must be safe to modify as it is mutated during the comparison which must ignore fields that will never match. +// Returns true if an update is required, in which case newRoleBinding should be passed to Update. +func PrepareForUpdateRoleBinding(newRoleBinding, existingRoleBinding *rbac.RoleBinding) bool { + // if we might need to update, we need to stomp fields that are never going to match like uid and creation time + newRoleBinding.ObjectMeta = prepareForUpdateObjectMeta(newRoleBinding.ObjectMeta, existingRoleBinding.ObjectMeta) + + // determine if they are not equal + return !apiequality.Semantic.DeepEqual(newRoleBinding, existingRoleBinding) +} + +// We check if we need to update by comparing the new and existing object meta. +// Thus we need to stomp fields that are never going to match. +func prepareForUpdateObjectMeta(newObjectMeta, existingObjectMeta v1.ObjectMeta) v1.ObjectMeta { + newObjectMeta.SelfLink = existingObjectMeta.SelfLink + newObjectMeta.UID = existingObjectMeta.UID + newObjectMeta.ResourceVersion = existingObjectMeta.ResourceVersion + newObjectMeta.CreationTimestamp = existingObjectMeta.CreationTimestamp + return newObjectMeta +} + +// normalizePolicyRules mutates the given rules and lowercases verbs, resources and API groups. // Origin's authorizer is case-insensitive to these fields but Kubernetes RBAC is not. Thus normalizing // the Origin rules before persisting them as RBAC will allow authorization to continue to work. -func NormalizePolicyRules(rules []rbac.PolicyRule) { +func normalizePolicyRules(rules []rbac.PolicyRule) { for i := range rules { rule := &rules[i] toLowerSlice(rule.Verbs) diff --git a/pkg/authorization/controller/authorizationsync/origin_to_rbac_clusterrole_controller.go b/pkg/authorization/controller/authorizationsync/origin_to_rbac_clusterrole_controller.go index 235c12ce91e4..26d154330e2b 100644 --- a/pkg/authorization/controller/authorizationsync/origin_to_rbac_clusterrole_controller.go +++ b/pkg/authorization/controller/authorizationsync/origin_to_rbac_clusterrole_controller.go @@ -9,8 +9,6 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" - kapi "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/apis/rbac" rbacclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/rbac/internalversion" rbacinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion/rbac/internalversion" rbaclister "k8s.io/kubernetes/pkg/client/listers/rbac/internalversion" @@ -75,52 +73,32 @@ func (c *OriginClusterRoleToRBACClusterRoleController) syncClusterRole(name stri return c.rbacClient.ClusterRoles().Delete(name, nil) } - // convert the origin role to an rbac role and compare the results - convertedClusterRole := &rbac.ClusterRole{} - if err := authorizationapi.Convert_api_ClusterRole_To_rbac_ClusterRole(originClusterRole, convertedClusterRole, nil); err != nil { + // determine if we need to create, update or do nothing + equivalentClusterRole, err := ConvertToRBACClusterRole(originClusterRole) + if err != nil { return err } - // do a deep copy here since conversion does not guarantee a new object. - equivalentClusterRole := &rbac.ClusterRole{} - if err := rbac.DeepCopy_rbac_ClusterRole(convertedClusterRole, equivalentClusterRole, cloner); err != nil { - return err - } - - // normalize rules before persisting so RBAC's case sensitive authorizer will work - NormalizePolicyRules(equivalentClusterRole.Rules) - - // there's one wrinkle. If `openshift.io/reconcile-protect` is to true, then we must set rbac.authorization.kubernetes.io/autoupdate to false to - if equivalentClusterRole.Annotations["openshift.io/reconcile-protect"] == "true" { - equivalentClusterRole.Annotations["rbac.authorization.kubernetes.io/autoupdate"] = "false" - delete(equivalentClusterRole.Annotations, "openshift.io/reconcile-protect") - } // if we're missing the rbacClusterRole, create it if apierrors.IsNotFound(rbacErr) { - equivalentClusterRole.ResourceVersion = "" _, err := c.rbacClient.ClusterRoles().Create(equivalentClusterRole) return err } - // if we might need to update, we need to stomp fields that are never going to match like uid and creation time - equivalentClusterRole.SelfLink = rbacClusterRole.SelfLink - equivalentClusterRole.UID = rbacClusterRole.UID - equivalentClusterRole.ResourceVersion = rbacClusterRole.ResourceVersion - equivalentClusterRole.CreationTimestamp = rbacClusterRole.CreationTimestamp - - // if they're equal, we have no work to do - if kapi.Semantic.DeepEqual(equivalentClusterRole, rbacClusterRole) { - return nil + // if they are not equal, we need to update + if PrepareForUpdateClusterRole(equivalentClusterRole, rbacClusterRole) { + glog.V(1).Infof("writing RBAC clusterrole %v", name) + _, err := c.rbacClient.ClusterRoles().Update(equivalentClusterRole) + // if the update was invalid, we're probably changing an immutable field or something like that + // either way, the existing object is wrong. Delete it and try again. + if apierrors.IsInvalid(err) { + c.rbacClient.ClusterRoles().Delete(name, nil) // ignore delete error + } + return err } - glog.V(1).Infof("writing RBAC clusterrole %v", name) - _, err := c.rbacClient.ClusterRoles().Update(equivalentClusterRole) - // if the update was invalid, we're probably changing an immutable field or something like that - // either way, the existing object is wrong. Delete it and try again. - if apierrors.IsInvalid(err) { - c.rbacClient.ClusterRoles().Delete(name, nil) - } - return err + // they are equal so we have no work to do + return nil } func (c *OriginClusterRoleToRBACClusterRoleController) clusterPolicyEventHandler() cache.ResourceEventHandler { diff --git a/pkg/authorization/controller/authorizationsync/origin_to_rbac_clusterrolebinding_controller.go b/pkg/authorization/controller/authorizationsync/origin_to_rbac_clusterrolebinding_controller.go index 19784d67ed52..ad869d7c87a1 100644 --- a/pkg/authorization/controller/authorizationsync/origin_to_rbac_clusterrolebinding_controller.go +++ b/pkg/authorization/controller/authorizationsync/origin_to_rbac_clusterrolebinding_controller.go @@ -9,8 +9,6 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" - kapi "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/apis/rbac" rbacclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/rbac/internalversion" rbacinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion/rbac/internalversion" rbaclister "k8s.io/kubernetes/pkg/client/listers/rbac/internalversion" @@ -75,43 +73,32 @@ func (c *OriginClusterRoleBindingToRBACClusterRoleBindingController) syncCluster return c.rbacClient.ClusterRoleBindings().Delete(name, nil) } - // convert the origin roleBinding to an rbac roleBinding and compare the results - convertedClusterRoleBinding := &rbac.ClusterRoleBinding{} - if err := authorizationapi.Convert_api_ClusterRoleBinding_To_rbac_ClusterRoleBinding(originClusterRoleBinding, convertedClusterRoleBinding, nil); err != nil { - return err - } - // do a deep copy here since conversion does not guarantee a new object. - equivalentClusterRoleBinding := &rbac.ClusterRoleBinding{} - if err := rbac.DeepCopy_rbac_ClusterRoleBinding(convertedClusterRoleBinding, equivalentClusterRoleBinding, cloner); err != nil { + // determine if we need to create, update or do nothing + equivalentClusterRoleBinding, err := ConvertToRBACClusterRoleBinding(originClusterRoleBinding) + if err != nil { return err } // if we're missing the rbacClusterRoleBinding, create it if apierrors.IsNotFound(rbacErr) { - equivalentClusterRoleBinding.ResourceVersion = "" _, err := c.rbacClient.ClusterRoleBindings().Create(equivalentClusterRoleBinding) return err } - // if we might need to update, we need to stomp fields that are never going to match like uid and creation time - equivalentClusterRoleBinding.SelfLink = rbacClusterRoleBinding.SelfLink - equivalentClusterRoleBinding.UID = rbacClusterRoleBinding.UID - equivalentClusterRoleBinding.ResourceVersion = rbacClusterRoleBinding.ResourceVersion - equivalentClusterRoleBinding.CreationTimestamp = rbacClusterRoleBinding.CreationTimestamp - - // if they're equal, we have no work to do - if kapi.Semantic.DeepEqual(equivalentClusterRoleBinding, rbacClusterRoleBinding) { - return nil + // if they are not equal, we need to update + if PrepareForUpdateClusterRoleBinding(equivalentClusterRoleBinding, rbacClusterRoleBinding) { + glog.V(1).Infof("writing RBAC clusterrolebinding %v", name) + _, err := c.rbacClient.ClusterRoleBindings().Update(equivalentClusterRoleBinding) + // if the update was invalid, we're probably changing an immutable field or something like that + // either way, the existing object is wrong. Delete it and try again. + if apierrors.IsInvalid(err) { + c.rbacClient.ClusterRoleBindings().Delete(name, nil) // ignore delete error + } + return err } - glog.V(1).Infof("writing RBAC clusterrolebinding %v", name) - _, err := c.rbacClient.ClusterRoleBindings().Update(equivalentClusterRoleBinding) - // if the update was invalid, we're probably changing an immutable field or something like that - // either way, the existing object is wrong. Delete it and try again. - if apierrors.IsInvalid(err) { - c.rbacClient.ClusterRoleBindings().Delete(name, nil) - } - return err + // they are equal so we have no work to do + return nil } func (c *OriginClusterRoleBindingToRBACClusterRoleBindingController) clusterPolicyBindingEventHandler() cache.ResourceEventHandler { diff --git a/pkg/authorization/controller/authorizationsync/origin_to_rbac_role_controller.go b/pkg/authorization/controller/authorizationsync/origin_to_rbac_role_controller.go index efc176a00cdd..4ae840214c57 100644 --- a/pkg/authorization/controller/authorizationsync/origin_to_rbac_role_controller.go +++ b/pkg/authorization/controller/authorizationsync/origin_to_rbac_role_controller.go @@ -9,8 +9,6 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" - kapi "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/apis/rbac" rbacclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/rbac/internalversion" rbacinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion/rbac/internalversion" rbaclister "k8s.io/kubernetes/pkg/client/listers/rbac/internalversion" @@ -80,46 +78,32 @@ func (c *OriginRoleToRBACRoleController) syncRole(key string) error { return c.rbacClient.Roles(namespace).Delete(name, nil) } - // convert the origin role to an rbac role and compare the results - convertedRole := &rbac.Role{} - if err := authorizationapi.Convert_api_Role_To_rbac_Role(originRole, convertedRole, nil); err != nil { - return err - } - // do a deep copy here since conversion does not guarantee a new object. - equivalentRole := &rbac.Role{} - if err := rbac.DeepCopy_rbac_Role(convertedRole, equivalentRole, cloner); err != nil { + // determine if we need to create, update or do nothing + equivalentRole, err := ConvertToRBACRole(originRole) + if err != nil { return err } - // normalize rules before persisting so RBAC's case sensitive authorizer will work - NormalizePolicyRules(equivalentRole.Rules) - // if we're missing the rbacRole, create it if apierrors.IsNotFound(rbacErr) { - equivalentRole.ResourceVersion = "" _, err := c.rbacClient.Roles(namespace).Create(equivalentRole) return err } - // if we might need to update, we need to stomp fields that are never going to match like uid and creation time - equivalentRole.SelfLink = rbacRole.SelfLink - equivalentRole.UID = rbacRole.UID - equivalentRole.ResourceVersion = rbacRole.ResourceVersion - equivalentRole.CreationTimestamp = rbacRole.CreationTimestamp - - // if they're equal, we have no work to do - if kapi.Semantic.DeepEqual(equivalentRole, rbacRole) { - return nil + // if they are not equal, we need to update + if PrepareForUpdateRole(equivalentRole, rbacRole) { + glog.V(1).Infof("writing RBAC role %v/%v", namespace, name) + _, err := c.rbacClient.Roles(namespace).Update(equivalentRole) + // if the update was invalid, we're probably changing an immutable field or something like that + // either way, the existing object is wrong. Delete it and try again. + if apierrors.IsInvalid(err) { + c.rbacClient.Roles(namespace).Delete(name, nil) // ignore delete error + } + return err } - glog.V(1).Infof("writing RBAC role %v/%v", namespace, name) - _, err = c.rbacClient.Roles(namespace).Update(equivalentRole) - // if the update was invalid, we're probably changing an immutable field or something like that - // either way, the existing object is wrong. Delete it and try again. - if apierrors.IsInvalid(err) { - c.rbacClient.Roles(namespace).Delete(name, nil) - } - return err + // they are equal so we have no work to do + return nil } func (c *OriginRoleToRBACRoleController) policyEventHandler() cache.ResourceEventHandler { diff --git a/pkg/authorization/controller/authorizationsync/origin_to_rbac_rolebinding_controller.go b/pkg/authorization/controller/authorizationsync/origin_to_rbac_rolebinding_controller.go index 250be4311fc6..fed901ac2a52 100644 --- a/pkg/authorization/controller/authorizationsync/origin_to_rbac_rolebinding_controller.go +++ b/pkg/authorization/controller/authorizationsync/origin_to_rbac_rolebinding_controller.go @@ -9,8 +9,6 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/workqueue" - kapi "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/apis/rbac" rbacclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/rbac/internalversion" rbacinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion/rbac/internalversion" rbaclister "k8s.io/kubernetes/pkg/client/listers/rbac/internalversion" @@ -80,43 +78,32 @@ func (c *OriginRoleBindingToRBACRoleBindingController) syncRoleBinding(key strin return c.rbacClient.RoleBindings(namespace).Delete(name, nil) } - // convert the origin roleBinding to an rbac roleBinding and compare the results - convertedRoleBinding := &rbac.RoleBinding{} - if err := authorizationapi.Convert_api_RoleBinding_To_rbac_RoleBinding(originRoleBinding, convertedRoleBinding, nil); err != nil { - return err - } - // do a deep copy here since conversion does not guarantee a new object. - equivalentRoleBinding := &rbac.RoleBinding{} - if err := rbac.DeepCopy_rbac_RoleBinding(convertedRoleBinding, equivalentRoleBinding, cloner); err != nil { + // determine if we need to create, update or do nothing + equivalentRoleBinding, err := ConvertToRBACRoleBinding(originRoleBinding) + if err != nil { return err } // if we're missing the rbacRoleBinding, create it if apierrors.IsNotFound(rbacErr) { - equivalentRoleBinding.ResourceVersion = "" _, err := c.rbacClient.RoleBindings(namespace).Create(equivalentRoleBinding) return err } - // if we might need to update, we need to stomp fields that are never going to match like uid and creation time - equivalentRoleBinding.SelfLink = rbacRoleBinding.SelfLink - equivalentRoleBinding.UID = rbacRoleBinding.UID - equivalentRoleBinding.ResourceVersion = rbacRoleBinding.ResourceVersion - equivalentRoleBinding.CreationTimestamp = rbacRoleBinding.CreationTimestamp - - // if they're equal, we have no work to do - if kapi.Semantic.DeepEqual(equivalentRoleBinding, rbacRoleBinding) { - return nil + // if they are not equal, we need to update + if PrepareForUpdateRoleBinding(equivalentRoleBinding, rbacRoleBinding) { + glog.V(1).Infof("writing RBAC rolebinding %v/%v", namespace, name) + _, err := c.rbacClient.RoleBindings(namespace).Update(equivalentRoleBinding) + // if the update was invalid, we're probably changing an immutable field or something like that + // either way, the existing object is wrong. Delete it and try again. + if apierrors.IsInvalid(err) { + c.rbacClient.RoleBindings(namespace).Delete(name, nil) // ignore delete error + } + return err } - glog.V(1).Infof("writing RBAC rolebinding %v/%v", namespace, name) - _, err = c.rbacClient.RoleBindings(namespace).Update(equivalentRoleBinding) - // if the update was invalid, we're probably changing an immutable field or something like that - // either way, the existing object is wrong. Delete it and try again. - if apierrors.IsInvalid(err) { - c.rbacClient.RoleBindings(namespace).Delete(name, nil) - } - return err + // they are equal so we have no work to do + return nil } func (c *OriginRoleBindingToRBACRoleBindingController) policyBindingEventHandler() cache.ResourceEventHandler { diff --git a/pkg/cmd/admin/migrate/authorization/authorization.go b/pkg/cmd/admin/migrate/authorization/authorization.go index 988ff37c956f..a82a58b71cfe 100644 --- a/pkg/cmd/admin/migrate/authorization/authorization.go +++ b/pkg/cmd/admin/migrate/authorization/authorization.go @@ -5,11 +5,9 @@ import ( "fmt" "io" - apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" utilerrors "k8s.io/apimachinery/pkg/util/errors" - "k8s.io/kubernetes/pkg/apis/rbac" rbacinternalversion "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/rbac/internalversion" kcmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" "k8s.io/kubernetes/pkg/kubectl/resource" @@ -130,8 +128,8 @@ func (o *MigrateAuthorizationOptions) checkClusterRole(originClusterRole *author var errlist []error // convert the origin role to a rbac role - convertedClusterRole := &rbac.ClusterRole{} - if err := authorizationapi.Convert_api_ClusterRole_To_rbac_ClusterRole(originClusterRole, convertedClusterRole, nil); err != nil { + convertedClusterRole, err := authorizationsync.ConvertToRBACClusterRole(originClusterRole) + if err != nil { errlist = append(errlist, err) } @@ -143,23 +141,8 @@ func (o *MigrateAuthorizationOptions) checkClusterRole(originClusterRole *author // compare the results if there have been no errors so far if len(errlist) == 0 { - // there's one wrinkle. If `openshift.io/reconcile-protect` is to true, then we must set rbac.authorization.kubernetes.io/autoupdate to false - if convertedClusterRole.Annotations["openshift.io/reconcile-protect"] == "true" { - convertedClusterRole.Annotations["rbac.authorization.kubernetes.io/autoupdate"] = "false" - delete(convertedClusterRole.Annotations, "openshift.io/reconcile-protect") - } - - // stomp fields that are never going to match like uid and creation time - convertedClusterRole.SelfLink = rbacClusterRole.SelfLink - convertedClusterRole.UID = rbacClusterRole.UID - convertedClusterRole.ResourceVersion = rbacClusterRole.ResourceVersion - convertedClusterRole.CreationTimestamp = rbacClusterRole.CreationTimestamp - - // normalize rules before comparing to match the controller's behavior - authorizationsync.NormalizePolicyRules(convertedClusterRole.Rules) - // if they are not equal, something has gone wrong and the two objects are not in sync - if !apiequality.Semantic.DeepEqual(convertedClusterRole, rbacClusterRole) { + if authorizationsync.PrepareForUpdateClusterRole(convertedClusterRole, rbacClusterRole) { errlist = append(errlist, errOutOfSync) } } @@ -171,8 +154,8 @@ func (o *MigrateAuthorizationOptions) checkRole(originRole *authorizationapi.Rol var errlist []error // convert the origin role to a rbac role - convertedRole := &rbac.Role{} - if err := authorizationapi.Convert_api_Role_To_rbac_Role(originRole, convertedRole, nil); err != nil { + convertedRole, err := authorizationsync.ConvertToRBACRole(originRole) + if err != nil { errlist = append(errlist, err) } @@ -184,17 +167,8 @@ func (o *MigrateAuthorizationOptions) checkRole(originRole *authorizationapi.Rol // compare the results if there have been no errors so far if len(errlist) == 0 { - // stomp fields that are never going to match like uid and creation time - convertedRole.SelfLink = rbacRole.SelfLink - convertedRole.UID = rbacRole.UID - convertedRole.ResourceVersion = rbacRole.ResourceVersion - convertedRole.CreationTimestamp = rbacRole.CreationTimestamp - - // normalize rules before comparing to match the controller's behavior - authorizationsync.NormalizePolicyRules(convertedRole.Rules) - // if they are not equal, something has gone wrong and the two objects are not in sync - if !apiequality.Semantic.DeepEqual(convertedRole, rbacRole) { + if authorizationsync.PrepareForUpdateRole(convertedRole, rbacRole) { errlist = append(errlist, errOutOfSync) } } @@ -206,8 +180,8 @@ func (o *MigrateAuthorizationOptions) checkClusterRoleBinding(originRoleBinding var errlist []error // convert the origin role binding to a rbac role binding - convertedRoleBinding := &rbac.ClusterRoleBinding{} - if err := authorizationapi.Convert_api_ClusterRoleBinding_To_rbac_ClusterRoleBinding(originRoleBinding, convertedRoleBinding, nil); err != nil { + convertedRoleBinding, err := authorizationsync.ConvertToRBACClusterRoleBinding(originRoleBinding) + if err != nil { errlist = append(errlist, err) } @@ -219,14 +193,8 @@ func (o *MigrateAuthorizationOptions) checkClusterRoleBinding(originRoleBinding // compare the results if there have been no errors so far if len(errlist) == 0 { - // stomp fields that are never going to match like uid and creation time - convertedRoleBinding.SelfLink = rbacRoleBinding.SelfLink - convertedRoleBinding.UID = rbacRoleBinding.UID - convertedRoleBinding.ResourceVersion = rbacRoleBinding.ResourceVersion - convertedRoleBinding.CreationTimestamp = rbacRoleBinding.CreationTimestamp - // if they are not equal, something has gone wrong and the two objects are not in sync - if !apiequality.Semantic.DeepEqual(convertedRoleBinding, rbacRoleBinding) { + if authorizationsync.PrepareForUpdateClusterRoleBinding(convertedRoleBinding, rbacRoleBinding) { errlist = append(errlist, errOutOfSync) } } @@ -238,8 +206,8 @@ func (o *MigrateAuthorizationOptions) checkRoleBinding(originRoleBinding *author var errlist []error // convert the origin role binding to a rbac role binding - convertedRoleBinding := &rbac.RoleBinding{} - if err := authorizationapi.Convert_api_RoleBinding_To_rbac_RoleBinding(originRoleBinding, convertedRoleBinding, nil); err != nil { + convertedRoleBinding, err := authorizationsync.ConvertToRBACRoleBinding(originRoleBinding) + if err != nil { errlist = append(errlist, err) } @@ -251,14 +219,8 @@ func (o *MigrateAuthorizationOptions) checkRoleBinding(originRoleBinding *author // compare the results if there have been no errors so far if len(errlist) == 0 { - // stomp fields that are never going to match like uid and creation time - convertedRoleBinding.SelfLink = rbacRoleBinding.SelfLink - convertedRoleBinding.UID = rbacRoleBinding.UID - convertedRoleBinding.ResourceVersion = rbacRoleBinding.ResourceVersion - convertedRoleBinding.CreationTimestamp = rbacRoleBinding.CreationTimestamp - // if they are not equal, something has gone wrong and the two objects are not in sync - if !apiequality.Semantic.DeepEqual(convertedRoleBinding, rbacRoleBinding) { + if authorizationsync.PrepareForUpdateRoleBinding(convertedRoleBinding, rbacRoleBinding) { errlist = append(errlist, errOutOfSync) } }