diff --git a/controllers/nstemplateset/namespaces.go b/controllers/nstemplateset/namespaces.go index fd877f75..28a936f6 100644 --- a/controllers/nstemplateset/namespaces.go +++ b/controllers/nstemplateset/namespaces.go @@ -428,13 +428,15 @@ func (r *namespacesManager) isUpToDateAndProvisioned(ctx context.Context, ns *co // check the names of the roles and roleBindings as well // ignore featured objects if the corresponding feature is not enabled in the NSTemplateSet for _, role := range processedRoles { - if found, err := r.containsRole(roleList.Items, role, spacename); (!found && shouldCreate(role, nsTmplSet)) || err != nil { + // It's NOT up-to-date if NOT found but should be created OR found but should NOT be created. + if found, err := r.containsRole(roleList.Items, role, spacename); (found != shouldCreate(role, nsTmplSet)) || err != nil { return false, err } } for _, rolebinding := range processedRoleBindings { - if found, err := r.containsRoleBindings(rolebindingList.Items, rolebinding, spacename); (!found && shouldCreate(rolebinding, nsTmplSet)) || err != nil { + // It's NOT up-to-date if NOT found but should be created OR found but should NOT be created. + if found, err := r.containsRoleBindings(rolebindingList.Items, rolebinding, spacename); (found != shouldCreate(rolebinding, nsTmplSet)) || err != nil { return false, err } } diff --git a/controllers/nstemplateset/namespaces_test.go b/controllers/nstemplateset/namespaces_test.go index 5c106132..e6610e6f 100644 --- a/controllers/nstemplateset/namespaces_test.go +++ b/controllers/nstemplateset/namespaces_test.go @@ -1297,7 +1297,7 @@ func TestUpdateNamespaces(t *testing.T) { t.Run("failure", func(t *testing.T) { - t.Run("update to abcde15 fails because it find the new template", func(t *testing.T) { + t.Run("update to abcde15 fails because it finds the new template", func(t *testing.T) { // given nsTmplSet := newNSTmplSet(namespaceName, spacename, "basic", withNamespaces("abcde15", "dev")) devNS := newNamespace("basic", spacename, "dev", withTemplateRefUsingRevision("abcde11")) @@ -1321,7 +1321,7 @@ func TestUpdateNamespaces(t *testing.T) { HasLabel(toolchainv1alpha1.TierLabelKey, "basic") }) - t.Run("update from abcde15 fails because it find current template", func(t *testing.T) { + t.Run("update from abcde15 fails because it finds current template", func(t *testing.T) { // given nsTmplSet := newNSTmplSet(namespaceName, spacename, "basic", withNamespaces("abcde11", "dev")) devNS := newNamespace("basic", spacename, "dev", withTemplateRefUsingRevision("abcde15")) @@ -1347,7 +1347,7 @@ func TestUpdateNamespaces(t *testing.T) { }) } -func TestDisablingFeatureInNSTemplateSet(t *testing.T) { +func TestDisablingAndEnablingFeatureInNSTemplateSet(t *testing.T) { // given logger := zap.New(zap.UseDevMode(true)) @@ -1359,27 +1359,51 @@ func TestDisablingFeatureInNSTemplateSet(t *testing.T) { restore := test.SetEnvVarAndRestore(t, commonconfig.WatchNamespaceEnvVar, "my-member-operator-namespace") t.Cleanup(restore) - nsTmplSet := newNSTmplSet(namespaceName, spacename, "advanced", withNamespaces("abcde11", "dev")) devNS := newNamespace("advanced", spacename, "dev", withTemplateRefUsingRevision("abcde11")) - featuredRB := newRoleBinding(devNS.Name, "feature-1-rb", spacename) // existing featured RB we need to make sure gets deleted - featuredRB.Annotations = map[string]string{ - toolchainv1alpha1.FeatureToggleNameAnnotationKey: "feature-1-rb", - } - manager, cl := prepareNamespacesManager(t, nsTmplSet, devNS, featuredRB) - - // when - updated, err := manager.ensure(ctx, nsTmplSet) - - // then - require.NoError(t, err) - assert.True(t, updated) - AssertThatNSTemplateSet(t, namespaceName, spacename, cl). - HasFinalizer(). - HasConditions(Updating()) - AssertThatNamespace(t, spacename+"-dev", cl). - HasResource("crtadmin-pods", &rbacv1.RoleBinding{}). - HasResource("crtadmin-view", &rbacv1.RoleBinding{}). - HasNoResource(featuredRB.Name, &rbacv1.RoleBinding{}) // The featured object is gone + ro := newRole(devNS.Name, "exec-pods", spacename) + rb := newRoleBinding(devNS.Name, "crtadmin-pods", spacename) + rb2 := newRoleBinding(devNS.Name, "crtadmin-view", spacename) + + t.Run("disable a feature for already provisioned NSTemplateSet", func(t *testing.T) { + nsTmplSet := newNSTmplSet(namespaceName, spacename, "advanced", withNamespaces("abcde11", "dev")) + featuredRB := newRoleBinding(devNS.Name, "feature-1-rb", spacename) // existing featured RB we need to make sure gets deleted + featuredRB.Annotations = map[string]string{ + toolchainv1alpha1.FeatureToggleNameAnnotationKey: "feature-1-rb", + } + manager, cl := prepareNamespacesManager(t, nsTmplSet, devNS, ro, rb, rb2, featuredRB) + + // when + updated, err := manager.ensure(ctx, nsTmplSet) + + // then + require.NoError(t, err) + assert.True(t, updated) + AssertThatNSTemplateSet(t, namespaceName, spacename, cl). + HasFinalizer(). + HasConditions(Updating()) + AssertThatNamespace(t, spacename+"-dev", cl). + HasResource("crtadmin-pods", &rbacv1.RoleBinding{}). + HasResource("crtadmin-view", &rbacv1.RoleBinding{}). + HasNoResource(featuredRB.Name, &rbacv1.RoleBinding{}) // The featured object is gone + }) + t.Run("enable a feature for already provisioned NSTemplateSet", func(t *testing.T) { + nsTmplSet := newNSTmplSet(namespaceName, spacename, "advanced", withNamespaces("abcde11", "dev"), withNSTemplateSetFeatureAnnotation("feature-1")) + manager, cl := prepareNamespacesManager(t, nsTmplSet, devNS, ro, rb, rb2) + + // when + updated, err := manager.ensure(ctx, nsTmplSet) + + // then + require.NoError(t, err) + assert.True(t, updated) + AssertThatNSTemplateSet(t, namespaceName, spacename, cl). + HasFinalizer(). + HasConditions(Updating()) + AssertThatNamespace(t, spacename+"-dev", cl). + HasResource("crtadmin-pods", &rbacv1.RoleBinding{}). + HasResource("crtadmin-view", &rbacv1.RoleBinding{}). + HasResource("feature-1-rb", &rbacv1.RoleBinding{}) // The featured object is created now + }) } func TestIsUpToDateAndProvisioned(t *testing.T) {