From b35b7b357c35da3966104ef59c2f8af18fd11d5f Mon Sep 17 00:00:00 2001 From: Abhineet Jain Date: Wed, 18 May 2022 01:37:31 +0000 Subject: [PATCH 01/11] Read params from file for template/workspace creation --- cli/create.go | 40 ++++++++++++++++++++++++++++++++++----- cli/templatecreate.go | 44 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/cli/create.go b/cli/create.go index 6a274afa4f353..69336ccefb466 100644 --- a/cli/create.go +++ b/cli/create.go @@ -2,11 +2,13 @@ package cli import ( "fmt" + "io/ioutil" "time" "github.com/spf13/cobra" "golang.org/x/exp/slices" "golang.org/x/xerrors" + "gopkg.in/yaml.v3" "github.com/coder/coder/cli/cliflag" "github.com/coder/coder/cli/cliui" @@ -18,6 +20,7 @@ func create() *cobra.Command { var ( workspaceName string templateName string + parameterFile string ) cmd := &cobra.Command{ Annotations: workspaceCommand, @@ -117,23 +120,49 @@ func create() *cobra.Command { return err } - printed := false + parameterValues := make(map[string]string) + + if parameterFile != "" { + parameterFileContents, err := ioutil.ReadFile(parameterFile) + + if err != nil { + return err + } + + err = yaml.Unmarshal(parameterFileContents, ¶meterValues) + + if err != nil { + return err + } + } + + disclaimerPrinted := false parameters := make([]codersdk.CreateParameterRequest, 0) for _, parameterSchema := range parameterSchemas { if !parameterSchema.AllowOverrideSource { continue } - if !printed { + if !disclaimerPrinted { _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("This template has customizable parameters. Values can be changed after create, but may have unintended side effects (like data loss).")+"\r\n") - printed = true + disclaimerPrinted = true } - value, err := cliui.ParameterSchema(cmd, parameterSchema) + + var parameterValue string + if parameterFile != "" { + if parameterValues[parameterSchema.Name] == "" { + return xerrors.Errorf("Parameter value absent in parameter file for %q!", parameterSchema.Name) + } + parameterValue = parameterValues[parameterSchema.Name] + } else { + parameterValue, err = cliui.ParameterSchema(cmd, parameterSchema) + } + if err != nil { return err } parameters = append(parameters, codersdk.CreateParameterRequest{ Name: parameterSchema.Name, - SourceValue: value, + SourceValue: parameterValue, SourceScheme: database.ParameterSourceSchemeData, DestinationScheme: parameterSchema.DefaultDestinationScheme, }) @@ -195,5 +224,6 @@ func create() *cobra.Command { } cliflag.StringVarP(cmd.Flags(), &templateName, "template", "t", "CODER_TEMPLATE_NAME", "", "Specify a template name.") + cliflag.StringVarP(cmd.Flags(), ¶meterFile, "parameter-file", "", "CODER_PARAMETER_FILE", "", "Specify a file path with parameter values.") return cmd } diff --git a/cli/templatecreate.go b/cli/templatecreate.go index 92d0c29465e71..7d7bab8db4d1c 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "io/ioutil" "os" "path/filepath" "sort" @@ -11,6 +12,7 @@ import ( "github.com/briandowns/spinner" "github.com/spf13/cobra" "golang.org/x/xerrors" + "gopkg.in/yaml.v3" "github.com/coder/coder/cli/cliui" "github.com/coder/coder/coderd/database" @@ -21,9 +23,10 @@ import ( func templateCreate() *cobra.Command { var ( - yes bool - directory string - provisioner string + yes bool + directory string + provisioner string + parameterFile string ) cmd := &cobra.Command{ Use: "create [name]", @@ -79,7 +82,7 @@ func templateCreate() *cobra.Command { } spin.Stop() - job, parameters, err := createValidTemplateVersion(cmd, client, organization, database.ProvisionerType(provisioner), resp.Hash) + job, parameters, err := createValidTemplateVersion(cmd, client, organization, database.ProvisionerType(provisioner), resp.Hash, parameterFile) if err != nil { return err } @@ -116,6 +119,7 @@ func templateCreate() *cobra.Command { currentDirectory, _ := os.Getwd() cmd.Flags().StringVarP(&directory, "directory", "d", currentDirectory, "Specify the directory to create from") cmd.Flags().StringVarP(&provisioner, "test.provisioner", "", "terraform", "Customize the provisioner backend") + cmd.Flags().StringVarP(¶meterFile, "parameter-file", "", "", "Specify a file path with parameter values.") // This is for testing! err := cmd.Flags().MarkHidden("test.provisioner") if err != nil { @@ -125,7 +129,7 @@ func templateCreate() *cobra.Command { return cmd } -func createValidTemplateVersion(cmd *cobra.Command, client *codersdk.Client, organization codersdk.Organization, provisioner database.ProvisionerType, hash string, parameters ...codersdk.CreateParameterRequest) (*codersdk.TemplateVersion, []codersdk.CreateParameterRequest, error) { +func createValidTemplateVersion(cmd *cobra.Command, client *codersdk.Client, organization codersdk.Organization, provisioner database.ProvisionerType, hash string, parameterFile string, parameters ...codersdk.CreateParameterRequest) (*codersdk.TemplateVersion, []codersdk.CreateParameterRequest, error) { before := time.Now() version, err := client.CreateTemplateVersion(cmd.Context(), organization.ID, codersdk.CreateTemplateVersionRequest{ StorageMethod: database.ProvisionerStorageMethodFile, @@ -184,20 +188,44 @@ func createValidTemplateVersion(cmd *cobra.Command, client *codersdk.Client, org missingSchemas = append(missingSchemas, parameterSchema) } _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("This template has required variables! They are scoped to the template, and not viewable after being set.")+"\r\n") + + parameterValuesFromFile := make(map[string]string) + if parameterFile != "" { + _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("Attempting to read the variables from the parameter file.")+"\r\n") + parameterFileContents, err := ioutil.ReadFile(parameterFile) + + if err != nil { + return nil, nil, err + } + + err = yaml.Unmarshal(parameterFileContents, ¶meterValuesFromFile) + + if err != nil { + return nil, nil, err + } + } for _, parameterSchema := range missingSchemas { - value, err := cliui.ParameterSchema(cmd, parameterSchema) + var parameterValue string + if parameterFile != "" { + if parameterValuesFromFile[parameterSchema.Name] == "" { + return nil, nil, xerrors.Errorf("Required parameter value absent in parameter file for %q!", parameterSchema.Name) + } + parameterValue = parameterValuesFromFile[parameterSchema.Name] + } else { + parameterValue, err = cliui.ParameterSchema(cmd, parameterSchema) + } if err != nil { return nil, nil, err } parameters = append(parameters, codersdk.CreateParameterRequest{ Name: parameterSchema.Name, - SourceValue: value, + SourceValue: parameterValue, SourceScheme: database.ParameterSourceSchemeData, DestinationScheme: parameterSchema.DefaultDestinationScheme, }) _, _ = fmt.Fprintln(cmd.OutOrStdout()) } - return createValidTemplateVersion(cmd, client, organization, provisioner, hash, parameters...) + return createValidTemplateVersion(cmd, client, organization, provisioner, hash, parameterFile, parameters...) } if version.Job.Status != codersdk.ProvisionerJobSucceeded { From e69ae3174ec9e17118641148c213d56aac6c350a Mon Sep 17 00:00:00 2001 From: Abhineet Jain Date: Wed, 18 May 2022 02:09:51 +0000 Subject: [PATCH 02/11] Use os.ReadFile --- cli/create.go | 4 ++-- cli/templatecreate.go | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cli/create.go b/cli/create.go index 69336ccefb466..9e239248b2e91 100644 --- a/cli/create.go +++ b/cli/create.go @@ -2,7 +2,7 @@ package cli import ( "fmt" - "io/ioutil" + "os" "time" "github.com/spf13/cobra" @@ -123,7 +123,7 @@ func create() *cobra.Command { parameterValues := make(map[string]string) if parameterFile != "" { - parameterFileContents, err := ioutil.ReadFile(parameterFile) + parameterFileContents, err := os.ReadFile(parameterFile) if err != nil { return err diff --git a/cli/templatecreate.go b/cli/templatecreate.go index 7d7bab8db4d1c..0c3c0d54fdf3a 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -2,7 +2,6 @@ package cli import ( "fmt" - "io/ioutil" "os" "path/filepath" "sort" @@ -192,7 +191,7 @@ func createValidTemplateVersion(cmd *cobra.Command, client *codersdk.Client, org parameterValuesFromFile := make(map[string]string) if parameterFile != "" { _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("Attempting to read the variables from the parameter file.")+"\r\n") - parameterFileContents, err := ioutil.ReadFile(parameterFile) + parameterFileContents, err := os.ReadFile(parameterFile) if err != nil { return nil, nil, err From dc805430cdf610e51fec2672512292b4256159f8 Mon Sep 17 00:00:00 2001 From: Abhineet Jain Date: Thu, 19 May 2022 04:57:35 +0000 Subject: [PATCH 03/11] Refactor reading params into a separate module --- cli/create.go | 27 ++++----------------- cli/parameter.go | 56 +++++++++++++++++++++++++++++++++++++++++++ cli/templatecreate.go | 22 +++-------------- go.mod | 2 +- 4 files changed, 64 insertions(+), 43 deletions(-) create mode 100644 cli/parameter.go diff --git a/cli/create.go b/cli/create.go index 9e239248b2e91..31e1bed417e18 100644 --- a/cli/create.go +++ b/cli/create.go @@ -2,13 +2,11 @@ package cli import ( "fmt" - "os" "time" "github.com/spf13/cobra" "golang.org/x/exp/slices" "golang.org/x/xerrors" - "gopkg.in/yaml.v3" "github.com/coder/coder/cli/cliflag" "github.com/coder/coder/cli/cliui" @@ -120,17 +118,10 @@ func create() *cobra.Command { return err } - parameterValues := make(map[string]string) - + var parameterMap map[string]string if parameterFile != "" { - parameterFileContents, err := os.ReadFile(parameterFile) - - if err != nil { - return err - } - - err = yaml.Unmarshal(parameterFileContents, ¶meterValues) - + _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("Attempting to read the variables from the parameter file.")+"\r\n") + parameterMap, err = createParameterMapFromFile(parameterFile) if err != nil { return err } @@ -146,17 +137,7 @@ func create() *cobra.Command { _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.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 } - - var parameterValue string - if parameterFile != "" { - if parameterValues[parameterSchema.Name] == "" { - return xerrors.Errorf("Parameter value absent in parameter file for %q!", parameterSchema.Name) - } - parameterValue = parameterValues[parameterSchema.Name] - } else { - parameterValue, err = cliui.ParameterSchema(cmd, parameterSchema) - } - + parameterValue, err := getParameterValueFromMapOrInput(cmd, parameterMap, parameterSchema) if err != nil { return err } diff --git a/cli/parameter.go b/cli/parameter.go new file mode 100644 index 0000000000000..3334c3b4fa090 --- /dev/null +++ b/cli/parameter.go @@ -0,0 +1,56 @@ +package cli + +import ( + "os" + + "golang.org/x/xerrors" + "gopkg.in/yaml.v3" + + "github.com/coder/coder/cli/cliui" + "github.com/coder/coder/codersdk" + "github.com/spf13/cobra" +) + +// 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 != "" { + parameterMap := make(map[string]string) + + parameterFileContents, err := os.ReadFile(parameterFile) + + if err != nil { + return nil, err + } + + err = yaml.Unmarshal(parameterFileContents, ¶meterMap) + + if err != nil { + return nil, err + } + + return parameterMap, nil + } + + return nil, xerrors.Errorf("Parameter file name is not specified") +} + +// Returns a parameter value from a given map, if the map exists, else takes input from the user. +// Throws an error if the map exists but does not include a value for the parameter. +func getParameterValueFromMapOrInput(cmd *cobra.Command, parameterMap map[string]string, parameterSchema codersdk.TemplateVersionParameterSchema) (string, error) { + var parameterValue string + if parameterMap != nil { + var ok bool + parameterValue, ok = parameterMap[parameterSchema.Name] + if !ok { + return "", xerrors.Errorf("Parameter value absent in parameter file for %q!", parameterSchema.Name) + } + } else { + var err error + parameterValue, err = cliui.ParameterSchema(cmd, parameterSchema) + if err != nil { + return "", err + } + } + return parameterValue, nil +} diff --git a/cli/templatecreate.go b/cli/templatecreate.go index 0c3c0d54fdf3a..385dd66e5edc7 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -11,7 +11,6 @@ import ( "github.com/briandowns/spinner" "github.com/spf13/cobra" "golang.org/x/xerrors" - "gopkg.in/yaml.v3" "github.com/coder/coder/cli/cliui" "github.com/coder/coder/coderd/database" @@ -188,31 +187,16 @@ func createValidTemplateVersion(cmd *cobra.Command, client *codersdk.Client, org } _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("This template has required variables! They are scoped to the template, and not viewable after being set.")+"\r\n") - parameterValuesFromFile := make(map[string]string) + var parameterMap map[string]string if parameterFile != "" { _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("Attempting to read the variables from the parameter file.")+"\r\n") - parameterFileContents, err := os.ReadFile(parameterFile) - - if err != nil { - return nil, nil, err - } - - err = yaml.Unmarshal(parameterFileContents, ¶meterValuesFromFile) - + parameterMap, err = createParameterMapFromFile(parameterFile) if err != nil { return nil, nil, err } } for _, parameterSchema := range missingSchemas { - var parameterValue string - if parameterFile != "" { - if parameterValuesFromFile[parameterSchema.Name] == "" { - return nil, nil, xerrors.Errorf("Required parameter value absent in parameter file for %q!", parameterSchema.Name) - } - parameterValue = parameterValuesFromFile[parameterSchema.Name] - } else { - parameterValue, err = cliui.ParameterSchema(cmd, parameterSchema) - } + parameterValue, err := getParameterValueFromMapOrInput(cmd, parameterMap, parameterSchema) if err != nil { return nil, nil, err } diff --git a/go.mod b/go.mod index 1876731d774f2..89d55c85aa843 100644 --- a/go.mod +++ b/go.mod @@ -120,6 +120,7 @@ require ( google.golang.org/protobuf v1.28.0 gopkg.in/DataDog/dd-trace-go.v1 v1.38.1 gopkg.in/natefinch/lumberjack.v2 v2.0.0 + gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 nhooyr.io/websocket v1.8.7 storj.io/drpc v0.0.30 @@ -250,5 +251,4 @@ require ( gopkg.in/ini.v1 v1.62.0 // indirect gopkg.in/square/go-jose.v2 v2.6.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect ) From 931e7d5e880cda865defe46398dde875128f8b45 Mon Sep 17 00:00:00 2001 From: Abhineet Jain Date: Fri, 20 May 2022 03:19:16 +0000 Subject: [PATCH 04/11] Add comments and unit tests --- cli/create.go | 1 + cli/create_test.go | 98 +++++++++++++++++++++ cli/parameter_internal_test.go | 53 ++++++++++++ cli/templatecreate.go | 4 + cli/templatecreate_test.go | 150 +++++++++++++++++++++++++++++++++ 5 files changed, 306 insertions(+) create mode 100644 cli/parameter_internal_test.go diff --git a/cli/create.go b/cli/create.go index 31e1bed417e18..667896924fa60 100644 --- a/cli/create.go +++ b/cli/create.go @@ -118,6 +118,7 @@ func create() *cobra.Command { return err } + // parameterMap can be nil if the file is not specified or invalid var parameterMap map[string]string if parameterFile != "" { _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("Attempting to read the variables from the parameter file.")+"\r\n") diff --git a/cli/create_test.go b/cli/create_test.go index 447daf471fddd..3eddfd5e4f514 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -1,6 +1,7 @@ package cli_test import ( + "os" "testing" "github.com/stretchr/testify/require" @@ -158,4 +159,101 @@ func TestCreate(t *testing.T) { } <-doneChan }) + t.Run("WithParameterFileContainingTheValue", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + coderdtest.NewProvisionerDaemon(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: []*proto.Parse_Response{{ + Type: &proto.Parse_Response_Complete{ + Complete: &proto.Parse_Complete{ + ParameterSchemas: []*proto.ParameterSchema{{ + AllowOverrideSource: true, + Name: "region", + Description: "description", + DefaultSource: &proto.ParameterSource{ + Scheme: proto.ParameterSource_DATA, + Value: "something", + }, + DefaultDestination: &proto.ParameterDestination{ + Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, + }, + }}, + }, + }, + }}, + Provision: echo.ProvisionComplete, + ProvisionDryRun: echo.ProvisionComplete, + }) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + parameterFile, _ := os.CreateTemp(t.TempDir(), "testParameterFile*.yaml") + _, _ = parameterFile.WriteString("region: \"bananas\"") + cmd, root := clitest.New(t, "create", "my-workspace", "--template", template.Name, "--parameter-file", parameterFile.Name()) + clitest.SetupConfig(t, client, root) + doneChan := make(chan struct{}) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + go func() { + defer close(doneChan) + err := cmd.Execute() + require.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 + }) + t.Run("WithParameterFileNotContainingTheValue", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + coderdtest.NewProvisionerDaemon(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: []*proto.Parse_Response{{ + Type: &proto.Parse_Response_Complete{ + Complete: &proto.Parse_Complete{ + ParameterSchemas: []*proto.ParameterSchema{{ + AllowOverrideSource: true, + Name: "region", + Description: "description", + DefaultSource: &proto.ParameterSource{ + Scheme: proto.ParameterSource_DATA, + Value: "something", + }, + DefaultDestination: &proto.ParameterDestination{ + Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, + }, + }}, + }, + }, + }}, + Provision: echo.ProvisionComplete, + ProvisionDryRun: echo.ProvisionComplete, + }) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + parameterFile, _ := os.CreateTemp(t.TempDir(), "testParameterFile*.yaml") + _, _ = parameterFile.WriteString("zone: \"bananas\"") + cmd, root := clitest.New(t, "create", "my-workspace", "--template", template.Name, "--parameter-file", parameterFile.Name()) + clitest.SetupConfig(t, client, root) + doneChan := make(chan struct{}) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + go func() { + defer close(doneChan) + err := cmd.Execute() + require.EqualError(t, err, "Parameter value absent in parameter file for \"region\"!") + }() + <-doneChan + }) } diff --git a/cli/parameter_internal_test.go b/cli/parameter_internal_test.go new file mode 100644 index 0000000000000..b45395f035a3e --- /dev/null +++ b/cli/parameter_internal_test.go @@ -0,0 +1,53 @@ +package cli + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCreateParameterMapFromFile(t *testing.T) { + t.Parallel() + t.Run("CreateParameterMapFromFile", func(t *testing.T) { + t.Parallel() + parameterFile, _ := os.CreateTemp(t.TempDir(), "testParameterFile*.yaml") + _, _ = parameterFile.WriteString("region: \"bananas\"\ndisk: \"20\"\n") + + parameterMapFromFile, err := createParameterMapFromFile(parameterFile.Name()) + + expectedMap := map[string]string{ + "region": "bananas", + "disk": "20", + } + + assert.Equal(t, expectedMap, parameterMapFromFile) + assert.Nil(t, err) + }) + 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") + + assert.Nil(t, parameterMapFromFile) + assert.EqualError(t, err, "open invalidFile.yaml: no such file or directory") + }) + t.Run("WithInvalidYAML", func(t *testing.T) { + t.Parallel() + parameterFile, _ := os.CreateTemp(t.TempDir(), "testParameterFile*.yaml") + _, _ = parameterFile.WriteString("region = \"bananas\"\ndisk = \"20\"\n") + + parameterMapFromFile, err := createParameterMapFromFile(parameterFile.Name()) + + assert.Nil(t, parameterMapFromFile) + assert.EqualError(t, err, "yaml: unmarshal errors:\n line 1: cannot unmarshal !!str `region ...` into map[string]string") + }) +} diff --git a/cli/templatecreate.go b/cli/templatecreate.go index 385dd66e5edc7..0fb355f7086e2 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -187,6 +187,7 @@ func createValidTemplateVersion(cmd *cobra.Command, client *codersdk.Client, org } _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("This template has required variables! They are scoped to the template, and not viewable after being set.")+"\r\n") + // parameterMap can be nil if the file is not specified or invalid var parameterMap map[string]string if parameterFile != "" { _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("Attempting to read the variables from the parameter file.")+"\r\n") @@ -208,6 +209,9 @@ func createValidTemplateVersion(cmd *cobra.Command, client *codersdk.Client, org }) _, _ = fmt.Fprintln(cmd.OutOrStdout()) } + + // This recursion is only 1 level deep in practice. + // The first pass populates the missing parameters, so it does not enter this `if` block again. return createValidTemplateVersion(cmd, client, organization, provisioner, hash, parameterFile, parameters...) } diff --git a/cli/templatecreate_test.go b/cli/templatecreate_test.go index 403b7d1ea018e..dd8e7a5921298 100644 --- a/cli/templatecreate_test.go +++ b/cli/templatecreate_test.go @@ -1,6 +1,7 @@ package cli_test import ( + "os" "testing" "github.com/stretchr/testify/require" @@ -9,6 +10,7 @@ import ( "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/coderd/database" "github.com/coder/coder/provisioner/echo" + "github.com/coder/coder/provisionersdk/proto" "github.com/coder/coder/pty/ptytest" ) @@ -48,4 +50,152 @@ func TestTemplateCreate(t *testing.T) { require.NoError(t, <-execDone) }) + t.Run("WithParameter", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + coderdtest.CreateFirstUser(t, client) + source := clitest.CreateTemplateVersionSource(t, &echo.Responses{ + Parse: []*proto.Parse_Response{{ + Type: &proto.Parse_Response_Complete{ + Complete: &proto.Parse_Complete{ + ParameterSchemas: []*proto.ParameterSchema{{ + AllowOverrideSource: true, + Name: "region", + Description: "description", + DefaultDestination: &proto.ParameterDestination{ + Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, + }, + }}, + }, + }, + }}, + Provision: echo.ProvisionComplete, + ProvisionDryRun: echo.ProvisionComplete, + }) + cmd, root := clitest.New(t, "templates", "create", "my-template", "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho)) + clitest.SetupConfig(t, client, root) + _ = coderdtest.NewProvisionerDaemon(t, client) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + + execDone := make(chan error) + go func() { + execDone <- cmd.Execute() + }() + + matches := []struct { + match string + write string + }{ + {match: "Create and upload", write: "yes"}, + {match: "Enter a value:", write: "bananas"}, + {match: "Confirm create?", write: "yes"}, + } + for _, m := range matches { + pty.ExpectMatch(m.match) + pty.WriteLine(m.write) + } + + require.NoError(t, <-execDone) + }) + t.Run("WithParameterFileContainingTheValue", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + coderdtest.CreateFirstUser(t, client) + source := clitest.CreateTemplateVersionSource(t, &echo.Responses{ + Parse: []*proto.Parse_Response{{ + Type: &proto.Parse_Response_Complete{ + Complete: &proto.Parse_Complete{ + ParameterSchemas: []*proto.ParameterSchema{{ + AllowOverrideSource: true, + Name: "region", + Description: "description", + DefaultDestination: &proto.ParameterDestination{ + Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, + }, + }}, + }, + }, + }}, + Provision: echo.ProvisionComplete, + ProvisionDryRun: echo.ProvisionComplete, + }) + parameterFile, _ := os.CreateTemp(t.TempDir(), "testParameterFile*.yaml") + _, _ = parameterFile.WriteString("region: \"bananas\"") + cmd, root := clitest.New(t, "templates", "create", "my-template", "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--parameter-file", parameterFile.Name()) + clitest.SetupConfig(t, client, root) + _ = coderdtest.NewProvisionerDaemon(t, client) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + + execDone := make(chan error) + go func() { + execDone <- cmd.Execute() + }() + + matches := []struct { + match string + write string + }{ + {match: "Create and upload", write: "yes"}, + {match: "Confirm create?", write: "yes"}, + } + for _, m := range matches { + pty.ExpectMatch(m.match) + pty.WriteLine(m.write) + } + + require.NoError(t, <-execDone) + }) + t.Run("WithParameterFileNotContainingTheValue", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + coderdtest.CreateFirstUser(t, client) + source := clitest.CreateTemplateVersionSource(t, &echo.Responses{ + Parse: []*proto.Parse_Response{{ + Type: &proto.Parse_Response_Complete{ + Complete: &proto.Parse_Complete{ + ParameterSchemas: []*proto.ParameterSchema{{ + AllowOverrideSource: true, + Name: "region", + Description: "description", + DefaultDestination: &proto.ParameterDestination{ + Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, + }, + }}, + }, + }, + }}, + Provision: echo.ProvisionComplete, + ProvisionDryRun: echo.ProvisionComplete, + }) + parameterFile, _ := os.CreateTemp(t.TempDir(), "testParameterFile*.yaml") + _, _ = parameterFile.WriteString("zone: \"bananas\"") + cmd, root := clitest.New(t, "templates", "create", "my-template", "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--parameter-file", parameterFile.Name()) + clitest.SetupConfig(t, client, root) + _ = coderdtest.NewProvisionerDaemon(t, client) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + + execDone := make(chan error) + go func() { + execDone <- cmd.Execute() + }() + + matches := []struct { + match string + write string + }{ + {match: "Create and upload", write: "yes"}, + } + for _, m := range matches { + pty.ExpectMatch(m.match) + pty.WriteLine(m.write) + } + + require.EqualError(t, <-execDone, "Parameter value absent in parameter file for \"region\"!") + }) } From 8a075f45fa07dfc3f5145c040b555b2137ad9614 Mon Sep 17 00:00:00 2001 From: Abhineet Jain Date: Fri, 20 May 2022 03:47:44 +0000 Subject: [PATCH 05/11] Rename variable --- cli/create.go | 8 ++++---- cli/templatecreate.go | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cli/create.go b/cli/create.go index cfbbe7e4981d4..ada45e529dd38 100644 --- a/cli/create.go +++ b/cli/create.go @@ -117,11 +117,11 @@ func create() *cobra.Command { return err } - // parameterMap can be nil if the file is not specified or invalid - var parameterMap map[string]string + // parameterMapFromFile can be nil if the file is not specified or invalid + var parameterMapFromFile map[string]string if parameterFile != "" { _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("Attempting to read the variables from the parameter file.")+"\r\n") - parameterMap, err = createParameterMapFromFile(parameterFile) + parameterMapFromFile, err = createParameterMapFromFile(parameterFile) if err != nil { return err } @@ -137,7 +137,7 @@ func create() *cobra.Command { _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.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 } - parameterValue, err := getParameterValueFromMapOrInput(cmd, parameterMap, parameterSchema) + parameterValue, err := getParameterValueFromMapOrInput(cmd, parameterMapFromFile, parameterSchema) if err != nil { return err } diff --git a/cli/templatecreate.go b/cli/templatecreate.go index daa6080216ff1..ef239f92e9709 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -187,17 +187,17 @@ func createValidTemplateVersion(cmd *cobra.Command, client *codersdk.Client, org } _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("This template has required variables! They are scoped to the template, and not viewable after being set.")+"\r\n") - // parameterMap can be nil if the file is not specified or invalid - var parameterMap map[string]string + // parameterMapFromFile can be nil if the file is not specified or invalid + var parameterMapFromFile map[string]string if parameterFile != "" { _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("Attempting to read the variables from the parameter file.")+"\r\n") - parameterMap, err = createParameterMapFromFile(parameterFile) + parameterMapFromFile, err = createParameterMapFromFile(parameterFile) if err != nil { return nil, nil, err } } for _, parameterSchema := range missingSchemas { - parameterValue, err := getParameterValueFromMapOrInput(cmd, parameterMap, parameterSchema) + parameterValue, err := getParameterValueFromMapOrInput(cmd, parameterMapFromFile, parameterSchema) if err != nil { return nil, nil, err } From 71e94e31f15451acece3ed3c0b300a93958b5c2d Mon Sep 17 00:00:00 2001 From: Abhineet Jain Date: Fri, 20 May 2022 03:52:57 +0000 Subject: [PATCH 06/11] Uncomment and fix unit test --- cli/create_test.go | 88 +++++++++++++++++++++++----------------------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/cli/create_test.go b/cli/create_test.go index 296492848ab06..ef719c32a008f 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -252,48 +252,48 @@ func TestCreate(t *testing.T) { } <-doneChan }) - // t.Run("WithParameterFileNotContainingTheValue", func(t *testing.T) { - // t.Parallel() - // client := coderdtest.New(t, nil) - // user := coderdtest.CreateFirstUser(t, client) - // coderdtest.NewProvisionerDaemon(t, client) - // version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ - // Parse: []*proto.Parse_Response{{ - // Type: &proto.Parse_Response_Complete{ - // Complete: &proto.Parse_Complete{ - // ParameterSchemas: []*proto.ParameterSchema{{ - // AllowOverrideSource: true, - // Name: "region", - // Description: "description", - // DefaultSource: &proto.ParameterSource{ - // Scheme: proto.ParameterSource_DATA, - // Value: "something", - // }, - // DefaultDestination: &proto.ParameterDestination{ - // Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, - // }, - // }}, - // }, - // }, - // }}, - // Provision: echo.ProvisionComplete, - // ProvisionDryRun: echo.ProvisionComplete, - // }) - // coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - // template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - // parameterFile, _ := os.CreateTemp(t.TempDir(), "testParameterFile*.yaml") - // _, _ = parameterFile.WriteString("zone: \"bananas\"") - // cmd, root := clitest.New(t, "create", "my-workspace", "--template", template.Name, "--parameter-file", parameterFile.Name()) - // clitest.SetupConfig(t, client, root) - // doneChan := make(chan struct{}) - // pty := ptytest.New(t) - // cmd.SetIn(pty.Input()) - // cmd.SetOut(pty.Output()) - // go func() { - // defer close(doneChan) - // err := cmd.Execute() - // require.EqualError(t, err, "Parameter value absent in parameter file for \"region\"!") - // }() - // <-doneChan - // }) + t.Run("WithParameterFileNotContainingTheValue", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user := coderdtest.CreateFirstUser(t, client) + + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: []*proto.Parse_Response{{ + Type: &proto.Parse_Response_Complete{ + Complete: &proto.Parse_Complete{ + ParameterSchemas: []*proto.ParameterSchema{{ + AllowOverrideSource: true, + Name: "region", + Description: "description", + DefaultSource: &proto.ParameterSource{ + Scheme: proto.ParameterSource_DATA, + Value: "something", + }, + DefaultDestination: &proto.ParameterDestination{ + Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, + }, + }}, + }, + }, + }}, + Provision: echo.ProvisionComplete, + ProvisionDryRun: echo.ProvisionComplete, + }) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + parameterFile, _ := os.CreateTemp(t.TempDir(), "testParameterFile*.yaml") + _, _ = parameterFile.WriteString("zone: \"bananas\"") + cmd, root := clitest.New(t, "create", "my-workspace", "--template", template.Name, "--parameter-file", parameterFile.Name()) + clitest.SetupConfig(t, client, root) + doneChan := make(chan struct{}) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + go func() { + defer close(doneChan) + err := cmd.Execute() + require.EqualError(t, err, "Parameter value absent in parameter file for \"region\"!") + }() + <-doneChan + }) } From d27d202f816eeafaff83dced1b8a92314ae541c2 Mon Sep 17 00:00:00 2001 From: Abhineet Jain Date: Fri, 20 May 2022 03:57:28 +0000 Subject: [PATCH 07/11] Fix comment --- cli/create.go | 2 +- cli/templatecreate.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/create.go b/cli/create.go index ada45e529dd38..4f09f71bc7eb3 100644 --- a/cli/create.go +++ b/cli/create.go @@ -117,7 +117,7 @@ func create() *cobra.Command { return err } - // parameterMapFromFile can be nil if the file is not specified or invalid + // parameterMapFromFile can be nil if parameter file is not specified var parameterMapFromFile map[string]string if parameterFile != "" { _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("Attempting to read the variables from the parameter file.")+"\r\n") diff --git a/cli/templatecreate.go b/cli/templatecreate.go index ef239f92e9709..6c98afd66cfc8 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -187,7 +187,7 @@ func createValidTemplateVersion(cmd *cobra.Command, client *codersdk.Client, org } _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("This template has required variables! They are scoped to the template, and not viewable after being set.")+"\r\n") - // parameterMapFromFile can be nil if the file is not specified or invalid + // parameterMapFromFile can be nil if parameter file is not specified var parameterMapFromFile map[string]string if parameterFile != "" { _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("Attempting to read the variables from the parameter file.")+"\r\n") From 692b97506c70bc7804cf8d95eba0e5373df1eead Mon Sep 17 00:00:00 2001 From: Abhineet Jain Date: Fri, 20 May 2022 04:06:48 +0000 Subject: [PATCH 08/11] Refactor tests --- cli/create_test.go | 124 ++++++++++++------------------------- cli/templatecreate_test.go | 62 ++++++------------- 2 files changed, 60 insertions(+), 126 deletions(-) diff --git a/cli/create_test.go b/cli/create_test.go index ef719c32a008f..a739462f08c44 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -114,39 +114,7 @@ func TestCreate(t *testing.T) { defaultValue := "something" version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ - Parse: []*proto.Parse_Response{{ - Type: &proto.Parse_Response_Complete{ - Complete: &proto.Parse_Complete{ - ParameterSchemas: []*proto.ParameterSchema{ - { - AllowOverrideSource: true, - Name: "region", - Description: "description 1", - DefaultSource: &proto.ParameterSource{ - Scheme: proto.ParameterSource_DATA, - Value: defaultValue, - }, - DefaultDestination: &proto.ParameterDestination{ - Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, - }, - }, - { - AllowOverrideSource: true, - Name: "username", - Description: "description 2", - DefaultSource: &proto.ParameterSource{ - Scheme: proto.ParameterSource_DATA, - // No default value - Value: "", - }, - DefaultDestination: &proto.ParameterDestination{ - Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, - }, - }, - }, - }, - }, - }}, + Parse: createTestParseResponseWithDefault(defaultValue), Provision: echo.ProvisionComplete, ProvisionDryRun: echo.ProvisionComplete, }) @@ -187,39 +155,7 @@ func TestCreate(t *testing.T) { defaultValue := "something" version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ - Parse: []*proto.Parse_Response{{ - Type: &proto.Parse_Response_Complete{ - Complete: &proto.Parse_Complete{ - ParameterSchemas: []*proto.ParameterSchema{ - { - AllowOverrideSource: true, - Name: "region", - Description: "description 1", - DefaultSource: &proto.ParameterSource{ - Scheme: proto.ParameterSource_DATA, - Value: defaultValue, - }, - DefaultDestination: &proto.ParameterDestination{ - Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, - }, - }, - { - AllowOverrideSource: true, - Name: "username", - Description: "description 2", - DefaultSource: &proto.ParameterSource{ - Scheme: proto.ParameterSource_DATA, - // No default value - Value: "", - }, - DefaultDestination: &proto.ParameterDestination{ - Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, - }, - }, - }, - }, - }, - }}, + Parse: createTestParseResponseWithDefault(defaultValue), Provision: echo.ProvisionComplete, ProvisionDryRun: echo.ProvisionComplete, }) @@ -257,25 +193,9 @@ func TestCreate(t *testing.T) { client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) user := coderdtest.CreateFirstUser(t, client) + defaultValue := "something" version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ - Parse: []*proto.Parse_Response{{ - Type: &proto.Parse_Response_Complete{ - Complete: &proto.Parse_Complete{ - ParameterSchemas: []*proto.ParameterSchema{{ - AllowOverrideSource: true, - Name: "region", - Description: "description", - DefaultSource: &proto.ParameterSource{ - Scheme: proto.ParameterSource_DATA, - Value: "something", - }, - DefaultDestination: &proto.ParameterDestination{ - Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, - }, - }}, - }, - }, - }}, + Parse: createTestParseResponseWithDefault(defaultValue), Provision: echo.ProvisionComplete, ProvisionDryRun: echo.ProvisionComplete, }) @@ -297,3 +217,39 @@ func TestCreate(t *testing.T) { <-doneChan }) } + +func createTestParseResponseWithDefault(defaultValue string) []*proto.Parse_Response { + return []*proto.Parse_Response{{ + Type: &proto.Parse_Response_Complete{ + Complete: &proto.Parse_Complete{ + ParameterSchemas: []*proto.ParameterSchema{ + { + AllowOverrideSource: true, + Name: "region", + Description: "description 1", + DefaultSource: &proto.ParameterSource{ + Scheme: proto.ParameterSource_DATA, + Value: defaultValue, + }, + DefaultDestination: &proto.ParameterDestination{ + Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, + }, + }, + { + AllowOverrideSource: true, + Name: "username", + Description: "description 2", + DefaultSource: &proto.ParameterSource{ + Scheme: proto.ParameterSource_DATA, + // No default value + Value: "", + }, + DefaultDestination: &proto.ParameterDestination{ + Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, + }, + }, + }, + }, + }, + }} +} diff --git a/cli/templatecreate_test.go b/cli/templatecreate_test.go index 565a410b3bb34..374ef227717a7 100644 --- a/cli/templatecreate_test.go +++ b/cli/templatecreate_test.go @@ -55,20 +55,7 @@ func TestTemplateCreate(t *testing.T) { client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) coderdtest.CreateFirstUser(t, client) source := clitest.CreateTemplateVersionSource(t, &echo.Responses{ - Parse: []*proto.Parse_Response{{ - Type: &proto.Parse_Response_Complete{ - Complete: &proto.Parse_Complete{ - ParameterSchemas: []*proto.ParameterSchema{{ - AllowOverrideSource: true, - Name: "region", - Description: "description", - DefaultDestination: &proto.ParameterDestination{ - Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, - }, - }}, - }, - }, - }}, + Parse: createTestParseResponse(), Provision: echo.ProvisionComplete, ProvisionDryRun: echo.ProvisionComplete, }) @@ -104,20 +91,7 @@ func TestTemplateCreate(t *testing.T) { client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) coderdtest.CreateFirstUser(t, client) source := clitest.CreateTemplateVersionSource(t, &echo.Responses{ - Parse: []*proto.Parse_Response{{ - Type: &proto.Parse_Response_Complete{ - Complete: &proto.Parse_Complete{ - ParameterSchemas: []*proto.ParameterSchema{{ - AllowOverrideSource: true, - Name: "region", - Description: "description", - DefaultDestination: &proto.ParameterDestination{ - Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, - }, - }}, - }, - }, - }}, + Parse: createTestParseResponse(), Provision: echo.ProvisionComplete, ProvisionDryRun: echo.ProvisionComplete, }) @@ -154,20 +128,7 @@ func TestTemplateCreate(t *testing.T) { client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) coderdtest.CreateFirstUser(t, client) source := clitest.CreateTemplateVersionSource(t, &echo.Responses{ - Parse: []*proto.Parse_Response{{ - Type: &proto.Parse_Response_Complete{ - Complete: &proto.Parse_Complete{ - ParameterSchemas: []*proto.ParameterSchema{{ - AllowOverrideSource: true, - Name: "region", - Description: "description", - DefaultDestination: &proto.ParameterDestination{ - Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, - }, - }}, - }, - }, - }}, + Parse: createTestParseResponse(), Provision: echo.ProvisionComplete, ProvisionDryRun: echo.ProvisionComplete, }) @@ -198,3 +159,20 @@ func TestTemplateCreate(t *testing.T) { require.EqualError(t, <-execDone, "Parameter value absent in parameter file for \"region\"!") }) } + +func createTestParseResponse() []*proto.Parse_Response { + return []*proto.Parse_Response{{ + Type: &proto.Parse_Response_Complete{ + Complete: &proto.Parse_Complete{ + ParameterSchemas: []*proto.ParameterSchema{{ + AllowOverrideSource: true, + Name: "region", + Description: "description", + DefaultDestination: &proto.ParameterDestination{ + Scheme: proto.ParameterDestination_PROVISIONER_VARIABLE, + }, + }}, + }, + }, + }} +} From 1a12be1805bc3075691f51917ddb1f1840775f37 Mon Sep 17 00:00:00 2001 From: Abhineet Jain Date: Fri, 20 May 2022 05:45:57 +0000 Subject: [PATCH 09/11] Fix unit tests for windows --- cli/parameter_internal_test.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/cli/parameter_internal_test.go b/cli/parameter_internal_test.go index b45395f035a3e..f9db0bf92980c 100644 --- a/cli/parameter_internal_test.go +++ b/cli/parameter_internal_test.go @@ -2,6 +2,7 @@ package cli import ( "os" + "runtime" "testing" "github.com/stretchr/testify/assert" @@ -23,6 +24,8 @@ func TestCreateParameterMapFromFile(t *testing.T) { assert.Equal(t, expectedMap, parameterMapFromFile) assert.Nil(t, err) + + removeTmpDirUntilSuccess(t) }) t.Run("WithEmptyFilename", func(t *testing.T) { t.Parallel() @@ -38,7 +41,14 @@ func TestCreateParameterMapFromFile(t *testing.T) { parameterMapFromFile, err := createParameterMapFromFile("invalidFile.yaml") assert.Nil(t, parameterMapFromFile) - assert.EqualError(t, err, "open invalidFile.yaml: no such file or directory") + + // On Unix based systems, it is: `open invalidFile.yaml: no such file or directory` + // On Windows, it is `open invalidFile.yaml: The system cannot find the file specified.` + if runtime.GOOS == "windows" { + assert.EqualError(t, err, "open invalidFile.yaml: The system cannot find the file specified.") + } else { + assert.EqualError(t, err, "open invalidFile.yaml: no such file or directory") + } }) t.Run("WithInvalidYAML", func(t *testing.T) { t.Parallel() @@ -49,5 +59,16 @@ func TestCreateParameterMapFromFile(t *testing.T) { assert.Nil(t, parameterMapFromFile) assert.EqualError(t, err, "yaml: unmarshal errors:\n line 1: cannot unmarshal !!str `region ...` into map[string]string") + + removeTmpDirUntilSuccess(t) + }) +} + +func removeTmpDirUntilSuccess(t *testing.T) { + t.Cleanup(func() { + err := os.RemoveAll(t.TempDir()) + for err != nil { + err = os.RemoveAll(t.TempDir()) + } }) } From 149162029a1347831c6a2c392c798c6ca1504ddc Mon Sep 17 00:00:00 2001 From: Abhineet Jain Date: Fri, 20 May 2022 14:40:19 +0000 Subject: [PATCH 10/11] Fix unit tests for Windows --- cli/create_test.go | 8 ++++++-- cli/parameter_internal_test.go | 17 ++++++++++------- cli/templatecreate_test.go | 18 ++++++++++++++++-- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/cli/create_test.go b/cli/create_test.go index a739462f08c44..955a001fa1245 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -162,7 +162,8 @@ func TestCreate(t *testing.T) { coderdtest.AwaitTemplateVersionJob(t, client, version.ID) _ = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - parameterFile, _ := os.CreateTemp(t.TempDir(), "testParameterFile*.yaml") + tempDir := t.TempDir() + parameterFile, _ := os.CreateTemp(tempDir, "testParameterFile*.yaml") _, _ = parameterFile.WriteString("region: \"bingo\"\nusername: \"boingo\"") cmd, root := clitest.New(t, "create", "", "--parameter-file", parameterFile.Name()) clitest.SetupConfig(t, client, root) @@ -187,6 +188,7 @@ func TestCreate(t *testing.T) { pty.WriteLine(value) } <-doneChan + removeTmpDirUntilSuccess(t, tempDir) }) t.Run("WithParameterFileNotContainingTheValue", func(t *testing.T) { t.Parallel() @@ -201,7 +203,8 @@ func TestCreate(t *testing.T) { }) coderdtest.AwaitTemplateVersionJob(t, client, version.ID) template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - parameterFile, _ := os.CreateTemp(t.TempDir(), "testParameterFile*.yaml") + tempDir := t.TempDir() + parameterFile, _ := os.CreateTemp(tempDir, "testParameterFile*.yaml") _, _ = parameterFile.WriteString("zone: \"bananas\"") cmd, root := clitest.New(t, "create", "my-workspace", "--template", template.Name, "--parameter-file", parameterFile.Name()) clitest.SetupConfig(t, client, root) @@ -215,6 +218,7 @@ func TestCreate(t *testing.T) { require.EqualError(t, err, "Parameter value absent in parameter file for \"region\"!") }() <-doneChan + removeTmpDirUntilSuccess(t, tempDir) }) } diff --git a/cli/parameter_internal_test.go b/cli/parameter_internal_test.go index f9db0bf92980c..d205a72d270bd 100644 --- a/cli/parameter_internal_test.go +++ b/cli/parameter_internal_test.go @@ -12,7 +12,8 @@ func TestCreateParameterMapFromFile(t *testing.T) { t.Parallel() t.Run("CreateParameterMapFromFile", func(t *testing.T) { t.Parallel() - parameterFile, _ := os.CreateTemp(t.TempDir(), "testParameterFile*.yaml") + tempDir := t.TempDir() + parameterFile, _ := os.CreateTemp(tempDir, "testParameterFile*.yaml") _, _ = parameterFile.WriteString("region: \"bananas\"\ndisk: \"20\"\n") parameterMapFromFile, err := createParameterMapFromFile(parameterFile.Name()) @@ -25,7 +26,7 @@ func TestCreateParameterMapFromFile(t *testing.T) { assert.Equal(t, expectedMap, parameterMapFromFile) assert.Nil(t, err) - removeTmpDirUntilSuccess(t) + removeTmpDirUntilSuccess(t, tempDir) }) t.Run("WithEmptyFilename", func(t *testing.T) { t.Parallel() @@ -52,7 +53,8 @@ func TestCreateParameterMapFromFile(t *testing.T) { }) t.Run("WithInvalidYAML", func(t *testing.T) { t.Parallel() - parameterFile, _ := os.CreateTemp(t.TempDir(), "testParameterFile*.yaml") + tempDir := t.TempDir() + parameterFile, _ := os.CreateTemp(tempDir, "testParameterFile*.yaml") _, _ = parameterFile.WriteString("region = \"bananas\"\ndisk = \"20\"\n") parameterMapFromFile, err := createParameterMapFromFile(parameterFile.Name()) @@ -60,15 +62,16 @@ func TestCreateParameterMapFromFile(t *testing.T) { assert.Nil(t, parameterMapFromFile) assert.EqualError(t, err, "yaml: unmarshal errors:\n line 1: cannot unmarshal !!str `region ...` into map[string]string") - removeTmpDirUntilSuccess(t) + removeTmpDirUntilSuccess(t, tempDir) }) } -func removeTmpDirUntilSuccess(t *testing.T) { +func removeTmpDirUntilSuccess(t *testing.T, tempDir string) { + t.Helper() t.Cleanup(func() { - err := os.RemoveAll(t.TempDir()) + err := os.RemoveAll(tempDir) for err != nil { - err = os.RemoveAll(t.TempDir()) + err = os.RemoveAll(tempDir) } }) } diff --git a/cli/templatecreate_test.go b/cli/templatecreate_test.go index 374ef227717a7..0a8989c51c747 100644 --- a/cli/templatecreate_test.go +++ b/cli/templatecreate_test.go @@ -95,7 +95,8 @@ func TestTemplateCreate(t *testing.T) { Provision: echo.ProvisionComplete, ProvisionDryRun: echo.ProvisionComplete, }) - parameterFile, _ := os.CreateTemp(t.TempDir(), "testParameterFile*.yaml") + tempDir := t.TempDir() + parameterFile, _ := os.CreateTemp(tempDir, "testParameterFile*.yaml") _, _ = parameterFile.WriteString("region: \"bananas\"") cmd, root := clitest.New(t, "templates", "create", "my-template", "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--parameter-file", parameterFile.Name()) clitest.SetupConfig(t, client, root) @@ -121,6 +122,7 @@ func TestTemplateCreate(t *testing.T) { } require.NoError(t, <-execDone) + removeTmpDirUntilSuccess(t, tempDir) }) t.Run("WithParameterFileNotContainingTheValue", func(t *testing.T) { @@ -132,7 +134,8 @@ func TestTemplateCreate(t *testing.T) { Provision: echo.ProvisionComplete, ProvisionDryRun: echo.ProvisionComplete, }) - parameterFile, _ := os.CreateTemp(t.TempDir(), "testParameterFile*.yaml") + tempDir := t.TempDir() + parameterFile, _ := os.CreateTemp(tempDir, "testParameterFile*.yaml") _, _ = parameterFile.WriteString("zone: \"bananas\"") cmd, root := clitest.New(t, "templates", "create", "my-template", "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--parameter-file", parameterFile.Name()) clitest.SetupConfig(t, client, root) @@ -157,6 +160,7 @@ func TestTemplateCreate(t *testing.T) { } require.EqualError(t, <-execDone, "Parameter value absent in parameter file for \"region\"!") + removeTmpDirUntilSuccess(t, tempDir) }) } @@ -176,3 +180,13 @@ func createTestParseResponse() []*proto.Parse_Response { }, }} } + +func removeTmpDirUntilSuccess(t *testing.T, tempDir string) { + t.Helper() + t.Cleanup(func() { + err := os.RemoveAll(tempDir) + for err != nil { + err = os.RemoveAll(tempDir) + } + }) +} From 74989acc64f4d8007dfef85f6829ace04c1ff805 Mon Sep 17 00:00:00 2001 From: Abhineet Jain Date: Fri, 20 May 2022 14:52:56 +0000 Subject: [PATCH 11/11] Add comments for the hotfix --- cli/parameter_internal_test.go | 2 ++ cli/templatecreate_test.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/cli/parameter_internal_test.go b/cli/parameter_internal_test.go index d205a72d270bd..f1316a43a87ad 100644 --- a/cli/parameter_internal_test.go +++ b/cli/parameter_internal_test.go @@ -66,6 +66,8 @@ func TestCreateParameterMapFromFile(t *testing.T) { }) } +// Need this for Windows because of a known issue with Go: +// https://github.com/golang/go/issues/52986 func removeTmpDirUntilSuccess(t *testing.T, tempDir string) { t.Helper() t.Cleanup(func() { diff --git a/cli/templatecreate_test.go b/cli/templatecreate_test.go index 0a8989c51c747..2dead6ee24b69 100644 --- a/cli/templatecreate_test.go +++ b/cli/templatecreate_test.go @@ -181,6 +181,8 @@ func createTestParseResponse() []*proto.Parse_Response { }} } +// Need this for Windows because of a known issue with Go: +// https://github.com/golang/go/issues/52986 func removeTmpDirUntilSuccess(t *testing.T, tempDir string) { t.Helper() t.Cleanup(func() {