WIP: ✨ docker: public.ecr.aws support #66

2025-09-01T17:56:24+08:00

Rirmach commented

2025-09-01 17:56:24 +08:00

(Migrated from github.com)

初步实现 public.ecr.aws 的认证逻辑

#65
因 public.ecr.aws 似乎不能通过 HEAD blobs 的方式获取 Layer 大小，因此暂时的解决方案是将 layer.Size() 的返回错误降级为 Warning，避免中断流程。此时下游拉取时将无法看到进度条。
一种解决方案是，使用 Range: bytes=0-0 的方式请求 bolb：

TOKEN=$(curl -s "https://public.ecr.aws/token/?service=public.ecr.aws&scope=repository:nginx/nginx:pull" | jq -r .token)
curl -L -H "Authorization: Bearer $TOKEN" -H "Range: bytes=0-0"  https://public.ecr.aws/v2/nginx/nginx/blobs/sha256:a2da0c0f2353a40d540821152b3b9453660db34259766b1ce68b0b1f708435fd -v

响应 206，响应头中包含 content-range：

* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
< HTTP/2 206 
< content-type: application/octet-stream
< content-length: 1
< last-modified: Thu, 14 Aug 2025 20:33:38 GMT
< x-amz-server-side-encryption: AES256
< x-amz-version-id: M9K2Ll2R5McB8Xyt3w9_2y5QQgMPaB9Z
< server: AmazonS3
< date: Mon, 01 Sep 2025 02:41:35 GMT
< etag: "3f060e5789388a0e5d243228fdc9f660-9"
< x-cache: Hit from cloudfront
< via: 1.1 a5a0710f4845ad00ea1f7ede6b345c78.cloudfront.net (CloudFront)
< x-amz-cf-pop: LAX50-P5
< alt-svc: h3=":443"; ma=86400
< x-amz-cf-id: Iq8_JRoQHAf89Td1PkCYJTzgRRoyHlcMY1JrhgAy_bxqpqaIik39Aw==
< age: 24719
< content-range: bytes 0-0/44068517

只需解析 content-range 即可获取 Layer size，但这个破坏了现有获取 layer size 的流程，后续可以考虑实现或者研究一下 HEAD 401 的原因

Summary by CodeRabbit

New Features
- Added built-in support for AWS Public ECR (public.ecr.aws), including automatic token-based authentication.
- Enabled default registry mapping for public.ecr.aws.
Improvements
- More resilient layer downloads: continues streaming even if size cannot be determined.
- Smart auth handling: uses token when available and gracefully falls back to anonymous access.
Chores
- Updated dependencies and adjusted module references.

## 初步实现 public.ecr.aws 的认证逻辑 #65 因 public.ecr.aws 似乎不能通过 HEAD blobs 的方式获取 Layer 大小，因此暂时的解决方案是将 layer.Size() 的返回错误降级为 Warning，避免中断流程。此时下游拉取时将无法看到进度条。一种解决方案是，使用 Range: bytes=0-0 的方式请求 bolb： ```shell TOKEN=$(curl -s "https://public.ecr.aws/token/?service=public.ecr.aws&scope=repository:nginx/nginx:pull" | jq -r .token) curl -L -H "Authorization: Bearer $TOKEN" -H "Range: bytes=0-0" https://public.ecr.aws/v2/nginx/nginx/blobs/sha256:a2da0c0f2353a40d540821152b3b9453660db34259766b1ce68b0b1f708435fd -v ``` 响应 206，响应头中包含 content-range： ```log * TLSv1.3 (IN), TLS handshake, Newsession Ticket (4): < HTTP/2 206 < content-type: application/octet-stream < content-length: 1 < last-modified: Thu, 14 Aug 2025 20:33:38 GMT < x-amz-server-side-encryption: AES256 < x-amz-version-id: M9K2Ll2R5McB8Xyt3w9_2y5QQgMPaB9Z < server: AmazonS3 < date: Mon, 01 Sep 2025 02:41:35 GMT < etag: "3f060e5789388a0e5d243228fdc9f660-9" < x-cache: Hit from cloudfront < via: 1.1 a5a0710f4845ad00ea1f7ede6b345c78.cloudfront.net (CloudFront) < x-amz-cf-pop: LAX50-P5 < alt-svc: h3=":443"; ma=86400 < x-amz-cf-id: Iq8_JRoQHAf89Td1PkCYJTzgRRoyHlcMY1JrhgAy_bxqpqaIik39Aw== < age: 24719 < content-range: bytes 0-0/44068517 ``` 只需解析 content-range 即可获取 Layer size，但这个破坏了现有获取 layer size 的流程，后续可以考虑实现或者研究一下 HEAD 401 的原因  ## Summary by CodeRabbit * **New Features** * Added built-in support for AWS Public ECR (public.ecr.aws), including automatic token-based authentication. * Enabled default registry mapping for public.ecr.aws. * **Improvements** * More resilient layer downloads: continues streaming even if size cannot be determined. * Smart auth handling: uses token when available and gracefully falls back to anonymous access. * **Chores** * Updated dependencies and adjusted module references.

