GCP-431: feat: Add CNCC support for GCP WIF in HyperShift#7824
GCP-431: feat: Add CNCC support for GCP WIF in HyperShift#7824apahim wants to merge 8 commits intoopenshift:mainfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@apahim: 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 "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. |
|
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:
📝 WalkthroughWalkthroughAdds a Network service account field to GCP workload identity (immutable, optional) and a validation ensuring it is scoped to the same GCP project; adds a new cloud-network IAM binding entry mapping to a Kubernetes SA; sets a CNO env var for CNCC credentials when platform is GCP; reconciles an additional CNCC credentials secret and exposes CNCCCredsSecret(...) constructor; introduces a CLI flag to set the network service account and updates tests for JSON serialization compatibility and fixtures. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI / CreateOptions
participant Controller as HostedCluster Controller
participant Platform as GCP Platform Spec
participant CredMgr as GCP Credential Manager
participant K8s as Kubernetes API
participant CNO as Cluster Network Operator
CLI->>Controller: ApplyPlatformSpecifics(with network SA)
Controller->>Platform: Set WorkloadIdentity.ServiceAccountsEmails.Network
Controller->>CredMgr: ReconcileCredentials()
alt Network SA configured
CredMgr->>K8s: Ensure Secret "cloud-network-config-controller-creds" in control-plane namespace
K8s-->>CredMgr: Secret created/reconciled
CredMgr-->>Controller: Credentials mapping added
Controller->>CNO: Deploy with env GCP_CNCC_CREDENTIALS_FILE=application_default_credentials.json
CNO->>K8s: Mount credentials secret
else Network SA not configured
Controller->>CNO: Deploy without CNCC credentials env/secret
end
Estimated code review effortModerate (~20–40 minutes) 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@apahim: 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 "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. |
|
/hold |
|
@apahim: 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 "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. |
|
PR needs rebase. 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. |
Add the --network-service-account CLI flag for specifying the CNCC Google Service Account email when creating a GCP hosted cluster. The flag is required. Includes unit tests and updated test fixture. Signed-off-by: Amador Pahim <apahim@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wire the --e2e.gcp-network-sa flag and GCPNetworkServiceAccount field into the e2e test framework. Update API UX validation test with the Network field. Signed-off-by: Amador Pahim <apahim@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update GCP hosted cluster creation and IAM docs with the new --network-service-account flag and cloud-network service account. Includes generated API reference and aggregated docs. Signed-off-by: Amador Pahim <apahim@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/retest-required |
|
/test e2e-gke |
|
/test e2e-v2-gke |
|
/retest |
…countDefinitions The unparam linter flagged that customConfigPath always receives an empty string. Remove the parameter and simplify the function to always use the embedded default configuration. Signed-off-by: Amador Pahim <apahim@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Amador Pahim <apahim@redhat.com>
|
/retest |
1 similar comment
|
/retest |
|
/test e2e-v2-gke |
|
/lgtm |
|
Scheduling tests matching the |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: apahim, csrwng, muraee 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 |
|
/retest-required |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
/retest-required |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
/verified by @apahim |
|
@apahim: This PR has been marked as verified 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. |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
Now I have a complete picture. Let me produce the final report: Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryBoth Root CauseThe root cause is in The CNCC deployment ( if cpContext.HCP.Spec.Networking.NetworkType == hyperv1.OVNKubernetes {
operandsDeploymentsList = []operand{
{DeploymentName: "ovnkube-control-plane", ...},
{DeploymentName: "network-node-identity", ...},
{DeploymentName: "cloud-network-config-controller", ...}, // ← Problem
}
}However, CNCC is only deployed on cloud platforms (AWS, Azure, GCP, OpenStack) that need cloud-specific network configuration. KubeVirt clusters do not deploy CNCC, but they DO use OVN networking ( A previous fix (commit The
Since This is a pre-existing bug on main, not introduced by PR #7824. The code in Recommendations
Evidence
|
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
@apahim: 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. |
What this PR does / why we need it:
Adds CNCC (Cloud Network Config Controller) support for GCP Workload Identity Federation in HyperShift HCP mode. Currently HyperShift does not provision WIF credentials
for CNCC on GCP, so CNCC cannot authenticate to GCP APIs to manage egress IPs and subnets.
This PR:
Which issue(s) this PR fixes:
Fixes GCP-431
Special notes for your reviewer:
This PR is part of a cross-repo effort:
--token-audience=openshift to the cloud-token minter
The Network field is +required, consistent with the other GSA fields. The iam-bindings.json entry uses predefined roles (roles/compute.instanceAdmin.v1 + roles/compute.networkUser), following the same pattern as the other service accounts.
Checklist:
Summary by CodeRabbit