diff --git a/controllers/nstemplateset/features.go b/controllers/nstemplateset/features.go index 851d3b40..c82656c9 100644 --- a/controllers/nstemplateset/features.go +++ b/controllers/nstemplateset/features.go @@ -2,6 +2,7 @@ package nstemplateset import ( toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" + "k8s.io/utils/strings/slices" runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" "strings" ) @@ -21,10 +22,15 @@ func shouldCreate(toCreate runtimeclient.Object, nsTmplSet *toolchainv1alpha1.NS if !found { return false // No feature winners in the NSTemplateSet at all. Skip this object. } - for _, winner := range strings.Split(winners, ",") { - if winner == feature { - return true - } + return slices.Contains(reallySplit(winners, ","), feature) +} + +// reallySplit acts exactly the same as strings.Split() but returns an empty slice for empty strings. +// To be used when, for example, we want to get an empty slice for empty comma separated list: +// strings.Split("", ",") returns [""] while reallySplit("", ",") returns [] +func reallySplit(s, sep string) []string { + if len(s) == 0 { + return []string{} } - return false + return strings.Split(s, sep) } diff --git a/controllers/nstemplateset/features_test.go b/controllers/nstemplateset/features_test.go index eddf77f3..4e5693d9 100644 --- a/controllers/nstemplateset/features_test.go +++ b/controllers/nstemplateset/features_test.go @@ -2,6 +2,7 @@ package nstemplateset import ( runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" + "strings" "testing" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" @@ -12,38 +13,68 @@ func TestShouldCreate(t *testing.T) { // given tests := []struct { name string - objFeature string - nsTemplateSetFeatures string + objFeature *string + nsTemplateSetFeatures *string expectedToBeCreated bool }{ + { + name: "object with an empty feature annotation should not be created when no features are enabled in NSTemplateSet", + objFeature: p(""), + nsTemplateSetFeatures: nil, + expectedToBeCreated: false, + }, + { + name: "object with an empty feature annotation should not be created when NSTemplateSet has an empty feature annotation", + objFeature: p(""), + nsTemplateSetFeatures: p(""), + expectedToBeCreated: false, + }, + { + name: "object with no feature annotation should be created when NSTemplateSet has an empty feature annotation", + objFeature: nil, + nsTemplateSetFeatures: p(""), + expectedToBeCreated: true, + }, + { + name: "object with an empty feature annotation should not be created when NSTemplateSet has some features enabled", + objFeature: p(""), + nsTemplateSetFeatures: p("feature-1"), + expectedToBeCreated: false, + }, + { + name: "object with a feature should not be created when NSTemplateSet has an empty feature annotation", + objFeature: p("feature-1"), + nsTemplateSetFeatures: p(""), + expectedToBeCreated: false, + }, { name: "object with no feature annotation should be created when no features are enabled in NSTemplateSet", - objFeature: "", - nsTemplateSetFeatures: "", + objFeature: nil, + nsTemplateSetFeatures: nil, expectedToBeCreated: true, }, { name: "object with no feature annotation should be created when some features are enabled in NSTemplateSet", - objFeature: "", - nsTemplateSetFeatures: "feature-1,feature-2", + objFeature: nil, + nsTemplateSetFeatures: p("feature-1,feature-2"), expectedToBeCreated: true, }, { name: "object with a feature annotation should not be created when no features are enabled in NSTemplateSet", - objFeature: "feature-1", - nsTemplateSetFeatures: "", + objFeature: p("feature-1"), + nsTemplateSetFeatures: nil, expectedToBeCreated: false, }, { name: "object with a feature annotation should not be created when different features are enabled in NSTemplateSet", - objFeature: "feature-1", - nsTemplateSetFeatures: "feature-2", + objFeature: p("feature-1"), + nsTemplateSetFeatures: p("feature-2"), expectedToBeCreated: false, }, { name: "object with a feature which is among enabled features should be created", - objFeature: "feature-2", - nsTemplateSetFeatures: "feature-1,feature-2,feature-3", + objFeature: p("feature-2"), + nsTemplateSetFeatures: p("feature-1,feature-2,feature-3"), expectedToBeCreated: true, }, } @@ -52,9 +83,9 @@ func TestShouldCreate(t *testing.T) { // given obj := objectWithFeature(testRun.objFeature) nsTmplSet := newNSTmplSet("na", "na", "na") - if testRun.nsTemplateSetFeatures != "" { + if testRun.nsTemplateSetFeatures != nil { nsTmplSet.Annotations = map[string]string{ - toolchainv1alpha1.FeatureToggleNameAnnotationKey: testRun.nsTemplateSetFeatures, + toolchainv1alpha1.FeatureToggleNameAnnotationKey: *testRun.nsTemplateSetFeatures, } } @@ -67,11 +98,21 @@ func TestShouldCreate(t *testing.T) { } } -func objectWithFeature(feature string) runtimeclient.Object { - obj := newRoleBinding("my-namespace", "rb-"+feature, "my-space") - if feature != "" { +func TestReallySplit(t *testing.T) { + assert.Empty(t, reallySplit("", ",")) + assert.Equal(t, strings.Split("1,2,3", ","), reallySplit("1,2,3", ",")) + assert.Equal(t, strings.Split("1", ","), reallySplit("1", ",")) +} + +func p(s string) *string { + return &s +} + +func objectWithFeature(feature *string) runtimeclient.Object { + obj := newRoleBinding("my-namespace", "rb", "my-space") + if feature != nil { obj.Annotations = map[string]string{ - toolchainv1alpha1.FeatureToggleNameAnnotationKey: feature, + toolchainv1alpha1.FeatureToggleNameAnnotationKey: *feature, } } return obj diff --git a/controllers/nstemplateset/namespaces.go b/controllers/nstemplateset/namespaces.go index 29b54ac5..6fef3d3b 100644 --- a/controllers/nstemplateset/namespaces.go +++ b/controllers/nstemplateset/namespaces.go @@ -3,11 +3,9 @@ package nstemplateset import ( "context" "fmt" + rbac "k8s.io/api/rbac/v1" "k8s.io/utils/strings/slices" "sort" - "strings" - - rbac "k8s.io/api/rbac/v1" toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" "github.com/codeready-toolchain/toolchain-common/pkg/configuration" @@ -198,7 +196,7 @@ func (r *namespacesManager) ensureInnerNamespaceResources(ctx context.Context, n // Now check if there is any featured object to be deleted in case the feature annotation does not present in the NSTemplateSet anymore toDeleteObsoleteFeaturedObjects := false featureStr := nsTmplSet.Annotations[toolchainv1alpha1.FeatureToggleNameAnnotationKey] - features := strings.Split(featureStr, ",") + features := reallySplit(featureStr, ",") for _, obj := range newObjs { objFeature, f := obj.GetAnnotations()[toolchainv1alpha1.FeatureToggleNameAnnotationKey] if f && !slices.Contains(features, objFeature) {