Test orion url fix for Prow censoring#78310
Test orion url fix for Prow censoring#78310mmnabeel317 wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mmnabeel317 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
needs-ok-to-test
|
Hi @mmnabeel317. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughOrion cloning logic was simplified to always clone a specific GitHub repo branch; a separate CI script received a single debug echo insertion. Other orchestration and execution steps remain unchanged. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ci-operator/step-registry/openshift-qe/orion/openshift-qe-orion-commands.sh`:
- Around line 13-21: The script currently hardcodes a personal fork and branch
in the git clone (the git clone --branch fix/prow-url-censoring
https://github.com/mmnabeel317/orion.git --depth 1 line), which ignores the
ORION_REPO and TAG knobs and creates a supply-chain risk; restore the original
logic that honors ORION_REPO and TAG (the conditional that sets LATEST_TAG from
the GitHub API when TAG == "latest" and then runs git clone --branch $LATEST_TAG
$ORION_REPO --depth 1) so callers can override repo/tag, and if you must test a
personal fix temporarily, gate it behind an explicit environment variable (e.g.,
ORION_FORK_OVERRIDE) and pin to a commit SHA (not a mutable branch) before
cloning; apply the same change to sibling scripts that clone Orion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cd310061-22cf-4e26-87e2-dcdc02b1628d
📒 Files selected for processing (1)
ci-operator/step-registry/openshift-qe/orion/openshift-qe-orion-commands.sh
| # Temporary: test base64 URL encoding fix for Prow secret censoring | ||
| # Original block should be restored after testing: | ||
| # if [[ $TAG == "latest" ]]; then | ||
| # LATEST_TAG=$(curl -s "https://api.github.com/repos/cloud-bulldozer/orion/releases/latest" | jq -r '.tag_name'); | ||
| # else | ||
| # LATEST_TAG=$TAG | ||
| # fi | ||
| # git clone --branch $LATEST_TAG $ORION_REPO --depth 1 | ||
| git clone --branch fix/prow-url-censoring https://github.com/mmnabeel317/orion.git --depth 1 |
There was a problem hiding this comment.
Do not merge: hardcoded clone of a personal fork branch targeting main.
This change pins Orion acquisition to https://github.com/mmnabeel317/orion.git on branch fix/prow-url-censoring. Two blocking concerns if this lands on main:
- Supply-chain / reliability hazard. A personal fork + branch ref is mutable (force-push, rename, delete, visibility change) and not governed by the
cloud-bulldozer/orionrelease process. Every job routed throughopenshift-qe-orion-commands.shwould silently execute whatever is at that ref, and would break the instant the branch moves or the fork disappears. A--depth 1clone of a branch also provides no integrity pinning (no tag, no commit SHA). - Documented override surface is silently dropped.
openshift-qe-orion-ref.yamlstill declaresORION_REPO(defaultcloud-bulldozer/orion.git) andTAG(defaultlatest) as user-overridable knobs, but this hunk ignores both. Any caller settingTAG=v...orORION_REPO=...on this step will get unexpected behavior with no warning. Sibling scripts (olmv1,report,rhosovariants) still honor the knobs, so this step diverges from the rest of the family.
Recommended path forward (in order of preference):
- Land the base64 fix upstream in
cloud-bulldozer/orion, cut a release, then bump via$TAG— no script change needed. - If a temporary test is unavoidable, do it via a rehearsal / one-off job override or a gated env var, not by editing the shared step on
main.
If you still want to keep the toggle in-script for a short-lived experiment, at minimum guard it behind an env var and pin to a commit SHA rather than a branch, e.g.:
🛠️ Suggested safer toggle (env-gated, SHA-pinned)
-# Temporary: test base64 URL encoding fix for Prow secret censoring
-# Original block should be restored after testing:
-# if [[ $TAG == "latest" ]]; then
-# LATEST_TAG=$(curl -s "https://api.github.com/repos/cloud-bulldozer/orion/releases/latest" | jq -r '.tag_name');
-# else
-# LATEST_TAG=$TAG
-# fi
-# git clone --branch $LATEST_TAG $ORION_REPO --depth 1
-git clone --branch fix/prow-url-censoring https://github.com/mmnabeel317/orion.git --depth 1
+if [[ -n "${ORION_TEST_FORK_URL:-}" && -n "${ORION_TEST_FORK_REF:-}" ]]; then
+ echo "WARNING: using experimental Orion fork ${ORION_TEST_FORK_URL}@${ORION_TEST_FORK_REF}"
+ git clone "${ORION_TEST_FORK_URL}" --depth 1
+ pushd orion
+ git fetch --depth 1 origin "${ORION_TEST_FORK_REF}"
+ git checkout FETCH_HEAD
+ popd
+else
+ if [[ $TAG == "latest" ]]; then
+ LATEST_TAG=$(curl -s "https://api.github.com/repos/cloud-bulldozer/orion/releases/latest" | jq -r '.tag_name')
+ else
+ LATEST_TAG=$TAG
+ fi
+ git clone --branch "$LATEST_TAG" "$ORION_REPO" --depth 1
+fiAlso worth noting: once the upstream fix is released, the sibling scripts (olmv1/openshift-qe-orion-olmv1-commands.sh, report, rhoso) will need the same bump to benefit from the Prow-censoring fix; please track that as part of the rollout.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Temporary: test base64 URL encoding fix for Prow secret censoring | |
| # Original block should be restored after testing: | |
| # if [[ $TAG == "latest" ]]; then | |
| # LATEST_TAG=$(curl -s "https://api.github.com/repos/cloud-bulldozer/orion/releases/latest" | jq -r '.tag_name'); | |
| # else | |
| # LATEST_TAG=$TAG | |
| # fi | |
| # git clone --branch $LATEST_TAG $ORION_REPO --depth 1 | |
| git clone --branch fix/prow-url-censoring https://github.com/mmnabeel317/orion.git --depth 1 | |
| if [[ -n "${ORION_TEST_FORK_URL:-}" && -n "${ORION_TEST_FORK_REF:-}" ]]; then | |
| echo "WARNING: using experimental Orion fork ${ORION_TEST_FORK_URL}@${ORION_TEST_FORK_REF}" | |
| git clone "${ORION_TEST_FORK_URL}" --depth 1 | |
| pushd orion | |
| git fetch --depth 1 origin "${ORION_TEST_FORK_REF}" | |
| git checkout FETCH_HEAD | |
| popd | |
| else | |
| if [[ $TAG == "latest" ]]; then | |
| LATEST_TAG=$(curl -s "https://api.github.com/repos/cloud-bulldozer/orion/releases/latest" | jq -r '.tag_name') | |
| else | |
| LATEST_TAG=$TAG | |
| fi | |
| git clone --branch "$LATEST_TAG" "$ORION_REPO" --depth 1 | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ci-operator/step-registry/openshift-qe/orion/openshift-qe-orion-commands.sh`
around lines 13 - 21, The script currently hardcodes a personal fork and branch
in the git clone (the git clone --branch fix/prow-url-censoring
https://github.com/mmnabeel317/orion.git --depth 1 line), which ignores the
ORION_REPO and TAG knobs and creates a supply-chain risk; restore the original
logic that honors ORION_REPO and TAG (the conditional that sets LATEST_TAG from
the GitHub API when TAG == "latest" and then runs git clone --branch $LATEST_TAG
$ORION_REPO --depth 1) so callers can override repo/tag, and if you must test a
personal fix temporarily, gate it behind an explicit environment variable (e.g.,
ORION_FORK_OVERRIDE) and pin to a commit SHA (not a mutable branch) before
cloning; apply the same change to sibling scripts that clone Orion.
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-aws-4.22-nightly-x86-payload-control-plane-6nodes |
|
@mohit-sheth: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@mohit-sheth: needs-ok-to-test label found, no rehearsals will be run |
|
/ok-to-test |
openshift-ci
Bot
added
ok-to-test
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-aws-4.22-nightly-x86-payload-control-plane-6nodes |
|
@mohit-sheth: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@mohit-sheth: job(s): periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-aws-4.22-nightly-x86-payload-control-plane-6nodes either don't exist or were not found to be affected, and cannot be rehearsed |
|
@mmnabeel317: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Signed-off-by: Nabeel Mohd <mmnabeel317@gmail.com>
mmnabeel317
force-pushed
the
test-orion-url-fix
branch
from
c1f5ea9 to
c8429a3
Compare
|
[REHEARSALNOTIFIER]
A total of 494 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-aws-4.22-nightly-x86-payload-control-plane-6nodes |
|
@mohit-sheth: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
2 participants
Summary
Temporarily points the Orion CI step to a fix branch (
mmnabeel317/orion#fix/prow-url-censoring) to test a fix for broken build URLs in Orion's HTML visualizations.Problem
When Orion generates
--vizHTML artifacts, thebuildUrlvalues (e.g. Prow job links) are embedded directly in the Plotly figure JSON. Prow's secret scanner detectsocp-qe-perfscaleas part of a secret and replaces it withXXXXXXXXin the uploaded artifact files. This breaks the click-to-open-build functionality on changepoint markers in the visualization.Fix being tested
The Orion fix base64-encodes all
buildUrlvalues in the Plotlycustomdatafield. The injected JavaScript click handler decodes them viaatob()before callingwindow.open(). Since the base64 representation does not contain the literal secret string, Prow's scanner skips it and the URLs survive intact.Orion PR branch: https://github.com/mmnabeel317/orion/tree/fix/prow-url-censoring
Changes in this PR
ci-operator/step-registry/openshift-qe/orion/openshift-qe-orion-commands.sh: temporarily clones from the fix branch instead of the latest release tag. The original block is commented out for easy restoration./cc @mohit-sheth
Summary by CodeRabbit
Bug Fixes
Chores