search

projectatomic / docker

Docker - the open-source application container engine
http://www.docker.com
Apache License 2.0
81 stars 58 forks source link

Change login success message and server selection #320

Closed TomSweeneyRedHat closed 3 years ago

TomSweeneyRedHat commented 6 years ago

Signed-off-by: TomSweeneyRedHat tsweeney@redhat.com

- What I did Docker login now takes the registry from /etc/containers/registries.conf if there's one and only one entry for registries in that file. Otherwise it defaults to docker.io. In either case the "Login Succeeded" message has been append with "to {servername}" to let the user know where the login was completed to.

- How I did it vi is my friend. Followed by testing.

- How to verify it docker login should show the registry that it logged into upon successful completion. If there is not just one registry listed in /etc/containers/registries.conf registries.search registries[] table and a server is not provided to docker login, then docker.io will be use as the server target.

- Description for the changelog

Testing:

# docker login
Login with your Docker ID to push and pull images from Docker Hub. If you don't have a Docker ID, head over to https://hub.docker.com to create one.
Username (tomsweeneyredhat):         
Password: 
Login Succeeded to https://registry-1.docker.io

- A picture of a cute animal (not mandatory but encouraged)

TomSweeneyRedHat commented 6 years ago

@runcom @redhatdan PTAL. Once this goes through, I'll post a similar patch to 1.13.1-RHEL.

TomSweeneyRedHat commented 6 years ago

This helps address: https://bugzilla.redhat.com/show_bug.cgi?id=1608460

runcom commented 6 years ago

This is breaking the Docker UI completely. People could have workflow where they might call Docker login w/o a registry and we are going to completely break it, meaning more bz on our way :)

TomSweeneyRedHat commented 6 years ago

I concur @runcom. I'm not crazy about this change for that reason and in addition it's adding a change that's different than upstream Docker and will need to be carried forward if we ever bump the Docker version. But on the other hand we're getting BZ's now in RHEL for people who do 'docker login' and they get "Login Succeeded" when they go against the default server in registries.conf and put in a bad un/pwd. They're freaking that's it a security concern. No good solution for this in my book.

rhatdan commented 6 years ago

Well we could compromise. If there is more then one registry, default to docker.io, when the user does not supply a registry. If just one then just use that registry.

rhatdan commented 6 years ago

We should also make it clear which registry you are logging into, by outputting the name of the registry.

TomSweeneyRedHat commented 6 years ago

@rhatdan @runcom I've changed this to login to docker.io by default if there is not just one entry in registries.conf and have touched up the 'docker login' success to show the server it succeeded on.

Note: This does not cure the original "Login Success" problem from the BZ, only hopefully enlightens the issue when hit.

TomSweeneyRedHat commented 3 years ago

I'm going to close this PR as it's been here for almost two years at this point and we're not taking any more changes to Docker at this point.