refactor

controllers/nstemplateset/cluster_resources.go

@@ -2,8 +2,6 @@ package nstemplateset
 
 import (
 	"context"
-	"fmt"
-
 	toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
 	applycl "github.com/codeready-toolchain/toolchain-common/pkg/client"
 	"github.com/codeready-toolchain/toolchain-common/pkg/template"
@@ -297,17 +295,6 @@ NewObjects:
 	return false, nil
 }
 
-// TODO Move to common
-// FeatureToggleAnnotationKey generates an annotation key for the feature name.
-// This key can be used in Space, NSTemplateSet, etc. CRs to indicate that the corresponding feature toggle should be enabled.
-// This is the format of such keys: toolchain.dev.openshift.com/feature/<featureName>
-func FeatureToggleAnnotationKey(featureName string) string {
-	return fmt.Sprintf("%s%s", FeatureAnnotationKeyPrefix, featureName)
-}
-
-// TODO Move to api
-const FeatureAnnotationKeyPrefix = toolchainv1alpha1.FeatureToggleNameAnnotationKey + "/"
-
 // delete deletes one cluster scoped resource owned by the user and returns 'true, nil'. If no cluster-scoped resource owned
 // by the user is found, then it returns 'false, nil'. In case of any errors 'false, error'
 func (r *clusterResourcesManager) delete(ctx context.Context, nsTmplSet *toolchainv1alpha1.NSTemplateSet) (bool, error) {

controllers/nstemplateset/features.go

@@ -3,30 +3,28 @@ package nstemplateset
 import (
 	toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
 	runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"strings"
 )
 
-// shouldCreate checks if the object has any feature toggle annotations. If it does then check if the corresponding
-// annotations are present in the NSTemplateSet. Returns true if the annotations match. It means this feature
-// should be enabled and the object should be created. It also returns true if the object doesn't have any feature annotation at all
-// which means it's a regular object and should be always created.
+// shouldCreate checks if the object has a feature toggle annotation. If it does then check if the corresponding
+// feature is referenced in the NSTemplateSet feature annotation. Returns true if yes. It means this feature
+// should be enabled and the object should be created. It also returns true if the object doesn't have a feature annotation at all
+// which means it's a regular object, and it's not managed by any feature toggle and should be always created.
 // Otherwise, returns false.
 func shouldCreate(toCreate runtimeclient.Object, nsTmplSet *toolchainv1alpha1.NSTemplateSet) bool {
-	var featureToggleObject bool
-	for objK, objV := range toCreate.GetAnnotations() {
-		if objK == toolchainv1alpha1.FeatureToggleNameAnnotationKey {
-			// This is a feature annotation in the object we are considering to create
-			// Let's generate an annotation key for the NSTemplateSet for this feature
-			expectedFeatureAnnotationKey := FeatureToggleAnnotationKey(objV)
-			// Now let's check if there is any annotation with such key in the NSTemplateSet
-			for nsK, _ := range nsTmplSet.Annotations {
-				if nsK == expectedFeatureAnnotationKey {
-					return true // Doesn't matter what value this annotation has. It only matters that the annotation is present.
-				}
-			}
-			featureToggleObject = true // There can be multiple feature annotations in the object. So don't give up and check all annotations.
+	feature, found := toCreate.GetAnnotations()[toolchainv1alpha1.FeatureToggleNameAnnotationKey]
+	if !found {
+		return true // This object is a regular object and not managed by a feature toggle. Always create it.
+	}
+	// This object represents a feature. Let's check if this feature is among winners in the NSTemplateSet.
+	winners, found := nsTmplSet.GetAnnotations()[toolchainv1alpha1.FeatureToggleNameAnnotationKey]
+	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
 		}
 	}
-	// if there was at least one feature annotations in the object and none of them present in the NSTemplateSet
-	// then we don't want to create this object. If there is no feature annotation at all then we do want to create the object.
-	return !featureToggleObject
+	return false
 }