Add support for public ecr #391

fer1592 · 2025-07-08T20:43:53Z

Following PR adds support for images hosted in the AWS public registry, aims to solve the following issue. WIP

Signed-off-by: Fernando Perez Osan <[email protected]>

davidcollom

Appreciate the draft PR 👍 - If you're looking for an existing public ECR - we have a deployed version-checker ECR for E2E testing @ public.ecr.aws/c3x5s3k7/public-8456b5e0/alpine which clones alpine:latest

pkg/client/ecrpublic/ecr.go

fer1592 · 2025-07-25T20:48:01Z

Quick update about this, using the ecrpublic seems that doesn't work for images on the public.ecr.aws registry. I did a quick test using the http client similarly on how the docker client does, and works well. I'll be updating this PR next week probably (need to clean up all the crap code that I got)

fer1592 · 2025-08-08T20:15:40Z

This should be ready for review. Like mentioned before, I stopped using the public ecr library and just performed http requests for it. I tested it with a couple of images, and seems to be working fine. Eg: for kube-proxy

Let me know what you think!

davidcollom · 2025-11-14T09:09:10Z

@fer1592 Sorry it's taken so long to get back to this 🙈 - I was curious as to if there was any reasoning around using the OCR Library? or https://github.com/google/go-containerregistry - as a project, we're trying to move away from managing too many client implementations and focusing on the authN/Z portion to the go-containerregistry or native libraries.

This is mainly due to maintenance and testing. We don't have a full E2E testing framework for all of the clients, which can make maintaining them difficult and we rely on the community to report issues and raise PR's to resolve issues.

fer1592 · 2025-11-28T16:16:32Z

