Skip to content

RHOAIENG-75598: add RHOAI pull secret step to opendatahub-operator e2e tests - #82751

Open
rinaldodev wants to merge 1 commit into
openshift:mainfrom
rinaldodev:rhoai-pull-secret
Open

RHOAIENG-75598: add RHOAI pull secret step to opendatahub-operator e2e tests#82751
rinaldodev wants to merge 1 commit into
openshift:mainfrom
rinaldodev:rhoai-pull-secret

Conversation

@rinaldodev

@rinaldodev rinaldodev commented Jul 31, 2026

Copy link
Copy Markdown

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 in pre: after the health check.

  • New step: opendatahub-io-ci-rhoai-quay-pull-secret
  • Wired into opendatahub-operator-e2e and opendatahub-operator-rhoai-e2e
  • Credentials come from Vault sync into test-credentials/opendatahub-rhoai-quay-pull (key pull-secret)
  • Pattern matches existing Quay-robot merge steps in this repo (merge global secret, wait for MCP)

Jira: RHOAIENG-75598

Type

  • Bug fix / CI reliability
  • New feature
  • Refactor
  • Docs

Testing

  • make registry-metadata
  • make ci-operator-checkconfig
  • In this PR: /pj-rehearse opendatahub-operator-e2e opendatahub-operator-rhoai-e2e

Summary 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-e2e and opendatahub-operator-rhoai-e2e workflows. This enables private quay.io/rhoai image pulls on borrowed clusters.

Validation includes registry metadata generation, CI configuration checks, and Prow rehearsals for both e2e jobs.

@openshift-ci-robot

openshift-ci-robot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

@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.

Details

In response to this:

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 in pre: after the health check.

  • New step: opendatahub-io-ci-rhoai-quay-pull-secret
  • Wired into opendatahub-operator-e2e and opendatahub-operator-rhoai-e2e
  • Credentials come from Vault sync into test-credentials/opendatahub-rhoai-quay-pull (key pull-secret)
  • Pattern matches existing Quay-robot merge steps in this repo (merge global secret, wait for MCP)

Jira: RHOAIENG-75598

Type

  • Bug fix / CI reliability
  • New feature
  • Refactor
  • Docs

Testing

  • make registry-metadata
  • make ci-operator-checkconfig
  • In this PR: /pj-rehearse opendatahub-operator-e2e opendatahub-operator-rhoai-e2e

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.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 31, 2026
@openshift-ci openshift-ci Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 31, 2026
@openshift-ci

openshift-ci Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@openshift-ci

openshift-ci Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rinaldodev
Once this PR has been reviewed and has the lgtm label, please assign zdtsw for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

The 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.

Changes

RHOAI Quay pull-secret preparation

Layer / File(s) Summary
Define the pull-secret step
ci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/*
The step registry mounts the credential, invokes the command script, sets resource requests, and defines metadata and approvers.
Validate and merge credentials
ci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-commands.sh
The script validates inputs, compares existing credentials, and merges changed authentication data into the global pull secret.
Wire workflows and wait for rollout
ci-operator/config/opendatahub-io/opendatahub-operator/opendatahub-io-opendatahub-operator-main.yaml, ci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-commands.sh
Both end-to-end workflows run the step before testing. The script waits for applicable MachineConfigs and machine-config pools.

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
Loading

Suggested reviewers: sosiouxme, kahowell


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
No-Weak-Crypto ❌ Error The new script compares the Vault credential AUTH with EXISTING_AUTH using Bash [[ == ]] at line 73, which is not constant-time. Remove the secret equality check or replace it with a vetted constant-time comparison such as hmac.compare_digest; keep credential tracing disabled.
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding the RHOAI pull secret step to opendatahub-operator end-to-end tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The patch adds CI configuration and a shell step only; it introduces no Ginkgo test declarations or test titles, dynamic or otherwise.
Test Structure And Quality ✅ Passed No Ginkgo test code is added or modified; the diff contains only YAML, metadata, OWNERS, and a shell CI step, so these requirements are not applicable.
Microshift Test Compatibility ✅ Passed The pull request adds CI YAML, metadata, and a shell step only; it adds no Ginkgo e2e tests or new MicroShift-sensitive test behavior.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The pull request adds CI configuration and a shell step only; it adds no Ginkgo e2e tests or multi-node test assumptions.
Topology-Aware Scheduling Compatibility ✅ Passed All five changed paths are CI registry/config files; scans found no affinity, topology spread, selectors, tolerations, replicas, PDB, or other workload scheduling constraints.
Ote Binary Stdout Contract ✅ Passed The PR changes only CI YAML, metadata, OWNERS, and a Bash step; it adds no OTE binary or process-level Go code. Bash echo output is outside this contract.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The PR adds only CI configuration and a shell step; it adds no Ginkgo e2e declarations or test code, so this check is not applicable.
Container-Privileges ✅ Passed Changed CI YAML adds only an upi-installer step, resource requests, and credentials; no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation, or root settings appear.
No-Sensitive-Data-In-Logs ✅ Passed The script disables xtrace before reading or passing AUTH, unsets it before tracing resumes, and redirects secret contents to files; logs contain only paths and status messages.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@openshift-ci
openshift-ci Bot requested review from kahowell and sosiouxme July 31, 2026 18:27
@openshift-ci openshift-ci Bot added the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label Jul 31, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 333c877 and 4378cd1.

📒 Files selected for processing (5)
  • 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-commands.sh
  • ci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-ref.metadata.json
  • ci-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.
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

[REHEARSALNOTIFIER]
@rinaldodev: the pj-rehearse plugin accommodates running rehearsal tests for the changes in this PR. Expand 'Interacting with pj-rehearse' for usage details. The following rehearsable tests have been affected by this change:

Test name Repo Type Reason
pull-ci-opendatahub-io-opendatahub-operator-main-opendatahub-operator-e2e opendatahub-io/opendatahub-operator presubmit Ci-operator config changed
pull-ci-opendatahub-io-opendatahub-operator-main-opendatahub-operator-rhoai-e2e opendatahub-io/opendatahub-operator presubmit Ci-operator config changed
Interacting with pj-rehearse

Comment: /pj-rehearse to run up to 5 rehearsals
Comment: /pj-rehearse skip to opt-out of rehearsals
Comment: /pj-rehearse {test-name}, with each test separated by a space, to run one or more specific rehearsals
Comment: /pj-rehearse more to run up to 10 rehearsals
Comment: /pj-rehearse max to run up to 25 rehearsals
Comment: /pj-rehearse auto-ack to run up to 5 rehearsals, and add the rehearsals-ack label on success
Comment: /pj-rehearse list to get an up-to-date list of affected jobs
Comment: /pj-rehearse abort to abort all active rehearsals
Comment: /pj-rehearse network-access-allowed to allow rehearsals of tests that have the restrict_network_access field set to false. This must be executed by an openshift org member who is not the PR author

Once you are satisfied with the results of the rehearsals, comment: /pj-rehearse ack to unblock merge. When the rehearsals-ack label is present on your PR, merge will no longer be blocked by rehearsals.
If you would like the rehearsals-ack label removed, comment: /pj-rehearse reject to re-block merging.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 4378cd1 and db2a695.

📒 Files selected for processing (5)
  • 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-commands.sh
  • ci-operator/step-registry/opendatahub-io/ci/rhoai-quay-pull-secret/opendatahub-io-ci-rhoai-quay-pull-secret-ref.metadata.json
  • ci-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

Comment on lines +72 to +89
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}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ 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'
fi

Repository: 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.

@openshift-ci

openshift-ci Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

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 /verify-owners in a comment.

  • rinaldodev
    • User is not a member of the org. User is not a collaborator. Satisfy at least one of these conditions to make the user trusted.

pre:
- ref: ipi-install-rbac
- ref: opendatahub-operator-e2e-cluster-preflight-health-check
- ref: opendatahub-io-ci-rhoai-quay-pull-secret

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should add it only for rhoai e2e?

Also, we'd need the same in the stable-2.x branch config

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants