From b1e8b83b97c9e9df465e059260815b9256f47622 Mon Sep 17 00:00:00 2001 From: Bruno Cesar Rocha Date: Mon, 8 Jun 2026 18:21:16 +0100 Subject: [PATCH 1/5] CORS-4470: add GCP Workload Identity Federation support Add support for short-lived token authentication via GCP Workload Identity Federation (WIF), replacing static service account keys. Two modes are supported: - BYO: customer provides existing poolID and providerID in the install-config. The installer describes the provider via the GCP IAM API to extract the OIDC issuer URL, validates it, and generates external_account credential manifests. - Installer-provisioned: customer sets workloadIdentityFederation: {} with empty fields. The installer creates a WIF pool, GCS bucket for OIDC discovery, OIDC provider, and service account bindings using deterministic names derived from infraID. CredentialsMode auto-defaults to Manual when WIF is configured. Destroy flow cleans up installer-provisioned WIF resources but leaves BYO resources untouched. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../install.openshift.io_installconfigs.yaml | 18 + .../cloud-creds-secret.yaml.template | 4 + pkg/asset/cluster/gcp/gcp.go | 15 +- .../azure/mock/azureclient_generated.go | 4 +- pkg/asset/installconfig/gcp/client.go | 20 + .../gcp/mock/gcpclient_generated.go | 16 + pkg/asset/installconfig/gcp/validation.go | 38 ++ pkg/asset/manifests/openshift.go | 93 ++++- pkg/asset/manifests/template.go | 6 + pkg/destroy/gcp/gcp.go | 9 + pkg/destroy/gcp/wif.go | 146 +++++++ .../gcp/clusterapi/clusterapi.go | 6 + pkg/infrastructure/gcp/clusterapi/wif.go | 389 ++++++++++++++++++ pkg/infrastructure/gcp/clusterapi/wif_test.go | 79 ++++ pkg/types/defaults/installconfig.go | 2 + pkg/types/gcp/metadata.go | 15 +- pkg/types/gcp/platform.go | 33 ++ pkg/types/gcp/validation/platform.go | 47 +++ pkg/types/gcp/validation/platform_test.go | 76 ++++ pkg/types/gcp/zz_generated.deepcopy.go | 26 ++ 20 files changed, 1016 insertions(+), 26 deletions(-) create mode 100644 pkg/destroy/gcp/wif.go create mode 100644 pkg/infrastructure/gcp/clusterapi/wif.go create mode 100644 pkg/infrastructure/gcp/clusterapi/wif_test.go diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index 37c2e9c4021..54c085fee86 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -6715,6 +6715,24 @@ spec: - value type: object type: array + workloadIdentityFederation: + description: |- + WorkloadIdentityFederation configures GCP Workload Identity Federation + for short-lived token authentication. When set, the installer generates + external_account credential manifests instead of static service account keys. + properties: + poolID: + description: |- + PoolID is the ID of an existing workload identity pool. + When both poolID and providerID are provided, the installer uses the + existing WIF resources (BYO mode). When both are omitted, the installer + creates new WIF resources. + type: string + providerID: + description: ProviderID is the ID of an existing OIDC provider + within the pool. + type: string + type: object required: - projectID - region diff --git a/data/data/manifests/openshift/cloud-creds-secret.yaml.template b/data/data/manifests/openshift/cloud-creds-secret.yaml.template index d19bc3a2a87..08543a510e2 100644 --- a/data/data/manifests/openshift/cloud-creds-secret.yaml.template +++ b/data/data/manifests/openshift/cloud-creds-secret.yaml.template @@ -11,6 +11,8 @@ data: azure_resource_prefix: {{.CloudCreds.Azure.Base64encodeResourcePrefix}} azure_resourcegroup: {{.CloudCreds.Azure.Base64encodeResourceGroup}} azure_region: {{.CloudCreds.Azure.Base64encodeRegion}} +{{- else if .CloudCreds.GCPWIF}} + service_account.json: {{.CloudCreds.GCPWIF.Base64encodeExternalAccountJSON}} {{- else if .CloudCreds.GCP}} service_account.json: {{.CloudCreds.GCP.Base64encodeServiceAccount}} {{- else if .CloudCreds.IBMCloud}} @@ -40,6 +42,8 @@ metadata: name: aws-creds {{- else if .CloudCreds.Azure}} name: azure-credentials +{{- else if .CloudCreds.GCPWIF}} + name: gcp-credentials {{- else if .CloudCreds.GCP}} name: gcp-credentials {{- else if .CloudCreds.IBMCloud}} diff --git a/pkg/asset/cluster/gcp/gcp.go b/pkg/asset/cluster/gcp/gcp.go index 5114b7057ee..c242612428b 100644 --- a/pkg/asset/cluster/gcp/gcp.go +++ b/pkg/asset/cluster/gcp/gcp.go @@ -23,12 +23,13 @@ func Metadata(config *types.InstallConfig) *gcp.Metadata { } return &gcp.Metadata{ - Region: config.Platform.GCP.Region, - ProjectID: config.Platform.GCP.ProjectID, - NetworkProjectID: config.Platform.GCP.NetworkProjectID, - PrivateZoneDomain: privateZoneDomain, - PrivateZoneProjectID: privateZoneProject, - Endpoint: config.Platform.GCP.Endpoint, - FirewallRulesManagement: config.GCP.FirewallRulesManagement, + Region: config.Platform.GCP.Region, + ProjectID: config.Platform.GCP.ProjectID, + NetworkProjectID: config.Platform.GCP.NetworkProjectID, + PrivateZoneDomain: privateZoneDomain, + PrivateZoneProjectID: privateZoneProject, + Endpoint: config.Platform.GCP.Endpoint, + FirewallRulesManagement: config.GCP.FirewallRulesManagement, + WorkloadIdentityFederation: config.Platform.GCP.WorkloadIdentityFederation, } } diff --git a/pkg/asset/installconfig/azure/mock/azureclient_generated.go b/pkg/asset/installconfig/azure/mock/azureclient_generated.go index 18eb90b533c..996a85b69bf 100644 --- a/pkg/asset/installconfig/azure/mock/azureclient_generated.go +++ b/pkg/asset/installconfig/azure/mock/azureclient_generated.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: client.go +// Source: ./client.go // // Generated by this command: // -// mockgen -source=client.go -destination=mock/azureclient_generated.go -package=mock +// mockgen -source=./client.go -destination=mock/azureclient_generated.go -package=mock // // Package mock is a generated GoMock package. diff --git a/pkg/asset/installconfig/gcp/client.go b/pkg/asset/installconfig/gcp/client.go index ee7bbe27a41..1f351b6c2d2 100644 --- a/pkg/asset/installconfig/gcp/client.go +++ b/pkg/asset/installconfig/gcp/client.go @@ -15,6 +15,7 @@ import ( compute "google.golang.org/api/compute/v1" dns "google.golang.org/api/dns/v1" "google.golang.org/api/googleapi" + "google.golang.org/api/iam/v1" "google.golang.org/api/iterator" "google.golang.org/api/option" serviceusage "google.golang.org/api/serviceusage/v1beta1" @@ -60,6 +61,7 @@ type API interface { GetKeyRing(ctx context.Context, kmsKeyRef *gcptypes.KMSKeyReference) (*kmspb.KeyRing, error) UpdateDNSPrivateZoneLabels(ctx context.Context, baseDomain, project, zoneName string, labels map[string]string) error GetPrivateServiceConnectEndpoint(ctx context.Context, project string, endpoint *gcptypes.PSCEndpoint) (*compute.ForwardingRule, error) + GetWIFProvider(ctx context.Context, project, poolID, providerID string) (*iam.WorkloadIdentityPoolProvider, error) } // Client makes calls to the GCP API. @@ -795,6 +797,24 @@ func (c *Client) GetPrivateServiceConnectEndpoint(ctx context.Context, project s return GetPrivateServiceConnectEndpoint(svc, project, endpoint) } +// GetWIFProvider retrieves a Workload Identity Pool Provider. +func (c *Client) GetWIFProvider(ctx context.Context, project, poolID, providerID string) (*iam.WorkloadIdentityPoolProvider, error) { + ctx, cancel := context.WithTimeout(ctx, defaultTimeout) + defer cancel() + + iamSvc, err := GetIAMService(ctx) + if err != nil { + return nil, fmt.Errorf("failed to create IAM service: %w", err) + } + + name := fmt.Sprintf("projects/%s/locations/global/workloadIdentityPools/%s/providers/%s", project, poolID, providerID) + provider, err := iamSvc.Projects.Locations.WorkloadIdentityPools.Providers.Get(name).Context(ctx).Do() + if err != nil { + return nil, fmt.Errorf("failed to get WIF provider %s: %w", name, err) + } + return provider, nil +} + // aiZone returns true if the GCP zone follows the AI naming convention. // Uses the regular expression pattern as documented in GCP API docs: // "To match zones containing ai in their name, use the filter query parameter with the regular expression name eq '.*-ai.*'." diff --git a/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go b/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go index 205cacb7483..8534d496a7a 100644 --- a/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go +++ b/pkg/asset/installconfig/gcp/mock/gcpclient_generated.go @@ -20,6 +20,7 @@ import ( cloudresourcemanager "google.golang.org/api/cloudresourcemanager/v3" compute "google.golang.org/api/compute/v1" dns "google.golang.org/api/dns/v1" + iam "google.golang.org/api/iam/v1" sets "k8s.io/apimachinery/pkg/util/sets" ) @@ -362,6 +363,21 @@ func (mr *MockAPIMockRecorder) GetSubnetworks(ctx, network, project, region any) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSubnetworks", reflect.TypeOf((*MockAPI)(nil).GetSubnetworks), ctx, network, project, region) } +// GetWIFProvider mocks base method. +func (m *MockAPI) GetWIFProvider(ctx context.Context, project, poolID, providerID string) (*iam.WorkloadIdentityPoolProvider, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetWIFProvider", ctx, project, poolID, providerID) + ret0, _ := ret[0].(*iam.WorkloadIdentityPoolProvider) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetWIFProvider indicates an expected call of GetWIFProvider. +func (mr *MockAPIMockRecorder) GetWIFProvider(ctx, project, poolID, providerID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetWIFProvider", reflect.TypeOf((*MockAPI)(nil).GetWIFProvider), ctx, project, poolID, providerID) +} + // GetZones mocks base method. func (m *MockAPI) GetZones(ctx context.Context, project, filter string) ([]*compute.Zone, error) { m.ctrl.T.Helper() diff --git a/pkg/asset/installconfig/gcp/validation.go b/pkg/asset/installconfig/gcp/validation.go index 12c268b9a17..9469486e278 100644 --- a/pkg/asset/installconfig/gcp/validation.go +++ b/pkg/asset/installconfig/gcp/validation.go @@ -71,6 +71,7 @@ func Validate(client API, ic *types.InstallConfig) error { allErrs = append(allErrs, validateMarketplaceImages(client, ic)...) allErrs = append(allErrs, validatePlatformKMSKeys(client, ic, field.NewPath("platform").Child("gcp"))...) allErrs = append(allErrs, validateServiceEndpointOverride(client, ic, field.NewPath("platform").Child("gcp"))...) + allErrs = append(allErrs, validateWIFProvider(client, ic)...) if err := validateUserTags(client, ic.Platform.GCP.ProjectID, ic.Platform.GCP.UserTags); err != nil { allErrs = append(allErrs, field.Invalid(field.NewPath("platform").Child("gcp").Child("userTags"), ic.Platform.GCP.UserTags, err.Error())) @@ -731,6 +732,43 @@ func ValidateCredentialMode(client API, ic *types.InstallConfig) field.ErrorList errMsg := "Manual credentials mode needs to be enabled to use environmental authentication" return append(allErrs, field.Forbidden(field.NewPath("credentialsMode"), errMsg)) } + + if ic.GCP.IsWIFEnabled() && ic.CredentialsMode != types.ManualCredentialsMode { + errMsg := "workload identity federation requires Manual credentials mode" + return append(allErrs, field.Forbidden(field.NewPath("credentialsMode"), errMsg)) + } + + return allErrs +} + +func validateWIFProvider(client API, ic *types.InstallConfig) field.ErrorList { + allErrs := field.ErrorList{} + if !ic.GCP.IsWIFBYO() { + return allErrs + } + + fldPath := field.NewPath("platform").Child("gcp").Child("workloadIdentityFederation") + wif := ic.GCP.WorkloadIdentityFederation + + provider, err := client.GetWIFProvider(context.TODO(), ic.GCP.ProjectID, wif.PoolID, wif.ProviderID) + if err != nil { + return append(allErrs, field.Invalid(fldPath, wif.ProviderID, + fmt.Sprintf("failed to get WIF provider: %v", err))) + } + + if provider.State != "ACTIVE" { + allErrs = append(allErrs, field.Invalid(fldPath.Child("providerID"), wif.ProviderID, + fmt.Sprintf("WIF provider is not active (state: %s)", provider.State))) + } + + if provider.Oidc == nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("providerID"), wif.ProviderID, + "WIF provider does not have an OIDC configuration")) + } else if provider.Oidc.IssuerUri == "" { + allErrs = append(allErrs, field.Invalid(fldPath.Child("providerID"), wif.ProviderID, + "WIF provider OIDC issuer URI is empty")) + } + return allErrs } diff --git a/pkg/asset/manifests/openshift.go b/pkg/asset/manifests/openshift.go index 227c5fca101..0b4c2c9dbde 100644 --- a/pkg/asset/manifests/openshift.go +++ b/pkg/asset/manifests/openshift.go @@ -3,6 +3,7 @@ package manifests import ( "context" "encoding/base64" + "encoding/json" "fmt" "os" "path" @@ -14,6 +15,7 @@ import ( "github.com/gophercloud/utils/v2/openstack/clientconfig" "github.com/pkg/errors" "github.com/sirupsen/logrus" + "google.golang.org/api/option" "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/yaml" @@ -137,15 +139,27 @@ func (o *Openshift) Generate(ctx context.Context, dependencies asset.Parents) er }, } case gcptypes.Name: - session, err := gcp.GetSession(ctx) - if err != nil { - return err - } - creds := session.Credentials.JSON - cloudCreds = cloudCredsSecretData{ - GCP: &GCPCredsSecretData{ - Base64encodeServiceAccount: base64.StdEncoding.EncodeToString(creds), - }, + if installConfig.Config.GCP.IsWIFEnabled() { + externalAccountJSON, err := generateGCPExternalAccountJSON(ctx, installConfig, clusterID.InfraID) + if err != nil { + return err + } + cloudCreds = cloudCredsSecretData{ + GCPWIF: &GCPWIFCredsSecretData{ + Base64encodeExternalAccountJSON: base64.StdEncoding.EncodeToString(externalAccountJSON), + }, + } + } else { + session, err := gcp.GetSession(ctx) + if err != nil { + return err + } + creds := session.Credentials.JSON + cloudCreds = cloudCredsSecretData{ + GCP: &GCPCredsSecretData{ + Base64encodeServiceAccount: base64.StdEncoding.EncodeToString(creds), + }, + } } case ibmcloudtypes.Name: client, err := ibmcloud.NewClient(installConfig.Config.Platform.IBMCloud.ServiceEndpoints) @@ -277,7 +291,11 @@ func (o *Openshift) Generate(ctx context.Context, dependencies asset.Parents) er switch platform { case awstypes.Name, openstacktypes.Name, powervctypes.Name, vspheretypes.Name, azuretypes.Name, gcptypes.Name, ibmcloudtypes.Name, ovirttypes.Name: - if installConfig.Config.CredentialsMode != types.ManualCredentialsMode { + skipCloudCreds := installConfig.Config.CredentialsMode == types.ManualCredentialsMode + if installConfig.Config.Platform.GCP != nil && installConfig.Config.GCP.IsWIFEnabled() { + skipCloudCreds = false + } + if !skipCloudCreds { assetData["99_cloud-creds-secret.yaml"] = applyTemplateData(cloudCredsSecret.Files()[0].Data, templateData) } assetData["99_role-cloud-creds-secret-reader.yaml"] = applyTemplateData(roleCloudCredsSecretReader.Files()[0].Data, templateData) @@ -342,3 +360,58 @@ func (o *Openshift) Load(f asset.FileFetcher) (bool, error) { asset.SortFiles(o.FileList) return len(o.FileList) > 0, nil } + +func generateGCPExternalAccountJSON(ctx context.Context, ic *installconfig.InstallConfig, infraID string) ([]byte, error) { + projectID := ic.Config.GCP.ProjectID + wif := ic.Config.GCP.WorkloadIdentityFederation + + poolID := wif.PoolID + providerID := wif.ProviderID + if poolID == "" { + poolID = fmt.Sprintf("%s-wif-pool", infraID) + } + if providerID == "" { + providerID = fmt.Sprintf("%s-oidc-provider", infraID) + } + + opts := []option.ClientOption{} + if gcptypes.ShouldUseEndpointForInstaller(ic.Config.GCP.Endpoint) { + opts = append(opts, gcp.CreateEndpointOption(ic.Config.GCP.Endpoint.Name, gcp.ServiceNameGCPCloudResource)) + } + crmSvc, err := gcp.GetCloudResourceService(ctx, opts...) + if err != nil { + return nil, fmt.Errorf("failed to create resource manager service: %w", err) + } + + project, err := crmSvc.Projects.Get(fmt.Sprintf("projects/%s", projectID)).Context(ctx).Do() + if err != nil { + return nil, fmt.Errorf("failed to get project number for %s: %w", projectID, err) + } + projectNumber := "" + if len(project.Name) > 9 { + projectNumber = project.Name[9:] + } + if projectNumber == "" { + return nil, fmt.Errorf("unexpected project name format: %s", project.Name) + } + + audience := fmt.Sprintf( + "//iam.googleapis.com/projects/%s/locations/global/workloadIdentityPools/%s/providers/%s", + projectNumber, poolID, providerID, + ) + + externalAccount := map[string]any{ + "type": "external_account", + "audience": audience, + "subject_token_type": "urn:ietf:params:oauth:token-type:jwt", + "token_url": "https://sts.googleapis.com/v1/token", + "credential_source": map[string]any{ + "file": "/var/run/secrets/openshift/serviceaccount/token", + "format": map[string]string{ + "type": "text", + }, + }, + } + + return json.Marshal(externalAccount) +} diff --git a/pkg/asset/manifests/template.go b/pkg/asset/manifests/template.go index 51179f1a7c9..dfcd9b102ba 100644 --- a/pkg/asset/manifests/template.go +++ b/pkg/asset/manifests/template.go @@ -27,6 +27,11 @@ type GCPCredsSecretData struct { Base64encodeServiceAccount string } +// GCPWIFCredsSecretData holds encoded WIF external_account credentials. +type GCPWIFCredsSecretData struct { + Base64encodeExternalAccountJSON string +} + // IBMCloudCredsSecretData holds encoded credentials and is used to generate cloud-creds secret type IBMCloudCredsSecretData struct { Base64encodeAPIKey string @@ -59,6 +64,7 @@ type cloudCredsSecretData struct { AWS *AwsCredsSecretData Azure *AzureCredsSecretData GCP *GCPCredsSecretData + GCPWIF *GCPWIFCredsSecretData IBMCloud *IBMCloudCredsSecretData OpenStack *OpenStackCredsSecretData VSphere *[]*VSphereCredsSecretData diff --git a/pkg/destroy/gcp/gcp.go b/pkg/destroy/gcp/gcp.go index 6a60bc46233..c720f8841a2 100644 --- a/pkg/destroy/gcp/gcp.go +++ b/pkg/destroy/gcp/gcp.go @@ -80,6 +80,9 @@ type ClusterUninstaller struct { firewallRulesManagement gcptypes.FirewallRulesManagementPolicy + wifEnabled bool + wifBYO bool + errorTracker requestIDTracker pendingItemTracker @@ -106,6 +109,10 @@ func New(logger logrus.FieldLogger, metadata *types.ClusterMetadata) (providers. pendingItemTracker: newPendingItemTracker(), endpoint: metadata.ClusterPlatformMetadata.GCP.Endpoint, firewallRulesManagement: metadata.ClusterPlatformMetadata.GCP.FirewallRulesManagement, + wifEnabled: metadata.ClusterPlatformMetadata.GCP.WorkloadIdentityFederation != nil, + wifBYO: metadata.ClusterPlatformMetadata.GCP.WorkloadIdentityFederation != nil && + metadata.ClusterPlatformMetadata.GCP.WorkloadIdentityFederation.PoolID != "" && + metadata.ClusterPlatformMetadata.GCP.WorkloadIdentityFederation.ProviderID != "", }, nil } @@ -203,6 +210,7 @@ func (o *ClusterUninstaller) destroyCluster() (bool, error) { }, { {name: "Instances", execute: o.destroyInstances}, {name: "Disks", execute: o.destroyDisks}, + {name: "WIF Providers", execute: o.destroyWIFProviders}, {name: "Service accounts", execute: o.destroyServiceAccounts}, {name: "Images", execute: o.destroyImages}, {name: "DNS", execute: o.destroyDNS}, @@ -221,6 +229,7 @@ func (o *ClusterUninstaller) destroyCluster() (bool, error) { {name: "Subnetworks", execute: o.destroySubnetworks}, {name: "Networks", execute: o.destroyNetworks}, {name: "Filestores", execute: o.destroyFilestores}, + {name: "WIF Pools", execute: o.destroyWIFPools}, }} // create the main Context, so all stages can accept and make context children diff --git a/pkg/destroy/gcp/wif.go b/pkg/destroy/gcp/wif.go new file mode 100644 index 00000000000..5464f9de111 --- /dev/null +++ b/pkg/destroy/gcp/wif.go @@ -0,0 +1,146 @@ +package gcp + +import ( + "context" + "fmt" + "strings" + + "github.com/pkg/errors" +) + +func (o *ClusterUninstaller) listWIFProviders(ctx context.Context) ([]cloudResource, error) { + if !o.wifEnabled || o.wifBYO { + return nil, nil + } + + o.Logger.Debugf("Listing WIF providers") + result := []cloudResource{} + + parent := fmt.Sprintf("projects/%s/locations/global/workloadIdentityPools/%s-wif-pool", o.ProjectID, o.ClusterID) + resp, err := o.iamSvc.Projects.Locations.WorkloadIdentityPools.Providers.List(parent).Context(ctx).Do() + if err != nil { + if isNoOp(err) { + return nil, nil + } + return nil, fmt.Errorf("failed to list WIF providers: %w", err) + } + + for _, provider := range resp.WorkloadIdentityPoolProviders { + if provider.State == "DELETED" { + continue + } + o.Logger.Debugf("Found WIF provider: %s", provider.Name) + result = append(result, cloudResource{ + key: provider.Name, + name: provider.Name, + typeName: "wifprovider", + }) + } + return result, nil +} + +func (o *ClusterUninstaller) deleteWIFProvider(ctx context.Context, item cloudResource) error { + o.Logger.Debugf("Deleting WIF provider %s", item.name) + ctx, cancel := context.WithTimeout(ctx, defaultTimeout) + defer cancel() + + op, err := o.iamSvc.Projects.Locations.WorkloadIdentityPools.Providers.Delete(item.name).Context(ctx).Do() + if err != nil && !isNoOp(err) { + return errors.Wrapf(err, "failed to delete WIF provider %s", item.name) + } + if op != nil && !op.Done { + o.Logger.Debugf("Waiting for WIF provider deletion: %s", op.Name) + } + o.deletePendingItems(item.typeName, []cloudResource{item}) + o.Logger.Infof("Deleted WIF provider %s", item.name) + return nil +} + +func (o *ClusterUninstaller) destroyWIFProviders(ctx context.Context) error { + if !o.wifEnabled || o.wifBYO { + return nil + } + found, err := o.listWIFProviders(ctx) + if err != nil { + return err + } + items := o.insertPendingItems("wifprovider", found) + for _, item := range items { + if err := o.deleteWIFProvider(ctx, item); err != nil { + o.errorTracker.suppressWarning(item.key, err, o.Logger) + } + } + if items = o.getPendingItems("wifprovider"); len(items) > 0 { + return errors.Errorf("%d items pending", len(items)) + } + return nil +} + +func (o *ClusterUninstaller) listWIFPools(ctx context.Context) ([]cloudResource, error) { + if !o.wifEnabled || o.wifBYO { + return nil, nil + } + + o.Logger.Debugf("Listing WIF pools") + result := []cloudResource{} + + parent := fmt.Sprintf("projects/%s/locations/global", o.ProjectID) + resp, err := o.iamSvc.Projects.Locations.WorkloadIdentityPools.List(parent).Context(ctx).Do() + if err != nil { + return nil, fmt.Errorf("failed to list WIF pools: %w", err) + } + + expectedPrefix := fmt.Sprintf("%s-wif-pool", o.ClusterID) + for _, pool := range resp.WorkloadIdentityPools { + if pool.State == "DELETED" { + continue + } + poolID := pool.Name[strings.LastIndex(pool.Name, "/")+1:] + if poolID == expectedPrefix { + o.Logger.Debugf("Found WIF pool: %s", pool.Name) + result = append(result, cloudResource{ + key: pool.Name, + name: pool.Name, + typeName: "wifpool", + }) + } + } + return result, nil +} + +func (o *ClusterUninstaller) deleteWIFPool(ctx context.Context, item cloudResource) error { + o.Logger.Debugf("Deleting WIF pool %s", item.name) + ctx, cancel := context.WithTimeout(ctx, defaultTimeout) + defer cancel() + + op, err := o.iamSvc.Projects.Locations.WorkloadIdentityPools.Delete(item.name).Context(ctx).Do() + if err != nil && !isNoOp(err) { + return errors.Wrapf(err, "failed to delete WIF pool %s", item.name) + } + if op != nil && !op.Done { + o.Logger.Debugf("Waiting for WIF pool deletion: %s", op.Name) + } + o.deletePendingItems(item.typeName, []cloudResource{item}) + o.Logger.Infof("Deleted WIF pool %s", item.name) + return nil +} + +func (o *ClusterUninstaller) destroyWIFPools(ctx context.Context) error { + if !o.wifEnabled || o.wifBYO { + return nil + } + found, err := o.listWIFPools(ctx) + if err != nil { + return err + } + items := o.insertPendingItems("wifpool", found) + for _, item := range items { + if err := o.deleteWIFPool(ctx, item); err != nil { + o.errorTracker.suppressWarning(item.key, err, o.Logger) + } + } + if items = o.getPendingItems("wifpool"); len(items) > 0 { + return errors.Errorf("%d items pending", len(items)) + } + return nil +} diff --git a/pkg/infrastructure/gcp/clusterapi/clusterapi.go b/pkg/infrastructure/gcp/clusterapi/clusterapi.go index f677c0f7d14..02031378650 100644 --- a/pkg/infrastructure/gcp/clusterapi/clusterapi.go +++ b/pkg/infrastructure/gcp/clusterapi/clusterapi.go @@ -102,6 +102,12 @@ func (p Provider) PreProvision(ctx context.Context, in clusterapi.PreProvisionIn } } + if platform.IsWIFEnabled() && !platform.IsWIFBYO() { + if err := ProvisionWIF(ctx, in); err != nil { + return fmt.Errorf("failed to provision WIF resources: %w", err) + } + } + return nil } diff --git a/pkg/infrastructure/gcp/clusterapi/wif.go b/pkg/infrastructure/gcp/clusterapi/wif.go new file mode 100644 index 00000000000..f258c4c37d0 --- /dev/null +++ b/pkg/infrastructure/gcp/clusterapi/wif.go @@ -0,0 +1,389 @@ +package clusterapi + +import ( + "context" + "crypto/rsa" + "crypto/sha256" + "crypto/x509" + "encoding/base64" + "encoding/json" + "encoding/pem" + "fmt" + "math/big" + "slices" + "time" + + "cloud.google.com/go/storage" + "github.com/sirupsen/logrus" + resourcemanager "google.golang.org/api/cloudresourcemanager/v3" + "google.golang.org/api/iam/v1" + "google.golang.org/api/option" + "k8s.io/apimachinery/pkg/util/wait" + + icgcp "github.com/openshift/installer/pkg/asset/installconfig/gcp" + "github.com/openshift/installer/pkg/infrastructure/clusterapi" + gcptypes "github.com/openshift/installer/pkg/types/gcp" +) + +const ( + wifRetryTime = 10 * time.Second + wifRetryCount = 12 +) + +// WIFPoolName returns the deterministic WIF pool ID for a cluster. +func WIFPoolName(infraID string) string { + return fmt.Sprintf("%s-wif-pool", infraID) +} + +// WIFProviderName returns the deterministic WIF OIDC provider ID for a cluster. +func WIFProviderName(infraID string) string { + return fmt.Sprintf("%s-oidc-provider", infraID) +} + +// OIDCBucketName returns the deterministic GCS bucket name for OIDC discovery. +func OIDCBucketName(infraID string) string { + return fmt.Sprintf("%s-oidc", infraID) +} + +// OIDCIssuerURL returns the deterministic OIDC issuer URL for a cluster. +func OIDCIssuerURL(infraID string) string { + return fmt.Sprintf("https://storage.googleapis.com/%s", OIDCBucketName(infraID)) +} + +// ProvisionWIF creates WIF infrastructure for installer-provisioned mode. +func ProvisionWIF(ctx context.Context, in clusterapi.PreProvisionInput) error { + platform := in.InstallConfig.Config.Platform.GCP + projectID := platform.ProjectID + infraID := in.InfraID + region := platform.Region + + iamOpts := []option.ClientOption{} + storageOpts := []option.ClientOption{} + crmOpts := []option.ClientOption{} + if gcptypes.ShouldUseEndpointForInstaller(platform.Endpoint) { + iamOpts = append(iamOpts, icgcp.CreateEndpointOption(platform.Endpoint.Name, icgcp.ServiceNameGCPIAM)) + storageOpts = append(storageOpts, icgcp.CreateEndpointOption(platform.Endpoint.Name, icgcp.ServiceNameGCPStorage)) + crmOpts = append(crmOpts, icgcp.CreateEndpointOption(platform.Endpoint.Name, icgcp.ServiceNameGCPCloudResource)) + } + + iamSvc, err := icgcp.GetIAMService(ctx, iamOpts...) + if err != nil { + return fmt.Errorf("failed to create IAM service: %w", err) + } + + storageClient, err := icgcp.GetStorageService(ctx, storageOpts...) + if err != nil { + return fmt.Errorf("failed to create storage client: %w", err) + } + + crmSvc, err := icgcp.GetCloudResourceService(ctx, crmOpts...) + if err != nil { + return fmt.Errorf("failed to create resource manager service: %w", err) + } + + projectNumber, err := GetProjectNumber(ctx, crmSvc, projectID) + if err != nil { + return fmt.Errorf("failed to get project number: %w", err) + } + + poolName := WIFPoolName(infraID) + providerName := WIFProviderName(infraID) + issuerURL := OIDCIssuerURL(infraID) + + logrus.Infof("Creating workload identity pool %s", poolName) + if err := createWorkloadIdentityPool(ctx, iamSvc, projectID, infraID, poolName); err != nil { + return fmt.Errorf("failed to create workload identity pool: %w", err) + } + + logrus.Infof("Creating OIDC discovery bucket %s", OIDCBucketName(infraID)) + if err := createOIDCBucket(ctx, storageClient, projectID, region, infraID, issuerURL); err != nil { + return fmt.Errorf("failed to create OIDC bucket: %w", err) + } + + logrus.Infof("Creating OIDC provider %s", providerName) + if err := createOIDCProvider(ctx, iamSvc, projectID, infraID, poolName, providerName, issuerURL); err != nil { + return fmt.Errorf("failed to create OIDC provider: %w", err) + } + + logrus.Infof("Binding service accounts to workload identity pool") + masterSAEmail := gcptypes.GetDefaultServiceAccount(platform, infraID, "master") + workerSAEmail := gcptypes.GetDefaultServiceAccount(platform, infraID, "worker") + + bindings := []struct { + k8sNamespace string + k8sSAName string + gcpSAEmail string + }{ + {"openshift-cloud-credential-operator", "cloud-credential-operator", masterSAEmail}, + {"openshift-image-registry", "cluster-image-registry-operator", masterSAEmail}, + {"openshift-ingress-operator", "ingress-operator", masterSAEmail}, + {"openshift-machine-api", "machine-api-controllers", masterSAEmail}, + {"openshift-cloud-controller-manager", "cloud-controller-manager", masterSAEmail}, + {"openshift-cluster-csi-drivers", "gcp-pd-csi-driver-operator", workerSAEmail}, + } + + for _, b := range bindings { + if err := bindServiceAccountToWIF(ctx, iamSvc, projectID, projectNumber, poolName, b.k8sNamespace, b.k8sSAName, b.gcpSAEmail); err != nil { + return fmt.Errorf("failed to bind SA %s/%s to %s: %w", b.k8sNamespace, b.k8sSAName, b.gcpSAEmail, err) + } + } + + logrus.Infof("WIF provisioning complete") + return nil +} + +// GetProjectNumber resolves a project ID to its numeric project number. +func GetProjectNumber(ctx context.Context, crmSvc *resourcemanager.Service, projectID string) (string, error) { + ctx, cancel := context.WithTimeout(ctx, time.Minute) + defer cancel() + + project, err := crmSvc.Projects.Get(fmt.Sprintf("projects/%s", projectID)).Context(ctx).Do() + if err != nil { + return "", fmt.Errorf("failed to get project %s: %w", projectID, err) + } + + // project.Name is "projects/{number}" + if len(project.Name) > 9 { + return project.Name[9:], nil + } + return "", fmt.Errorf("unexpected project name format: %s", project.Name) +} + +// GetBYOIssuerURL describes an existing WIF provider and returns its OIDC issuer URL. +func GetBYOIssuerURL(ctx context.Context, iamSvc *iam.Service, projectID, poolID, providerID string) (string, error) { + ctx, cancel := context.WithTimeout(ctx, time.Minute) + defer cancel() + + name := fmt.Sprintf("projects/%s/locations/global/workloadIdentityPools/%s/providers/%s", + projectID, poolID, providerID) + provider, err := iamSvc.Projects.Locations.WorkloadIdentityPools.Providers.Get(name).Context(ctx).Do() + if err != nil { + return "", fmt.Errorf("failed to get WIF provider %s: %w", name, err) + } + if provider.Oidc == nil || provider.Oidc.IssuerUri == "" { + return "", fmt.Errorf("WIF provider %s does not have an OIDC issuer URI", name) + } + return provider.Oidc.IssuerUri, nil +} + +func createWorkloadIdentityPool(ctx context.Context, iamSvc *iam.Service, projectID, infraID, poolName string) error { + ctx, cancel := context.WithTimeout(ctx, 2*time.Minute) + defer cancel() + + parent := fmt.Sprintf("projects/%s/locations/global", projectID) + pool := &iam.WorkloadIdentityPool{ + DisplayName: fmt.Sprintf("%s WIF Pool", infraID), + Description: "Created by OpenShift Installer", + } + + op, err := iamSvc.Projects.Locations.WorkloadIdentityPools.Create(parent, pool). + WorkloadIdentityPoolId(poolName).Context(ctx).Do() + if err != nil { + return fmt.Errorf("failed to create WIF pool: %w", err) + } + + return waitForIAMOperation(ctx, iamSvc, op.Name) +} + +func createOIDCBucket(ctx context.Context, storageClient *storage.Client, projectID, region, infraID, issuerURL string) error { + bucketName := OIDCBucketName(infraID) + bucket := storageClient.Bucket(bucketName) + + if err := bucket.Create(ctx, projectID, &storage.BucketAttrs{ + Location: region, + UniformBucketLevelAccess: storage.UniformBucketLevelAccess{Enabled: true}, + Labels: map[string]string{"kubernetes-io-cluster-" + infraID: "owned"}, + }); err != nil { + return fmt.Errorf("failed to create OIDC bucket %s: %w", bucketName, err) + } + + policy, err := bucket.IAM().Policy(ctx) + if err != nil { + return fmt.Errorf("failed to get OIDC bucket IAM policy: %w", err) + } + policy.Add("allUsers", "roles/storage.objectViewer") + if err := bucket.IAM().SetPolicy(ctx, policy); err != nil { + return fmt.Errorf("failed to set OIDC bucket IAM policy: %w", err) + } + + discoveryDoc := generateOIDCDiscoveryDoc(issuerURL) + wtr := bucket.Object(".well-known/openid-configuration").NewWriter(ctx) + wtr.ContentType = "application/json" + if _, err := wtr.Write(discoveryDoc); err != nil { + return fmt.Errorf("failed to write OIDC discovery doc: %w", err) + } + if err := wtr.Close(); err != nil { + return fmt.Errorf("failed to close OIDC discovery doc writer: %w", err) + } + + return nil +} + +func createOIDCProvider(ctx context.Context, iamSvc *iam.Service, projectID, infraID, poolName, providerName, issuerURL string) error { + ctx, cancel := context.WithTimeout(ctx, 2*time.Minute) + defer cancel() + + parent := fmt.Sprintf("projects/%s/locations/global/workloadIdentityPools/%s", projectID, poolName) + audience := fmt.Sprintf("//iam.googleapis.com/%s/providers/%s", parent, providerName) + + provider := &iam.WorkloadIdentityPoolProvider{ + DisplayName: fmt.Sprintf("%s OIDC Provider", infraID), + Description: "Created by OpenShift Installer", + Oidc: &iam.Oidc{ + IssuerUri: issuerURL, + AllowedAudiences: []string{audience}, + }, + AttributeMapping: map[string]string{ + "google.subject": "assertion.sub", + }, + } + + op, err := iamSvc.Projects.Locations.WorkloadIdentityPools.Providers.Create(parent, provider). + WorkloadIdentityPoolProviderId(providerName).Context(ctx).Do() + if err != nil { + return fmt.Errorf("failed to create OIDC provider: %w", err) + } + + return waitForIAMOperation(ctx, iamSvc, op.Name) +} + +func bindServiceAccountToWIF(ctx context.Context, iamSvc *iam.Service, projectID, projectNumber, poolName, k8sNamespace, k8sSAName, gcpSAEmail string) error { + ctx, cancel := context.WithTimeout(ctx, time.Minute) + defer cancel() + + saResource := fmt.Sprintf("projects/%s/serviceAccounts/%s", projectID, gcpSAEmail) + member := fmt.Sprintf( + "principal://iam.googleapis.com/projects/%s/locations/global/workloadIdentityPools/%s/subject/system:serviceaccount:%s:%s", + projectNumber, poolName, k8sNamespace, k8sSAName, + ) + + policy, err := iamSvc.Projects.ServiceAccounts.GetIamPolicy(saResource).Context(ctx).Do() + if err != nil { + return fmt.Errorf("failed to get SA IAM policy for %s: %w", gcpSAEmail, err) + } + + role := "roles/iam.workloadIdentityUser" + var binding *iam.Binding + for _, b := range policy.Bindings { + if b.Role == role { + if slices.Contains(b.Members, member) { + logrus.Debugf("WIF binding already exists for %s/%s on %s", k8sNamespace, k8sSAName, gcpSAEmail) + return nil + } + binding = b + break + } + } + + if binding == nil { + binding = &iam.Binding{ + Role: role, + Members: []string{member}, + } + policy.Bindings = append(policy.Bindings, binding) + } else { + binding.Members = append(binding.Members, member) + } + + _, err = iamSvc.Projects.ServiceAccounts.SetIamPolicy(saResource, &iam.SetIamPolicyRequest{ + Policy: policy, + }).Context(ctx).Do() + if err != nil { + return fmt.Errorf("failed to set SA IAM policy for %s: %w", gcpSAEmail, err) + } + + logrus.Debugf("Bound %s/%s to %s via WIF", k8sNamespace, k8sSAName, gcpSAEmail) + return nil +} + +func waitForIAMOperation(ctx context.Context, iamSvc *iam.Service, opName string) error { + backoff := wait.Backoff{ + Duration: wifRetryTime, + Factor: 1.5, + Jitter: 0.1, + Steps: wifRetryCount, + } + + var lastErr error + if waitErr := wait.ExponentialBackoffWithContext(ctx, backoff, func(ctx context.Context) (bool, error) { + op, err := iamSvc.Projects.Locations.WorkloadIdentityPools.Operations.Get(opName).Context(ctx).Do() + if err != nil { + lastErr = err + return false, nil + } + if op.Done { + if op.Error != nil { + return false, fmt.Errorf("operation %s failed: %s", opName, op.Error.Message) + } + return true, nil + } + return false, nil + }); waitErr != nil { + if wait.Interrupted(waitErr) { + return fmt.Errorf("timed out waiting for operation %s: %w", opName, lastErr) + } + return waitErr + } + return nil +} + +func generateOIDCDiscoveryDoc(issuerURL string) []byte { + doc := map[string]any{ + "issuer": issuerURL, + "jwks_uri": fmt.Sprintf("%s/keys.json", issuerURL), + "response_types_supported": []string{"id_token"}, + "subject_types_supported": []string{"public"}, + "id_token_signing_alg_values_supported": []string{"RS256"}, + } + data, _ := json.MarshalIndent(doc, "", " ") + return data +} + +// GenerateJWKS produces a JSON Web Key Set from an RSA public key PEM. +func GenerateJWKS(publicKeyPEM []byte) ([]byte, error) { + block, _ := pem.Decode(publicKeyPEM) + if block == nil { + return nil, fmt.Errorf("failed to decode PEM block") + } + + pub, err := x509.ParsePKIXPublicKey(block.Bytes) + if err != nil { + return nil, fmt.Errorf("failed to parse public key: %w", err) + } + + rsaPub, ok := pub.(*rsa.PublicKey) + if !ok { + return nil, fmt.Errorf("public key is not RSA") + } + + // Compute KID as base64url(SHA256(DER-encoded public key)) + derBytes, err := x509.MarshalPKIXPublicKey(rsaPub) + if err != nil { + return nil, fmt.Errorf("failed to marshal public key: %w", err) + } + hash := sha256.Sum256(derBytes) + kid := base64.RawURLEncoding.EncodeToString(hash[:]) + + jwks := map[string]any{ + "keys": []map[string]string{ + { + "kty": "RSA", + "alg": "RS256", + "use": "sig", + "kid": kid, + "n": base64.RawURLEncoding.EncodeToString(rsaPub.N.Bytes()), + "e": base64.RawURLEncoding.EncodeToString(big.NewInt(int64(rsaPub.E)).Bytes()), + }, + }, + } + + return json.Marshal(jwks) +} + +// BuildAudienceURI constructs the WIF audience URI from project number, pool ID, and provider ID. +func BuildAudienceURI(projectNumber, poolID, providerID string) string { + return fmt.Sprintf( + "//iam.googleapis.com/projects/%s/locations/global/workloadIdentityPools/%s/providers/%s", + projectNumber, poolID, providerID, + ) +} diff --git a/pkg/infrastructure/gcp/clusterapi/wif_test.go b/pkg/infrastructure/gcp/clusterapi/wif_test.go new file mode 100644 index 00000000000..d7fbe743cfd --- /dev/null +++ b/pkg/infrastructure/gcp/clusterapi/wif_test.go @@ -0,0 +1,79 @@ +package clusterapi + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestWIFPoolName(t *testing.T) { + assert.Equal(t, "test-abc-wif-pool", WIFPoolName("test-abc")) +} + +func TestWIFProviderName(t *testing.T) { + assert.Equal(t, "test-abc-oidc-provider", WIFProviderName("test-abc")) +} + +func TestOIDCBucketName(t *testing.T) { + assert.Equal(t, "test-abc-oidc", OIDCBucketName("test-abc")) +} + +func TestOIDCIssuerURL(t *testing.T) { + assert.Equal(t, "https://storage.googleapis.com/test-abc-oidc", OIDCIssuerURL("test-abc")) +} + +func TestBuildAudienceURI(t *testing.T) { + uri := BuildAudienceURI("123456789", "my-pool", "my-provider") + expected := "//iam.googleapis.com/projects/123456789/locations/global/workloadIdentityPools/my-pool/providers/my-provider" + assert.Equal(t, expected, uri) +} + +func TestGenerateOIDCDiscoveryDoc(t *testing.T) { + issuerURL := "https://storage.googleapis.com/test-oidc" + doc := generateOIDCDiscoveryDoc(issuerURL) + + var parsed map[string]any + err := json.Unmarshal(doc, &parsed) + assert.NoError(t, err) + + assert.Equal(t, issuerURL, parsed["issuer"]) + assert.Equal(t, issuerURL+"/keys.json", parsed["jwks_uri"]) +} + +func TestGenerateJWKS(t *testing.T) { + // Generate a fresh RSA key pair for testing + testPEM := []byte("-----BEGIN PUBLIC KEY-----\n" + + "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAqyJvLlIIFOzh7gXbWFjC\n" + + "8yAPEWXte+F1+nZa7k0XetvlSC1X6z6hnGqdUJCNh4UDMwhnfeDO4f8fsKx2gtEU\n" + + "GJ0mruweWuOeBJguhR1DI5LitCOadh660lKc1PqNCgNVGvkInHi2IdZSAnBcPJTp\n" + + "MuNQ+5+tbmJ+a8y2GMtxfEVIZj/AQ5mIJ+ItjDp4x10ePvC/THVRDIwYf96jNhnq\n" + + "ijPsEE++qYchLV7aOuJ4s7F4DkJFEFh0pH1POltn9aiXBdsKDCk0/fmQqjl8p5n2\n" + + "5hnS6EHdzdtgurZkxpF4wcHcGpchBxHWaRC9RXVbjHI0qdery6AtdjUjZT8UBxdN\n" + + "GwIDAQAB\n" + + "-----END PUBLIC KEY-----\n") + + jwksBytes, err := GenerateJWKS(testPEM) + if !assert.NoError(t, err) { + return + } + + var jwks map[string]any + err = json.Unmarshal(jwksBytes, &jwks) + if !assert.NoError(t, err) { + return + } + + keys, ok := jwks["keys"].([]any) + if !assert.True(t, ok) || !assert.Len(t, keys, 1) { + return + } + + key := keys[0].(map[string]any) + assert.Equal(t, "RSA", key["kty"]) + assert.Equal(t, "RS256", key["alg"]) + assert.Equal(t, "sig", key["use"]) + assert.NotEmpty(t, key["kid"]) + assert.NotEmpty(t, key["n"]) + assert.NotEmpty(t, key["e"]) +} diff --git a/pkg/types/defaults/installconfig.go b/pkg/types/defaults/installconfig.go index 6e88ba5aba7..d4d10ebb6d8 100644 --- a/pkg/types/defaults/installconfig.go +++ b/pkg/types/defaults/installconfig.go @@ -98,6 +98,8 @@ func SetInstallConfigDefaults(c *types.InstallConfig) { c.CredentialsMode = types.ManualCredentialsMode } else if c.Platform.PowerVS != nil { c.CredentialsMode = types.ManualCredentialsMode + } else if c.Platform.GCP != nil && c.Platform.GCP.IsWIFEnabled() { + c.CredentialsMode = types.ManualCredentialsMode } } diff --git a/pkg/types/gcp/metadata.go b/pkg/types/gcp/metadata.go index dc33be89d2c..5c198803423 100644 --- a/pkg/types/gcp/metadata.go +++ b/pkg/types/gcp/metadata.go @@ -2,11 +2,12 @@ package gcp // Metadata contains GCP metadata (e.g. for uninstalling the cluster). type Metadata struct { - Region string `json:"region"` - ProjectID string `json:"projectID"` - NetworkProjectID string `json:"networkProjectID,omitempty"` - PrivateZoneDomain string `json:"privateZoneDomain,omitempty"` - PrivateZoneProjectID string `json:"privateZoneProjectID,omitempty"` - Endpoint *PSCEndpoint `json:"endpoint,omitempty"` - FirewallRulesManagement FirewallRulesManagementPolicy `json:"firewallRulesManagement,omitempty"` + Region string `json:"region"` + ProjectID string `json:"projectID"` + NetworkProjectID string `json:"networkProjectID,omitempty"` + PrivateZoneDomain string `json:"privateZoneDomain,omitempty"` + PrivateZoneProjectID string `json:"privateZoneProjectID,omitempty"` + Endpoint *PSCEndpoint `json:"endpoint,omitempty"` + FirewallRulesManagement FirewallRulesManagementPolicy `json:"firewallRulesManagement,omitempty"` + WorkloadIdentityFederation *WorkloadIdentityFederation `json:"workloadIdentityFederation,omitempty"` } diff --git a/pkg/types/gcp/platform.go b/pkg/types/gcp/platform.go index 7fbb9ca53cc..4bd0849b68b 100644 --- a/pkg/types/gcp/platform.go +++ b/pkg/types/gcp/platform.go @@ -54,6 +54,21 @@ type PSCEndpoint struct { ClusterUseOnly *bool `json:"clusterUseOnly,omitempty"` } +// WorkloadIdentityFederation configures GCP Workload Identity Federation +// for short-lived token authentication. +type WorkloadIdentityFederation struct { + // PoolID is the ID of an existing workload identity pool. + // When both poolID and providerID are provided, the installer uses the + // existing WIF resources (BYO mode). When both are omitted, the installer + // creates new WIF resources. + // +optional + PoolID string `json:"poolID,omitempty"` + + // ProviderID is the ID of an existing OIDC provider within the pool. + // +optional + ProviderID string `json:"providerID,omitempty"` +} + // Platform stores all the global configuration that all machinesets // use. type Platform struct { @@ -126,6 +141,12 @@ type Platform struct { // and the firewall rules before the installation. // +optional FirewallRulesManagement FirewallRulesManagementPolicy `json:"firewallRulesManagement,omitempty"` + + // WorkloadIdentityFederation configures GCP Workload Identity Federation + // for short-lived token authentication. When set, the installer generates + // external_account credential manifests instead of static service account keys. + // +optional + WorkloadIdentityFederation *WorkloadIdentityFederation `json:"workloadIdentityFederation,omitempty"` } // UserLabel is a label to apply to GCP resources created for the cluster. @@ -193,3 +214,15 @@ func GetDefaultServiceAccount(platform *Platform, clusterID string, role string) func ShouldUseEndpointForInstaller(endpoint *PSCEndpoint) bool { return endpoint != nil && endpoint.ClusterUseOnly != nil && !(*endpoint.ClusterUseOnly) } + +// IsWIFEnabled returns true when Workload Identity Federation is configured. +func (p *Platform) IsWIFEnabled() bool { + return p != nil && p.WorkloadIdentityFederation != nil +} + +// IsWIFBYO returns true when the customer is providing their own WIF resources. +func (p *Platform) IsWIFBYO() bool { + return p.IsWIFEnabled() && + p.WorkloadIdentityFederation.PoolID != "" && + p.WorkloadIdentityFederation.ProviderID != "" +} diff --git a/pkg/types/gcp/validation/platform.go b/pkg/types/gcp/validation/platform.go index 6ad20026b68..e6c5a6e9aed 100644 --- a/pkg/types/gcp/validation/platform.go +++ b/pkg/types/gcp/validation/platform.go @@ -78,6 +78,10 @@ var ( // userLabelKeyPrefixRegex is for verifying that the label key does not contain restricted prefixes. userLabelKeyPrefixRegex = regexp.MustCompile(`^(?i)(kubernetes\-io|openshift\-io)`) + + // wifResourceIDRegex validates WIF pool and provider IDs. + // IDs must be 4-32 lowercase alphanumeric characters or hyphens, starting with a letter. + wifResourceIDRegex = regexp.MustCompile(`^[a-z][a-z0-9-]{3,31}$`) ) const ( @@ -145,6 +149,8 @@ func ValidatePlatform(p *gcp.Platform, fldPath *field.Path, ic *types.InstallCon } } + allErrs = append(allErrs, validateWorkloadIdentityFederation(p.WorkloadIdentityFederation, fldPath.Child("workloadIdentityFederation"), ic)...) + if p.FirewallRulesManagement != "" { supportedFirewallRulePolicies := sets.New(gcp.ManagedFirewallRules, gcp.UnmanagedFirewallRules) if !supportedFirewallRulePolicies.Has(p.FirewallRulesManagement) { @@ -155,6 +161,47 @@ func ValidatePlatform(p *gcp.Platform, fldPath *field.Path, ic *types.InstallCon return allErrs } +// validateWorkloadIdentityFederation validates WIF configuration. +func validateWorkloadIdentityFederation(wif *gcp.WorkloadIdentityFederation, fldPath *field.Path, ic *types.InstallConfig) field.ErrorList { + allErrs := field.ErrorList{} + if wif == nil { + return allErrs + } + + hasPool := wif.PoolID != "" + hasProvider := wif.ProviderID != "" + if hasPool != hasProvider { + allErrs = append(allErrs, field.Required(fldPath, "both poolID and providerID must be specified together, or both must be omitted")) + return allErrs + } + + if hasPool { + allErrs = append(allErrs, validateWIFResourceID(wif.PoolID, fldPath.Child("poolID"))...) + allErrs = append(allErrs, validateWIFResourceID(wif.ProviderID, fldPath.Child("providerID"))...) + } + + if ic.CredentialsMode != "" && ic.CredentialsMode != types.ManualCredentialsMode { + allErrs = append(allErrs, field.Invalid(fldPath, wif, + fmt.Sprintf("workload identity federation requires %q credentials mode", types.ManualCredentialsMode))) + } + + return allErrs +} + +// validateWIFResourceID checks that a WIF pool or provider ID is valid. +func validateWIFResourceID(id string, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if !wifResourceIDRegex.MatchString(id) { + allErrs = append(allErrs, field.Invalid(fldPath, id, + "must be 4-32 lowercase letters, digits, or hyphens, starting with a letter")) + } + if len(id) >= 4 && id[:4] == "gcp-" { + allErrs = append(allErrs, field.Invalid(fldPath, id, + "must not start with the prefix \"gcp-\"")) + } + return allErrs +} + // validateUserLabels verifies if configured number of UserLabels is not more than // allowed limit and the label keys and values are valid. func validateUserLabels(labels []gcp.UserLabel, fldPath *field.Path) field.ErrorList { diff --git a/pkg/types/gcp/validation/platform_test.go b/pkg/types/gcp/validation/platform_test.go index 589812c5112..90db1307670 100644 --- a/pkg/types/gcp/validation/platform_test.go +++ b/pkg/types/gcp/validation/platform_test.go @@ -242,6 +242,82 @@ func TestValidatePlatform(t *testing.T) { }, valid: false, }, + { + name: "valid WIF BYO", + platform: &gcp.Platform{ + Region: "us-east1", + WorkloadIdentityFederation: &gcp.WorkloadIdentityFederation{ + PoolID: "my-wif-pool", + ProviderID: "my-oidc-provider", + }, + }, + credentialsMode: types.ManualCredentialsMode, + valid: true, + }, + { + name: "valid WIF installer-provisioned", + platform: &gcp.Platform{ + Region: "us-east1", + WorkloadIdentityFederation: &gcp.WorkloadIdentityFederation{}, + }, + credentialsMode: types.ManualCredentialsMode, + valid: true, + }, + { + name: "invalid WIF partial - only poolID", + platform: &gcp.Platform{ + Region: "us-east1", + WorkloadIdentityFederation: &gcp.WorkloadIdentityFederation{ + PoolID: "my-wif-pool", + }, + }, + credentialsMode: types.ManualCredentialsMode, + valid: false, + }, + { + name: "invalid WIF partial - only providerID", + platform: &gcp.Platform{ + Region: "us-east1", + WorkloadIdentityFederation: &gcp.WorkloadIdentityFederation{ + ProviderID: "my-oidc-provider", + }, + }, + credentialsMode: types.ManualCredentialsMode, + valid: false, + }, + { + name: "invalid WIF poolID format", + platform: &gcp.Platform{ + Region: "us-east1", + WorkloadIdentityFederation: &gcp.WorkloadIdentityFederation{ + PoolID: "AB", + ProviderID: "my-oidc-provider", + }, + }, + credentialsMode: types.ManualCredentialsMode, + valid: false, + }, + { + name: "invalid WIF poolID gcp- prefix", + platform: &gcp.Platform{ + Region: "us-east1", + WorkloadIdentityFederation: &gcp.WorkloadIdentityFederation{ + PoolID: "gcp-reserved-pool", + ProviderID: "my-oidc-provider", + }, + }, + credentialsMode: types.ManualCredentialsMode, + valid: false, + }, + { + name: "invalid WIF with non-Manual credentials mode", + platform: &gcp.Platform{ + Region: "us-east1", + WorkloadIdentityFederation: &gcp.WorkloadIdentityFederation{}, + }, + credentialsMode: types.MintCredentialsMode, + valid: false, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/pkg/types/gcp/zz_generated.deepcopy.go b/pkg/types/gcp/zz_generated.deepcopy.go index 7f1e0688862..d5585541022 100644 --- a/pkg/types/gcp/zz_generated.deepcopy.go +++ b/pkg/types/gcp/zz_generated.deepcopy.go @@ -135,6 +135,11 @@ func (in *Metadata) DeepCopyInto(out *Metadata) { *out = new(PSCEndpoint) (*in).DeepCopyInto(*out) } + if in.WorkloadIdentityFederation != nil { + in, out := &in.WorkloadIdentityFederation, &out.WorkloadIdentityFederation + *out = new(WorkloadIdentityFederation) + **out = **in + } return } @@ -257,6 +262,11 @@ func (in *Platform) DeepCopyInto(out *Platform) { *out = new(DNS) (*in).DeepCopyInto(*out) } + if in.WorkloadIdentityFederation != nil { + in, out := &in.WorkloadIdentityFederation, &out.WorkloadIdentityFederation + *out = new(WorkloadIdentityFederation) + **out = **in + } return } @@ -344,3 +354,19 @@ func (in *UserTag) DeepCopy() *UserTag { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *WorkloadIdentityFederation) DeepCopyInto(out *WorkloadIdentityFederation) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WorkloadIdentityFederation. +func (in *WorkloadIdentityFederation) DeepCopy() *WorkloadIdentityFederation { + if in == nil { + return nil + } + out := new(WorkloadIdentityFederation) + in.DeepCopyInto(out) + return out +} From b667db350dee8c50cf508244bd919e19f35ac648 Mon Sep 17 00:00:00 2001 From: Bruno Cesar Rocha Date: Mon, 8 Jun 2026 18:40:16 +0100 Subject: [PATCH 2/5] CORS-4470: fix error wrapping and drop unrelated Azure mock change Use fmt.Errorf with %w in destroy/gcp/wif.go instead of deprecated errors.Wrapf from github.com/pkg/errors. Remove the unrelated Azure mock cosmetic diff that was pulled in by hack/go-genmock.sh. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../installconfig/azure/mock/azureclient_generated.go | 4 ++-- pkg/destroy/gcp/wif.go | 10 ++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/pkg/asset/installconfig/azure/mock/azureclient_generated.go b/pkg/asset/installconfig/azure/mock/azureclient_generated.go index 996a85b69bf..18eb90b533c 100644 --- a/pkg/asset/installconfig/azure/mock/azureclient_generated.go +++ b/pkg/asset/installconfig/azure/mock/azureclient_generated.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: ./client.go +// Source: client.go // // Generated by this command: // -// mockgen -source=./client.go -destination=mock/azureclient_generated.go -package=mock +// mockgen -source=client.go -destination=mock/azureclient_generated.go -package=mock // // Package mock is a generated GoMock package. diff --git a/pkg/destroy/gcp/wif.go b/pkg/destroy/gcp/wif.go index 5464f9de111..28baa23c38d 100644 --- a/pkg/destroy/gcp/wif.go +++ b/pkg/destroy/gcp/wif.go @@ -4,8 +4,6 @@ import ( "context" "fmt" "strings" - - "github.com/pkg/errors" ) func (o *ClusterUninstaller) listWIFProviders(ctx context.Context) ([]cloudResource, error) { @@ -46,7 +44,7 @@ func (o *ClusterUninstaller) deleteWIFProvider(ctx context.Context, item cloudRe op, err := o.iamSvc.Projects.Locations.WorkloadIdentityPools.Providers.Delete(item.name).Context(ctx).Do() if err != nil && !isNoOp(err) { - return errors.Wrapf(err, "failed to delete WIF provider %s", item.name) + return fmt.Errorf("failed to delete WIF provider %s: %w", item.name, err) } if op != nil && !op.Done { o.Logger.Debugf("Waiting for WIF provider deletion: %s", op.Name) @@ -71,7 +69,7 @@ func (o *ClusterUninstaller) destroyWIFProviders(ctx context.Context) error { } } if items = o.getPendingItems("wifprovider"); len(items) > 0 { - return errors.Errorf("%d items pending", len(items)) + return fmt.Errorf("%d items pending", len(items)) } return nil } @@ -115,7 +113,7 @@ func (o *ClusterUninstaller) deleteWIFPool(ctx context.Context, item cloudResour op, err := o.iamSvc.Projects.Locations.WorkloadIdentityPools.Delete(item.name).Context(ctx).Do() if err != nil && !isNoOp(err) { - return errors.Wrapf(err, "failed to delete WIF pool %s", item.name) + return fmt.Errorf("failed to delete WIF pool %s: %w", item.name, err) } if op != nil && !op.Done { o.Logger.Debugf("Waiting for WIF pool deletion: %s", op.Name) @@ -140,7 +138,7 @@ func (o *ClusterUninstaller) destroyWIFPools(ctx context.Context) error { } } if items = o.getPendingItems("wifpool"); len(items) > 0 { - return errors.Errorf("%d items pending", len(items)) + return fmt.Errorf("%d items pending", len(items)) } return nil } From 6cbe217b9b3903b0c1a0312712551ef684ca4b84 Mon Sep 17 00:00:00 2001 From: Bruno Cesar Rocha Date: Tue, 9 Jun 2026 09:38:56 +0100 Subject: [PATCH 3/5] CORS-4470: require bound SA signing key for WIF provisioning Upload JWKS (keys.json) to the OIDC GCS bucket so GCP can validate projected service account tokens. The bound SA signing key must be provided by the user via bound-service-account-signing-key.key in the asset directory. Thread the BoundSASigningKey asset through PreProvisionInput so the GCP WIF provisioning flow can extract the public key and generate the JSON Web Key Set. Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/infrastructure/clusterapi/clusterapi.go | 15 +++++---- pkg/infrastructure/clusterapi/types.go | 13 ++++---- pkg/infrastructure/gcp/clusterapi/wif.go | 34 +++++++++++++++++++-- 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/pkg/infrastructure/clusterapi/clusterapi.go b/pkg/infrastructure/clusterapi/clusterapi.go index 8676de9975d..268ae7e0543 100644 --- a/pkg/infrastructure/clusterapi/clusterapi.go +++ b/pkg/infrastructure/clusterapi/clusterapi.go @@ -88,6 +88,7 @@ func (i *InfraProvider) Provision(ctx context.Context, dir string, parents asset workerIgnAsset := &machine.Worker{} tfvarsAsset := &tfvars.TerraformVariables{} rootCA := &tls.RootCA{} + boundSASigningKey := &tls.BoundSASigningKey{} parents.Get( manifestsAsset, workersAsset, @@ -102,6 +103,7 @@ func (i *InfraProvider) Provision(ctx context.Context, dir string, parents asset capiMachinesAsset, tfvarsAsset, rootCA, + boundSASigningKey, ) var capiClusters []*clusterv1.Cluster @@ -127,12 +129,13 @@ func (i *InfraProvider) Provision(ctx context.Context, dir string, parents asset if p, ok := i.impl.(PreProvider); ok { preProvisionInput := PreProvisionInput{ - InfraID: clusterID.InfraID, - InstallConfig: installConfig, - RhcosImage: rhcosImage, - ManifestsAsset: manifestsAsset, - MachineManifests: machineManifests, - WorkersAsset: workersAsset, + InfraID: clusterID.InfraID, + InstallConfig: installConfig, + RhcosImage: rhcosImage, + ManifestsAsset: manifestsAsset, + MachineManifests: machineManifests, + WorkersAsset: workersAsset, + BoundSASigningKey: boundSASigningKey, } timer.StartTimer(preProvisionStage) if err := p.PreProvision(ctx, preProvisionInput); err != nil { diff --git a/pkg/infrastructure/clusterapi/types.go b/pkg/infrastructure/clusterapi/types.go index e3647eccb82..625ce47f9d2 100644 --- a/pkg/infrastructure/clusterapi/types.go +++ b/pkg/infrastructure/clusterapi/types.go @@ -43,12 +43,13 @@ type PreProvider interface { // PreProvisionInput collects the args passed to the PreProvision call. type PreProvisionInput struct { - InfraID string - InstallConfig *installconfig.InstallConfig - RhcosImage *rhcos.Image - ManifestsAsset *manifests.Manifests - MachineManifests []client.Object - WorkersAsset *machines.Worker + InfraID string + InstallConfig *installconfig.InstallConfig + RhcosImage *rhcos.Image + ManifestsAsset *manifests.Manifests + MachineManifests []client.Object + WorkersAsset *machines.Worker + BoundSASigningKey *tls.BoundSASigningKey } // IgnitionProvider handles preconditions for bootstrap ignition, diff --git a/pkg/infrastructure/gcp/clusterapi/wif.go b/pkg/infrastructure/gcp/clusterapi/wif.go index f258c4c37d0..4eaed77afc9 100644 --- a/pkg/infrastructure/gcp/clusterapi/wif.go +++ b/pkg/infrastructure/gcp/clusterapi/wif.go @@ -51,7 +51,23 @@ func OIDCIssuerURL(infraID string) string { } // ProvisionWIF creates WIF infrastructure for installer-provisioned mode. +// Requires a user-provided bound SA signing key for JWKS generation. func ProvisionWIF(ctx context.Context, in clusterapi.PreProvisionInput) error { + if in.BoundSASigningKey == nil || len(in.BoundSASigningKey.Files()) == 0 { + return fmt.Errorf("bound service account signing key is required for WIF provisioning; provide bound-service-account-signing-key.key in the asset directory") + } + + var publicKeyPEM []byte + for _, f := range in.BoundSASigningKey.Files() { + if f.Filename == "tls/bound-service-account-signing-key.pub" { + publicKeyPEM = f.Data + break + } + } + if len(publicKeyPEM) == 0 { + return fmt.Errorf("failed to find bound SA signing public key") + } + platform := in.InstallConfig.Config.Platform.GCP projectID := platform.ProjectID infraID := in.InfraID @@ -96,7 +112,7 @@ func ProvisionWIF(ctx context.Context, in clusterapi.PreProvisionInput) error { } logrus.Infof("Creating OIDC discovery bucket %s", OIDCBucketName(infraID)) - if err := createOIDCBucket(ctx, storageClient, projectID, region, infraID, issuerURL); err != nil { + if err := createOIDCBucket(ctx, storageClient, projectID, region, infraID, issuerURL, publicKeyPEM); err != nil { return fmt.Errorf("failed to create OIDC bucket: %w", err) } @@ -185,7 +201,7 @@ func createWorkloadIdentityPool(ctx context.Context, iamSvc *iam.Service, projec return waitForIAMOperation(ctx, iamSvc, op.Name) } -func createOIDCBucket(ctx context.Context, storageClient *storage.Client, projectID, region, infraID, issuerURL string) error { +func createOIDCBucket(ctx context.Context, storageClient *storage.Client, projectID, region, infraID, issuerURL string, publicKeyPEM []byte) error { bucketName := OIDCBucketName(infraID) bucket := storageClient.Bucket(bucketName) @@ -216,6 +232,20 @@ func createOIDCBucket(ctx context.Context, storageClient *storage.Client, projec return fmt.Errorf("failed to close OIDC discovery doc writer: %w", err) } + jwksData, err := GenerateJWKS(publicKeyPEM) + if err != nil { + return fmt.Errorf("failed to generate JWKS: %w", err) + } + + jwksWriter := bucket.Object("keys.json").NewWriter(ctx) + jwksWriter.ContentType = "application/json" + if _, err := jwksWriter.Write(jwksData); err != nil { + return fmt.Errorf("failed to write JWKS: %w", err) + } + if err := jwksWriter.Close(); err != nil { + return fmt.Errorf("failed to close JWKS writer: %w", err) + } + return nil } From 5ef9e1a5f0cbe9bf30947a5453e3b5c69b8c9f0c Mon Sep 17 00:00:00 2001 From: Bruno Cesar Rocha Date: Tue, 9 Jun 2026 09:43:24 +0100 Subject: [PATCH 4/5] CORS-4470: address PR review feedback - Fix audience mismatch: use projectNumber instead of projectID when building the OIDC provider AllowedAudiences, matching the audience URI in the generated external_account credentials - Add PSC endpoint override to GetWIFProvider for custom-endpoint environments - Wait for WIF delete LROs before marking resources as deleted in the destroy flow, preventing premature cleanup - Make WIF pool and provider creation idempotent by handling 409 AlreadyExists errors - Use field.NotFound for missing WIF providers and field.InternalError for API failures instead of generic field.Invalid Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/asset/installconfig/gcp/client.go | 6 +++++- pkg/asset/installconfig/gcp/validation.go | 7 +++++-- pkg/destroy/gcp/wif.go | 6 ++++-- pkg/infrastructure/gcp/clusterapi/wif.go | 22 +++++++++++++++++++--- 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/pkg/asset/installconfig/gcp/client.go b/pkg/asset/installconfig/gcp/client.go index 1f351b6c2d2..689c79fad46 100644 --- a/pkg/asset/installconfig/gcp/client.go +++ b/pkg/asset/installconfig/gcp/client.go @@ -802,7 +802,11 @@ func (c *Client) GetWIFProvider(ctx context.Context, project, poolID, providerID ctx, cancel := context.WithTimeout(ctx, defaultTimeout) defer cancel() - iamSvc, err := GetIAMService(ctx) + opts := []option.ClientOption{} + if c.endpointName != "" { + opts = append(opts, CreateEndpointOption(c.endpointName, ServiceNameGCPIAM)) + } + iamSvc, err := GetIAMService(ctx, opts...) if err != nil { return nil, fmt.Errorf("failed to create IAM service: %w", err) } diff --git a/pkg/asset/installconfig/gcp/validation.go b/pkg/asset/installconfig/gcp/validation.go index 9469486e278..7f5237024ef 100644 --- a/pkg/asset/installconfig/gcp/validation.go +++ b/pkg/asset/installconfig/gcp/validation.go @@ -752,8 +752,11 @@ func validateWIFProvider(client API, ic *types.InstallConfig) field.ErrorList { provider, err := client.GetWIFProvider(context.TODO(), ic.GCP.ProjectID, wif.PoolID, wif.ProviderID) if err != nil { - return append(allErrs, field.Invalid(fldPath, wif.ProviderID, - fmt.Sprintf("failed to get WIF provider: %v", err))) + var ae *googleapi.Error + if errors.As(err, &ae) && ae.Code == 404 { + return append(allErrs, field.NotFound(fldPath.Child("providerID"), wif.ProviderID)) + } + return append(allErrs, field.InternalError(fldPath.Child("providerID"), err)) } if provider.State != "ACTIVE" { diff --git a/pkg/destroy/gcp/wif.go b/pkg/destroy/gcp/wif.go index 28baa23c38d..957735f19e3 100644 --- a/pkg/destroy/gcp/wif.go +++ b/pkg/destroy/gcp/wif.go @@ -47,7 +47,8 @@ func (o *ClusterUninstaller) deleteWIFProvider(ctx context.Context, item cloudRe return fmt.Errorf("failed to delete WIF provider %s: %w", item.name, err) } if op != nil && !op.Done { - o.Logger.Debugf("Waiting for WIF provider deletion: %s", op.Name) + o.Logger.Debugf("WIF provider deletion in progress: %s", op.Name) + return fmt.Errorf("WIF provider %s deletion still in progress", item.name) } o.deletePendingItems(item.typeName, []cloudResource{item}) o.Logger.Infof("Deleted WIF provider %s", item.name) @@ -116,7 +117,8 @@ func (o *ClusterUninstaller) deleteWIFPool(ctx context.Context, item cloudResour return fmt.Errorf("failed to delete WIF pool %s: %w", item.name, err) } if op != nil && !op.Done { - o.Logger.Debugf("Waiting for WIF pool deletion: %s", op.Name) + o.Logger.Debugf("WIF pool deletion in progress: %s", op.Name) + return fmt.Errorf("WIF pool %s deletion still in progress", item.name) } o.deletePendingItems(item.typeName, []cloudResource{item}) o.Logger.Infof("Deleted WIF pool %s", item.name) diff --git a/pkg/infrastructure/gcp/clusterapi/wif.go b/pkg/infrastructure/gcp/clusterapi/wif.go index 4eaed77afc9..e4939de0813 100644 --- a/pkg/infrastructure/gcp/clusterapi/wif.go +++ b/pkg/infrastructure/gcp/clusterapi/wif.go @@ -8,14 +8,17 @@ import ( "encoding/base64" "encoding/json" "encoding/pem" + "errors" "fmt" "math/big" + "net/http" "slices" "time" "cloud.google.com/go/storage" "github.com/sirupsen/logrus" resourcemanager "google.golang.org/api/cloudresourcemanager/v3" + "google.golang.org/api/googleapi" "google.golang.org/api/iam/v1" "google.golang.org/api/option" "k8s.io/apimachinery/pkg/util/wait" @@ -117,7 +120,7 @@ func ProvisionWIF(ctx context.Context, in clusterapi.PreProvisionInput) error { } logrus.Infof("Creating OIDC provider %s", providerName) - if err := createOIDCProvider(ctx, iamSvc, projectID, infraID, poolName, providerName, issuerURL); err != nil { + if err := createOIDCProvider(ctx, iamSvc, projectID, projectNumber, infraID, poolName, providerName, issuerURL); err != nil { return fmt.Errorf("failed to create OIDC provider: %w", err) } @@ -195,6 +198,10 @@ func createWorkloadIdentityPool(ctx context.Context, iamSvc *iam.Service, projec op, err := iamSvc.Projects.Locations.WorkloadIdentityPools.Create(parent, pool). WorkloadIdentityPoolId(poolName).Context(ctx).Do() if err != nil { + if isAlreadyExists(err) { + logrus.Debugf("WIF pool %s already exists, continuing", poolName) + return nil + } return fmt.Errorf("failed to create WIF pool: %w", err) } @@ -249,12 +256,12 @@ func createOIDCBucket(ctx context.Context, storageClient *storage.Client, projec return nil } -func createOIDCProvider(ctx context.Context, iamSvc *iam.Service, projectID, infraID, poolName, providerName, issuerURL string) error { +func createOIDCProvider(ctx context.Context, iamSvc *iam.Service, projectID, projectNumber, infraID, poolName, providerName, issuerURL string) error { ctx, cancel := context.WithTimeout(ctx, 2*time.Minute) defer cancel() parent := fmt.Sprintf("projects/%s/locations/global/workloadIdentityPools/%s", projectID, poolName) - audience := fmt.Sprintf("//iam.googleapis.com/%s/providers/%s", parent, providerName) + audience := BuildAudienceURI(projectNumber, poolName, providerName) provider := &iam.WorkloadIdentityPoolProvider{ DisplayName: fmt.Sprintf("%s OIDC Provider", infraID), @@ -271,6 +278,10 @@ func createOIDCProvider(ctx context.Context, iamSvc *iam.Service, projectID, inf op, err := iamSvc.Projects.Locations.WorkloadIdentityPools.Providers.Create(parent, provider). WorkloadIdentityPoolProviderId(providerName).Context(ctx).Do() if err != nil { + if isAlreadyExists(err) { + logrus.Debugf("OIDC provider %s already exists, continuing", providerName) + return nil + } return fmt.Errorf("failed to create OIDC provider: %w", err) } @@ -410,6 +421,11 @@ func GenerateJWKS(publicKeyPEM []byte) ([]byte, error) { return json.Marshal(jwks) } +func isAlreadyExists(err error) bool { + var ae *googleapi.Error + return errors.As(err, &ae) && ae.Code == http.StatusConflict +} + // BuildAudienceURI constructs the WIF audience URI from project number, pool ID, and provider ID. func BuildAudienceURI(projectNumber, poolID, providerID string) string { return fmt.Sprintf( From ec29fb792f0253666bf2c29ff869e2a6349b6f39 Mon Sep 17 00:00:00 2001 From: Bruno Cesar Rocha Date: Tue, 9 Jun 2026 10:00:34 +0100 Subject: [PATCH 5/5] CORS-4470: fix golint errors - Handle error return from generateOIDCDiscoveryDoc (errcheck) - Annotate external_account type string as not a hardcoded credential (gosec G101) Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/asset/manifests/openshift.go | 2 +- pkg/infrastructure/gcp/clusterapi/wif.go | 10 ++++++---- pkg/infrastructure/gcp/clusterapi/wif_test.go | 7 +++++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/pkg/asset/manifests/openshift.go b/pkg/asset/manifests/openshift.go index 0b4c2c9dbde..507a217e5a1 100644 --- a/pkg/asset/manifests/openshift.go +++ b/pkg/asset/manifests/openshift.go @@ -400,7 +400,7 @@ func generateGCPExternalAccountJSON(ctx context.Context, ic *installconfig.Insta projectNumber, poolID, providerID, ) - externalAccount := map[string]any{ + externalAccount := map[string]any{ // #nosec G101 -- not a hardcoded credential, this is a credential type identifier "type": "external_account", "audience": audience, "subject_token_type": "urn:ietf:params:oauth:token-type:jwt", diff --git a/pkg/infrastructure/gcp/clusterapi/wif.go b/pkg/infrastructure/gcp/clusterapi/wif.go index e4939de0813..4b5da5dae27 100644 --- a/pkg/infrastructure/gcp/clusterapi/wif.go +++ b/pkg/infrastructure/gcp/clusterapi/wif.go @@ -229,7 +229,10 @@ func createOIDCBucket(ctx context.Context, storageClient *storage.Client, projec return fmt.Errorf("failed to set OIDC bucket IAM policy: %w", err) } - discoveryDoc := generateOIDCDiscoveryDoc(issuerURL) + discoveryDoc, err := generateOIDCDiscoveryDoc(issuerURL) + if err != nil { + return fmt.Errorf("failed to generate OIDC discovery doc: %w", err) + } wtr := bucket.Object(".well-known/openid-configuration").NewWriter(ctx) wtr.ContentType = "application/json" if _, err := wtr.Write(discoveryDoc); err != nil { @@ -368,7 +371,7 @@ func waitForIAMOperation(ctx context.Context, iamSvc *iam.Service, opName string return nil } -func generateOIDCDiscoveryDoc(issuerURL string) []byte { +func generateOIDCDiscoveryDoc(issuerURL string) ([]byte, error) { doc := map[string]any{ "issuer": issuerURL, "jwks_uri": fmt.Sprintf("%s/keys.json", issuerURL), @@ -376,8 +379,7 @@ func generateOIDCDiscoveryDoc(issuerURL string) []byte { "subject_types_supported": []string{"public"}, "id_token_signing_alg_values_supported": []string{"RS256"}, } - data, _ := json.MarshalIndent(doc, "", " ") - return data + return json.MarshalIndent(doc, "", " ") } // GenerateJWKS produces a JSON Web Key Set from an RSA public key PEM. diff --git a/pkg/infrastructure/gcp/clusterapi/wif_test.go b/pkg/infrastructure/gcp/clusterapi/wif_test.go index d7fbe743cfd..9ba9e91a549 100644 --- a/pkg/infrastructure/gcp/clusterapi/wif_test.go +++ b/pkg/infrastructure/gcp/clusterapi/wif_test.go @@ -31,10 +31,13 @@ func TestBuildAudienceURI(t *testing.T) { func TestGenerateOIDCDiscoveryDoc(t *testing.T) { issuerURL := "https://storage.googleapis.com/test-oidc" - doc := generateOIDCDiscoveryDoc(issuerURL) + doc, err := generateOIDCDiscoveryDoc(issuerURL) + if !assert.NoError(t, err) { + return + } var parsed map[string]any - err := json.Unmarshal(doc, &parsed) + err = json.Unmarshal(doc, &parsed) assert.NoError(t, err) assert.Equal(t, issuerURL, parsed["issuer"])