🔒️✨ docker: Support basic auth for docker hub #76
Conversation
WalkthroughAdded Docker Hub auth config (username, token) to config files and structures, initialized defaults, and updated Docker proxy initialization to use basic auth when credentials are provided, otherwise anonymous. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Config
participant H as InitDockerProxy
participant R as Docker Remote
H->>C: Load DockerHubAuth (username, token)
alt Credentials provided
H->>R: Build options with BasicAuth(username, token)
note right of R: New/changed path
else No credentials
H->>R: Build options with AnonymousAuth()
end
H->>R: Apply user agent and transport
R-->>H: Remote client initialized
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config/config.go (1)
228-271: Add environment variable support for Docker Hub credentials.The
overrideFromEnvfunction does not provide environment variable overrides forDockerHubAuth, which is a common pattern for managing secrets in containerized environments. This limits deployment flexibility, especially in Docker/Kubernetes where credentials should not be hardcoded in config files.Add environment variable support:
if val := os.Getenv("MAX_IMAGES"); val != "" { if maxImages, err := strconv.Atoi(val); err == nil && maxImages > 0 { cfg.Download.MaxImages = maxImages } } + + if val := os.Getenv("DOCKERHUB_USERNAME"); val != "" { + cfg.DockerHubAuth.Username = val + } + if val := os.Getenv("DOCKERHUB_TOKEN"); val != "" { + cfg.DockerHubAuth.Token = val + } }
🧹 Nitpick comments (2)
src/config.toml (1)
55-58: LGTM! Consider clarifying the token comment.The new
[dockerHubAuth]section is well-structured and the empty defaults correctly support anonymous pulls as stated in the PR objectives.However, the token comment could be more specific about the expected token type.
Consider this enhancement to clarify the token type:
# Docker Hub 认证信息,留空则匿名拉取 [dockerHubAuth] username = "" # e.g., user1 -token = "" # e.g., dckr_pat_*** +token = "" # Docker Hub Access Token (e.g., dckr_pat_***)src/handlers/docker.go (1)
74-82: Add validation and logging for Docker Hub authentication.The conditional auth logic correctly checks for both credentials before enabling Basic auth, but silently falls back to anonymous mode when credentials are incomplete. This could lead to confusion when users expect authenticated access.
Consider these improvements:
- Trim whitespace from credentials to prevent accidental empty values:
dockerHubAuth := config.GetConfig().DockerHubAuth - if dockerHubAuth.Token != "" && dockerHubAuth.Username != "" { + username := strings.TrimSpace(dockerHubAuth.Username) + token := strings.TrimSpace(dockerHubAuth.Token) + + if token != "" && username != "" { options = append(options, remote.WithAuth(&authn.Basic{ - Username: dockerHubAuth.Username, - Password: dockerHubAuth.Token, + Username: username, + Password: token, })) + fmt.Println("Docker Hub: Using Basic authentication") } else { options = append(options, remote.WithAuth(authn.Anonymous)) + if token != "" || username != "" { + fmt.Println("Warning: Docker Hub credentials incomplete, using anonymous authentication") + } else { + fmt.Println("Docker Hub: Using anonymous authentication") + } }
- Alternative: Fail fast if credentials are incomplete to make misconfiguration explicit:
dockerHubAuth := config.GetConfig().DockerHubAuth - if dockerHubAuth.Token != "" && dockerHubAuth.Username != "" { + username := strings.TrimSpace(dockerHubAuth.Username) + token := strings.TrimSpace(dockerHubAuth.Token) + + hasUsername := username != "" + hasToken := token != "" + + if hasUsername && hasToken { options = append(options, remote.WithAuth(&authn.Basic{ - Username: dockerHubAuth.Username, - Password: dockerHubAuth.Token, + Username: username, + Password: token, })) - } else { + fmt.Println("Docker Hub: Using Basic authentication") + } else if !hasUsername && !hasToken { options = append(options, remote.WithAuth(authn.Anonymous)) + fmt.Println("Docker Hub: Using anonymous authentication") + } else { + fmt.Printf("Warning: Docker Hub credentials incomplete (username=%t, token=%t), using anonymous authentication\n", hasUsername, hasToken) + options = append(options, remote.WithAuth(authn.Anonymous)) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/config.toml(1 hunks)src/config/config.go(2 hunks)src/handlers/docker.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/handlers/docker.go (1)
src/config/config.go (1)
GetConfig(160-196)
🔇 Additional comments (2)
src/config/config.go (2)
51-54: LGTM! Struct definition is correct.The
DockerHubAuthstruct is properly defined with appropriate TOML tags and integrates cleanly into theAppConfigstructure.
116-122: LGTM! Default initialization aligns with anonymous pull behavior.The empty default values for
UsernameandTokencorrectly support anonymous Docker Hub pulls as specified in the PR objectives.
1 participant
初步支持 docker hub 认证拉取
Summary by CodeRabbit