diff --git a/cmd/config.go b/cmd/config.go index 8888dd4..ae7b031 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -14,6 +14,11 @@ func assembleConfigForCommand(configBase *deployer.Config, configFromArgs deploy config = deployer.DefaultConfig() } else { config = *configBase + // Neutralize operator-level values that older roxie baked into the + // persisted manifest, so they re-resolve from the roxie section (and + // later overrides take effect). Only meaningful for a loaded manifest, + // and must happen before user config and args are merged on top. + config.DropBakedOperatorDefaults() } // Apply user config on top (overriding defaults). diff --git a/cmd/deploy.go b/cmd/deploy.go index 1fc3cfa..4106850 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -462,13 +462,6 @@ func configureConfig(components component.Component, deploySettings *deployer.Co log.Warning("You are deploying SecuredCluster to a local cluster, it is recommended to specify a resource profile (or --resources=auto)") } - // We need to do this regardless of whether the operator is deployed or not, because - // this includes the transformation of StackRox main image tags to semver compatible versions, - // which we will make use of later for checking version constraints. - if err := deploySettings.Operator.Configure(&deploySettings.Roxie); err != nil { - return fmt.Errorf("configuring operator configuration: %w", err) - } - if components.IncludesCentral() { if err := deploySettings.Central.ConfigureSpec(&deploySettings.Roxie); err != nil { return fmt.Errorf("configuring Central spec: %w", err) diff --git a/internal/deployer/config.go b/internal/deployer/config.go index 54fe922..1ce40a3 100644 --- a/internal/deployer/config.go +++ b/internal/deployer/config.go @@ -53,6 +53,33 @@ func (c *Config) DeepCopy() (*Config, error) { return ©, nil } +// DropBakedOperatorDefaults removes operator-level values that older roxie +// versions baked into the persisted manifest by copying them verbatim from the +// roxie section (a now-removed "Configure" step set operator.version = +// roxie.version unconditionally, and operator.konfluxImages likewise). Such a +// baked value is indistinguishable from the roxie value, so when the two are +// equal we drop the operator-level one and let it re-resolve from the roxie +// section (e.g. so a later --tag takes effect). A value that differs was set +// deliberately and is preserved. +// +// This should be called on a Config loaded from the cluster manifest, before +// command-line overrides are merged on top. +func (c *Config) DropBakedOperatorDefaults() { + if c.Operator.Version == c.Roxie.Version { + c.Operator.Version = "" + } + if boolPtrEqual(c.Operator.KonfluxImages, c.Roxie.KonfluxImages) { + c.Operator.KonfluxImages = nil + } +} + +func boolPtrEqual(a, b *bool) bool { + if a == nil || b == nil { + return a == b + } + return *a == *b +} + // RoxieConfig holds roxie-level settings such as version and feature flags. type RoxieConfig struct { Version imagetag.MainTag `yaml:"version,omitempty"` @@ -167,15 +194,6 @@ func (c *OperatorConfig) DeployViaOlmEnabled() bool { return c.DeployViaOlm != nil && *c.DeployViaOlm } -// Configure derives the operator version from the roxie configuration. -func (c *OperatorConfig) Configure(roxieConfig *RoxieConfig) error { - c.Version = roxieConfig.Version - if c.KonfluxImages == nil { - c.KonfluxImages = roxieConfig.KonfluxImages - } - return nil -} - // WaitConfig describes how to wait for a component to become ready. type WaitConfig struct { Namespace string diff --git a/internal/deployer/config_test.go b/internal/deployer/config_test.go new file mode 100644 index 0000000..cfbc2bf --- /dev/null +++ b/internal/deployer/config_test.go @@ -0,0 +1,48 @@ +package deployer + +import ( + "testing" + + "github.com/stackrox/roxie/internal/imagetag" + "github.com/stretchr/testify/assert" +) + +// A manifest written by old roxie baked operator.version / operator.konfluxImages +// equal to the roxie values; both are dropped so they re-resolve (e.g. a later +// --tag takes effect). +func TestDropBakedOperatorDefaults_ClearsBaked(t *testing.T) { + cfg := Config{ + Roxie: RoxieConfig{Version: "4.10.0", KonfluxImages: new(true)}, + Operator: OperatorConfig{OperatorInstanceConfig: OperatorInstanceConfig{ + Version: "4.10.0", + KonfluxImages: new(true), + }}, + } + cfg.DropBakedOperatorDefaults() + + assert.Empty(t, cfg.Operator.Version) + assert.Nil(t, cfg.Operator.KonfluxImages) + + // --tag updates roxie.version; the operator version follows. + cfg.Roxie.Version = "4.11.0" + assert.Equal(t, imagetag.MainTag("4.11.0"), cfg.CentralVersion()) + assert.Equal(t, imagetag.MainTag("4.11.0"), cfg.SecuredClusterVersion()) +} + +// Operator values that differ from the roxie values were set deliberately and +// must be preserved. +func TestDropBakedOperatorDefaults_PreservesDeliberate(t *testing.T) { + cfg := Config{ + Roxie: RoxieConfig{Version: "4.11.0", KonfluxImages: new(false)}, + Operator: OperatorConfig{OperatorInstanceConfig: OperatorInstanceConfig{ + Version: "4.8.0", + KonfluxImages: new(true), + }}, + } + cfg.DropBakedOperatorDefaults() + + assert.Equal(t, imagetag.MainTag("4.8.0"), cfg.Operator.Version) + if assert.NotNil(t, cfg.Operator.KonfluxImages) { + assert.True(t, *cfg.Operator.KonfluxImages) + } +} diff --git a/internal/deployer/operator_instances.go b/internal/deployer/operator_instances.go index 10fbcd1..8beadcc 100644 --- a/internal/deployer/operator_instances.go +++ b/internal/deployer/operator_instances.go @@ -31,21 +31,27 @@ var AllOperatorNamespaces = []string{ } // CentralVersion returns the main image tag for Central. -// Uses central.operator.version if set, otherwise falls back to operator.version. +// Resolution order: central.operator.version, then operator.version, then roxie.version. func (c *Config) CentralVersion() imagetag.MainTag { if c.Central.Operator.Version != "" { return c.Central.Operator.Version } - return c.Operator.Version + if c.Operator.Version != "" { + return c.Operator.Version + } + return c.Roxie.Version } // SecuredClusterVersion returns the main image tag for SecuredCluster. -// Uses securedCluster.operator.version if set, otherwise falls back to operator.version. +// Resolution order: securedCluster.operator.version, then operator.version, then roxie.version. func (c *Config) SecuredClusterVersion() imagetag.MainTag { if c.SecuredCluster.Operator.Version != "" { return c.SecuredCluster.Operator.Version } - return c.Operator.Version + if c.Operator.Version != "" { + return c.Operator.Version + } + return c.Roxie.Version } // HasMixedVersions reports whether Central and SecuredCluster use different operator versions. @@ -113,13 +119,17 @@ func (c *Config) OperatorInstances() ([]OperatorInstanceConfig, bool, error) { }, false, err } -// resolveKonfluxImages returns the effective KonfluxImages setting for a per-component -// operator config, falling back to Operator.KonfluxImages if not set at the component level. +// resolveKonfluxImages returns the effective KonfluxImages setting for an operator +// instance. Resolution order: the per-component value, then operator.konfluxImages, +// then roxie.konfluxImages. func (c *Config) resolveKonfluxImages(instanceCfg *OperatorInstanceConfig) *bool { if instanceCfg.KonfluxImagesSet() { return instanceCfg.KonfluxImages } - return c.Operator.KonfluxImages + if c.Operator.KonfluxImagesSet() { + return c.Operator.KonfluxImages + } + return c.Roxie.KonfluxImages } // NewestOperatorInstance returns the operator instance with the highest version. diff --git a/internal/deployer/operator_instances_test.go b/internal/deployer/operator_instances_test.go index 697284f..edf4f3c 100644 --- a/internal/deployer/operator_instances_test.go +++ b/internal/deployer/operator_instances_test.go @@ -9,18 +9,44 @@ import ( "github.com/stretchr/testify/require" ) -func TestConfigure_PropagatesRoxieVersion(t *testing.T) { +// With nothing but roxie.version set, the operator versions resolve to it. +func TestVersionResolvesFromRoxieVersion(t *testing.T) { cfg := Config{ Roxie: RoxieConfig{Version: "4.11.1"}, } - err := cfg.Operator.Configure(&cfg.Roxie) - require.NoError(t, err) - - assert.Equal(t, imagetag.MainTag("4.11.1"), cfg.Operator.Version) assert.Equal(t, imagetag.MainTag("4.11.1"), cfg.CentralVersion()) assert.Equal(t, imagetag.MainTag("4.11.1"), cfg.SecuredClusterVersion()) } +// An explicit operator.version must take precedence over roxie.version. +func TestExplicitOperatorVersionHonoredOverRoxieVersion(t *testing.T) { + cfg := Config{ + Roxie: RoxieConfig{Version: "5.1.0-rcd", ImageRegistry: constants.DefaultRegistry}, + Operator: OperatorConfig{OperatorInstanceConfig: OperatorInstanceConfig{Version: "5.1.0"}}, + } + + assert.Equal(t, imagetag.MainTag("5.1.0"), cfg.CentralVersion()) + assert.Equal(t, imagetag.MainTag("5.1.0"), cfg.SecuredClusterVersion()) + + inst := cfg.NewestOperatorInstance() + assert.Equal(t, constants.DefaultRegistry+"/stackrox-operator-bundle:v5.1.0", inst.BundleImage()) +} + +// KonfluxImages must resolve from roxie.konfluxImages when not set at the +// operator or per-component level. +func TestKonfluxImagesResolveFromRoxie(t *testing.T) { + konflux := true + cfg := Config{ + Roxie: RoxieConfig{Version: "4.8.0", KonfluxImages: &konflux}, + Operator: OperatorConfig{OperatorInstanceConfig: OperatorInstanceConfig{Version: "4.8.0"}}, + } + + instances, _, err := cfg.OperatorInstances() + require.NoError(t, err) + require.Len(t, instances, 1) + assert.True(t, instances[0].KonfluxImagesEnabled()) +} + func TestVersions_DefaultToRoxieVersion(t *testing.T) { cfg := Config{ Roxie: RoxieConfig{Version: "4.9.0"},