From 75245d12ece994e923eee3d1aa6dc6d74ab411d1 Mon Sep 17 00:00:00 2001
From: Tyler Gillson <tyler.gillson@gmail.com>
Date: Sat, 29 Jun 2024 12:00:40 -0600
Subject: [PATCH] fix: respect user-specified features in
 ConvertToFeatureGateAPI

Signed-off-by: Tyler Gillson <tyler.gillson@gmail.com>
---
 pkg/genericclioptions/feature_gates.go      | 18 ++--
 pkg/genericclioptions/feature_gates_test.go | 93 +++++++++++++++++++++
 2 files changed, 105 insertions(+), 6 deletions(-)
 create mode 100644 pkg/genericclioptions/feature_gates_test.go

diff --git a/pkg/genericclioptions/feature_gates.go b/pkg/genericclioptions/feature_gates.go
index 64cb357d9..2c516c8ab 100644
--- a/pkg/genericclioptions/feature_gates.go
+++ b/pkg/genericclioptions/feature_gates.go
@@ -25,16 +25,22 @@ func init() {
 
 func ConvertToFeatureGateAPI(featureGates featuregate.MutableFeatureGate, defaultFeatureGate map[featuregate.Feature]featuregate.FeatureSpec) []operatorv1.FeatureGate {
 	var features []operatorv1.FeatureGate
-	for feature := range featureGates.GetAll() {
-		spec, ok := defaultFeatureGate[feature]
-		if !ok {
+	featureGatesMap := featureGates.GetAll()
+
+	// enable user-specified feature gates
+	for feature := range featureGatesMap {
+		if _, ok := defaultFeatureGate[feature]; !ok {
 			continue
 		}
+		if featureGates.Enabled(feature) {
+			features = append(features, operatorv1.FeatureGate{Feature: string(feature), Mode: operatorv1.FeatureGateModeTypeEnable})
+		}
+	}
 
-		if featureGates.Enabled(feature) && !spec.Default {
+	// enable default feature gates
+	for feature, spec := range defaultFeatureGate {
+		if _, ok := featureGatesMap[feature]; !ok && spec.Default {
 			features = append(features, operatorv1.FeatureGate{Feature: string(feature), Mode: operatorv1.FeatureGateModeTypeEnable})
-		} else if !featureGates.Enabled(feature) && spec.Default {
-			features = append(features, operatorv1.FeatureGate{Feature: string(feature), Mode: operatorv1.FeatureGateModeTypeDisable})
 		}
 	}
 
diff --git a/pkg/genericclioptions/feature_gates_test.go b/pkg/genericclioptions/feature_gates_test.go
new file mode 100644
index 000000000..e3bafdb2b
--- /dev/null
+++ b/pkg/genericclioptions/feature_gates_test.go
@@ -0,0 +1,93 @@
+// Copyright Contributors to the Open Cluster Management project
+package genericclioptions
+
+import (
+	"slices"
+	"testing"
+
+	"k8s.io/component-base/featuregate"
+	ocmfeature "open-cluster-management.io/api/feature"
+	operatorv1 "open-cluster-management.io/api/operator/v1"
+)
+
+func TestConvertToFeatureGateAPI(t *testing.T) {
+	tests := []struct {
+		name               string
+		featureGates       func() featuregate.MutableFeatureGate
+		defaultFeatureGate map[featuregate.Feature]featuregate.FeatureSpec
+		expected           []operatorv1.FeatureGate
+	}{
+		{
+			name: "disable default feature gate",
+			featureGates: func() featuregate.MutableFeatureGate {
+				fg := featuregate.NewFeatureGate()
+				_ = fg.Add(map[featuregate.Feature]featuregate.FeatureSpec{
+					"AddonManagement": {Default: false},
+				})
+				return fg
+			},
+			defaultFeatureGate: ocmfeature.DefaultHubAddonManagerFeatureGates,
+			expected:           []operatorv1.FeatureGate{},
+		},
+		{
+			name: "enable default feature gate",
+			featureGates: func() featuregate.MutableFeatureGate {
+				fg := featuregate.NewFeatureGate()
+				_ = fg.Add(map[featuregate.Feature]featuregate.FeatureSpec{
+					"AddonManagement": {Default: true},
+				})
+				return fg
+			},
+			defaultFeatureGate: ocmfeature.DefaultHubAddonManagerFeatureGates,
+			expected: []operatorv1.FeatureGate{
+				{Feature: "AddonManagement", Mode: operatorv1.FeatureGateModeTypeEnable},
+			},
+		},
+		{
+			name: "enable non-default feature gate",
+			featureGates: func() featuregate.MutableFeatureGate {
+				fg := featuregate.NewFeatureGate()
+				_ = fg.Add(map[featuregate.Feature]featuregate.FeatureSpec{
+					"ManifestWorkReplicaSet": {Default: true},
+				})
+				return fg
+			},
+			defaultFeatureGate: ocmfeature.DefaultHubWorkFeatureGates,
+			expected: []operatorv1.FeatureGate{
+				{Feature: "ManifestWorkReplicaSet", Mode: operatorv1.FeatureGateModeTypeEnable},
+			},
+		},
+		{
+			name: "enable non-default feature gate, ensure default feature gates remain enabled",
+			featureGates: func() featuregate.MutableFeatureGate {
+				fg := featuregate.NewFeatureGate()
+				_ = fg.Add(map[featuregate.Feature]featuregate.FeatureSpec{
+					"MultipleHubs": {Default: true},
+				})
+				return fg
+			},
+			defaultFeatureGate: ocmfeature.DefaultSpokeRegistrationFeatureGates,
+			expected: []operatorv1.FeatureGate{
+				{Feature: "AddonManagement", Mode: operatorv1.FeatureGateModeTypeEnable},
+				{Feature: "ClusterClaim", Mode: operatorv1.FeatureGateModeTypeEnable},
+				{Feature: "MultipleHubs", Mode: operatorv1.FeatureGateModeTypeEnable},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			actual := ConvertToFeatureGateAPI(tt.featureGates(), tt.defaultFeatureGate)
+			slices.SortFunc(actual, func(i, j operatorv1.FeatureGate) int {
+				if i.Feature < j.Feature {
+					return -1
+				} else if i.Feature > j.Feature {
+					return 1
+				}
+				return 0
+			})
+			if !slices.Equal(actual, tt.expected) {
+				t.Errorf("expected %v, got %v", tt.expected, actual)
+			}
+		})
+	}
+}