Skip to content

ROX-33117: support for operator chart publishing#19107

Merged
rhacs-bot merged 1 commit intomasterfrom
porridge/ROX-33117-1
Mar 18, 2026
Merged

ROX-33117: support for operator chart publishing#19107
rhacs-bot merged 1 commit intomasterfrom
porridge/ROX-33117-1

Conversation

@porridge
Copy link
Contributor

@porridge porridge commented Feb 19, 2026

Description

Update the release pipeline for the operator chart for both the community and downstream flavor.

I think this change should be sufficient, because I don't see any chart specific logic that needs updating in the release-artifacts repo, nor in the downstream mirror job, but maybe I'm not aware of everything since the docs seem to be somewhat outdated.

FWIW updating the README in https://github.com/stackrox/helm-charts will be covered by a dedicated ticket.

Also the chart itself will get some additional polish (description, icon link, etc), but that can be done independently of this PR.

Docs:

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • not modified existing tests

How I validated my change

Tested in https://github.com/stackrox/release-artifacts/pull/210 using these additional changes to make it as similar as possible to the real thing:

@openshift-ci
Copy link

openshift-ci bot commented Feb 19, 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

sourcery-ai[bot]

This comment was marked as outdated.

@rhacs-bot
Copy link
Contributor

rhacs-bot commented Feb 19, 2026

Images are ready for the commit at 284f4ce.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-362-g284f4ceff6.

@codecov
Copy link

codecov bot commented Feb 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.25%. Comparing base (6b9ea66) to head (284f4ce).
⚠️ Report is 13 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #19107   +/-   ##
=======================================
  Coverage   49.25%   49.25%           
=======================================
  Files        2725     2725           
  Lines      205582   205582           
=======================================
+ Hits       101261   101267    +6     
+ Misses      96784    96780    -4     
+ Partials     7537     7535    -2     
Flag Coverage Δ
go-unit-tests 49.25% <ø> (+<0.01%) ⬆️

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.

@porridge
Copy link
Contributor Author

porridge commented Mar 4, 2026

@sourcery-ai review

Copy link
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 issue, and left some high level feedback:

  • The publish-helm-charts.sh script now hard-requires an operator_chart_dir argument; if you expect any reuse of this script outside the updated push_helm_charts path, consider making the operator chart parameter optional (with a clear conditional branch) to avoid surprising failures in other callers.
  • In push_helm_charts, you now create an additional mktemp directory for the operator chart but never clean it up; adding a simple trap 'rm -rf ...' EXIT to remove all temporary directories would keep the script from leaking temp data across repeated runs.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `publish-helm-charts.sh` script now hard-requires an `operator_chart_dir` argument; if you expect any reuse of this script outside the updated `push_helm_charts` path, consider making the operator chart parameter optional (with a clear conditional branch) to avoid surprising failures in other callers.
- In `push_helm_charts`, you now create an additional `mktemp` directory for the operator chart but never clean it up; adding a simple `trap 'rm -rf ...' EXIT` to remove all temporary directories would keep the script from leaking temp data across repeated runs.

## Individual Comments

### Comment 1
<location path="operator/Makefile" line_range="326-328" />
<code_context>
 chart: kubebuilder manifests ## Generate a helm chart with all necessary resources.
 # The dependency above makes sure protos are up to date, so we can skip this time-consuming process below
 # by specifying the SKIP env var. Otherwise each target that kubebuilder invokes (and there is a bunch) would regen protos.
+	if [ "$(ROX_IMAGE_FLAVOR)" = opensource ]; then sed -i'.bak' -e 's,^projectName: rhacs-operator,projectName: stackrox-operator,' PROJECT; fi
 	ROX_OPERATOR_SKIP_PROTO_GENERATED_SRCS=true $(KUBEBUILDER) edit --plugins=helm/v2-alpha --force
+	if [ "$(ROX_IMAGE_FLAVOR)" = opensource ]; then mv PROJECT.bak PROJECT; fi
 	sed -i'.bak' -e 's,0.1.0,$(VERSION),g' dist/chart/Chart.yaml
 	rm -f dist/chart/Chart.yaml.bak
</code_context>
<issue_to_address>
**issue:** Guard against leaving PROJECT mutated if the kubebuilder step fails mid-target.

Because `PROJECT` is edited in place for `ROX_IMAGE_FLAVOR=opensource`, a failure in `$(KUBEBUILDER) edit ...` will skip the restore step and leave `PROJECT` in the temporary state. To avoid leaving the repo dirty, consider running kubebuilder against a copy of `PROJECT` or adding a trap/cleanup so the original is always restored on failure.
</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.

@porridge porridge changed the title ROX-33117: basic support for operator chart publishing ROX-33117: support for operator chart publishing Mar 4, 2026
@porridge porridge marked this pull request as ready for review March 4, 2026 12:53
@porridge porridge requested review from a team as code owners March 4, 2026 12:53
@porridge porridge requested review from GrimmiMeloni and removed request for a team March 4, 2026 12:53
Copy link
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 issue, and left some high level feedback:

  • The updated push_helm_charts/publish-helm-charts.sh signatures now hard-require operator chart inputs; if these scripts are reused in any other context (e.g., manual runs, older branches, or ad‑hoc tooling), consider making the operator chart arguments optional with sensible fallbacks to preserve backward compatibility.
  • In .github/workflows/build.yaml, the operator chart tar step uses tar -cvzf which can produce very verbose logs; unless you rely on that output, consider dropping the -v flag to keep CI logs smaller and easier to scan.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The updated `push_helm_charts`/`publish-helm-charts.sh` signatures now hard-require operator chart inputs; if these scripts are reused in any other context (e.g., manual runs, older branches, or ad‑hoc tooling), consider making the operator chart arguments optional with sensible fallbacks to preserve backward compatibility.
- In `.github/workflows/build.yaml`, the operator chart tar step uses `tar -cvzf` which can produce very verbose logs; unless you rely on that output, consider dropping the `-v` flag to keep CI logs smaller and easier to scan.

## Individual Comments

### Comment 1
<location path="operator/Makefile" line_range="326" />
<code_context>
 chart: kubebuilder manifests ## Generate a helm chart with all necessary resources.
 # The dependency above makes sure protos are up to date, so we can skip this time-consuming process below
 # by specifying the SKIP env var. Otherwise each target that kubebuilder invokes (and there is a bunch) would regen protos.
+	if [ "$(ROX_IMAGE_FLAVOR)" = opensource ]; then sed -i'.bak' -e 's,^projectName: rhacs-operator,projectName: stackrox-operator,' PROJECT; fi
 	ROX_OPERATOR_SKIP_PROTO_GENERATED_SRCS=true $(KUBEBUILDER) edit --plugins=helm/v2-alpha --force
+	if [ "$(ROX_IMAGE_FLAVOR)" = opensource ]; then mv PROJECT.bak PROJECT; fi
 	sed -i'.bak' -e 's,0.1.0,$(VERSION),g' dist/chart/Chart.yaml
 	rm -f dist/chart/Chart.yaml.bak
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The conditional PROJECT mutation is somewhat brittle on the exact `projectName` format.

This depends on `PROJECT` containing exactly `projectName: rhacs-operator` with no indentation, so any upstream formatting change (whitespace, key order, etc.) would cause the `sed` to no-op and leave the `rhacs-operator` name in the opensource chart. Please either make the match more tolerant (e.g., allow leading whitespace) or move the flavor-specific `projectName` into a templated/derived file instead of editing the kubebuilder-generated `PROJECT` in place.

```suggestion
	if [ "$(ROX_IMAGE_FLAVOR)" = opensource ]; then sed -i'.bak' -e 's,^[[:space:]]*projectName:[[:space:]]*rhacs-operator,projectName: stackrox-operator,' PROJECT; fi
```
</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.

@porridge porridge requested review from a team and msugakov March 4, 2026 12:56
Copy link
Contributor

@msugakov msugakov left a comment

Choose a reason for hiding this comment

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

I mark this review as "Request changes" because the used approach seems the one that puts least resistance during implementation above the experience of ourselves who'd have to use it.
How do I want this to be changed? roxctl helm output is a good example of how things could be. It's not necessary we plug into that but I'd say we deserve something better than having to run ROX_IMAGE_FLAVOR=development_build make -C operator chart as a step to deploy the product.

Copy link
Contributor Author

@porridge porridge left a comment

Choose a reason for hiding this comment

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

PTAL @msugakov

@porridge porridge requested a review from msugakov March 9, 2026 23:27
@janisz janisz removed the request for review from a team March 10, 2026 10:48
@porridge porridge force-pushed the porridge/ROX-33117-1 branch from 08e4c8c to 150f393 Compare March 17, 2026 12:19
@porridge porridge requested a review from msugakov March 17, 2026 12:22
Copy link
Contributor

@msugakov msugakov left a comment

Choose a reason for hiding this comment

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

Small things but I felt I need to comment on them. The primary reason I'm not approving is that we converge on #19107 (comment)

@porridge porridge requested a review from msugakov March 18, 2026 08:34
@porridge porridge force-pushed the porridge/ROX-33117-1 branch from 446fb4e to 284f4ce Compare March 18, 2026 13:17
@porridge porridge added auto-merge Auto-merge minor and patch version bumps auto-retest PRs with this label will be automatically retested if prow checks fails labels Mar 18, 2026
@rhacs-bot
Copy link
Contributor

/retest

1 similar comment
@rhacs-bot
Copy link
Contributor

/retest

@rhacs-bot rhacs-bot merged commit 697b66a into master Mar 18, 2026
111 checks passed
@rhacs-bot rhacs-bot deleted the porridge/ROX-33117-1 branch March 18, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review area/ci area/operator auto-merge Auto-merge minor and patch version bumps auto-retest PRs with this label will be automatically retested if prow checks fails

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants