🔒 docker: Support basic auth for docker hub #76

Open
Rirmach wants to merge 2 commits from Rirmach/dockerhub_auth into main
Rirmach commented 2025-10-03 16:52:06 +08:00 (Migrated from github.com)

初步支持 docker hub 认证拉取

  1. 考虑到 docker hub 的拉取流程独立于自定义上游进行,因此相应配置也独立于 registries 数组
  2. 默认配置中,认证信息留空(匿名拉取)

Summary by CodeRabbit

  • New Features
    • Added optional Docker Hub authentication. You can now provide a username and token in the app configuration to enable authenticated pulls.
    • When credentials are supplied, the app automatically uses basic authentication; if not, it continues with anonymous access.
    • Default behavior remains unchanged for users who do not configure credentials.
# 初步支持 docker hub 认证拉取 1. 考虑到 docker hub 的拉取流程独立于自定义上游进行,因此相应配置也独立于 registries 数组 2. 默认配置中,认证信息留空(匿名拉取) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added optional Docker Hub authentication. You can now provide a username and token in the app configuration to enable authenticated pulls. * When credentials are supplied, the app automatically uses basic authentication; if not, it continues with anonymous access. * Default behavior remains unchanged for users who do not configure credentials. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
coderabbitai[bot] commented 2025-10-03 16:52:13 +08:00 (Migrated from github.com)

Walkthrough

Added 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

Cohort / File(s) Summary
Configuration schema (TOML)
src/config.toml
Introduced [dockerHubAuth] with username and token; minor formatting tweak on proxy line.
Configuration model (Go)
src/config/config.go
Added DockerHubAuth struct to AppConfig with Username, Token; set empty defaults in DefaultConfig.
Docker proxy auth handling
src/handlers/docker.go
InitDockerProxy now selects basic auth if DockerHubAuth credentials exist; otherwise uses anonymous auth; other options unchanged.

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

I hop through configs, nose a-twitch—
Two keys appear, a snuggly switch:
If name and token line the burrow,
I pass with grace—no need to furrow.
Else I scurry, light and sly,
Anonymous, beneath the sky. 🐇🛠️

