-
Notifications
You must be signed in to change notification settings - Fork 319
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
[feat] ecr-credential-provider support to authenticate public registries #603
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,24 +30,34 @@ import ( | |
"github.com/aws/aws-sdk-go/aws" | ||
"github.com/aws/aws-sdk-go/aws/session" | ||
"github.com/aws/aws-sdk-go/service/ecr" | ||
"github.com/aws/aws-sdk-go/service/ecrpublic" | ||
|
||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"k8s.io/klog/v2" | ||
"k8s.io/kubelet/pkg/apis/credentialprovider/v1" | ||
) | ||
|
||
const ecrPublicRegion string = "us-east-1" | ||
const ecrPublicURL string = "public.ecr.aws" | ||
|
||
var ecrPattern = regexp.MustCompile(`^(\d{12})\.dkr\.ecr(\-fips)?\.([a-zA-Z0-9][a-zA-Z0-9-_]*)\.(amazonaws\.com(\.cn)?|sc2s\.sgov\.gov|c2s\.ic\.gov)$`) | ||
|
||
// ECR abstracts the calls we make to aws-sdk for testing purposes | ||
type ECR interface { | ||
GetAuthorizationToken(input *ecr.GetAuthorizationTokenInput) (*ecr.GetAuthorizationTokenOutput, error) | ||
} | ||
|
||
// ECRPublic abstracts the calls we make to aws-sdk for testing purposes | ||
type ECRPublic interface { | ||
GetAuthorizationToken(input *ecrpublic.GetAuthorizationTokenInput) (*ecrpublic.GetAuthorizationTokenOutput, error) | ||
} | ||
|
||
type ecrPlugin struct { | ||
ecr ECR | ||
ecr ECR | ||
ecrPublic ECRPublic | ||
} | ||
|
||
func defaultECRProvider(region string, registryID string) (*ecr.ECR, error) { | ||
func defaultECRProvider(region string) (*ecr.ECR, error) { | ||
sess, err := session.NewSessionWithOptions(session.Options{ | ||
Config: aws.Config{Region: aws.String(region)}, | ||
SharedConfigState: session.SharedConfigEnable, | ||
|
@@ -59,14 +69,66 @@ func defaultECRProvider(region string, registryID string) (*ecr.ECR, error) { | |
return ecr.New(sess), nil | ||
} | ||
|
||
func (e *ecrPlugin) GetCredentials(ctx context.Context, image string, args []string) (*v1.CredentialProviderResponse, error) { | ||
func publicECRProvider() (*ecrpublic.ECRPublic, error) { | ||
// ECR public registries are only in one region and only accessible from regions | ||
// in the "aws" partition. | ||
sess, err := session.NewSessionWithOptions(session.Options{ | ||
Config: aws.Config{Region: aws.String(ecrPublicRegion)}, | ||
SharedConfigState: session.SharedConfigEnable, | ||
}) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return ecrpublic.New(sess), nil | ||
} | ||
|
||
type credsData struct { | ||
registry string | ||
authToken *string | ||
expiresAt *time.Time | ||
} | ||
|
||
func (e *ecrPlugin) getPublicCredsData() (*credsData, error) { | ||
klog.Infof("Getting creds for public registry") | ||
var err error | ||
|
||
if e.ecrPublic == nil { | ||
e.ecrPublic, err = publicECRProvider() | ||
} | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
output, err := e.ecrPublic.GetAuthorizationToken(&ecrpublic.GetAuthorizationTokenInput{}) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
if output == nil { | ||
return nil, errors.New("response output from ECR was nil") | ||
} | ||
|
||
if output.AuthorizationData == nil { | ||
return nil, errors.New("authorization data was empty") | ||
} | ||
|
||
return &credsData{ | ||
registry: ecrPublicURL, | ||
authToken: output.AuthorizationData.AuthorizationToken, | ||
expiresAt: output.AuthorizationData.ExpiresAt, | ||
}, nil | ||
} | ||
|
||
func (e *ecrPlugin) getPrivateCredsData(image string) (*credsData, error) { | ||
klog.Infof("Getting creds for private registry %s", image) | ||
registryID, region, registry, err := parseRepoURL(image) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
if e.ecr == nil { | ||
e.ecr, err = defaultECRProvider(region, registryID) | ||
e.ecr, err = defaultECRProvider(region) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
@@ -87,12 +149,32 @@ func (e *ecrPlugin) GetCredentials(ctx context.Context, image string, args []str | |
return nil, errors.New("authorization data was empty") | ||
} | ||
|
||
data := output.AuthorizationData[0] | ||
if data.AuthorizationToken == nil { | ||
return &credsData{ | ||
registry: registry, | ||
authToken: output.AuthorizationData[0].AuthorizationToken, | ||
expiresAt: output.AuthorizationData[0].ExpiresAt, | ||
}, nil | ||
} | ||
|
||
func (e *ecrPlugin) GetCredentials(ctx context.Context, image string, args []string) (*v1.CredentialProviderResponse, error) { | ||
var creds *credsData | ||
var err error | ||
|
||
if strings.Contains(image, ecrPublicURL) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ya, I had considered that. However, this logic made me this that it could have
It could be that the if statement is overly defensive, but I took the same approach here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍🏾 |
||
creds, err = e.getPublicCredsData() | ||
} else { | ||
creds, err = e.getPrivateCredsData(image) | ||
} | ||
|
||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
if creds.authToken == nil { | ||
return nil, errors.New("authorization token in response was nil") | ||
} | ||
|
||
decodedToken, err := base64.StdEncoding.DecodeString(aws.StringValue(data.AuthorizationToken)) | ||
decodedToken, err := base64.StdEncoding.DecodeString(aws.StringValue(creds.authToken)) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
@@ -102,13 +184,13 @@ func (e *ecrPlugin) GetCredentials(ctx context.Context, image string, args []str | |
return nil, errors.New("error parsing username and password from authorization token") | ||
} | ||
|
||
cacheDuration := getCacheDuration(data.ExpiresAt) | ||
cacheDuration := getCacheDuration(creds.expiresAt) | ||
|
||
return &v1.CredentialProviderResponse{ | ||
CacheKeyType: v1.RegistryPluginCacheKeyType, | ||
CacheDuration: cacheDuration, | ||
Auth: map[string]v1.AuthConfig{ | ||
registry: { | ||
creds.registry: { | ||
Username: parts[0], | ||
Password: parts[1], | ||
}, | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
@jlbutler any comments about reachability of ECR public?
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.
AFAIK, this is the only region that has an ECR public endpoint. While the endpoint would be reachable from any non-isolated partitions, it won't be able to make authenticated calls from other partitions because you'd need a partition-specific endpoint so that the IAM creds work correctly. If there are endpoints in any other partitions, I can add support for those here.
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.
Correct, for the purposes here (using the SDK to make API calls for auth) us-east-1 is the region
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 @jlbutler