Add --no-container to fly ssh console - #5075
Open
dangra wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a consistent “session target” concept across SSH console and machine exec so users can explicitly choose between a container and the machine’s own namespace (via --no-container, and --container for fly machine exec).
Changes:
- Introduces
ssh.SessionTargetand updatesssh.Client.Shell/ssh.Consoleto accept it instead of a raw container string. - Adds
--no-containertofly ssh consoleand enforces mutual exclusivity with--containerduring container selection. - Adds
--containerand--no-containertofly machine execand wires them into theMachineExecRequest.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ssh/client.go | Adds SessionTarget and updates Client.Shell to set target-selection env vars. |
| internal/command/ssh/ssh_terminal.go | Updates Shell call site to pass an empty SessionTarget. |
| internal/command/ssh/console.go | Adds --no-container, passes SessionTarget through to ssh.Console, and enforces exclusivity with --container. |
| internal/command/ssh/console_test.go | Adds unit tests covering selectContainer behavior with --no-container. |
| internal/command/ssh/connect.go | Extends ConnectParams with Machine (for passing selection through command flow). |
| internal/command/postgres/barman.go | Updates ssh.Console call site for new SessionTarget signature. |
| internal/command/machine/run.go | Updates ssh.Console call site for new SessionTarget signature. |
| internal/command/machine/exec.go | Adds --container / --no-container flags and sets MachineExecRequest.Container / .Machine. |
| internal/command/console/console.go | Updates ssh.Console call site to pass SessionTarget. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+134
to
+138
| switch { | ||
| case target.Machine: | ||
| if err := sess.Setenv("FLY_SSH_MACHINE", "1"); err != nil { | ||
| return err | ||
| } |
dangra
force-pushed
the
pilot-machine-namespace-ssh
branch
from
August 13, 2026 04:48
6c493ed to
d84e821
Compare
On a machine with containers, `fly ssh console` always lands you inside one: the only container, the first of several, or one picked under --select. There was no way to reach the machine itself, which is what you get on a machine without containers. --no-container asks for the machine's own namespace instead. It sends an explicit opt-in rather than just omitting the container, so a hand-rolled SSH client using a cert from `fly ssh issue` is unaffected. Console and Client.Shell now take a SessionTarget rather than a container name, so the two mutually exclusive choices travel together.
dangra
force-pushed
the
pilot-machine-namespace-ssh
branch
from
August 13, 2026 04:56
d84e821 to
553175f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On a machine that has containers,
fly ssh consolealways lands you inside one: the only container, the first of several, or one you choose under--select. There was no way to ask for the machine itself, which is what you get on a machine without containers.--no-containerasks for the machine's own namespace:Passing it together with
--containeris an error.Behaviour
fly ssh issueis unaffected — it sets no environment and keeps landing where it does today.Notes for the reviewer
ssh.Consoleandssh.Client.Shelltake aSessionTargetin place of the trailing container name, since the container and the machine-namespace choice are mutually exclusive and belong together. Both are exported; this changes their signatures.SessionTargetnow rejects a target that sets both rather than quietly preferring one, andShelldefersClose()immediately after opening the session — previously an error fromSetenvcould return without closing it.fly sftpon a machine with containers already reads the machine's own filesystem rather than a container's. This PR does not change that, but it is worth knowing when comparing the two surfaces.fly machine execgets the same flag separately in #5077.