Skip to content

ROX-35435: Support differing versions of Central and SecuredCluster #246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vladbologa wants to merge 17 commits into
base: main
Choose a base branch
from
Open

ROX-35435: Support differing versions of Central and SecuredCluster #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
85ebd0d
ROX-35435: Support differing versions of Central and SecuredCluster
vladbologa Jul 17, 2026
05bd699
Derive the single operator from the effective component version
vladbologa Jul 18, 2026
c89ec22
Use 'mixed' instead of 'split' versions
vladbologa Jul 21, 2026
ca14c02
Replace 'unwanted' with 'stale'
vladbologa Jul 21, 2026
d63b2af
reject invalid configuration
vladbologa Jul 21, 2026
e96547a
Restructure version overrides to use central.operator/securedCluster.…
vladbologa Jul 22, 2026
1e43049
Simplify Konflux handling
vladbologa Jul 28, 2026
3fd9d1e
Make OperatorConfig non-pointer + remove merging
vladbologa Jul 28, 2026
8b86e83
Drop Effective version naming
vladbologa Jul 28, 2026
23f70af
Remove dead code
vladbologa Jul 28, 2026
9169418
Simplify imagesForConfig & uniqueMainVersions
vladbologa Jul 28, 2026
80adfab
Apply suggestions from code review
vladbologa Jul 28, 2026
c765e2a
Apply suggestions from code review
vladbologa Jul 28, 2026
a7a6b9e
Move code from runDeploy to deployValidate
vladbologa Jul 28, 2026
570f0e2
Simplify error messages
vladbologa Jul 28, 2026
114c96f
Clarify operator tags / main tags mixup
vladbologa Jul 29, 2026
d1c85ce
Address CodeRabbit comment
vladbologa Jul 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 47 additions & 23 deletions cmd/deploy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,20 @@ this flag can be used to tell roxie how to pre-load images for the current clust
}),
)

registerFlag(cmd, settings, "central-tag", "Image tag for Central (overrides --tag for Central)",

@mclasmeier mclasmeier Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to think a bit about this PR from a conceptual viewpoint. But, looking at the concrete changes, I can already provide limited feedback.
For example, I don't think we need these flags. Direct CLI flags (IMHO) are remnants from the time before we had the unified roxie config. Everything can be set there and these settings specifically are not settings that we need every day (outside of automation), hence I think it's perfectly fine to drop these and let users configure those in the roxie config if they need to.

@vladbologa vladbologa Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a roxie user I'd actually like to have them.

To give you an example, for cert refresh and CA rotation I had to do a lot of mixed version testing. It was quite tedious to do and because of that I didn't do as much compatibility testing as I should have. And these flags would make it quite easy to be able to switch around versions easily.

@mclasmeier mclasmeier Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they are important for you, fine.

But you do know that you can do the same with --set, right?

@vladbologa vladbologa Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, didn't think about that. Then I'm inclined to agree with you.

Having a flag is still somewhat useful because it makes the feature more easily discoverable, that's the only argument I have for it now.

withApplyFn("version", func(config *deployer.Config, tag string) error {
config.Central.Operator.Version = tag
return nil
}),
)

registerFlag(cmd, settings, "secured-cluster-tag", "Image tag for SecuredCluster (overrides --tag for SecuredCluster)",
withApplyFn("version", func(config *deployer.Config, tag string) error {
config.SecuredCluster.Operator.Version = tag
return nil
}),
)

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 {
key, value, err := deployer.ParseOperatorEnvVar(envExpr)
Expand Down Expand Up @@ -241,25 +255,6 @@ func runDeploy(cmd *cobra.Command, args []string) error {
return err
}

if !deploySettings.Central.EarlyReadinessEnabled() || !deploySettings.SecuredCluster.EarlyReadinessEnabled() {
// Explanation on the versions involved here:
// Deploying StackRox begins with picking a "main image tag" -- this is a version identifier, which cannot be reliably parsed as a semver.
// But there is a derived version from that -- the operator version -- which can be parsed as a semver.
//
// The invocation of deploySettings.Operator.Configure() above in this function prepares the operator deployment config in the sense
// that top-level roxie configuration options are propagated to the concrete operator deployment configuration. This includes also
// storing of the derived operator version within the operator configuration.
//
// This is why we use the operator version here when checking version constraints.
hasSupport, err := stackroxversions.SupportsAdditionalPrinterColumns(deploySettings.Operator.Version)
if err != nil {
return fmt.Errorf("checking version constraint on main image tag %s: %w", deploySettings.Roxie.Version, err)
}
if !hasSupport {
return fmt.Errorf("--early-readiness=false can only be used for StackRox versions satisfying %s", stackroxversions.SupportsAdditionalPrinterColumnsConstraint.String())
}
}

d, err := deployer.New(log)
if err != nil {
return fmt.Errorf("failed to create deployer: %w", err)
Expand Down Expand Up @@ -390,10 +385,6 @@ func configureConfig(log *logger.Logger, components component.Component, deployS
return fmt.Errorf("configuring operator configuration: %w", err)
}

if deploySettings.Roxie.KonfluxImagesEnabled() {
deployer.PopulateKonfluxEnvVars(deploySettings)
}

if components.IncludesCentral() {
if err := deploySettings.Central.ConfigureSpec(&deploySettings.Roxie); err != nil {
return fmt.Errorf("configuring Central spec: %w", err)
Expand Down Expand Up @@ -458,5 +449,38 @@ func deployValidate(components component.Component, deploySettings *deployer.Con
}
}

if deploySettings.HasMixedVersions() {
globalLogger.Dimf("Mixed versions detected (configured via --central-tag / --secured-cluster-tag or central.operator / securedCluster.operator)")

@mclasmeier mclasmeier Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, I'd suggest to propagate the logger from here for consistency -- I think currently we are nowhere using the globalLogger directly.

if components.IncludesOperatorExplicitly() {
return errors.New("mixed versions are not supported with operator-only deploy")
}
if deploySettings.Operator.DeployViaOlmEnabled() {
return errors.New("mixed versions are not supported with OLM deployment mode")
}
}

if !deploySettings.Central.EarlyReadinessEnabled() || !deploySettings.SecuredCluster.EarlyReadinessEnabled() {
// Explanation on the versions involved here:
// Deploying StackRox begins with picking a "main image tag" -- this is a version identifier, which cannot be reliably parsed as a semver.
// But there is a derived version from that -- the operator version -- which can be parsed as a semver.
//
// The invocation of deploySettings.Operator.Configure() above in this function prepares the operator deployment config in the sense
// that top-level roxie configuration options are propagated to the concrete operator deployment configuration. This includes also
// storing of the derived operator version within the operator configuration.
//
// This is why we use the operator version here when checking version constraints.
// Check every operator instance that will be deployed.
for _, instance := range deploySettings.OperatorInstances() {
hasSupport, err := stackroxversions.SupportsAdditionalPrinterColumns(instance.Version)

@mclasmeier mclasmeier Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, we are conflating the central&securedCluster versions here, right?
If central has earlyReadiness disabled and the central version supports it, but the secured Cluster version doesn't support it, we would still fail, right?

if err != nil {
return fmt.Errorf("checking version constraint on operator version %s: %w", instance.Version, err)
}
if !hasSupport {
return fmt.Errorf("--early-readiness=false can only be used for StackRox versions satisfying %s (operator version %s in %s does not)",
stackroxversions.SupportsAdditionalPrinterColumnsConstraint.String(), instance.Version, instance.Namespace)
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return nil
}
45 changes: 33 additions & 12 deletions internal/deployer/acs_images.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,30 +7,51 @@ import (
)

func imagesForConfig(config Config) []string {
images := make([]string, 0)
var images []string
prefix := ""
if config.Roxie.KonfluxImagesEnabled() {
prefix = "release-"
}

imageRegistry := constants.DefaultRegistry
images = append(images, fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "main", config.Roxie.Version))
images = append(images, fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "central-db", config.Roxie.Version))
images = append(images, fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "scanner-v4-db", config.Roxie.Version))
images = append(images, fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "scanner-v4", config.Roxie.Version))

for _, mainTag := range uniqueMainVersions(config) {
images = append(images,
fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "main", mainTag),
fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "central-db", mainTag),
fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "scanner-v4-db", mainTag),
fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "scanner-v4", mainTag),
)
}