Pre-merge checks and finishing touches

Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title accurately summarizes the main change by indicating support for basic auth with Docker Hub, but it includes decorative emojis that are considered noise. Including emojis can detract from clarity and professionalism in pull request history. Removing them would keep the title concise and focused on the key change. Please remove the emojis from the title so it reads “docker: Support basic auth for Docker Hub” to maintain clarity and professionalism.
Passed checks (2 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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.

<!-- This is an auto-generated comment: summarize by coderabbit.ai --> <!-- walkthrough_start --> ## Walkthrough Added 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 | Cohort / File(s) | Summary | |---|---| | **Configuration schema (TOML)**<br>`src/config.toml` | Introduced `[dockerHubAuth]` with `username` and `token`; minor formatting tweak on proxy line. | | **Configuration model (Go)**<br>`src/config/config.go` | Added `DockerHubAuth` struct to `AppConfig` with `Username`, `Token`; set empty defaults in `DefaultConfig`. | | **Docker proxy auth handling**<br>`src/handlers/docker.go` | `InitDockerProxy` now selects basic auth if `DockerHubAuth` credentials exist; otherwise uses anonymous auth; other options unchanged. | ## Sequence Diagram(s) ```mermaid 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 > I hop through configs, nose a-twitch— > Two keys appear, a snuggly switch: > If name and token line the burrow, > I pass with grace—no need to furrow. > Else I scurry, light and sly, > Anonymous, beneath the sky. 🐇🛠️ <!-- walkthrough_end --> <!-- pre_merge_checks_walkthrough_start --> ## Pre-merge checks and finishing touches <details> <summary>❌ Failed checks (1 warning)</summary> | Check name | Status | Explanation | Resolution | | :---------: | :--------- | :---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :--------------------------------------------------------------------------------------------------------------------------------------- | | Title Check | ⚠️ Warning | The title accurately summarizes the main change by indicating support for basic auth with Docker Hub, but it includes decorative emojis that are considered noise. Including emojis can detract from clarity and professionalism in pull request history. Removing them would keep the title concise and focused on the key change. | Please remove the emojis from the title so it reads “docker: Support basic auth for Docker Hub” to maintain clarity and professionalism. | </details> <details> <summary>✅ Passed checks (2 passed)</summary> | Check name | Status | Explanation | | :----------------: | :------- | :----------------------------------------------------------------------------------- | | Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. | | Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. | </details> <!-- pre_merge_checks_walkthrough_end --> <!-- finishing_touch_checkbox_start --> <details> <summary>✨ Finishing touches</summary> - [ ] <!-- {"checkboxId": "7962f53c-55bc-4827-bfbf-6a18da830691"} --> 📝 Generate docstrings <details> <summary>🧪 Generate unit tests (beta)</summary> - [ ] <!-- {"checkboxId": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} --> Create PR with unit tests - [ ] <!-- {"checkboxId": "07f1e7d6-8a8e-4e23-9900-8731c2c87f58", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} --> Post copyable unit tests in a comment </details> </details> <!-- finishing_touch_checkbox_end --> <!-- tips_start --> --- 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> <!-- tips_end --> <!-- internal state start --> <!-- DwQgtGAEAqAWCWBnSTIEMB26CuAXA9mAOYCmGJATmriQCaQDG+Ats2bgFyQAOFk+AIwBWJBrngA3EsgEBPRvlqU0AgfFwA6NPEgQAfACgjoCEYDEZyAAUASpETZWaCrKPR1AGxJdAvBuAkXcB4P8AKckhafAYAa0ouAGVsbm58ClxIATREeAYcXFhIADMk0PCovlhsAUhIAwA5RwFoyAB2ADZKgwBVGwAZLlhcXG5EDgB6YaJ1MoENJmZhxAjZACZFgGY14cnefAAPWWHubA8PYZa29sQGm3gKZjQGXKqY/GwKBhJUqgx7rjDIykmAPpoPAPAzQZykFICT7fSC3eBYR64ajYIb8bhkNo2EgSeAkADulDRAAoMPhyJAPEgaLQAJRtLoqEgeElkilUxA0+lVADCFBI1Do6E4kEWAAZFgBWMAARjFYDFK2gYoAHBxJYsOGLmgAtIwAEWkDAo8G44nJHAMUAAgrRaMgEep4GgPPZ4olkvlCvripRIAAJcrZWDsTLUeDkyD4kNYA5HBFEFC3UiIDRWyDVfA0ZA5aiQH1/PiBipx115Dz4fEoB0YJQY2vsfh5RiogjMSDxTn8tDMRAAGns+BQuGQTAweXgRBe4cjqCiZvsJG4zkF+QoLEgOXeJG21ITkH54y7eOQziosjTUENeWBHhSY4nU6o5qwXjQUlPINDDBnWAnzPtSASGYM15AIdAjkrdA2VkZhnmQUtUyMfRjHAKAyHofBm2BAhiDIZQaQUVh2C4Xh+GEURxA/VJ5CYJQqFUdQtB0FCTCgOBUFQTBskIUhyGfIUZjYDARSoKsHCcFwaIUeiVDUTRtF0MBDFQ0wDEQV5hgfScNDbDxLQAIiMgwLEga0AEk8P4wV6Ak24pKwxhYEwFNkLMu0hTQSByCrA4BCpLJtKfX9UgrSJIAAbV+Epi2tEEAF0FBE7QMH3XB8SHf8PHtLhUUoDAe3eTB6AIKIMA0GAEBrXB11obA3lHfklBE50WS9PgCxKAMgxwmNxB/F8UCwLcksfacX0vSAAFkESSAc2TAPJsC+F8XXa25+n3e4XPeSMtl2SkEXeVAmEQNh+ug+gwmkbyswgmg+HqZzcSSNMjBMyxrTvAiIwwHMhxGpQGA8FcX2QRydw9QjCj8gKgJa8RpDcmJKN+9APNoLgAANosoWKQSxobICxjSGC08lH10lgPCxgxKigaoCR4coAvJ8dJ3GtH/OKbGot9Ch8ZyeLafpjMmaynLibyigCrYQniWPDAiFpAcsdKsh5cV5XzDMIwAFFOXgDbBMUd5+VxJmSDyApki4Ka6HgRwDCMgzkLU0m2cfT2dKIfBDOM0yLKsgihTs5x5Ec7alaR9NbSUegvJ80aOefNGHAEC4xDRzq8fKOKck3IdrQSHkKcnAcx2RR0lcgc58sKy6YHwMr8jxbLkGJPTNzQIg0Wl2WSAHdWMHpZb6KKQtBdgSbzNS8QXXgAAvIUc4FvOQSJ69b1wUv2cTfEJiAkDcHkOuZYb4qm5biQXWwaQ3oN8RjfoOizZxPEqytm2RXt2hHeYZ2xkrTu00s5WsXgKCIGGLjCgGhfb+1doHSyfEQ62UcPZCOzYo6uTBCGSAs91CrysOuA6S0VpozJFWAQ2B4DtwPMBLMu0zS/WQLQWQsswxHHkGkC4mEsCrynhVcyzYBHrxyBoaAzdMSX1EQIfO08z4D3QPyHg/ILgiQHOodACQMKnlSOkTIwZ0SDQPgXHI+ALiMCaqGF0iAADc/AtwUAPpYrRaAdG1lPDBOCqJgwVQ4sgfk3AQYNU3Hg3g794IQXxGgWQiAwCYHJLBKJSgbyHBSJfNAdpRzkj/qtV0vVvwhQrOMLIpjYDPAyd9E0Ncswhj4PgZh5IO7S3QHxDJtZNyfEQFDeh8IR4VV3jVfAZYKxVkoUNYG2AlCBJIDVPEN9XSOVkfItcG4gqNy8mOPJv01rQkwPcQuTl8AWO3BMP0AAhAxWRer8D4Nabx8E3o6zMtU38/0wnvCBiDVOzSmxAW2FDIUMMWaGNDIjRAblMxOR2h8yGSQaT7FBVkcF8gMhEAKrgF4999aG2fjJN+FtP7WwRXbB2TsXZuxUmxeGmFsJ4F4vhASL8WDCVEmgcS6Dw7SVfgxeSzElKGBpUJdQAJ4D2gBObD+dAAScmcCkFCBgaUABYxSShWMqlUMpGgyk1WKAQiw0ArDyLQAAnBq01og8jNAYLqvIAgVgMENQ62gUpBWqSgCK3AYqJVSoJDKjC7qaURIBGwCgpAAT3FEBERAsrkSekVQAbzppAAySBbAXLClEWgpdiIiSsBYmkBkuA3hZIPFNBlEAVMOLQTNxRbDFvyLY8tlQ02IAAPJSAoCaDyGBG2louH2Ctf9aA2GWgWGI8ylaIB5CGSIjaap3yHa2kdY6MDuFwF4Wd0aF0UCXcO8Va7DSIGNKaF82750lubcu1NVIMDZvMogBw0hJ0UEbUZG9BkQacgvREbEDg7yIEbRFFNlRk2VAg6mqNkRqiFXfce09TSsC/oMjeyDlbkRYqA1wRdLbIOpshiDTFv132/vsBEU0GJ6BQFLkoGwcl1CAEwCZACAiCwDAF4KQrow5SVQGQFQXhaAaFQ6BiDBk4JKHfTEmWCYRP4dTUkScCIXS/tg2wd9MzEMvldpBgAvmhyA4H8MGWgxENTJB30bq8JAFDBnW1yqw/269onW2EcwL+SzeDxCbqKgwBg41mRoq5SaZeOY8H9JhdHaSCI/4DX3A4BICL2r6IyDcje5T8z826gIAc1CUhaIRFMmZoRRBJHDFII++AhCoFzBklRY4Mj0SFGSJAJAhFfA8NM/cDDqujm4koGqdwUh5HXO2YGzh1DyEvlsPI0gMjkgXmdImpZ6EAEc76ckgFVAgLgKrYjgriGuW52wZRrZAKIS5Pmbk8O8McDBWuNwKP53h/Bhp4KiLRMBpBhN2dTWokZeASNcAMlYN8lj+QHfeCNHrqARsbhGt56ziAhxaO7IBQAOAQwNiO6JLPDDG3Jtplws2XAC4BEc/pVcsDjZNCfRuM25u7I5MwH7LnU0SYs8D6TqUlZyeM/yIK2KnNlt+wZRT4wCoeFU3B4HiOLOif06Joz6HTPmfg+ELWNn8Bdp7hZkXDnUS7v3fJgybniPkjVwwDXTBtekGrJAOUYoNBijFAAUijAgQ5qAHDW0yHiES/i8H8nW9cIUOQ1EVOynblUjvncu5Z8b9nUnnDc6ILz9DYvlOS7nWZ6XqbfhayA/LlN8VP3ftwLYBDJokPvpIKarVMo8hSjFKayUDBTWNAYCsGULeVjNAFLQZVjfJQCFNQIFUjRFiupWGgXVKpTVoDyKq1Q1tlUMElGKWgzvzW0GaPURYvOv3pHLzYKzHPU1d7FCQFUDBGhikaGgRoAhJTNEaJvuUKxaAz7tM0PIKpxQykWGaFNQYGVQb0WGtkALQFWH1TFAYEVBNUWGVQ1WVSti7xVFdl0w9VURIFDUoAjVM1jUDX0CAA=== --> <!-- internal state end -->
coderabbitai[bot] (Migrated from github.com) reviewed 2025-10-03 16:53:56 +08:00
coderabbitai[bot] (Migrated from github.com) left a 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 overrideFromEnv function does not provide environment variable overrides for DockerHubAuth, 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:

  1. 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")
+		}
 	}
  1. 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

