Skip to content
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

Add support for pulling ID Tokens from the metadata server #8

Merged
merged 3 commits into from Feb 16, 2022

Conversation

sethvargo
Copy link
Member

This adds support for cloud-run-proxy to pull ID Tokens from the metadata server instead of always assuming gcloud. This means it will work on a GCE VM or Cloud Run service.

However, this requires a user to specify an audience value for the JWT. When using the gcloud token, Cloud Run trusts the gcloud client IDs as valid aud values, but the only truly accepted value is the URL of the server. That's fine - we have the URL of the service because we need it to proxy, but it does introduce an edge case where a Cloud Run service is fronted by a Load Balancer and the Load Balancer is serving a vanity URL. In this case, the user must specify the "host" value as the Load Balancer DNS entry, but the "audience" value must be the .run.app URL.

This adds support for cloud-run-proxy to pull ID Tokens from the metadata server instead of always assuming gcloud. This means it will work on a GCE VM or Cloud Run service.

However, this requires a user to specify an audience value for the JWT. When using the gcloud token, Cloud Run trusts the gcloud client IDs as valid aud values, but the only truly accepted value is the URL of the server. That's fine - we have the URL of the service because we need it to proxy, but it does introduce an edge case where a Cloud Run service is fronted by a Load Balancer and the Load Balancer is serving a vanity URL. In this case, the user must specify the "host" value as the Load Balancer DNS entry, but the "audience" value must be the .run.app URL.
main.go Show resolved Hide resolved
main.go Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
main.go Show resolved Hide resolved
@sethvargo
Copy link
Member Author

@yanweiguo updated

main.go Outdated Show resolved Hide resolved
@yanweiguo
Copy link
Member

Left one more comment. LGTM

@sethvargo sethvargo merged commit 263984c into main Feb 16, 2022
@sethvargo sethvargo deleted the sethvargo/both branch February 16, 2022 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants