GCP-410: feat(gcp): add HCCO credential propagation for GCP image registry#7896
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@cblecker: This pull request references GCP-410 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 "4.22.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. |
📝 WalkthroughWalkthroughAdds GCP Workload Identity credential construction and reconciliation for hosted clusters: introduces SetupOperandCredentials to enumerate per-credential configs, validate capability gates, build external_account JSON via gcputil.BuildWorkloadIdentityCredentials, and upsert operand Secrets (e.g., image-registry service_account.json). Adds a manifest helper for the GCP image-registry secret, integrates GCP into cloud credential reconciliation, moves an in-file builder to support/gcputil, and adds unit tests for the new flows and validation. Changes
Sequence Diagram(s)sequenceDiagram
participant Reconciler as reconcileCloudCredentialSecrets
participant GCP as gcpresources.SetupOperandCredentials
participant Builder as gcputil.BuildWorkloadIdentityCredentials
participant Upsert as upsertProvider
participant K8s as Kubernetes API
Reconciler->>GCP: Invoke for HCP with GCP platform
GCP->>GCP: Enumerate credential configs (capability checks)
GCP->>Builder: Request JSON credential for serviceAccountEmail + WIF config
Builder->>Builder: Validate fields (project#, poolID, providerID, email)
alt valid
Builder-->>GCP: Return external_account JSON
GCP->>Upsert: Upsert Secret with `service_account.json`
Upsert->>K8s: Apply Secret
K8s-->>Upsert: Success
Upsert-->>GCP: Success
else invalid
Builder-->>GCP: Return validation error
GCP-->>Reconciler: Aggregate errors
end
GCP-->>Reconciler: Return result/errors
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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
area/control-plane-operator
openshift-ci
Bot
added
do-not-merge/work-in-progress
|
@cblecker: This pull request references GCP-410 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 "4.22.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. |
|
/test e2e-aws-techpreview |
|
LGTM |
cblecker
force-pushed
the
feat/gcp-410-hcco-image-registry-creds
branch
from
53670d4 to
d4bfcbb
Compare
|
/test e2e-aws-techpreview |
|
@cblecker: This pull request references GCP-410 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 "4.22.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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@control-plane-operator/hostedclusterconfigoperator/controllers/resources/gcp/gcp_test.go`:
- Around line 102-104: Test currently accepts any error in the else branch;
change the assertion to require a Kubernetes NotFound error so the
capability-gating check is specific. Replace the
g.Expect(err).To(HaveOccurred()) assertion with an explicit check using
apierrors.IsNotFound(err) (e.g.,
g.Expect(apierrors.IsNotFound(err)).To(BeTrue())), and add the import for
"k8s.io/apimachinery/pkg/api/errors" aliased as apierrors if not already
present; keep the same g test variable and location in gcp_test.go.
In
`@control-plane-operator/hostedclusterconfigoperator/controllers/resources/gcp/gcp.go`:
- Around line 73-79: When cfg.capabilityChecker(hcp.Spec.Capabilities) returns
false you must remove any previously created resource instead of simply
continuing; update the loop in the reconciliation (where configs is iterated and
cfg.capabilityChecker is checked) to call the appropriate delete logic for the
secret produced by cfg.manifestFunc() (the installer-cloud-credentials secret)
when the capability is off: call cfg.manifestFunc() or otherwise resolve the
secret name, check for its existence in the cluster, and delete it (handling
not-found as success) before continuing so the GCP credentials are removed when
ImageRegistry/capability is disabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 35f02a7e-c7ce-4648-ab02-57e9173991aa
📒 Files selected for processing (4)
control-plane-operator/hostedclusterconfigoperator/controllers/resources/gcp/gcp.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/gcp/gcp_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/manifests/creds.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go
🚧 Files skipped from review as they are similar to previous changes (1)
- control-plane-operator/hostedclusterconfigoperator/controllers/resources/resources.go
cblecker
force-pushed
the
feat/gcp-410-hcco-image-registry-creds
branch
from
d4bfcbb to
3128d86
Compare
|
@cblecker: This pull request references GCP-410 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 "4.22.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. |
|
/test e2e-aws-techpreview |
openshift-ci
Bot
added
the
area/hypershift-operator
|
Scheduling tests matching the |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
/retest-required |
|
/test e2e-aws-4-22 |
|
/verified later |
|
@cblecker: 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. |
|
/verified later @cblecker |
openshift-ci-robot
added
verified-later
verified
|
@cblecker: This PR has been marked to be verified later by 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. |
…shared package Move the GCP Workload Identity Federation credential builder out of the HO's internal GCP platform package and into a new shared support/gcputil package, so the HCCO can reuse it without creating an import cycle. - Add support/gcputil package with BuildWorkloadIdentityCredentials and exported ExternalAccountCredential/CredentialSource/CredentialSourceFormat types - Update hypershift-operator's GCP platform to call gcputil.BuildWorkloadIdentityCredentials instead of the now-removed private buildGCPWorkloadIdentityCredentials - Remove the now-duplicate private type definitions and test coverage from the HO package (tests live in support/gcputil) Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Christoph Blecker <cblecker@redhat.com>
… image registry Propagate GCP Workload Identity Federation credentials into the guest cluster's openshift-image-registry namespace so the image registry operator can authenticate to GCS. - Add control-plane-operator/hostedclusterconfigoperator/controllers/resources/gcp package with SetupOperandCredentials, following the Azure HCCO pattern - Add GCPImageRegistryCloudCredsSecret manifest targeting openshift-image-registry/installer-cloud-credentials with service_account.json - Wire SetupOperandCredentials into the resources controller's reconcileCloudCredentialSecrets GCP case - Guard secret upsert on namespace existence to handle bootstrap race - Skip credential creation when ImageRegistry capability is disabled Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Christoph Blecker <cblecker@redhat.com>
cblecker
force-pushed
the
feat/gcp-410-hcco-image-registry-creds
branch
from
715f629 to
440fbf4
Compare
openshift-ci-robot
removed
the
verified
|
/verified later @cblecker |
openshift-ci-robot
added
verified-later
verified
|
@cblecker: This PR has been marked to be verified later by 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. |
|
/pipeline required |
|
Scheduling tests matching the |
|
/lgtm |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
Confirmed: I now have all the evidence needed for the report. Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryThe Root CauseThe failure is caused by a missing CI step configuration, not by PR #7896 code changes. The dependency chain:
PR #7896 is innocent. It modifies Recommendations
Evidence
|
|
@cblecker: The following tests failed, say
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. |
openshift-merge-bot
Bot
merged commit 23d5637
into
openshift:main
5 participants
Summary
Enables the GCP image registry operator to authenticate to GCS in HyperShift-hosted
clusters by propagating a WIF credential secret into the guest cluster's
openshift-image-registrynamespace.support/gcputilpackage exposingBuildWorkloadIdentityCredentialsand theassociated
ExternalAccountCredentialtypes so both the HO and HCCO can share thecredential builder without an import cycle
gcputil.BuildWorkloadIdentityCredentials(identical behaviour, no functional change to the HO side)
control-plane-operator/hostedclusterconfigoperator/controllers/resources/gcppackage with
SetupOperandCredentials, following the Azure HCCO pattern; currentlymanages a single credential —
openshift-image-registry/installer-cloud-credentialspopulated with a WIF JSON blob for the image registry GSA
reconcileCloudCredentialSecretsin the HCCO resources controllernot yet exist on first reconcile); mirrors the same pattern used for AWS
ImageRegistrycapability is explicitly disabledThe
service_account.jsondata key matches the CCO-provisioned format expected bycluster-image-registry-operator. Only image registry is wired through HCCO for GCP;other GCP operators receive their credentials directly from HO-provisioned secrets.
Test plan
go test ./support/gcputil/...— unit tests covering happy path and all validationerror branches (
ProjectNumber,PoolID,ProviderID,ServiceAccountEmailempty)go test ./control-plane-operator/hostedclusterconfigoperator/controllers/resources/gcp/...— unit tests covering: capability enabled (secret created with correct WIF JSON structure),
capability disabled (secret skipped), target namespace absent (skipped without error)
make test— full unit test suiteimageRegistryGSA set, verifyopenshift-image-registry/installer-cloud-credentialsexists in the guest cluster witha valid
service_account.jsonkey, and confirm the image registry operator reachesAvailable(tracked in GCP-413)