-
Notifications
You must be signed in to change notification settings - Fork 44
refactor: switch GCP Cloud Connectors auth to AWS Workload Identity Federation #3851
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
This pull request does not have a backport label. Could you fix it @amirbenun? 🙏
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the GCP Cloud Connectors authentication flow from direct OIDC/JWT-based token exchange to AWS Workload Identity Federation. The new approach first assumes an AWS role using the JWT via AssumeRoleWithWebIdentity, then uses the resulting AWS credentials for GCP Workload Identity Federation token exchange before impersonating the target service account in the customer's GCP project.
Changes:
- Modified authentication flow to use AWS role assumption as an intermediary step before GCP credential exchange
- Updated CloudConnectorsConfig to include JWTFilePath and properly pass it to GCP auth providers
- Added awsCredentialsSupplier type to handle AWS role assumption with web identity
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/config/config.go | Added JWTFilePath field to CloudConnectorsConfig and initialization logic for GCP Cloud Connectors when service_account_email is set |
| internal/resources/providers/gcplib/auth/auth_provider.go | Implemented new AWS-based authentication flow with awsCredentialsSupplier type that assumes AWS role before GCP token exchange |
| internal/resources/providers/gcplib/auth/credentials.go | Updated FindCloudConnectorsCredentials signature to accept CloudConnectorsConfig parameter and updated log message |
| internal/resources/providers/gcplib/auth/credentials_mock.go | Updated mock function signatures to match new CloudConnectorsConfig parameter |
| internal/resources/providers/gcplib/auth/credentials_test.go | Updated test mock calls to include new ccConfig parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ccConfig.ResourceID == "" { | ||
| return nil, fmt.Errorf("cloud connectors config ResourceID is required") | ||
| } | ||
|
|
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing validation for required parameters 'audience' and 'serviceAccountEmail'. These parameters are used directly in the configuration without checking if they are empty strings. If either is empty, it could lead to runtime errors when constructing the GCP IAM credentials URL or configuring the external account. Consider adding validation checks similar to the other required fields.
| if audience == "" { | |
| return nil, fmt.Errorf("cloud connectors audience is required") | |
| } | |
| if serviceAccountEmail == "" { | |
| return nil, fmt.Errorf("cloud connectors serviceAccountEmail is required") | |
| } |
| // awsCredentialsSupplier implements externalaccount.AwsSecurityCredentialsSupplier | ||
| // It assumes an AWS role using a JWT token and provides the resulting credentials to GCP. | ||
| type awsCredentialsSupplier struct { | ||
| jwtFilePath string | ||
| globalRoleARN string | ||
| roleSessionID string | ||
| region string | ||
| } | ||
|
|
||
| // AwsRegion returns the AWS region for the credentials. | ||
| func (s *awsCredentialsSupplier) AwsRegion(ctx context.Context, options externalaccount.SupplierOptions) (string, error) { | ||
| return s.region, nil | ||
| } | ||
|
|
||
| // AwsSecurityCredentials assumes the AWS role using the JWT and returns the temporary credentials. | ||
| func (s *awsCredentialsSupplier) AwsSecurityCredentials(ctx context.Context, options externalaccount.SupplierOptions) (*externalaccount.AwsSecurityCredentials, error) { | ||
| // Create STS client without credentials (we're using web identity) | ||
| stsClient := sts.New(sts.Options{ | ||
| Region: s.region, | ||
| }) | ||
|
|
||
| // Use the AWS SDK's built-in web identity provider | ||
| webIdentityProvider := stscreds.NewWebIdentityRoleProvider( | ||
| stsClient, | ||
| s.globalRoleARN, | ||
| stscreds.IdentityTokenFile(s.jwtFilePath), | ||
| func(o *stscreds.WebIdentityRoleOptions) { | ||
| o.RoleSessionName = s.roleSessionID | ||
| }, | ||
| ) | ||
|
|
||
| // Retrieve credentials using the web identity provider | ||
| creds, err := webIdentityProvider.Retrieve(ctx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to assume role %s with web identity: %w", s.globalRoleARN, err) | ||
| } | ||
|
|
||
| return &externalaccount.AwsSecurityCredentials{ | ||
| AccessKeyID: creds.AccessKeyID, | ||
| SecretAccessKey: creds.SecretAccessKey, | ||
| SessionToken: creds.SessionToken, | ||
| }, nil | ||
| } |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new awsCredentialsSupplier type and its methods (AwsRegion and AwsSecurityCredentials) lack unit test coverage. While the FindCloudConnectorsCredentials method is tested through mocks in credentials_test.go, the actual AWS role assumption logic and credential retrieval flow are not directly tested. Consider adding unit tests to verify the behavior of the awsCredentialsSupplier, including error handling when JWT file is missing, invalid, or when AWS role assumption fails.
| } | ||
|
|
||
| // AwsSecurityCredentials assumes the AWS role using the JWT and returns the temporary credentials. | ||
| func (s *awsCredentialsSupplier) AwsSecurityCredentials(ctx context.Context, _ externalaccount.SupplierOptions) (*externalaccount.AwsSecurityCredentials, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fairly similar to code that we have in:
I think that this can be shared and not duplicated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is not a lot of shared code but I took it out into a new file web_identity.go
Refactors the GCP Cloud Connectors authentication flow from direct OIDC/JWT-based token exchange to AWS Workload Identity Federation. The new approach first assumes an AWS role using the JWT via AssumeRoleWithWebIdentity, then uses the resulting AWS credentials for GCP Workload Identity Federation token exchange before impersonating the target service account in the customer's GCP project. This change aligns the GCP Cloud Connectors authentication with the AWS-based trust pattern and includes updates to the configuration to properly pass CloudConnectorsConfig with the JWT file path to GCP auth providers.