From ae44ecfc0749999c2708388751465c4585d0860e Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Thu, 3 Apr 2025 14:13:34 -0400 Subject: [PATCH 001/433] chore: update prismjs to 1.30.0 (#17215) - Resolves GHSA-x7hr-w5r2-h6wg --- site/package.json | 3 ++- site/pnpm-lock.yaml | 17 ++++++----------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/site/package.json b/site/package.json index 1c02cc55bd141..279430d032622 100644 --- a/site/package.json +++ b/site/package.json @@ -203,7 +203,8 @@ "pnpm": { "overrides": { "@babel/runtime": "7.26.10", - "@babel/helpers": "7.26.10" + "@babel/helpers": "7.26.10", + "prismjs": "1.30.0" } } } diff --git a/site/pnpm-lock.yaml b/site/pnpm-lock.yaml index 2a9c99029b149..48228e03d82db 100644 --- a/site/pnpm-lock.yaml +++ b/site/pnpm-lock.yaml @@ -9,6 +9,7 @@ overrides: semver: 7.6.2 '@babel/runtime': 7.26.10 '@babel/helpers': 7.26.10 + prismjs: 1.30.0 importers: @@ -5325,12 +5326,8 @@ packages: resolution: {integrity: sha512-Pdlw/oPxN+aXdmM9R00JVC9WVFoCLTKJvDVLgmJ+qAffBMxsV85l/Lu7sNx4zSzPyoL2euImuEwHhOXdEgNFZQ==, tarball: https://registry.npmjs.org/pretty-format/-/pretty-format-29.7.0.tgz} engines: {node: ^14.15.0 || ^16.10.0 || >=18.0.0} - prismjs@1.27.0: - resolution: {integrity: sha512-t13BGPUlFDR7wRB5kQDG4jjl7XeuH6jbJGt11JHPL96qwsEHNX2+68tFXqc1/k+/jALsbSWJKUOT/hcYAZ5LkA==, tarball: https://registry.npmjs.org/prismjs/-/prismjs-1.27.0.tgz} - engines: {node: '>=6'} - - prismjs@1.29.0: - resolution: {integrity: sha512-Kx/1w86q/epKcmte75LNrEoT+lX8pBpavuAbvJWRXar7Hz8jrtF+e3vY751p0R8H9HdArwaCTNDDzHg/ScJK1Q==, tarball: https://registry.npmjs.org/prismjs/-/prismjs-1.29.0.tgz} + prismjs@1.30.0: + resolution: {integrity: sha512-DEvV2ZF2r2/63V+tK8hQvrR2ZGn10srHbXviTlcv7Kpzw8jWiNTqbVgjO3IY8RxrrOUF8VPMQQFysYYYv0YZxw==, tarball: https://registry.npmjs.org/prismjs/-/prismjs-1.30.0.tgz} engines: {node: '>=6'} process-nextick-args@2.0.1: @@ -12113,9 +12110,7 @@ snapshots: ansi-styles: 5.2.0 react-is: 18.3.1 - prismjs@1.27.0: {} - - prismjs@1.29.0: {} + prismjs@1.30.0: {} process-nextick-args@2.0.1: {} @@ -12372,7 +12367,7 @@ snapshots: highlight.js: 10.7.3 highlightjs-vue: 1.0.0 lowlight: 1.20.0 - prismjs: 1.29.0 + prismjs: 1.30.0 react: 18.3.1 refractor: 3.6.0 @@ -12464,7 +12459,7 @@ snapshots: dependencies: hastscript: 6.0.0 parse-entities: 2.0.0 - prismjs: 1.27.0 + prismjs: 1.30.0 regenerator-runtime@0.14.1: {} From 54ff17bec6b05f3f452ae367117bafc2a3a45d22 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 3 Apr 2025 21:39:12 +0100 Subject: [PATCH 002/433] feat: create experimental CreateWorkspacePage and dynamic-parameters experiment (#17240) The purpose of the PR is to make a copy of the CreateWorkspacePage and create an experimental version that will use when the dynamic-parameters experiment is enabled. The Figma designs for this page are still in progress but this first PR will start to move to the new designs. Figma design: https://www.figma.com/design/SMg6H8VKXnPSkE6h9KPoAD/UX-Presets?node-id=2121-2383&t=CtgPUz8eNsTI5b1t-1 Much of the existing code will be left behind and will slowly migrated over the course of several PRs to make sure no existing functionality is forgotten in the migration to dynamic paramaters. --- coderd/apidoc/docs.go | 7 +- coderd/apidoc/swagger.json | 7 +- codersdk/deployment.go | 1 + docs/reference/api/schemas.md | 1 + site/src/api/typesGenerated.ts | 1 + .../CreateWorkspaceExperimentRouter.tsx | 18 + .../CreateWorkspacePageExperimental.tsx | 327 +++++++++++++ .../CreateWorkspacePageViewExperimental.tsx | 446 ++++++++++++++++++ site/src/router.tsx | 6 +- 9 files changed, 807 insertions(+), 7 deletions(-) create mode 100644 site/src/pages/CreateWorkspacePage/CreateWorkspaceExperimentRouter.tsx create mode 100644 site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx create mode 100644 site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 134031a2fa5f0..c93af6a64a41c 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -12097,10 +12097,12 @@ const docTemplate = `{ "auto-fill-parameters", "notifications", "workspace-usage", - "web-push" + "web-push", + "dynamic-parameters" ], "x-enum-comments": { "ExperimentAutoFillParameters": "This should not be taken out of experiments until we have redesigned the feature.", + "ExperimentDynamicParameters": "Enables dynamic parameters when creating a workspace.", "ExperimentExample": "This isn't used for anything.", "ExperimentNotifications": "Sends notifications via SMTP and webhooks following certain events.", "ExperimentWebPush": "Enables web push notifications through the browser.", @@ -12111,7 +12113,8 @@ const docTemplate = `{ "ExperimentAutoFillParameters", "ExperimentNotifications", "ExperimentWorkspaceUsage", - "ExperimentWebPush" + "ExperimentWebPush", + "ExperimentDynamicParameters" ] }, "codersdk.ExternalAuth": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 66821355e7387..da4d7a4fcf41c 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -10833,10 +10833,12 @@ "auto-fill-parameters", "notifications", "workspace-usage", - "web-push" + "web-push", + "dynamic-parameters" ], "x-enum-comments": { "ExperimentAutoFillParameters": "This should not be taken out of experiments until we have redesigned the feature.", + "ExperimentDynamicParameters": "Enables dynamic parameters when creating a workspace.", "ExperimentExample": "This isn't used for anything.", "ExperimentNotifications": "Sends notifications via SMTP and webhooks following certain events.", "ExperimentWebPush": "Enables web push notifications through the browser.", @@ -10847,7 +10849,8 @@ "ExperimentAutoFillParameters", "ExperimentNotifications", "ExperimentWorkspaceUsage", - "ExperimentWebPush" + "ExperimentWebPush", + "ExperimentDynamicParameters" ] }, "codersdk.ExternalAuth": { diff --git a/codersdk/deployment.go b/codersdk/deployment.go index dc0bc36a85d5d..a67682489f81d 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -3194,6 +3194,7 @@ const ( ExperimentNotifications Experiment = "notifications" // Sends notifications via SMTP and webhooks following certain events. ExperimentWorkspaceUsage Experiment = "workspace-usage" // Enables the new workspace usage tracking. ExperimentWebPush Experiment = "web-push" // Enables web push notifications through the browser. + ExperimentDynamicParameters Experiment = "dynamic-parameters" // Enables dynamic parameters when creating a workspace. ) // ExperimentsAll should include all experiments that are safe for diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index f8af45a5e6787..4791967b53c9e 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -2845,6 +2845,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `notifications` | | `workspace-usage` | | `web-push` | +| `dynamic-parameters` | ## codersdk.ExternalAuth diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index ab8e58d4574f4..2df1c351d9db1 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -749,6 +749,7 @@ export const EntitlementsWarningHeader = "X-Coder-Entitlements-Warning"; // From codersdk/deployment.go export type Experiment = | "auto-fill-parameters" + | "dynamic-parameters" | "example" | "notifications" | "web-push" diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspaceExperimentRouter.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspaceExperimentRouter.tsx new file mode 100644 index 0000000000000..36cd921e28000 --- /dev/null +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspaceExperimentRouter.tsx @@ -0,0 +1,18 @@ +import { useDashboard } from "modules/dashboard/useDashboard"; +import type { FC } from "react"; +import CreateWorkspacePage from "./CreateWorkspacePage"; +import CreateWorkspacePageExperimental from "./CreateWorkspacePageExperimental"; + +const CreateWorkspaceExperimentRouter: FC = () => { + const { experiments } = useDashboard(); + + const dynamicParametersEnabled = experiments.includes("dynamic-parameters"); + + if (dynamicParametersEnabled) { + return ; + } + + return ; +}; + +export default CreateWorkspaceExperimentRouter; diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx new file mode 100644 index 0000000000000..cc843798b1d4c --- /dev/null +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx @@ -0,0 +1,327 @@ +import { API } from "api/api"; +import type { ApiErrorResponse } from "api/errors"; +import { checkAuthorization } from "api/queries/authCheck"; +import { + richParameters, + templateByName, + templateVersionExternalAuth, + templateVersionPresets, +} from "api/queries/templates"; +import { autoCreateWorkspace, createWorkspace } from "api/queries/workspaces"; +import type { + TemplateVersionParameter, + UserParameter, + Workspace, +} from "api/typesGenerated"; +import { Loader } from "components/Loader/Loader"; +import { useAuthenticated } from "contexts/auth/RequireAuth"; +import { useEffectEvent } from "hooks/hookPolyfills"; +import { useDashboard } from "modules/dashboard/useDashboard"; +import { + type WorkspacePermissions, + workspacePermissionChecks, +} from "modules/permissions/workspaces"; +import { generateWorkspaceName } from "modules/workspaces/generateWorkspaceName"; +import { type FC, useCallback, useEffect, useRef, useState } from "react"; +import { Helmet } from "react-helmet-async"; +import { useMutation, useQuery, useQueryClient } from "react-query"; +import { useNavigate, useParams, useSearchParams } from "react-router-dom"; +import { pageTitle } from "utils/page"; +import type { AutofillBuildParameter } from "utils/richParameters"; +import { paramsUsedToCreateWorkspace } from "utils/workspace"; +import { CreateWorkspacePageViewExperimental } from "./CreateWorkspacePageViewExperimental"; +export const createWorkspaceModes = ["form", "auto", "duplicate"] as const; +export type CreateWorkspaceMode = (typeof createWorkspaceModes)[number]; + +export type ExternalAuthPollingState = "idle" | "polling" | "abandoned"; + +const CreateWorkspacePageExperimental: FC = () => { + const { organization: organizationName = "default", template: templateName } = + useParams() as { organization?: string; template: string }; + const { user: me } = useAuthenticated(); + const navigate = useNavigate(); + const [searchParams] = useSearchParams(); + const { experiments } = useDashboard(); + + const customVersionId = searchParams.get("version") ?? undefined; + const defaultName = searchParams.get("name"); + const disabledParams = searchParams.get("disable_params")?.split(","); + const [mode, setMode] = useState(() => getWorkspaceMode(searchParams)); + const [autoCreateError, setAutoCreateError] = + useState(null); + + const queryClient = useQueryClient(); + const autoCreateWorkspaceMutation = useMutation( + autoCreateWorkspace(queryClient), + ); + const createWorkspaceMutation = useMutation(createWorkspace(queryClient)); + + const templateQuery = useQuery( + templateByName(organizationName, templateName), + ); + const templateVersionPresetsQuery = useQuery({ + ...templateVersionPresets(templateQuery.data?.active_version_id ?? ""), + enabled: templateQuery.data !== undefined, + }); + const permissionsQuery = useQuery( + templateQuery.data + ? checkAuthorization({ + checks: workspacePermissionChecks(templateQuery.data.organization_id), + }) + : { enabled: false }, + ); + const realizedVersionId = + customVersionId ?? templateQuery.data?.active_version_id; + const organizationId = templateQuery.data?.organization_id; + const richParametersQuery = useQuery({ + ...richParameters(realizedVersionId ?? ""), + enabled: realizedVersionId !== undefined, + }); + const realizedParameters = richParametersQuery.data + ? richParametersQuery.data.filter(paramsUsedToCreateWorkspace) + : undefined; + + const { + externalAuth, + externalAuthPollingState, + startPollingExternalAuth, + isLoadingExternalAuth, + } = useExternalAuth(realizedVersionId); + + const isLoadingFormData = + templateQuery.isLoading || + permissionsQuery.isLoading || + richParametersQuery.isLoading; + const loadFormDataError = + templateQuery.error ?? permissionsQuery.error ?? richParametersQuery.error; + + const title = autoCreateWorkspaceMutation.isLoading + ? "Creating workspace..." + : "Create workspace"; + + const onCreateWorkspace = useCallback( + (workspace: Workspace) => { + navigate(`/@${workspace.owner_name}/${workspace.name}`); + }, + [navigate], + ); + + // Auto fill parameters + const autofillEnabled = experiments.includes("auto-fill-parameters"); + const userParametersQuery = useQuery({ + queryKey: ["userParameters"], + queryFn: () => API.getUserParameters(templateQuery.data!.id), + enabled: autofillEnabled && templateQuery.isSuccess, + }); + const autofillParameters = getAutofillParameters( + searchParams, + userParametersQuery.data ? userParametersQuery.data : [], + ); + + const autoCreationStartedRef = useRef(false); + const automateWorkspaceCreation = useEffectEvent(async () => { + if (autoCreationStartedRef.current || !organizationId) { + return; + } + + try { + autoCreationStartedRef.current = true; + const newWorkspace = await autoCreateWorkspaceMutation.mutateAsync({ + organizationId, + templateName, + buildParameters: autofillParameters, + workspaceName: defaultName ?? generateWorkspaceName(), + templateVersionId: realizedVersionId, + match: searchParams.get("match"), + }); + + onCreateWorkspace(newWorkspace); + } catch { + setMode("form"); + } + }); + + const hasAllRequiredExternalAuth = Boolean( + !isLoadingExternalAuth && + externalAuth?.every((auth) => auth.optional || auth.authenticated), + ); + + let autoCreateReady = + mode === "auto" && + (!autofillEnabled || userParametersQuery.isSuccess) && + hasAllRequiredExternalAuth; + + // `mode=auto` was set, but a prerequisite has failed, and so auto-mode should be abandoned. + if ( + mode === "auto" && + !isLoadingExternalAuth && + !hasAllRequiredExternalAuth + ) { + // Prevent suddenly resuming auto-mode if the user connects to all of the required + // external auth providers. + setMode("form"); + // Ensure this is always false, so that we don't ever let `automateWorkspaceCreation` + // fire when we're trying to disable it. + autoCreateReady = false; + // Show an error message to explain _why_ the workspace was not created automatically. + const subject = + externalAuth?.length === 1 + ? "an external authentication provider that is" + : "external authentication providers that are"; + setAutoCreateError({ + message: `This template requires ${subject} not connected.`, + detail: + "Auto-creation has been disabled. Please connect all required external authentication providers before continuing.", + }); + } + + useEffect(() => { + if (autoCreateReady) { + void automateWorkspaceCreation(); + } + }, [automateWorkspaceCreation, autoCreateReady]); + + return ( + <> + + {pageTitle(title)} + + {isLoadingFormData || isLoadingExternalAuth || autoCreateReady ? ( + + ) : ( + { + navigate(-1); + }} + onSubmit={async (request, owner) => { + if (realizedVersionId) { + request = { + ...request, + template_id: undefined, + template_version_id: realizedVersionId, + }; + } + + const workspace = await createWorkspaceMutation.mutateAsync({ + ...request, + userId: owner.id, + }); + onCreateWorkspace(workspace); + }} + /> + )} + + ); +}; + +const useExternalAuth = (versionId: string | undefined) => { + const [externalAuthPollingState, setExternalAuthPollingState] = + useState("idle"); + + const startPollingExternalAuth = useCallback(() => { + setExternalAuthPollingState("polling"); + }, []); + + const { data: externalAuth, isLoading: isLoadingExternalAuth } = useQuery( + versionId + ? { + ...templateVersionExternalAuth(versionId), + refetchInterval: + externalAuthPollingState === "polling" ? 1000 : false, + } + : { enabled: false }, + ); + + const allSignedIn = externalAuth?.every((it) => it.authenticated); + + useEffect(() => { + if (allSignedIn) { + setExternalAuthPollingState("idle"); + return; + } + + if (externalAuthPollingState !== "polling") { + return; + } + + // Poll for a maximum of one minute + const quitPolling = setTimeout( + () => setExternalAuthPollingState("abandoned"), + 60_000, + ); + return () => { + clearTimeout(quitPolling); + }; + }, [externalAuthPollingState, allSignedIn]); + + return { + startPollingExternalAuth, + externalAuth, + externalAuthPollingState, + isLoadingExternalAuth, + }; +}; + +const getAutofillParameters = ( + urlSearchParams: URLSearchParams, + userParameters: UserParameter[], +): AutofillBuildParameter[] => { + const userParamMap = userParameters.reduce((acc, param) => { + acc.set(param.name, param); + return acc; + }, new Map()); + + const buildValues: AutofillBuildParameter[] = Array.from( + urlSearchParams.keys(), + ) + .filter((key) => key.startsWith("param.")) + .map((key) => { + const name = key.replace("param.", ""); + const value = urlSearchParams.get(key) ?? ""; + // URL should take precedence over user parameters + userParamMap.delete(name); + return { name, value, source: "url" }; + }); + + for (const param of userParamMap.values()) { + buildValues.push({ + name: param.name, + value: param.value, + source: "user_history", + }); + } + return buildValues; +}; + +export default CreateWorkspacePageExperimental; + +function getWorkspaceMode(params: URLSearchParams): CreateWorkspaceMode { + const paramMode = params.get("mode"); + if (createWorkspaceModes.includes(paramMode as CreateWorkspaceMode)) { + return paramMode as CreateWorkspaceMode; + } + + return "form"; +} diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx new file mode 100644 index 0000000000000..2eb58f515ec3c --- /dev/null +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx @@ -0,0 +1,446 @@ +import type { Interpolation, Theme } from "@emotion/react"; +import type * as TypesGen from "api/typesGenerated"; +import { Alert } from "components/Alert/Alert"; +import { ErrorAlert } from "components/Alert/ErrorAlert"; +import { Avatar } from "components/Avatar/Avatar"; +import { Button } from "components/Button/Button"; +import { FeatureStageBadge } from "components/FeatureStageBadge/FeatureStageBadge"; +import { SelectFilter } from "components/Filter/SelectFilter"; +import { Input } from "components/Input/Input"; +import { Label } from "components/Label/Label"; +import { Pill } from "components/Pill/Pill"; +import { RichParameterInput } from "components/RichParameterInput/RichParameterInput"; +import { Spinner } from "components/Spinner/Spinner"; +import { Stack } from "components/Stack/Stack"; +import { UserAutocomplete } from "components/UserAutocomplete/UserAutocomplete"; +import { type FormikContextType, useFormik } from "formik"; +import { ArrowLeft } from "lucide-react"; +import type { WorkspacePermissions } from "modules/permissions/workspaces"; +import { generateWorkspaceName } from "modules/workspaces/generateWorkspaceName"; +import { + type FC, + useCallback, + useEffect, + useId, + useMemo, + useState, +} from "react"; +import { Link } from "react-router-dom"; +import { + getFormHelpers, + nameValidator, + onChangeTrimmed, +} from "utils/formUtils"; +import { + type AutofillBuildParameter, + getInitialRichParameterValues, + useValidationSchemaForRichParameters, +} from "utils/richParameters"; +import * as Yup from "yup"; +import type { + CreateWorkspaceMode, + ExternalAuthPollingState, +} from "./CreateWorkspacePage"; +import { ExternalAuthButton } from "./ExternalAuthButton"; + +export const Language = { + duplicationWarning: + "Duplicating a workspace only copies its parameters. No state from the old workspace is copied over.", +} as const; + +export interface CreateWorkspacePageViewExperimentalProps { + mode: CreateWorkspaceMode; + defaultName?: string | null; + disabledParams?: string[]; + error: unknown; + resetMutation: () => void; + defaultOwner: TypesGen.User; + template: TypesGen.Template; + versionId?: string; + externalAuth: TypesGen.TemplateVersionExternalAuth[]; + externalAuthPollingState: ExternalAuthPollingState; + startPollingExternalAuth: () => void; + hasAllRequiredExternalAuth: boolean; + parameters: TypesGen.TemplateVersionParameter[]; + autofillParameters: AutofillBuildParameter[]; + presets: TypesGen.Preset[]; + permissions: WorkspacePermissions; + creatingWorkspace: boolean; + onCancel: () => void; + onSubmit: ( + req: TypesGen.CreateWorkspaceRequest, + owner: TypesGen.User, + ) => void; +} + +export const CreateWorkspacePageViewExperimental: FC< + CreateWorkspacePageViewExperimentalProps +> = ({ + mode, + defaultName, + disabledParams, + error, + resetMutation, + defaultOwner, + template, + versionId, + externalAuth, + externalAuthPollingState, + startPollingExternalAuth, + hasAllRequiredExternalAuth, + parameters, + autofillParameters, + presets = [], + permissions, + creatingWorkspace, + onSubmit, + onCancel, +}) => { + const [owner, setOwner] = useState(defaultOwner); + const [suggestedName, setSuggestedName] = useState(() => + generateWorkspaceName(), + ); + const id = useId(); + + const rerollSuggestedName = useCallback(() => { + setSuggestedName(() => generateWorkspaceName()); + }, []); + + const form: FormikContextType = + useFormik({ + initialValues: { + name: defaultName ?? "", + template_id: template.id, + rich_parameter_values: getInitialRichParameterValues( + parameters, + autofillParameters, + ), + }, + validationSchema: Yup.object({ + name: nameValidator("Workspace Name"), + rich_parameter_values: useValidationSchemaForRichParameters(parameters), + }), + enableReinitialize: true, + onSubmit: (request) => { + if (!hasAllRequiredExternalAuth) { + return; + } + + onSubmit(request, owner); + }, + }); + + useEffect(() => { + if (error) { + window.scrollTo(0, 0); + } + }, [error]); + + const getFieldHelpers = getFormHelpers( + form, + error, + ); + + const autofillByName = useMemo( + () => + Object.fromEntries( + autofillParameters.map((param) => [param.name, param]), + ), + [autofillParameters], + ); + + const [presetOptions, setPresetOptions] = useState([ + { label: "None", value: "" }, + ]); + useEffect(() => { + setPresetOptions([ + { label: "None", value: "" }, + ...presets.map((preset) => ({ + label: preset.Name, + value: preset.ID, + })), + ]); + }, [presets]); + + const [selectedPresetIndex, setSelectedPresetIndex] = useState(0); + const [presetParameterNames, setPresetParameterNames] = useState( + [], + ); + + useEffect(() => { + const selectedPresetOption = presetOptions[selectedPresetIndex]; + let selectedPreset: TypesGen.Preset | undefined; + for (const preset of presets) { + if (preset.ID === selectedPresetOption.value) { + selectedPreset = preset; + break; + } + } + + if (!selectedPreset || !selectedPreset.Parameters) { + setPresetParameterNames([]); + return; + } + + setPresetParameterNames(selectedPreset.Parameters.map((p) => p.Name)); + + for (const presetParameter of selectedPreset.Parameters) { + const parameterIndex = parameters.findIndex( + (p) => p.name === presetParameter.Name, + ); + if (parameterIndex === -1) continue; + + const parameterField = `rich_parameter_values.${parameterIndex}`; + + form.setFieldValue(parameterField, { + name: presetParameter.Name, + value: presetParameter.Value, + }); + } + }, [ + presetOptions, + selectedPresetIndex, + presets, + parameters, + form.setFieldValue, + ]); + + return ( + <> +
+ +
+
+
+
+ +

+ {template.display_name.length > 0 + ? template.display_name + : template.name} +

+
+

New workspace

+ + {template.deprecated && Deprecated} +
+ +
+ {Boolean(error) && } + + {mode === "duplicate" && ( + + {Language.duplicationWarning} + + )} + +
+
+

General

+

+ {permissions.createWorkspaceForUser + ? "Only admins can create workspaces for other users." + : "The name of your new workspace."} +

+
+
+ {versionId && versionId !== template.active_version_id && ( +
+ + + + This parameter has been preset, and cannot be modified. + +
+ )} +
+
+ +
+ { + form.setFieldValue("name", e.target.value.trim()); + resetMutation(); + }} + disabled={creatingWorkspace} + /> +
+ Need a suggestion? + +
+
+
+ {permissions.createWorkspaceForUser && ( +
+ + { + setOwner(user ?? defaultOwner); + }} + size="medium" + /> +
+ )} +
+
+
+ + {externalAuth && externalAuth.length > 0 && ( +
+
+

+ External Authentication +

+

+ This template uses external services for authentication. +

+
+
+ {Boolean(error) && !hasAllRequiredExternalAuth && ( + + To create a workspace using this template, please connect to + all required external authentication providers listed below. + + )} + {externalAuth.map((auth) => ( + + ))} +
+
+ )} + + {parameters.length > 0 && ( +
+
+

Parameters

+

+ These are the settings used by your template. Please note that + immutable parameters cannot be modified once the workspace is + created. +

+
+ {presets.length > 0 && ( + +
+
+ + +
+
+ { + const index = presetOptions.findIndex( + (preset) => preset.value === option?.value, + ); + if (index === -1) { + return; + } + setSelectedPresetIndex(index); + }} + placeholder="Select a preset" + selectedOption={presetOptions[selectedPresetIndex]} + /> +
+
+ + )} + +
+ {parameters.map((parameter, index) => { + const parameterField = `rich_parameter_values.${index}`; + const parameterInputName = `${parameterField}.value`; + const isDisabled = + disabledParams?.includes( + parameter.name.toLowerCase().replace(/ /g, "_"), + ) || + creatingWorkspace || + presetParameterNames.includes(parameter.name); + + return ( + { + await form.setFieldValue(parameterField, { + name: parameter.name, + value, + }); + }} + key={parameter.name} + parameter={parameter} + parameterAutofill={autofillByName[parameter.name]} + disabled={isDisabled} + /> + ); + })} +
+
+ )} + +
+ +
+ +
+ + ); +}; + +const styles = { + description: (theme) => ({ + fontSize: 13, + color: theme.palette.text.secondary, + }), +} satisfies Record>; diff --git a/site/src/router.tsx b/site/src/router.tsx index d1e3e903eb3fa..4f9ba95d1e05c 100644 --- a/site/src/router.tsx +++ b/site/src/router.tsx @@ -95,8 +95,8 @@ const TemplatePermissionsPage = lazy( const TemplateSummaryPage = lazy( () => import("./pages/TemplatePage/TemplateSummaryPage/TemplateSummaryPage"), ); -const CreateWorkspacePage = lazy( - () => import("./pages/CreateWorkspacePage/CreateWorkspacePage"), +const CreateWorkspaceExperimentRouter = lazy( + () => import("./pages/CreateWorkspacePage/CreateWorkspaceExperimentRouter"), ); const OverviewPage = lazy( () => import("./pages/DeploymentSettingsPage/OverviewPage/OverviewPage"), @@ -334,7 +334,7 @@ const templateRouter = () => { } /> - } /> + } /> }> } /> From e64140e9995f44a03c3723bd39c3c36c9ef58bca Mon Sep 17 00:00:00 2001 From: brettkolodny Date: Thu, 3 Apr 2025 18:02:39 -0400 Subject: [PATCH 003/433] fix: fix frontend build errors (#17252) --- .../CreateWorkspacePageExperimental.tsx | 5 ++++- .../CreateWorkspacePageViewExperimental.tsx | 11 +++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx index cc843798b1d4c..96198eb172cf8 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx @@ -66,7 +66,10 @@ const CreateWorkspacePageExperimental: FC = () => { const permissionsQuery = useQuery( templateQuery.data ? checkAuthorization({ - checks: workspacePermissionChecks(templateQuery.data.organization_id), + checks: workspacePermissionChecks( + templateQuery.data.organization_id, + me.id, + ), }) : { enabled: false }, ); diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx index 2eb58f515ec3c..a200a76f61081 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx @@ -25,12 +25,7 @@ import { useMemo, useState, } from "react"; -import { Link } from "react-router-dom"; -import { - getFormHelpers, - nameValidator, - onChangeTrimmed, -} from "utils/formUtils"; +import { getFormHelpers, nameValidator } from "utils/formUtils"; import { type AutofillBuildParameter, getInitialRichParameterValues, @@ -258,7 +253,7 @@ export const CreateWorkspacePageViewExperimental: FC<

General

- {permissions.createWorkspaceForUser + {permissions.createWorkspace ? "Only admins can create workspaces for other users." : "The name of your new workspace."}

@@ -305,7 +300,7 @@ export const CreateWorkspacePageViewExperimental: FC< - {permissions.createWorkspaceForUser && ( + {permissions.createWorkspace && (
+
+ +
+ From 510bc37cbcff3921c64b8f2d6a18339cd2648588 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Fri, 4 Apr 2025 10:09:37 -0300 Subject: [PATCH 009/433] refactor: update avatar sizes in groups, users and members (#17230) We updated the template and workspace avatars to be "lg" so to keep it consistent we have to do the same for the other avatars too. --- site/src/components/Avatar/Avatar.tsx | 6 +- site/src/components/Avatar/AvatarData.tsx | 8 +- .../components/Avatar/AvatarDataSkeleton.tsx | 2 +- site/src/components/LastSeen/LastSeen.tsx | 10 +- site/src/components/Table/Table.tsx | 4 +- site/src/pages/GroupsPage/GroupPage.tsx | 8 +- site/src/pages/GroupsPage/GroupsPageView.tsx | 38 +++--- .../OrganizationMembersPageView.tsx | 109 ++++++++++-------- .../TemplatePermissionsPageView.tsx | 1 + .../UsersPage/UsersTable/UsersTableBody.tsx | 4 +- 10 files changed, 110 insertions(+), 80 deletions(-) diff --git a/site/src/components/Avatar/Avatar.tsx b/site/src/components/Avatar/Avatar.tsx index 46316950c80b6..8661dceda0f6a 100644 --- a/site/src/components/Avatar/Avatar.tsx +++ b/site/src/components/Avatar/Avatar.tsx @@ -22,9 +22,9 @@ const avatarVariants = cva( { variants: { size: { - lg: "h-[--avatar-lg] w-[--avatar-lg] rounded-[6px] text-sm font-medium", - md: "h-[--avatar-default] w-[--avatar-default] text-2xs", - sm: "h-[--avatar-sm] w-[--avatar-sm] text-[8px]", + lg: "size-[--avatar-lg] rounded-[6px] text-sm font-medium", + md: "size-[--avatar-default] text-2xs", + sm: "size-[--avatar-sm] text-[8px]", }, variant: { default: null, diff --git a/site/src/components/Avatar/AvatarData.tsx b/site/src/components/Avatar/AvatarData.tsx index 5eda0e326d24b..8f55e1e7ae39b 100644 --- a/site/src/components/Avatar/AvatarData.tsx +++ b/site/src/components/Avatar/AvatarData.tsx @@ -1,6 +1,4 @@ -import { useTheme } from "@emotion/react"; import { Avatar } from "components/Avatar/Avatar"; -import { Stack } from "components/Stack/Stack"; import type { FC, ReactNode } from "react"; export interface AvatarDataProps { @@ -26,10 +24,10 @@ export const AvatarData: FC = ({ imgFallbackText, avatar, }) => { - const theme = useTheme(); if (!avatar) { avatar = ( @@ -41,7 +39,9 @@ export const AvatarData: FC = ({ {avatar}
- {title} + + {title} + {subtitle && ( {subtitle} diff --git a/site/src/components/Avatar/AvatarDataSkeleton.tsx b/site/src/components/Avatar/AvatarDataSkeleton.tsx index 13083ce8b02e3..5aa18fdcbc2b0 100644 --- a/site/src/components/Avatar/AvatarDataSkeleton.tsx +++ b/site/src/components/Avatar/AvatarDataSkeleton.tsx @@ -5,7 +5,7 @@ import type { FC } from "react"; export const AvatarDataSkeleton: FC = () => { return (
- +
diff --git a/site/src/components/LastSeen/LastSeen.tsx b/site/src/components/LastSeen/LastSeen.tsx index 61510319a1208..081e3ae624fa4 100644 --- a/site/src/components/LastSeen/LastSeen.tsx +++ b/site/src/components/LastSeen/LastSeen.tsx @@ -2,6 +2,7 @@ import { useTheme } from "@emotion/react"; import dayjs from "dayjs"; import relativeTime from "dayjs/plugin/relativeTime"; import type { FC, HTMLAttributes } from "react"; +import { cn } from "utils/cn"; dayjs.extend(relativeTime); @@ -11,7 +12,7 @@ interface LastSeenProps "data-chromatic"?: string; // prevents a type error in the stories } -export const LastSeen: FC = ({ at, ...attrs }) => { +export const LastSeen: FC = ({ at, className, ...attrs }) => { const theme = useTheme(); const t = dayjs(at); const now = dayjs(); @@ -35,7 +36,12 @@ export const LastSeen: FC = ({ at, ...attrs }) => { } return ( - + {message} ); diff --git a/site/src/components/Table/Table.tsx b/site/src/components/Table/Table.tsx index 604fc3d4f4196..269248207c39b 100644 --- a/site/src/components/Table/Table.tsx +++ b/site/src/components/Table/Table.tsx @@ -82,7 +82,7 @@ export const TableHead = React.forwardRef<
[role=checkbox]]:translate-y-[2px]", className, )} @@ -98,7 +98,7 @@ export const TableCell = React.forwardRef< ref={ref} className={cn( "border-0 border-t border-border border-solid", - "py-2 px-4 align-middle [&:has([role=checkbox])]:pr-0 [&>[role=checkbox]]:translate-y-[2px]", + "p-3 align-middle [&:has([role=checkbox])]:pr-0 [&>[role=checkbox]]:translate-y-[2px]", className, )} {...props} diff --git a/site/src/pages/GroupsPage/GroupPage.tsx b/site/src/pages/GroupsPage/GroupPage.tsx index f723f48a4b211..8dbd5d3f8fb31 100644 --- a/site/src/pages/GroupsPage/GroupPage.tsx +++ b/site/src/pages/GroupsPage/GroupPage.tsx @@ -310,7 +310,13 @@ const GroupMemberRow: FC = ({ } + avatar={ + + } title={member.username} subtitle={member.email} /> diff --git a/site/src/pages/GroupsPage/GroupsPageView.tsx b/site/src/pages/GroupsPage/GroupsPageView.tsx index 3ca28c31f59bf..4d5f70bfae6c7 100644 --- a/site/src/pages/GroupsPage/GroupsPageView.tsx +++ b/site/src/pages/GroupsPage/GroupsPageView.tsx @@ -1,12 +1,12 @@ import type { Interpolation, Theme } from "@emotion/react"; import AddOutlined from "@mui/icons-material/AddOutlined"; import KeyboardArrowRight from "@mui/icons-material/KeyboardArrowRight"; -import AvatarGroup from "@mui/material/AvatarGroup"; import Skeleton from "@mui/material/Skeleton"; import type { Group } from "api/typesGenerated"; import { Avatar } from "components/Avatar/Avatar"; import { AvatarData } from "components/Avatar/AvatarData"; import { AvatarDataSkeleton } from "components/Avatar/AvatarDataSkeleton"; +import { Badge } from "components/Badge/Badge"; import { Button } from "components/Button/Button"; import { ChooseOne, Cond } from "components/Conditionals/ChooseOne"; import { EmptyState } from "components/EmptyState/EmptyState"; @@ -115,6 +115,8 @@ const GroupRow: FC = ({ group }) => { const rowProps = useClickableTableRow({ onClick: () => navigate(group.name), }); + const memberAvatars = group.members.slice(0, 5); + const remainingAvatars = group.members.length - memberAvatars.length; return ( @@ -122,6 +124,8 @@ const GroupRow: FC = ({ group }) => { @@ -132,20 +136,24 @@ const GroupRow: FC = ({ group }) => { - {group.members.length === 0 && "-"} - - {group.members.map((member) => ( - - ))} - + {group.members.length > 0 ? ( +
+ {memberAvatars.map((member) => ( + + ))} + {remainingAvatars > 0 && ( + + +{remainingAvatars} + + )} +
+ ) : ( + "-" + )} diff --git a/site/src/pages/OrganizationSettingsPage/OrganizationMembersPageView.tsx b/site/src/pages/OrganizationSettingsPage/OrganizationMembersPageView.tsx index c21b6fbd4dca7..d0bb441a1ed25 100644 --- a/site/src/pages/OrganizationSettingsPage/OrganizationMembersPageView.tsx +++ b/site/src/pages/OrganizationSettingsPage/OrganizationMembersPageView.tsx @@ -11,6 +11,7 @@ import { ErrorAlert } from "components/Alert/ErrorAlert"; import { Avatar } from "components/Avatar/Avatar"; import { AvatarData } from "components/Avatar/AvatarData"; import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; +import { Loader } from "components/Loader/Loader"; import { MoreMenu, MoreMenuContent, @@ -32,6 +33,7 @@ import { TableHeader, TableRow, } from "components/Table/Table"; +import { TableLoader } from "components/TableLoader/TableLoader"; import { UserAutocomplete } from "components/UserAutocomplete/UserAutocomplete"; import type { PaginationResultInfo } from "hooks/usePaginatedQuery"; import { TriangleAlert } from "lucide-react"; @@ -125,58 +127,67 @@ export const OrganizationMembersPageView: FC< - {members?.map((member) => ( - - - - } - title={member.name || member.username} - subtitle={member.email} + {members ? ( + members.map((member) => ( + + + + } + title={member.name || member.username} + subtitle={member.email} + /> + + { + try { + await updateMemberRoles(member, roles); + displaySuccess("Roles updated successfully."); + } catch (error) { + displayError( + getErrorMessage(error, "Failed to update roles."), + ); + } + }} /> - - { - try { - await updateMemberRoles(member, roles); - displaySuccess("Roles updated successfully."); - } catch (error) { - displayError( - getErrorMessage(error, "Failed to update roles."), - ); - } - }} - /> - - - {member.user_id !== me.id && canEditMembers && ( - - - - - - removeMember(member)} - > - Remove - - - - )} + + + {member.user_id !== me.id && canEditMembers && ( + + + + + + removeMember(member)} + > + Remove + + + + )} + + + )) + ) : ( + + + - ))} + )}
diff --git a/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx b/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx index 33f1b227e0af2..46f62e10c1601 100644 --- a/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx @@ -258,6 +258,7 @@ export const TemplatePermissionsPageView: FC< diff --git a/site/src/pages/UsersPage/UsersTable/UsersTableBody.tsx b/site/src/pages/UsersPage/UsersTable/UsersTableBody.tsx index 8e447b8c05a4e..f746b35aba75f 100644 --- a/site/src/pages/UsersPage/UsersTable/UsersTableBody.tsx +++ b/site/src/pages/UsersPage/UsersTable/UsersTableBody.tsx @@ -87,9 +87,7 @@ export const UsersTableBody: FC = ({ -
- -
+ From ae67e33c66ce22c4594bad28300ccd317812ea71 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 4 Apr 2025 14:59:01 +0100 Subject: [PATCH 010/433] fix: set permissions for experimental Createworkspace page (#17254) --- site/src/modules/permissions/workspaces.ts | 2 +- .../CreateWorkspacePage.tsx | 7 ++++-- .../CreateWorkspacePageExperimental.tsx | 15 +++++------- .../CreateWorkspacePageView.stories.tsx | 2 +- .../CreateWorkspacePageView.tsx | 8 +++---- .../CreateWorkspacePageViewExperimental.tsx | 9 ++++--- .../pages/CreateWorkspacePage/permissions.ts | 4 ++-- .../src/pages/TemplatePage/TemplateLayout.tsx | 9 +++++-- .../TemplatePageHeader.stories.tsx | 4 ++-- .../pages/TemplatePage/TemplatePageHeader.tsx | 24 ++++++++++--------- .../TemplatesPageView.stories.tsx | 4 ++-- .../pages/TemplatesPage/TemplatesPageView.tsx | 2 +- .../pages/WorkspacesPage/WorkspacesPage.tsx | 11 +++++---- 13 files changed, 54 insertions(+), 47 deletions(-) diff --git a/site/src/modules/permissions/workspaces.ts b/site/src/modules/permissions/workspaces.ts index b0834877e8e6c..8410571fa1f8e 100644 --- a/site/src/modules/permissions/workspaces.ts +++ b/site/src/modules/permissions/workspaces.ts @@ -3,7 +3,7 @@ export const workspacePermissionChecks = ( userId: string, ) => ({ - createWorkspace: { + createWorkspaceForUserID: { object: { resource_type: "workspace", organization_id: organizationId, diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx index 150a79bd69487..fd88e0cc23e72 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.tsx @@ -26,7 +26,10 @@ import { pageTitle } from "utils/page"; import type { AutofillBuildParameter } from "utils/richParameters"; import { paramsUsedToCreateWorkspace } from "utils/workspace"; import { CreateWorkspacePageView } from "./CreateWorkspacePageView"; -import { type CreateWSPermissions, createWorkspaceChecks } from "./permissions"; +import { + type CreateWorkspacePermissions, + createWorkspaceChecks, +} from "./permissions"; export const createWorkspaceModes = ["form", "auto", "duplicate"] as const; export type CreateWorkspaceMode = (typeof createWorkspaceModes)[number]; @@ -206,7 +209,7 @@ const CreateWorkspacePage: FC = () => { externalAuthPollingState={externalAuthPollingState} startPollingExternalAuth={startPollingExternalAuth} hasAllRequiredExternalAuth={hasAllRequiredExternalAuth} - permissions={permissionsQuery.data as CreateWSPermissions} + permissions={permissionsQuery.data as CreateWorkspacePermissions} parameters={realizedParameters as TemplateVersionParameter[]} presets={templateVersionPresetsQuery.data ?? []} creatingWorkspace={createWorkspaceMutation.isLoading} diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx index 96198eb172cf8..8598085c948e5 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx @@ -17,10 +17,6 @@ import { Loader } from "components/Loader/Loader"; import { useAuthenticated } from "contexts/auth/RequireAuth"; import { useEffectEvent } from "hooks/hookPolyfills"; import { useDashboard } from "modules/dashboard/useDashboard"; -import { - type WorkspacePermissions, - workspacePermissionChecks, -} from "modules/permissions/workspaces"; import { generateWorkspaceName } from "modules/workspaces/generateWorkspaceName"; import { type FC, useCallback, useEffect, useRef, useState } from "react"; import { Helmet } from "react-helmet-async"; @@ -32,6 +28,10 @@ import { paramsUsedToCreateWorkspace } from "utils/workspace"; import { CreateWorkspacePageViewExperimental } from "./CreateWorkspacePageViewExperimental"; export const createWorkspaceModes = ["form", "auto", "duplicate"] as const; export type CreateWorkspaceMode = (typeof createWorkspaceModes)[number]; +import { + type CreateWorkspacePermissions, + createWorkspaceChecks, +} from "./permissions"; export type ExternalAuthPollingState = "idle" | "polling" | "abandoned"; @@ -66,10 +66,7 @@ const CreateWorkspacePageExperimental: FC = () => { const permissionsQuery = useQuery( templateQuery.data ? checkAuthorization({ - checks: workspacePermissionChecks( - templateQuery.data.organization_id, - me.id, - ), + checks: createWorkspaceChecks(templateQuery.data.organization_id), }) : { enabled: false }, ); @@ -211,7 +208,7 @@ const CreateWorkspacePageExperimental: FC = () => { externalAuthPollingState={externalAuthPollingState} startPollingExternalAuth={startPollingExternalAuth} hasAllRequiredExternalAuth={hasAllRequiredExternalAuth} - permissions={permissionsQuery.data as WorkspacePermissions} + permissions={permissionsQuery.data as CreateWorkspacePermissions} parameters={realizedParameters as TemplateVersionParameter[]} presets={templateVersionPresetsQuery.data ?? []} creatingWorkspace={createWorkspaceMutation.isLoading} diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx index 47d1198765452..564209f076b08 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.stories.tsx @@ -27,7 +27,7 @@ const meta: Meta = { hasAllRequiredExternalAuth: true, mode: "form", permissions: { - createWorkspaceForUser: true, + createWorkspaceForAny: true, }, onCancel: action("onCancel"), }, diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx index 656e18563eb60..5dc9c8d0a4818 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageView.tsx @@ -46,7 +46,7 @@ import type { ExternalAuthPollingState, } from "./CreateWorkspacePage"; import { ExternalAuthButton } from "./ExternalAuthButton"; -import type { CreateWSPermissions } from "./permissions"; +import type { CreateWorkspacePermissions } from "./permissions"; export const Language = { duplicationWarning: "Duplicating a workspace only copies its parameters. No state from the old workspace is copied over.", @@ -68,7 +68,7 @@ export interface CreateWorkspacePageViewProps { parameters: TypesGen.TemplateVersionParameter[]; autofillParameters: AutofillBuildParameter[]; presets: TypesGen.Preset[]; - permissions: CreateWSPermissions; + permissions: CreateWorkspacePermissions; creatingWorkspace: boolean; onCancel: () => void; onSubmit: ( @@ -255,7 +255,7 @@ export const CreateWorkspacePageView: FC = ({ = ({ - {permissions.createWorkspaceForUser && ( + {permissions.createWorkspaceForAny && ( { diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx index a200a76f61081..ff8c2836be311 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx @@ -15,7 +15,6 @@ import { Stack } from "components/Stack/Stack"; import { UserAutocomplete } from "components/UserAutocomplete/UserAutocomplete"; import { type FormikContextType, useFormik } from "formik"; import { ArrowLeft } from "lucide-react"; -import type { WorkspacePermissions } from "modules/permissions/workspaces"; import { generateWorkspaceName } from "modules/workspaces/generateWorkspaceName"; import { type FC, @@ -37,7 +36,7 @@ import type { ExternalAuthPollingState, } from "./CreateWorkspacePage"; import { ExternalAuthButton } from "./ExternalAuthButton"; - +import type { CreateWorkspacePermissions } from "./permissions"; export const Language = { duplicationWarning: "Duplicating a workspace only copies its parameters. No state from the old workspace is copied over.", @@ -59,7 +58,7 @@ export interface CreateWorkspacePageViewExperimentalProps { parameters: TypesGen.TemplateVersionParameter[]; autofillParameters: AutofillBuildParameter[]; presets: TypesGen.Preset[]; - permissions: WorkspacePermissions; + permissions: CreateWorkspacePermissions; creatingWorkspace: boolean; onCancel: () => void; onSubmit: ( @@ -253,7 +252,7 @@ export const CreateWorkspacePageViewExperimental: FC<

General

- {permissions.createWorkspace + {permissions.createWorkspaceForAny ? "Only admins can create workspaces for other users." : "The name of your new workspace."}

@@ -300,7 +299,7 @@ export const CreateWorkspacePageViewExperimental: FC< - {permissions.createWorkspace && ( + {permissions.createWorkspaceForAny && (
+ {presets.length > 0 && (
@@ -502,3 +499,44 @@ export const CreateWorkspacePageViewExperimental: FC< ); }; + +interface DiagnosticsProps { + diagnostics: PreviewParameter["diagnostics"]; +} + +export const Diagnostics: FC = ({ diagnostics }) => { + return ( +
+ {diagnostics.map((diagnostic, index) => ( +
+
+ {diagnostic.severity === "error" && ( +
+ {diagnostic.detail &&

{diagnostic.detail}

} +
+ ))} +
+ ); +}; diff --git a/site/tailwind.config.js b/site/tailwind.config.js index 971a729332aff..3e612408596f5 100644 --- a/site/tailwind.config.js +++ b/site/tailwind.config.js @@ -52,6 +52,7 @@ module.exports = { }, border: { DEFAULT: "hsl(var(--border-default))", + warning: "hsl(var(--border-warning))", destructive: "hsl(var(--border-destructive))", success: "hsl(var(--border-success))", hover: "hsl(var(--border-hover))", From d20966d5004f0f3564ba611b76e78b6dc5824e66 Mon Sep 17 00:00:00 2001 From: Eric Paulsen Date: Wed, 16 Apr 2025 20:11:02 +0200 Subject: [PATCH 134/433] chore: update go to 1.24.2 (#17356) this updates `go` to the latest stable patch version `1.24.2` in: - `go.mod` - `dogfood/coder/Dockerfile` - `.github/actions/setup-go/action.yaml` - `flake.nix` written with the assistance of ClaudeCode. --------- Co-authored-by: Thomas Kosiewski --- .github/actions/setup-go/action.yaml | 2 +- dogfood/coder/Dockerfile | 2 +- flake.nix | 4 ++-- go.mod | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/actions/setup-go/action.yaml b/.github/actions/setup-go/action.yaml index 7858b8ecc6cac..76b7c5d87d206 100644 --- a/.github/actions/setup-go/action.yaml +++ b/.github/actions/setup-go/action.yaml @@ -4,7 +4,7 @@ description: | inputs: version: description: "The Go version to use." - default: "1.24.1" + default: "1.24.2" runs: using: "composite" steps: diff --git a/dogfood/coder/Dockerfile b/dogfood/coder/Dockerfile index 1559279e41aa9..8a8f02e79e5dd 100644 --- a/dogfood/coder/Dockerfile +++ b/dogfood/coder/Dockerfile @@ -9,7 +9,7 @@ RUN cargo install typos-cli watchexec-cli && \ FROM ubuntu:jammy@sha256:0e5e4a57c2499249aafc3b40fcd541e9a456aab7296681a3994d631587203f97 AS go # Install Go manually, so that we can control the version -ARG GO_VERSION=1.24.1 +ARG GO_VERSION=1.24.2 # Boring Go is needed to build FIPS-compliant binaries. RUN apt-get update && \ diff --git a/flake.nix b/flake.nix index bb8f466383f04..af8c2b42bf00f 100644 --- a/flake.nix +++ b/flake.nix @@ -130,7 +130,7 @@ gnused gnugrep gnutar - go_1_22 + unstablePkgs.go_1_24 go-migrate (pinnedPkgs.golangci-lint) gopls @@ -196,7 +196,7 @@ # slim bundle into it's own derivation. buildFat = osArch: - pkgs.buildGo122Module { + unstablePkgs.buildGo124Module { name = "coder-${osArch}"; # Updated with ./scripts/update-flake.sh`. # This should be updated whenever go.mod changes! diff --git a/go.mod b/go.mod index d3e9c55f3d937..826d5cd2c0235 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/coder/coder/v2 -go 1.24.1 +go 1.24.2 // Required until a v3 of chroma is created to lazily initialize all XML files. // None of our dependencies seem to use the registries anyways, so this From c4d3dd27917da9f9782095233f53f430458118e6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 16 Apr 2025 14:39:57 -0500 Subject: [PATCH 135/433] chore: prevent null loading sync settings (#17430) Nulls passed to the frontend caused a page to fail to load. `Record` can be `nil` in golang --- coderd/idpsync/group.go | 3 +++ coderd/idpsync/organization.go | 3 +++ coderd/idpsync/role.go | 3 +++ 3 files changed, 9 insertions(+) diff --git a/coderd/idpsync/group.go b/coderd/idpsync/group.go index 4524284260359..b85ce1b749e28 100644 --- a/coderd/idpsync/group.go +++ b/coderd/idpsync/group.go @@ -268,6 +268,9 @@ func (s *GroupSyncSettings) Set(v string) error { } func (s *GroupSyncSettings) String() string { + if s.Mapping == nil { + s.Mapping = make(map[string][]uuid.UUID) + } return runtimeconfig.JSONString(s) } diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index be65daba369df..5d56bc7d239a5 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -217,6 +217,9 @@ func (s *OrganizationSyncSettings) Set(v string) error { } func (s *OrganizationSyncSettings) String() string { + if s.Mapping == nil { + s.Mapping = make(map[string][]uuid.UUID) + } return runtimeconfig.JSONString(s) } diff --git a/coderd/idpsync/role.go b/coderd/idpsync/role.go index 54ec787661826..c21e7c99c4614 100644 --- a/coderd/idpsync/role.go +++ b/coderd/idpsync/role.go @@ -286,5 +286,8 @@ func (s *RoleSyncSettings) Set(v string) error { } func (s *RoleSyncSettings) String() string { + if s.Mapping == nil { + s.Mapping = make(map[string][]string) + } return runtimeconfig.JSONString(s) } From 2e5cd299f2004a25e772c86c798a30df897a05b4 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 16 Apr 2025 15:55:37 -0500 Subject: [PATCH 136/433] chore: load 'assign_default' value from legacy value (#17428) If this value was set before v2.19.0, then assign_default was in a json field that would not match. And it would default to `false`. This corrects that. --- coderd/idpsync/organization.go | 11 +++++ coderd/idpsync/organizations_test.go | 68 ++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/coderd/idpsync/organization.go b/coderd/idpsync/organization.go index 5d56bc7d239a5..f0736e1ea7559 100644 --- a/coderd/idpsync/organization.go +++ b/coderd/idpsync/organization.go @@ -213,6 +213,17 @@ type OrganizationSyncSettings struct { } func (s *OrganizationSyncSettings) Set(v string) error { + legacyCheck := make(map[string]any) + err := json.Unmarshal([]byte(v), &legacyCheck) + if assign, ok := legacyCheck["AssignDefault"]; err == nil && ok { + // The legacy JSON key was 'AssignDefault' instead of 'assign_default' + // Set the default value from the legacy if it exists. + isBool, ok := assign.(bool) + if ok { + s.AssignDefault = isBool + } + } + return json.Unmarshal([]byte(v), s) } diff --git a/coderd/idpsync/organizations_test.go b/coderd/idpsync/organizations_test.go index 3a00499bdbced..c3c0a052f7100 100644 --- a/coderd/idpsync/organizations_test.go +++ b/coderd/idpsync/organizations_test.go @@ -2,6 +2,7 @@ package idpsync_test import ( "database/sql" + "fmt" "testing" "github.com/golang-jwt/jwt/v4" @@ -19,6 +20,73 @@ import ( "github.com/coder/coder/v2/testutil" ) +func TestFromLegacySettings(t *testing.T) { + t.Parallel() + + legacy := func(assignDefault bool) string { + return fmt.Sprintf(`{ + "Field":"groups", + "Mapping":{ + "engineering":[ + "10b2bd19-f5ca-4905-919f-bf02e95e3b6a" + ] + }, + "AssignDefault":%t + }`, assignDefault) + } + + t.Run("AssignDefault,True", func(t *testing.T) { + t.Parallel() + + var settings idpsync.OrganizationSyncSettings + settings.AssignDefault = true + err := settings.Set(legacy(true)) + require.NoError(t, err) + + require.Equal(t, settings.Field, "groups", "field") + require.Equal(t, settings.Mapping, map[string][]uuid.UUID{ + "engineering": { + uuid.MustParse("10b2bd19-f5ca-4905-919f-bf02e95e3b6a"), + }, + }, "mapping") + require.True(t, settings.AssignDefault, "assign default") + }) + + t.Run("AssignDefault,False", func(t *testing.T) { + t.Parallel() + + var settings idpsync.OrganizationSyncSettings + settings.AssignDefault = true + err := settings.Set(legacy(false)) + require.NoError(t, err) + + require.Equal(t, settings.Field, "groups", "field") + require.Equal(t, settings.Mapping, map[string][]uuid.UUID{ + "engineering": { + uuid.MustParse("10b2bd19-f5ca-4905-919f-bf02e95e3b6a"), + }, + }, "mapping") + require.False(t, settings.AssignDefault, "assign default") + }) + + t.Run("CorrectAssign", func(t *testing.T) { + t.Parallel() + + var settings idpsync.OrganizationSyncSettings + settings.AssignDefault = true + err := settings.Set(legacy(false)) + require.NoError(t, err) + + require.Equal(t, settings.Field, "groups", "field") + require.Equal(t, settings.Mapping, map[string][]uuid.UUID{ + "engineering": { + uuid.MustParse("10b2bd19-f5ca-4905-919f-bf02e95e3b6a"), + }, + }, "mapping") + require.False(t, settings.AssignDefault, "assign default") + }) +} + func TestParseOrganizationClaims(t *testing.T) { t.Parallel() From 7f6e5139eb62d62f96e04721bf76715b78166199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=B1=E3=82=A4=E3=83=A9?= Date: Wed, 16 Apr 2025 16:21:14 -0700 Subject: [PATCH 137/433] chore: format code (#17438) --- coderd/idpsync/organizations_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/coderd/idpsync/organizations_test.go b/coderd/idpsync/organizations_test.go index c3c0a052f7100..c3f17cefebd28 100644 --- a/coderd/idpsync/organizations_test.go +++ b/coderd/idpsync/organizations_test.go @@ -25,13 +25,13 @@ func TestFromLegacySettings(t *testing.T) { legacy := func(assignDefault bool) string { return fmt.Sprintf(`{ - "Field":"groups", - "Mapping":{ - "engineering":[ - "10b2bd19-f5ca-4905-919f-bf02e95e3b6a" - ] - }, - "AssignDefault":%t + "Field": "groups", + "Mapping": { + "engineering": [ + "10b2bd19-f5ca-4905-919f-bf02e95e3b6a" + ] + }, + "AssignDefault": %t }`, assignDefault) } From 0bc49ff5ae1202bfb2853a6c080801738f54ff49 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 16 Apr 2025 19:14:11 -0500 Subject: [PATCH 138/433] test: fix flake in TestRoleSyncTable with test cases sharing resources (#17441) The test case definition shares maps that can have concurrent access if run in parallel. --- coderd/idpsync/role_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/coderd/idpsync/role_test.go b/coderd/idpsync/role_test.go index 7d686442144b1..d766ada6057f7 100644 --- a/coderd/idpsync/role_test.go +++ b/coderd/idpsync/role_test.go @@ -225,9 +225,8 @@ func TestRoleSyncTable(t *testing.T) { // deployment. This tests all organizations being synced together. // The reason we do them individually, is that it is much easier to // debug a single test case. + //nolint:paralleltest, tparallel // This should run after all the individual tests t.Run("AllTogether", func(t *testing.T) { - t.Parallel() - db, _ := dbtestutil.NewDB(t) manager := runtimeconfig.NewManager() s := idpsync.NewAGPLSync(slogtest.Make(t, &slogtest.Options{ From 6f5da1e2ee07a385d425240262999109a263dec1 Mon Sep 17 00:00:00 2001 From: M Atif Ali Date: Thu, 17 Apr 2025 12:09:46 +0500 Subject: [PATCH 139/433] chore: add windsurf icon (#17443) --- site/src/theme/icons.json | 1 + site/static/icon/windsurf.svg | 43 +++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) create mode 100644 site/static/icon/windsurf.svg diff --git a/site/src/theme/icons.json b/site/src/theme/icons.json index 7c7d8dac9e9db..a9307bfc78446 100644 --- a/site/src/theme/icons.json +++ b/site/src/theme/icons.json @@ -101,5 +101,6 @@ "vault.svg", "webstorm.svg", "widgets.svg", + "windsurf.svg", "zed.svg" ] diff --git a/site/static/icon/windsurf.svg b/site/static/icon/windsurf.svg new file mode 100644 index 0000000000000..a7684d4cb7862 --- /dev/null +++ b/site/static/icon/windsurf.svg @@ -0,0 +1,43 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 3b54254177599abddf91028381507893bdf250ff Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Thu, 17 Apr 2025 11:23:24 +0400 Subject: [PATCH 140/433] feat: add coder connect exists hidden subcommand (#17418) Adds a new hidden subcommand `coder connect exists ` that checks if the name exists via Coder Connect. This will be used in SSH config to match only if Coder Connect is unavailable for the hostname in question, so that the SSH client will directly dial the workspace over an existing Coder Connect tunnel. Also refactors the way we inject a test DNS resolver into the lookup functions so that we can test from outside the `workspacesdk` package. --- cli/configssh.go | 3 +- cli/connect.go | 47 ++++++++++ cli/connect_test.go | 76 ++++++++++++++++ cli/root.go | 12 ++- cli/root_test.go | 3 +- codersdk/workspacesdk/workspacesdk.go | 39 +++++++-- .../workspacesdk_internal_test.go | 86 ------------------- codersdk/workspacesdk/workspacesdk_test.go | 74 ++++++++++++++++ 8 files changed, 242 insertions(+), 98 deletions(-) create mode 100644 cli/connect.go create mode 100644 cli/connect_test.go delete mode 100644 codersdk/workspacesdk/workspacesdk_internal_test.go diff --git a/cli/configssh.go b/cli/configssh.go index 6a0f41c2a2fbc..c089141846d39 100644 --- a/cli/configssh.go +++ b/cli/configssh.go @@ -22,9 +22,10 @@ import ( "golang.org/x/exp/constraints" "golang.org/x/xerrors" + "github.com/coder/serpent" + "github.com/coder/coder/v2/cli/cliui" "github.com/coder/coder/v2/codersdk" - "github.com/coder/serpent" ) const ( diff --git a/cli/connect.go b/cli/connect.go new file mode 100644 index 0000000000000..d1245147f3848 --- /dev/null +++ b/cli/connect.go @@ -0,0 +1,47 @@ +package cli + +import ( + "github.com/coder/serpent" + + "github.com/coder/coder/v2/codersdk/workspacesdk" +) + +func (r *RootCmd) connectCmd() *serpent.Command { + cmd := &serpent.Command{ + Use: "connect", + Short: "Commands related to Coder Connect (OS-level tunneled connection to workspaces).", + Handler: func(i *serpent.Invocation) error { + return i.Command.HelpHandler(i) + }, + Hidden: true, + Children: []*serpent.Command{ + r.existsCmd(), + }, + } + return cmd +} + +func (*RootCmd) existsCmd() *serpent.Command { + cmd := &serpent.Command{ + Use: "exists ", + Short: "Checks if the given hostname exists via Coder Connect.", + Long: "This command is designed to be used in scripts to check if the given hostname exists via Coder " + + "Connect. It prints no output. It returns exit code 0 if it does exist and code 1 if it does not.", + Middleware: serpent.Chain( + serpent.RequireNArgs(1), + ), + Handler: func(inv *serpent.Invocation) error { + hostname := inv.Args[0] + exists, err := workspacesdk.ExistsViaCoderConnect(inv.Context(), hostname) + if err != nil { + return err + } + if !exists { + // we don't want to print any output, since this command is designed to be a check in scripts / SSH config. + return ErrSilent + } + return nil + }, + } + return cmd +} diff --git a/cli/connect_test.go b/cli/connect_test.go new file mode 100644 index 0000000000000..031cd2f95b1f9 --- /dev/null +++ b/cli/connect_test.go @@ -0,0 +1,76 @@ +package cli_test + +import ( + "bytes" + "context" + "net" + "testing" + + "github.com/stretchr/testify/require" + "tailscale.com/net/tsaddr" + + "github.com/coder/serpent" + + "github.com/coder/coder/v2/cli" + "github.com/coder/coder/v2/codersdk/workspacesdk" + "github.com/coder/coder/v2/testutil" +) + +func TestConnectExists_Running(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + + var root cli.RootCmd + cmd, err := root.Command(root.AGPL()) + require.NoError(t, err) + + inv := (&serpent.Invocation{ + Command: cmd, + Args: []string{"connect", "exists", "test.example"}, + }).WithContext(withCoderConnectRunning(ctx)) + stdout := new(bytes.Buffer) + stderr := new(bytes.Buffer) + inv.Stdout = stdout + inv.Stderr = stderr + err = inv.Run() + require.NoError(t, err) +} + +func TestConnectExists_NotRunning(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + + var root cli.RootCmd + cmd, err := root.Command(root.AGPL()) + require.NoError(t, err) + + inv := (&serpent.Invocation{ + Command: cmd, + Args: []string{"connect", "exists", "test.example"}, + }).WithContext(withCoderConnectNotRunning(ctx)) + stdout := new(bytes.Buffer) + stderr := new(bytes.Buffer) + inv.Stdout = stdout + inv.Stderr = stderr + err = inv.Run() + require.ErrorIs(t, err, cli.ErrSilent) +} + +type fakeResolver struct { + shouldReturnSuccess bool +} + +func (f *fakeResolver) LookupIP(_ context.Context, _, _ string) ([]net.IP, error) { + if f.shouldReturnSuccess { + return []net.IP{net.ParseIP(tsaddr.CoderServiceIPv6().String())}, nil + } + return nil, &net.DNSError{IsNotFound: true} +} + +func withCoderConnectRunning(ctx context.Context) context.Context { + return workspacesdk.WithTestOnlyCoderContextResolver(ctx, &fakeResolver{shouldReturnSuccess: true}) +} + +func withCoderConnectNotRunning(ctx context.Context) context.Context { + return workspacesdk.WithTestOnlyCoderContextResolver(ctx, &fakeResolver{shouldReturnSuccess: false}) +} diff --git a/cli/root.go b/cli/root.go index 75cbb4dd2ca1a..5c70379b75a44 100644 --- a/cli/root.go +++ b/cli/root.go @@ -31,6 +31,8 @@ import ( "github.com/coder/pretty" + "github.com/coder/serpent" + "github.com/coder/coder/v2/buildinfo" "github.com/coder/coder/v2/cli/cliui" "github.com/coder/coder/v2/cli/config" @@ -38,7 +40,6 @@ import ( "github.com/coder/coder/v2/cli/telemetry" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/agentsdk" - "github.com/coder/serpent" ) var ( @@ -49,6 +50,10 @@ var ( workspaceCommand = map[string]string{ "workspaces": "", } + + // ErrSilent is a sentinel error that tells the command handler to just exit with a non-zero error, but not print + // anything. + ErrSilent = xerrors.New("silent error") ) const ( @@ -122,6 +127,7 @@ func (r *RootCmd) CoreSubcommands() []*serpent.Command { r.whoami(), // Hidden + r.connectCmd(), r.expCmd(), r.gitssh(), r.support(), @@ -175,6 +181,10 @@ func (r *RootCmd) RunWithSubcommands(subcommands []*serpent.Command) { //nolint:revive,gocritic os.Exit(code) } + if errors.Is(err, ErrSilent) { + //nolint:revive,gocritic + os.Exit(code) + } f := PrettyErrorFormatter{w: os.Stderr, verbose: r.verbose} if err != nil { f.Format(err) diff --git a/cli/root_test.go b/cli/root_test.go index ac1454152672e..698c9aff60186 100644 --- a/cli/root_test.go +++ b/cli/root_test.go @@ -10,12 +10,13 @@ import ( "sync/atomic" "testing" + "github.com/coder/serpent" + "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/pty/ptytest" "github.com/coder/coder/v2/testutil" - "github.com/coder/serpent" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" diff --git a/codersdk/workspacesdk/workspacesdk.go b/codersdk/workspacesdk/workspacesdk.go index 25188917dafc9..83f236a215b56 100644 --- a/codersdk/workspacesdk/workspacesdk.go +++ b/codersdk/workspacesdk/workspacesdk.go @@ -20,11 +20,12 @@ import ( "cdr.dev/slog" + "github.com/coder/quartz" + "github.com/coder/websocket" + "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/tailnet" "github.com/coder/coder/v2/tailnet/proto" - "github.com/coder/quartz" - "github.com/coder/websocket" ) var ErrSkipClose = xerrors.New("skip tailnet close") @@ -128,19 +129,16 @@ func init() { } } -type resolver interface { +type Resolver interface { LookupIP(ctx context.Context, network, host string) ([]net.IP, error) } type Client struct { client *codersdk.Client - - // overridden in tests - resolver resolver } func New(c *codersdk.Client) *Client { - return &Client{client: c, resolver: net.DefaultResolver} + return &Client{client: c} } // AgentConnectionInfo returns required information for establishing @@ -392,6 +390,12 @@ func (c *Client) AgentReconnectingPTY(ctx context.Context, opts WorkspaceAgentRe return websocket.NetConn(context.Background(), conn, websocket.MessageBinary), nil } +func WithTestOnlyCoderContextResolver(ctx context.Context, r Resolver) context.Context { + return context.WithValue(ctx, dnsResolverContextKey{}, r) +} + +type dnsResolverContextKey struct{} + type CoderConnectQueryOptions struct { HostnameSuffix string } @@ -409,15 +413,32 @@ func (c *Client) IsCoderConnectRunning(ctx context.Context, o CoderConnectQueryO suffix = info.HostnameSuffix } domainName := fmt.Sprintf(tailnet.IsCoderConnectEnabledFmtString, suffix) + return ExistsViaCoderConnect(ctx, domainName) +} + +func testOrDefaultResolver(ctx context.Context) Resolver { + // check the context for a non-default resolver. This is only used in testing. + resolver, ok := ctx.Value(dnsResolverContextKey{}).(Resolver) + if !ok || resolver == nil { + resolver = net.DefaultResolver + } + return resolver +} + +// ExistsViaCoderConnect checks if the given hostname exists via Coder Connect. This doesn't guarantee the +// workspace is actually reachable, if, for example, its agent is unhealthy, but rather that Coder Connect knows about +// the workspace and advertises the hostname via DNS. +func ExistsViaCoderConnect(ctx context.Context, hostname string) (bool, error) { + resolver := testOrDefaultResolver(ctx) var dnsError *net.DNSError - ips, err := c.resolver.LookupIP(ctx, "ip6", domainName) + ips, err := resolver.LookupIP(ctx, "ip6", hostname) if xerrors.As(err, &dnsError) { if dnsError.IsNotFound { return false, nil } } if err != nil { - return false, xerrors.Errorf("lookup DNS %s: %w", domainName, err) + return false, xerrors.Errorf("lookup DNS %s: %w", hostname, err) } // The returned IP addresses are probably from the Coder Connect DNS server, but there are sometimes weird captive diff --git a/codersdk/workspacesdk/workspacesdk_internal_test.go b/codersdk/workspacesdk/workspacesdk_internal_test.go deleted file mode 100644 index 1b98ebdc2e671..0000000000000 --- a/codersdk/workspacesdk/workspacesdk_internal_test.go +++ /dev/null @@ -1,86 +0,0 @@ -package workspacesdk - -import ( - "context" - "fmt" - "net" - "net/http" - "net/http/httptest" - "net/url" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "golang.org/x/xerrors" - - "github.com/coder/coder/v2/coderd/httpapi" - "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/testutil" - - "tailscale.com/net/tsaddr" - - "github.com/coder/coder/v2/tailnet" -) - -func TestClient_IsCoderConnectRunning(t *testing.T) { - t.Parallel() - ctx := testutil.Context(t, testutil.WaitShort) - - srv := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/api/v2/workspaceagents/connection", r.URL.Path) - httpapi.Write(ctx, rw, http.StatusOK, AgentConnectionInfo{ - HostnameSuffix: "test", - }) - })) - defer srv.Close() - - apiURL, err := url.Parse(srv.URL) - require.NoError(t, err) - sdkClient := codersdk.New(apiURL) - client := New(sdkClient) - - // Right name, right IP - expectedName := fmt.Sprintf(tailnet.IsCoderConnectEnabledFmtString, "test") - client.resolver = &fakeResolver{t: t, hostMap: map[string][]net.IP{ - expectedName: {net.ParseIP(tsaddr.CoderServiceIPv6().String())}, - }} - - result, err := client.IsCoderConnectRunning(ctx, CoderConnectQueryOptions{}) - require.NoError(t, err) - require.True(t, result) - - // Wrong name - result, err = client.IsCoderConnectRunning(ctx, CoderConnectQueryOptions{HostnameSuffix: "coder"}) - require.NoError(t, err) - require.False(t, result) - - // Not found - client.resolver = &fakeResolver{t: t, err: &net.DNSError{IsNotFound: true}} - result, err = client.IsCoderConnectRunning(ctx, CoderConnectQueryOptions{}) - require.NoError(t, err) - require.False(t, result) - - // Some other error - client.resolver = &fakeResolver{t: t, err: xerrors.New("a bad thing happened")} - _, err = client.IsCoderConnectRunning(ctx, CoderConnectQueryOptions{}) - require.Error(t, err) - - // Right name, wrong IP - client.resolver = &fakeResolver{t: t, hostMap: map[string][]net.IP{ - expectedName: {net.ParseIP("2001::34")}, - }} - result, err = client.IsCoderConnectRunning(ctx, CoderConnectQueryOptions{}) - require.NoError(t, err) - require.False(t, result) -} - -type fakeResolver struct { - t testing.TB - hostMap map[string][]net.IP - err error -} - -func (f *fakeResolver) LookupIP(_ context.Context, network, host string) ([]net.IP, error) { - assert.Equal(f.t, "ip6", network) - return f.hostMap[host], f.err -} diff --git a/codersdk/workspacesdk/workspacesdk_test.go b/codersdk/workspacesdk/workspacesdk_test.go index e7ccd96e208fa..16a523b2d4d53 100644 --- a/codersdk/workspacesdk/workspacesdk_test.go +++ b/codersdk/workspacesdk/workspacesdk_test.go @@ -1,12 +1,18 @@ package workspacesdk_test import ( + "context" + "fmt" + "net" "net/http" "net/http/httptest" "net/url" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/xerrors" + "tailscale.com/net/tsaddr" "tailscale.com/tailcfg" "github.com/coder/websocket" @@ -15,6 +21,7 @@ import ( "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/agentsdk" "github.com/coder/coder/v2/codersdk/workspacesdk" + "github.com/coder/coder/v2/tailnet" "github.com/coder/coder/v2/testutil" ) @@ -72,3 +79,70 @@ func TestWorkspaceDialerFailure(t *testing.T) { // Then: an error indicating a database issue is returned, to conditionalize the behavior of the caller. require.ErrorIs(t, err, codersdk.ErrDatabaseNotReachable) } + +func TestClient_IsCoderConnectRunning(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + + srv := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/api/v2/workspaceagents/connection", r.URL.Path) + httpapi.Write(ctx, rw, http.StatusOK, workspacesdk.AgentConnectionInfo{ + HostnameSuffix: "test", + }) + })) + defer srv.Close() + + apiURL, err := url.Parse(srv.URL) + require.NoError(t, err) + sdkClient := codersdk.New(apiURL) + client := workspacesdk.New(sdkClient) + + // Right name, right IP + expectedName := fmt.Sprintf(tailnet.IsCoderConnectEnabledFmtString, "test") + ctxResolveExpected := workspacesdk.WithTestOnlyCoderContextResolver(ctx, + &fakeResolver{t: t, hostMap: map[string][]net.IP{ + expectedName: {net.ParseIP(tsaddr.CoderServiceIPv6().String())}, + }}) + + result, err := client.IsCoderConnectRunning(ctxResolveExpected, workspacesdk.CoderConnectQueryOptions{}) + require.NoError(t, err) + require.True(t, result) + + // Wrong name + result, err = client.IsCoderConnectRunning(ctxResolveExpected, workspacesdk.CoderConnectQueryOptions{HostnameSuffix: "coder"}) + require.NoError(t, err) + require.False(t, result) + + // Not found + ctxResolveNotFound := workspacesdk.WithTestOnlyCoderContextResolver(ctx, + &fakeResolver{t: t, err: &net.DNSError{IsNotFound: true}}) + result, err = client.IsCoderConnectRunning(ctxResolveNotFound, workspacesdk.CoderConnectQueryOptions{}) + require.NoError(t, err) + require.False(t, result) + + // Some other error + ctxResolverErr := workspacesdk.WithTestOnlyCoderContextResolver(ctx, + &fakeResolver{t: t, err: xerrors.New("a bad thing happened")}) + _, err = client.IsCoderConnectRunning(ctxResolverErr, workspacesdk.CoderConnectQueryOptions{}) + require.Error(t, err) + + // Right name, wrong IP + ctxResolverWrongIP := workspacesdk.WithTestOnlyCoderContextResolver(ctx, + &fakeResolver{t: t, hostMap: map[string][]net.IP{ + expectedName: {net.ParseIP("2001::34")}, + }}) + result, err = client.IsCoderConnectRunning(ctxResolverWrongIP, workspacesdk.CoderConnectQueryOptions{}) + require.NoError(t, err) + require.False(t, result) +} + +type fakeResolver struct { + t testing.TB + hostMap map[string][]net.IP + err error +} + +func (f *fakeResolver) LookupIP(_ context.Context, network, host string) ([]net.IP, error) { + assert.Equal(f.t, "ip6", network) + return f.hostMap[host], f.err +} From b0854aa97139f05301dfc89aff5e0e76fce6ce90 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Thu, 17 Apr 2025 12:04:00 +0400 Subject: [PATCH 141/433] feat: modify config-ssh to check for Coder Connect (#17419) relates to #16828 Changes SSH config so that suffixes only match if Coder Connect is not running / available. This means that we will use the existing Coder Connect tunnel if it is available, rather than creating a new tunnel via `coder ssh --stdio`. --- cli/configssh.go | 283 +++++++++++++++++++++++------------------- cli/configssh_test.go | 15 ++- 2 files changed, 166 insertions(+), 132 deletions(-) diff --git a/cli/configssh.go b/cli/configssh.go index c089141846d39..e90c8080abb0d 100644 --- a/cli/configssh.go +++ b/cli/configssh.go @@ -48,13 +48,17 @@ const ( type sshConfigOptions struct { waitEnum string // Deprecated: moving away from prefix to hostnameSuffix - userHostPrefix string - hostnameSuffix string - sshOptions []string - disableAutostart bool - header []string - headerCommand string - removedKeys map[string]bool + userHostPrefix string + hostnameSuffix string + sshOptions []string + disableAutostart bool + header []string + headerCommand string + removedKeys map[string]bool + globalConfigPath string + coderBinaryPath string + skipProxyCommand bool + forceUnixSeparators bool } // addOptions expects options in the form of "option=value" or "option value". @@ -107,6 +111,80 @@ func (o sshConfigOptions) equal(other sshConfigOptions) bool { o.hostnameSuffix == other.hostnameSuffix } +func (o sshConfigOptions) writeToBuffer(buf *bytes.Buffer) error { + escapedCoderBinary, err := sshConfigExecEscape(o.coderBinaryPath, o.forceUnixSeparators) + if err != nil { + return xerrors.Errorf("escape coder binary for ssh failed: %w", err) + } + + escapedGlobalConfig, err := sshConfigExecEscape(o.globalConfigPath, o.forceUnixSeparators) + if err != nil { + return xerrors.Errorf("escape global config for ssh failed: %w", err) + } + + rootFlags := fmt.Sprintf("--global-config %s", escapedGlobalConfig) + for _, h := range o.header { + rootFlags += fmt.Sprintf(" --header %q", h) + } + if o.headerCommand != "" { + rootFlags += fmt.Sprintf(" --header-command %q", o.headerCommand) + } + + flags := "" + if o.waitEnum != "auto" { + flags += " --wait=" + o.waitEnum + } + if o.disableAutostart { + flags += " --disable-autostart=true" + } + + // Prefix block: + if o.userHostPrefix != "" { + _, _ = buf.WriteString("Host") + + _, _ = buf.WriteString(" ") + _, _ = buf.WriteString(o.userHostPrefix) + _, _ = buf.WriteString("*\n") + + for _, v := range o.sshOptions { + _, _ = buf.WriteString("\t") + _, _ = buf.WriteString(v) + _, _ = buf.WriteString("\n") + } + if !o.skipProxyCommand && o.userHostPrefix != "" { + _, _ = buf.WriteString("\t") + _, _ = fmt.Fprintf(buf, + "ProxyCommand %s %s ssh --stdio%s --ssh-host-prefix %s %%h", + escapedCoderBinary, rootFlags, flags, o.userHostPrefix, + ) + _, _ = buf.WriteString("\n") + } + } + + // Suffix block + if o.hostnameSuffix == "" { + return nil + } + _, _ = fmt.Fprintf(buf, "\nHost *.%s\n", o.hostnameSuffix) + for _, v := range o.sshOptions { + _, _ = buf.WriteString("\t") + _, _ = buf.WriteString(v) + _, _ = buf.WriteString("\n") + } + // the ^^ options should always apply, but we only want to use the proxy command if Coder Connect is not running. + if !o.skipProxyCommand { + _, _ = fmt.Fprintf(buf, "\nMatch host *.%s !exec \"%s connect exists %%h\"\n", + o.hostnameSuffix, escapedCoderBinary) + _, _ = buf.WriteString("\t") + _, _ = fmt.Fprintf(buf, + "ProxyCommand %s %s ssh --stdio%s --hostname-suffix %s %%h", + escapedCoderBinary, rootFlags, flags, o.hostnameSuffix, + ) + _, _ = buf.WriteString("\n") + } + return nil +} + // slicesSortedEqual compares two slices without side-effects or regard to order. func slicesSortedEqual[S ~[]E, E constraints.Ordered](a, b S) bool { if len(a) != len(b) { @@ -147,13 +225,11 @@ func (o sshConfigOptions) asList() (list []string) { func (r *RootCmd) configSSH() *serpent.Command { var ( - sshConfigFile string - sshConfigOpts sshConfigOptions - usePreviousOpts bool - dryRun bool - skipProxyCommand bool - forceUnixSeparators bool - coderCliPath string + sshConfigFile string + sshConfigOpts sshConfigOptions + usePreviousOpts bool + dryRun bool + coderCliPath string ) client := new(codersdk.Client) cmd := &serpent.Command{ @@ -177,7 +253,7 @@ func (r *RootCmd) configSSH() *serpent.Command { Handler: func(inv *serpent.Invocation) error { ctx := inv.Context() - if sshConfigOpts.waitEnum != "auto" && skipProxyCommand { + if sshConfigOpts.waitEnum != "auto" && sshConfigOpts.skipProxyCommand { // The wait option is applied to the ProxyCommand. If the user // specifies skip-proxy-command, then wait cannot be applied. return xerrors.Errorf("cannot specify both --skip-proxy-command and --wait") @@ -207,18 +283,7 @@ func (r *RootCmd) configSSH() *serpent.Command { return err } } - - escapedCoderBinary, err := sshConfigExecEscape(coderBinary, forceUnixSeparators) - if err != nil { - return xerrors.Errorf("escape coder binary for ssh failed: %w", err) - } - root := r.createConfig() - escapedGlobalConfig, err := sshConfigExecEscape(string(root), forceUnixSeparators) - if err != nil { - return xerrors.Errorf("escape global config for ssh failed: %w", err) - } - homedir, err := os.UserHomeDir() if err != nil { return xerrors.Errorf("user home dir failed: %w", err) @@ -320,94 +385,15 @@ func (r *RootCmd) configSSH() *serpent.Command { coderdConfig.HostnamePrefix = "coder." } - if sshConfigOpts.userHostPrefix != "" { - // Override with user flag. - coderdConfig.HostnamePrefix = sshConfigOpts.userHostPrefix - } - if sshConfigOpts.hostnameSuffix != "" { - // Override with user flag. - coderdConfig.HostnameSuffix = sshConfigOpts.hostnameSuffix - } - - // Write agent configuration. - defaultOptions := []string{ - "ConnectTimeout=0", - "StrictHostKeyChecking=no", - // Without this, the "REMOTE HOST IDENTITY CHANGED" - // message will appear. - "UserKnownHostsFile=/dev/null", - // This disables the "Warning: Permanently added 'hostname' (RSA) to the list of known hosts." - // message from appearing on every SSH. This happens because we ignore the known hosts. - "LogLevel ERROR", - } - - if !skipProxyCommand { - rootFlags := fmt.Sprintf("--global-config %s", escapedGlobalConfig) - for _, h := range sshConfigOpts.header { - rootFlags += fmt.Sprintf(" --header %q", h) - } - if sshConfigOpts.headerCommand != "" { - rootFlags += fmt.Sprintf(" --header-command %q", sshConfigOpts.headerCommand) - } - - flags := "" - if sshConfigOpts.waitEnum != "auto" { - flags += " --wait=" + sshConfigOpts.waitEnum - } - if sshConfigOpts.disableAutostart { - flags += " --disable-autostart=true" - } - if coderdConfig.HostnamePrefix != "" { - flags += " --ssh-host-prefix " + coderdConfig.HostnamePrefix - } - if coderdConfig.HostnameSuffix != "" { - flags += " --hostname-suffix " + coderdConfig.HostnameSuffix - } - defaultOptions = append(defaultOptions, fmt.Sprintf( - "ProxyCommand %s %s ssh --stdio%s %%h", - escapedCoderBinary, rootFlags, flags, - )) - } - - // Create a copy of the options so we can modify them. - configOptions := sshConfigOpts - configOptions.sshOptions = nil - - // User options first (SSH only uses the first - // option unless it can be given multiple times) - for _, opt := range sshConfigOpts.sshOptions { - err := configOptions.addOptions(opt) - if err != nil { - return xerrors.Errorf("add flag config option %q: %w", opt, err) - } - } - - // Deployment options second, allow them to - // override standard options. - for k, v := range coderdConfig.SSHConfigOptions { - opt := fmt.Sprintf("%s %s", k, v) - err := configOptions.addOptions(opt) - if err != nil { - return xerrors.Errorf("add coderd config option %q: %w", opt, err) - } - } - - // Finally, add the standard options. - if err := configOptions.addOptions(defaultOptions...); err != nil { + configOptions, err := mergeSSHOptions(sshConfigOpts, coderdConfig, string(root), coderBinary) + if err != nil { return err } - - hostBlock := []string{ - sshConfigHostLinePatterns(coderdConfig), - } - // Prefix with '\t' - for _, v := range configOptions.sshOptions { - hostBlock = append(hostBlock, "\t"+v) + err = configOptions.writeToBuffer(buf) + if err != nil { + return err } - _, _ = buf.WriteString(strings.Join(hostBlock, "\n")) - _ = buf.WriteByte('\n') - sshConfigWriteSectionEnd(buf) // Write the remainder of the users config file to buf. @@ -523,7 +509,7 @@ func (r *RootCmd) configSSH() *serpent.Command { Flag: "skip-proxy-command", Env: "CODER_SSH_SKIP_PROXY_COMMAND", Description: "Specifies whether the ProxyCommand option should be skipped. Useful for testing.", - Value: serpent.BoolOf(&skipProxyCommand), + Value: serpent.BoolOf(&sshConfigOpts.skipProxyCommand), Hidden: true, }, { @@ -564,7 +550,7 @@ func (r *RootCmd) configSSH() *serpent.Command { Description: "By default, 'config-ssh' uses the os path separator when writing the ssh config. " + "This might be an issue in Windows machine that use a unix-like shell. " + "This flag forces the use of unix file paths (the forward slash '/').", - Value: serpent.BoolOf(&forceUnixSeparators), + Value: serpent.BoolOf(&sshConfigOpts.forceUnixSeparators), // On non-windows showing this command is useless because it is a noop. // Hide vs disable it though so if a command is copied from a Windows // machine to a unix machine it will still work and not throw an @@ -577,6 +563,63 @@ func (r *RootCmd) configSSH() *serpent.Command { return cmd } +func mergeSSHOptions( + user sshConfigOptions, coderd codersdk.SSHConfigResponse, globalConfigPath, coderBinaryPath string, +) ( + sshConfigOptions, error, +) { + // Write agent configuration. + defaultOptions := []string{ + "ConnectTimeout=0", + "StrictHostKeyChecking=no", + // Without this, the "REMOTE HOST IDENTITY CHANGED" + // message will appear. + "UserKnownHostsFile=/dev/null", + // This disables the "Warning: Permanently added 'hostname' (RSA) to the list of known hosts." + // message from appearing on every SSH. This happens because we ignore the known hosts. + "LogLevel ERROR", + } + + // Create a copy of the options so we can modify them. + configOptions := user + configOptions.sshOptions = nil + + configOptions.globalConfigPath = globalConfigPath + configOptions.coderBinaryPath = coderBinaryPath + // user config takes precedence + if user.userHostPrefix == "" { + configOptions.userHostPrefix = coderd.HostnamePrefix + } + if user.hostnameSuffix == "" { + configOptions.hostnameSuffix = coderd.HostnameSuffix + } + + // User options first (SSH only uses the first + // option unless it can be given multiple times) + for _, opt := range user.sshOptions { + err := configOptions.addOptions(opt) + if err != nil { + return sshConfigOptions{}, xerrors.Errorf("add flag config option %q: %w", opt, err) + } + } + + // Deployment options second, allow them to + // override standard options. + for k, v := range coderd.SSHConfigOptions { + opt := fmt.Sprintf("%s %s", k, v) + err := configOptions.addOptions(opt) + if err != nil { + return sshConfigOptions{}, xerrors.Errorf("add coderd config option %q: %w", opt, err) + } + } + + // Finally, add the standard options. + if err := configOptions.addOptions(defaultOptions...); err != nil { + return sshConfigOptions{}, err + } + return configOptions, nil +} + //nolint:revive func sshConfigWriteSectionHeader(w io.Writer, addNewline bool, o sshConfigOptions) { nl := "\n" @@ -844,19 +887,3 @@ func diffBytes(name string, b1, b2 []byte, color bool) ([]byte, error) { } return b, nil } - -func sshConfigHostLinePatterns(config codersdk.SSHConfigResponse) string { - builder := strings.Builder{} - // by inspection, WriteString always returns nil error - _, _ = builder.WriteString("Host") - if config.HostnamePrefix != "" { - _, _ = builder.WriteString(" ") - _, _ = builder.WriteString(config.HostnamePrefix) - _, _ = builder.WriteString("*") - } - if config.HostnameSuffix != "" { - _, _ = builder.WriteString(" *.") - _, _ = builder.WriteString(config.HostnameSuffix) - } - return builder.String() -} diff --git a/cli/configssh_test.go b/cli/configssh_test.go index b42241b6b3aad..72faaa00c1ca0 100644 --- a/cli/configssh_test.go +++ b/cli/configssh_test.go @@ -615,13 +615,21 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { name: "Hostname Suffix", args: []string{ "--yes", + "--ssh-option", "Foo=bar", "--hostname-suffix", "testy", }, wantErr: false, hasAgent: true, wantConfig: wantConfig{ - ssh: []string{"Host coder.* *.testy"}, - regexMatch: `ProxyCommand .* ssh .* --hostname-suffix testy %h`, + ssh: []string{ + "Host *.testy", + "Foo=bar", + "ConnectTimeout=0", + "StrictHostKeyChecking=no", + "UserKnownHostsFile=/dev/null", + "LogLevel ERROR", + }, + regexMatch: `Match host \*\.testy !exec ".* connect exists %h"\n\tProxyCommand .* ssh .* --hostname-suffix testy %h`, }, }, { @@ -634,8 +642,7 @@ func TestConfigSSH_FileWriteAndOptionsFlow(t *testing.T) { wantErr: false, hasAgent: true, wantConfig: wantConfig{ - ssh: []string{"Host presto.* *.testy"}, - regexMatch: `ProxyCommand .* ssh .* --ssh-host-prefix presto\. --hostname-suffix testy %h`, + ssh: []string{"Host presto.*", "Match host *.testy !exec"}, }, }, } From 9fe3fd4e2875f96850ab6b473e763cc3dd3e3ccd Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Thu, 17 Apr 2025 12:16:29 +0400 Subject: [PATCH 142/433] chore: change config-ssh Call to Action to use suffix (#17445) fixes #16828 With all the recent changes, I believe it is now safe to change the Call to Action for `config-ssh` to use the hostname suffix rather than prefix if it was set. --- cli/configssh.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cli/configssh.go b/cli/configssh.go index e90c8080abb0d..65f36697d873f 100644 --- a/cli/configssh.go +++ b/cli/configssh.go @@ -457,7 +457,11 @@ func (r *RootCmd) configSSH() *serpent.Command { if len(res.Workspaces) > 0 { _, _ = fmt.Fprintln(out, "You should now be able to ssh into your workspace.") - _, _ = fmt.Fprintf(out, "For example, try running:\n\n\t$ ssh %s%s\n", coderdConfig.HostnamePrefix, res.Workspaces[0].Name) + if configOptions.hostnameSuffix != "" { + _, _ = fmt.Fprintf(out, "For example, try running:\n\n\t$ ssh %s.%s\n", res.Workspaces[0].Name, configOptions.hostnameSuffix) + } else if configOptions.userHostPrefix != "" { + _, _ = fmt.Fprintf(out, "For example, try running:\n\n\t$ ssh %s%s\n", configOptions.userHostPrefix, res.Workspaces[0].Name) + } } else { _, _ = fmt.Fprint(out, "You don't have any workspaces yet, try creating one with:\n\n\t$ coder create \n") } From 67a912796a716c757c9e458bec601ed3f4de367f Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 17 Apr 2025 11:27:18 +0100 Subject: [PATCH 143/433] feat: add slider component (#17431) The slider component is part of the components supported by Dynamic Parameters There are no Figma designs for the slider component. This is based on the shadcn slider. Screenshot 2025-04-16 at 19 26 11 --- site/src/components/Slider/Slider.stories.tsx | 57 +++++++++++++++++++ site/src/components/Slider/Slider.tsx | 39 +++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 site/src/components/Slider/Slider.stories.tsx create mode 100644 site/src/components/Slider/Slider.tsx diff --git a/site/src/components/Slider/Slider.stories.tsx b/site/src/components/Slider/Slider.stories.tsx new file mode 100644 index 0000000000000..480e12c090382 --- /dev/null +++ b/site/src/components/Slider/Slider.stories.tsx @@ -0,0 +1,57 @@ +import type { Meta, StoryObj } from "@storybook/react"; +import React from "react"; +import { Slider } from "./Slider"; + +const meta: Meta = { + title: "components/Slider", + component: Slider, + args: {}, + argTypes: { + value: { + control: "number", + description: "The controlled value of the slider", + }, + defaultValue: { + control: "number", + description: "The default value when initially rendered", + }, + disabled: { + control: "boolean", + description: + "When true, prevents the user from interacting with the slider", + }, + }, +}; + +export default meta; +type Story = StoryObj; + +export const Default: Story = {}; + +export const Controlled: Story = { + render: (args) => { + const [value, setValue] = React.useState(50); + return ( + setValue(v)} /> + ); + }, + args: { value: [50], min: 0, max: 100, step: 1 }, +}; + +export const Uncontrolled: Story = { + args: { defaultValue: [30], min: 0, max: 100, step: 1 }, +}; + +export const Disabled: Story = { + args: { defaultValue: [40], disabled: true }, +}; + +export const MultipleThumbs: Story = { + args: { + defaultValue: [20, 80], + min: 0, + max: 100, + step: 5, + minStepsBetweenThumbs: 1, + }, +}; diff --git a/site/src/components/Slider/Slider.tsx b/site/src/components/Slider/Slider.tsx new file mode 100644 index 0000000000000..4fdd21353e963 --- /dev/null +++ b/site/src/components/Slider/Slider.tsx @@ -0,0 +1,39 @@ +/** + * Copied from shadc/ui on 04/16/2025 + * @see {@link https://ui.shadcn.com/docs/components/slider} + */ +import * as SliderPrimitive from "@radix-ui/react-slider"; +import * as React from "react"; + +import { cn } from "utils/cn"; + +export const Slider = React.forwardRef< + React.ElementRef, + React.ComponentPropsWithoutRef +>(({ className, ...props }, ref) => ( + + + + + + + +)); From daafa0d689518122805ad848cb596035d25ceb3a Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Thu, 17 Apr 2025 13:50:18 +0200 Subject: [PATCH 144/433] chore: add missing prometheus tests for UNKNOWN/STATIC paths (#17446) --- coderd/httpmw/prometheus_test.go | 58 ++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/coderd/httpmw/prometheus_test.go b/coderd/httpmw/prometheus_test.go index d40558f5ca5e7..e05ae53d3836c 100644 --- a/coderd/httpmw/prometheus_test.go +++ b/coderd/httpmw/prometheus_test.go @@ -106,6 +106,64 @@ func TestPrometheus(t *testing.T) { require.Equal(t, "/api/v2/users/{user}", concurrentRequests["path"]) require.Equal(t, "GET", concurrentRequests["method"]) }) + + t.Run("StaticRoute", func(t *testing.T) { + t.Parallel() + reg := prometheus.NewRegistry() + promMW := httpmw.Prometheus(reg) + + r := chi.NewRouter() + r.Use(promMW) + r.NotFound(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + r.Get("/static/", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + }) + + req := httptest.NewRequest("GET", "/static/bundle.js", nil) + sw := &tracing.StatusWriter{ResponseWriter: httptest.NewRecorder()} + + r.ServeHTTP(sw, req) + + metrics, err := reg.Gather() + require.NoError(t, err) + require.Greater(t, len(metrics), 0) + metricLabels := getMetricLabels(metrics) + + reqProcessed, ok := metricLabels["coderd_api_requests_processed_total"] + require.True(t, ok, "coderd_api_requests_processed_total metric not found") + require.Equal(t, "STATIC", reqProcessed["path"]) + require.Equal(t, "GET", reqProcessed["method"]) + }) + + t.Run("UnknownRoute", func(t *testing.T) { + t.Parallel() + reg := prometheus.NewRegistry() + promMW := httpmw.Prometheus(reg) + + r := chi.NewRouter() + r.Use(promMW) + r.NotFound(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + r.Get("/api/v2/users/{user}", func(w http.ResponseWriter, r *http.Request) {}) + + req := httptest.NewRequest("GET", "/api/v2/weird_path", nil) + sw := &tracing.StatusWriter{ResponseWriter: httptest.NewRecorder()} + + r.ServeHTTP(sw, req) + + metrics, err := reg.Gather() + require.NoError(t, err) + require.Greater(t, len(metrics), 0) + metricLabels := getMetricLabels(metrics) + + reqProcessed, ok := metricLabels["coderd_api_requests_processed_total"] + require.True(t, ok, "coderd_api_requests_processed_total metric not found") + require.Equal(t, "UNKNOWN", reqProcessed["path"]) + require.Equal(t, "GET", reqProcessed["method"]) + }) } func getMetricLabels(metrics []*cm.MetricFamily) map[string]map[string]string { From b3aba6dab7fcba75a2f33b1f071051ce081ea737 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Thu, 17 Apr 2025 16:17:19 +0400 Subject: [PATCH 145/433] test: ignore context.Canceled in acquireWithCancel (#17448) fixes https://github.com/coder/internal/issues/584 Ignore canceled error when sending an acquired job, since dRPC is racy and will sometimes return this error even after successfully sending the job, if the test is quickly finished. --- provisionerd/provisionerd_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/provisionerd/provisionerd_test.go b/provisionerd/provisionerd_test.go index 8d5ba1621b8b7..c711e0d4925c8 100644 --- a/provisionerd/provisionerd_test.go +++ b/provisionerd/provisionerd_test.go @@ -1270,6 +1270,11 @@ func (a *acquireOne) acquireWithCancel(stream proto.DRPCProvisionerDaemon_Acquir return nil } err := stream.Send(a.job) - assert.NoError(a.t, err) + // dRPC is racy, and sometimes will return context.Canceled after it has successfully sent the message if we cancel + // right away, e.g. in unit tests that complete. So, just swallow the error in that case. If we are canceled before + // the job was acquired, presumably something else in the test will have failed. + if !xerrors.Is(err, context.Canceled) { + assert.NoError(a.t, err) + } return nil } From 6a79965948d49f3e9b0133b7994c953f382b6354 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 17 Apr 2025 13:50:51 +0100 Subject: [PATCH 146/433] fix(agent/agentcontainers): handle race between docker ps and docker inspect (#17447) Fixes https://github.com/coder/internal/issues/586#event-17291038671 --- agent/agentcontainers/containers_dockercli.go | 31 ++++++++++--------- agent/agentcontainers/devcontainercli_test.go | 3 ++ 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/agent/agentcontainers/containers_dockercli.go b/agent/agentcontainers/containers_dockercli.go index 208c3ec2ea89b..d5499f6b1af2b 100644 --- a/agent/agentcontainers/containers_dockercli.go +++ b/agent/agentcontainers/containers_dockercli.go @@ -24,19 +24,6 @@ import ( "github.com/coder/coder/v2/codersdk" ) -// DockerCLILister is a ContainerLister that lists containers using the docker CLI -type DockerCLILister struct { - execer agentexec.Execer -} - -var _ Lister = &DockerCLILister{} - -func NewDocker(execer agentexec.Execer) Lister { - return &DockerCLILister{ - execer: agentexec.DefaultExecer, - } -} - // DockerEnvInfoer is an implementation of agentssh.EnvInfoer that returns // information about a container. type DockerEnvInfoer struct { @@ -241,6 +228,19 @@ func run(ctx context.Context, execer agentexec.Execer, cmd string, args ...strin return stdout, stderr, err } +// DockerCLILister is a ContainerLister that lists containers using the docker CLI +type DockerCLILister struct { + execer agentexec.Execer +} + +var _ Lister = &DockerCLILister{} + +func NewDocker(execer agentexec.Execer) Lister { + return &DockerCLILister{ + execer: agentexec.DefaultExecer, + } +} + func (dcl *DockerCLILister) List(ctx context.Context) (codersdk.WorkspaceAgentListContainersResponse, error) { var stdoutBuf, stderrBuf bytes.Buffer // List all container IDs, one per line, with no truncation @@ -319,9 +319,12 @@ func runDockerInspect(ctx context.Context, execer agentexec.Execer, ids ...strin stdout = bytes.TrimSpace(stdoutBuf.Bytes()) stderr = bytes.TrimSpace(stderrBuf.Bytes()) if err != nil { + if bytes.Contains(stderr, []byte("No such object:")) { + // This can happen if a container is deleted between the time we check for its existence and the time we inspect it. + return stdout, stderr, nil + } return stdout, stderr, err } - return stdout, stderr, nil } diff --git a/agent/agentcontainers/devcontainercli_test.go b/agent/agentcontainers/devcontainercli_test.go index 22a81fb8e38a2..d768b997cc1e1 100644 --- a/agent/agentcontainers/devcontainercli_test.go +++ b/agent/agentcontainers/devcontainercli_test.go @@ -229,6 +229,9 @@ func TestDockerDevcontainerCLI(t *testing.T) { if os.Getenv("CODER_TEST_USE_DOCKER") != "1" { t.Skip("skipping Docker test; set CODER_TEST_USE_DOCKER=1 to run") } + if _, err := exec.LookPath("devcontainer"); err != nil { + t.Fatal("this test requires the devcontainer CLI: npm install -g @devcontainers/cli") + } // Connect to Docker. pool, err := dockertest.NewPool("") From 27bc60d1b9eae069dfe63eb468f8f719751931ef Mon Sep 17 00:00:00 2001 From: Yevhenii Shcherbina Date: Thu, 17 Apr 2025 09:29:29 -0400 Subject: [PATCH 147/433] feat: implement reconciliation loop (#17261) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes https://github.com/coder/internal/issues/510
Refactoring Summary ### 1) `CalculateActions` Function #### Issues Before Refactoring: - Large function (~150 lines), making it difficult to read and maintain. - The control flow is hard to follow due to complex conditional logic. - The `ReconciliationActions` struct was partially initialized early, then mutated in multiple places, making the flow error-prone. Original source: https://github.com/coder/coder/blob/fe60b569ad754245e28bac71e0ef3c83536631bb/coderd/prebuilds/state.go#L13-L167 #### Improvements After Refactoring: - Simplified and broken down into smaller, focused helper methods. - The flow of the function is now more linear and easier to understand. - Struct initialization is cleaner, avoiding partial and incremental mutations. Refactored function: https://github.com/coder/coder/blob/eeb0407d783cdda71ec2418c113f325542c47b1c/coderd/prebuilds/state.go#L67-L84 --- ### 2) `ReconciliationActions` Struct #### Issues Before Refactoring: - The struct mixed both actionable decisions and diagnostic state, which blurred its purpose. - It was unclear which fields were necessary for reconciliation logic, and which were purely for logging/observability. #### Improvements After Refactoring: - Split into two clear, purpose-specific structs: - **`ReconciliationActions`** — defines the intended reconciliation action. - **`ReconciliationState`** — captures runtime state and metadata, primarily for logging and diagnostics. Original struct: https://github.com/coder/coder/blob/fe60b569ad754245e28bac71e0ef3c83536631bb/coderd/prebuilds/reconcile.go#L29-L41
--------- Signed-off-by: Danny Kopping Co-authored-by: Sas Swart Co-authored-by: Danny Kopping Co-authored-by: Dean Sheather Co-authored-by: Spike Curtis Co-authored-by: Danny Kopping --- coderd/database/lock.go | 3 +- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 8 +- coderd/database/queries/prebuilds.sql | 6 +- coderd/prebuilds/api.go | 27 + coderd/prebuilds/global_snapshot.go | 66 ++ coderd/prebuilds/noop.go | 35 + coderd/prebuilds/preset_snapshot.go | 254 ++++ coderd/prebuilds/preset_snapshot_test.go | 758 ++++++++++++ coderd/prebuilds/util.go | 26 + coderd/util/slice/slice.go | 11 + coderd/util/slice/slice_test.go | 59 + codersdk/deployment.go | 13 + enterprise/coderd/prebuilds/reconcile.go | 541 +++++++++ enterprise/coderd/prebuilds/reconcile_test.go | 1027 +++++++++++++++++ site/src/api/typesGenerated.ts | 7 + 16 files changed, 2834 insertions(+), 9 deletions(-) create mode 100644 coderd/prebuilds/api.go create mode 100644 coderd/prebuilds/global_snapshot.go create mode 100644 coderd/prebuilds/noop.go create mode 100644 coderd/prebuilds/preset_snapshot.go create mode 100644 coderd/prebuilds/preset_snapshot_test.go create mode 100644 coderd/prebuilds/util.go create mode 100644 enterprise/coderd/prebuilds/reconcile.go create mode 100644 enterprise/coderd/prebuilds/reconcile_test.go diff --git a/coderd/database/lock.go b/coderd/database/lock.go index 7ccb3b8f56fec..e5091cdfd29cc 100644 --- a/coderd/database/lock.go +++ b/coderd/database/lock.go @@ -12,8 +12,7 @@ const ( LockIDDBPurge LockIDNotificationsReportGenerator LockIDCryptoKeyRotation - LockIDReconcileTemplatePrebuilds - LockIDDeterminePrebuildsState + LockIDReconcilePrebuilds ) // GenLockID generates a unique and consistent lock ID from a given string. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 1cef5ada197f5..9fbfbde410d40 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -64,7 +64,7 @@ type sqlcQuerier interface { CleanTailnetCoordinators(ctx context.Context) error CleanTailnetLostPeers(ctx context.Context) error CleanTailnetTunnels(ctx context.Context) error - // CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped by template version ID and transition. + // CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped by preset ID and transition. // Prebuild considered in-progress if it's in the "starting", "stopping", or "deleting" state. CountInProgressPrebuilds(ctx context.Context) ([]CountInProgressPrebuildsRow, error) CountUnreadInboxNotificationsByUserID(ctx context.Context, userID uuid.UUID) (int64, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 72f2c4a8fcb8e..60416b1a35730 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5938,7 +5938,7 @@ func (q *sqlQuerier) ClaimPrebuiltWorkspace(ctx context.Context, arg ClaimPrebui } const countInProgressPrebuilds = `-- name: CountInProgressPrebuilds :many -SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition)::int AS count +SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition)::int AS count, wlb.template_version_preset_id as preset_id FROM workspace_latest_builds wlb INNER JOIN workspace_prebuild_builds wpb ON wpb.id = wlb.id -- We only need these counts for active template versions. @@ -5949,7 +5949,7 @@ FROM workspace_latest_builds wlb -- prebuilds that are still building. INNER JOIN templates t ON t.active_version_id = wlb.template_version_id WHERE wlb.job_status IN ('pending'::provisioner_job_status, 'running'::provisioner_job_status) -GROUP BY t.id, wpb.template_version_id, wpb.transition +GROUP BY t.id, wpb.template_version_id, wpb.transition, wlb.template_version_preset_id ` type CountInProgressPrebuildsRow struct { @@ -5957,9 +5957,10 @@ type CountInProgressPrebuildsRow struct { TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"` Transition WorkspaceTransition `db:"transition" json:"transition"` Count int32 `db:"count" json:"count"` + PresetID uuid.NullUUID `db:"preset_id" json:"preset_id"` } -// CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped by template version ID and transition. +// CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped by preset ID and transition. // Prebuild considered in-progress if it's in the "starting", "stopping", or "deleting" state. func (q *sqlQuerier) CountInProgressPrebuilds(ctx context.Context) ([]CountInProgressPrebuildsRow, error) { rows, err := q.db.QueryContext(ctx, countInProgressPrebuilds) @@ -5975,6 +5976,7 @@ func (q *sqlQuerier) CountInProgressPrebuilds(ctx context.Context) ([]CountInPro &i.TemplateVersionID, &i.Transition, &i.Count, + &i.PresetID, ); err != nil { return nil, err } diff --git a/coderd/database/queries/prebuilds.sql b/coderd/database/queries/prebuilds.sql index 53f5020f3607e..1d3a827c98586 100644 --- a/coderd/database/queries/prebuilds.sql +++ b/coderd/database/queries/prebuilds.sql @@ -57,9 +57,9 @@ WHERE (b.transition = 'start'::workspace_transition AND b.job_status = 'succeeded'::provisioner_job_status); -- name: CountInProgressPrebuilds :many --- CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped by template version ID and transition. +-- CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped by preset ID and transition. -- Prebuild considered in-progress if it's in the "starting", "stopping", or "deleting" state. -SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition)::int AS count +SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition)::int AS count, wlb.template_version_preset_id as preset_id FROM workspace_latest_builds wlb INNER JOIN workspace_prebuild_builds wpb ON wpb.id = wlb.id -- We only need these counts for active template versions. @@ -70,7 +70,7 @@ FROM workspace_latest_builds wlb -- prebuilds that are still building. INNER JOIN templates t ON t.active_version_id = wlb.template_version_id WHERE wlb.job_status IN ('pending'::provisioner_job_status, 'running'::provisioner_job_status) -GROUP BY t.id, wpb.template_version_id, wpb.transition; +GROUP BY t.id, wpb.template_version_id, wpb.transition, wlb.template_version_preset_id; -- GetPresetsBackoff groups workspace builds by preset ID. -- Each preset is associated with exactly one template version ID. diff --git a/coderd/prebuilds/api.go b/coderd/prebuilds/api.go new file mode 100644 index 0000000000000..6ebfb8acced44 --- /dev/null +++ b/coderd/prebuilds/api.go @@ -0,0 +1,27 @@ +package prebuilds + +import ( + "context" +) + +// ReconciliationOrchestrator manages the lifecycle of prebuild reconciliation. +// It runs a continuous loop to check and reconcile prebuild states, and can be stopped gracefully. +type ReconciliationOrchestrator interface { + Reconciler + + // RunLoop starts a continuous reconciliation loop that periodically calls ReconcileAll + // to ensure all prebuilds are in their desired states. The loop runs until the context + // is canceled or Stop is called. + RunLoop(ctx context.Context) + + // Stop gracefully shuts down the orchestrator with the given cause. + // The cause is used for logging and error reporting. + Stop(ctx context.Context, cause error) +} + +type Reconciler interface { + // ReconcileAll orchestrates the reconciliation of all prebuilds across all templates. + // It takes a global snapshot of the system state and then reconciles each preset + // in parallel, creating or deleting prebuilds as needed to reach their desired states. + ReconcileAll(ctx context.Context) error +} diff --git a/coderd/prebuilds/global_snapshot.go b/coderd/prebuilds/global_snapshot.go new file mode 100644 index 0000000000000..0cf3fa3facc3a --- /dev/null +++ b/coderd/prebuilds/global_snapshot.go @@ -0,0 +1,66 @@ +package prebuilds + +import ( + "github.com/google/uuid" + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/util/slice" +) + +// GlobalSnapshot represents a full point-in-time snapshot of state relating to prebuilds across all templates. +type GlobalSnapshot struct { + Presets []database.GetTemplatePresetsWithPrebuildsRow + RunningPrebuilds []database.GetRunningPrebuiltWorkspacesRow + PrebuildsInProgress []database.CountInProgressPrebuildsRow + Backoffs []database.GetPresetsBackoffRow +} + +func NewGlobalSnapshot( + presets []database.GetTemplatePresetsWithPrebuildsRow, + runningPrebuilds []database.GetRunningPrebuiltWorkspacesRow, + prebuildsInProgress []database.CountInProgressPrebuildsRow, + backoffs []database.GetPresetsBackoffRow, +) GlobalSnapshot { + return GlobalSnapshot{ + Presets: presets, + RunningPrebuilds: runningPrebuilds, + PrebuildsInProgress: prebuildsInProgress, + Backoffs: backoffs, + } +} + +func (s GlobalSnapshot) FilterByPreset(presetID uuid.UUID) (*PresetSnapshot, error) { + preset, found := slice.Find(s.Presets, func(preset database.GetTemplatePresetsWithPrebuildsRow) bool { + return preset.ID == presetID + }) + if !found { + return nil, xerrors.Errorf("no preset found with ID %q", presetID) + } + + running := slice.Filter(s.RunningPrebuilds, func(prebuild database.GetRunningPrebuiltWorkspacesRow) bool { + if !prebuild.CurrentPresetID.Valid { + return false + } + return prebuild.CurrentPresetID.UUID == preset.ID + }) + + inProgress := slice.Filter(s.PrebuildsInProgress, func(prebuild database.CountInProgressPrebuildsRow) bool { + return prebuild.PresetID.UUID == preset.ID + }) + + var backoffPtr *database.GetPresetsBackoffRow + backoff, found := slice.Find(s.Backoffs, func(row database.GetPresetsBackoffRow) bool { + return row.PresetID == preset.ID + }) + if found { + backoffPtr = &backoff + } + + return &PresetSnapshot{ + Preset: preset, + Running: running, + InProgress: inProgress, + Backoff: backoffPtr, + }, nil +} diff --git a/coderd/prebuilds/noop.go b/coderd/prebuilds/noop.go new file mode 100644 index 0000000000000..ffe4e7b442af9 --- /dev/null +++ b/coderd/prebuilds/noop.go @@ -0,0 +1,35 @@ +package prebuilds + +import ( + "context" + + "github.com/coder/coder/v2/coderd/database" +) + +type NoopReconciler struct{} + +func NewNoopReconciler() *NoopReconciler { + return &NoopReconciler{} +} + +func (NoopReconciler) RunLoop(context.Context) {} + +func (NoopReconciler) Stop(context.Context, error) {} + +func (NoopReconciler) ReconcileAll(context.Context) error { + return nil +} + +func (NoopReconciler) SnapshotState(context.Context, database.Store) (*GlobalSnapshot, error) { + return &GlobalSnapshot{}, nil +} + +func (NoopReconciler) ReconcilePreset(context.Context, PresetSnapshot) error { + return nil +} + +func (NoopReconciler) CalculateActions(context.Context, PresetSnapshot) (*ReconciliationActions, error) { + return &ReconciliationActions{}, nil +} + +var _ ReconciliationOrchestrator = NoopReconciler{} diff --git a/coderd/prebuilds/preset_snapshot.go b/coderd/prebuilds/preset_snapshot.go new file mode 100644 index 0000000000000..b6f05e588a6c0 --- /dev/null +++ b/coderd/prebuilds/preset_snapshot.go @@ -0,0 +1,254 @@ +package prebuilds + +import ( + "slices" + "time" + + "github.com/google/uuid" + + "github.com/coder/quartz" + + "github.com/coder/coder/v2/coderd/database" +) + +// ActionType represents the type of action needed to reconcile prebuilds. +type ActionType int + +const ( + // ActionTypeUndefined represents an uninitialized or invalid action type. + ActionTypeUndefined ActionType = iota + + // ActionTypeCreate indicates that new prebuilds should be created. + ActionTypeCreate + + // ActionTypeDelete indicates that existing prebuilds should be deleted. + ActionTypeDelete + + // ActionTypeBackoff indicates that prebuild creation should be delayed. + ActionTypeBackoff +) + +// PresetSnapshot is a filtered view of GlobalSnapshot focused on a single preset. +// It contains the raw data needed to calculate the current state of a preset's prebuilds, +// including running prebuilds, in-progress builds, and backoff information. +type PresetSnapshot struct { + Preset database.GetTemplatePresetsWithPrebuildsRow + Running []database.GetRunningPrebuiltWorkspacesRow + InProgress []database.CountInProgressPrebuildsRow + Backoff *database.GetPresetsBackoffRow +} + +// ReconciliationState represents the processed state of a preset's prebuilds, +// calculated from a PresetSnapshot. While PresetSnapshot contains raw data, +// ReconciliationState contains derived metrics that are directly used to +// determine what actions are needed (create, delete, or backoff). +// For example, it calculates how many prebuilds are eligible, how many are +// extraneous, and how many are in various transition states. +type ReconciliationState struct { + Actual int32 // Number of currently running prebuilds + Desired int32 // Number of prebuilds desired as defined in the preset + Eligible int32 // Number of prebuilds that are ready to be claimed + Extraneous int32 // Number of extra running prebuilds beyond the desired count + + // Counts of prebuilds in various transition states + Starting int32 + Stopping int32 + Deleting int32 +} + +// ReconciliationActions represents actions needed to reconcile the current state with the desired state. +// Based on ActionType, exactly one of Create, DeleteIDs, or BackoffUntil will be set. +type ReconciliationActions struct { + // ActionType determines which field is set and what action should be taken + ActionType ActionType + + // Create is set when ActionType is ActionTypeCreate and indicates the number of prebuilds to create + Create int32 + + // DeleteIDs is set when ActionType is ActionTypeDelete and contains the IDs of prebuilds to delete + DeleteIDs []uuid.UUID + + // BackoffUntil is set when ActionType is ActionTypeBackoff and indicates when to retry creating prebuilds + BackoffUntil time.Time +} + +// CalculateState computes the current state of prebuilds for a preset, including: +// - Actual: Number of currently running prebuilds +// - Desired: Number of prebuilds desired as defined in the preset +// - Eligible: Number of prebuilds that are ready to be claimed +// - Extraneous: Number of extra running prebuilds beyond the desired count +// - Starting/Stopping/Deleting: Counts of prebuilds in various transition states +// +// The function takes into account whether the preset is active (using the active template version) +// and calculates appropriate counts based on the current state of running prebuilds and +// in-progress transitions. This state information is used to determine what reconciliation +// actions are needed to reach the desired state. +func (p PresetSnapshot) CalculateState() *ReconciliationState { + var ( + actual int32 + desired int32 + eligible int32 + extraneous int32 + ) + + if p.isActive() { + // #nosec G115 - Safe conversion as p.Running slice length is expected to be within int32 range + actual = int32(len(p.Running)) + desired = p.Preset.DesiredInstances.Int32 + eligible = p.countEligible() + extraneous = max(actual-desired, 0) + } + + starting, stopping, deleting := p.countInProgress() + + return &ReconciliationState{ + Actual: actual, + Desired: desired, + Eligible: eligible, + Extraneous: extraneous, + + Starting: starting, + Stopping: stopping, + Deleting: deleting, + } +} + +// CalculateActions determines what actions are needed to reconcile the current state with the desired state. +// The function: +// 1. First checks if a backoff period is needed (if previous builds failed) +// 2. If the preset is inactive (template version is not active), it will delete all running prebuilds +// 3. For active presets, it calculates the number of prebuilds to create or delete based on: +// - The desired number of instances +// - Currently running prebuilds +// - Prebuilds in transition states (starting/stopping/deleting) +// - Any extraneous prebuilds that need to be removed +// +// The function returns a ReconciliationActions struct that will have exactly one action type set: +// - ActionTypeBackoff: Only BackoffUntil is set, indicating when to retry +// - ActionTypeCreate: Only Create is set, indicating how many prebuilds to create +// - ActionTypeDelete: Only DeleteIDs is set, containing IDs of prebuilds to delete +func (p PresetSnapshot) CalculateActions(clock quartz.Clock, backoffInterval time.Duration) (*ReconciliationActions, error) { + // TODO: align workspace states with how we represent them on the FE and the CLI + // right now there's some slight differences which can lead to additional prebuilds being created + + // TODO: add mechanism to prevent prebuilds being reconciled from being claimable by users; i.e. if a prebuild is + // about to be deleted, it should not be deleted if it has been claimed - beware of TOCTOU races! + + actions, needsBackoff := p.needsBackoffPeriod(clock, backoffInterval) + if needsBackoff { + return actions, nil + } + + if !p.isActive() { + return p.handleInactiveTemplateVersion() + } + + return p.handleActiveTemplateVersion() +} + +// isActive returns true if the preset's template version is the active version, and it is neither deleted nor deprecated. +// This determines whether we should maintain prebuilds for this preset or delete them. +func (p PresetSnapshot) isActive() bool { + return p.Preset.UsingActiveVersion && !p.Preset.Deleted && !p.Preset.Deprecated +} + +// handleActiveTemplateVersion deletes excess prebuilds if there are too many, +// otherwise creates new ones to reach the desired count. +func (p PresetSnapshot) handleActiveTemplateVersion() (*ReconciliationActions, error) { + state := p.CalculateState() + + // If we have more prebuilds than desired, delete the oldest ones + if state.Extraneous > 0 { + return &ReconciliationActions{ + ActionType: ActionTypeDelete, + DeleteIDs: p.getOldestPrebuildIDs(int(state.Extraneous)), + }, nil + } + + // Calculate how many new prebuilds we need to create + // We subtract starting prebuilds since they're already being created + prebuildsToCreate := max(state.Desired-state.Actual-state.Starting, 0) + + return &ReconciliationActions{ + ActionType: ActionTypeCreate, + Create: prebuildsToCreate, + }, nil +} + +// handleInactiveTemplateVersion deletes all running prebuilds except those already being deleted +// to avoid duplicate deletion attempts. +func (p PresetSnapshot) handleInactiveTemplateVersion() (*ReconciliationActions, error) { + prebuildsToDelete := len(p.Running) + deleteIDs := p.getOldestPrebuildIDs(prebuildsToDelete) + + return &ReconciliationActions{ + ActionType: ActionTypeDelete, + DeleteIDs: deleteIDs, + }, nil +} + +// needsBackoffPeriod checks if we should delay prebuild creation due to recent failures. +// If there were failures, it calculates a backoff period based on the number of failures +// and returns true if we're still within that period. +func (p PresetSnapshot) needsBackoffPeriod(clock quartz.Clock, backoffInterval time.Duration) (*ReconciliationActions, bool) { + if p.Backoff == nil || p.Backoff.NumFailed == 0 { + return nil, false + } + backoffUntil := p.Backoff.LastBuildAt.Add(time.Duration(p.Backoff.NumFailed) * backoffInterval) + if clock.Now().After(backoffUntil) { + return nil, false + } + + return &ReconciliationActions{ + ActionType: ActionTypeBackoff, + BackoffUntil: backoffUntil, + }, true +} + +// countEligible returns the number of prebuilds that are ready to be claimed. +// A prebuild is eligible if it's running and its agents are in ready state. +func (p PresetSnapshot) countEligible() int32 { + var count int32 + for _, prebuild := range p.Running { + if prebuild.Ready { + count++ + } + } + return count +} + +// countInProgress returns counts of prebuilds in transition states (starting, stopping, deleting). +// These counts are tracked at the template level, so all presets sharing the same template see the same values. +func (p PresetSnapshot) countInProgress() (starting int32, stopping int32, deleting int32) { + for _, progress := range p.InProgress { + num := progress.Count + switch progress.Transition { + case database.WorkspaceTransitionStart: + starting += num + case database.WorkspaceTransitionStop: + stopping += num + case database.WorkspaceTransitionDelete: + deleting += num + } + } + + return starting, stopping, deleting +} + +// getOldestPrebuildIDs returns the IDs of the N oldest prebuilds, sorted by creation time. +// This is used when we need to delete prebuilds, ensuring we remove the oldest ones first. +func (p PresetSnapshot) getOldestPrebuildIDs(n int) []uuid.UUID { + // Sort by creation time, oldest first + slices.SortFunc(p.Running, func(a, b database.GetRunningPrebuiltWorkspacesRow) int { + return a.CreatedAt.Compare(b.CreatedAt) + }) + + // Take the first N IDs + n = min(n, len(p.Running)) + ids := make([]uuid.UUID, n) + for i := 0; i < n; i++ { + ids[i] = p.Running[i].ID + } + + return ids +} diff --git a/coderd/prebuilds/preset_snapshot_test.go b/coderd/prebuilds/preset_snapshot_test.go new file mode 100644 index 0000000000000..cce8ea67cb05c --- /dev/null +++ b/coderd/prebuilds/preset_snapshot_test.go @@ -0,0 +1,758 @@ +package prebuilds_test + +import ( + "database/sql" + "fmt" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/coder/quartz" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/prebuilds" +) + +type options struct { + templateID uuid.UUID + templateVersionID uuid.UUID + presetID uuid.UUID + presetName string + prebuiltWorkspaceID uuid.UUID + workspaceName string +} + +// templateID is common across all option sets. +var templateID = uuid.UUID{1} + +const ( + backoffInterval = time.Second * 5 + + optionSet0 = iota + optionSet1 + optionSet2 +) + +var opts = map[uint]options{ + optionSet0: { + templateID: templateID, + templateVersionID: uuid.UUID{11}, + presetID: uuid.UUID{12}, + presetName: "my-preset", + prebuiltWorkspaceID: uuid.UUID{13}, + workspaceName: "prebuilds0", + }, + optionSet1: { + templateID: templateID, + templateVersionID: uuid.UUID{21}, + presetID: uuid.UUID{22}, + presetName: "my-preset", + prebuiltWorkspaceID: uuid.UUID{23}, + workspaceName: "prebuilds1", + }, + optionSet2: { + templateID: templateID, + templateVersionID: uuid.UUID{31}, + presetID: uuid.UUID{32}, + presetName: "my-preset", + prebuiltWorkspaceID: uuid.UUID{33}, + workspaceName: "prebuilds2", + }, +} + +// A new template version with a preset without prebuilds configured should result in no prebuilds being created. +func TestNoPrebuilds(t *testing.T) { + t.Parallel() + current := opts[optionSet0] + clock := quartz.NewMock(t) + + presets := []database.GetTemplatePresetsWithPrebuildsRow{ + preset(true, 0, current), + } + + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, nil) + ps, err := snapshot.FilterByPreset(current.presetID) + require.NoError(t, err) + + state := ps.CalculateState() + actions, err := ps.CalculateActions(clock, backoffInterval) + require.NoError(t, err) + + validateState(t, prebuilds.ReconciliationState{ /*all zero values*/ }, *state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + Create: 0, + }, *actions) +} + +// A new template version with a preset with prebuilds configured should result in a new prebuild being created. +func TestNetNew(t *testing.T) { + t.Parallel() + current := opts[optionSet0] + clock := quartz.NewMock(t) + + presets := []database.GetTemplatePresetsWithPrebuildsRow{ + preset(true, 1, current), + } + + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, nil, nil) + ps, err := snapshot.FilterByPreset(current.presetID) + require.NoError(t, err) + + state := ps.CalculateState() + actions, err := ps.CalculateActions(clock, backoffInterval) + require.NoError(t, err) + + validateState(t, prebuilds.ReconciliationState{ + Desired: 1, + }, *state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + Create: 1, + }, *actions) +} + +// A new template version is created with a preset with prebuilds configured; this outdates the older version and +// requires the old prebuilds to be destroyed and new prebuilds to be created. +func TestOutdatedPrebuilds(t *testing.T) { + t.Parallel() + outdated := opts[optionSet0] + current := opts[optionSet1] + clock := quartz.NewMock(t) + + // GIVEN: 2 presets, one outdated and one new. + presets := []database.GetTemplatePresetsWithPrebuildsRow{ + preset(false, 1, outdated), + preset(true, 1, current), + } + + // GIVEN: a running prebuild for the outdated preset. + running := []database.GetRunningPrebuiltWorkspacesRow{ + prebuiltWorkspace(outdated, clock), + } + + // GIVEN: no in-progress builds. + var inProgress []database.CountInProgressPrebuildsRow + + // WHEN: calculating the outdated preset's state. + snapshot := prebuilds.NewGlobalSnapshot(presets, running, inProgress, nil) + ps, err := snapshot.FilterByPreset(outdated.presetID) + require.NoError(t, err) + + // THEN: we should identify that this prebuild is outdated and needs to be deleted. + state := ps.CalculateState() + actions, err := ps.CalculateActions(clock, backoffInterval) + require.NoError(t, err) + validateState(t, prebuilds.ReconciliationState{}, *state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeDelete, + DeleteIDs: []uuid.UUID{outdated.prebuiltWorkspaceID}, + }, *actions) + + // WHEN: calculating the current preset's state. + ps, err = snapshot.FilterByPreset(current.presetID) + require.NoError(t, err) + + // THEN: we should not be blocked from creating a new prebuild while the outdate one deletes. + state = ps.CalculateState() + actions, err = ps.CalculateActions(clock, backoffInterval) + require.NoError(t, err) + validateState(t, prebuilds.ReconciliationState{Desired: 1}, *state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + Create: 1, + }, *actions) +} + +// Make sure that outdated prebuild will be deleted, even if deletion of another outdated prebuild is already in progress. +func TestDeleteOutdatedPrebuilds(t *testing.T) { + t.Parallel() + outdated := opts[optionSet0] + clock := quartz.NewMock(t) + + // GIVEN: 1 outdated preset. + presets := []database.GetTemplatePresetsWithPrebuildsRow{ + preset(false, 1, outdated), + } + + // GIVEN: one running prebuild for the outdated preset. + running := []database.GetRunningPrebuiltWorkspacesRow{ + prebuiltWorkspace(outdated, clock), + } + + // GIVEN: one deleting prebuild for the outdated preset. + inProgress := []database.CountInProgressPrebuildsRow{ + { + TemplateID: outdated.templateID, + TemplateVersionID: outdated.templateVersionID, + Transition: database.WorkspaceTransitionDelete, + Count: 1, + PresetID: uuid.NullUUID{ + UUID: outdated.presetID, + Valid: true, + }, + }, + } + + // WHEN: calculating the outdated preset's state. + snapshot := prebuilds.NewGlobalSnapshot(presets, running, inProgress, nil) + ps, err := snapshot.FilterByPreset(outdated.presetID) + require.NoError(t, err) + + // THEN: we should identify that this prebuild is outdated and needs to be deleted. + // Despite the fact that deletion of another outdated prebuild is already in progress. + state := ps.CalculateState() + actions, err := ps.CalculateActions(clock, backoffInterval) + require.NoError(t, err) + validateState(t, prebuilds.ReconciliationState{ + Deleting: 1, + }, *state) + + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeDelete, + DeleteIDs: []uuid.UUID{outdated.prebuiltWorkspaceID}, + }, *actions) +} + +// A new template version is created with a preset with prebuilds configured; while a prebuild is provisioning up or down, +// the calculated actions should indicate the state correctly. +func TestInProgressActions(t *testing.T) { + t.Parallel() + current := opts[optionSet0] + clock := quartz.NewMock(t) + + cases := []struct { + name string + transition database.WorkspaceTransition + desired int32 + running int32 + inProgress int32 + checkFn func(state prebuilds.ReconciliationState, actions prebuilds.ReconciliationActions) + }{ + // With no running prebuilds and one starting, no creations/deletions should take place. + { + name: fmt.Sprintf("%s-short", database.WorkspaceTransitionStart), + transition: database.WorkspaceTransitionStart, + desired: 1, + running: 0, + inProgress: 1, + checkFn: func(state prebuilds.ReconciliationState, actions prebuilds.ReconciliationActions) { + validateState(t, prebuilds.ReconciliationState{Desired: 1, Starting: 1}, state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + }, actions) + }, + }, + // With one running prebuild and one starting, no creations/deletions should occur since we're approaching the correct state. + { + name: fmt.Sprintf("%s-balanced", database.WorkspaceTransitionStart), + transition: database.WorkspaceTransitionStart, + desired: 2, + running: 1, + inProgress: 1, + checkFn: func(state prebuilds.ReconciliationState, actions prebuilds.ReconciliationActions) { + validateState(t, prebuilds.ReconciliationState{Actual: 1, Desired: 2, Starting: 1}, state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + }, actions) + }, + }, + // With one running prebuild and one starting, no creations/deletions should occur + // SIDE-NOTE: once the starting prebuild completes, the older of the two will be considered extraneous since we only desire 2. + { + name: fmt.Sprintf("%s-extraneous", database.WorkspaceTransitionStart), + transition: database.WorkspaceTransitionStart, + desired: 2, + running: 2, + inProgress: 1, + checkFn: func(state prebuilds.ReconciliationState, actions prebuilds.ReconciliationActions) { + validateState(t, prebuilds.ReconciliationState{Actual: 2, Desired: 2, Starting: 1}, state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + }, actions) + }, + }, + // With one prebuild desired and one stopping, a new prebuild will be created. + { + name: fmt.Sprintf("%s-short", database.WorkspaceTransitionStop), + transition: database.WorkspaceTransitionStop, + desired: 1, + running: 0, + inProgress: 1, + checkFn: func(state prebuilds.ReconciliationState, actions prebuilds.ReconciliationActions) { + validateState(t, prebuilds.ReconciliationState{Desired: 1, Stopping: 1}, state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + Create: 1, + }, actions) + }, + }, + // With 3 prebuilds desired, 2 running, and 1 stopping, a new prebuild will be created. + { + name: fmt.Sprintf("%s-balanced", database.WorkspaceTransitionStop), + transition: database.WorkspaceTransitionStop, + desired: 3, + running: 2, + inProgress: 1, + checkFn: func(state prebuilds.ReconciliationState, actions prebuilds.ReconciliationActions) { + validateState(t, prebuilds.ReconciliationState{Actual: 2, Desired: 3, Stopping: 1}, state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + Create: 1, + }, actions) + }, + }, + // With 3 prebuilds desired, 3 running, and 1 stopping, no creations/deletions should occur since the desired state is already achieved. + { + name: fmt.Sprintf("%s-extraneous", database.WorkspaceTransitionStop), + transition: database.WorkspaceTransitionStop, + desired: 3, + running: 3, + inProgress: 1, + checkFn: func(state prebuilds.ReconciliationState, actions prebuilds.ReconciliationActions) { + validateState(t, prebuilds.ReconciliationState{Actual: 3, Desired: 3, Stopping: 1}, state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + }, actions) + }, + }, + // With one prebuild desired and one deleting, a new prebuild will be created. + { + name: fmt.Sprintf("%s-short", database.WorkspaceTransitionDelete), + transition: database.WorkspaceTransitionDelete, + desired: 1, + running: 0, + inProgress: 1, + checkFn: func(state prebuilds.ReconciliationState, actions prebuilds.ReconciliationActions) { + validateState(t, prebuilds.ReconciliationState{Desired: 1, Deleting: 1}, state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + Create: 1, + }, actions) + }, + }, + // With 2 prebuilds desired, 1 running, and 1 deleting, a new prebuild will be created. + { + name: fmt.Sprintf("%s-balanced", database.WorkspaceTransitionDelete), + transition: database.WorkspaceTransitionDelete, + desired: 2, + running: 1, + inProgress: 1, + checkFn: func(state prebuilds.ReconciliationState, actions prebuilds.ReconciliationActions) { + validateState(t, prebuilds.ReconciliationState{Actual: 1, Desired: 2, Deleting: 1}, state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + Create: 1, + }, actions) + }, + }, + // With 2 prebuilds desired, 2 running, and 1 deleting, no creations/deletions should occur since the desired state is already achieved. + { + name: fmt.Sprintf("%s-extraneous", database.WorkspaceTransitionDelete), + transition: database.WorkspaceTransitionDelete, + desired: 2, + running: 2, + inProgress: 1, + checkFn: func(state prebuilds.ReconciliationState, actions prebuilds.ReconciliationActions) { + validateState(t, prebuilds.ReconciliationState{Actual: 2, Desired: 2, Deleting: 1}, state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + }, actions) + }, + }, + // With 3 prebuilds desired, 1 running, and 2 starting, no creations should occur since the builds are in progress. + { + name: fmt.Sprintf("%s-inhibit", database.WorkspaceTransitionStart), + transition: database.WorkspaceTransitionStart, + desired: 3, + running: 1, + inProgress: 2, + checkFn: func(state prebuilds.ReconciliationState, actions prebuilds.ReconciliationActions) { + validateState(t, prebuilds.ReconciliationState{Actual: 1, Desired: 3, Starting: 2}, state) + validateActions(t, prebuilds.ReconciliationActions{ActionType: prebuilds.ActionTypeCreate, Create: 0}, actions) + }, + }, + // With 3 prebuilds desired, 5 running, and 2 deleting, no deletions should occur since the builds are in progress. + { + name: fmt.Sprintf("%s-inhibit", database.WorkspaceTransitionDelete), + transition: database.WorkspaceTransitionDelete, + desired: 3, + running: 5, + inProgress: 2, + checkFn: func(state prebuilds.ReconciliationState, actions prebuilds.ReconciliationActions) { + expectedState := prebuilds.ReconciliationState{Actual: 5, Desired: 3, Deleting: 2, Extraneous: 2} + expectedActions := prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeDelete, + } + + validateState(t, expectedState, state) + assert.EqualValuesf(t, expectedActions.ActionType, actions.ActionType, "'ActionType' did not match expectation") + assert.Len(t, actions.DeleteIDs, 2, "'deleteIDs' did not match expectation") + assert.EqualValuesf(t, expectedActions.Create, actions.Create, "'create' did not match expectation") + assert.EqualValuesf(t, expectedActions.BackoffUntil, actions.BackoffUntil, "'BackoffUntil' did not match expectation") + }, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + // GIVEN: a preset. + defaultPreset := preset(true, tc.desired, current) + presets := []database.GetTemplatePresetsWithPrebuildsRow{ + defaultPreset, + } + + // GIVEN: running prebuilt workspaces for the preset. + running := make([]database.GetRunningPrebuiltWorkspacesRow, 0, tc.running) + for range tc.running { + name, err := prebuilds.GenerateName() + require.NoError(t, err) + running = append(running, database.GetRunningPrebuiltWorkspacesRow{ + ID: uuid.New(), + Name: name, + TemplateID: current.templateID, + TemplateVersionID: current.templateVersionID, + CurrentPresetID: uuid.NullUUID{UUID: current.presetID, Valid: true}, + Ready: false, + CreatedAt: clock.Now(), + }) + } + + // GIVEN: some prebuilds for the preset which are currently transitioning. + inProgress := []database.CountInProgressPrebuildsRow{ + { + TemplateID: current.templateID, + TemplateVersionID: current.templateVersionID, + Transition: tc.transition, + Count: tc.inProgress, + PresetID: uuid.NullUUID{ + UUID: defaultPreset.ID, + Valid: true, + }, + }, + } + + // WHEN: calculating the current preset's state. + snapshot := prebuilds.NewGlobalSnapshot(presets, running, inProgress, nil) + ps, err := snapshot.FilterByPreset(current.presetID) + require.NoError(t, err) + + // THEN: we should identify that this prebuild is in progress. + state := ps.CalculateState() + actions, err := ps.CalculateActions(clock, backoffInterval) + require.NoError(t, err) + tc.checkFn(*state, *actions) + }) + } +} + +// Additional prebuilds exist for a given preset configuration; these must be deleted. +func TestExtraneous(t *testing.T) { + t.Parallel() + current := opts[optionSet0] + clock := quartz.NewMock(t) + + // GIVEN: a preset with 1 desired prebuild. + presets := []database.GetTemplatePresetsWithPrebuildsRow{ + preset(true, 1, current), + } + + var older uuid.UUID + // GIVEN: 2 running prebuilds for the preset. + running := []database.GetRunningPrebuiltWorkspacesRow{ + prebuiltWorkspace(current, clock, func(row database.GetRunningPrebuiltWorkspacesRow) database.GetRunningPrebuiltWorkspacesRow { + // The older of the running prebuilds will be deleted in order to maintain freshness. + row.CreatedAt = clock.Now().Add(-time.Hour) + older = row.ID + return row + }), + prebuiltWorkspace(current, clock, func(row database.GetRunningPrebuiltWorkspacesRow) database.GetRunningPrebuiltWorkspacesRow { + row.CreatedAt = clock.Now() + return row + }), + } + + // GIVEN: NO prebuilds in progress. + var inProgress []database.CountInProgressPrebuildsRow + + // WHEN: calculating the current preset's state. + snapshot := prebuilds.NewGlobalSnapshot(presets, running, inProgress, nil) + ps, err := snapshot.FilterByPreset(current.presetID) + require.NoError(t, err) + + // THEN: an extraneous prebuild is detected and marked for deletion. + state := ps.CalculateState() + actions, err := ps.CalculateActions(clock, backoffInterval) + require.NoError(t, err) + validateState(t, prebuilds.ReconciliationState{ + Actual: 2, Desired: 1, Extraneous: 1, Eligible: 2, + }, *state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeDelete, + DeleteIDs: []uuid.UUID{older}, + }, *actions) +} + +// A template marked as deprecated will not have prebuilds running. +func TestDeprecated(t *testing.T) { + t.Parallel() + current := opts[optionSet0] + clock := quartz.NewMock(t) + + // GIVEN: a preset with 1 desired prebuild. + presets := []database.GetTemplatePresetsWithPrebuildsRow{ + preset(true, 1, current, func(row database.GetTemplatePresetsWithPrebuildsRow) database.GetTemplatePresetsWithPrebuildsRow { + row.Deprecated = true + return row + }), + } + + // GIVEN: 1 running prebuilds for the preset. + running := []database.GetRunningPrebuiltWorkspacesRow{ + prebuiltWorkspace(current, clock), + } + + // GIVEN: NO prebuilds in progress. + var inProgress []database.CountInProgressPrebuildsRow + + // WHEN: calculating the current preset's state. + snapshot := prebuilds.NewGlobalSnapshot(presets, running, inProgress, nil) + ps, err := snapshot.FilterByPreset(current.presetID) + require.NoError(t, err) + + // THEN: all running prebuilds should be deleted because the template is deprecated. + state := ps.CalculateState() + actions, err := ps.CalculateActions(clock, backoffInterval) + require.NoError(t, err) + validateState(t, prebuilds.ReconciliationState{}, *state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeDelete, + DeleteIDs: []uuid.UUID{current.prebuiltWorkspaceID}, + }, *actions) +} + +// If the latest build failed, backoff exponentially with the given interval. +func TestLatestBuildFailed(t *testing.T) { + t.Parallel() + current := opts[optionSet0] + other := opts[optionSet1] + clock := quartz.NewMock(t) + + // GIVEN: two presets. + presets := []database.GetTemplatePresetsWithPrebuildsRow{ + preset(true, 1, current), + preset(true, 1, other), + } + + // GIVEN: running prebuilds only for one preset (the other will be failing, as evidenced by the backoffs below). + running := []database.GetRunningPrebuiltWorkspacesRow{ + prebuiltWorkspace(other, clock), + } + + // GIVEN: NO prebuilds in progress. + var inProgress []database.CountInProgressPrebuildsRow + + // GIVEN: a backoff entry. + lastBuildTime := clock.Now() + numFailed := 1 + backoffs := []database.GetPresetsBackoffRow{ + { + TemplateVersionID: current.templateVersionID, + PresetID: current.presetID, + NumFailed: int32(numFailed), + LastBuildAt: lastBuildTime, + }, + } + + // WHEN: calculating the current preset's state. + snapshot := prebuilds.NewGlobalSnapshot(presets, running, inProgress, backoffs) + psCurrent, err := snapshot.FilterByPreset(current.presetID) + require.NoError(t, err) + + // THEN: reconciliation should backoff. + state := psCurrent.CalculateState() + actions, err := psCurrent.CalculateActions(clock, backoffInterval) + require.NoError(t, err) + validateState(t, prebuilds.ReconciliationState{ + Actual: 0, Desired: 1, + }, *state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeBackoff, + BackoffUntil: lastBuildTime.Add(time.Duration(numFailed) * backoffInterval), + }, *actions) + + // WHEN: calculating the other preset's state. + psOther, err := snapshot.FilterByPreset(other.presetID) + require.NoError(t, err) + + // THEN: it should NOT be in backoff because all is OK. + state = psOther.CalculateState() + actions, err = psOther.CalculateActions(clock, backoffInterval) + require.NoError(t, err) + validateState(t, prebuilds.ReconciliationState{ + Actual: 1, Desired: 1, Eligible: 1, + }, *state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + BackoffUntil: time.Time{}, + }, *actions) + + // WHEN: the clock is advanced a backoff interval. + clock.Advance(backoffInterval + time.Microsecond) + + // THEN: a new prebuild should be created. + psCurrent, err = snapshot.FilterByPreset(current.presetID) + require.NoError(t, err) + state = psCurrent.CalculateState() + actions, err = psCurrent.CalculateActions(clock, backoffInterval) + require.NoError(t, err) + validateState(t, prebuilds.ReconciliationState{ + Actual: 0, Desired: 1, + }, *state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + Create: 1, // <--- NOTE: we're now able to create a new prebuild because the interval has elapsed. + + }, *actions) +} + +func TestMultiplePresetsPerTemplateVersion(t *testing.T) { + t.Parallel() + + templateID := uuid.New() + templateVersionID := uuid.New() + presetOpts1 := options{ + templateID: templateID, + templateVersionID: templateVersionID, + presetID: uuid.New(), + presetName: "my-preset-1", + prebuiltWorkspaceID: uuid.New(), + workspaceName: "prebuilds1", + } + presetOpts2 := options{ + templateID: templateID, + templateVersionID: templateVersionID, + presetID: uuid.New(), + presetName: "my-preset-2", + prebuiltWorkspaceID: uuid.New(), + workspaceName: "prebuilds2", + } + + clock := quartz.NewMock(t) + + presets := []database.GetTemplatePresetsWithPrebuildsRow{ + preset(true, 1, presetOpts1), + preset(true, 1, presetOpts2), + } + + inProgress := []database.CountInProgressPrebuildsRow{ + { + TemplateID: templateID, + TemplateVersionID: templateVersionID, + Transition: database.WorkspaceTransitionStart, + Count: 1, + PresetID: uuid.NullUUID{ + UUID: presetOpts1.presetID, + Valid: true, + }, + }, + } + + snapshot := prebuilds.NewGlobalSnapshot(presets, nil, inProgress, nil) + + // Nothing has to be created for preset 1. + { + ps, err := snapshot.FilterByPreset(presetOpts1.presetID) + require.NoError(t, err) + + state := ps.CalculateState() + actions, err := ps.CalculateActions(clock, backoffInterval) + require.NoError(t, err) + + validateState(t, prebuilds.ReconciliationState{ + Starting: 1, + Desired: 1, + }, *state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + Create: 0, + }, *actions) + } + + // One prebuild has to be created for preset 2. Make sure preset 1 doesn't block preset 2. + { + ps, err := snapshot.FilterByPreset(presetOpts2.presetID) + require.NoError(t, err) + + state := ps.CalculateState() + actions, err := ps.CalculateActions(clock, backoffInterval) + require.NoError(t, err) + + validateState(t, prebuilds.ReconciliationState{ + Starting: 0, + Desired: 1, + }, *state) + validateActions(t, prebuilds.ReconciliationActions{ + ActionType: prebuilds.ActionTypeCreate, + Create: 1, + }, *actions) + } +} + +func preset(active bool, instances int32, opts options, muts ...func(row database.GetTemplatePresetsWithPrebuildsRow) database.GetTemplatePresetsWithPrebuildsRow) database.GetTemplatePresetsWithPrebuildsRow { + entry := database.GetTemplatePresetsWithPrebuildsRow{ + TemplateID: opts.templateID, + TemplateVersionID: opts.templateVersionID, + ID: opts.presetID, + UsingActiveVersion: active, + Name: opts.presetName, + DesiredInstances: sql.NullInt32{ + Valid: true, + Int32: instances, + }, + Deleted: false, + Deprecated: false, + } + + for _, mut := range muts { + entry = mut(entry) + } + return entry +} + +func prebuiltWorkspace( + opts options, + clock quartz.Clock, + muts ...func(row database.GetRunningPrebuiltWorkspacesRow) database.GetRunningPrebuiltWorkspacesRow, +) database.GetRunningPrebuiltWorkspacesRow { + entry := database.GetRunningPrebuiltWorkspacesRow{ + ID: opts.prebuiltWorkspaceID, + Name: opts.workspaceName, + TemplateID: opts.templateID, + TemplateVersionID: opts.templateVersionID, + CurrentPresetID: uuid.NullUUID{UUID: opts.presetID, Valid: true}, + Ready: true, + CreatedAt: clock.Now(), + } + + for _, mut := range muts { + entry = mut(entry) + } + return entry +} + +func validateState(t *testing.T, expected, actual prebuilds.ReconciliationState) { + require.Equal(t, expected, actual) +} + +// validateActions is a convenience func to make tests more readable; it exploits the fact that the default states for +// prebuilds align with zero values. +func validateActions(t *testing.T, expected, actual prebuilds.ReconciliationActions) { + require.Equal(t, expected, actual) +} diff --git a/coderd/prebuilds/util.go b/coderd/prebuilds/util.go new file mode 100644 index 0000000000000..2cc5311d5ed99 --- /dev/null +++ b/coderd/prebuilds/util.go @@ -0,0 +1,26 @@ +package prebuilds + +import ( + "crypto/rand" + "encoding/base32" + "fmt" + "strings" +) + +// GenerateName generates a 20-byte prebuild name which should safe to use without truncation in most situations. +// UUIDs may be too long for a resource name in cloud providers (since this ID will be used in the prebuild's name). +// +// We're generating a 9-byte suffix (72 bits of entropy): +// 1 - e^(-1e9^2 / (2 * 2^72)) = ~0.01% likelihood of collision in 1 billion IDs. +// See https://en.wikipedia.org/wiki/Birthday_attack. +func GenerateName() (string, error) { + b := make([]byte, 9) + + _, err := rand.Read(b) + if err != nil { + return "", err + } + + // Encode the bytes to Base32 (A-Z2-7), strip any '=' padding + return fmt.Sprintf("prebuild-%s", strings.ToLower(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(b))), nil +} diff --git a/coderd/util/slice/slice.go b/coderd/util/slice/slice.go index b4ee79291d73f..f3811650786b7 100644 --- a/coderd/util/slice/slice.go +++ b/coderd/util/slice/slice.go @@ -90,6 +90,17 @@ func Find[T any](haystack []T, cond func(T) bool) (T, bool) { return empty, false } +// Filter returns all elements that satisfy the condition. +func Filter[T any](haystack []T, cond func(T) bool) []T { + out := make([]T, 0, len(haystack)) + for _, hay := range haystack { + if cond(hay) { + out = append(out, hay) + } + } + return out +} + // Overlap returns if the 2 sets have any overlap (element(s) in common) func Overlap[T comparable](a []T, b []T) bool { return OverlapCompare(a, b, func(a, b T) bool { diff --git a/coderd/util/slice/slice_test.go b/coderd/util/slice/slice_test.go index df8d119273652..006337794faee 100644 --- a/coderd/util/slice/slice_test.go +++ b/coderd/util/slice/slice_test.go @@ -2,6 +2,7 @@ package slice_test import ( "math/rand" + "strings" "testing" "github.com/google/uuid" @@ -82,6 +83,64 @@ func TestContains(t *testing.T) { ) } +func TestFilter(t *testing.T) { + t.Parallel() + + type testCase[T any] struct { + haystack []T + cond func(T) bool + expected []T + } + + { + testCases := []*testCase[int]{ + { + haystack: []int{1, 2, 3, 4, 5}, + cond: func(num int) bool { + return num%2 == 1 + }, + expected: []int{1, 3, 5}, + }, + { + haystack: []int{1, 2, 3, 4, 5}, + cond: func(num int) bool { + return num%2 == 0 + }, + expected: []int{2, 4}, + }, + } + + for _, tc := range testCases { + actual := slice.Filter(tc.haystack, tc.cond) + require.Equal(t, tc.expected, actual) + } + } + + { + testCases := []*testCase[string]{ + { + haystack: []string{"hello", "hi", "bye"}, + cond: func(str string) bool { + return strings.HasPrefix(str, "h") + }, + expected: []string{"hello", "hi"}, + }, + { + haystack: []string{"hello", "hi", "bye"}, + cond: func(str string) bool { + return strings.HasPrefix(str, "b") + }, + expected: []string{"bye"}, + }, + } + + for _, tc := range testCases { + actual := slice.Filter(tc.haystack, tc.cond) + require.Equal(t, tc.expected, actual) + } + } +} + func TestOverlap(t *testing.T) { t.Parallel() diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 9db5a030ebc18..8b447e2c96e06 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -791,6 +791,19 @@ type NotificationsWebhookConfig struct { Endpoint serpent.URL `json:"endpoint" typescript:",notnull"` } +type PrebuildsConfig struct { + // ReconciliationInterval defines how often the workspace prebuilds state should be reconciled. + ReconciliationInterval serpent.Duration `json:"reconciliation_interval" typescript:",notnull"` + + // ReconciliationBackoffInterval specifies the amount of time to increase the backoff interval + // when errors occur during reconciliation. + ReconciliationBackoffInterval serpent.Duration `json:"reconciliation_backoff_interval" typescript:",notnull"` + + // ReconciliationBackoffLookback determines the time window to look back when calculating + // the number of failed prebuilds, which influences the backoff strategy. + ReconciliationBackoffLookback serpent.Duration `json:"reconciliation_backoff_lookback" typescript:",notnull"` +} + const ( annotationFormatDuration = "format_duration" annotationEnterpriseKey = "enterprise" diff --git a/enterprise/coderd/prebuilds/reconcile.go b/enterprise/coderd/prebuilds/reconcile.go new file mode 100644 index 0000000000000..f74e019207c18 --- /dev/null +++ b/enterprise/coderd/prebuilds/reconcile.go @@ -0,0 +1,541 @@ +package prebuilds + +import ( + "context" + "database/sql" + "fmt" + "math" + "sync/atomic" + "time" + + "github.com/hashicorp/go-multierror" + + "github.com/coder/quartz" + + "github.com/coder/coder/v2/coderd/audit" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/provisionerjobs" + "github.com/coder/coder/v2/coderd/database/pubsub" + "github.com/coder/coder/v2/coderd/prebuilds" + "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/coderd/rbac/policy" + "github.com/coder/coder/v2/coderd/wsbuilder" + "github.com/coder/coder/v2/codersdk" + + "cdr.dev/slog" + + "github.com/google/uuid" + "golang.org/x/sync/errgroup" + "golang.org/x/xerrors" +) + +type StoreReconciler struct { + store database.Store + cfg codersdk.PrebuildsConfig + pubsub pubsub.Pubsub + logger slog.Logger + clock quartz.Clock + + cancelFn context.CancelCauseFunc + stopped atomic.Bool + done chan struct{} +} + +var _ prebuilds.ReconciliationOrchestrator = &StoreReconciler{} + +func NewStoreReconciler( + store database.Store, + ps pubsub.Pubsub, + cfg codersdk.PrebuildsConfig, + logger slog.Logger, + clock quartz.Clock, +) *StoreReconciler { + return &StoreReconciler{ + store: store, + pubsub: ps, + logger: logger, + cfg: cfg, + clock: clock, + done: make(chan struct{}, 1), + } +} + +func (c *StoreReconciler) RunLoop(ctx context.Context) { + reconciliationInterval := c.cfg.ReconciliationInterval.Value() + if reconciliationInterval <= 0 { // avoids a panic + reconciliationInterval = 5 * time.Minute + } + + c.logger.Info(ctx, "starting reconciler", + slog.F("interval", reconciliationInterval), + slog.F("backoff_interval", c.cfg.ReconciliationBackoffInterval.String()), + slog.F("backoff_lookback", c.cfg.ReconciliationBackoffLookback.String())) + + ticker := c.clock.NewTicker(reconciliationInterval) + defer ticker.Stop() + defer func() { + c.done <- struct{}{} + }() + + // nolint:gocritic // Reconciliation Loop needs Prebuilds Orchestrator permissions. + ctx, cancel := context.WithCancelCause(dbauthz.AsPrebuildsOrchestrator(ctx)) + c.cancelFn = cancel + + for { + select { + // TODO: implement pubsub listener to allow reconciling a specific template imperatively once it has been changed, + // instead of waiting for the next reconciliation interval + case <-ticker.C: + // Trigger a new iteration on each tick. + err := c.ReconcileAll(ctx) + if err != nil { + c.logger.Error(context.Background(), "reconciliation failed", slog.Error(err)) + } + case <-ctx.Done(): + // nolint:gocritic // it's okay to use slog.F() for an error in this case + // because we want to differentiate two different types of errors: ctx.Err() and context.Cause() + c.logger.Warn( + context.Background(), + "reconciliation loop exited", + slog.Error(ctx.Err()), + slog.F("cause", context.Cause(ctx)), + ) + return + } + } +} + +func (c *StoreReconciler) Stop(ctx context.Context, cause error) { + if cause != nil { + c.logger.Error(context.Background(), "stopping reconciler due to an error", slog.Error(cause)) + } else { + c.logger.Info(context.Background(), "gracefully stopping reconciler") + } + + if c.isStopped() { + return + } + c.stopped.Store(true) + if c.cancelFn != nil { + c.cancelFn(cause) + } + + select { + // Give up waiting for control loop to exit. + case <-ctx.Done(): + // nolint:gocritic // it's okay to use slog.F() for an error in this case + // because we want to differentiate two different types of errors: ctx.Err() and context.Cause() + c.logger.Error( + context.Background(), + "reconciler stop exited prematurely", + slog.Error(ctx.Err()), + slog.F("cause", context.Cause(ctx)), + ) + // Wait for the control loop to exit. + case <-c.done: + c.logger.Info(context.Background(), "reconciler stopped") + } +} + +func (c *StoreReconciler) isStopped() bool { + return c.stopped.Load() +} + +// ReconcileAll will attempt to resolve the desired vs actual state of all templates which have presets with prebuilds configured. +// +// NOTE: +// +// This function will kick of n provisioner jobs, based on the calculated state modifications. +// +// These provisioning jobs are fire-and-forget. We DO NOT wait for the prebuilt workspaces to complete their +// provisioning. As a consequence, it's possible that another reconciliation run will occur, which will mean that +// multiple preset versions could be reconciling at once. This may mean some temporary over-provisioning, but the +// reconciliation loop will bring these resources back into their desired numbers in an EVENTUALLY-consistent way. +// +// For example: we could decide to provision 1 new instance in this reconciliation. +// While that workspace is being provisioned, another template version is created which means this same preset will +// be reconciled again, leading to another workspace being provisioned. Two workspace builds will be occurring +// simultaneously for the same preset, but once both jobs have completed the reconciliation loop will notice the +// extraneous instance and delete it. +func (c *StoreReconciler) ReconcileAll(ctx context.Context) error { + logger := c.logger.With(slog.F("reconcile_context", "all")) + + select { + case <-ctx.Done(): + logger.Warn(context.Background(), "reconcile exiting prematurely; context done", slog.Error(ctx.Err())) + return nil + default: + } + + logger.Debug(ctx, "starting reconciliation") + + err := c.WithReconciliationLock(ctx, logger, func(ctx context.Context, db database.Store) error { + snapshot, err := c.SnapshotState(ctx, db) + if err != nil { + return xerrors.Errorf("determine current snapshot: %w", err) + } + if len(snapshot.Presets) == 0 { + logger.Debug(ctx, "no templates found with prebuilds configured") + return nil + } + + var eg errgroup.Group + // Reconcile presets in parallel. Each preset in its own goroutine. + for _, preset := range snapshot.Presets { + ps, err := snapshot.FilterByPreset(preset.ID) + if err != nil { + logger.Warn(ctx, "failed to find preset snapshot", slog.Error(err), slog.F("preset_id", preset.ID.String())) + continue + } + + eg.Go(func() error { + // Pass outer context. + err = c.ReconcilePreset(ctx, *ps) + if err != nil { + logger.Error( + ctx, + "failed to reconcile prebuilds for preset", + slog.Error(err), + slog.F("preset_id", preset.ID), + ) + } + // DO NOT return error otherwise the tx will end. + return nil + }) + } + + // Release lock only when all preset reconciliation goroutines are finished. + return eg.Wait() + }) + if err != nil { + logger.Error(ctx, "failed to reconcile", slog.Error(err)) + } + + return err +} + +// SnapshotState captures the current state of all prebuilds across templates. +func (c *StoreReconciler) SnapshotState(ctx context.Context, store database.Store) (*prebuilds.GlobalSnapshot, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + + var state prebuilds.GlobalSnapshot + + err := store.InTx(func(db database.Store) error { + // TODO: implement template-specific reconciliations later + presetsWithPrebuilds, err := db.GetTemplatePresetsWithPrebuilds(ctx, uuid.NullUUID{}) + if err != nil { + return xerrors.Errorf("failed to get template presets with prebuilds: %w", err) + } + if len(presetsWithPrebuilds) == 0 { + return nil + } + allRunningPrebuilds, err := db.GetRunningPrebuiltWorkspaces(ctx) + if err != nil { + return xerrors.Errorf("failed to get running prebuilds: %w", err) + } + + allPrebuildsInProgress, err := db.CountInProgressPrebuilds(ctx) + if err != nil { + return xerrors.Errorf("failed to get prebuilds in progress: %w", err) + } + + presetsBackoff, err := db.GetPresetsBackoff(ctx, c.clock.Now().Add(-c.cfg.ReconciliationBackoffLookback.Value())) + if err != nil { + return xerrors.Errorf("failed to get backoffs for presets: %w", err) + } + + state = prebuilds.NewGlobalSnapshot(presetsWithPrebuilds, allRunningPrebuilds, allPrebuildsInProgress, presetsBackoff) + return nil + }, &database.TxOptions{ + Isolation: sql.LevelRepeatableRead, // This mirrors the MVCC snapshotting Postgres does when using CTEs + ReadOnly: true, + TxIdentifier: "prebuilds_state_determination", + }) + + return &state, err +} + +func (c *StoreReconciler) ReconcilePreset(ctx context.Context, ps prebuilds.PresetSnapshot) error { + logger := c.logger.With( + slog.F("template_id", ps.Preset.TemplateID.String()), + slog.F("template_name", ps.Preset.TemplateName), + slog.F("template_version_id", ps.Preset.TemplateVersionID), + slog.F("template_version_name", ps.Preset.TemplateVersionName), + slog.F("preset_id", ps.Preset.ID), + slog.F("preset_name", ps.Preset.Name), + ) + + state := ps.CalculateState() + actions, err := c.CalculateActions(ctx, ps) + if err != nil { + logger.Error(ctx, "failed to calculate actions for preset", slog.Error(err), slog.F("preset_id", ps.Preset.ID)) + return nil + } + + // nolint:gocritic // ReconcilePreset needs Prebuilds Orchestrator permissions. + prebuildsCtx := dbauthz.AsPrebuildsOrchestrator(ctx) + + levelFn := logger.Debug + switch { + case actions.ActionType == prebuilds.ActionTypeBackoff: + levelFn = logger.Warn + // Log at info level when there's a change to be effected. + case actions.ActionType == prebuilds.ActionTypeCreate && actions.Create > 0: + levelFn = logger.Info + case actions.ActionType == prebuilds.ActionTypeDelete && len(actions.DeleteIDs) > 0: + levelFn = logger.Info + } + + fields := []any{ + slog.F("action_type", actions.ActionType), + slog.F("create_count", actions.Create), slog.F("delete_count", len(actions.DeleteIDs)), + slog.F("to_delete", actions.DeleteIDs), + slog.F("desired", state.Desired), slog.F("actual", state.Actual), + slog.F("extraneous", state.Extraneous), slog.F("starting", state.Starting), + slog.F("stopping", state.Stopping), slog.F("deleting", state.Deleting), + slog.F("eligible", state.Eligible), + } + + levelFn(ctx, "calculated reconciliation actions for preset", fields...) + + switch actions.ActionType { + case prebuilds.ActionTypeBackoff: + // If there is anything to backoff for (usually a cycle of failed prebuilds), then log and bail out. + levelFn(ctx, "template prebuild state retrieved, backing off", + append(fields, + slog.F("backoff_until", actions.BackoffUntil.Format(time.RFC3339)), + slog.F("backoff_secs", math.Round(actions.BackoffUntil.Sub(c.clock.Now()).Seconds())), + )...) + + return nil + + case prebuilds.ActionTypeCreate: + // Unexpected things happen (i.e. bugs or bitflips); let's defend against disastrous outcomes. + // See https://blog.robertelder.org/causes-of-bit-flips-in-computer-memory/. + // This is obviously not comprehensive protection against this sort of problem, but this is one essential check. + desired := ps.Preset.DesiredInstances.Int32 + if actions.Create > desired { + logger.Critical(ctx, "determined excessive count of prebuilds to create; clamping to desired count", + slog.F("create_count", actions.Create), slog.F("desired_count", desired)) + + actions.Create = desired + } + + var multiErr multierror.Error + + for range actions.Create { + if err := c.createPrebuiltWorkspace(prebuildsCtx, uuid.New(), ps.Preset.TemplateID, ps.Preset.ID); err != nil { + logger.Error(ctx, "failed to create prebuild", slog.Error(err)) + multiErr.Errors = append(multiErr.Errors, err) + } + } + + return multiErr.ErrorOrNil() + + case prebuilds.ActionTypeDelete: + var multiErr multierror.Error + + for _, id := range actions.DeleteIDs { + if err := c.deletePrebuiltWorkspace(prebuildsCtx, id, ps.Preset.TemplateID, ps.Preset.ID); err != nil { + logger.Error(ctx, "failed to delete prebuild", slog.Error(err)) + multiErr.Errors = append(multiErr.Errors, err) + } + } + + return multiErr.ErrorOrNil() + + default: + return xerrors.Errorf("unknown action type: %v", actions.ActionType) + } +} + +func (c *StoreReconciler) CalculateActions(ctx context.Context, snapshot prebuilds.PresetSnapshot) (*prebuilds.ReconciliationActions, error) { + if ctx.Err() != nil { + return nil, ctx.Err() + } + + return snapshot.CalculateActions(c.clock, c.cfg.ReconciliationBackoffInterval.Value()) +} + +func (c *StoreReconciler) WithReconciliationLock( + ctx context.Context, + logger slog.Logger, + fn func(ctx context.Context, db database.Store) error, +) error { + // This tx holds a global lock, which prevents any other coderd replica from starting a reconciliation and + // possibly getting an inconsistent view of the state. + // + // The lock MUST be held until ALL modifications have been effected. + // + // It is run with RepeatableRead isolation, so it's effectively snapshotting the data at the start of the tx. + // + // This is a read-only tx, so returning an error (i.e. causing a rollback) has no impact. + return c.store.InTx(func(db database.Store) error { + start := c.clock.Now() + + // Try to acquire the lock. If we can't get it, another replica is handling reconciliation. + acquired, err := db.TryAcquireLock(ctx, database.LockIDReconcilePrebuilds) + if err != nil { + // This is a real database error, not just lock contention + logger.Error(ctx, "failed to acquire reconciliation lock due to database error", slog.Error(err)) + return err + } + if !acquired { + // Normal case: another replica has the lock + return nil + } + + logger.Debug(ctx, + "acquired top-level reconciliation lock", + slog.F("acquire_wait_secs", fmt.Sprintf("%.4f", c.clock.Since(start).Seconds())), + ) + + return fn(ctx, db) + }, &database.TxOptions{ + Isolation: sql.LevelRepeatableRead, + ReadOnly: true, + TxIdentifier: "prebuilds", + }) +} + +func (c *StoreReconciler) createPrebuiltWorkspace(ctx context.Context, prebuiltWorkspaceID uuid.UUID, templateID uuid.UUID, presetID uuid.UUID) error { + name, err := prebuilds.GenerateName() + if err != nil { + return xerrors.Errorf("failed to generate unique prebuild ID: %w", err) + } + + return c.store.InTx(func(db database.Store) error { + template, err := db.GetTemplateByID(ctx, templateID) + if err != nil { + return xerrors.Errorf("failed to get template: %w", err) + } + + now := c.clock.Now() + + minimumWorkspace, err := db.InsertWorkspace(ctx, database.InsertWorkspaceParams{ + ID: prebuiltWorkspaceID, + CreatedAt: now, + UpdatedAt: now, + OwnerID: prebuilds.SystemUserID, + OrganizationID: template.OrganizationID, + TemplateID: template.ID, + Name: name, + LastUsedAt: c.clock.Now(), + AutomaticUpdates: database.AutomaticUpdatesNever, + AutostartSchedule: sql.NullString{}, + Ttl: sql.NullInt64{}, + NextStartAt: sql.NullTime{}, + }) + if err != nil { + return xerrors.Errorf("insert workspace: %w", err) + } + + // We have to refetch the workspace for the joined in fields. + workspace, err := db.GetWorkspaceByID(ctx, minimumWorkspace.ID) + if err != nil { + return xerrors.Errorf("get workspace by ID: %w", err) + } + + c.logger.Info(ctx, "attempting to create prebuild", slog.F("name", name), + slog.F("workspace_id", prebuiltWorkspaceID.String()), slog.F("preset_id", presetID.String())) + + return c.provision(ctx, db, prebuiltWorkspaceID, template, presetID, database.WorkspaceTransitionStart, workspace) + }, &database.TxOptions{ + Isolation: sql.LevelRepeatableRead, + ReadOnly: false, + }) +} + +func (c *StoreReconciler) deletePrebuiltWorkspace(ctx context.Context, prebuiltWorkspaceID uuid.UUID, templateID uuid.UUID, presetID uuid.UUID) error { + return c.store.InTx(func(db database.Store) error { + workspace, err := db.GetWorkspaceByID(ctx, prebuiltWorkspaceID) + if err != nil { + return xerrors.Errorf("get workspace by ID: %w", err) + } + + template, err := db.GetTemplateByID(ctx, templateID) + if err != nil { + return xerrors.Errorf("failed to get template: %w", err) + } + + if workspace.OwnerID != prebuilds.SystemUserID { + return xerrors.Errorf("prebuilt workspace is not owned by prebuild user anymore, probably it was claimed") + } + + c.logger.Info(ctx, "attempting to delete prebuild", + slog.F("workspace_id", prebuiltWorkspaceID.String()), slog.F("preset_id", presetID.String())) + + return c.provision(ctx, db, prebuiltWorkspaceID, template, presetID, database.WorkspaceTransitionDelete, workspace) + }, &database.TxOptions{ + Isolation: sql.LevelRepeatableRead, + ReadOnly: false, + }) +} + +func (c *StoreReconciler) provision( + ctx context.Context, + db database.Store, + prebuildID uuid.UUID, + template database.Template, + presetID uuid.UUID, + transition database.WorkspaceTransition, + workspace database.Workspace, +) error { + tvp, err := db.GetPresetParametersByTemplateVersionID(ctx, template.ActiveVersionID) + if err != nil { + return xerrors.Errorf("fetch preset details: %w", err) + } + + var params []codersdk.WorkspaceBuildParameter + for _, param := range tvp { + // TODO: don't fetch in the first place. + if param.TemplateVersionPresetID != presetID { + continue + } + + params = append(params, codersdk.WorkspaceBuildParameter{ + Name: param.Name, + Value: param.Value, + }) + } + + builder := wsbuilder.New(workspace, transition). + Reason(database.BuildReasonInitiator). + Initiator(prebuilds.SystemUserID). + VersionID(template.ActiveVersionID). + MarkPrebuild(). + TemplateVersionPresetID(presetID) + + // We only inject the required params when the prebuild is being created. + // This mirrors the behavior of regular workspace deletion (see cli/delete.go). + if transition != database.WorkspaceTransitionDelete { + builder = builder.RichParameterValues(params) + } + + _, provisionerJob, _, err := builder.Build( + ctx, + db, + func(_ policy.Action, _ rbac.Objecter) bool { + return true // TODO: harden? + }, + audit.WorkspaceBuildBaggage{}, + ) + if err != nil { + return xerrors.Errorf("provision workspace: %w", err) + } + + err = provisionerjobs.PostJob(c.pubsub, *provisionerJob) + if err != nil { + // Client probably doesn't care about this error, so just log it. + c.logger.Error(ctx, "failed to post provisioner job to pubsub", slog.Error(err)) + } + + c.logger.Info(ctx, "prebuild job scheduled", slog.F("transition", transition), + slog.F("prebuild_id", prebuildID.String()), slog.F("preset_id", presetID.String()), + slog.F("job_id", provisionerJob.ID)) + + return nil +} diff --git a/enterprise/coderd/prebuilds/reconcile_test.go b/enterprise/coderd/prebuilds/reconcile_test.go new file mode 100644 index 0000000000000..a5bd4a728a4ea --- /dev/null +++ b/enterprise/coderd/prebuilds/reconcile_test.go @@ -0,0 +1,1027 @@ +package prebuilds_test + +import ( + "context" + "database/sql" + "fmt" + "sync" + "testing" + "time" + + "github.com/coder/coder/v2/coderd/util/slice" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + "tailscale.com/types/ptr" + + "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/quartz" + + "github.com/coder/serpent" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/database/pubsub" + agplprebuilds "github.com/coder/coder/v2/coderd/prebuilds" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/enterprise/coderd/prebuilds" + "github.com/coder/coder/v2/testutil" +) + +func TestNoReconciliationActionsIfNoPresets(t *testing.T) { + // Scenario: No reconciliation actions are taken if there are no presets + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres") + } + + clock := quartz.NewMock(t) + ctx := testutil.Context(t, testutil.WaitLong) + db, ps := dbtestutil.NewDB(t) + cfg := codersdk.PrebuildsConfig{ + ReconciliationInterval: serpent.Duration(testutil.WaitLong), + } + logger := testutil.Logger(t) + controller := prebuilds.NewStoreReconciler(db, ps, cfg, logger, quartz.NewMock(t)) + + // given a template version with no presets + org := dbgen.Organization(t, db, database.Organization{}) + user := dbgen.User(t, db, database.User{}) + template := dbgen.Template(t, db, database.Template{ + CreatedBy: user.ID, + OrganizationID: org.ID, + }) + templateVersion := dbgen.TemplateVersion(t, db, database.TemplateVersion{ + TemplateID: uuid.NullUUID{UUID: template.ID, Valid: true}, + OrganizationID: org.ID, + CreatedBy: user.ID, + }) + // verify that the db state is correct + gotTemplateVersion, err := db.GetTemplateVersionByID(ctx, templateVersion.ID) + require.NoError(t, err) + require.Equal(t, templateVersion, gotTemplateVersion) + + // when we trigger the reconciliation loop for all templates + require.NoError(t, controller.ReconcileAll(ctx)) + + // then no reconciliation actions are taken + // because without presets, there are no prebuilds + // and without prebuilds, there is nothing to reconcile + jobs, err := db.GetProvisionerJobsCreatedAfter(ctx, clock.Now().Add(earlier)) + require.NoError(t, err) + require.Empty(t, jobs) +} + +func TestNoReconciliationActionsIfNoPrebuilds(t *testing.T) { + // Scenario: No reconciliation actions are taken if there are no prebuilds + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres") + } + + clock := quartz.NewMock(t) + ctx := testutil.Context(t, testutil.WaitLong) + db, ps := dbtestutil.NewDB(t) + cfg := codersdk.PrebuildsConfig{ + ReconciliationInterval: serpent.Duration(testutil.WaitLong), + } + logger := testutil.Logger(t) + controller := prebuilds.NewStoreReconciler(db, ps, cfg, logger, quartz.NewMock(t)) + + // given there are presets, but no prebuilds + org := dbgen.Organization(t, db, database.Organization{}) + user := dbgen.User(t, db, database.User{}) + template := dbgen.Template(t, db, database.Template{ + CreatedBy: user.ID, + OrganizationID: org.ID, + }) + templateVersion := dbgen.TemplateVersion(t, db, database.TemplateVersion{ + TemplateID: uuid.NullUUID{UUID: template.ID, Valid: true}, + OrganizationID: org.ID, + CreatedBy: user.ID, + }) + preset, err := db.InsertPreset(ctx, database.InsertPresetParams{ + TemplateVersionID: templateVersion.ID, + Name: "test", + }) + require.NoError(t, err) + _, err = db.InsertPresetParameters(ctx, database.InsertPresetParametersParams{ + TemplateVersionPresetID: preset.ID, + Names: []string{"test"}, + Values: []string{"test"}, + }) + require.NoError(t, err) + + // verify that the db state is correct + presetParameters, err := db.GetPresetParametersByTemplateVersionID(ctx, templateVersion.ID) + require.NoError(t, err) + require.NotEmpty(t, presetParameters) + + // when we trigger the reconciliation loop for all templates + require.NoError(t, controller.ReconcileAll(ctx)) + + // then no reconciliation actions are taken + // because without prebuilds, there is nothing to reconcile + // even if there are presets + jobs, err := db.GetProvisionerJobsCreatedAfter(ctx, clock.Now().Add(earlier)) + require.NoError(t, err) + require.Empty(t, jobs) +} + +func TestPrebuildReconciliation(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres") + } + + type testCase struct { + name string + prebuildLatestTransitions []database.WorkspaceTransition + prebuildJobStatuses []database.ProvisionerJobStatus + templateVersionActive []bool + templateDeleted []bool + shouldCreateNewPrebuild *bool + shouldDeleteOldPrebuild *bool + } + + testCases := []testCase{ + { + name: "never create prebuilds for inactive template versions", + prebuildLatestTransitions: allTransitions, + prebuildJobStatuses: allJobStatuses, + templateVersionActive: []bool{false}, + shouldCreateNewPrebuild: ptr.To(false), + templateDeleted: []bool{false}, + }, + { + name: "no need to create a new prebuild if one is already running", + prebuildLatestTransitions: []database.WorkspaceTransition{ + database.WorkspaceTransitionStart, + }, + prebuildJobStatuses: []database.ProvisionerJobStatus{ + database.ProvisionerJobStatusSucceeded, + }, + templateVersionActive: []bool{true}, + shouldCreateNewPrebuild: ptr.To(false), + templateDeleted: []bool{false}, + }, + { + name: "don't create a new prebuild if one is queued to build or already building", + prebuildLatestTransitions: []database.WorkspaceTransition{ + database.WorkspaceTransitionStart, + }, + prebuildJobStatuses: []database.ProvisionerJobStatus{ + database.ProvisionerJobStatusPending, + database.ProvisionerJobStatusRunning, + }, + templateVersionActive: []bool{true}, + shouldCreateNewPrebuild: ptr.To(false), + templateDeleted: []bool{false}, + }, + { + name: "create a new prebuild if one is in a state that disqualifies it from ever being claimed", + prebuildLatestTransitions: []database.WorkspaceTransition{ + database.WorkspaceTransitionStop, + database.WorkspaceTransitionDelete, + }, + prebuildJobStatuses: []database.ProvisionerJobStatus{ + database.ProvisionerJobStatusPending, + database.ProvisionerJobStatusRunning, + database.ProvisionerJobStatusCanceling, + database.ProvisionerJobStatusSucceeded, + }, + templateVersionActive: []bool{true}, + shouldCreateNewPrebuild: ptr.To(true), + templateDeleted: []bool{false}, + }, + { + // See TestFailedBuildBackoff for the start/failed case. + name: "create a new prebuild if one is in any kind of exceptional state", + prebuildLatestTransitions: []database.WorkspaceTransition{ + database.WorkspaceTransitionStop, + database.WorkspaceTransitionDelete, + }, + prebuildJobStatuses: []database.ProvisionerJobStatus{ + database.ProvisionerJobStatusCanceled, + }, + templateVersionActive: []bool{true}, + shouldCreateNewPrebuild: ptr.To(true), + templateDeleted: []bool{false}, + }, + { + name: "never attempt to interfere with active builds", + // The workspace builder does not allow scheduling a new build if there is already a build + // pending, running, or canceling. As such, we should never attempt to start, stop or delete + // such prebuilds. Rather, we should wait for the existing build to complete and reconcile + // again in the next cycle. + prebuildLatestTransitions: allTransitions, + prebuildJobStatuses: []database.ProvisionerJobStatus{ + database.ProvisionerJobStatusPending, + database.ProvisionerJobStatusRunning, + database.ProvisionerJobStatusCanceling, + }, + templateVersionActive: []bool{true, false}, + shouldDeleteOldPrebuild: ptr.To(false), + templateDeleted: []bool{false}, + }, + { + name: "never delete prebuilds in an exceptional state", + // We don't want to destroy evidence that might be useful to operators + // when troubleshooting issues. So we leave these prebuilds in place. + // Operators are expected to manually delete these prebuilds. + prebuildLatestTransitions: allTransitions, + prebuildJobStatuses: []database.ProvisionerJobStatus{ + database.ProvisionerJobStatusCanceled, + database.ProvisionerJobStatusFailed, + }, + templateVersionActive: []bool{true, false}, + shouldDeleteOldPrebuild: ptr.To(false), + templateDeleted: []bool{false}, + }, + { + name: "delete running prebuilds for inactive template versions", + // We only support prebuilds for active template versions. + // If a template version is inactive, we should delete any prebuilds + // that are running. + prebuildLatestTransitions: []database.WorkspaceTransition{ + database.WorkspaceTransitionStart, + }, + prebuildJobStatuses: []database.ProvisionerJobStatus{ + database.ProvisionerJobStatusSucceeded, + }, + templateVersionActive: []bool{false}, + shouldDeleteOldPrebuild: ptr.To(true), + templateDeleted: []bool{false}, + }, + { + name: "don't delete running prebuilds for active template versions", + prebuildLatestTransitions: []database.WorkspaceTransition{ + database.WorkspaceTransitionStart, + }, + prebuildJobStatuses: []database.ProvisionerJobStatus{ + database.ProvisionerJobStatusSucceeded, + }, + templateVersionActive: []bool{true}, + shouldDeleteOldPrebuild: ptr.To(false), + templateDeleted: []bool{false}, + }, + { + name: "don't delete stopped or already deleted prebuilds", + // We don't ever stop prebuilds. A stopped prebuild is an exceptional state. + // As such we keep it, to allow operators to investigate the cause. + prebuildLatestTransitions: []database.WorkspaceTransition{ + database.WorkspaceTransitionStop, + database.WorkspaceTransitionDelete, + }, + prebuildJobStatuses: []database.ProvisionerJobStatus{ + database.ProvisionerJobStatusSucceeded, + }, + templateVersionActive: []bool{true, false}, + shouldDeleteOldPrebuild: ptr.To(false), + templateDeleted: []bool{false}, + }, + { + name: "delete prebuilds for deleted templates", + prebuildLatestTransitions: []database.WorkspaceTransition{database.WorkspaceTransitionStart}, + prebuildJobStatuses: []database.ProvisionerJobStatus{database.ProvisionerJobStatusSucceeded}, + templateVersionActive: []bool{true, false}, + shouldDeleteOldPrebuild: ptr.To(true), + templateDeleted: []bool{true}, + }, + } + for _, tc := range testCases { + tc := tc // capture for parallel + for _, templateVersionActive := range tc.templateVersionActive { + for _, prebuildLatestTransition := range tc.prebuildLatestTransitions { + for _, prebuildJobStatus := range tc.prebuildJobStatuses { + for _, templateDeleted := range tc.templateDeleted { + t.Run(fmt.Sprintf("%s - %s - %s", tc.name, prebuildLatestTransition, prebuildJobStatus), func(t *testing.T) { + t.Parallel() + t.Cleanup(func() { + if t.Failed() { + t.Logf("failed to run test: %s", tc.name) + t.Logf("templateVersionActive: %t", templateVersionActive) + t.Logf("prebuildLatestTransition: %s", prebuildLatestTransition) + t.Logf("prebuildJobStatus: %s", prebuildJobStatus) + } + }) + clock := quartz.NewMock(t) + ctx := testutil.Context(t, testutil.WaitShort) + cfg := codersdk.PrebuildsConfig{} + logger := slogtest.Make( + t, &slogtest.Options{IgnoreErrors: true}, + ).Leveled(slog.LevelDebug) + db, pubSub := dbtestutil.NewDB(t) + controller := prebuilds.NewStoreReconciler(db, pubSub, cfg, logger, quartz.NewMock(t)) + + ownerID := uuid.New() + dbgen.User(t, db, database.User{ + ID: ownerID, + }) + org, template := setupTestDBTemplate(t, db, ownerID, templateDeleted) + templateVersionID := setupTestDBTemplateVersion( + ctx, + t, + clock, + db, + pubSub, + org.ID, + ownerID, + template.ID, + ) + preset := setupTestDBPreset( + t, + db, + templateVersionID, + 1, + uuid.New().String(), + ) + prebuild := setupTestDBPrebuild( + t, + clock, + db, + pubSub, + prebuildLatestTransition, + prebuildJobStatus, + org.ID, + preset, + template.ID, + templateVersionID, + ) + + if !templateVersionActive { + // Create a new template version and mark it as active + // This marks the template version that we care about as inactive + setupTestDBTemplateVersion(ctx, t, clock, db, pubSub, org.ID, ownerID, template.ID) + } + + // Run the reconciliation multiple times to ensure idempotency + // 8 was arbitrary, but large enough to reasonably trust the result + for i := 1; i <= 8; i++ { + require.NoErrorf(t, controller.ReconcileAll(ctx), "failed on iteration %d", i) + + if tc.shouldCreateNewPrebuild != nil { + newPrebuildCount := 0 + workspaces, err := db.GetWorkspacesByTemplateID(ctx, template.ID) + require.NoError(t, err) + for _, workspace := range workspaces { + if workspace.ID != prebuild.ID { + newPrebuildCount++ + } + } + // This test configures a preset that desires one prebuild. + // In cases where new prebuilds should be created, there should be exactly one. + require.Equal(t, *tc.shouldCreateNewPrebuild, newPrebuildCount == 1) + } + + if tc.shouldDeleteOldPrebuild != nil { + builds, err := db.GetWorkspaceBuildsByWorkspaceID(ctx, database.GetWorkspaceBuildsByWorkspaceIDParams{ + WorkspaceID: prebuild.ID, + }) + require.NoError(t, err) + if *tc.shouldDeleteOldPrebuild { + require.Equal(t, 2, len(builds)) + require.Equal(t, database.WorkspaceTransitionDelete, builds[0].Transition) + } else { + require.Equal(t, 1, len(builds)) + require.Equal(t, prebuildLatestTransition, builds[0].Transition) + } + } + } + }) + } + } + } + } + } +} + +func TestMultiplePresetsPerTemplateVersion(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres") + } + + prebuildLatestTransition := database.WorkspaceTransitionStart + prebuildJobStatus := database.ProvisionerJobStatusRunning + templateDeleted := false + + clock := quartz.NewMock(t) + ctx := testutil.Context(t, testutil.WaitShort) + cfg := codersdk.PrebuildsConfig{} + logger := slogtest.Make( + t, &slogtest.Options{IgnoreErrors: true}, + ).Leveled(slog.LevelDebug) + db, pubSub := dbtestutil.NewDB(t) + controller := prebuilds.NewStoreReconciler(db, pubSub, cfg, logger, quartz.NewMock(t)) + + ownerID := uuid.New() + dbgen.User(t, db, database.User{ + ID: ownerID, + }) + org, template := setupTestDBTemplate(t, db, ownerID, templateDeleted) + templateVersionID := setupTestDBTemplateVersion( + ctx, + t, + clock, + db, + pubSub, + org.ID, + ownerID, + template.ID, + ) + preset := setupTestDBPreset( + t, + db, + templateVersionID, + 4, + uuid.New().String(), + ) + preset2 := setupTestDBPreset( + t, + db, + templateVersionID, + 10, + uuid.New().String(), + ) + prebuildIDs := make([]uuid.UUID, 0) + for i := 0; i < int(preset.DesiredInstances.Int32); i++ { + prebuild := setupTestDBPrebuild( + t, + clock, + db, + pubSub, + prebuildLatestTransition, + prebuildJobStatus, + org.ID, + preset, + template.ID, + templateVersionID, + ) + prebuildIDs = append(prebuildIDs, prebuild.ID) + } + + // Run the reconciliation multiple times to ensure idempotency + // 8 was arbitrary, but large enough to reasonably trust the result + for i := 1; i <= 8; i++ { + require.NoErrorf(t, controller.ReconcileAll(ctx), "failed on iteration %d", i) + + newPrebuildCount := 0 + workspaces, err := db.GetWorkspacesByTemplateID(ctx, template.ID) + require.NoError(t, err) + for _, workspace := range workspaces { + if slice.Contains(prebuildIDs, workspace.ID) { + continue + } + newPrebuildCount++ + } + + // NOTE: preset1 doesn't block creation of instances in preset2 + require.Equal(t, preset2.DesiredInstances.Int32, int32(newPrebuildCount)) // nolint:gosec + } +} + +func TestInvalidPreset(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres") + } + + templateDeleted := false + + clock := quartz.NewMock(t) + ctx := testutil.Context(t, testutil.WaitShort) + cfg := codersdk.PrebuildsConfig{} + logger := slogtest.Make( + t, &slogtest.Options{IgnoreErrors: true}, + ).Leveled(slog.LevelDebug) + db, pubSub := dbtestutil.NewDB(t) + controller := prebuilds.NewStoreReconciler(db, pubSub, cfg, logger, quartz.NewMock(t)) + + ownerID := uuid.New() + dbgen.User(t, db, database.User{ + ID: ownerID, + }) + org, template := setupTestDBTemplate(t, db, ownerID, templateDeleted) + templateVersionID := setupTestDBTemplateVersion( + ctx, + t, + clock, + db, + pubSub, + org.ID, + ownerID, + template.ID, + ) + // Add required param, which is not set in preset. It means that creating of prebuild will constantly fail. + dbgen.TemplateVersionParameter(t, db, database.TemplateVersionParameter{ + TemplateVersionID: templateVersionID, + Name: "required-param", + Description: "required param to make sure creating prebuild will fail", + Type: "bool", + DefaultValue: "", + Required: true, + }) + setupTestDBPreset( + t, + db, + templateVersionID, + 1, + uuid.New().String(), + ) + + // Run the reconciliation multiple times to ensure idempotency + // 8 was arbitrary, but large enough to reasonably trust the result + for i := 1; i <= 8; i++ { + require.NoErrorf(t, controller.ReconcileAll(ctx), "failed on iteration %d", i) + + workspaces, err := db.GetWorkspacesByTemplateID(ctx, template.ID) + require.NoError(t, err) + newPrebuildCount := len(workspaces) + + // NOTE: we don't have any new prebuilds, because their creation constantly fails. + require.Equal(t, int32(0), int32(newPrebuildCount)) // nolint:gosec + } +} + +func TestRunLoop(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres") + } + + prebuildLatestTransition := database.WorkspaceTransitionStart + prebuildJobStatus := database.ProvisionerJobStatusRunning + templateDeleted := false + + clock := quartz.NewMock(t) + ctx := testutil.Context(t, testutil.WaitShort) + backoffInterval := time.Minute + cfg := codersdk.PrebuildsConfig{ + // Given: explicitly defined backoff configuration to validate timings. + ReconciliationBackoffLookback: serpent.Duration(muchEarlier * -10), // Has to be positive. + ReconciliationBackoffInterval: serpent.Duration(backoffInterval), + ReconciliationInterval: serpent.Duration(time.Second), + } + logger := slogtest.Make( + t, &slogtest.Options{IgnoreErrors: true}, + ).Leveled(slog.LevelDebug) + db, pubSub := dbtestutil.NewDB(t) + controller := prebuilds.NewStoreReconciler(db, pubSub, cfg, logger, clock) + + ownerID := uuid.New() + dbgen.User(t, db, database.User{ + ID: ownerID, + }) + org, template := setupTestDBTemplate(t, db, ownerID, templateDeleted) + templateVersionID := setupTestDBTemplateVersion( + ctx, + t, + clock, + db, + pubSub, + org.ID, + ownerID, + template.ID, + ) + preset := setupTestDBPreset( + t, + db, + templateVersionID, + 4, + uuid.New().String(), + ) + preset2 := setupTestDBPreset( + t, + db, + templateVersionID, + 10, + uuid.New().String(), + ) + prebuildIDs := make([]uuid.UUID, 0) + for i := 0; i < int(preset.DesiredInstances.Int32); i++ { + prebuild := setupTestDBPrebuild( + t, + clock, + db, + pubSub, + prebuildLatestTransition, + prebuildJobStatus, + org.ID, + preset, + template.ID, + templateVersionID, + ) + prebuildIDs = append(prebuildIDs, prebuild.ID) + } + getNewPrebuildCount := func() int32 { + newPrebuildCount := 0 + workspaces, err := db.GetWorkspacesByTemplateID(ctx, template.ID) + require.NoError(t, err) + for _, workspace := range workspaces { + if slice.Contains(prebuildIDs, workspace.ID) { + continue + } + newPrebuildCount++ + } + + return int32(newPrebuildCount) // nolint:gosec + } + + // we need to wait until ticker is initialized, and only then use clock.Advance() + // otherwise clock.Advance() will be ignored + trap := clock.Trap().NewTicker() + go controller.RunLoop(ctx) + // wait until ticker is initialized + trap.MustWait(ctx).Release() + // start 1st iteration of ReconciliationLoop + // NOTE: at this point MustWait waits that iteration is started (ReconcileAll is called), but it doesn't wait until it completes + clock.Advance(cfg.ReconciliationInterval.Value()).MustWait(ctx) + + // wait until ReconcileAll is completed + // TODO: is it possible to avoid Eventually and replace it with quartz? + // Ideally to have all control on test-level, and be able to advance loop iterations from the test. + require.Eventually(t, func() bool { + newPrebuildCount := getNewPrebuildCount() + + // NOTE: preset1 doesn't block creation of instances in preset2 + return preset2.DesiredInstances.Int32 == newPrebuildCount + }, testutil.WaitShort, testutil.IntervalFast) + + // setup one more preset with 5 prebuilds + preset3 := setupTestDBPreset( + t, + db, + templateVersionID, + 5, + uuid.New().String(), + ) + newPrebuildCount := getNewPrebuildCount() + // nothing changed, because we didn't trigger a new iteration of a loop + require.Equal(t, preset2.DesiredInstances.Int32, newPrebuildCount) + + // start 2nd iteration of ReconciliationLoop + // NOTE: at this point MustWait waits that iteration is started (ReconcileAll is called), but it doesn't wait until it completes + clock.Advance(cfg.ReconciliationInterval.Value()).MustWait(ctx) + + // wait until ReconcileAll is completed + require.Eventually(t, func() bool { + newPrebuildCount := getNewPrebuildCount() + + // both prebuilds for preset2 and preset3 were created + return preset2.DesiredInstances.Int32+preset3.DesiredInstances.Int32 == newPrebuildCount + }, testutil.WaitShort, testutil.IntervalFast) + + // gracefully stop the reconciliation loop + controller.Stop(ctx, nil) +} + +func TestFailedBuildBackoff(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres") + } + ctx := testutil.Context(t, testutil.WaitSuperLong) + + // Setup. + clock := quartz.NewMock(t) + backoffInterval := time.Minute + cfg := codersdk.PrebuildsConfig{ + // Given: explicitly defined backoff configuration to validate timings. + ReconciliationBackoffLookback: serpent.Duration(muchEarlier * -10), // Has to be positive. + ReconciliationBackoffInterval: serpent.Duration(backoffInterval), + ReconciliationInterval: serpent.Duration(time.Second), + } + logger := slogtest.Make( + t, &slogtest.Options{IgnoreErrors: true}, + ).Leveled(slog.LevelDebug) + db, ps := dbtestutil.NewDB(t) + reconciler := prebuilds.NewStoreReconciler(db, ps, cfg, logger, clock) + + // Given: an active template version with presets and prebuilds configured. + const desiredInstances = 2 + userID := uuid.New() + dbgen.User(t, db, database.User{ + ID: userID, + }) + org, template := setupTestDBTemplate(t, db, userID, false) + templateVersionID := setupTestDBTemplateVersion(ctx, t, clock, db, ps, org.ID, userID, template.ID) + + preset := setupTestDBPreset(t, db, templateVersionID, desiredInstances, "test") + for range desiredInstances { + _ = setupTestDBPrebuild(t, clock, db, ps, database.WorkspaceTransitionStart, database.ProvisionerJobStatusFailed, org.ID, preset, template.ID, templateVersionID) + } + + // When: determining what actions to take next, backoff is calculated because the prebuild is in a failed state. + snapshot, err := reconciler.SnapshotState(ctx, db) + require.NoError(t, err) + require.Len(t, snapshot.Presets, 1) + presetState, err := snapshot.FilterByPreset(preset.ID) + require.NoError(t, err) + state := presetState.CalculateState() + actions, err := reconciler.CalculateActions(ctx, *presetState) + require.NoError(t, err) + + // Then: the backoff time is in the future, no prebuilds are running, and we won't create any new prebuilds. + require.EqualValues(t, 0, state.Actual) + require.EqualValues(t, 0, actions.Create) + require.EqualValues(t, desiredInstances, state.Desired) + require.True(t, clock.Now().Before(actions.BackoffUntil)) + + // Then: the backoff time is as expected based on the number of failed builds. + require.NotNil(t, presetState.Backoff) + require.EqualValues(t, desiredInstances, presetState.Backoff.NumFailed) + require.EqualValues(t, backoffInterval*time.Duration(presetState.Backoff.NumFailed), clock.Until(actions.BackoffUntil).Truncate(backoffInterval)) + + // When: advancing to the next tick which is still within the backoff time. + clock.Advance(cfg.ReconciliationInterval.Value()) + + // Then: the backoff interval will not have changed. + snapshot, err = reconciler.SnapshotState(ctx, db) + require.NoError(t, err) + presetState, err = snapshot.FilterByPreset(preset.ID) + require.NoError(t, err) + newState := presetState.CalculateState() + newActions, err := reconciler.CalculateActions(ctx, *presetState) + require.NoError(t, err) + require.EqualValues(t, 0, newState.Actual) + require.EqualValues(t, 0, newActions.Create) + require.EqualValues(t, desiredInstances, newState.Desired) + require.EqualValues(t, actions.BackoffUntil, newActions.BackoffUntil) + + // When: advancing beyond the backoff time. + clock.Advance(clock.Until(actions.BackoffUntil.Add(time.Second))) + + // Then: we will attempt to create a new prebuild. + snapshot, err = reconciler.SnapshotState(ctx, db) + require.NoError(t, err) + presetState, err = snapshot.FilterByPreset(preset.ID) + require.NoError(t, err) + state = presetState.CalculateState() + actions, err = reconciler.CalculateActions(ctx, *presetState) + require.NoError(t, err) + require.EqualValues(t, 0, state.Actual) + require.EqualValues(t, desiredInstances, state.Desired) + require.EqualValues(t, desiredInstances, actions.Create) + + // When: the desired number of new prebuild are provisioned, but one fails again. + for i := 0; i < desiredInstances; i++ { + status := database.ProvisionerJobStatusFailed + if i == 1 { + status = database.ProvisionerJobStatusSucceeded + } + _ = setupTestDBPrebuild(t, clock, db, ps, database.WorkspaceTransitionStart, status, org.ID, preset, template.ID, templateVersionID) + } + + // Then: the backoff time is roughly equal to two backoff intervals, since another build has failed. + snapshot, err = reconciler.SnapshotState(ctx, db) + require.NoError(t, err) + presetState, err = snapshot.FilterByPreset(preset.ID) + require.NoError(t, err) + state = presetState.CalculateState() + actions, err = reconciler.CalculateActions(ctx, *presetState) + require.NoError(t, err) + require.EqualValues(t, 1, state.Actual) + require.EqualValues(t, desiredInstances, state.Desired) + require.EqualValues(t, 0, actions.Create) + require.EqualValues(t, 3, presetState.Backoff.NumFailed) + require.EqualValues(t, backoffInterval*time.Duration(presetState.Backoff.NumFailed), clock.Until(actions.BackoffUntil).Truncate(backoffInterval)) +} + +func TestReconciliationLock(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres") + } + + ctx := testutil.Context(t, testutil.WaitSuperLong) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + db, ps := dbtestutil.NewDB(t) + + wg := sync.WaitGroup{} + mutex := sync.Mutex{} + for i := 0; i < 5; i++ { + wg.Add(1) + go func() { + defer wg.Done() + reconciler := prebuilds.NewStoreReconciler( + db, + ps, + codersdk.PrebuildsConfig{}, + slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug), + quartz.NewMock(t), + ) + reconciler.WithReconciliationLock(ctx, logger, func(_ context.Context, _ database.Store) error { + lockObtained := mutex.TryLock() + // As long as the postgres lock is held, this mutex should always be unlocked when we get here. + // If this mutex is ever locked at this point, then that means that the postgres lock is not being held while we're + // inside WithReconciliationLock, which is meant to hold the lock. + require.True(t, lockObtained) + // Sleep a bit to give reconcilers more time to contend for the lock + time.Sleep(time.Second) + defer mutex.Unlock() + return nil + }) + }() + } + wg.Wait() +} + +// nolint:revive // It's a control flag, but this is a test. +func setupTestDBTemplate( + t *testing.T, + db database.Store, + userID uuid.UUID, + templateDeleted bool, +) ( + database.Organization, + database.Template, +) { + t.Helper() + org := dbgen.Organization(t, db, database.Organization{}) + + template := dbgen.Template(t, db, database.Template{ + CreatedBy: userID, + OrganizationID: org.ID, + CreatedAt: time.Now().Add(muchEarlier), + }) + if templateDeleted { + ctx := testutil.Context(t, testutil.WaitShort) + require.NoError(t, db.UpdateTemplateDeletedByID(ctx, database.UpdateTemplateDeletedByIDParams{ + ID: template.ID, + Deleted: true, + })) + } + return org, template +} + +const ( + earlier = -time.Hour + muchEarlier = -time.Hour * 2 +) + +func setupTestDBTemplateVersion( + ctx context.Context, + t *testing.T, + clock quartz.Clock, + db database.Store, + ps pubsub.Pubsub, + orgID uuid.UUID, + userID uuid.UUID, + templateID uuid.UUID, +) uuid.UUID { + t.Helper() + templateVersionJob := dbgen.ProvisionerJob(t, db, ps, database.ProvisionerJob{ + CreatedAt: clock.Now().Add(muchEarlier), + CompletedAt: sql.NullTime{Time: clock.Now().Add(earlier), Valid: true}, + OrganizationID: orgID, + InitiatorID: userID, + }) + templateVersion := dbgen.TemplateVersion(t, db, database.TemplateVersion{ + TemplateID: uuid.NullUUID{UUID: templateID, Valid: true}, + OrganizationID: orgID, + CreatedBy: userID, + JobID: templateVersionJob.ID, + CreatedAt: time.Now().Add(muchEarlier), + }) + require.NoError(t, db.UpdateTemplateActiveVersionByID(ctx, database.UpdateTemplateActiveVersionByIDParams{ + ID: templateID, + ActiveVersionID: templateVersion.ID, + })) + return templateVersion.ID +} + +func setupTestDBPreset( + t *testing.T, + db database.Store, + templateVersionID uuid.UUID, + desiredInstances int32, + presetName string, +) database.TemplateVersionPreset { + t.Helper() + preset := dbgen.Preset(t, db, database.InsertPresetParams{ + TemplateVersionID: templateVersionID, + Name: presetName, + DesiredInstances: sql.NullInt32{ + Valid: true, + Int32: desiredInstances, + }, + }) + dbgen.PresetParameter(t, db, database.InsertPresetParametersParams{ + TemplateVersionPresetID: preset.ID, + Names: []string{"test"}, + Values: []string{"test"}, + }) + return preset +} + +func setupTestDBPrebuild( + t *testing.T, + clock quartz.Clock, + db database.Store, + ps pubsub.Pubsub, + transition database.WorkspaceTransition, + prebuildStatus database.ProvisionerJobStatus, + orgID uuid.UUID, + preset database.TemplateVersionPreset, + templateID uuid.UUID, + templateVersionID uuid.UUID, +) database.WorkspaceTable { + t.Helper() + return setupTestDBWorkspace(t, clock, db, ps, transition, prebuildStatus, orgID, preset, templateID, templateVersionID, agplprebuilds.SystemUserID, agplprebuilds.SystemUserID) +} + +func setupTestDBWorkspace( + t *testing.T, + clock quartz.Clock, + db database.Store, + ps pubsub.Pubsub, + transition database.WorkspaceTransition, + prebuildStatus database.ProvisionerJobStatus, + orgID uuid.UUID, + preset database.TemplateVersionPreset, + templateID uuid.UUID, + templateVersionID uuid.UUID, + initiatorID uuid.UUID, + ownerID uuid.UUID, +) database.WorkspaceTable { + t.Helper() + cancelledAt := sql.NullTime{} + completedAt := sql.NullTime{} + + startedAt := sql.NullTime{} + if prebuildStatus != database.ProvisionerJobStatusPending { + startedAt = sql.NullTime{Time: clock.Now().Add(muchEarlier), Valid: true} + } + + buildError := sql.NullString{} + if prebuildStatus == database.ProvisionerJobStatusFailed { + completedAt = sql.NullTime{Time: clock.Now().Add(earlier), Valid: true} + buildError = sql.NullString{String: "build failed", Valid: true} + } + + switch prebuildStatus { + case database.ProvisionerJobStatusCanceling: + cancelledAt = sql.NullTime{Time: clock.Now().Add(earlier), Valid: true} + case database.ProvisionerJobStatusCanceled: + completedAt = sql.NullTime{Time: clock.Now().Add(earlier), Valid: true} + cancelledAt = sql.NullTime{Time: clock.Now().Add(earlier), Valid: true} + case database.ProvisionerJobStatusSucceeded: + completedAt = sql.NullTime{Time: clock.Now().Add(earlier), Valid: true} + default: + } + + workspace := dbgen.Workspace(t, db, database.WorkspaceTable{ + TemplateID: templateID, + OrganizationID: orgID, + OwnerID: ownerID, + Deleted: false, + }) + job := dbgen.ProvisionerJob(t, db, ps, database.ProvisionerJob{ + InitiatorID: initiatorID, + CreatedAt: clock.Now().Add(muchEarlier), + StartedAt: startedAt, + CompletedAt: completedAt, + CanceledAt: cancelledAt, + OrganizationID: orgID, + Error: buildError, + }) + dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ + WorkspaceID: workspace.ID, + InitiatorID: initiatorID, + TemplateVersionID: templateVersionID, + JobID: job.ID, + TemplateVersionPresetID: uuid.NullUUID{UUID: preset.ID, Valid: true}, + Transition: transition, + CreatedAt: clock.Now(), + }) + + return workspace +} + +var allTransitions = []database.WorkspaceTransition{ + database.WorkspaceTransitionStart, + database.WorkspaceTransitionStop, + database.WorkspaceTransitionDelete, +} + +var allJobStatuses = []database.ProvisionerJobStatus{ + database.ProvisionerJobStatusPending, + database.ProvisionerJobStatusRunning, + database.ProvisionerJobStatusSucceeded, + database.ProvisionerJobStatusFailed, + database.ProvisionerJobStatusCanceled, + database.ProvisionerJobStatusCanceling, +} + +// TODO (sasswart): test mutual exclusion diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 38e8e91ac8c1a..f01cb9c98dc64 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1695,6 +1695,13 @@ export interface PprofConfig { readonly address: string; } +// From codersdk/deployment.go +export interface PrebuildsConfig { + readonly reconciliation_interval: number; + readonly reconciliation_backoff_interval: number; + readonly reconciliation_backoff_lookback: number; +} + // From codersdk/presets.go export interface Preset { readonly ID: string; From aa02c9ffb8337e337bd2a63507109b7c88562144 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 17 Apr 2025 10:48:23 -0300 Subject: [PATCH 148/433] chore: reduce storybook flakes (#17427) A few storybook tests have been false positives quite frequently. To reduce this noise, I'm implementing a few hacks to avoid that. We can always rollback these changes if we notice they were leading to a lack in the tests. --- .../OverviewPage/OverviewPageView.stories.tsx | 5 +++++ .../OverviewPage/UserEngagementChart.tsx | 1 + .../PermissionPillsList.stories.tsx | 7 +++++++ .../JobRow.tsx | 2 +- .../ProvisionerRow.tsx | 4 ++-- .../TerminalPage/TerminalPage.stories.tsx | 19 +++++++++++++++++-- .../WorkspacePage/WorkspaceTopbar.stories.tsx | 5 +++++ site/src/utils/schedule.tsx | 8 ++++---- 8 files changed, 42 insertions(+), 9 deletions(-) diff --git a/site/src/pages/DeploymentSettingsPage/OverviewPage/OverviewPageView.stories.tsx b/site/src/pages/DeploymentSettingsPage/OverviewPage/OverviewPageView.stories.tsx index b3398f8b1f204..3535d4ffd1d47 100644 --- a/site/src/pages/DeploymentSettingsPage/OverviewPage/OverviewPageView.stories.tsx +++ b/site/src/pages/DeploymentSettingsPage/OverviewPage/OverviewPageView.stories.tsx @@ -39,6 +39,11 @@ const meta: Meta = { invalidExperiments: [], safeExperiments: [], }, + parameters: { + chromatic: { + diffThreshold: 0.5, + }, + }, }; export default meta; diff --git a/site/src/pages/DeploymentSettingsPage/OverviewPage/UserEngagementChart.tsx b/site/src/pages/DeploymentSettingsPage/OverviewPage/UserEngagementChart.tsx index 585088f02db1d..711e57242ce88 100644 --- a/site/src/pages/DeploymentSettingsPage/OverviewPage/UserEngagementChart.tsx +++ b/site/src/pages/DeploymentSettingsPage/OverviewPage/UserEngagementChart.tsx @@ -156,6 +156,7 @@ export const UserEngagementChart: FC = ({ data }) => { = { title: "pages/OrganizationCustomRolesPage/PermissionPillsList", component: PermissionPillsList, + decorators: [ + (Story) => ( +
+ +
+ ), + ], }; export default meta; diff --git a/site/src/pages/OrganizationSettingsPage/OrganizationProvisionerJobsPage/JobRow.tsx b/site/src/pages/OrganizationSettingsPage/OrganizationProvisionerJobsPage/JobRow.tsx index 94d4687565275..3e20863b25d51 100644 --- a/site/src/pages/OrganizationSettingsPage/OrganizationProvisionerJobsPage/JobRow.tsx +++ b/site/src/pages/OrganizationSettingsPage/OrganizationProvisionerJobsPage/JobRow.tsx @@ -121,7 +121,7 @@ export const JobRow: FC = ({ job }) => {
{job.metadata.workspace_name ?? "null"}
Creation time:
-
{job.created_at}
+
{job.created_at}
Queue:
diff --git a/site/src/pages/OrganizationSettingsPage/OrganizationProvisionersPage/ProvisionerRow.tsx b/site/src/pages/OrganizationSettingsPage/OrganizationProvisionersPage/ProvisionerRow.tsx index 2e40fe4d5388e..2c47578f67a6a 100644 --- a/site/src/pages/OrganizationSettingsPage/OrganizationProvisionersPage/ProvisionerRow.tsx +++ b/site/src/pages/OrganizationSettingsPage/OrganizationProvisionersPage/ProvisionerRow.tsx @@ -111,10 +111,10 @@ export const ProvisionerRow: FC = ({ ])} >
Last seen:
-
{provisioner.last_seen_at}
+
{provisioner.last_seen_at}
Creation time:
-
{provisioner.created_at}
+
{provisioner.created_at}
Version:
diff --git a/site/src/pages/TerminalPage/TerminalPage.stories.tsx b/site/src/pages/TerminalPage/TerminalPage.stories.tsx index aa24485353894..2eed419423c12 100644 --- a/site/src/pages/TerminalPage/TerminalPage.stories.tsx +++ b/site/src/pages/TerminalPage/TerminalPage.stories.tsx @@ -14,6 +14,7 @@ import { MockAuthMethodsAll, MockBuildInfo, MockDefaultOrganization, + MockDeploymentConfig, MockEntitlements, MockExperiments, MockUser, @@ -78,13 +79,27 @@ const meta = { data: { editWorkspaceProxies: true }, }, { key: ["me", "appearance"], data: MockUserAppearanceSettings }, + { + key: ["deployment", "config"], + data: { + ...MockDeploymentConfig, + config: { + ...MockDeploymentConfig.config, + web_terminal_renderer: "canvas", + }, + }, + }, ], - chromatic: { delay: 300 }, + chromatic: { + diffThreshold: 0.3, + }, }, decorators: [ (Story) => ( - +
+ +
), ], diff --git a/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx b/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx index f5706a4facc3b..482abc9d6fad1 100644 --- a/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx @@ -316,6 +316,11 @@ export const TemplateInfoPopover: Story = { ); }); }, + parameters: { + chromatic: { + diffThreshold: 0.3, + }, + }, }; export const TemplateInfoPopoverWithoutDisplayName: Story = { diff --git a/site/src/utils/schedule.tsx b/site/src/utils/schedule.tsx index 97479c021fe8c..21a112137ade0 100644 --- a/site/src/utils/schedule.tsx +++ b/site/src/utils/schedule.tsx @@ -148,7 +148,7 @@ export const autostopDisplay = ( if (template.autostop_requirement && template.allow_user_autostop) { title = Autostop schedule; reason = ( - <> + {" "} because this workspace has enabled autostop. You can disable autostop from this workspace's{" "} @@ -156,18 +156,18 @@ export const autostopDisplay = ( schedule settings . - + ); } return { message: `Stop ${deadline.fromNow()}`, tooltip: ( - <> + {title} This workspace will be stopped on{" "} {deadline.format("MMMM D [at] h:mm A")} {reason} - + ), danger: isShutdownSoon(workspace), }; From c8edadae10989c6aa667ef252abfb1cf585fdbc1 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 17 Apr 2025 10:57:02 -0300 Subject: [PATCH 149/433] refactor: redesign workspace status on workspaces table (#17425) Closes https://github.com/coder/coder/issues/17310 **Before:** Screenshot 2025-04-16 at 11 49 52 **After:** Screenshot 2025-04-16 at 11 49 19 **Notice!** - I've create a new size variation for the badge, `xs`. Since we reduced the line-height for the `text-xs` to be 16px instead of 18px, having a smaller badge, reducing the vertical size and horizontal paddings, just worked better. - I have to update Figma to reflect these changes. I tried, but I was not able to get it working and updated correctly. I'm going to take a pause during this week to learn that. - Updated the destructive, and warning badges to use borders as defined in the designs [here](https://www.figma.com/design/WfqIgsTFXN2BscBSSyXWF8/Coder-kit?node-id=489-3472&t=gfnYeLOIFUqHx6qv-0). --- site/src/components/Badge/Badge.tsx | 5 +- site/src/index.css | 6 +- .../WorkspaceDormantBadge.tsx | 12 +-- .../pages/WorkspacesPage/WorkspacesTable.tsx | 99 +++++++++++++------ site/src/utils/workspace.tsx | 37 ++++++- site/tailwind.config.js | 1 + 6 files changed, 119 insertions(+), 41 deletions(-) diff --git a/site/src/components/Badge/Badge.tsx b/site/src/components/Badge/Badge.tsx index 7ee7cc4f94fe0..e405966c8c235 100644 --- a/site/src/components/Badge/Badge.tsx +++ b/site/src/components/Badge/Badge.tsx @@ -17,9 +17,12 @@ export const badgeVariants = cva( default: "border-transparent bg-surface-secondary text-content-secondary shadow", warning: - "border-transparent bg-surface-orange text-content-warning shadow", + "border border-solid border-border-warning bg-surface-orange text-content-warning shadow", + destructive: + "border border-solid border-border-destructive bg-surface-red text-content-highlight-red shadow", }, size: { + xs: "text-2xs font-regular h-5 [&_svg]:hidden rounded px-1.5", sm: "text-2xs font-regular h-5.5 [&_svg]:size-icon-xs", md: "text-xs font-medium [&_svg]:size-icon-sm", }, diff --git a/site/src/index.css b/site/src/index.css index fe8699bc62b07..e2b71d7be6516 100644 --- a/site/src/index.css +++ b/site/src/index.css @@ -28,11 +28,13 @@ --surface-grey: 240 5% 96%; --surface-orange: 34 100% 92%; --surface-sky: 201 94% 86%; + --surface-red: 0 93% 94%; --border-default: 240 6% 90%; --border-success: 142 76% 36%; --border-warning: 30.66, 97.16%, 72.35%; --border-destructive: 0 84% 60%; - --border-hover: 240, 5%, 34%; + --border-warning: 27 96% 61%; + --border-hover: 240 5% 34%; --overlay-default: 240 5% 84% / 80%; --radius: 0.5rem; --highlight-purple: 262 83% 58%; @@ -66,10 +68,12 @@ --surface-grey: 240 6% 10%; --surface-orange: 13 81% 15%; --surface-sky: 204 80% 16%; + --surface-red: 0 75% 15%; --border-default: 240 4% 16%; --border-success: 142 76% 36%; --border-warning: 30.66, 97.16%, 72.35%; --border-destructive: 0 91% 71%; + --border-warning: 31 97% 72%; --border-hover: 240, 5%, 34%; --overlay-default: 240 10% 4% / 80%; --highlight-purple: 252 95% 85%; diff --git a/site/src/modules/workspaces/WorkspaceDormantBadge/WorkspaceDormantBadge.tsx b/site/src/modules/workspaces/WorkspaceDormantBadge/WorkspaceDormantBadge.tsx index 9c87cd4eae01c..f3c9c80d085fd 100644 --- a/site/src/modules/workspaces/WorkspaceDormantBadge/WorkspaceDormantBadge.tsx +++ b/site/src/modules/workspaces/WorkspaceDormantBadge/WorkspaceDormantBadge.tsx @@ -1,8 +1,6 @@ -import AutoDeleteIcon from "@mui/icons-material/AutoDelete"; -import RecyclingIcon from "@mui/icons-material/Recycling"; import Tooltip from "@mui/material/Tooltip"; import type { Workspace } from "api/typesGenerated"; -import { Pill } from "components/Pill/Pill"; +import { Badge } from "components/Badge/Badge"; import { formatDistanceToNow } from "date-fns"; import type { FC } from "react"; @@ -35,9 +33,9 @@ export const WorkspaceDormantBadge: FC = ({ } > - } type="error"> + Deletion Pending - + ) : ( = ({ } > - } type="warning"> + Dormant - + ); }; diff --git a/site/src/pages/WorkspacesPage/WorkspacesTable.tsx b/site/src/pages/WorkspacesPage/WorkspacesTable.tsx index 9fe72c23910e5..a9d585fccf58c 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesTable.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesTable.tsx @@ -13,6 +13,11 @@ import { AvatarData } from "components/Avatar/AvatarData"; import { AvatarDataSkeleton } from "components/Avatar/AvatarDataSkeleton"; import { InfoTooltip } from "components/InfoTooltip/InfoTooltip"; import { Stack } from "components/Stack/Stack"; +import { + StatusIndicator, + StatusIndicatorDot, + type StatusIndicatorProps, +} from "components/StatusIndicator/StatusIndicator"; import { Table, TableBody, @@ -25,19 +30,26 @@ import { TableLoaderSkeleton, TableRowSkeleton, } from "components/TableLoader/TableLoader"; +import dayjs from "dayjs"; +import relativeTime from "dayjs/plugin/relativeTime"; import { useClickableTableRow } from "hooks/useClickableTableRow"; import { useDashboard } from "modules/dashboard/useDashboard"; import { WorkspaceAppStatus } from "modules/workspaces/WorkspaceAppStatus/WorkspaceAppStatus"; import { WorkspaceDormantBadge } from "modules/workspaces/WorkspaceDormantBadge/WorkspaceDormantBadge"; import { WorkspaceOutdatedTooltip } from "modules/workspaces/WorkspaceOutdatedTooltip/WorkspaceOutdatedTooltip"; -import { WorkspaceStatusBadge } from "modules/workspaces/WorkspaceStatusBadge/WorkspaceStatusBadge"; -import { LastUsed } from "pages/WorkspacesPage/LastUsed"; import { type FC, type ReactNode, useMemo } from "react"; import { useNavigate } from "react-router-dom"; import { cn } from "utils/cn"; -import { getDisplayWorkspaceTemplateName } from "utils/workspace"; +import { + type DisplayWorkspaceStatusType, + getDisplayWorkspaceStatus, + getDisplayWorkspaceTemplateName, + lastUsedMessage, +} from "utils/workspace"; import { WorkspacesEmpty } from "./WorkspacesEmpty"; +dayjs.extend(relativeTime); + export interface WorkspacesTableProps { workspaces?: readonly Workspace[]; checkedWorkspaces: readonly Workspace[]; @@ -125,8 +137,7 @@ export const WorkspacesTable: FC = ({ {hasAppStatus && Activity} Template - Last used - Status + Status @@ -248,26 +259,7 @@ export const WorkspacesTable: FC = ({ /> - - - - - -
- - {workspace.latest_build.status === "running" && - !workspace.health.healthy && ( - - )} - {workspace.dormant_at && ( - - )} -
- +
@@ -345,14 +337,11 @@ const TableLoader: FC = ({ canCheckWorkspaces }) => { - - - - - + + - + @@ -362,3 +351,51 @@ const TableLoader: FC = ({ canCheckWorkspaces }) => { const cantBeChecked = (workspace: Workspace) => { return ["deleting", "pending"].includes(workspace.latest_build.status); }; + +type WorkspaceStatusCellProps = { + workspace: Workspace; +}; + +const variantByStatusType: Record< + DisplayWorkspaceStatusType, + StatusIndicatorProps["variant"] +> = { + active: "pending", + inactive: "inactive", + success: "success", + error: "failed", + danger: "warning", + warning: "warning", +}; + +const WorkspaceStatusCell: FC = ({ workspace }) => { + const { text, type } = getDisplayWorkspaceStatus( + workspace.latest_build.status, + workspace.latest_build.job, + ); + + return ( + +
+ + + {text} + {workspace.latest_build.status === "running" && + !workspace.health.healthy && ( + + )} + {workspace.dormant_at && ( + + )} + + + {lastUsedMessage(workspace.last_used_at)} + +
+ + ); +}; diff --git a/site/src/utils/workspace.tsx b/site/src/utils/workspace.tsx index 963adf58a7270..32fd6ce153d0e 100644 --- a/site/src/utils/workspace.tsx +++ b/site/src/utils/workspace.tsx @@ -168,14 +168,29 @@ export const getDisplayWorkspaceTemplateName = ( : workspace.template_name; }; +export type DisplayWorkspaceStatusType = + | "success" + | "active" + | "inactive" + | "error" + | "warning" + | "danger"; + +type DisplayWorkspaceStatus = { + text: string; + type: DisplayWorkspaceStatusType; + icon: React.ReactNode; +}; + export const getDisplayWorkspaceStatus = ( workspaceStatus: TypesGen.WorkspaceStatus, provisionerJob?: TypesGen.ProvisionerJob, -) => { +): DisplayWorkspaceStatus => { switch (workspaceStatus) { case undefined: return { text: "Loading", + type: "active", icon: , } as const; case "running": @@ -307,3 +322,23 @@ const FALLBACK_ICON = "/icon/widgets.svg"; export const getResourceIconPath = (resourceType: string): string => { return BUILT_IN_ICON_PATHS[resourceType] ?? FALLBACK_ICON; }; + +export const lastUsedMessage = (lastUsedAt: string | Date): string => { + const t = dayjs(lastUsedAt); + const now = dayjs(); + let message = t.fromNow(); + + if (t.isAfter(now.subtract(1, "hour"))) { + message = "Now"; + } else if (t.isAfter(now.subtract(3, "day"))) { + message = t.fromNow(); + } else if (t.isAfter(now.subtract(1, "month"))) { + message = t.fromNow(); + } else if (t.isAfter(now.subtract(100, "year"))) { + message = t.fromNow(); + } else { + message = "Never"; + } + + return message; +}; diff --git a/site/tailwind.config.js b/site/tailwind.config.js index 3e612408596f5..142a4711b56f3 100644 --- a/site/tailwind.config.js +++ b/site/tailwind.config.js @@ -49,6 +49,7 @@ module.exports = { grey: "hsl(var(--surface-grey))", orange: "hsl(var(--surface-orange))", sky: "hsl(var(--surface-sky))", + red: "hsl(var(--surface-red))", }, border: { DEFAULT: "hsl(var(--border-default))", From 5e4050e529130ad1ec164dce1f4244796c8ac5bf Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 17 Apr 2025 11:08:13 -0300 Subject: [PATCH 150/433] chore: fix additional storybook flakes (#17450) Fix new storybook flakes catch by https://www.chromatic.com/test?appId=624de63c6aacee003aa84340&id=680107825818a9747e57236c --- .../CustomRolesPage/PermissionPillsList.stories.tsx | 5 +++++ site/src/pages/TerminalPage/TerminalPage.stories.tsx | 2 +- site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx | 3 +++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/PermissionPillsList.stories.tsx b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/PermissionPillsList.stories.tsx index 5a4d896c2c425..56eb382067d84 100644 --- a/site/src/pages/OrganizationSettingsPage/CustomRolesPage/PermissionPillsList.stories.tsx +++ b/site/src/pages/OrganizationSettingsPage/CustomRolesPage/PermissionPillsList.stories.tsx @@ -13,6 +13,11 @@ const meta: Meta = {
), ], + parameters: { + chromatic: { + diffThreshold: 0.5, + }, + }, }; export default meta; diff --git a/site/src/pages/TerminalPage/TerminalPage.stories.tsx b/site/src/pages/TerminalPage/TerminalPage.stories.tsx index 2eed419423c12..7a34d57fbf83d 100644 --- a/site/src/pages/TerminalPage/TerminalPage.stories.tsx +++ b/site/src/pages/TerminalPage/TerminalPage.stories.tsx @@ -91,7 +91,7 @@ const meta = { }, ], chromatic: { - diffThreshold: 0.3, + diffThreshold: 0.5, }, }, decorators: [ diff --git a/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx b/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx index 482abc9d6fad1..1ae3ff9e2ebc9 100644 --- a/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceTopbar.stories.tsx @@ -38,6 +38,9 @@ const meta: Meta = { parameters: { layout: "fullscreen", features: ["advanced_template_scheduling"], + chromatic: { + diffThreshold: 0.3, + }, }, }; From 8723fe99f5b9512f1931db7731ac40721ca9d159 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 17 Apr 2025 17:40:37 +0100 Subject: [PATCH 151/433] feat: add slider to dynamic parameters (#17453) This adds the slider to the dynamic parameters component and does some additional styling cleanup for the dynamic parameters form Screenshot 2025-04-17 at 16 54 05 --- site/src/components/Checkbox/Checkbox.tsx | 6 ++--- .../DynamicParameter/DynamicParameter.tsx | 25 ++++++++++++++++++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/site/src/components/Checkbox/Checkbox.tsx b/site/src/components/Checkbox/Checkbox.tsx index 6bc1338955122..1278fb12ea899 100644 --- a/site/src/components/Checkbox/Checkbox.tsx +++ b/site/src/components/Checkbox/Checkbox.tsx @@ -18,7 +18,7 @@ export const Checkbox = React.forwardRef<
{(props.checked === true || props.defaultChecked === true) && ( - + )} {props.checked === "indeterminate" && ( - + )}
diff --git a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx index e1e79bdcd7a06..223c541bcfe9b 100644 --- a/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx +++ b/site/src/modules/workspaces/DynamicParameter/DynamicParameter.tsx @@ -22,6 +22,7 @@ import { SelectTrigger, SelectValue, } from "components/Select/Select"; +import { Slider } from "components/Slider/Slider"; import { Switch } from "components/Switch/Switch"; import { Tooltip, @@ -91,7 +92,7 @@ const ParameterLabel: FC = ({ parameter, isPreset }) => { )} -
+
{hasDescription && ( @@ -278,6 +284,23 @@ const ParameterField: FC = ({
); + + case "slider": + return ( + onChange(value.toString())} + min={parameter.validations[0]?.validation_min ?? 0} + max={parameter.validations[0]?.validation_max ?? 100} + disabled={disabled} + /> + ); + case "input": { const inputType = parameter.type === "number" ? "number" : "text"; const inputProps: Record = {}; From 144c60dd87e314afef664360e8a2a371551839f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=B1=E3=82=A4=E3=83=A9?= Date: Thu, 17 Apr 2025 10:17:09 -0700 Subject: [PATCH 152/433] chore: upgrade fish to v4 (#17440) --- .../files/etc/apt/sources.list.d/ppa.list | 2 +- .../files/usr/share/keyrings/fish-shell.gpg | Bin 371 -> 1163 bytes dogfood/coder/update-keys.sh | 8 ++++---- site/src/modules/resources/AgentLogs/mocks.tsx | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dogfood/coder/files/etc/apt/sources.list.d/ppa.list b/dogfood/coder/files/etc/apt/sources.list.d/ppa.list index a0d67bd17895a..fbdbef53ea60a 100644 --- a/dogfood/coder/files/etc/apt/sources.list.d/ppa.list +++ b/dogfood/coder/files/etc/apt/sources.list.d/ppa.list @@ -1,6 +1,6 @@ deb [signed-by=/usr/share/keyrings/ansible.gpg] https://ppa.launchpadcontent.net/ansible/ansible/ubuntu jammy main -deb [signed-by=/usr/share/keyrings/fish-shell.gpg] https://ppa.launchpadcontent.net/fish-shell/release-3/ubuntu/ jammy main +deb [signed-by=/usr/share/keyrings/fish-shell.gpg] https://ppa.launchpadcontent.net/fish-shell/release-4/ubuntu/ jammy main deb [signed-by=/usr/share/keyrings/git-core.gpg] https://ppa.launchpadcontent.net/git-core/ppa/ubuntu jammy main diff --git a/dogfood/coder/files/usr/share/keyrings/fish-shell.gpg b/dogfood/coder/files/usr/share/keyrings/fish-shell.gpg index 58ed31417d174aa9af164185a087044442ff9de0..bcaac170cb9d7fd284af9280110ba00cedba818d 100644 GIT binary patch delta 1132 zcmV-y1e5#o0*eWM#=%VlW;Bbz0T2MY^RR@UM%(osrGpfsz)b6XL5kRQUs8mhDk6w3 zRX?Zc@N^e(%%g@9*>*d{xI^NZlK~qKiDe|C#fLb>*e8$Lq)K=5%cdr1DiACD)O(Ut zFM6S|9*TFldmR{DY&QL~(NQudyWuu63>GS=l)jO#9ooW*~@btqy%}v0gZ4Q=;Xcc2#Gy{w{aYRUl?cEijQaxl(;e(4aZ9+f|JGj({eJSDZ1;bo$0507`Ks$w7N*|H$h zA|<(yt(l4Knf11m>Pg4lJe4%-J-UU&(5`*HuJP@EL0(mPxcwx`uPW&|m~QiuF<>xy zDnF>pm0GMt;GbizmAQi8;*F-#y@mS=`(^Q0VZf>^?3}YVPtxF6 z7GbSm0TENPTx}N~xy~ZmO;w&;panfZy77}9grJ+(Ezrhge56Z;MrQgf@Wi^YRUj+} z@YyF1dzPn#>?ci9Py6Herg3UxJmetVr1fY$_C2JeK##91ujXD-lRj^C*8gkpM_& za-9wkBp=h!6_~`$K(Dm#VQ}TL z5*VJUl!H{(`H9d11*VE4WviWVV&X$7wT4We>~%Pnf!&iO3SUUVO)nhuPbo&Q>A}(D zD2?e6npIl%wa&?pfrZii6heQujMd}$a9?Gj3T6QWXIGza_L=ZLIM>9+cmsYqb6f;G z4xeV-u*d>yWoV>h9j{d=r*y*JiyvZGn%;JmOehliB31tk(wYjcW6k;dd=aCJrUQ{n z%dJJ6)t%At;xTe(>QxTiG$&qxZd$rACWd9HPo3f)$;^1R><>LP`v`vm)Of@+SMjo^ zPIt=sEiVfJ3!;XMtDwb9;B6^s?Pa1oM%Z1K<8QJVq}hsI+pm8hX!5%^iN~{MMJARx zb|-Gzg(4JX4R+!drt2g(2g4s-M_jfglnO(gx_Xqj7l&({o+y#ia8(Cmnexhh{V$bE zXQm~=iT$Z<75uy3oo!T!nzhqZSj2_(C=zWMa!(WnrKj5m(rN_A*rk;7vLH8e1?>U3 y5+cL_KtB!kpa%#e3Y0}3#Z-A}jEy-Ev)LmD`7a5}#~Pm?Sy-kvTw|J$nX>s%^&bcT delta 335 zcmV-V0kHmy3G)Jf#*GA06gHs&1OTyA)UL$VZY7%RSYKH5W=zM4Afe#OR}OwmNs6f; zpLD-+H~%WPQm7Pm%m|aq|L+WFibhLF@Bm?UI!mf1pnip+A}B^~$n_Y>wToM&Mb4Ph zOz3?50xg^kqdYUQ&|h|t2Q;g+y=#`wsR+KqT*5}cIvkh_nLcOJ3DkvOz*gy#3j#2I zxC9dc0stZf0#Xz3qRs=o66^;V@Avlz0%VqdFDBy7BV5$y;&Ujmd=`KJMt^Jc$W^uqCwyGUL hbHXcyS^i>AHHuYNG$;&v69qdjViXA|p;^rN_OJG>i$MSY diff --git a/dogfood/coder/update-keys.sh b/dogfood/coder/update-keys.sh index 10b2660b5f58b..4d45f348bfcda 100755 --- a/dogfood/coder/update-keys.sh +++ b/dogfood/coder/update-keys.sh @@ -18,7 +18,7 @@ gpg_flags=( pushd "$PROJECT_ROOT/dogfood/coder/files/usr/share/keyrings" # Ansible PPA signing key -curl "${curl_flags[@]}" "https://keyserver.ubuntu.com/pks/lookup?op=get&search=0x6125e2a8c77f2818fb7bd15b93c4a3fd7bb9c367" | +curl "${curl_flags[@]}" "https://keyserver.ubuntu.com/pks/lookup?op=get&search=0X6125E2A8C77F2818FB7BD15B93C4A3FD7BB9C367" | gpg "${gpg_flags[@]}" --output="ansible.gpg" # Upstream Docker signing key @@ -26,7 +26,7 @@ curl "${curl_flags[@]}" "https://download.docker.com/linux/ubuntu/gpg" | gpg "${gpg_flags[@]}" --output="docker.gpg" # Fish signing key -curl "${curl_flags[@]}" "https://keyserver.ubuntu.com/pks/lookup?op=get&search=0x59fda1ce1b84b3fad89366c027557f056dc33ca5" | +curl "${curl_flags[@]}" "https://keyserver.ubuntu.com/pks/lookup?op=get&search=0x88421E703EDC7AF54967DED473C9FCC9E2BB48DA" | gpg "${gpg_flags[@]}" --output="fish-shell.gpg" # Git-Core signing key @@ -50,7 +50,7 @@ curl "${curl_flags[@]}" "https://apt.releases.hashicorp.com/gpg" | gpg "${gpg_flags[@]}" --output="hashicorp.gpg" # Helix signing key -curl "${curl_flags[@]}" "https://keyserver.ubuntu.com/pks/lookup?op=get&search=0x27642b9fd7f1a161fc2524e3355a4fa515d7c855" | +curl "${curl_flags[@]}" "https://keyserver.ubuntu.com/pks/lookup?op=get&search=0x27642B9FD7F1A161FC2524E3355A4FA515D7C855" | gpg "${gpg_flags[@]}" --output="helix.gpg" # Microsoft repository signing key (Edge) @@ -58,7 +58,7 @@ curl "${curl_flags[@]}" "https://packages.microsoft.com/keys/microsoft.asc" | gpg "${gpg_flags[@]}" --output="microsoft.gpg" # Neovim signing key -curl "${curl_flags[@]}" "https://keyserver.ubuntu.com/pks/lookup?op=get&search=0x9dbb0be9366964f134855e2255f96fcf8231b6dd" | +curl "${curl_flags[@]}" "https://keyserver.ubuntu.com/pks/lookup?op=get&search=0x9DBB0BE9366964F134855E2255F96FCF8231B6DD" | gpg "${gpg_flags[@]}" --output="neovim.gpg" # NodeSource signing key diff --git a/site/src/modules/resources/AgentLogs/mocks.tsx b/site/src/modules/resources/AgentLogs/mocks.tsx index 059e01fdbad64..de08e816614c0 100644 --- a/site/src/modules/resources/AgentLogs/mocks.tsx +++ b/site/src/modules/resources/AgentLogs/mocks.tsx @@ -612,7 +612,7 @@ export const MockLogs = [ id: 3295813, level: "info", output: - "Hit:16 https://ppa.launchpadcontent.net/fish-shell/release-3/ubuntu jammy InRelease", + "Hit:16 https://ppa.launchpadcontent.net/fish-shell/release-4/ubuntu jammy InRelease", time: "2024-03-14T11:31:07.827832Z", sourceId: "d9475581-8a42-4bce-b4d0-e4d2791d5c98", }, From 183146e2c981223747eb38be5e16ef00e1c97587 Mon Sep 17 00:00:00 2001 From: Yevhenii Shcherbina Date: Thu, 17 Apr 2025 13:43:24 -0400 Subject: [PATCH 153/433] fix: add minor fix to reconciliation loop (#17454) Follow-up PR to https://github.com/coder/coder/pull/17261 I noticed that 1 metrics-related test fails in `dk/prebuilds` after merging my PR into `dk/prebuilds`. --- coderd/prebuilds/preset_snapshot.go | 5 +++-- coderd/prebuilds/preset_snapshot_test.go | 9 +++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/coderd/prebuilds/preset_snapshot.go b/coderd/prebuilds/preset_snapshot.go index b6f05e588a6c0..2db9694f7f376 100644 --- a/coderd/prebuilds/preset_snapshot.go +++ b/coderd/prebuilds/preset_snapshot.go @@ -91,9 +91,10 @@ func (p PresetSnapshot) CalculateState() *ReconciliationState { extraneous int32 ) + // #nosec G115 - Safe conversion as p.Running slice length is expected to be within int32 range + actual = int32(len(p.Running)) + if p.isActive() { - // #nosec G115 - Safe conversion as p.Running slice length is expected to be within int32 range - actual = int32(len(p.Running)) desired = p.Preset.DesiredInstances.Int32 eligible = p.countEligible() extraneous = max(actual-desired, 0) diff --git a/coderd/prebuilds/preset_snapshot_test.go b/coderd/prebuilds/preset_snapshot_test.go index cce8ea67cb05c..a5acb40e5311f 100644 --- a/coderd/prebuilds/preset_snapshot_test.go +++ b/coderd/prebuilds/preset_snapshot_test.go @@ -146,7 +146,9 @@ func TestOutdatedPrebuilds(t *testing.T) { state := ps.CalculateState() actions, err := ps.CalculateActions(clock, backoffInterval) require.NoError(t, err) - validateState(t, prebuilds.ReconciliationState{}, *state) + validateState(t, prebuilds.ReconciliationState{ + Actual: 1, + }, *state) validateActions(t, prebuilds.ReconciliationActions{ ActionType: prebuilds.ActionTypeDelete, DeleteIDs: []uuid.UUID{outdated.prebuiltWorkspaceID}, @@ -208,6 +210,7 @@ func TestDeleteOutdatedPrebuilds(t *testing.T) { actions, err := ps.CalculateActions(clock, backoffInterval) require.NoError(t, err) validateState(t, prebuilds.ReconciliationState{ + Actual: 1, Deleting: 1, }, *state) @@ -530,7 +533,9 @@ func TestDeprecated(t *testing.T) { state := ps.CalculateState() actions, err := ps.CalculateActions(clock, backoffInterval) require.NoError(t, err) - validateState(t, prebuilds.ReconciliationState{}, *state) + validateState(t, prebuilds.ReconciliationState{ + Actual: 1, + }, *state) validateActions(t, prebuilds.ReconciliationActions{ ActionType: prebuilds.ActionTypeDelete, DeleteIDs: []uuid.UUID{current.prebuiltWorkspaceID}, From 90eacc17de890517cf270dc200f95f2e48e645c7 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 17 Apr 2025 21:16:08 +0100 Subject: [PATCH 154/433] fix: fix issues with dynamic parameters in the state (#17459) --- .../CreateWorkspacePageViewExperimental.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx index 3674884c1fb37..1e0fbbf2281ff 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx @@ -117,8 +117,8 @@ export const CreateWorkspacePageViewExperimental: FC< rich_parameter_values: useValidationSchemaForDynamicParameters(parameters), }), - enableReinitialize: true, - validateOnChange: false, + enableReinitialize: false, + validateOnChange: true, validateOnBlur: true, onSubmit: (request) => { if (!hasAllRequiredExternalAuth) { From ea65ddc17d208e9b0a9215eeb83882bcd81790fe Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 17 Apr 2025 15:42:23 -0500 Subject: [PATCH 155/433] fix: correct user roles being passed into terraform context (#17460) Roles were being passed into the workspace context incorrectly. Site wide scopes were being org scoped. Roles outside the org should also not be sent. --- .../provisionerdserver/provisionerdserver.go | 19 ++++++++---- .../provisionerdserver_test.go | 31 +++++++++++++++++-- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index a4e28741ce988..78f597fa55369 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -595,17 +595,24 @@ func (s *server) acquireProtoJob(ctx context.Context, job database.ProvisionerJo }) } - roles, err := s.Database.GetAuthorizationUserRoles(ctx, owner.ID) + allUserRoles, err := s.Database.GetAuthorizationUserRoles(ctx, owner.ID) if err != nil { return nil, failJob(fmt.Sprintf("get owner authorization roles: %s", err)) } ownerRbacRoles := []*sdkproto.Role{} - for _, role := range roles.Roles { - if s.OrganizationID == uuid.Nil { - ownerRbacRoles = append(ownerRbacRoles, &sdkproto.Role{Name: role, OrgId: ""}) - continue + roles, err := allUserRoles.RoleNames() + if err == nil { + for _, role := range roles { + if role.OrganizationID != uuid.Nil && role.OrganizationID != s.OrganizationID { + continue // Only include site wide and org specific roles + } + + orgID := role.OrganizationID.String() + if role.OrganizationID == uuid.Nil { + orgID = "" + } + ownerRbacRoles = append(ownerRbacRoles, &sdkproto.Role{Name: role.Name, OrgId: orgID}) } - ownerRbacRoles = append(ownerRbacRoles, &sdkproto.Role{Name: role, OrgId: s.OrganizationID.String()}) } protoJob.Type = &proto.AcquiredJob_WorkspaceBuild_{ diff --git a/coderd/provisionerdserver/provisionerdserver_test.go b/coderd/provisionerdserver/provisionerdserver_test.go index 9a9eb91ac8b73..caeef8a9793b7 100644 --- a/coderd/provisionerdserver/provisionerdserver_test.go +++ b/coderd/provisionerdserver/provisionerdserver_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "io" "net/url" + "slices" "strconv" "strings" "sync" @@ -22,6 +23,7 @@ import ( "storj.io/drpc" "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/quartz" "github.com/coder/serpent" @@ -203,6 +205,20 @@ func TestAcquireJob(t *testing.T) { GroupID: group1.ID, }) require.NoError(t, err) + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + UserID: user.ID, + OrganizationID: pd.OrganizationID, + Roles: []string{rbac.RoleOrgAuditor()}, + }) + + // Add extra erronous roles + secondOrg := dbgen.Organization(t, db, database.Organization{}) + dbgen.OrganizationMember(t, db, database.OrganizationMember{ + UserID: user.ID, + OrganizationID: secondOrg.ID, + Roles: []string{rbac.RoleOrgAuditor()}, + }) + link := dbgen.UserLink(t, db, database.UserLink{ LoginType: database.LoginTypeOIDC, UserID: user.ID, @@ -350,7 +366,7 @@ func TestAcquireJob(t *testing.T) { WorkspaceOwnerEmail: user.Email, WorkspaceOwnerName: user.Name, WorkspaceOwnerOidcAccessToken: link.OAuthAccessToken, - WorkspaceOwnerGroups: []string{group1.Name}, + WorkspaceOwnerGroups: []string{"Everyone", group1.Name}, WorkspaceId: workspace.ID.String(), WorkspaceOwnerId: user.ID.String(), TemplateId: template.ID.String(), @@ -361,11 +377,15 @@ func TestAcquireJob(t *testing.T) { WorkspaceOwnerSshPrivateKey: sshKey.PrivateKey, WorkspaceBuildId: build.ID.String(), WorkspaceOwnerLoginType: string(user.LoginType), - WorkspaceOwnerRbacRoles: []*sdkproto.Role{{Name: "member", OrgId: pd.OrganizationID.String()}}, + WorkspaceOwnerRbacRoles: []*sdkproto.Role{{Name: rbac.RoleOrgMember(), OrgId: pd.OrganizationID.String()}, {Name: "member", OrgId: ""}, {Name: rbac.RoleOrgAuditor(), OrgId: pd.OrganizationID.String()}}, } if prebuiltWorkspace { wantedMetadata.IsPrebuild = true } + + slices.SortFunc(wantedMetadata.WorkspaceOwnerRbacRoles, func(a, b *sdkproto.Role) int { + return strings.Compare(a.Name+a.OrgId, b.Name+b.OrgId) + }) want, err := json.Marshal(&proto.AcquiredJob_WorkspaceBuild_{ WorkspaceBuild: &proto.AcquiredJob_WorkspaceBuild{ WorkspaceBuildId: build.ID.String(), @@ -467,6 +487,13 @@ func TestAcquireJob(t *testing.T) { job, err := tc.acquire(ctx, srv) require.NoError(t, err) + // sort + if wk, ok := job.Type.(*proto.AcquiredJob_WorkspaceBuild_); ok { + slices.SortFunc(wk.WorkspaceBuild.Metadata.WorkspaceOwnerRbacRoles, func(a, b *sdkproto.Role) int { + return strings.Compare(a.Name+a.OrgId, b.Name+b.OrgId) + }) + } + got, err := json.Marshal(job.Type) require.NoError(t, err) From 2cc56ab5156287b83049ab6977215832c390a907 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=B1=E3=82=A4=E3=83=A9?= Date: Thu, 17 Apr 2025 13:51:50 -0700 Subject: [PATCH 156/433] chore: fill out workspace owner data for dynamic parameters (#17366) --- coderd/apidoc/docs.go | 66 +++-- coderd/apidoc/swagger.json | 62 +++-- coderd/coderd.go | 15 +- coderd/parameters.go | 250 ++++++++++++++++++ coderd/parameters_test.go | 134 ++++++++++ coderd/templateversions.go | 133 ---------- coderd/templateversions_test.go | 72 ----- .../groups/main.tf | 4 - .../groups/plan.json | 24 +- coderd/testdata/parameters/public_key/main.tf | 14 + .../testdata/parameters/public_key/plan.json | 80 ++++++ codersdk/parameters.go | 28 ++ codersdk/templateversions.go | 18 -- docs/reference/api/templates.md | 53 ++-- go.mod | 7 +- go.sum | 16 +- site/src/api/typesGenerated.ts | 4 +- 17 files changed, 635 insertions(+), 345 deletions(-) create mode 100644 coderd/parameters.go create mode 100644 coderd/parameters_test.go rename coderd/testdata/{dynamicparameters => parameters}/groups/main.tf (85%) rename coderd/testdata/{dynamicparameters => parameters}/groups/plan.json (76%) create mode 100644 coderd/testdata/parameters/public_key/main.tf create mode 100644 coderd/testdata/parameters/public_key/plan.json create mode 100644 codersdk/parameters.go diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index dcb7eba98b653..268cfd7a894ba 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -5686,35 +5686,6 @@ const docTemplate = `{ } } }, - "/templateversions/{templateversion}/dynamic-parameters": { - "get": { - "security": [ - { - "CoderSessionToken": [] - } - ], - "tags": [ - "Templates" - ], - "summary": "Open dynamic parameters WebSocket by template version", - "operationId": "open-dynamic-parameters-websocket-by-template-version", - "parameters": [ - { - "type": "string", - "format": "uuid", - "description": "Template version ID", - "name": "templateversion", - "in": "path", - "required": true - } - ], - "responses": { - "101": { - "description": "Switching Protocols" - } - } - } - }, "/templateversions/{templateversion}/external-auth": { "get": { "security": [ @@ -7570,6 +7541,43 @@ const docTemplate = `{ } } }, + "/users/{user}/templateversions/{templateversion}/parameters": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "tags": [ + "Templates" + ], + "summary": "Open dynamic parameters WebSocket by template version", + "operationId": "open-dynamic-parameters-websocket-by-template-version", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Template version ID", + "name": "user", + "in": "path", + "required": true + }, + { + "type": "string", + "format": "uuid", + "description": "Template version ID", + "name": "templateversion", + "in": "path", + "required": true + } + ], + "responses": { + "101": { + "description": "Switching Protocols" + } + } + } + }, "/users/{user}/webpush/subscription": { "post": { "security": [ diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 0464733070ef3..e973f11849547 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -5029,33 +5029,6 @@ } } }, - "/templateversions/{templateversion}/dynamic-parameters": { - "get": { - "security": [ - { - "CoderSessionToken": [] - } - ], - "tags": ["Templates"], - "summary": "Open dynamic parameters WebSocket by template version", - "operationId": "open-dynamic-parameters-websocket-by-template-version", - "parameters": [ - { - "type": "string", - "format": "uuid", - "description": "Template version ID", - "name": "templateversion", - "in": "path", - "required": true - } - ], - "responses": { - "101": { - "description": "Switching Protocols" - } - } - } - }, "/templateversions/{templateversion}/external-auth": { "get": { "security": [ @@ -6693,6 +6666,41 @@ } } }, + "/users/{user}/templateversions/{templateversion}/parameters": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "tags": ["Templates"], + "summary": "Open dynamic parameters WebSocket by template version", + "operationId": "open-dynamic-parameters-websocket-by-template-version", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Template version ID", + "name": "user", + "in": "path", + "required": true + }, + { + "type": "string", + "format": "uuid", + "description": "Template version ID", + "name": "templateversion", + "in": "path", + "required": true + } + ], + "responses": { + "101": { + "description": "Switching Protocols" + } + } + } + }, "/users/{user}/webpush/subscription": { "post": { "security": [ diff --git a/coderd/coderd.go b/coderd/coderd.go index 72ebce81120fa..e9d7a15a53059 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1108,10 +1108,6 @@ func New(options *Options) *API { // The idea is to return an empty [], so that the coder CLI won't get blocked accidentally. r.Get("/schema", templateVersionSchemaDeprecated) r.Get("/parameters", templateVersionParametersDeprecated) - r.Group(func(r chi.Router) { - r.Use(httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentDynamicParameters)) - r.Get("/dynamic-parameters", api.templateVersionDynamicParameters) - }) r.Get("/rich-parameters", api.templateVersionRichParameters) r.Get("/external-auth", api.templateVersionExternalAuth) r.Get("/variables", api.templateVersionVariables) @@ -1177,6 +1173,17 @@ func New(options *Options) *API { // organization member. This endpoint should match the authz story of // postWorkspacesByOrganization r.Post("/workspaces", api.postUserWorkspaces) + + // Similarly to creating a workspace, evaluating parameters for a + // new workspace should also match the authz story of + // postWorkspacesByOrganization + r.Route("/templateversions/{templateversion}", func(r chi.Router) { + r.Use( + httpmw.ExtractTemplateVersionParam(options.Database), + httpmw.RequireExperiment(api.Experiments, codersdk.ExperimentDynamicParameters), + ) + r.Get("/parameters", api.templateVersionDynamicParameters) + }) }) r.Group(func(r chi.Router) { diff --git a/coderd/parameters.go b/coderd/parameters.go new file mode 100644 index 0000000000000..78126789429d2 --- /dev/null +++ b/coderd/parameters.go @@ -0,0 +1,250 @@ +package coderd + +import ( + "context" + "database/sql" + "encoding/json" + "net/http" + "time" + + "github.com/google/uuid" + "golang.org/x/sync/errgroup" + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/coderd/httpmw" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/codersdk/wsjson" + "github.com/coder/preview" + previewtypes "github.com/coder/preview/types" + "github.com/coder/websocket" +) + +// @Summary Open dynamic parameters WebSocket by template version +// @ID open-dynamic-parameters-websocket-by-template-version +// @Security CoderSessionToken +// @Tags Templates +// @Param user path string true "Template version ID" format(uuid) +// @Param templateversion path string true "Template version ID" format(uuid) +// @Success 101 +// @Router /users/{user}/templateversions/{templateversion}/parameters [get] +func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http.Request) { + ctx, cancel := context.WithTimeout(r.Context(), 30*time.Minute) + defer cancel() + user := httpmw.UserParam(r) + templateVersion := httpmw.TemplateVersionParam(r) + + // Check that the job has completed successfully + job, err := api.Database.GetProvisionerJobByID(ctx, templateVersion.JobID) + if httpapi.Is404Error(err) { + httpapi.ResourceNotFound(rw) + return + } + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching provisioner job.", + Detail: err.Error(), + }) + return + } + if !job.CompletedAt.Valid { + httpapi.Write(ctx, rw, http.StatusTooEarly, codersdk.Response{ + Message: "Template version job has not finished", + }) + return + } + + // nolint:gocritic // We need to fetch the templates files for the Terraform + // evaluator, and the user likely does not have permission. + fileCtx := dbauthz.AsProvisionerd(ctx) + fileID, err := api.Database.GetFileIDByTemplateVersionID(fileCtx, templateVersion.ID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error finding template version Terraform.", + Detail: err.Error(), + }) + return + } + + fs, err := api.FileCache.Acquire(fileCtx, fileID) + defer api.FileCache.Release(fileID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: "Internal error fetching template version Terraform.", + Detail: err.Error(), + }) + return + } + + // Having the Terraform plan available for the evaluation engine is helpful + // for populating values from data blocks, but isn't strictly required. If + // we don't have a cached plan available, we just use an empty one instead. + plan := json.RawMessage("{}") + tf, err := api.Database.GetTemplateVersionTerraformValues(ctx, templateVersion.ID) + if err == nil { + plan = tf.CachedPlan + } else if !xerrors.Is(err, sql.ErrNoRows) { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to retrieve Terraform values for template version", + Detail: err.Error(), + }) + return + } + + owner, err := api.getWorkspaceOwnerData(ctx, user, templateVersion.OrganizationID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching workspace owner.", + Detail: err.Error(), + }) + return + } + + input := preview.Input{ + PlanJSON: plan, + ParameterValues: map[string]string{}, + Owner: owner, + } + + conn, err := websocket.Accept(rw, r, nil) + if err != nil { + httpapi.Write(ctx, rw, http.StatusUpgradeRequired, codersdk.Response{ + Message: "Failed to accept WebSocket.", + Detail: err.Error(), + }) + return + } + stream := wsjson.NewStream[codersdk.DynamicParametersRequest, codersdk.DynamicParametersResponse]( + conn, + websocket.MessageText, + websocket.MessageText, + api.Logger, + ) + + // Send an initial form state, computed without any user input. + result, diagnostics := preview.Preview(ctx, input, fs) + response := codersdk.DynamicParametersResponse{ + ID: -1, + Diagnostics: previewtypes.Diagnostics(diagnostics), + } + if result != nil { + response.Parameters = result.Parameters + } + err = stream.Send(response) + if err != nil { + stream.Drop() + return + } + + // As the user types into the form, reprocess the state using their input, + // and respond with updates. + updates := stream.Chan() + for { + select { + case <-ctx.Done(): + stream.Close(websocket.StatusGoingAway) + return + case update, ok := <-updates: + if !ok { + // The connection has been closed, so there is no one to write to + return + } + input.ParameterValues = update.Inputs + result, diagnostics := preview.Preview(ctx, input, fs) + response := codersdk.DynamicParametersResponse{ + ID: update.ID, + Diagnostics: previewtypes.Diagnostics(diagnostics), + } + if result != nil { + response.Parameters = result.Parameters + } + err = stream.Send(response) + if err != nil { + stream.Drop() + return + } + } + } +} + +func (api *API) getWorkspaceOwnerData( + ctx context.Context, + user database.User, + organizationID uuid.UUID, +) (previewtypes.WorkspaceOwner, error) { + var g errgroup.Group + + var ownerRoles []previewtypes.WorkspaceOwnerRBACRole + g.Go(func() error { + // nolint:gocritic // This is kind of the wrong query to use here, but it + // matches how the provisioner currently works. We should figure out + // something that needs less escalation but has the correct behavior. + row, err := api.Database.GetAuthorizationUserRoles(dbauthz.AsSystemRestricted(ctx), user.ID) + if err != nil { + return err + } + roles, err := row.RoleNames() + if err != nil { + return err + } + ownerRoles = make([]previewtypes.WorkspaceOwnerRBACRole, 0, len(roles)) + for _, it := range roles { + if it.OrganizationID != uuid.Nil && it.OrganizationID != organizationID { + continue + } + var orgID string + if it.OrganizationID != uuid.Nil { + orgID = it.OrganizationID.String() + } + ownerRoles = append(ownerRoles, previewtypes.WorkspaceOwnerRBACRole{ + Name: it.Name, + OrgID: orgID, + }) + } + return nil + }) + + var publicKey string + g.Go(func() error { + key, err := api.Database.GetGitSSHKey(ctx, user.ID) + if err != nil { + return err + } + publicKey = key.PublicKey + return nil + }) + + var groupNames []string + g.Go(func() error { + groups, err := api.Database.GetGroups(ctx, database.GetGroupsParams{ + OrganizationID: organizationID, + HasMemberID: user.ID, + }) + if err != nil { + return err + } + groupNames = make([]string, 0, len(groups)) + for _, it := range groups { + groupNames = append(groupNames, it.Group.Name) + } + return nil + }) + + err := g.Wait() + if err != nil { + return previewtypes.WorkspaceOwner{}, err + } + + return previewtypes.WorkspaceOwner{ + ID: user.ID.String(), + Name: user.Username, + FullName: user.Name, + Email: user.Email, + LoginType: string(user.LoginType), + RBACRoles: ownerRoles, + SSHPublicKey: publicKey, + Groups: groupNames, + }, nil +} diff --git a/coderd/parameters_test.go b/coderd/parameters_test.go new file mode 100644 index 0000000000000..60189e9aeaa33 --- /dev/null +++ b/coderd/parameters_test.go @@ -0,0 +1,134 @@ +package coderd_test + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/provisioner/echo" + "github.com/coder/coder/v2/provisionersdk/proto" + "github.com/coder/coder/v2/testutil" + "github.com/coder/websocket" +) + +func TestDynamicParametersOwnerGroups(t *testing.T) { + t.Parallel() + + cfg := coderdtest.DeploymentValues(t) + cfg.Experiments = []string{string(codersdk.ExperimentDynamicParameters)} + ownerClient := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, DeploymentValues: cfg}) + owner := coderdtest.CreateFirstUser(t, ownerClient) + templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin()) + + dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/groups/main.tf") + require.NoError(t, err) + dynamicParametersTerraformPlan, err := os.ReadFile("testdata/parameters/groups/plan.json") + require.NoError(t, err) + + files := echo.WithExtraFiles(map[string][]byte{ + "main.tf": dynamicParametersTerraformSource, + }) + files.ProvisionPlan = []*proto.Response{{ + Type: &proto.Response_Plan{ + Plan: &proto.PlanComplete{ + Plan: dynamicParametersTerraformPlan, + }, + }, + }} + + version := coderdtest.CreateTemplateVersion(t, templateAdmin, owner.OrganizationID, files) + coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, version.ID) + _ = coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID) + + ctx := testutil.Context(t, testutil.WaitShort) + stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, templateAdminUser.ID, version.ID) + require.NoError(t, err) + defer stream.Close(websocket.StatusGoingAway) + + previews := stream.Chan() + + // Should automatically send a form state with all defaulted/empty values + preview := testutil.RequireReceive(ctx, t, previews) + require.Equal(t, -1, preview.ID) + require.Empty(t, preview.Diagnostics) + require.Equal(t, "group", preview.Parameters[0].Name) + require.True(t, preview.Parameters[0].Value.Valid()) + require.Equal(t, "Everyone", preview.Parameters[0].Value.Value.AsString()) + + // Send a new value, and see it reflected + err = stream.Send(codersdk.DynamicParametersRequest{ + ID: 1, + Inputs: map[string]string{"group": "Bloob"}, + }) + require.NoError(t, err) + preview = testutil.RequireReceive(ctx, t, previews) + require.Equal(t, 1, preview.ID) + require.Empty(t, preview.Diagnostics) + require.Equal(t, "group", preview.Parameters[0].Name) + require.True(t, preview.Parameters[0].Value.Valid()) + require.Equal(t, "Bloob", preview.Parameters[0].Value.Value.AsString()) + + // Back to default + err = stream.Send(codersdk.DynamicParametersRequest{ + ID: 3, + Inputs: map[string]string{}, + }) + require.NoError(t, err) + preview = testutil.RequireReceive(ctx, t, previews) + require.Equal(t, 3, preview.ID) + require.Empty(t, preview.Diagnostics) + require.Equal(t, "group", preview.Parameters[0].Name) + require.True(t, preview.Parameters[0].Value.Valid()) + require.Equal(t, "Everyone", preview.Parameters[0].Value.Value.AsString()) +} + +func TestDynamicParametersOwnerSSHPublicKey(t *testing.T) { + t.Parallel() + + cfg := coderdtest.DeploymentValues(t) + cfg.Experiments = []string{string(codersdk.ExperimentDynamicParameters)} + ownerClient := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, DeploymentValues: cfg}) + owner := coderdtest.CreateFirstUser(t, ownerClient) + templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin()) + + dynamicParametersTerraformSource, err := os.ReadFile("testdata/parameters/public_key/main.tf") + require.NoError(t, err) + dynamicParametersTerraformPlan, err := os.ReadFile("testdata/parameters/public_key/plan.json") + require.NoError(t, err) + sshKey, err := templateAdmin.GitSSHKey(t.Context(), "me") + require.NoError(t, err) + + files := echo.WithExtraFiles(map[string][]byte{ + "main.tf": dynamicParametersTerraformSource, + }) + files.ProvisionPlan = []*proto.Response{{ + Type: &proto.Response_Plan{ + Plan: &proto.PlanComplete{ + Plan: dynamicParametersTerraformPlan, + }, + }, + }} + + version := coderdtest.CreateTemplateVersion(t, templateAdmin, owner.OrganizationID, files) + coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, version.ID) + _ = coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID) + + ctx := testutil.Context(t, testutil.WaitShort) + stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, templateAdminUser.ID, version.ID) + require.NoError(t, err) + defer stream.Close(websocket.StatusGoingAway) + + previews := stream.Chan() + + // Should automatically send a form state with all defaulted/empty values + preview := testutil.RequireReceive(ctx, t, previews) + require.Equal(t, -1, preview.ID) + require.Empty(t, preview.Diagnostics) + require.Equal(t, "public_key", preview.Parameters[0].Name) + require.True(t, preview.Parameters[0].Value.Valid()) + require.Equal(t, sshKey.PublicKey, preview.Parameters[0].Value.Value.AsString()) +} diff --git a/coderd/templateversions.go b/coderd/templateversions.go index a60897ddb725a..7b682eac14ea0 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -35,14 +35,10 @@ import ( "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/codersdk/wsjson" "github.com/coder/coder/v2/examples" "github.com/coder/coder/v2/provisioner/terraform/tfparse" "github.com/coder/coder/v2/provisionersdk" sdkproto "github.com/coder/coder/v2/provisionersdk/proto" - "github.com/coder/preview" - previewtypes "github.com/coder/preview/types" - "github.com/coder/websocket" ) // @Summary Get template version by ID @@ -270,135 +266,6 @@ func (api *API) patchCancelTemplateVersion(rw http.ResponseWriter, r *http.Reque }) } -// @Summary Open dynamic parameters WebSocket by template version -// @ID open-dynamic-parameters-websocket-by-template-version -// @Security CoderSessionToken -// @Tags Templates -// @Param templateversion path string true "Template version ID" format(uuid) -// @Success 101 -// @Router /templateversions/{templateversion}/dynamic-parameters [get] -func (api *API) templateVersionDynamicParameters(rw http.ResponseWriter, r *http.Request) { - ctx := r.Context() - templateVersion := httpmw.TemplateVersionParam(r) - - // Check that the job has completed successfully - job, err := api.Database.GetProvisionerJobByID(ctx, templateVersion.JobID) - if httpapi.Is404Error(err) { - httpapi.ResourceNotFound(rw) - return - } - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching provisioner job.", - Detail: err.Error(), - }) - return - } - if !job.CompletedAt.Valid { - httpapi.Write(ctx, rw, http.StatusTooEarly, codersdk.Response{ - Message: "Template version job has not finished", - }) - return - } - - // Having the Terraform plan available for the evaluation engine is helpful - // for populating values from data blocks, but isn't strictly required. If - // we don't have a cached plan available, we just use an empty one instead. - plan := json.RawMessage("{}") - tf, err := api.Database.GetTemplateVersionTerraformValues(ctx, templateVersion.ID) - if err == nil { - plan = tf.CachedPlan - } - - input := preview.Input{ - PlanJSON: plan, - ParameterValues: map[string]string{}, - // TODO: write a db query that fetches all of the data needed to fill out - // this owner value - Owner: previewtypes.WorkspaceOwner{ - Groups: []string{"Everyone"}, - }, - } - - // nolint:gocritic // We need to fetch the templates files for the Terraform - // evaluator, and the user likely does not have permission. - fileCtx := dbauthz.AsProvisionerd(ctx) - fileID, err := api.Database.GetFileIDByTemplateVersionID(fileCtx, templateVersion.ID) - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error finding template version Terraform.", - Detail: err.Error(), - }) - return - } - - fs, err := api.FileCache.Acquire(fileCtx, fileID) - defer api.FileCache.Release(fileID) - if err != nil { - httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ - Message: "Internal error fetching template version Terraform.", - Detail: err.Error(), - }) - return - } - - conn, err := websocket.Accept(rw, r, nil) - if err != nil { - httpapi.Write(ctx, rw, http.StatusUpgradeRequired, codersdk.Response{ - Message: "Failed to accept WebSocket.", - Detail: err.Error(), - }) - return - } - - stream := wsjson.NewStream[codersdk.DynamicParametersRequest, codersdk.DynamicParametersResponse](conn, websocket.MessageText, websocket.MessageText, api.Logger) - - // Send an initial form state, computed without any user input. - result, diagnostics := preview.Preview(ctx, input, fs) - response := codersdk.DynamicParametersResponse{ - ID: -1, - Diagnostics: previewtypes.Diagnostics(diagnostics), - } - if result != nil { - response.Parameters = result.Parameters - } - err = stream.Send(response) - if err != nil { - stream.Drop() - return - } - - // As the user types into the form, reprocess the state using their input, - // and respond with updates. - updates := stream.Chan() - for { - select { - case <-ctx.Done(): - stream.Close(websocket.StatusGoingAway) - return - case update, ok := <-updates: - if !ok { - // The connection has been closed, so there is no one to write to - return - } - input.ParameterValues = update.Inputs - result, diagnostics := preview.Preview(ctx, input, fs) - response := codersdk.DynamicParametersResponse{ - ID: update.ID, - Diagnostics: previewtypes.Diagnostics(diagnostics), - } - if result != nil { - response.Parameters = result.Parameters - } - err = stream.Send(response) - if err != nil { - stream.Drop() - return - } - } - } -} - // @Summary Get rich parameters by template version // @ID get-rich-parameters-by-template-version // @Security CoderSessionToken diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 83a5fd67a9761..e4027a1f14605 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "net/http" - "os" "regexp" "strings" "testing" @@ -28,7 +27,6 @@ import ( "github.com/coder/coder/v2/provisionersdk" "github.com/coder/coder/v2/provisionersdk/proto" "github.com/coder/coder/v2/testutil" - "github.com/coder/websocket" ) func TestTemplateVersion(t *testing.T) { @@ -2134,73 +2132,3 @@ func TestTemplateArchiveVersions(t *testing.T) { require.NoError(t, err, "fetch all versions") require.Len(t, remaining, totalVersions-len(expArchived)-len(allFailed)+1, "remaining versions") } - -func TestTemplateVersionDynamicParameters(t *testing.T) { - t.Parallel() - - cfg := coderdtest.DeploymentValues(t) - cfg.Experiments = []string{string(codersdk.ExperimentDynamicParameters)} - ownerClient := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, DeploymentValues: cfg}) - owner := coderdtest.CreateFirstUser(t, ownerClient) - templateAdmin, _ := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin()) - - dynamicParametersTerraformSource, err := os.ReadFile("testdata/dynamicparameters/groups/main.tf") - require.NoError(t, err) - dynamicParametersTerraformPlan, err := os.ReadFile("testdata/dynamicparameters/groups/plan.json") - require.NoError(t, err) - - files := echo.WithExtraFiles(map[string][]byte{ - "main.tf": dynamicParametersTerraformSource, - }) - files.ProvisionPlan = []*proto.Response{{ - Type: &proto.Response_Plan{ - Plan: &proto.PlanComplete{ - Plan: dynamicParametersTerraformPlan, - }, - }, - }} - - version := coderdtest.CreateTemplateVersion(t, templateAdmin, owner.OrganizationID, files) - coderdtest.AwaitTemplateVersionJobCompleted(t, templateAdmin, version.ID) - _ = coderdtest.CreateTemplate(t, templateAdmin, owner.OrganizationID, version.ID) - - ctx := testutil.Context(t, testutil.WaitShort) - stream, err := templateAdmin.TemplateVersionDynamicParameters(ctx, version.ID) - require.NoError(t, err) - defer stream.Close(websocket.StatusGoingAway) - - previews := stream.Chan() - - // Should automatically send a form state with all defaulted/empty values - preview := testutil.TryReceive(ctx, t, previews) - require.Empty(t, preview.Diagnostics) - require.Equal(t, "group", preview.Parameters[0].Name) - require.True(t, preview.Parameters[0].Value.Valid()) - require.Equal(t, "Everyone", preview.Parameters[0].Value.Value.AsString()) - - // Send a new value, and see it reflected - err = stream.Send(codersdk.DynamicParametersRequest{ - ID: 1, - Inputs: map[string]string{"group": "Bloob"}, - }) - require.NoError(t, err) - preview = testutil.TryReceive(ctx, t, previews) - require.Equal(t, 1, preview.ID) - require.Empty(t, preview.Diagnostics) - require.Equal(t, "group", preview.Parameters[0].Name) - require.True(t, preview.Parameters[0].Value.Valid()) - require.Equal(t, "Bloob", preview.Parameters[0].Value.Value.AsString()) - - // Back to default - err = stream.Send(codersdk.DynamicParametersRequest{ - ID: 3, - Inputs: map[string]string{}, - }) - require.NoError(t, err) - preview = testutil.TryReceive(ctx, t, previews) - require.Equal(t, 3, preview.ID) - require.Empty(t, preview.Diagnostics) - require.Equal(t, "group", preview.Parameters[0].Name) - require.True(t, preview.Parameters[0].Value.Valid()) - require.Equal(t, "Everyone", preview.Parameters[0].Value.Value.AsString()) -} diff --git a/coderd/testdata/dynamicparameters/groups/main.tf b/coderd/testdata/parameters/groups/main.tf similarity index 85% rename from coderd/testdata/dynamicparameters/groups/main.tf rename to coderd/testdata/parameters/groups/main.tf index a69b0463bb653..9356cc2840e91 100644 --- a/coderd/testdata/dynamicparameters/groups/main.tf +++ b/coderd/testdata/parameters/groups/main.tf @@ -8,10 +8,6 @@ terraform { data "coder_workspace_owner" "me" {} -output "groups" { - value = data.coder_workspace_owner.me.groups -} - data "coder_parameter" "group" { name = "group" default = try(data.coder_workspace_owner.me.groups[0], "") diff --git a/coderd/testdata/dynamicparameters/groups/plan.json b/coderd/testdata/parameters/groups/plan.json similarity index 76% rename from coderd/testdata/dynamicparameters/groups/plan.json rename to coderd/testdata/parameters/groups/plan.json index 8242f0dc43c58..1a6c45b40b7ab 100644 --- a/coderd/testdata/dynamicparameters/groups/plan.json +++ b/coderd/testdata/parameters/groups/plan.json @@ -17,12 +17,12 @@ "provider_name": "registry.terraform.io/coder/coder", "schema_version": 0, "values": { - "id": "25e81ec3-0eb9-4ee3-8b6d-738b8552f7a9", - "name": "default", - "email": "default@example.com", + "id": "", + "name": "", + "email": "", "groups": [], - "full_name": "default", - "login_type": null, + "full_name": "", + "login_type": "", "rbac_roles": [], "session_token": "", "ssh_public_key": "", @@ -74,19 +74,7 @@ "relevant_attributes": [ { "resource": "data.coder_workspace_owner.me", - "attribute": ["full_name"] - }, - { - "resource": "data.coder_workspace_owner.me", - "attribute": ["email"] - }, - { - "resource": "data.coder_workspace_owner.me", - "attribute": ["id"] - }, - { - "resource": "data.coder_workspace_owner.me", - "attribute": ["name"] + "attribute": ["groups"] } ] } diff --git a/coderd/testdata/parameters/public_key/main.tf b/coderd/testdata/parameters/public_key/main.tf new file mode 100644 index 0000000000000..6dd94d857d1fc --- /dev/null +++ b/coderd/testdata/parameters/public_key/main.tf @@ -0,0 +1,14 @@ +terraform { + required_providers { + coder = { + source = "coder/coder" + } + } +} + +data "coder_workspace_owner" "me" {} + +data "coder_parameter" "public_key" { + name = "public_key" + default = data.coder_workspace_owner.me.ssh_public_key +} diff --git a/coderd/testdata/parameters/public_key/plan.json b/coderd/testdata/parameters/public_key/plan.json new file mode 100644 index 0000000000000..3ff57d34b1015 --- /dev/null +++ b/coderd/testdata/parameters/public_key/plan.json @@ -0,0 +1,80 @@ +{ + "terraform_version": "1.11.2", + "format_version": "1.2", + "checks": [], + "complete": true, + "timestamp": "2025-04-02T01:29:59Z", + "variables": {}, + "prior_state": { + "values": { + "root_module": { + "resources": [ + { + "mode": "data", + "name": "me", + "type": "coder_workspace_owner", + "address": "data.coder_workspace_owner.me", + "provider_name": "registry.terraform.io/coder/coder", + "schema_version": 0, + "values": { + "id": "", + "name": "", + "email": "", + "groups": [], + "full_name": "", + "login_type": "", + "rbac_roles": [], + "session_token": "", + "ssh_public_key": "", + "ssh_private_key": "", + "oidc_access_token": "" + }, + "sensitive_values": { + "groups": [], + "rbac_roles": [], + "ssh_private_key": true + } + } + ], + "child_modules": [] + } + }, + "format_version": "1.0", + "terraform_version": "1.11.2" + }, + "configuration": { + "root_module": { + "resources": [ + { + "mode": "data", + "name": "me", + "type": "coder_workspace_owner", + "address": "data.coder_workspace_owner.me", + "schema_version": 0, + "provider_config_key": "coder" + } + ], + "variables": {}, + "module_calls": {} + }, + "provider_config": { + "coder": { + "name": "coder", + "full_name": "registry.terraform.io/coder/coder" + } + } + }, + "planned_values": { + "root_module": { + "resources": [], + "child_modules": [] + } + }, + "resource_changes": [], + "relevant_attributes": [ + { + "resource": "data.coder_workspace_owner.me", + "attribute": ["ssh_public_key"] + } + ] +} diff --git a/codersdk/parameters.go b/codersdk/parameters.go new file mode 100644 index 0000000000000..881aaf99f573c --- /dev/null +++ b/codersdk/parameters.go @@ -0,0 +1,28 @@ +package codersdk + +import ( + "context" + "fmt" + + "github.com/google/uuid" + + "github.com/coder/coder/v2/codersdk/wsjson" + previewtypes "github.com/coder/preview/types" + "github.com/coder/websocket" +) + +// FriendlyDiagnostic is included to guarantee it is generated in the output +// types. This is used as the type override for `previewtypes.Diagnostic`. +type FriendlyDiagnostic = previewtypes.FriendlyDiagnostic + +// NullHCLString is included to guarantee it is generated in the output +// types. This is used as the type override for `previewtypes.HCLString`. +type NullHCLString = previewtypes.NullHCLString + +func (c *Client) TemplateVersionDynamicParameters(ctx context.Context, userID, version uuid.UUID) (*wsjson.Stream[DynamicParametersResponse, DynamicParametersRequest], error) { + conn, err := c.Dial(ctx, fmt.Sprintf("/api/v2/users/%s/templateversions/%s/parameters", userID, version), nil) + if err != nil { + return nil, err + } + return wsjson.NewStream[DynamicParametersResponse, DynamicParametersRequest](conn, websocket.MessageText, websocket.MessageText, c.Logger()), nil +} diff --git a/codersdk/templateversions.go b/codersdk/templateversions.go index 0bcc4b5463903..42b381fadebce 100644 --- a/codersdk/templateversions.go +++ b/codersdk/templateversions.go @@ -10,9 +10,7 @@ import ( "github.com/google/uuid" - "github.com/coder/coder/v2/codersdk/wsjson" previewtypes "github.com/coder/preview/types" - "github.com/coder/websocket" ) type TemplateVersionWarning string @@ -141,22 +139,6 @@ type DynamicParametersResponse struct { // TODO: Workspace tags } -// FriendlyDiagnostic is included to guarantee it is generated in the output -// types. This is used as the type override for `previewtypes.Diagnostic`. -type FriendlyDiagnostic = previewtypes.FriendlyDiagnostic - -// NullHCLString is included to guarantee it is generated in the output -// types. This is used as the type override for `previewtypes.HCLString`. -type NullHCLString = previewtypes.NullHCLString - -func (c *Client) TemplateVersionDynamicParameters(ctx context.Context, version uuid.UUID) (*wsjson.Stream[DynamicParametersResponse, DynamicParametersRequest], error) { - conn, err := c.Dial(ctx, fmt.Sprintf("/api/v2/templateversions/%s/dynamic-parameters", version), nil) - if err != nil { - return nil, err - } - return wsjson.NewStream[DynamicParametersResponse, DynamicParametersRequest](conn, websocket.MessageText, websocket.MessageText, c.Logger()), nil -} - // TemplateVersionParameters returns parameters a template version exposes. func (c *Client) TemplateVersionRichParameters(ctx context.Context, version uuid.UUID) ([]TemplateVersionParameter, error) { res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/templateversions/%s/rich-parameters", version), nil) diff --git a/docs/reference/api/templates.md b/docs/reference/api/templates.md index 0f21cfccac670..ef136764bf2c5 100644 --- a/docs/reference/api/templates.md +++ b/docs/reference/api/templates.md @@ -2541,32 +2541,6 @@ Status Code **200** To perform this operation, you must be authenticated. [Learn more](authentication.md). -## Open dynamic parameters WebSocket by template version - -### Code samples - -```shell -# Example request using curl -curl -X GET http://coder-server:8080/api/v2/templateversions/{templateversion}/dynamic-parameters \ - -H 'Coder-Session-Token: API_KEY' -``` - -`GET /templateversions/{templateversion}/dynamic-parameters` - -### Parameters - -| Name | In | Type | Required | Description | -|-------------------|------|--------------|----------|---------------------| -| `templateversion` | path | string(uuid) | true | Template version ID | - -### Responses - -| Status | Meaning | Description | Schema | -|--------|--------------------------------------------------------------------------|---------------------|--------| -| 101 | [Switching Protocols](https://tools.ietf.org/html/rfc7231#section-6.2.2) | Switching Protocols | | - -To perform this operation, you must be authenticated. [Learn more](authentication.md). - ## Get external auth by template version ### Code samples @@ -3325,3 +3299,30 @@ Status Code **200** | `type` | `bool` | To perform this operation, you must be authenticated. [Learn more](authentication.md). + +## Open dynamic parameters WebSocket by template version + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/users/{user}/templateversions/{templateversion}/parameters \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /users/{user}/templateversions/{templateversion}/parameters` + +### Parameters + +| Name | In | Type | Required | Description | +|-------------------|------|--------------|----------|---------------------| +| `user` | path | string(uuid) | true | Template version ID | +| `templateversion` | path | string(uuid) | true | Template version ID | + +### Responses + +| Status | Meaning | Description | Schema | +|--------|--------------------------------------------------------------------------|---------------------|--------| +| 101 | [Switching Protocols](https://tools.ietf.org/html/rfc7231#section-6.2.2) | Switching Protocols | | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/go.mod b/go.mod index 826d5cd2c0235..11da4f20eb80d 100644 --- a/go.mod +++ b/go.mod @@ -139,7 +139,7 @@ require ( github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-reap v0.0.0-20170704170343-bf58d8a43e7b github.com/hashicorp/go-version v1.7.0 - github.com/hashicorp/hc-install v0.9.1 + github.com/hashicorp/hc-install v0.9.2 github.com/hashicorp/terraform-config-inspect v0.0.0-20211115214459-90acf1ca460f github.com/hashicorp/terraform-json v0.24.0 github.com/hashicorp/yamux v0.1.2 @@ -245,7 +245,7 @@ require ( github.com/KyleBanks/depth v1.2.1 // indirect github.com/Microsoft/go-winio v0.6.2 // indirect github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 // indirect - github.com/ProtonMail/go-crypto v1.1.5 // indirect + github.com/ProtonMail/go-crypto v1.1.6 // indirect github.com/agext/levenshtein v1.2.3 // indirect github.com/agnivade/levenshtein v1.2.1 // indirect github.com/akutz/memconn v0.1.0 // indirect @@ -488,7 +488,7 @@ require ( ) require ( - github.com/coder/preview v0.0.0-20250409162646-62939c63c71a + github.com/coder/preview v0.0.0-20250417203026-7edcb9e02d99 github.com/kylecarbs/aisdk-go v0.0.5 github.com/mark3labs/mcp-go v0.20.1 ) @@ -514,7 +514,6 @@ require ( github.com/hashicorp/go-getter v1.7.8 // indirect github.com/hashicorp/go-safetemp v1.0.0 // indirect github.com/klauspost/cpuid/v2 v2.2.10 // indirect - github.com/liamg/memoryfs v1.6.0 // indirect github.com/moby/sys/user v0.3.0 // indirect github.com/openai/openai-go v0.1.0-beta.6 // indirect github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect diff --git a/go.sum b/go.sum index 1943077cedafd..4bb24abd6be6b 100644 --- a/go.sum +++ b/go.sum @@ -680,8 +680,8 @@ github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 h1:TngWCqHvy9oXAN6lEVMRuU21PR1EtLVZJmdB18Gu3Rw= github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5/go.mod h1:lmUJ/7eu/Q8D7ML55dXQrVaamCz2vxCfdQBasLZfHKk= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= -github.com/ProtonMail/go-crypto v1.1.5 h1:eoAQfK2dwL+tFSFpr7TbOaPNUbPiJj4fLYwwGE1FQO4= -github.com/ProtonMail/go-crypto v1.1.5/go.mod h1:rA3QumHc/FZ8pAHreoekgiAbzpNsfQAosU5td4SnOrE= +github.com/ProtonMail/go-crypto v1.1.6 h1:ZcV+Ropw6Qn0AX9brlQLAUXfqLBc7Bl+f/DmNxpLfdw= +github.com/ProtonMail/go-crypto v1.1.6/go.mod h1:rA3QumHc/FZ8pAHreoekgiAbzpNsfQAosU5td4SnOrE= github.com/SherClockHolmes/webpush-go v1.4.0 h1:ocnzNKWN23T9nvHi6IfyrQjkIc0oJWv1B1pULsf9i3s= github.com/SherClockHolmes/webpush-go v1.4.0/go.mod h1:XSq8pKX11vNV8MJEMwjrlTkxhAj1zKfxmyhdV7Pd6UA= github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d h1:licZJFw2RwpHMqeKTCYkitsPqHNxTmd4SNR5r94FGM8= @@ -907,8 +907,8 @@ github.com/coder/pq v1.10.5-0.20240813183442-0c420cb5a048 h1:3jzYUlGH7ZELIH4XggX github.com/coder/pq v1.10.5-0.20240813183442-0c420cb5a048/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0 h1:3A0ES21Ke+FxEM8CXx9n47SZOKOpgSE1bbJzlE4qPVs= github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0/go.mod h1:5UuS2Ts+nTToAMeOjNlnHFkPahrtDkmpydBen/3wgZc= -github.com/coder/preview v0.0.0-20250409162646-62939c63c71a h1:1fvDm7hpNwKDQhHpStp7p1W05/33nBwptGorugNaE94= -github.com/coder/preview v0.0.0-20250409162646-62939c63c71a/go.mod h1:H9BInar+i5VALTTQ9Ulxmn94Eo2fWEhoxd0S9WakDIs= +github.com/coder/preview v0.0.0-20250417203026-7edcb9e02d99 h1:ek8akG49hG304Dsj6T+k8qd4t4rEjUyJ97EiQ9xqkYA= +github.com/coder/preview v0.0.0-20250417203026-7edcb9e02d99/go.mod h1:nyq3UKfaBu4jmA6ddJH05kD5K6paHEMLpRmwLdYJctU= github.com/coder/quartz v0.1.2 h1:PVhc9sJimTdKd3VbygXtS4826EOCpB1fXoRlLnCrE+s= github.com/coder/quartz v0.1.2/go.mod h1:vsiCc+AHViMKH2CQpGIpFgdHIEQsxwm8yCscqKmzbRA= github.com/coder/retry v1.5.1 h1:iWu8YnD8YqHs3XwqrqsjoBTAVqT9ml6z9ViJ2wlMiqc= @@ -1369,16 +1369,16 @@ github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ github.com/hashicorp/golang-lru v1.0.2 h1:dV3g9Z/unq5DpblPpw+Oqcv4dU/1omnb4Ok8iPY6p1c= github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= -github.com/hashicorp/hc-install v0.9.1 h1:gkqTfE3vVbafGQo6VZXcy2v5yoz2bE0+nhZXruCuODQ= -github.com/hashicorp/hc-install v0.9.1/go.mod h1:pWWvN/IrfeBK4XPeXXYkL6EjMufHkCK5DvwxeLKuBf0= +github.com/hashicorp/hc-install v0.9.2 h1:v80EtNX4fCVHqzL9Lg/2xkp62bbvQMnvPQ0G+OmtO24= +github.com/hashicorp/hc-install v0.9.2/go.mod h1:XUqBQNnuT4RsxoxiM9ZaUk0NX8hi2h+Lb6/c0OZnC/I= github.com/hashicorp/hcl v1.0.1-vault-7 h1:ag5OxFVy3QYTFTJODRzTKVZ6xvdfLLCA1cy/Y6xGI0I= github.com/hashicorp/hcl v1.0.1-vault-7/go.mod h1:XYhtn6ijBSAj6n4YqAaf7RBPS4I06AItNorpy+MoQNM= github.com/hashicorp/hcl/v2 v2.23.0 h1:Fphj1/gCylPxHutVSEOf2fBOh1VE4AuLV7+kbJf3qos= github.com/hashicorp/hcl/v2 v2.23.0/go.mod h1:62ZYHrXgPoX8xBnzl8QzbWq4dyDsDtfCRgIq1rbJEvA= github.com/hashicorp/logutils v1.0.0 h1:dLEQVugN8vlakKOUE3ihGLTZJRB4j+M2cdTm/ORI65Y= github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64= -github.com/hashicorp/terraform-exec v0.22.0 h1:G5+4Sz6jYZfRYUCg6eQgDsqTzkNXV+fP8l+uRmZHj64= -github.com/hashicorp/terraform-exec v0.22.0/go.mod h1:bjVbsncaeh8jVdhttWYZuBGj21FcYw6Ia/XfHcNO7lQ= +github.com/hashicorp/terraform-exec v0.23.0 h1:MUiBM1s0CNlRFsCLJuM5wXZrzA3MnPYEsiXmzATMW/I= +github.com/hashicorp/terraform-exec v0.23.0/go.mod h1:mA+qnx1R8eePycfwKkCRk3Wy65mwInvlpAeOwmA7vlY= github.com/hashicorp/terraform-json v0.24.0 h1:rUiyF+x1kYawXeRth6fKFm/MdfBS6+lW4NbeATsYz8Q= github.com/hashicorp/terraform-json v0.24.0/go.mod h1:Nfj5ubo9xbu9uiAoZVBsNOjvNKB66Oyrvtit74kC7ow= github.com/hashicorp/terraform-plugin-go v0.26.0 h1:cuIzCv4qwigug3OS7iKhpGAbZTiypAfFQmw8aE65O2M= diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index f01cb9c98dc64..d160b7683e92e 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -917,7 +917,7 @@ export const FeatureSets: FeatureSet[] = ["enterprise", "", "premium"]; // From codersdk/files.go export const FormatZip = "zip"; -// From codersdk/templateversions.go +// From codersdk/parameters.go export interface FriendlyDiagnostic { readonly severity: PreviewDiagnosticSeverityString; readonly summary: string; @@ -1401,7 +1401,7 @@ export interface NotificationsWebhookConfig { readonly endpoint: string; } -// From codersdk/templateversions.go +// From codersdk/parameters.go export interface NullHCLString { readonly value: string; readonly valid: boolean; From 5f0ce7f543f6b46b9db3f8e005dca55e45c5e160 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=82=B1=E3=82=A4=E3=83=A9?= Date: Thu, 17 Apr 2025 15:01:44 -0700 Subject: [PATCH 157/433] fix: update url for parameters websocket endpoint (#17462) --- site/src/api/api.ts | 3 ++- .../CreateWorkspacePageExperimental.tsx | 22 +++++++++++------ .../CreateWorkspacePageViewExperimental.tsx | 24 +++++-------------- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index f7e0cd0889f70..fa62afadee608 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -1010,6 +1010,7 @@ class ApiMethods { }; templateVersionDynamicParameters = ( + userId: string, versionId: string, { onMessage, @@ -1020,7 +1021,7 @@ class ApiMethods { }, ): WebSocket => { const socket = createWebSocket( - `/api/v2/templateversions/${versionId}/dynamic-parameters`, + `/api/v2/users/${userId}/templateversions/${versionId}/parameters`, ); socket.addEventListener("message", (event) => diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx index 27d76a23a83cd..5711517855ebd 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx @@ -57,6 +57,8 @@ const CreateWorkspacePageExperimental: FC = () => { const [mode, setMode] = useState(() => getWorkspaceMode(searchParams)); const [autoCreateError, setAutoCreateError] = useState(null); + const defaultOwner = me; + const [owner, setOwner] = useState(defaultOwner); const queryClient = useQueryClient(); const autoCreateWorkspaceMutation = useMutation( @@ -96,19 +98,23 @@ const CreateWorkspacePageExperimental: FC = () => { return; } - const socket = API.templateVersionDynamicParameters(realizedVersionId, { - onMessage, - onError: (error) => { - setWsError(error); + const socket = API.templateVersionDynamicParameters( + owner.id, + realizedVersionId, + { + onMessage, + onError: (error) => { + setWsError(error); + }, }, - }); + ); ws.current = socket; return () => { socket.close(); }; - }, [realizedVersionId, onMessage]); + }, [owner.id, realizedVersionId, onMessage]); const sendMessage = useCallback((formValues: Record) => { setWSResponseId((prevId) => { @@ -237,7 +243,9 @@ const CreateWorkspacePageExperimental: FC = () => { defaultName={defaultName} diagnostics={currentResponse?.diagnostics ?? []} disabledParams={disabledParams} - defaultOwner={me} + defaultOwner={defaultOwner} + owner={owner} + setOwner={setOwner} autofillParameters={autofillParameters} error={ wsError || diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx index 1e0fbbf2281ff..0b33c27d57434 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx @@ -22,14 +22,7 @@ import { useValidationSchemaForDynamicParameters, } from "modules/workspaces/DynamicParameter/DynamicParameter"; import { generateWorkspaceName } from "modules/workspaces/generateWorkspaceName"; -import { - type FC, - useCallback, - useEffect, - useId, - useMemo, - useState, -} from "react"; +import { type FC, useCallback, useEffect, useId, useState } from "react"; import { getFormHelpers, nameValidator } from "utils/formUtils"; import type { AutofillBuildParameter } from "utils/richParameters"; import * as Yup from "yup"; @@ -65,6 +58,8 @@ export interface CreateWorkspacePageViewExperimentalProps { resetMutation: () => void; sendMessage: (message: Record) => void; startPollingExternalAuth: () => void; + owner: TypesGen.User; + setOwner: (user: TypesGen.User) => void; } export const CreateWorkspacePageViewExperimental: FC< @@ -91,8 +86,9 @@ export const CreateWorkspacePageViewExperimental: FC< resetMutation, sendMessage, startPollingExternalAuth, + owner, + setOwner, }) => { - const [owner, setOwner] = useState(defaultOwner); const [suggestedName, setSuggestedName] = useState(() => generateWorkspaceName(), ); @@ -140,14 +136,6 @@ export const CreateWorkspacePageViewExperimental: FC< error, ); - const autofillByName = useMemo( - () => - Object.fromEntries( - autofillParameters.map((param) => [param.name, param]), - ), - [autofillParameters], - ); - const [presetOptions, setPresetOptions] = useState([ { label: "None", value: "" }, ]); @@ -252,7 +240,7 @@ export const CreateWorkspacePageViewExperimental: FC< return ( <> -
+