Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore: refactor TestServer_X11 to use inproc networking#18564

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
spikecurtis wants to merge1 commit intospike/testutil-in-proc-net
base:spike/testutil-in-proc-net
Choose a base branch
Loading
fromspike/18263-x11-network
Open
Show file tree
Hide file tree
Changes fromall commits
Commits
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
10 changes: 10 additions & 0 deletionsagent/agentssh/agentssh.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -117,6 +117,10 @@ type Config struct {
// Note that this is different from the devcontainers feature, which uses
// subagents.
ExperimentalContainersbool
// X11Net allows overriding the networking implementation used for X11
// forwarding listeners. When nil, a default implementation backed by the
// standard library networking package is used.
X11NetX11Network
}

typeServerstruct {
Expand DownExpand Up@@ -196,6 +200,12 @@ func NewServer(ctx context.Context, logger slog.Logger, prometheusRegistry *prom
displayOffset:*config.X11DisplayOffset,
sessions:make(map[*x11Session]struct{}),
connections:make(map[net.Conn]struct{}),
network:func()X11Network {
ifconfig.X11Net!=nil {
returnconfig.X11Net
}
returnosNet{}
}(),
},
}

Expand Down
58 changes: 43 additions & 15 deletionsagent/agentssh/x11.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"io"
"math"
"net"
"os"
"path/filepath"
Expand DownExpand Up@@ -37,12 +38,30 @@ const (
X11MaxPort = X11StartPort + X11MaxDisplays
)

// X11Network abstracts the creation of network listeners for X11 forwarding.
// It is intended mainly for testing; production code uses the default
// implementation backed by the operating system networking stack.
type X11Network interface {
Listen(network, address string) (net.Listener, error)
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: It'd be nice to keep this as aListen(ctx, ...) as previously via ListenConfig.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I decided against this because I wanted to reuseInProcNet, which needs to matchthis interface in Serpent. Code reuse beat out the presumably slight benefit of being able to bail out of a Listen call before the OS responds.

}

// osNet is the default X11Network implementation that uses the standard
// library network stack.
type osNet struct{}

func (osNet) Listen(network, address string) (net.Listener, error) {
return net.Listen(network, address)
}

