Skip to content

fix: resolve operator version and Konflux selection at read time - #316

Open
porridge wants to merge 2 commits into
mainfrom
fix-operator-version-resolution
Open

porridge wants to merge 2 commits into
mainfrom
fix-operator-version-resolution

Conversation

@porridge

@porridge porridge commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Why

OperatorConfig.Configure used to bake resolved values into the persisted config:

  • it set operator.version = roxie.version unconditionally, so an explicit operator.version was ignored. In CI for test: Migrate race-condition tests to roxie stackrox#23397 this made the operator bundle incorrectly resolve to -rcd tag (which does not exist for that image)
  • it baked roxie.konfluxImages into operator.konfluxImages.

Because the post-Configure config is persisted into the cluster manifest and reused as the base on later runs, the baked values won on redeploy — a subsequent --tag / --konflux on the same cluster became a no-op.

What

  • Resolve the operator version lazily in CentralVersion/SecuredClusterVersion: component.operator.version → operator.version → roxie.version.
  • Resolve Konflux selection the same way in resolveKonfluxImages.
  • With nothing left to bake, remove OperatorConfig.Configure and its call site.

Nothing version/Konflux-related is persisted anymore, an explicit operator.version wins, and --tag/--konflux always take effect on redeploy.

Regression tests added that fail at tip of main and pass here.

porridge and others added 2 commits October 19, 2026 09:29
OperatorConfig.Configure set operator.version = roxie.version
unconditionally, discarding an explicitly configured operator.version. In
CI this caused the operator bundle to be fetched with the main image's
"-rcd" tag (roxie.version) rather than the configured operator.version,
yielding MANIFEST_UNKNOWN since no "-rcd" operator bundle is published.

Only assigning when empty would fix that symptom, but it introduces a new
bug: the post-Configure config is persisted into the cluster manifest, so
the resolved version gets baked in and wins on subsequent runs. A later
`roxie deploy --tag=<new>` onto the same cluster would then be a no-op for
the operator, because operator.version (baked) is non-empty and --tag only
sets roxie.version.

Instead of baking, resolve lazily. Configure no longer touches the version;
CentralVersion/SecuredClusterVersion now fall back
central/securedCluster.operator.version -> operator.version -> roxie.version.
Nothing version-related is persisted, an explicit operator.version still
wins, and --tag always takes effect on redeploy.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…figure

Apply the same read-time resolution to KonfluxImages as was done for the
operator version: resolveKonfluxImages now falls back per-component ->
operator.konfluxImages -> roxie.konfluxImages, instead of relying on
Configure to bake roxie.konfluxImages into operator.konfluxImages. As with
the version, baking persisted the resolved value into the cluster manifest,
so toggling --konflux on an existing cluster was a no-op on redeploy.

With both the version and Konflux fallbacks resolved lazily, Configure had
nothing left to do, so it and its call site in configureConfig are removed.
The comment at that call site was already stale: it claimed to perform the
"transformation of main image tags to semver compatible versions", which is
actually done by MainTag.ToOperatorTag() at image-reference build time, not
in Configure.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: stackrox/roxie/.coderabbit.yml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 505d34a5-7e53-41d0-b945-b349be9fe5ec
📥 Commits

Reviewing files that changed from the base of the PR and between 1773975 and 3341562.

📒 Files selected for processing: 4
  • cmd/deploy.go
  • internal/deployer/config.go
  • internal/deployer/operator_instances.go
  • internal/deployer/operator_instances_test.go
💤 Files with no reviewable changes: 2
  • cmd/deploy.go
  • internal/deployer/config.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Operator version and Konflux image resolution now falls back to Roxie configuration after checking component-specific and shared operator settings. The deployment command no longer calls Operator.Configure, and that configuration method was removed.

Changes: Operator configuration fallback

Layer / File(s) Summary
Resolve operator settings from configuration
internal/deployer/operator_instances.go, internal/deployer/operator_instances_test.go, cmd/deploy.go, internal/deployer/config.go
Central and SecuredCluster versions and Konflux images now fall back to Roxie settings after component-specific and shared operator settings. Tests cover fallback behavior and operator-version precedence. The configure-time call and method were removed.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: vladbologa, mclasmeier

Merge Risk: ⚪ Minimal · up to 33415

Operator versions and Konflux images now resolve from component, shared operator, then Roxie settings. The removed configuration-time copy does not disrupt tag validation, and no concrete merge-blocking risk is established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely describes the main change: resolving operator version and Konflux selection at read time.
  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@porridge porridge changed the title Resolve operator version and Konflux selection at read time fix: resolve operator version and Konflux selection at read time Oct 19, 2026
@porridge
porridge marked this pull request as ready for review October 19, 2026 07:50
@porridge
porridge requested a review from vladbologa October 19, 2026 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant