From aa5818cb6e80595d3de76f191beecb4f0d18f8c1 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 20 Feb 2024 11:49:38 +0100 Subject: [PATCH 1/9] update dependency on terraform-provider-coder --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 2befd4ba56749..73f300ef42c14 100644 --- a/go.mod +++ b/go.mod @@ -96,7 +96,7 @@ require ( github.com/coder/flog v1.1.0 github.com/coder/pretty v0.0.0-20230908205945-e89ba86370e0 github.com/coder/retry v1.5.1 - github.com/coder/terraform-provider-coder v0.17.0 + github.com/coder/terraform-provider-coder v0.18.0 github.com/coder/wgtunnel v0.1.13-0.20231127054351-578bfff9b92a github.com/coreos/go-oidc/v3 v3.9.0 github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf diff --git a/go.sum b/go.sum index e8125401358a3..84c8a2f481e38 100644 --- a/go.sum +++ b/go.sum @@ -204,8 +204,8 @@ github.com/coder/ssh v0.0.0-20231128192721-70855dedb788 h1:YoUSJ19E8AtuUFVYBpXuO github.com/coder/ssh v0.0.0-20231128192721-70855dedb788/go.mod h1:aGQbuCLyhRLMzZF067xc84Lh7JDs1FKwCmF1Crl9dxQ= github.com/coder/tailscale v1.1.1-0.20240214140224-3788ab894ba1 h1:A7dZHNidAVH6Kxn5D3hTEH+iRO8slnM0aRer6/cxlyE= github.com/coder/tailscale v1.1.1-0.20240214140224-3788ab894ba1/go.mod h1:L8tPrwSi31RAMEMV8rjb0vYTGs7rXt8rAHbqY/p41j4= -github.com/coder/terraform-provider-coder v0.17.0 h1:qwdLSbh6vPN+QDDvw1WNSYYEFlFwJFwzzP9vrvwr/ks= -github.com/coder/terraform-provider-coder v0.17.0/go.mod h1:pACHRoXSHBGyY696mLeQ1hR/Ag1G2wFk5bw0mT5Zp2g= +github.com/coder/terraform-provider-coder v0.18.0 h1:JWSBsOuzyiCev3C2Aj8Y1dvJkm5JMysIrIylMJtzPAY= +github.com/coder/terraform-provider-coder v0.18.0/go.mod h1:pACHRoXSHBGyY696mLeQ1hR/Ag1G2wFk5bw0mT5Zp2g= github.com/coder/wgtunnel v0.1.13-0.20231127054351-578bfff9b92a h1:KhR9LUVllMZ+e9lhubZ1HNrtJDgH5YLoTvpKwmrGag4= github.com/coder/wgtunnel v0.1.13-0.20231127054351-578bfff9b92a/go.mod h1:QzfptVUdEO+XbkzMKx1kw13i9wwpJlfI1RrZ6SNZ0hA= github.com/coder/wireguard-go v0.0.0-20230807234434-d825b45ccbf5 h1:eDk/42Kj4xN4yfE504LsvcFEo3dWUiCOaBiWJ2uIH2A= From 0e1f35c80a3fff610db856cc7b65f30654d4b5f5 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 20 Feb 2024 12:47:47 +0100 Subject: [PATCH 2/9] test --- codersdk/richparameters_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/codersdk/richparameters_test.go b/codersdk/richparameters_test.go index a7ab416b98bff..a3494be3b4a42 100644 --- a/codersdk/richparameters_test.go +++ b/codersdk/richparameters_test.go @@ -376,3 +376,24 @@ func TestParameterResolver_ValidateResolve_Ephemeral_UseEmptyDefault(t *testing. require.NoError(t, err) require.Equal(t, "", v) } + +func TestParameterResolver_ValidateResolve_Number_CustomError(t *testing.T) { + t.Parallel() + uut := codersdk.ParameterResolver{} + p := codersdk.TemplateVersionParameter{ + Name: "n", + Type: "number", + Mutable: true, + DefaultValue: "5", + + ValidationMin: ptr.Ref(int32(4)), + ValidationMax: ptr.Ref(int32(6)), + ValidationError: "These are values for testing purposes: {min}, {max}, and {value}.", + } + _, err := uut.ValidateResolve(p, &codersdk.WorkspaceBuildParameter{ + Name: "n", + Value: "8", + }) + require.Error(t, err) + require.Contains(t, err.Error(), "These are values for testing purposes: 4, 6, and 8.") +} From e57505d043b85d5149b17700d70dc69c18b74f97 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 20 Feb 2024 13:32:42 +0100 Subject: [PATCH 3/9] ts --- site/src/utils/richParameters.ts | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/site/src/utils/richParameters.ts b/site/src/utils/richParameters.ts index b09a70883cd98..c87b57b0e19e8 100644 --- a/site/src/utils/richParameters.ts +++ b/site/src/utils/richParameters.ts @@ -79,7 +79,7 @@ export const useValidationSchemaForRichParameters = ( if (Number(val) < templateParameter.validation_min) { return ctx.createError({ path: ctx.path, - message: `Value must be greater than ${templateParameter.validation_min}.`, + message: takeFirstError(errorRendered(templateParameter, val), `Value must be greater than ${templateParameter.validation_min}.`), }); } } else if ( @@ -89,7 +89,7 @@ export const useValidationSchemaForRichParameters = ( if (templateParameter.validation_max < Number(val)) { return ctx.createError({ path: ctx.path, - message: `Value must be less than ${templateParameter.validation_max}.`, + message: takeFirstError(errorRendered(templateParameter, val), `Value must be less than ${templateParameter.validation_max}.`), }); } } else if ( @@ -102,7 +102,7 @@ export const useValidationSchemaForRichParameters = ( ) { return ctx.createError({ path: ctx.path, - message: `Value must be between ${templateParameter.validation_min} and ${templateParameter.validation_max}.`, + message: takeFirstError(errorRendered(templateParameter, val), `Value must be between ${templateParameter.validation_min} and ${templateParameter.validation_max}.`), }); } } @@ -149,7 +149,7 @@ export const useValidationSchemaForRichParameters = ( if (val && !regex.test(val)) { return ctx.createError({ path: ctx.path, - message: `${templateParameter.validation_error} (value does not match the pattern ${templateParameter.validation_regex})`, + message: errorRendered(templateParameter, val), }); } } @@ -162,3 +162,25 @@ export const useValidationSchemaForRichParameters = ( ) .required(); }; + +const takeFirstError = (...errorMessages: string[]): string => { + for (const errorMessage of errorMessages) { + if (errorMessage) { + return errorMessage; + } + } + return "developer error: error message is not provided"; +}; + +const errorRendered = (parameter: TemplateVersionParameter, value?: string): string => { + if (!parameter.validation_error || !value) { + return ""; + } + + const r = new Map([ + ["{min}", parameter.validation_min !== undefined ? parameter.validation_min.toString() : ""], + ["{max}", parameter.validation_max !== undefined ? parameter.validation_max.toString() : ""], + ["{value}", value] + ]); + return parameter.validation_error.replace(/{min}|{max}|{value}/g, match => r.get(match) || ""); +} From c02c20fbf744131a97d65356c732aec0e6a18447 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 20 Feb 2024 13:41:05 +0100 Subject: [PATCH 4/9] fmt --- site/src/utils/richParameters.ts | 59 +++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/site/src/utils/richParameters.ts b/site/src/utils/richParameters.ts index c87b57b0e19e8..314c55b892489 100644 --- a/site/src/utils/richParameters.ts +++ b/site/src/utils/richParameters.ts @@ -79,7 +79,10 @@ export const useValidationSchemaForRichParameters = ( if (Number(val) < templateParameter.validation_min) { return ctx.createError({ path: ctx.path, - message: takeFirstError(errorRendered(templateParameter, val), `Value must be greater than ${templateParameter.validation_min}.`), + message: takeFirstError( + errorRendered(templateParameter, val), + `Value must be greater than ${templateParameter.validation_min}.`, + ), }); } } else if ( @@ -89,7 +92,10 @@ export const useValidationSchemaForRichParameters = ( if (templateParameter.validation_max < Number(val)) { return ctx.createError({ path: ctx.path, - message: takeFirstError(errorRendered(templateParameter, val), `Value must be less than ${templateParameter.validation_max}.`), + message: takeFirstError( + errorRendered(templateParameter, val), + `Value must be less than ${templateParameter.validation_max}.`, + ), }); } } else if ( @@ -102,7 +108,10 @@ export const useValidationSchemaForRichParameters = ( ) { return ctx.createError({ path: ctx.path, - message: takeFirstError(errorRendered(templateParameter, val), `Value must be between ${templateParameter.validation_min} and ${templateParameter.validation_max}.`), + message: takeFirstError( + errorRendered(templateParameter, val), + `Value must be between ${templateParameter.validation_min} and ${templateParameter.validation_max}.`, + ), }); } } @@ -165,22 +174,38 @@ export const useValidationSchemaForRichParameters = ( const takeFirstError = (...errorMessages: string[]): string => { for (const errorMessage of errorMessages) { - if (errorMessage) { - return errorMessage; - } + if (errorMessage) { + return errorMessage; + } } return "developer error: error message is not provided"; }; -const errorRendered = (parameter: TemplateVersionParameter, value?: string): string => { - if (!parameter.validation_error || !value) { - return ""; - } +const errorRendered = ( + parameter: TemplateVersionParameter, + value?: string, +): string => { + if (!parameter.validation_error || !value) { + return ""; + } - const r = new Map([ - ["{min}", parameter.validation_min !== undefined ? parameter.validation_min.toString() : ""], - ["{max}", parameter.validation_max !== undefined ? parameter.validation_max.toString() : ""], - ["{value}", value] - ]); - return parameter.validation_error.replace(/{min}|{max}|{value}/g, match => r.get(match) || ""); -} + const r = new Map([ + [ + "{min}", + parameter.validation_min !== undefined + ? parameter.validation_min.toString() + : "", + ], + [ + "{max}", + parameter.validation_max !== undefined + ? parameter.validation_max.toString() + : "", + ], + ["{value}", value], + ]); + return parameter.validation_error.replace( + /{min}|{max}|{value}/g, + (match) => r.get(match) || "", + ); +}; From 41c02e39daf50d0eeb91f96af06460569e742f98 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 20 Feb 2024 13:46:44 +0100 Subject: [PATCH 5/9] fmt --- .../src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx index d301e21bbe839..ef5c50d4be72f 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx @@ -25,7 +25,6 @@ import { rest } from "msw"; const nameLabelText = "Workspace Name"; const createWorkspaceText = "Create Workspace"; const validationNumberNotInRangeText = "Value must be between 1 and 3."; -const validationPatternNotMatched = `${MockTemplateVersionParameter3.validation_error} (value does not match the pattern ^[a-z]{3}$)`; const renderCreateWorkspacePage = () => { return renderWithAuth(, { @@ -152,7 +151,7 @@ describe("CreateWorkspacePage", () => { fireEvent.submit(thirdParameterField); const validationError = await screen.findByText( - validationPatternNotMatched, + MockTemplateVersionParameter3.validation_error as string, ); expect(validationError).toBeInTheDocument(); }); From 4a1ae603b1d63877b5ff94d9426ffd35c8f8b260 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 20 Feb 2024 14:13:48 +0100 Subject: [PATCH 6/9] test for ts --- .../CreateWorkspacePage.test.tsx | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx index ef5c50d4be72f..822c85efaaa46 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx @@ -156,6 +156,35 @@ describe("CreateWorkspacePage", () => { expect(validationError).toBeInTheDocument(); }); + it("rich parameter: number validation fails with custom error", async () => { + jest + .spyOn(API, "getTemplateVersionRichParameters") + .mockResolvedValueOnce([ + MockTemplateVersionParameter1, + { + ...MockTemplateVersionParameter2, + validation_error: "These are values: {min}, {max}, and {value}.", + validation_monotonic: undefined // only needs min-max rules + }, + ]); + + renderCreateWorkspacePage(); + await waitForLoaderToBeRemoved(); + + const secondParameterField = await screen.findByLabelText( + MockTemplateVersionParameter2.name, + { exact: false }, + ); + expect(secondParameterField).toBeDefined(); + fireEvent.change(secondParameterField, { + target: { value: "4" }, + }); + fireEvent.submit(secondParameterField); + + const validationError = await screen.findByText("These are values: 1, 3, and 4."); + expect(validationError).toBeInTheDocument(); + }); + it("auto create a workspace if uses mode=auto", async () => { const param = "first_parameter"; const paramValue = "It works!"; From cb46c9d9025160e49d2305a68b95e8b9c2e9cda9 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 20 Feb 2024 14:30:46 +0100 Subject: [PATCH 7/9] CLI test --- cli/create_test.go | 46 +++++++++++++++++++ .../CreateWorkspacePage.test.tsx | 22 ++++----- 2 files changed, 57 insertions(+), 11 deletions(-) diff --git a/cli/create_test.go b/cli/create_test.go index 903694167fd72..6ef1d06e3cfdd 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -556,6 +556,14 @@ func TestCreateValidateRichParameters(t *testing.T) { {Name: numberParameterName, Type: "number", Mutable: true, ValidationMin: ptr.Ref(int32(3)), ValidationMax: ptr.Ref(int32(10))}, } + numberCustomErrorRichParameters := []*proto.RichParameter{ + { + Name: numberParameterName, Type: "number", Mutable: true, + ValidationMin: ptr.Ref(int32(3)), ValidationMax: ptr.Ref(int32(10)), + ValidationError: "These are values: {min}, {max}, and {value}.", + }, + } + stringRichParameters := []*proto.RichParameter{ {Name: stringParameterName, Type: "string", Mutable: true, ValidationRegex: "^[a-z]+$", ValidationError: "this is error"}, } @@ -644,6 +652,44 @@ func TestCreateValidateRichParameters(t *testing.T) { <-doneChan }) + t.Run("ValidateNumber_CustomError", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + owner := coderdtest.CreateFirstUser(t, client) + member, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) + version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, prepareEchoResponses(numberCustomErrorRichParameters)) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + + template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID) + + inv, root := clitest.New(t, "create", "my-workspace", "--template", template.Name) + clitest.SetupConfig(t, member, root) + doneChan := make(chan struct{}) + pty := ptytest.New(t).Attach(inv) + go func() { + defer close(doneChan) + err := inv.Run() + assert.NoError(t, err) + }() + + matches := []string{ + numberParameterName, "12", + "These are values: 3, 10, and 12.", "", + "Enter a value", "8", + "Confirm create?", "yes", + } + for i := 0; i < len(matches); i += 2 { + match := matches[i] + value := matches[i+1] + pty.ExpectMatch(match) + if value != "" { + pty.WriteLine(value) + } + } + <-doneChan + }) + t.Run("ValidateBool", func(t *testing.T) { t.Parallel() diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx index 822c85efaaa46..61482cbb0f7a2 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePage.test.tsx @@ -157,16 +157,14 @@ describe("CreateWorkspacePage", () => { }); it("rich parameter: number validation fails with custom error", async () => { - jest - .spyOn(API, "getTemplateVersionRichParameters") - .mockResolvedValueOnce([ - MockTemplateVersionParameter1, - { - ...MockTemplateVersionParameter2, - validation_error: "These are values: {min}, {max}, and {value}.", - validation_monotonic: undefined // only needs min-max rules - }, - ]); + jest.spyOn(API, "getTemplateVersionRichParameters").mockResolvedValueOnce([ + MockTemplateVersionParameter1, + { + ...MockTemplateVersionParameter2, + validation_error: "These are values: {min}, {max}, and {value}.", + validation_monotonic: undefined, // only needs min-max rules + }, + ]); renderCreateWorkspacePage(); await waitForLoaderToBeRemoved(); @@ -181,7 +179,9 @@ describe("CreateWorkspacePage", () => { }); fireEvent.submit(secondParameterField); - const validationError = await screen.findByText("These are values: 1, 3, and 4."); + const validationError = await screen.findByText( + "These are values: 1, 3, and 4.", + ); expect(validationError).toBeInTheDocument(); }); From 0365c73d070d401e24136745336fce07a04b57bf Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 20 Feb 2024 14:50:28 +0100 Subject: [PATCH 8/9] docs --- docs/templates/parameters.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/templates/parameters.md b/docs/templates/parameters.md index 3dfc181cb4e9c..8084bec2a34db 100644 --- a/docs/templates/parameters.md +++ b/docs/templates/parameters.md @@ -249,6 +249,23 @@ data "coder_parameter" "instances" { } ``` +It is possible to override the default `error` message for a `number` parameter, +along with its associated `min` and/or `max` properties. The following message +placeholders are available `{min}`, `{max}`, and `{value}`. + +```hcl +data "coder_parameter" "instances" { + name = "Instances" + type = "number" + description = "Number of compute instances" + validation { + min = 1 + max = 4 + error = "Sorry, we can't provision too many instances - maximum limit: {max}, wanted: {value}." + } +} +``` + ### String You can validate a `string` parameter to match a regular expression. The `regex` From 9228348166c5a29879afa1c372683cd5635d7892 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 20 Feb 2024 15:46:58 +0100 Subject: [PATCH 9/9] Bruno's feedback --- site/src/utils/richParameters.ts | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/site/src/utils/richParameters.ts b/site/src/utils/richParameters.ts index 314c55b892489..a2baa4459ae68 100644 --- a/site/src/utils/richParameters.ts +++ b/site/src/utils/richParameters.ts @@ -79,10 +79,9 @@ export const useValidationSchemaForRichParameters = ( if (Number(val) < templateParameter.validation_min) { return ctx.createError({ path: ctx.path, - message: takeFirstError( - errorRendered(templateParameter, val), + message: + parameterError(templateParameter, val) ?? `Value must be greater than ${templateParameter.validation_min}.`, - ), }); } } else if ( @@ -92,10 +91,9 @@ export const useValidationSchemaForRichParameters = ( if (templateParameter.validation_max < Number(val)) { return ctx.createError({ path: ctx.path, - message: takeFirstError( - errorRendered(templateParameter, val), + message: + parameterError(templateParameter, val) ?? `Value must be less than ${templateParameter.validation_max}.`, - ), }); } } else if ( @@ -108,10 +106,9 @@ export const useValidationSchemaForRichParameters = ( ) { return ctx.createError({ path: ctx.path, - message: takeFirstError( - errorRendered(templateParameter, val), + message: + parameterError(templateParameter, val) ?? `Value must be between ${templateParameter.validation_min} and ${templateParameter.validation_max}.`, - ), }); } } @@ -158,7 +155,7 @@ export const useValidationSchemaForRichParameters = ( if (val && !regex.test(val)) { return ctx.createError({ path: ctx.path, - message: errorRendered(templateParameter, val), + message: parameterError(templateParameter, val), }); } } @@ -172,21 +169,12 @@ export const useValidationSchemaForRichParameters = ( .required(); }; -const takeFirstError = (...errorMessages: string[]): string => { - for (const errorMessage of errorMessages) { - if (errorMessage) { - return errorMessage; - } - } - return "developer error: error message is not provided"; -}; - -const errorRendered = ( +const parameterError = ( parameter: TemplateVersionParameter, value?: string, -): string => { +): string | undefined => { if (!parameter.validation_error || !value) { - return ""; + return; } const r = new Map([