coderabbitai[bot] commented

2025-09-01 17:56:31 +08:00

(Migrated from github.com)

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Adds AWS ECR token-based auth support for upstream requests. Introduces conditional Bearer vs Anonymous auth selection at runtime. Adds a public ECR registry mapping in default config. Adjusts blob retrieval to proceed even if size fetch fails. Updates dependencies to include json-iterator directly and testify indirectly.

Changes

Cohort / File(s)	Summary
Config: Add public ECR registry mapping `src/config/config.go`	Adds a registry entry for "public.ecr.aws" with AuthType "aws-ecr", AuthHost "public.ecr.aws/token", Enabled true.
Dependencies: JSON and tests `src/go.mod`	Promotes `github.com/json-iterator/go v1.1.12` to direct dependency; adds `github.com/stretchr/testify v1.10.0` as indirect.
Docker handler: AWS ECR auth + blob flow tweak `src/handlers/docker.go`	Implements AWS ECR token retrieval (10s client, JSON parse via json-iterator), selects Bearer or Anonymous based on token, refactors upstream option creation, and continues blob streaming even if size lookup fails.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant P as Proxy
  participant T as Token Host (AuthHost)
  participant R as Upstream Registry

  Note over P: Request arrives (manifest/blob/tags)

  P->>P: Determine registry mapping
  alt AuthType == "aws-ecr"
    P->>T: GET https://{AuthHost}
    T-->>P: 200 JSON { token? }
    alt token present
      P->>P: Set Authorization: Bearer <token>
    else token missing/error
      P->>P: Use Anonymous auth
    end
  else other/none
    P->>P: Use configured auth (may be Anonymous)
  end

  P->>R: Upstream request (with chosen auth)
  alt Blob request
    P->>R: Try HEAD/size
    R-->>P: size or error
    P->>R: GET blob (continue even if size failed)
    R-->>P: Stream layer
  else Manifest/Tags
    R-->>P: Response payload
  end

  P-->>C: Proxy response

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

I nibbled code and fetched a token, neat,
From ECR’s burrow with a stealthy beat.
If Bearer’s found, I bound ahead—
If not, hop anon, no carrot shed.
Blobs may bloat, yet onward flows the stream—
A hare’s swift proxy, chasing the upstream dream. 🥕🐇

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

> [!IMPORTANT] > ## Review skipped > > Draft detected. > > Please check the settings in the CodeRabbit UI or the `.coderabbit.yaml` file in this repository. To trigger a single review, invoke the `@coderabbitai review` command. > > You can disable this status message by setting the `reviews.review_status` to `false` in the CodeRabbit configuration file.   ## Walkthrough Adds AWS ECR token-based auth support for upstream requests. Introduces conditional Bearer vs Anonymous auth selection at runtime. Adds a public ECR registry mapping in default config. Adjusts blob retrieval to proceed even if size fetch fails. Updates dependencies to include json-iterator directly and testify indirectly. ## Changes | Cohort / File(s) | Summary | |---|---| | **Config: Add public ECR registry mapping** `src/config/config.go` | Adds a registry entry for "public.ecr.aws" with AuthType "aws-ecr", AuthHost "public.ecr.aws/token", Enabled true. | | **Dependencies: JSON and tests** `src/go.mod` | Promotes `github.com/json-iterator/go v1.1.12` to direct dependency; adds `github.com/stretchr/testify v1.10.0` as indirect. | | **Docker handler: AWS ECR auth + blob flow tweak** `src/handlers/docker.go` | Implements AWS ECR token retrieval (10s client, JSON parse via json-iterator), selects Bearer or Anonymous based on token, refactors upstream option creation, and continues blob streaming even if size lookup fails. | ## Sequence Diagram(s) ```mermaid sequenceDiagram autonumber participant C as Client participant P as Proxy participant T as Token Host (AuthHost) participant R as Upstream Registry Note over P: Request arrives (manifest/blob/tags) P->>P: Determine registry mapping alt AuthType == "aws-ecr" P->>T: GET https://{AuthHost} T-->>P: 200 JSON { token? } alt token present P->>P: Set Authorization: Bearer <token> else token missing/error P->>P: Use Anonymous auth end else other/none P->>P: Use configured auth (may be Anonymous) end P->>R: Upstream request (with chosen auth) alt Blob request P->>R: Try HEAD/size R-->>P: size or error P->>R: GET blob (continue even if size failed) R-->>P: Stream layer else Manifest/Tags R-->>P: Response payload end P-->>C: Proxy response ``` ## Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes ## Poem > I nibbled code and fetched a token, neat, > From ECR’s burrow with a stealthy beat. > If Bearer’s found, I bound ahead— > If not, hop anon, no carrot shed. > Blobs may bloat, yet onward flows the stream— > A hare’s swift proxy, chasing the upstream dream. 🥕🐇   <details> <summary>✨ Finishing touches</summary> <details> <summary>🧪 Generate unit tests (beta)</summary> - [ ]  Create PR with unit tests - [ ]  Post copyable unit tests in a comment </details> </details>   --- Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. <details> <summary>❤️ Share</summary> - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code) </details> Comment `@coderabbitai help` to get the list of available commands and usage tips.

coderabbitai[bot] (Migrated from github.com) reviewed 2025-09-01 18:04:40 +08:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/handlers/docker.go (1)
545-552: Critical: 500 is written then streaming continues; Content-Length may be wrong.

On layer.Size() error you write a 500 body but then continue with 200 and stream data; also you still set Content-Length using an undefined/zero size, which will corrupt responses. Stream without Content-Length when size is unknown and do not write an error body.

Apply:
@@
-  size, err := layer.Size()
-  if err != nil {
-    fmt.Printf("获取layer大小失败: %v\n", err)
-    c.String(http.StatusInternalServerError, "Failed to get layer size")
-    // return
-  }
+  size, err := layer.Size()
+  if err != nil {
+    // Don't fail the request; stream without Content-Length to allow clients to pull.
+    fmt.Printf("获取layer大小失败，继续以未知长度流式传输: %v\n", err)
+  } else {
+    c.Header("Content-Length", fmt.Sprintf("%d", size))
+  }
@@
-  c.Header("Content-Type", "application/octet-stream")
-  c.Header("Content-Length", fmt.Sprintf("%d", size))
+  c.Header("Content-Type", "application/octet-stream")
   c.Header("Docker-Content-Digest", digest)
 
   c.Status(http.StatusOK)
   io.Copy(c.Writer, reader)
Also applies to: 560-566

🧹 Nitpick comments (2)

src/config/config.go (1)
136-141: public.ecr.aws mapping added — verify AuthHost shape to avoid double path.

Using AuthHost: "public.ecr.aws/token" matches the style of other entries, but proxyDockerAuthOriginal concatenates https://<AuthHost> + c.Request.URL.Path. If the incoming path is /token, this becomes https://public.ecr.aws/token/token. Confirm the intended join logic or change AuthHost to just the host and let the path come from the request.

