From 30e99a5a2846376bc8def3a6ee94b9b9f3ab256f Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 4 Aug 2023 09:32:59 +0200 Subject: [PATCH 01/30] Use rich parameter flags --- cli/create.go | 20 ++++++++------------ cli/restart.go | 4 ++-- cli/start.go | 43 +++++++++++++++++++++++++++++++++++-------- cli/update.go | 16 +++++----------- 4 files changed, 50 insertions(+), 33 deletions(-) diff --git a/cli/create.go b/cli/create.go index 602b7b40a45bc..e4fe63800337a 100644 --- a/cli/create.go +++ b/cli/create.go @@ -18,11 +18,12 @@ import ( func (r *RootCmd) create() *clibase.Cmd { var ( - richParameterFile string - templateName string - startAt string - stopAfter time.Duration - workspaceName string + templateName string + startAt string + stopAfter time.Duration + workspaceName string + + parameterFlags workspaceParameterFlags ) client := new(codersdk.Client) cmd := &clibase.Cmd{ @@ -131,7 +132,7 @@ func (r *RootCmd) create() *clibase.Cmd { buildParams, err := prepWorkspaceBuild(inv, client, prepWorkspaceBuildArgs{ Template: template, - RichParameterFile: richParameterFile, + RichParameterFile: parameterFlags.richParameterFile, NewWorkspaceName: workspaceName, }) if err != nil { @@ -179,12 +180,6 @@ func (r *RootCmd) create() *clibase.Cmd { Description: "Specify a template name.", Value: clibase.StringOf(&templateName), }, - clibase.Option{ - Flag: "rich-parameter-file", - Env: "CODER_RICH_PARAMETER_FILE", - Description: "Specify a file path with values for rich parameters defined in the template.", - Value: clibase.StringOf(&richParameterFile), - }, clibase.Option{ Flag: "start-at", Env: "CODER_WORKSPACE_START_AT", @@ -199,6 +194,7 @@ func (r *RootCmd) create() *clibase.Cmd { }, cliui.SkipPromptOption(), ) + cmd.Options = append(cmd.Options, parameterFlags.richParameters()...) return cmd } diff --git a/cli/restart.go b/cli/restart.go index 4cff7ac7571d7..1a5351403bf2a 100644 --- a/cli/restart.go +++ b/cli/restart.go @@ -37,8 +37,8 @@ func (r *RootCmd) restart() *clibase.Cmd { } buildParams, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ - Template: template, - BuildOptions: parameterFlags.buildOptions, + Template: template, + PromptBuildOptions: parameterFlags.promptBuildOptions, }) if err != nil { return err diff --git a/cli/start.go b/cli/start.go index 5bd35867fd105..feb0f8e8c806c 100644 --- a/cli/start.go +++ b/cli/start.go @@ -11,17 +11,44 @@ import ( "github.com/coder/coder/codersdk" ) -// workspaceParameterFlags are used by "start", "restart", and "update". +// workspaceParameterFlags are used by commands requiring rich parameters and/or build options. type workspaceParameterFlags struct { - buildOptions bool + promptBuildOptions bool + buildOptions []string + + richParameterFile string + parameters []string } func (wpf *workspaceParameterFlags) options() []clibase.Option { return clibase.OptionSet{ + clibase.Option{ + Flag: "parameter", + Env: "CODER_BUILD_OPTION", + Description: `Build option value in the format "name=value"`, + Value: clibase.StringArrayOf(&wpf.buildOptions), + }, { Flag: "build-options", Description: "Prompt for one-time build options defined with ephemeral parameters.", - Value: clibase.BoolOf(&wpf.buildOptions), + Value: clibase.BoolOf(&wpf.promptBuildOptions), + }, + } +} + +func (wpf *workspaceParameterFlags) richParameters() []clibase.Option { + return clibase.OptionSet{ + clibase.Option{ + Flag: "parameter", + Env: "CODER_RICH_PARAMETER", + Description: `Rich parameter value in the format "name=value"`, + Value: clibase.StringArrayOf(&wpf.parameters), + }, + clibase.Option{ + Flag: "rich-parameter-file", + Env: "CODER_RICH_PARAMETER_FILE", + Description: "Specify a file path with values for rich parameters defined in the template.", + Value: clibase.StringOf(&wpf.richParameterFile), }, } } @@ -51,8 +78,8 @@ func (r *RootCmd) start() *clibase.Cmd { } buildParams, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ - Template: template, - BuildOptions: parameterFlags.buildOptions, + Template: template, + PromptBuildOptions: parameterFlags.promptBuildOptions, }) if err != nil { return err @@ -79,8 +106,8 @@ func (r *RootCmd) start() *clibase.Cmd { } type prepStartWorkspaceArgs struct { - Template codersdk.Template - BuildOptions bool + Template codersdk.Template + PromptBuildOptions bool } func prepStartWorkspace(inv *clibase.Invocation, client *codersdk.Client, args prepStartWorkspaceArgs) (*buildParameters, error) { @@ -97,7 +124,7 @@ func prepStartWorkspace(inv *clibase.Invocation, client *codersdk.Client, args p } richParameters := make([]codersdk.WorkspaceBuildParameter, 0) - if !args.BuildOptions { + if !args.PromptBuildOptions { return &buildParameters{ richParameters: richParameters, }, nil diff --git a/cli/update.go b/cli/update.go index 64710217bb996..980cc1e8c8ca8 100644 --- a/cli/update.go +++ b/cli/update.go @@ -9,8 +9,7 @@ import ( func (r *RootCmd) update() *clibase.Cmd { var ( - richParameterFile string - alwaysPrompt bool + alwaysPrompt bool parameterFlags workspaceParameterFlags ) @@ -30,7 +29,7 @@ func (r *RootCmd) update() *clibase.Cmd { if err != nil { return err } - if !workspace.Outdated && !alwaysPrompt && !parameterFlags.buildOptions { + if !workspace.Outdated && !alwaysPrompt && !parameterFlags.promptBuildOptions { _, _ = fmt.Fprintf(inv.Stdout, "Workspace isn't outdated!\n") return nil } @@ -50,13 +49,13 @@ func (r *RootCmd) update() *clibase.Cmd { buildParams, err := prepWorkspaceBuild(inv, client, prepWorkspaceBuildArgs{ Template: template, ExistingRichParams: existingRichParams, - RichParameterFile: richParameterFile, + RichParameterFile: parameterFlags.richParameterFile, NewWorkspaceName: workspace.Name, UpdateWorkspace: true, WorkspaceID: workspace.LatestBuild.ID, - BuildOptions: parameterFlags.buildOptions, + BuildOptions: parameterFlags.promptBuildOptions, }) if err != nil { return err @@ -92,13 +91,8 @@ func (r *RootCmd) update() *clibase.Cmd { Description: "Always prompt all parameters. Does not pull parameter values from existing workspace.", Value: clibase.BoolOf(&alwaysPrompt), }, - { - Flag: "rich-parameter-file", - Description: "Specify a file path with values for rich parameters defined in the template.", - Env: "CODER_RICH_PARAMETER_FILE", - Value: clibase.StringOf(&richParameterFile), - }, } cmd.Options = append(cmd.Options, parameterFlags.options()...) + cmd.Options = append(cmd.Options, parameterFlags.richParameters()...) return cmd } From 61e9785a3afe9fff32c569961d2ca9a84f1c1711 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 4 Aug 2023 10:26:49 +0200 Subject: [PATCH 02/30] WIP --- cli/create.go | 2 +- cli/start.go | 12 ++++++------ cli/update.go | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cli/create.go b/cli/create.go index e4fe63800337a..5153e8d6fcf0a 100644 --- a/cli/create.go +++ b/cli/create.go @@ -194,7 +194,7 @@ func (r *RootCmd) create() *clibase.Cmd { }, cliui.SkipPromptOption(), ) - cmd.Options = append(cmd.Options, parameterFlags.richParameters()...) + cmd.Options = append(cmd.Options, parameterFlags.parameters()...) return cmd } diff --git a/cli/start.go b/cli/start.go index feb0f8e8c806c..4cc849ea48377 100644 --- a/cli/start.go +++ b/cli/start.go @@ -17,15 +17,15 @@ type workspaceParameterFlags struct { buildOptions []string richParameterFile string - parameters []string + richParameters []string } func (wpf *workspaceParameterFlags) options() []clibase.Option { return clibase.OptionSet{ clibase.Option{ - Flag: "parameter", + Flag: "build-option", Env: "CODER_BUILD_OPTION", - Description: `Build option value in the format "name=value"`, + Description: `Build option value in the format "name=value".`, Value: clibase.StringArrayOf(&wpf.buildOptions), }, { @@ -36,13 +36,13 @@ func (wpf *workspaceParameterFlags) options() []clibase.Option { } } -func (wpf *workspaceParameterFlags) richParameters() []clibase.Option { +func (wpf *workspaceParameterFlags) parameters() []clibase.Option { return clibase.OptionSet{ clibase.Option{ Flag: "parameter", Env: "CODER_RICH_PARAMETER", - Description: `Rich parameter value in the format "name=value"`, - Value: clibase.StringArrayOf(&wpf.parameters), + Description: `Rich parameter value in the format "name=value".`, + Value: clibase.StringArrayOf(&wpf.richParameters), }, clibase.Option{ Flag: "rich-parameter-file", diff --git a/cli/update.go b/cli/update.go index 980cc1e8c8ca8..08b15114541c7 100644 --- a/cli/update.go +++ b/cli/update.go @@ -93,6 +93,6 @@ func (r *RootCmd) update() *clibase.Cmd { }, } cmd.Options = append(cmd.Options, parameterFlags.options()...) - cmd.Options = append(cmd.Options, parameterFlags.richParameters()...) + cmd.Options = append(cmd.Options, parameterFlags.parameters()...) return cmd } From 5d5e5884332ac51296af7fdb669742e8d2df816e Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 4 Aug 2023 10:38:14 +0200 Subject: [PATCH 03/30] make update-golden-files --- cli/testdata/coder_create_--help.golden | 3 +++ cli/testdata/coder_restart_--help.golden | 3 +++ cli/testdata/coder_start_--help.golden | 3 +++ cli/testdata/coder_update_--help.golden | 6 ++++++ 4 files changed, 15 insertions(+) diff --git a/cli/testdata/coder_create_--help.golden b/cli/testdata/coder_create_--help.golden index 6c8bcc46908c9..2e080b6e85ca7 100644 --- a/cli/testdata/coder_create_--help.golden +++ b/cli/testdata/coder_create_--help.golden @@ -7,6 +7,9 @@ Create a workspace  $ coder create /  Options + --parameter string-array, $CODER_RICH_PARAMETER + Rich parameter value in the format "name=value". + --rich-parameter-file string, $CODER_RICH_PARAMETER_FILE Specify a file path with values for rich parameters defined in the template. diff --git a/cli/testdata/coder_restart_--help.golden b/cli/testdata/coder_restart_--help.golden index c2079b9065dca..e16a6f9ff7e99 100644 --- a/cli/testdata/coder_restart_--help.golden +++ b/cli/testdata/coder_restart_--help.golden @@ -3,6 +3,9 @@ Usage: coder restart [flags] Restart a workspace Options + --build-option string-array, $CODER_BUILD_OPTION + Build option value in the format "name=value". + --build-options bool Prompt for one-time build options defined with ephemeral parameters. diff --git a/cli/testdata/coder_start_--help.golden b/cli/testdata/coder_start_--help.golden index aa447240e9bbb..b03c9975925f4 100644 --- a/cli/testdata/coder_start_--help.golden +++ b/cli/testdata/coder_start_--help.golden @@ -3,6 +3,9 @@ Usage: coder start [flags] Start a workspace Options + --build-option string-array, $CODER_BUILD_OPTION + Build option value in the format "name=value". + --build-options bool Prompt for one-time build options defined with ephemeral parameters. diff --git a/cli/testdata/coder_update_--help.golden b/cli/testdata/coder_update_--help.golden index 40e899cd37348..669bda831caa6 100644 --- a/cli/testdata/coder_update_--help.golden +++ b/cli/testdata/coder_update_--help.golden @@ -9,9 +9,15 @@ Use --always-prompt to change the parameter values of the workspace. Always prompt all parameters. Does not pull parameter values from existing workspace. + --build-option string-array, $CODER_BUILD_OPTION + Build option value in the format "name=value". + --build-options bool Prompt for one-time build options defined with ephemeral parameters. + --parameter string-array, $CODER_RICH_PARAMETER + Rich parameter value in the format "name=value". + --rich-parameter-file string, $CODER_RICH_PARAMETER_FILE Specify a file path with values for rich parameters defined in the template. From f739c8d365d86365108bd6505cf72cfde6fe1cbf Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 4 Aug 2023 09:02:53 +0000 Subject: [PATCH 04/30] make gen --- docs/cli/create.md | 9 +++++++++ docs/cli/restart.md | 9 +++++++++ docs/cli/start.md | 9 +++++++++ docs/cli/update.md | 18 ++++++++++++++++++ 4 files changed, 45 insertions(+) diff --git a/docs/cli/create.md b/docs/cli/create.md index 3f7ff099b16c8..8c36ea44a3dd7 100644 --- a/docs/cli/create.md +++ b/docs/cli/create.md @@ -20,6 +20,15 @@ coder create [flags] [name] ## Options +### --parameter + +| | | +| ----------- | ---------------------------------- | +| Type | string-array | +| Environment | $CODER_RICH_PARAMETER | + +Rich parameter value in the format "name=value". + ### --rich-parameter-file | | | diff --git a/docs/cli/restart.md b/docs/cli/restart.md index 72daa5dec405d..d3b6010a92c2e 100644 --- a/docs/cli/restart.md +++ b/docs/cli/restart.md @@ -12,6 +12,15 @@ coder restart [flags] ## Options +### --build-option + +| | | +| ----------- | -------------------------------- | +| Type | string-array | +| Environment | $CODER_BUILD_OPTION | + +Build option value in the format "name=value". + ### --build-options | | | diff --git a/docs/cli/start.md b/docs/cli/start.md index 8c3fd7f71276e..120edfde679eb 100644 --- a/docs/cli/start.md +++ b/docs/cli/start.md @@ -12,6 +12,15 @@ coder start [flags] ## Options +### --build-option + +| | | +| ----------- | -------------------------------- | +| Type | string-array | +| Environment | $CODER_BUILD_OPTION | + +Build option value in the format "name=value". + ### --build-options | | | diff --git a/docs/cli/update.md b/docs/cli/update.md index 0b3d31a9755b8..b81172df6b9ca 100644 --- a/docs/cli/update.md +++ b/docs/cli/update.md @@ -26,6 +26,15 @@ Use --always-prompt to change the parameter values of the workspace. Always prompt all parameters. Does not pull parameter values from existing workspace. +### --build-option + +| | | +| ----------- | -------------------------------- | +| Type | string-array | +| Environment | $CODER_BUILD_OPTION | + +Build option value in the format "name=value". + ### --build-options | | | @@ -34,6 +43,15 @@ Always prompt all parameters. Does not pull parameter values from existing works Prompt for one-time build options defined with ephemeral parameters. +### --parameter + +| | | +| ----------- | ---------------------------------- | +| Type | string-array | +| Environment | $CODER_RICH_PARAMETER | + +Rich parameter value in the format "name=value". + ### --rich-parameter-file | | | From e622f4f3c4a54371865ceb50458fcd4e29a14331 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 4 Aug 2023 11:10:15 +0200 Subject: [PATCH 05/30] fix --- cli/start.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/start.go b/cli/start.go index 4cc849ea48377..bdbff5ae42334 100644 --- a/cli/start.go +++ b/cli/start.go @@ -22,7 +22,7 @@ type workspaceParameterFlags struct { func (wpf *workspaceParameterFlags) options() []clibase.Option { return clibase.OptionSet{ - clibase.Option{ + { Flag: "build-option", Env: "CODER_BUILD_OPTION", Description: `Build option value in the format "name=value".`, From c8cbfe17e9f536c11422f6987c01fdb4afcd2e83 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 4 Aug 2023 11:32:28 +0200 Subject: [PATCH 06/30] Build options --- cli/create.go | 6 ++++-- cli/restart.go | 6 ++++++ cli/start.go | 26 +++++++++++++++++++++++++- cli/update.go | 9 ++++++++- 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/cli/create.go b/cli/create.go index 5153e8d6fcf0a..6b6ca55b5d624 100644 --- a/cli/create.go +++ b/cli/create.go @@ -205,8 +205,10 @@ type prepWorkspaceBuildArgs struct { NewWorkspaceName string UpdateWorkspace bool - BuildOptions bool WorkspaceID uuid.UUID + + PromptBuildOptions bool + BuildOptions []codersdk.WorkspaceBuildParameter } type buildParameters struct { @@ -244,7 +246,7 @@ func prepWorkspaceBuild(inv *clibase.Invocation, client *codersdk.Client, args p richParameters := make([]codersdk.WorkspaceBuildParameter, 0) PromptRichParamLoop: for _, templateVersionParameter := range templateVersionParameters { - if !args.BuildOptions && templateVersionParameter.Ephemeral { + if !args.PromptBuildOptions && templateVersionParameter.Ephemeral { continue } diff --git a/cli/restart.go b/cli/restart.go index 1a5351403bf2a..322911f19127a 100644 --- a/cli/restart.go +++ b/cli/restart.go @@ -36,9 +36,15 @@ func (r *RootCmd) restart() *clibase.Cmd { return err } + buildOptions, err := asWorkspaceBuildParameters(parameterFlags.buildOptions) + if err != nil { + return err + } + buildParams, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ Template: template, PromptBuildOptions: parameterFlags.promptBuildOptions, + BuildOptions: buildOptions, }) if err != nil { return err diff --git a/cli/start.go b/cli/start.go index bdbff5ae42334..e439c12be8fdf 100644 --- a/cli/start.go +++ b/cli/start.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "strings" "time" "golang.org/x/xerrors" @@ -77,9 +78,15 @@ func (r *RootCmd) start() *clibase.Cmd { return err } + buildOptions, err := asWorkspaceBuildParameters(parameterFlags.buildOptions) + if err != nil { + return err + } + buildParams, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ Template: template, PromptBuildOptions: parameterFlags.promptBuildOptions, + BuildOptions: buildOptions, }) if err != nil { return err @@ -106,8 +113,10 @@ func (r *RootCmd) start() *clibase.Cmd { } type prepStartWorkspaceArgs struct { - Template codersdk.Template + Template codersdk.Template + PromptBuildOptions bool + BuildOptions []codersdk.WorkspaceBuildParameter } func prepStartWorkspace(inv *clibase.Invocation, client *codersdk.Client, args prepStartWorkspaceArgs) (*buildParameters, error) { @@ -150,3 +159,18 @@ func prepStartWorkspace(inv *clibase.Invocation, client *codersdk.Client, args p richParameters: richParameters, }, nil } + +func asWorkspaceBuildParameters(nameValuePairs []string) ([]codersdk.WorkspaceBuildParameter, error) { + var params []codersdk.WorkspaceBuildParameter + for _, nameValue := range nameValuePairs { + split := strings.SplitN(nameValue, "=", 2) + if len(split) < 2 { + return nil, xerrors.Errorf("format key=value expected, but got %s", nameValue) + } + params = append(params, codersdk.WorkspaceBuildParameter{ + Name: split[0], + Value: split[1], + }) + } + return params, nil +} diff --git a/cli/update.go b/cli/update.go index 08b15114541c7..a4cab045047c1 100644 --- a/cli/update.go +++ b/cli/update.go @@ -33,6 +33,12 @@ func (r *RootCmd) update() *clibase.Cmd { _, _ = fmt.Fprintf(inv.Stdout, "Workspace isn't outdated!\n") return nil } + + buildOptions, err := asWorkspaceBuildParameters(parameterFlags.buildOptions) + if err != nil { + return err + } + template, err := client.Template(inv.Context(), workspace.TemplateID) if err != nil { return err @@ -55,7 +61,8 @@ func (r *RootCmd) update() *clibase.Cmd { UpdateWorkspace: true, WorkspaceID: workspace.LatestBuild.ID, - BuildOptions: parameterFlags.promptBuildOptions, + PromptBuildOptions: parameterFlags.promptBuildOptions, + BuildOptions: buildOptions, }) if err != nil { return err From f340daebb1d24869d7a75b480ebd4eec7baaebc1 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 4 Aug 2023 12:06:02 +0200 Subject: [PATCH 07/30] Pull build options from command line --- cli/start.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/cli/start.go b/cli/start.go index e439c12be8fdf..7612d1fd32c02 100644 --- a/cli/start.go +++ b/cli/start.go @@ -133,7 +133,7 @@ func prepStartWorkspace(inv *clibase.Invocation, client *codersdk.Client, args p } richParameters := make([]codersdk.WorkspaceBuildParameter, 0) - if !args.PromptBuildOptions { + if !args.PromptBuildOptions && len(args.BuildOptions) == 0 { return &buildParameters{ richParameters: richParameters, }, nil @@ -144,7 +144,7 @@ func prepStartWorkspace(inv *clibase.Invocation, client *codersdk.Client, args p continue } - parameterValue, err := cliui.RichParameter(inv, templateVersionParameter) + parameterValue, err := getParameterValueFromCommandLineOrInput(inv, args.PromptBuildOptions, args.BuildOptions, templateVersionParameter) if err != nil { return nil, err } @@ -174,3 +174,20 @@ func asWorkspaceBuildParameters(nameValuePairs []string) ([]codersdk.WorkspaceBu } return params, nil } + +//nolint:revive +func getParameterValueFromCommandLineOrInput(inv *clibase.Invocation, promptBuildOptions bool, buildOptions []codersdk.WorkspaceBuildParameter, templateVersionParameter codersdk.TemplateVersionParameter) (string, error) { + if !promptBuildOptions { + for _, bo := range buildOptions { + if bo.Name == templateVersionParameter.Name { + return bo.Value, nil + } + } + } + + parameterValue, err := cliui.RichParameter(inv, templateVersionParameter) + if err != nil { + return "", err + } + return parameterValue, nil +} From 4a96845dd67c8dd045e05509724882b95ae56cba Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 4 Aug 2023 15:09:12 +0200 Subject: [PATCH 08/30] refactor --- cli/create.go | 114 +++++++++------------------ cli/parameter.go | 157 +++++++++++++++++++++++++++----------- cli/parameter_resolver.go | 154 +++++++++++++++++++++++++++++++++++++ cli/restart.go | 10 ++- cli/start.go | 122 ++++------------------------- cli/update.go | 28 ++++--- 6 files changed, 342 insertions(+), 243 deletions(-) create mode 100644 cli/parameter_resolver.go diff --git a/cli/create.go b/cli/create.go index 6b6ca55b5d624..2c727ac207227 100644 --- a/cli/create.go +++ b/cli/create.go @@ -130,10 +130,18 @@ func (r *RootCmd) create() *clibase.Cmd { schedSpec = ptr.Ref(sched.String()) } - buildParams, err := prepWorkspaceBuild(inv, client, prepWorkspaceBuildArgs{ - Template: template, + cliRichParameters, err := asWorkspaceBuildParameters(parameterFlags.richParameters) + if err != nil { + return xerrors.Errorf("can't parse given parameter values: %w", err) + } + + richParameters, err := prepWorkspaceBuild(inv, client, prepWorkspaceBuildArgs{ + Action: WorkspaceCreate, + Template: template, + NewWorkspaceName: workspaceName, + RichParameterFile: parameterFlags.richParameterFile, - NewWorkspaceName: workspaceName, + RichParameters: cliRichParameters, }) if err != nil { return xerrors.Errorf("prepare build: %w", err) @@ -157,7 +165,7 @@ func (r *RootCmd) create() *clibase.Cmd { Name: workspaceName, AutostartSchedule: schedSpec, TTLMillis: ttlMillis, - RichParameterValues: buildParams.richParameters, + RichParameterValues: richParameters, }) if err != nil { return xerrors.Errorf("create workspace: %w", err) @@ -194,102 +202,56 @@ func (r *RootCmd) create() *clibase.Cmd { }, cliui.SkipPromptOption(), ) - cmd.Options = append(cmd.Options, parameterFlags.parameters()...) + cmd.Options = append(cmd.Options, parameterFlags.cliParameters()...) return cmd } type prepWorkspaceBuildArgs struct { - Template codersdk.Template - ExistingRichParams []codersdk.WorkspaceBuildParameter - RichParameterFile string - NewWorkspaceName string - - UpdateWorkspace bool - WorkspaceID uuid.UUID + Action WorkspaceCLIAction + Template codersdk.Template + NewWorkspaceName string + WorkspaceID uuid.UUID PromptBuildOptions bool BuildOptions []codersdk.WorkspaceBuildParameter -} -type buildParameters struct { - // Rich parameters stores values for build parameters annotated with description, icon, type, etc. - richParameters []codersdk.WorkspaceBuildParameter + LastBuildParameters []codersdk.WorkspaceBuildParameter + RichParameterFile string + RichParameters []codersdk.WorkspaceBuildParameter } // prepWorkspaceBuild will ensure a workspace build will succeed on the latest template version. -// Any missing params will be prompted to the user. It supports legacy and rich parameters. -func prepWorkspaceBuild(inv *clibase.Invocation, client *codersdk.Client, args prepWorkspaceBuildArgs) (*buildParameters, error) { +// Any missing params will be prompted to the user. It supports rich parameters. +func prepWorkspaceBuild(inv *clibase.Invocation, client *codersdk.Client, args prepWorkspaceBuildArgs) ([]codersdk.WorkspaceBuildParameter, error) { ctx := inv.Context() templateVersion, err := client.TemplateVersion(ctx, args.Template.ActiveVersionID) if err != nil { - return nil, err + return nil, xerrors.Errorf("get template version: %w", err) } - // Rich parameters templateVersionParameters, err := client.TemplateVersionRichParameters(inv.Context(), templateVersion.ID) if err != nil { return nil, xerrors.Errorf("get template version rich parameters: %w", err) } - parameterMapFromFile := map[string]string{} - useParamFile := false + parameterFile := map[string]string{} if args.RichParameterFile != "" { - useParamFile = true - _, _ = fmt.Fprintln(inv.Stdout, cliui.DefaultStyles.Paragraph.Render("Attempting to read the variables from the rich parameter file.")+"\r\n") - parameterMapFromFile, err = createParameterMapFromFile(args.RichParameterFile) + parameterFile, err = parseParameterMapFile(args.RichParameterFile) if err != nil { - return nil, err + return nil, xerrors.Errorf("can't parse parameter map file: %w", err) } } - disclaimerPrinted := false - richParameters := make([]codersdk.WorkspaceBuildParameter, 0) -PromptRichParamLoop: - for _, templateVersionParameter := range templateVersionParameters { - if !args.PromptBuildOptions && templateVersionParameter.Ephemeral { - continue - } - - if !disclaimerPrinted { - _, _ = fmt.Fprintln(inv.Stdout, cliui.DefaultStyles.Paragraph.Render("This template has customizable parameters. Values can be changed after create, but may have unintended side effects (like data loss).")+"\r\n") - disclaimerPrinted = true - } - - // Param file is all or nothing - if !useParamFile && !templateVersionParameter.Ephemeral { - for _, e := range args.ExistingRichParams { - if e.Name == templateVersionParameter.Name { - // If the param already exists, we do not need to prompt it again. - // The workspace scope will reuse params for each build. - continue PromptRichParamLoop - } - } - } - if args.UpdateWorkspace && !templateVersionParameter.Mutable { - // Check if the immutable parameter was used in the previous build. If so, then it isn't a fresh one - // and the user should be warned. - exists, err := workspaceBuildParameterExists(ctx, client, args.WorkspaceID, templateVersionParameter) - if err != nil { - return nil, err - } - - if exists { - _, _ = fmt.Fprintln(inv.Stdout, cliui.DefaultStyles.Warn.Render(fmt.Sprintf(`Parameter %q is not mutable, so can't be customized after workspace creation.`, templateVersionParameter.Name))) - continue - } - } - - parameterValue, err := getWorkspaceBuildParameterValueFromMapOrInput(inv, parameterMapFromFile, templateVersionParameter) - if err != nil { - return nil, err - } - - richParameters = append(richParameters, *parameterValue) - } - - if disclaimerPrinted { - _, _ = fmt.Fprintln(inv.Stdout) + resolver := new(ParameterResolver). + WithPromptBuildOptions(args.PromptBuildOptions). + WithBuildOptions(args.BuildOptions). + WithLastBuildParameters(args.LastBuildParameters). + WithRichParameters(args.RichParameters). + WithRichParametersFile(parameterFile) + buildParameters, err := resolver.Resolve(args.Action, templateVersionParameters) + if err != nil { + return nil, err } err = cliui.GitAuth(ctx, inv.Stdout, cliui.GitAuthOptions{ @@ -304,7 +266,7 @@ PromptRichParamLoop: // Run a dry-run with the given parameters to check correctness dryRun, err := client.CreateTemplateVersionDryRun(inv.Context(), templateVersion.ID, codersdk.CreateTemplateVersionDryRunRequest{ WorkspaceName: args.NewWorkspaceName, - RichParameterValues: richParameters, + RichParameterValues: buildParameters, }) if err != nil { return nil, xerrors.Errorf("begin workspace dry-run: %w", err) @@ -344,9 +306,7 @@ PromptRichParamLoop: return nil, xerrors.Errorf("get resources: %w", err) } - return &buildParameters{ - richParameters: richParameters, - }, nil + return buildParameters, nil } func workspaceBuildParameterExists(ctx context.Context, client *codersdk.Client, workspaceID uuid.UUID, templateVersionParameter codersdk.TemplateVersionParameter) (bool, error) { diff --git a/cli/parameter.go b/cli/parameter.go index 77e8ccdc8ee67..ecef0d9a57500 100644 --- a/cli/parameter.go +++ b/cli/parameter.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "os" + "strings" "golang.org/x/xerrors" "gopkg.in/yaml.v3" @@ -13,61 +14,131 @@ import ( "github.com/coder/coder/codersdk" ) -// Reads a YAML file and populates a string -> string map. -// Throws an error if the file name is empty. -func createParameterMapFromFile(parameterFile string) (map[string]string, error) { - if parameterFile != "" { - parameterFileContents, err := os.ReadFile(parameterFile) - if err != nil { - return nil, err - } +// workspaceParameterFlags are used by commands processing rich parameters and/or build options. +type workspaceParameterFlags struct { + promptBuildOptions bool + buildOptions []string - mapStringInterface := make(map[string]interface{}) - err = yaml.Unmarshal(parameterFileContents, &mapStringInterface) - if err != nil { - return nil, err - } + richParameterFile string + richParameters []string +} - parameterMap := map[string]string{} - for k, v := range mapStringInterface { - switch val := v.(type) { - case string, bool, int: - parameterMap[k] = fmt.Sprintf("%v", val) - case []interface{}: - b, err := json.Marshal(&val) - if err != nil { - return nil, err - } - parameterMap[k] = string(b) - default: - return nil, xerrors.Errorf("invalid parameter type: %T", v) - } - } - return parameterMap, nil +func (wpf *workspaceParameterFlags) cliBuildOptions() []clibase.Option { + return clibase.OptionSet{ + { + Flag: "build-option", + Env: "CODER_BUILD_OPTION", + Description: `Build option value in the format "name=value".`, + Value: clibase.StringArrayOf(&wpf.buildOptions), + }, + { + Flag: "build-options", + Description: "Prompt for one-time build options defined with ephemeral parameters.", + Value: clibase.BoolOf(&wpf.promptBuildOptions), + }, + } +} + +func (wpf *workspaceParameterFlags) cliParameters() []clibase.Option { + return clibase.OptionSet{ + clibase.Option{ + Flag: "parameter", + Env: "CODER_RICH_PARAMETER", + Description: `Rich parameter value in the format "name=value".`, + Value: clibase.StringArrayOf(&wpf.richParameters), + }, + clibase.Option{ + Flag: "rich-parameter-file", + Env: "CODER_RICH_PARAMETER_FILE", + Description: "Specify a file path with values for rich parameters defined in the template.", + Value: clibase.StringOf(&wpf.richParameterFile), + }, } +} - return nil, xerrors.Errorf("Parameter file name is not specified") +func asWorkspaceBuildParameters(nameValuePairs []string) ([]codersdk.WorkspaceBuildParameter, error) { + var params []codersdk.WorkspaceBuildParameter + for _, nameValue := range nameValuePairs { + split := strings.SplitN(nameValue, "=", 2) + if len(split) < 2 { + return nil, xerrors.Errorf("format key=value expected, but got %s", nameValue) + } + params = append(params, codersdk.WorkspaceBuildParameter{ + Name: split[0], + Value: split[1], + }) + } + return params, nil } -func getWorkspaceBuildParameterValueFromMapOrInput(inv *clibase.Invocation, parameterMap map[string]string, templateVersionParameter codersdk.TemplateVersionParameter) (*codersdk.WorkspaceBuildParameter, error) { - var parameterValue string - var err error - if parameterMap != nil { - var ok bool - parameterValue, ok = parameterMap[templateVersionParameter.Name] - if !ok { - parameterValue, err = cliui.RichParameter(inv, templateVersionParameter) +func parseParameterMapFile(parameterFile string) (map[string]string, error) { + parameterFileContents, err := os.ReadFile(parameterFile) + if err != nil { + return nil, err + } + + mapStringInterface := make(map[string]interface{}) + err = yaml.Unmarshal(parameterFileContents, &mapStringInterface) + if err != nil { + return nil, err + } + + parameterMap := map[string]string{} + for k, v := range mapStringInterface { + switch val := v.(type) { + case string, bool, int: + parameterMap[k] = fmt.Sprintf("%v", val) + case []interface{}: + b, err := json.Marshal(&val) if err != nil { return nil, err } + parameterMap[k] = string(b) + default: + return nil, xerrors.Errorf("invalid parameter type: %T", v) + } + } + return parameterMap, nil +} + +type getParameterArgs struct { + templateVersionParameter codersdk.TemplateVersionParameter + + promptBuildOptions bool + buildOptions []codersdk.WorkspaceBuildParameter + parameterMap map[string]string +} + +func getParameter(inv *clibase.Invocation, args getParameterArgs) (codersdk.WorkspaceBuildParameter, error) { + if args.parameterMap != nil { + if parameterValue, ok := args.parameterMap[args.templateVersionParameter.Name]; ok { + return codersdk.WorkspaceBuildParameter{ + Name: args.templateVersionParameter.Name, + Value: parameterValue, + }, nil } - } else { - parameterValue, err = cliui.RichParameter(inv, templateVersionParameter) - if err != nil { - return nil, err + } + return getParameterFromCommandLineOrInput(inv, args.promptBuildOptions, args.buildOptions, args.templateVersionParameter) +} + +//nolint:revive +func getParameterFromCommandLineOrInput(inv *clibase.Invocation, promptBuildOptions bool, buildOptions []codersdk.WorkspaceBuildParameter, templateVersionParameter codersdk.TemplateVersionParameter) (codersdk.WorkspaceBuildParameter, error) { + if templateVersionParameter.Ephemeral { + for _, bo := range buildOptions { + if bo.Name == templateVersionParameter.Name { + return codersdk.WorkspaceBuildParameter{ + Name: templateVersionParameter.Name, + Value: bo.Value, + }, nil + } } } - return &codersdk.WorkspaceBuildParameter{ + + parameterValue, err := cliui.RichParameter(inv, templateVersionParameter) + if err != nil { + return codersdk.WorkspaceBuildParameter{}, err + } + return codersdk.WorkspaceBuildParameter{ Name: templateVersionParameter.Name, Value: parameterValue, }, nil diff --git a/cli/parameter_resolver.go b/cli/parameter_resolver.go new file mode 100644 index 0000000000000..0b7cc51492bde --- /dev/null +++ b/cli/parameter_resolver.go @@ -0,0 +1,154 @@ +package cli + +import "github.com/coder/coder/codersdk" + +type WorkspaceCLIAction int + +const ( + WorkspaceCreate WorkspaceCLIAction = iota + WorkspaceStart + WorkspaceUpdate + WorkspaceRestart +) + +type ParameterResolver struct { + action WorkspaceCLIAction + templateVersionParameters []codersdk.TemplateVersionParameter + + lastBuildParameters []codersdk.WorkspaceBuildParameter + richParameters []codersdk.WorkspaceBuildParameter + buildOptions []codersdk.WorkspaceBuildParameter + richParametersFile map[string]string + + alwaysPrompt bool + promptBuildOptions bool +} + +func (pr *ParameterResolver) WithLastBuildParameters(params []codersdk.WorkspaceBuildParameter) *ParameterResolver { + pr.lastBuildParameters = params + return pr +} + +func (pr *ParameterResolver) WithRichParameters(params []codersdk.WorkspaceBuildParameter) *ParameterResolver { + pr.richParameters = params + return pr +} + +func (pr *ParameterResolver) WithBuildOptions(params []codersdk.WorkspaceBuildParameter) *ParameterResolver { + pr.buildOptions = params + return pr +} + +func (pr *ParameterResolver) WithRichParametersFile(fileMap map[string]string) *ParameterResolver { + pr.richParametersFile = fileMap + return pr +} + +func (pr *ParameterResolver) WithAlwaysPrompt(alwaysPrompt bool) *ParameterResolver { + pr.alwaysPrompt = alwaysPrompt + return pr +} + +func (pr *ParameterResolver) WithPromptBuildOptions(promptBuildOptions bool) *ParameterResolver { + pr.promptBuildOptions = promptBuildOptions + return pr +} + +func (pr *ParameterResolver) Resolve(action WorkspaceCLIAction, templateVersionParameters []codersdk.TemplateVersionParameter) ([]codersdk.WorkspaceBuildParameter, error) { + panic("not implemented yet") + /* + if Start or Restart { + + richParameters := make([]codersdk.WorkspaceBuildParameter, 0) + if !args.PromptBuildOptions && len(args.BuildOptions) == 0 { + return &buildParameters{ + richParameters: richParameters, + }, nil + } + + for _, templateVersionParameter := range templateVersionParameters { + if !templateVersionParameter.Ephemeral { + continue + } + + buildOption, err := getParameterFromCommandLineOrInput(inv, args.PromptBuildOptions, args.BuildOptions, templateVersionParameter) + if err != nil { + return nil, err + } + richParameters = append(richParameters, buildOption) + } + + } + + if Create or Update { + + parameterMapFromFile := map[string]string{} + useParamFile := false + if args.RichParameterFile != "" { + useParamFile = true + _, _ = fmt.Fprintln(inv.Stdout, cliui.DefaultStyles.Paragraph.Render("Attempting to read parameters from the rich parameter file.")+"\r\n") + parameterMapFromFile, err = createParameterMapFromFile(args.RichParameterFile) + if err != nil { + return nil, err + } + } + disclaimerPrinted := false + richParameters := make([]codersdk.WorkspaceBuildParameter, 0) + + PromptRichParamLoop: + + for _, templateVersionParameter := range templateVersionParameters { + if !args.PromptBuildOptions && len(args.BuildOptions) == 0 && templateVersionParameter.Ephemeral { + continue + } + + if !disclaimerPrinted { + _, _ = fmt.Fprintln(inv.Stdout, cliui.DefaultStyles.Paragraph.Render("This template has customizable parameters. Values can be changed after create, but may have unintended side effects (like data loss).")+"\r\n") + disclaimerPrinted = true + } + + // Param file is all or nothing + if !useParamFile && !templateVersionParameter.Ephemeral { + for _, e := range args.ExistingRichParams { + if e.Name == templateVersionParameter.Name { + // If the param already exists, we do not need to prompt it again. + // The workspace scope will reuse params for each build. + continue PromptRichParamLoop + } + } + } + + if args.UpdateWorkspace && !templateVersionParameter.Mutable { + // Check if the immutable parameter was used in the previous build. If so, then it isn't a fresh one + // and the user should be warned. + exists, err := workspaceBuildParameterExists(ctx, client, args.WorkspaceID, templateVersionParameter) + if err != nil { + return nil, err + } + + if exists { + _, _ = fmt.Fprintln(inv.Stdout, cliui.DefaultStyles.Warn.Render(fmt.Sprintf(`Parameter %q is not mutable, so can't be customized after workspace creation.`, templateVersionParameter.Name))) + continue + } + } + + parameter, err := getParameter(inv, getParameterArgs{ + promptBuildOptions: args.PromptBuildOptions, + buildOptions: args.BuildOptions, + parameterMap: parameterMapFromFile, + templateVersionParameter: templateVersionParameter, + }) + if err != nil { + return nil, err + } + + richParameters = append(richParameters, parameter) + } + + if disclaimerPrinted { + _, _ = fmt.Fprintln(inv.Stdout) + } + + } + */ +} diff --git a/cli/restart.go b/cli/restart.go index 322911f19127a..e81d4df5e6367 100644 --- a/cli/restart.go +++ b/cli/restart.go @@ -7,6 +7,7 @@ import ( "github.com/coder/coder/cli/clibase" "github.com/coder/coder/cli/cliui" "github.com/coder/coder/codersdk" + "golang.org/x/xerrors" ) func (r *RootCmd) restart() *clibase.Cmd { @@ -21,7 +22,7 @@ func (r *RootCmd) restart() *clibase.Cmd { clibase.RequireNArgs(1), r.InitClient(client), ), - Options: append(parameterFlags.options(), cliui.SkipPromptOption()), + Options: append(parameterFlags.cliBuildOptions(), cliui.SkipPromptOption()), Handler: func(inv *clibase.Invocation) error { ctx := inv.Context() out := inv.Stdout @@ -38,10 +39,11 @@ func (r *RootCmd) restart() *clibase.Cmd { buildOptions, err := asWorkspaceBuildParameters(parameterFlags.buildOptions) if err != nil { - return err + return xerrors.Errorf("can't parse build options: %w", err) } - buildParams, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ + buildParameters, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ + Action: WorkspaceRestart, Template: template, PromptBuildOptions: parameterFlags.promptBuildOptions, BuildOptions: buildOptions, @@ -71,7 +73,7 @@ func (r *RootCmd) restart() *clibase.Cmd { build, err = client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ Transition: codersdk.WorkspaceTransitionStart, - RichParameterValues: buildParams.richParameters, + RichParameterValues: buildParameters, }) if err != nil { return err diff --git a/cli/start.go b/cli/start.go index 7612d1fd32c02..6003ceba99b37 100644 --- a/cli/start.go +++ b/cli/start.go @@ -2,7 +2,6 @@ package cli import ( "fmt" - "strings" "time" "golang.org/x/xerrors" @@ -12,48 +11,6 @@ import ( "github.com/coder/coder/codersdk" ) -// workspaceParameterFlags are used by commands requiring rich parameters and/or build options. -type workspaceParameterFlags struct { - promptBuildOptions bool - buildOptions []string - - richParameterFile string - richParameters []string -} - -func (wpf *workspaceParameterFlags) options() []clibase.Option { - return clibase.OptionSet{ - { - Flag: "build-option", - Env: "CODER_BUILD_OPTION", - Description: `Build option value in the format "name=value".`, - Value: clibase.StringArrayOf(&wpf.buildOptions), - }, - { - Flag: "build-options", - Description: "Prompt for one-time build options defined with ephemeral parameters.", - Value: clibase.BoolOf(&wpf.promptBuildOptions), - }, - } -} - -func (wpf *workspaceParameterFlags) parameters() []clibase.Option { - return clibase.OptionSet{ - clibase.Option{ - Flag: "parameter", - Env: "CODER_RICH_PARAMETER", - Description: `Rich parameter value in the format "name=value".`, - Value: clibase.StringArrayOf(&wpf.richParameters), - }, - clibase.Option{ - Flag: "rich-parameter-file", - Env: "CODER_RICH_PARAMETER_FILE", - Description: "Specify a file path with values for rich parameters defined in the template.", - Value: clibase.StringOf(&wpf.richParameterFile), - }, - } -} - func (r *RootCmd) start() *clibase.Cmd { var parameterFlags workspaceParameterFlags @@ -66,7 +23,7 @@ func (r *RootCmd) start() *clibase.Cmd { clibase.RequireNArgs(1), r.InitClient(client), ), - Options: append(parameterFlags.options(), cliui.SkipPromptOption()), + Options: append(parameterFlags.cliBuildOptions(), cliui.SkipPromptOption()), Handler: func(inv *clibase.Invocation) error { workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) if err != nil { @@ -80,10 +37,11 @@ func (r *RootCmd) start() *clibase.Cmd { buildOptions, err := asWorkspaceBuildParameters(parameterFlags.buildOptions) if err != nil { - return err + return xerrors.Errorf("can't parse build options: %w", err) } - buildParams, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ + buildParameters, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ + Action: WorkspaceStart, Template: template, PromptBuildOptions: parameterFlags.promptBuildOptions, BuildOptions: buildOptions, @@ -94,7 +52,7 @@ func (r *RootCmd) start() *clibase.Cmd { build, err := client.CreateWorkspaceBuild(inv.Context(), workspace.ID, codersdk.CreateWorkspaceBuildRequest{ Transition: codersdk.WorkspaceTransitionStart, - RichParameterValues: buildParams.richParameters, + RichParameterValues: buildParameters, }) if err != nil { return err @@ -113,18 +71,18 @@ func (r *RootCmd) start() *clibase.Cmd { } type prepStartWorkspaceArgs struct { - Template codersdk.Template - + Action WorkspaceCLIAction + Template codersdk.Template PromptBuildOptions bool BuildOptions []codersdk.WorkspaceBuildParameter } -func prepStartWorkspace(inv *clibase.Invocation, client *codersdk.Client, args prepStartWorkspaceArgs) (*buildParameters, error) { +func prepStartWorkspace(inv *clibase.Invocation, client *codersdk.Client, args prepStartWorkspaceArgs) ([]codersdk.WorkspaceBuildParameter, error) { ctx := inv.Context() templateVersion, err := client.TemplateVersion(ctx, args.Template.ActiveVersionID) if err != nil { - return nil, err + return nil, xerrors.Errorf("get template version: %w", err) } templateVersionParameters, err := client.TemplateVersionRichParameters(inv.Context(), templateVersion.ID) @@ -132,62 +90,8 @@ func prepStartWorkspace(inv *clibase.Invocation, client *codersdk.Client, args p return nil, xerrors.Errorf("get template version rich parameters: %w", err) } - richParameters := make([]codersdk.WorkspaceBuildParameter, 0) - if !args.PromptBuildOptions && len(args.BuildOptions) == 0 { - return &buildParameters{ - richParameters: richParameters, - }, nil - } - - for _, templateVersionParameter := range templateVersionParameters { - if !templateVersionParameter.Ephemeral { - continue - } - - parameterValue, err := getParameterValueFromCommandLineOrInput(inv, args.PromptBuildOptions, args.BuildOptions, templateVersionParameter) - if err != nil { - return nil, err - } - - richParameters = append(richParameters, codersdk.WorkspaceBuildParameter{ - Name: templateVersionParameter.Name, - Value: parameterValue, - }) - } - - return &buildParameters{ - richParameters: richParameters, - }, nil -} - -func asWorkspaceBuildParameters(nameValuePairs []string) ([]codersdk.WorkspaceBuildParameter, error) { - var params []codersdk.WorkspaceBuildParameter - for _, nameValue := range nameValuePairs { - split := strings.SplitN(nameValue, "=", 2) - if len(split) < 2 { - return nil, xerrors.Errorf("format key=value expected, but got %s", nameValue) - } - params = append(params, codersdk.WorkspaceBuildParameter{ - Name: split[0], - Value: split[1], - }) - } - return params, nil -} - -//nolint:revive -func getParameterValueFromCommandLineOrInput(inv *clibase.Invocation, promptBuildOptions bool, buildOptions []codersdk.WorkspaceBuildParameter, templateVersionParameter codersdk.TemplateVersionParameter) (string, error) { - if !promptBuildOptions { - for _, bo := range buildOptions { - if bo.Name == templateVersionParameter.Name { - return bo.Value, nil - } - } - } - - parameterValue, err := cliui.RichParameter(inv, templateVersionParameter) - if err != nil { - return "", err - } - return parameterValue, nil + resolver := new(ParameterResolver). + WithPromptBuildOptions(args.PromptBuildOptions). + WithBuildOptions(args.BuildOptions) + return resolver.Resolve(args.Action, templateVersionParameters) } diff --git a/cli/update.go b/cli/update.go index a4cab045047c1..c2d46d6ad5b0d 100644 --- a/cli/update.go +++ b/cli/update.go @@ -5,6 +5,7 @@ import ( "github.com/coder/coder/cli/clibase" "github.com/coder/coder/codersdk" + "golang.org/x/xerrors" ) func (r *RootCmd) update() *clibase.Cmd { @@ -52,17 +53,24 @@ func (r *RootCmd) update() *clibase.Cmd { } } - buildParams, err := prepWorkspaceBuild(inv, client, prepWorkspaceBuildArgs{ - Template: template, - ExistingRichParams: existingRichParams, - RichParameterFile: parameterFlags.richParameterFile, - NewWorkspaceName: workspace.Name, + cliRichParameters, err := asWorkspaceBuildParameters(parameterFlags.richParameters) + if err != nil { + return xerrors.Errorf("can't parse given parameter values: %w", err) + } - UpdateWorkspace: true, - WorkspaceID: workspace.LatestBuild.ID, + richParameters, err := prepWorkspaceBuild(inv, client, prepWorkspaceBuildArgs{ + Action: WorkspaceUpdate, + Template: template, + NewWorkspaceName: workspace.Name, + WorkspaceID: workspace.LatestBuild.ID, + + LastBuildParameters: existingRichParams, PromptBuildOptions: parameterFlags.promptBuildOptions, BuildOptions: buildOptions, + + RichParameterFile: parameterFlags.richParameterFile, + RichParameters: cliRichParameters, }) if err != nil { return err @@ -71,7 +79,7 @@ func (r *RootCmd) update() *clibase.Cmd { build, err := client.CreateWorkspaceBuild(inv.Context(), workspace.ID, codersdk.CreateWorkspaceBuildRequest{ TemplateVersionID: template.ActiveVersionID, Transition: codersdk.WorkspaceTransitionStart, - RichParameterValues: buildParams.richParameters, + RichParameterValues: richParameters, }) if err != nil { return err @@ -99,7 +107,7 @@ func (r *RootCmd) update() *clibase.Cmd { Value: clibase.BoolOf(&alwaysPrompt), }, } - cmd.Options = append(cmd.Options, parameterFlags.options()...) - cmd.Options = append(cmd.Options, parameterFlags.parameters()...) + cmd.Options = append(cmd.Options, parameterFlags.cliBuildOptions()...) + cmd.Options = append(cmd.Options, parameterFlags.cliParameters()...) return cmd } From 29d0eab3511c39b52251f3a27eeb4c4de263621a Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 4 Aug 2023 15:11:18 +0200 Subject: [PATCH 09/30] rename --- cli/update.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/update.go b/cli/update.go index c2d46d6ad5b0d..e9615a98f6695 100644 --- a/cli/update.go +++ b/cli/update.go @@ -58,7 +58,7 @@ func (r *RootCmd) update() *clibase.Cmd { return xerrors.Errorf("can't parse given parameter values: %w", err) } - richParameters, err := prepWorkspaceBuild(inv, client, prepWorkspaceBuildArgs{ + buildParameters, err := prepWorkspaceBuild(inv, client, prepWorkspaceBuildArgs{ Action: WorkspaceUpdate, Template: template, NewWorkspaceName: workspace.Name, @@ -79,7 +79,7 @@ func (r *RootCmd) update() *clibase.Cmd { build, err := client.CreateWorkspaceBuild(inv.Context(), workspace.ID, codersdk.CreateWorkspaceBuildRequest{ TemplateVersionID: template.ActiveVersionID, Transition: codersdk.WorkspaceTransitionStart, - RichParameterValues: richParameters, + RichParameterValues: buildParameters, }) if err != nil { return err From 9af782cbc23890b0abb519377085476e316f4fc4 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 4 Aug 2023 15:34:01 +0200 Subject: [PATCH 10/30] fixes --- cli/parameter_internal_test.go | 14 +++----------- cli/restart.go | 3 ++- cli/update.go | 3 ++- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/cli/parameter_internal_test.go b/cli/parameter_internal_test.go index 81dfcefdf49b2..935486c6eae26 100644 --- a/cli/parameter_internal_test.go +++ b/cli/parameter_internal_test.go @@ -16,7 +16,7 @@ func TestCreateParameterMapFromFile(t *testing.T) { parameterFile, _ := os.CreateTemp(tempDir, "testParameterFile*.yaml") _, _ = parameterFile.WriteString("region: \"bananas\"\ndisk: \"20\"\n") - parameterMapFromFile, err := createParameterMapFromFile(parameterFile.Name()) + parameterMapFromFile, err := parseParameterMapFile(parameterFile.Name()) expectedMap := map[string]string{ "region": "bananas", @@ -28,18 +28,10 @@ func TestCreateParameterMapFromFile(t *testing.T) { removeTmpDirUntilSuccess(t, tempDir) }) - t.Run("WithEmptyFilename", func(t *testing.T) { - t.Parallel() - - parameterMapFromFile, err := createParameterMapFromFile("") - - assert.Nil(t, parameterMapFromFile) - assert.EqualError(t, err, "Parameter file name is not specified") - }) t.Run("WithInvalidFilename", func(t *testing.T) { t.Parallel() - parameterMapFromFile, err := createParameterMapFromFile("invalidFile.yaml") + parameterMapFromFile, err := parseParameterMapFile("invalidFile.yaml") assert.Nil(t, parameterMapFromFile) @@ -57,7 +49,7 @@ func TestCreateParameterMapFromFile(t *testing.T) { parameterFile, _ := os.CreateTemp(tempDir, "testParameterFile*.yaml") _, _ = parameterFile.WriteString("region = \"bananas\"\ndisk = \"20\"\n") - parameterMapFromFile, err := createParameterMapFromFile(parameterFile.Name()) + parameterMapFromFile, err := parseParameterMapFile(parameterFile.Name()) assert.Nil(t, parameterMapFromFile) assert.EqualError(t, err, "yaml: unmarshal errors:\n line 1: cannot unmarshal !!str `region ...` into map[string]interface {}") diff --git a/cli/restart.go b/cli/restart.go index e81d4df5e6367..03603de213cb4 100644 --- a/cli/restart.go +++ b/cli/restart.go @@ -4,10 +4,11 @@ import ( "fmt" "time" + "golang.org/x/xerrors" + "github.com/coder/coder/cli/clibase" "github.com/coder/coder/cli/cliui" "github.com/coder/coder/codersdk" - "golang.org/x/xerrors" ) func (r *RootCmd) restart() *clibase.Cmd { diff --git a/cli/update.go b/cli/update.go index e9615a98f6695..d64e8abad60f5 100644 --- a/cli/update.go +++ b/cli/update.go @@ -3,9 +3,10 @@ package cli import ( "fmt" + "golang.org/x/xerrors" + "github.com/coder/coder/cli/clibase" "github.com/coder/coder/codersdk" - "golang.org/x/xerrors" ) func (r *RootCmd) update() *clibase.Cmd { From 34c71bf8c5562f40c555e08691d515c2ea20f4f6 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 8 Aug 2023 11:25:12 +0200 Subject: [PATCH 11/30] WIP --- cli/create.go | 13 ++- cli/parameter_resolver.go | 202 ++++++++++++++++++-------------------- cli/stat.go | 2 +- cli/update.go | 5 +- 4 files changed, 110 insertions(+), 112 deletions(-) diff --git a/cli/create.go b/cli/create.go index 2c727ac207227..96f4e0f915090 100644 --- a/cli/create.go +++ b/cli/create.go @@ -212,12 +212,14 @@ type prepWorkspaceBuildArgs struct { NewWorkspaceName string WorkspaceID uuid.UUID + LastBuildParameters []codersdk.WorkspaceBuildParameter + PromptBuildOptions bool BuildOptions []codersdk.WorkspaceBuildParameter - LastBuildParameters []codersdk.WorkspaceBuildParameter - RichParameterFile string - RichParameters []codersdk.WorkspaceBuildParameter + PromptRichParameters bool + RichParameters []codersdk.WorkspaceBuildParameter + RichParameterFile string } // prepWorkspaceBuild will ensure a workspace build will succeed on the latest template version. @@ -244,12 +246,13 @@ func prepWorkspaceBuild(inv *clibase.Invocation, client *codersdk.Client, args p } resolver := new(ParameterResolver). + WithLastBuildParameters(args.LastBuildParameters). WithPromptBuildOptions(args.PromptBuildOptions). WithBuildOptions(args.BuildOptions). - WithLastBuildParameters(args.LastBuildParameters). + WithPromptRichParameters(args.PromptRichParameters). WithRichParameters(args.RichParameters). WithRichParametersFile(parameterFile) - buildParameters, err := resolver.Resolve(args.Action, templateVersionParameters) + buildParameters, err := resolver.Resolve(inv, args.Action, templateVersionParameters) if err != nil { return nil, err } diff --git a/cli/parameter_resolver.go b/cli/parameter_resolver.go index 0b7cc51492bde..88a346f6806f8 100644 --- a/cli/parameter_resolver.go +++ b/cli/parameter_resolver.go @@ -1,6 +1,9 @@ package cli -import "github.com/coder/coder/codersdk" +import ( + "github.com/coder/coder/cli/clibase" + "github.com/coder/coder/codersdk" +) type WorkspaceCLIAction int @@ -16,12 +19,13 @@ type ParameterResolver struct { templateVersionParameters []codersdk.TemplateVersionParameter lastBuildParameters []codersdk.WorkspaceBuildParameter - richParameters []codersdk.WorkspaceBuildParameter - buildOptions []codersdk.WorkspaceBuildParameter - richParametersFile map[string]string - alwaysPrompt bool - promptBuildOptions bool + richParameters []codersdk.WorkspaceBuildParameter + richParametersFile map[string]string + buildOptions []codersdk.WorkspaceBuildParameter + + promptRichParameters bool + promptBuildOptions bool } func (pr *ParameterResolver) WithLastBuildParameters(params []codersdk.WorkspaceBuildParameter) *ParameterResolver { @@ -44,8 +48,8 @@ func (pr *ParameterResolver) WithRichParametersFile(fileMap map[string]string) * return pr } -func (pr *ParameterResolver) WithAlwaysPrompt(alwaysPrompt bool) *ParameterResolver { - pr.alwaysPrompt = alwaysPrompt +func (pr *ParameterResolver) WithPromptRichParameters(promptRichParameters bool) *ParameterResolver { + pr.promptRichParameters = promptRichParameters return pr } @@ -54,101 +58,91 @@ func (pr *ParameterResolver) WithPromptBuildOptions(promptBuildOptions bool) *Pa return pr } -func (pr *ParameterResolver) Resolve(action WorkspaceCLIAction, templateVersionParameters []codersdk.TemplateVersionParameter) ([]codersdk.WorkspaceBuildParameter, error) { +func (pr *ParameterResolver) Resolve(inv *clibase.Invocation, action WorkspaceCLIAction, templateVersionParameters []codersdk.TemplateVersionParameter) ([]codersdk.WorkspaceBuildParameter, error) { + var staged []codersdk.WorkspaceBuildParameter + var err error + + staged = pr.resolveWithParametersMapFile(staged) + staged = pr.resolveWithCommandLineOrEnv(staged) + if err = pr.verifyConstraints(staged); err != nil { + return nil, err + } + staged = pr.resolveWithLastBuildParameters(staged) + staged = pr.resolveWithInput(staged, inv, action) + return staged, nil +} + +func (pr *ParameterResolver) resolveWithParametersMapFile(resolved []codersdk.WorkspaceBuildParameter) []codersdk.WorkspaceBuildParameter { + for name, value := range pr.richParametersFile { + for i, r := range resolved { + if r.Name == name { + resolved[i].Value = value + goto done + } + } + + resolved = append(resolved, codersdk.WorkspaceBuildParameter{ + Name: name, + Value: value, + }) + done: + } + return resolved +} + +func (pr *ParameterResolver) resolveWithCommandLineOrEnv(resolved []codersdk.WorkspaceBuildParameter) []codersdk.WorkspaceBuildParameter { + for _, richParameter := range pr.richParameters { + for i, r := range resolved { + if r.Name == richParameter.Name { + resolved[i].Value = richParameter.Value + goto richParameterDone + } + } + + resolved = append(resolved, richParameter) + richParameterDone: + } + + if pr.promptBuildOptions { + for _, buildOption := range pr.buildOptions { + for i, r := range resolved { + if r.Name == buildOption.Name { + resolved[i].Value = buildOption.Value + goto buildOptionDone + } + } + + resolved = append(resolved, buildOption) + buildOptionDone: + } + } + return resolved +} + +func (pr *ParameterResolver) resolveWithLastBuildParameters(resolved []codersdk.WorkspaceBuildParameter) []codersdk.WorkspaceBuildParameter { + for _, buildParameter := range pr.lastBuildParameters { + for i, r := range resolved { + if r.Name == buildParameter.Name { + resolved[i].Value = buildParameter.Value + goto done + } + } + + resolved = append(resolved, buildParameter) + done: + } + return resolved +} + +func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuildParameter, iv *clibase.Invocation, action WorkspaceCLIAction) []codersdk.WorkspaceBuildParameter { + // update == then skip if in last build parameters unless prompt-all, build options + // update == immutable + panic("not implemented yet") - /* - if Start or Restart { - - richParameters := make([]codersdk.WorkspaceBuildParameter, 0) - if !args.PromptBuildOptions && len(args.BuildOptions) == 0 { - return &buildParameters{ - richParameters: richParameters, - }, nil - } - - for _, templateVersionParameter := range templateVersionParameters { - if !templateVersionParameter.Ephemeral { - continue - } - - buildOption, err := getParameterFromCommandLineOrInput(inv, args.PromptBuildOptions, args.BuildOptions, templateVersionParameter) - if err != nil { - return nil, err - } - richParameters = append(richParameters, buildOption) - } - - } - - if Create or Update { - - parameterMapFromFile := map[string]string{} - useParamFile := false - if args.RichParameterFile != "" { - useParamFile = true - _, _ = fmt.Fprintln(inv.Stdout, cliui.DefaultStyles.Paragraph.Render("Attempting to read parameters from the rich parameter file.")+"\r\n") - parameterMapFromFile, err = createParameterMapFromFile(args.RichParameterFile) - if err != nil { - return nil, err - } - } - disclaimerPrinted := false - richParameters := make([]codersdk.WorkspaceBuildParameter, 0) - - PromptRichParamLoop: - - for _, templateVersionParameter := range templateVersionParameters { - if !args.PromptBuildOptions && len(args.BuildOptions) == 0 && templateVersionParameter.Ephemeral { - continue - } - - if !disclaimerPrinted { - _, _ = fmt.Fprintln(inv.Stdout, cliui.DefaultStyles.Paragraph.Render("This template has customizable parameters. Values can be changed after create, but may have unintended side effects (like data loss).")+"\r\n") - disclaimerPrinted = true - } - - // Param file is all or nothing - if !useParamFile && !templateVersionParameter.Ephemeral { - for _, e := range args.ExistingRichParams { - if e.Name == templateVersionParameter.Name { - // If the param already exists, we do not need to prompt it again. - // The workspace scope will reuse params for each build. - continue PromptRichParamLoop - } - } - } - - if args.UpdateWorkspace && !templateVersionParameter.Mutable { - // Check if the immutable parameter was used in the previous build. If so, then it isn't a fresh one - // and the user should be warned. - exists, err := workspaceBuildParameterExists(ctx, client, args.WorkspaceID, templateVersionParameter) - if err != nil { - return nil, err - } - - if exists { - _, _ = fmt.Fprintln(inv.Stdout, cliui.DefaultStyles.Warn.Render(fmt.Sprintf(`Parameter %q is not mutable, so can't be customized after workspace creation.`, templateVersionParameter.Name))) - continue - } - } - - parameter, err := getParameter(inv, getParameterArgs{ - promptBuildOptions: args.PromptBuildOptions, - buildOptions: args.BuildOptions, - parameterMap: parameterMapFromFile, - templateVersionParameter: templateVersionParameter, - }) - if err != nil { - return nil, err - } - - richParameters = append(richParameters, parameter) - } - - if disclaimerPrinted { - _, _ = fmt.Fprintln(inv.Stdout) - } - - } - */ + + return resolved +} + +func (pr *ParameterResolver) verifyConstraints(resolved []codersdk.WorkspaceBuildParameter) error { + } diff --git a/cli/stat.go b/cli/stat.go index 3657a4f3c71c9..a531782f378fb 100644 --- a/cli/stat.go +++ b/cli/stat.go @@ -241,7 +241,7 @@ func (*RootCmd) statDisk(s *clistat.Statter) *clibase.Cmd { if err != nil { if os.IsNotExist(err) { // fmt.Errorf produces a more concise error. - return fmt.Errorf("not found: %q", pathArg) + return xerrors.New("not found: %q", pathArg) } return err } diff --git a/cli/update.go b/cli/update.go index d64e8abad60f5..c033efaac492c 100644 --- a/cli/update.go +++ b/cli/update.go @@ -70,8 +70,9 @@ func (r *RootCmd) update() *clibase.Cmd { PromptBuildOptions: parameterFlags.promptBuildOptions, BuildOptions: buildOptions, - RichParameterFile: parameterFlags.richParameterFile, - RichParameters: cliRichParameters, + PromptRichParameters: alwaysPrompt, + RichParameters: cliRichParameters, + RichParameterFile: parameterFlags.richParameterFile, }) if err != nil { return err From fd793800d3dc4636ef4bb5c586c230021ef18b56 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 8 Aug 2023 11:25:39 +0200 Subject: [PATCH 12/30] fix --- cli/parameter_resolver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/parameter_resolver.go b/cli/parameter_resolver.go index 88a346f6806f8..297b54ec06731 100644 --- a/cli/parameter_resolver.go +++ b/cli/parameter_resolver.go @@ -144,5 +144,5 @@ func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuild } func (pr *ParameterResolver) verifyConstraints(resolved []codersdk.WorkspaceBuildParameter) error { - + panic("not implemented yet") } From 6e854a367721758b91066814da7a7b762f32c568 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 8 Aug 2023 11:51:05 +0200 Subject: [PATCH 13/30] verifyConstraints --- cli/parameter_resolver.go | 49 ++++++++++++++++++++++++++++++++++----- cli/start.go | 2 +- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/cli/parameter_resolver.go b/cli/parameter_resolver.go index 297b54ec06731..a0230cc80373e 100644 --- a/cli/parameter_resolver.go +++ b/cli/parameter_resolver.go @@ -3,6 +3,7 @@ package cli import ( "github.com/coder/coder/cli/clibase" "github.com/coder/coder/codersdk" + "golang.org/x/xerrors" ) type WorkspaceCLIAction int @@ -64,10 +65,10 @@ func (pr *ParameterResolver) Resolve(inv *clibase.Invocation, action WorkspaceCL staged = pr.resolveWithParametersMapFile(staged) staged = pr.resolveWithCommandLineOrEnv(staged) - if err = pr.verifyConstraints(staged); err != nil { + staged = pr.resolveWithLastBuildParameters(staged, action) + if err = pr.verifyConstraints(staged, action); err != nil { return nil, err } - staged = pr.resolveWithLastBuildParameters(staged) staged = pr.resolveWithInput(staged, inv, action) return staged, nil } @@ -119,8 +120,21 @@ func (pr *ParameterResolver) resolveWithCommandLineOrEnv(resolved []codersdk.Wor return resolved } -func (pr *ParameterResolver) resolveWithLastBuildParameters(resolved []codersdk.WorkspaceBuildParameter) []codersdk.WorkspaceBuildParameter { +func (pr *ParameterResolver) resolveWithLastBuildParameters(resolved []codersdk.WorkspaceBuildParameter, action WorkspaceCLIAction) []codersdk.WorkspaceBuildParameter { for _, buildParameter := range pr.lastBuildParameters { + tvp := pr.findTemplateVersionParameter(buildParameter) + if tvp == nil { + continue // it looks like this parameter is not present anymore + } + + if tvp.Ephemeral { + continue // ephemeral parameters should not be passed to consecutive builds + } + + if !tvp.Mutable { + continue // immutables should not be passed to consecutive builds + } + for i, r := range resolved { if r.Name == buildParameter.Name { resolved[i].Value = buildParameter.Value @@ -134,7 +148,25 @@ func (pr *ParameterResolver) resolveWithLastBuildParameters(resolved []codersdk. return resolved } -func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuildParameter, iv *clibase.Invocation, action WorkspaceCLIAction) []codersdk.WorkspaceBuildParameter { +func (pr *ParameterResolver) verifyConstraints(resolved []codersdk.WorkspaceBuildParameter, action WorkspaceCLIAction) error { + for _, r := range resolved { + tvp := pr.findTemplateVersionParameter(r) + if tvp == nil { + return xerrors.Errorf("parameter %q is not present in the template", r.Name) + } + + if tvp.Ephemeral && !pr.promptBuildOptions { + return xerrors.Errorf("ephemeral parameter %q can be used only with --build-options flag", r.Name) + } + + if !tvp.Mutable && action != WorkspaceCreate { + return xerrors.Errorf("parameter %q is immutable and can't be updated", r.Name) + } + } + return nil +} + +func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuildParameter, inv *clibase.Invocation, action WorkspaceCLIAction) []codersdk.WorkspaceBuildParameter { // update == then skip if in last build parameters unless prompt-all, build options // update == immutable @@ -143,6 +175,11 @@ func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuild return resolved } -func (pr *ParameterResolver) verifyConstraints(resolved []codersdk.WorkspaceBuildParameter) error { - panic("not implemented yet") +func (pr *ParameterResolver) findTemplateVersionParameter(workspaceBuildParameter codersdk.WorkspaceBuildParameter) *codersdk.TemplateVersionParameter { + for _, tvp := range pr.templateVersionParameters { + if tvp.Name == workspaceBuildParameter.Name { + return &tvp + } + } + return nil } diff --git a/cli/start.go b/cli/start.go index 6003ceba99b37..90a83533e7a96 100644 --- a/cli/start.go +++ b/cli/start.go @@ -93,5 +93,5 @@ func prepStartWorkspace(inv *clibase.Invocation, client *codersdk.Client, args p resolver := new(ParameterResolver). WithPromptBuildOptions(args.PromptBuildOptions). WithBuildOptions(args.BuildOptions) - return resolver.Resolve(args.Action, templateVersionParameters) + return resolver.Resolve(inv, args.Action, templateVersionParameters) } From 751fcf3f9e158282cbcffb9d3f778fa227fb508e Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 8 Aug 2023 12:02:53 +0200 Subject: [PATCH 14/30] Fix? --- cli/parameter_resolver.go | 3 ++- cli/stat.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/parameter_resolver.go b/cli/parameter_resolver.go index a0230cc80373e..19af526f68c56 100644 --- a/cli/parameter_resolver.go +++ b/cli/parameter_resolver.go @@ -1,9 +1,10 @@ package cli import ( + "golang.org/x/xerrors" + "github.com/coder/coder/cli/clibase" "github.com/coder/coder/codersdk" - "golang.org/x/xerrors" ) type WorkspaceCLIAction int diff --git a/cli/stat.go b/cli/stat.go index a531782f378fb..6fb24b1fb6442 100644 --- a/cli/stat.go +++ b/cli/stat.go @@ -241,7 +241,7 @@ func (*RootCmd) statDisk(s *clistat.Statter) *clibase.Cmd { if err != nil { if os.IsNotExist(err) { // fmt.Errorf produces a more concise error. - return xerrors.New("not found: %q", pathArg) + return xerrors.Errorf("not found: %q", pathArg) } return err } From c1aeae09e95216f6992e71f30987becfaa8a1cad Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 8 Aug 2023 13:01:44 +0200 Subject: [PATCH 15/30] Resolver implemented --- cli/parameter.go | 44 --------------------------- cli/parameter_resolver.go | 64 ++++++++++++++++++++++++++++----------- cli/update.go | 11 +++---- 3 files changed, 51 insertions(+), 68 deletions(-) diff --git a/cli/parameter.go b/cli/parameter.go index ecef0d9a57500..efc415692eae0 100644 --- a/cli/parameter.go +++ b/cli/parameter.go @@ -10,7 +10,6 @@ import ( "gopkg.in/yaml.v3" "github.com/coder/coder/cli/clibase" - "github.com/coder/coder/cli/cliui" "github.com/coder/coder/codersdk" ) @@ -100,46 +99,3 @@ func parseParameterMapFile(parameterFile string) (map[string]string, error) { } return parameterMap, nil } - -type getParameterArgs struct { - templateVersionParameter codersdk.TemplateVersionParameter - - promptBuildOptions bool - buildOptions []codersdk.WorkspaceBuildParameter - parameterMap map[string]string -} - -func getParameter(inv *clibase.Invocation, args getParameterArgs) (codersdk.WorkspaceBuildParameter, error) { - if args.parameterMap != nil { - if parameterValue, ok := args.parameterMap[args.templateVersionParameter.Name]; ok { - return codersdk.WorkspaceBuildParameter{ - Name: args.templateVersionParameter.Name, - Value: parameterValue, - }, nil - } - } - return getParameterFromCommandLineOrInput(inv, args.promptBuildOptions, args.buildOptions, args.templateVersionParameter) -} - -//nolint:revive -func getParameterFromCommandLineOrInput(inv *clibase.Invocation, promptBuildOptions bool, buildOptions []codersdk.WorkspaceBuildParameter, templateVersionParameter codersdk.TemplateVersionParameter) (codersdk.WorkspaceBuildParameter, error) { - if templateVersionParameter.Ephemeral { - for _, bo := range buildOptions { - if bo.Name == templateVersionParameter.Name { - return codersdk.WorkspaceBuildParameter{ - Name: templateVersionParameter.Name, - Value: bo.Value, - }, nil - } - } - } - - parameterValue, err := cliui.RichParameter(inv, templateVersionParameter) - if err != nil { - return codersdk.WorkspaceBuildParameter{}, err - } - return codersdk.WorkspaceBuildParameter{ - Name: templateVersionParameter.Name, - Value: parameterValue, - }, nil -} diff --git a/cli/parameter_resolver.go b/cli/parameter_resolver.go index 19af526f68c56..e75659218c1e1 100644 --- a/cli/parameter_resolver.go +++ b/cli/parameter_resolver.go @@ -4,6 +4,7 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/cli/clibase" + "github.com/coder/coder/cli/cliui" "github.com/coder/coder/codersdk" ) @@ -17,9 +18,6 @@ const ( ) type ParameterResolver struct { - action WorkspaceCLIAction - templateVersionParameters []codersdk.TemplateVersionParameter - lastBuildParameters []codersdk.WorkspaceBuildParameter richParameters []codersdk.WorkspaceBuildParameter @@ -66,11 +64,13 @@ func (pr *ParameterResolver) Resolve(inv *clibase.Invocation, action WorkspaceCL staged = pr.resolveWithParametersMapFile(staged) staged = pr.resolveWithCommandLineOrEnv(staged) - staged = pr.resolveWithLastBuildParameters(staged, action) - if err = pr.verifyConstraints(staged, action); err != nil { + staged = pr.resolveWithLastBuildParameters(staged, action, templateVersionParameters) + if err = pr.verifyConstraints(staged, action, templateVersionParameters); err != nil { + return nil, err + } + if staged, err = pr.resolveWithInput(staged, inv, action, templateVersionParameters); err != nil { return nil, err } - staged = pr.resolveWithInput(staged, inv, action) return staged, nil } @@ -121,9 +121,13 @@ func (pr *ParameterResolver) resolveWithCommandLineOrEnv(resolved []codersdk.Wor return resolved } -func (pr *ParameterResolver) resolveWithLastBuildParameters(resolved []codersdk.WorkspaceBuildParameter, action WorkspaceCLIAction) []codersdk.WorkspaceBuildParameter { +func (pr *ParameterResolver) resolveWithLastBuildParameters(resolved []codersdk.WorkspaceBuildParameter, action WorkspaceCLIAction, templateVersionParameters []codersdk.TemplateVersionParameter) []codersdk.WorkspaceBuildParameter { + if pr.promptRichParameters { + return resolved // don't pull parameters from last build + } + for _, buildParameter := range pr.lastBuildParameters { - tvp := pr.findTemplateVersionParameter(buildParameter) + tvp := findTemplateVersionParameter(buildParameter, templateVersionParameters) if tvp == nil { continue // it looks like this parameter is not present anymore } @@ -149,9 +153,9 @@ func (pr *ParameterResolver) resolveWithLastBuildParameters(resolved []codersdk. return resolved } -func (pr *ParameterResolver) verifyConstraints(resolved []codersdk.WorkspaceBuildParameter, action WorkspaceCLIAction) error { +func (pr *ParameterResolver) verifyConstraints(resolved []codersdk.WorkspaceBuildParameter, action WorkspaceCLIAction, templateVersionParameters []codersdk.TemplateVersionParameter) error { for _, r := range resolved { - tvp := pr.findTemplateVersionParameter(r) + tvp := findTemplateVersionParameter(r, templateVersionParameters) if tvp == nil { return xerrors.Errorf("parameter %q is not present in the template", r.Name) } @@ -167,20 +171,46 @@ func (pr *ParameterResolver) verifyConstraints(resolved []codersdk.WorkspaceBuil return nil } -func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuildParameter, inv *clibase.Invocation, action WorkspaceCLIAction) []codersdk.WorkspaceBuildParameter { - // update == then skip if in last build parameters unless prompt-all, build options - // update == immutable +func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuildParameter, inv *clibase.Invocation, action WorkspaceCLIAction, templateVersionParameters []codersdk.TemplateVersionParameter) ([]codersdk.WorkspaceBuildParameter, error) { + for _, tvp := range templateVersionParameters { + p := findWorkspaceBuildParameter(tvp, resolved) + if p != nil { + continue + } - panic("not implemented yet") + if (tvp.Ephemeral && pr.promptBuildOptions) || + tvp.Required || + (!tvp.Mutable && action == WorkspaceUpdate) { - return resolved + parameterValue, err := cliui.RichParameter(inv, tvp) + if err != nil { + return nil, err + } + + resolved = append(resolved, codersdk.WorkspaceBuildParameter{ + Name: tvp.Name, + Value: parameterValue, + }) + } + + } + return resolved, nil } -func (pr *ParameterResolver) findTemplateVersionParameter(workspaceBuildParameter codersdk.WorkspaceBuildParameter) *codersdk.TemplateVersionParameter { - for _, tvp := range pr.templateVersionParameters { +func findTemplateVersionParameter(workspaceBuildParameter codersdk.WorkspaceBuildParameter, templateVersionParameters []codersdk.TemplateVersionParameter) *codersdk.TemplateVersionParameter { + for _, tvp := range templateVersionParameters { if tvp.Name == workspaceBuildParameter.Name { return &tvp } } return nil } + +func findWorkspaceBuildParameter(tvp codersdk.TemplateVersionParameter, resolved []codersdk.WorkspaceBuildParameter) *codersdk.WorkspaceBuildParameter { + for _, r := range resolved { + if r.Name == tvp.Name { + return &r + } + } + return nil +} diff --git a/cli/update.go b/cli/update.go index c033efaac492c..70072fa205710 100644 --- a/cli/update.go +++ b/cli/update.go @@ -46,12 +46,9 @@ func (r *RootCmd) update() *clibase.Cmd { return err } - var existingRichParams []codersdk.WorkspaceBuildParameter - if !alwaysPrompt { - existingRichParams, err = client.WorkspaceBuildParameters(inv.Context(), workspace.LatestBuild.ID) - if err != nil { - return err - } + lastBuildParameters, err := client.WorkspaceBuildParameters(inv.Context(), workspace.LatestBuild.ID) + if err != nil { + return err } cliRichParameters, err := asWorkspaceBuildParameters(parameterFlags.richParameters) @@ -65,7 +62,7 @@ func (r *RootCmd) update() *clibase.Cmd { NewWorkspaceName: workspace.Name, WorkspaceID: workspace.LatestBuild.ID, - LastBuildParameters: existingRichParams, + LastBuildParameters: lastBuildParameters, PromptBuildOptions: parameterFlags.promptBuildOptions, BuildOptions: buildOptions, From 91d601d44d78641886c0e6dc496edfdaab5de4c9 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 8 Aug 2023 13:05:01 +0200 Subject: [PATCH 16/30] revent: cli stat --- cli/stat.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/stat.go b/cli/stat.go index 6fb24b1fb6442..3657a4f3c71c9 100644 --- a/cli/stat.go +++ b/cli/stat.go @@ -241,7 +241,7 @@ func (*RootCmd) statDisk(s *clistat.Statter) *clibase.Cmd { if err != nil { if os.IsNotExist(err) { // fmt.Errorf produces a more concise error. - return xerrors.Errorf("not found: %q", pathArg) + return fmt.Errorf("not found: %q", pathArg) } return err } From 9e05ce61853ca8ed82519a1cebfd45801c8f6db5 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 8 Aug 2023 13:28:01 +0200 Subject: [PATCH 17/30] Fix: restart, create --- cli/parameter_resolver.go | 3 ++- cli/restart.go | 12 ++++++++++-- cli/start.go | 20 ++++++++++++++++---- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/cli/parameter_resolver.go b/cli/parameter_resolver.go index e75659218c1e1..993db70f9f7c2 100644 --- a/cli/parameter_resolver.go +++ b/cli/parameter_resolver.go @@ -180,7 +180,8 @@ func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuild if (tvp.Ephemeral && pr.promptBuildOptions) || tvp.Required || - (!tvp.Mutable && action == WorkspaceUpdate) { + (!tvp.Mutable && action == WorkspaceUpdate) || + (!tvp.Ephemeral && action == WorkspaceCreate) { parameterValue, err := cliui.RichParameter(inv, tvp) if err != nil { diff --git a/cli/restart.go b/cli/restart.go index 03603de213cb4..f03b1ab0c4755 100644 --- a/cli/restart.go +++ b/cli/restart.go @@ -33,6 +33,11 @@ func (r *RootCmd) restart() *clibase.Cmd { return err } + lastBuildParameters, err := client.WorkspaceBuildParameters(inv.Context(), workspace.LatestBuild.ID) + if err != nil { + return err + } + template, err := client.Template(inv.Context(), workspace.TemplateID) if err != nil { return err @@ -44,8 +49,11 @@ func (r *RootCmd) restart() *clibase.Cmd { } buildParameters, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ - Action: WorkspaceRestart, - Template: template, + Action: WorkspaceRestart, + Template: template, + + LastBuildParameters: lastBuildParameters, + PromptBuildOptions: parameterFlags.promptBuildOptions, BuildOptions: buildOptions, }) diff --git a/cli/start.go b/cli/start.go index 90a83533e7a96..b543d2a3fdcc9 100644 --- a/cli/start.go +++ b/cli/start.go @@ -30,6 +30,11 @@ func (r *RootCmd) start() *clibase.Cmd { return err } + lastBuildParameters, err := client.WorkspaceBuildParameters(inv.Context(), workspace.LatestBuild.ID) + if err != nil { + return err + } + template, err := client.Template(inv.Context(), workspace.TemplateID) if err != nil { return err @@ -41,8 +46,11 @@ func (r *RootCmd) start() *clibase.Cmd { } buildParameters, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ - Action: WorkspaceStart, - Template: template, + Action: WorkspaceStart, + Template: template, + + LastBuildParameters: lastBuildParameters, + PromptBuildOptions: parameterFlags.promptBuildOptions, BuildOptions: buildOptions, }) @@ -71,8 +79,11 @@ func (r *RootCmd) start() *clibase.Cmd { } type prepStartWorkspaceArgs struct { - Action WorkspaceCLIAction - Template codersdk.Template + Action WorkspaceCLIAction + Template codersdk.Template + + LastBuildParameters []codersdk.WorkspaceBuildParameter + PromptBuildOptions bool BuildOptions []codersdk.WorkspaceBuildParameter } @@ -91,6 +102,7 @@ func prepStartWorkspace(inv *clibase.Invocation, client *codersdk.Client, args p } resolver := new(ParameterResolver). + WithLastBuildParameters(args.LastBuildParameters). WithPromptBuildOptions(args.PromptBuildOptions). WithBuildOptions(args.BuildOptions) return resolver.Resolve(inv, args.Action, templateVersionParameters) From f18d320043024eff9da34d37842a861167490a13 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 8 Aug 2023 14:40:02 +0200 Subject: [PATCH 18/30] fix --- cli/parameter_resolver.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/cli/parameter_resolver.go b/cli/parameter_resolver.go index 993db70f9f7c2..5d944da1fee35 100644 --- a/cli/parameter_resolver.go +++ b/cli/parameter_resolver.go @@ -180,8 +180,9 @@ func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuild if (tvp.Ephemeral && pr.promptBuildOptions) || tvp.Required || - (!tvp.Mutable && action == WorkspaceUpdate) || - (!tvp.Ephemeral && action == WorkspaceCreate) { + (action == WorkspaceUpdate && !tvp.Mutable && pr.isFirstTimeUse(tvp)) || + (action == WorkspaceUpdate && tvp.Mutable && !tvp.Ephemeral && pr.promptRichParameters) || + (action == WorkspaceCreate && !tvp.Ephemeral) { parameterValue, err := cliui.RichParameter(inv, tvp) if err != nil { @@ -198,6 +199,10 @@ func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuild return resolved, nil } +func (pr *ParameterResolver) isFirstTimeUse(tvp codersdk.TemplateVersionParameter) bool { + return findWorkspaceBuildParameter(tvp, pr.lastBuildParameters) == nil +} + func findTemplateVersionParameter(workspaceBuildParameter codersdk.WorkspaceBuildParameter, templateVersionParameters []codersdk.TemplateVersionParameter) *codersdk.TemplateVersionParameter { for _, tvp := range templateVersionParameters { if tvp.Name == workspaceBuildParameter.Name { @@ -207,10 +212,10 @@ func findTemplateVersionParameter(workspaceBuildParameter codersdk.WorkspaceBuil return nil } -func findWorkspaceBuildParameter(tvp codersdk.TemplateVersionParameter, resolved []codersdk.WorkspaceBuildParameter) *codersdk.WorkspaceBuildParameter { - for _, r := range resolved { - if r.Name == tvp.Name { - return &r +func findWorkspaceBuildParameter(tvp codersdk.TemplateVersionParameter, params []codersdk.WorkspaceBuildParameter) *codersdk.WorkspaceBuildParameter { + for _, p := range params { + if p.Name == tvp.Name { + return &p } } return nil From 47d4dd6a0a59e672686bde1e8c35731f7655bd64 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 8 Aug 2023 14:54:44 +0200 Subject: [PATCH 19/30] fix lint --- cli/create.go | 14 -------------- cli/parameter_resolver.go | 5 ++--- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/cli/create.go b/cli/create.go index 96f4e0f915090..7a8c4ec417fac 100644 --- a/cli/create.go +++ b/cli/create.go @@ -311,17 +311,3 @@ func prepWorkspaceBuild(inv *clibase.Invocation, client *codersdk.Client, args p return buildParameters, nil } - -func workspaceBuildParameterExists(ctx context.Context, client *codersdk.Client, workspaceID uuid.UUID, templateVersionParameter codersdk.TemplateVersionParameter) (bool, error) { - lastBuildParameters, err := client.WorkspaceBuildParameters(ctx, workspaceID) - if err != nil { - return false, xerrors.Errorf("can't fetch last workspace build parameters: %w", err) - } - - for _, p := range lastBuildParameters { - if p.Name == templateVersionParameter.Name { - return true, nil - } - } - return false, nil -} diff --git a/cli/parameter_resolver.go b/cli/parameter_resolver.go index 5d944da1fee35..ef28deef0efb0 100644 --- a/cli/parameter_resolver.go +++ b/cli/parameter_resolver.go @@ -64,7 +64,7 @@ func (pr *ParameterResolver) Resolve(inv *clibase.Invocation, action WorkspaceCL staged = pr.resolveWithParametersMapFile(staged) staged = pr.resolveWithCommandLineOrEnv(staged) - staged = pr.resolveWithLastBuildParameters(staged, action, templateVersionParameters) + staged = pr.resolveWithLastBuildParameters(staged, templateVersionParameters) if err = pr.verifyConstraints(staged, action, templateVersionParameters); err != nil { return nil, err } @@ -121,7 +121,7 @@ func (pr *ParameterResolver) resolveWithCommandLineOrEnv(resolved []codersdk.Wor return resolved } -func (pr *ParameterResolver) resolveWithLastBuildParameters(resolved []codersdk.WorkspaceBuildParameter, action WorkspaceCLIAction, templateVersionParameters []codersdk.TemplateVersionParameter) []codersdk.WorkspaceBuildParameter { +func (pr *ParameterResolver) resolveWithLastBuildParameters(resolved []codersdk.WorkspaceBuildParameter, templateVersionParameters []codersdk.TemplateVersionParameter) []codersdk.WorkspaceBuildParameter { if pr.promptRichParameters { return resolved // don't pull parameters from last build } @@ -194,7 +194,6 @@ func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuild Value: parameterValue, }) } - } return resolved, nil } From 2a903d8d052bbbd9be2c7435895cf4e98034e1c9 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 8 Aug 2023 15:06:39 +0200 Subject: [PATCH 20/30] Fix: old OptionalParameterAdded --- cli/update_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/cli/update_test.go b/cli/update_test.go index 886adf9bea264..445b721bb4f12 100644 --- a/cli/update_test.go +++ b/cli/update_test.go @@ -545,14 +545,11 @@ func TestUpdateValidateRichParameters(t *testing.T) { }() matches := []string{ - "added_parameter", "", - `Enter a value (default: "foobar")`, "abc", + "Planning workspace...", "", } for i := 0; i < len(matches); i += 2 { match := matches[i] - value := matches[i+1] pty.ExpectMatch(match) - pty.WriteLine(value) } <-doneChan }) From 66ce556dbb049404e5c295764b32cd7186f81b31 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 8 Aug 2023 15:16:12 +0200 Subject: [PATCH 21/30] info --- cli/parameter_resolver.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cli/parameter_resolver.go b/cli/parameter_resolver.go index ef28deef0efb0..297b059af9c10 100644 --- a/cli/parameter_resolver.go +++ b/cli/parameter_resolver.go @@ -1,6 +1,8 @@ package cli import ( + "fmt" + "golang.org/x/xerrors" "github.com/coder/coder/cli/clibase" @@ -178,9 +180,11 @@ func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuild continue } + firstTimeUse := pr.isFirstTimeUse(tvp) + if (tvp.Ephemeral && pr.promptBuildOptions) || tvp.Required || - (action == WorkspaceUpdate && !tvp.Mutable && pr.isFirstTimeUse(tvp)) || + (action == WorkspaceUpdate && !tvp.Mutable && firstTimeUse) || (action == WorkspaceUpdate && tvp.Mutable && !tvp.Ephemeral && pr.promptRichParameters) || (action == WorkspaceCreate && !tvp.Ephemeral) { @@ -193,6 +197,8 @@ func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuild Name: tvp.Name, Value: parameterValue, }) + } else if action == WorkspaceUpdate && !tvp.Mutable && !firstTimeUse { + _, _ = fmt.Fprintln(inv.Stdout, cliui.DefaultStyles.Warn.Render(fmt.Sprintf("Parameter %q is not mutable, so can't be customized after workspace creation.", tvp.Name))) } } return resolved, nil From 5763ef2db37c8e7644533de47d6049a35b5953a5 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 8 Aug 2023 15:27:33 +0200 Subject: [PATCH 22/30] Fix: lint --- cli/parameter_resolver.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/parameter_resolver.go b/cli/parameter_resolver.go index 297b059af9c10..bcce7ada59846 100644 --- a/cli/parameter_resolver.go +++ b/cli/parameter_resolver.go @@ -187,7 +187,6 @@ func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuild (action == WorkspaceUpdate && !tvp.Mutable && firstTimeUse) || (action == WorkspaceUpdate && tvp.Mutable && !tvp.Ephemeral && pr.promptRichParameters) || (action == WorkspaceCreate && !tvp.Ephemeral) { - parameterValue, err := cliui.RichParameter(inv, tvp) if err != nil { return nil, err From 02a6b1d11be47529a5b1efca5f430a945e69ac9e Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 8 Aug 2023 15:43:00 +0200 Subject: [PATCH 23/30] naming convention --- cli/{parameter_resolver.go => parameterresolver.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename cli/{parameter_resolver.go => parameterresolver.go} (100%) diff --git a/cli/parameter_resolver.go b/cli/parameterresolver.go similarity index 100% rename from cli/parameter_resolver.go rename to cli/parameterresolver.go From 177127c85728ddeb3c5dd258650132310ea68e8c Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 8 Aug 2023 15:56:22 +0200 Subject: [PATCH 24/30] More fixes --- cli/create_test.go | 36 ++++++++++++++++++++++++++++++++++++ cli/parameterresolver.go | 22 ++++++++++------------ 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/cli/create_test.go b/cli/create_test.go index 8f2bb6719a377..a86113eea854d 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -2,6 +2,7 @@ package cli_test import ( "context" + "fmt" "net/http" "os" "regexp" @@ -357,6 +358,41 @@ func TestCreateWithRichParameters(t *testing.T) { } <-doneChan }) + + t.Run("ParameterFlags", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, echoResponses) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + + inv, root := clitest.New(t, "create", "my-workspace", "--template", template.Name, + "--parameter", fmt.Sprintf("%s=%s", firstParameterName, firstParameterValue), + "--parameter", fmt.Sprintf("%s=%s", secondParameterName, secondParameterValue), + "--parameter", fmt.Sprintf("%s=%s", immutableParameterName, immutableParameterValue)) + clitest.SetupConfig(t, client, 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{ + "Confirm create?", "yes", + } + for i := 0; i < len(matches); i += 2 { + match := matches[i] + value := matches[i+1] + pty.ExpectMatch(match) + pty.WriteLine(value) + } + <-doneChan + }) } func TestCreateValidateRichParameters(t *testing.T) { diff --git a/cli/parameterresolver.go b/cli/parameterresolver.go index bcce7ada59846..43a43ebe8919e 100644 --- a/cli/parameterresolver.go +++ b/cli/parameterresolver.go @@ -107,18 +107,16 @@ func (pr *ParameterResolver) resolveWithCommandLineOrEnv(resolved []codersdk.Wor richParameterDone: } - if pr.promptBuildOptions { - for _, buildOption := range pr.buildOptions { - for i, r := range resolved { - if r.Name == buildOption.Name { - resolved[i].Value = buildOption.Value - goto buildOptionDone - } + for _, buildOption := range pr.buildOptions { + for i, r := range resolved { + if r.Name == buildOption.Name { + resolved[i].Value = buildOption.Value + goto buildOptionDone } - - resolved = append(resolved, buildOption) - buildOptionDone: } + + resolved = append(resolved, buildOption) + buildOptionDone: } return resolved } @@ -162,8 +160,8 @@ func (pr *ParameterResolver) verifyConstraints(resolved []codersdk.WorkspaceBuil return xerrors.Errorf("parameter %q is not present in the template", r.Name) } - if tvp.Ephemeral && !pr.promptBuildOptions { - return xerrors.Errorf("ephemeral parameter %q can be used only with --build-options flag", r.Name) + if tvp.Ephemeral && !pr.promptBuildOptions && len(pr.buildOptions) == 0 { + return xerrors.Errorf("ephemeral parameter %q can be used only with --build-options or --build-option flag", r.Name) } if !tvp.Mutable && action != WorkspaceCreate { From 0f1b564e421e1111d788906a3dac264e3627a499 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 9 Aug 2023 11:45:27 +0200 Subject: [PATCH 25/30] and cannot --- cli/parameterresolver.go | 2 +- cli/update_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/parameterresolver.go b/cli/parameterresolver.go index 43a43ebe8919e..675fffa50162a 100644 --- a/cli/parameterresolver.go +++ b/cli/parameterresolver.go @@ -195,7 +195,7 @@ func (pr *ParameterResolver) resolveWithInput(resolved []codersdk.WorkspaceBuild Value: parameterValue, }) } else if action == WorkspaceUpdate && !tvp.Mutable && !firstTimeUse { - _, _ = fmt.Fprintln(inv.Stdout, cliui.DefaultStyles.Warn.Render(fmt.Sprintf("Parameter %q is not mutable, so can't be customized after workspace creation.", tvp.Name))) + _, _ = fmt.Fprintln(inv.Stdout, cliui.DefaultStyles.Warn.Render(fmt.Sprintf("Parameter %q is not mutable, and cannot be customized after workspace creation.", tvp.Name))) } } return resolved, nil diff --git a/cli/update_test.go b/cli/update_test.go index 445b721bb4f12..c29de255f5a71 100644 --- a/cli/update_test.go +++ b/cli/update_test.go @@ -159,7 +159,7 @@ func TestUpdateWithRichParameters(t *testing.T) { matches := []string{ firstParameterDescription, firstParameterValue, - fmt.Sprintf("Parameter %q is not mutable, so can't be customized after workspace creation.", immutableParameterName), "", + fmt.Sprintf("Parameter %q is not mutable, and cannot be customized after workspace creation.", immutableParameterName), "", secondParameterDescription, secondParameterValue, } for i := 0; i < len(matches); i += 2 { From 7c5a5201206cbae7ce09a22ea595586bfabff122 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 9 Aug 2023 11:48:56 +0200 Subject: [PATCH 26/30] more formal --- cli/start.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/start.go b/cli/start.go index b543d2a3fdcc9..212ee1abbcb85 100644 --- a/cli/start.go +++ b/cli/start.go @@ -42,7 +42,7 @@ func (r *RootCmd) start() *clibase.Cmd { buildOptions, err := asWorkspaceBuildParameters(parameterFlags.buildOptions) if err != nil { - return xerrors.Errorf("can't parse build options: %w", err) + return xerrors.Errorf("unable to parse build options: %w", err) } buildParameters, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ From 2e017b4c6f4b95df65585fa7d690e836cce956ea Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 9 Aug 2023 11:49:52 +0200 Subject: [PATCH 27/30] more formal --- cli/parameterresolver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/parameterresolver.go b/cli/parameterresolver.go index 675fffa50162a..9e803356b45bf 100644 --- a/cli/parameterresolver.go +++ b/cli/parameterresolver.go @@ -165,7 +165,7 @@ func (pr *ParameterResolver) verifyConstraints(resolved []codersdk.WorkspaceBuil } if !tvp.Mutable && action != WorkspaceCreate { - return xerrors.Errorf("parameter %q is immutable and can't be updated", r.Name) + return xerrors.Errorf("parameter %q is immutable and cannot be updated", r.Name) } } return nil From 1d4c0e0097929a5becc655208efd43a3b94a61ba Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 9 Aug 2023 12:06:18 +0200 Subject: [PATCH 28/30] start_test.go --- cli/start_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/cli/start_test.go b/cli/start_test.go index a302fe2ac1c40..b532500fb0777 100644 --- a/cli/start_test.go +++ b/cli/start_test.go @@ -1,6 +1,7 @@ package cli_test import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -99,4 +100,43 @@ func TestStart(t *testing.T) { Value: ephemeralParameterValue, }) }) + + t.Run("BuildOptionFlags", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, echoResponses) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + inv, root := clitest.New(t, "start", workspace.Name, + "--build-option", fmt.Sprintf("%s=%s", ephemeralParameterName, ephemeralParameterValue)) + clitest.SetupConfig(t, client, root) + doneChan := make(chan struct{}) + pty := ptytest.New(t).Attach(inv) + go func() { + defer close(doneChan) + err := inv.Run() + assert.NoError(t, err) + }() + + pty.ExpectMatch("workspace has been started") + <-doneChan + + // Verify if build option is set + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + workspace, err := client.WorkspaceByOwnerAndName(ctx, workspace.OwnerName, workspace.Name, codersdk.WorkspaceOptions{}) + require.NoError(t, err) + actualParameters, err := client.WorkspaceBuildParameters(ctx, workspace.LatestBuild.ID) + require.NoError(t, err) + require.Contains(t, actualParameters, codersdk.WorkspaceBuildParameter{ + Name: ephemeralParameterName, + Value: ephemeralParameterValue, + }) + }) } From 34016f4dac6702aa45db8aaebc382e8180833d93 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 9 Aug 2023 12:38:17 +0200 Subject: [PATCH 29/30] update_test.go --- cli/update.go | 2 +- cli/update_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/cli/update.go b/cli/update.go index 70072fa205710..02ef238b36e21 100644 --- a/cli/update.go +++ b/cli/update.go @@ -31,7 +31,7 @@ func (r *RootCmd) update() *clibase.Cmd { if err != nil { return err } - if !workspace.Outdated && !alwaysPrompt && !parameterFlags.promptBuildOptions { + if !workspace.Outdated && !alwaysPrompt && !parameterFlags.promptBuildOptions && len(parameterFlags.buildOptions) == 0 { _, _ = fmt.Fprintf(inv.Stdout, "Workspace isn't outdated!\n") return nil } diff --git a/cli/update_test.go b/cli/update_test.go index c29de255f5a71..e830aabbde435 100644 --- a/cli/update_test.go +++ b/cli/update_test.go @@ -236,6 +236,55 @@ func TestUpdateWithRichParameters(t *testing.T) { Value: ephemeralParameterValue, }) }) + + t.Run("BuildOptionFlags", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, echoResponses) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + + const workspaceName = "my-workspace" + + inv, root := clitest.New(t, "create", workspaceName, "--template", template.Name, "-y", + "--parameter", fmt.Sprintf("%s=%s", firstParameterName, firstParameterValue), + "--parameter", fmt.Sprintf("%s=%s", immutableParameterName, immutableParameterValue), + "--parameter", fmt.Sprintf("%s=%s", secondParameterName, secondParameterValue)) + clitest.SetupConfig(t, client, root) + err := inv.Run() + assert.NoError(t, err) + + inv, root = clitest.New(t, "update", workspaceName, + "--build-option", fmt.Sprintf("%s=%s", ephemeralParameterName, ephemeralParameterValue)) + clitest.SetupConfig(t, client, root) + + doneChan := make(chan struct{}) + pty := ptytest.New(t).Attach(inv) + go func() { + defer close(doneChan) + err := inv.Run() + assert.NoError(t, err) + }() + + pty.ExpectMatch("Planning workspace") + <-doneChan + + // Verify if build option is set + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + workspace, err := client.WorkspaceByOwnerAndName(ctx, user.UserID.String(), workspaceName, codersdk.WorkspaceOptions{}) + require.NoError(t, err) + actualParameters, err := client.WorkspaceBuildParameters(ctx, workspace.LatestBuild.ID) + require.NoError(t, err) + require.Contains(t, actualParameters, codersdk.WorkspaceBuildParameter{ + Name: ephemeralParameterName, + Value: ephemeralParameterValue, + }) + }) } func TestUpdateValidateRichParameters(t *testing.T) { From 1d23f0951d6fc245bfe38a674ee4bbf7f3d36138 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 9 Aug 2023 12:47:09 +0200 Subject: [PATCH 30/30] restart_test.go --- cli/restart_test.go | 54 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/cli/restart_test.go b/cli/restart_test.go index 83b066e4defc5..be2c6ea423416 100644 --- a/cli/restart_test.go +++ b/cli/restart_test.go @@ -2,6 +2,7 @@ package cli_test import ( "context" + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -126,4 +127,57 @@ func TestRestart(t *testing.T) { Value: ephemeralParameterValue, }) }) + + t.Run("BuildOptionFlags", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, echoResponses) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + inv, root := clitest.New(t, "restart", workspace.Name, + "--build-option", fmt.Sprintf("%s=%s", ephemeralParameterName, ephemeralParameterValue)) + clitest.SetupConfig(t, client, 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{ + "Confirm restart workspace?", "yes", + "Stopping workspace", "", + "Starting workspace", "", + "workspace has been restarted", "", + } + for i := 0; i < len(matches); i += 2 { + match := matches[i] + value := matches[i+1] + pty.ExpectMatch(match) + + if value != "" { + pty.WriteLine(value) + } + } + <-doneChan + + // Verify if build option is set + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + workspace, err := client.WorkspaceByOwnerAndName(ctx, user.UserID.String(), workspace.Name, codersdk.WorkspaceOptions{}) + require.NoError(t, err) + actualParameters, err := client.WorkspaceBuildParameters(ctx, workspace.LatestBuild.ID) + require.NoError(t, err) + require.Contains(t, actualParameters, codersdk.WorkspaceBuildParameter{ + Name: ephemeralParameterName, + Value: ephemeralParameterValue, + }) + }) }