diff --git a/cmd/nerdctl/login/login_linux_test.go b/cmd/nerdctl/login/login_linux_test.go index 6d851d2e731..3c294b1b390 100644 --- a/cmd/nerdctl/login/login_linux_test.go +++ b/cmd/nerdctl/login/login_linux_test.go @@ -21,6 +21,7 @@ package login import ( + "errors" "fmt" "net" "strconv" @@ -344,10 +345,19 @@ func TestLoginAgainstVariants(t *testing.T) { // TODO: remove specialization when we fix the localhost mess if !rl.IsLocalhost() || !tc.tls { - c.Cmd(helpers, "http://"+regHost).Run(&test.Expected{ExitCode: expect.ExitCodeSuccess}) - c.Cmd(helpers, "https://"+regHost).Run(&test.Expected{ExitCode: expect.ExitCodeSuccess}) - c.Cmd(helpers, "http://"+regHost+"/whatever?foo=bar;foo:bar#foo=bar").Run(&test.Expected{ExitCode: expect.ExitCodeSuccess}) - c.Cmd(helpers, "https://"+regHost+"/whatever?foo=bar&bar=foo;foo=foo+bar:bar#foo=bar").Run(&test.Expected{ExitCode: expect.ExitCodeSuccess}) + // Providing an explicit scheme succeeds, but a warning telling the + // user that the scheme is ignored must be displayed + // (https://github.com/containerd/nerdctl/issues/3052) + schemeWarned := func() *test.Expected { + return &test.Expected{ + ExitCode: expect.ExitCodeSuccess, + Errors: []error{errors.New("accepted only as a convenience")}, + } + } + c.Cmd(helpers, "http://"+regHost).Run(schemeWarned()) + c.Cmd(helpers, "https://"+regHost).Run(schemeWarned()) + c.Cmd(helpers, "http://"+regHost+"/whatever?foo=bar;foo:bar#foo=bar").Run(schemeWarned()) + c.Cmd(helpers, "https://"+regHost+"/whatever?foo=bar&bar=foo;foo=foo+bar:bar#foo=bar").Run(schemeWarned()) } } diff --git a/pkg/cmd/login/login.go b/pkg/cmd/login/login.go index 773bf8edc76..7b1105ad3a6 100644 --- a/pkg/cmd/login/login.go +++ b/pkg/cmd/login/login.go @@ -23,6 +23,7 @@ import ( "io" "net/http" "net/url" + "strings" "golang.org/x/net/context/ctxhttp" @@ -41,7 +42,28 @@ Configure a credential helper to remove this warning. See https://docs.docker.com/engine/reference/commandline/login/#credentials-store ` +// schemeWarning returns a warning message if the provided server address contains an explicit +// http or https scheme, and an empty string otherwise. +// Accepting a scheme in the server address is only a convenience: the scheme itself is ignored +// (see dockerconfigresolver.Parse), which may surprise users (https://github.com/containerd/nerdctl/issues/3052). +func schemeWarning(serverAddress string) string { + sch, _, found := strings.Cut(serverAddress, "://") + if !found || (sch != "http" && sch != "https") { + return "" + } + warning := fmt.Sprintf("The %s:// scheme in the provided server address %q is accepted only as a convenience, "+ + "and is otherwise ignored: connections to registries are made over https by default.", sch, serverAddress) + if sch == "http" { + warning += " To log in to a registry served over plain http, use the global --insecure-registry flag instead." + } + return warning +} + func Login(ctx context.Context, options types.LoginCommandOptions, stdout io.Writer) error { + if warning := schemeWarning(options.ServerAddress); warning != "" { + log.G(ctx).Warn(warning) + } + registryURL, err := dockerconfigresolver.Parse(options.ServerAddress) if err != nil { return err diff --git a/pkg/cmd/login/login_test.go b/pkg/cmd/login/login_test.go new file mode 100644 index 00000000000..de90a3c7f70 --- /dev/null +++ b/pkg/cmd/login/login_test.go @@ -0,0 +1,84 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package login + +import ( + "strings" + "testing" + + "gotest.tools/v3/assert" +) + +func TestSchemeWarning(t *testing.T) { + tests := []struct { + name string + address string + wantWarning bool + wantHint bool + }{ + { + name: "empty address", + address: "", + }, + { + name: "no scheme", + address: "index.docker.io", + }, + { + name: "hostname with port but no scheme", + address: "localhost:5000", + }, + { + name: "https scheme", + address: "https://index.docker.io/v1/", + wantWarning: true, + }, + { + name: "https scheme with port", + address: "https://localhost:5000", + wantWarning: true, + }, + { + name: "http scheme", + address: "http://localhost:5000", + wantWarning: true, + wantHint: true, + }, + { + // Unsupported schemes are rejected with an explicit error by + // dockerconfigresolver.Parse, so no warning is needed. + name: "unsupported scheme", + address: "oci://registry.example.com", + }, + { + // The experimental scheme is meaningful and not ignored, so no warning. + name: "experimental scheme", + address: "nerdctl-experimental://index.docker.io/v1/", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + warning := schemeWarning(tc.address) + assert.Equal(t, warning != "", tc.wantWarning) + if tc.wantWarning { + assert.Assert(t, strings.Contains(warning, "ignored")) + } + assert.Equal(t, strings.Contains(warning, "--insecure-registry"), tc.wantHint) + }) + } +}