From 350ba3d101496f76e24c67e42b9a433e5699fa14 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 13 Nov 2024 12:45:21 +0000 Subject: [PATCH 1/4] chore(docs): document how to correctly override list(string) parameters --- cli/create_test.go | 74 +++++++++++++++---- .../extending-templates/parameters.md | 29 ++++++++ 2 files changed, 87 insertions(+), 16 deletions(-) diff --git a/cli/create_test.go b/cli/create_test.go index 1f505d0523d84..c50c6bc46c5b4 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -1,11 +1,15 @@ package cli_test import ( + "bytes" "context" + "encoding/csv" + "encoding/json" "fmt" "net/http" "os" "regexp" + "strings" "testing" "time" @@ -864,24 +868,62 @@ func TestCreateValidateRichParameters(t *testing.T) { 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) - pty := ptytest.New(t).Attach(inv) - clitest.Start(t, inv) + t.Run("Prompt", func(t *testing.T) { + inv, root := clitest.New(t, "create", "my-workspace-1", "--template", template.Name) + clitest.SetupConfig(t, member, root) + pty := ptytest.New(t).Attach(inv) + clitest.Start(t, inv) - matches := []string{ - listOfStringsParameterName, "", - "aaa, bbb, ccc", "", - "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) + matches := []string{ + listOfStringsParameterName, "", + "aaa, bbb, ccc", "", + "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) + } + } + }) + + t.Run("Default", func(t *testing.T) { + t.Parallel() + inv, root := clitest.New(t, "create", "my-workspace-2", "--template", template.Name, "--yes") + clitest.SetupConfig(t, member, root) + clitest.Run(t, inv) + }) + + t.Run("CLIOverride/DoubleQuote", func(t *testing.T) { + t.Parallel() + inv, root := clitest.New(t, "create", "my-workspace-3", "--template", template.Name, "--parameter", "\""+listOfStringsParameterName+"=[\"\"ddd=foo\"\",\"\"eee=bar\"\",\"\"fff=baz\"\"]\"", "--yes") + clitest.SetupConfig(t, member, root) + clitest.Run(t, inv) + }) + + t.Run("CLIOverride/SingleQuote", func(t *testing.T) { + t.Parallel() + inv, root := clitest.New(t, "create", "my-workspace-4", "--template", template.Name, "--parameter", "\""+listOfStringsParameterName+"=[''ddd=foo'',''eee=bar'',''fff=baz'']\"", "--yes") + clitest.SetupConfig(t, member, root) + clitest.Run(t, inv) + }) + + t.Run("WhatShouldItLookLike", func(t *testing.T) { + t.Parallel() + + var ( + b bytes.Buffer + sb strings.Builder + ) + require.NoError(t, json.NewEncoder(&b).Encode([]string{"ddd=foo", "eee=bar", "fff=baz"})) + cw := csv.NewWriter(&sb) + require.NoError(t, cw.Write([]string{listOfStringsParameterName + "=" + b.String()})) + cw.Flush() + require.NoError(t, cw.Error()) + t.Logf("it looks like this: \n%q", strings.TrimSpace(sb.String())) + }) }) t.Run("ValidateListOfStrings_YAMLFile", func(t *testing.T) { diff --git a/docs/admin/templates/extending-templates/parameters.md b/docs/admin/templates/extending-templates/parameters.md index ee72f4bbe2dc4..acc30c25a9c9b 100644 --- a/docs/admin/templates/extending-templates/parameters.md +++ b/docs/admin/templates/extending-templates/parameters.md @@ -79,6 +79,35 @@ data "coder_parameter" "security_groups" { } ``` +> [!NOTE] Overriding a `list(string)` on the CLI is tricky because: +> +> - `--parameter "parameter_name=parameter_value"` is parsed as CSV. +> - `parameter_value` is parsed as JSON. +> +> So, to properly specify a `list(string)` with the `--parameter` CLI argument, +> you will need to take care of both CSV quoting and shell quoting. +> +> For the above example, to override the default values of the `security_groups` +> parameter, you will need to pass the following argument to `coder create`: +> +> ``` +> --parameter "\"security_groups=[\"\"DevOps Security Group\"\",\"\"Backend Security Group\"\"]\"" +> ``` +> +> You can use [this Go Playground link](https://go.dev/play/p/yvI9rdtS0ch) to generate a +> correctly-quoted argument. +> +> Alternatively, you can use `--rich-parameter-file` to work around the above issues. +> This allows you to specify parameters as YAML. +> An equivalent parameter file for the above `--parameter` is provided below: +> +> ```yaml +> security_groups: +> - DevOps Security Group +> - Backend Security Group +> ``` + + ## Options A `string` parameter can provide a set of options to limit the user's choices: From 0a7ddf02c3f6d0ec1645155a25614cb1571a1daf Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 13 Nov 2024 12:58:36 +0000 Subject: [PATCH 2/4] double quotes only, soz --- cli/create_test.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/cli/create_test.go b/cli/create_test.go index c50c6bc46c5b4..8386b8e8d919f 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -903,13 +903,6 @@ func TestCreateValidateRichParameters(t *testing.T) { clitest.Run(t, inv) }) - t.Run("CLIOverride/SingleQuote", func(t *testing.T) { - t.Parallel() - inv, root := clitest.New(t, "create", "my-workspace-4", "--template", template.Name, "--parameter", "\""+listOfStringsParameterName+"=[''ddd=foo'',''eee=bar'',''fff=baz'']\"", "--yes") - clitest.SetupConfig(t, member, root) - clitest.Run(t, inv) - }) - t.Run("WhatShouldItLookLike", func(t *testing.T) { t.Parallel() From 4e164c725e8b90ce17e1a4de2febbb6f754d33e7 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 13 Nov 2024 13:00:56 +0000 Subject: [PATCH 3/4] make fmt --- .../admin/templates/extending-templates/parameters.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/admin/templates/extending-templates/parameters.md b/docs/admin/templates/extending-templates/parameters.md index acc30c25a9c9b..ad935d74f91ef 100644 --- a/docs/admin/templates/extending-templates/parameters.md +++ b/docs/admin/templates/extending-templates/parameters.md @@ -94,12 +94,12 @@ data "coder_parameter" "security_groups" { > --parameter "\"security_groups=[\"\"DevOps Security Group\"\",\"\"Backend Security Group\"\"]\"" > ``` > -> You can use [this Go Playground link](https://go.dev/play/p/yvI9rdtS0ch) to generate a -> correctly-quoted argument. +> You can use [this Go Playground link](https://go.dev/play/p/yvI9rdtS0ch) to +> generate a correctly-quoted argument. > -> Alternatively, you can use `--rich-parameter-file` to work around the above issues. -> This allows you to specify parameters as YAML. -> An equivalent parameter file for the above `--parameter` is provided below: +> Alternatively, you can use `--rich-parameter-file` to work around the above +> issues. This allows you to specify parameters as YAML. An equivalent parameter +> file for the above `--parameter` is provided below: > > ```yaml > security_groups: @@ -107,7 +107,6 @@ data "coder_parameter" "security_groups" { > - Backend Security Group > ``` - ## Options A `string` parameter can provide a set of options to limit the user's choices: From b1d032648b1019e06a841327bf3aff94c0dcb779 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 13 Nov 2024 15:16:14 +0000 Subject: [PATCH 4/4] address PR comments --- cli/create_test.go | 41 ++++--------------- .../extending-templates/parameters.md | 3 -- 2 files changed, 8 insertions(+), 36 deletions(-) diff --git a/cli/create_test.go b/cli/create_test.go index 8386b8e8d919f..89f467ba6dd71 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -1,15 +1,11 @@ package cli_test import ( - "bytes" "context" - "encoding/csv" - "encoding/json" "fmt" "net/http" "os" "regexp" - "strings" "testing" "time" @@ -874,19 +870,10 @@ func TestCreateValidateRichParameters(t *testing.T) { pty := ptytest.New(t).Attach(inv) clitest.Start(t, inv) - matches := []string{ - listOfStringsParameterName, "", - "aaa, bbb, ccc", "", - "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) - } - } + pty.ExpectMatch(listOfStringsParameterName) + pty.ExpectMatch("aaa, bbb, ccc") + pty.ExpectMatch("Confirm create?") + pty.WriteLine("yes") }) t.Run("Default", func(t *testing.T) { @@ -898,25 +885,13 @@ func TestCreateValidateRichParameters(t *testing.T) { t.Run("CLIOverride/DoubleQuote", func(t *testing.T) { t.Parallel() - inv, root := clitest.New(t, "create", "my-workspace-3", "--template", template.Name, "--parameter", "\""+listOfStringsParameterName+"=[\"\"ddd=foo\"\",\"\"eee=bar\"\",\"\"fff=baz\"\"]\"", "--yes") + + // Note: see https://go.dev/play/p/vhTUTZsVrEb for how to escape this properly + parameterArg := fmt.Sprintf(`"%s=[""ddd=foo"",""eee=bar"",""fff=baz""]"`, listOfStringsParameterName) + inv, root := clitest.New(t, "create", "my-workspace-3", "--template", template.Name, "--parameter", parameterArg, "--yes") clitest.SetupConfig(t, member, root) clitest.Run(t, inv) }) - - t.Run("WhatShouldItLookLike", func(t *testing.T) { - t.Parallel() - - var ( - b bytes.Buffer - sb strings.Builder - ) - require.NoError(t, json.NewEncoder(&b).Encode([]string{"ddd=foo", "eee=bar", "fff=baz"})) - cw := csv.NewWriter(&sb) - require.NoError(t, cw.Write([]string{listOfStringsParameterName + "=" + b.String()})) - cw.Flush() - require.NoError(t, cw.Error()) - t.Logf("it looks like this: \n%q", strings.TrimSpace(sb.String())) - }) }) t.Run("ValidateListOfStrings_YAMLFile", func(t *testing.T) { diff --git a/docs/admin/templates/extending-templates/parameters.md b/docs/admin/templates/extending-templates/parameters.md index ad935d74f91ef..5ea82c0934b65 100644 --- a/docs/admin/templates/extending-templates/parameters.md +++ b/docs/admin/templates/extending-templates/parameters.md @@ -94,9 +94,6 @@ data "coder_parameter" "security_groups" { > --parameter "\"security_groups=[\"\"DevOps Security Group\"\",\"\"Backend Security Group\"\"]\"" > ``` > -> You can use [this Go Playground link](https://go.dev/play/p/yvI9rdtS0ch) to -> generate a correctly-quoted argument. -> > Alternatively, you can use `--rich-parameter-file` to work around the above > issues. This allows you to specify parameters as YAML. An equivalent parameter > file for the above `--parameter` is provided below: