Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
59 changes: 53 additions & 6 deletions cmd/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,26 @@ 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)",

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.

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.

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?

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 {
if config.Central.Operator == nil {
config.Central.Operator = &deployer.OperatorConfig{}
}
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 {
if config.SecuredCluster.Operator == nil {
config.SecuredCluster.Operator = &deployer.OperatorConfig{}
}
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 @@ -251,12 +271,19 @@ func runDeploy(cmd *cobra.Command, args []string) error {
// 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)
// Check every operator instance that will be deployed.
var versionsToCheck []string
for _, instance := range deploySettings.OperatorInstances() {
versionsToCheck = append(versionsToCheck, instance.Version)
}
if !hasSupport {
return fmt.Errorf("--early-readiness=false can only be used for StackRox versions satisfying %s", stackroxversions.SupportsAdditionalPrinterColumnsConstraint.String())
for _, opVersion := range versionsToCheck {
hasSupport, err := stackroxversions.SupportsAdditionalPrinterColumns(opVersion)
if err != nil {
return fmt.Errorf("checking version constraint on operator version %s: %w", opVersion, err)
}
if !hasSupport {
return fmt.Errorf("--early-readiness=false can only be used for StackRox versions satisfying %s", stackroxversions.SupportsAdditionalPrinterColumnsConstraint.String())
}
}
}

Expand Down Expand Up @@ -390,7 +417,10 @@ func configureConfig(log *logger.Logger, components component.Component, deployS
return fmt.Errorf("configuring operator configuration: %w", err)
}

if deploySettings.Roxie.KonfluxImagesEnabled() {
// For the single-operator path (including OLM), populate RELATED_IMAGE_* on the
// top-level OperatorConfig. Mixed-version deployments apply Konflux env vars
// per OperatorInstance during deployment instead.
if deploySettings.Roxie.KonfluxImagesEnabled() && !deploySettings.HasMixedVersions() {
deployer.PopulateKonfluxEnvVars(deploySettings)
}

Expand Down Expand Up @@ -458,5 +488,22 @@ func deployValidate(components component.Component, deploySettings *deployer.Con
}
}

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

centralVer := deploySettings.EffectiveCentralVersion()
scVer := deploySettings.EffectiveSecuredClusterVersion()
if centralVer == scVer && centralVer != deploySettings.Roxie.Version &&
deploySettings.Central.Operator != nil && deploySettings.SecuredCluster.Operator != nil {
return fmt.Errorf("both --central-tag and --secured-cluster-tag are set to %s which differs from --tag %s; use --tag %s instead",
centralVer, deploySettings.Roxie.Version, centralVer)
}

return nil
}
63 changes: 53 additions & 10 deletions internal/deployer/acs_images.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"

"github.com/stackrox/roxie/internal/constants"
"github.com/stackrox/roxie/internal/helpers"
)

func imagesForConfig(config Config) []string {
Expand All @@ -14,23 +15,65 @@ func imagesForConfig(config Config) []string {
}

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))
seen := make(map[string]bool)
add := func(image string) {
if seen[image] {
return
}
seen[image] = true
images = append(images, image)
}

for _, mainTag := range uniqueMainVersions(config) {
add(fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "main", mainTag))
add(fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "central-db", mainTag))
add(fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "scanner-v4-db", mainTag))
add(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() {
add(fmt.Sprintf("%s/%s%s:%s", imageRegistry, operatorPrefix, "operator", instance.Version))
add(OperatorBundleImageForVersion(instance.Version, config.Roxie.KonfluxImagesEnabled()))
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
images = append(images, fmt.Sprintf("%s/%s%s:%s", imageRegistry, prefix, "operator", config.Operator.Version))
images = append(images, OperatorBundleImage(config))

return images
}

func uniqueMainVersions(config Config) []string {
versions := []string{config.EffectiveCentralVersion(), config.EffectiveSecuredClusterVersion()}
seen := make(map[string]bool)
var unique []string
for _, v := range versions {
if v == "" || seen[v] {
continue
}
seen[v] = true
unique = append(unique, v)
}
if len(unique) == 0 && config.Roxie.Version != "" {
unique = append(unique, config.Roxie.Version)
}
return unique
}

// OperatorBundleImage returns the operator bundle image for the top-level operator version.
func OperatorBundleImage(config Config) string {
version := config.Operator.Version
if version == "" {
version = helpers.ConvertMainTagToOperatorTag(config.Roxie.Version)
}
return OperatorBundleImageForVersion(version, config.Roxie.KonfluxImagesEnabled())
}

// OperatorBundleImageForVersion returns the operator bundle image for a specific operator version.
func OperatorBundleImageForVersion(operatorVersion string, konflux bool) string {
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)
}
6 changes: 6 additions & 0 deletions internal/deployer/config.go

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 think before diving deeper, we should align on the changes to the config structure. I guess there are many ways we could implement this.

What I consider a limitation of your approach here is that there is no way to define per-operator settings (there is just one operator:).

Just thinking loud here, what about we allow users to have a <component>.operator configuration (of type OperatorConfig), where component can be central or securedCluster. If this per-component operator config exists for a component, than that takes precedence and the multi-operator mode of operation kicks in.
This way we could configure the operators differently.

Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ type WaitConfig struct {

// CentralConfig holds deployment settings for the Central component.
type CentralConfig struct {
// Operator, when set, provides a per-component operator config for Central.
// Its presence (with a version) triggers 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 +265,9 @@ func (c *CentralConfig) CustomResource() (map[string]interface{}, error) {

// SecuredClusterConfig holds deployment settings for the SecuredCluster component.
type SecuredClusterConfig struct {
// Operator, when set, provides a per-component operator config for SecuredCluster.
// Its presence (with a version) triggers 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