diff --git a/internal/featuregates/mapper.go b/internal/featuregates/mapper.go index 54255c147..2b2cdb2e2 100644 --- a/internal/featuregates/mapper.go +++ b/internal/featuregates/mapper.go @@ -2,11 +2,12 @@ package featuregates import ( "errors" - "sort" "strings" configv1 "github.com/openshift/api/config/v1" "github.com/openshift/api/features" + + "github.com/openshift/cluster-olm-operator/pkg/helmvalues" ) // Add your new upstream feature gate here @@ -19,36 +20,60 @@ const ( // SingleOwnNamespaceInstallSupport: Enables support for Single- and OwnNamespace install modes. SingleOwnNamespaceInstallSupport = "SingleOwnNamespaceInstallSupport" // WebhookProviderOpenshiftServiceCA: Enables support for the installation of bundles containing webhooks using the openshift-serviceca tls certificate provider + // WebhookProviderCertManager: This is something that always needs to be disabled downstream WebhookProviderOpenshiftServiceCA = "WebhookProviderOpenshiftServiceCA" + WebhookProviderCertManager = "WebhookProviderCertManager" ) type MapperInterface interface { - OperatorControllerUpstreamForDownstream(downstreamGate configv1.FeatureGateName) []string - OperatorControllerDownstreamFeatureGates() []configv1.FeatureGateName - CatalogdUpstreamForDownstream(downstreamGate configv1.FeatureGateName) []string - CatalogdDownstreamFeatureGates() []configv1.FeatureGateName + UpstreamForDownstream(downstreamGate configv1.FeatureGateName) func(*helmvalues.HelmValues, bool) error + DownstreamFeatureGates() []configv1.FeatureGateName } // Mapper knows the mapping between downstream and upstream feature gates for both OLM components + +type gateMapFunc map[configv1.FeatureGateName]func(*helmvalues.HelmValues, bool) error + type Mapper struct { - operatorControllerGates map[configv1.FeatureGateName][]string - catalogdGates map[configv1.FeatureGateName][]string + featureGates gateMapFunc } func NewMapper() *Mapper { // Add your downstream to upstream mapping here - operatorControllerGates := map[configv1.FeatureGateName][]string{ - // features.FeatureGateNewOLMMyDownstreamFeature: {MyUpstreamControllerOperatorFeature} - features.FeatureGateNewOLMPreflightPermissionChecks: {PreflightPermissions}, - features.FeatureGateNewOLMOwnSingleNamespace: {SingleOwnNamespaceInstallSupport}, - features.FeatureGateNewOLMWebhookProviderOpenshiftServiceCA: {WebhookProviderOpenshiftServiceCA}, - } - catalogdGates := map[configv1.FeatureGateName][]string{ - // features.FeatureGateNewOLMMyDownstreamFeature: {MyUpstreamCatalogdFeature} - features.FeatureGateNewOLMCatalogdAPIV1Metas: {APIV1MetasHandler}, + + featureGates := gateMapFunc{ + // features.FeatureGateNewOLMMyDownstreamFeature: functon that returns a list of enabled and disabled gates + features.FeatureGateNewOLMPreflightPermissionChecks: func(v *helmvalues.HelmValues, enabled bool) error { + if enabled { + return v.AddListValue(helmvalues.EnableOperatorController, PreflightPermissions) + } + return v.AddListValue(helmvalues.DisableOperatorController, PreflightPermissions) + }, + features.FeatureGateNewOLMOwnSingleNamespace: func(v *helmvalues.HelmValues, enabled bool) error { + if enabled { + return v.AddListValue(helmvalues.EnableOperatorController, SingleOwnNamespaceInstallSupport) + } + return v.AddListValue(helmvalues.DisableOperatorController, SingleOwnNamespaceInstallSupport) + }, + features.FeatureGateNewOLMWebhookProviderOpenshiftServiceCA: func(v *helmvalues.HelmValues, enabled bool) error { + var errs []error + if enabled { + errs = append(errs, v.AddListValue(helmvalues.EnableOperatorController, WebhookProviderOpenshiftServiceCA)) + } else { + errs = append(errs, v.AddListValue(helmvalues.DisableOperatorController, WebhookProviderOpenshiftServiceCA)) + } + errs = append(errs, v.AddListValue(helmvalues.DisableOperatorController, WebhookProviderCertManager)) + return errors.Join(errs...) + }, + features.FeatureGateNewOLMCatalogdAPIV1Metas: func(v *helmvalues.HelmValues, enabled bool) error { + if enabled { + return v.AddListValue(helmvalues.EnableCatalogd, APIV1MetasHandler) + } + return v.AddListValue(helmvalues.DisableCatalogd, APIV1MetasHandler) + }, } - for _, m := range []map[configv1.FeatureGateName][]string{operatorControllerGates, catalogdGates} { + for _, m := range []gateMapFunc{featureGates} { for downstreamGate := range m { // features.FeatureGateNewOLM is a GA-enabled downstream feature gate. // If there is a need to enable upstream alpha/beta features in the downstream GA release @@ -63,48 +88,21 @@ func NewMapper() *Mapper { } } - return &Mapper{operatorControllerGates: operatorControllerGates, catalogdGates: catalogdGates} -} - -// OperatorControllerDownstreamFeatureGates returns a list of all downstream feature gates -// which have an upstream mapping configured for the operator-controller component -func (m *Mapper) OperatorControllerDownstreamFeatureGates() []configv1.FeatureGateName { - return getKeys(m.operatorControllerGates) + return &Mapper{featureGates: featureGates} } -// CatalogdDownstreamFeatureGates returns a list of all downstream feature gates -// which have an upstream mapping configured for the catalogd component -func (m *Mapper) CatalogdDownstreamFeatureGates() []configv1.FeatureGateName { - return getKeys(m.catalogdGates) -} - -// OperatorControllerUpstreamForDownstream returns upstream feature gates which are configured -// for a given downstream feature gate for the operator-controller component -func (m *Mapper) OperatorControllerUpstreamForDownstream(downstreamGate configv1.FeatureGateName) []string { - return m.operatorControllerGates[downstreamGate] -} - -// CatalogdUpstreamForDownstream returns upstream feature gates which are configured -// for a given downstream feature gate for the catalogd component -func (m *Mapper) CatalogdUpstreamForDownstream(downstreamGate configv1.FeatureGateName) []string { - return m.catalogdGates[downstreamGate] -} - -// FormatAsEnabledArgs combines list of feature gate names into -// an all-enabled arg format of =true,=true etc. -func FormatAsEnabledArgs(enabledFeatureGates []string) string { - args := make([]string, 0, len(enabledFeatureGates)) - sort.Strings(enabledFeatureGates) - for _, gateName := range enabledFeatureGates { - args = append(args, gateName+"=true") - } - return strings.Join(args, ",") -} - -func getKeys(m map[configv1.FeatureGateName][]string) []configv1.FeatureGateName { - keys := make([]configv1.FeatureGateName, 0, len(m)) - for k := range m { +// DownstreamFeatureGates returns a list of all downstream feature gates +// which have an upstream mapping configured +func (m *Mapper) DownstreamFeatureGates() []configv1.FeatureGateName { + keys := make([]configv1.FeatureGateName, 0, len(m.featureGates)) + for k := range m.featureGates { keys = append(keys, k) } return keys } + +// UpstreamForDownstream returns upstream feature gates which are configured +// for a given downstream feature gate +func (m *Mapper) UpstreamForDownstream(downstreamGate configv1.FeatureGateName) func(*helmvalues.HelmValues, bool) error { + return m.featureGates[downstreamGate] +} diff --git a/internal/featuregates/mapper_test.go b/internal/featuregates/mapper_test.go index 1fa811043..e0cc7b1f5 100644 --- a/internal/featuregates/mapper_test.go +++ b/internal/featuregates/mapper_test.go @@ -1,73 +1,300 @@ package featuregates import ( + "reflect" "slices" + "strings" "testing" configv1 "github.com/openshift/api/config/v1" "github.com/openshift/api/features" + + "github.com/openshift/cluster-olm-operator/pkg/helmvalues" ) -func TestMapper_ControllerUpstreamForDownstream(t *testing.T) { - t.Run("returns mapped upstream gates for operator-controller", func(t *testing.T) { - expectedUpstreamGates := []string{"HelloGate", "WorldGate"} +func TestNewMapper(t *testing.T) { + mapper := NewMapper() + if mapper == nil { + t.Fatal("NewMapper returned nil") + } + if mapper.featureGates == nil { + t.Fatal("featureGates map is nil") + } + if len(mapper.featureGates) == 0 { + t.Fatal("featureGates map is empty") + } +} + +func TestMapper_DownstreamFeatureGates(t *testing.T) { + mapper := NewMapper() + gates := mapper.DownstreamFeatureGates() - mapper := NewMapper() - mapper.operatorControllerGates = map[configv1.FeatureGateName][]string{ - features.FeatureGateNewOLM: expectedUpstreamGates, - } - upstream := mapper.OperatorControllerUpstreamForDownstream(features.FeatureGateNewOLM) - if !slices.Equal(upstream, expectedUpstreamGates) { - t.Fatalf("expected and returned upstream gates differ: upstream: %+v, expected: %+v", - upstream, expectedUpstreamGates, - ) + expectedGates := []configv1.FeatureGateName{ + features.FeatureGateNewOLMPreflightPermissionChecks, + features.FeatureGateNewOLMOwnSingleNamespace, + features.FeatureGateNewOLMWebhookProviderOpenshiftServiceCA, + features.FeatureGateNewOLMCatalogdAPIV1Metas, + } + + if len(gates) != len(expectedGates) { + t.Fatalf("Expected %d gates, got %d", len(expectedGates), len(gates)) + } + + for _, expectedGate := range expectedGates { + if !slices.Contains(gates, expectedGate) { + t.Errorf("Expected gate %s not found in returned gates", expectedGate) } - }) + } } -func TestMapper_CatalogdUpstreamForDownstream(t *testing.T) { - t.Run("returns mapped upstream gates for catalogd", func(t *testing.T) { - expectedUpstreamGates := []string{"HelloGate", "WorldGate"} +func TestMapper_UpstreamForDownstream(t *testing.T) { + mapper := NewMapper() - mapper := NewMapper() - mapper.catalogdGates = map[configv1.FeatureGateName][]string{ - features.FeatureGateNewOLM: expectedUpstreamGates, - } - upstream := mapper.CatalogdUpstreamForDownstream(features.FeatureGateNewOLM) - if !slices.Equal(upstream, expectedUpstreamGates) { - t.Fatalf("expected and returned upstream gates differ: upstream: %+v, expected: %+v", - upstream, expectedUpstreamGates, - ) - } - }) + tests := []struct { + name string + downstreamGate configv1.FeatureGateName + enabled bool + expectFunc bool + }{ + { + name: "valid downstream gate - preflight permissions", + downstreamGate: features.FeatureGateNewOLMPreflightPermissionChecks, + enabled: true, + expectFunc: true, + }, + { + name: "valid downstream gate - own single namespace", + downstreamGate: features.FeatureGateNewOLMOwnSingleNamespace, + enabled: false, + expectFunc: true, + }, + { + name: "valid downstream gate - webhook provider", + downstreamGate: features.FeatureGateNewOLMWebhookProviderOpenshiftServiceCA, + enabled: true, + expectFunc: true, + }, + { + name: "valid downstream gate - catalogd api v1 metas", + downstreamGate: features.FeatureGateNewOLMCatalogdAPIV1Metas, + enabled: false, + expectFunc: true, + }, + { + name: "invalid downstream gate", + downstreamGate: "InvalidGate", + enabled: true, + expectFunc: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fn := mapper.UpstreamForDownstream(tt.downstreamGate) + if tt.expectFunc && fn == nil { + t.Errorf("Expected function for gate %s, got nil", tt.downstreamGate) + } + if !tt.expectFunc && fn != nil { + t.Errorf("Expected nil function for gate %s, got non-nil", tt.downstreamGate) + } + }) + } } -func TestFormatAsEnabledArgs(t *testing.T) { - testCases := []struct { - name string - in []string - expected string +func TestMapper_FeatureGateMappings(t *testing.T) { + tests := []struct { + name string + downstreamGate configv1.FeatureGateName + enabled bool + expectedValues map[string]interface{} }{ { - name: "empty", + name: "PreflightPermissionChecks enabled", + downstreamGate: features.FeatureGateNewOLMPreflightPermissionChecks, + enabled: true, + expectedValues: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{PreflightPermissions}, + }, + }, + }, + }, + }, + { + name: "PreflightPermissionChecks disabled", + downstreamGate: features.FeatureGateNewOLMPreflightPermissionChecks, + enabled: false, + expectedValues: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "disabled": []interface{}{PreflightPermissions}, + }, + }, + }, + }, }, { - name: "single feature gate", - in: []string{"testGate1"}, - expected: "testGate1=true", + name: "OwnSingleNamespace enabled", + downstreamGate: features.FeatureGateNewOLMOwnSingleNamespace, + enabled: true, + expectedValues: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{SingleOwnNamespaceInstallSupport}, + }, + }, + }, + }, }, { - name: "multiple feature gates", - in: []string{"testGate3", "testGate2", "testGate1"}, - expected: "testGate1=true,testGate2=true,testGate3=true", + name: "OwnSingleNamespace disabled", + downstreamGate: features.FeatureGateNewOLMOwnSingleNamespace, + enabled: false, + expectedValues: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "disabled": []interface{}{SingleOwnNamespaceInstallSupport}, + }, + }, + }, + }, + }, + { + name: "WebhookProviderOpenshiftServiceCA enabled", + downstreamGate: features.FeatureGateNewOLMWebhookProviderOpenshiftServiceCA, + enabled: true, + expectedValues: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{WebhookProviderOpenshiftServiceCA}, + "disabled": []interface{}{WebhookProviderCertManager}, + }, + }, + }, + }, + }, + { + name: "WebhookProviderOpenshiftServiceCA disabled", + downstreamGate: features.FeatureGateNewOLMWebhookProviderOpenshiftServiceCA, + enabled: false, + expectedValues: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "disabled": []interface{}{WebhookProviderCertManager, WebhookProviderOpenshiftServiceCA}, + }, + }, + }, + }, + }, + { + name: "CatalogdAPIV1Metas enabled", + downstreamGate: features.FeatureGateNewOLMCatalogdAPIV1Metas, + enabled: true, + expectedValues: map[string]interface{}{ + "options": map[string]interface{}{ + "catalogd": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{APIV1MetasHandler}, + }, + }, + }, + }, + }, + { + name: "CatalogdAPIV1Metas disabled", + downstreamGate: features.FeatureGateNewOLMCatalogdAPIV1Metas, + enabled: false, + expectedValues: map[string]interface{}{ + "options": map[string]interface{}{ + "catalogd": map[string]interface{}{ + "features": map[string]interface{}{ + "disabled": []interface{}{APIV1MetasHandler}, + }, + }, + }, + }, }, } - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - result := FormatAsEnabledArgs(testCase.in) - if result != testCase.expected { - t.Fatalf("result and expected differ, expected: %q, got: %q", testCase.expected, result) + + mapper := NewMapper() + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hv := helmvalues.NewHelmValues() + fn := mapper.UpstreamForDownstream(tt.downstreamGate) + if fn == nil { + t.Fatalf("No mapping function found for gate %s", tt.downstreamGate) + } + + err := fn(hv, tt.enabled) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + actual := hv.GetValues() + if !reflect.DeepEqual(actual, tt.expectedValues) { + t.Errorf("Expected values %v, got %v", tt.expectedValues, actual) } }) } } + +func TestMapper_FeatureGateValidation(t *testing.T) { + t.Run("all feature gates have proper NewOLM prefix", func(t *testing.T) { + mapper := NewMapper() + gates := mapper.DownstreamFeatureGates() + + for _, gate := range gates { + gateStr := string(gate) + if !strings.HasPrefix(gateStr, string(features.FeatureGateNewOLM)) { + t.Errorf("Feature gate %s does not have NewOLM prefix", gate) + } + if gate == features.FeatureGateNewOLM { + t.Errorf("Feature gate should not be FeatureGateNewOLM directly: %s", gate) + } + } + }) + + t.Run("validates constants are not empty", func(t *testing.T) { + constants := []string{ + APIV1MetasHandler, + PreflightPermissions, + SingleOwnNamespaceInstallSupport, + WebhookProviderOpenshiftServiceCA, + WebhookProviderCertManager, + } + + for i, constant := range constants { + if constant == "" { + t.Errorf("Constant at index %d is empty", i) + } + } + }) +} + +func TestMapper_Constants(t *testing.T) { + expectedConstants := map[string]string{ + "APIV1MetasHandler": APIV1MetasHandler, + "PreflightPermissions": PreflightPermissions, + "SingleOwnNamespaceInstallSupport": SingleOwnNamespaceInstallSupport, + "WebhookProviderOpenshiftServiceCA": WebhookProviderOpenshiftServiceCA, + "WebhookProviderCertManager": WebhookProviderCertManager, + } + + for name, constant := range expectedConstants { + if constant == "" { + t.Errorf("Constant %s is empty", name) + } + } + + if APIV1MetasHandler == PreflightPermissions { + t.Error("APIV1MetasHandler and PreflightPermissions should be different") + } +} diff --git a/pkg/controller/featuregates_hook.go b/pkg/controller/featuregates_hook.go index e434b86c0..23f3b02da 100644 --- a/pkg/controller/featuregates_hook.go +++ b/pkg/controller/featuregates_hook.go @@ -1,37 +1,27 @@ package controller import ( - "slices" + "errors" configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/cluster-olm-operator/pkg/helmvalues" "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" ) -// upstreamFeatureGates build and returns a unique and ordered list of upstream feature gates names -// that map to the provided enabled downstream feature gates +// upstreamFeatureGates builds a set of helm values for the downsteeam feature-gates that are +// mapped to upstream feature-gates func upstreamFeatureGates( clusterGatesConfig featuregates.FeatureGate, downstreamGates []configv1.FeatureGateName, - downstreamToUpstreamFunc func(configv1.FeatureGateName) []string, -) []string { - var upstreamGates []string + downstreamToUpstreamFunc func(configv1.FeatureGateName) func(*helmvalues.HelmValues, bool) error, +) (*helmvalues.HelmValues, error) { + errs := make([]error, 0, len(downstreamGates)) + values := helmvalues.NewHelmValues() - seen := make(map[string]struct{}) for _, downstreamGate := range downstreamGates { - if !clusterGatesConfig.Enabled(downstreamGate) { - continue - } - - for _, upstreamGate := range downstreamToUpstreamFunc(downstreamGate) { - if _, found := seen[upstreamGate]; found { - continue - } - - seen[upstreamGate] = struct{}{} - upstreamGates = append(upstreamGates, upstreamGate) - } + f := downstreamToUpstreamFunc(downstreamGate) + errs = append(errs, f(values, clusterGatesConfig.Enabled(downstreamGate))) } - slices.Sort(upstreamGates) - return upstreamGates + return values, errors.Join(errs...) } diff --git a/pkg/controller/featuregates_hook_test.go b/pkg/controller/featuregates_hook_test.go new file mode 100644 index 000000000..703afe342 --- /dev/null +++ b/pkg/controller/featuregates_hook_test.go @@ -0,0 +1,418 @@ +package controller + +import ( + "errors" + "reflect" + "testing" + + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/api/features" + "github.com/openshift/cluster-olm-operator/pkg/helmvalues" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" +) + +// mockFeatureGate implements featuregates.FeatureGate for testing +type mockFeatureGate struct { + enabledGates []configv1.FeatureGateName + disabledGates []configv1.FeatureGateName +} + +func newMockFeatureGate(enabled, disabled []configv1.FeatureGateName) *mockFeatureGate { + return &mockFeatureGate{ + enabledGates: enabled, + disabledGates: disabled, + } +} + +func (m *mockFeatureGate) Enabled(gate configv1.FeatureGateName) bool { + for _, enabledGate := range m.enabledGates { + if enabledGate == gate { + return true + } + } + return false +} + +func (m *mockFeatureGate) KnownFeatures() []configv1.FeatureGateName { + all := make([]configv1.FeatureGateName, 0, len(m.enabledGates)+len(m.disabledGates)) + all = append(all, m.enabledGates...) + all = append(all, m.disabledGates...) + return all +} + +func TestUpstreamFeatureGates(t *testing.T) { + tests := []struct { + name string + clusterGatesConfig featuregates.FeatureGate + downstreamGates []configv1.FeatureGateName + downstreamToUpstreamFunc func(configv1.FeatureGateName) func(*helmvalues.HelmValues, bool) error + expectedValues map[string]interface{} + expectError bool + }{ + { + name: "empty downstream gates", + clusterGatesConfig: newMockFeatureGate( + []configv1.FeatureGateName{}, + []configv1.FeatureGateName{}, + ), + downstreamGates: []configv1.FeatureGateName{}, + downstreamToUpstreamFunc: func(_ configv1.FeatureGateName) func(*helmvalues.HelmValues, bool) error { + return func(_ *helmvalues.HelmValues, _ bool) error { + return nil + } + }, + expectedValues: map[string]interface{}{}, + expectError: false, + }, + { + name: "single enabled feature gate", + clusterGatesConfig: newMockFeatureGate( + []configv1.FeatureGateName{features.FeatureGateNewOLMPreflightPermissionChecks}, + []configv1.FeatureGateName{}, + ), + downstreamGates: []configv1.FeatureGateName{features.FeatureGateNewOLMPreflightPermissionChecks}, + downstreamToUpstreamFunc: func(gate configv1.FeatureGateName) func(*helmvalues.HelmValues, bool) error { + if gate == features.FeatureGateNewOLMPreflightPermissionChecks { + return func(hv *helmvalues.HelmValues, enabled bool) error { + if enabled { + return hv.AddListValue("options.operatorController.features.enabled", "PreflightPermissions") + } + return hv.AddListValue("options.operatorController.features.disabled", "PreflightPermissions") + } + } + return func(_ *helmvalues.HelmValues, _ bool) error { + return nil + } + }, + expectedValues: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"PreflightPermissions"}, + }, + }, + }, + }, + expectError: false, + }, + { + name: "single disabled feature gate", + clusterGatesConfig: newMockFeatureGate( + []configv1.FeatureGateName{}, + []configv1.FeatureGateName{features.FeatureGateNewOLMPreflightPermissionChecks}, + ), + downstreamGates: []configv1.FeatureGateName{features.FeatureGateNewOLMPreflightPermissionChecks}, + downstreamToUpstreamFunc: func(gate configv1.FeatureGateName) func(*helmvalues.HelmValues, bool) error { + if gate == features.FeatureGateNewOLMPreflightPermissionChecks { + return func(hv *helmvalues.HelmValues, enabled bool) error { + if enabled { + return hv.AddListValue("options.operatorController.features.enabled", "PreflightPermissions") + } + return hv.AddListValue("options.operatorController.features.disabled", "PreflightPermissions") + } + } + return func(_ *helmvalues.HelmValues, _ bool) error { + return nil + } + }, + expectedValues: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "disabled": []interface{}{"PreflightPermissions"}, + }, + }, + }, + }, + expectError: false, + }, + { + name: "multiple feature gates mixed enabled/disabled", + clusterGatesConfig: newMockFeatureGate( + []configv1.FeatureGateName{ + features.FeatureGateNewOLMPreflightPermissionChecks, + features.FeatureGateNewOLMOwnSingleNamespace, + }, + []configv1.FeatureGateName{ + features.FeatureGateNewOLMWebhookProviderOpenshiftServiceCA, + }, + ), + downstreamGates: []configv1.FeatureGateName{ + features.FeatureGateNewOLMPreflightPermissionChecks, + features.FeatureGateNewOLMOwnSingleNamespace, + features.FeatureGateNewOLMWebhookProviderOpenshiftServiceCA, + }, + downstreamToUpstreamFunc: func(gate configv1.FeatureGateName) func(*helmvalues.HelmValues, bool) error { + switch gate { + case features.FeatureGateNewOLMPreflightPermissionChecks: + return func(hv *helmvalues.HelmValues, enabled bool) error { + if enabled { + return hv.AddListValue("options.operatorController.features.enabled", "PreflightPermissions") + } + return hv.AddListValue("options.operatorController.features.disabled", "PreflightPermissions") + } + case features.FeatureGateNewOLMOwnSingleNamespace: + return func(hv *helmvalues.HelmValues, enabled bool) error { + if enabled { + return hv.AddListValue("options.operatorController.features.enabled", "SingleOwnNamespaceInstallSupport") + } + return hv.AddListValue("options.operatorController.features.disabled", "SingleOwnNamespaceInstallSupport") + } + case features.FeatureGateNewOLMWebhookProviderOpenshiftServiceCA: + return func(hv *helmvalues.HelmValues, enabled bool) error { + if enabled { + return hv.AddListValue("options.operatorController.features.enabled", "WebhookProviderOpenshiftServiceCA") + } + return hv.AddListValue("options.operatorController.features.disabled", "WebhookProviderOpenshiftServiceCA") + } + } + return func(_ *helmvalues.HelmValues, _ bool) error { + return nil + } + }, + expectedValues: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"PreflightPermissions", "SingleOwnNamespaceInstallSupport"}, + "disabled": []interface{}{"WebhookProviderOpenshiftServiceCA"}, + }, + }, + }, + }, + expectError: false, + }, + { + name: "mapping function returns error", + clusterGatesConfig: newMockFeatureGate( + []configv1.FeatureGateName{features.FeatureGateNewOLMPreflightPermissionChecks}, + []configv1.FeatureGateName{}, + ), + downstreamGates: []configv1.FeatureGateName{features.FeatureGateNewOLMPreflightPermissionChecks}, + downstreamToUpstreamFunc: func(_ configv1.FeatureGateName) func(*helmvalues.HelmValues, bool) error { + return func(_ *helmvalues.HelmValues, _ bool) error { + return errors.New("mapping function error") + } + }, + expectedValues: map[string]interface{}{}, + expectError: true, + }, + { + name: "multiple mapping function errors", + clusterGatesConfig: newMockFeatureGate( + []configv1.FeatureGateName{ + features.FeatureGateNewOLMPreflightPermissionChecks, + features.FeatureGateNewOLMOwnSingleNamespace, + }, + []configv1.FeatureGateName{}, + ), + downstreamGates: []configv1.FeatureGateName{ + features.FeatureGateNewOLMPreflightPermissionChecks, + features.FeatureGateNewOLMOwnSingleNamespace, + }, + downstreamToUpstreamFunc: func(gate configv1.FeatureGateName) func(*helmvalues.HelmValues, bool) error { + return func(_ *helmvalues.HelmValues, _ bool) error { + return errors.New("mapping error for " + string(gate)) + } + }, + expectedValues: map[string]interface{}{}, + expectError: true, + }, + { + name: "unknown downstream gate", + clusterGatesConfig: newMockFeatureGate( + []configv1.FeatureGateName{}, + []configv1.FeatureGateName{"UnknownGate"}, + ), + downstreamGates: []configv1.FeatureGateName{"UnknownGate"}, + downstreamToUpstreamFunc: func(_ configv1.FeatureGateName) func(*helmvalues.HelmValues, bool) error { + // Return no-op function for unknown gates (no mapping available) + return func(_ *helmvalues.HelmValues, _ bool) error { + return nil + } + }, + expectedValues: map[string]interface{}{}, + expectError: false, + }, + { + name: "complex webhook provider scenario", + clusterGatesConfig: newMockFeatureGate( + []configv1.FeatureGateName{features.FeatureGateNewOLMWebhookProviderOpenshiftServiceCA}, + []configv1.FeatureGateName{}, + ), + downstreamGates: []configv1.FeatureGateName{features.FeatureGateNewOLMWebhookProviderOpenshiftServiceCA}, + downstreamToUpstreamFunc: func(gate configv1.FeatureGateName) func(*helmvalues.HelmValues, bool) error { + if gate == features.FeatureGateNewOLMWebhookProviderOpenshiftServiceCA { + return func(hv *helmvalues.HelmValues, enabled bool) error { + var errs []error + if enabled { + errs = append(errs, hv.AddListValue("options.operatorController.features.enabled", "WebhookProviderOpenshiftServiceCA")) + } else { + errs = append(errs, hv.AddListValue("options.operatorController.features.disabled", "WebhookProviderOpenshiftServiceCA")) + } + // Always disable cert manager + errs = append(errs, hv.AddListValue("options.operatorController.features.disabled", "WebhookProviderCertManager")) + return errors.Join(errs...) + } + } + return func(_ *helmvalues.HelmValues, _ bool) error { + return nil + } + }, + expectedValues: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"WebhookProviderOpenshiftServiceCA"}, + "disabled": []interface{}{"WebhookProviderCertManager"}, + }, + }, + }, + }, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := upstreamFeatureGates( + tt.clusterGatesConfig, + tt.downstreamGates, + tt.downstreamToUpstreamFunc, + ) + + if tt.expectError && err == nil { + t.Errorf("Expected error, but got none") + } + if !tt.expectError && err != nil { + t.Errorf("Unexpected error: %v", err) + } + + if result == nil { + t.Fatal("Result HelmValues is nil") + } + + actualValues := result.GetValues() + if !reflect.DeepEqual(actualValues, tt.expectedValues) { + t.Errorf("Expected values %v, got %v", tt.expectedValues, actualValues) + } + }) + } +} + +func TestUpstreamFeatureGates_NilMappingFunction(t *testing.T) { + // Test case where the mapping function returns nil (no mapping for a gate) + clusterGatesConfig := newMockFeatureGate( + []configv1.FeatureGateName{features.FeatureGateNewOLMPreflightPermissionChecks}, + []configv1.FeatureGateName{}, + ) + downstreamGates := []configv1.FeatureGateName{features.FeatureGateNewOLMPreflightPermissionChecks} + + // This should panic when trying to call a nil function + defer func() { + if r := recover(); r == nil { + t.Errorf("Expected panic when calling nil mapping function") + } + }() + + downstreamToUpstreamFunc := func(_ configv1.FeatureGateName) func(*helmvalues.HelmValues, bool) error { + return nil // Return nil function + } + + _, _ = upstreamFeatureGates(clusterGatesConfig, downstreamGates, downstreamToUpstreamFunc) +} + +func TestUpstreamFeatureGates_EdgeCases(t *testing.T) { + // Test with no gates enabled/disabled but gates still present + t.Run("gates present but none enabled", func(t *testing.T) { + clusterGatesConfig := newMockFeatureGate( + []configv1.FeatureGateName{}, + []configv1.FeatureGateName{}, + ) + downstreamGates := []configv1.FeatureGateName{features.FeatureGateNewOLMPreflightPermissionChecks} + + result, err := upstreamFeatureGates( + clusterGatesConfig, + downstreamGates, + func(_ configv1.FeatureGateName) func(*helmvalues.HelmValues, bool) error { + return func(hv *helmvalues.HelmValues, enabled bool) error { + if enabled { + return hv.AddListValue("options.operatorController.features.enabled", "TestFeature") + } + return hv.AddListValue("options.operatorController.features.disabled", "TestFeature") + } + }, + ) + + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + expectedValues := map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "disabled": []interface{}{"TestFeature"}, + }, + }, + }, + } + + if !reflect.DeepEqual(result.GetValues(), expectedValues) { + t.Errorf("Expected values %v, got %v", expectedValues, result.GetValues()) + } + }) +} + +func TestMockFeatureGate(t *testing.T) { + // Test the mock implementation itself + enabled := []configv1.FeatureGateName{ + features.FeatureGateNewOLMPreflightPermissionChecks, + features.FeatureGateNewOLMOwnSingleNamespace, + } + disabled := []configv1.FeatureGateName{ + features.FeatureGateNewOLMWebhookProviderOpenshiftServiceCA, + } + + mock := newMockFeatureGate(enabled, disabled) + + // Test Enabled method + for _, gate := range enabled { + if !mock.Enabled(gate) { + t.Errorf("Expected gate %s to be enabled", gate) + } + } + + for _, gate := range disabled { + if mock.Enabled(gate) { + t.Errorf("Expected gate %s to be disabled", gate) + } + } + + // Test unknown gate + if mock.Enabled("UnknownGate") { + t.Errorf("Expected unknown gate to be disabled") + } + + // Test KnownFeatures method + known := mock.KnownFeatures() + expectedCount := len(enabled) + len(disabled) + if len(known) != expectedCount { + t.Errorf("Expected %d known features, got %d", expectedCount, len(known)) + } + + // Verify all expected gates are in known features + allExpected := append(enabled, disabled...) + for _, expectedGate := range allExpected { + found := false + for _, knownGate := range known { + if knownGate == expectedGate { + found = true + break + } + } + if !found { + t.Errorf("Expected gate %s not found in known features", expectedGate) + } + } +} diff --git a/pkg/controller/helm.go b/pkg/controller/helm.go index 2f150a5c8..0b69099af 100644 --- a/pkg/controller/helm.go +++ b/pkg/controller/helm.go @@ -3,15 +3,13 @@ package controller import ( "bufio" "context" - "errors" "fmt" "os" "path/filepath" "regexp" - "slices" "strings" - "github.com/TwiN/deepmerge" + "github.com/openshift/cluster-olm-operator/pkg/helmvalues" yaml3 "gopkg.in/yaml.v3" @@ -35,17 +33,22 @@ func (b *Builder) renderHelmTemplate(helmPath, manifestDir string) error { if err != nil { return fmt.Errorf("CurrentFeatureGates failed: %w", err) } - catalogdFeatures := upstreamFeatureGates(clusterGatesConfig, - b.Clients.FeatureGateMapper.CatalogdDownstreamFeatureGates(), - b.Clients.FeatureGateMapper.CatalogdUpstreamForDownstream) - operatorControllerFeatures := upstreamFeatureGates(clusterGatesConfig, - b.Clients.FeatureGateMapper.OperatorControllerDownstreamFeatureGates(), - b.Clients.FeatureGateMapper.OperatorControllerUpstreamForDownstream) - if len(catalogdFeatures) > 0 || len(operatorControllerFeatures) > 0 { + + featureGateValues, err := upstreamFeatureGates(clusterGatesConfig, + b.Clients.FeatureGateMapper.DownstreamFeatureGates(), + b.Clients.FeatureGateMapper.UpstreamForDownstream) + if err != nil { + return err + } + hasEnabledFeatureGates, err := featureGateValues.HasEnabledFeatureGates() + if err != nil { + return err + } + if hasEnabledFeatureGates { useExperimental = true } - // Determine and generate the values + // Determine and generate the values from the files (equivalent to --values) valuesFiles := []string{filepath.Join(helmPath, "openshift.yaml")} if useExperimental { log.Info("Using experimental values") @@ -53,31 +56,27 @@ func (b *Builder) renderHelmTemplate(helmPath, manifestDir string) error { } else { log.Info("Using standard values") } - values, err := gatherHelmValues(valuesFiles) + values, err := helmvalues.NewHelmValuesFromFiles(valuesFiles) if err != nil { - return fmt.Errorf("gatherHelmValues failed: %w", err) + return fmt.Errorf("error from GatherHelmValuesFromFiles: %w", err) } + // Add the featureGateValues + if err := values.AddValues(featureGateValues); err != nil { + return fmt.Errorf("error from AddValues: %w", err) + } // Log verbosity and proxies are dynamic, so they are not included here. // Feature flags are listed here, and if they change cluster-olm-operator // will resart, as the manifest needs to be regenerated - newvalues := []struct { - location string - value any - }{ - {"catalogdFeatures", catalogdFeatures}, - {"operatorControllerFeatures", operatorControllerFeatures}, - {"options.operatorController.deployment.image", os.Getenv("OPERATOR_CONTROLLER_IMAGE")}, - {"options.catalogd.deployment.image", os.Getenv("CATALOGD_IMAGE")}, + // Add more values (equivalent to --set) + if err := values.SetStringValue("options.catalogd.deployment.image", os.Getenv("CATALOGD_IMAGE")); err != nil { + return fmt.Errorf("error setting CATALOGD_IMAGE: %w", err) } - - for _, v := range newvalues { - values, err = setHelmValue(values, v.location, v.value) - if err != nil { - return fmt.Errorf("error setting Helm values %q=%q: %w", v.location, v.value, err) - } + if err := values.SetStringValue("options.operatorController.deployment.image", os.Getenv("OPERATOR_CONTROLLER_IMAGE")); err != nil { + return fmt.Errorf("error setting OPERATOR_CONTROLLER_IMAGE: %w", err) } - log.Info("Calculated values", "values", values) + + log.Info("Calculated values", "values", values.GetValues()) // Load the helm chart chart, err := loader.Load(filepath.Join(helmPath, "olmv1")) @@ -93,7 +92,7 @@ func (b *Builder) renderHelmTemplate(helmPath, manifestDir string) error { client.DisableHooks = true // Render the chart into memory - rel, err := client.Run(chart, values) + rel, err := client.Run(chart, values.GetValues()) if err != nil { return fmt.Errorf("render Run failed: %w", err) } @@ -173,46 +172,6 @@ func (b *Builder) renderHelmTemplate(helmPath, manifestDir string) error { return nil } -func gatherHelmValues(files []string) (map[string]any, error) { - values := make(map[string]any) - for _, a := range files { - newvalues := make(map[string]any) - data, err := os.ReadFile(a) - if err != nil { - return nil, err - } - if err := yaml3.Unmarshal(data, newvalues); err != nil { - return nil, err - } - if err := deepmerge.DeepMerge(values, newvalues, deepmerge.Config{}); err != nil { - return nil, err - } - } - return values, nil -} - -func setHelmValue(values map[string]any, location string, value any) (map[string]any, error) { - ss := strings.Split(location, ".") - if len(ss) < 1 { - return nil, errors.New("location string has no locations") - } - - // Build a tree in reverse - slices.Reverse(ss) - v := make(map[string]any) - v[ss[0]] = value - for _, s := range ss[1:] { - newmap := make(map[string]any) - newmap[s] = v - v = newmap - } - - if err := deepmerge.DeepMerge(values, v, deepmerge.Config{}); err != nil { - return nil, err - } - return values, nil -} - // YAML Splitting Code type K8sResource struct { diff --git a/pkg/controller/helm_test.go b/pkg/controller/helm_test.go index b7f0837e9..7919fde0b 100644 --- a/pkg/controller/helm_test.go +++ b/pkg/controller/helm_test.go @@ -4,12 +4,15 @@ import ( "io/fs" "os" "path/filepath" + "reflect" + "strings" "testing" configv1 "github.com/openshift/api/config/v1" internalfeatures "github.com/openshift/cluster-olm-operator/internal/featuregates" "github.com/openshift/cluster-olm-operator/pkg/clients" "github.com/stretchr/testify/require" + yaml3 "gopkg.in/yaml.v3" ) func TestRenderHelmTemplate(t *testing.T) { @@ -71,3 +74,475 @@ func TestRenderHelmTemplate(t *testing.T) { require.NoError(t, err) require.Equal(t, compareData, testData) } + +func TestSplitYAMLDocuments(t *testing.T) { + tests := []struct { + name string + input string + expected []string + }{ + { + name: "empty input", + input: "", + expected: nil, + }, + { + name: "single document", + input: "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test", + expected: []string{"apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test"}, + }, + { + name: "multiple documents with separators", + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test1 +--- +apiVersion: v1 +kind: Secret +metadata: + name: test2`, + expected: []string{ + "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test1", + "apiVersion: v1\nkind: Secret\nmetadata:\n name: test2", + }, + }, + { + name: "documents with empty sections", + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test1 +--- + +--- +apiVersion: v1 +kind: Secret +metadata: + name: test2`, + expected: []string{ + "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test1", + "apiVersion: v1\nkind: Secret\nmetadata:\n name: test2", + }, + }, + { + name: "documents with comments only", + input: `apiVersion: v1 +kind: ConfigMap +metadata: + name: test1 +--- +# This is just a comment +# Another comment +--- +apiVersion: v1 +kind: Secret +metadata: + name: test2`, + expected: []string{ + "apiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test1", + "apiVersion: v1\nkind: Secret\nmetadata:\n name: test2", + }, + }, + { + name: "documents with mixed content", + input: `# Header comment +apiVersion: v1 +kind: ConfigMap +metadata: + name: test1 + # inline comment +data: + key: value +--- +apiVersion: v1 +kind: Secret +metadata: + name: test2 +type: Opaque`, + expected: []string{ + "# Header comment\napiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test1\n # inline comment\ndata:\n key: value", + "apiVersion: v1\nkind: Secret\nmetadata:\n name: test2\ntype: Opaque", + }, + }, + { + name: "documents with leading separator", + input: `--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test1 +--- +apiVersion: v1 +kind: Secret +metadata: + name: test2`, + expected: []string{ + "---\napiVersion: v1\nkind: ConfigMap\nmetadata:\n name: test1", + "apiVersion: v1\nkind: Secret\nmetadata:\n name: test2", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := splitYAMLDocuments(tt.input) + if !reflect.DeepEqual(result, tt.expected) { + t.Errorf("splitYAMLDocuments() = %v, want %v", result, tt.expected) + } + }) + } +} + +func TestGenerateFilename(t *testing.T) { + tests := []struct { + name string + resource K8sResource + expected string + }{ + { + name: "basic resource without namespace", + resource: K8sResource{ + Kind: "ConfigMap", + Metadata: struct { + Name string `yaml:"name"` + Namespace string `yaml:"namespace,omitempty"` + }{ + Name: "my-config", + }, + }, + expected: "configmap-my-config.yaml", + }, + { + name: "resource with namespace", + resource: K8sResource{ + Kind: "Secret", + Metadata: struct { + Name string `yaml:"name"` + Namespace string `yaml:"namespace,omitempty"` + }{ + Name: "my-secret", + Namespace: "kube-system", + }, + }, + expected: "kube-system-secret-my-secret.yaml", + }, + { + name: "resource with special characters", + resource: K8sResource{ + Kind: "ServiceAccount", + Metadata: struct { + Name string `yaml:"name"` + Namespace string `yaml:"namespace,omitempty"` + }{ + Name: "test@user_account", + Namespace: "my-namespace!", + }, + }, + expected: "my-namespace--serviceaccount-test-user-account.yaml", + }, + { + name: "cluster-scoped resource", + resource: K8sResource{ + Kind: "ClusterRole", + Metadata: struct { + Name string `yaml:"name"` + Namespace string `yaml:"namespace,omitempty"` + }{ + Name: "cluster-admin", + }, + }, + expected: "clusterrole-cluster-admin.yaml", + }, + { + name: "custom resource", + resource: K8sResource{ + Kind: "MyCustomResource", + Metadata: struct { + Name string `yaml:"name"` + Namespace string `yaml:"namespace,omitempty"` + }{ + Name: "custom-instance", + Namespace: "default", + }, + }, + expected: "default-mycustomresource-custom-instance.yaml", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := generateFilename(tt.resource) + if result != tt.expected { + t.Errorf("generateFilename() = %v, want %v", result, tt.expected) + } + }) + } +} + +func TestSanitizeFilename(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "valid filename unchanged", + input: "valid-filename.yaml", + expected: "valid-filename.yaml", + }, + { + name: "replace special characters", + input: "my@file$name&test", + expected: "my-file-name-test", + }, + { + name: "replace spaces", + input: "file name with spaces", + expected: "file-name-with-spaces", + }, + { + name: "replace slashes", + input: "path/to/file", + expected: "path-to-file", + }, + { + name: "preserve valid characters", + input: "file123-name.test", + expected: "file123-name.test", + }, + { + name: "unicode characters", + input: "файл名前", + expected: "------", + }, + { + name: "mixed valid and invalid", + input: "test_file@2023.yaml", + expected: "test-file-2023.yaml", + }, + { + name: "empty string", + input: "", + expected: "", + }, + { + name: "only special characters", + input: "@#$%^&*()", + expected: "---------", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := sanitizeFilename(tt.input) + if result != tt.expected { + t.Errorf("sanitizeFilename() = %v, want %v", result, tt.expected) + } + }) + } +} + +func TestWriteDocument(t *testing.T) { + tests := []struct { + name string + content string + expectPrefix bool + expectNewline bool + expectedOutput string + }{ + { + name: "content without document separator", + content: "apiVersion: v1\nkind: ConfigMap", + expectPrefix: true, + expectNewline: true, + expectedOutput: "---\napiVersion: v1\nkind: ConfigMap\n", + }, + { + name: "content with document separator", + content: "---\napiVersion: v1\nkind: ConfigMap", + expectPrefix: false, + expectNewline: true, + expectedOutput: "---\napiVersion: v1\nkind: ConfigMap\n", + }, + { + name: "content with trailing newline", + content: "apiVersion: v1\nkind: ConfigMap\n", + expectPrefix: true, + expectNewline: false, + expectedOutput: "---\napiVersion: v1\nkind: ConfigMap\n", + }, + { + name: "empty content", + content: "", + expectPrefix: true, + expectNewline: true, + expectedOutput: "---\n\n", + }, + { + name: "single line content", + content: "test: value", + expectPrefix: true, + expectNewline: true, + expectedOutput: "---\ntest: value\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := t.TempDir() + filePath := filepath.Join(tmpDir, "test.yaml") + + err := writeDocument(filePath, tt.content) + if err != nil { + t.Fatalf("writeDocument() error = %v", err) + } + + // Read the written file + data, err := os.ReadFile(filePath) + if err != nil { + t.Fatalf("Failed to read written file: %v", err) + } + + result := string(data) + if result != tt.expectedOutput { + t.Errorf("writeDocument() output = %q, want %q", result, tt.expectedOutput) + } + + // Verify document separator behavior + if tt.expectPrefix && !strings.HasPrefix(result, "---\n") { + t.Errorf("Expected document separator prefix, but not found") + } + + // Verify newline behavior + if tt.expectNewline && !strings.HasSuffix(result, "\n") { + t.Errorf("Expected trailing newline, but not found") + } + }) + } +} + +func TestWriteDocument_FileErrors(t *testing.T) { + t.Run("invalid directory", func(t *testing.T) { + err := writeDocument("/invalid/directory/file.yaml", "content") + if err == nil { + t.Error("Expected error when writing to invalid directory") + } + }) + + t.Run("read-only directory", func(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("Skipping test when running as root") + } + + tmpDir := t.TempDir() + + // Make directory read-only + err := os.Chmod(tmpDir, 0444) + if err != nil { + t.Fatalf("Failed to make directory read-only: %v", err) + } + + // Restore permissions for cleanup + defer func() { + _ = os.Chmod(tmpDir, 0755) + }() + + filePath := filepath.Join(tmpDir, "test.yaml") + err = writeDocument(filePath, "content") + if err == nil { + t.Error("Expected error when writing to read-only directory") + } + }) +} + +func TestK8sResourceStructure(t *testing.T) { + t.Run("yaml unmarshaling", func(t *testing.T) { + yamlContent := ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-deployment + namespace: test-namespace +` + var resource K8sResource + err := yaml3.Unmarshal([]byte(yamlContent), &resource) + if err != nil { + t.Fatalf("Failed to unmarshal YAML: %v", err) + } + + if resource.APIVersion != "apps/v1" { + t.Errorf("Expected APIVersion 'apps/v1', got %q", resource.APIVersion) + } + if resource.Kind != "Deployment" { + t.Errorf("Expected Kind 'Deployment', got %q", resource.Kind) + } + if resource.Metadata.Name != "test-deployment" { + t.Errorf("Expected Name 'test-deployment', got %q", resource.Metadata.Name) + } + if resource.Metadata.Namespace != "test-namespace" { + t.Errorf("Expected Namespace 'test-namespace', got %q", resource.Metadata.Namespace) + } + }) + + t.Run("yaml marshaling", func(t *testing.T) { + resource := K8sResource{ + APIVersion: "v1", + Kind: "ConfigMap", + Metadata: struct { + Name string `yaml:"name"` + Namespace string `yaml:"namespace,omitempty"` + }{ + Name: "test-config", + Namespace: "default", + }, + } + + data, err := yaml3.Marshal(&resource) + if err != nil { + t.Fatalf("Failed to marshal YAML: %v", err) + } + + yamlStr := string(data) + if !strings.Contains(yamlStr, "apiVersion: v1") { + t.Error("Marshaled YAML missing apiVersion") + } + if !strings.Contains(yamlStr, "kind: ConfigMap") { + t.Error("Marshaled YAML missing kind") + } + if !strings.Contains(yamlStr, "name: test-config") { + t.Error("Marshaled YAML missing name") + } + if !strings.Contains(yamlStr, "namespace: default") { + t.Error("Marshaled YAML missing namespace") + } + }) +} + +func TestDocumentInfo(t *testing.T) { + resource := K8sResource{ + Kind: "ConfigMap", + Metadata: struct { + Name string `yaml:"name"` + Namespace string `yaml:"namespace,omitempty"` + }{ + Name: "test", + }, + } + + doc := DocumentInfo{ + Resource: resource, + Text: "apiVersion: v1\nkind: ConfigMap", + Order: 5, + } + + if doc.Resource.Kind != "ConfigMap" { + t.Errorf("Expected Kind 'ConfigMap', got %q", doc.Resource.Kind) + } + if doc.Order != 5 { + t.Errorf("Expected Order 5, got %d", doc.Order) + } + if !strings.Contains(doc.Text, "ConfigMap") { + t.Error("Expected Text to contain 'ConfigMap'") + } +} diff --git a/pkg/helmvalues/helmvalues.go b/pkg/helmvalues/helmvalues.go new file mode 100644 index 000000000..523f6c214 --- /dev/null +++ b/pkg/helmvalues/helmvalues.go @@ -0,0 +1,108 @@ +package helmvalues + +import ( + "errors" + "fmt" + "os" + "slices" + "strings" + + "github.com/TwiN/deepmerge" + + yaml3 "gopkg.in/yaml.v3" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +const ( + // Helm Values Locations + EnableOperatorController = "options.operatorController.features.enabled" + DisableOperatorController = "options.operatorController.features.disabled" + EnableCatalogd = "options.catalogd.features.enabled" + DisableCatalogd = "options.catalogd.features.disabled" +) + +type HelmValues struct { + values map[string]interface{} +} + +func NewHelmValues() *HelmValues { + return &HelmValues{ + values: make(map[string]interface{}), + } +} + +func NewHelmValuesFromFiles(files []string) (*HelmValues, error) { + values := NewHelmValues() + for _, a := range files { + newvalues := make(map[string]any) + data, err := os.ReadFile(a) + if err != nil { + return nil, err + } + if err := yaml3.Unmarshal(data, newvalues); err != nil { + return nil, err + } + if err := deepmerge.DeepMerge(values.values, newvalues, deepmerge.Config{}); err != nil { + return nil, err + } + } + return values, nil +} + +func (v *HelmValues) GetValues() map[string]interface{} { + return v.values +} + +func (v *HelmValues) HasEnabledFeatureGates() (bool, error) { + ss := strings.Split(EnableOperatorController, ".") + values, found, err := unstructured.NestedStringSlice(v.values, ss...) + if err != nil { + return false, err + } + if found && len(values) > 0 { + return true, nil + } + ss = strings.Split(EnableCatalogd, ".") + values, found, err = unstructured.NestedStringSlice(v.values, ss...) + if err != nil { + return false, err + } + if found && len(values) > 0 { + return true, nil + } + return false, nil +} + +func (v *HelmValues) SetStringValue(location string, newValue string) error { + if location == "" { + return errors.New("location string has no locations") + } + ss := strings.Split(location, ".") + + return unstructured.SetNestedField(v.values, newValue, ss...) +} + +func (v *HelmValues) AddListValue(location string, newValue string) error { + if location == "" { + return errors.New("location string has no locations") + } + ss := strings.Split(location, ".") + values, found, err := unstructured.NestedStringSlice(v.values, ss...) + if err != nil { + return err + } + if found { + if slices.Index(values, newValue) != -1 { + return fmt.Errorf("newValue=%q already present in values=%v", newValue, values) + } + values = append(values, newValue) + slices.Sort(values) + return unstructured.SetNestedStringSlice(v.values, values, ss...) + } + return unstructured.SetNestedStringSlice(v.values, []string{newValue}, ss...) +} + +func (v *HelmValues) AddValues(newValues *HelmValues) error { + return deepmerge.DeepMerge(v.values, newValues.values, deepmerge.Config{}) +} diff --git a/pkg/helmvalues/helmvalues_test.go b/pkg/helmvalues/helmvalues_test.go new file mode 100644 index 000000000..0e2a2fbbf --- /dev/null +++ b/pkg/helmvalues/helmvalues_test.go @@ -0,0 +1,501 @@ +package helmvalues + +import ( + "os" + "path/filepath" + "reflect" + "testing" +) + +func TestNewHelmValues(t *testing.T) { + hv := NewHelmValues() + if hv == nil { + t.Fatal("NewHelmValues returned nil") + } + if hv.values == nil { + t.Errorf("Expected values to be initialized, got nil") + } + if len(hv.values) != 0 { + t.Errorf("Expected empty values map, got %v", hv.values) + } +} + +func TestNewHelmValuesFromFiles(t *testing.T) { + tests := []struct { + name string + files []string + setupFiles map[string]string + expectedError bool + expectedVals map[string]interface{} + }{ + { + name: "empty files list", + files: []string{}, + expectedError: false, + expectedVals: map[string]interface{}{}, + }, + { + name: "single valid yaml file", + files: []string{"test1.yaml"}, + setupFiles: map[string]string{ + "test1.yaml": ` +options: + operatorController: + features: + enabled: ["feature1", "feature2"] +`, + }, + expectedError: false, + expectedVals: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"feature1", "feature2"}, + }, + }, + }, + }, + }, + { + name: "multiple yaml files with merge", + files: []string{"test1.yaml", "test2.yaml"}, + setupFiles: map[string]string{ + "test1.yaml": ` +options: + operatorController: + features: + enabled: ["feature1"] +`, + "test2.yaml": ` +options: + operatorController: + features: + disabled: ["feature2"] +`, + }, + expectedError: false, + expectedVals: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"feature1"}, + "disabled": []interface{}{"feature2"}, + }, + }, + }, + }, + }, + { + name: "nonexistent file", + files: []string{"nonexistent.yaml"}, + expectedError: true, + }, + { + name: "invalid yaml", + files: []string{"invalid.yaml"}, + setupFiles: map[string]string{ + "invalid.yaml": `invalid: yaml: content: [`, + }, + expectedError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := t.TempDir() + var filePaths []string + + for filename, content := range tt.setupFiles { + path := filepath.Join(tmpDir, filename) + if err := os.WriteFile(path, []byte(content), 0600); err != nil { + t.Fatalf("Failed to create test file %s: %v", filename, err) + } + filePaths = append(filePaths, path) + } + + if len(tt.files) > 0 && len(tt.setupFiles) == 0 { + for _, f := range tt.files { + filePaths = append(filePaths, filepath.Join(tmpDir, f)) + } + } + + hv, err := NewHelmValuesFromFiles(filePaths) + + if tt.expectedError { + if err == nil { + t.Errorf("Expected error, got nil") + } + return + } + + if err != nil { + t.Errorf("Unexpected error: %v", err) + return + } + + if !reflect.DeepEqual(hv.values, tt.expectedVals) { + t.Errorf("Expected values %v, got %v", tt.expectedVals, hv.values) + } + }) + } +} + +func TestGetValues(t *testing.T) { + hv := NewHelmValues() + values := hv.GetValues() + if len(values) != 0 { + t.Errorf("Expected empty values, got %v", values) + } + + hv.values = map[string]interface{}{ + "key": "value", + } + values = hv.GetValues() + expected := map[string]interface{}{ + "key": "value", + } + if !reflect.DeepEqual(values, expected) { + t.Errorf("Expected %v, got %v", expected, values) + } +} + +func TestHasEnabledFeatureGates(t *testing.T) { + tests := []struct { + name string + values map[string]interface{} + expected bool + hasError bool + }{ + { + name: "nil values", + values: nil, + expected: false, + hasError: false, + }, + { + name: "empty values", + values: map[string]interface{}{}, + expected: false, + hasError: false, + }, + { + name: "enabled operator controller features", + values: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"feature1", "feature2"}, + }, + }, + }, + }, + expected: true, + hasError: false, + }, + { + name: "empty enabled operator controller features", + values: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{}, + }, + }, + }, + }, + expected: false, + hasError: false, + }, + { + name: "enabled catalogd features", + values: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"catalogd-feature"}, + }, + }, + }, + }, + expected: true, + hasError: false, + }, + { + name: "only disabled features", + values: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "disabled": []interface{}{"feature1"}, + }, + }, + }, + }, + expected: false, + hasError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hv := NewHelmValues() + if tt.values != nil { + hv.values = tt.values + } + result, err := hv.HasEnabledFeatureGates() + + if tt.hasError && err == nil { + t.Errorf("Expected error, got nil") + } + if !tt.hasError && err != nil { + t.Errorf("Unexpected error: %v", err) + } + if result != tt.expected { + t.Errorf("Expected %v, got %v", tt.expected, result) + } + }) + } +} + +func TestSetStringValue(t *testing.T) { + tests := []struct { + name string + location string + value string + expectError bool + expectedVal map[string]interface{} + }{ + { + name: "empty location", + location: "", + value: "test", + expectError: true, + }, + { + name: "simple location", + location: "key", + value: "value", + expectError: false, + expectedVal: map[string]interface{}{ + "key": "value", + }, + }, + { + name: "nested location", + location: "options.operatorController.name", + value: "test-controller", + expectError: false, + expectedVal: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "name": "test-controller", + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hv := NewHelmValues() + hv.values = make(map[string]interface{}) + + err := hv.SetStringValue(tt.location, tt.value) + + if tt.expectError && err == nil { + t.Errorf("Expected error, got nil") + } + if !tt.expectError && err != nil { + t.Errorf("Unexpected error: %v", err) + } + if !tt.expectError && !reflect.DeepEqual(hv.values, tt.expectedVal) { + t.Errorf("Expected %v, got %v", tt.expectedVal, hv.values) + } + }) + } +} + +func TestAddListValue(t *testing.T) { + tests := []struct { + name string + initialVals map[string]interface{} + location string + value string + expectError bool + expectedVals map[string]interface{} + }{ + { + name: "empty location", + location: "", + value: "test", + expectError: true, + }, + { + name: "add to new location", + initialVals: make(map[string]interface{}), + location: "options.features.enabled", + value: "feature1", + expectError: false, + expectedVals: map[string]interface{}{ + "options": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"feature1"}, + }, + }, + }, + }, + { + name: "add to existing list", + initialVals: map[string]interface{}{ + "options": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"feature1"}, + }, + }, + }, + location: "options.features.enabled", + value: "feature2", + expectError: false, + expectedVals: map[string]interface{}{ + "options": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"feature1", "feature2"}, + }, + }, + }, + }, + { + name: "add duplicate value", + initialVals: map[string]interface{}{ + "options": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"feature1"}, + }, + }, + }, + location: "options.features.enabled", + value: "feature1", + expectError: true, + }, + { + name: "add with sorting", + initialVals: map[string]interface{}{ + "options": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"feature2", "feature1"}, + }, + }, + }, + location: "options.features.enabled", + value: "feature0", + expectError: false, + expectedVals: map[string]interface{}{ + "options": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"feature0", "feature1", "feature2"}, + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hv := NewHelmValues() + hv.values = tt.initialVals + + err := hv.AddListValue(tt.location, tt.value) + + if tt.expectError && err == nil { + t.Errorf("Expected error, got nil") + } + if !tt.expectError && err != nil { + t.Errorf("Unexpected error: %v", err) + } + if !tt.expectError && !reflect.DeepEqual(hv.values, tt.expectedVals) { + t.Errorf("Expected %v, got %v", tt.expectedVals, hv.values) + } + }) + } +} + +func TestAddValues(t *testing.T) { + tests := []struct { + name string + initialVals map[string]interface{} + newVals map[string]interface{} + expectedVals map[string]interface{} + }{ + { + name: "add to empty values", + initialVals: make(map[string]interface{}), + newVals: map[string]interface{}{ + "key": "value", + }, + expectedVals: map[string]interface{}{ + "key": "value", + }, + }, + { + name: "merge nested values", + initialVals: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"feature1"}, + }, + }, + }, + }, + newVals: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "disabled": []interface{}{"feature2"}, + }, + }, + }, + }, + expectedVals: map[string]interface{}{ + "options": map[string]interface{}{ + "operatorController": map[string]interface{}{ + "features": map[string]interface{}{ + "enabled": []interface{}{"feature1"}, + "disabled": []interface{}{"feature2"}, + }, + }, + }, + }, + }, + { + name: "overwrite values", + initialVals: map[string]interface{}{ + "key": "oldvalue", + }, + newVals: map[string]interface{}{ + "key": "newvalue", + }, + expectedVals: map[string]interface{}{ + "key": "newvalue", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hv := NewHelmValues() + hv.values = tt.initialVals + newHv := NewHelmValues() + newHv.values = tt.newVals + + err := hv.AddValues(newHv) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + if !reflect.DeepEqual(hv.values, tt.expectedVals) { + t.Errorf("Expected %v, got %v", tt.expectedVals, hv.values) + } + }) + } +}