From c2bc664101b46341030eb91e46d34195d6da009e Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Mon, 12 Sep 2022 02:08:44 +0000 Subject: [PATCH 1/3] feat: Use Tailscale networking by default Removal of WebRTC code will happen in another PR, but it felt dangerious to default and remove in a single commit. Ideally, we can release this version and collect final thoughts and feedback before a full commitment. --- cli/agent_test.go | 6 +++--- cli/configssh.go | 2 +- cli/gitssh_test.go | 2 +- cli/portforward.go | 2 +- cli/portforward_test.go | 2 +- cli/server.go | 2 +- cli/ssh.go | 2 +- coderd/coderd.go | 6 ++++++ 8 files changed, 15 insertions(+), 9 deletions(-) diff --git a/cli/agent_test.go b/cli/agent_test.go index 39662a1cde89f..82c199cd6268f 100644 --- a/cli/agent_test.go +++ b/cli/agent_test.go @@ -47,7 +47,7 @@ func TestWorkspaceAgent(t *testing.T) { workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - cmd, _ := clitest.New(t, "agent", "--auth", "azure-instance-identity", "--agent-url", client.URL.String(), "--wireguard=false") + cmd, _ := clitest.New(t, "agent", "--auth", "azure-instance-identity", "--agent-url", client.URL.String()) ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() errC := make(chan error) @@ -105,7 +105,7 @@ func TestWorkspaceAgent(t *testing.T) { workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - cmd, _ := clitest.New(t, "agent", "--auth", "aws-instance-identity", "--agent-url", client.URL.String(), "--wireguard=false") + cmd, _ := clitest.New(t, "agent", "--auth", "aws-instance-identity", "--agent-url", client.URL.String()) ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() errC := make(chan error) @@ -163,7 +163,7 @@ func TestWorkspaceAgent(t *testing.T) { workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - cmd, _ := clitest.New(t, "agent", "--auth", "google-instance-identity", "--agent-url", client.URL.String(), "--wireguard=false") + cmd, _ := clitest.New(t, "agent", "--auth", "google-instance-identity", "--agent-url", client.URL.String()) ctx, cancelFunc := context.WithCancel(context.Background()) defer cancelFunc() errC := make(chan error) diff --git a/cli/configssh.go b/cli/configssh.go index 79eb280559697..7ebb9dddf5d8b 100644 --- a/cli/configssh.go +++ b/cli/configssh.go @@ -374,7 +374,7 @@ func configSSH() *cobra.Command { cmd.Flags().BoolVarP(&skipProxyCommand, "skip-proxy-command", "", false, "Specifies whether the ProxyCommand option should be skipped. Useful for testing.") _ = cmd.Flags().MarkHidden("skip-proxy-command") cliflag.BoolVarP(cmd.Flags(), &usePreviousOpts, "use-previous-options", "", "CODER_SSH_USE_PREVIOUS_OPTIONS", false, "Specifies whether or not to keep options from previous run of config-ssh.") - cliflag.BoolVarP(cmd.Flags(), &wireguard, "wireguard", "", "CODER_CONFIG_SSH_WIREGUARD", false, "Whether to use Wireguard for SSH tunneling.") + cliflag.BoolVarP(cmd.Flags(), &wireguard, "wireguard", "", "CODER_CONFIG_SSH_WIREGUARD", true, "Whether to use Wireguard for SSH tunneling.") _ = cmd.Flags().MarkHidden("wireguard") cliui.AllowSkipPrompt(cmd) diff --git a/cli/gitssh_test.go b/cli/gitssh_test.go index cdbbe6bd1f4ff..d8846430bee30 100644 --- a/cli/gitssh_test.go +++ b/cli/gitssh_test.go @@ -59,7 +59,7 @@ func TestGitSSH(t *testing.T) { coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) // start workspace agent - cmd, root := clitest.New(t, "agent", "--agent-token", agentToken, "--agent-url", client.URL.String(), "--wireguard=false") + cmd, root := clitest.New(t, "agent", "--agent-token", agentToken, "--agent-url", client.URL.String()) agentClient := client clitest.SetupConfig(t, agentClient, root) ctx, cancelFunc := context.WithCancel(context.Background()) diff --git a/cli/portforward.go b/cli/portforward.go index ac4561163f837..54817f71f590e 100644 --- a/cli/portforward.go +++ b/cli/portforward.go @@ -166,7 +166,7 @@ func portForward() *cobra.Command { cmd.Flags().StringArrayVarP(&tcpForwards, "tcp", "p", []string{}, "Forward a TCP port from the workspace to the local machine") cmd.Flags().StringArrayVar(&udpForwards, "udp", []string{}, "Forward a UDP port from the workspace to the local machine. The UDP connection has TCP-like semantics to support stateful UDP protocols") cmd.Flags().StringArrayVar(&unixForwards, "unix", []string{}, "Forward a Unix socket in the workspace to a local Unix socket or TCP port") - cmd.Flags().BoolVarP(&wireguard, "wireguard", "", false, "Specifies whether to use wireguard networking or not.") + cmd.Flags().BoolVarP(&wireguard, "wireguard", "", true, "Specifies whether to use wireguard networking or not.") _ = cmd.Flags().MarkHidden("wireguard") return cmd } diff --git a/cli/portforward_test.go b/cli/portforward_test.go index d98fbec7920c0..8ff3e2a846d09 100644 --- a/cli/portforward_test.go +++ b/cli/portforward_test.go @@ -391,7 +391,7 @@ func runAgent(t *testing.T, client *codersdk.Client, userID uuid.UUID) ([]coders coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) // Start workspace agent in a goroutine - cmd, root := clitest.New(t, "agent", "--agent-token", agentToken, "--agent-url", client.URL.String(), "--wireguard=false") + cmd, root := clitest.New(t, "agent", "--agent-token", agentToken, "--agent-url", client.URL.String()) clitest.SetupConfig(t, client, root) errC := make(chan error) agentCtx, agentCancel := context.WithCancel(ctx) diff --git a/cli/server.go b/cli/server.go index 925a1a619a840..90fb8700c85ba 100644 --- a/cli/server.go +++ b/cli/server.go @@ -812,7 +812,7 @@ func Server(newAPI func(*coderd.Options) *coderd.API) *cobra.Command { "Specifies an issuer URL to use for OIDC.") cliflag.StringArrayVarP(root.Flags(), &oidcScopes, "oidc-scopes", "", "CODER_OIDC_SCOPES", []string{oidc.ScopeOpenID, "profile", "email"}, "Specifies scopes to grant when authenticating with OIDC.") - cliflag.BoolVarP(root.Flags(), &tailscaleEnable, "tailscale", "", "CODER_TAILSCALE", false, + cliflag.BoolVarP(root.Flags(), &tailscaleEnable, "tailscale", "", "CODER_TAILSCALE", true, "Specifies whether Tailscale networking is used for web applications and terminals.") _ = root.Flags().MarkHidden("tailscale") enableTelemetryByDefault := !isTest() diff --git a/cli/ssh.go b/cli/ssh.go index 7f23cce706c20..cf72c6f384394 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -214,7 +214,7 @@ func ssh() *cobra.Command { cliflag.BoolVarP(cmd.Flags(), &forwardAgent, "forward-agent", "A", "CODER_SSH_FORWARD_AGENT", false, "Specifies whether to forward the SSH agent specified in $SSH_AUTH_SOCK") cliflag.StringVarP(cmd.Flags(), &identityAgent, "identity-agent", "", "CODER_SSH_IDENTITY_AGENT", "", "Specifies which identity agent to use (overrides $SSH_AUTH_SOCK), forward agent must also be enabled") cliflag.DurationVarP(cmd.Flags(), &wsPollInterval, "workspace-poll-interval", "", "CODER_WORKSPACE_POLL_INTERVAL", workspacePollInterval, "Specifies how often to poll for workspace automated shutdown.") - cliflag.BoolVarP(cmd.Flags(), &wireguard, "wireguard", "", "CODER_SSH_WIREGUARD", false, "Whether to use Wireguard for SSH tunneling.") + cliflag.BoolVarP(cmd.Flags(), &wireguard, "wireguard", "", "CODER_SSH_WIREGUARD", true, "Whether to use Wireguard for SSH tunneling.") _ = cmd.Flags().MarkHidden("wireguard") return cmd diff --git a/coderd/coderd.go b/coderd/coderd.go index 14a18e7986aae..efcf6a3cea11a 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -95,6 +95,12 @@ func New(options *Options) *API { if options.APIRateLimit == 0 { options.APIRateLimit = 512 } + if options.AgentStatsRefreshInterval == 0 { + options.AgentStatsRefreshInterval = 10 * time.Minute + } + if options.MetricsCacheRefreshInterval == 0 { + options.MetricsCacheRefreshInterval = time.Hour + } if options.Authorizer == nil { var err error options.Authorizer, err = rbac.NewAuthorizer() From 16ab669c9cb19df16565efc0e7dc0192ea7d0956 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 13 Sep 2022 14:03:10 +0000 Subject: [PATCH 2/3] Remove UNIX forwarding Tailscale doesn't support this, and adding support for it shouldn't block our rollout. Customers can always forward over SSH. --- cli/portforward.go | 93 +++------------------------------------- cli/portforward_test.go | 94 +++-------------------------------------- 2 files changed, 13 insertions(+), 174 deletions(-) diff --git a/cli/portforward.go b/cli/portforward.go index 54817f71f590e..7828b7fe61cff 100644 --- a/cli/portforward.go +++ b/cli/portforward.go @@ -6,7 +6,6 @@ import ( "net" "os" "os/signal" - "runtime" "strconv" "strings" "sync" @@ -24,10 +23,9 @@ import ( func portForward() *cobra.Command { var ( - tcpForwards []string // : - udpForwards []string // : - unixForwards []string // : OR : - wireguard bool + tcpForwards []string // : + udpForwards []string // : + wireguard bool ) cmd := &cobra.Command{ Use: "port-forward ", @@ -43,14 +41,6 @@ func portForward() *cobra.Command { Description: "Port forward a single UDP port from port 9000 to port 9000 on your local machine", Command: "coder port-forward --udp 9000", }, - example{ - Description: "Forward a Unix socket in the workspace to a local Unix socket", - Command: "coder port-forward --unix ./local.sock:~/remote.sock", - }, - example{ - Description: "Forward a Unix socket in the workspace to a local TCP port", - Command: "coder port-forward --unix 8080:~/remote.sock", - }, example{ Description: "Port forward multiple TCP ports and a UDP port", Command: "coder port-forward --tcp 8080:8080 --tcp 9000:3000 --udp 5353:53", @@ -60,7 +50,7 @@ func portForward() *cobra.Command { ctx, cancel := context.WithCancel(cmd.Context()) defer cancel() - specs, err := parsePortForwards(tcpForwards, udpForwards, unixForwards) + specs, err := parsePortForwards(tcpForwards, udpForwards) if err != nil { return xerrors.Errorf("parse port-forward specs: %w", err) } @@ -165,7 +155,6 @@ func portForward() *cobra.Command { cmd.Flags().StringArrayVarP(&tcpForwards, "tcp", "p", []string{}, "Forward a TCP port from the workspace to the local machine") cmd.Flags().StringArrayVar(&udpForwards, "udp", []string{}, "Forward a UDP port from the workspace to the local machine. The UDP connection has TCP-like semantics to support stateful UDP protocols") - cmd.Flags().StringArrayVar(&unixForwards, "unix", []string{}, "Forward a Unix socket in the workspace to a local Unix socket or TCP port") cmd.Flags().BoolVarP(&wireguard, "wireguard", "", true, "Specifies whether to use wireguard networking or not.") _ = cmd.Flags().MarkHidden("wireguard") return cmd @@ -198,8 +187,6 @@ func listenAndPortForward(ctx context.Context, cmd *cobra.Command, conn agent.Co IP: net.ParseIP(host), Port: portInt, }) - case "unix": - l, err = net.Listen(spec.listenNetwork, spec.listenAddress) default: return nil, xerrors.Errorf("unknown listen network %q", spec.listenNetwork) } @@ -236,14 +223,14 @@ func listenAndPortForward(ctx context.Context, cmd *cobra.Command, conn agent.Co } type portForwardSpec struct { - listenNetwork string // tcp, udp, unix + listenNetwork string // tcp, udp listenAddress string // : or path - dialNetwork string // tcp, udp, unix + dialNetwork string // tcp, udp dialAddress string // : or path } -func parsePortForwards(tcpSpecs, udpSpecs, unixSpecs []string) ([]portForwardSpec, error) { +func parsePortForwards(tcpSpecs, udpSpecs []string) ([]portForwardSpec, error) { specs := []portForwardSpec{} for _, spec := range tcpSpecs { @@ -274,29 +261,6 @@ func parsePortForwards(tcpSpecs, udpSpecs, unixSpecs []string) ([]portForwardSpe }) } - for _, specStr := range unixSpecs { - localPath, localTCP, remotePath, err := parseUnixUnix(specStr) - if err != nil { - return nil, xerrors.Errorf("failed to parse Unix port-forward specification %q: %w", specStr, err) - } - - spec := portForwardSpec{ - dialNetwork: "unix", - dialAddress: remotePath, - } - if localPath == "" { - spec.listenNetwork = "tcp" - spec.listenAddress = fmt.Sprintf("127.0.0.1:%v", localTCP) - } else { - if runtime.GOOS == "windows" { - return nil, xerrors.Errorf("Unix port-forwarding is not supported on Windows") - } - spec.listenNetwork = "unix" - spec.listenAddress = localPath - } - specs = append(specs, spec) - } - // Check for duplicate entries. locals := map[string]struct{}{} for _, spec := range specs { @@ -322,15 +286,6 @@ func parsePort(in string) (uint16, error) { return uint16(port), nil } -func parseUnixPath(in string) (string, error) { - path, err := agent.ExpandRelativeHomePath(strings.TrimSpace(in)) - if err != nil { - return "", xerrors.Errorf("tidy path %q: %w", in, err) - } - - return path, nil -} - func parsePortPort(in string) (local uint16, remote uint16, err error) { parts := strings.Split(in, ":") if len(parts) > 2 { @@ -352,37 +307,3 @@ func parsePortPort(in string) (local uint16, remote uint16, err error) { return local, remote, nil } - -func parsePortOrUnixPath(in string) (string, uint16, error) { - port, err := parsePort(in) - if err == nil { - return "", port, nil - } - - path, err := parseUnixPath(in) - if err != nil { - return "", 0, xerrors.Errorf("could not parse port or unix path %q: %w", in, err) - } - - return path, 0, nil -} - -func parseUnixUnix(in string) (string, uint16, string, error) { - parts := strings.Split(in, ":") - if len(parts) > 2 { - return "", 0, "", xerrors.Errorf("invalid port-forward specification %q", in) - } - if len(parts) == 1 { - // Duplicate the single part - parts = append(parts, parts[0]) - } - - localPath, localPort, err := parsePortOrUnixPath(parts[0]) - if err != nil { - return "", 0, "", xerrors.Errorf("parse local part of spec %q: %w", in, err) - } - - // We don't really touch the remote path at all since it gets cleaned - // up/expanded on the remote. - return localPath, localPort, parts[1], nil -} diff --git a/cli/portforward_test.go b/cli/portforward_test.go index 8ff3e2a846d09..fec01ee12b2ec 100644 --- a/cli/portforward_test.go +++ b/cli/portforward_test.go @@ -6,8 +6,6 @@ import ( "fmt" "io" "net" - "path/filepath" - "runtime" "strings" "sync" "testing" @@ -58,7 +56,7 @@ func TestPortForward(t *testing.T) { // setupRemote creates a "remote" listener to emulate a service in the // workspace. setupRemote func(t *testing.T) net.Listener - // setupLocal returns an available port or Unix socket path that the + // setupLocal returns an available port that the // port-forward command will listen on "locally". Returns the address // you pass to net.Dial, and the port/path you pass to `coder // port-forward`. @@ -110,26 +108,6 @@ func TestPortForward(t *testing.T) { return l.Addr().String(), port }, }, - { - name: "Unix", - network: "unix", - flag: "--unix=%v:%v", - setupRemote: func(t *testing.T) net.Listener { - if runtime.GOOS == "windows" { - t.Skip("Unix socket forwarding isn't supported on Windows") - } - - tmpDir := t.TempDir() - l, err := net.Listen("unix", filepath.Join(tmpDir, "test.sock")) - require.NoError(t, err, "create UDP listener") - return l - }, - setupLocal: func(t *testing.T) (string, string) { - tmpDir := t.TempDir() - path := filepath.Join(tmpDir, "test.sock") - return path, path - }, - }, } // Setup agent once to be shared between test-cases (avoid expensive @@ -234,74 +212,16 @@ func TestPortForward(t *testing.T) { }) } - // Test doing a TCP -> Unix forward. - //nolint:paralleltest - t.Run("TCP2Unix", func(t *testing.T) { - var ( - // Find the TCP and Unix cases so we can use their setupLocal and - // setupRemote methods respectively. - tcpCase = cases[0] - unixCase = cases[2] - - // Setup remote Unix listener. - p1 = setupTestListener(t, unixCase.setupRemote(t)) - ) - - // Create a flag that forwards from local TCP to Unix listener 1. - // Notably this is a --unix flag. - localAddress, localFlag := tcpCase.setupLocal(t) - flag := fmt.Sprintf(unixCase.flag, localFlag, p1) - - // Launch port-forward in a goroutine so we can start dialing - // the "local" listener. - cmd, root := clitest.New(t, "port-forward", workspace.Name, flag) - clitest.SetupConfig(t, client, root) - buf := newThreadSafeBuffer() - cmd.SetOut(buf) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - errC := make(chan error) - go func() { - errC <- cmd.ExecuteContext(ctx) - }() - waitForPortForwardReady(t, buf) - - t.Parallel() // Port is reserved, enable parallel execution. - - // Open two connections simultaneously and test them out of - // sync. - d := net.Dialer{Timeout: testutil.WaitShort} - c1, err := d.DialContext(ctx, tcpCase.network, localAddress) - require.NoError(t, err, "open connection 1 to 'local' listener") - defer c1.Close() - c2, err := d.DialContext(ctx, tcpCase.network, localAddress) - require.NoError(t, err, "open connection 2 to 'local' listener") - defer c2.Close() - testDial(t, c2) - testDial(t, c1) - - cancel() - err = <-errC - require.ErrorIs(t, err, context.Canceled) - }) - - // Test doing TCP, UDP and Unix at the same time. + // Test doing TCP and UDP at the same time. //nolint:paralleltest t.Run("All", func(t *testing.T) { var ( - // These aren't fixed size because we exclude Unix on Windows. dials = []addr{} flags = []string{} ) // Start listeners and populate arrays with the cases. for _, c := range cases { - if strings.HasPrefix(c.network, "unix") && runtime.GOOS == "windows" { - // Unix isn't supported on Windows, but we can still - // test other protocols together. - continue - } - p := setupTestListener(t, c.setupRemote(t)) localAddress, localFlag := c.setupLocal(t) @@ -412,7 +332,7 @@ func runAgent(t *testing.T, client *codersdk.Client, userID uuid.UUID) ([]coders } // setupTestListener starts accepting connections and echoing a single packet. -// Returns the listener and the listen port or Unix path. +// Returns the listener and the listen port. func setupTestListener(t *testing.T, l net.Listener) string { t.Helper() @@ -444,11 +364,9 @@ func setupTestListener(t *testing.T, l net.Listener) string { }() addr := l.Addr().String() - if !strings.HasPrefix(l.Addr().Network(), "unix") { - _, port, err := net.SplitHostPort(addr) - require.NoErrorf(t, err, "split non-Unix listen path %q", addr) - addr = port - } + _, port, err := net.SplitHostPort(addr) + require.NoErrorf(t, err, "split listen path %q", addr) + addr = port return addr } From ceb544bde552c5628b8b669f68f89cb99e143789 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 13 Sep 2022 10:16:03 -0500 Subject: [PATCH 3/3] Update cli/portforward_test.go Co-authored-by: Dean Sheather --- cli/portforward_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/portforward_test.go b/cli/portforward_test.go index fec01ee12b2ec..df92f20177a07 100644 --- a/cli/portforward_test.go +++ b/cli/portforward_test.go @@ -212,7 +212,7 @@ func TestPortForward(t *testing.T) { }) } - // Test doing TCP and UDP at the same time. + // Test doing TCP and UDP at the same time. //nolint:paralleltest t.Run("All", func(t *testing.T) { var (