Merge pull request #158 from fluxcd/rbac-diff

ssa: Improve RBAC reconciliation

ssa/manager_apply_test.go

@@ -45,6 +45,7 @@ func TestApply(t *testing.T) {
 
 	configMapName, configMap := getFirstObject(objects, "ConfigMap", id)
 	secretName, secret := getFirstObject(objects, "Secret", id)
+	crbName, crb := getFirstObject(objects, "ClusterRoleBinding", id)
 
 	t.Run("creates objects in order", func(t *testing.T) {
 		// create objects
@@ -195,4 +196,28 @@ func TestApply(t *testing.T) {
 			t.Errorf("Mismatch from expected value (-want +got):\n%s", diff)
 		}
 	})
+
+	t.Run("recreates immutable RBAC", func(t *testing.T) {
+		// update roleRef
+		err = unstructured.SetNestedField(crb.Object, "test", "roleRef", "name")
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// force apply
+		changeSet, err := manager.ApplyAllStaged(ctx, objects, true, timeout)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// verify the binding was recreated
+		for _, entry := range changeSet.Entries {
+			if entry.Subject == crbName {
+				if diff := cmp.Diff(string(CreatedAction), entry.Action); diff != "" {
+					t.Errorf("Mismatch from expected value (-want +got):\n%s", diff)
+				}
+				break
+			}
+		}
+	})
 }

ssa/manager_diff.go

@@ -90,15 +90,17 @@ func (m *ResourceManager) hasDrifted(existingObject, dryRunObject *unstructured.
 		return true
 	}
 
-	if _, ok := existingObject.Object["spec"]; ok {
-		if !apiequality.Semantic.DeepDerivative(dryRunObject.Object["spec"], existingObject.Object["spec"]) {
-			return true
+	var found bool
+	for _, field := range []string{"spec", "webhooks", "rules", "subjects", "roleRef", "subsets"} {
+		if _, ok := existingObject.Object[field]; ok {
+			found = true
 		}
-	} else if _, ok := existingObject.Object["webhooks"]; ok {
-		if !apiequality.Semantic.DeepDerivative(dryRunObject.Object["webhooks"], existingObject.Object["webhooks"]) {
+		if hasFieldDrifted(existingObject, dryRunObject, field) {
 			return true
 		}
-	} else {
+	}
+
+	if !found {
 		if !apiequality.Semantic.DeepDerivative(dryRunObject.Object, existingObject.Object) {
 			return true
 		}
@@ -107,6 +109,13 @@ func (m *ResourceManager) hasDrifted(existingObject, dryRunObject *unstructured.
 	return false
 }
 
+func hasFieldDrifted(existingObject, dryRunObject *unstructured.Unstructured, field string) bool {
+	if _, ok := existingObject.Object[field]; ok {
+		return !apiequality.Semantic.DeepDerivative(dryRunObject.Object[field], existingObject.Object[field])
+	}
+	return false
+}
+
 // validationError formats the given error and hides sensitive data
 // if the error was caused by an invalid Kubernetes secrets.
 func (m *ResourceManager) validationError(object *unstructured.Unstructured, err error) error {
@@ -129,7 +138,11 @@ func (m *ResourceManager) validationError(object *unstructured.Unstructured, err
 		reason = fmt.Sprintf("%v", status.Type)
 	}
 
-	return fmt.Errorf("%s dry-run falied, reason: %s, error: %w",
+	if reason != "" {
+		reason = fmt.Sprintf(", reason: %s", reason)
+	}
+
+	return fmt.Errorf("%s dry-run failed%s, error: %w",
 		FmtUnstructured(object), reason, err)
 
 }

ssa/utils.go

@@ -201,5 +201,10 @@ func IsKustomization(object *unstructured.Unstructured) bool {
 }
 
 func isImmutableError(err error) bool {
-	return strings.Contains(err.Error(), "field is immutable")
+	for _, s := range []string{"field is immutable", "cannot change roleRef"} {
+		if strings.Contains(err.Error(), s) {
+			return true
+		}
+	}
+	return false
 }