Added fallback for docker.io registry in Docker proxy configuration #78

2025-10-05T23:31:20+08:00

Mystery00 commented

2025-10-05 23:31:20 +08:00

(Migrated from github.com)

查阅了代码之后，发现有一个简易的实现方案：在registry配置中，解析一个默认的字符串“docker.io”，在detectRegistryDomain函数中增加一个默认的处理逻辑，如果registry配置中有对docker.io的镜像配置，则使用，否则使用dockerProxy来处理镜像逻辑

Summary by CodeRabbit

New Features
- Automatic detection and mapping of Docker Hub when configured, improving image resolution.
- Default initialization of the Docker registry proxy with sensible network settings for smoother connectivity.
Bug Fixes
- More consistent registry domain resolution prevents mis-parsing of image paths.
- Reduced intermittent connection issues by standardizing HTTP transport across requests.

查阅了代码之后，发现有一个简易的实现方案：在registry配置中，解析一个默认的字符串“docker.io”，在detectRegistryDomain函数中增加一个默认的处理逻辑，如果registry配置中有对docker.io的镜像配置，则使用，否则使用dockerProxy来处理镜像逻辑  ## Summary by CodeRabbit * New Features * Automatic detection and mapping of Docker Hub when configured, improving image resolution. * Default initialization of the Docker registry proxy with sensible network settings for smoother connectivity. * Bug Fixes * More consistent registry domain resolution prevents mis-parsing of image paths. * Reduced intermittent connection issues by standardizing HTTP transport across requests.

coderabbitai[bot] commented

2025-10-05 23:31:30 +08:00

(Migrated from github.com)

Walkthrough

Updated docker registry handling: imports added; RegistryDetector.detectRegistryDomain now short-circuits to docker.io when configured; InitDockerProxy initializes a default registry handle for registry-1.docker.io using utils.GetGlobalHTTPClient().Transport and emits a prelude newline.

Changes

Cohort / File(s)	Summary
Docker registry handling `src/handlers/docker.go`	Added internal imports; detectRegistryDomain now returns "docker.io" with original path when cfg.Registries includes docker.io; InitDockerProxy prints a leading newline, creates a default registry handle for registry-1.docker.io, and wires default remote options using the global HTTP transport.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant CLI as CLI/Daemon
  participant RD as RegistryDetector
  participant DP as InitDockerProxy
  participant Reg as registry-1.docker.io

  rect rgba(230,240,255,0.5)
    Note right of DP: Startup
    User->>CLI: Start Docker proxy
    CLI->>DP: InitDockerProxy()
    DP-->>CLI: Print prelude newline
    DP->>Reg: Create default registry handle<br/>with global HTTP transport
    DP-->>CLI: Return initialized proxy
  end

  rect rgba(240,255,230,0.5)
    Note over RD: Image resolution
    CLI->>RD: detectRegistryDomain(imageRef, cfg)
    alt docker.io present in cfg.Registries
      RD-->>CLI: domain="docker.io", path=original
    else other registry rules
      RD-->>CLI: domain/path computed as before
    end
  end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nudge the clouds of code with a hop,
Dock to docker, I boop and bop—
A registry mapped, a proxy spun,
Newlines blink, connections run.
With global paws on transport light,
I ship my carrots through the night. 🥕🚀

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title concisely and accurately summarizes the primary change by indicating the addition of a fallback mechanism for the docker.io registry within the Docker proxy configuration, making it clear to reviewers what the pull request implements.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 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.}

## Walkthrough Updated docker registry handling: imports added; RegistryDetector.detectRegistryDomain now short-circuits to docker.io when configured; InitDockerProxy initializes a default registry handle for registry-1.docker.io using utils.GetGlobalHTTPClient().Transport and emits a prelude newline. ## Changes | Cohort / File(s) | Summary | | --- | --- | | **Docker registry handling**<br>`src/handlers/docker.go` | Added internal imports; detectRegistryDomain now returns "docker.io" with original path when cfg.Registries includes docker.io; InitDockerProxy prints a leading newline, creates a default registry handle for registry-1.docker.io, and wires default remote options using the global HTTP transport. | ## Sequence Diagram(s) ```mermaid sequenceDiagram autonumber actor User participant CLI as CLI/Daemon participant RD as RegistryDetector participant DP as InitDockerProxy participant Reg as registry-1.docker.io rect rgba(230,240,255,0.5) Note right of DP: Startup User->>CLI: Start Docker proxy CLI->>DP: InitDockerProxy() DP-->>CLI: Print prelude newline DP->>Reg: Create default registry handle<br/>with global HTTP transport DP-->>CLI: Return initialized proxy end rect rgba(240,255,230,0.5) Note over RD: Image resolution CLI->>RD: detectRegistryDomain(imageRef, cfg) alt docker.io present in cfg.Registries RD-->>CLI: domain="docker.io", path=original else other registry rules RD-->>CLI: domain/path computed as before end end ``` ## Estimated code review effort 🎯 2 (Simple) | ⏱️ ~10 minutes ## Poem > I nudge the clouds of code with a hop, > Dock to docker, I boop and bop— > A registry mapped, a proxy spun, > Newlines blink, connections run. > With global paws on transport light, > I ship my carrots through the night. 🥕🚀   ## Pre-merge checks and finishing touches <details> <summary>✅ Passed checks (3 passed)</summary> | Check name | Status | Explanation | | :----------------: | :------- | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. | | Title Check | ✅ Passed | The title concisely and accurately summarizes the primary change by indicating the addition of a fallback mechanism for the docker.io registry within the Docker proxy configuration, making it clear to reviewers what the pull request implements. | | Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. | </details>   <details> <summary>✨ Finishing touches</summary> - [ ]  📝 Generate docstrings <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> <sub>Comment `@coderabbitai help` to get the list of available commands and usage tips.</sub>

Mystery00 commented

2025-10-05 23:31:51 +08:00

(Migrated from github.com)

这个提交会修复 https://github.com/sky22333/hubproxy/issues/77

Mystery00 commented

2025-10-05 23:32:25 +08:00

(Migrated from github.com)

@sky22333

coderabbitai[bot] (Migrated from github.com) reviewed 2025-10-05 23:34:04 +08:00

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3f614e8011 and 426b1344f2.

📒 Files selected for processing (1)

src/handlers/docker.go (3 hunks)

🔇 Additional comments (2)

src/handlers/docker.go (2)

11-12: LGTM!

The new imports are properly utilized in the subsequent changes.

79-79: Initialization order validated

utils.InitHTTPClients() (main.go:51) runs before handlers.InitDockerProxy() (main.go:57), ensuring globalHTTPClient is initialized.

**Actionable comments posted: 1** <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 3f614e8011223fe8cb0391938a317dce965d1344 and 426b1344f2f15026ff26d609f7001860cc43f99b. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `src/handlers/docker.go` (3 hunks) </details> <details> <summary>🔇 Additional comments (2)</summary><blockquote> <details> <summary>src/handlers/docker.go (2)</summary><blockquote> `11-12`: **LGTM!** The new imports are properly utilized in the subsequent changes. --- `79-79`: **Initialization order validated** `utils.InitHTTPClients()` (main.go:51) runs before `handlers.InitDockerProxy()` (main.go:57), ensuring `globalHTTPClient` is initialized. </blockquote></details> </blockquote></details> </details>

src/handlers/docker.go

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

2025-10-05 23:34:03 +08:00

⚠️ Potential issue | 🔴 Critical

Fallback logic incorrectly routes unrecognized registries to docker.io.

The current implementation treats ANY path without a matching registry prefix as a docker.io image. This causes incorrect routing when paths contain unrecognized registry domains.

Problem: Given path quay.io/myorg/image where quay.io is not configured but docker.io is configured:

Lines 35-40: No prefix match
Lines 42-44: Returns ("docker.io", "quay.io/myorg/image")
Result: Attempts to fetch docker.io/quay.io/myorg/image ❌

The fallback should only apply to paths without a registry-like prefix (e.g., library/nginx), not to paths with unknown registry domains (e.g., quay.io/image).

Apply this diff to fix the logic:

+	// Check if path starts with a registry-like domain
+	firstPart := path
+	if idx := strings.Index(path, "/"); idx != -1 {
+		firstPart = path[:idx]
+	}
+	// If first part contains "." or ":", it looks like a registry domain
+	if strings.Contains(firstPart, ".") || strings.Contains(firstPart, ":") {
+		return "", path
+	}
+
 	if _, exist := cfg.Registries["docker.io"]; exist {
 		return "docker.io", path
 	}

🤖 Prompt for AI Agents

In src/handlers/docker.go around lines 42 to 44, the fallback currently always
returns ("docker.io", path) when no configured registry prefix matches, which
incorrectly rewrites paths that already contain an unknown registry host; change
the fallback to only select docker.io when the path has no registry-like prefix.
Specifically, extract the first path segment (strings.SplitN(path, "/", 2)[0])
and if that segment contains a '.' or ':' or equals "localhost" treat it as an
explicit (unknown) registry and return ("" , path) or otherwise return
("docker.io", path); update imports to include strings if needed.

_⚠️ Potential issue_ | _🔴 Critical_ **Fallback logic incorrectly routes unrecognized registries to docker.io.** The current implementation treats ANY path without a matching registry prefix as a docker.io image. This causes incorrect routing when paths contain unrecognized registry domains. **Problem**: Given path `quay.io/myorg/image` where `quay.io` is not configured but `docker.io` is configured: - Lines 35-40: No prefix match - Lines 42-44: Returns `("docker.io", "quay.io/myorg/image")` - Result: Attempts to fetch `docker.io/quay.io/myorg/image` ❌ The fallback should only apply to paths without a registry-like prefix (e.g., `library/nginx`), not to paths with unknown registry domains (e.g., `quay.io/image`). Apply this diff to fix the logic: ```diff + // Check if path starts with a registry-like domain + firstPart := path + if idx := strings.Index(path, "/"); idx != -1 { + firstPart = path[:idx] + } + // If first part contains "." or ":", it looks like a registry domain + if strings.Contains(firstPart, ".") || strings.Contains(firstPart, ":") { + return "", path + } + if _, exist := cfg.Registries["docker.io"]; exist { return "docker.io", path } ``` <details> <summary>🤖 Prompt for AI Agents</summary> ``` In src/handlers/docker.go around lines 42 to 44, the fallback currently always returns ("docker.io", path) when no configured registry prefix matches, which incorrectly rewrites paths that already contain an unknown registry host; change the fallback to only select docker.io when the path has no registry-like prefix. Specifically, extract the first path segment (strings.SplitN(path, "/", 2)[0]) and if that segment contains a '.' or ':' or equals "localhost" treat it as an explicit (unknown) registry and return ("" , path) or otherwise return ("docker.io", path); update imports to include strings if needed. ``` </details>

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#78

Added fallback for docker.io registry in Docker proxy configuration #78

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Pull request closed

Added fallback for `docker.io` registry in Docker proxy configuration #78