Skip to content
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

When the port is 443, also save credentials without port #3766

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions pkg/cmd/login/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,17 @@ func Login(ctx context.Context, options types.LoginCommandOptions, stdout io.Wri
return fmt.Errorf("error saving credentials: %w", err)
}

// When the port is the https default (443), other clients cannot be expected to necessarily lookup the variants with port
// so save it both with and without port.
// This is the case for at least buildctl: https://github.com/containerd/nerdctl/issues/3748
if registryURL.Port() == dockerconfigresolver.StandardHTTPSPort {
registryURL.Host = registryURL.Hostname()
err = credStore.Store(registryURL, credentials)
if err != nil {
return fmt.Errorf("error saving credentials: %w", err)
}
}

_, err = fmt.Fprintln(stdout, "Login Succeeded")

return err
Expand Down
7 changes: 4 additions & 3 deletions pkg/imgutil/dockerconfigresolver/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ import "errors"
type scheme string

const (
standardHTTPSPort = "443"
schemeHTTP scheme = "http"
schemeHTTPS scheme = "https"
StandardHTTPSPort = "443"

schemeHTTPS scheme = "https"
schemeHTTP scheme = "http"
// schemeNerdctlExperimental is currently provisional, to unlock namespace based host authentication
// This may change or break without notice, and you should have no expectations that credentials saved like that
// will be supported in the future
Expand Down
2 changes: 1 addition & 1 deletion pkg/imgutil/dockerconfigresolver/hostsstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func hostDirsFromRoot(registryURL *RegistryURL, dirs []string) (string, error) {
return found, err
}
// If not found, and the port is standard, try again without the port
if registryURL.Port() == standardHTTPSPort {
if registryURL.Port() == StandardHTTPSPort {
found, err = config.HostDirFromRoot(hostsDir)(registryURL.Hostname())
if (err != nil && !errors.Is(err, errdefs.ErrNotFound)) || (found != "") {
return found, err
Expand Down
8 changes: 4 additions & 4 deletions pkg/imgutil/dockerconfigresolver/registryurl.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func Parse(address string) (*RegistryURL, error) {
}
// If it has no port, add the standard port explicitly
if u.Port() == "" {
u.Host = u.Hostname() + ":" + standardHTTPSPort
u.Host = u.Hostname() + ":" + StandardHTTPSPort
}
reg := &RegistryURL{URL: *u}
queryParams := u.Query()
Expand All @@ -74,7 +74,7 @@ type RegistryURL struct {
// CanonicalIdentifier returns the identifier expected to be used to save credentials to docker auth config
func (rn *RegistryURL) CanonicalIdentifier() string {
// If it is the docker index over https, port 443, on the /v1/ path, we use the docker fully qualified identifier
if rn.Scheme == string(schemeHTTPS) && rn.Hostname() == "index.docker.io" && rn.Path == "/v1/" && rn.Port() == standardHTTPSPort ||
if rn.Scheme == string(schemeHTTPS) && rn.Hostname() == "index.docker.io" && rn.Path == "/v1/" && rn.Port() == StandardHTTPSPort ||
rn.URL.String() == dockerIndexServer {
return dockerIndexServer
}
Expand Down Expand Up @@ -102,7 +102,7 @@ func (rn *RegistryURL) AllIdentifiers() []string {

// Docker behavior: if the domain was index.docker.io over 443, we are allowed to additionally read the canonical
// docker credentials
if rn.Port() == standardHTTPSPort {
if rn.Port() == StandardHTTPSPort {
if rn.Hostname() == "index.docker.io" || rn.Hostname() == "registry-1.docker.io" {
fullList = append(fullList, dockerIndexServer)
}
Expand All @@ -116,7 +116,7 @@ func (rn *RegistryURL) AllIdentifiers() []string {

// Note that docker does not try to be smart wrt explicit port vs. implied port
// If standard port, allow retrieving credentials from the variant without a port as well
if rn.Port() == standardHTTPSPort {
if rn.Port() == StandardHTTPSPort {
fullList = append(
fullList,
rn.Hostname(),
Expand Down