Handle private ECR image references containing public.ecr.aws

[cartermckinnon]

What type of PR is this?

/kind bug

What this PR does / why we need it:

When support was added for public.ecr.aws images, a simple string.Contains was used to determine whether a requested image reference was a private or public ECR repository. If you use public.ecr.aws anywhere in your image reference, such as:

123456789123.dkr.ecr.us-west-2.amazonaws.com/public.ecr.aws/foo:latest

Then public ECR creds will be retrieved, and image pulls will fail (or you'll fail to call ecr-public::GetAuthorizationToken if your IAM creds don't have permissions for it).

Which issue(s) this PR fixes:

Fixes #651

Special notes for your reviewer:

I did some cleanup while I was at it:

• I removed the RegistryIds parameter to GetAuthorizationTokenInput. This field is deprecated and has no effect on the token returned.

Does this PR introduce a user-facing change?:

Fixes an issue with private ECR image references that contain `public.ecr.aws` being misidentified as public ECR repositories.

[k8s-ci-robot]

This issue is currently awaiting triage.

If cloud-provider-aws contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[cartermckinnon]

/assign mmerkes

[k8s-ci-robot]

@cartermckinnon: GitHub didn't allow me to assign the following users: mmerkes.

Note that only kubernetes members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign mmerkes

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[cartermckinnon]

@mmerkes please review?

[cartermckinnon]

/assign tzneal

[cartermckinnon]

This is the bug.

[mmerkes]

Overall, the change LGTM. What testing did you do on this? At a minimum, we should verify that this can pull a public and private image.

[cartermckinnon]

I sanity checked this with the log output:

> echo '{
  "kind":"CredentialProviderRequest",
  "apiVersion":"credentialprovider.kubelet.k8s.io/v1",
  "image":"123456789123.dkr.ecr.us-west-2.amazonaws.com/public.ecr.aws/foo"
}' | ./ecr-credential-provider

I0919 09:33:41.036509   63515 main.go:125] Getting creds for private image 123456789123.dkr.ecr.us-west-2.amazonaws.com/public.ecr.aws/foo
{"kind":"CredentialProviderResponse","apiVersion":"credentialprovider.kubelet.k8s.io/v1","cacheKeyType":"Registry","cacheDuration":"6h0m0s","auth":{"123456789123.dkr.ecr.us-west-2.amazonaws.com":{"username":"AWS","password":"<secret>"}}}

> echo '{
  "kind":"CredentialProviderRequest",
  "apiVersion":"credentialprovider.kubelet.k8s.io/v1",
  "image":"public.ecr.aws/foo"
}' | ./ecr-credential-provider

I0919 09:34:36.638610   63660 main.go:94] Getting creds for public registry
{"kind":"CredentialProviderResponse","apiVersion":"credentialprovider.kubelet.k8s.io/v1","cacheKeyType":"Registry","cacheDuration":"6h0m0s","auth":{"public.ecr.aws":{"username":"AWS","password":"<secret>"}}}

[mmerkes]

LGTM

[k8s-ci-robot]

@mmerkes: changing LGTM is restricted to collaborators

In response to this:

LGTM

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[cartermckinnon]

/assign @nckturner

can you give this a look Nick?

[tzneal]

/lgtm

[cartermckinnon]

@dims PTAL?

[dims]

/approve
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, mmerkes, tzneal

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [dims]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[matty-rose]

hey @cartermckinnon we ran into this issue quite recently, super nice to see the fix merged in already 🎉 - do you have an idea on the cadence/ETA that the ecr-credential-provider binary with this fix will be incorporated into the EKS AMIs?