Sorry, been some busy weeks and I forgot to reply! Basically the reasoning was to replicate the same thing than the dockerClient, mostly thinking about maintenance really (usually I avoid using different libraries from the ones used unless it's completely necessary). Since public ECR has some small differences than docker, eg. you need to request a token first to an AWS endpoint before being able to reach ECR), paths are different as well as the response from ECR, it felt like the right move to just replicate the same thing as the dockerClient with the required modifications.

Copilot

Pull request overview

This PR adds support for AWS Elastic Container Registry (ECR) Public, enabling version-checker to query image tags from the public.ecr.aws registry. This extends the existing ECR private registry support to handle the publicly accessible AWS container registry.

Introduces a new ecrpublic client package following the existing client implementation pattern
Adds anonymous token-based authentication for ECR Public API access
Integrates the new client into the main client registry alongside other providers

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
pkg/client/ecrpublic/types.go	Defines data structures for ECR Public API responses (AuthResponse and TagResponse)
pkg/client/ecrpublic/path.go	Implements host matching (public.ecr.aws) and path parsing for repository/image extraction
pkg/client/ecrpublic/path_test.go	Tests for host detection and path parsing logic
pkg/client/ecrpublic/ecrpublic.go	Main client implementation with tag fetching, request handling, and anonymous authentication
pkg/client/client.go	Registers the ECRPublic client in the main client factory and configuration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-09T10:01:10Z

pkg/client/client.go

 	ACR        acr.Options
 	Docker     docker.Options
 	ECR        ecr.Options
+	ECRPublic  ecrpublic.Options // Add ECRPublic options


[nitpick] Remove the inline comment "Add ECRPublic options". Comments that simply describe what the code obviously does add no value and should be removed. The field name ECRPublic is self-documenting.

Suggested change

ECRPublic ecrpublic.Options // Add ECRPublic options

ECRPublic ecrpublic.Options

Copilot · 2025-12-09T10:01:10Z

pkg/client/client.go

 	if opts.Transport != nil {
 		opts.Quay.Transporter = opts.Transport
 		opts.ECR.Transporter = opts.Transport
+		opts.ECRPublic.Transporter = opts.Transport // Add ECR public transporter


[nitpick] Remove the inline comment "Add ECR public transporter". The code is self-explanatory and the comment adds no value.

Suggested change

opts.ECRPublic.Transporter = opts.Transport // Add ECR public transporter

opts.ECRPublic.Transporter = opts.Transport

Copilot · 2025-12-09T10:01:11Z

pkg/client/ecrpublic/ecrpublic.go

+}
+
+func (c *Client) Tags(ctx context.Context, _, repo, image string) ([]api.ImageTag, error) {
+	url := fmt.Sprintf(ecrPublicLookupURL, repo, image)


When image is empty (as in the test case "nginx" where repo="nginx" and image=""), the URL will be constructed as https://public.ecr.aws/v2/nginx//tags/list with a double slash. This may not be the correct format for the ECR Public API. Verify that this URL format works correctly, or handle the case where image is empty by constructing the URL differently, e.g., https://public.ecr.aws/v2/nginx/tags/list.

Suggested change

url := fmt.Sprintf(ecrPublicLookupURL, repo, image)

var url string

if image == "" {

url = fmt.Sprintf("https://public.ecr.aws/v2/%s/tags/list", repo)

} else {

url = fmt.Sprintf(ecrPublicLookupURL, repo, image)

}

Copilot · 2025-12-09T10:01:11Z

pkg/client/ecrpublic/ecrpublic.go

+	*http.Client
+	Options
+}
+


Missing compile-time interface assertion. All other client implementations in the codebase include var _ api.ImageClient = (*Client)(nil) to ensure the Client type implements the api.ImageClient interface at compile time. This should be added at the package level (typically after the Client struct definition).

Suggested change

var _ api.ImageClient = (*Client)(nil)

Copilot · 2025-12-09T10:01:11Z

pkg/client/ecrpublic/ecrpublic.go

+func (c *Client) Tags(ctx context.Context, _, repo, image string) ([]api.ImageTag, error) {
+	url := fmt.Sprintf(ecrPublicLookupURL, repo, image)
+
+	var tags []api.ImageTag
+	for url != "" {
+		response, err := c.doRequest(ctx, url)
+		if err != nil {
+			return nil, err
+		}
+
+		for _, tag := range response.Tags {
+			// No images in this result, so continue early
+			if len(tag) == 0 {
+				continue
+			}
+
+			tags = append(tags, api.ImageTag{
+				Tag: tag,
+			})
+		}
+
+		url = response.Next
+	}
+
+	return tags, nil
+}
+
+func (c *Client) doRequest(ctx context.Context, url string) (*TagResponse, error) {
+	req, err := http.NewRequest(http.MethodGet, url, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	// Always get a token for ECR Public
+	token, err := getAnonymousToken(ctx, c.Client)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get anonymous token: %s", err)
+	}
+
+	req.URL.Scheme = "https"
+	req = req.WithContext(ctx)
+	if len(token) > 0 {
+		req.Header.Add("Authorization", "Bearer "+token)
+	}
+
+	resp, err := c.Do(req)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get %q image: %s", c.Name(), err)
+	}
+	defer func() { _ = resp.Body.Close() }()
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	response := new(TagResponse)
+	if err := json.Unmarshal(body, response); err != nil {
+		return nil, fmt.Errorf("unexpected image tags response: %s", body)
+	}
+
+	return response, nil
+}
+
+func getAnonymousToken(ctx context.Context, client *http.Client) (string, error) {
+	req, err := http.NewRequest(http.MethodGet, loginURL, nil)
+	if err != nil {
+		return "", err
+	}
+
+	req = req.WithContext(ctx)
+
+	resp, err := client.Do(req)
+	if err != nil {
+		return "", err
+	}
+	defer func() { _ = resp.Body.Close() }()
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return "", err
+	}
+
+	if resp.StatusCode != http.StatusOK {
+		return "", errors.New(string(body))
+	}
+
+	response := new(AuthResponse)
+	if err := json.Unmarshal(body, response); err != nil {
+		return "", err
+	}
+
+	return response.Token, nil
+}


The main client functionality in ecrpublic.go lacks test coverage. While path_test.go tests IsHost() and RepoImageFromPath(), there are no tests for the core functions Tags(), doRequest(), and getAnonymousToken(). Other client implementations like docker and ghcr include comprehensive tests for these core functions. Consider adding tests similar to those in pkg/client/docker/docker_test.go that use httptest to mock API responses.

Copilot · 2025-12-09T10:01:12Z

pkg/client/ecrpublic/ecrpublic.go

+	resp, err := c.Do(req)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get %q image: %s", c.Name(), err)
+	}
+	defer func() { _ = resp.Body.Close() }()
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	response := new(TagResponse)
+	if err := json.Unmarshal(body, response); err != nil {
+		return nil, fmt.Errorf("unexpected image tags response: %s", body)
+	}


The doRequest function doesn't check the HTTP status code before attempting to unmarshal the response body. If the API returns an error status (e.g., 404, 401, 500), the function will try to unmarshal the error response as a TagResponse, which will likely fail with a confusing error message. Consider adding a status code check similar to getAnonymousToken at line 142:

