From f395d05451e1ee47c66d8f5164f1790f30a417b6 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Wed, 2 Aug 2023 05:55:15 +0000 Subject: [PATCH 1/2] feat: add --derp-only flag to wsproxy --- coderd/apidoc/docs.go | 7 ++ coderd/apidoc/swagger.json | 7 ++ coderd/database/dbfake/dbfake.go | 2 + coderd/database/dump.sql | 5 +- .../000144_proxy_derp_only.down.sql | 1 + .../migrations/000144_proxy_derp_only.up.sql | 8 ++ coderd/database/models.go | 2 + coderd/database/queries.sql.go | 29 +++++-- coderd/database/queries/proxies.sql | 4 +- codersdk/workspaceproxy.go | 1 + docs/admin/audit-logs.md | 2 +- docs/api/enterprise.md | 5 ++ docs/api/schemas.md | 5 ++ enterprise/audit/table.go | 1 + enterprise/cli/proxyserver.go | 17 ++++ enterprise/coderd/coderdenttest/proxytest.go | 15 +--- enterprise/coderd/workspaceproxy.go | 19 ++++- enterprise/wsproxy/wsproxy.go | 57 +++++++++++--- enterprise/wsproxy/wsproxy_test.go | 77 ++++++++++++++++++- enterprise/wsproxy/wsproxysdk/wsproxysdk.go | 3 + site/src/api/typesGenerated.ts | 1 + 21 files changed, 229 insertions(+), 39 deletions(-) create mode 100644 coderd/database/migrations/000144_proxy_derp_only.down.sql create mode 100644 coderd/database/migrations/000144_proxy_derp_only.up.sql diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index d5ccfb06dfc47..7456ca013b04a 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -10924,6 +10924,9 @@ const docTemplate = `{ "derp_enabled": { "type": "boolean" }, + "derp_only": { + "type": "boolean" + }, "display_name": { "type": "string" }, @@ -11652,6 +11655,10 @@ const docTemplate = `{ "description": "DerpEnabled indicates whether the proxy should be included in the DERP\nmap or not.", "type": "boolean" }, + "derp_only": { + "description": "DerpOnly indicates whether the proxy should only be included in the DERP\nmap and should not be used for serving apps.", + "type": "boolean" + }, "hostname": { "description": "ReplicaHostname is the OS hostname of the machine that the proxy is running\non. This is only used for tracking purposes in the replicas table.", "type": "string" diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 69b3e1f6a5453..e8cd134d998d0 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -9914,6 +9914,9 @@ "derp_enabled": { "type": "boolean" }, + "derp_only": { + "type": "boolean" + }, "display_name": { "type": "string" }, @@ -10630,6 +10633,10 @@ "description": "DerpEnabled indicates whether the proxy should be included in the DERP\nmap or not.", "type": "boolean" }, + "derp_only": { + "description": "DerpOnly indicates whether the proxy should only be included in the DERP\nmap and should not be used for serving apps.", + "type": "boolean" + }, "hostname": { "description": "ReplicaHostname is the OS hostname of the machine that the proxy is running\non. This is only used for tracking purposes in the replicas table.", "type": "string" diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 8e29bae4349fb..6c124c68d7e36 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -4164,6 +4164,7 @@ func (q *FakeQuerier) InsertWorkspaceProxy(_ context.Context, arg database.Inser DisplayName: arg.DisplayName, Icon: arg.Icon, DerpEnabled: arg.DerpEnabled, + DerpOnly: arg.DerpOnly, TokenHashedSecret: arg.TokenHashedSecret, RegionID: lastRegionID + 1, CreatedAt: arg.CreatedAt, @@ -4238,6 +4239,7 @@ func (q *FakeQuerier) RegisterWorkspaceProxy(_ context.Context, arg database.Reg p.Url = arg.Url p.WildcardHostname = arg.WildcardHostname p.DerpEnabled = arg.DerpEnabled + p.DerpOnly = arg.DerpOnly p.UpdatedAt = database.Now() q.workspaceProxies[i] = p return p, nil diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 0bf80bbfb536a..c7cf57e9db3c1 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -875,7 +875,8 @@ CREATE TABLE workspace_proxies ( deleted boolean NOT NULL, token_hashed_secret bytea NOT NULL, region_id integer NOT NULL, - derp_enabled boolean DEFAULT true NOT NULL + derp_enabled boolean DEFAULT true NOT NULL, + derp_only boolean DEFAULT false NOT NULL ); COMMENT ON COLUMN workspace_proxies.icon IS 'Expects an emoji character. (/emojis/1f1fa-1f1f8.png)'; @@ -888,6 +889,8 @@ COMMENT ON COLUMN workspace_proxies.deleted IS 'Boolean indicator of a deleted w COMMENT ON COLUMN workspace_proxies.token_hashed_secret IS 'Hashed secret is used to authenticate the workspace proxy using a session token.'; +COMMENT ON COLUMN workspace_proxies.derp_only IS 'Disables app/terminal proxying for this proxy and only acts as a DERP relay.'; + CREATE SEQUENCE workspace_proxies_region_id_seq AS integer START WITH 1 diff --git a/coderd/database/migrations/000144_proxy_derp_only.down.sql b/coderd/database/migrations/000144_proxy_derp_only.down.sql new file mode 100644 index 0000000000000..37eebeb8f2cce --- /dev/null +++ b/coderd/database/migrations/000144_proxy_derp_only.down.sql @@ -0,0 +1 @@ +ALTER TABLE workspace_proxies DROP COLUMN derp_only; diff --git a/coderd/database/migrations/000144_proxy_derp_only.up.sql b/coderd/database/migrations/000144_proxy_derp_only.up.sql new file mode 100644 index 0000000000000..d63c602f78b70 --- /dev/null +++ b/coderd/database/migrations/000144_proxy_derp_only.up.sql @@ -0,0 +1,8 @@ +BEGIN; + +ALTER TABLE workspace_proxies + ADD COLUMN "derp_only" BOOLEAN NOT NULL DEFAULT false; + +COMMENT ON COLUMN workspace_proxies.derp_only IS 'Disables app/terminal proxying for this proxy and only acts as a DERP relay.'; + +COMMIT; diff --git a/coderd/database/models.go b/coderd/database/models.go index 58083303b05d7..086d86d2d278e 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -2010,6 +2010,8 @@ type WorkspaceProxy struct { TokenHashedSecret []byte `db:"token_hashed_secret" json:"token_hashed_secret"` RegionID int32 `db:"region_id" json:"region_id"` DerpEnabled bool `db:"derp_enabled" json:"derp_enabled"` + // Disables app/terminal proxying for this proxy and only acts as a DERP relay. + DerpOnly bool `db:"derp_only" json:"derp_only"` } type WorkspaceResource struct { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 87d86e9621fbb..f32c591198aa5 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2848,7 +2848,7 @@ func (q *sqlQuerier) UpdateProvisionerJobWithCompleteByID(ctx context.Context, a const getWorkspaceProxies = `-- name: GetWorkspaceProxies :many SELECT - id, name, display_name, icon, url, wildcard_hostname, created_at, updated_at, deleted, token_hashed_secret, region_id, derp_enabled + id, name, display_name, icon, url, wildcard_hostname, created_at, updated_at, deleted, token_hashed_secret, region_id, derp_enabled, derp_only FROM workspace_proxies WHERE @@ -2877,6 +2877,7 @@ func (q *sqlQuerier) GetWorkspaceProxies(ctx context.Context) ([]WorkspaceProxy, &i.TokenHashedSecret, &i.RegionID, &i.DerpEnabled, + &i.DerpOnly, ); err != nil { return nil, err } @@ -2893,7 +2894,7 @@ func (q *sqlQuerier) GetWorkspaceProxies(ctx context.Context) ([]WorkspaceProxy, const getWorkspaceProxyByHostname = `-- name: GetWorkspaceProxyByHostname :one SELECT - id, name, display_name, icon, url, wildcard_hostname, created_at, updated_at, deleted, token_hashed_secret, region_id, derp_enabled + id, name, display_name, icon, url, wildcard_hostname, created_at, updated_at, deleted, token_hashed_secret, region_id, derp_enabled, derp_only FROM workspace_proxies WHERE @@ -2951,13 +2952,14 @@ func (q *sqlQuerier) GetWorkspaceProxyByHostname(ctx context.Context, arg GetWor &i.TokenHashedSecret, &i.RegionID, &i.DerpEnabled, + &i.DerpOnly, ) return i, err } const getWorkspaceProxyByID = `-- name: GetWorkspaceProxyByID :one SELECT - id, name, display_name, icon, url, wildcard_hostname, created_at, updated_at, deleted, token_hashed_secret, region_id, derp_enabled + id, name, display_name, icon, url, wildcard_hostname, created_at, updated_at, deleted, token_hashed_secret, region_id, derp_enabled, derp_only FROM workspace_proxies WHERE @@ -2982,13 +2984,14 @@ func (q *sqlQuerier) GetWorkspaceProxyByID(ctx context.Context, id uuid.UUID) (W &i.TokenHashedSecret, &i.RegionID, &i.DerpEnabled, + &i.DerpOnly, ) return i, err } const getWorkspaceProxyByName = `-- name: GetWorkspaceProxyByName :one SELECT - id, name, display_name, icon, url, wildcard_hostname, created_at, updated_at, deleted, token_hashed_secret, region_id, derp_enabled + id, name, display_name, icon, url, wildcard_hostname, created_at, updated_at, deleted, token_hashed_secret, region_id, derp_enabled, derp_only FROM workspace_proxies WHERE @@ -3014,6 +3017,7 @@ func (q *sqlQuerier) GetWorkspaceProxyByName(ctx context.Context, name string) ( &i.TokenHashedSecret, &i.RegionID, &i.DerpEnabled, + &i.DerpOnly, ) return i, err } @@ -3028,13 +3032,14 @@ INSERT INTO display_name, icon, derp_enabled, + derp_only, token_hashed_secret, created_at, updated_at, deleted ) VALUES - ($1, '', '', $2, $3, $4, $5, $6, $7, $8, false) RETURNING id, name, display_name, icon, url, wildcard_hostname, created_at, updated_at, deleted, token_hashed_secret, region_id, derp_enabled + ($1, '', '', $2, $3, $4, $5, $6, $7, $8, $9, false) RETURNING id, name, display_name, icon, url, wildcard_hostname, created_at, updated_at, deleted, token_hashed_secret, region_id, derp_enabled, derp_only ` type InsertWorkspaceProxyParams struct { @@ -3043,6 +3048,7 @@ type InsertWorkspaceProxyParams struct { DisplayName string `db:"display_name" json:"display_name"` Icon string `db:"icon" json:"icon"` DerpEnabled bool `db:"derp_enabled" json:"derp_enabled"` + DerpOnly bool `db:"derp_only" json:"derp_only"` TokenHashedSecret []byte `db:"token_hashed_secret" json:"token_hashed_secret"` CreatedAt time.Time `db:"created_at" json:"created_at"` UpdatedAt time.Time `db:"updated_at" json:"updated_at"` @@ -3055,6 +3061,7 @@ func (q *sqlQuerier) InsertWorkspaceProxy(ctx context.Context, arg InsertWorkspa arg.DisplayName, arg.Icon, arg.DerpEnabled, + arg.DerpOnly, arg.TokenHashedSecret, arg.CreatedAt, arg.UpdatedAt, @@ -3073,6 +3080,7 @@ func (q *sqlQuerier) InsertWorkspaceProxy(ctx context.Context, arg InsertWorkspa &i.TokenHashedSecret, &i.RegionID, &i.DerpEnabled, + &i.DerpOnly, ) return i, err } @@ -3084,16 +3092,18 @@ SET url = $1 :: text, wildcard_hostname = $2 :: text, derp_enabled = $3 :: boolean, + derp_only = $4 :: boolean, updated_at = Now() WHERE - id = $4 -RETURNING id, name, display_name, icon, url, wildcard_hostname, created_at, updated_at, deleted, token_hashed_secret, region_id, derp_enabled + id = $5 +RETURNING id, name, display_name, icon, url, wildcard_hostname, created_at, updated_at, deleted, token_hashed_secret, region_id, derp_enabled, derp_only ` type RegisterWorkspaceProxyParams struct { Url string `db:"url" json:"url"` WildcardHostname string `db:"wildcard_hostname" json:"wildcard_hostname"` DerpEnabled bool `db:"derp_enabled" json:"derp_enabled"` + DerpOnly bool `db:"derp_only" json:"derp_only"` ID uuid.UUID `db:"id" json:"id"` } @@ -3102,6 +3112,7 @@ func (q *sqlQuerier) RegisterWorkspaceProxy(ctx context.Context, arg RegisterWor arg.Url, arg.WildcardHostname, arg.DerpEnabled, + arg.DerpOnly, arg.ID, ) var i WorkspaceProxy @@ -3118,6 +3129,7 @@ func (q *sqlQuerier) RegisterWorkspaceProxy(ctx context.Context, arg RegisterWor &i.TokenHashedSecret, &i.RegionID, &i.DerpEnabled, + &i.DerpOnly, ) return i, err } @@ -3140,7 +3152,7 @@ SET updated_at = Now() WHERE id = $5 -RETURNING id, name, display_name, icon, url, wildcard_hostname, created_at, updated_at, deleted, token_hashed_secret, region_id, derp_enabled +RETURNING id, name, display_name, icon, url, wildcard_hostname, created_at, updated_at, deleted, token_hashed_secret, region_id, derp_enabled, derp_only ` type UpdateWorkspaceProxyParams struct { @@ -3174,6 +3186,7 @@ func (q *sqlQuerier) UpdateWorkspaceProxy(ctx context.Context, arg UpdateWorkspa &i.TokenHashedSecret, &i.RegionID, &i.DerpEnabled, + &i.DerpOnly, ) return i, err } diff --git a/coderd/database/queries/proxies.sql b/coderd/database/queries/proxies.sql index d283ef87fe936..f43ac6465ca6f 100644 --- a/coderd/database/queries/proxies.sql +++ b/coderd/database/queries/proxies.sql @@ -8,13 +8,14 @@ INSERT INTO display_name, icon, derp_enabled, + derp_only, token_hashed_secret, created_at, updated_at, deleted ) VALUES - ($1, '', '', $2, $3, $4, $5, $6, $7, $8, false) RETURNING *; + ($1, '', '', $2, $3, $4, $5, $6, $7, $8, $9, false) RETURNING *; -- name: RegisterWorkspaceProxy :one UPDATE @@ -23,6 +24,7 @@ SET url = @url :: text, wildcard_hostname = @wildcard_hostname :: text, derp_enabled = @derp_enabled :: boolean, + derp_only = @derp_only :: boolean, updated_at = Now() WHERE id = @id diff --git a/codersdk/workspaceproxy.go b/codersdk/workspaceproxy.go index ef2ccd638c618..efdc3cc93c57d 100644 --- a/codersdk/workspaceproxy.go +++ b/codersdk/workspaceproxy.go @@ -49,6 +49,7 @@ type WorkspaceProxy struct { // Extends Region with extra information Region `table:"region,recursive_inline"` DerpEnabled bool `json:"derp_enabled" table:"derp_enabled"` + DerpOnly bool `json:"derp_only" table:"derp_only"` // Status is the latest status check of the proxy. This will be empty for deleted // proxies. This value can be used to determine if a workspace proxy is healthy diff --git a/docs/admin/audit-logs.md b/docs/admin/audit-logs.md index 27ccbb763ab2a..1dc32df7272a3 100644 --- a/docs/admin/audit-logs.md +++ b/docs/admin/audit-logs.md @@ -21,7 +21,7 @@ We track the following resources: | User
create, write, delete |
FieldTracked
avatar_urlfalse
created_atfalse
deletedtrue
emailtrue
hashed_passwordtrue
idtrue
last_seen_atfalse
login_typetrue
quiet_hours_scheduletrue
rbac_rolestrue
statustrue
updated_atfalse
usernametrue
| | Workspace
create, write, delete |
FieldTracked
autostart_scheduletrue
created_atfalse
deletedfalse
deleting_attrue
idtrue
last_used_atfalse
locked_attrue
nametrue
organization_idfalse
owner_idtrue
template_idtrue
ttltrue
updated_atfalse
| | WorkspaceBuild
start, stop |
FieldTracked
build_numberfalse
created_atfalse
daily_costfalse
deadlinefalse
idfalse
initiator_by_avatar_urlfalse
initiator_by_usernamefalse
initiator_idfalse
job_idfalse
max_deadlinefalse
provisioner_statefalse
reasonfalse
template_version_idtrue
transitionfalse
updated_atfalse
workspace_idfalse
| -| WorkspaceProxy
|
FieldTracked
created_attrue
deletedfalse
derp_enabledtrue
display_nametrue
icontrue
idtrue
nametrue
region_idtrue
token_hashed_secrettrue
updated_atfalse
urltrue
wildcard_hostnametrue
| +| WorkspaceProxy
|
FieldTracked
created_attrue
deletedfalse
derp_enabledtrue
derp_onlytrue
display_nametrue
icontrue
idtrue
nametrue
region_idtrue
token_hashed_secrettrue
updated_atfalse
urltrue
wildcard_hostnametrue
| diff --git a/docs/api/enterprise.md b/docs/api/enterprise.md index 03610ba53651f..5e1aa9864acf1 100644 --- a/docs/api/enterprise.md +++ b/docs/api/enterprise.md @@ -1452,6 +1452,7 @@ curl -X GET http://coder-server:8080/api/v2/workspaceproxies \ "created_at": "2019-08-24T14:15:22Z", "deleted": true, "derp_enabled": true, + "derp_only": true, "display_name": "string", "healthy": true, "icon_url": "string", @@ -1491,6 +1492,7 @@ Status Code **200** | `»» created_at` | string(date-time) | false | | | | `»» deleted` | boolean | false | | | | `»» derp_enabled` | boolean | false | | | +| `»» derp_only` | boolean | false | | | | `»» display_name` | string | false | | | | `»» healthy` | boolean | false | | | | `»» icon_url` | string | false | | | @@ -1556,6 +1558,7 @@ curl -X POST http://coder-server:8080/api/v2/workspaceproxies \ "created_at": "2019-08-24T14:15:22Z", "deleted": true, "derp_enabled": true, + "derp_only": true, "display_name": "string", "healthy": true, "icon_url": "string", @@ -1611,6 +1614,7 @@ curl -X GET http://coder-server:8080/api/v2/workspaceproxies/{workspaceproxy} \ "created_at": "2019-08-24T14:15:22Z", "deleted": true, "derp_enabled": true, + "derp_only": true, "display_name": "string", "healthy": true, "icon_url": "string", @@ -1724,6 +1728,7 @@ curl -X PATCH http://coder-server:8080/api/v2/workspaceproxies/{workspaceproxy} "created_at": "2019-08-24T14:15:22Z", "deleted": true, "derp_enabled": true, + "derp_only": true, "display_name": "string", "healthy": true, "icon_url": "string", diff --git a/docs/api/schemas.md b/docs/api/schemas.md index b9d5e3d1b78a1..d7e5dd9821095 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -3820,6 +3820,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "created_at": "2019-08-24T14:15:22Z", "deleted": true, "derp_enabled": true, + "derp_only": true, "display_name": "string", "healthy": true, "icon_url": "string", @@ -6020,6 +6021,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| "created_at": "2019-08-24T14:15:22Z", "deleted": true, "derp_enabled": true, + "derp_only": true, "display_name": "string", "healthy": true, "icon_url": "string", @@ -6046,6 +6048,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| | `created_at` | string | false | | | | `deleted` | boolean | false | | | | `derp_enabled` | boolean | false | | | +| `derp_only` | boolean | false | | | | `display_name` | string | false | | | | `healthy` | boolean | false | | | | `icon_url` | string | false | | | @@ -7417,6 +7420,7 @@ _None_ { "access_url": "string", "derp_enabled": true, + "derp_only": true, "hostname": "string", "replica_error": "string", "replica_id": "string", @@ -7432,6 +7436,7 @@ _None_ | ------------------------------------------------------------------------------------------------- | ------- | -------- | ------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `access_url` | string | false | | Access URL that hits the workspace proxy api. | | `derp_enabled` | boolean | false | | Derp enabled indicates whether the proxy should be included in the DERP map or not. | +| `derp_only` | boolean | false | | Derp only indicates whether the proxy should only be included in the DERP map and should not be used for serving apps. | | `hostname` | string | false | | Hostname is the OS hostname of the machine that the proxy is running on. This is only used for tracking purposes in the replicas table. | | `replica_error` | string | false | | Replica error is the error that the replica encountered when trying to dial it's peers. This is stored in the replicas table for debugging purposes but does not affect the proxy's ability to register. | | This value is only stored on subsequent requests to the register endpoint, not the first request. | diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index cea72f7c703cb..7eb93f241e4fa 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -198,6 +198,7 @@ var auditableResourcesTypes = map[any]map[string]Action{ "deleted": ActionIgnore, "token_hashed_secret": ActionSecret, "derp_enabled": ActionTrack, + "derp_only": ActionTrack, "region_id": ActionTrack, }, } diff --git a/enterprise/cli/proxyserver.go b/enterprise/cli/proxyserver.go index 3fb706cb84143..f40d12d6d4cde 100644 --- a/enterprise/cli/proxyserver.go +++ b/enterprise/cli/proxyserver.go @@ -56,6 +56,7 @@ func (*RootCmd) proxyServer() *clibase.Cmd { } proxySessionToken clibase.String primaryAccessURL clibase.URL + derpOnly clibase.Bool ) opts.Add( // Options only for external workspace proxies @@ -88,6 +89,17 @@ func (*RootCmd) proxyServer() *clibase.Cmd { Group: &externalProxyOptionGroup, Hidden: false, }, + clibase.Option{ + Name: "DERP-only proxy", + Description: "Run a proxy server that only supports DERP connections and does not proxy workspace app/terminal traffic.", + Flag: "derp-only", + Env: "CODER_PROXY_DERP_ONLY", + YAML: "derpOnly", + Required: false, + Value: &derpOnly, + Group: &externalProxyOptionGroup, + Hidden: false, + }, ) cmd := &clibase.Cmd{ @@ -163,6 +175,10 @@ func (*RootCmd) proxyServer() *clibase.Cmd { } } + if derpOnly.Value() && !cfg.DERP.Server.Enable.Value() { + return xerrors.Errorf("cannot use --derp-only with DERP server disabled") + } + // TODO: @emyrk I find this strange that we add this to the context // at the root here. ctx, httpClient, err := cli.ConfigureHTTPClient( @@ -236,6 +252,7 @@ func (*RootCmd) proxyServer() *clibase.Cmd { ProxySessionToken: proxySessionToken.Value(), AllowAllCors: cfg.Dangerous.AllowAllCors.Value(), DERPEnabled: cfg.DERP.Server.Enable.Value(), + DERPOnly: derpOnly.Value(), DERPServerRelayAddress: cfg.DERP.Server.RelayURL.String(), }) if err != nil { diff --git a/enterprise/coderd/coderdenttest/proxytest.go b/enterprise/coderd/coderdenttest/proxytest.go index 2918659ac57c9..c544e14b44a46 100644 --- a/enterprise/coderd/coderdenttest/proxytest.go +++ b/enterprise/coderd/coderdenttest/proxytest.go @@ -32,6 +32,8 @@ type ProxyOptions struct { TLSCertificates []tls.Certificate AppHostname string DisablePathApps bool + DerpDisabled bool + DerpOnly bool // ProxyURL is optional ProxyURL *url.URL @@ -91,16 +93,6 @@ func NewWorkspaceProxy(t *testing.T, coderdAPI *coderd.API, owner *codersdk.Clie accessURL = serverURL } - // TODO: Stun and derp stuff - // derpPort, err := strconv.Atoi(serverURL.Port()) - // require.NoError(t, err) - // - // stunAddr, stunCleanup := stuntest.ServeWithPacketListener(t, nettype.Std{}) - // t.Cleanup(stunCleanup) - // - // derpServer := derp.NewServer(key.NewNode(), tailnet.Logger(slogtest.Make(t, nil).Named("derp").Leveled(slog.LevelDebug))) - // derpServer.SetMeshKey("test-key") - var appHostnameRegex *regexp.Regexp if options.AppHostname != "" { var err error @@ -134,7 +126,8 @@ func NewWorkspaceProxy(t *testing.T, coderdAPI *coderd.API, owner *codersdk.Clie // We need a new registry to not conflict with the coderd internal // proxy metrics. PrometheusRegistry: prometheus.NewRegistry(), - DERPEnabled: true, + DERPEnabled: !options.DerpDisabled, + DERPOnly: options.DerpOnly, DERPServerRelayAddress: accessURL.String(), }) require.NoError(t, err) diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index 1842302cb4d82..591f2fa33374f 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -63,8 +63,8 @@ func (api *API) fetchRegions(ctx context.Context) (codersdk.RegionsResponse[code regions := make([]codersdk.Region, 0, len(proxies.Regions)) for i := range proxies.Regions { - // Ignore deleted proxies. - if proxies.Regions[i].Deleted { + // Ignore deleted and DERP-only proxies. + if proxies.Regions[i].Deleted || proxies.Regions[i].DerpOnly { continue } // Append the inner region data. @@ -353,8 +353,10 @@ func (api *API) postWorkspaceProxy(rw http.ResponseWriter, r *http.Request) { // Enabled by default, but will be disabled on register if the proxy has // it disabled. DerpEnabled: true, - CreatedAt: database.Now(), - UpdatedAt: database.Now(), + // Disabled by default, but blah blah blah. + DerpOnly: false, + CreatedAt: database.Now(), + UpdatedAt: database.Now(), }) if database.IsUniqueViolation(err, database.UniqueWorkspaceProxiesLowerNameIndex) { httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ @@ -569,6 +571,13 @@ func (api *API) workspaceProxyRegister(rw http.ResponseWriter, r *http.Request) return } + if req.DerpOnly && !req.DerpEnabled { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "DerpOnly cannot be true when DerpEnabled is false.", + }) + return + } + startingRegionID, _ := getProxyDERPStartingRegionID(api.Options.BaseDERPMap) regionID := int32(startingRegionID) + proxy.RegionID @@ -578,6 +587,7 @@ func (api *API) workspaceProxyRegister(rw http.ResponseWriter, r *http.Request) ID: proxy.ID, Url: req.AccessURL, DerpEnabled: req.DerpEnabled, + DerpOnly: req.DerpOnly, WildcardHostname: req.WildcardHostname, }) if err != nil { @@ -899,6 +909,7 @@ func convertProxy(p database.WorkspaceProxy, status proxyhealth.ProxyStatus) cod return codersdk.WorkspaceProxy{ Region: convertRegion(p, status), DerpEnabled: p.DerpEnabled, + DerpOnly: p.DerpOnly, CreatedAt: p.CreatedAt, UpdatedAt: p.UpdatedAt, Deleted: p.Deleted, diff --git a/enterprise/wsproxy/wsproxy.go b/enterprise/wsproxy/wsproxy.go index 1532e38914ddb..843b79ee394bf 100644 --- a/enterprise/wsproxy/wsproxy.go +++ b/enterprise/wsproxy/wsproxy.go @@ -70,6 +70,9 @@ type Options struct { DisablePathApps bool DERPEnabled bool DERPServerRelayAddress string + // DERPOnly determines whether this proxy only provides DERP and does not + // provide access to workspace apps/terminal. + DERPOnly bool ProxySessionToken string // AllowAllCors will set all CORs headers to '*'. @@ -208,6 +211,7 @@ func New(ctx context.Context, opts *Options) (*Server, error) { AccessURL: opts.AccessURL.String(), WildcardHostname: opts.AppHostname, DerpEnabled: opts.DERPEnabled, + DerpOnly: opts.DERPOnly, ReplicaID: replicaID, ReplicaHostname: osHostname, ReplicaError: "", @@ -327,18 +331,51 @@ func New(ctx context.Context, opts *Options) (*Server, error) { ) // Attach workspace apps routes. - r.Group(func(r chi.Router) { - r.Use(apiRateLimiter) - s.AppServer.Attach(r) - }) + if !opts.DERPOnly { + r.Group(func(r chi.Router) { + r.Use(apiRateLimiter) + s.AppServer.Attach(r) + }) + } else { + r.Group(func(r chi.Router) { + derpOnlyHandler := func(rw http.ResponseWriter, r *http.Request) { + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Title: "Head to the Dashboard", + Status: http.StatusBadRequest, + HideStatus: true, + Description: "This workspace proxy is DERP-only and cannot be used for browser connections. " + + "Please use a different region directly from the dashboard. Click to be redirected!", + RetryEnabled: false, + DashboardURL: opts.DashboardURL.String(), + }) + } + serveDerpOnlyHandler := func(r chi.Router) { + r.HandleFunc("/*", derpOnlyHandler) + } - r.Route("/derp", func(r chi.Router) { - r.Get("/", derpHandler.ServeHTTP) - // This is used when UDP is blocked, and latency must be checked via HTTP(s). - r.Get("/latency-check", func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) + r.Route("/%40{user}/{workspace_and_agent}/apps/{workspaceapp}", serveDerpOnlyHandler) + r.Route("/@{user}/{workspace_and_agent}/apps/{workspaceapp}", serveDerpOnlyHandler) + r.Get("/api/v2/workspaceagents/{workspaceagent}/pty", derpOnlyHandler) }) - }) + } + + if opts.DERPEnabled { + r.Route("/derp", func(r chi.Router) { + r.Get("/", derpHandler.ServeHTTP) + // This is used when UDP is blocked, and latency must be checked via HTTP(s). + r.Get("/latency-check", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + }) + } else { + r.Route("/derp", func(r chi.Router) { + r.HandleFunc("/*", func(rw http.ResponseWriter, r *http.Request) { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "DERP is disabled on this proxy.", + }) + }) + }) + } r.Get("/api/v2/buildinfo", s.buildInfo) r.Get("/healthz", func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("OK")) }) diff --git a/enterprise/wsproxy/wsproxy_test.go b/enterprise/wsproxy/wsproxy_test.go index 137b4326661bb..afcb3d1f16143 100644 --- a/enterprise/wsproxy/wsproxy_test.go +++ b/enterprise/wsproxy/wsproxy_test.go @@ -9,7 +9,9 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "tailscale.com/derp/derphttp" "tailscale.com/tailcfg" + "tailscale.com/types/key" "cdr.dev/slog" "cdr.dev/slog/sloggers/slogtest" @@ -29,6 +31,54 @@ import ( "github.com/coder/coder/testutil" ) +func TestDERPOnly(t *testing.T) { + t.Parallel() + + deploymentValues := coderdtest.DeploymentValues(t) + deploymentValues.Experiments = []string{ + string(codersdk.ExperimentMoons), + "*", + } + + client, closer, api, _ := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + DeploymentValues: deploymentValues, + AppHostname: "*.primary.test.coder.com", + IncludeProvisionerDaemon: true, + RealIPConfig: &httpmw.RealIPConfig{ + TrustedOrigins: []*net.IPNet{{ + IP: net.ParseIP("127.0.0.1"), + Mask: net.CIDRMask(8, 32), + }}, + TrustedHeaders: []string{ + "CF-Connecting-IP", + }, + }, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureWorkspaceProxy: 1, + }, + }, + }) + t.Cleanup(func() { + _ = closer.Close() + }) + + // Create an external proxy. + _ = coderdenttest.NewWorkspaceProxy(t, api, client, &coderdenttest.ProxyOptions{ + Name: "best-proxy", + DerpOnly: true, + }) + + // Should not show up in the regions list. + ctx := testutil.Context(t, testutil.WaitLong) + regions, err := client.Regions(ctx) + require.NoError(t, err) + require.Len(t, regions, 1) + require.Equal(t, api.Options.AccessURL.String(), regions[0].PathAppURL) +} + func TestDERP(t *testing.T) { t.Parallel() @@ -71,6 +121,12 @@ func TestDERP(t *testing.T) { Name: "worst-proxy", }) + // Create a running external proxy with DERP disabled. + proxyAPI3 := coderdenttest.NewWorkspaceProxy(t, api, client, &coderdenttest.ProxyOptions{ + Name: "no-derp-proxy", + DerpDisabled: true, + }) + // Create a proxy that is never started. createProxyCtx := testutil.Context(t, testutil.WaitLong) _, err := client.CreateWorkspaceProxy(createProxyCtx, codersdk.CreateWorkspaceProxyRequest{ @@ -90,19 +146,19 @@ func TestDERP(t *testing.T) { if !assert.NoError(t, err) { return false } - if !assert.Len(t, regions, 4) { + if !assert.Len(t, regions, 5) { return false } // The first 3 regions should be healthy. - for _, r := range regions[:3] { + for _, r := range regions[:4] { if !r.Healthy { return false } } // The last region should never be healthy. - assert.False(t, regions[3].Healthy) + assert.False(t, regions[4].Healthy) return true }, testutil.WaitLong, testutil.IntervalMedium) @@ -170,6 +226,9 @@ resourceLoop: proxy2Region = r continue } + // The no-derp-proxy shouldn't show up in the map. + // The last region is never started, which means it's never healthy, + // which means it's never added to the DERP map. t.Fatalf("unexpected region: %+v", r) } @@ -238,6 +297,18 @@ resourceLoop: }) } }) + + t.Run("DERPDisabled", func(t *testing.T) { + t.Parallel() + + // Try to connect to the DERP server on the no-derp-proxy region. + client, err := derphttp.NewClient(key.NewNode(), proxyAPI3.Options.AccessURL.String(), func(format string, args ...any) {}) + require.NoError(t, err) + + ctx := testutil.Context(t, testutil.WaitLong) + err = client.Connect(ctx) + require.Error(t, err) + }) } func TestDERPMapStunNodes(t *testing.T) { diff --git a/enterprise/wsproxy/wsproxysdk/wsproxysdk.go b/enterprise/wsproxy/wsproxysdk/wsproxysdk.go index f98ab3673eadd..6d1a4b1227b5d 100644 --- a/enterprise/wsproxy/wsproxysdk/wsproxysdk.go +++ b/enterprise/wsproxy/wsproxysdk/wsproxysdk.go @@ -160,6 +160,9 @@ type RegisterWorkspaceProxyRequest struct { // DerpEnabled indicates whether the proxy should be included in the DERP // map or not. DerpEnabled bool `json:"derp_enabled"` + // DerpOnly indicates whether the proxy should only be included in the DERP + // map and should not be used for serving apps. + DerpOnly bool `json:"derp_only"` // ReplicaID is a unique identifier for the replica of the proxy that is // registering. It should be generated by the client on startup and diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index e374e46e192f1..eca55835b2fb8 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1454,6 +1454,7 @@ export interface WorkspaceOptions { // From codersdk/workspaceproxy.go export interface WorkspaceProxy extends Region { readonly derp_enabled: boolean + readonly derp_only: boolean readonly status?: WorkspaceProxyStatus readonly created_at: string readonly updated_at: string From 1f51c66ab393c49a8162f168c47d6bd742852a69 Mon Sep 17 00:00:00 2001 From: Dean Sheather Date: Wed, 2 Aug 2023 06:56:49 +0000 Subject: [PATCH 2/2] fixup! feat: add --derp-only flag to wsproxy --- site/src/testHelpers/entities.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 930263fe2049c..6fffb71e76eea 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -80,6 +80,7 @@ export const MockPrimaryWorkspaceProxy: TypesGen.WorkspaceProxy = { path_app_url: "https://coder.com", wildcard_hostname: "*.coder.com", derp_enabled: true, + derp_only: false, created_at: new Date().toISOString(), updated_at: new Date().toISOString(), deleted: false, @@ -98,6 +99,7 @@ export const MockHealthyWildWorkspaceProxy: TypesGen.WorkspaceProxy = { path_app_url: "https://external.com", wildcard_hostname: "*.external.com", derp_enabled: true, + derp_only: false, created_at: new Date().toISOString(), updated_at: new Date().toISOString(), deleted: false, @@ -116,6 +118,7 @@ export const MockUnhealthyWildWorkspaceProxy: TypesGen.WorkspaceProxy = { path_app_url: "https://unhealthy.coder.com", wildcard_hostname: "*unhealthy..coder.com", derp_enabled: true, + derp_only: true, created_at: new Date().toISOString(), updated_at: new Date().toISOString(), deleted: false, @@ -142,6 +145,7 @@ export const MockWorkspaceProxies: TypesGen.WorkspaceProxy[] = [ path_app_url: "https://cowboy.coder.com", wildcard_hostname: "", derp_enabled: false, + derp_only: false, created_at: new Date().toISOString(), updated_at: new Date().toISOString(), deleted: false,