diff --git a/cli/create.go b/cli/create.go index 602b7b40a4..7a8c4ec417 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{ @@ -129,10 +130,18 @@ func (r *RootCmd) create() *clibase.Cmd { schedSpec = ptr.Ref(sched.String()) } - buildParams, err := prepWorkspaceBuild(inv, client, prepWorkspaceBuildArgs{ - Template: template, - RichParameterFile: richParameterFile, - NewWorkspaceName: workspaceName, + 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, + RichParameters: cliRichParameters, }) if err != nil { return xerrors.Errorf("prepare build: %w", err) @@ -156,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) @@ -179,12 +188,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,99 +202,59 @@ func (r *RootCmd) create() *clibase.Cmd { }, cliui.SkipPromptOption(), ) + cmd.Options = append(cmd.Options, parameterFlags.cliParameters()...) return cmd } type prepWorkspaceBuildArgs struct { - Template codersdk.Template - ExistingRichParams []codersdk.WorkspaceBuildParameter - RichParameterFile string - NewWorkspaceName string + Action WorkspaceCLIAction + Template codersdk.Template + NewWorkspaceName string + WorkspaceID uuid.UUID - UpdateWorkspace bool - BuildOptions bool - WorkspaceID uuid.UUID -} + LastBuildParameters []codersdk.WorkspaceBuildParameter -type buildParameters struct { - // Rich parameters stores values for build parameters annotated with description, icon, type, etc. - richParameters []codersdk.WorkspaceBuildParameter + PromptBuildOptions bool + BuildOptions []codersdk.WorkspaceBuildParameter + + PromptRichParameters bool + RichParameters []codersdk.WorkspaceBuildParameter + RichParameterFile string } // 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.BuildOptions && 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). + WithLastBuildParameters(args.LastBuildParameters). + WithPromptBuildOptions(args.PromptBuildOptions). + WithBuildOptions(args.BuildOptions). + WithPromptRichParameters(args.PromptRichParameters). + WithRichParameters(args.RichParameters). + WithRichParametersFile(parameterFile) + buildParameters, err := resolver.Resolve(inv, args.Action, templateVersionParameters) + if err != nil { + return nil, err } err = cliui.GitAuth(ctx, inv.Stdout, cliui.GitAuthOptions{ @@ -306,7 +269,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) @@ -346,21 +309,5 @@ PromptRichParamLoop: return nil, xerrors.Errorf("get resources: %w", err) } - return &buildParameters{ - richParameters: richParameters, - }, 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 + return buildParameters, nil } diff --git a/cli/create_test.go b/cli/create_test.go index 8f2bb6719a..a86113eea8 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/parameter.go b/cli/parameter.go index 77e8ccdc8e..efc415692e 100644 --- a/cli/parameter.go +++ b/cli/parameter.go @@ -4,71 +4,98 @@ import ( "encoding/json" "fmt" "os" + "strings" "golang.org/x/xerrors" "gopkg.in/yaml.v3" "github.com/coder/coder/cli/clibase" - "github.com/coder/coder/cli/cliui" "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 - } - - 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 - } - - return nil, xerrors.Errorf("Parameter file name is not specified") + richParameterFile string + richParameters []string } -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 (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), + }, + } +} + +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 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 } - } - } else { - parameterValue, err = cliui.RichParameter(inv, templateVersionParameter) - if err != nil { - return nil, err + parameterMap[k] = string(b) + default: + return nil, xerrors.Errorf("invalid parameter type: %T", v) } } - return &codersdk.WorkspaceBuildParameter{ - Name: templateVersionParameter.Name, - Value: parameterValue, - }, nil + return parameterMap, nil } diff --git a/cli/parameter_internal_test.go b/cli/parameter_internal_test.go index 81dfcefdf4..935486c6ea 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/parameterresolver.go b/cli/parameterresolver.go new file mode 100644 index 0000000000..9e803356b4 --- /dev/null +++ b/cli/parameterresolver.go @@ -0,0 +1,224 @@ +package cli + +import ( + "fmt" + + "golang.org/x/xerrors" + + "github.com/coder/coder/cli/clibase" + "github.com/coder/coder/cli/cliui" + "github.com/coder/coder/codersdk" +) + +type WorkspaceCLIAction int + +const ( + WorkspaceCreate WorkspaceCLIAction = iota + WorkspaceStart + WorkspaceUpdate + WorkspaceRestart +) + +type ParameterResolver struct { + lastBuildParameters []codersdk.WorkspaceBuildParameter + + richParameters []codersdk.WorkspaceBuildParameter + richParametersFile map[string]string + buildOptions []codersdk.WorkspaceBuildParameter + + promptRichParameters 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) WithPromptRichParameters(promptRichParameters bool) *ParameterResolver { + pr.promptRichParameters = promptRichParameters + return pr +} + +func (pr *ParameterResolver) WithPromptBuildOptions(promptBuildOptions bool) *ParameterResolver { + pr.promptBuildOptions = promptBuildOptions + return pr +} + +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) + staged = pr.resolveWithLastBuildParameters(staged, 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 + } + 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: + } + + 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, templateVersionParameters []codersdk.TemplateVersionParameter) []codersdk.WorkspaceBuildParameter { + if pr.promptRichParameters { + return resolved // don't pull parameters from last build + } + + for _, buildParameter := range pr.lastBuildParameters { + tvp := findTemplateVersionParameter(buildParameter, templateVersionParameters) + 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 + goto done + } + } + + resolved = append(resolved, buildParameter) + done: + } + return resolved +} + +func (pr *ParameterResolver) verifyConstraints(resolved []codersdk.WorkspaceBuildParameter, action WorkspaceCLIAction, templateVersionParameters []codersdk.TemplateVersionParameter) error { + for _, r := range resolved { + tvp := findTemplateVersionParameter(r, templateVersionParameters) + if tvp == nil { + return xerrors.Errorf("parameter %q is not present in the template", 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 { + return xerrors.Errorf("parameter %q is immutable and cannot be updated", r.Name) + } + } + return nil +} + +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 + } + + firstTimeUse := pr.isFirstTimeUse(tvp) + + if (tvp.Ephemeral && pr.promptBuildOptions) || + tvp.Required || + (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 + } + + resolved = append(resolved, codersdk.WorkspaceBuildParameter{ + 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, and cannot be customized after workspace creation.", tvp.Name))) + } + } + 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 { + return &tvp + } + } + return nil +} + +func findWorkspaceBuildParameter(tvp codersdk.TemplateVersionParameter, params []codersdk.WorkspaceBuildParameter) *codersdk.WorkspaceBuildParameter { + for _, p := range params { + if p.Name == tvp.Name { + return &p + } + } + return nil +} diff --git a/cli/restart.go b/cli/restart.go index 4cff7ac757..f03b1ab0c4 100644 --- a/cli/restart.go +++ b/cli/restart.go @@ -4,6 +4,8 @@ import ( "fmt" "time" + "golang.org/x/xerrors" + "github.com/coder/coder/cli/clibase" "github.com/coder/coder/cli/cliui" "github.com/coder/coder/codersdk" @@ -21,7 +23,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 @@ -31,14 +33,29 @@ 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 } - buildParams, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ - Template: template, - BuildOptions: parameterFlags.buildOptions, + buildOptions, err := asWorkspaceBuildParameters(parameterFlags.buildOptions) + if err != nil { + return xerrors.Errorf("can't parse build options: %w", err) + } + + buildParameters, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ + Action: WorkspaceRestart, + Template: template, + + LastBuildParameters: lastBuildParameters, + + PromptBuildOptions: parameterFlags.promptBuildOptions, + BuildOptions: buildOptions, }) if err != nil { return err @@ -65,7 +82,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/restart_test.go b/cli/restart_test.go index 83b066e4de..be2c6ea423 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, + }) + }) } diff --git a/cli/start.go b/cli/start.go index 5bd35867fd..212ee1abbc 100644 --- a/cli/start.go +++ b/cli/start.go @@ -11,21 +11,6 @@ import ( "github.com/coder/coder/codersdk" ) -// workspaceParameterFlags are used by "start", "restart", and "update". -type workspaceParameterFlags struct { - buildOptions bool -} - -func (wpf *workspaceParameterFlags) options() []clibase.Option { - return clibase.OptionSet{ - { - Flag: "build-options", - Description: "Prompt for one-time build options defined with ephemeral parameters.", - Value: clibase.BoolOf(&wpf.buildOptions), - }, - } -} - func (r *RootCmd) start() *clibase.Cmd { var parameterFlags workspaceParameterFlags @@ -38,21 +23,36 @@ 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 { 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 } - buildParams, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ - Template: template, - BuildOptions: parameterFlags.buildOptions, + buildOptions, err := asWorkspaceBuildParameters(parameterFlags.buildOptions) + if err != nil { + return xerrors.Errorf("unable to parse build options: %w", err) + } + + buildParameters, err := prepStartWorkspace(inv, client, prepStartWorkspaceArgs{ + Action: WorkspaceStart, + Template: template, + + LastBuildParameters: lastBuildParameters, + + PromptBuildOptions: parameterFlags.promptBuildOptions, + BuildOptions: buildOptions, }) if err != nil { return err @@ -60,7 +60,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 @@ -79,16 +79,21 @@ func (r *RootCmd) start() *clibase.Cmd { } type prepStartWorkspaceArgs struct { - Template codersdk.Template - BuildOptions bool + Action WorkspaceCLIAction + Template codersdk.Template + + LastBuildParameters []codersdk.WorkspaceBuildParameter + + 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) @@ -96,30 +101,9 @@ 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.BuildOptions { - return &buildParameters{ - richParameters: richParameters, - }, nil - } - - for _, templateVersionParameter := range templateVersionParameters { - if !templateVersionParameter.Ephemeral { - continue - } - - parameterValue, err := cliui.RichParameter(inv, templateVersionParameter) - if err != nil { - return nil, err - } - - richParameters = append(richParameters, codersdk.WorkspaceBuildParameter{ - Name: templateVersionParameter.Name, - Value: parameterValue, - }) - } - - return &buildParameters{ - richParameters: richParameters, - }, nil + resolver := new(ParameterResolver). + WithLastBuildParameters(args.LastBuildParameters). + WithPromptBuildOptions(args.PromptBuildOptions). + WithBuildOptions(args.BuildOptions) + return resolver.Resolve(inv, args.Action, templateVersionParameters) } diff --git a/cli/start_test.go b/cli/start_test.go index a302fe2ac1..b532500fb0 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, + }) + }) } diff --git a/cli/testdata/coder_create_--help.golden b/cli/testdata/coder_create_--help.golden index 6c8bcc4690..2e080b6e85 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 c2079b9065..e16a6f9ff7 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 aa447240e9..b03c997592 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 40e899cd37..669bda831c 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. diff --git a/cli/update.go b/cli/update.go index 64710217bb..02ef238b36 100644 --- a/cli/update.go +++ b/cli/update.go @@ -3,14 +3,15 @@ package cli import ( "fmt" + "golang.org/x/xerrors" + "github.com/coder/coder/cli/clibase" "github.com/coder/coder/codersdk" ) func (r *RootCmd) update() *clibase.Cmd { var ( - richParameterFile string - alwaysPrompt bool + alwaysPrompt bool parameterFlags workspaceParameterFlags ) @@ -30,33 +31,45 @@ func (r *RootCmd) update() *clibase.Cmd { if err != nil { return err } - if !workspace.Outdated && !alwaysPrompt && !parameterFlags.buildOptions { + if !workspace.Outdated && !alwaysPrompt && !parameterFlags.promptBuildOptions && len(parameterFlags.buildOptions) == 0 { _, _ = 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 } - 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 } - buildParams, err := prepWorkspaceBuild(inv, client, prepWorkspaceBuildArgs{ - Template: template, - ExistingRichParams: existingRichParams, - RichParameterFile: 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, + buildParameters, err := prepWorkspaceBuild(inv, client, prepWorkspaceBuildArgs{ + Action: WorkspaceUpdate, + Template: template, + NewWorkspaceName: workspace.Name, + WorkspaceID: workspace.LatestBuild.ID, - BuildOptions: parameterFlags.buildOptions, + LastBuildParameters: lastBuildParameters, + + PromptBuildOptions: parameterFlags.promptBuildOptions, + BuildOptions: buildOptions, + + PromptRichParameters: alwaysPrompt, + RichParameters: cliRichParameters, + RichParameterFile: parameterFlags.richParameterFile, }) if err != nil { return err @@ -65,7 +78,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: buildParameters, }) if err != nil { return err @@ -92,13 +105,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.cliBuildOptions()...) + cmd.Options = append(cmd.Options, parameterFlags.cliParameters()...) return cmd } diff --git a/cli/update_test.go b/cli/update_test.go index 886adf9bea..e830aabbde 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 { @@ -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) { @@ -545,14 +594,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 }) diff --git a/docs/cli/create.md b/docs/cli/create.md index 3f7ff099b1..8c36ea44a3 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 72daa5dec4..d3b6010a92 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 8c3fd7f712..120edfde67 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 0b3d31a975..b81172df6b 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 | | |