type x11Forwarder struct {
logger slog.Logger
x11HandlerErrors *prometheus.CounterVec
fs afero.Fs
displayOffset int

// network creates X11 listener sockets. Defaults to osNet{}.
network X11Network

mu sync.Mutex
sessions map[*x11Session]struct{}
connections map[net.Conn]struct{}
Expand DownExpand Up@@ -145,26 +164,35 @@ func (x *x11Forwarder) listenForConnections(ctx context.Context, session *x11Ses
x.cleanSession(session)
}

tcpConn, ok := conn.(*net.TCPConn)
if !ok {
x.logger.Warn(ctx, fmt.Sprintf("failed to cast connection to TCPConn. got: %T", conn))
_ = conn.Close()
continue
var originAddr string
var originPort uint32

if tcpConn, ok := conn.(*net.TCPConn); ok {
if tcpAddr, ok := tcpConn.LocalAddr().(*net.TCPAddr); ok {
originAddr = tcpAddr.IP.String()
// #nosec G115 - Safe conversion as TCP port numbers are within uint32 range (0-65535)
originPort = uint32(tcpAddr.Port)
}
}
tcpAddr, ok := tcpConn.LocalAddr().(*net.TCPAddr)
if !ok {
x.logger.Warn(ctx, fmt.Sprintf("failed to cast local address to TCPAddr. got: %T", tcpConn.LocalAddr()))
_ = conn.Close()
continue
// Fallback values for in-memory or non-TCP connections.
if originAddr == "" {
originAddr = "127.0.0.1"
}
if originPort == 0 {
p := X11StartPort + session.display
if p > math.MaxUint32 {
panic("overflow")
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should panic here as that risks killing the whole agent in the unlikely event that we don't have aTCPConn. Logging and closing the connection seems more appropriate especially considering the comment above "or non-TCP connections" suggesting it's not only for tests.

}
// #nosec G115 - Safe conversion as port number is within uint32 range
originPort = uint32(p)
}

channel, reqs, err := serverConn.OpenChannel("x11", gossh.Marshal(struct {
OriginatorAddress string
OriginatorPort uint32
}{
OriginatorAddress: tcpAddr.IP.String(),
// #nosec G115 - Safe conversion as TCP port numbers are within uint32 range (0-65535)
OriginatorPort: uint32(tcpAddr.Port),
OriginatorAddress: originAddr,
OriginatorPort: originPort,
}))
if err != nil {
x.logger.Warn(ctx, "failed to open X11 channel", slog.Error(err))
Expand DownExpand Up@@ -281,13 +309,13 @@ func (x *x11Forwarder) evictLeastRecentlyUsedSession() {
// createX11Listener creates a listener for X11 forwarding, it will use
// the next available port starting from X11StartPort and displayOffset.
func (x *x11Forwarder) createX11Listener(ctx context.Context) (ln net.Listener, display int, err error) {
var lc net.ListenConfig
// Look for an open port to listen on.
for port := X11StartPort + x.displayOffset; port <= X11MaxPort; port++ {
if ctx.Err() != nil {
return nil, -1, ctx.Err()
}
ln, err = lc.Listen(ctx, "tcp", fmt.Sprintf("localhost:%d", port))

ln, err = x.network.Listen("tcp", fmt.Sprintf("localhost:%d", port))
if err == nil {
display = port - X11StartPort
return ln, display, nil
Expand Down
32 changes: 19 additions & 13 deletionsagent/agentssh/x11_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,7 +3,6 @@ package agentssh_test
import (
"bufio"
"bytes"
"context"
"encoding/hex"
"fmt"
"net"
Expand DownExpand Up@@ -32,10 +31,19 @@ func TestServer_X11(t *testing.T) {
t.Skip("X11 forwarding is only supported on Linux")
}

ctx :=context.Background()
ctx :=testutil.Context(t, testutil.WaitShort)
logger := testutil.Logger(t)
fs := afero.NewMemMapFs()
s, err := agentssh.NewServer(ctx, logger, prometheus.NewRegistry(), fs, agentexec.DefaultExecer, &agentssh.Config{})

// Use in-process networking for X11 forwarding.
inproc := testutil.NewInProcNet()

// Create server config with custom X11 listener.
cfg := &agentssh.Config{
X11Net: inproc,
}

s, err := agentssh.NewServer(ctx, logger, prometheus.NewRegistry(), fs, agentexec.DefaultExecer, cfg)
require.NoError(t, err)
defer s.Close()
err = s.UpdateHostSigner(42)
Expand DownExpand Up@@ -93,17 +101,15 @@ func TestServer_X11(t *testing.T) {

x11Chans := c.HandleChannelOpen("x11")
payload := "hello world"
require.Eventually(t, func() bool {
conn, err := net.Dial("tcp", fmt.Sprintf("localhost:%d", agentssh.X11StartPort+displayNumber))
if err == nil {
_, err = conn.Write([]byte(payload))
assert.NoError(t, err)
_ = conn.Close()
}
return err == nil
}, testutil.WaitShort, testutil.IntervalFast)
go func() {
conn, err := inproc.Dial(ctx, testutil.NewAddr("tcp", fmt.Sprintf("localhost:%d", agentssh.X11StartPort+displayNumber)))
assert.NoError(t, err)
_, err = conn.Write([]byte(payload))
assert.NoError(t, err)
_ = conn.Close()
}()

x11 :=<-x11Chans
x11 :=testutil.RequireReceive(ctx, t,x11Chans)
ch, reqs, err := x11.Accept()
require.NoError(t, err)
go gossh.DiscardRequests(reqs)
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp