RHOAIENG-75598: add RHOAI pull secret step to opendatahub-operator e2e tests - #82751
RHOAIENG-75598: add RHOAI pull secret step to opendatahub-operator e2e tests#82751rinaldodev wants to merge 1 commit into
Conversation
|
@rinaldodev: This pull request references RHOAIENG-75598 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
Hi @rinaldodev. 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rinaldodev 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 |
WalkthroughThe PR adds a CI step that merges RHOAI Quay credentials into the cluster pull secret. The step waits for machine-config rollout and runs before standard and RHOAI end-to-end tests. ChangesRHOAI Quay pull-secret preparation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow as End-to-end workflow
participant Step as RHOAI Quay pull-secret step
participant OpenShift as OpenShift API
participant MCP as Machine-config pools
Workflow->>Step: Run before tests
Step->>OpenShift: Read and update global pull secret
OpenShift->>MCP: Render updated MachineConfigs
Step->>MCP: Wait for applicable pools
Step-->>Workflow: Complete preparation
Workflow->>Workflow: Start end-to-end tests
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-commands.sh`:
- Around line 55-72: Update the MCP wait flow after the pull-secret operation to
compare the existing quay.io/rhoai auth value with AUTH and branch on whether
the merged secret changed. If unchanged, skip the UPDATING=True waits and only
wait for Updated=True or complete the step; if changed, wait for the new
rendered configuration before accepting Updated=True, avoiding satisfaction by
an unrelated rollout. Preserve the existing NUM_WORKERS handling in the worker
and master MCP waits.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: f9a48786-1379-46d4-b2fc-7483a4c27b7a
📒 Files selected for processing (5)
ci-operator/config/opendatahub-io/opendatahub-operator/opendatahub-io-opendatahub-operator-main.yamlci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/OWNERSci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-commands.shci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-ref.metadata.jsonci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-ref.yaml
…or e2e tests Introduces a new CI step that merges quay.io/rhoai robot account credentials into the cluster global pull-secret, and wires it into the opendatahub-operator main e2e test workflows so private RHOAI component images can be pulled by digest.
4378cd1 to
db2a695
Compare
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-commands.sh`:
- Around line 72-89: The merge logic should replace the complete quay.io/rhoai
credential record rather than deep-merging it. Update the comparison around
EXISTING_AUTH to compare .auths["quay.io/rhoai"] with the generated credential
object, and change the jq merge using merged_pull_secret_file so the existing
registry entry is fully replaced, removing stale fields such as identitytoken
while preserving unrelated registry entries.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 7fec3e2a-badc-4b2e-8e2d-e08db154a7be
📒 Files selected for processing (5)
ci-operator/config/opendatahub-io/opendatahub-operator/opendatahub-io-opendatahub-operator-main.yamlci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/OWNERSci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-commands.shci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-ref.metadata.jsonci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-ref.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- ci-operator/config/opendatahub-io/opendatahub-operator/opendatahub-io-opendatahub-operator-main.yaml
- ci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/OWNERS
- ci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-ref.yaml
- ci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-ref.metadata.json
| EXISTING_AUTH="$(jq -r '.auths["quay.io/rhoai"].auth // empty' "${cluster_pull_secret_file}")" | ||
| if [[ "${EXISTING_AUTH}" == "${AUTH}" ]]; then | ||
| pull_secret_changed=false | ||
| else | ||
| pull_secret_changed=true | ||
| fi | ||
| unset EXISTING_AUTH | ||
|
|
||
| echo "Get RHOAI Quay credentials" | ||
| jq -n --arg auth "${AUTH}" \ | ||
| '{"auths":{"quay.io/rhoai":{"auth":$auth,"email":""}}}' \ | ||
| > "${rhoai_pull_secret_file}" | ||
|
|
||
| echo "Merge RHOAI Quay credentials and the global cluster pull secret" | ||
| jq -s '.[0] * .[1]' \ | ||
| "${cluster_pull_secret_file}" \ | ||
| "${rhoai_pull_secret_file}" \ | ||
| > "${merged_pull_secret_file}" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
trap 'rm -rf "${tmpdir}"' EXIT
cat >"${tmpdir}/cluster.json" <<'EOF'
{"auths":{"quay.io/rhoai":{"auth":"b2xkOnRva2Vu","identitytoken":"stale-token"}}}
EOF
cat >"${tmpdir}/rhoai.json" <<'EOF'
{"auths":{"quay.io/rhoai":{"auth":"bmV3OnRva2Vu","email":""}}}
EOF
jq -s '.[0] * .[1]' "${tmpdir}/cluster.json" "${tmpdir}/rhoai.json" >"${tmpdir}/deep-merge.json"
jq -e '.auths["quay.io/rhoai"] | has("identitytoken")' "${tmpdir}/deep-merge.json"
jq -s '(.[0].auths["quay.io/rhoai"] = .[1].auths["quay.io/rhoai"]) | .[0]' \
"${tmpdir}/cluster.json" "${tmpdir}/rhoai.json" >"${tmpdir}/replacement.json"
jq -e '.auths["quay.io/rhoai"] == {"auth":"bmV3OnRva2Vu","email":""}' "${tmpdir}/replacement.json"Repository: openshift/release
Length of output: 165
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant script section and nearby context without running repository code.
if [ -f ci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-commands.sh ]; then
sed -n '1,130p' ci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-commands.sh | cat -n
elif [ -f ci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-commands.sh ]; then
find . -name 'opendatahub-io-ci-rhoai-quay-pull-secret-commands.sh' -print
else
echo 'target script not found'
fiRepository: openshift/release
Length of output: 4924
Replace the quay.io/rhoai credential entry instead of deep-merging it.
jq -s '.[0] * .[1]' keeps fields from the existing registry entry below auth, such as identitytoken. A stale entry with a new auth value would remain unchanged because lines 72–77 compare only .auth. Compare .auths["quay.io/rhoai"] against the generated credential record, then replace the registry entry with that record.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 73-73: A credential-bearing variable (e.g. PASSWORD, PASSWD, SECRET, TOKEN, API_KEY) is assigned a hardcoded string literal. Secrets committed to a script are exposed in source control, process listings, and shell history, and cannot be rotated without a code change. Read the value from a secrets manager or an injected environment variable at runtime instead (e.g. PASSWORD="${DB_PASSWORD:?must be set}"), and never commit the literal.
Context: pull_secret_changed=false
Note: [CWE-798] Use of Hard-coded Credentials.
(hardcoded-password-assignment-bash)
[warning] 75-75: A credential-bearing variable (e.g. PASSWORD, PASSWD, SECRET, TOKEN, API_KEY) is assigned a hardcoded string literal. Secrets committed to a script are exposed in source control, process listings, and shell history, and cannot be rotated without a code change. Read the value from a secrets manager or an injected environment variable at runtime instead (e.g. PASSWORD="${DB_PASSWORD:?must be set}"), and never commit the literal.
Context: pull_secret_changed=true
Note: [CWE-798] Use of Hard-coded Credentials.
(hardcoded-password-assignment-bash)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@ci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-commands.sh`
around lines 72 - 89, The merge logic should replace the complete quay.io/rhoai
credential record rather than deep-merging it. Update the comparison around
EXISTING_AUTH to compare .auths["quay.io/rhoai"] with the generated credential
object, and change the jq merge using merged_pull_secret_file so the existing
registry entry is fully replaced, removing stale fields such as identitytoken
while preserving unrelated registry entries.
|
The OWNERS file contains untrusted users, which makes it INVALID. The following users are mentioned in OWNERS file(s) but are untrusted for the following reasons. One way to make the user trusted is to add them as members of the openshift org. You can then trigger verification by writing
|
| pre: | ||
| - ref: ipi-install-rbac | ||
| - ref: opendatahub-operator-e2e-cluster-preflight-health-check | ||
| - ref: opendatahub-io-ci-rhoai-quay-pull-secret |
There was a problem hiding this comment.
We should add it only for rhoai e2e?
Also, we'd need the same in the stable-2.x branch config
Description
E2e jobs on borrowed clusters need to pull private images from
quay.io/rhoai. The job Pod can see Vault credentials, but the cluster nodes cannot, so ImagePull fails without auth on the cluster.This PR adds a reusable CI step that merges the RHOAI Quay robot into the cluster-wide pull secret (
openshift-config/pull-secret), then waits for MachineConfigPools to finish applying it. Both main e2e tests run that step inpre:after the health check.opendatahub-io-ci-rhoai-quay-pull-secretopendatahub-operator-e2eandopendatahub-operator-rhoai-e2etest-credentials/opendatahub-rhoai-quay-pull(keypull-secret)Jira: RHOAIENG-75598
Type
Testing
make registry-metadatamake ci-operator-checkconfig/pj-rehearse opendatahub-operator-e2e opendatahub-operator-rhoai-e2eSummary by CodeRabbit
This PR adds a reusable CI step for the OpenDataHub operator test infrastructure. The step merges RHOAI Quay robot credentials from Vault into
openshift-config/pull-secret.The step waits for MachineConfigPools to apply the updated secret. It supports SNO and compact clusters.
The step runs before the
opendatahub-operator-e2eandopendatahub-operator-rhoai-e2eworkflows. This enables privatequay.io/rhoaiimage pulls on borrowed clusters.Validation includes registry metadata generation, CI configuration checks, and Prow rehearsals for both e2e jobs.