fix(coderd): pass block endpoints into servertailnet (#12149)

coadler · mtojek · commit 3f0b383312db · 2024-03-08T14:14:06.000+01:00
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -489,6 +489,7 @@ func New(options *Options) *API {
 		func(context.Context) (tailnet.MultiAgentConn, error) {
 			return (*api.TailnetCoordinator.Load()).ServeMultiAgent(uuid.New()), nil
 		},
+		options.DeploymentValues.DERP.Config.BlockDirect.Value(),
 		api.TracerProvider,
 	)
 	if err != nil {
diff --git a/coderd/tailnet.go b/coderd/tailnet.go
@@ -49,13 +49,15 @@ func NewServerTailnet(
 	derpMapFn func() *tailcfg.DERPMap,
 	derpForceWebSockets bool,
 	getMultiAgent func(context.Context) (tailnet.MultiAgentConn, error),
+	blockEndpoints bool,
 	traceProvider trace.TracerProvider,
 ) (*ServerTailnet, error) {
 	logger = logger.Named("servertailnet")
 	conn, err := tailnet.NewConn(&tailnet.Options{
 		Addresses:           []netip.Prefix{netip.PrefixFrom(tailnet.IP(), 128)},
 		DERPForceWebSockets: derpForceWebSockets,
 		Logger:              logger,
+		BlockEndpoints:      blockEndpoints,
 	})
 	if err != nil {
 		return nil, xerrors.Errorf("create tailnet conn: %w", err)
@@ -166,6 +168,12 @@ func NewServerTailnet(
 	return tn, nil
 }
 
+// Conn is used to access the underlying tailnet conn of the ServerTailnet. It
+// should only be used for read-only purposes.
+func (s *ServerTailnet) Conn() *tailnet.Conn {
+	return s.conn
+}
+
 func (s *ServerTailnet) nodeCallback(node *tailnet.Node) {
 	pn, err := tailnet.NodeToProto(node)
 	if err != nil {
diff --git a/coderd/tailnet_test.go b/coderd/tailnet_test.go
@@ -303,6 +303,36 @@ func TestServerTailnet_ReverseProxy(t *testing.T) {
 
 		assert.Equal(t, expectedResponseCode, res.StatusCode)
 	})
+
+	t.Run("BlockEndpoints", func(t *testing.T) {
+		t.Parallel()
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		agents, serverTailnet := setupServerTailnetAgent(t, 1, tailnettest.DisableSTUN)
+		a := agents[0]
+
+		require.True(t, serverTailnet.Conn().GetBlockEndpoints(), "expected BlockEndpoints to be set")
+
+		u, err := url.Parse(fmt.Sprintf("http://127.0.0.1:%d", codersdk.WorkspaceAgentHTTPAPIServerPort))
+		require.NoError(t, err)
+
+		rp := serverTailnet.ReverseProxy(u, u, a.id)
+
+		rw := httptest.NewRecorder()
+		req := httptest.NewRequest(
+			http.MethodGet,
+			u.String(),
+			nil,
+		).WithContext(ctx)
+
+		rp.ServeHTTP(rw, req)
+		res := rw.Result()
+		defer res.Body.Close()
+
+		assert.Equal(t, http.StatusOK, res.StatusCode)
+	})
 }
 
 type wrappedListener struct {
@@ -375,6 +405,7 @@ func setupServerTailnetAgent(t *testing.T, agentNum int, opts ...tailnettest.DER
 		func() *tailcfg.DERPMap { return derpMap },
 		false,
 		func(context.Context) (tailnet.MultiAgentConn, error) { return coord.ServeMultiAgent(uuid.New()), nil },
+		!derpMap.HasSTUN(),
 		trace.NewNoopTracerProvider(),
 	)
 	require.NoError(t, err)
diff --git a/enterprise/derpmesh/derpmesh.go b/enterprise/derpmesh/derpmesh.go
@@ -12,9 +12,8 @@ import (
 	"tailscale.com/derp/derphttp"
 	"tailscale.com/types/key"
 
-	"github.com/coder/coder/v2/tailnet"
-
 	"cdr.dev/slog"
+	"github.com/coder/coder/v2/tailnet"
 )
 
 // New constructs a new mesh for DERP servers.
diff --git a/enterprise/wsproxy/wsproxy.go b/enterprise/wsproxy/wsproxy.go
@@ -251,6 +251,7 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
 		},
 		regResp.DERPForceWebSockets,
 		s.DialCoordinator,
+		false, // TODO: this will be covered in a subsequent pr.
 		s.TracerProvider,
 	)
 	if err != nil {
diff --git a/tailnet/configmaps.go b/tailnet/configmaps.go
@@ -254,6 +254,14 @@ func (c *configMaps) setBlockEndpoints(blockEndpoints bool) {
 	c.Broadcast()
 }
 
+// getBlockEndpoints returns the value of the most recent setBlockEndpoints
+// call.
+func (c *configMaps) getBlockEndpoints() bool {
+	c.L.Lock()
+	defer c.L.Unlock()
+	return c.blockEndpoints
+}
+
 // setDERPMap sets the DERP map, triggering a configuration of the engine if it has changed.
 // c.L MUST NOT be held.
 func (c *configMaps) setDERPMap(derpMap *tailcfg.DERPMap) {
diff --git a/tailnet/conn.go b/tailnet/conn.go
@@ -311,6 +311,10 @@ type Conn struct {
 	trafficStats *connstats.Statistics
 }
 
+func (c *Conn) GetBlockEndpoints() bool {
+	return c.configMaps.getBlockEndpoints() && c.nodeUpdater.getBlockEndpoints()
+}
+
 func (c *Conn) InstallCaptureHook(f capture.Callback) {
 	c.mutex.Lock()
 	defer c.mutex.Unlock()
diff --git a/tailnet/node.go b/tailnet/node.go
@@ -239,3 +239,11 @@ func (u *nodeUpdater) fillPeerDiagnostics(d *PeerDiagnostics) {
 	d.PreferredDERP = u.preferredDERP
 	d.SentNode = u.sentNode
 }
+
+// getBlockEndpoints returns the value of the most recent setBlockEndpoints
+// call.
+func (u *nodeUpdater) getBlockEndpoints() bool {
+	u.L.Lock()
+	defer u.L.Unlock()
+	return u.blockEndpoints
+}

Original file line number	Diff line number	Diff line change
`@@ -489,6 +489,7 @@ func New(options Options) API {`
`489`	`489`	`func(context.Context) (tailnet.MultiAgentConn, error) {`
`490`	`490`	`return (*api.TailnetCoordinator.Load()).ServeMultiAgent(uuid.New()), nil`
`491`	`491`	`},`
	`492`	`+ options.DeploymentValues.DERP.Config.BlockDirect.Value(),`
`492`	`493`	`api.TracerProvider,`
`493`	`494`	`)`
`494`	`495`	`if err != nil {`
Original file line number	Diff line number	Diff line change
`@@ -12,9 +12,8 @@ import (`
`12`	`12`	`"tailscale.com/derp/derphttp"`
`13`	`13`	`"tailscale.com/types/key"`
`14`	`14`
`15`		`- "github.com/coder/coder/v2/tailnet"`
`16`		`-`
`17`	`15`	`"cdr.dev/slog"`
	`16`	`+ "github.com/coder/coder/v2/tailnet"`
`18`	`17`	`)`
`19`	`18`
`20`	`19`	`// New constructs a new mesh for DERP servers.`
Original file line number	Diff line number	Diff line change
`@@ -251,6 +251,7 @@ func New(ctx context.Context, opts Options) (Server, error) {`
`251`	`251`	`},`
`252`	`252`	`regResp.DERPForceWebSockets,`
`253`	`253`	`s.DialCoordinator,`
	`254`	`+ false, // TODO: this will be covered in a subsequent pr.`
`254`	`255`	`s.TracerProvider,`
`255`	`256`	`)`
`256`	`257`	`if err != nil {`