ROSAENG-1067: add gcp marketplace testing to rosa-e2e#79383
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@bmeng: This pull request references ROSAENG-1067 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new rosa-e2e-osd-gcp workflow, metadata, and OWNERS; schedules a new periodic job for OSD GCP (OpenShift 4.22); and updates provisioning/deprovisioning scripts to select OCM authentication via offline token or SSO credentials from CLUSTER_PROFILE_DIR. ChangesROSA E2E OSD GCP Test Pipeline
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ci-operator/step-registry/osd-ccs/cluster/deprovision/osd-ccs-cluster-deprovision-commands.sh (1)
11-23: ⚡ Quick winWrap credential handling with
set +xto prevent potential log exposure.Same pattern as the provision script—sensitive credential operations should be wrapped with tracing protection.
🛡️ Proposed defensive fix
# Log in OCM_VERSION=$(ocm version) +{ set +x; } 2>/dev/null SSO_CLIENT_ID=$(cat "${CLUSTER_PROFILE_DIR}/sso-client-id" 2>/dev/null || true) SSO_CLIENT_SECRET=$(cat "${CLUSTER_PROFILE_DIR}/sso-client-secret" 2>/dev/null || true) OCM_TOKEN=$(cat "${CLUSTER_PROFILE_DIR}/ocm-token" 2>/dev/null || true) if [[ -n "${SSO_CLIENT_ID}" && -n "${SSO_CLIENT_SECRET}" ]]; then echo "Logging into ${OCM_LOGIN_ENV} with SSO credentials using ocm cli ${OCM_VERSION}" ocm login --url "${OCM_LOGIN_ENV}" --client-id "${SSO_CLIENT_ID}" --client-secret "${SSO_CLIENT_SECRET}" elif [[ -n "${OCM_TOKEN}" ]]; then echo "Logging into ${OCM_LOGIN_ENV} with offline token using ocm cli ${OCM_VERSION}" ocm login --url "${OCM_LOGIN_ENV}" --token "${OCM_TOKEN}" else echo "Cannot login! You need to securely supply SSO credentials or an ocm-token!" exit 1 fi +set -x 2>/dev/null || trueAs per coding guidelines: "Use set +x and save/restore tracing state when handling passwords, tokens, or sensitive URLs in step registry scripts to prevent credential leakage in CI logs"
🤖 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/osd-ccs/cluster/deprovision/osd-ccs-cluster-deprovision-commands.sh` around lines 11 - 23, Wrap the sensitive credential reads and ocm login calls with tracing disabled: before reading SSO_CLIENT_ID/SSO_CLIENT_SECRET/OCM_TOKEN and before invoking the ocm login commands, save the current xtrace state, run set +x to disable tracing, perform the cat reads and the ocm login invocation(s) that use SSO_CLIENT_ID, SSO_CLIENT_SECRET or OCM_TOKEN, then restore the original tracing state; apply this around both the SSO branch (ocm login --client-id/--client-secret) and the token branch (ocm login --token) so the sensitive values are never echoed to CI logs.ci-operator/step-registry/osd-ccs/cluster/provision/gcp/osd-ccs-cluster-provision-gcp-commands.sh (1)
148-160: ⚡ Quick winWrap credential handling with
set +xto prevent potential log exposure.Per coding guidelines, sensitive data operations (reading tokens/secrets and passing them to commands) should be wrapped with tracing protection to prevent credential leakage if
set -xis enabled for debugging or by the CI environment.🛡️ Proposed defensive fix
# Log in OCM_VERSION=$(ocm version) +{ set +x; } 2>/dev/null SSO_CLIENT_ID=$(cat "${CLUSTER_PROFILE_DIR}/sso-client-id" 2>/dev/null || true) SSO_CLIENT_SECRET=$(cat "${CLUSTER_PROFILE_DIR}/sso-client-secret" 2>/dev/null || true) OCM_TOKEN=$(cat "${CLUSTER_PROFILE_DIR}/ocm-token" 2>/dev/null || true) if [[ -n "${SSO_CLIENT_ID}" && -n "${SSO_CLIENT_SECRET}" ]]; then logger "INFO" "Logging into ${OCM_LOGIN_ENV} with SSO credentials using ocm cli ${OCM_VERSION}" ocm login --url "${OCM_LOGIN_ENV}" --client-id "${SSO_CLIENT_ID}" --client-secret "${SSO_CLIENT_SECRET}" elif [[ -n "${OCM_TOKEN}" ]]; then logger "INFO" "Logging into ${OCM_LOGIN_ENV} with offline token using ocm cli ${OCM_VERSION}" ocm login --url "${OCM_LOGIN_ENV}" --token "${OCM_TOKEN}" else logger "ERROR" "Cannot login! You need to securely supply SSO credentials or an ocm-token!" exit 1 fi +set -x 2>/dev/null || trueAs per coding guidelines: "Use set +x and save/restore tracing state when handling passwords, tokens, or sensitive URLs in step registry scripts to prevent credential leakage in CI logs"
🤖 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/osd-ccs/cluster/provision/gcp/osd-ccs-cluster-provision-gcp-commands.sh` around lines 148 - 160, The credential reads and ocm login calls (SSO_CLIENT_ID, SSO_CLIENT_SECRET, OCM_TOKEN and the ocm login invocations) must be executed with shell tracing disabled to avoid leaking secrets; save the current xtrace state, run set +x before reading files from CLUSTER_PROFILE_DIR and before invoking ocm login (both SSO and token branches), then restore the original xtrace state afterward; keep the logger messages and exit logic unchanged and ensure the save/restore handles both enabled and disabled trace states so tracing is restored exactly as it was before.
🤖 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.
Nitpick comments:
In
`@ci-operator/step-registry/osd-ccs/cluster/deprovision/osd-ccs-cluster-deprovision-commands.sh`:
- Around line 11-23: Wrap the sensitive credential reads and ocm login calls
with tracing disabled: before reading SSO_CLIENT_ID/SSO_CLIENT_SECRET/OCM_TOKEN
and before invoking the ocm login commands, save the current xtrace state, run
set +x to disable tracing, perform the cat reads and the ocm login invocation(s)
that use SSO_CLIENT_ID, SSO_CLIENT_SECRET or OCM_TOKEN, then restore the
original tracing state; apply this around both the SSO branch (ocm login
--client-id/--client-secret) and the token branch (ocm login --token) so the
sensitive values are never echoed to CI logs.
In
`@ci-operator/step-registry/osd-ccs/cluster/provision/gcp/osd-ccs-cluster-provision-gcp-commands.sh`:
- Around line 148-160: The credential reads and ocm login calls (SSO_CLIENT_ID,
SSO_CLIENT_SECRET, OCM_TOKEN and the ocm login invocations) must be executed
with shell tracing disabled to avoid leaking secrets; save the current xtrace
state, run set +x before reading files from CLUSTER_PROFILE_DIR and before
invoking ocm login (both SSO and token branches), then restore the original
xtrace state afterward; keep the logger messages and exit logic unchanged and
ensure the save/restore handles both enabled and disabled trace states so
tracing is restored exactly as it was before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 634399c0-f1f1-490d-a4e6-588c176d88b0
📒 Files selected for processing (4)
ci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__periodics.yamlci-operator/step-registry/osd-ccs/cluster/deprovision/osd-ccs-cluster-deprovision-commands.shci-operator/step-registry/osd-ccs/cluster/provision/gcp/osd-ccs-cluster-provision-gcp-commands.shci-operator/step-registry/rosa/e2e/osd-gcp/rosa-e2e-osd-gcp-workflow.yaml
bmeng
force-pushed
the
gcpwif-e2e
branch
2 times, most recently
from
0d55994 to
1d6eeb8
Compare
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/osd-ccs/cluster/provision/gcp/osd-ccs-cluster-provision-gcp-commands.sh`:
- Around line 148-160: This code exposes sensitive values (OCM_TOKEN,
SSO_CLIENT_ID, SSO_CLIENT_SECRET and the ocm login URL) if shell tracing is
enabled; wrap the credential-reading and ocm login sequence with saved/restored
xtrace state: capture current tracing state, disable tracing (set +x) while
reading the files and invoking ocm login (the block referencing OCM_TOKEN,
SSO_CLIENT_ID, SSO_CLIENT_SECRET, ocm login and logger), then restore the
original tracing state immediately after to avoid leaking secrets in CI logs.
🪄 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: Enterprise
Run ID: 340ac11c-2010-4e6c-a84d-3dbe8747427d
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (6)
ci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__periodics.yamlci-operator/step-registry/osd-ccs/cluster/deprovision/osd-ccs-cluster-deprovision-commands.shci-operator/step-registry/osd-ccs/cluster/provision/gcp/osd-ccs-cluster-provision-gcp-commands.shci-operator/step-registry/rosa/e2e/osd-gcp/OWNERSci-operator/step-registry/rosa/e2e/osd-gcp/rosa-e2e-osd-gcp-workflow.metadata.jsonci-operator/step-registry/rosa/e2e/osd-gcp/rosa-e2e-osd-gcp-workflow.yaml
✅ Files skipped from review due to trivial changes (2)
- ci-operator/step-registry/rosa/e2e/osd-gcp/OWNERS
- ci-operator/step-registry/rosa/e2e/osd-gcp/rosa-e2e-osd-gcp-workflow.metadata.json
🚧 Files skipped from review as they are similar to previous changes (3)
- ci-operator/step-registry/rosa/e2e/osd-gcp/rosa-e2e-osd-gcp-workflow.yaml
- ci-operator/step-registry/osd-ccs/cluster/deprovision/osd-ccs-cluster-deprovision-commands.sh
- ci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__periodics.yaml
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-periodics-osd-gcp-e2e-candidate-4-22 |
|
@bmeng: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
bmeng
force-pushed
the
gcpwif-e2e
branch
2 times, most recently
from
9799106 to
da7c9a6
Compare
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-periodics-osd-gcp-e2e-candidate-4-22 |
|
@bmeng: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ci-operator/step-registry/osd-ccs/cluster/provision/gcp/osd-ccs-cluster-provision-gcp-ref.yaml (1)
64-66:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate step docs to reflect SSO login support.
The documentation still says the cluster profile should include only
ocm-token, but the script now supports either offline token or SSO client credentials. This mismatch can mislead step consumers.Suggested fix
documentation: |- - Using ocm cli to create an osd ccs GCP cluster with the provided cluster profile. The cluster profile should include the offline token ocm-token to login. + Using ocm cli to create an osd ccs GCP cluster with the provided cluster profile. + The cluster profile must include either ocm-token or both sso-client-id and sso-client-secret for login.🤖 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/osd-ccs/cluster/provision/gcp/osd-ccs-cluster-provision-gcp-ref.yaml` around lines 64 - 66, The documentation YAML block under the documentation key currently states the cluster profile should include only `ocm-token`; update that text to reflect the script now accepts either an offline token (`ocm-token`) or SSO client credentials (e.g., `sso_client_id` and `sso_client_secret` or the specific SSO fields your step expects) for login; locate the documentation string (the documentation: |- block) and edit the sentence to mention both supported authentication methods and any required field names/format so consumers know they can use offline token or SSO client credentials.
🤖 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/osd-ccs/gcp/wif-config/deprovision/osd-ccs-gcp-wif-config-deprovision-commands.sh`:
- Around line 16-29: Wrap reading of OCM_TOKEN/SSO_CLIENT_ID/SSO_CLIENT_SECRET
and the subsequent ocm login calls with a saved xtrace state: capture current
xtrace with e.g. old_xtrace=$(set -o | grep xtrace) or check "$-" for x, disable
tracing (set +x), perform the sensitive reads and both ocm login branches (the
blocks using OCM_TOKEN, SSO_CLIENT_ID, SSO_CLIENT_SECRET and the ocm login
calls), then restore the original tracing state (re-enable set -x only if it was
previously enabled). Ensure you reference the existing variables OCM_TOKEN,
SSO_CLIENT_ID, SSO_CLIENT_SECRET and the ocm login branches so the
save/disable/restore wraps exactly those operations.
In
`@ci-operator/step-registry/osd-ccs/gcp/wif-config/provision/osd-ccs-gcp-wif-config-provision-commands.sh`:
- Around line 16-29: The ocm login block (checking OCM_TOKEN,
SSO_CLIENT_ID/SSO_CLIENT_SECRET and calling ocm login) does not protect
sensitive values from shell xtrace; wrap the sections that read and use
OCM_TOKEN/SSO_CLIENT_SECRET in code that saves the current xtrace state,
disables tracing (set +x) before reading/using secrets and then restores the
original tracing state afterward so tokens/secrets are never printed in CI logs;
apply this around the branches that call ocm login (references: OCM_TOKEN,
SSO_CLIENT_ID, SSO_CLIENT_SECRET, and the ocm login calls).
---
Outside diff comments:
In
`@ci-operator/step-registry/osd-ccs/cluster/provision/gcp/osd-ccs-cluster-provision-gcp-ref.yaml`:
- Around line 64-66: The documentation YAML block under the documentation key
currently states the cluster profile should include only `ocm-token`; update
that text to reflect the script now accepts either an offline token
(`ocm-token`) or SSO client credentials (e.g., `sso_client_id` and
`sso_client_secret` or the specific SSO fields your step expects) for login;
locate the documentation string (the documentation: |- block) and edit the
sentence to mention both supported authentication methods and any required field
names/format so consumers know they can use offline token or SSO client
credentials.
🪄 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: Enterprise
Run ID: 0d4bf12c-d781-44cc-8566-72fdd569259b
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (14)
ci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__periodics.yamlci-operator/step-registry/osd-ccs/cluster/deprovision/osd-ccs-cluster-deprovision-commands.shci-operator/step-registry/osd-ccs/cluster/provision/gcp/osd-ccs-cluster-provision-gcp-commands.shci-operator/step-registry/osd-ccs/cluster/provision/gcp/osd-ccs-cluster-provision-gcp-ref.yamlci-operator/step-registry/osd-ccs/gcp/wif-config/OWNERSci-operator/step-registry/osd-ccs/gcp/wif-config/deprovision/OWNERSci-operator/step-registry/osd-ccs/gcp/wif-config/deprovision/osd-ccs-gcp-wif-config-deprovision-commands.shci-operator/step-registry/osd-ccs/gcp/wif-config/deprovision/osd-ccs-gcp-wif-config-deprovision-ref.yamlci-operator/step-registry/osd-ccs/gcp/wif-config/provision/OWNERSci-operator/step-registry/osd-ccs/gcp/wif-config/provision/osd-ccs-gcp-wif-config-provision-commands.shci-operator/step-registry/osd-ccs/gcp/wif-config/provision/osd-ccs-gcp-wif-config-provision-ref.yamlci-operator/step-registry/rosa/e2e/osd-gcp/OWNERSci-operator/step-registry/rosa/e2e/osd-gcp/rosa-e2e-osd-gcp-workflow.metadata.jsonci-operator/step-registry/rosa/e2e/osd-gcp/rosa-e2e-osd-gcp-workflow.yaml
✅ Files skipped from review due to trivial changes (4)
- ci-operator/step-registry/osd-ccs/gcp/wif-config/deprovision/OWNERS
- ci-operator/step-registry/osd-ccs/gcp/wif-config/provision/OWNERS
- ci-operator/step-registry/osd-ccs/gcp/wif-config/OWNERS
- ci-operator/step-registry/rosa/e2e/osd-gcp/rosa-e2e-osd-gcp-workflow.metadata.json
🚧 Files skipped from review as they are similar to previous changes (4)
- ci-operator/config/openshift-online/rosa-e2e/openshift-online-rosa-e2e-main__periodics.yaml
- ci-operator/step-registry/rosa/e2e/osd-gcp/OWNERS
- ci-operator/step-registry/osd-ccs/cluster/deprovision/osd-ccs-cluster-deprovision-commands.sh
- ci-operator/step-registry/rosa/e2e/osd-gcp/rosa-e2e-osd-gcp-workflow.yaml
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-periodics-osd-gcp-e2e-candidate-4-22 |
|
@bmeng: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-periodics-osd-gcp-e2e-candidate-4-22 |
|
@bmeng: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-periodics-osd-gcp-e2e-candidate-4-22 |
|
@bmeng: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-periodics-osd-gcp-e2e-candidate-4-22 |
|
@bmeng: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-periodics-osd-gcp-e2e-candidate-4-22 |
|
@bmeng: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-online-rosa-e2e-main-periodics-osd-gcp-e2e-candidate-4-22 |
|
@bmeng: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
A total of 58 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-release-main-nightly-4.20-e2e-osd-ccs-gcp |
|
@bmeng: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
Please add myself and @miguelhbrito to the list of reviewers and approvers.
|
/lgtm |
|
@dustman9000: your |
|
/retest rehearse-79383-periodic-ci-openshift-release-main-nightly-4.20-e2e-osd-ccs-gcp |
|
@dustman9000: The The following commands are available to trigger optional jobs: Use 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 kubernetes-sigs/prow repository. |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-4.20-e2e-osd-ccs-gcp |
|
@dustman9000: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/lgtm |
|
@dustman9000: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-merge-bot
Bot
added
the
rehearsals-ack
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bmeng, dustman9000 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
Bot
merged commit 4a22f4c
into
openshift:main
4 participants
Note: as the job is currently using labelfilter OSD-GCP and we do not have the Label implement in rosa-e2e yet, there is no real testing performed, only test the workflow with gcp-marketplace + wif
==================================
This PR updates OpenShift CI configuration in the openshift/release repository to add GCP Marketplace testing for rosa-e2e and to make ROSA OCM authentication and provisioning steps support Workload Identity Federation (WIF) and SSO client credentials.
What is affected (practical terms)
Key changes
New periodic job
New workflow, WIF steps, refs, metadata and OWNERS
Provisioning / deprovisioning authentication and WIF control
Impact and rationale
Additional notes