diff --git a/internal/deployer/acs_images.go b/internal/deployer/acs_images.go index f888b75d..48043b0e 100644 --- a/internal/deployer/acs_images.go +++ b/internal/deployer/acs_images.go @@ -1,22 +1,15 @@ package deployer -import ( - "fmt" -) - +// TODO(#303): Also include explicit RELATED_IMAGE_* env var overrides in the preload list. func imagesForConfig(config Config) []string { var images []string - for _, instance := range config.OperatorInstances() { - prefix := "" - if instance.KonfluxImagesEnabled() { - prefix = "release-" + instances, _, _ := config.OperatorInstances() + for _, instance := range instances { + for _, imageName := range relatedImages { + images = append(images, constructOperandImageFromInstance(instance, imageName)) } images = append(images, - fmt.Sprintf("%s/%s%s:%s", instance.ImageRegistry, prefix, "main", instance.Version), - fmt.Sprintf("%s/%s%s:%s", instance.ImageRegistry, prefix, "central-db", instance.Version), - fmt.Sprintf("%s/%s%s:%s", instance.ImageRegistry, prefix, "scanner-v4-db", instance.Version), - fmt.Sprintf("%s/%s%s:%s", instance.ImageRegistry, prefix, "scanner-v4", instance.Version), instance.OperatorImage(), instance.BundleImage(), ) diff --git a/internal/deployer/config.go b/internal/deployer/config.go index 5e68da43..54fe922a 100644 --- a/internal/deployer/config.go +++ b/internal/deployer/config.go @@ -88,9 +88,11 @@ func NewRoxieConfig() RoxieConfig { // is used directly. In mixed-operator mode, CentralConfig.Operator and // SecuredClusterConfig.Operator override the top-level defaults. type OperatorInstanceConfig struct { - Version imagetag.MainTag `yaml:"version,omitempty"` - EnvVars map[string]string `yaml:"envVars,omitempty"` - KonfluxImages *bool `yaml:"konfluxImages,omitempty"` + Version imagetag.MainTag `yaml:"version,omitempty"` + OperandTag imagetag.MainTag `yaml:"operandTag,omitempty"` + OperandTagOverrides map[string]imagetag.MainTag `yaml:"operandTagOverrides,omitempty"` + EnvVars map[string]string `yaml:"envVars,omitempty"` + KonfluxImages *bool `yaml:"konfluxImages,omitempty"` // The following fields are computed internally and are not user-configurable. Namespace string `yaml:"-"` diff --git a/internal/deployer/deploy_via_operator.go b/internal/deployer/deploy_via_operator.go index f5099359..8541dc63 100644 --- a/internal/deployer/deploy_via_operator.go +++ b/internal/deployer/deploy_via_operator.go @@ -59,8 +59,21 @@ func (d *Deployer) ensureOperatorDeployed(ctx context.Context) error { return fmt.Errorf("failed to ensure CRDs installed: %w", err) } - if d.config.Operator.DeployViaOlmEnabled() { - return d.ensureOperatorDeployedOLM(ctx) + instances, olmMode, err := d.config.OperatorInstances() + if err != nil { + return err + } + + if err := d.teardownStaleOperatorNamespaces(ctx, instances); err != nil { + return err + } + + for i := range instances { + populateOperatorEnvVars(&instances[i]) + } + + if olmMode { + return d.ensureOperatorDeployedOLM(ctx, instances[0]) } // Non-OLM case. @@ -76,15 +89,7 @@ func (d *Deployer) ensureOperatorDeployed(ctx context.Context) error { } } - instances := d.config.OperatorInstances() - if err := d.teardownStaleOperatorNamespaces(ctx, instances); err != nil { - return err - } - for _, instance := range instances { - if instance.KonfluxImagesEnabled() { - populateKonfluxEnvVars(&instance) - } if err := d.ensureOperatorInstanceNonOLM(ctx, instance); err != nil { return fmt.Errorf("failed to deploy operator in namespace %s: %w", instance.Namespace, err) } @@ -155,13 +160,7 @@ func (d *Deployer) ensureOperatorInstanceNonOLM(ctx context.Context, instance Op } // ensureOperatorDeployedOLM is the single-operator OLM path (mixed versions are rejected in validation). -func (d *Deployer) ensureOperatorDeployedOLM(ctx context.Context) error { - // Remove any leftover dual-operator namespaces before installing via OLM. - desired := []OperatorInstanceConfig{{Namespace: operatorNamespaceSystem}} - if err := d.teardownStaleOperatorNamespaces(ctx, desired); err != nil { - return err - } - +func (d *Deployer) ensureOperatorDeployedOLM(ctx context.Context, instance OperatorInstanceConfig) error { operatorExists, currentMode, err := d.detectOperatorDeploymentMode(ctx) if err != nil { return fmt.Errorf("detecting operator deployment mode: %w", err) @@ -176,13 +175,10 @@ func (d *Deployer) ensureOperatorDeployedOLM(ctx context.Context) error { needsTeardown = true needsDeployment = true } else { - instance := OperatorInstanceConfig{ - Version: d.config.Operator.Version, - Namespace: operatorNamespaceSystem, - EnvVars: d.config.Operator.EnvVars, - } if d.isOperatorImageCorrect(ctx, instance) { log.Info("✓ Operator already deployed with correct image") + // TODO(#304): Changes in the operator environment configuration do not + // trigger a redeployment of the operator. } else { log.Info("🔄 Operator image mismatch, redeploying...") needsTeardown = true @@ -203,7 +199,7 @@ func (d *Deployer) ensureOperatorDeployedOLM(ctx context.Context) error { } if needsDeployment { - if err := d.deployOperatorViaOLM(ctx); err != nil { + if err := d.deployOperatorViaOLM(ctx, instance); err != nil { return fmt.Errorf("failed to deploy operator via OLM: %w", err) } } diff --git a/internal/deployer/konflux.go b/internal/deployer/konflux.go deleted file mode 100644 index 7291a36b..00000000 --- a/internal/deployer/konflux.go +++ /dev/null @@ -1,41 +0,0 @@ -package deployer - -import ( - "fmt" - - "github.com/stackrox/roxie/internal/constants" -) - -var konfluxRelatedImages = map[string]string{ - "RELATED_IMAGE_MAIN": "main", - "RELATED_IMAGE_CENTRAL_DB": "central-db", - "RELATED_IMAGE_SCANNER": "scanner", - "RELATED_IMAGE_SCANNER_SLIM": "scanner-slim", - "RELATED_IMAGE_SCANNER_DB": "scanner-db", - "RELATED_IMAGE_SCANNER_DB_SLIM": "scanner-db-slim", - "RELATED_IMAGE_COLLECTOR": "collector", - "RELATED_IMAGE_SCANNER_V4_DB": "scanner-v4-db", - "RELATED_IMAGE_SCANNER_V4": "scanner-v4", - "RELATED_IMAGE_FACT": "fact", -} - -// populateKonfluxEnvVars adds RELATED_IMAGE_* entries to the instance's EnvVars -// for its version. Explicitly-provided env vars (e.g. from --operator-env) take -// precedence and are not overwritten. -func populateKonfluxEnvVars(instance *OperatorInstanceConfig) { - if instance.EnvVars == nil { - instance.EnvVars = make(map[string]string, len(konfluxRelatedImages)) - } - operatorTag := instance.Version.ToOperatorTag() - for envName, imageSuffix := range konfluxRelatedImages { - if _, exists := instance.EnvVars[envName]; exists { - continue - } - instance.EnvVars[envName] = fmt.Sprintf( - "%s/release-%s:%s", - constants.DefaultRegistry, - imageSuffix, - operatorTag, // Konflux built images use the "operator tag" - ) - } -} diff --git a/internal/deployer/konflux_test.go b/internal/deployer/konflux_test.go deleted file mode 100644 index 9cc5bf8c..00000000 --- a/internal/deployer/konflux_test.go +++ /dev/null @@ -1,66 +0,0 @@ -package deployer - -import ( - "fmt" - "testing" - - "github.com/stackrox/roxie/internal/constants" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestOperatorImage_Konflux(t *testing.T) { - instance := OperatorInstanceConfig{Version: "4.9.2", KonfluxImages: new(true), ImageRegistry: constants.DefaultRegistry} - expected := fmt.Sprintf("%s/release-operator:4.9.2", constants.DefaultRegistry) - assert.Equal(t, expected, instance.OperatorImage()) -} - -func TestOperatorImage_NonKonflux(t *testing.T) { - instance := OperatorInstanceConfig{Version: "4.9.2", KonfluxImages: new(false), ImageRegistry: constants.DefaultRegistry} - expected := fmt.Sprintf("%s/stackrox-operator:4.9.2", constants.DefaultRegistry) - assert.Equal(t, expected, instance.OperatorImage()) -} - -func TestOperatorImage_RegistryOverride(t *testing.T) { - instance := OperatorInstanceConfig{Version: "4.9.2", ImageRegistry: "quay.io/stackrox-io"} - assert.Equal(t, "quay.io/stackrox-io/stackrox-operator:4.9.2", instance.OperatorImage()) -} - -func TestPopulateKonfluxEnvVars_AllEntries(t *testing.T) { - instance := &OperatorInstanceConfig{Version: "4.9.2", KonfluxImages: new(true)} - populateKonfluxEnvVars(instance) - - require.Len(t, instance.EnvVars, len(konfluxRelatedImages)) - - for envName, imageSuffix := range konfluxRelatedImages { - expected := fmt.Sprintf("%s/release-%s:%s", constants.DefaultRegistry, imageSuffix, "4.9.2") - assert.Equal(t, expected, instance.EnvVars[envName], "mismatch for %s", envName) - } -} - -func TestPopulateKonfluxEnvVars_UserOverridePreserved(t *testing.T) { - userValue := "quay.io/custom/my-main:latest" - instance := &OperatorInstanceConfig{ - Version: "4.9.2", - KonfluxImages: new(true), - EnvVars: map[string]string{ - "RELATED_IMAGE_MAIN": userValue, - }, - } - - populateKonfluxEnvVars(instance) - - assert.Equal(t, userValue, instance.EnvVars["RELATED_IMAGE_MAIN"], - "user override should be preserved") - - assert.Len(t, instance.EnvVars, len(konfluxRelatedImages), - "all other entries should be populated") - - for envName, imageSuffix := range konfluxRelatedImages { - if envName == "RELATED_IMAGE_MAIN" { - continue - } - expected := fmt.Sprintf("%s/release-%s:%s", constants.DefaultRegistry, imageSuffix, "4.9.2") - assert.Equal(t, expected, instance.EnvVars[envName], "mismatch for %s", envName) - } -} diff --git a/internal/deployer/operand_images.go b/internal/deployer/operand_images.go new file mode 100644 index 00000000..f41d4dc6 --- /dev/null +++ b/internal/deployer/operand_images.go @@ -0,0 +1,98 @@ +package deployer + +import ( + "fmt" + + "github.com/stackrox/roxie/internal/constants" + "github.com/stackrox/roxie/internal/imagetag" +) + +var relatedImages = map[string]string{ + "RELATED_IMAGE_MAIN": "main", + "RELATED_IMAGE_CENTRAL_DB": "central-db", + "RELATED_IMAGE_COLLECTOR": "collector", + "RELATED_IMAGE_SCANNER_V4_DB": "scanner-v4-db", + "RELATED_IMAGE_SCANNER_V4": "scanner-v4", + "RELATED_IMAGE_FACT": "fact", + + // Old scanner images need to stay for now to be able to deploy older ACS versions. + "RELATED_IMAGE_SCANNER": "scanner", + "RELATED_IMAGE_SCANNER_SLIM": "scanner-slim", + "RELATED_IMAGE_SCANNER_DB": "scanner-db", + "RELATED_IMAGE_SCANNER_DB_SLIM": "scanner-db-slim", +} + +// populateOperatorEnvVars adds RELATED_IMAGE_* env vars to the instance +// when the effective operand image differs from what the operator would +// use by default (i.e. when OperandTag or OperandTagOverrides are set). +// In Konflux mode, all entries are added unconditionally. +// Explicitly-provided env vars (e.g. from --operator-env) take precedence. +func populateOperatorEnvVars(instance *OperatorInstanceConfig) { + if instance == nil { + // Nothing to do. + return + } + + envVars := make(map[string]string) + + for envName, imageName := range relatedImages { + envVars[envName] = constructOperandImageFromInstance(*instance, imageName) + } + + instanceWithoutOverrides := *instance + instanceWithoutOverrides.OperandTag = imagetag.MainTag("") + instanceWithoutOverrides.OperandTagOverrides = nil + + if instance.EnvVars == nil { + instance.EnvVars = make(map[string]string) + } + for envName, imageRef := range envVars { + if _, exists := instance.EnvVars[envName]; exists { + // Explicitly set env vars for related images take precedence. + continue + } + if !instance.KonfluxImagesEnabled() { + // Only add env var if it differs from what the operator would use anyway. + // For Konflux mode we need to add the overwrites unconditionally, because the operator + // happily references non-existing downstream operand images. + defaultImageRef := constructOperandImageFromInstance(instanceWithoutOverrides, relatedImages[envName]) + if imageRef == defaultImageRef { + continue + } + } + instance.EnvVars[envName] = imageRef + } +} + +func constructOperandImageFromInstance(instance OperatorInstanceConfig, imageName string) string { + imagePrefix := "" + useKonflux := instance.KonfluxImagesEnabled() + if useKonflux { + imagePrefix = "release-" + } + imageRegistry := constants.DefaultRegistry + if !useKonflux && instance.ImageRegistry != "" { // Konflux images are only supported for default registry. + imageRegistry = instance.ImageRegistry + } + + // The top-level version in an OperatorInstanceConfig can be + // selectively overwritten for specific images. + effectiveOperandImageVersion := instance.Version + if instance.OperandTag.IsSet() { + effectiveOperandImageVersion = instance.OperandTag + } + if perImageVersion := instance.OperandTagOverrides[imageName]; perImageVersion.IsSet() { + // A custom version for this particular image is requested. + effectiveOperandImageVersion = perImageVersion + } + + var effectiveVersionString string + if useKonflux { + // Konflux built images use the "operator tag". + effectiveVersionString = effectiveOperandImageVersion.ToOperatorTag().String() + } else { + effectiveVersionString = effectiveOperandImageVersion.String() + } + + return fmt.Sprintf("%s/%s%s:%s", imageRegistry, imagePrefix, imageName, effectiveVersionString) +} diff --git a/internal/deployer/operand_images_test.go b/internal/deployer/operand_images_test.go new file mode 100644 index 00000000..b29c86da --- /dev/null +++ b/internal/deployer/operand_images_test.go @@ -0,0 +1,279 @@ +package deployer + +import ( + "fmt" + "testing" + + "github.com/stackrox/roxie/internal/constants" + "github.com/stackrox/roxie/internal/imagetag" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestOperatorImage_Konflux(t *testing.T) { + instance := OperatorInstanceConfig{Version: "4.9.2", KonfluxImages: new(true), ImageRegistry: constants.DefaultRegistry} + expected := fmt.Sprintf("%s/release-operator:4.9.2", constants.DefaultRegistry) + assert.Equal(t, expected, instance.OperatorImage()) +} + +func TestOperatorImage_NonKonflux(t *testing.T) { + instance := OperatorInstanceConfig{Version: "4.9.2", KonfluxImages: new(false), ImageRegistry: constants.DefaultRegistry} + expected := fmt.Sprintf("%s/stackrox-operator:4.9.2", constants.DefaultRegistry) + assert.Equal(t, expected, instance.OperatorImage()) +} + +func TestOperatorImage_RegistryOverride(t *testing.T) { + instance := OperatorInstanceConfig{Version: "4.9.2", ImageRegistry: "quay.io/stackrox-io"} + assert.Equal(t, "quay.io/stackrox-io/stackrox-operator:4.9.2", instance.OperatorImage()) +} + +func TestPopulateKonfluxEnvVars_AllEntries(t *testing.T) { + instance := &OperatorInstanceConfig{Version: "4.9.2", KonfluxImages: new(true)} + populateOperatorEnvVars(instance) + + require.Len(t, instance.EnvVars, len(relatedImages)) + + for envName, imageSuffix := range relatedImages { + expected := fmt.Sprintf("%s/release-%s:%s", constants.DefaultRegistry, imageSuffix, "4.9.2") + assert.Equal(t, expected, instance.EnvVars[envName], "mismatch for %s", envName) + } +} + +func TestPopulateKonfluxEnvVars_UserOverridePreserved(t *testing.T) { + userValue := "quay.io/custom/my-main:latest" + instance := &OperatorInstanceConfig{ + Version: "4.9.2", + KonfluxImages: new(true), + EnvVars: map[string]string{ + "RELATED_IMAGE_MAIN": userValue, + }, + } + + populateOperatorEnvVars(instance) + + assert.Equal(t, userValue, instance.EnvVars["RELATED_IMAGE_MAIN"], + "user override should be preserved") + + assert.Len(t, instance.EnvVars, len(relatedImages), + "all other entries should be populated") + + for envName, imageSuffix := range relatedImages { + if envName == "RELATED_IMAGE_MAIN" { + continue + } + expected := fmt.Sprintf("%s/release-%s:%s", constants.DefaultRegistry, imageSuffix, "4.9.2") + assert.Equal(t, expected, instance.EnvVars[envName], "mismatch for %s", envName) + } +} + +func TestConstructOperandImageFromInstance(t *testing.T) { + const img = "some-image" + + tests := []struct { + name string + instance OperatorInstanceConfig + expected string + }{ + { + name: "non-konflux plain version", + instance: OperatorInstanceConfig{ + Version: "4.9.2", + ImageRegistry: constants.DefaultRegistry, + }, + expected: constants.DefaultRegistry + "/" + img + ":4.9.2", + }, + { + name: "non-konflux dirty tag preserved", + instance: OperatorInstanceConfig{ + Version: "4.11.x-dirty", + ImageRegistry: constants.DefaultRegistry, + }, + expected: constants.DefaultRegistry + "/" + img + ":4.11.x-dirty", + }, + { + name: "non-konflux custom registry", + instance: OperatorInstanceConfig{ + Version: "4.9.2", + ImageRegistry: "quay.io/stackrox-io", + }, + expected: "quay.io/stackrox-io/" + img + ":4.9.2", + }, + { + name: "konflux adds release- prefix", + instance: OperatorInstanceConfig{ + Version: "4.9.2", + KonfluxImages: new(true), + ImageRegistry: constants.DefaultRegistry, + }, + expected: constants.DefaultRegistry + "/release-" + img + ":4.9.2", + }, + { + name: "konflux normalizes dirty tag via ToOperatorTag", + instance: OperatorInstanceConfig{ + Version: "4.11.x-dirty", + KonfluxImages: new(true), + ImageRegistry: constants.DefaultRegistry, + }, + expected: constants.DefaultRegistry + "/release-" + img + ":4.11.0", + }, + { + name: "konflux ignores custom registry", + instance: OperatorInstanceConfig{ + Version: "4.9.2", + KonfluxImages: new(true), + ImageRegistry: "quay.io/stackrox-io", + }, + expected: constants.DefaultRegistry + "/release-" + img + ":4.9.2", + }, + { + name: "operand tag overrides version", + instance: OperatorInstanceConfig{ + Version: "4.9.2", + OperandTag: "4.10.0", + ImageRegistry: constants.DefaultRegistry, + }, + expected: constants.DefaultRegistry + "/" + img + ":4.10.0", + }, + { + name: "operand tag overrides version in konflux mode", + instance: OperatorInstanceConfig{ + Version: "4.9.2", + OperandTag: "4.10.x-dirty", + KonfluxImages: new(true), + ImageRegistry: constants.DefaultRegistry, + }, + expected: constants.DefaultRegistry + "/release-" + img + ":4.10.0", + }, + { + name: "per-image overwrite", + instance: OperatorInstanceConfig{ + Version: "4.9.2", + ImageRegistry: constants.DefaultRegistry, + OperandTagOverrides: map[string]imagetag.MainTag{ + img: "4.10.0", + }, + }, + expected: constants.DefaultRegistry + "/" + img + ":4.10.0", + }, + { + name: "per-image overwrite does not affect other images", + instance: OperatorInstanceConfig{ + Version: "4.9.2", + ImageRegistry: constants.DefaultRegistry, + OperandTagOverrides: map[string]imagetag.MainTag{ + "other-image": "4.10.0", + }, + }, + expected: constants.DefaultRegistry + "/" + img + ":4.9.2", + }, + { + name: "per-image overwrite in konflux mode uses operator tag", + instance: OperatorInstanceConfig{ + Version: "4.9.2", + KonfluxImages: new(true), + ImageRegistry: constants.DefaultRegistry, + OperandTagOverrides: map[string]imagetag.MainTag{ + img: "4.10.x-dirty", + }, + }, + expected: constants.DefaultRegistry + "/release-" + img + ":4.10.0", + }, + { + name: "per-image overwrite in konflux does not affect other images", + instance: OperatorInstanceConfig{ + Version: "4.9.2", + KonfluxImages: new(true), + ImageRegistry: constants.DefaultRegistry, + OperandTagOverrides: map[string]imagetag.MainTag{ + "other-image": "4.10.x-dirty", + }, + }, + expected: constants.DefaultRegistry + "/release-" + img + ":4.9.2", + }, + { + name: "all three set: overwrite wins for matched image", + instance: OperatorInstanceConfig{ + Version: "4.9.2", + OperandTag: "4.10.0", + ImageRegistry: constants.DefaultRegistry, + OperandTagOverrides: map[string]imagetag.MainTag{ + img: "4.11.0", + }, + }, + expected: constants.DefaultRegistry + "/" + img + ":4.11.0", + }, + { + name: "all three set: operand tag wins for non-matched image", + instance: OperatorInstanceConfig{ + Version: "4.9.2", + OperandTag: "4.10.0", + ImageRegistry: constants.DefaultRegistry, + OperandTagOverrides: map[string]imagetag.MainTag{ + "other-image": "4.11.0", + }, + }, + expected: constants.DefaultRegistry + "/" + img + ":4.10.0", + }, + { + name: "all three set in konflux: overwrite wins for matched image", + instance: OperatorInstanceConfig{ + Version: "4.9.x-dirty", + OperandTag: "4.10.x-dirty", + KonfluxImages: new(true), + ImageRegistry: constants.DefaultRegistry, + OperandTagOverrides: map[string]imagetag.MainTag{ + img: "4.11.x-dirty", + }, + }, + expected: constants.DefaultRegistry + "/release-" + img + ":4.11.0", + }, + { + name: "all three set in konflux: operand tag wins for non-matched image", + instance: OperatorInstanceConfig{ + Version: "4.9.x-dirty", + OperandTag: "4.10.x-dirty", + KonfluxImages: new(true), + ImageRegistry: constants.DefaultRegistry, + OperandTagOverrides: map[string]imagetag.MainTag{ + "other-image": "4.11.x-dirty", + }, + }, + expected: constants.DefaultRegistry + "/release-" + img + ":4.10.0", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := constructOperandImageFromInstance(tt.instance, img) + assert.Equal(t, tt.expected, got) + }) + } +} + +func TestPopulateOperatorEnvVars_OverwriteResetsToVersion(t *testing.T) { + const pinnedImage = "scanner-v4" + + instance := &OperatorInstanceConfig{ + Version: "4.9.2", + OperandTag: "4.10.0", + ImageRegistry: constants.DefaultRegistry, + OperandTagOverrides: map[string]imagetag.MainTag{ + pinnedImage: "4.9.2", // Pin this image back to the operator version. + }, + } + populateOperatorEnvVars(instance) + + envNameForPinnedImage := "" + for envName, imageName := range relatedImages { + if imageName == pinnedImage { + envNameForPinnedImage = envName + continue + } + expected := fmt.Sprintf("%s/%s:%s", constants.DefaultRegistry, imageName, "4.10.0") + assert.Equal(t, expected, instance.EnvVars[envName], + "non-pinned image %s should use operand tag", imageName) + } + + assert.NotContains(t, instance.EnvVars, envNameForPinnedImage, + "pinned image reset to version should not need an env var override") +} diff --git a/internal/deployer/operator_env_vars_test.go b/internal/deployer/operator_env_vars_test.go new file mode 100644 index 00000000..9da12e28 --- /dev/null +++ b/internal/deployer/operator_env_vars_test.go @@ -0,0 +1 @@ +package deployer diff --git a/internal/deployer/operator_instances.go b/internal/deployer/operator_instances.go index ccff3a60..10fbcd1a 100644 --- a/internal/deployer/operator_instances.go +++ b/internal/deployer/operator_instances.go @@ -2,6 +2,7 @@ package deployer import ( "cmp" + "errors" "maps" "slices" "strings" @@ -57,16 +58,27 @@ func (c *Config) HasMixedVersions() bool { // OperatorInstances builds the operator deployment plan for this config. // When versions match, a single operator is deployed to rhacs-operator-system. // When they differ, two operators are deployed with reconciler toggles. -func (c *Config) OperatorInstances() []OperatorInstanceConfig { +// Returns a boolean indicating if OLM-mode is requested. +func (c *Config) OperatorInstances() ([]OperatorInstanceConfig, bool, error) { registry := c.Roxie.ImageRegistry if !c.HasMixedVersions() { return []OperatorInstanceConfig{{ - Version: c.CentralVersion(), // Per-component overrides may both equal each other but differ from Operator.Version. - Namespace: operatorNamespaceSystem, - EnvVars: maps.Clone(c.Operator.EnvVars), - KonfluxImages: c.resolveKonfluxImages(&c.Operator.OperatorInstanceConfig), - ImageRegistry: registry, - }} + Version: c.CentralVersion(), // Per-component overrides may both equal each other but differ from Operator.Version. + Namespace: operatorNamespaceSystem, + EnvVars: maps.Clone(c.Operator.EnvVars), + KonfluxImages: c.resolveKonfluxImages(&c.Operator.OperatorInstanceConfig), + ImageRegistry: registry, + OperandTag: c.Operator.OperandTag, + OperandTagOverrides: maps.Clone(c.Operator.OperandTagOverrides), + }}, c.Operator.DeployViaOlmEnabled(), nil + } + + var err error + if c.Operator.DeployViaOlmEnabled() { + // We return this error, but keep going so that callers of this function + // can always expect to receive at least one operator instance and then + // decide how the handling of the error should proceed. + err = errors.New("OLM mode is incompatible with mixed operator versions") } centralOperatorEnvVars := make(map[string]string, len(c.Central.Operator.EnvVars)+1) @@ -79,22 +91,26 @@ func (c *Config) OperatorInstances() []OperatorInstanceConfig { return []OperatorInstanceConfig{ { - Version: c.CentralVersion(), - Namespace: operatorNamespaceCentral, - EnvVars: centralOperatorEnvVars, - RoleNameSuffix: roleNameSuffixCentral, - KonfluxImages: c.resolveKonfluxImages(&c.Central.Operator), - ImageRegistry: registry, + Version: c.CentralVersion(), + Namespace: operatorNamespaceCentral, + EnvVars: centralOperatorEnvVars, + RoleNameSuffix: roleNameSuffixCentral, + KonfluxImages: c.resolveKonfluxImages(&c.Central.Operator), + ImageRegistry: registry, + OperandTag: c.Central.Operator.OperandTag, + OperandTagOverrides: maps.Clone(c.Central.Operator.OperandTagOverrides), }, { - Version: c.SecuredClusterVersion(), - Namespace: operatorNamespaceSensor, - EnvVars: sensorOperatorEnvVars, - RoleNameSuffix: roleNameSuffixSensor, - KonfluxImages: c.resolveKonfluxImages(&c.SecuredCluster.Operator), - ImageRegistry: registry, + Version: c.SecuredClusterVersion(), + Namespace: operatorNamespaceSensor, + EnvVars: sensorOperatorEnvVars, + RoleNameSuffix: roleNameSuffixSensor, + KonfluxImages: c.resolveKonfluxImages(&c.SecuredCluster.Operator), + ImageRegistry: registry, + OperandTag: c.SecuredCluster.Operator.OperandTag, + OperandTagOverrides: maps.Clone(c.SecuredCluster.Operator.OperandTagOverrides), }, - } + }, false, err } // resolveKonfluxImages returns the effective KonfluxImages setting for a per-component @@ -110,7 +126,7 @@ func (c *Config) resolveKonfluxImages(instanceCfg *OperatorInstanceConfig) *bool // This is used to determine which bundle to use for CRD installation, since CRDs // are cluster-scoped and must be at the newest version. func (c *Config) NewestOperatorInstance() OperatorInstanceConfig { - instances := c.OperatorInstances() + instances, _, _ := c.OperatorInstances() return slices.MaxFunc(instances, func(a, b OperatorInstanceConfig) int { av, aerr := parseSemver(a.Version) bv, berr := parseSemver(b.Version) diff --git a/internal/deployer/operator_instances_test.go b/internal/deployer/operator_instances_test.go index aa6313fb..697284f3 100644 --- a/internal/deployer/operator_instances_test.go +++ b/internal/deployer/operator_instances_test.go @@ -63,8 +63,10 @@ func TestVersions_BothOverridesSame_NoMixed(t *testing.T) { assert.Equal(t, imagetag.MainTag("4.8.0"), cfg.SecuredClusterVersion()) assert.False(t, cfg.HasMixedVersions()) - instances := cfg.OperatorInstances() + instances, olmMode, err := cfg.OperatorInstances() require.Len(t, instances, 1) + assert.False(t, olmMode, "unexpected OLM mode") + assert.NoError(t, err, "unexpected error from OperatorInstances") assert.Equal(t, imagetag.MainTag("4.8.0"), instances[0].Version, "operator should use the override version, not Roxie.Version") } @@ -79,8 +81,10 @@ func TestOperatorInstances_SingleVersion(t *testing.T) { }, } - instances := cfg.OperatorInstances() + instances, olmMode, err := cfg.OperatorInstances() require.Len(t, instances, 1) + assert.False(t, olmMode, "unexpected OLM mode") + assert.NoError(t, err, "unexpected error from OperatorInstances") assert.Equal(t, imagetag.MainTag("4.9.0-dirty"), instances[0].Version) assert.Equal(t, operatorNamespaceSystem, instances[0].Namespace) assert.Equal(t, "", instances[0].RoleNameSuffix) @@ -108,8 +112,10 @@ func TestOperatorInstances_MixedVersions(t *testing.T) { }, } - instances := cfg.OperatorInstances() + instances, olmMode, err := cfg.OperatorInstances() require.Len(t, instances, 2) + assert.False(t, olmMode, "unexpected OLM mode") + assert.NoError(t, err, "unexpected error from OperatorInstances") central := instances[0] assert.Equal(t, imagetag.MainTag("4.8.0"), central.Version) @@ -158,8 +164,10 @@ func TestOperatorInstances_PerComponentEnvVars(t *testing.T) { }, } - instances := cfg.OperatorInstances() + instances, olmMode, err := cfg.OperatorInstances() require.Len(t, instances, 2) + assert.False(t, olmMode, "unexpected OLM mode") + assert.NoError(t, err, "unexpected error from OperatorInstances") // Top-level Operator.EnvVars are NOT inherited in dual-operator mode. assert.NotContains(t, instances[0].EnvVars, "SHARED") diff --git a/internal/deployer/operator_olm.go b/internal/deployer/operator_olm.go index 752b0cba..dd0ba69d 100644 --- a/internal/deployer/operator_olm.go +++ b/internal/deployer/operator_olm.go @@ -32,12 +32,12 @@ const ( ) // deployOperatorViaOLM deploys the RHACS operator using OLM. -func (d *Deployer) deployOperatorViaOLM(ctx context.Context) error { +func (d *Deployer) deployOperatorViaOLM(ctx context.Context, instance OperatorInstanceConfig) error { log.Info("🚀 Deploying operator via OLM...") - log.Infof("Operator tag: %s", d.config.Operator.Version) - if len(d.config.Operator.EnvVars) > 0 { - log.Infof("Custom operator env vars: %d", len(d.config.Operator.EnvVars)) - for _, envVar := range envVarsToSortedList(d.config.Operator.EnvVars) { + log.Infof("Operator tag: %s", instance.Version) + if len(instance.EnvVars) > 0 { + log.Infof("Custom operator env vars: %d", len(instance.EnvVars)) + for _, envVar := range envVarsToSortedList(instance.EnvVars) { ev := envVar.(map[string]interface{}) log.Dimf(" %s=%s", ev["name"], ev["value"]) } @@ -47,7 +47,7 @@ func (d *Deployer) deployOperatorViaOLM(ctx context.Context) error { return err } - indexImage := d.getOperatorIndexImage() + indexImage := d.getOperatorIndexImage(instance) log.Infof("Index image: %s", indexImage) if err := d.prepareNamespace(ctx, operatorNamespace, false); err != nil { @@ -62,15 +62,15 @@ func (d *Deployer) deployOperatorViaOLM(ctx context.Context) error { return fmt.Errorf("failed to create OperatorGroup: %w", err) } - if err := d.createSubscription(ctx); err != nil { + if err := d.createSubscription(ctx, instance); err != nil { return fmt.Errorf("failed to create Subscription: %w", err) } - if err := d.waitForAndApproveInstallPlan(ctx); err != nil { + if err := d.waitForAndApproveInstallPlan(ctx, instance); err != nil { return fmt.Errorf("failed to approve InstallPlan: %w", err) } - if err := d.waitForCSVSuccess(ctx); err != nil { + if err := d.waitForCSVSuccess(ctx, instance); err != nil { return fmt.Errorf("failed waiting for CSV: %w", err) } @@ -129,8 +129,8 @@ func (d *Deployer) checkOLMInstalled(ctx context.Context) error { } // getOperatorIndexImage returns the operator index image reference. -func (d *Deployer) getOperatorIndexImage() string { - return fmt.Sprintf(operatorIndexImage+":v%s", d.config.Operator.Version) +func (d *Deployer) getOperatorIndexImage(instance OperatorInstanceConfig) string { + return fmt.Sprintf(operatorIndexImage+":v%s", instance.Version) } // createCatalogSource creates the CatalogSource for the operator. @@ -204,10 +204,10 @@ func (d *Deployer) createOperatorGroup(ctx context.Context) error { } // createSubscription creates the Subscription for the operator. -func (d *Deployer) createSubscription(ctx context.Context) error { +func (d *Deployer) createSubscription(ctx context.Context, instance OperatorInstanceConfig) error { log.Info("Creating Subscription...") - startingCSV := fmt.Sprintf("rhacs-operator.v%s", d.config.Operator.Version) + startingCSV := fmt.Sprintf("rhacs-operator.v%s", instance.Version) subscriptionSpec := map[string]interface{}{ "channel": operatorChannel, @@ -218,9 +218,9 @@ func (d *Deployer) createSubscription(ctx context.Context) error { "startingCSV": startingCSV, } - if len(d.config.Operator.EnvVars) > 0 { + if len(instance.EnvVars) > 0 { subscriptionSpec["config"] = map[string]interface{}{ - "env": envVarsToSortedList(d.config.Operator.EnvVars), + "env": envVarsToSortedList(instance.EnvVars), } } @@ -252,7 +252,7 @@ func (d *Deployer) createSubscription(ctx context.Context) error { } // waitForAndApproveInstallPlan waits for the InstallPlan to be created and approves it. -func (d *Deployer) waitForAndApproveInstallPlan(ctx context.Context) error { +func (d *Deployer) waitForAndApproveInstallPlan(ctx context.Context, instance OperatorInstanceConfig) error { log.Info("⏳ Waiting for InstallPlan to be created...") // Wait for subscription to have InstallPlanPending condition. @@ -275,7 +275,7 @@ func (d *Deployer) waitForAndApproveInstallPlan(ctx context.Context) error { } // Sanity check:Verify currentCSV matches expected version. - expectedCSV := fmt.Sprintf("rhacs-operator.v%s", d.config.Operator.Version) + expectedCSV := fmt.Sprintf("rhacs-operator.v%s", instance.Version) result, err := k8s.RunKubectl(ctx, k8s.KubectlOptions{ Args: []string{"get", "subscription", subscriptionName, "-n", operatorNamespace, "-o", "jsonpath={.status.currentCSV}"}, }) @@ -316,8 +316,8 @@ func (d *Deployer) waitForAndApproveInstallPlan(ctx context.Context) error { } // waitForCSVSuccess waits for the CSV to reach Succeeded phase. -func (d *Deployer) waitForCSVSuccess(ctx context.Context) error { - csvName := fmt.Sprintf("rhacs-operator.v%s", d.config.Operator.Version) +func (d *Deployer) waitForCSVSuccess(ctx context.Context, instance OperatorInstanceConfig) error { + csvName := fmt.Sprintf("rhacs-operator.v%s", instance.Version) log.Infof("⏳ Waiting for CSV %s to succeed...", csvName) start := time.Now() diff --git a/internal/imagetag/imagetag.go b/internal/imagetag/imagetag.go index a8b97a83..b2fb2d7e 100644 --- a/internal/imagetag/imagetag.go +++ b/internal/imagetag/imagetag.go @@ -20,3 +20,6 @@ func (t MainTag) ToOperatorTag() OperatorTag { func (t MainTag) String() string { return string(t) } func (t OperatorTag) String() string { return string(t) } + +func (t MainTag) IsSet() bool { return string(t) != "" } +func (t OperatorTag) IsSet() bool { return string(t) != "" } diff --git a/tests/e2e/helpers.go b/tests/e2e/helpers.go index 712ffd96..db5312b4 100644 --- a/tests/e2e/helpers.go +++ b/tests/e2e/helpers.go @@ -472,3 +472,19 @@ func verifyOperatorVersion(t *testing.T, namespace, expectedTag string) { assert.Equalf(t, expectedTag, actualTag, "Operator image tag mismatch in namespace %s", namespace) t.Logf("✓ Operator image in namespace %s: %s", namespace, image) } + +func verifyOperatorDeploymentEnvVar(t *testing.T, namespace, envName, expectedValue string) { + t.Helper() + + jsonpath := fmt.Sprintf( + `{.spec.template.spec.containers[?(@.name=="manager")].env[?(@.name=="%s")].value}`, envName) + cmd := exec.Command("kubectl", "get", "deployment", operatorDeploymentName, + "-n", namespace, "-o", "jsonpath="+jsonpath) + output, err := cmd.Output() + require.NoErrorf(t, err, "Failed to get env var %s from operator deployment in %s", envName, namespace) + + actualValue := strings.TrimSpace(string(output)) + assert.Equalf(t, expectedValue, actualValue, + "Operator deployment env var %s mismatch in %s", envName, namespace) + t.Logf("✓ Operator deployment in %s has %s=%q", namespace, envName, actualValue) +} diff --git a/tests/e2e/mixed_versions_test.go b/tests/e2e/mixed_versions_test.go index 2d6adae5..b3a4c674 100644 --- a/tests/e2e/mixed_versions_test.go +++ b/tests/e2e/mixed_versions_test.go @@ -14,6 +14,10 @@ import ( "github.com/stackrox/roxie/internal/stackroxversions" ) +const ( + nonExistentTag = "0.0.1-override-test" +) + // lookupTwoReleasedTags returns two different pullable release tags. func lookupTwoReleasedTags(t *testing.T) (string, string) { t.Helper() @@ -64,15 +68,30 @@ func TestMixedVersionOperatorDeploy(t *testing.T) { verifyOperatorVersion(t, operatorCentralNamespace, centralTag) verifyOperatorVersion(t, operatorSensorNamespace, mainTag) - // Step 2: Switch back to single-version operator - t.Log("=== Step 2: Deploy operator with single version (switch from mixed to single) ===") - singleArgs := append([]string{roxieBinary, "deploy", "operator", "--tag", mainTag, "--central-tag", mainTag, "--secured-cluster-tag", mainTag}, commonDeployArgs...) + t.Log("=== Step 2: Deploy single-version operator with operand tag overrides ===") + // Switch back to single-version operator, with operand tag overrides. + // This verifies that populateOperatorEnvVars injects RELATED_IMAGE_* env vars + // into the non-OLM operator Deployment when operandTag / operandTagOverrides + // differ from the operator version. + singleArgs := append([]string{ + roxieBinary, "deploy", "operator", + "--tag", mainTag, "--central-tag", mainTag, "--secured-cluster-tag", mainTag, + "--set", "operator.operandTag=" + centralTag, + "--set", "operator.operandTagOverrides.central-db=" + nonExistentTag, + }, commonDeployArgs...) runCommand(t, operatorDeployTimeout, nil, singleArgs...) verifyOperatorDeploymentExists(t, operatorSystemNamespace) verifyOperatorNotInNamespace(t, operatorCentralNamespace) verifyOperatorNotInNamespace(t, operatorSensorNamespace) + // operandTag applies to all images by default. + verifyOperatorDeploymentEnvVar(t, + operatorSystemNamespace, "RELATED_IMAGE_MAIN", fmt.Sprintf("quay.io/rhacs-eng/main:%s", centralTag)) + // operandTagOverrides takes precedence for the specific image. + verifyOperatorDeploymentEnvVar(t, + operatorSystemNamespace, "RELATED_IMAGE_CENTRAL_DB", fmt.Sprintf("quay.io/rhacs-eng/central-db:%s", nonExistentTag)) + // Step 3: Switch from single back to mixed t.Log("=== Step 3: Deploy operator with mixed versions again (switch from single to mixed) ===") runCommand(t, operatorDeployTimeout, nil, args...) diff --git a/tests/e2e/olm_switch_test.go b/tests/e2e/olm_switch_test.go index b5d1df3e..37821cc9 100644 --- a/tests/e2e/olm_switch_test.go +++ b/tests/e2e/olm_switch_test.go @@ -4,6 +4,7 @@ package e2e import ( + "fmt" "os" "os/exec" "strings" @@ -135,7 +136,8 @@ func TestNonOLMToOLMSwitch(t *testing.T) { verifyCentralNotInstalled(t, centralNamespace) } -// TestOLMOperatorVersionUpgrade tests that OLM operator version mismatches trigger teardown and redeploy +// TestOLMOperatorVersionUpgrade tests that OLM operator version mismatches trigger teardown and redeploy, +// and that operand tag overrides propagate RELATED_IMAGE_* env vars to the OLM Subscription. func TestOLMOperatorVersionUpgrade(t *testing.T) { dumpClusterStateOnFailure(t) @@ -143,9 +145,15 @@ func TestOLMOperatorVersionUpgrade(t *testing.T) { t.Skip("SKIP_OLM_TESTS is set") } - // This test requires two different operator versions - // We can simulate this by deploying twice with the same version, - // but the logic should handle version changes by tearing down and redeploying + // Look up two release tags different from the main tag. We use one as a blanket + // operand tag override and the other as a per-image override for central-db. + tag1, tag2 := lookupTwoReleasedTags(t) + mainTag := os.Getenv("MAIN_IMAGE_TAG") + operandTag, perImageTag := tag1, tag2 + if operandTag == mainTag { + operandTag, perImageTag = tag2, tag1 + } + t.Logf("Main tag: %s, operandTag: %s, per-image override (central-db): %s", mainTag, operandTag, perImageTag) // Create temporary envrc file envrcFile, err := os.CreateTemp(t.TempDir(), ".envrc.roxie-test-*") @@ -155,9 +163,12 @@ func TestOLMOperatorVersionUpgrade(t *testing.T) { envrcPath := envrcFile.Name() envrcFile.Close() - // Step 1: Deploy central with OLM operator - t.Log("=== Step 1: Deploy central with OLM operator ===") - args := append([]string{roxieBinary, "deploy", "central", "--olm", "--envrc", envrcPath}, commonDeployArgs...) + t.Log("=== Step 1: Deploy central with OLM operator and operand tag overrides ===") + args := append([]string{ + roxieBinary, "deploy", "central", "--olm", "--envrc", envrcPath, + "--set", "operator.operandTag=" + operandTag, + "--set", "operator.operandTagOverrides.central-db=" + perImageTag, + }, commonDeployArgs...) runCommand(t, deployTimeout, nil, args...) // Verify operator is in OLM mode @@ -165,10 +176,19 @@ func TestOLMOperatorVersionUpgrade(t *testing.T) { verifyOperatorMode(t, true) verifyOperatorDeploymentExists(t, operatorSystemNamespace) + // Verify that operandTag caused RELATED_IMAGE_MAIN to use the blanket override. + verifyOperatorDeploymentEnvVar(t, + operatorSystemNamespace, "RELATED_IMAGE_MAIN", fmt.Sprintf("quay.io/rhacs-eng/main:%s", operandTag)) + + // Verify that operandTagOverrides caused RELATED_IMAGE_CENTRAL_DB to use + // the per-image tag instead of the blanket operandTag. + if perImageTag != mainTag { + verifyOperatorDeploymentEnvVar(t, operatorSystemNamespace, + "RELATED_IMAGE_CENTRAL_DB", fmt.Sprintf("quay.io/rhacs-eng/central-db:%s", perImageTag)) + } + // Get the current operator version - operatorNamespace := operatorSystemNamespace - deploymentName := operatorDeploymentName - cmd := exec.Command("kubectl", "get", "deployment", deploymentName, "-n", operatorNamespace, + cmd := exec.Command("kubectl", "get", "deployment", operatorDeploymentName, "-n", operatorSystemNamespace, "-o", "jsonpath={.spec.template.spec.containers[0].image}") output, err := cmd.Output() if err != nil { @@ -240,7 +260,9 @@ func TestSecuredClusterWithOLMSwitch(t *testing.T) { // Step 3: Switch to non-OLM by redeploying secured-cluster without --olm t.Log("=== Step 3: Redeploy secured-cluster without OLM (triggering mode switch) ===") - args = append([]string{roxieBinary, "deploy", "--early-readiness", "secured-cluster"}, commonDeployArgs...) + // Note: we need to *explicitly* disable OLM because of an pre-existing issue where the on-cluster manifest influences + // the effective deployment configuration on a re-deployment. + args = append([]string{roxieBinary, "deploy", "--early-readiness", "secured-cluster", "--olm=false"}, commonDeployArgs...) runCommand(t, deployTimeout, envrcEnv, args...) // Verify operator switched to non-OLM mode