Skip to content

cli/command: PromptUserForCredentials: don't mutate cli #7084

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.

Sign up for GitHub

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

Open
thaJeztah wants to merge 1 commit into
base: master
Choose a base branch
from
+12 −7
Open

cli/command: PromptUserForCredentials: don't mutate cli #7084

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 12 additions & 7 deletions cli/command/registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,17 +97,22 @@ func GetDefaultAuthConfig(cfg *configfile.ConfigFile, checkCredStore bool, serve
// If defaultUsername is not empty, the username prompt includes that username
// and the user can hit enter without inputting a username to use that default
// username.
func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword, defaultUsername, serverAddress string) (registrytypes.AuthConfig, error) {
func PromptUserForCredentials(ctx context.Context, cli Streams, argUser, argPassword, defaultUsername, serverAddress string) (registrytypes.AuthConfig, error) {
// On Windows, force the use of the regular OS stdin stream.
//
// StdStreams() may wrap stdin with windowsconsole.NewAnsiReader to
// emulate VT input on consoles that do not support it natively, but
// that wrapper has historically caused interactive prompts to hang
// or behave incorrectly.
//
// See:
// - https://github.com/moby/moby/issues/14336
// - https://github.com/moby/moby/issues/14210
// - https://github.com/moby/moby/pull/17738
//
// TODO(thaJeztah): we need to confirm if this special handling is still needed, as we may not be doing this in other places.
stdIn := cli.In()
if runtime.GOOS == "windows" {
cli.SetIn(streams.NewIn(os.Stdin))
// TODO(thaJeztah); change to io.Reader and skip wrapping once prompt.DisableInputEcho no longer requires a streams.In
stdIn = streams.NewIn(os.Stdin)
}

argUser = strings.TrimSpace(argUser)
Expand All @@ -132,7 +137,7 @@ func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword
}

var err error
argUser, err = prompt.ReadInput(ctx, cli.In(), cli.Out(), msg)
argUser, err = prompt.ReadInput(ctx, stdIn, cli.Out(), msg)
if err != nil {
return registrytypes.AuthConfig{}, err
}
Expand All @@ -146,7 +151,7 @@ func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword

isEmpty := strings.TrimSpace(argPassword) == ""
if isEmpty {
restoreInput, err := prompt.DisableInputEcho(cli.In())
restoreInput, err := prompt.DisableInputEcho(stdIn)
if err != nil {
return registrytypes.AuthConfig{}, err
}
Expand All @@ -166,7 +171,7 @@ func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword
"To create a PAT, visit " + aec.Underline.Apply("https://app.docker.com/settings") + "\n\n")
}

argPassword, err = prompt.ReadInput(ctx, cli.In(), cli.Out(), "Password: ")
argPassword, err = prompt.ReadInput(ctx, stdIn, cli.Out(), "Password: ")
if err != nil {
return registrytypes.AuthConfig{}, err
}
Expand Down
Loading