diff --git a/lib/resourcemerge/rbac.go b/lib/resourcemerge/rbac.go index 0d872038c..ccd9c43c0 100644 --- a/lib/resourcemerge/rbac.go +++ b/lib/resourcemerge/rbac.go @@ -9,6 +9,7 @@ import ( // modified is set to true when existing had to be updated with required. func EnsureClusterRoleBinding(modified *bool, existing *rbacv1.ClusterRoleBinding, required rbacv1.ClusterRoleBinding) { EnsureObjectMeta(modified, &existing.ObjectMeta, required.ObjectMeta) + ensureRoleRefDefaultsv1(&required.RoleRef) if !equality.Semantic.DeepEqual(existing.Subjects, required.Subjects) { *modified = true existing.Subjects = required.Subjects @@ -37,6 +38,7 @@ func EnsureClusterRole(modified *bool, existing *rbacv1.ClusterRole, required rb // modified is set to true when existing had to be updated with required. func EnsureRoleBinding(modified *bool, existing *rbacv1.RoleBinding, required rbacv1.RoleBinding) { EnsureObjectMeta(modified, &existing.ObjectMeta, required.ObjectMeta) + ensureRoleRefDefaultsv1(&required.RoleRef) if !equality.Semantic.DeepEqual(existing.Subjects, required.Subjects) { *modified = true existing.Subjects = required.Subjects @@ -47,6 +49,12 @@ func EnsureRoleBinding(modified *bool, existing *rbacv1.RoleBinding, required rb } } +func ensureRoleRefDefaultsv1(roleRef *rbacv1.RoleRef) { + if roleRef.APIGroup == "" { + roleRef.APIGroup = rbacv1.GroupName + } +} + // EnsureRole ensures that the existing matches the required. // modified is set to true when existing had to be updated with required. func EnsureRole(modified *bool, existing *rbacv1.Role, required rbacv1.Role) { diff --git a/lib/resourcemerge/rbac_test.go b/lib/resourcemerge/rbac_test.go new file mode 100644 index 000000000..57b5db615 --- /dev/null +++ b/lib/resourcemerge/rbac_test.go @@ -0,0 +1,278 @@ +package resourcemerge + +import ( + "testing" + + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/util/diff" + "k8s.io/utils/pointer" +) + +func TestEnsureClusterRoleBindingsv1(t *testing.T) { + tests := []struct { + name string + existing rbacv1.ClusterRoleBinding + input rbacv1.ClusterRoleBinding + + expectedModified bool + expected rbacv1.ClusterRoleBinding + }{ + { + name: "add subject", + existing: rbacv1.ClusterRoleBinding{ + Subjects: []rbacv1.Subject{}, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + input: rbacv1.ClusterRoleBinding{ + Subjects: []rbacv1.Subject{ + { + Name: "foo", + Namespace: "bar", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + expectedModified: true, + expected: rbacv1.ClusterRoleBinding{ + Subjects: []rbacv1.Subject{ + { + Name: "foo", + Namespace: "bar", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + }, + { + name: "remove subject", + existing: rbacv1.ClusterRoleBinding{ + Subjects: []rbacv1.Subject{ + { + Name: "foo", + Namespace: "bar", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + input: rbacv1.ClusterRoleBinding{ + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + expectedModified: true, + expected: rbacv1.ClusterRoleBinding{ + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + }, + { + name: "replace subject", + existing: rbacv1.ClusterRoleBinding{ + Subjects: []rbacv1.Subject{ + { + Name: "foo", + Namespace: "bar", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + input: rbacv1.ClusterRoleBinding{ + Subjects: []rbacv1.Subject{ + { + Name: "cake", + Namespace: "pie", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + expectedModified: true, + expected: rbacv1.ClusterRoleBinding{ + Subjects: []rbacv1.Subject{ + { + Name: "cake", + Namespace: "pie", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + }, + { + name: "same subject", + existing: rbacv1.ClusterRoleBinding{ + Subjects: []rbacv1.Subject{ + { + Name: "foo", + Namespace: "bar", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + input: rbacv1.ClusterRoleBinding{ + Subjects: []rbacv1.Subject{ + { + Name: "foo", + Namespace: "bar", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + expectedModified: false, + expected: rbacv1.ClusterRoleBinding{ + Subjects: []rbacv1.Subject{ + { + Name: "foo", + Namespace: "bar", + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + }, + { + name: "add roleref", + existing: rbacv1.ClusterRoleBinding{}, + input: rbacv1.ClusterRoleBinding{ + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + expectedModified: true, + expected: rbacv1.ClusterRoleBinding{ + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + }, + { + name: "add roleref (empty apigroup)", + existing: rbacv1.ClusterRoleBinding{}, + input: rbacv1.ClusterRoleBinding{ + RoleRef: rbacv1.RoleRef{ + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + expectedModified: true, + expected: rbacv1.ClusterRoleBinding{ + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + }, + { + name: "replace roleref", + existing: rbacv1.ClusterRoleBinding{ + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + input: rbacv1.ClusterRoleBinding{ + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-cake", + }, + }, + expectedModified: true, + expected: rbacv1.ClusterRoleBinding{ + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-cake", + }, + }, + }, + { + name: "same roleref", + existing: rbacv1.ClusterRoleBinding{ + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + input: rbacv1.ClusterRoleBinding{ + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + expectedModified: false, + expected: rbacv1.ClusterRoleBinding{ + RoleRef: rbacv1.RoleRef{ + APIGroup: "rbac.authorization.k8s.io", + Kind: "ClusterRole", + Name: "role-baz", + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + modified := pointer.BoolPtr(false) + EnsureClusterRoleBinding(modified, &test.existing, test.input) + if *modified != test.expectedModified { + t.Errorf("mismatch modified got: %v want: %v", *modified, test.expectedModified) + } + + if !equality.Semantic.DeepEqual(test.existing, test.expected) { + t.Errorf("unexpected: %s", diff.ObjectReflectDiff(test.expected, test.existing)) + } + }) + } +}