Allow manifest selection based on feature gates

pkg/manifest/manifest.go

@@ -26,6 +26,7 @@ const (
 	CapabilityAnnotation  = "capability.openshift.io/name"
 	DefaultClusterProfile = "self-managed-high-availability"
 	featureSetAnnotation  = "release.openshift.io/feature-set"
+	featureGateAnnotation = "release.openshift.io/feature-gate"
 )
 
 var knownFeatureSets = sets.Set[string]{}
@@ -171,6 +172,16 @@ func getFeatureSets(annotations map[string]string) (sets.Set[string], bool, erro
 	return ret, specified, nil
 }
 
+func hasFeatureSetAnnotation(annotations map[string]string) bool {
+	_, ok := annotations[featureSetAnnotation]
+	return ok
+}
+
+func hasFeatureGateAnnotation(annotations map[string]string) bool {
+	_, ok := annotations[featureGateAnnotation]
+	return ok
+}
+
 func checkFeatureSets(requiredFeatureSet string, annotations map[string]string) error {
 	requiredAnnotationValue := requiredFeatureSet
 	if len(requiredFeatureSet) == 0 {
@@ -187,12 +198,46 @@ func checkFeatureSets(requiredFeatureSet string, annotations map[string]string)
 	return nil
 }
 
+// checkFeatureGates validates if manifest should be included based on feature gate requirements
+func checkFeatureGates(enabledGates sets.Set[string], annotations map[string]string) error {
+	if annotations == nil {
+		return nil // No annotations, include by default
+	}
+	gateRequirements, ok := annotations[featureGateAnnotation]
+	if !ok {
+		return nil // No requirements, include by default
+	}
+
+	requirements := strings.Split(gateRequirements, ",")
+	for _, req := range requirements {
+		req = strings.TrimSpace(req)
+		if req == "" {
+			continue
+		}
+
+		if strings.HasPrefix(req, "-") {
+			// Exclusion: gate must NOT be enabled
+			gate := req[1:]
+			if enabledGates.Has(gate) {
+				return fmt.Errorf("feature gate %s is enabled but manifest requires it to be disabled", gate)
+			}
+		} else {
+			// Inclusion: gate must be enabled
+			if !enabledGates.Has(req) {
+				return fmt.Errorf("feature gate %s is required but not enabled", req)
+			}
+		}
+	}
+
+	return nil
+}
+
 // Include returns an error if the manifest fails an inclusion filter and should be excluded from further
 // processing by cluster version operator. Pointer arguments can be set nil to avoid excluding based on that
 // filter. For example, setting profile non-nil and capabilities nil will return an error if the manifest's
 // profile does not match, but will never return an error about capability issues.
-func (m *Manifest) Include(excludeIdentifier *string, requiredFeatureSet *string, profile *string, capabilities *configv1.ClusterVersionCapabilitiesStatus, overrides []configv1.ComponentOverride) error {
-	return m.IncludeAllowUnknownCapabilities(excludeIdentifier, requiredFeatureSet, profile, capabilities, overrides, false)
+func (m *Manifest) Include(excludeIdentifier *string, requiredFeatureSet *string, profile *string, capabilities *configv1.ClusterVersionCapabilitiesStatus, overrides []configv1.ComponentOverride, enabledFeatureGates sets.Set[string]) error {
+	return m.IncludeAllowUnknownCapabilities(excludeIdentifier, requiredFeatureSet, profile, capabilities, overrides, enabledFeatureGates, false)
 }
 
 // IncludeAllowUnknownCapabilities returns an error if the manifest fails an inclusion filter and should be excluded from
@@ -202,7 +247,7 @@ func (m *Manifest) Include(excludeIdentifier *string, requiredFeatureSet *string
 // to capabilities filtering. When set to true a manifest will not be excluded simply because it contains an unknown
 // capability. This is necessary to allow updates to an OCP version containing newly defined capabilities.
 func (m *Manifest) IncludeAllowUnknownCapabilities(excludeIdentifier *string, requiredFeatureSet *string, profile *string,
-	capabilities *configv1.ClusterVersionCapabilitiesStatus, overrides []configv1.ComponentOverride, allowUnknownCapabilities bool) error {
+	capabilities *configv1.ClusterVersionCapabilitiesStatus, overrides []configv1.ComponentOverride, enabledFeatureGates sets.Set[string], allowUnknownCapabilities bool) error {
 
 	annotations := m.Obj.GetAnnotations()
 	if annotations == nil {
@@ -216,13 +261,27 @@ func (m *Manifest) IncludeAllowUnknownCapabilities(excludeIdentifier *string, re
 		}
 	}
 
+	if requiredFeatureSet != nil && enabledFeatureGates != nil {
+		if hasFeatureSetAnnotation(annotations) && hasFeatureGateAnnotation(annotations) {
+			return fmt.Errorf("both feature set and feature gate annotations are present: manifests may specify either a feature set or a feature gate, but not both")
+		}
+	}
+
 	if requiredFeatureSet != nil {
 		err := checkFeatureSets(*requiredFeatureSet, annotations)
 		if err != nil {
 			return err
 		}
 	}
 
+	// Feature gate filtering
+	if enabledFeatureGates != nil {
+		err := checkFeatureGates(enabledFeatureGates, annotations)
+		if err != nil {
+			return err
+		}
+	}
+
 	if profile != nil {
 		profileAnnotation := fmt.Sprintf("include.release.openshift.io/%s", *profile)
 		if val, ok := annotations[profileAnnotation]; ok && val != "true" {

pkg/manifest/manifest_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/klog/v2"
 )
 
@@ -599,13 +600,14 @@ func Test_include(t *testing.T) {
 	singleNodeProfile := "single-node"
 
 	tests := []struct {
-		name               string
-		exclude            *string
-		requiredFeatureSet *string
-		profile            *string
-		annotations        map[string]interface{}
-		caps               *configv1.ClusterVersionCapabilitiesStatus
-		overrides          []configv1.ComponentOverride
+		name                string
+		exclude             *string
+		requiredFeatureSet  *string
+		profile             *string
+		annotations         map[string]interface{}
+		caps                *configv1.ClusterVersionCapabilitiesStatus
+		overrides           []configv1.ComponentOverride
+		enabledFeatureGates sets.Set[string]
 
 		expected error
 	}{
@@ -809,6 +811,39 @@ func Test_include(t *testing.T) {
 			},
 			expected: fmt.Errorf("overridden"),
 		},
+		{
+			name:                "feature gate required but not enabled",
+			enabledFeatureGates: sets.New("FeatureGate1"),
+			annotations: map[string]interface{}{
+				"release.openshift.io/feature-gate": "FeatureGate2",
+			},
+			expected: fmt.Errorf("feature gate FeatureGate2 is required but not enabled"),
+		},
+		{
+			name:                "feature gate enabled but manifest requires it to be disabled",
+			enabledFeatureGates: sets.New("FeatureGate1"),
+			annotations: map[string]interface{}{
+				"release.openshift.io/feature-gate": "-FeatureGate1",
+			},
+			expected: fmt.Errorf("feature gate FeatureGate1 is enabled but manifest requires it to be disabled"),
+		},
+		{
+			name:                "feature gate required and is enabled",
+			enabledFeatureGates: sets.New("FeatureGate1"),
+			annotations: map[string]interface{}{
+				"release.openshift.io/feature-gate": "FeatureGate1",
+			},
+		},
+		{
+			name:                "feature set required and feature gate specified",
+			requiredFeatureSet:  ptr.To("FeatureSet1"),
+			enabledFeatureGates: sets.New("FeatureGate1"),
+			annotations: map[string]interface{}{
+				"release.openshift.io/feature-set":  "FeatureSet1",
+				"release.openshift.io/feature-gate": "FeatureGate1",
+			},
+			expected: fmt.Errorf("both feature set and feature gate annotations are present: manifests may specify either a feature set or a feature gate, but not both"),
+		},
 		{
 			name:        "all nil",
 			profile:     nil,
@@ -838,7 +873,7 @@ func Test_include(t *testing.T) {
 			err := m.populateFromObj()
 			assert.Equal(t, nil, err)
 
-			err = m.Include(tt.exclude, tt.requiredFeatureSet, tt.profile, tt.caps, tt.overrides)
+			err = m.Include(tt.exclude, tt.requiredFeatureSet, tt.profile, tt.caps, tt.overrides, tt.enabledFeatureGates)
 			assert.Equal(t, tt.expected, err)
 		})
 	}
@@ -847,14 +882,15 @@ func Test_include(t *testing.T) {
 func TestIncludeAllowUnknownCapabilities(t *testing.T) {
 
 	tests := []struct {
-		name               string
-		exclude            *string
-		requiredFeatureSet *string
-		profile            *string
-		annotations        map[string]interface{}
-		caps               *configv1.ClusterVersionCapabilitiesStatus
-		overrides          []configv1.ComponentOverride
-		allowUnknown       bool
+		name                string
+		exclude             *string
+		requiredFeatureSet  *string
+		profile             *string
+		annotations         map[string]interface{}
+		caps                *configv1.ClusterVersionCapabilitiesStatus
+		overrides           []configv1.ComponentOverride
+		enabledFeatureGates sets.Set[string]
+		allowUnknown        bool
 
 		expected error
 	}{
@@ -897,7 +933,7 @@ func TestIncludeAllowUnknownCapabilities(t *testing.T) {
 					},
 				},
 			}
-			err := m.IncludeAllowUnknownCapabilities(tt.exclude, tt.requiredFeatureSet, tt.profile, tt.caps, tt.overrides, tt.allowUnknown)
+			err := m.IncludeAllowUnknownCapabilities(tt.exclude, tt.requiredFeatureSet, tt.profile, tt.caps, tt.overrides, tt.enabledFeatureGates, tt.allowUnknown)
 			assert.Equal(t, tt.expected, err)
 		})
 	}
@@ -1009,3 +1045,169 @@ func TestSameResourceID(t *testing.T) {
 		})
 	}
 }
