-
Notifications
You must be signed in to change notification settings - Fork 203
Added fallback for docker.io registry in Docker proxy configuration
#78
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughUpdated 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
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
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
这个提交会修复 #77 |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 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 beforehandlers.InitDockerProxy()(main.go:57), ensuringglobalHTTPClientis initialized.
| if _, exist := cfg.Registries["docker.io"]; exist { | ||
| return "docker.io", path | ||
| } |
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.
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.
查阅了代码之后,发现有一个简易的实现方案:在registry配置中,解析一个默认的字符串“docker.io”,在detectRegistryDomain函数中增加一个默认的处理逻辑,如果registry配置中有对docker.io的镜像配置,则使用,否则使用dockerProxy来处理镜像逻辑
Summary by CodeRabbit
New Features
Bug Fixes