feat: add GCP short-lived credentials support #709
Conversation
- Support service account impersonation with configurable token lifetime - Add authentication via ADC, static keys, or separate source credentials - Include comprehensive documentation and examples - Maintain full backward compatibility with existing configurations
WalkthroughAdds GCP short-lived credential support via the IAM Service Account Credentials API, configurable token lifetime and source credential options, documentation and .gitignore updates, unit tests, and provider integration wiring to use short-lived or standard token sources. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as CLI User
participant CL as cloudlist (GCP provider)
participant Auth as registerWithOptions
participant IAM as GCP IAM Credentials API
participant GCP as GCP Services
User->>CL: Run discovery with GCP options
CL->>Auth: Request credential setup (use_short_lived? yes/no)
alt use_short_lived = true
Auth->>IAM: generateAccessToken(target SA, lifetime)
IAM-->>Auth: access token + expiry
Auth-->>CL: TokenSource (short-lived)
else
Auth-->>CL: Standard credential TokenSource / ClientOption
end
loop discovery
CL->>GCP: API call with bearer token
alt token expired
CL->>Auth: refresh token
Auth->>IAM: generateAccessToken(...)
IAM-->>Auth: new token
Auth-->>CL: Token
end
end
CL-->>User: Discovered resources
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (17)
.gitignore (1)
6-13: Clarify intent of ignoringcloudlistand tighten secret patterns
- The line "cloudlist" will ignore any top-level file/dir named cloudlist (often the built binary). If the intent is to ignore only the binary at repo root, prefer anchoring: "/cloudlist".
- Consider adding patterns for other common GCP key names to avoid accidental commits, e.g., "application_default_credentials.json" and "*-adc.json".
- cloudlist + /cloudlist + application_default_credentials.json + *-adc.jsonpkg/providers/gcp/auth_test.go (1)
80-135: Test name vs. behavior mismatch; consider renaming or exercising actual validationThis test is named for ValidateLifetime but only checks parsed duration range. Either:
- rename to reflect behavior (e.g., TestTokenLifetimeRange), or
- add a small validation helper in prod code (e.g., validateTokenLifetime(string) error) and invoke it here.
docs/GCP_SHORT_LIVED_CREDENTIALS.md (3)
60-85: CI example: add explicit note that source_credentials is a filesystem pathSmall clarity tweak: explicitly state source_credentials expects a path to a JSON key file readable by the process; ADC will be used if omitted.
167-176: Parameter table is clear; consider noting accepted formats next to token_lifetimeAdd “Supports seconds and Go duration (e.g., 3600s, 1h, 1h30m)” directly in the Description to reduce back-and-forth.
193-205: Troubleshooting impersonation: include serviceAccount member example for CIAdd explicit example member for service accounts: serviceAccount:MINIMAL_SA@PROJECT.iam.gserviceaccount.com, to mirror the user:… example.
PROVIDERS.md (4)
50-64: Nice addition explaining modes; consider heading levels instead of bold to satisfy markdownlintConvert bold section titles to proper headings (e.g., “#### Traditional Authentication”, “#### Short‑lived Credentials”) to address MD036.
-**A. Traditional Authentication (Static Credentials)** +#### A. Traditional Authentication (Static Credentials) ... -**B. Short-lived Credentials (Recommended for Enhanced Security)** +#### B. Short-lived Credentials (Recommended for Enhanced Security)Based on static analysis hints
68-133: Options 1–5: same heading nit and minor consistency
- Use heading syntax for “Option N” lines (MD036).
- Keep parameter names consistent with code: service_account_email, source_credentials, token_lifetime (already correct).
Based on static analysis hints
150-155: Parameter doc is clear; add explicit range/format to token_lifetime here tooMirror the range (1s–43200s) and format info inline.
208-214: Convert bare URLs in “References” to markdown links (MD034)Replace bare links with text to satisfy markdownlint.
Based on static analysis hints
pkg/providers/gcp/gcp.go (5)
231-253: Short‑lived config parsing duplicated in org/individual paths; centralize and pre‑validate
- Factor parsing/validation of use_short_lived_credentials, service_account_email, source_credentials, token_lifetime into a helper to avoid duplication and drift.
- Optionally pre‑validate token_lifetime format here to fail fast before calling registerWithOptions.
254-269: Prefer context with timeout for token generationregisterWithOptions likely calls IAM Credentials API. Wrap with context.WithTimeout to avoid indefinite hangs.
- creds, err = registerWithOptions( - context.Background(), + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + creds, err = registerWithOptions( - context.Background(), + ctx,
326-340: Project listing: warn on pagination/list errorsYou wrap creation error, but if Pages returns an error, you propagate it; good. Consider emitting a warning before returning to aid diagnosis.
532-569: Duplicate short‑lived parsing in org provider—apply the same helperSame recommendation as individual provider to reduce duplication and keep validation consistent.
555-569: Add timeout to org provider registerWithOptions call tooSame context.WithTimeout recommendation as above.
pkg/providers/gcp/auth.go (3)
80-94: Prefer structured googleapi error handling; also fix permission nameUse googleapi.Error instead of string contains, and correct permission to iam.serviceAccounts.getAccessToken.
+ // imports: + // "errors" + // "google.golang.org/api/googleapi" @@ - if err != nil { - // Provide helpful error messages for common issues - errMsg := err.Error() - if strings.Contains(errMsg, "403") { - return nil, errkit.Wrap(err, "permission denied: ensure source credentials have 'roles/iam.serviceAccountTokenCreator' or 'iam.serviceAccounts.generateAccessToken' permission on target service account") - } - if strings.Contains(errMsg, "404") { - return nil, errkit.Wrap(err, "service account not found: verify the service_account_email is correct") - } - if strings.Contains(errMsg, "PERMISSION_DENIED") || strings.Contains(errMsg, "IAM_PERMISSION_DENIED") { - return nil, errkit.Wrap(err, "IAM Service Account Credentials API may not be enabled or accessible") - } - return nil, errkit.Wrap(err, "failed to generate access token") - } + if err != nil { + var gerr *googleapi.Error + if errors.As(err, &gerr) { + switch gerr.Code { + case 403: + return nil, errkit.Wrap(err, "permission denied: ensure source credentials have 'roles/iam.serviceAccountTokenCreator' (permission 'iam.serviceAccounts.getAccessToken') on the target service account") + case 404: + return nil, errkit.Wrap(err, "service account not found: verify the service_account_email is correct") + } + } + // Fallback on text signals for non-HTTP code cases + if strings.Contains(err.Error(), "PERMISSION_DENIED") || strings.Contains(err.Error(), "IAM_PERMISSION_DENIED") { + return nil, errkit.Wrap(err, "IAM Service Account Credentials API may not be enabled or accessible") + } + return nil, errkit.Wrap(err, "failed to generate access token") + }Add imports at the top:
import ( "context" "fmt" "net/http" + "errors" "os" "strconv" "strings" "time" @@ "golang.org/x/oauth2/google" "google.golang.org/api/iamcredentials/v1" + "google.golang.org/api/googleapi" "google.golang.org/api/option" "k8s.io/client-go/rest" )
43-49: Parse lifetime once at construction to avoid repeated parsing and reduce runtime errorsStore a parsed time.Duration in the token source. This also surfaces bad input earlier.
type shortLivedTokenSource struct { ctx context.Context sourceCredentials *google.Credentials targetServiceAccount string - lifetime string + lifetimeDur time.Duration } @@ - // Parse lifetime duration - lifetimeDuration, err := parseDuration(s.lifetime) - if err != nil { - return nil, errkit.Wrap(err, "invalid token lifetime") - } + // lifetime already parsed at construction as s.lifetimeDur @@ req := &iamcredentials.GenerateAccessTokenRequest{ Scope: []string{scope}, - Lifetime: fmt.Sprintf("%ds", int(lifetimeDuration.Seconds())), + Lifetime: fmt.Sprintf("%ds", int(s.lifetimeDur.Seconds())), } @@ - gologger.Debug().Msgf("Generating short-lived token for service account: %s (lifetime: %s)", s.targetServiceAccount, s.lifetime) + gologger.Debug().Msgf("Generating short-lived token for service account: %s (lifetime: %s)", s.targetServiceAccount, s.lifetimeDur)And initialize it in registerWithOptions:
- shortLivedSource := &shortLivedTokenSource{ - ctx: ctx, - sourceCredentials: sourceCreds, - targetServiceAccount: targetServiceAccount, - lifetime: tokenLifetime, - } + lifetimeDur, err := parseDuration(tokenLifetime) + if err != nil { + return nil, errkit.Wrap(err, "invalid token lifetime") + } + shortLivedSource := &shortLivedTokenSource{ + ctx: ctx, + sourceCredentials: sourceCreds, + targetServiceAccount: targetServiceAccount, + lifetimeDur: lifetimeDur, + }Also applies to: 53-64, 184-191, 77-77, 74-75
184-201: Optional: use impersonate package to simplify IAM token mintinggoogle.golang.org/api/impersonate provides a first‑party TokenSource with built‑in caching and bounds checking. It reduces maintenance and uses the IAM Credentials API under the hood.
Example:
import "google.golang.org/api/impersonate" ts, err := impersonate.CredentialsTokenSource( ctx, impersonate.CredentialsConfig{ TargetPrincipal: targetServiceAccount, Scopes: []string{scope}, // Lifetime defaults to 1h; optional: Lifetime: time.Minute * 55, }, option.WithCredentials(sourceCreds), ) if err != nil { return nil, errkit.Wrap(err, "create impersonated token source") } finalTokenSource = oauth2.ReuseTokenSource(nil, ts)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
examples/gcp-short-lived-credentials.yamlis excluded by!**/*.yaml
📒 Files selected for processing (6)
.gitignore(1 hunks)PROVIDERS.md(4 hunks)docs/GCP_SHORT_LIVED_CREDENTIALS.md(1 hunks)pkg/providers/gcp/auth.go(2 hunks)pkg/providers/gcp/auth_test.go(1 hunks)pkg/providers/gcp/gcp.go(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
pkg/providers/gcp/auth.go (1)
pkg/providers/gcp/gcp.go (1)
New(184-202)
pkg/providers/gcp/gcp.go (2)
pkg/providers/k8s/kubernetes.go (1)
New(33-89)pkg/providers/aws/aws.go (1)
New(125-214)
🪛 markdownlint-cli2 (0.18.1)
PROVIDERS.md
54-54: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
58-58: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
68-68: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
91-91: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
102-102: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
113-113: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
123-123: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
209-209: Bare URL used
(MD034, no-bare-urls)
210-210: Bare URL used
(MD034, no-bare-urls)
211-211: Bare URL used
(MD034, no-bare-urls)
212-212: Bare URL used
(MD034, no-bare-urls)
213-213: Bare URL used
(MD034, no-bare-urls)
⏰ 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). (6)
- GitHub Check: release-test
- GitHub Check: Analyze (go)
- GitHub Check: Test Builds (1.22.x, ubuntu-latest)
- GitHub Check: Test Builds (1.22.x, windows-latest)
- GitHub Check: Test Builds (1.22.x, macOS-latest)
- GitHub Check: Lint Test
🔇 Additional comments (8)
pkg/providers/gcp/auth_test.go (2)
10-78: Good coverage for duration parsing formats and edge cases
179-185: Default lifetime assertion looks goodpkg/providers/gcp/gcp.go (5)
21-21: Import of option is appropriate for credential wiring
580-625: Extended metadata wiring looks good; errors are downgraded to warningsLGTM; good resilience by continuing when specific APIs aren’t permitted.
783-830: Provider.Verify logic is pragmatic and safeChecks minimal API availability across services; returns useful errors. LGTM.
832-859: OrgProvider.Verify via Asset API is soundSingle-page probe with informative error context. LGTM.
184-202: registerWithOptions correctly falls back to ADC when no service account key is providedpkg/providers/gcp/auth.go (1)
112-129: Duration parser is clear and robustCovers both “Ns” and Go duration formats. LGTM.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
pkg/providers/gcp/auth.go (1)
232-238: Handle "already registered" errors gracefully.
RegisterAuthProviderPluginreturns an error if the plugin is already registered. IfregisterWithOptionsis called multiple times (e.g., when creating multiple GCP provider instances), subsequent calls will fail. The previous review suggested checking for "already registered" errors, but this wasn't fully implemented.Apply this diff to handle duplicate registration gracefully:
- err = rest.RegisterAuthProviderPlugin(googleAuthPlugin, + err := rest.RegisterAuthProviderPlugin(googleAuthPlugin, func(clusterAddress string, config map[string]string, persister rest.AuthProviderConfigPersister) (rest.AuthProvider, error) { return &googleAuthProvider{tokenSource: finalTokenSource}, nil }) - if err != nil { - return nil, errkit.Wrap(err, "failed to register GKE auth provider plugin") - } + if err != nil && !strings.Contains(strings.ToLower(err.Error()), "already registered") { + return nil, errkit.Wrap(err, "failed to register GKE auth provider plugin") + }
🧹 Nitpick comments (1)
pkg/providers/gcp/auth.go (1)
81-94: Error string matching is fragile but acceptable.The error handling checks for substring patterns like "403", "404", "PERMISSION_DENIED" to provide helpful user-facing messages. While this approach is common for external APIs without structured error types, it can break if GCP changes their error message format. Consider using type assertions or checking for specific error types if the GCP SDK provides them. However, the current implementation provides good user experience with actionable error messages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/providers/gcp/auth.go(2 hunks)pkg/providers/gcp/auth_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/providers/gcp/auth_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/providers/gcp/auth.go (1)
pkg/providers/gcp/gcp.go (1)
New(184-202)
⏰ 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). (5)
- GitHub Check: Test Builds (1.22.x, windows-latest)
- GitHub Check: Test Builds (1.22.x, ubuntu-latest)
- GitHub Check: Test Builds (1.22.x, macOS-latest)
- GitHub Check: Lint Test
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
pkg/providers/gcp/auth.go (1)
209-214: Verify context lifetime for token refresh scenarios.The context passed to
registerWithOptionsis stored inshortLivedTokenSourceand reused for all token refresh operations. If this context is canceled (e.g., a short-lived request context), subsequent token refreshes will fail, even during long-running operations.Ensure the context passed to this function has an appropriate lifetime that covers the intended duration of token usage. For long-running services, consider using
context.Background()or a context with a lifetime matching the service's operation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/providers/gcp/auth.go (2)
82-94: Consider more robust error detection for IAM API failures.The current error handling uses case-sensitive string matching (
strings.Contains) to detect specific error conditions. This approach is fragile and may miss error variations or changes in API error messages.Consider these improvements:
- Check for HTTP status codes if available from the API error:
if apiErr, ok := err.(*googleapi.Error); ok { switch apiErr.Code { case 403: return nil, errkit.Wrap(err, "permission denied: ensure source credentials have 'roles/iam.serviceAccountTokenCreator' or 'iam.serviceAccounts.generateAccessToken' permission on target service account") case 404: return nil, errkit.Wrap(err, "service account not found: verify the service_account_email is correct") } }
- If using string matching, use case-insensitive checks:
-if strings.Contains(errMsg, "403") { +if strings.Contains(strings.ToLower(errMsg), "403") || strings.Contains(strings.ToLower(errMsg), "permission denied") {This makes error detection more reliable and maintainable.
154-156: Consider extracting the default token lifetime as a constant.The default value
"3600s"is hardcoded in the function signature. For better maintainability and clarity, consider defining it as a package-level constant.Apply this diff:
+const ( + googleAuthPlugin = "gcp-cloudlist" + scope = "https://www.googleapis.com/auth/cloud-platform" + defaultTokenLifetime = "3600s" +)func register(ctx context.Context, serviceAccountKey []byte) (option.ClientOption, error) { - return registerWithOptions(ctx, serviceAccountKey, false, "", "", "3600s") + return registerWithOptions(ctx, serviceAccountKey, false, "", "", defaultTokenLifetime) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/providers/gcp/auth.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/providers/gcp/auth.go (1)
pkg/providers/gcp/gcp.go (1)
New(184-202)
⏰ 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). (6)
- GitHub Check: Lint Test
- GitHub Check: Test Builds (1.22.x, ubuntu-latest)
- GitHub Check: Test Builds (1.22.x, windows-latest)
- GitHub Check: Test Builds (1.22.x, macOS-latest)
- GitHub Check: release-test
- GitHub Check: Analyze (go)
🔇 Additional comments (7)
pkg/providers/gcp/auth.go (7)
1-24: LGTM! Import additions and plugin name fix look good.The new imports support the short-lived credentials functionality, and the plugin name change to
gcp-cloudlistresolves the previous collision with client-go's built-in GCP auth provider.
112-129: LGTM! Duration parsing implementation is clean and flexible.The function correctly handles both seconds format ("3600s") and standard Go duration format ("1h", "30m"), with clear error messages.
131-152: LGTM! Configuration validation is thorough.The validation logic correctly enforces requirements for short-lived credentials and provides clear error messages when configuration is invalid.
158-201: LGTM! Credential source selection logic is well-structured.The three-tier credential source precedence (explicit path → service account key → ADC) is implemented correctly with appropriate logging and error handling for each path. The fallback to ADC aligns with GCP best practices.
203-229: LGTM! Short-lived token source configuration is robust.The implementation correctly:
- Wraps the short-lived source with
ReuseTokenSourcefor automatic caching/refresh- Tests token generation immediately to catch errors early
- Provides clear logging for both configuration modes
231-243: LGTM! Plugin registration error handling is correct.The error handling now properly checks for the "was registered twice" error message and wraps other errors appropriately. The debug logging for duplicate registrations is helpful for troubleshooting.
245-252: No changes required – both branches return option.ClientOption
All GCP client constructors acceptoption.ClientOption, so bothWithTokenSourceandWithCredentialswork as expected.
* feat: add GCP short-lived credentials support - Support service account impersonation with configurable token lifetime - Add authentication via ADC, static keys, or separate source credentials - Include comprehensive documentation and examples - Maintain full backward compatibility with existing configurations * Limit GCP short-lived token lifetime to 1 hour max * Add validation for short-lived GCP credentials config * Update auth.go
Closes #707
Summary by CodeRabbit
New Features
Documentation
Tests
Chores