login: warn when the server address contains an explicit scheme - #5169
login: warn when the server address contains an explicit scheme#5169locker95 wants to merge 1 commit into
Conversation
`nerdctl login` accepts a server address with an explicit scheme (e.g. `https://index.docker.io/v1/`) only as a convenience: the scheme is silently disregarded and connections are made over https regardless, which is surprising when the user asked for `http://`. Warn the user that the scheme is ignored, and point at the global --insecure-registry flag when plain http was requested. Credential lookup paths reusing dockerconfigresolver.Parse (pull/push) are intentionally left silent. Fixes containerd#3052 Signed-off-by: Dean Chen <862469039@qq.com>
Please responsibly test your PR with actual registries, by running |
| 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, "+ |
There was a problem hiding this comment.
Does docker login have such a message?
There was a problem hiding this comment.
no — docker login just strips the scheme in ConvertToHostname and stays quiet. I can drop the warning if you would rather match that.
|
on testing: I do not have a registry handy here. the unit tests pass; the linux integration job already logs in with the http(s):// variants. on the warning: docker login does not print one (ConvertToHostname just strips the scheme). happy to drop it if you would rather keep parity. |
A contributor has to learn how to test their PR ahead of the submission. |
2 participants
Fixes #3052.
As noted by @fahedouch in the issue ("It looks like a bug. It is not the intended behavior."),
nerdctl loginsilently drops an explicit scheme from the server address. Following the direction suggested by @apostasie, this change warns the user that this syntax is only supported as a convenience and that the scheme is ignored; whenhttp://was requested, the warning additionally points at the global--insecure-registryflag. The warning is emitted only from thelogincode path —dockerconfigresolver.Parseis untouched, so credential lookup during pull/push stays silent.Testing: a platform-independent unit test covers the new detection helper (verified locally on macOS);
TestLoginAgainstVariantsnow also asserts the warning on stderr for the scheme variants. I could only compile-verify the Linux integration test locally (GOOS=linux go build ./.../go vet/go test -c), so I am relying on CI to exercise it.This patch was developed with the assistance of an AI tool (Claude); I have reviewed and tested the change and take responsibility for it.