📥 Commits

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

📒 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 DockerHubAuth struct is properly defined with appropriate TOML tags and integrates cleanly into the AppConfig structure.


116-122: LGTM! Default initialization aligns with anonymous pull behavior.

The empty default values for Username and Token correctly support anonymous Docker Hub pulls as specified in the PR objectives.

**Actionable comments posted: 0** > [!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/config/config.go (1)</summary><blockquote> > > `228-271`: **Add environment variable support for Docker Hub credentials.** > > The `overrideFromEnv` function does not provide environment variable overrides for `DockerHubAuth`, 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: > > ```diff > 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 > + } > } > ``` > > </blockquote></details> > > </blockquote></details> <details> <summary>🧹 Nitpick comments (2)</summary><blockquote> <details> <summary>src/config.toml (1)</summary><blockquote> `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: ```diff # Docker Hub 认证信息,留空则匿名拉取 [dockerHubAuth] username = "" # e.g., user1 -token = "" # e.g., dckr_pat_*** +token = "" # Docker Hub Access Token (e.g., dckr_pat_***) ``` </blockquote></details> <details> <summary>src/handlers/docker.go (1)</summary><blockquote> `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: 1. **Trim whitespace** from credentials to prevent accidental empty values: ```diff 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") + } } ``` 2. **Alternative**: Fail fast if credentials are incomplete to make misconfiguration explicit: ```diff 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)) } ``` </blockquote></details> </blockquote></details> <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 405348171480b2a3fd9349ecf6c14fb3c2ab3d25. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `src/config.toml` (1 hunks) * `src/config/config.go` (2 hunks) * `src/handlers/docker.go` (1 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/config/config.go (1)</summary> * `GetConfig` (160-196) </details> </blockquote></details> </details> </details> <details> <summary>🔇 Additional comments (2)</summary><blockquote> <details> <summary>src/config/config.go (2)</summary><blockquote> `51-54`: **LGTM! Struct definition is correct.** The `DockerHubAuth` struct is properly defined with appropriate TOML tags and integrates cleanly into the `AppConfig` structure. --- `116-122`: **LGTM! Default initialization aligns with anonymous pull behavior.** The empty default values for `Username` and `Token` correctly support anonymous Docker Hub pulls as specified in the PR objectives. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Checking for merge conflicts…
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin Rirmach/dockerhub_auth:Rirmach/dockerhub_auth
git checkout Rirmach/dockerhub_auth
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: 3344/hubproxy#76