Skip to content

ROX-27677: Eliminate Python dependency from operator build#18947

Merged
GrimmiMeloni merged 159 commits intomasterfrom
mh/combine-bundle-helpers
Mar 26, 2026
Merged

ROX-27677: Eliminate Python dependency from operator build#18947
GrimmiMeloni merged 159 commits intomasterfrom
mh/combine-bundle-helpers

Conversation

@GrimmiMeloni
Copy link
Copy Markdown
Contributor

@GrimmiMeloni GrimmiMeloni commented Feb 10, 2026

Description

Ports the operator bundle preparation tools from Python to Go, making the Go implementation the default while keeping the Python scripts to verify equivalence via CI.

What changes:

  • New single-binary Go tool (operator/bundle_helpers/main.go) implementing two subcommands:
    • fix-spec-descriptor-order — fix specDescriptor ordering in the operator bundle
    • patch-csv — patch the ClusterServiceVersion file (CSV version handling, image replacement, related images logic for omit/downstream/konflux modes)
  • New dispatch.sh wrapper replaces direct Python script invocations in prepare-bundle-manifests.sh and the Makefile. It routes to the Go binary by default (USE_GO_BUNDLE_HELPER=true) and falls back to Python when explicitly disabled.
  • New CI workflow (.github/workflows/test-bundle-helpers.yaml) that runs both implementations and verifies byte-by-byte output equivalence, ensuring the Go port is correct.
  • Python scripts are retained as a reference implementation and to keep the CI equivalence check meaningful.
  • New RELATED_IMAGES_MODE Makefile variable (omit/downstream/konflux) to control how image references are handled in the CSV.

User-facing documentation

  • CHANGELOG.md is updated OR update is not needed (internal build tooling)
  • documentation PR is created and is linked above OR is not needed (internal)

Testing and quality

  • the change is production ready
  • CI results are inspected (will inspect after creation)

Automated testing

  • added unit tests (comprehensive csv-patcher tests)

How I validated my change

Local validation:

  • All unit tests pass (go test ./...)
  • Linter passes (make golangci-lint)
  • Local comparison script validates byte-by-byte equivalence for all 3 modes (omit, downstream, konflux)
  • operator-sdk bundle validation passes for all 3 modes

CI validation:

  • CI workflow validates byte-by-byte equivalence between Go and Python implementations

Scope:
Internal build tooling only.

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Feb 10, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@GrimmiMeloni
Copy link
Copy Markdown
Contributor Author

/test all

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 1 security issue, and 2 other issues