operatorPrefix := prefix
if !config.Roxie.KonfluxImagesEnabled() {
prefix = "stackrox-"
operatorPrefix = "stackrox-"
}
for _, instance := range config.OperatorInstances() {
images = append(images,
fmt.Sprintf("%s/%s%s:%s", imageRegistry, operatorPrefix, "operator", instance.Version),
OperatorBundleImage(instance.Version, config.Roxie.KonfluxImagesEnabled()),
)
}
images = append(images, fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "operator", config.Operator.Version))
images = append(images, OperatorBundleImage(config))

return images
}

func OperatorBundleImage(config Config) string {
func uniqueMainVersions(config Config) []string {

@mclasmeier mclasmeier Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My feeling is that this can be simplified.
For example, don't Effect*Version() already handle the Roxie.Version fallback?
In that case, wouldn't a single slices.Compact() on the versions slice be sufficient?

@vladbologa vladbologa Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9169418

central := config.CentralVersion()
sc := config.SecuredClusterVersion()
if central == sc {
return []string{central}
}
return []string{central, sc}
}

// OperatorBundleImage returns the operator bundle image for a specific operator version.
func OperatorBundleImage(operatorVersion string, konflux bool) string {

@mclasmeier mclasmeier Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been asking myself, why you changed this function from

func OperatorBundleImage(config Config) string { ... }

to

func OperatorBundleImage(operatorVersion string, konflux bool) string { ... }

the answer seems to be: you made it working for "OperatorInstances". This got me thinking:

  1. Isn't it actually a bogus decision that we have roxie.konfluxImages now? Seems to me that this should actually also be a field in OperatorConfig!

  2. The OperatorInstance -- do we really need it? I have the impression we could simplify the code more if we were just using the OperatorConfig instead -- that essentially describes how an operator instance should look like.

  3. The only thing that the Operator Instance currently provides, what the OperatorConfig currently doens't provide is the "suffix". But we could add that to OperatorConfig -- in the usual case a user wouldn't have a reason to set it, but even if the user did set it, it wouldn't harm. And we gut drop the OperatorInstance and get rid of the conversion, no?

WDYT?

@vladbologa vladbologa Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should start with OperatorConfig, because as it right now it allows overriding some fields that don't make sense per individual operator (SkipDeployment, DeployViaOlm and also KonfluxImages if we add it to the struct).

E.g. now you can do things like:

central.operator.deployViaOlm = true
securedCluster.operator.deployViaOlm = false

So to address this, and also your concerns (btw at point 3, there's also the namespace that OperatorInstance adds), I'm thinking of something like this:

// OperatorInstanceConfig describes how to deploy a single operator instance.
type OperatorInstanceConfig struct {
    Version        string            `yaml:"version,omitempty"`
    EnvVars        map[string]string `yaml:"envVars,omitempty"`
    Namespace      string            `yaml:"namespace,omitempty"`
    RoleNameSuffix string            `yaml:"roleNameSuffix,omitempty"`
}

// OperatorConfig is the top-level operator configuration.
type OperatorConfig struct {
    SkipDeployment *bool `yaml:"skipDeployment,omitempty"`
    DeployViaOlm   *bool `yaml:"deployViaOlm,omitempty"`
    KonfluxImages  bool              `yaml:"konfluxImages,omitempty"`
    OperatorInstanceConfig   `yaml:",inline"`
}

type CentralConfig struct {
    Operator  OperatorInstanceConfig  `yaml:"operator,omitempty"`
    // ... other fields
}

type SecuredClusterConfig struct {
    Operator  OperatorInstanceConfig  `yaml:"operator,omitempty"`
    // ... other fields
}

type Config struct {
    Roxie          RoxieConfig          `yaml:"roxie,omitempty"`
    Operator       OperatorConfig       `yaml:"operator,omitempty"`
    Central        CentralConfig        `yaml:"central,omitempty"`
    SecuredCluster SecuredClusterConfig `yaml:"securedCluster,omitempty"`
}

This is backwards compatible (OperatorInstanceConfig is added inline to OperatorConfig), doesn't allow inconsistent configurations, and also OperatorInstance can go away, we just return []OperatorConfig.

And also OperatorBundleImage can take just a config.

imageRegistry := constants.DefaultRegistry
if config.Roxie.KonfluxImagesEnabled() {
return fmt.Sprintf("%s/release-operator-bundle:v%s", imageRegistry, config.Operator.Version)
if konflux {
return fmt.Sprintf("%s/release-operator-bundle:v%s", imageRegistry, operatorVersion)
}
return fmt.Sprintf("%s/stackrox-operator-bundle:v%s", imageRegistry, config.Operator.Version)
return fmt.Sprintf("%s/stackrox-operator-bundle:v%s", imageRegistry, operatorVersion)
}
16 changes: 11 additions & 5 deletions internal/deployer/config.go
View file
Open in desktop
Comment thread
mclasmeier marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,12 @@ 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"`
EnvVars map[string]string `yaml:"envVars,omitempty"`
SkipDeployment *bool `yaml:"skipDeployment,omitempty"`
DeployViaOlm *bool `yaml:"deployViaOlm,omitempty"`
// Version can hold either a main image tag or an operator tag. It should be normalized to an
// operator tag via ConvertToOperatorTag before use (the function is idempotent, so either form is accepted).
Version string `yaml:"version,omitempty"`
EnvVars map[string]string `yaml:"envVars,omitempty"`
}

func (c *OperatorConfig) SkipDeploymentSet() bool {
Expand All @@ -101,7 +103,7 @@ func (c *OperatorConfig) DeployViaOlmEnabled() bool {

// Configure derives the operator version from the roxie configuration.
func (c *OperatorConfig) Configure(roxieConfig *RoxieConfig) error {
c.Version = helpers.ConvertMainTagToOperatorTag(roxieConfig.Version)
c.Version = helpers.ConvertToOperatorTag(roxieConfig.Version)
return nil
}

Expand All @@ -115,6 +117,8 @@ type WaitConfig struct {

// CentralConfig holds deployment settings for the Central component.
type CentralConfig struct {
// Operator allows per-component operator overrides; only used in dual-operator mode.
Operator OperatorConfig `yaml:"operator,omitempty"`
Namespace string `yaml:"namespace,omitempty"`
ResourceProfile types.ResourceProfile `yaml:"resourceProfile,omitempty"`
PauseReconciliation *bool `yaml:"pauseReconciliation,omitempty"`
Expand Down Expand Up @@ -262,6 +266,8 @@ func (c *CentralConfig) CustomResource() (map[string]interface{}, error) {

// SecuredClusterConfig holds deployment settings for the SecuredCluster component.
type SecuredClusterConfig struct {
// Operator allows per-component operator overrides; only used in dual-operator mode.
Operator OperatorConfig `yaml:"operator,omitempty"`
Namespace string `yaml:"namespace,omitempty"`
ResourceProfile types.ResourceProfile `yaml:"resourceProfile,omitempty"`
PauseReconciliation *bool `yaml:"pauseReconciliation,omitempty"`
Expand Down
Loading
Loading