If you decide to keep path in AuthHost, guard the join:
- authURL = "https://" + mapping.AuthHost + c.Request.URL.Path
+ base := strings.TrimRight(mapping.AuthHost, "/")
+ path := "/" + strings.TrimLeft(c.Request.URL.Path, "/")
+ authURL = "https://" + base + path
src/handlers/docker.go (1)

10-16: Import additions are fine; consider standardizing on one JSON lib.

You now have goccy/go-json (indirect), sonic (indirect), and json-iterator (direct). For maintainability, prefer one JSON library project-wide unless there’s a measured perf/compat need per call site.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0cd5a7334d and 13f24c9d26.

⛔ Files ignored due to path filters (1)

src/go.sum is excluded by !**/*.sum

📒 Files selected for processing (3)

src/config/config.go (1 hunks)
src/go.mod (2 hunks)
src/handlers/docker.go (3 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/handlers/docker.go (1)

src/utils/http_client.go (1)

GetGlobalHTTPClient (60-62)

🔇 Additional comments (1)

src/go.mod (1)

28-28: Dependency changes verified; go.mod tidy & verify successful

json-iterator/go v1.1.12 correctly promoted to a direct dependency

github.com/stretchr/testify remains indirect (test-only)

govulncheck not installed, vulnerability scan skipped

**Actionable comments posted: 1** > [!CAUTION] > Some comments are outside the diff and can’t be posted inline due to platform limitations. > > > > <details> > <summary>⚠️ Outside diff range comments (1)</summary><blockquote> > > <details> > <summary>src/handlers/docker.go (1)</summary><blockquote> > > `545-552`: **Critical: 500 is written then streaming continues; Content-Length may be wrong.** > > On `layer.Size()` error you write a 500 body but then continue with 200 and stream data; also you still set `Content-Length` using an undefined/zero size, which will corrupt responses. Stream without `Content-Length` when size is unknown and do not write an error body. > > > Apply: > ```diff > @@ > - size, err := layer.Size() > - if err != nil { > - fmt.Printf("获取layer大小失败: %v\n", err) > - c.String(http.StatusInternalServerError, "Failed to get layer size") > - // return > - } > + size, err := layer.Size() > + if err != nil { > + // Don't fail the request; stream without Content-Length to allow clients to pull. > + fmt.Printf("获取layer大小失败，继续以未知长度流式传输: %v\n", err) > + } else { > + c.Header("Content-Length", fmt.Sprintf("%d", size)) > + } > @@ > - c.Header("Content-Type", "application/octet-stream") > - c.Header("Content-Length", fmt.Sprintf("%d", size)) > + c.Header("Content-Type", "application/octet-stream") > c.Header("Docker-Content-Digest", digest) > > c.Status(http.StatusOK) > io.Copy(c.Writer, reader) > ``` > > > Also applies to: 560-566 > > </blockquote></details> > > </blockquote></details> <details> <summary>🧹 Nitpick comments (2)</summary><blockquote> <details> <summary>src/config/config.go (1)</summary><blockquote> `136-141`: **public.ecr.aws mapping added — verify AuthHost shape to avoid double path.** Using `AuthHost: "public.ecr.aws/token"` matches the style of other entries, but `proxyDockerAuthOriginal` concatenates `https://<AuthHost> + c.Request.URL.Path`. If the incoming path is `/token`, this becomes `https://public.ecr.aws/token/token`. Confirm the intended join logic or change `AuthHost` to just the host and let the path come from the request. If you decide to keep path in `AuthHost`, guard the join: ```diff - authURL = "https://" + mapping.AuthHost + c.Request.URL.Path + base := strings.TrimRight(mapping.AuthHost, "/") + path := "/" + strings.TrimLeft(c.Request.URL.Path, "/") + authURL = "https://" + base + path ``` </blockquote></details> <details> <summary>src/handlers/docker.go (1)</summary><blockquote> `10-16`: **Import additions are fine; consider standardizing on one JSON lib.** You now have goccy/go-json (indirect), sonic (indirect), and json-iterator (direct). For maintainability, prefer one JSON library project-wide unless there’s a measured perf/compat need per call site. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro **💡 Knowledge Base configuration:** - MCP integration is disabled by default for public repositories - Jira integration is disabled by default for public repositories - Linear integration is disabled by default for public repositories You can enable these sources in your CodeRabbit configuration. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 0cd5a7334d6e6231e69a8672348fa20e04126822 and 13f24c9d26d5f668d6ac39e70597b37bc7137d2a. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `src/go.sum` is excluded by `!**/*.sum` </details> <details> <summary>📒 Files selected for processing (3)</summary> * `src/config/config.go` (1 hunks) * `src/go.mod` (2 hunks) * `src/handlers/docker.go` (3 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧬 Code graph analysis (1)</summary> <details> <summary>src/handlers/docker.go (1)</summary><blockquote> <details> <summary>src/utils/http_client.go (1)</summary> * `GetGlobalHTTPClient` (60-62) </details> </blockquote></details> </details> </details> <details> <summary>🔇 Additional comments (1)</summary><blockquote> <details> <summary>src/go.mod (1)</summary><blockquote> `28-28`: **Dependency changes verified; go.mod tidy & verify successful** - json-iterator/go v1.1.12 correctly promoted to a direct dependency - github.com/stretchr/testify remains indirect (test-only) - govulncheck not installed, vulnerability scan skipped </blockquote></details> </blockquote></details> </details>

src/handlers/docker.go

coderabbitai[bot] (Migrated from github.com) commented

2025-09-01 18:04:39 +08:00

💡 Verification agent

❓ Verification inconclusive

AWS ECR token fetch: add status check, fix logging, type-safe JSON; consider scoping.

No check for non-200 token responses.
Logging uses err when token is missing (nil at that point).
Use a typed struct for unmarshalling.
Consider adding service and repo scope query params; currently the token may lack required scopes, leading to 401 loops when used as a static Bearer. This likely needs passing the repository name into createUpstreamOptions to build scope=repository:<repo>:pull. If that’s too large for this PR, at least keep anonymous so the standard challenge flow can obtain a scoped token.

Apply minimal safety fixes now:

@@
-        authUrl := fmt.Sprint("https://", mapping.AuthHost)
-        resp, err := client.Get(authUrl)
+        authURL := "https://" + mapping.AuthHost
+        resp, err := client.Get(authURL)
         if err != nil {
             fmt.Printf("%s: Failed to get token, fallback to anonymous: %v\n", mapping.AuthType, err)
             break
         }
         defer resp.Body.Close()
+        if resp.StatusCode != http.StatusOK {
+            fmt.Printf("%s: token endpoint returned %d, fallback to anonymous\n", mapping.AuthType, resp.StatusCode)
+            break
+        }
         c, err := io.ReadAll(resp.Body)
         if err != nil {
             fmt.Printf("%s: Failed to read token, fallback to anonymous: %v\n", mapping.AuthType, err)
             break
         }
-        var tokenRet map[string]string
-        if err := jsoniter.Unmarshal(c, &tokenRet); err != nil {
+        var tokenRet struct {
+            Token string `json:"token"`
+        }
+        if err := jsoniter.Unmarshal(c, &tokenRet); err != nil {
             fmt.Printf("%s: Failed to read token, fallback to anonymous: %v\n", mapping.AuthType, err)
             break
         }
@@
-        if token, ok := tokenRet["token"]; !ok {
-            fmt.Printf("%s: token is missing in response, fallback to anonymous: %v\n", mapping.AuthType, err)
-            break
-        } else {
+        if tokenRet.Token == "" {
+            fmt.Printf("%s: token is missing in response, fallback to anonymous\n", mapping.AuthType)
+            break
+        } else {
             // Step 3: Use the token fetched as Bearer auth
             auth := &authn.Bearer{
-                Token: token,
+                Token: tokenRet.Token,
             }
             options = append(options, remote.WithAuth(auth))
             useAnonymous = false
         }

Scope suggestion (follow-up): change signature to accept repo name and build authURL := fmt.Sprintf("https://%s?service=public.ecr.aws&scope=repository:%s:pull", mapping.AuthHost, repo); pass the repo from callers (handleUpstream*).

Add status-code check, correct logging and JSON handling, and scope ECR tokens

Check resp.StatusCode != http.StatusOK before reading the body; fallback to anonymous on non-200.
Remove the stale err from the missing-token log and unmarshal into a typed struct.
Pass service and scope=repository:<repo>:pull query params (e.g. ?service=public.ecr.aws&scope=repository:${repo}:pull); consider updating createUpstreamOptions signature to accept the repo name.

@@ src/handlers/docker.go:600
-        authUrl := fmt.Sprint("https://", mapping.AuthHost)
+        authURL := "https://" + mapping.AuthHost
         resp, err := client.Get(authURL)
         if err != nil {
             fmt.Printf("%s: Failed to get token, fallback to anonymous: %v\n", mapping.AuthType, err)
             break
         }
-        defer resp.Body.Close()
+        defer resp.Body.Close()
+        if resp.StatusCode != http.StatusOK {
+            fmt.Printf("%s: token endpoint returned %d, fallback to anonymous\n", mapping.AuthType, resp.StatusCode)
+            break
+        }
         c, err := io.ReadAll(resp.Body)
         if err != nil {
             fmt.Printf("%s: Failed to read token, fallback to anonymous: %v\n", mapping.AuthType, err)
             break
         }
-        var tokenRet map[string]string
-        if err := jsoniter.Unmarshal(c, &tokenRet); err != nil {
+        var tokenRet struct {
+            Token string `json:"token"`
+        }
+        if err := jsoniter.Unmarshal(c, &tokenRet); err != nil {
             fmt.Printf("%s: Failed to read token, fallback to anonymous: %v\n", mapping.AuthType, err)
             break
         }
-        if token, ok := tokenRet["token"]; !ok {
-            fmt.Printf("%s: token is missing in response, fallback to anonymous: %v\n", mapping.AuthType, err)
-            break
-        } else {
+        if tokenRet.Token == "" {
+            fmt.Printf("%s: token is missing in response, fallback to anonymous\n", mapping.AuthType)
+            break
+        } else {
             auth := &authn.Bearer{
-                Token: token,
+                Token: tokenRet.Token,
             }
             options = append(options, remote.WithAuth(auth))
             useAnonymous = false
         }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

	useAnonymous := true

	// 预留将来不同Registry的差异化认证逻辑扩展点
	switch mapping.AuthType {
	case "github":
	case "google":
	case "quay":
	case "aws-ecr":

		// Step 1: Get anonymous token from https://public.ecr.aws/token/
		client := &http.Client{
			Timeout:   10 * time.Second,
			Transport: utils.GetGlobalHTTPClient().Transport,
		}
		authURL := "https://" + mapping.AuthHost
		resp, err := client.Get(authURL)
		if err != nil {
			fmt.Printf("%s: Failed to get token, fallback to anonymous: %v\n", mapping.AuthType, err)
			break
		}
		defer resp.Body.Close()
		if resp.StatusCode != http.StatusOK {
			fmt.Printf("%s: token endpoint returned %d, fallback to anonymous\n", mapping.AuthType, resp.StatusCode)
			break
		}
		c, err := io.ReadAll(resp.Body)
		if err != nil {
			fmt.Printf("%s: Failed to read token, fallback to anonymous: %v\n", mapping.AuthType, err)
			break
		}
		var tokenRet struct {
			Token string `json:"token"`
		}
		if err := jsoniter.Unmarshal(c, &tokenRet); err != nil {
			fmt.Printf("%s: Failed to read token, fallback to anonymous: %v\n", mapping.AuthType, err)
			break
		}

		// Step 2: Successfully got token like:
		/*
			{"token":"[Token here]"}
		*/
		if tokenRet.Token == "" {
			fmt.Printf("%s: token is missing in response, fallback to anonymous\n", mapping.AuthType)
			break
		} else {
			// Step 3: Use the token fetched as Bearer auth
			auth := &authn.Bearer{
				Token: tokenRet.Token,
			}
			options = append(options, remote.WithAuth(auth))
			useAnonymous = false
		}

	}

	if useAnonymous {
		options = append(options, remote.WithAuth(authn.Anonymous))
	}
	return options

🤖 Prompt for AI Agents

In src/handlers/docker.go around lines 600 to 653, the ECR token fetch flow
needs: 1) check resp.StatusCode != http.StatusOK and fallback to anonymous
without reading body on non-200; 2) stop referencing the stale err in the "token
missing" log and log a clear message instead; 3) unmarshal the response into a
typed struct (e.g. struct{ Token string `json:"token"` }) rather than a
map[string]string; 4) include the required query params when requesting the
token by appending ?service=<service>&scope=repository:<repo>:pull (so update
createUpstreamOptions signature to accept the repo name and pass it here), and
if token present use authn.Bearer with the token, otherwise fall back to
authn.Anonymous.

_💡 Verification agent_ <details> <summary>❓ Verification inconclusive</summary> **AWS ECR token fetch: add status check, fix logging, type-safe JSON; consider scoping.** - No check for non-200 token responses. - Logging uses `err` when token is missing (nil at that point). - Use a typed struct for unmarshalling. - Consider adding `service` and repo `scope` query params; currently the token may lack required scopes, leading to 401 loops when used as a static Bearer. This likely needs passing the repository name into `createUpstreamOptions` to build `scope=repository:<repo>:pull`. If that’s too large for this PR, at least keep anonymous so the standard challenge flow can obtain a scoped token. Apply minimal safety fixes now: ```diff @@ - authUrl := fmt.Sprint("https://", mapping.AuthHost) - resp, err := client.Get(authUrl) + authURL := "https://" + mapping.AuthHost + resp, err := client.Get(authURL) if err != nil { fmt.Printf("%s: Failed to get token, fallback to anonymous: %v\n", mapping.AuthType, err) break } defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + fmt.Printf("%s: token endpoint returned %d, fallback to anonymous\n", mapping.AuthType, resp.StatusCode) + break + } c, err := io.ReadAll(resp.Body) if err != nil { fmt.Printf("%s: Failed to read token, fallback to anonymous: %v\n", mapping.AuthType, err) break } - var tokenRet map[string]string - if err := jsoniter.Unmarshal(c, &tokenRet); err != nil { + var tokenRet struct { + Token string `json:"token"` + } + if err := jsoniter.Unmarshal(c, &tokenRet); err != nil { fmt.Printf("%s: Failed to read token, fallback to anonymous: %v\n", mapping.AuthType, err) break } @@ - if token, ok := tokenRet["token"]; !ok { - fmt.Printf("%s: token is missing in response, fallback to anonymous: %v\n", mapping.AuthType, err) - break - } else { + if tokenRet.Token == "" { + fmt.Printf("%s: token is missing in response, fallback to anonymous\n", mapping.AuthType) + break + } else { // Step 3: Use the token fetched as Bearer auth auth := &authn.Bearer{ - Token: token, + Token: tokenRet.Token, } options = append(options, remote.WithAuth(auth)) useAnonymous = false } ``` Scope suggestion (follow-up): change signature to accept repo name and build `authURL := fmt.Sprintf("https://%s?service=public.ecr.aws&scope=repository:%s:pull", mapping.AuthHost, repo)`; pass the repo from callers (`handleUpstream*`). --- </details> **Add status-code check, correct logging and JSON handling, and scope ECR tokens** - Check `resp.StatusCode != http.StatusOK` before reading the body; fallback to anonymous on non-200. - Remove the stale `err` from the missing-token log and unmarshal into a typed struct. - Pass `service` and `scope=repository:<repo>:pull` query params (e.g. `?service=public.ecr.aws&scope=repository:${repo}:pull`); consider updating `createUpstreamOptions` signature to accept the repo name. ```diff @@ src/handlers/docker.go:600 - authUrl := fmt.Sprint("https://", mapping.AuthHost) + authURL := "https://" + mapping.AuthHost resp, err := client.Get(authURL) if err != nil { fmt.Printf("%s: Failed to get token, fallback to anonymous: %v\n", mapping.AuthType, err) break } - defer resp.Body.Close() + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + fmt.Printf("%s: token endpoint returned %d, fallback to anonymous\n", mapping.AuthType, resp.StatusCode) + break + } c, err := io.ReadAll(resp.Body) if err != nil { fmt.Printf("%s: Failed to read token, fallback to anonymous: %v\n", mapping.AuthType, err) break } - var tokenRet map[string]string - if err := jsoniter.Unmarshal(c, &tokenRet); err != nil { + var tokenRet struct { + Token string `json:"token"` + } + if err := jsoniter.Unmarshal(c, &tokenRet); err != nil { fmt.Printf("%s: Failed to read token, fallback to anonymous: %v\n", mapping.AuthType, err) break } - if token, ok := tokenRet["token"]; !ok { - fmt.Printf("%s: token is missing in response, fallback to anonymous: %v\n", mapping.AuthType, err) - break - } else { + if tokenRet.Token == "" { + fmt.Printf("%s: token is missing in response, fallback to anonymous\n", mapping.AuthType) + break + } else { auth := &authn.Bearer{ - Token: token, + Token: tokenRet.Token, } options = append(options, remote.WithAuth(auth)) useAnonymous = false } ```  <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion useAnonymous := true // 预留将来不同Registry的差异化认证逻辑扩展点 switch mapping.AuthType { case "github": case "google": case "quay": case "aws-ecr": // Step 1: Get anonymous token from https://public.ecr.aws/token/ client := &http.Client{ Timeout: 10 * time.Second, Transport: utils.GetGlobalHTTPClient().Transport, } authURL := "https://" + mapping.AuthHost resp, err := client.Get(authURL) if err != nil { fmt.Printf("%s: Failed to get token, fallback to anonymous: %v\n", mapping.AuthType, err) break } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { fmt.Printf("%s: token endpoint returned %d, fallback to anonymous\n", mapping.AuthType, resp.StatusCode) break } c, err := io.ReadAll(resp.Body) if err != nil { fmt.Printf("%s: Failed to read token, fallback to anonymous: %v\n", mapping.AuthType, err) break } var tokenRet struct { Token string `json:"token"` } if err := jsoniter.Unmarshal(c, &tokenRet); err != nil { fmt.Printf("%s: Failed to read token, fallback to anonymous: %v\n", mapping.AuthType, err) break } // Step 2: Successfully got token like: /* {"token":"[Token here]"} */ if tokenRet.Token == "" { fmt.Printf("%s: token is missing in response, fallback to anonymous\n", mapping.AuthType) break } else { // Step 3: Use the token fetched as Bearer auth auth := &authn.Bearer{ Token: tokenRet.Token, } options = append(options, remote.WithAuth(auth)) useAnonymous = false } } if useAnonymous { options = append(options, remote.WithAuth(authn.Anonymous)) } return options ``` </details>  <details> <summary>🤖 Prompt for AI Agents</summary> ``` In src/handlers/docker.go around lines 600 to 653, the ECR token fetch flow needs: 1) check resp.StatusCode != http.StatusOK and fallback to anonymous without reading body on non-200; 2) stop referencing the stale err in the "token missing" log and log a clear message instead; 3) unmarshal the response into a typed struct (e.g. struct{ Token string `json:"token"` }) rather than a map[string]string; 4) include the required query params when requesting the token by appending ?service=<service>&scope=repository:<repo>:pull (so update createUpstreamOptions signature to accept the repo name and pass it here), and if token present use authn.Bearer with the token, otherwise fall back to authn.Anonymous. ``` </details>

Rirmach commented

2025-10-03 15:50:23 +08:00

(Migrated from github.com)

考虑到使用 GET "Range: bytes=0-0" 的方式过于 hack，且会破坏当前处理流程和 google/go-containerregistry 的封装，考虑等到上游支持 HEAD blob 的时候（aws/containers-roadmap#2671）再合并。

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: 3344/hubproxy#66