diff --git a/ssa/manager_apply_test.go b/ssa/manager_apply_test.go index e8f48e9b..598f395f 100644 --- a/ssa/manager_apply_test.go +++ b/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 + } + } + }) } diff --git a/ssa/manager_diff.go b/ssa/manager_diff.go index dea46f6f..6a457639 100644 --- a/ssa/manager_diff.go +++ b/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) } diff --git a/ssa/utils.go b/ssa/utils.go index 3bb1281a..1fe35393 100644 --- a/ssa/utils.go +++ b/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 }