Skip to content

ci: install latest roxie#20111

Draft
davdhacs wants to merge 33 commits intomasterfrom
davdhacs/roxie-in-ci-latest
Draft

ci: install latest roxie#20111
davdhacs wants to merge 33 commits intomasterfrom
davdhacs/roxie-in-ci-latest

Conversation

@davdhacs
Copy link
Copy Markdown
Contributor

auto-install latest roxie

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 20, 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

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 3 issues, and left some high level feedback:

  • In .openshift-ci/dispatch.sh, the curl ... || break pattern is invalid outside of a loop and will cause a shell error; replace break with an explicit exit/return and ensure the failure path is handled cleanly.
  • In deploy_stackrox_with_roxie_compat, the retrying_kubectl ... delete configmap ... calls are not guarded with || true, so a missing ConfigMap can fail the whole script under set -e; consider making these deletions tolerant of non-existent resources.
  • In handle_trusted_ca_file, trusted_ca_as_string is assigned without being declared local, which can unintentionally leak into the global scope; declare it local to avoid side effects on the surrounding script.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `.openshift-ci/dispatch.sh`, the `curl ... || break` pattern is invalid outside of a loop and will cause a shell error; replace `break` with an explicit `exit`/`return` and ensure the failure path is handled cleanly.
- In `deploy_stackrox_with_roxie_compat`, the `retrying_kubectl ... delete configmap ...` calls are not guarded with `|| true`, so a missing ConfigMap can fail the whole script under `set -e`; consider making these deletions tolerant of non-existent resources.
- In `handle_trusted_ca_file`, `trusted_ca_as_string` is assigned without being declared local, which can unintentionally leak into the global scope; declare it `local` to avoid side effects on the surrounding script.

## Individual Comments

### Comment 1
<location path=".openshift-ci/dispatch.sh" line_range="75-76" />
<code_context>
+if [[ "${USE_ROXIE_DEPLOY:-false}" != 'false' ]]; then
+    info 'Installing latest roxie release...'
+    if [[ ${USE_ROXIE_VERSION:-latest} == 'latest' ]]; then
+        USE_ROXIE_VERSION="$(curl -s -H "Authorization: token ${RHACS_BOT_GITHUB_TOKEN}" \
+          "https://api.github.com/repos/stackrox/roxie/releases/latest" | jq -r '.tag_name')"
+    fi
+    curl -o /tmp/roxie \
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider handling missing/invalid GitHub token and curl/jq failures explicitly.

If `RHACS_BOT_GITHUB_TOKEN` is unset/invalid, this call silently uses unauthenticated rate limits, and any curl/jq failure will set `USE_ROXIE_VERSION` to an empty string, which then propagates into the download URL. Please either fail fast when the token is required but missing, and/or check curl/jq exit codes before using `.tag_name` (e.g. with `|| { error ...; exit 1; }`) to avoid constructing an invalid URL.
</issue_to_address>

### Comment 2
<location path=".openshift-ci/dispatch.sh" line_range="81" />
<code_context>
+    curl -o /tmp/roxie \
+      -H "Authorization: token ${RHACS_BOT_GITHUB_TOKEN}" \
+      -H "Accept:application/octet-stream"\
+      "https://github.com/stackrox/roxie/releases/download/${USE_ROXIE_VERSION}/roxie-linux-amd64" || break
+    install -b /tmp/roxie /usr/bin/
+    roxie version || { error 'roxie failed, restoring original version:'; cp /usr/bin/roxie.old /usr/bin/roxie; roxie version; }
</code_context>
<issue_to_address>
**issue (bug_risk):** `break` here may not behave as expected outside of a loop.

On curl failure, `break` will either throw a runtime error if this isn’t inside a loop, or exit an unrelated enclosing loop. If the goal is to stop the script/function, use `exit 1` or `return 1` instead for explicit, predictable control flow.
</issue_to_address>

### Comment 3
<location path=".openshift-ci/dispatch.sh" line_range="82-83" />
<code_context>
+      -H "Authorization: token ${RHACS_BOT_GITHUB_TOKEN}" \
+      -H "Accept:application/octet-stream"\
+      "https://github.com/stackrox/roxie/releases/download/${USE_ROXIE_VERSION}/roxie-linux-amd64" || break
+    install -b /tmp/roxie /usr/bin/
+    roxie version || { error 'roxie failed, restoring original version:'; cp /usr/bin/roxie.old /usr/bin/roxie; roxie version; }
+fi
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The backup filename used in `install -b` does not match the file restored later.

GNU `install -b` defaults to a `~` backup suffix (e.g. `/usr/bin/roxie~`), but the rollback expects `/usr/bin/roxie.old`. Unless you pass `--suffix=.old` (or similar) to `install`, the restore command will fail because `roxie.old` is never created. Please align the backup suffix with the file used in the restore step or programmatically determine the backup name before restoring.
</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.

Comment thread .openshift-ci/dispatch.sh
Comment thread .openshift-ci/dispatch.sh Outdated
Comment thread .openshift-ci/dispatch.sh Outdated
@davdhacs
Copy link
Copy Markdown
Contributor Author

/test gke-qa-e2e-tests

@davdhacs
Copy link
Copy Markdown
Contributor Author

/test gke-qa-e2e-tests

@davdhacs davdhacs changed the title wip ci: install latest roxie Apr 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 20, 2026

🚀 Build Images Ready

Images are ready for commit 3329320. To use with deploy scripts:

export MAIN_IMAGE_TAG=4.11.x-716-g3329320a1a

@davdhacs
Copy link
Copy Markdown
Contributor Author

/test gke-qa-e2e-tests

@davdhacs
Copy link
Copy Markdown
Contributor Author

/test gke-qa-e2e-tests

@davdhacs
Copy link
Copy Markdown
Contributor Author

/test gke-qa-e2e-tests

@davdhacs
Copy link
Copy Markdown
Contributor Author

/test gke-qa-e2e-tests

@davdhacs
Copy link
Copy Markdown
Contributor Author

/test gke-qa-e2e-tests

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.75%. Comparing base (6093cea) to head (3329320).
⚠️ Report is 28 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #20111      +/-   ##
==========================================
+ Coverage   49.67%   49.75%   +0.08%     
==========================================
  Files        2765     2766       +1     
  Lines      209049   209549     +500     
==========================================
+ Hits       103850   104267     +417     
- Misses      97521    97583      +62     
- Partials     7678     7699      +21     
Flag Coverage Δ
go-unit-tests 49.75% <ø> (+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.

@davdhacs
Copy link
Copy Markdown
Contributor Author

/test gke-qa-e2e-tests

@davdhacs
Copy link
Copy Markdown
Contributor Author

/test gke-qa-e2e-tests

@davdhacs
Copy link
Copy Markdown
Contributor Author

@mclasmeier this should work if we just find a good place to put it! I wasn't considering that the test pod may have paths locked down like this (I was sure I could install to /usr/local/bin). I am thinking creating a .bin/ on the working directory may be better than this /tmp/roxie/ (if that works). If you have another idea and want to try it, feel free to push to this branch.

@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 21, 2026

@davdhacs: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/gke-qa-e2e-tests 3329320 link false /test gke-qa-e2e-tests

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

@mclasmeier
Copy link
Copy Markdown
Contributor

This is pretty cool, thank you @davdhacs

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