GCP-431: feat: pass cloud-network SA to GCP HyperShift e2e tests#78529
GCP-431: feat: pass cloud-network SA to GCP HyperShift e2e tests#78529patjlm wants to merge 1 commit intoopenshift:mainfrom
Conversation
Extract the cloud-network (CNCC) service account email from the IAM output and pass it to the e2e test binary via --e2e.gcp-network-sa. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
openshift-ci-robot
added
the
jira/valid-reference
|
@patjlm: This pull request references GCP-431 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. |
WalkthroughThe changes add support for a GCP network service account parameter to the HyperShift GCP CI workflow. The hosted-cluster-setup script extracts a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: patjlm 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-ci
Bot
added
the
approved
|
[REHEARSALNOTIFIER]
Prior to this PR being merged, you will need to either run and acknowledge or opt to skip these rehearsals. Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, 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/hypershift/gcp/run-e2e/hypershift-gcp-run-e2e-commands.sh`:
- Around line 75-77: Add a strict non-empty guard for the NETWORK_SA variable
after it is read from SHARED_DIR so an empty string cannot be propagated into
test execution; check that NETWORK_SA is set and not empty (e.g., test -n or [[
-n "${NETWORK_SA}" ]]) and exit with an error message if it is empty, so
wherever NETWORK_SA is later used in the script (the test execution invocation
that currently forwards NETWORK_SA) you can assume a valid value; update the
block that reads NETWORK_SA and add the guard/early-exit handling with a clear
error log.
🪄 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: cc2a67a1-0b98-43b2-af3a-6b2350eec26e
📒 Files selected for processing (2)
ci-operator/step-registry/hypershift/gcp/hosted-cluster-setup/hypershift-gcp-hosted-cluster-setup-commands.shci-operator/step-registry/hypershift/gcp/run-e2e/hypershift-gcp-run-e2e-commands.sh
| if [[ -f "${SHARED_DIR}/network-sa" ]]; then | ||
| NETWORK_SA="$(<"${SHARED_DIR}/network-sa")" | ||
| fi |
There was a problem hiding this comment.
Add an explicit non-empty guard for NETWORK_SA before test execution.
Right now, an empty value still gets forwarded on Line 160. Failing early here would make workflow breakages much easier to diagnose.
Suggested hardening
if [[ -f "${SHARED_DIR}/network-sa" ]]; then
NETWORK_SA="$(<"${SHARED_DIR}/network-sa")"
fi
+
+if [[ -z "${NETWORK_SA}" ]]; then
+ echo "ERROR: network service account not found or empty at ${SHARED_DIR}/network-sa"
+ echo "Ensure hypershift-gcp-hosted-cluster-setup step produced network-sa"
+ exit 1
+fiAlso applies to: 160-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ci-operator/step-registry/hypershift/gcp/run-e2e/hypershift-gcp-run-e2e-commands.sh`
around lines 75 - 77, Add a strict non-empty guard for the NETWORK_SA variable
after it is read from SHARED_DIR so an empty string cannot be propagated into
test execution; check that NETWORK_SA is set and not empty (e.g., test -n or [[
-n "${NETWORK_SA}" ]]) and exit with an error message if it is empty, so
wherever NETWORK_SA is later used in the script (the test execution invocation
that currently forwards NETWORK_SA) you can assume a valid value; update the
block that reads NETWORK_SA and add the guard/early-exit handling with a clear
error log.
|
Closing - local clone was stale, will recreate from a fresh base. |
2 participants
Summary
cloud-networkSA email from IAM output in hosted-cluster-setup step--e2e.gcp-network-saflag to e2e test binary in run-e2e stepContext
HyperShift PR openshift/hypershift#7824 added e2e flag plumbing for the CNCC
network service account (
--e2e.gcp-network-sa). The CI step needs to extractthe SA email and pass it through.
Dependencies
Jira
Summary by CodeRabbit