Skip to content

Commit 1161bcd

Browse files
committed
hostagent: trim comments in SSH banner probe to one-liners
Drop block comments that narrate the race rationale or restate what adjacent code does. The protocol-level reasoning lives in the PR description and commit messages; the code itself reads. Signed-off-by: Weishi Z <amwish.zeng@gmail.com>
1 parent 1e434c0 commit 1161bcd

2 files changed

Lines changed: 5 additions & 18 deletions

File tree

pkg/hostagent/requirements.go

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -160,9 +160,7 @@ func (a *HostAgent) waitForRequirement(ctx context.Context, r requirement) error
160160
return nil
161161
}
162162

163-
// requirement describes one readiness check performed before Ready fires.
164-
// If check is non-nil, it is invoked directly and script is ignored;
165-
// otherwise script is executed in the guest via ssh.ExecuteScript.
163+
// requirement is one readiness check; check takes precedence over script.
166164
type requirement struct {
167165
description string
168166
script string
@@ -174,12 +172,6 @@ type requirement struct {
174172

175173
func (a *HostAgent) essentialRequirements() []requirement {
176174
req := make([]requirement, 0)
177-
// Run a host-side TCP probe before the script-based "ssh" requirement
178-
// below. The script path uses Lima's own SSH client, which retries
179-
// internally and so can mask a race where the host agent accepts on the
180-
// forwarded port before guest sshd has bound :22. External clients
181-
// (ssh-keyscan, sftp, rsync, ...) that connect during that window get
182-
// EPIPE on their first write.
183175
logLocation := "/var/log/cloud-init-output.log in the guest"
184176
if a.instConfig.OS != nil && *a.instConfig.OS == limatype.DARWIN {
185177
logLocation = "serialv.log in the host"
@@ -335,13 +327,12 @@ Check "` + logLocation + `" to see where the process is blocked!
335327

336328
const (
337329
probeSSHBannerTimeout = 5 * time.Second
338-
// RFC 4253 §4.2 allows the server to emit lines before the identification
339-
// string. Cap the read so a misbehaving peer can't loop forever.
330+
// RFC 4253 §4.2: server may emit lines before the identification string.
340331
probeSSHBannerMaxLines = 50
341332
)
342333

343-
// probeSSHBannerOnLocalPort dials <address>:<port> and returns nil when a
344-
// line starting with "SSH-" is read within probeSSHBannerMaxLines.
334+
// probeSSHBannerOnLocalPort returns nil when a line starting with "SSH-" is
335+
// read from address:port within probeSSHBannerMaxLines.
345336
func probeSSHBannerOnLocalPort(ctx context.Context, address string, port int) error {
346337
addr := net.JoinHostPort(address, strconv.Itoa(port))
347338
d := net.Dialer{Timeout: probeSSHBannerTimeout}
@@ -350,7 +341,6 @@ func probeSSHBannerOnLocalPort(ctx context.Context, address string, port int) er
350341
return fmt.Errorf("dial %s: %w", addr, err)
351342
}
352343
defer conn.Close()
353-
// Propagate ctx cancellation to the in-flight read.
354344
stop := context.AfterFunc(ctx, func() { _ = conn.Close() })
355345
defer stop()
356346
if err := conn.SetReadDeadline(time.Now().Add(probeSSHBannerTimeout)); err != nil {

pkg/hostagent/requirements_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ import (
1616

1717
const testHost = "127.0.0.1"
1818

19-
// startBannerServer listens on 127.0.0.1:0 and writes banner to each accepted
20-
// connection before closing it. An empty banner triggers immediate close.
19+
// startBannerServer writes banner to each accepted conn then closes it.
2120
func startBannerServer(t *testing.T, banner string) int {
2221
t.Helper()
2322
var lc net.ListenConfig
@@ -61,7 +60,6 @@ func TestProbeSSHBannerOnLocalPort_AnySSHPrefix(t *testing.T) {
6160
}
6261
}
6362

64-
// RFC 4253 §4.2: server MAY send lines of data before the identification string.
6563
func TestProbeSSHBannerOnLocalPort_WithPreamble(t *testing.T) {
6664
port := startBannerServer(t, "Welcome to ExampleHost\r\nLegal notice\r\nSSH-2.0-OpenSSH_9.6p1\r\n")
6765
assert.NilError(t, probeSSHBannerOnLocalPort(t.Context(), testHost, port))
@@ -94,7 +92,6 @@ func TestProbeSSHBannerOnLocalPort_NoListener(t *testing.T) {
9492
assert.Assert(t, strings.Contains(err.Error(), "dial 127.0.0.1:"+portStr))
9593
}
9694

97-
// Server accepts but never writes nor closes; probe must honor its read deadline.
9895
func TestProbeSSHBannerOnLocalPort_HungWrite(t *testing.T) {
9996
var lc net.ListenConfig
10097
ln, err := lc.Listen(t.Context(), "tcp", "127.0.0.1:0")

0 commit comments

Comments
 (0)