+
+func TestCheckFeatureGates(t *testing.T) {
+	tests := []struct {
+		name         string
+		enabledGates sets.Set[string]
+		annotations  map[string]string
+		expectedErr  string
+	}{
+		{
+			name:         "no feature gate annotation",
+			enabledGates: sets.New("gate1", "gate2"),
+			annotations:  map[string]string{},
+			expectedErr:  "",
+		},
+		{
+			name:         "empty feature gate annotation",
+			enabledGates: sets.New("gate1", "gate2"),
+			annotations:  map[string]string{featureGateAnnotation: ""},
+			expectedErr:  "",
+		},
+		{
+			name:         "required gate is enabled - should include",
+			enabledGates: sets.New("TechPreviewFeatureGate", "gate2"),
+			annotations:  map[string]string{featureGateAnnotation: "TechPreviewFeatureGate"},
+			expectedErr:  "",
+		},
+		{
+			name:         "required gate is not enabled - should exclude",
+			enabledGates: sets.New("gate1", "gate2"),
+			annotations:  map[string]string{featureGateAnnotation: "TechPreviewFeatureGate"},
+			expectedErr:  "feature gate TechPreviewFeatureGate is required but not enabled",
+		},
+		{
+			name:         "excluded gate is not enabled - should include",
+			enabledGates: sets.New("gate1", "gate2"),
+			annotations:  map[string]string{featureGateAnnotation: "-TechPreviewFeatureGate"},
+			expectedErr:  "",
+		},
+		{
+			name:         "excluded gate is enabled - should exclude",
+			enabledGates: sets.New("TechPreviewFeatureGate", "gate2"),
+			annotations:  map[string]string{featureGateAnnotation: "-TechPreviewFeatureGate"},
+			expectedErr:  "feature gate TechPreviewFeatureGate is enabled but manifest requires it to be disabled",
+		},
+		{
+			name:         "multiple requirements - all satisfied",
+			enabledGates: sets.New("gate1", "gate2"),
+			annotations:  map[string]string{featureGateAnnotation: "gate1,gate2,-gate3"},
+			expectedErr:  "",
+		},
+		{
+			name:         "multiple requirements - inclusion not satisfied",
+			enabledGates: sets.New("gate1"),
+			annotations:  map[string]string{featureGateAnnotation: "gate1,gate2,-gate3"},
+			expectedErr:  "feature gate gate2 is required but not enabled",
+		},
+		{
+			name:         "multiple requirements - exclusion not satisfied",
+			enabledGates: sets.New("gate1", "gate2", "gate3"),
+			annotations:  map[string]string{featureGateAnnotation: "gate1,gate2,-gate3"},
+			expectedErr:  "feature gate gate3 is enabled but manifest requires it to be disabled",
+		},
+		{
+			name:         "whitespace handling",
+			enabledGates: sets.New("gate1", "gate2"),
+			annotations:  map[string]string{featureGateAnnotation: " gate1 , gate2 , -gate3 "},
+			expectedErr:  "",
+		},
+		{
+			name:         "empty requirements in list",
+			enabledGates: sets.New("gate1"),
+			annotations:  map[string]string{featureGateAnnotation: "gate1,,"},
+			expectedErr:  "",
+		},
+		{
+			name:         "complex scenario - OR logic for inclusion",
+			enabledGates: sets.New("NewStorageFeature"),
+			annotations:  map[string]string{featureGateAnnotation: "NewStorageFeature,AlternativeStorageFeature"},
+			expectedErr:  "feature gate AlternativeStorageFeature is required but not enabled",
+		},
+		{
+			name:         "complex scenario - AND logic for mixed requirements",
+			enabledGates: sets.New("FeatureA"),
+			annotations:  map[string]string{featureGateAnnotation: "FeatureA,-FeatureB"},
+			expectedErr:  "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := checkFeatureGates(tt.enabledGates, tt.annotations)
+			if tt.expectedErr == "" {
+				if err != nil {
+					t.Errorf("checkFeatureGates() expected no error, got %v", err)
+				}
+			} else {
+				if err == nil {
+					t.Errorf("checkFeatureGates() expected error %q, got nil", tt.expectedErr)
+				} else if err.Error() != tt.expectedErr {
+					t.Errorf("checkFeatureGates() expected error %q, got %q", tt.expectedErr, err.Error())
+				}
+			}
+		})
+	}
+}
+
+func TestManifestIncludeWithFeatureGates(t *testing.T) {
+	// Test just the feature gate functionality by testing manifest with no other filtering requirements
+	tests := []struct {
+		name         string
+		enabledGates sets.Set[string]
+		annotations  map[string]interface{}
+		expectedErr  string
+	}{
+		{
+			name:         "manifest included when feature gate enabled",
+			enabledGates: sets.New("TechPreviewFeatureGate"),
+			annotations: map[string]interface{}{
+				featureGateAnnotation: "TechPreviewFeatureGate",
+			},
+			expectedErr: "",
+		},
+		{
+			name:         "manifest excluded when feature gate disabled",
+			enabledGates: sets.New("SomeOtherGate"),
+			annotations: map[string]interface{}{
+				featureGateAnnotation: "TechPreviewFeatureGate",
+			},
+			expectedErr: "feature gate TechPreviewFeatureGate is required but not enabled",
+		},
+	}
+
+	for _, tt := range tests {
+		metadata := map[string]interface{}{
+			"name":      "test",
+			"namespace": "test",
+		}
+
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.annotations != nil {
+				metadata["annotations"] = tt.annotations
+			}
+			manifest := Manifest{
+				Obj: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"metadata": metadata,
+					},
+				},
+			}
+
+			// Test only feature gates, set all other filters to nil
+			err := manifest.Include(nil, nil, nil, nil, nil, tt.enabledGates)
+			if tt.expectedErr == "" {
+				if err != nil {
+					t.Errorf("manifest.Include() expected no error, got %v", err)
+				}
+			} else {
+				if err == nil {
+					t.Errorf("manifest.Include() expected error containing %q, got nil", tt.expectedErr)
+				} else if !strings.Contains(err.Error(), tt.expectedErr) {
+					t.Errorf("manifest.Include() expected error containing %q, got %q", tt.expectedErr, err.Error())
+				}
+			}
+		})
+	}
+}