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

plugin/rest: Add GCP metadata server support #2938

Merged

Conversation

kelseyhightower
Copy link
Contributor

Adds support for fetching access and identity tokens from a GCP
metadata server. Identity tokens are used to authenticate to third
party applications running behind Google authentication proxies
such as containers deployed to Google's Cloud Run.

Access tokens are used to authenticate to first party GCP services
such as Google Cloud Storage.

Signed-off-by: Kelsey Hightower kelsey.hightower@gmail.com

Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 Thanks a lot 🎉

Mostly questions inline, please bear with me. 🙃

docs/content/configuration.md Outdated Show resolved Hide resolved
plugins/rest/gcp.go Outdated Show resolved Hide resolved
plugins/rest/rest_test.go Outdated Show resolved Hide resolved

request.Header.Add("Metadata-Flavor", "Google")

timeout := time.Duration(5) * time.Second
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hardcoded 5s looks weird. I suppose we could also use c.ResponseHeaderTimeoutSeconds? https://github.com/open-policy-agent/opa/pull/2938/files#diff-99a9ce78de44f106d9f1ade633bb6c6f425af0e6704663032abb475bb00f3d37R83

Not worse than

opa/plugins/rest/aws.go

Lines 308 to 309 in 5c6c963

// construct an HTTP client with a reasonably short timeout
client := &http.Client{Timeout: time.Second * 10}
anyways 😅

I guess there's some potential for improving the timeout config here cross the different plugins. Outside of the scope of this PR, of course.

plugins/rest/gcp.go Show resolved Hide resolved
plugins/rest/gcp.go Show resolved Hide resolved
@kelseyhightower kelseyhightower force-pushed the gcp-metadata-auth branch 3 times, most recently from 94540e6 to 459ebd0 Compare November 23, 2020 17:06
srenatus
srenatus previously approved these changes Nov 23, 2020
Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for addressing my nitpicks. LGTM 😃

Adds support for fetching access and identity tokens from a GCP
metadata server. Identity tokens are used to authenticate to third
party applications running behind Google authentication proxies
such as containers deployed to Google's Cloud Run.

Access tokens are used to authenticate to first party GCP services
such as Google Cloud Storage.

Signed-off-by: Kelsey Hightower <kelsey.hightower@gmail.com>
Copy link
Member

@tsandall tsandall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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.

3 participants