-
-
Notifications
You must be signed in to change notification settings - Fork 501
feat: enable authenticated pulls #1580
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
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.
Pull Request Overview
This PR enables authenticated image pulls in k3d by integrating Docker CLI authentication mechanisms. This allows users to pull private images and avoid rate limits on public registries by leveraging existing Docker credentials stored in ${HOME}/.docker/config.json
.
Key Changes
- Added authentication resolution logic to extract Docker credentials for image registries
- Integrated authentication into the image pull process with proper error handling
- Updated dependencies to expose the
github.com/distribution/reference
package
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
pkg/runtimes/docker/container.go | Added resolveAuth function and integrated authentication into pullImage function |
go.mod | Changed github.com/distribution/reference from indirect to direct dependency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
l.Log().Warnf("Failed to get auth: %v", err) | ||
} | ||
encoded, err := registrytypes.EncodeAuthConfig(authConfig) | ||
if err != nil { | ||
l.Log().Warnf("Failed to encode auth: %v", err) |
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.
When resolveAuth
fails, the code continues with a potentially invalid authConfig
struct. Similarly, when EncodeAuthConfig
fails, the code continues with a potentially invalid encoded
value. Consider using empty/default values or returning early on critical authentication failures to ensure predictable behavior.
l.Log().Warnf("Failed to get auth: %v", err) | |
} | |
encoded, err := registrytypes.EncodeAuthConfig(authConfig) | |
if err != nil { | |
l.Log().Warnf("Failed to encode auth: %v", err) | |
return fmt.Errorf("failed to get auth: %w", err) | |
} | |
encoded, err := registrytypes.EncodeAuthConfig(authConfig) | |
if err != nil { | |
return fmt.Errorf("failed to encode auth: %w", err) |
Copilot uses AI. Check for mistakes.
if authKey == "docker.io" || authKey == "index.docker.io" { | ||
authKey = "https://index.docker.io/v1/" | ||
} |
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.
[nitpick] The hardcoded registry URL transformation should be extracted into a constant or helper function for better maintainability. Consider defining const DockerHubAuthKey = \"https://index.docker.io/v1/\"
at the package level.
Copilot uses AI. Check for mistakes.
What
Enables pulling of images that require authentication using the k3d docker client
Why
Enables usage of credentials when pulling from registries which allows access to private images, and bypass unauthenticated rate limits
Implications
${HOME}/docker/config.json
pulls could fail with 401 instead of trying anonymous pull to indexes likedocker.io