From 9cb6421c3babff749b69d5f68e32328b95500b97 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 7 Jun 2023 09:53:56 -0500 Subject: [PATCH 01/13] chore: add database methods for editing the default workspace proxy --- coderd/database/dbauthz/querier.go | 5 +++ coderd/database/dbauthz/querier_test.go | 3 ++ coderd/database/dbauthz/system.go | 7 ++++ coderd/database/dbauthz/system_test.go | 3 ++ coderd/database/dbfake/databasefake.go | 36 ++++++++++++---- coderd/database/dbmetrics/dbmetrics.go | 14 +++++++ coderd/database/dbmock/store.go | 29 +++++++++++++ coderd/database/querier.go | 5 +++ coderd/database/querier_test.go | 55 +++++++++++++++++++++++++ coderd/database/queries.sql.go | 41 ++++++++++++++++++ coderd/database/queries/siteconfig.sql | 20 +++++++++ coderd/workspaceproxies.go | 6 +-- 12 files changed, 211 insertions(+), 13 deletions(-) diff --git a/coderd/database/dbauthz/querier.go b/coderd/database/dbauthz/querier.go index 0956ba88926f3..30d1dfdff3647 100644 --- a/coderd/database/dbauthz/querier.go +++ b/coderd/database/dbauthz/querier.go @@ -369,6 +369,11 @@ func (q *querier) DeleteLicense(ctx context.Context, id int32) (int32, error) { return id, nil } +func (q *querier) GetDefaultProxyConfig(ctx context.Context) (database.GetDefaultProxyConfigRow, error) { + // No authz checks + return q.db.GetDefaultProxyConfig(ctx) +} + func (q *querier) GetDeploymentID(ctx context.Context) (string, error) { // No authz checks return q.db.GetDeploymentID(ctx) diff --git a/coderd/database/dbauthz/querier_test.go b/coderd/database/dbauthz/querier_test.go index fd158ef49f5fc..701a45fa44aa3 100644 --- a/coderd/database/dbauthz/querier_test.go +++ b/coderd/database/dbauthz/querier_test.go @@ -335,6 +335,9 @@ func (s *MethodTestSuite) TestLicense() { s.Run("GetDeploymentID", s.Subtest(func(db database.Store, check *expects) { check.Args().Asserts().Returns("") })) + s.Run("GetDefaultProxyConfig", s.Subtest(func(db database.Store, check *expects) { + check.Args().Asserts().Returns(database.GetDefaultProxyConfigRow{}) + })) s.Run("GetLogoURL", s.Subtest(func(db database.Store, check *expects) { err := db.UpsertLogoURL(context.Background(), "value") require.NoError(s.T(), err) diff --git a/coderd/database/dbauthz/system.go b/coderd/database/dbauthz/system.go index 3383ef638d999..f1ca0a686e29a 100644 --- a/coderd/database/dbauthz/system.go +++ b/coderd/database/dbauthz/system.go @@ -431,3 +431,10 @@ func (q *querier) GetWorkspaceProxyByHostname(ctx context.Context, params databa } return q.db.GetWorkspaceProxyByHostname(ctx, params) } + +func (q *querier) UpsertDefaultProxy(ctx context.Context, arg database.UpsertDefaultProxyParams) error { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + return err + } + return q.db.UpsertDefaultProxy(ctx, arg) +} diff --git a/coderd/database/dbauthz/system_test.go b/coderd/database/dbauthz/system_test.go index 4154398d2a93e..98f9e493e2177 100644 --- a/coderd/database/dbauthz/system_test.go +++ b/coderd/database/dbauthz/system_test.go @@ -25,6 +25,9 @@ func (s *MethodTestSuite) TestSystemFunctions() { LoginType: database.LoginTypeGithub, }).Asserts(rbac.ResourceSystem, rbac.ActionUpdate).Returns(l) })) + s.Run("UpsertDefaultProxy", s.Subtest(func(db database.Store, check *expects) { + check.Args(database.UpsertDefaultProxyParams{}).Asserts(rbac.ResourceSystem, rbac.ActionUpdate).Returns() + })) s.Run("GetUserLinkByLinkedID", s.Subtest(func(db database.Store, check *expects) { l := dbgen.UserLink(s.T(), db, database.UserLink{}) check.Args(l.LinkedID).Asserts(rbac.ResourceSystem, rbac.ActionRead).Returns(l) diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go index 9294335106304..3f6b362de704e 100644 --- a/coderd/database/dbfake/databasefake.go +++ b/coderd/database/dbfake/databasefake.go @@ -41,7 +41,7 @@ var errDuplicateKey = &pq.Error{ // New returns an in-memory fake of the database. func New() database.Store { - return &fakeQuerier{ + q := &fakeQuerier{ mutex: &sync.RWMutex{}, data: &data{ apiKeys: make([]database.APIKey, 0), @@ -73,6 +73,9 @@ func New() database.Store { locks: map[int64]struct{}{}, }, } + q.defaultProxyDisplayName = "Default" + q.defaultProxyIconURL = "/emojis/1f3e1.png" + return q } type rwMutex interface { @@ -144,14 +147,16 @@ type data struct { // Locks is a map of lock names. Any keys within the map are currently // locked. - locks map[int64]struct{} - deploymentID string - derpMeshKey string - lastUpdateCheck []byte - serviceBanner []byte - logoURL string - appSecurityKey string - lastLicenseID int32 + locks map[int64]struct{} + deploymentID string + derpMeshKey string + lastUpdateCheck []byte + serviceBanner []byte + logoURL string + appSecurityKey string + lastLicenseID int32 + defaultProxyDisplayName string + defaultProxyIconURL string } func validateDatabaseTypeWithValid(v reflect.Value) (handled bool, err error) { @@ -5170,3 +5175,16 @@ func isNull(v interface{}) bool { func isNotNull(v interface{}) bool { return reflect.ValueOf(v).FieldByName("Valid").Bool() } + +func (q *fakeQuerier) GetDefaultProxyConfig(ctx context.Context) (database.GetDefaultProxyConfigRow, error) { + return database.GetDefaultProxyConfigRow{ + DisplayName: q.defaultProxyDisplayName, + IconUrl: q.defaultProxyIconURL, + }, nil +} + +func (q *fakeQuerier) UpsertDefaultProxy(ctx context.Context, arg database.UpsertDefaultProxyParams) error { + q.defaultProxyDisplayName = arg.DisplayName + q.defaultProxyIconURL = arg.IconUrl + return nil +} diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index 2fbbd0119d7ba..136d972f4510b 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -1518,3 +1518,17 @@ func (m metricsStore) GetAuthorizedUserCount(ctx context.Context, arg database.G m.queryLatencies.WithLabelValues("GetAuthorizedUserCount").Observe(time.Since(start).Seconds()) return count, err } + +func (m metricsStore) UpsertDefaultProxy(ctx context.Context, arg database.UpsertDefaultProxyParams) error { + start := time.Now() + err := m.s.UpsertDefaultProxy(ctx, arg) + m.queryLatencies.WithLabelValues("UpsertDefaultProxy").Observe(time.Since(start).Seconds()) + return err +} + +func (m metricsStore) GetDefaultProxyConfig(ctx context.Context) (database.GetDefaultProxyConfigRow, error) { + start := time.Now() + resp, err := m.s.GetDefaultProxyConfig(ctx) + m.queryLatencies.WithLabelValues("GetDefaultProxyConfig").Observe(time.Since(start).Seconds()) + return resp, err +} diff --git a/coderd/database/dbmock/store.go b/coderd/database/dbmock/store.go index b0eb1bd02dca4..a1c75353b7a96 100644 --- a/coderd/database/dbmock/store.go +++ b/coderd/database/dbmock/store.go @@ -418,6 +418,21 @@ func (mr *MockStoreMockRecorder) GetDERPMeshKey(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDERPMeshKey", reflect.TypeOf((*MockStore)(nil).GetDERPMeshKey), arg0) } +// GetDefaultProxyConfig mocks base method. +func (m *MockStore) GetDefaultProxyConfig(arg0 context.Context) (database.GetDefaultProxyConfigRow, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetDefaultProxyConfig", arg0) + ret0, _ := ret[0].(database.GetDefaultProxyConfigRow) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetDefaultProxyConfig indicates an expected call of GetDefaultProxyConfig. +func (mr *MockStoreMockRecorder) GetDefaultProxyConfig(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDefaultProxyConfig", reflect.TypeOf((*MockStore)(nil).GetDefaultProxyConfig), arg0) +} + // GetDeploymentDAUs mocks base method. func (m *MockStore) GetDeploymentDAUs(arg0 context.Context, arg1 int32) ([]database.GetDeploymentDAUsRow, error) { m.ctrl.T.Helper() @@ -3088,6 +3103,20 @@ func (mr *MockStoreMockRecorder) UpsertAppSecurityKey(arg0, arg1 interface{}) *g return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertAppSecurityKey", reflect.TypeOf((*MockStore)(nil).UpsertAppSecurityKey), arg0, arg1) } +// UpsertDefaultProxy mocks base method. +func (m *MockStore) UpsertDefaultProxy(arg0 context.Context, arg1 database.UpsertDefaultProxyParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpsertDefaultProxy", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpsertDefaultProxy indicates an expected call of UpsertDefaultProxy. +func (mr *MockStoreMockRecorder) UpsertDefaultProxy(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertDefaultProxy", reflect.TypeOf((*MockStore)(nil).UpsertDefaultProxy), arg0, arg1) +} + // UpsertLastUpdateCheck mocks base method. func (m *MockStore) UpsertLastUpdateCheck(arg0 context.Context, arg1 string) error { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 21f19315b4995..c427ac768c79f 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -54,6 +54,7 @@ type sqlcQuerier interface { // are included. GetAuthorizationUserRoles(ctx context.Context, userID uuid.UUID) (GetAuthorizationUserRolesRow, error) GetDERPMeshKey(ctx context.Context) (string, error) + GetDefaultProxyConfig(ctx context.Context) (GetDefaultProxyConfigRow, error) GetDeploymentDAUs(ctx context.Context, tzOffset int32) ([]GetDeploymentDAUsRow, error) GetDeploymentID(ctx context.Context) (string, error) GetDeploymentWorkspaceAgentStats(ctx context.Context, createdAt time.Time) (GetDeploymentWorkspaceAgentStatsRow, error) @@ -254,6 +255,10 @@ type sqlcQuerier interface { UpdateWorkspaceTTL(ctx context.Context, arg UpdateWorkspaceTTLParams) error UpdateWorkspaceTTLToBeWithinTemplateMax(ctx context.Context, arg UpdateWorkspaceTTLToBeWithinTemplateMaxParams) error UpsertAppSecurityKey(ctx context.Context, value string) error + // The default proxy is implied and not actually stored in the database. + // So we need to store it's configuration here for display purposes. + // The functional values are immutable and controlled implicitly. + UpsertDefaultProxy(ctx context.Context, arg UpsertDefaultProxyParams) error UpsertLastUpdateCheck(ctx context.Context, value string) error UpsertLogoURL(ctx context.Context, value string) error UpsertServiceBanner(ctx context.Context, value string) error diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 19c80e7d5ba90..73bc960488a15 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -14,6 +14,7 @@ import ( "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/database/dbgen" "github.com/coder/coder/coderd/database/migrations" + "github.com/coder/coder/testutil" ) func TestGetDeploymentWorkspaceAgentStats(t *testing.T) { @@ -257,3 +258,57 @@ func TestProxyByHostname(t *testing.T) { }) } } + +func TestDefaultProxy(t *testing.T) { + t.Parallel() + if testing.Short() { + t.SkipNow() + } + sqlDB := testSQLDB(t) + err := migrations.Up(sqlDB) + require.NoError(t, err) + db := database.New(sqlDB) + + ctx := testutil.Context(t, testutil.WaitLong) + depID := uuid.NewString() + err = db.InsertDeploymentID(ctx, depID) + require.NoError(t, err, "insert deployment id") + + // Fetch empty proxy values + defProxy, err := db.GetDefaultProxyConfig(ctx) + require.NoError(t, err, "get def proxy") + + require.Equal(t, defProxy.DisplayName, "Default") + require.Equal(t, defProxy.IconUrl, "/emojis/1f3e1.png") + + // Set the proxy values + args := database.UpsertDefaultProxyParams{ + DisplayName: "displayname", + IconUrl: "/icon.png", + } + err = db.UpsertDefaultProxy(ctx, args) + require.NoError(t, err, "insert def proxy") + + defProxy, err = db.GetDefaultProxyConfig(ctx) + require.NoError(t, err, "get def proxy") + require.Equal(t, defProxy.DisplayName, args.DisplayName) + require.Equal(t, defProxy.IconUrl, args.IconUrl) + + // Upsert values + args = database.UpsertDefaultProxyParams{ + DisplayName: "newdisplayname", + IconUrl: "/newicon.png", + } + err = db.UpsertDefaultProxy(ctx, args) + require.NoError(t, err, "upsert def proxy") + + defProxy, err = db.GetDefaultProxyConfig(ctx) + require.NoError(t, err, "get def proxy") + require.Equal(t, defProxy.DisplayName, args.DisplayName) + require.Equal(t, defProxy.IconUrl, args.IconUrl) + + // Ensure other site configs are the same + found, err := db.GetDeploymentID(ctx) + require.NoError(t, err, "get deployment id") + require.Equal(t, depID, found) +} diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 6c132aa9b2126..b381aa64b1b81 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3028,6 +3028,24 @@ func (q *sqlQuerier) GetDERPMeshKey(ctx context.Context) (string, error) { return value, err } +const getDefaultProxyConfig = `-- name: GetDefaultProxyConfig :one +SELECT + COALESCE((SELECT value FROM site_configs WHERE key = 'default_proxy_display_name'), 'Default') :: text AS display_name, + COALESCE((SELECT value FROM site_configs WHERE key = 'default_proxy_icon_url'), '/emojis/1f3e1.png') :: text AS icon_url +` + +type GetDefaultProxyConfigRow struct { + DisplayName string `db:"display_name" json:"display_name"` + IconUrl string `db:"icon_url" json:"icon_url"` +} + +func (q *sqlQuerier) GetDefaultProxyConfig(ctx context.Context) (GetDefaultProxyConfigRow, error) { + row := q.db.QueryRowContext(ctx, getDefaultProxyConfig) + var i GetDefaultProxyConfigRow + err := row.Scan(&i.DisplayName, &i.IconUrl) + return i, err +} + const getDeploymentID = `-- name: GetDeploymentID :one SELECT value FROM site_configs WHERE key = 'deployment_id' ` @@ -3100,6 +3118,29 @@ func (q *sqlQuerier) UpsertAppSecurityKey(ctx context.Context, value string) err return err } +const upsertDefaultProxy = `-- name: UpsertDefaultProxy :exec +INSERT INTO site_configs (key, value) +VALUES + ('default_proxy_display_name', $1 :: text), + ('default_proxy_icon_url', $2 :: text) +ON CONFLICT + (key) +DO UPDATE SET value = EXCLUDED.value WHERE site_configs.key = EXCLUDED.key +` + +type UpsertDefaultProxyParams struct { + DisplayName string `db:"display_name" json:"display_name"` + IconUrl string `db:"icon_url" json:"icon_url"` +} + +// The default proxy is implied and not actually stored in the database. +// So we need to store it's configuration here for display purposes. +// The functional values are immutable and controlled implicitly. +func (q *sqlQuerier) UpsertDefaultProxy(ctx context.Context, arg UpsertDefaultProxyParams) error { + _, err := q.db.ExecContext(ctx, upsertDefaultProxy, arg.DisplayName, arg.IconUrl) + return err +} + const upsertLastUpdateCheck = `-- name: UpsertLastUpdateCheck :exec INSERT INTO site_configs (key, value) VALUES ('last_update_check', $1) ON CONFLICT (key) DO UPDATE SET value = $1 WHERE site_configs.key = 'last_update_check' diff --git a/coderd/database/queries/siteconfig.sql b/coderd/database/queries/siteconfig.sql index 1fb4a067fa0ad..324d08bfed17c 100644 --- a/coderd/database/queries/siteconfig.sql +++ b/coderd/database/queries/siteconfig.sql @@ -1,3 +1,23 @@ +-- name: UpsertDefaultProxy :exec +-- The default proxy is implied and not actually stored in the database. +-- So we need to store it's configuration here for display purposes. +-- The functional values are immutable and controlled implicitly. +INSERT INTO site_configs (key, value) +VALUES + ('default_proxy_display_name', @display_name :: text), + ('default_proxy_icon_url', @icon_url :: text) +ON CONFLICT + (key) +DO UPDATE SET value = EXCLUDED.value WHERE site_configs.key = EXCLUDED.key +; + +-- name: GetDefaultProxyConfig :one +SELECT + COALESCE((SELECT value FROM site_configs WHERE key = 'default_proxy_display_name'), 'Default') :: text AS display_name, + COALESCE((SELECT value FROM site_configs WHERE key = 'default_proxy_icon_url'), '/emojis/1f3e1.png') :: text AS icon_url +; + + -- name: InsertDeploymentID :exec INSERT INTO site_configs (key, value) VALUES ('deployment_id', $1); diff --git a/coderd/workspaceproxies.go b/coderd/workspaceproxies.go index 7bd5eed3f479b..f4b20e63083e5 100644 --- a/coderd/workspaceproxies.go +++ b/coderd/workspaceproxies.go @@ -29,12 +29,10 @@ func (api *API) PrimaryRegion(ctx context.Context) (codersdk.Region, error) { } return codersdk.Region{ - ID: deploymentID, - // TODO: provide some way to customize these fields for the primary - // region + ID: deploymentID, Name: "primary", DisplayName: "Default", - IconURL: "/emojis/1f60e.png", // face with sunglasses + IconURL: "/emojis/1f3e1.png", // House with garden Healthy: true, PathAppURL: api.AccessURL.String(), WildcardHostname: api.AppHostname, From 9db95ae2b2164cf97f022bf4ecf245deef3fa431 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 7 Jun 2023 10:23:18 -0500 Subject: [PATCH 02/13] Unit test to fetch primary with httpmw --- coderd/httpmw/workspaceproxy.go | 9 ++- coderd/httpmw/workspaceproxy_test.go | 41 +++++++++++- coderd/workspaceproxies.go | 35 +++++++++-- enterprise/coderd/coderd.go | 8 ++- enterprise/coderd/workspaceproxy.go | 93 ++++++++++++++++++++++++---- 5 files changed, 163 insertions(+), 23 deletions(-) diff --git a/coderd/httpmw/workspaceproxy.go b/coderd/httpmw/workspaceproxy.go index 692f51b83d2b4..5df5e64a0cc07 100644 --- a/coderd/httpmw/workspaceproxy.go +++ b/coderd/httpmw/workspaceproxy.go @@ -173,7 +173,7 @@ func WorkspaceProxyParam(r *http.Request) database.WorkspaceProxy { // parameter. // //nolint:revive -func ExtractWorkspaceProxyParam(db database.Store) func(http.Handler) http.Handler { +func ExtractWorkspaceProxyParam(db database.Store, deploymentID string, fetchPrimaryProxy func(ctx context.Context) (database.WorkspaceProxy, error)) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() @@ -188,9 +188,14 @@ func ExtractWorkspaceProxyParam(db database.Store) func(http.Handler) http.Handl var proxy database.WorkspaceProxy var dbErr error - if proxyID, err := uuid.Parse(proxyQuery); err == nil { + if proxyQuery == "primary" || proxyQuery == deploymentID { + // Requesting primary proxy + proxy, dbErr = fetchPrimaryProxy(ctx) + } else if proxyID, err := uuid.Parse(proxyQuery); err == nil { + // Request proxy by id proxy, dbErr = db.GetWorkspaceProxyByID(ctx, proxyID) } else { + // Request proxy by name proxy, dbErr = db.GetWorkspaceProxyByName(ctx, proxyQuery) } if httpapi.Is404Error(dbErr) { diff --git a/coderd/httpmw/workspaceproxy_test.go b/coderd/httpmw/workspaceproxy_test.go index a2bbe9dc49b8a..818acf85c4442 100644 --- a/coderd/httpmw/workspaceproxy_test.go +++ b/coderd/httpmw/workspaceproxy_test.go @@ -212,7 +212,7 @@ func TestExtractWorkspaceProxyParam(t *testing.T) { routeContext.URLParams.Add("workspaceproxy", proxy.Name) r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, routeContext)) - httpmw.ExtractWorkspaceProxyParam(db)(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + httpmw.ExtractWorkspaceProxyParam(db, uuid.NewString(), nil)(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { // Checks that it exists on the context! _ = httpmw.WorkspaceProxyParam(request) successHandler.ServeHTTP(writer, request) @@ -236,7 +236,7 @@ func TestExtractWorkspaceProxyParam(t *testing.T) { routeContext.URLParams.Add("workspaceproxy", proxy.ID.String()) r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, routeContext)) - httpmw.ExtractWorkspaceProxyParam(db)(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + httpmw.ExtractWorkspaceProxyParam(db, uuid.NewString(), nil)(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { // Checks that it exists on the context! _ = httpmw.WorkspaceProxyParam(request) successHandler.ServeHTTP(writer, request) @@ -258,9 +258,44 @@ func TestExtractWorkspaceProxyParam(t *testing.T) { routeContext.URLParams.Add("workspaceproxy", uuid.NewString()) r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, routeContext)) - httpmw.ExtractWorkspaceProxyParam(db)(successHandler).ServeHTTP(rw, r) + httpmw.ExtractWorkspaceProxyParam(db, uuid.NewString(), nil)(successHandler).ServeHTTP(rw, r) res := rw.Result() defer res.Body.Close() require.Equal(t, http.StatusNotFound, res.StatusCode) }) + + t.Run("FetchPrimary", func(t *testing.T) { + t.Parallel() + var ( + db = dbfake.New() + r = httptest.NewRequest("GET", "/", nil) + rw = httptest.NewRecorder() + deploymentID = uuid.New() + primaryProxy = database.WorkspaceProxy{ + ID: deploymentID, + Name: "primary", + DisplayName: "Default", + Icon: "Icon", + Url: "Url", + WildcardHostname: "Wildcard", + } + fetchPrimary = func(ctx context.Context) (database.WorkspaceProxy, error) { + return primaryProxy, nil + } + ) + + routeContext := chi.NewRouteContext() + routeContext.URLParams.Add("workspaceproxy", deploymentID.String()) + r = r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, routeContext)) + + httpmw.ExtractWorkspaceProxyParam(db, deploymentID.String(), fetchPrimary)(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + // Checks that it exists on the context! + found := httpmw.WorkspaceProxyParam(request) + require.Equal(t, primaryProxy, found) + successHandler.ServeHTTP(writer, request) + })).ServeHTTP(rw, r) + res := rw.Result() + defer res.Body.Close() + require.Equal(t, http.StatusOK, res.StatusCode) + }) } diff --git a/coderd/workspaceproxies.go b/coderd/workspaceproxies.go index f4b20e63083e5..8ba8c28db9616 100644 --- a/coderd/workspaceproxies.go +++ b/coderd/workspaceproxies.go @@ -3,16 +3,18 @@ package coderd import ( "context" "database/sql" - "net/http" - "github.com/google/uuid" "golang.org/x/xerrors" + "net/http" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/codersdk" ) +// PrimaryRegion exposes the user facing values of a workspace proxy to +// be used by a user. func (api *API) PrimaryRegion(ctx context.Context) (codersdk.Region, error) { deploymentIDStr, err := api.Database.GetDeploymentID(ctx) if xerrors.Is(err, sql.ErrNoRows) { @@ -28,17 +30,42 @@ func (api *API) PrimaryRegion(ctx context.Context) (codersdk.Region, error) { deploymentID = uuid.Nil } + proxy, err := api.Database.GetDefaultProxyConfig(ctx) + if err != nil { + return codersdk.Region{}, xerrors.Errorf("get default proxy config: %w", err) + } + return codersdk.Region{ ID: deploymentID, Name: "primary", - DisplayName: "Default", - IconURL: "/emojis/1f3e1.png", // House with garden + DisplayName: proxy.DisplayName, + IconURL: proxy.IconUrl, Healthy: true, PathAppURL: api.AccessURL.String(), WildcardHostname: api.AppHostname, }, nil } +// PrimaryWorkspaceProxy returns the primary workspace proxy for the site. +func (api *API) PrimaryWorkspaceProxy(ctx context.Context) (database.WorkspaceProxy, error) { + region, err := api.PrimaryRegion(ctx) + if err != nil { + return database.WorkspaceProxy{}, err + } + + // The default proxy is an edge case because these values are computed + // rather then being stored in the database. + return database.WorkspaceProxy{ + ID: region.ID, + Name: region.Name, + DisplayName: region.DisplayName, + Icon: region.IconURL, + Url: region.PathAppURL, + WildcardHostname: region.WildcardHostname, + Deleted: false, + }, nil +} + // @Summary Get site-wide regions for workspace connections // @ID get-site-wide-regions-for-workspace-connections // @Security CoderSessionToken diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index ff44aed60c676..ee860cfba9ed0 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -72,6 +72,11 @@ func New(ctx context.Context, options *Options) (*API, error) { RedirectToLogin: false, }) + deploymentID, err := options.Database.GetDeploymentID(ctx) + if err != nil { + return nil, xerrors.Errorf("failed to get deployment ID: %w", err) + } + api.AGPL.APIHandler.Group(func(r chi.Router) { r.Get("/entitlements", api.serveEntitlements) // /regions overrides the AGPL /regions endpoint @@ -118,7 +123,7 @@ func New(ctx context.Context, options *Options) (*API, error) { r.Route("/{workspaceproxy}", func(r chi.Router) { r.Use( apiKeyMiddleware, - httpmw.ExtractWorkspaceProxyParam(api.Database), + httpmw.ExtractWorkspaceProxyParam(api.Database, deploymentID, api.AGPL.PrimaryWorkspaceProxy), ) r.Get("/", api.workspaceProxy) @@ -225,7 +230,6 @@ func New(ctx context.Context, options *Options) (*API, error) { RootCAs: meshRootCA, ServerName: options.AccessURL.Hostname(), } - var err error api.replicaManager, err = replicasync.New(ctx, options.Logger, options.Database, options.Pubsub, &replicasync.Options{ ID: api.AGPL.ID, RelayAddress: options.DERPServerRelayAddress, diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index 07ac38a990bae..faaf6c1d9b4b0 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -127,23 +127,85 @@ func (api *API) patchWorkspaceProxy(rw http.ResponseWriter, r *http.Request) { } } - updatedProxy, err := api.Database.UpdateWorkspaceProxy(ctx, database.UpdateWorkspaceProxyParams{ - Name: req.Name, - DisplayName: req.DisplayName, - Icon: req.Icon, - ID: proxy.ID, - // If hashedSecret is nil or empty, this will not update the secret. - TokenHashedSecret: hashedSecret, - }) - if httpapi.Is404Error(err) { - httpapi.ResourceNotFound(rw) - return - } + deploymentIDStr, err := api.Database.GetDeploymentID(ctx) if err != nil { httpapi.InternalServerError(rw, err) return } + var updatedProxy database.WorkspaceProxy + if proxy.ID.String() == deploymentIDStr { + // User is editing the default primary proxy. + if req.Name != "" { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Cannot update name of default primary proxy", + Validations: []codersdk.ValidationError{ + {Field: "name", Detail: "Cannot update name of default primary proxy"}, + }, + }) + return + } + if req.DisplayName == "" && req.Icon == "" { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "No update arguments provided. Nothing to do.", + Validations: []codersdk.ValidationError{ + {Field: "display_name", Detail: "No value provided."}, + {Field: "icon", Detail: "No value provided."}, + }, + }) + return + } + + args := database.UpsertDefaultProxyParams{ + DisplayName: req.DisplayName, + IconUrl: req.Icon, + } + if req.DisplayName == "" || req.Icon == "" { + // If the user has not specified an update value, use the existing value. + existing, err := api.Database.GetDefaultProxyConfig(ctx) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + if req.DisplayName == "" { + args.DisplayName = existing.DisplayName + } + if req.Icon == "" { + args.IconUrl = existing.IconUrl + } + } + + err = api.Database.UpsertDefaultProxy(ctx, args) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + + // Use the primary region to fetch the default proxy values. + updatedProxy, err = api.AGPL.PrimaryWorkspaceProxy(ctx) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + } else { + updatedProxy, err = api.Database.UpdateWorkspaceProxy(ctx, database.UpdateWorkspaceProxyParams{ + Name: req.Name, + DisplayName: req.DisplayName, + Icon: req.Icon, + ID: proxy.ID, + // If hashedSecret is nil or empty, this will not update the secret. + TokenHashedSecret: hashedSecret, + }) + if httpapi.Is404Error(err) { + httpapi.ResourceNotFound(rw) + return + } + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + } + aReq.New = updatedProxy status, ok := api.ProxyHealth.HealthStatus()[updatedProxy.ID] if !ok { @@ -182,6 +244,13 @@ func (api *API) deleteWorkspaceProxy(rw http.ResponseWriter, r *http.Request) { aReq.Old = proxy defer commitAudit() + if proxy.Name == "primary" { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Cannot delete primary proxy", + }) + return + } + err := api.Database.UpdateWorkspaceProxyDeleted(ctx, database.UpdateWorkspaceProxyDeletedParams{ ID: proxy.ID, Deleted: true, From e2356b20dfa7a371c011b55638582f50da6ec8e5 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 7 Jun 2023 11:16:21 -0500 Subject: [PATCH 03/13] Show default proxy in proxy list --- coderd/database/modelmethods.go | 4 ++++ enterprise/coderd/workspaceproxy.go | 24 ++++++++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index 1c20a0cced741..bf3b81f3ff74c 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -186,6 +186,10 @@ func (w WorkspaceProxy) RBACObject() rbac.Object { WithID(w.ID) } +func (w WorkspaceProxy) IsPrimary() bool { + return w.Name == "primary" +} + func (f File) RBACObject() rbac.Object { return rbac.ResourceFile. WithID(f.ID). diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index faaf6c1d9b4b0..38061e551e492 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -138,7 +138,7 @@ func (api *API) patchWorkspaceProxy(rw http.ResponseWriter, r *http.Request) { // User is editing the default primary proxy. if req.Name != "" { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Cannot update name of default primary proxy", + Message: "Cannot update name of default primary proxy, did you mean to update the 'display name'?", Validations: []codersdk.ValidationError{ {Field: "name", Detail: "Cannot update name of default primary proxy"}, }, @@ -244,7 +244,7 @@ func (api *API) deleteWorkspaceProxy(rw http.ResponseWriter, r *http.Request) { aReq.Old = proxy defer commitAudit() - if proxy.Name == "primary" { + if proxy.IsPrimary() { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Cannot delete primary proxy", }) @@ -403,6 +403,14 @@ func (api *API) workspaceProxies(rw http.ResponseWriter, r *http.Request) { return } + // Add the primary as well + primaryProxy, err := api.AGPL.PrimaryWorkspaceProxy(ctx) + if err != nil && !xerrors.Is(err, sql.ErrNoRows) { + httpapi.InternalServerError(rw, err) + return + } + proxies = append(proxies, primaryProxy) + statues := api.ProxyHealth.HealthStatus() httpapi.Write(ctx, rw, http.StatusOK, convertProxies(proxies, statues)) } @@ -680,6 +688,18 @@ func convertProxies(p []database.WorkspaceProxy, statuses map[uuid.UUID]proxyhea } func convertProxy(p database.WorkspaceProxy, status proxyhealth.ProxyStatus) codersdk.WorkspaceProxy { + if p.IsPrimary() { + // Primary is always healthy since the primary serves the api that this + // is returned from. + u, _ := url.Parse(p.Url) + status = proxyhealth.ProxyStatus{ + Proxy: p, + ProxyHost: u.Host, + Status: proxyhealth.Healthy, + Report: codersdk.ProxyHealthReport{}, + CheckedAt: time.Now(), + } + } if status.Status == "" { status.Status = proxyhealth.Unknown } From d4f00cece41f01bd9820f1fc5253fecca61888e3 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 7 Jun 2023 11:19:32 -0500 Subject: [PATCH 04/13] fixup! Show default proxy in proxy list --- enterprise/coderd/workspaceproxy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index 38061e551e492..b9808e02b5592 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -136,7 +136,7 @@ func (api *API) patchWorkspaceProxy(rw http.ResponseWriter, r *http.Request) { var updatedProxy database.WorkspaceProxy if proxy.ID.String() == deploymentIDStr { // User is editing the default primary proxy. - if req.Name != "" { + if req.Name != "" && req.Name != proxy.Name { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Cannot update name of default primary proxy, did you mean to update the 'display name'?", Validations: []codersdk.ValidationError{ From 9a6e5e1220cfda9133d68d3e9c109f864ad1283d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 7 Jun 2023 16:25:07 +0000 Subject: [PATCH 05/13] Import order --- coderd/workspaceproxies.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/coderd/workspaceproxies.go b/coderd/workspaceproxies.go index 8ba8c28db9616..e7db84898bf49 100644 --- a/coderd/workspaceproxies.go +++ b/coderd/workspaceproxies.go @@ -3,9 +3,10 @@ package coderd import ( "context" "database/sql" + "net/http" + "github.com/google/uuid" "golang.org/x/xerrors" - "net/http" "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/database/dbauthz" From 55b452784e284145da1bac789e147765a73c9fe6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 7 Jun 2023 11:25:19 -0500 Subject: [PATCH 06/13] Lint --- coderd/database/dbfake/databasefake.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go index 3f6b362de704e..cf4fdcbf10bb4 100644 --- a/coderd/database/dbfake/databasefake.go +++ b/coderd/database/dbfake/databasefake.go @@ -5176,14 +5176,14 @@ func isNotNull(v interface{}) bool { return reflect.ValueOf(v).FieldByName("Valid").Bool() } -func (q *fakeQuerier) GetDefaultProxyConfig(ctx context.Context) (database.GetDefaultProxyConfigRow, error) { +func (q *fakeQuerier) GetDefaultProxyConfig(_ context.Context) (database.GetDefaultProxyConfigRow, error) { return database.GetDefaultProxyConfigRow{ DisplayName: q.defaultProxyDisplayName, IconUrl: q.defaultProxyIconURL, }, nil } -func (q *fakeQuerier) UpsertDefaultProxy(ctx context.Context, arg database.UpsertDefaultProxyParams) error { +func (q *fakeQuerier) UpsertDefaultProxy(_ context.Context, arg database.UpsertDefaultProxyParams) error { q.defaultProxyDisplayName = arg.DisplayName q.defaultProxyIconURL = arg.IconUrl return nil From d4a8b81be75da016f5558180a999f43b105a4404 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 7 Jun 2023 11:47:40 -0500 Subject: [PATCH 07/13] fix unit tests --- coderd/database/dbauthz/querier_test.go | 5 ++++- enterprise/cli/workspaceproxy_test.go | 13 ++++++++++--- enterprise/coderd/workspaceproxy_test.go | 3 ++- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/coderd/database/dbauthz/querier_test.go b/coderd/database/dbauthz/querier_test.go index 701a45fa44aa3..00355d6f98e0a 100644 --- a/coderd/database/dbauthz/querier_test.go +++ b/coderd/database/dbauthz/querier_test.go @@ -336,7 +336,10 @@ func (s *MethodTestSuite) TestLicense() { check.Args().Asserts().Returns("") })) s.Run("GetDefaultProxyConfig", s.Subtest(func(db database.Store, check *expects) { - check.Args().Asserts().Returns(database.GetDefaultProxyConfigRow{}) + check.Args().Asserts().Returns(database.GetDefaultProxyConfigRow{ + DisplayName: "Default", + IconUrl: "/emojis/1f3e1.png", + }) })) s.Run("GetLogoURL", s.Subtest(func(db database.Store, check *expects) { err := db.UpsertLogoURL(context.Background(), "value") diff --git a/enterprise/cli/workspaceproxy_test.go b/enterprise/cli/workspaceproxy_test.go index de2316f9ab7a9..357c609125754 100644 --- a/enterprise/cli/workspaceproxy_test.go +++ b/enterprise/cli/workspaceproxy_test.go @@ -82,8 +82,15 @@ func Test_ProxyCRUD(t *testing.T) { // Also check via the api proxies, err := client.WorkspaceProxies(ctx) require.NoError(t, err, "failed to get workspace proxies") - require.Len(t, proxies, 1, "expected 1 proxy") - require.Equal(t, expectedName, proxies[0].Name, "expected proxy name to match") + // Include primary + require.Len(t, proxies, 2, "expected 1 proxy") + found := false + for _, proxy := range proxies { + if proxy.Name == expectedName { + found = true + } + } + require.True(t, found, "expected proxy to be found") }) t.Run("Delete", func(t *testing.T) { @@ -130,6 +137,6 @@ func Test_ProxyCRUD(t *testing.T) { proxies, err := client.WorkspaceProxies(ctx) require.NoError(t, err, "failed to get workspace proxies") - require.Len(t, proxies, 0, "expected no proxies") + require.Len(t, proxies, 1, "expected only primary proxy") }) } diff --git a/enterprise/coderd/workspaceproxy_test.go b/enterprise/coderd/workspaceproxy_test.go index 755c2c6afa517..0eca54c5a75af 100644 --- a/enterprise/coderd/workspaceproxy_test.go +++ b/enterprise/coderd/workspaceproxy_test.go @@ -325,7 +325,8 @@ func TestWorkspaceProxyCRUD(t *testing.T) { proxies, err := client.WorkspaceProxies(ctx) require.NoError(t, err) - require.Len(t, proxies, 0) + // Default proxy is always there + require.Len(t, proxies, 1) }) } From 28d80adf8c9de5466647eb1a65e853de61d7879d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 7 Jun 2023 13:18:12 -0500 Subject: [PATCH 08/13] Make sure unit tests have deployment ids --- coderd/coderdtest/coderdtest.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index c058925f99403..a8ba86cce2fe7 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -206,6 +206,10 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can options.Database = dbauthz.New(options.Database, options.Authorizer, slogtest.Make(t, nil).Leveled(slog.LevelDebug)) } + // Some routes expect a deployment ID + err := options.Database.InsertDeploymentID(dbauthz.AsSystemRestricted(context.Background()), uuid.NewString()) + require.NoError(t, err, "insert a deployment id") + if options.DeploymentValues == nil { options.DeploymentValues = DeploymentValues(t) } From 904ce34141fa9db3246af948ef673259f84c8ca6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 7 Jun 2023 13:40:27 -0500 Subject: [PATCH 09/13] Unit test deployment id stuff --- coderd/coderdtest/coderdtest.go | 13 ++++++++++--- enterprise/coderd/workspaceproxy.go | 2 +- enterprise/coderd/workspaceproxy_test.go | 21 +++++++++------------ 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index a8ba86cce2fe7..f0c57552087cf 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -10,6 +10,7 @@ import ( "crypto/tls" "crypto/x509" "crypto/x509/pkix" + "database/sql" "encoding/base64" "encoding/json" "encoding/pem" @@ -206,9 +207,15 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can options.Database = dbauthz.New(options.Database, options.Authorizer, slogtest.Make(t, nil).Leveled(slog.LevelDebug)) } - // Some routes expect a deployment ID - err := options.Database.InsertDeploymentID(dbauthz.AsSystemRestricted(context.Background()), uuid.NewString()) - require.NoError(t, err, "insert a deployment id") + // Some routes expect a deployment ID, so just make sure one exists. + // Check first incase the caller already set up this database. + // nolint:gocritic // Setting up unit test data inside test helper + depID, err := options.Database.GetDeploymentID(dbauthz.AsSystemRestricted(context.Background())) + if xerrors.Is(err, sql.ErrNoRows) || depID == "" { + // nolint:gocritic // Setting up unit test data inside test helper + err := options.Database.InsertDeploymentID(dbauthz.AsSystemRestricted(context.Background()), uuid.NewString()) + require.NoError(t, err, "insert a deployment id") + } if options.DeploymentValues == nil { options.DeploymentValues = DeploymentValues(t) diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index b9808e02b5592..b1ddb8fc06ff1 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -409,7 +409,7 @@ func (api *API) workspaceProxies(rw http.ResponseWriter, r *http.Request) { httpapi.InternalServerError(rw, err) return } - proxies = append(proxies, primaryProxy) + proxies = append([]database.WorkspaceProxy{primaryProxy}, proxies...) statues := api.ProxyHealth.HealthStatus() httpapi.Write(ctx, rw, http.StatusOK, convertProxies(proxies, statues)) diff --git a/enterprise/coderd/workspaceproxy_test.go b/enterprise/coderd/workspaceproxy_test.go index 0eca54c5a75af..78d32bdeb223a 100644 --- a/enterprise/coderd/workspaceproxy_test.go +++ b/enterprise/coderd/workspaceproxy_test.go @@ -44,11 +44,6 @@ func TestRegions(t *testing.T) { } db, pubsub := dbtestutil.NewDB(t) - deploymentID := uuid.New() - - ctx := testutil.Context(t, testutil.WaitLong) - err := db.InsertDeploymentID(ctx, deploymentID.String()) - require.NoError(t, err) client := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ @@ -58,14 +53,18 @@ func TestRegions(t *testing.T) { DeploymentValues: dv, }, }) + _ = coderdtest.CreateFirstUser(t, client) + ctx := testutil.Context(t, testutil.WaitLong) + deploymentID, err := db.GetDeploymentID(ctx) + require.NoError(t, err, "get deployment ID") regions, err := client.Regions(ctx) require.NoError(t, err) require.Len(t, regions, 1) require.NotEqual(t, uuid.Nil, regions[0].ID) - require.Equal(t, regions[0].ID, deploymentID) + require.Equal(t, regions[0].ID.String(), deploymentID) require.Equal(t, "primary", regions[0].Name) require.Equal(t, "Default", regions[0].DisplayName) require.NotEmpty(t, regions[0].IconURL) @@ -89,11 +88,6 @@ func TestRegions(t *testing.T) { } db, pubsub := dbtestutil.NewDB(t) - deploymentID := uuid.New() - - ctx := testutil.Context(t, testutil.WaitLong) - err := db.InsertDeploymentID(ctx, deploymentID.String()) - require.NoError(t, err) client, closer, api := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ Options: &coderdtest.Options{ @@ -106,6 +100,9 @@ func TestRegions(t *testing.T) { t.Cleanup(func() { _ = closer.Close() }) + ctx := testutil.Context(t, testutil.WaitLong) + deploymentID, err := db.GetDeploymentID(ctx) + require.NoError(t, err, "get deployment ID") _ = coderdtest.CreateFirstUser(t, client) _ = coderdenttest.AddLicense(t, client, coderdenttest.LicenseOptions{ Features: license.Features{ @@ -131,7 +128,7 @@ func TestRegions(t *testing.T) { // Region 0 is the primary require.Len(t, regions, 1) require.NotEqual(t, uuid.Nil, regions[0].ID) - require.Equal(t, regions[0].ID, deploymentID) + require.Equal(t, regions[0].ID.String(), deploymentID) require.Equal(t, "primary", regions[0].Name) require.Equal(t, "Default", regions[0].DisplayName) require.NotEmpty(t, regions[0].IconURL) From bf0e1ced478ca0f7da69c2d6b3021d4d3bf8f733 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 7 Jun 2023 14:01:56 -0500 Subject: [PATCH 10/13] Fix data race' --- enterprise/coderd/workspaceproxy_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/enterprise/coderd/workspaceproxy_test.go b/enterprise/coderd/workspaceproxy_test.go index 78d32bdeb223a..c04f7f749cc54 100644 --- a/enterprise/coderd/workspaceproxy_test.go +++ b/enterprise/coderd/workspaceproxy_test.go @@ -385,11 +385,10 @@ func TestIssueSignedAppToken(t *testing.T) { }) require.NoError(t, err) - proxyClient := wsproxysdk.New(client.URL) - proxyClient.SetSessionToken(proxyRes.ProxyToken) - t.Run("BadAppRequest", func(t *testing.T) { t.Parallel() + proxyClient := wsproxysdk.New(client.URL) + proxyClient.SetSessionToken(proxyRes.ProxyToken) ctx := testutil.Context(t, testutil.WaitLong) _, err = proxyClient.IssueSignedAppToken(ctx, workspaceapps.IssueTokenRequest{ @@ -410,6 +409,8 @@ func TestIssueSignedAppToken(t *testing.T) { } t.Run("OK", func(t *testing.T) { t.Parallel() + proxyClient := wsproxysdk.New(client.URL) + proxyClient.SetSessionToken(proxyRes.ProxyToken) ctx := testutil.Context(t, testutil.WaitLong) _, err = proxyClient.IssueSignedAppToken(ctx, goodRequest) @@ -418,6 +419,8 @@ func TestIssueSignedAppToken(t *testing.T) { t.Run("OKHTML", func(t *testing.T) { t.Parallel() + proxyClient := wsproxysdk.New(client.URL) + proxyClient.SetSessionToken(proxyRes.ProxyToken) rw := httptest.NewRecorder() ctx := testutil.Context(t, testutil.WaitLong) From 881895b50fa318280426e2ba236d7a6b858f847a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 8 Jun 2023 08:54:04 -0500 Subject: [PATCH 11/13] sql spacing --- coderd/database/queries/siteconfig.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/queries/siteconfig.sql b/coderd/database/queries/siteconfig.sql index 324d08bfed17c..5853940b5e04d 100644 --- a/coderd/database/queries/siteconfig.sql +++ b/coderd/database/queries/siteconfig.sql @@ -5,7 +5,7 @@ INSERT INTO site_configs (key, value) VALUES ('default_proxy_display_name', @display_name :: text), - ('default_proxy_icon_url', @icon_url :: text) + ('default_proxy_icon_url', @icon_url :: text) ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value WHERE site_configs.key = EXCLUDED.key From 356dc3a7cd94a7b14e374f37d1243db6a40f4844 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 8 Jun 2023 09:52:27 -0500 Subject: [PATCH 12/13] factor out patch primary --- enterprise/coderd/workspaceproxy.go | 114 ++++++++++++++++------------ 1 file changed, 65 insertions(+), 49 deletions(-) diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index b1ddb8fc06ff1..8da03397b4e6e 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -136,55 +136,9 @@ func (api *API) patchWorkspaceProxy(rw http.ResponseWriter, r *http.Request) { var updatedProxy database.WorkspaceProxy if proxy.ID.String() == deploymentIDStr { // User is editing the default primary proxy. - if req.Name != "" && req.Name != proxy.Name { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Cannot update name of default primary proxy, did you mean to update the 'display name'?", - Validations: []codersdk.ValidationError{ - {Field: "name", Detail: "Cannot update name of default primary proxy"}, - }, - }) - return - } - if req.DisplayName == "" && req.Icon == "" { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "No update arguments provided. Nothing to do.", - Validations: []codersdk.ValidationError{ - {Field: "display_name", Detail: "No value provided."}, - {Field: "icon", Detail: "No value provided."}, - }, - }) - return - } - - args := database.UpsertDefaultProxyParams{ - DisplayName: req.DisplayName, - IconUrl: req.Icon, - } - if req.DisplayName == "" || req.Icon == "" { - // If the user has not specified an update value, use the existing value. - existing, err := api.Database.GetDefaultProxyConfig(ctx) - if err != nil { - httpapi.InternalServerError(rw, err) - return - } - if req.DisplayName == "" { - args.DisplayName = existing.DisplayName - } - if req.Icon == "" { - args.IconUrl = existing.IconUrl - } - } - - err = api.Database.UpsertDefaultProxy(ctx, args) - if err != nil { - httpapi.InternalServerError(rw, err) - return - } - - // Use the primary region to fetch the default proxy values. - updatedProxy, err = api.AGPL.PrimaryWorkspaceProxy(ctx) - if err != nil { - httpapi.InternalServerError(rw, err) + var ok bool + updatedProxy, ok = api.patchPrimaryWorkspaceProxy(req, rw, r) + if !ok { return } } else { @@ -221,6 +175,68 @@ func (api *API) patchWorkspaceProxy(rw http.ResponseWriter, r *http.Request) { go api.forceWorkspaceProxyHealthUpdate(api.ctx) } +// patchPrimaryWorkspaceProxy handles the special case of updating the default +func (api *API) patchPrimaryWorkspaceProxy(req codersdk.PatchWorkspaceProxy, rw http.ResponseWriter, r *http.Request) (database.WorkspaceProxy, bool) { + var ( + ctx = r.Context() + proxy = httpmw.WorkspaceProxyParam(r) + ) + + // User is editing the default primary proxy. + if req.Name != "" && req.Name != proxy.Name { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Cannot update name of default primary proxy, did you mean to update the 'display name'?", + Validations: []codersdk.ValidationError{ + {Field: "name", Detail: "Cannot update name of default primary proxy"}, + }, + }) + return database.WorkspaceProxy{}, false + } + if req.DisplayName == "" && req.Icon == "" { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "No update arguments provided. Nothing to do.", + Validations: []codersdk.ValidationError{ + {Field: "display_name", Detail: "No value provided."}, + {Field: "icon", Detail: "No value provided."}, + }, + }) + return database.WorkspaceProxy{}, false + } + + args := database.UpsertDefaultProxyParams{ + DisplayName: req.DisplayName, + IconUrl: req.Icon, + } + if req.DisplayName == "" || req.Icon == "" { + // If the user has not specified an update value, use the existing value. + existing, err := api.Database.GetDefaultProxyConfig(ctx) + if err != nil { + httpapi.InternalServerError(rw, err) + return database.WorkspaceProxy{}, false + } + if req.DisplayName == "" { + args.DisplayName = existing.DisplayName + } + if req.Icon == "" { + args.IconUrl = existing.IconUrl + } + } + + err := api.Database.UpsertDefaultProxy(ctx, args) + if err != nil { + httpapi.InternalServerError(rw, err) + return database.WorkspaceProxy{}, false + } + + // Use the primary region to fetch the default proxy values. + updatedProxy, err := api.AGPL.PrimaryWorkspaceProxy(ctx) + if err != nil { + httpapi.InternalServerError(rw, err) + return database.WorkspaceProxy{}, false + } + return updatedProxy, true +} + // @Summary Delete workspace proxy // @ID delete-workspace-proxy // @Security CoderSessionToken From 79e2637e791bcd8e4375f248a4a7e9762c22d80d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 8 Jun 2023 15:03:27 +0000 Subject: [PATCH 13/13] Make gen --- coderd/database/queries.sql.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index b381aa64b1b81..d57ca4b8ae036 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3122,7 +3122,7 @@ const upsertDefaultProxy = `-- name: UpsertDefaultProxy :exec INSERT INTO site_configs (key, value) VALUES ('default_proxy_display_name', $1 :: text), - ('default_proxy_icon_url', $2 :: text) + ('default_proxy_icon_url', $2 :: text) ON CONFLICT (key) DO UPDATE SET value = EXCLUDED.value WHERE site_configs.key = EXCLUDED.key