Skip to content

fix(ci): use dynamic API version in upgrade test - #2998

Merged
rm3l merged 1 commit into
redhat-developer:mainfrom
rm3l:RHDHBUGS-3349--rhdh-operator-nightly-upgrade-tests-1-10-main-failing
Jun 12, 2026
Merged

fix(ci): use dynamic API version in upgrade test#2998
rm3l merged 1 commit into
redhat-developer:mainfrom
rm3l:RHDHBUGS-3349--rhdh-operator-nightly-upgrade-tests-1-10-main-failing

Conversation

@rm3l

@rm3l rm3l commented Jun 12, 2026

Copy link
Copy Markdown
Member

Description

The upgrade test hardcoded rhdh.redhat.com/v1alpha3 when creating the Backstage CR against the "from" operator. Since 1.10+ no longer serves v1alpha3 (#2727), the 1.10 => main upgrade path fails: https://github.com/redhat-developer/rhdh-operator/actions/runs/27315253434/job/80694281540

This PR queries the installed CRD for its storage version instead, which is always served regardless of the branch.

Will need to be cherry-picked to release-1.10 as well.

Which issue(s) does this PR fix or relate to

PR acceptance criteria

  • Tests
  • Documentation

How to test changes / Special notes to the reviewer

The upgrade test hardcoded `rhdh.redhat.com/v1alpha3` when creating the
Backstage CR against the "from" operator. Since release-1.10 no longer
serves v1alpha3, the 1.10=>main upgrade path fails.

Query the installed CRD for its storage version instead, which is always
served regardless of the branch.

Assisted-by: Claude
@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Jun 12, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Context used
✅ Tickets: RHDHBUGS-3349
✅ Compliance rules (platform): 18 rules

Grey Divider


Informational

1. Unvalidated CRD version string 🐞 Bug ☼ Reliability
Description
GetBackstageCRDStorageVersion trims the raw output of helper.Run (which merges stdout and stderr)
and returns it without validating it’s a single version token; any warning text or extra tokens
would be embedded into the YAML apiVersion and can make the subsequent apply fail. This is an
edge-case reliability risk that can cause flaky upgrade tests depending on cluster/CLI warning
behavior.
Code

tests/helper/helper_backstage.go[R95-105]

+	cmd := exec.Command(GetPlatformTool(), "get", "crd", "backstages.rhdh.redhat.com",
+		"-o", `jsonpath={.spec.versions[?(@.storage==true)].name}`) // #nosec G204
+	out, err := Run(cmd)
+	if err != nil {
+		return "", fmt.Errorf("failed to get Backstage CRD storage version: %w", err)
+	}
+	version := strings.TrimSpace(string(out))
+	if version == "" {
+		return "", fmt.Errorf("no storage version found in Backstage CRD")
+	}
+	return version, nil
Relevance

⭐ Low

Test reliability hardening suggestions often rejected (e.g., pod selection robustness, DownloadFile
hardening) in PRs #1652, #1650.

PR-#1652
PR-#1650

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new helper returns the trimmed command output as the API version, but the command runner
combines stdout and stderr into the returned bytes; this unvalidated string is then interpolated
directly into the YAML apiVersion field used by kubectl apply in the upgrade test.

tests/helper/helper_backstage.go[93-106]
tests/helper/utils.go[216-243]
tests/e2e/e2e_upgrade_test.go[52-74]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`GetBackstageCRDStorageVersion()` uses `helper.Run(cmd)` and then `strings.TrimSpace(string(out))` as the version. `helper.Run` intentionally merges command stdout and stderr into one returned buffer. If `kubectl/oc` emits any warnings on stderr (or if the jsonpath returns multiple values), the resulting string can become an invalid Kubernetes API version and will break the YAML apply in the upgrade test.

### Issue Context
This risk is introduced by the new dynamic API version lookup and its direct interpolation into the Backstage CR YAML.

### Fix Focus Areas
- tests/helper/helper_backstage.go[93-106]
- tests/helper/utils.go[216-243]
- tests/e2e/e2e_upgrade_test.go[52-74]

### Suggested fix
- In `GetBackstageCRDStorageVersion`, capture **stdout only** for the jsonpath result (e.g., run the command without `helper.Run`, or add a `RunStdoutOnly` helper that does not merge stderr into the returned bytes).
- Validate the extracted value:
 - `fields := strings.Fields(stdoutString)`
 - require `len(fields) == 1`, otherwise return an error that includes the raw output for debugging.
- (Optional) Ensure the version matches expected format (e.g., `^v\d` / `^v\d+(alpha\d+|beta\d+)?$`) before returning.
- Keep stderr output logged to `GinkgoWriter` for debuggability, but don’t let it affect the parsed version.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@rm3l

rm3l commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

/cherry-pick release-1.10

@sonarqubecloud

Copy link
Copy Markdown

@openshift-cherrypick-robot

Copy link
Copy Markdown

@rm3l: once the present PR merges, I will cherry-pick it on top of release-1.10 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-1.10

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

Fix upgrade e2e test by deriving Backstage CRD storage API version
🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

Walkthroughs

Description
• Fix 1.10→main upgrade e2e failures caused by hardcoded Backstage API version.
• Derive the Backstage CR apiVersion from the installed CRD storage version.
• Add helper to query CRD storage version via platform CLI jsonpath.
Diagram
graph TD
  A["Upgrade e2e test"] --> B["Read CRD storage ver"] --> C{{"oc/kubectl"}} --> D[("Backstage CRD")]
  A --> E["Apply Backstage CR"] --> C --> F["Backstage CR"]

  subgraph Legend
    direction LR
    _t["Test code"] ~~~ _cli{{"CLI"}} ~~~ _crd[("CRD")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use API discovery (RESTMapper) instead of CLI jsonpath
  • ➕ Avoids shelling out to oc/kubectl from tests
  • ➕ Can be more structured/typed and resilient to CLI output quirks
  • ➖ Requires Kubernetes client wiring/auth config in test harness
  • ➖ More code and moving parts than needed for a simple e2e helper
2. Try multiple known apiVersions (fallback list)
  • ➕ No CRD query needed; simple control flow
  • ➖ Still brittle across future removals/renames
  • ➖ May mask real incompatibilities; needs ongoing maintenance

Recommendation: Current approach (query CRD storage version and use it as the CR apiVersion) is the most robust and lowest-maintenance option for cross-branch upgrade testing, with minimal additional complexity. The main caveat is continued reliance on the platform CLI, but that is already a core part of the existing e2e harness.

Grey Divider

File Changes

Bug fix (1)
e2e_upgrade_test.go Use CRD storage version for Backstage CR apiVersion in upgrade test +6/-2

Use CRD storage version for Backstage CR apiVersion in upgrade test

• The upgrade e2e test now determines the Backstage CRD storage API version before creating the Backstage CR. It uses this version in the applied manifest and logs the chosen apiVersion for debugging.

tests/e2e/e2e_upgrade_test.go


Other (1)
helper_backstage.go Add helper to read Backstage CRD storage version via CLI jsonpath +15/-0

Add helper to read Backstage CRD storage version via CLI jsonpath

• Adds GetBackstageCRDStorageVersion(), which queries the installed backstages.rhdh.redhat.com CRD for the storage version name. The helper trims/validates output and returns clear errors when unavailable.

tests/helper/helper_backstage.go


Grey Divider

Qodo Logo

@rhdh-qodo-merge

Copy link
Copy Markdown

Preparing PR labels...

@rm3l

rm3l commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

Merging so we can manually trigger the upgrade nightly checks..

@rm3l
rm3l merged commit ce108e3 into redhat-developer:main Jun 12, 2026
10 of 11 checks passed
@rm3l
rm3l deleted the RHDHBUGS-3349--rhdh-operator-nightly-upgrade-tests-1-10-main-failing branch June 12, 2026 11:50
@openshift-cherrypick-robot

Copy link
Copy Markdown

@rm3l: new pull request created: #2999

Details

In response to this:

/cherry-pick release-1.10

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants