-
Notifications
You must be signed in to change notification settings - Fork 894
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
Added support for access tokens in gcpkms #1358
Added support for access tokens in gcpkms #1358
Conversation
797ef4f
to
26f3545
Compare
7a7136f
to
ec9a4a7
Compare
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.
Thanks for contributing @christoffer-eide 🥇
Looks good to me. Can you quickly fix the merge conflicts?
cc @getsops/maintainers if anyone else wants to take a look
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.
(Meant to comment, not approve yet)
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.
Can you add a note to the GCP KMS docs for others to discover this feature?
Hey @christoffer-eide 👋 - someone else just opened a PR #1578 for the same functionality. Are you able to address the comments, so we can get this merged? |
ec9a4a7
to
6c23bb9
Compare
Signed-off-by: Christoffer Eide <[email protected]>
Signed-off-by: Christoffer Eide <[email protected]>
6c23bb9
to
242b218
Compare
I'm so sorry, I completely forgot about this 😱 Regarding the proposed change in #1578, it would make it more convenient to have a separate env var for setting the access token. It would interact better with the CLOUDSDK_AUTH_ACCESS_TOKEN ="$(gcloud auth print-access-token)" vs GOOGLE_CREDENTIALS ="$(gcloud auth print-access-token --format json | jq '{"access_token": .token}')" If you think that #1578 is a better solution, this PR can be closed without merging. |
All good @christoffer-eide! I agree that using a separate env var Appreciate the contribution! Closing in favor of #1578 |
This PR adds support for access token (via the
GOOGLE_CREDENTIALS
env var).If the env var
GOOGLE_CREDENTIALS
is not set, the gcloud sdk fetches an access token from the instance metadata endpointhttp://169.254.169.254/computeMetadata/v1/instance/service-accounts/default/token
.In our workload on GKE, we don't want to use the access token returned by the metadata endpoint directly. We use this access token to impersonate another service account, which has the minimum of permissions required for the kms decrypt.
There is no facility to set access tokens in sops, only static (long lived) credentials via the credentials file/
GOOGLE_CREDENTIALS
.