fix: use SignJwt instead of GenerateAccessToken for Vault GCP Workload Identity auth#5828
fix: use SignJwt instead of GenerateAccessToken for Vault GCP Workload Identity auth#5828Skarlso wants to merge 1 commit intoexternal-secrets:mainfrom
Conversation
…d Identity auth Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
WalkthroughThis pull request consolidates GCP authentication logic from the GCP Secret Manager provider into a new shared runtime package, enabling reuse across providers. The Vault provider is enhanced with GCP IAM authentication using signed JWTs. Documentation is updated to reflect GCP authentication support. Changes
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@providers/v1/gcp/secretmanager/client.go`:
- Line 65: Remove the duplicate constant by deleting the local errNoProjectID
declaration from providers/v1/gcp/secretmanager/client.go and instead reference
the existing errNoProjectID from the shared auth package where it is defined;
add the auth package import and update any local references to use that imported
symbol so there is a single canonical definition.
🧹 Nitpick comments (4)
runtime/constants/constants.go (1)
36-36: Consider relocating constant to the GCP section for better organization.The
CallGCPSMSignJwtconstant is placed among AWS-related constants (lines 22-35) rather than with the other GCP SM constants (lines 59-69). Moving it to the GCP section would improve code organization and maintainability.♻️ Suggested placement
CallGCPSMGenerateSAToken = "GenerateServiceAccountToken" CallGCPSMGenerateIDBindToken = "GenerateIDBindToken" CallGCPSMGenerateAccessToken = "GenerateAccessToken" + CallGCPSMSignJwt = "SignJwt"And remove from line 36.
docs/snippets/vault-gcp-store.yaml (1)
29-33: Consider adding namespace hint to the commentedsecretRefoption.For consistency with line 28 where namespace is noted as required for
ClusterSecretStore, the commentedsecretRefalternative should also include a namespace field hint to guide users.📝 Suggested improvement
# Option 2: Authenticate using a service account key stored in a Secret # secretRef: # secretAccessKeySecretRef: # name: "gcpsm-secret" # key: "secret-access-credentials" + # # Namespace is required for ClusterSecretStore + # namespace: "default"runtime/gcp/auth/auth_test.go (1)
156-161: Unreachable else branch in test.This else branch is dead code since all test cases have
expectError: true. Consider removing it to avoid confusion, as the comment on Lines 131-132 already explains that successful JWT generation is tested via integration tests.♻️ Suggested simplification
if tt.expectError { assert.Error(t, err) assert.Empty(t, jwt) if tt.errorMsg != "" { assert.Contains(t, err.Error(), tt.errorMsg) } - } else { - // We can't actually test JWT generation without real GCP credentials, - // but we can verify it doesn't error with the mock setup - assert.NoError(t, err) - assert.NotEmpty(t, jwt) }providers/v1/gcp/go.mod (1)
16-16: Consider aligning google.golang.org/genproto package versions.There's a version mismatch between genproto sub-packages:
google.golang.org/genproto(line 16) andgenproto/googleapis/api(line 99) both use timestamp20251202230838genproto/googleapis/rpc(line 100) uses a newer timestamp20251222181119While this may be intentional due to separate release cycles, verify that the version difference doesn't introduce compatibility issues.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
providers/v1/gcp/go.sumis excluded by!**/*.sumproviders/v1/vault/go.sumis excluded by!**/*.sumruntime/go.sumis excluded by!**/*.sum
📒 Files selected for processing (19)
docs/provider/hashicorp-vault.mddocs/snippets/vault-gcp-store.yamlproviders/v1/gcp/go.modproviders/v1/gcp/secretmanager/auth.goproviders/v1/gcp/secretmanager/client.goproviders/v1/gcp/secretmanager/workload_identity_federation_test.goproviders/v1/gcp/secretmanager/workload_identity_test.goproviders/v1/vault/auth_gcp.goproviders/v1/vault/client.goproviders/v1/vault/go.modruntime/constants/constants.goruntime/gcp/auth/auth.goruntime/gcp/auth/auth_test.goruntime/gcp/auth/testing_helpers_test.goruntime/gcp/auth/workload_identity.goruntime/gcp/auth/workload_identity_federation.goruntime/gcp/auth/workload_identity_federation_test.goruntime/gcp/auth/workload_identity_test.goruntime/go.mod
💤 Files with no reviewable changes (2)
- providers/v1/gcp/secretmanager/workload_identity_federation_test.go
- providers/v1/gcp/secretmanager/workload_identity_test.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-09T19:14:48.246Z
Learnt from: rbstp
Repo: external-secrets/external-secrets PR: 5712
File: config/crds/bases/external-secrets.io_secretstores.yaml:1958-2044
Timestamp: 2026-01-09T19:14:48.246Z
Learning: In external-secrets CRDs, new providers are added to the GA v1 schema only; v1beta1 is maintained for backward compatibility and does not receive new features (e.g., DVLS should be v1-only).
Applied to files:
docs/snippets/vault-gcp-store.yamlruntime/gcp/auth/auth.goproviders/v1/gcp/go.modruntime/go.modproviders/v1/vault/go.mod
🧬 Code graph analysis (8)
providers/v1/vault/client.go (1)
providers/v1/vault/util/vault.go (4)
Client(51-61)Auth(31-33)Logical(42-47)Token(36-39)
runtime/gcp/auth/workload_identity_federation_test.go (2)
apis/externalsecrets/v1/secretstore_gcpsm_types.go (4)
GCPWorkloadIdentityFederation(92-120)ConfigMapReference(123-144)AwsCredentialsConfig(147-163)SecretReference(166-180)apis/meta/v1/types.go (1)
ServiceAccountSelector(46-66)
runtime/gcp/auth/auth_test.go (1)
runtime/gcp/auth/auth.go (2)
NewTokenSource(49-85)GenerateSignedJWTForVault(90-111)
runtime/gcp/auth/workload_identity_test.go (2)
apis/meta/v1/types.go (1)
ServiceAccountSelector(46-66)runtime/gcp/auth/workload_identity.go (1)
IamClient(103-107)
runtime/gcp/auth/auth.go (2)
providers/v1/gcp/secretmanager/auth.go (1)
NewTokenSource(36-38)providers/v1/gcp/secretmanager/client.go (2)
Client(88-96)CloudPlatformRole(54-54)
providers/v1/gcp/secretmanager/auth.go (1)
runtime/gcp/auth/auth.go (1)
NewTokenSource(49-85)
providers/v1/vault/auth_gcp.go (4)
providers/v1/gcp/secretmanager/auth.go (1)
NewTokenSource(36-38)runtime/gcp/auth/auth.go (2)
NewTokenSource(49-85)GenerateSignedJWTForVault(90-111)apis/externalsecrets/v1/secretstore_vault_types.go (1)
VaultGCPAuth(403-432)runtime/constants/constants.go (2)
ProviderHCVault(71-71)CallHCVaultLogin(72-72)
runtime/gcp/auth/workload_identity_federation.go (2)
providers/v1/gcp/secretmanager/client.go (1)
Client(88-96)apis/externalsecrets/v1/secretstore_gcpsm_types.go (1)
GCPWorkloadIdentityFederation(92-120)
🪛 Gitleaks (8.30.0)
runtime/gcp/auth/auth_test.go
[high] 177-177: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze project (actions, none)
- GitHub Check: Analyze project (go, autobuild)
🔇 Additional comments (29)
providers/v1/vault/go.mod (2)
35-168: Test gRPC v1.78.0 compatibility before production—Vault submodules use older versions.This change updates several indirect dependencies, including
google.golang.org/grpcto v1.78.0. While no security vulnerabilities were found in this or related package updates (protobuf v1.36.11, cloud.google.com/go/auth v0.18.0), there is a compatibility consideration: Vault's submodules currently pin much older gRPC versions (e.g., v1.63.2 in hcp_link/proto, v1.41.0 in other modules), and there is no official HashiCorp statement confirming support for v1.78.0.Since this is an indirect dependency cascading from upstream changes, verify that the build works with Vault and perform integration testing before deploying to production. If you control the Vault build, check the gRPC v1.78.0 release notes for breaking changes relative to the versions Vault currently expects.
5-33: GCP authentication refactoring verified successfully.The migration of GCP authentication logic from the dedicated GCP provider to the shared runtime package is complete:
runtime/gcp/auth/provides the necessary authentication implementations (SignJwt, WorkloadIdentity, WorkloadIdentityFederation)providers/v1/vault/auth_gcp.gocorrectly imports and usesgithub.com/external-secrets/external-secrets/runtime/gcp/auth- No remaining references to the old
providers/v1/gcpdependency- The
golang.org/x/oauth2upgrade from v0.32.0 to v0.34.0 contains only backward-compatible changes (no breaking changes)providers/v1/gcp/secretmanager/client.go (2)
88-96: LGTM!The
Clientstruct simplification aligns with the broader refactor to consolidate workload identity handling into the sharedruntime/gcp/authpackage. The remaining fields are appropriate for the client's responsibilities.
646-656: LGTM!The
Closemethod correctly handles the simplified client structure, closing only thesmClientnow that workload identity management has been moved elsewhere.docs/snippets/vault-gcp-store.yaml (1)
1-28: LGTM!This is a well-structured documentation example that clearly demonstrates GCP IAM authentication for Vault. The workload identity configuration is recommended as the primary option, the YAML includes helpful comments, and the namespace requirement for
ClusterSecretStoreis correctly documented.docs/provider/hashicorp-vault.md (2)
271-281: LGTM!The authentication methods list is updated correctly to include GCP authentication with proper link formatting and consistent presentation.
380-388: LGTM!The new GCP authentication section is well-written and follows the established documentation pattern. It clearly describes both authentication methods (Workload Identity and service account keys) and includes the appropriate namespace note for
ClusterSecretStore.runtime/go.mod (3)
21-22: LGTM!The
golang.org/x/oauth2andgoogle.golang.org/apidependencies are essential for GCP authentication flows and API interactions.
86-91: OpenTelemetry dependencies are transitive.These OpenTelemetry instrumentation packages are pulled in as indirect dependencies by the GCP client libraries. This is expected and provides observability capabilities for gRPC and HTTP calls to GCP services.
6-8: GCP dependencies are at current versions.All three GCP dependencies (cloud.google.com/go/secretmanager v1.16.0, cloud.google.com/go/iam v1.5.3, and cloud.google.com/go/compute/metadata v0.9.0) are pinned to their latest available versions and are appropriate for the GCP authentication functionality.
providers/v1/vault/client.go (1)
52-53: LGTM! New fields support GCP Workload Identity authentication for Vault.The addition of
gcpSignedJWTandgcpServiceAccountEmailfields appropriately extends the client struct to store GCP authentication state. The inline comments clearly document each field's purpose.runtime/gcp/auth/auth_test.go (2)
166-181: Static analysis false positive: Test fixture uses intentionally invalid/truncated private key.The Gitleaks warning for Line 177 is a false positive. The private key in
validSecretis intentionally truncated and invalid—it's only used to test JSON unmarshaling, not actual cryptographic operations. This is a common pattern for test fixtures.
40-118: Good test coverage for NewTokenSource with appropriate mocking.The test structure is well-organized with proper cleanup of mock state via
t.Cleanup. The table-driven approach with explicitexpectTokenandexpectErrorflags makes test intentions clear.runtime/gcp/auth/testing_helpers_test.go (1)
51-57: Well-documented TestMain for package-wide test setup.The TestMain approach correctly replaces the SA token generator factory for all tests. The comments (Lines 41-50) thoroughly explain why restoration isn't needed and how individual tests can override the mock if needed.
runtime/gcp/auth/workload_identity_federation.go (2)
129-161: Good use of functional options pattern for testability.The
wifOptiontype andwithWifSATokenGeneratorhelper enable clean dependency injection for tests without cluttering the production API. The lazy initialization pattern (Lines 152-159) correctly falls back to the real SA token generator when no mock is injected.
210-210: Typo fixes in error messages.Good catch fixing "acccount" → "account" in error messages.
Also applies to: 223-223
providers/v1/gcp/secretmanager/auth.go (1)
34-38: Clean refactoring to shared GCP auth implementation.This simplification follows DRY principles by delegating to the centralized
runtime/gcp/authpackage. The public API remains unchanged, ensuring backward compatibility for callers.runtime/gcp/auth/workload_identity_test.go (1)
1-426: LGTM! Comprehensive test coverage for workload identity flows.The test file provides good coverage for:
- Token source creation with and without GCP service account annotations
- Signed JWT generation for Vault authentication
- Error handling for missing service accounts and annotations
- Mock implementations are well-structured and follow standard patterns
The test assertions appropriately verify both the request payloads (e.g., service account email, vault role in JWT) and the expected responses.
runtime/gcp/auth/workload_identity_federation_test.go (1)
1-411: LGTM! Good validation test coverage for workload identity federation.The tests comprehensively cover:
- Configuration validation (exactly one auth method required, audience requirements)
- ConfigMap handling (missing configmap, missing key, empty value, invalid JSON, wrong credential type)
- AWS credential handling (missing secret, missing required keys)
- Namespace resolution for ClusterSecretStore
The comment on line 311 clarifying the intentional test behavior is helpful.
providers/v1/vault/auth_gcp.go (2)
71-74: LGTM! Clean separation of JWT-based login flow.The early return when
gcpSignedJWTis present correctly bypasses the standard GCP auth library flow and uses the direct JWT login, which is the intended behavior for Workload Identity authentication with Vault.
264-291: LGTM! Proper Vault JWT login implementation.The
loginWithSignedJWTmethod correctly:
- Constructs the login payload with role and JWT
- Uses the appropriate Vault API endpoint format
- Records metrics for observability
- Validates the response thoroughly before setting the token
runtime/gcp/auth/auth.go (1)
1-132: LGTM! Well-structured shared GCP authentication package.The implementation provides a clean API with:
- Clear fallback chain: service account → workload identity → workload identity federation → default credentials
- Proper resource cleanup via defers
- Testability via
defaultTokenSourceFuncoverride- Consistent early-return pattern throughout
The
GenerateSignedJWTForVaultfunction properly validates input and delegates to the workload identity implementation.runtime/gcp/auth/workload_identity.go (3)
142-188: LGTM! Thread-safe refreshable token source implementation.The
refreshableTokenSourcecorrectly:
- Uses mutex for thread-safe access
- Refreshes tokens 1 minute before expiry to avoid race conditions
- Uses an independent context with timeout for refresh operations
- Handles edge cases like nil tokens and zero expiry times
362-438: LGTM! Correct JWT signing for Vault GCP IAM authentication.The
SignedJWTForVaultimplementation:
- Properly validates the GCP service account annotation requirement
- Uses the correct JWT payload structure with
sub,aud,exp,iatclaims- Formats the audience as
vault/{role}per Vault documentation- Records metrics for the SignJwt API call
- Properly manages IAM client lifecycle with defer
267-285: Good refactor to use refreshable token source.Wrapping the token generation in a
refreshableTokenSourceensures that long-running operations won't fail due to token expiration. The initial token generation validates the configuration early, which is a good practice.providers/v1/gcp/go.mod (4)
26-29: Indirect dependency classification is correct.The packages marked as indirect (
cloud.google.com/go/auth,cloud.google.com/go/compute/metadata,cloud.google.com/go/iam,k8s.io/apimachinery,k8s.io/client-go) are not directly imported by code inproviders/v1/gcp/. Instead, they are used transitively through the runtime package, particularlyruntime/gcp/authimported insecretmanager/auth.go. The refactoring to consolidate GCP authentication logic into the shared runtime package is correctly reflected in these go.mod changes.
3-3: No action needed. Go 1.25.5 is the current stable release as of January 2026 (released December 2, 2025) and is appropriate for this module.Likely an incorrect or invalid review comment.
91-92: No security concerns identified. Bothgolang.org/x/crypto v0.46.0andgolang.org/x/net v0.48.0are current stable versions with no unfixed vulnerabilities. All recent CVEs affecting these packages have been resolved in earlier releases, and both versions are newer than the fixes.
10-10: Direct dependency versions are secure and current.The updated dependencies—
googleapis/gax-go/v2 v2.16.0,golang.org/x/oauth2 v0.34.0,google.golang.org/api v0.260.0,google.golang.org/grpc v1.78.0, andgoogle.golang.org/protobuf v1.36.11—are all recent, stable releases with no known security vulnerabilities.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| errClientGetSecretAccess = "unable to access Secret from SecretManager Client: %w" | ||
| errJSONSecretUnmarshal = "unable to unmarshal secret from JSON: %w" | ||
|
|
||
| errNoProjectID = "unable to find ProjectID in storeSpec" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'errNoProjectID' --type goRepository: external-secrets/external-secrets
Length of output: 383
🏁 Script executed:
sed -n '1,20p' providers/v1/gcp/secretmanager/provider.goRepository: external-secrets/external-secrets
Length of output: 689
🏁 Script executed:
sed -n '150,165p' providers/v1/gcp/secretmanager/provider.goRepository: external-secrets/external-secrets
Length of output: 636
🏁 Script executed:
# Check the package declaration and if both files are in the same package
head -25 providers/v1/gcp/secretmanager/provider.go | grep "^package"
head -25 providers/v1/gcp/secretmanager/client.go | grep "^package"Repository: external-secrets/external-secrets
Length of output: 120
Remove duplicate errNoProjectID constant definition.
The error constant errNoProjectID is already defined in runtime/gcp/auth/workload_identity.go:61 with the same value. Having duplicate constants across packages creates maintenance burden and inconsistency. Consolidate to a single definition in the shared auth package and import it where needed.
🤖 Prompt for AI Agents
In `@providers/v1/gcp/secretmanager/client.go` at line 65, Remove the duplicate
constant by deleting the local errNoProjectID declaration from
providers/v1/gcp/secretmanager/client.go and instead reference the existing
errNoProjectID from the shared auth package where it is defined; add the auth
package import and update any local references to use that imported symbol so
there is a single canonical definition.
Problem Statement
What is the problem you're trying to solve?
Related Issue
Fixes #...
Proposed Changes
How do you like to solve the issue and why?
Format
Please ensure that your PR follows the following format for the title:
Where
scopeis optionally one of:Checklist
git commit --signoffmake testmake reviewableSummary
This PR refactors Vault's GCP authentication to use SignJwt instead of GenerateAccessToken for Workload Identity authentication, improving the authentication flow for GCP-based secrets retrieval.
Key Changes
Vault GCP Auth (providers/v1/vault/auth_gcp.go)
requestTokenWithGcpAuthto prioritize signed JWT if availablesetupServiceAccountKeyAuthto extract and store the service account emailsetupWorkloadIdentityAuthto:gcpauth.GenerateSignedJWTForVaultloginWithSignedJWTmethod to authenticate with Vault using signed JWT tokensgcpSignedJWTandgcpServiceAccountEmailNew Shared GCP Auth Module (runtime/gcp/auth/)
NewTokenSourcefunction for OAuth2 token source creation supporting service accounts, workload identity, and federationGenerateSignedJWTForVaultfor Vault-specific JWT generationworkloadIdentitywith:SignedJWTForVaultmethod for generating signed JWTsSignJwtmethod added toIamClientinterfaceworkloadIdentityFederationto use new options patternGCP Secret Manager Refactoring (providers/v1/gcp/secretmanager/)
gcpauthimplementationDocumentation & Configuration
Dependency Updates