if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("failed to get tags, status code: %d, body: %s", resp.StatusCode, body) }

Copilot · 2025-12-09T10:01:12Z

pkg/client/ecrpublic/path_test.go

+		path              string
+		expRepo, expImage string
+	}{
+		"single image should return registry and image": {


The test name says "single image should return registry and image" but the expected expImage is empty. This is confusing and the test name doesn't match the expectation. Consider renaming to something like "single segment path should return repo with empty image" to better reflect what's being tested.

Suggested change

"single image should return registry and image": {

"single segment path should return repo with empty image": {

Copilot · 2025-12-09T10:01:13Z

pkg/client/ecrpublic/ecrpublic.go

+	// Always get a token for ECR Public
+	token, err := getAnonymousToken(ctx, c.Client)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get anonymous token: %s", err)


The error message formatting is inconsistent with the error returned from getAnonymousToken. The error should use %w instead of %s to properly wrap the error, allowing error unwrapping with errors.Is() and errors.As(). Change to: fmt.Errorf("failed to get anonymous token: %w", err)

Suggested change

return nil, fmt.Errorf("failed to get anonymous token: %s", err)

return nil, fmt.Errorf("failed to get anonymous token: %w", err)

Copilot · 2025-12-09T10:01:13Z

pkg/client/ecrpublic/ecrpublic.go

+
+	resp, err := c.Do(req)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get %q image: %s", c.Name(), err)


Similarly, this error should use %w instead of %s to properly wrap the error. Change to: fmt.Errorf("failed to get %q image: %w", c.Name(), err)

Suggested change

return nil, fmt.Errorf("failed to get %q image: %s", c.Name(), err)

return nil, fmt.Errorf("failed to get %q image: %w", c.Name(), err)

Add support for public ecr

c8be71c

Signed-off-by: Fernando Perez Osan <[email protected]>

fer1592 requested a review from davidcollom as a code owner July 8, 2025 20:43

fer1592 marked this pull request as draft July 8, 2025 20:44

fer1592 mentioned this pull request Jul 8, 2025

[BUG] Problem checking tags on ECR public repository #390

Closed

davidcollom reviewed Jul 9, 2025

View reviewed changes

pkg/client/ecrpublic/ecr.go Outdated Show resolved Hide resolved

davidcollom added the enhancement New feature or request label Jul 17, 2025

Don't use ecrpublic library, adopt approach similar to docker

3bd6b18

fer1592 marked this pull request as ready for review August 8, 2025 20:09

Merge branch 'main' into issue-390-publicecr

eaf2aa7

Merge branch 'main' into issue-390-publicecr

16e18e7

Merge branch 'main' into issue-390-publicecr

d33d50c

davidcollom requested a review from maria-reynoso as a code owner November 14, 2025 09:09

davidcollom requested a review from Copilot December 9, 2025 09:55

Copilot started reviewing on behalf of davidcollom December 9, 2025 09:55 View session

Copilot AI reviewed Dec 9, 2025

View reviewed changes

	ECRPublic ecrpublic.Options // Add ECRPublic options
	ECRPublic ecrpublic.Options

	opts.ECRPublic.Transporter = opts.Transport // Add ECR public transporter
	opts.ECRPublic.Transporter = opts.Transport

-	url := fmt.Sprintf(ecrPublicLookupURL, repo, image)
+	var url string
+	if image == "" {
+		url = fmt.Sprintf("https://public.ecr.aws/v2/%s/tags/list", repo)
+	} else {
+		url = fmt.Sprintf(ecrPublicLookupURL, repo, image)
+	}

	"single image should return registry and image": {
	"single segment path should return repo with empty image": {

	return nil, fmt.Errorf("failed to get anonymous token: %s", err)
	return nil, fmt.Errorf("failed to get anonymous token: %w", err)

	return nil, fmt.Errorf("failed to get %q image: %s", c.Name(), err)
	return nil, fmt.Errorf("failed to get %q image: %w", c.Name(), err)

Add support for public ecr #391

Are you sure you want to change the base?

Add support for public ecr #391

Uh oh!

Conversation

fer1592 commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidcollom left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fer1592 commented Jul 25, 2025

Uh oh!

fer1592 commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidcollom commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fer1592 commented Nov 28, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fer1592 commented Jul 8, 2025 •

edited

Loading

fer1592 commented Aug 8, 2025 •

edited

Loading

davidcollom commented Nov 14, 2025 •

edited

Loading