Skip to content

cmd/docker: Allow interactive cloud context resolution - #7354

Closed
nico1510 wants to merge 1 commit into
docker:masterfrom
nico1510:cloud-resolver-interactive-input
Closed

nico1510 wants to merge 1 commit into
docker:masterfrom
nico1510:cloud-resolver-interactive-input

Conversation

@nico1510

@nico1510 nico1510 commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Follow up to #7343.

Summary

Allow --cloud context providers to prompt during interactive resolution by forwarding stdin and stderr when both are terminals. Stdout remains reserved for the JSON response.

Pass terminal files directly to preserve terminal detection and avoid intermediary input buffering. On Windows, use the corresponding standard console handles when terminal wrappers do not expose their underlying files. Piped or redirected stdin remains available to the requested command.

Existing tests cover interactive input, piped and redirected stdin, redirected stderr, and noninteractive execution. go test ./cmd/docker, lint for the changed code, and cross-compilation for Windows amd64 pass.

The full unit suite previously had one failure, TestNodeAddrOptionSetHostOnlyIPv6, with Go 1.26.0. The same failure reproduces on unmodified master.

@codecov-commenter

codecov-commenter commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 58.33333% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cmd/docker/cloud.go 58.33% 4 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

Comment thread cmd/docker/cloud.go
// hiding terminal identity and potentially consuming the command's input.
cmd.Stdin = stdin
cmd.Stderr = stderr
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ISTR there were some quirks on Windows where we need to pass os.StdIn directly because term.StdStreams() can wrap the Windows console streams. In that case IsTerminal() is true but File() does not return the underlying *os.File.

I'm not on Windows to verify, but quick draft from my LLM;

stdinTerminal := dockerCli.In().IsTerminal()
stderrTerminal := dockerCli.Err().IsTerminal()

stdin, stdinFile := dockerCli.In().File()
stderr, stderrFile := dockerCli.Err().File()

// term.StdStreams may wrap the Windows console streams, in which case
// File() cannot expose the underlying *os.File. Pass the standard handles
// directly when the streams are terminals.
if runtime.GOOS == "windows" {
	if stdinTerminal {
		stdin, stdinFile = os.Stdin, true
	}
	if stderrTerminal {
		stderr, stderrFile = os.Stderr, true
	}
}

if stdinTerminal && stderrTerminal && stdinFile && stderrFile {
	// Pass files directly: wrapping them makes os/exec copy through pipes,
	// hiding terminal identity and potentially consuming the command's input.
	cmd.Stdin = stdin
	cmd.Stderr = stderr
}

@thaJeztah

Copy link
Copy Markdown
Member

FWIW; this test failure is unrelated, and can be ignored;

=== FAIL: e2e/container TestRunAttachedFromRemoteImageAndRemove (0.48s)
    run_test.go:42: assertion failed: 
        --- expected
        +++ actual
        @@ -1,4 +1,5 @@
         Unable to find image 'registry:5000/alpine:test-run-pulls' locally
         test-run-pulls: Pulling from alpine
        +63b65145d645: Pulling fs layer
         Digest: sha256:e2e16842c9b54d985bf1ef9242a313f36b856181f188de21313820e177002501
         Status: Downloaded newer image for registry:5000/alpine:test-run-pulls

Forward stdin and stderr to cloud context providers when both are
terminals, allowing prompts during --cloud resolution. Pass file handles
directly to preserve terminal detection and leave piped or redirected
input available to the requested command.

Use the standard console handles when Windows terminal wrappers hide
their underlying files. Keep stdout reserved for the JSON response.

Signed-off-by: Nicolas Beck <nicolas.beck@docker.com>
@thaJeztah

Copy link
Copy Markdown
Member

merged through #7356

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants