Skip to content

chore: integrate crdify CRD upgrade safety checks into CI [RHIDP-8670] - #3290

Open
Fortune-Ndlovu wants to merge 6 commits into
redhat-developer:mainfrom
Fortune-Ndlovu:RHIDP-8670-integrate-crdify-ci-olm-v1-checks
Open

chore: integrate crdify CRD upgrade safety checks into CI [RHIDP-8670]#3290
Fortune-Ndlovu wants to merge 6 commits into
redhat-developer:mainfrom
Fortune-Ndlovu:RHIDP-8670-integrate-crdify-ci-olm-v1-checks

Conversation

@Fortune-Ndlovu

@Fortune-Ndlovu Fortune-Ndlovu commented Jul 29, 2026

Copy link
Copy Markdown
Member

Description

OLM v1mod.go file uses crdify (https://github.com/kubernetes-sigs/crdify) to perform CRD upgrade safety preflight checks at runtime. If the RHDH operator's CRD has breaking changes between versions, OLM v1 rejects the upgrade on customer clusters see https://github.com/operator-framework/operator-controller/blob/main/internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety.go. This PR integrates crdify into CI so we catch these issues at PR time rather than at runtime.

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

PR acceptance criteria

  • Tests
  • Documentation

How to test changes / Special notes to the reviewer

# 1. Check against main (default baseline)
make crd-upgrade-check

# 2. Check against a specific release branch
make crd-upgrade-check CRD_BASELINE_REF=release-1.10

# 3. Negative test temporarily break the CRD, verify crdify catches it
sed -i '/^\s*replicas:/d' config/crd/bases/rhdh.redhat.com_backstages.yaml
make crd-upgrade-check
# Should FAIL with NoExistingFieldRemoved

# 4. Restore the CRD after negative test
git checkout config/crd/bases/rhdh.redhat.com_backstages.yaml

Building Container Images for Testing

Need to test container images from this PR?

For Maintainers: To trigger a test image build, review the code and comment /build-images.
This always builds the HEAD of the PR branch.

For Contributors: Ask a maintainer to run /build-images.

Images will be built and pushed to Quay with links posted in comments.

…ecks that OLM v1 does

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
@Fortune-Ndlovu
Fortune-Ndlovu requested a review from a team as a code owner July 29, 2026 17:58
@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

Integrate crdify CRD upgrade safety checks into PR CI

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Add a PR workflow to run CRD upgrade safety checks on CRD/API changes.
• Introduce a Makefile target that runs crdify against the latest relevant git tag.
• Configure crdify validation behavior to align with OLM v1 preflight expectations.
Diagram

graph TD
  A["Pull request"] --> B["GH Actions workflow"] --> C["make crd-upgrade-check"] --> D["crdify"] --> E["Baseline vs current"]
  E --> F["Baseline CRD (git tag)"]
  E --> G["Current CRD (working tree)"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use a pinned crdify release binary/container instead of go install
  • ➕ Faster CI execution (no module compilation)
  • ➕ Less reliance on Go toolchain behavior and module proxy availability
  • ➖ Requires handling OS/arch selection and checksum/signature verification
  • ➖ Adds separate supply-chain management outside existing go-install-tool pattern
2. Run checks via a reusable workflow/action shared across repos
  • ➕ Centralizes maintenance and policy changes
  • ➕ Consistent CRD upgrade enforcement across multiple operator repos
  • ➖ Additional indirection when debugging failures
  • ➖ Harder to tailor baseline selection logic per release branch

Recommendation: Current approach is solid for this repo: it reuses the existing Makefile tool-install pattern, scopes execution to CRD/API-changing PRs, and selects an appropriate baseline tag for release branches. If CI runtime becomes a concern, consider switching the workflow to download a pinned crdify binary (with checksums) while keeping the Makefile target as a local developer entry point.

Files changed (3) +110 / -0

Enhancement (1) +35 / -0
MakefileAdd crdify tool install + CRD upgrade-check target +35/-0

Add crdify tool install + CRD upgrade-check target

• Adds a local crdify tool installation target (version pinned) and a crd-upgrade-check Make target. The upgrade check compares the baseline-tag CRD to the working tree CRD, prints parsed JSON output when possible, treats CRD-level and same-version errors as blocking, and reports served-version issues as warnings.

Makefile

Other (2) +75 / -0
.crdify.yamlAdd crdify validation config (warn on description validation) +3/-0

Add crdify validation config (warn on description validation)

• Introduces a crdify configuration file to control which validations run and their enforcement level. Currently sets the "description" validation to Warn to reduce noise while still reporting issues.

.crdify.yaml

pr-crd-upgrade-check.yamlAdd PR workflow to run CRD upgrade safety checks +72/-0

Add PR workflow to run CRD upgrade safety checks

• Adds a GitHub Actions workflow triggered on PRs to main and release-* branches when CRD/API-related paths change. The job determines the baseline release tag (matching release branch prefix when applicable), verifies the CRD exists at that tag, then runs the Makefile upgrade check.

.github/workflows/pr-crd-upgrade-check.yaml

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Jul 29, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Context used
⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials
✅ Compliance rules (platform): 18 rules
✅ Cross-repo context
  Not relevant to this PR: redhat-developer/rhdh
  Not relevant to this PR: redhat-developer/rhdh-plugins

Grey Divider


Action required

1. crdify failures can false-pass ✓ Resolved 🐞 Bug ☼ Reliability
Description
make crd-upgrade-check forces success (|| true) and treats any jq parse failure as zero
errors, so a crdify execution failure or non-JSON output can incorrectly result in “CRD upgrade
safety check passed.”
Code

Makefile[R239-244]

+	RESULTS=$$($(CRDIFY) "git://$(CRD_BASELINE_REF)?path=$(CRD_FILE)" "file://$(CRD_FILE)" \
+		--config .crdify.yaml -o json 2>&1) || true; \
+	echo "$$RESULTS" | jq -r . 2>/dev/null || echo "$$RESULTS"; \
+	CRD_ERRORS=$$(echo "$$RESULTS" | jq '[(.crdValidation // [])[] | (.errors // [])[] ] | length' 2>/dev/null || echo "0"); \
+	SAME_VER_ERRORS=$$(echo "$$RESULTS" | jq '[(.sameVersionValidation // [])[] | (.errors // [])[] ] | length' 2>/dev/null || echo "0"); \
+	SERVED_VER_ERRORS=$$(echo "$$RESULTS" | jq '[(.servedVersionValidation // [])[] | (.errors // [])[] ] | length' 2>/dev/null || echo "0"); \
Relevance

●●● Strong

Team has accepted hardening scripts to not silently succeed on parse/validation failures;
false-green CI checks are undesirable.

PR-#1844
PR-#2828

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The recipe explicitly appends || true to the crdify invocation and then computes error counts
using jq ... || echo "0"; if crdify fails or emits non-JSON (especially with 2>&1), jq can
fail and the code will treat it as zero errors, allowing a false-green pass.

Makefile[229-255]
PR-#3169

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

### Issue description
The `crd-upgrade-check` target masks failure signals:
- `crdify` exit status is ignored via `|| true`.
- Output is captured with `2>&1`, which can mix logs/errors into stdout.
- Each `jq` parse failure falls back to `echo "0"`, which can produce `BLOCKING_ERRORS=0` and pass the check even when no valid JSON result was produced.

### Issue Context
This target is invoked by CI. If `crdify` fails (bad ref/path, tool error, unexpected output), CI may still succeed and claim the safety check passed.

### Fix Focus Areas
- Makefile[233-255]

### Suggested change
1. Capture and check `crdify`’s exit code.
2. Don’t merge stderr into the JSON stream (or write JSON to a temp file).
3. Require valid JSON with `jq -e` and fail if parsing fails.

Example approach (bash):
- Run `crdify ... -o json > results.json` (leave stderr alone)
- If `crdify` exits non-zero: print stderr and `exit 1`
- Validate JSON: `jq -e . results.json >/dev/null` else `exit 1`
- Compute error counts from the validated JSON.

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



Informational

2. Release branch filter mismatch 🐞 Bug ≡ Correctness
Description
The workflow’s on.pull_request.branches uses a regex-like pattern (release-[0-9]+.[0-9]+), but
GitHub Actions branch filters are glob-style patterns, so typical release branches like
release-1.2 will not match and the check won’t run on those PRs.
Code

.github/workflows/pr-crd-upgrade-check.yaml[R6-9]

+    branches:
+      - 'main'
+      - 'release-[0-9]+.[0-9]+'
+    paths:
Relevance

● Weak

Exact precedent rejected changing regex-like branch filters; team kept release-[0-9]+.[0-9]+
despite glob mismatch.

PR-#3189

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow declares branches: ['main', 'release-[0-9]+.[0-9]+']; GitHub Actions branch filters
are glob-based, so + is literal and the pattern won’t match common release-<digits>.<digits>
branch names (e.g., release-1.2).

.github/workflows/pr-crd-upgrade-check.yaml[4-14]

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

### Issue description
The workflow is intended to run on `main` and `release-X.Y` branches, but the current `branches` filter uses a regex-like pattern (`release-[0-9]+.[0-9]+`). GitHub Actions uses glob matching for these filters, so `+` is treated as a literal character and common branch names like `release-1.2` won’t match.

### Issue Context
This prevents the CRD upgrade safety workflow from running for release-branch PRs, creating a coverage gap exactly where upgrade safety checks are most important.

### Fix Focus Areas
- .github/workflows/pr-crd-upgrade-check.yaml[6-9]

### Suggested change
Replace the current release filter with a glob that matches your naming convention, for example:
- `release-[0-9]*.[0-9]*` (broad)
- or `release-*.*` (simpler)
If you need stricter validation, enforce it inside the job (e.g., a bash regex check) rather than in the `branches` glob.

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


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

@rhdh-qodo-merge rhdh-qodo-merge Bot added enhancement New feature or request Tests labels Jul 29, 2026
- Remove .crdify.yaml config file
- Compare PR CRD against base branch (origin/main) instead of release tags
- Mark v1alpha4 as unserved to fix served-version incompatibilities
- Regenerate CRD manifests

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
…nch comparison

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
@Fortune-Ndlovu

Copy link
Copy Markdown
Member Author

/agentic_review

…k PRs

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
@rhdh-qodo-merge

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 2ac8a3b

The git:// URL parser splits on '/' so origin/main was parsed as
ref='origin' not 'origin/main'. Fetch base branch into a local ref
and pass just the branch name instead.

Signed-off-by: Fortune Ndlovu <fndlovu@redhat.com>
@sonarqubecloud

Copy link
Copy Markdown

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

Labels

enhancement New feature or request Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant