From 4ab6e9c38a9ccd2313c7d3423655686333132f92 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 15 Apr 2024 15:43:36 +0200 Subject: [PATCH 01/13] Add experiments detail API & tests Signed-off-by: Danny Kopping --- coderd/coderd.go | 3 +- coderd/experiments.go | 12 ++++ coderd/experiments_test.go | 142 ++++++++++++++++++++++++++++++++++++- codersdk/deployment.go | 55 ++++++++++++++ 4 files changed, 208 insertions(+), 4 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 67b16e9032bfe..576b43e14f9a6 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -754,6 +754,7 @@ func New(options *Options) *API { r.Route("/experiments", func(r chi.Router) { r.Use(apiKeyMiddleware) r.Get("/available", handleExperimentsSafe) + r.Get("/detail", api.handleExperimentsDetail) r.Get("/", api.handleExperimentsGet) }) r.Get("/updatecheck", api.updateCheck) @@ -1455,7 +1456,7 @@ func ReadExperiments(log slog.Logger, raw []string) codersdk.Experiments { exps := make([]codersdk.Experiment, 0, len(raw)) for _, v := range raw { switch v { - case "*": + case codersdk.ExperimentsAllWildcard: exps = append(exps, codersdk.ExperimentsAll...) default: ex := codersdk.Experiment(strings.ToLower(v)) diff --git a/coderd/experiments.go b/coderd/experiments.go index f7debd8c68bbb..af663292a73c2 100644 --- a/coderd/experiments.go +++ b/coderd/experiments.go @@ -32,3 +32,15 @@ func handleExperimentsSafe(rw http.ResponseWriter, r *http.Request) { Safe: codersdk.ExperimentsAll, }) } + +// @Summary Get experiments' details +// @ID get-experiments-details +// @Security CoderSessionToken +// @Produce json +// @Tags General +// @Success 200 {array} codersdk.ExperimentDetail +// @Router /experiments/detail [get] +func (api *API) handleExperimentsDetail(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + httpapi.Write(ctx, rw, http.StatusOK, codersdk.ExperimentDetails(api.Experiments)) +} diff --git a/coderd/experiments_test.go b/coderd/experiments_test.go index 4288b9953fec6..6f15f7f5f4608 100644 --- a/coderd/experiments_test.go +++ b/coderd/experiments_test.go @@ -57,7 +57,7 @@ func Test_Experiments(t *testing.T) { t.Run("wildcard", func(t *testing.T) { t.Parallel() cfg := coderdtest.DeploymentValues(t) - cfg.Experiments = []string{"*"} + cfg.Experiments = []string{codersdk.ExperimentsAllWildcard} client := coderdtest.New(t, &coderdtest.Options{ DeploymentValues: cfg, }) @@ -79,7 +79,7 @@ func Test_Experiments(t *testing.T) { t.Run("alternate wildcard with manual opt-in", func(t *testing.T) { t.Parallel() cfg := coderdtest.DeploymentValues(t) - cfg.Experiments = []string{"*", "dAnGeR"} + cfg.Experiments = []string{codersdk.ExperimentsAllWildcard, "dAnGeR"} client := coderdtest.New(t, &coderdtest.Options{ DeploymentValues: cfg, }) @@ -102,7 +102,7 @@ func Test_Experiments(t *testing.T) { t.Run("Unauthorized", func(t *testing.T) { t.Parallel() cfg := coderdtest.DeploymentValues(t) - cfg.Experiments = []string{"*"} + cfg.Experiments = []string{codersdk.ExperimentsAllWildcard} client := coderdtest.New(t, &coderdtest.Options{ DeploymentValues: cfg, }) @@ -133,4 +133,140 @@ func Test_Experiments(t *testing.T) { require.NotNil(t, experiments) require.ElementsMatch(t, codersdk.ExperimentsAll, experiments.Safe) }) + + t.Run("experiments detail", func(t *testing.T) { + t.Parallel() + + const ( + invalidExp = "bob" + expiredExp = "auto-fill-parameters" // using a string here not a constant since this experiment has expired & will be deleted eventually + ) + + tests := []struct { + name string + enabledValid []codersdk.Experiment + enabledInvalid []codersdk.Experiment + expectedExtraCount int + }{ + { + name: "using defaults", + }, + { + name: "use all (*)", + enabledValid: []codersdk.Experiment{codersdk.Experiment(codersdk.ExperimentsAllWildcard)}, + }, + { + name: "only valid experiments", + enabledValid: codersdk.ExperimentsAll, + }, + { + name: "use all (*) + invalid", + enabledValid: []codersdk.Experiment{codersdk.Experiment(codersdk.ExperimentsAllWildcard), codersdk.Experiment(expiredExp)}, + expectedExtraCount: 1, + }, + { + name: "valid + expired experiments", + enabledValid: codersdk.ExperimentsAll, + enabledInvalid: []codersdk.Experiment{codersdk.Experiment(expiredExp)}, + expectedExtraCount: 1, + }, + { + name: "valid + expired + invalid experiments", + enabledValid: codersdk.ExperimentsAll, + enabledInvalid: []codersdk.Experiment{codersdk.Experiment(invalidExp), codersdk.Experiment(expiredExp)}, + expectedExtraCount: 2, + }, + { + name: "only expired", + enabledInvalid: []codersdk.Experiment{codersdk.Experiment(expiredExp)}, + expectedExtraCount: 1, + }, + { + name: "only invalid", + enabledInvalid: []codersdk.Experiment{codersdk.Experiment(invalidExp)}, + expectedExtraCount: 1, + }, + { + name: "expired + invalid experiments", + enabledInvalid: []codersdk.Experiment{codersdk.Experiment(invalidExp), codersdk.Experiment(expiredExp)}, + expectedExtraCount: 2, + }, + } + + for _, tc := range tests { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + var exps []string + + // given + for _, e := range tc.enabledValid { + exps = append(exps, string(e)) + } + for _, e := range tc.enabledInvalid { + exps = append(exps, string(e)) + } + + cfg := coderdtest.DeploymentValues(t) + cfg.Experiments = exps + client := coderdtest.New(t, &coderdtest.Options{ + DeploymentValues: cfg, + }) + _ = coderdtest.CreateFirstUser(t, client) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + // when + experiments, err := client.ExperimentDetails(ctx) + + // then + require.NoError(t, err) + require.Len(t, experiments, len(codersdk.ExperimentsAll)+tc.expectedExtraCount) + require.Conditionf(t, func() (success bool) { + var enabled []bool + + var validCount int + for _, exp := range tc.enabledValid { + // don't count wildcard experiment itself as a single experiment + if exp == codersdk.ExperimentsAllWildcard { + validCount += len(codersdk.ExperimentsAll) + } else { + validCount++ + } + } + + for _, exp := range append(tc.enabledValid, tc.enabledInvalid...) { + for _, e := range experiments { + // * is special-cased to mean all experiments + if (exp == codersdk.ExperimentsAllWildcard || e.Name == exp) && e.Enabled { + // codersdk.ExperimentsAllWildcard cannot include invalid experiments + if exp == codersdk.ExperimentsAllWildcard && e.Invalid { + continue + } + + enabled = append(enabled, true) + } + } + } + + return len(enabled) == validCount+len(tc.enabledInvalid) + }, "enabled experiment(s) were either not found or not marked as enabled") + require.Conditionf(t, func() (success bool) { + var invalid []bool + for _, exp := range tc.enabledInvalid { + for _, e := range experiments { + if e.Name == exp && e.Invalid { + invalid = append(invalid, true) + } + } + } + + return len(invalid) == len(tc.enabledInvalid) + }, "invalid experiment(s) were either not found or not marked as invalid") + }) + } + }) } diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 34eaa4edd4c40..814fc4281a19b 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -2178,6 +2178,12 @@ func (c *Client) BuildInfo(ctx context.Context) (BuildInfoResponse, error) { type Experiment string +type ExperimentDetail struct { + Name Experiment `json:"name"` + Enabled bool `json:"enabled"` + Invalid bool `json:"invalid"` +} + const ( // Add new experiments here! ExperimentExample Experiment = "example" // This isn't used for anything. @@ -2193,6 +2199,8 @@ var ExperimentsAll = Experiments{ ExperimentSharedPorts, } +const ExperimentsAllWildcard = "*" + // Experiments is a list of experiments. // Multiple experiments may be enabled at the same time. // Experiments are not safe for production use, and are not guaranteed to @@ -2209,6 +2217,39 @@ func (e Experiments) Enabled(ex Experiment) bool { return false } +// ExperimentDetails returns a list of all experiments, including those enabled via configuration options, and returns +// details for each experiment about whether they are active or invalid. +// Unknown experiments are indistinguishable from removed experiments, so we treat them the same way (as "invalid"). +func ExperimentDetails(exps Experiments) []ExperimentDetail { + invalid := make(map[Experiment]bool, len(exps)) + enabled := make(map[Experiment]struct{}, len(exps)) + + // ExperimentsAll gives us all safe experiments, which we mark as not invalid + for _, e := range ExperimentsAll { + invalid[e] = false + } + + // given experiments might not be included in the list of safe experiments, and are therefore marked invalid + for _, e := range exps { + enabled[e] = struct{}{} + + if _, found := invalid[e]; found { + // already known to be safe, can be skipped + continue + } + + invalid[e] = true + } + + out := make([]ExperimentDetail, 0, len(invalid)) + for e, expired := range invalid { + _, found := enabled[e] + out = append(out, ExperimentDetail{Name: e, Invalid: expired, Enabled: found}) + } + + return out +} + func (c *Client) Experiments(ctx context.Context) (Experiments, error) { res, err := c.Request(ctx, http.MethodGet, "/api/v2/experiments", nil) if err != nil { @@ -2241,6 +2282,20 @@ func (c *Client) SafeExperiments(ctx context.Context) (AvailableExperiments, err return exp, json.NewDecoder(res.Body).Decode(&exp) } +func (c *Client) ExperimentDetails(ctx context.Context) ([]ExperimentDetail, error) { + var exp []ExperimentDetail + + res, err := c.Request(ctx, http.MethodGet, "/api/v2/experiments/detail", nil) + if err != nil { + return exp, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return exp, ReadBodyAsError(res) + } + return exp, json.NewDecoder(res.Body).Decode(&exp) +} + type DAUsResponse struct { Entries []DAUEntry `json:"entries"` TZHourOffset int `json:"tz_hour_offset"` From c65406ee77bffffb2536a131b45fac4038a727c8 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 15 Apr 2024 15:50:39 +0200 Subject: [PATCH 02/13] make site/src/api/typesGenerated.ts Signed-off-by: Danny Kopping --- site/src/api/api.ts | 13 +++++++++++++ site/src/api/queries/experiments.ts | 7 +++++++ site/src/api/typesGenerated.ts | 9 ++++++++- 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 12c2a63b2c014..3e93ee0c027a3 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -949,6 +949,19 @@ export const getAvailableExperiments = } }; +export const getExperimentsDetail = + async (): Promise => { + try { + const response = await axios.get("/api/v2/experiments/detail"); + return response.data; + } catch (error) { + if (axios.isAxiosError(error) && error.response?.status === 404) { + return []; + } + throw error; + } + }; + export const getExternalAuthProvider = async ( provider: string, ): Promise => { diff --git a/site/src/api/queries/experiments.ts b/site/src/api/queries/experiments.ts index 1f7e04901ae59..9b8d709fa0074 100644 --- a/site/src/api/queries/experiments.ts +++ b/site/src/api/queries/experiments.ts @@ -20,3 +20,10 @@ export const availableExperiments = () => { queryFn: async () => API.getAvailableExperiments(), }; }; + +export const experimentsDetail = () => { + return { + queryKey: ["experimentsDetail"], + queryFn: async () => API.getExperimentsDetail(), + }; +}; diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 54f4e57d775eb..7e6dd4549df52 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -462,7 +462,14 @@ export interface Entitlements { } // From codersdk/deployment.go -export type Experiments = readonly Experiment[]; +export interface ExperimentDetail { + readonly name: Experiment; + readonly enabled: boolean; + readonly invalid: boolean; +} + +// From codersdk/deployment.go +export type Experiments = Experiment[]; // From codersdk/externalauth.go export interface ExternalAuth { From 31d5d6062d6c52318189abc51d13708a9c6ba272 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 15 Apr 2024 15:51:23 +0200 Subject: [PATCH 03/13] Render invalid experiments warning Signed-off-by: Danny Kopping --- site/src/api/api.ts | 23 ++++++------ .../GeneralSettingsPage.tsx | 6 +-- .../GeneralSettingsPageView.stories.tsx | 4 +- .../GeneralSettingsPageView.tsx | 37 +++++++++++++++++-- .../pages/DeploySettingsPage/optionValue.ts | 2 +- 5 files changed, 51 insertions(+), 21 deletions(-) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 3e93ee0c027a3..d1359d99dd823 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -949,18 +949,19 @@ export const getAvailableExperiments = } }; -export const getExperimentsDetail = - async (): Promise => { - try { - const response = await axios.get("/api/v2/experiments/detail"); - return response.data; - } catch (error) { - if (axios.isAxiosError(error) && error.response?.status === 404) { - return []; - } - throw error; +export const getExperimentsDetail = async (): Promise< + TypesGen.ExperimentDetail[] +> => { + try { + const response = await axios.get("/api/v2/experiments/detail"); + return response.data; + } catch (error) { + if (axios.isAxiosError(error) && error.response?.status === 404) { + return []; } - }; + throw error; + } +}; export const getExternalAuthProvider = async ( provider: string, diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx index 4f9f627a0b2dc..25b4eafc47477 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx @@ -3,7 +3,7 @@ import { Helmet } from "react-helmet-async"; import { useQuery } from "react-query"; import { deploymentDAUs } from "api/queries/deployment"; import { entitlements } from "api/queries/entitlements"; -import { availableExperiments } from "api/queries/experiments"; +import { experimentsDetail } from "api/queries/experiments"; import { pageTitle } from "utils/page"; import { useDeploySettings } from "../DeploySettingsLayout"; import { GeneralSettingsPageView } from "./GeneralSettingsPageView"; @@ -12,7 +12,7 @@ const GeneralSettingsPage: FC = () => { const { deploymentValues } = useDeploySettings(); const deploymentDAUsQuery = useQuery(deploymentDAUs()); const entitlementsQuery = useQuery(entitlements()); - const experimentsQuery = useQuery(availableExperiments()); + const experimentsQuery = useQuery(experimentsDetail()); return ( <> @@ -24,7 +24,7 @@ const GeneralSettingsPage: FC = () => { deploymentDAUs={deploymentDAUsQuery.data} deploymentDAUsError={deploymentDAUsQuery.error} entitlements={entitlementsQuery.data} - safeExperiments={experimentsQuery.data?.safe ?? []} + experiments={experimentsQuery.data} /> ); diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx index c945ef9381f87..5baccc55369a7 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx @@ -40,7 +40,7 @@ const meta: Meta = { }, ], deploymentDAUs: MockDeploymentDAUResponse, - safeExperiments: [], + experiments: [], }, }; @@ -102,6 +102,6 @@ export const allExperimentsEnabled: Story = { hidden: false, }, ], - safeExperiments: [], + experiments: [], }, }; diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx index 612a4f280ad89..e6185f70263f3 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx @@ -1,9 +1,10 @@ +import AlertTitle from "@mui/material/AlertTitle"; import type { FC } from "react"; import type { SerpentOption, DAUsResponse, Entitlements, - Experiments, + ExperimentDetail, } from "api/typesGenerated"; import { ActiveUserChart, @@ -13,6 +14,7 @@ import { ErrorAlert } from "components/Alert/ErrorAlert"; import { Stack } from "components/Stack/Stack"; import { useDeploymentOptions } from "utils/deployOptions"; import { docs } from "utils/docs"; +import { Alert } from "../../../components/Alert/Alert"; import { Header } from "../Header"; import OptionsTable from "../OptionsTable"; import { ChartSection } from "./ChartSection"; @@ -22,7 +24,7 @@ export type GeneralSettingsPageViewProps = { deploymentDAUs?: DAUsResponse; deploymentDAUsError: unknown; entitlements: Entitlements | undefined; - safeExperiments: Experiments | undefined; + experiments?: ExperimentDetail[]; }; export const GeneralSettingsPageView: FC = ({ @@ -30,8 +32,21 @@ export const GeneralSettingsPageView: FC = ({ deploymentDAUs, deploymentDAUsError, entitlements, - safeExperiments, + experiments, }) => { + const safe: string[] = []; + const invalid: string[] = []; + + if (experiments) { + experiments?.forEach(function (value) { + if (value.invalid) { + invalid.push(value.name); + } else { + safe.push(value.name); + } + }); + } + return ( <>
= ({ )} + {invalid.length > 0 && ( + + Invalid experiments in use: +
    + {invalid.map((it) => ( +
  • + 
    {it}
    +
    • + ))} +
+ It is recommended that you remove these experiments from your + configuration as they have no effect. + + )} = ({ "Wildcard Access URL", "Experiments", )} - additionalValues={safeExperiments} + additionalValues={safe} /> diff --git a/site/src/pages/DeploySettingsPage/optionValue.ts b/site/src/pages/DeploySettingsPage/optionValue.ts index 75435a894c12c..f429fee5212c3 100644 --- a/site/src/pages/DeploySettingsPage/optionValue.ts +++ b/site/src/pages/DeploySettingsPage/optionValue.ts @@ -41,7 +41,7 @@ export function optionValue( const experimentMap: Record | undefined = additionalValues?.reduce( (acc, v) => { - return { ...acc, [v]: option.value.includes("*") ? true : false }; + return { ...acc, [v]: option.value.includes("*") }; }, {} as Record, ); From bd10174fcf9b92a708701ba97b21cd6168b1ea9d Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 15 Apr 2024 16:21:18 +0200 Subject: [PATCH 04/13] make gen Signed-off-by: Danny Kopping --- coderd/apidoc/docs.go | 42 +++++++++++++++++++++++++++++ coderd/apidoc/swagger.json | 38 +++++++++++++++++++++++++++ docs/api/general.md | 54 ++++++++++++++++++++++++++++++++++++++ docs/api/schemas.md | 18 +++++++++++++ 4 files changed, 152 insertions(+) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index bcd2b3b15ccd6..5aa0d7cf8bb1a 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -809,6 +809,34 @@ const docTemplate = `{ } } }, + "/experiments/detail": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "General" + ], + "summary": "Get experiments' details", + "operationId": "get-experiments-details", + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.ExperimentDetail" + } + } + } + } + } + }, "/external-auth": { "get": { "security": [ @@ -9520,6 +9548,20 @@ const docTemplate = `{ "ExperimentAutoFillParameters" ] }, + "codersdk.ExperimentDetail": { + "type": "object", + "properties": { + "enabled": { + "type": "boolean" + }, + "invalid": { + "type": "boolean" + }, + "name": { + "$ref": "#/definitions/codersdk.Experiment" + } + } + }, "codersdk.ExternalAuth": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 47bac4fc4ecab..04d4f4e38bf21 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -691,6 +691,30 @@ } } }, + "/experiments/detail": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["General"], + "summary": "Get experiments' details", + "operationId": "get-experiments-details", + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.ExperimentDetail" + } + } + } + } + } + }, "/external-auth": { "get": { "security": [ @@ -8517,6 +8541,20 @@ "ExperimentAutoFillParameters" ] }, + "codersdk.ExperimentDetail": { + "type": "object", + "properties": { + "enabled": { + "type": "boolean" + }, + "invalid": { + "type": "boolean" + }, + "name": { + "$ref": "#/definitions/codersdk.Experiment" + } + } + }, "codersdk.ExternalAuth": { "type": "object", "properties": { diff --git a/docs/api/general.md b/docs/api/general.md index 330c41a335b9b..1a319aa8ae6da 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -614,6 +614,60 @@ Status Code **200** To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Get experiments' details + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/experiments/detail \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /experiments/detail` + +### Example responses + +> 200 Response + +```json +[ + { + "enabled": true, + "invalid": true, + "name": "example" + } +] +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | ------------------------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | array of [codersdk.ExperimentDetail](schemas.md#codersdkexperimentdetail) | + +

Response Schema

+ +Status Code **200** + +| Name | Type | Required | Restrictions | Description | +| -------------- | ---------------------------------------------------- | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» enabled` | boolean | false | | | +| `» invalid` | boolean | false | | | +| `» name` | [codersdk.Experiment](schemas.md#codersdkexperiment) | false | | | + +#### Enumerated Values + +| Property | Value | +| -------- | ---------------------- | +| `name` | `example` | +| `name` | `shared-ports` | +| `name` | `auto-fill-parameters` | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Update check ### Code samples diff --git a/docs/api/schemas.md b/docs/api/schemas.md index efc3a38f01219..99ef16e6818ce 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -2662,6 +2662,24 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `shared-ports` | | `auto-fill-parameters` | +## codersdk.ExperimentDetail + +```json +{ + "enabled": true, + "invalid": true, + "name": "example" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| --------- | ------------------------------------------ | -------- | ------------ | ----------- | +| `enabled` | boolean | false | | | +| `invalid` | boolean | false | | | +| `name` | [codersdk.Experiment](#codersdkexperiment) | false | | | + ## codersdk.ExternalAuth ```json From ead1d7e2ad4faa975b8fcc56db6c87fccaa521de Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 15 Apr 2024 16:26:58 +0200 Subject: [PATCH 05/13] make site/src/api/typesGenerated.ts Signed-off-by: Danny Kopping --- site/src/api/typesGenerated.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 7e6dd4549df52..339117aafbc26 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -469,7 +469,7 @@ export interface ExperimentDetail { } // From codersdk/deployment.go -export type Experiments = Experiment[]; +export type Experiments = readonly Experiment[]; // From codersdk/externalauth.go export interface ExternalAuth { From 5e5f2ac5199fd8e44a76265388dcc77517a763bd Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 16 Apr 2024 10:28:02 +0200 Subject: [PATCH 06/13] Reset all backend changes Signed-off-by: Danny Kopping --- coderd/apidoc/docs.go | 42 ----------- coderd/apidoc/swagger.json | 38 ---------- coderd/coderd.go | 3 +- coderd/experiments.go | 12 ---- coderd/experiments_test.go | 142 +------------------------------------ codersdk/deployment.go | 55 -------------- docs/api/general.md | 54 -------------- docs/api/schemas.md | 18 ----- 8 files changed, 4 insertions(+), 360 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 5aa0d7cf8bb1a..bcd2b3b15ccd6 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -809,34 +809,6 @@ const docTemplate = `{ } } }, - "/experiments/detail": { - "get": { - "security": [ - { - "CoderSessionToken": [] - } - ], - "produces": [ - "application/json" - ], - "tags": [ - "General" - ], - "summary": "Get experiments' details", - "operationId": "get-experiments-details", - "responses": { - "200": { - "description": "OK", - "schema": { - "type": "array", - "items": { - "$ref": "#/definitions/codersdk.ExperimentDetail" - } - } - } - } - } - }, "/external-auth": { "get": { "security": [ @@ -9548,20 +9520,6 @@ const docTemplate = `{ "ExperimentAutoFillParameters" ] }, - "codersdk.ExperimentDetail": { - "type": "object", - "properties": { - "enabled": { - "type": "boolean" - }, - "invalid": { - "type": "boolean" - }, - "name": { - "$ref": "#/definitions/codersdk.Experiment" - } - } - }, "codersdk.ExternalAuth": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 04d4f4e38bf21..47bac4fc4ecab 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -691,30 +691,6 @@ } } }, - "/experiments/detail": { - "get": { - "security": [ - { - "CoderSessionToken": [] - } - ], - "produces": ["application/json"], - "tags": ["General"], - "summary": "Get experiments' details", - "operationId": "get-experiments-details", - "responses": { - "200": { - "description": "OK", - "schema": { - "type": "array", - "items": { - "$ref": "#/definitions/codersdk.ExperimentDetail" - } - } - } - } - } - }, "/external-auth": { "get": { "security": [ @@ -8541,20 +8517,6 @@ "ExperimentAutoFillParameters" ] }, - "codersdk.ExperimentDetail": { - "type": "object", - "properties": { - "enabled": { - "type": "boolean" - }, - "invalid": { - "type": "boolean" - }, - "name": { - "$ref": "#/definitions/codersdk.Experiment" - } - } - }, "codersdk.ExternalAuth": { "type": "object", "properties": { diff --git a/coderd/coderd.go b/coderd/coderd.go index 576b43e14f9a6..67b16e9032bfe 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -754,7 +754,6 @@ func New(options *Options) *API { r.Route("/experiments", func(r chi.Router) { r.Use(apiKeyMiddleware) r.Get("/available", handleExperimentsSafe) - r.Get("/detail", api.handleExperimentsDetail) r.Get("/", api.handleExperimentsGet) }) r.Get("/updatecheck", api.updateCheck) @@ -1456,7 +1455,7 @@ func ReadExperiments(log slog.Logger, raw []string) codersdk.Experiments { exps := make([]codersdk.Experiment, 0, len(raw)) for _, v := range raw { switch v { - case codersdk.ExperimentsAllWildcard: + case "*": exps = append(exps, codersdk.ExperimentsAll...) default: ex := codersdk.Experiment(strings.ToLower(v)) diff --git a/coderd/experiments.go b/coderd/experiments.go index af663292a73c2..f7debd8c68bbb 100644 --- a/coderd/experiments.go +++ b/coderd/experiments.go @@ -32,15 +32,3 @@ func handleExperimentsSafe(rw http.ResponseWriter, r *http.Request) { Safe: codersdk.ExperimentsAll, }) } - -// @Summary Get experiments' details -// @ID get-experiments-details -// @Security CoderSessionToken -// @Produce json -// @Tags General -// @Success 200 {array} codersdk.ExperimentDetail -// @Router /experiments/detail [get] -func (api *API) handleExperimentsDetail(rw http.ResponseWriter, r *http.Request) { - ctx := r.Context() - httpapi.Write(ctx, rw, http.StatusOK, codersdk.ExperimentDetails(api.Experiments)) -} diff --git a/coderd/experiments_test.go b/coderd/experiments_test.go index 6f15f7f5f4608..4288b9953fec6 100644 --- a/coderd/experiments_test.go +++ b/coderd/experiments_test.go @@ -57,7 +57,7 @@ func Test_Experiments(t *testing.T) { t.Run("wildcard", func(t *testing.T) { t.Parallel() cfg := coderdtest.DeploymentValues(t) - cfg.Experiments = []string{codersdk.ExperimentsAllWildcard} + cfg.Experiments = []string{"*"} client := coderdtest.New(t, &coderdtest.Options{ DeploymentValues: cfg, }) @@ -79,7 +79,7 @@ func Test_Experiments(t *testing.T) { t.Run("alternate wildcard with manual opt-in", func(t *testing.T) { t.Parallel() cfg := coderdtest.DeploymentValues(t) - cfg.Experiments = []string{codersdk.ExperimentsAllWildcard, "dAnGeR"} + cfg.Experiments = []string{"*", "dAnGeR"} client := coderdtest.New(t, &coderdtest.Options{ DeploymentValues: cfg, }) @@ -102,7 +102,7 @@ func Test_Experiments(t *testing.T) { t.Run("Unauthorized", func(t *testing.T) { t.Parallel() cfg := coderdtest.DeploymentValues(t) - cfg.Experiments = []string{codersdk.ExperimentsAllWildcard} + cfg.Experiments = []string{"*"} client := coderdtest.New(t, &coderdtest.Options{ DeploymentValues: cfg, }) @@ -133,140 +133,4 @@ func Test_Experiments(t *testing.T) { require.NotNil(t, experiments) require.ElementsMatch(t, codersdk.ExperimentsAll, experiments.Safe) }) - - t.Run("experiments detail", func(t *testing.T) { - t.Parallel() - - const ( - invalidExp = "bob" - expiredExp = "auto-fill-parameters" // using a string here not a constant since this experiment has expired & will be deleted eventually - ) - - tests := []struct { - name string - enabledValid []codersdk.Experiment - enabledInvalid []codersdk.Experiment - expectedExtraCount int - }{ - { - name: "using defaults", - }, - { - name: "use all (*)", - enabledValid: []codersdk.Experiment{codersdk.Experiment(codersdk.ExperimentsAllWildcard)}, - }, - { - name: "only valid experiments", - enabledValid: codersdk.ExperimentsAll, - }, - { - name: "use all (*) + invalid", - enabledValid: []codersdk.Experiment{codersdk.Experiment(codersdk.ExperimentsAllWildcard), codersdk.Experiment(expiredExp)}, - expectedExtraCount: 1, - }, - { - name: "valid + expired experiments", - enabledValid: codersdk.ExperimentsAll, - enabledInvalid: []codersdk.Experiment{codersdk.Experiment(expiredExp)}, - expectedExtraCount: 1, - }, - { - name: "valid + expired + invalid experiments", - enabledValid: codersdk.ExperimentsAll, - enabledInvalid: []codersdk.Experiment{codersdk.Experiment(invalidExp), codersdk.Experiment(expiredExp)}, - expectedExtraCount: 2, - }, - { - name: "only expired", - enabledInvalid: []codersdk.Experiment{codersdk.Experiment(expiredExp)}, - expectedExtraCount: 1, - }, - { - name: "only invalid", - enabledInvalid: []codersdk.Experiment{codersdk.Experiment(invalidExp)}, - expectedExtraCount: 1, - }, - { - name: "expired + invalid experiments", - enabledInvalid: []codersdk.Experiment{codersdk.Experiment(invalidExp), codersdk.Experiment(expiredExp)}, - expectedExtraCount: 2, - }, - } - - for _, tc := range tests { - tc := tc - - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - var exps []string - - // given - for _, e := range tc.enabledValid { - exps = append(exps, string(e)) - } - for _, e := range tc.enabledInvalid { - exps = append(exps, string(e)) - } - - cfg := coderdtest.DeploymentValues(t) - cfg.Experiments = exps - client := coderdtest.New(t, &coderdtest.Options{ - DeploymentValues: cfg, - }) - _ = coderdtest.CreateFirstUser(t, client) - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - // when - experiments, err := client.ExperimentDetails(ctx) - - // then - require.NoError(t, err) - require.Len(t, experiments, len(codersdk.ExperimentsAll)+tc.expectedExtraCount) - require.Conditionf(t, func() (success bool) { - var enabled []bool - - var validCount int - for _, exp := range tc.enabledValid { - // don't count wildcard experiment itself as a single experiment - if exp == codersdk.ExperimentsAllWildcard { - validCount += len(codersdk.ExperimentsAll) - } else { - validCount++ - } - } - - for _, exp := range append(tc.enabledValid, tc.enabledInvalid...) { - for _, e := range experiments { - // * is special-cased to mean all experiments - if (exp == codersdk.ExperimentsAllWildcard || e.Name == exp) && e.Enabled { - // codersdk.ExperimentsAllWildcard cannot include invalid experiments - if exp == codersdk.ExperimentsAllWildcard && e.Invalid { - continue - } - - enabled = append(enabled, true) - } - } - } - - return len(enabled) == validCount+len(tc.enabledInvalid) - }, "enabled experiment(s) were either not found or not marked as enabled") - require.Conditionf(t, func() (success bool) { - var invalid []bool - for _, exp := range tc.enabledInvalid { - for _, e := range experiments { - if e.Name == exp && e.Invalid { - invalid = append(invalid, true) - } - } - } - - return len(invalid) == len(tc.enabledInvalid) - }, "invalid experiment(s) were either not found or not marked as invalid") - }) - } - }) } diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 814fc4281a19b..34eaa4edd4c40 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -2178,12 +2178,6 @@ func (c *Client) BuildInfo(ctx context.Context) (BuildInfoResponse, error) { type Experiment string -type ExperimentDetail struct { - Name Experiment `json:"name"` - Enabled bool `json:"enabled"` - Invalid bool `json:"invalid"` -} - const ( // Add new experiments here! ExperimentExample Experiment = "example" // This isn't used for anything. @@ -2199,8 +2193,6 @@ var ExperimentsAll = Experiments{ ExperimentSharedPorts, } -const ExperimentsAllWildcard = "*" - // Experiments is a list of experiments. // Multiple experiments may be enabled at the same time. // Experiments are not safe for production use, and are not guaranteed to @@ -2217,39 +2209,6 @@ func (e Experiments) Enabled(ex Experiment) bool { return false } -// ExperimentDetails returns a list of all experiments, including those enabled via configuration options, and returns -// details for each experiment about whether they are active or invalid. -// Unknown experiments are indistinguishable from removed experiments, so we treat them the same way (as "invalid"). -func ExperimentDetails(exps Experiments) []ExperimentDetail { - invalid := make(map[Experiment]bool, len(exps)) - enabled := make(map[Experiment]struct{}, len(exps)) - - // ExperimentsAll gives us all safe experiments, which we mark as not invalid - for _, e := range ExperimentsAll { - invalid[e] = false - } - - // given experiments might not be included in the list of safe experiments, and are therefore marked invalid - for _, e := range exps { - enabled[e] = struct{}{} - - if _, found := invalid[e]; found { - // already known to be safe, can be skipped - continue - } - - invalid[e] = true - } - - out := make([]ExperimentDetail, 0, len(invalid)) - for e, expired := range invalid { - _, found := enabled[e] - out = append(out, ExperimentDetail{Name: e, Invalid: expired, Enabled: found}) - } - - return out -} - func (c *Client) Experiments(ctx context.Context) (Experiments, error) { res, err := c.Request(ctx, http.MethodGet, "/api/v2/experiments", nil) if err != nil { @@ -2282,20 +2241,6 @@ func (c *Client) SafeExperiments(ctx context.Context) (AvailableExperiments, err return exp, json.NewDecoder(res.Body).Decode(&exp) } -func (c *Client) ExperimentDetails(ctx context.Context) ([]ExperimentDetail, error) { - var exp []ExperimentDetail - - res, err := c.Request(ctx, http.MethodGet, "/api/v2/experiments/detail", nil) - if err != nil { - return exp, err - } - defer res.Body.Close() - if res.StatusCode != http.StatusOK { - return exp, ReadBodyAsError(res) - } - return exp, json.NewDecoder(res.Body).Decode(&exp) -} - type DAUsResponse struct { Entries []DAUEntry `json:"entries"` TZHourOffset int `json:"tz_hour_offset"` diff --git a/docs/api/general.md b/docs/api/general.md index 1a319aa8ae6da..330c41a335b9b 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -614,60 +614,6 @@ Status Code **200** To perform this operation, you must be authenticated. [Learn more](authentication.md). -## Get experiments' details - -### Code samples - -```shell -# Example request using curl -curl -X GET http://coder-server:8080/api/v2/experiments/detail \ - -H 'Accept: application/json' \ - -H 'Coder-Session-Token: API_KEY' -``` - -`GET /experiments/detail` - -### Example responses - -> 200 Response - -```json -[ - { - "enabled": true, - "invalid": true, - "name": "example" - } -] -``` - -### Responses - -| Status | Meaning | Description | Schema | -| ------ | ------------------------------------------------------- | ----------- | ------------------------------------------------------------------------- | -| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | array of [codersdk.ExperimentDetail](schemas.md#codersdkexperimentdetail) | - -

Response Schema

- -Status Code **200** - -| Name | Type | Required | Restrictions | Description | -| -------------- | ---------------------------------------------------- | -------- | ------------ | ----------- | -| `[array item]` | array | false | | | -| `» enabled` | boolean | false | | | -| `» invalid` | boolean | false | | | -| `» name` | [codersdk.Experiment](schemas.md#codersdkexperiment) | false | | | - -#### Enumerated Values - -| Property | Value | -| -------- | ---------------------- | -| `name` | `example` | -| `name` | `shared-ports` | -| `name` | `auto-fill-parameters` | - -To perform this operation, you must be authenticated. [Learn more](authentication.md). - ## Update check ### Code samples diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 99ef16e6818ce..efc3a38f01219 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -2662,24 +2662,6 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `shared-ports` | | `auto-fill-parameters` | -## codersdk.ExperimentDetail - -```json -{ - "enabled": true, - "invalid": true, - "name": "example" -} -``` - -### Properties - -| Name | Type | Required | Restrictions | Description | -| --------- | ------------------------------------------ | -------- | ------------ | ----------- | -| `enabled` | boolean | false | | | -| `invalid` | boolean | false | | | -| `name` | [codersdk.Experiment](#codersdkexperiment) | false | | | - ## codersdk.ExternalAuth ```json From 54f2462084cdc647b90c45ac9476da854546cc61 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 16 Apr 2024 12:01:39 +0200 Subject: [PATCH 07/13] Moving towards frontend-only solution Replaced green circle with more neutral icon to indicate enabled experiments Signed-off-by: Danny Kopping --- site/src/api/api.ts | 14 -------- site/src/api/queries/experiments.ts | 7 ---- site/src/api/typesGenerated.ts | 7 ---- .../GeneralSettingsPage.tsx | 26 ++++++++------- .../GeneralSettingsPageView.stories.tsx | 5 +-- .../GeneralSettingsPageView.tsx | 32 ++++++++++++------- site/src/pages/DeploySettingsPage/Option.tsx | 6 ++-- 7 files changed, 40 insertions(+), 57 deletions(-) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index d1359d99dd823..12c2a63b2c014 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -949,20 +949,6 @@ export const getAvailableExperiments = } }; -export const getExperimentsDetail = async (): Promise< - TypesGen.ExperimentDetail[] -> => { - try { - const response = await axios.get("/api/v2/experiments/detail"); - return response.data; - } catch (error) { - if (axios.isAxiosError(error) && error.response?.status === 404) { - return []; - } - throw error; - } -}; - export const getExternalAuthProvider = async ( provider: string, ): Promise => { diff --git a/site/src/api/queries/experiments.ts b/site/src/api/queries/experiments.ts index 9b8d709fa0074..1f7e04901ae59 100644 --- a/site/src/api/queries/experiments.ts +++ b/site/src/api/queries/experiments.ts @@ -20,10 +20,3 @@ export const availableExperiments = () => { queryFn: async () => API.getAvailableExperiments(), }; }; - -export const experimentsDetail = () => { - return { - queryKey: ["experimentsDetail"], - queryFn: async () => API.getExperimentsDetail(), - }; -}; diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 339117aafbc26..54f4e57d775eb 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -461,13 +461,6 @@ export interface Entitlements { readonly refreshed_at: string; } -// From codersdk/deployment.go -export interface ExperimentDetail { - readonly name: Experiment; - readonly enabled: boolean; - readonly invalid: boolean; -} - // From codersdk/deployment.go export type Experiments = readonly Experiment[]; diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx index 25b4eafc47477..672ed84c35f7b 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx @@ -1,18 +1,19 @@ -import type { FC } from "react"; -import { Helmet } from "react-helmet-async"; -import { useQuery } from "react-query"; -import { deploymentDAUs } from "api/queries/deployment"; -import { entitlements } from "api/queries/entitlements"; -import { experimentsDetail } from "api/queries/experiments"; -import { pageTitle } from "utils/page"; -import { useDeploySettings } from "../DeploySettingsLayout"; -import { GeneralSettingsPageView } from "./GeneralSettingsPageView"; +import type {FC} from "react"; +import {Helmet} from "react-helmet-async"; +import {useQuery} from "react-query"; +import {deploymentDAUs} from "api/queries/deployment"; +import {entitlements} from "api/queries/entitlements"; +import {availableExperiments, experiments} from "api/queries/experiments"; +import {pageTitle} from "utils/page"; +import {useDeploySettings} from "../DeploySettingsLayout"; +import {GeneralSettingsPageView} from "./GeneralSettingsPageView"; const GeneralSettingsPage: FC = () => { - const { deploymentValues } = useDeploySettings(); + const {deploymentValues} = useDeploySettings(); const deploymentDAUsQuery = useQuery(deploymentDAUs()); const entitlementsQuery = useQuery(entitlements()); - const experimentsQuery = useQuery(experimentsDetail()); + const enabledExperimentsQuery = useQuery(experiments()); + const availableExperimentsQuery = useQuery(availableExperiments()); return ( <> @@ -24,7 +25,8 @@ const GeneralSettingsPage: FC = () => { deploymentDAUs={deploymentDAUsQuery.data} deploymentDAUsError={deploymentDAUsQuery.error} entitlements={entitlementsQuery.data} - experiments={experimentsQuery.data} + enabledExperiments={enabledExperimentsQuery.data ?? []} + safeExperiments={availableExperimentsQuery.data?.safe ?? []} /> ); diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx index 5baccc55369a7..bd2c29a0c6c93 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx @@ -40,7 +40,8 @@ const meta: Meta = { }, ], deploymentDAUs: MockDeploymentDAUResponse, - experiments: [], + enabledExperiments: [], + safeExperiments: [], }, }; @@ -102,6 +103,6 @@ export const allExperimentsEnabled: Story = { hidden: false, }, ], - experiments: [], + safeExperiments: [], }, }; diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx index e6185f70263f3..9eb99cfd69e0a 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx @@ -4,7 +4,7 @@ import type { SerpentOption, DAUsResponse, Entitlements, - ExperimentDetail, + Experiments, } from "api/typesGenerated"; import { ActiveUserChart, @@ -24,7 +24,8 @@ export type GeneralSettingsPageViewProps = { deploymentDAUs?: DAUsResponse; deploymentDAUsError: unknown; entitlements: Entitlements | undefined; - experiments?: ExperimentDetail[]; + enabledExperiments: Experiments; + safeExperiments: Experiments; }; export const GeneralSettingsPageView: FC = ({ @@ -32,20 +33,27 @@ export const GeneralSettingsPageView: FC = ({ deploymentDAUs, deploymentDAUsError, entitlements, - experiments, + enabledExperiments, + safeExperiments, }) => { const safe: string[] = []; const invalid: string[] = []; - if (experiments) { - experiments?.forEach(function (value) { - if (value.invalid) { - invalid.push(value.name); - } else { - safe.push(value.name); + enabledExperiments.forEach(function (exp) { + let found = false; + safeExperiments.forEach(function (name) { + if (exp === name) { + found = true; + return; } - }); - } + }) + + if (found) { + safe.push(exp); + } else { + invalid.push(exp); + } + }); return ( <> @@ -84,7 +92,7 @@ export const GeneralSettingsPageView: FC = ({ ))} It is recommended that you remove these experiments from your - configuration as they have no effect. + configuration as they have no effect. See the documentation for more details. )} = (props) => { }} > {isEnabled && ( - ({ width: 16, height: 16, - color: theme.palette.success.light, + color: theme.palette.info.light, margin: "0 8px", })} /> From acb0d039b20ab821fc7bda72b6d7a20ffeb509bf Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 16 Apr 2024 12:06:29 +0200 Subject: [PATCH 08/13] Add storybook test Signed-off-by: Danny Kopping --- .../GeneralSettingsPageView.stories.tsx | 40 ++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx index bd2c29a0c6c93..60198e0e2b617 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx @@ -5,6 +5,7 @@ import { MockEntitlementsWithUserLimit, } from "testHelpers/entities"; import { GeneralSettingsPageView } from "./GeneralSettingsPageView"; +import {Experiments} from "../../../api/typesGenerated"; const meta: Meta = { title: "pages/DeploySettingsPage/GeneralSettingsPageView", @@ -103,6 +104,43 @@ export const allExperimentsEnabled: Story = { hidden: false, }, ], - safeExperiments: [], + safeExperiments: ["shared-ports"], + enabledExperiments: ["shared-ports"], + }, +}; + +export const invalidExperimentsEnabled: Story = { + args: { + deploymentOptions: [ + { + name: "Access URL", + description: + "The URL that users will use to access the Coder deployment.", + flag: "access-url", + flag_shorthand: "", + value: "https://dev.coder.com", + hidden: false, + }, + { + name: "Wildcard Access URL", + description: + 'Specifies the wildcard hostname to use for workspace applications in the form "*.example.com".', + flag: "wildcard-access-url", + flag_shorthand: "", + value: "*--apps.dev.coder.com", + hidden: false, + }, + { + name: "Experiments", + description: + "Enable one or more experiments. These are not ready for production. Separate multiple experiments with commas, or enter '*' to opt-in to all available experiments.", + flag: "experiments", + value: ["invalid", "*"], + flag_shorthand: "", + hidden: false, + }, + ], + safeExperiments: ["shared-ports"], + enabledExperiments: ["invalid"], }, }; From 66f7ef46fd56196fba9dec4aae343ac61676e7da Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 16 Apr 2024 12:22:40 +0200 Subject: [PATCH 09/13] Appeasing the linter Signed-off-by: Danny Kopping --- .../GeneralSettingsPage.tsx | 20 +++++++++---------- .../GeneralSettingsPageView.stories.tsx | 1 - .../GeneralSettingsPageView.tsx | 16 +++++++++++---- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx index 672ed84c35f7b..af14faa295ff2 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx @@ -1,15 +1,15 @@ -import type {FC} from "react"; -import {Helmet} from "react-helmet-async"; -import {useQuery} from "react-query"; -import {deploymentDAUs} from "api/queries/deployment"; -import {entitlements} from "api/queries/entitlements"; -import {availableExperiments, experiments} from "api/queries/experiments"; -import {pageTitle} from "utils/page"; -import {useDeploySettings} from "../DeploySettingsLayout"; -import {GeneralSettingsPageView} from "./GeneralSettingsPageView"; +import type { FC } from "react"; +import { Helmet } from "react-helmet-async"; +import { useQuery } from "react-query"; +import { deploymentDAUs } from "api/queries/deployment"; +import { entitlements } from "api/queries/entitlements"; +import { availableExperiments, experiments } from "api/queries/experiments"; +import { pageTitle } from "utils/page"; +import { useDeploySettings } from "../DeploySettingsLayout"; +import { GeneralSettingsPageView } from "./GeneralSettingsPageView"; const GeneralSettingsPage: FC = () => { - const {deploymentValues} = useDeploySettings(); + const { deploymentValues } = useDeploySettings(); const deploymentDAUsQuery = useQuery(deploymentDAUs()); const entitlementsQuery = useQuery(entitlements()); const enabledExperimentsQuery = useQuery(experiments()); diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx index 60198e0e2b617..31be933f66a06 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx @@ -5,7 +5,6 @@ import { MockEntitlementsWithUserLimit, } from "testHelpers/entities"; import { GeneralSettingsPageView } from "./GeneralSettingsPageView"; -import {Experiments} from "../../../api/typesGenerated"; const meta: Meta = { title: "pages/DeploySettingsPage/GeneralSettingsPageView", diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx index 9eb99cfd69e0a..e54dc670a09f0 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx @@ -24,8 +24,8 @@ export type GeneralSettingsPageViewProps = { deploymentDAUs?: DAUsResponse; deploymentDAUsError: unknown; entitlements: Entitlements | undefined; - enabledExperiments: Experiments; - safeExperiments: Experiments; + enabledExperiments: Experiments | string[]; + safeExperiments: Experiments | string[]; }; export const GeneralSettingsPageView: FC = ({ @@ -46,7 +46,7 @@ export const GeneralSettingsPageView: FC = ({ found = true; return; } - }) + }); if (found) { safe.push(exp); @@ -92,7 +92,15 @@ export const GeneralSettingsPageView: FC = ({ ))} It is recommended that you remove these experiments from your - configuration as they have no effect. See the documentation for more details. + configuration as they have no effect. See{" "} + + the documentation + {" "} + for more details. )} Date: Wed, 17 Apr 2024 12:10:31 +0200 Subject: [PATCH 10/13] Review feedback Signed-off-by: Danny Kopping --- .../GeneralSettingsPage.tsx | 21 +++++++++++-- .../GeneralSettingsPageView.stories.tsx | 6 ++-- .../GeneralSettingsPageView.tsx | 31 ++++--------------- site/src/pages/DeploySettingsPage/Option.tsx | 6 ++-- .../pages/DeploySettingsPage/optionValue.ts | 14 ++++----- 5 files changed, 37 insertions(+), 41 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx index af14faa295ff2..63624fe3084a3 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx @@ -13,7 +13,22 @@ const GeneralSettingsPage: FC = () => { const deploymentDAUsQuery = useQuery(deploymentDAUs()); const entitlementsQuery = useQuery(entitlements()); const enabledExperimentsQuery = useQuery(experiments()); - const availableExperimentsQuery = useQuery(availableExperiments()); + const safeExperimentsQuery = useQuery(availableExperiments()); + + const safeExperiments: string[] = []; + const invalidExperiments: string[] = []; + + (enabledExperimentsQuery.data ?? []).forEach(function (exp) { + const found = (safeExperimentsQuery.data?.safe ?? []).find((value) => { + return exp === value; + }); + + if (found) { + safeExperiments.push(exp); + } else { + invalidExperiments.push(exp); + } + }); return ( <> @@ -25,8 +40,8 @@ const GeneralSettingsPage: FC = () => { deploymentDAUs={deploymentDAUsQuery.data} deploymentDAUsError={deploymentDAUsQuery.error} entitlements={entitlementsQuery.data} - enabledExperiments={enabledExperimentsQuery.data ?? []} - safeExperiments={availableExperimentsQuery.data?.safe ?? []} + invalidExperiments={invalidExperiments} + safeExperiments={safeExperiments} /> ); diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx index 31be933f66a06..a04270b9e3128 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.stories.tsx @@ -40,7 +40,7 @@ const meta: Meta = { }, ], deploymentDAUs: MockDeploymentDAUResponse, - enabledExperiments: [], + invalidExperiments: [], safeExperiments: [], }, }; @@ -104,7 +104,7 @@ export const allExperimentsEnabled: Story = { }, ], safeExperiments: ["shared-ports"], - enabledExperiments: ["shared-ports"], + invalidExperiments: ["invalid"], }, }; @@ -140,6 +140,6 @@ export const invalidExperimentsEnabled: Story = { }, ], safeExperiments: ["shared-ports"], - enabledExperiments: ["invalid"], + invalidExperiments: ["invalid"], }, }; diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx index e54dc670a09f0..fbbe4127c9889 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPageView.tsx @@ -24,8 +24,8 @@ export type GeneralSettingsPageViewProps = { deploymentDAUs?: DAUsResponse; deploymentDAUsError: unknown; entitlements: Entitlements | undefined; - enabledExperiments: Experiments | string[]; - safeExperiments: Experiments | string[]; + readonly invalidExperiments: Experiments | string[]; + readonly safeExperiments: Experiments | string[]; }; export const GeneralSettingsPageView: FC = ({ @@ -33,28 +33,9 @@ export const GeneralSettingsPageView: FC = ({ deploymentDAUs, deploymentDAUsError, entitlements, - enabledExperiments, safeExperiments, + invalidExperiments, }) => { - const safe: string[] = []; - const invalid: string[] = []; - - enabledExperiments.forEach(function (exp) { - let found = false; - safeExperiments.forEach(function (name) { - if (exp === name) { - found = true; - return; - } - }); - - if (found) { - safe.push(exp); - } else { - invalid.push(exp); - } - }); - return ( <>
= ({ )} - {invalid.length > 0 && ( + {invalidExperiments.length > 0 && ( Invalid experiments in use:
    - {invalid.map((it) => ( + {invalidExperiments.map((it) => ( 
  • {it}
    • @@ -110,7 +91,7 @@ export const GeneralSettingsPageView: FC = ({ "Wildcard Access URL", "Experiments", )} - additionalValues={safe} + additionalValues={safeExperiments} /> diff --git a/site/src/pages/DeploySettingsPage/Option.tsx b/site/src/pages/DeploySettingsPage/Option.tsx index 66ff2aa970d02..aac776219ef21 100644 --- a/site/src/pages/DeploySettingsPage/Option.tsx +++ b/site/src/pages/DeploySettingsPage/Option.tsx @@ -1,5 +1,5 @@ import { css, type Interpolation, type Theme, useTheme } from "@emotion/react"; -import LabelImportant from "@mui/icons-material/LabelImportant"; +import BuildCircleOutlinedIcon from "@mui/icons-material/BuildCircleOutlined"; import type { FC, HTMLAttributes, PropsWithChildren } from "react"; import { DisabledBadge, EnabledBadge } from "components/Badges/Badges"; import { MONOSPACE_FONT_FAMILY } from "theme/constants"; @@ -91,11 +91,11 @@ export const OptionValue: FC = (props) => { }} > {isEnabled && ( - ({ width: 16, height: 16, - color: theme.palette.info.light, + color: theme.palette.mode, margin: "0 8px", })} /> diff --git a/site/src/pages/DeploySettingsPage/optionValue.ts b/site/src/pages/DeploySettingsPage/optionValue.ts index f429fee5212c3..596a3333eb520 100644 --- a/site/src/pages/DeploySettingsPage/optionValue.ts +++ b/site/src/pages/DeploySettingsPage/optionValue.ts @@ -38,13 +38,13 @@ export function optionValue( ([key, value]) => `"${key}"->"${value}"`, ); case "Experiments": { - const experimentMap: Record | undefined = - additionalValues?.reduce( - (acc, v) => { - return { ...acc, [v]: option.value.includes("*") }; - }, - {} as Record, - ); + const experimentMap = additionalValues?.reduce>( + (acc, v) => { + acc[v] = option.value.includes("*"); + return acc; + }, + {}, + ); if (!experimentMap) { break; From 8836f25c152eb26c332d4061adf34a458a3e78cc Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 17 Apr 2024 12:20:45 +0200 Subject: [PATCH 11/13] Fixing e2e tests by displaying safe but unset experiments which was the previous behaviour Signed-off-by: Danny Kopping --- .../GeneralSettingsPage/GeneralSettingsPage.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx index 63624fe3084a3..80e0fa50086c0 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx @@ -18,14 +18,16 @@ const GeneralSettingsPage: FC = () => { const safeExperiments: string[] = []; const invalidExperiments: string[] = []; + (safeExperimentsQuery.data?.safe ?? []).forEach((value) => + safeExperiments.push(value), + ); + (enabledExperimentsQuery.data ?? []).forEach(function (exp) { const found = (safeExperimentsQuery.data?.safe ?? []).find((value) => { return exp === value; }); - if (found) { - safeExperiments.push(exp); - } else { + if (!found) { invalidExperiments.push(exp); } }); From a604f17fb7e202e2ea1fe4ddb413428516b01bdc Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 17 Apr 2024 16:36:22 +0200 Subject: [PATCH 12/13] Review comments Signed-off-by: Danny Kopping --- .../GeneralSettingsPage.tsx | 21 +++++-------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx index 80e0fa50086c0..63a63f48b6e8e 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx @@ -15,22 +15,11 @@ const GeneralSettingsPage: FC = () => { const enabledExperimentsQuery = useQuery(experiments()); const safeExperimentsQuery = useQuery(availableExperiments()); - const safeExperiments: string[] = []; - const invalidExperiments: string[] = []; - - (safeExperimentsQuery.data?.safe ?? []).forEach((value) => - safeExperiments.push(value), - ); - - (enabledExperimentsQuery.data ?? []).forEach(function (exp) { - const found = (safeExperimentsQuery.data?.safe ?? []).find((value) => { - return exp === value; - }); - - if (!found) { - invalidExperiments.push(exp); - } - }); + const safeExperiments = safeExperimentsQuery.data?.safe ?? []; + const invalidExperiments = + enabledExperimentsQuery.data?.filter(exp => { + return !safeExperiments.includes(exp); + }) ?? []; return ( <> From 236dc9d76ece5dd78d390e114cb4bb11d066a0dd Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Wed, 17 Apr 2024 16:44:22 +0200 Subject: [PATCH 13/13] make fmt Signed-off-by: Danny Kopping --- .../GeneralSettingsPage/GeneralSettingsPage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx index 63a63f48b6e8e..2e79b36f460b2 100644 --- a/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx +++ b/site/src/pages/DeploySettingsPage/GeneralSettingsPage/GeneralSettingsPage.tsx @@ -17,7 +17,7 @@ const GeneralSettingsPage: FC = () => { const safeExperiments = safeExperimentsQuery.data?.safe ?? []; const invalidExperiments = - enabledExperimentsQuery.data?.filter(exp => { + enabledExperimentsQuery.data?.filter((exp) => { return !safeExperiments.includes(exp); }) ?? [];