-
Notifications
You must be signed in to change notification settings - Fork 8
Expose GCP secrets key IDs #14
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Mario Weigel <[email protected]>
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 adds support for exposing GCP service account private key IDs across mount configuration, rolesets, and static accounts. This addresses issue #13 by providing visibility into which key is currently active when reading configurations and returning key IDs when rotating credentials.
Key changes include:
- Added private_key_id field to read operations for mount config, rolesets, and static accounts
- Modified rotate operations to return the new private_key_id after successful rotation
- Updated test expectations to include the new private_key_id field
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| secrets/gcp/path_config.go | Enhanced config read to extract and return private_key_id from credentials |
| secrets/gcp/path_static_account.go | Added private_key_id to static account read response |
| secrets/gcp/path_static_account_rotate_key.go | Modified rotate response to include new private_key_id |
| secrets/gcp/path_role_set.go | Added private_key_id to roleset read and rotate responses |
| secrets/gcp/path_config_test.go | Updated test expectations to include private_key_id field |
| creds, err := gcputil.Credentials(cfg.CredentialsRaw) | ||
| if err != nil { | ||
| resp.Warnings = []string{ | ||
| fmt.Sprintf("Failed to parse key private key ID: %v", err), |
Copilot
AI
Jul 20, 2025
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 error message contains a grammatical error. It should be "Failed to parse private key ID" instead of "Failed to parse key private key ID".
| fmt.Sprintf("Failed to parse key private key ID: %v", err), | |
| fmt.Sprintf("Failed to parse private key ID: %v", err), |
| } | ||
|
|
||
| creds, err := gcputil.Credentials(cfg.CredentialsRaw) | ||
| if err != nil { |
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.
Returning the Private Key ID will only work when the user passes the service account credentials in the credentials settings. From my experience it is much more common to use the default credentials passed to an application running in GCP as described in our docs.
I'd prefer it if this only returns an error if cfg.CredentialsRaw is not empty. Also I think this should try to parse the GOOGLE_APPLICATION_CREDENTIALS environment variable and use the key specified in there if it is not empty.
Attempt to address Expose GCP secrets key IDs #13
Mount operations
Static account operations
Roleset operations