Security issues:

  • Detected non-static command inside Command. Audit the input to 'exec.Command'. If unverified user data can reach this call site, this is a code injection vulnerability. A malicious actor can inject a malicious script to execute arbitrary code. (link)
Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `operator/bundle_helpers/cmd/fix_descriptors.go:63-64` </location>
<code_context>
+// This handles formatting quirks (quote styles, line wrapping, etc.) while keeping
+// all business logic in Go.
+func normalizeYAMLOutput(goYAML []byte, w io.Writer) error {
+	wd, _ := os.Getwd()
+	normalizerPath := filepath.Join(wd, "bundle_helpers", "yaml-normalizer.py")
+
+	// Run the normalizer
</code_context>

<issue_to_address>
**issue (bug_risk):** YAML normalizer path is likely incorrect when run via dispatch.sh (ends up with bundle_helpers/bundle_helpers/...)

Because `dispatch.sh` runs `go run` from `.../bundle_helpers`, `os.Getwd()` will already be `.../bundle_helpers`. Joining `bundle_helpers/yaml-normalizer.py` produces `.../bundle_helpers/bundle_helpers/yaml-normalizer.py`, which doesn’t exist, so normalization will always fail.

Resolve the script path relative to the source file or executable (e.g., via `os.Executable()` or an embed-based approach). If you can rely on the current working directory being `bundle_helpers`, just join `yaml-normalizer.py` directly instead of adding the extra directory level.
</issue_to_address>

### Comment 2
<location> `operator/bundle_helpers/pkg/rewrite/rewriter_test.go:19` </location>
<code_context>
+		{
+			name:     "bare string cannot be modified",
</code_context>

<issue_to_address>
**suggestion (testing):** Consider a test where nested structures are traversed but no replacements occur to assert modified=false

Adding a case where nested structures are traversed but no values match `old`, and asserting `RewriteStrings` returns `false` and leaves the structure unchanged, would help ensure `modified` isn’t incorrectly set just because traversal occurred.
</issue_to_address>

### Comment 3
<location> `operator/bundle_helpers/cmd/fix_descriptors.go:67` </location>
<code_context>
	cmd := exec.Command(normalizerPath)
</code_context>

<issue_to_address>
**security (go.lang.security.audit.dangerous-exec-command):** Detected non-static command inside Command. Audit the input to 'exec.Command'. If unverified user data can reach this call site, this is a code injection vulnerability. A malicious actor can inject a malicious script to execute arbitrary code.

*Source: opengrep*
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@rhacs-bot
Copy link
Copy Markdown
Contributor

rhacs-bot commented Feb 10, 2026

Images are ready for the commit at 6b43bf6.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-594-g6b43bf674d.

@GrimmiMeloni GrimmiMeloni added the konflux-build Run Konflux in PR. Push commit to trigger it. label Feb 10, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 10, 2026

Codecov Report

❌ Patch coverage is 70.16807% with 142 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.37%. Comparing base (be2f16f) to head (6b43bf6).
⚠️ Report is 23 commits behind head on master.

Files with missing lines Patch % Lines
operator/bundle_helpers/pkg/csv/patcher.go 54.12% 73 Missing and 27 partials ⚠️
operator/bundle_helpers/pkg/csv/version.go 84.11% 12 Missing and 5 partials ⚠️
operator/bundle_helpers/pkg/descriptor/sorter.go 84.21% 10 Missing and 5 partials ⚠️
operator/bundle_helpers/pkg/rewrite/rewriter.go 72.00% 6 Missing and 1 partial ⚠️
operator/bundle_helpers/pkg/values/values.go 90.32% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #18947      +/-   ##
==========================================
+ Coverage   49.28%   49.37%   +0.08%     
==========================================
  Files        2735     2742       +7     
  Lines      206170   206921     +751     
==========================================
+ Hits       101617   102159     +542     
- Misses      97012    97176     +164     
- Partials     7541     7586      +45     
Flag Coverage Δ
go-unit-tests 49.37% <70.16%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@GrimmiMeloni
Copy link
Copy Markdown
Contributor Author

/test all

@GrimmiMeloni
Copy link
Copy Markdown
Contributor Author

/retest

1 similar comment
@GrimmiMeloni
Copy link
Copy Markdown
Contributor Author

/retest

@GrimmiMeloni
Copy link
Copy Markdown
Contributor Author

/retest operator-bundle-on-push

@stackrox stackrox deleted a comment from openshift-ci bot Feb 11, 2026
@stackrox stackrox deleted a comment from openshift-ci bot Feb 11, 2026
@stackrox stackrox deleted a comment from openshift-ci bot Feb 11, 2026
@stackrox stackrox deleted a comment from openshift-ci bot Feb 11, 2026
@stackrox stackrox deleted a comment from openshift-ci bot Feb 11, 2026
@stackrox stackrox deleted a comment from openshift-ci bot Feb 11, 2026
@stackrox stackrox deleted a comment from openshift-ci bot Feb 11, 2026
@stackrox stackrox deleted a comment from openshift-ci bot Feb 11, 2026
@GrimmiMeloni
Copy link
Copy Markdown
Contributor Author

/test gke-nongroovy-e2e-tests

@GrimmiMeloni
Copy link
Copy Markdown
Contributor Author

/test gke-ui-e2e-tests

1 similar comment
@GrimmiMeloni
Copy link
Copy Markdown
Contributor Author

/test gke-ui-e2e-tests

@GrimmiMeloni
Copy link
Copy Markdown
Contributor Author

/test gke-upgrade-tests

Copy link
Copy Markdown
Contributor Author

@GrimmiMeloni GrimmiMeloni left a comment

Choose a reason for hiding this comment

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

LGTM from my side

@porridge your are up next

@GrimmiMeloni GrimmiMeloni marked this pull request as ready for review February 13, 2026 16:58
@GrimmiMeloni GrimmiMeloni force-pushed the mh/combine-bundle-helpers branch from 5802001 to d9c63cf Compare March 25, 2026 06:13
GrimmiMeloni and others added 2 commits March 25, 2026 07:16
All other Konflux Dockerfiles already use rhel_9_golang_1.25.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The rhel_9 golang builder has an openssl-devel conflict with
python3.12-libs. Use ubi9-minimal (like master) and install
golang alongside python3.12-pyyaml via microdnf instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@GrimmiMeloni
Copy link
Copy Markdown
Contributor Author

/retest

@GrimmiMeloni GrimmiMeloni added the auto-retest PRs with this label will be automatically retested if prow checks fails label Mar 26, 2026
@GrimmiMeloni
Copy link
Copy Markdown
Contributor Author

/konflux-retest operator-bundle-on-push

@GrimmiMeloni GrimmiMeloni enabled auto-merge (squash) March 26, 2026 13:59
GrimmiMeloni and others added 3 commits March 26, 2026 15:42
The setup-go action's built-in cache uses go.sum as key without
job-name scoping, which can race with other workflows.
Since this workflow is temporary, caching is not worth the risk.

Addresses review comment from janisz on PR #18947.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of just disabling caching, use the repo's standard
cache-go-dependencies composite action which properly scopes
cache keys by job name and separates module/build caches.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

/konflux-retest operator-bundle-on-push

@GrimmiMeloni GrimmiMeloni merged commit 15ab6e7 into master Mar 26, 2026
122 of 130 checks passed
@GrimmiMeloni GrimmiMeloni deleted the mh/combine-bundle-helpers branch March 26, 2026 18:58
porridge added a commit that referenced this pull request Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-assisted ai-review area/ci area/operator auto-retest PRs with this label will be automatically retested if prow checks fails konflux-build Run Konflux in PR. Push commit to trigger it.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants