diff --git a/cli/templatecreate.go b/cli/templatecreate.go index 873eb983133b0..4a139a72dd1b1 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -25,6 +25,8 @@ func templateCreate() *cobra.Command { provisioner string provisionerTags []string parameterFile string + variablesFile string + variables []string defaultTTL time.Duration uploadFlags templateUploadFlags @@ -76,6 +78,8 @@ func templateCreate() *cobra.Command { FileID: resp.ID, ParameterFile: parameterFile, ProvisionerTags: tags, + VariablesFile: variablesFile, + Variables: variables, }) if err != nil { return err @@ -113,6 +117,8 @@ func templateCreate() *cobra.Command { }, } cmd.Flags().StringVarP(¶meterFile, "parameter-file", "", "", "Specify a file path with parameter values.") + cmd.Flags().StringVarP(&variablesFile, "variables-file", "", "", "Specify a file path with values for Terraform-managed variables.") + cmd.Flags().StringArrayVarP(&variables, "variable", "", []string{}, "Specify a set of values for Terraform-managed variables.") cmd.Flags().StringArrayVarP(&provisionerTags, "provisioner-tag", "", []string{}, "Specify a set of tags to target provisioner daemons.") cmd.Flags().DurationVarP(&defaultTTL, "default-ttl", "", 24*time.Hour, "Specify a default TTL for workspaces created from this template.") uploadFlags.register(cmd.Flags()) @@ -133,7 +139,10 @@ type createValidTemplateVersionArgs struct { Provisioner database.ProvisionerType FileID uuid.UUID ParameterFile string - ValuesFile string + + VariablesFile string + Variables []string + // Template is only required if updating a template's active version. Template *codersdk.Template // ReuseParameters will attempt to reuse params from the Template field @@ -146,12 +155,16 @@ type createValidTemplateVersionArgs struct { func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVersionArgs, parameters ...codersdk.CreateParameterRequest) (*codersdk.TemplateVersion, []codersdk.CreateParameterRequest, error) { client := args.Client - // FIXME(mtojek): I will iterate on CLI experience in the follow-up. - // see: https://github.com/coder/coder/issues/5980 - variableValues, err := loadVariableValues(args.ValuesFile) + variableValues, err := loadVariableValuesFromFile(args.VariablesFile) + if err != nil { + return nil, nil, err + } + + variableValuesFromKeyValues, err := loadVariableValuesFromOptions(args.Variables) if err != nil { return nil, nil, err } + variableValues = append(variableValues, variableValuesFromKeyValues...) req := codersdk.CreateTemplateVersionRequest{ Name: args.Name, diff --git a/cli/templatecreate_test.go b/cli/templatecreate_test.go index e44c8c7e2a2ce..b9308854dc868 100644 --- a/cli/templatecreate_test.go +++ b/cli/templatecreate_test.go @@ -299,6 +299,161 @@ func TestTemplateCreate(t *testing.T) { require.EqualError(t, <-execDone, "Template name must be less than 32 characters") }) + + t.Run("WithVariablesFileWithoutRequiredValue", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + coderdtest.CreateFirstUser(t, client) + + templateVariables := []*proto.TemplateVariable{ + { + Name: "first_variable", + Description: "This is the first variable", + Type: "string", + Required: true, + Sensitive: true, + }, + { + Name: "second_variable", + Description: "This is the first variable", + Type: "string", + DefaultValue: "abc", + Required: false, + Sensitive: true, + }, + } + source := clitest.CreateTemplateVersionSource(t, + createEchoResponsesWithTemplateVariables(templateVariables)) + tempDir := t.TempDir() + removeTmpDirUntilSuccessAfterTest(t, tempDir) + variablesFile, _ := os.CreateTemp(tempDir, "variables*.yaml") + _, _ = variablesFile.WriteString(`second_variable: foobar`) + cmd, root := clitest.New(t, "templates", "create", "my-template", "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--variables-file", variablesFile.Name()) + clitest.SetupConfig(t, client, root) + 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: "Upload", write: "yes"}, + } + for _, m := range matches { + pty.ExpectMatch(m.match) + if len(m.write) > 0 { + pty.WriteLine(m.write) + } + } + + require.Error(t, <-execDone) + }) + + t.Run("WithVariablesFileWithTheRequiredValue", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + coderdtest.CreateFirstUser(t, client) + + templateVariables := []*proto.TemplateVariable{ + { + Name: "first_variable", + Description: "This is the first variable", + Type: "string", + Required: true, + Sensitive: true, + }, + { + Name: "second_variable", + Description: "This is the second variable", + Type: "string", + DefaultValue: "abc", + Required: false, + Sensitive: true, + }, + } + source := clitest.CreateTemplateVersionSource(t, + createEchoResponsesWithTemplateVariables(templateVariables)) + tempDir := t.TempDir() + removeTmpDirUntilSuccessAfterTest(t, tempDir) + variablesFile, _ := os.CreateTemp(tempDir, "variables*.yaml") + _, _ = variablesFile.WriteString(`first_variable: foobar`) + cmd, root := clitest.New(t, "templates", "create", "my-template", "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--variables-file", variablesFile.Name()) + clitest.SetupConfig(t, client, root) + 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: "Upload", write: "yes"}, + {match: "Confirm create?", write: "yes"}, + } + for _, m := range matches { + pty.ExpectMatch(m.match) + if len(m.write) > 0 { + pty.WriteLine(m.write) + } + } + + require.NoError(t, <-execDone) + }) + t.Run("WithVariableOption", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + coderdtest.CreateFirstUser(t, client) + + templateVariables := []*proto.TemplateVariable{ + { + Name: "first_variable", + Description: "This is the first variable", + Type: "string", + Required: true, + Sensitive: true, + }, + } + source := clitest.CreateTemplateVersionSource(t, + createEchoResponsesWithTemplateVariables(templateVariables)) + cmd, root := clitest.New(t, "templates", "create", "my-template", "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--variable", "first_variable=foobar") + clitest.SetupConfig(t, client, root) + 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: "Upload", write: "yes"}, + {match: "Confirm create?", write: "yes"}, + } + for _, m := range matches { + pty.ExpectMatch(m.match) + pty.WriteLine(m.write) + } + + require.NoError(t, <-execDone) + }) } func createTestParseResponse() []*proto.Parse_Response { diff --git a/cli/templatepush.go b/cli/templatepush.go index ad52b683cd8ab..c9217cc35f4a6 100644 --- a/cli/templatepush.go +++ b/cli/templatepush.go @@ -93,7 +93,8 @@ func templatePush() *cobra.Command { versionName string provisioner string parameterFile string - valuesFile string + variablesFile string + variables []string alwaysPrompt bool provisionerTags []string uploadFlags templateUploadFlags @@ -140,7 +141,8 @@ func templatePush() *cobra.Command { Provisioner: database.ProvisionerType(provisioner), FileID: resp.ID, ParameterFile: parameterFile, - ValuesFile: valuesFile, + VariablesFile: variablesFile, + Variables: variables, Template: &template, ReuseParameters: !alwaysPrompt, ProvisionerTags: tags, @@ -167,7 +169,8 @@ func templatePush() *cobra.Command { cmd.Flags().StringVarP(&provisioner, "test.provisioner", "", "terraform", "Customize the provisioner backend") cmd.Flags().StringVarP(¶meterFile, "parameter-file", "", "", "Specify a file path with parameter values.") - cmd.Flags().StringVarP(&valuesFile, "values-file", "", "", "Specify a file path with values for managed variables.") + cmd.Flags().StringVarP(&variablesFile, "variables-file", "", "", "Specify a file path with values for Terraform-managed variables.") + cmd.Flags().StringArrayVarP(&variables, "variable", "", []string{}, "Specify a set of values for Terraform-managed variables.") cmd.Flags().StringVarP(&versionName, "name", "", "", "Specify a name for the new template version. It will be automatically generated if not provided.") cmd.Flags().StringArrayVarP(&provisionerTags, "provisioner-tag", "", []string{}, "Specify a set of tags to target provisioner daemons.") cmd.Flags().BoolVar(&alwaysPrompt, "always-prompt", false, "Always prompt all parameters. Does not pull parameter values from active template version") diff --git a/cli/templatepush_test.go b/cli/templatepush_test.go index 0ade770324813..90af2d90ad75a 100644 --- a/cli/templatepush_test.go +++ b/cli/templatepush_test.go @@ -3,6 +3,7 @@ package cli_test import ( "bytes" "context" + "os" "path/filepath" "testing" @@ -15,6 +16,7 @@ import ( "github.com/coder/coder/coderd/database" "github.com/coder/coder/codersdk" "github.com/coder/coder/provisioner/echo" + "github.com/coder/coder/provisionersdk/proto" "github.com/coder/coder/pty/ptytest" ) @@ -250,6 +252,250 @@ func TestTemplatePush(t *testing.T) { assert.Len(t, templateVersions, 2) assert.NotEqual(t, template.ActiveVersionID, templateVersions[1].ID) }) + + t.Run("Variables", func(t *testing.T) { + t.Parallel() + + initialTemplateVariables := []*proto.TemplateVariable{ + { + Name: "first_variable", + Description: "This is the first variable", + Type: "string", + DefaultValue: "abc", + Required: false, + Sensitive: true, + }, + } + + t.Run("VariableIsRequired", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + + templateVersion := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, createEchoResponsesWithTemplateVariables(initialTemplateVariables)) + _ = coderdtest.AwaitTemplateVersionJob(t, client, templateVersion.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, templateVersion.ID) + + // Test the cli command. + modifiedTemplateVariables := append(initialTemplateVariables, + &proto.TemplateVariable{ + Name: "second_variable", + Description: "This is the second variable", + Type: "string", + Required: true, + }, + ) + source := clitest.CreateTemplateVersionSource(t, createEchoResponsesWithTemplateVariables(modifiedTemplateVariables)) + tempDir := t.TempDir() + removeTmpDirUntilSuccessAfterTest(t, tempDir) + variablesFile, _ := os.CreateTemp(tempDir, "variables*.yaml") + _, _ = variablesFile.WriteString(`second_variable: foobar`) + cmd, root := clitest.New(t, "templates", "push", template.Name, "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--name", "example", "--variables-file", variablesFile.Name()) + clitest.SetupConfig(t, client, root) + 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: "Upload", write: "yes"}, + } + for _, m := range matches { + pty.ExpectMatch(m.match) + pty.WriteLine(m.write) + } + + require.NoError(t, <-execDone) + + // Assert that the template version changed. + templateVersions, err := client.TemplateVersionsByTemplate(context.Background(), codersdk.TemplateVersionsByTemplateRequest{ + TemplateID: template.ID, + }) + require.NoError(t, err) + assert.Len(t, templateVersions, 2) + assert.NotEqual(t, template.ActiveVersionID, templateVersions[1].ID) + require.Equal(t, "example", templateVersions[1].Name) + + templateVariables, err := client.TemplateVersionVariables(context.Background(), templateVersions[1].ID) + require.NoError(t, err) + assert.Len(t, templateVariables, 2) + require.Equal(t, "second_variable", templateVariables[1].Name) + require.Equal(t, "foobar", templateVariables[1].Value) + }) + + t.Run("VariableIsRequiredButNotProvided", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + + templateVersion := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, createEchoResponsesWithTemplateVariables(initialTemplateVariables)) + _ = coderdtest.AwaitTemplateVersionJob(t, client, templateVersion.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, templateVersion.ID) + + // Test the cli command. + modifiedTemplateVariables := append(initialTemplateVariables, + &proto.TemplateVariable{ + Name: "second_variable", + Description: "This is the second variable", + Type: "string", + Required: true, + }, + ) + source := clitest.CreateTemplateVersionSource(t, createEchoResponsesWithTemplateVariables(modifiedTemplateVariables)) + cmd, root := clitest.New(t, "templates", "push", template.Name, "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--name", "example") + clitest.SetupConfig(t, client, root) + 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: "Upload", write: "yes"}, + } + for _, m := range matches { + pty.ExpectMatch(m.match) + pty.WriteLine(m.write) + } + + wantErr := <-execDone + require.Error(t, wantErr) + require.Contains(t, wantErr.Error(), "required template variables need values") + }) + + t.Run("VariableIsOptionalButNotProvided", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + + templateVersion := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, createEchoResponsesWithTemplateVariables(initialTemplateVariables)) + _ = coderdtest.AwaitTemplateVersionJob(t, client, templateVersion.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, templateVersion.ID) + + // Test the cli command. + modifiedTemplateVariables := append(initialTemplateVariables, + &proto.TemplateVariable{ + Name: "second_variable", + Description: "This is the second variable", + Type: "string", + DefaultValue: "abc", + Required: true, + }, + ) + source := clitest.CreateTemplateVersionSource(t, createEchoResponsesWithTemplateVariables(modifiedTemplateVariables)) + cmd, root := clitest.New(t, "templates", "push", template.Name, "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--name", "example") + clitest.SetupConfig(t, client, root) + 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: "Upload", write: "yes"}, + } + for _, m := range matches { + pty.ExpectMatch(m.match) + pty.WriteLine(m.write) + } + + require.NoError(t, <-execDone) + + // Assert that the template version changed. + templateVersions, err := client.TemplateVersionsByTemplate(context.Background(), codersdk.TemplateVersionsByTemplateRequest{ + TemplateID: template.ID, + }) + require.NoError(t, err) + assert.Len(t, templateVersions, 2) + assert.NotEqual(t, template.ActiveVersionID, templateVersions[1].ID) + require.Equal(t, "example", templateVersions[1].Name) + + templateVariables, err := client.TemplateVersionVariables(context.Background(), templateVersions[1].ID) + require.NoError(t, err) + assert.Len(t, templateVariables, 2) + require.Equal(t, "second_variable", templateVariables[1].Name) + require.Equal(t, "abc", templateVariables[1].Value) + require.Equal(t, templateVariables[1].DefaultValue, templateVariables[1].Value) + }) + + t.Run("WithVariableOption", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + + templateVersion := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, createEchoResponsesWithTemplateVariables(initialTemplateVariables)) + _ = coderdtest.AwaitTemplateVersionJob(t, client, templateVersion.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, templateVersion.ID) + + // Test the cli command. + modifiedTemplateVariables := append(initialTemplateVariables, + &proto.TemplateVariable{ + Name: "second_variable", + Description: "This is the second variable", + Type: "string", + Required: true, + }, + ) + source := clitest.CreateTemplateVersionSource(t, createEchoResponsesWithTemplateVariables(modifiedTemplateVariables)) + cmd, root := clitest.New(t, "templates", "push", template.Name, "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho), "--name", "example", "--variable", "second_variable=foobar") + clitest.SetupConfig(t, client, root) + 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: "Upload", write: "yes"}, + } + for _, m := range matches { + pty.ExpectMatch(m.match) + pty.WriteLine(m.write) + } + + require.NoError(t, <-execDone) + + // Assert that the template version changed. + templateVersions, err := client.TemplateVersionsByTemplate(context.Background(), codersdk.TemplateVersionsByTemplateRequest{ + TemplateID: template.ID, + }) + require.NoError(t, err) + assert.Len(t, templateVersions, 2) + assert.NotEqual(t, template.ActiveVersionID, templateVersions[1].ID) + require.Equal(t, "example", templateVersions[1].Name) + + templateVariables, err := client.TemplateVersionVariables(context.Background(), templateVersions[1].ID) + require.NoError(t, err) + assert.Len(t, templateVariables, 2) + require.Equal(t, "second_variable", templateVariables[1].Name) + require.Equal(t, "foobar", templateVariables[1].Value) + }) + }) } func latestTemplateVersion(t *testing.T, client *codersdk.Client, templateID uuid.UUID) (codersdk.TemplateVersion, []codersdk.Parameter) { @@ -265,3 +511,19 @@ func latestTemplateVersion(t *testing.T, client *codersdk.Client, templateID uui return tv, params } + +func createEchoResponsesWithTemplateVariables(templateVariables []*proto.TemplateVariable) *echo.Responses { + return &echo.Responses{ + Parse: []*proto.Parse_Response{ + { + Type: &proto.Parse_Response_Complete{ + Complete: &proto.Parse_Complete{ + TemplateVariables: templateVariables, + }, + }, + }, + }, + ProvisionPlan: echo.ProvisionComplete, + ProvisionApply: echo.ProvisionComplete, + } +} diff --git a/cli/templatevariables.go b/cli/templatevariables.go index fa49235c64ffd..888b8a04e30f5 100644 --- a/cli/templatevariables.go +++ b/cli/templatevariables.go @@ -2,6 +2,7 @@ package cli import ( "os" + "strings" "golang.org/x/xerrors" "gopkg.in/yaml.v3" @@ -9,7 +10,7 @@ import ( "github.com/coder/coder/codersdk" ) -func loadVariableValues(variablesFile string) ([]codersdk.VariableValue, error) { +func loadVariableValuesFromFile(variablesFile string) ([]codersdk.VariableValue, error) { var values []codersdk.VariableValue if variablesFile == "" { return values, nil @@ -48,3 +49,19 @@ func createVariablesMapFromFile(variablesFile string) (map[string]string, error) } return variablesMap, nil } + +func loadVariableValuesFromOptions(variables []string) ([]codersdk.VariableValue, error) { + var values []codersdk.VariableValue + for _, keyValue := range variables { + split := strings.SplitN(keyValue, "=", 2) + if len(split) < 2 { + return nil, xerrors.Errorf("format key=value expected, but got %s", keyValue) + } + + values = append(values, codersdk.VariableValue{ + Name: split[0], + Value: split[1], + }) + } + return values, nil +} diff --git a/cli/testdata/coder_templates_create_--help.golden b/cli/testdata/coder_templates_create_--help.golden index 0ca9259692326..1e950b50117af 100644 --- a/cli/testdata/coder_templates_create_--help.golden +++ b/cli/testdata/coder_templates_create_--help.golden @@ -11,6 +11,9 @@ Flags: -h, --help help for create --parameter-file string Specify a file path with parameter values. --provisioner-tag stringArray Specify a set of tags to target provisioner daemons. + --variable stringArray Specify a set of values for Terraform-managed variables. + --variables-file string Specify a file path with values for Terraform-managed + variables. -y, --yes Bypass prompts Global Flags: diff --git a/cli/testdata/coder_templates_push_--help.golden b/cli/testdata/coder_templates_push_--help.golden index 0fa75221f7546..62c38172a4923 100644 --- a/cli/testdata/coder_templates_push_--help.golden +++ b/cli/testdata/coder_templates_push_--help.golden @@ -13,7 +13,9 @@ Flags: automatically generated if not provided. --parameter-file string Specify a file path with parameter values. --provisioner-tag stringArray Specify a set of tags to target provisioner daemons. - --values-file string Specify a file path with values for managed variables. + --variable stringArray Specify a set of values for Terraform-managed variables. + --variables-file string Specify a file path with values for Terraform-managed + variables. -y, --yes Bypass prompts Global Flags: diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index 97483af3fba6c..68f3fab3b87a5 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -264,9 +264,14 @@ func (server *Server) AcquireJob(ctx context.Context, _ *proto.Empty) (*proto.Ac return nil, failJob(fmt.Sprintf("unmarshal job input %q: %s", job.Input, err)) } + userVariableValues, err := server.includeLastVariableValues(ctx, input.TemplateVersionID, input.UserVariableValues) + if err != nil { + return nil, failJob(err.Error()) + } + protoJob.Type = &proto.AcquiredJob_TemplateImport_{ TemplateImport: &proto.AcquiredJob_TemplateImport{ - UserVariableValues: convertVariableValues(input.UserVariableValues), + UserVariableValues: convertVariableValues(userVariableValues), Metadata: &sdkproto.Provision_Metadata{ CoderUrl: server.AccessURL.String(), }, @@ -290,6 +295,58 @@ func (server *Server) AcquireJob(ctx context.Context, _ *proto.Empty) (*proto.Ac return protoJob, err } +func (server *Server) includeLastVariableValues(ctx context.Context, templateVersionID uuid.UUID, userVariableValues []codersdk.VariableValue) ([]codersdk.VariableValue, error) { + var values []codersdk.VariableValue + values = append(values, userVariableValues...) + + if templateVersionID == uuid.Nil { + return values, nil + } + + templateVersion, err := server.Database.GetTemplateVersionByID(ctx, templateVersionID) + if err != nil { + return nil, fmt.Errorf("get template version: %w", err) + } + + if templateVersion.TemplateID.UUID == uuid.Nil { + return values, nil + } + + template, err := server.Database.GetTemplateByID(ctx, templateVersion.TemplateID.UUID) + if err != nil { + return nil, fmt.Errorf("get template: %w", err) + } + + if template.ActiveVersionID == uuid.Nil { + return values, nil + } + + templateVariables, err := server.Database.GetTemplateVersionVariables(ctx, template.ActiveVersionID) + if err != nil && !xerrors.Is(err, sql.ErrNoRows) { + return nil, fmt.Errorf("get template version variables: %w", err) + } + + for _, templateVariable := range templateVariables { + var alreadyAdded bool + for _, uvv := range userVariableValues { + if uvv.Name == templateVariable.Name { + alreadyAdded = true + break + } + } + + if alreadyAdded { + continue + } + + values = append(values, codersdk.VariableValue{ + Name: templateVariable.Name, + Value: templateVariable.Value, + }) + } + return values, nil +} + func (server *Server) CommitQuota(ctx context.Context, request *proto.CommitQuotaRequest) (*proto.CommitQuotaResponse, error) { //nolint:gocritic // Provisionerd has specific authz rules. ctx = dbauthz.AsProvisionerd(ctx) @@ -1222,8 +1279,9 @@ func convertVariableValues(variableValues []codersdk.VariableValue) []*sdkproto. protoVariableValues := make([]*sdkproto.VariableValue, len(variableValues)) for i, variableValue := range variableValues { protoVariableValues[i] = &sdkproto.VariableValue{ - Name: variableValue.Name, - Value: variableValue.Value, + Name: variableValue.Name, + Value: variableValue.Value, + Sensitive: true, // Without the template variable schema we have to assume that every variable may be sensitive. } } return protoVariableValues diff --git a/coderd/provisionerdserver/provisionerdserver_test.go b/coderd/provisionerdserver/provisionerdserver_test.go index b628520347f3a..e456d5d4e0c4e 100644 --- a/coderd/provisionerdserver/provisionerdserver_test.go +++ b/coderd/provisionerdserver/provisionerdserver_test.go @@ -316,7 +316,7 @@ func TestAcquireJob(t *testing.T) { want, err := json.Marshal(&proto.AcquiredJob_TemplateImport_{ TemplateImport: &proto.AcquiredJob_TemplateImport{ UserVariableValues: []*sdkproto.VariableValue{ - {Name: "first", Value: "first_value"}, + {Name: "first", Sensitive: true, Value: "first_value"}, }, Metadata: &sdkproto.Provision_Metadata{ CoderUrl: srv.AccessURL.String(), diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 528b5993d3ef8..ff60c576b9831 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -1531,8 +1531,10 @@ func convertTemplateVersionVariables(dbVariables []database.TemplateVersionVaria return variables } +const redacted = "*redacted*" + func convertTemplateVersionVariable(variable database.TemplateVersionVariable) codersdk.TemplateVersionVariable { - return codersdk.TemplateVersionVariable{ + templateVariable := codersdk.TemplateVersionVariable{ Name: variable.Name, Description: variable.Description, Type: variable.Type, @@ -1541,6 +1543,11 @@ func convertTemplateVersionVariable(variable database.TemplateVersionVariable) c Required: variable.Required, Sensitive: variable.Sensitive, } + if templateVariable.Sensitive { + templateVariable.Value = redacted + templateVariable.DefaultValue = redacted + } + return templateVariable } func watchTemplateChannel(id uuid.UUID) string { diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 30c766d582736..9e7a7d46bda9f 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -1132,7 +1132,6 @@ func TestTemplateVersionVariables(t *testing.T) { Description: "This is the first variable", Type: "string", Required: true, - Sensitive: true, }, } const firstVariableValue = "foobar" @@ -1180,7 +1179,6 @@ func TestTemplateVersionVariables(t *testing.T) { Description: "This is the first variable", Type: "string", Required: true, - Sensitive: true, }, { Name: "second_variable", @@ -1218,4 +1216,52 @@ func TestTemplateVersionVariables(t *testing.T) { require.Equal(t, "", actualVariables[0].Value) require.Equal(t, templateVariables[1].DefaultValue, actualVariables[1].Value) }) + + t.Run("Redact sensitive variables", func(t *testing.T) { + t.Parallel() + + templateVariables := []*proto.TemplateVariable{ + { + Name: "first_variable", + Description: "This is the first variable", + Type: "string", + Required: true, + Sensitive: true, + }, + } + const firstVariableValue = "foobar" + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, + createEchoResponses(templateVariables), + func(ctvr *codersdk.CreateTemplateVersionRequest) { + ctvr.UserVariableValues = []codersdk.VariableValue{ + { + Name: templateVariables[0].Name, + Value: firstVariableValue, + }, + } + }, + ) + templateVersion := coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + + // As user passed the value for the first parameter, the job will succeed. + require.Empty(t, templateVersion.Job.Error) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + actualVariables, err := client.TemplateVersionVariables(ctx, templateVersion.ID) + require.NoError(t, err) + + require.Len(t, actualVariables, 1) + require.Equal(t, templateVariables[0].Name, actualVariables[0].Name) + require.Equal(t, templateVariables[0].Description, actualVariables[0].Description) + require.Equal(t, templateVariables[0].Type, actualVariables[0].Type) + require.Equal(t, templateVariables[0].Required, actualVariables[0].Required) + require.Equal(t, templateVariables[0].Sensitive, actualVariables[0].Sensitive) + require.Equal(t, "*redacted*", actualVariables[0].DefaultValue) + require.Equal(t, "*redacted*", actualVariables[0].Value) + }) } diff --git a/docs/cli/coder_templates_create.md b/docs/cli/coder_templates_create.md index 7a1b8654e017c..67b1df12f89f9 100644 --- a/docs/cli/coder_templates_create.md +++ b/docs/cli/coder_templates_create.md @@ -14,6 +14,8 @@ coder templates create [name] [flags] -h, --help help for create --parameter-file string Specify a file path with parameter values. --provisioner-tag stringArray Specify a set of tags to target provisioner daemons. + --variable stringArray Specify a set of values for Terraform-managed variables. + --variables-file string Specify a file path with values for Terraform-managed variables. -y, --yes Bypass prompts ``` diff --git a/docs/cli/coder_templates_push.md b/docs/cli/coder_templates_push.md index eaa88a95bec32..2d4e5ef5c2beb 100644 --- a/docs/cli/coder_templates_push.md +++ b/docs/cli/coder_templates_push.md @@ -15,7 +15,8 @@ coder templates push [template] [flags] --name string Specify a name for the new template version. It will be automatically generated if not provided. --parameter-file string Specify a file path with parameter values. --provisioner-tag stringArray Specify a set of tags to target provisioner daemons. - --values-file string Specify a file path with values for managed variables. + --variable stringArray Specify a set of values for Terraform-managed variables. + --variables-file string Specify a file path with values for Terraform-managed variables. -y, --yes Bypass prompts ``` diff --git a/provisioner/terraform/parse.go b/provisioner/terraform/parse.go index 768241d260531..413cc9e522de6 100644 --- a/provisioner/terraform/parse.go +++ b/provisioner/terraform/parse.go @@ -225,8 +225,9 @@ func convertTerraformVariableToManagedVariable(variable *tfconfig.Variable) (*pr Description: variable.Description, Type: variable.Type, DefaultValue: defaultData, - Required: defaultData == "", // variable.Required is always false? - Sensitive: variable.Sensitive, + // variable.Required is always false. Empty string is a valid default value, so it doesn't enforce required to be "true". + Required: variable.Default == nil, + Sensitive: variable.Sensitive, }, nil } diff --git a/provisioner/terraform/parse_test.go b/provisioner/terraform/parse_test.go index 16d2acea40152..e13a25935cbb0 100644 --- a/provisioner/terraform/parse_test.go +++ b/provisioner/terraform/parse_test.go @@ -223,6 +223,37 @@ func TestParse(t *testing.T) { }, }, }, + { + Name: "enable-managed-variables-with-default-empty-string", + Files: map[string]string{ + "main.tf": `variable "A" { + description = "Testing!" + type = string + default = "" + sensitive = true + } + + provider "coder" { + feature_use_managed_variables = true + }`, + }, + Response: &proto.Parse_Response{ + Type: &proto.Parse_Response_Complete{ + Complete: &proto.Parse_Complete{ + TemplateVariables: []*proto.TemplateVariable{ + { + Name: "A", + Description: "Testing!", + Type: "string", + DefaultValue: "", + Required: false, + Sensitive: true, + }, + }, + }, + }, + }, + }, { Name: "enable-managed-variables-without-default", Files: map[string]string{ @@ -241,11 +272,12 @@ func TestParse(t *testing.T) { Complete: &proto.Parse_Complete{ TemplateVariables: []*proto.TemplateVariable{ { - Name: "A", - Description: "Testing!", - Type: "string", - Required: true, - Sensitive: true, + Name: "A", + Description: "Testing!", + Type: "string", + DefaultValue: "", + Required: true, + Sensitive: true, }, }, },