really split

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)
 }

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

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) {