From 69eaa825a0ab49a3f7afebd899331737b42ac3ea Mon Sep 17 00:00:00 2001 From: Marcin Owsiany Date: Mon, 22 Jun 2026 14:15:53 +0200 Subject: [PATCH 1/4] Add support for operator environment variables Allow overriding environment variables on the operator deployment, useful for setting RELATED_IMAGE_* vars to test with custom-built component images. Supports both non-OLM (direct injection into the manager container) and OLM (via Subscription spec.config.env) paths. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/deploy.go | 16 ++++ cmd/deploy_test.go | 42 +++++++++ internal/deployer/config.go | 7 +- internal/deployer/operator.go | 51 +++++++++++ internal/deployer/operator_env.go | 52 +++++++++++ internal/deployer/operator_env_test.go | 119 +++++++++++++++++++++++++ internal/deployer/operator_olm.go | 31 +++++-- 7 files changed, 307 insertions(+), 11 deletions(-) create mode 100644 internal/deployer/operator_env.go create mode 100644 internal/deployer/operator_env_test.go diff --git a/cmd/deploy.go b/cmd/deploy.go index 5d14635c..56d544cc 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -197,6 +197,22 @@ Examples: }), ) + registerFlag(cmd, settings, "operator-env", "Operator environment variables (e.g., RELATED_IMAGE_MAIN=quay.io/...)", + withApplyFn("env-var", func(config *deployer.Config, envExpr string) error { + envVars, err := deployer.ParseOperatorEnvVars([]string{envExpr}) + if err != nil { + return fmt.Errorf("parsing operator env vars: %w", err) + } + if config.Operator.EnvVars == nil { + config.Operator.EnvVars = make(map[string]string) + } + for k, v := range envVars { + config.Operator.EnvVars[k] = v + } + return nil + }), + ) + registerFlag(cmd, settings, "features", "Feature flag settings (e.g., +ROX_FOO,-ROX_BAR,ROX_BAZ=true)", withApplyFn("feature-flags", func(config *deployer.Config, featureFlagExpr string) error { featureFlags, err := deployer.ParseFeatureFlags([]string{featureFlagExpr}) diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index 6443b022..63203ab6 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -144,6 +144,32 @@ func TestNewDeployCmd_Flags(t *testing.T) { assert.Equal(t, types.ResourceProfileSmall, cfg.Central.ResourceProfile, "Central.ResourceProfile mismatch") }, }, + { + name: "operator-env single", + args: []string{"--operator-env", "RELATED_IMAGE_MAIN=quay.io/main:4.7.0"}, + assert: func(t *testing.T, cfg deployer.Config) { + require.NotNil(t, cfg.Operator.EnvVars, "Operator.EnvVars should be set") + assert.Equal(t, "quay.io/main:4.7.0", cfg.Operator.EnvVars["RELATED_IMAGE_MAIN"]) + }, + }, + { + name: "operator-env comma separated", + args: []string{"--operator-env", "FOO=bar,BAZ=qux"}, + assert: func(t *testing.T, cfg deployer.Config) { + require.NotNil(t, cfg.Operator.EnvVars, "Operator.EnvVars should be set") + assert.Equal(t, "bar", cfg.Operator.EnvVars["FOO"]) + assert.Equal(t, "qux", cfg.Operator.EnvVars["BAZ"]) + }, + }, + { + name: "operator-env multiple flags", + args: []string{"--operator-env", "FOO=bar", "--operator-env", "BAZ=qux"}, + assert: func(t *testing.T, cfg deployer.Config) { + require.NotNil(t, cfg.Operator.EnvVars, "Operator.EnvVars should be set") + assert.Equal(t, "bar", cfg.Operator.EnvVars["FOO"]) + assert.Equal(t, "qux", cfg.Operator.EnvVars["BAZ"]) + }, + }, { name: "config file can be used", config: ` @@ -166,6 +192,22 @@ securedCluster: }, }, + { + name: "config file with operator env vars", + config: ` +operator: + envVars: + RELATED_IMAGE_MAIN: quay.io/rhacs-eng/main:4.7.0 + RELATED_IMAGE_SCANNER: quay.io/rhacs-eng/scanner:4.7.0 +`, + args: []string{"--config", configFilePath}, + assert: func(t *testing.T, cfg deployer.Config) { + require.NotNil(t, cfg.Operator.EnvVars, "Operator.EnvVars should be set") + assert.Equal(t, "quay.io/rhacs-eng/main:4.7.0", cfg.Operator.EnvVars["RELATED_IMAGE_MAIN"]) + assert.Equal(t, "quay.io/rhacs-eng/scanner:4.7.0", cfg.Operator.EnvVars["RELATED_IMAGE_SCANNER"]) + }, + }, + { name: "config file can disable early-readiness", config: ` diff --git a/internal/deployer/config.go b/internal/deployer/config.go index 01e541eb..ea0bd827 100644 --- a/internal/deployer/config.go +++ b/internal/deployer/config.go @@ -76,9 +76,10 @@ func NewRoxieConfig() RoxieConfig { // OperatorConfig controls how the ACS operator is deployed. type OperatorConfig struct { - SkipDeployment *bool `yaml:"skipDeployment,omitempty"` - DeployViaOlm *bool `yaml:"deployViaOlm,omitempty"` - Version string `yaml:"version,omitempty"` + SkipDeployment *bool `yaml:"skipDeployment,omitempty"` + DeployViaOlm *bool `yaml:"deployViaOlm,omitempty"` + Version string `yaml:"version,omitempty"` + EnvVars map[string]string `yaml:"envVars,omitempty"` } func (c *OperatorConfig) SkipDeploymentSet() bool { diff --git a/internal/deployer/operator.go b/internal/deployer/operator.go index f271a4a6..75b30b86 100644 --- a/internal/deployer/operator.go +++ b/internal/deployer/operator.go @@ -324,6 +324,13 @@ func (d *Deployer) deployOperatorFromCSV(ctx context.Context, bundleDir string) d.logger.Dim(fmt.Sprintf(" • Namespace: %s", operatorNamespace)) d.logger.Dim(fmt.Sprintf(" • ServiceAccount: %s", serviceAccountName)) d.logger.Dim(fmt.Sprintf(" • Setting up pull secrets: %v", d.useOperatorPullSecrets)) + if len(d.config.Operator.EnvVars) > 0 { + d.logger.Dim(fmt.Sprintf(" • Custom operator env vars: %d", len(d.config.Operator.EnvVars))) + for _, envVar := range operatorEnvVarsToSortedList(d.config.Operator.EnvVars) { + ev := envVar.(map[string]interface{}) + d.logger.Dim(fmt.Sprintf(" %s=%s", ev["name"], ev["value"])) + } + } if err := d.prepareNamespace(ctx, operatorNamespace, d.useOperatorPullSecrets); err != nil { return err @@ -530,6 +537,12 @@ func (d *Deployer) createDeploymentFromCSV(ctx context.Context, namespace string if template, ok := spec["template"].(map[string]interface{}); ok { if podSpec, ok := template["spec"].(map[string]interface{}); ok { podSpec["serviceAccountName"] = deploymentSpec["service_account"] + + if len(d.config.Operator.EnvVars) > 0 { + if containers, ok := podSpec["containers"].([]interface{}); ok { + d.injectEnvVarsIntoManagerContainer(containers) + } + } } } @@ -548,6 +561,44 @@ func (d *Deployer) createDeploymentFromCSV(ctx context.Context, namespace string return nil } +const managerContainerName = "manager" + +// injectEnvVarsIntoManagerContainer merges configured operator env vars into +// the manager container, overriding any existing env vars with the same name. +func (d *Deployer) injectEnvVarsIntoManagerContainer(containers []interface{}) { + for _, c := range containers { + container, ok := c.(map[string]interface{}) + if !ok { + continue + } + if container["name"] != managerContainerName { + continue + } + + existing := make(map[string]int) + envList, _ := container["env"].([]interface{}) + for i, item := range envList { + if envVar, ok := item.(map[string]interface{}); ok { + if name, ok := envVar["name"].(string); ok { + existing[name] = i + } + } + } + + for _, envVar := range operatorEnvVarsToSortedList(d.config.Operator.EnvVars) { + name := envVar.(map[string]interface{})["name"].(string) + if idx, found := existing[name]; found { + envList[idx] = envVar + } else { + envList = append(envList, envVar) + } + } + + container["env"] = envList + return + } +} + func (d *Deployer) applyBundleServiceResources(ctx context.Context, bundleDir, namespace string) error { serviceFile := filepath.Join(bundleDir, "rhacs-operator-controller-manager-metrics-service_v1_service.yaml") if _, err := os.Stat(serviceFile); err == nil { diff --git a/internal/deployer/operator_env.go b/internal/deployer/operator_env.go new file mode 100644 index 00000000..6fb10b08 --- /dev/null +++ b/internal/deployer/operator_env.go @@ -0,0 +1,52 @@ +package deployer + +import ( + "fmt" + "sort" + "strings" +) + +// ParseOperatorEnvVars parses a slice of operator environment variable strings. +// Each string can contain one or more comma-separated KEY=VALUE pairs. +// Values may contain '=' characters (only the first '=' is used as the separator). +func ParseOperatorEnvVars(envExprs []string) (map[string]string, error) { + result := make(map[string]string) + + for _, expr := range envExprs { + for _, part := range strings.Split(expr, ",") { + part = strings.TrimSpace(part) + if part == "" { + continue + } + + key, value, found := strings.Cut(part, "=") + if !found { + return nil, fmt.Errorf("invalid operator env var %q: expected KEY=VALUE format", part) + } + key = strings.TrimSpace(key) + if key == "" { + return nil, fmt.Errorf("invalid operator env var %q: empty key", part) + } + result[key] = value + } + } + + return result, nil +} + +// operatorEnvVarsToSortedList converts a map of env vars to a sorted list of +// {name, value} maps suitable for use in Kubernetes container or OLM Subscription specs. +func operatorEnvVarsToSortedList(envVars map[string]string) []interface{} { + result := make([]interface{}, 0, len(envVars)) + for name, value := range envVars { + result = append(result, map[string]interface{}{ + "name": name, + "value": value, + }) + } + sort.Slice(result, func(i, j int) bool { + return result[i].(map[string]interface{})["name"].(string) < + result[j].(map[string]interface{})["name"].(string) + }) + return result +} diff --git a/internal/deployer/operator_env_test.go b/internal/deployer/operator_env_test.go new file mode 100644 index 00000000..8d3a110d --- /dev/null +++ b/internal/deployer/operator_env_test.go @@ -0,0 +1,119 @@ +package deployer + +import ( + "reflect" + "testing" +) + +func TestParseOperatorEnvVars(t *testing.T) { + tests := []struct { + name string + input []string + expected map[string]string + expectError bool + }{ + { + name: "single KEY=VALUE", + input: []string{"RELATED_IMAGE_MAIN=quay.io/rhacs-eng/main:4.7.0"}, + expected: map[string]string{"RELATED_IMAGE_MAIN": "quay.io/rhacs-eng/main:4.7.0"}, + }, + { + name: "comma-separated pairs", + input: []string{"RELATED_IMAGE_MAIN=quay.io/main:4.7.0,RELATED_IMAGE_SCANNER=quay.io/scanner:4.7.0"}, + expected: map[string]string{ + "RELATED_IMAGE_MAIN": "quay.io/main:4.7.0", + "RELATED_IMAGE_SCANNER": "quay.io/scanner:4.7.0", + }, + }, + { + name: "multiple array elements", + input: []string{"FOO=bar", "BAZ=qux"}, + expected: map[string]string{ + "FOO": "bar", + "BAZ": "qux", + }, + }, + { + name: "value containing equals sign", + input: []string{"MY_VAR=a=b=c"}, + expected: map[string]string{"MY_VAR": "a=b=c"}, + }, + { + name: "empty value", + input: []string{"MY_VAR="}, + expected: map[string]string{"MY_VAR": ""}, + }, + { + name: "duplicate key last wins", + input: []string{"FOO=old", "FOO=new"}, + expected: map[string]string{"FOO": "new"}, + }, + { + name: "empty string input", + input: []string{""}, + expected: map[string]string{}, + }, + { + name: "empty elements in comma list", + input: []string{"FOO=bar,,,BAZ=qux"}, + expected: map[string]string{"FOO": "bar", "BAZ": "qux"}, + }, + { + name: "whitespace around pairs trimmed", + input: []string{" FOO=bar , BAZ=qux "}, + expected: map[string]string{ + "FOO": "bar", + "BAZ": "qux", + }, + }, + { + name: "missing equals sign", + input: []string{"NO_VALUE"}, + expectError: true, + }, + { + name: "empty key", + input: []string{"=value"}, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ParseOperatorEnvVars(tt.input) + if tt.expectError { + if err == nil { + t.Errorf("expected error but got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !reflect.DeepEqual(result, tt.expected) { + t.Errorf("got %v, want %v", result, tt.expected) + } + }) + } +} + +func TestOperatorEnvVarsToSortedList(t *testing.T) { + input := map[string]string{ + "ZZZ": "z", + "AAA": "a", + "MMM": "m", + } + result := operatorEnvVarsToSortedList(input) + + if len(result) != 3 { + t.Fatalf("expected 3 items, got %d", len(result)) + } + + expectedOrder := []string{"AAA", "MMM", "ZZZ"} + for i, item := range result { + name := item.(map[string]interface{})["name"].(string) + if name != expectedOrder[i] { + t.Errorf("index %d: got %q, want %q", i, name, expectedOrder[i]) + } + } +} diff --git a/internal/deployer/operator_olm.go b/internal/deployer/operator_olm.go index 9428cac4..4043713c 100644 --- a/internal/deployer/operator_olm.go +++ b/internal/deployer/operator_olm.go @@ -33,6 +33,13 @@ const ( func (d *Deployer) deployOperatorViaOLM(ctx context.Context) error { d.logger.Info("🚀 Deploying operator via OLM...") d.logger.Infof("Operator tag: %s", d.config.Operator.Version) + if len(d.config.Operator.EnvVars) > 0 { + d.logger.Infof("Custom operator env vars: %d", len(d.config.Operator.EnvVars)) + for _, envVar := range operatorEnvVarsToSortedList(d.config.Operator.EnvVars) { + ev := envVar.(map[string]interface{}) + d.logger.Dim(fmt.Sprintf(" %s=%s", ev["name"], ev["value"])) + } + } if err := d.checkOLMInstalled(ctx); err != nil { return err @@ -200,6 +207,21 @@ func (d *Deployer) createSubscription(ctx context.Context) error { startingCSV := fmt.Sprintf("rhacs-operator.v%s", d.config.Operator.Version) + subscriptionSpec := map[string]interface{}{ + "channel": operatorChannel, + "name": "rhacs-operator", + "source": catalogSourceName, + "sourceNamespace": operatorNamespace, + "installPlanApproval": "Manual", + "startingCSV": startingCSV, + } + + if len(d.config.Operator.EnvVars) > 0 { + subscriptionSpec["config"] = map[string]interface{}{ + "env": operatorEnvVarsToSortedList(d.config.Operator.EnvVars), + } + } + subscription := map[string]interface{}{ "apiVersion": "operators.coreos.com/v1alpha1", "kind": "Subscription", @@ -207,14 +229,7 @@ func (d *Deployer) createSubscription(ctx context.Context) error { "name": subscriptionName, "namespace": operatorNamespace, }, - "spec": map[string]interface{}{ - "channel": operatorChannel, - "name": "rhacs-operator", - "source": catalogSourceName, - "sourceNamespace": operatorNamespace, - "installPlanApproval": "Manual", - "startingCSV": startingCSV, - }, + "spec": subscriptionSpec, } yamlData, err := yaml.Marshal(subscription) From dfbad9a172e4537fb8308c430b5b69cd2ccb21d7 Mon Sep 17 00:00:00 2001 From: Marcin Owsiany Date: Tue, 23 Jun 2026 14:06:38 +0200 Subject: [PATCH 2/4] Address review feedback for operator env vars - Rename operatorEnvVarsToSortedList to envVarsToSortedList - Simplify ParseOperatorEnvVars to ParseOperatorEnvVar taking a single KEY=VALUE string (no comma splitting or space trimming) - Use Dimf instead of Dim(fmt.Sprintf(...)) - Switch tests to use testify require/assert Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/deploy.go | 8 +- internal/deployer/operator.go | 14 ++-- internal/deployer/operator_env.go | 37 +++------ internal/deployer/operator_env_test.go | 107 +++++++++---------------- internal/deployer/operator_olm.go | 6 +- 5 files changed, 63 insertions(+), 109 deletions(-) diff --git a/cmd/deploy.go b/cmd/deploy.go index 56d544cc..a2f631d4 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -199,16 +199,14 @@ Examples: registerFlag(cmd, settings, "operator-env", "Operator environment variables (e.g., RELATED_IMAGE_MAIN=quay.io/...)", withApplyFn("env-var", func(config *deployer.Config, envExpr string) error { - envVars, err := deployer.ParseOperatorEnvVars([]string{envExpr}) + key, value, err := deployer.ParseOperatorEnvVar(envExpr) if err != nil { - return fmt.Errorf("parsing operator env vars: %w", err) + return fmt.Errorf("parsing operator env var: %w", err) } if config.Operator.EnvVars == nil { config.Operator.EnvVars = make(map[string]string) } - for k, v := range envVars { - config.Operator.EnvVars[k] = v - } + config.Operator.EnvVars[key] = value return nil }), ) diff --git a/internal/deployer/operator.go b/internal/deployer/operator.go index 75b30b86..d66c9d32 100644 --- a/internal/deployer/operator.go +++ b/internal/deployer/operator.go @@ -321,14 +321,14 @@ func (d *Deployer) deployOperatorFromCSV(ctx context.Context, bundleDir string) d.useOperatorPullSecrets = d.config.Roxie.KonfluxImagesEnabled() && d.config.Roxie.ClusterType.NeedsPullSecrets() d.logger.Info("📋 Operator deployment plan:") - d.logger.Dim(fmt.Sprintf(" • Namespace: %s", operatorNamespace)) - d.logger.Dim(fmt.Sprintf(" • ServiceAccount: %s", serviceAccountName)) - d.logger.Dim(fmt.Sprintf(" • Setting up pull secrets: %v", d.useOperatorPullSecrets)) + d.logger.Dimf(" • Namespace: %s", operatorNamespace) + d.logger.Dimf(" • ServiceAccount: %s", serviceAccountName) + d.logger.Dimf(" • Setting up pull secrets: %v", d.useOperatorPullSecrets) if len(d.config.Operator.EnvVars) > 0 { - d.logger.Dim(fmt.Sprintf(" • Custom operator env vars: %d", len(d.config.Operator.EnvVars))) - for _, envVar := range operatorEnvVarsToSortedList(d.config.Operator.EnvVars) { + d.logger.Dimf(" • Custom operator env vars: %d", len(d.config.Operator.EnvVars)) + for _, envVar := range envVarsToSortedList(d.config.Operator.EnvVars) { ev := envVar.(map[string]interface{}) - d.logger.Dim(fmt.Sprintf(" %s=%s", ev["name"], ev["value"])) + d.logger.Dimf(" %s=%s", ev["name"], ev["value"]) } } @@ -585,7 +585,7 @@ func (d *Deployer) injectEnvVarsIntoManagerContainer(containers []interface{}) { } } - for _, envVar := range operatorEnvVarsToSortedList(d.config.Operator.EnvVars) { + for _, envVar := range envVarsToSortedList(d.config.Operator.EnvVars) { name := envVar.(map[string]interface{})["name"].(string) if idx, found := existing[name]; found { envList[idx] = envVar diff --git a/internal/deployer/operator_env.go b/internal/deployer/operator_env.go index 6fb10b08..7d595a19 100644 --- a/internal/deployer/operator_env.go +++ b/internal/deployer/operator_env.go @@ -6,37 +6,22 @@ import ( "strings" ) -// ParseOperatorEnvVars parses a slice of operator environment variable strings. -// Each string can contain one or more comma-separated KEY=VALUE pairs. +// ParseOperatorEnvVar parses a single KEY=VALUE environment variable string. // Values may contain '=' characters (only the first '=' is used as the separator). -func ParseOperatorEnvVars(envExprs []string) (map[string]string, error) { - result := make(map[string]string) - - for _, expr := range envExprs { - for _, part := range strings.Split(expr, ",") { - part = strings.TrimSpace(part) - if part == "" { - continue - } - - key, value, found := strings.Cut(part, "=") - if !found { - return nil, fmt.Errorf("invalid operator env var %q: expected KEY=VALUE format", part) - } - key = strings.TrimSpace(key) - if key == "" { - return nil, fmt.Errorf("invalid operator env var %q: empty key", part) - } - result[key] = value - } +func ParseOperatorEnvVar(envExpr string) (string, string, error) { + key, value, found := strings.Cut(envExpr, "=") + if !found { + return "", "", fmt.Errorf("invalid operator env var %q: expected KEY=VALUE format", envExpr) } - - return result, nil + if key == "" { + return "", "", fmt.Errorf("invalid operator env var %q: empty key", envExpr) + } + return key, value, nil } -// operatorEnvVarsToSortedList converts a map of env vars to a sorted list of +// envVarsToSortedList converts a map of env vars to a sorted list of // {name, value} maps suitable for use in Kubernetes container or OLM Subscription specs. -func operatorEnvVarsToSortedList(envVars map[string]string) []interface{} { +func envVarsToSortedList(envVars map[string]string) []interface{} { result := make([]interface{}, 0, len(envVars)) for name, value := range envVars { result = append(result, map[string]interface{}{ diff --git a/internal/deployer/operator_env_test.go b/internal/deployer/operator_env_test.go index 8d3a110d..27edaded 100644 --- a/internal/deployer/operator_env_test.go +++ b/internal/deployer/operator_env_test.go @@ -1,119 +1,90 @@ package deployer import ( - "reflect" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestParseOperatorEnvVars(t *testing.T) { +func TestParseOperatorEnvVar(t *testing.T) { tests := []struct { name string - input []string - expected map[string]string + input string + expectedKey string + expectedVal string expectError bool }{ { - name: "single KEY=VALUE", - input: []string{"RELATED_IMAGE_MAIN=quay.io/rhacs-eng/main:4.7.0"}, - expected: map[string]string{"RELATED_IMAGE_MAIN": "quay.io/rhacs-eng/main:4.7.0"}, - }, - { - name: "comma-separated pairs", - input: []string{"RELATED_IMAGE_MAIN=quay.io/main:4.7.0,RELATED_IMAGE_SCANNER=quay.io/scanner:4.7.0"}, - expected: map[string]string{ - "RELATED_IMAGE_MAIN": "quay.io/main:4.7.0", - "RELATED_IMAGE_SCANNER": "quay.io/scanner:4.7.0", - }, - }, - { - name: "multiple array elements", - input: []string{"FOO=bar", "BAZ=qux"}, - expected: map[string]string{ - "FOO": "bar", - "BAZ": "qux", - }, - }, - { - name: "value containing equals sign", - input: []string{"MY_VAR=a=b=c"}, - expected: map[string]string{"MY_VAR": "a=b=c"}, + name: "simple KEY=VALUE", + input: "RELATED_IMAGE_MAIN=quay.io/rhacs-eng/main:4.7.0", + expectedKey: "RELATED_IMAGE_MAIN", + expectedVal: "quay.io/rhacs-eng/main:4.7.0", }, { - name: "empty value", - input: []string{"MY_VAR="}, - expected: map[string]string{"MY_VAR": ""}, + name: "value containing equals sign", + input: "MY_VAR=a=b=c", + expectedKey: "MY_VAR", + expectedVal: "a=b=c", }, { - name: "duplicate key last wins", - input: []string{"FOO=old", "FOO=new"}, - expected: map[string]string{"FOO": "new"}, + name: "empty value", + input: "MY_VAR=", + expectedKey: "MY_VAR", + expectedVal: "", }, { - name: "empty string input", - input: []string{""}, - expected: map[string]string{}, + name: "value with spaces", + input: "MY_VAR= hello world ", + expectedKey: "MY_VAR", + expectedVal: " hello world ", }, { - name: "empty elements in comma list", - input: []string{"FOO=bar,,,BAZ=qux"}, - expected: map[string]string{"FOO": "bar", "BAZ": "qux"}, - }, - { - name: "whitespace around pairs trimmed", - input: []string{" FOO=bar , BAZ=qux "}, - expected: map[string]string{ - "FOO": "bar", - "BAZ": "qux", - }, + name: "value with commas", + input: "MY_VAR=a,b,c", + expectedKey: "MY_VAR", + expectedVal: "a,b,c", }, { name: "missing equals sign", - input: []string{"NO_VALUE"}, + input: "NO_VALUE", expectError: true, }, { name: "empty key", - input: []string{"=value"}, + input: "=value", expectError: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result, err := ParseOperatorEnvVars(tt.input) + key, value, err := ParseOperatorEnvVar(tt.input) if tt.expectError { - if err == nil { - t.Errorf("expected error but got nil") - } + require.Error(t, err) return } - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !reflect.DeepEqual(result, tt.expected) { - t.Errorf("got %v, want %v", result, tt.expected) - } + require.NoError(t, err) + assert.Equal(t, tt.expectedKey, key) + assert.Equal(t, tt.expectedVal, value) }) } } -func TestOperatorEnvVarsToSortedList(t *testing.T) { + +func TestEnvVarsToSortedList(t *testing.T) { input := map[string]string{ "ZZZ": "z", "AAA": "a", "MMM": "m", } - result := operatorEnvVarsToSortedList(input) + result := envVarsToSortedList(input) - if len(result) != 3 { - t.Fatalf("expected 3 items, got %d", len(result)) - } + require.Len(t, result, 3) expectedOrder := []string{"AAA", "MMM", "ZZZ"} for i, item := range result { name := item.(map[string]interface{})["name"].(string) - if name != expectedOrder[i] { - t.Errorf("index %d: got %q, want %q", i, name, expectedOrder[i]) - } + assert.Equal(t, expectedOrder[i], name, "index %d", i) } } diff --git a/internal/deployer/operator_olm.go b/internal/deployer/operator_olm.go index 4043713c..c55c0fca 100644 --- a/internal/deployer/operator_olm.go +++ b/internal/deployer/operator_olm.go @@ -35,9 +35,9 @@ func (d *Deployer) deployOperatorViaOLM(ctx context.Context) error { d.logger.Infof("Operator tag: %s", d.config.Operator.Version) if len(d.config.Operator.EnvVars) > 0 { d.logger.Infof("Custom operator env vars: %d", len(d.config.Operator.EnvVars)) - for _, envVar := range operatorEnvVarsToSortedList(d.config.Operator.EnvVars) { + for _, envVar := range envVarsToSortedList(d.config.Operator.EnvVars) { ev := envVar.(map[string]interface{}) - d.logger.Dim(fmt.Sprintf(" %s=%s", ev["name"], ev["value"])) + d.logger.Dimf(" %s=%s", ev["name"], ev["value"]) } } @@ -218,7 +218,7 @@ func (d *Deployer) createSubscription(ctx context.Context) error { if len(d.config.Operator.EnvVars) > 0 { subscriptionSpec["config"] = map[string]interface{}{ - "env": operatorEnvVarsToSortedList(d.config.Operator.EnvVars), + "env": envVarsToSortedList(d.config.Operator.EnvVars), } } From d59ebd5020b796535ce688e4b38116345ba98879 Mon Sep 17 00:00:00 2001 From: Marcin Owsiany Date: Tue, 23 Jun 2026 14:18:43 +0200 Subject: [PATCH 3/4] Return error from injectEnvVarsIntoManagerContainer on failure Make env var injection fail loudly instead of silently succeeding when the manager container is not found in the deployment spec. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/deployer/operator.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/internal/deployer/operator.go b/internal/deployer/operator.go index d66c9d32..02e39994 100644 --- a/internal/deployer/operator.go +++ b/internal/deployer/operator.go @@ -539,8 +539,12 @@ func (d *Deployer) createDeploymentFromCSV(ctx context.Context, namespace string podSpec["serviceAccountName"] = deploymentSpec["service_account"] if len(d.config.Operator.EnvVars) > 0 { - if containers, ok := podSpec["containers"].([]interface{}); ok { - d.injectEnvVarsIntoManagerContainer(containers) + containers, ok := podSpec["containers"].([]interface{}) + if !ok { + return errors.New("no containers found in deployment pod spec") + } + if err := d.injectEnvVarsIntoManagerContainer(containers); err != nil { + return fmt.Errorf("failed to inject operator env vars: %w", err) } } } @@ -565,7 +569,7 @@ const managerContainerName = "manager" // injectEnvVarsIntoManagerContainer merges configured operator env vars into // the manager container, overriding any existing env vars with the same name. -func (d *Deployer) injectEnvVarsIntoManagerContainer(containers []interface{}) { +func (d *Deployer) injectEnvVarsIntoManagerContainer(containers []interface{}) error { for _, c := range containers { container, ok := c.(map[string]interface{}) if !ok { @@ -595,8 +599,9 @@ func (d *Deployer) injectEnvVarsIntoManagerContainer(containers []interface{}) { } container["env"] = envList - return + return nil } + return fmt.Errorf("container %q not found in deployment", managerContainerName) } func (d *Deployer) applyBundleServiceResources(ctx context.Context, bundleDir, namespace string) error { From 10218096b9c89e8ab5d6d4d446a29ffceee135a4 Mon Sep 17 00:00:00 2001 From: Marcin Owsiany Date: Thu, 25 Jun 2026 07:36:16 +0200 Subject: [PATCH 4/4] trim spaces, fix/add tests --- cmd/deploy_test.go | 8 ++++---- internal/deployer/operator_env.go | 1 + internal/deployer/operator_env_test.go | 12 +++++++++++- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/cmd/deploy_test.go b/cmd/deploy_test.go index 63203ab6..ccf6749f 100644 --- a/cmd/deploy_test.go +++ b/cmd/deploy_test.go @@ -153,12 +153,12 @@ func TestNewDeployCmd_Flags(t *testing.T) { }, }, { - name: "operator-env comma separated", - args: []string{"--operator-env", "FOO=bar,BAZ=qux"}, + name: "operator-env containing commas", + args: []string{"--operator-env", "FOO=bar,BAZ=qux,quux"}, assert: func(t *testing.T, cfg deployer.Config) { require.NotNil(t, cfg.Operator.EnvVars, "Operator.EnvVars should be set") - assert.Equal(t, "bar", cfg.Operator.EnvVars["FOO"]) - assert.Equal(t, "qux", cfg.Operator.EnvVars["BAZ"]) + assert.Equal(t, "bar,BAZ=qux,quux", cfg.Operator.EnvVars["FOO"]) + assert.NotContains(t, cfg.Operator.EnvVars, "BAZ") }, }, { diff --git a/internal/deployer/operator_env.go b/internal/deployer/operator_env.go index 7d595a19..0bcfd84f 100644 --- a/internal/deployer/operator_env.go +++ b/internal/deployer/operator_env.go @@ -10,6 +10,7 @@ import ( // Values may contain '=' characters (only the first '=' is used as the separator). func ParseOperatorEnvVar(envExpr string) (string, string, error) { key, value, found := strings.Cut(envExpr, "=") + key = strings.TrimSpace(key) if !found { return "", "", fmt.Errorf("invalid operator env var %q: expected KEY=VALUE format", envExpr) } diff --git a/internal/deployer/operator_env_test.go b/internal/deployer/operator_env_test.go index 27edaded..3f8d9016 100644 --- a/internal/deployer/operator_env_test.go +++ b/internal/deployer/operator_env_test.go @@ -33,6 +33,12 @@ func TestParseOperatorEnvVar(t *testing.T) { expectedKey: "MY_VAR", expectedVal: "", }, + { + name: "key with spaces", + input: " MY_VAR =foo", + expectedKey: "MY_VAR", + expectedVal: "foo", + }, { name: "value with spaces", input: "MY_VAR= hello world ", @@ -55,6 +61,11 @@ func TestParseOperatorEnvVar(t *testing.T) { input: "=value", expectError: true, }, + { + name: "whitespace key", + input: " =value", + expectError: true, + }, } for _, tt := range tests { @@ -71,7 +82,6 @@ func TestParseOperatorEnvVar(t *testing.T) { } } - func TestEnvVarsToSortedList(t *testing.T) { input := map[string]string{ "ZZZ": "z",