From 3b54254177599abddf91028381507893bdf250ff Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Thu, 17 Apr 2025 11:23:24 +0400 Subject: [PATCH] feat: add coder connect exists hidden subcommand (#17418) Adds a new hidden subcommand `coder connect exists ` that checks if the name exists via Coder Connect. This will be used in SSH config to match only if Coder Connect is unavailable for the hostname in question, so that the SSH client will directly dial the workspace over an existing Coder Connect tunnel. Also refactors the way we inject a test DNS resolver into the lookup functions so that we can test from outside the `workspacesdk` package. --- cli/configssh.go | 3 +- cli/connect.go | 47 ++++++++++ cli/connect_test.go | 76 ++++++++++++++++ cli/root.go | 12 ++- cli/root_test.go | 3 +- codersdk/workspacesdk/workspacesdk.go | 39 +++++++-- .../workspacesdk_internal_test.go | 86 ------------------- codersdk/workspacesdk/workspacesdk_test.go | 74 ++++++++++++++++ 8 files changed, 242 insertions(+), 98 deletions(-) create mode 100644 cli/connect.go create mode 100644 cli/connect_test.go delete mode 100644 codersdk/workspacesdk/workspacesdk_internal_test.go diff --git a/cli/configssh.go b/cli/configssh.go index 6a0f41c2a2..c089141846 100644 --- a/cli/configssh.go +++ b/cli/configssh.go @@ -22,9 +22,10 @@ import ( "golang.org/x/exp/constraints" "golang.org/x/xerrors" + "github.com/coder/serpent" + "github.com/coder/coder/v2/cli/cliui" "github.com/coder/coder/v2/codersdk" - "github.com/coder/serpent" ) const ( diff --git a/cli/connect.go b/cli/connect.go new file mode 100644 index 0000000000..d1245147f3 --- /dev/null +++ b/cli/connect.go @@ -0,0 +1,47 @@ +package cli + +import ( + "github.com/coder/serpent" + + "github.com/coder/coder/v2/codersdk/workspacesdk" +) + +func (r *RootCmd) connectCmd() *serpent.Command { + cmd := &serpent.Command{ + Use: "connect", + Short: "Commands related to Coder Connect (OS-level tunneled connection to workspaces).", + Handler: func(i *serpent.Invocation) error { + return i.Command.HelpHandler(i) + }, + Hidden: true, + Children: []*serpent.Command{ + r.existsCmd(), + }, + } + return cmd +} + +func (*RootCmd) existsCmd() *serpent.Command { + cmd := &serpent.Command{ + Use: "exists ", + Short: "Checks if the given hostname exists via Coder Connect.", + Long: "This command is designed to be used in scripts to check if the given hostname exists via Coder " + + "Connect. It prints no output. It returns exit code 0 if it does exist and code 1 if it does not.", + Middleware: serpent.Chain( + serpent.RequireNArgs(1), + ), + Handler: func(inv *serpent.Invocation) error { + hostname := inv.Args[0] + exists, err := workspacesdk.ExistsViaCoderConnect(inv.Context(), hostname) + if err != nil { + return err + } + if !exists { + // we don't want to print any output, since this command is designed to be a check in scripts / SSH config. + return ErrSilent + } + return nil + }, + } + return cmd +} diff --git a/cli/connect_test.go b/cli/connect_test.go new file mode 100644 index 0000000000..031cd2f95b --- /dev/null +++ b/cli/connect_test.go @@ -0,0 +1,76 @@ +package cli_test + +import ( + "bytes" + "context" + "net" + "testing" + + "github.com/stretchr/testify/require" + "tailscale.com/net/tsaddr" + + "github.com/coder/serpent" + + "github.com/coder/coder/v2/cli" + "github.com/coder/coder/v2/codersdk/workspacesdk" + "github.com/coder/coder/v2/testutil" +) + +func TestConnectExists_Running(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + + var root cli.RootCmd + cmd, err := root.Command(root.AGPL()) + require.NoError(t, err) + + inv := (&serpent.Invocation{ + Command: cmd, + Args: []string{"connect", "exists", "test.example"}, + }).WithContext(withCoderConnectRunning(ctx)) + stdout := new(bytes.Buffer) + stderr := new(bytes.Buffer) + inv.Stdout = stdout + inv.Stderr = stderr + err = inv.Run() + require.NoError(t, err) +} + +func TestConnectExists_NotRunning(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + + var root cli.RootCmd + cmd, err := root.Command(root.AGPL()) + require.NoError(t, err) + + inv := (&serpent.Invocation{ + Command: cmd, + Args: []string{"connect", "exists", "test.example"}, + }).WithContext(withCoderConnectNotRunning(ctx)) + stdout := new(bytes.Buffer) + stderr := new(bytes.Buffer) + inv.Stdout = stdout + inv.Stderr = stderr + err = inv.Run() + require.ErrorIs(t, err, cli.ErrSilent) +} + +type fakeResolver struct { + shouldReturnSuccess bool +} + +func (f *fakeResolver) LookupIP(_ context.Context, _, _ string) ([]net.IP, error) { + if f.shouldReturnSuccess { + return []net.IP{net.ParseIP(tsaddr.CoderServiceIPv6().String())}, nil + } + return nil, &net.DNSError{IsNotFound: true} +} + +func withCoderConnectRunning(ctx context.Context) context.Context { + return workspacesdk.WithTestOnlyCoderContextResolver(ctx, &fakeResolver{shouldReturnSuccess: true}) +} + +func withCoderConnectNotRunning(ctx context.Context) context.Context { + return workspacesdk.WithTestOnlyCoderContextResolver(ctx, &fakeResolver{shouldReturnSuccess: false}) +} diff --git a/cli/root.go b/cli/root.go index 75cbb4dd2c..5c70379b75 100644 --- a/cli/root.go +++ b/cli/root.go @@ -31,6 +31,8 @@ import ( "github.com/coder/pretty" + "github.com/coder/serpent" + "github.com/coder/coder/v2/buildinfo" "github.com/coder/coder/v2/cli/cliui" "github.com/coder/coder/v2/cli/config" @@ -38,7 +40,6 @@ import ( "github.com/coder/coder/v2/cli/telemetry" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/agentsdk" - "github.com/coder/serpent" ) var ( @@ -49,6 +50,10 @@ var ( workspaceCommand = map[string]string{ "workspaces": "", } + + // ErrSilent is a sentinel error that tells the command handler to just exit with a non-zero error, but not print + // anything. + ErrSilent = xerrors.New("silent error") ) const ( @@ -122,6 +127,7 @@ func (r *RootCmd) CoreSubcommands() []*serpent.Command { r.whoami(), // Hidden + r.connectCmd(), r.expCmd(), r.gitssh(), r.support(), @@ -175,6 +181,10 @@ func (r *RootCmd) RunWithSubcommands(subcommands []*serpent.Command) { //nolint:revive,gocritic os.Exit(code) } + if errors.Is(err, ErrSilent) { + //nolint:revive,gocritic + os.Exit(code) + } f := PrettyErrorFormatter{w: os.Stderr, verbose: r.verbose} if err != nil { f.Format(err) diff --git a/cli/root_test.go b/cli/root_test.go index ac14541526..698c9aff60 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -10,12 +10,13 @@ import ( "sync/atomic" "testing" + "github.com/coder/serpent" + "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/pty/ptytest" "github.com/coder/coder/v2/testutil" - "github.com/coder/serpent" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/codersdk/workspacesdk/workspacesdk.go b/codersdk/workspacesdk/workspacesdk.go index 25188917da..83f236a215 100644 --- a/codersdk/workspacesdk/workspacesdk.go +++ b/codersdk/workspacesdk/workspacesdk.go @@ -20,11 +20,12 @@ import ( "cdr.dev/slog" + "github.com/coder/quartz" + "github.com/coder/websocket" + "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/tailnet" "github.com/coder/coder/v2/tailnet/proto" - "github.com/coder/quartz" - "github.com/coder/websocket" ) var ErrSkipClose = xerrors.New("skip tailnet close") @@ -128,19 +129,16 @@ func init() { } } -type resolver interface { +type Resolver interface { LookupIP(ctx context.Context, network, host string) ([]net.IP, error) } type Client struct { client *codersdk.Client - - // overridden in tests - resolver resolver } func New(c *codersdk.Client) *Client { - return &Client{client: c, resolver: net.DefaultResolver} + return &Client{client: c} } // AgentConnectionInfo returns required information for establishing @@ -392,6 +390,12 @@ func (c *Client) AgentReconnectingPTY(ctx context.Context, opts WorkspaceAgentRe return websocket.NetConn(context.Background(), conn, websocket.MessageBinary), nil } +func WithTestOnlyCoderContextResolver(ctx context.Context, r Resolver) context.Context { + return context.WithValue(ctx, dnsResolverContextKey{}, r) +} + +type dnsResolverContextKey struct{} + type CoderConnectQueryOptions struct { HostnameSuffix string } @@ -409,15 +413,32 @@ func (c *Client) IsCoderConnectRunning(ctx context.Context, o CoderConnectQueryO suffix = info.HostnameSuffix } domainName := fmt.Sprintf(tailnet.IsCoderConnectEnabledFmtString, suffix) + return ExistsViaCoderConnect(ctx, domainName) +} + +func testOrDefaultResolver(ctx context.Context) Resolver { + // check the context for a non-default resolver. This is only used in testing. + resolver, ok := ctx.Value(dnsResolverContextKey{}).(Resolver) + if !ok || resolver == nil { + resolver = net.DefaultResolver + } + return resolver +} + +// ExistsViaCoderConnect checks if the given hostname exists via Coder Connect. This doesn't guarantee the +// workspace is actually reachable, if, for example, its agent is unhealthy, but rather that Coder Connect knows about +// the workspace and advertises the hostname via DNS. +func ExistsViaCoderConnect(ctx context.Context, hostname string) (bool, error) { + resolver := testOrDefaultResolver(ctx) var dnsError *net.DNSError - ips, err := c.resolver.LookupIP(ctx, "ip6", domainName) + ips, err := resolver.LookupIP(ctx, "ip6", hostname) if xerrors.As(err, &dnsError) { if dnsError.IsNotFound { return false, nil } } if err != nil { - return false, xerrors.Errorf("lookup DNS %s: %w", domainName, err) + return false, xerrors.Errorf("lookup DNS %s: %w", hostname, err) } // The returned IP addresses are probably from the Coder Connect DNS server, but there are sometimes weird captive diff --git a/codersdk/workspacesdk/workspacesdk_internal_test.go b/codersdk/workspacesdk/workspacesdk_internal_test.go deleted file mode 100644 index 1b98ebdc2e..0000000000 --- a/codersdk/workspacesdk/workspacesdk_internal_test.go +++ /dev/null @@ -1,86 +0,0 @@ -package workspacesdk - -import ( - "context" - "fmt" - "net" - "net/http" - "net/http/httptest" - "net/url" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "golang.org/x/xerrors" - - "github.com/coder/coder/v2/coderd/httpapi" - "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/testutil" - - "tailscale.com/net/tsaddr" - - "github.com/coder/coder/v2/tailnet" -) - -func TestClient_IsCoderConnectRunning(t *testing.T) { - t.Parallel() - ctx := testutil.Context(t, testutil.WaitShort) - - srv := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/api/v2/workspaceagents/connection", r.URL.Path) - httpapi.Write(ctx, rw, http.StatusOK, AgentConnectionInfo{ - HostnameSuffix: "test", - }) - })) - defer srv.Close() - - apiURL, err := url.Parse(srv.URL) - require.NoError(t, err) - sdkClient := codersdk.New(apiURL) - client := New(sdkClient) - - // Right name, right IP - expectedName := fmt.Sprintf(tailnet.IsCoderConnectEnabledFmtString, "test") - client.resolver = &fakeResolver{t: t, hostMap: map[string][]net.IP{ - expectedName: {net.ParseIP(tsaddr.CoderServiceIPv6().String())}, - }} - - result, err := client.IsCoderConnectRunning(ctx, CoderConnectQueryOptions{}) - require.NoError(t, err) - require.True(t, result) - - // Wrong name - result, err = client.IsCoderConnectRunning(ctx, CoderConnectQueryOptions{HostnameSuffix: "coder"}) - require.NoError(t, err) - require.False(t, result) - - // Not found - client.resolver = &fakeResolver{t: t, err: &net.DNSError{IsNotFound: true}} - result, err = client.IsCoderConnectRunning(ctx, CoderConnectQueryOptions{}) - require.NoError(t, err) - require.False(t, result) - - // Some other error - client.resolver = &fakeResolver{t: t, err: xerrors.New("a bad thing happened")} - _, err = client.IsCoderConnectRunning(ctx, CoderConnectQueryOptions{}) - require.Error(t, err) - - // Right name, wrong IP - client.resolver = &fakeResolver{t: t, hostMap: map[string][]net.IP{ - expectedName: {net.ParseIP("2001::34")}, - }} - result, err = client.IsCoderConnectRunning(ctx, CoderConnectQueryOptions{}) - require.NoError(t, err) - require.False(t, result) -} - -type fakeResolver struct { - t testing.TB - hostMap map[string][]net.IP - err error -} - -func (f *fakeResolver) LookupIP(_ context.Context, network, host string) ([]net.IP, error) { - assert.Equal(f.t, "ip6", network) - return f.hostMap[host], f.err -} diff --git a/codersdk/workspacesdk/workspacesdk_test.go b/codersdk/workspacesdk/workspacesdk_test.go index e7ccd96e20..16a523b2d4 100644 --- a/codersdk/workspacesdk/workspacesdk_test.go +++ b/codersdk/workspacesdk/workspacesdk_test.go @@ -1,12 +1,18 @@ package workspacesdk_test import ( + "context" + "fmt" + "net" "net/http" "net/http/httptest" "net/url" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/xerrors" + "tailscale.com/net/tsaddr" "tailscale.com/tailcfg" "github.com/coder/websocket" @@ -15,6 +21,7 @@ import ( "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/agentsdk" "github.com/coder/coder/v2/codersdk/workspacesdk" + "github.com/coder/coder/v2/tailnet" "github.com/coder/coder/v2/testutil" ) @@ -72,3 +79,70 @@ func TestWorkspaceDialerFailure(t *testing.T) { // Then: an error indicating a database issue is returned, to conditionalize the behavior of the caller. require.ErrorIs(t, err, codersdk.ErrDatabaseNotReachable) } + +func TestClient_IsCoderConnectRunning(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + + srv := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/api/v2/workspaceagents/connection", r.URL.Path) + httpapi.Write(ctx, rw, http.StatusOK, workspacesdk.AgentConnectionInfo{ + HostnameSuffix: "test", + }) + })) + defer srv.Close() + + apiURL, err := url.Parse(srv.URL) + require.NoError(t, err) + sdkClient := codersdk.New(apiURL) + client := workspacesdk.New(sdkClient) + + // Right name, right IP + expectedName := fmt.Sprintf(tailnet.IsCoderConnectEnabledFmtString, "test") + ctxResolveExpected := workspacesdk.WithTestOnlyCoderContextResolver(ctx, + &fakeResolver{t: t, hostMap: map[string][]net.IP{ + expectedName: {net.ParseIP(tsaddr.CoderServiceIPv6().String())}, + }}) + + result, err := client.IsCoderConnectRunning(ctxResolveExpected, workspacesdk.CoderConnectQueryOptions{}) + require.NoError(t, err) + require.True(t, result) + + // Wrong name + result, err = client.IsCoderConnectRunning(ctxResolveExpected, workspacesdk.CoderConnectQueryOptions{HostnameSuffix: "coder"}) + require.NoError(t, err) + require.False(t, result) + + // Not found + ctxResolveNotFound := workspacesdk.WithTestOnlyCoderContextResolver(ctx, + &fakeResolver{t: t, err: &net.DNSError{IsNotFound: true}}) + result, err = client.IsCoderConnectRunning(ctxResolveNotFound, workspacesdk.CoderConnectQueryOptions{}) + require.NoError(t, err) + require.False(t, result) + + // Some other error + ctxResolverErr := workspacesdk.WithTestOnlyCoderContextResolver(ctx, + &fakeResolver{t: t, err: xerrors.New("a bad thing happened")}) + _, err = client.IsCoderConnectRunning(ctxResolverErr, workspacesdk.CoderConnectQueryOptions{}) + require.Error(t, err) + + // Right name, wrong IP + ctxResolverWrongIP := workspacesdk.WithTestOnlyCoderContextResolver(ctx, + &fakeResolver{t: t, hostMap: map[string][]net.IP{ + expectedName: {net.ParseIP("2001::34")}, + }}) + result, err = client.IsCoderConnectRunning(ctxResolverWrongIP, workspacesdk.CoderConnectQueryOptions{}) + require.NoError(t, err) + require.False(t, result) +} + +type fakeResolver struct { + t testing.TB + hostMap map[string][]net.IP + err error +} + +func (f *fakeResolver) LookupIP(_ context.Context, network, host string) ([]net.IP, error) { + assert.Equal(f.t, "ip6", network) + return f.hostMap[host], f.err +}