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 <[email protected]>
openshift · Jun 10, 2017 · 171b08c · 171b08c
1 parent 7867f03
commit 171b08c
Show file tree

Hide file tree

Showing 6 changed files with 220 additions and 178 deletions.
diff --git 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
@@ -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/...uthorization/controller/authorizationsync/origin_to_rbac_clusterrolebinding_controller.go b/...uthorization/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
@@ -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 {