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 GCR default credentials #1120

Merged
merged 2 commits into from
Sep 25, 2020

Conversation

viktorradnai
Copy link
Contributor

@viktorradnai viktorradnai commented Sep 25, 2020

Q A
Bug fix? no
New feature? yes
API breaks? no
Deprecations? no
Related tickets partially #966
License Apache 2.0

What's in this PR?

This PR adds support for authenticating against Google Container Registry without explicit username / password.

It works similarly to the already present Amazon ECR support: If there are no username / password / imagePullSecrets specified, then it checks the registry hostname against the known GCR hotsts. If the pattern matches then it tries to obtain an auth token from the metadata service endpoint when running inside Google Cloud (which also works with workload identity if running in GKE).

Why?

Using default credentials avoids the need for pre-shared credentials (passwords or manually generated service account tokens) which would need to be managed.

Additional context

Checklist

  • Code meets the Developer Guide
  • User guide and development docs updated (if needed)

To Do

This PR does not address authenticating with an user supplied Google service account token (although that may be possible using the username/password mechanism but was not tested).

@CLAassistant
Copy link

CLAassistant commented Sep 25, 2020

CLA assistant check
All committers have signed the CLA.

@@ -52,6 +55,9 @@ func init() {

// Adapted from https://github.com/awslabs/amazon-ecr-credential-helper/blob/master/ecr-login/api/client.go#L34
ecrHostPattern = regexp.MustCompile(`([a-zA-Z0-9][a-zA-Z0-9-_]*)\.dkr\.ecr(\-fips)?\.([a-zA-Z0-9][a-zA-Z0-9-_]*)\.amazonaws\.com(\.cn)?`)

// From https://cloud.google.com/container-registry/docs/overview
gcrHostPattern = regexp.MustCompile(`((us|eu|asia)\.)?gcr.io`)
Copy link
Member

Choose a reason for hiding this comment

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

Code scanning just gave some hints here about the unescaped dot, which is highly unlikely to cause any issues, but we probably should escape it ;)

bonifaido
bonifaido previously approved these changes Sep 25, 2020
Copy link
Member

@bonifaido bonifaido left a comment

Choose a reason for hiding this comment

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

LGTM, other than the small code scanning hint. Thank you @viktorradnai !

@bonifaido
Copy link
Member

Once this gets merged, we should add it to the docs, that this works automatically for GCR as well here: https://github.com/banzaicloud/bank-vaults-docs/blob/12b992e5262acad355767a72371d3449a408960a/docs/mutating-webhook/_index.md#registry-access

Copy link
Member

@bonifaido bonifaido left a comment

Choose a reason for hiding this comment

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

👍

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

3 participants