From ed33a1c738c29c363bc1fdea39120cf0c0dc89f1 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 14 Jun 2022 10:45:59 -0500 Subject: [PATCH 01/16] WIP: feat: Update templates also updates parameters --- cli/templatecreate.go | 37 +++++++++++++++---- cli/templateupdate.go | 86 +++++++++++++++++++++++++++++++------------ coderd/templates.go | 4 +- codersdk/templates.go | 3 +- 4 files changed, 95 insertions(+), 35 deletions(-) diff --git a/cli/templatecreate.go b/cli/templatecreate.go index 936c574b57396..afd7b87d051ab 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -8,6 +8,8 @@ import ( "strings" "time" + "github.com/google/uuid" + "github.com/briandowns/spinner" "github.com/spf13/cobra" "golang.org/x/xerrors" @@ -82,7 +84,13 @@ func templateCreate() *cobra.Command { } spin.Stop() - job, parameters, err := createValidTemplateVersion(cmd, client, organization, database.ProvisionerType(provisioner), resp.Hash, parameterFile) + job, parameters, err := createValidTemplateVersion(cmd, createValidTemplateVersionArgs{ + Client: client, + Organization: organization, + Provisioner: database.ProvisionerType(provisioner), + FileHash: resp.Hash, + ParameterFile: parameterFile, + }) if err != nil { return err } @@ -133,13 +141,26 @@ func templateCreate() *cobra.Command { return cmd } -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) { +type createValidTemplateVersionArgs struct { + Client *codersdk.Client + Organization codersdk.Organization + Provisioner database.ProvisionerType + FileHash string + ParameterFile string + // TemplateID is only required if updating a template's active version. + TemplateID uuid.UUID +} + +func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVersionArgs, parameters ...codersdk.CreateParameterRequest) (*codersdk.TemplateVersion, []codersdk.CreateParameterRequest, error) { before := time.Now() - version, err := client.CreateTemplateVersion(cmd.Context(), organization.ID, codersdk.CreateTemplateVersionRequest{ + client := args.Client + + version, err := client.CreateTemplateVersion(cmd.Context(), args.Organization.ID, codersdk.CreateTemplateVersionRequest{ StorageMethod: codersdk.ProvisionerStorageMethodFile, - StorageSource: hash, - Provisioner: codersdk.ProvisionerType(provisioner), + StorageSource: args.FileHash, + Provisioner: codersdk.ProvisionerType(args.Provisioner), ParameterValues: parameters, + TemplateID: args.TemplateID, }) if err != nil { return nil, nil, err @@ -195,9 +216,9 @@ func createValidTemplateVersion(cmd *cobra.Command, client *codersdk.Client, org // parameterMapFromFile can be nil if parameter file is not specified var parameterMapFromFile map[string]string - if parameterFile != "" { + if args.ParameterFile != "" { _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("Attempting to read the variables from the parameter file.")+"\r\n") - parameterMapFromFile, err = createParameterMapFromFile(parameterFile) + parameterMapFromFile, err = createParameterMapFromFile(args.ParameterFile) if err != nil { return nil, nil, err } @@ -218,7 +239,7 @@ func createValidTemplateVersion(cmd *cobra.Command, client *codersdk.Client, org // 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...) + return createValidTemplateVersion(cmd, args, parameters...) } if version.Job.Status != codersdk.ProvisionerJobSucceeded { diff --git a/cli/templateupdate.go b/cli/templateupdate.go index 80276fd9d66c9..f6f1e2bc35498 100644 --- a/cli/templateupdate.go +++ b/cli/templateupdate.go @@ -5,9 +5,12 @@ import ( "os" "time" + "golang.org/x/xerrors" + + "github.com/coder/coder/coderd/database" + "github.com/briandowns/spinner" "github.com/spf13/cobra" - "golang.org/x/xerrors" "github.com/coder/coder/cli/cliui" "github.com/coder/coder/codersdk" @@ -16,8 +19,9 @@ import ( func templateUpdate() *cobra.Command { var ( - directory string - provisioner string + directory string + provisioner string + parameterFile string ) cmd := &cobra.Command{ @@ -64,42 +68,75 @@ func templateUpdate() *cobra.Command { } spin.Stop() - before := time.Now() - templateVersion, err := client.CreateTemplateVersion(cmd.Context(), organization.ID, codersdk.CreateTemplateVersionRequest{ + //before := time.Now() + + job, parameters, err := createValidTemplateVersion(cmd, createValidTemplateVersionArgs{ + Client: client, + Organization: organization, + Provisioner: database.ProvisionerType(provisioner), + FileHash: resp.Hash, + ParameterFile: parameterFile, TemplateID: template.ID, - StorageMethod: codersdk.ProvisionerStorageMethodFile, - StorageSource: resp.Hash, - Provisioner: codersdk.ProvisionerType(provisioner), }) if err != nil { return err } - logs, err := client.TemplateVersionLogsAfter(cmd.Context(), templateVersion.ID, before) - if err != nil { - return err - } - for { - log, ok := <-logs - if !ok { - break - } - _, _ = fmt.Printf("%s (%s): %s\n", provisioner, log.Level, log.Output) + + if job.Job.Status != codersdk.ProvisionerJobSucceeded { + return xerrors.Errorf("job failed: %s", job.Job.Status) } - templateVersion, err = client.TemplateVersion(cmd.Context(), templateVersion.ID) + + _, err = cliui.Prompt(cmd, cliui.PromptOptions{ + Text: "Confirm create?", + IsConfirm: true, + }) if err != nil { return err } - if templateVersion.Job.Status != codersdk.ProvisionerJobSucceeded { - return xerrors.Errorf("job failed: %s", templateVersion.Job.Error) - } - err = client.UpdateActiveTemplateVersion(cmd.Context(), template.ID, codersdk.UpdateActiveTemplateVersion{ - ID: templateVersion.ID, + ID: job.ID, + ParameterValues: parameters, }) if err != nil { return err } + + //templateVersion, err := client.CreateTemplateVersion(cmd.Context(), organization.ID, codersdk.CreateTemplateVersionRequest{ + // TemplateID: template.ID, + // StorageMethod: codersdk.ProvisionerStorageMethodFile, + // StorageSource: resp.Hash, + // Provisioner: codersdk.ProvisionerType(provisioner), + //}) + //if err != nil { + // return xerrors.Errorf("create template: %w", err) + //} + //logs, err := client.TemplateVersionLogsAfter(cmd.Context(), templateVersion.ID, before) + //if err != nil { + // return err + //} + //for { + // log, ok := <-logs + // if !ok { + // break + // } + // _, _ = fmt.Printf("%s (%s): %s\n", provisioner, log.Level, log.Output) + //} + //templateVersion, err = client.TemplateVersion(cmd.Context(), templateVersion.ID) + //if err != nil { + // return err + //} + // + //if templateVersion.Job.Status != codersdk.ProvisionerJobSucceeded { + // return xerrors.Errorf("job failed: %s", templateVersion.Job.Error) + //} + // + //err = client.UpdateActiveTemplateVersion(cmd.Context(), template.ID, codersdk.UpdateActiveTemplateVersion{ + // ID: templateVersion.ID, + //}) + //if err != nil { + // return err + //} _, _ = fmt.Printf("Updated version!\n") return nil }, @@ -108,6 +145,7 @@ func templateUpdate() *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.") cliui.AllowSkipPrompt(cmd) // This is for testing! err := cmd.Flags().MarkHidden("test.provisioner") diff --git a/coderd/templates.go b/coderd/templates.go index c24ccf544465e..1ac0cd7afdae1 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -209,8 +209,8 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque Name: parameterValue.Name, CreatedAt: database.Now(), UpdatedAt: database.Now(), - Scope: database.ParameterScopeTemplate, - ScopeID: dbTemplate.ID, + Scope: database.ParameterScopeImportJob, + ScopeID: templateVersion.JobID, SourceScheme: database.ParameterSourceScheme(parameterValue.SourceScheme), SourceValue: parameterValue.SourceValue, DestinationScheme: database.ParameterDestinationScheme(parameterValue.DestinationScheme), diff --git a/codersdk/templates.go b/codersdk/templates.go index 266de9305c905..a8c3c2f472c80 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -30,7 +30,8 @@ type Template struct { } type UpdateActiveTemplateVersion struct { - ID uuid.UUID `json:"id" validate:"required"` + ID uuid.UUID `json:"id" validate:"required"` + ParameterValues []CreateParameterRequest `json:"parameter_values,omitempty"` } type UpdateTemplateMeta struct { From 1f2892c334d913b6dad5cbf0cd50a1febc238302 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 15 Jun 2022 10:07:49 -0500 Subject: [PATCH 02/16] Insert params for template version update --- coderd/templateversions.go | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 64345457e969c..e2b910e49a9f3 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -545,9 +545,33 @@ func (api *API) patchActiveTemplateVersion(rw http.ResponseWriter, r *http.Reque }) return } - err = api.Database.UpdateTemplateActiveVersionByID(r.Context(), database.UpdateTemplateActiveVersionByIDParams{ - ID: template.ID, - ActiveVersionID: req.ID, + + err = api.Database.InTx(func(store database.Store) error { + for _, parameterValue := range req.ParameterValues { + _, err = store.InsertParameterValue(r.Context(), database.InsertParameterValueParams{ + ID: uuid.New(), + Name: parameterValue.Name, + CreatedAt: database.Now(), + UpdatedAt: database.Now(), + Scope: database.ParameterScopeImportJob, + ScopeID: version.JobID, + SourceScheme: database.ParameterSourceScheme(parameterValue.SourceScheme), + SourceValue: parameterValue.SourceValue, + DestinationScheme: database.ParameterDestinationScheme(parameterValue.DestinationScheme), + }) + if err != nil { + return xerrors.Errorf("insert parameter value: %w", err) + } + } + + err = store.UpdateTemplateActiveVersionByID(r.Context(), database.UpdateTemplateActiveVersionByIDParams{ + ID: template.ID, + ActiveVersionID: req.ID, + }) + if err != nil { + return xerrors.Errorf("update active version: %w", err) + } + return nil }) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ From 8d62c96685d75a9d9c3e3ecbf49f27087b0a30fd Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 15 Jun 2022 11:20:00 -0500 Subject: [PATCH 03/16] feat: Implement parameters list - Allow more columns on template list --- cli/parameters.go | 47 +++++++++++++++++++++++++++ cli/parameterslist.go | 73 ++++++++++++++++++++++++++++++++++++++++++ cli/root.go | 1 + cli/templatelist.go | 27 +++++----------- cli/templates.go | 40 ++++++++++++++++++++++- cli/users.go | 2 +- codersdk/parameters.go | 5 +-- 7 files changed, 172 insertions(+), 23 deletions(-) create mode 100644 cli/parameters.go create mode 100644 cli/parameterslist.go diff --git a/cli/parameters.go b/cli/parameters.go new file mode 100644 index 0000000000000..3b6e80bde0c22 --- /dev/null +++ b/cli/parameters.go @@ -0,0 +1,47 @@ +package cli + +import ( + "github.com/coder/coder/cli/cliui" + "github.com/coder/coder/codersdk" + "github.com/jedib0t/go-pretty/v6/table" + "github.com/spf13/cobra" +) + +func parameters() *cobra.Command { + cmd := &cobra.Command{ + Short: "List parameters for a given scope", + Use: "parameters", + Aliases: []string{"params"}, + } + cmd.AddCommand( + parameterList(), + ) + return cmd +} + +// displayParameters will return a table displaying all parameters passed in. +// filterColumns must be a subset of the parameter fields and will determine which +// columns to display +func displayParameters(filterColumns []string, params ...codersdk.Parameter) string { + tableWriter := cliui.Table() + header := table.Row{"id", "scope", "scope id", "name", "source scheme", "destination scheme", "created at", "updated at"} + tableWriter.AppendHeader(header) + tableWriter.SetColumnConfigs(cliui.FilterTableColumns(header, filterColumns)) + tableWriter.SortBy([]table.SortBy{{ + Name: "name", + }}) + for _, param := range params { + //fmt.Println(param, filterColumns) + tableWriter.AppendRow(table.Row{ + param.ID.String(), + param.Scope, + param.ScopeID.String(), + param.Name, + param.SourceScheme, + param.DestinationScheme, + param.CreatedAt, + param.UpdatedAt, + }) + } + return tableWriter.Render() +} diff --git a/cli/parameterslist.go b/cli/parameterslist.go new file mode 100644 index 0000000000000..ad467b6558c72 --- /dev/null +++ b/cli/parameterslist.go @@ -0,0 +1,73 @@ +package cli + +import ( + "fmt" + + "github.com/google/uuid" + "golang.org/x/xerrors" + + "github.com/coder/coder/codersdk" + "github.com/spf13/cobra" +) + +func parameterList() *cobra.Command { + var ( + columns []string + ) + cmd := &cobra.Command{ + Use: "list", + Aliases: []string{"ls"}, + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + scope, name := args[0], args[1] + + client, err := createClient(cmd) + if err != nil { + return err + } + + organization, err := currentOrganization(cmd, client) + if err != nil { + return xerrors.Errorf("get current organization: %w", err) + } + + var scopeID uuid.UUID + switch codersdk.ParameterScope(scope) { + case codersdk.ParameterWorkspace: + workspace, err := namedWorkspace(cmd, client, name) + if err != nil { + return err + } + scopeID = workspace.ID + case codersdk.ParameterTemplate: + template, err := client.TemplateByName(cmd.Context(), organization.ID, name) + if err != nil { + return xerrors.Errorf("get workspace template: %w", err) + } + scopeID = template.ID + + case codersdk.ParameterScopeImportJob, "template_version": + scope = string(codersdk.ParameterScopeImportJob) + scopeID, err = uuid.Parse(name) + if err != nil { + return xerrors.Errorf("%q must be a uuid for this scope type", name) + } + default: + return xerrors.Errorf("%q is an unsupported scope, use %v", scope, []codersdk.ParameterScope{ + codersdk.ParameterWorkspace, codersdk.ParameterTemplate, codersdk.ParameterScopeImportJob, + }) + } + + params, err := client.Parameters(cmd.Context(), codersdk.ParameterScope(args[0]), scopeID) + if err != nil { + return xerrors.Errorf("fetch params: %w", err) + } + + _, err = fmt.Fprintln(cmd.OutOrStdout(), displayParameters(columns, params...)) + return err + }, + } + cmd.Flags().StringArrayVarP(&columns, "column", "c", []string{"name", "source_scheme", "destination_scheme"}, + "Specify a column to filter in the table.") + return cmd +} diff --git a/cli/root.go b/cli/root.go index cb59816fe10ec..087f052a2a377 100644 --- a/cli/root.go +++ b/cli/root.go @@ -90,6 +90,7 @@ func Root() *cobra.Command { portForward(), workspaceAgent(), versionCmd(), + parameters(), ) cmd.SetUsageTemplate(usageTemplate()) diff --git a/cli/templatelist.go b/cli/templatelist.go index 41760599e415a..a8a1315a1f0c9 100644 --- a/cli/templatelist.go +++ b/cli/templatelist.go @@ -4,14 +4,14 @@ import ( "fmt" "github.com/fatih/color" - "github.com/jedib0t/go-pretty/v6/table" "github.com/spf13/cobra" - - "github.com/coder/coder/cli/cliui" ) func templateList() *cobra.Command { - return &cobra.Command{ + var ( + columns []string + ) + cmd := &cobra.Command{ Use: "list", Aliases: []string{"ls"}, RunE: func(cmd *cobra.Command, args []string) error { @@ -34,22 +34,11 @@ func templateList() *cobra.Command { return nil } - tableWriter := cliui.Table() - tableWriter.AppendHeader(table.Row{"Name", "Last updated", "Used by"}) - - for _, template := range templates { - suffix := "" - if template.WorkspaceOwnerCount != 1 { - suffix = "s" - } - tableWriter.AppendRow(table.Row{ - template.Name, - template.UpdatedAt.Format("January 2, 2006"), - cliui.Styles.Fuschia.Render(fmt.Sprintf("%d developer%s", template.WorkspaceOwnerCount, suffix)), - }) - } - _, err = fmt.Fprintln(cmd.OutOrStdout(), tableWriter.Render()) + _, err = fmt.Fprintln(cmd.OutOrStdout(), displayTemplates(columns, templates...)) return err }, } + cmd.Flags().StringArrayVarP(&columns, "column", "c", []string{"name", "last_updated", "used_by"}, + "Specify a column to filter in the table.") + return cmd } diff --git a/cli/templates.go b/cli/templates.go index c737bb6c134c0..769e02161965d 100644 --- a/cli/templates.go +++ b/cli/templates.go @@ -1,9 +1,14 @@ package cli import ( + "fmt" + "time" + + "github.com/jedib0t/go-pretty/v6/table" "github.com/spf13/cobra" "github.com/coder/coder/cli/cliui" + "github.com/coder/coder/codersdk" ) func templates() *cobra.Command { @@ -17,7 +22,7 @@ func templates() *cobra.Command { ` + cliui.Styles.Code.Render("$ coder templates create") + ` - Make changes to your template, and plan the changes - + ` + cliui.Styles.Code.Render("$ coder templates plan ") + ` - Update the template. Your developers can update their workspaces @@ -37,3 +42,36 @@ func templates() *cobra.Command { return cmd } + +// displayTemplates will return a table displaying all templates passed in. +// filterColumns must be a subset of the template fields and will determine which +// columns to display +func displayTemplates(filterColumns []string, templates ...codersdk.Template) string { + tableWriter := cliui.Table() + header := table.Row{ + "Name", "Created At", "Last Updated", "Organization ID", "Provisioner", + "Active Version ID", "Used By", "Max TTL", "Min Autostart"} + tableWriter.AppendHeader(header) + tableWriter.SetColumnConfigs(cliui.FilterTableColumns(header, filterColumns)) + tableWriter.SortBy([]table.SortBy{{ + Name: "name", + }}) + for _, template := range templates { + suffix := "" + if template.WorkspaceOwnerCount != 1 { + suffix = "s" + } + tableWriter.AppendRow(table.Row{ + template.Name, + template.CreatedAt.Format("January 2, 2006"), + template.UpdatedAt.Format("January 2, 2006"), + template.OrganizationID.String(), + template.Provisioner, + template.ActiveVersionID.String(), + cliui.Styles.Fuschia.Render(fmt.Sprintf("%d developer%s", template.WorkspaceOwnerCount, suffix)), + fmt.Sprintf("%s", time.Duration(template.MaxTTLMillis)*time.Millisecond), + fmt.Sprintf("%s", time.Duration(template.MinAutostartIntervalMillis)*time.Millisecond), + }) + } + return tableWriter.Render() +} diff --git a/cli/users.go b/cli/users.go index 3bbb3067fbd92..80df42ebf6787 100644 --- a/cli/users.go +++ b/cli/users.go @@ -30,7 +30,7 @@ func users() *cobra.Command { // columns to display func displayUsers(filterColumns []string, users ...codersdk.User) string { tableWriter := cliui.Table() - header := table.Row{"id", "username", "email", "created_at", "status"} + header := table.Row{"id", "username", "email", "created at", "status"} tableWriter.AppendHeader(header) tableWriter.SetColumnConfigs(cliui.FilterTableColumns(header, filterColumns)) tableWriter.SortBy([]table.SortBy{{ diff --git a/codersdk/parameters.go b/codersdk/parameters.go index 073efafb63fbb..d1e8869c59514 100644 --- a/codersdk/parameters.go +++ b/codersdk/parameters.go @@ -14,8 +14,9 @@ import ( type ParameterScope string const ( - ParameterTemplate ParameterScope = "template" - ParameterWorkspace ParameterScope = "workspace" + ParameterTemplate ParameterScope = "template" + ParameterWorkspace ParameterScope = "workspace" + ParameterScopeImportJob ParameterScope = "import_job" ) type ParameterSourceScheme string From aceca623a9b8604ea27c009bf399962edd781e43 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 15 Jun 2022 18:28:09 -0500 Subject: [PATCH 04/16] Working implementation of inherited params --- cli/parameterslist.go | 6 +- cli/templatecreate.go | 32 ++++ cli/templateupdate.go | 7 +- coderd/database/databasefake/databasefake.go | 40 ++++- coderd/database/querier.go | 3 +- coderd/database/queries.sql.go | 145 +++++++++++++------ coderd/database/queries/parametervalues.sql | 32 +++- coderd/parameter/compute.go | 22 +-- coderd/parameters.go | 17 +-- coderd/templateversions.go | 64 +++++--- codersdk/parameters.go | 13 +- codersdk/templates.go | 3 +- scripts/develop.sh | 2 +- site/src/api/typesGenerated.ts | 15 +- 14 files changed, 287 insertions(+), 114 deletions(-) diff --git a/cli/parameterslist.go b/cli/parameterslist.go index ad467b6558c72..b21c8dd79eddd 100644 --- a/cli/parameterslist.go +++ b/cli/parameterslist.go @@ -46,15 +46,15 @@ func parameterList() *cobra.Command { } scopeID = template.ID - case codersdk.ParameterScopeImportJob, "template_version": - scope = string(codersdk.ParameterScopeImportJob) + case codersdk.ParameterImportJob, "template_version": + scope = string(codersdk.ParameterImportJob) scopeID, err = uuid.Parse(name) if err != nil { return xerrors.Errorf("%q must be a uuid for this scope type", name) } default: return xerrors.Errorf("%q is an unsupported scope, use %v", scope, []codersdk.ParameterScope{ - codersdk.ParameterWorkspace, codersdk.ParameterTemplate, codersdk.ParameterScopeImportJob, + codersdk.ParameterWorkspace, codersdk.ParameterTemplate, codersdk.ParameterImportJob, }) } diff --git a/cli/templatecreate.go b/cli/templatecreate.go index afd7b87d051ab..53d46c724199e 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -196,11 +196,37 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers return nil, nil, err } + // lastParameterValues are pulled from the current active template version if + // templateID is provided. This allows pulling params from the last + // version if we are updating template versions. + lastParameterValues := make(map[string]codersdk.Parameter) + if args.TemplateID != uuid.Nil { + template, err := client.Template(cmd.Context(), args.TemplateID) + if err != nil { + return nil, nil, xerrors.Errorf("Fetch template: %w", err) + } + + activeVersion, err := client.TemplateVersion(cmd.Context(), template.ActiveVersionID) + if err != nil { + return nil, nil, xerrors.Errorf("Fetch current active template version: %w", err) + } + + // We don't want to compute the params, we only want to copy from this scope + values, err := client.Parameters(cmd.Context(), codersdk.ParameterImportJob, activeVersion.Job.ID) + if err != nil { + return nil, nil, xerrors.Errorf("Fetch previous version parameters: %w", err) + } + for _, value := range values { + lastParameterValues[value.Name] = value + } + } + if provisionerd.IsMissingParameterError(version.Job.Error) { valuesBySchemaID := map[string]codersdk.TemplateVersionParameter{} for _, parameterValue := range parameterValues { valuesBySchemaID[parameterValue.SchemaID.String()] = parameterValue } + sort.Slice(parameterSchemas, func(i, j int) bool { return parameterSchemas[i].Name < parameterSchemas[j].Name }) @@ -224,6 +250,12 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers } } for _, parameterSchema := range missingSchemas { + if inherit, ok := lastParameterValues[parameterSchema.Name]; ok { + parameters = append(parameters, codersdk.CreateParameterRequest{ + CopyFromParameter: inherit.ID, + }) + continue + } parameterValue, err := getParameterValueFromMapOrInput(cmd, parameterMapFromFile, parameterSchema) if err != nil { return nil, nil, err diff --git a/cli/templateupdate.go b/cli/templateupdate.go index f6f1e2bc35498..802115a0ea80d 100644 --- a/cli/templateupdate.go +++ b/cli/templateupdate.go @@ -68,9 +68,7 @@ func templateUpdate() *cobra.Command { } spin.Stop() - //before := time.Now() - - job, parameters, err := createValidTemplateVersion(cmd, createValidTemplateVersionArgs{ + job, _, err := createValidTemplateVersion(cmd, createValidTemplateVersionArgs{ Client: client, Organization: organization, Provisioner: database.ProvisionerType(provisioner), @@ -95,8 +93,7 @@ func templateUpdate() *cobra.Command { } err = client.UpdateActiveTemplateVersion(cmd.Context(), template.ID, codersdk.UpdateActiveTemplateVersion{ - ID: job.ID, - ParameterValues: parameters, + ID: job.ID, }) if err != nil { return err diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index fcc214a5745b7..b825d502437c7 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -101,6 +101,20 @@ func (q *fakeQuerier) AcquireProvisionerJob(_ context.Context, arg database.Acqu return database.ProvisionerJob{}, sql.ErrNoRows } +func (q *fakeQuerier) ParameterValue(_ context.Context, id uuid.UUID) (database.ParameterValue, error) { + q.mutex.Lock() + defer q.mutex.Unlock() + + for _, parameterValue := range q.parameterValues { + if parameterValue.ID.String() != id.String() { + continue + } + return parameterValue, nil + + } + return database.ParameterValue{}, sql.ErrNoRows +} + func (q *fakeQuerier) DeleteParameterValueByID(_ context.Context, id uuid.UUID) error { q.mutex.Lock() defer q.mutex.Unlock() @@ -716,17 +730,26 @@ func (q *fakeQuerier) GetOrganizationsByUserID(_ context.Context, userID uuid.UU return organizations, nil } -func (q *fakeQuerier) GetParameterValuesByScope(_ context.Context, arg database.GetParameterValuesByScopeParams) ([]database.ParameterValue, error) { +func (q *fakeQuerier) ParameterValues(_ context.Context, arg database.ParameterValuesParams) ([]database.ParameterValue, error) { q.mutex.RLock() defer q.mutex.RUnlock() parameterValues := make([]database.ParameterValue, 0) for _, parameterValue := range q.parameterValues { - if parameterValue.Scope != arg.Scope { + if arg.Scope != "" && parameterValue.Scope != arg.Scope { continue } - if parameterValue.ScopeID != arg.ScopeID { - continue + + if len(arg.ScopeIds) > 0 { + if !contains(arg.ScopeIds, parameterValue.ScopeID) { + continue + } + } + + if len(arg.Ids) > 0 { + if !contains(arg.Ids, parameterValue.ID) { + continue + } } parameterValues = append(parameterValues, parameterValue) } @@ -1980,3 +2003,12 @@ func (q *fakeQuerier) InsertAuditLog(_ context.Context, arg database.InsertAudit return alog, nil } + +func contains[T comparable](haystack []T, needle T) bool { + for _, hay := range haystack { + if needle == hay { + return true + } + } + return false +} diff --git a/coderd/database/querier.go b/coderd/database/querier.go index c3f57d3a9f795..fe5c0a5e8c259 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -41,7 +41,6 @@ type querier interface { GetOrganizationsByUserID(ctx context.Context, userID uuid.UUID) ([]Organization, error) GetParameterSchemasByJobID(ctx context.Context, jobID uuid.UUID) ([]ParameterSchema, error) GetParameterValueByScopeAndName(ctx context.Context, arg GetParameterValueByScopeAndNameParams) (ParameterValue, error) - GetParameterValuesByScope(ctx context.Context, arg GetParameterValuesByScopeParams) ([]ParameterValue, error) GetProvisionerDaemonByID(ctx context.Context, id uuid.UUID) (ProvisionerDaemon, error) GetProvisionerDaemons(ctx context.Context) ([]ProvisionerDaemon, error) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (ProvisionerJob, error) @@ -100,6 +99,8 @@ type querier interface { InsertWorkspaceApp(ctx context.Context, arg InsertWorkspaceAppParams) (WorkspaceApp, error) InsertWorkspaceBuild(ctx context.Context, arg InsertWorkspaceBuildParams) (WorkspaceBuild, error) InsertWorkspaceResource(ctx context.Context, arg InsertWorkspaceResourceParams) (WorkspaceResource, error) + ParameterValue(ctx context.Context, id uuid.UUID) (ParameterValue, error) + ParameterValues(ctx context.Context, arg ParameterValuesParams) ([]ParameterValue, error) UpdateAPIKeyByID(ctx context.Context, arg UpdateAPIKeyByIDParams) error UpdateGitSSHKey(ctx context.Context, arg UpdateGitSSHKeyParams) error UpdateMemberRoles(ctx context.Context, arg UpdateMemberRolesParams) (OrganizationMember, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 38afa4aa40694..69d6ebefc5d17 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -994,54 +994,6 @@ func (q *sqlQuerier) GetParameterValueByScopeAndName(ctx context.Context, arg Ge return i, err } -const getParameterValuesByScope = `-- name: GetParameterValuesByScope :many -SELECT - id, created_at, updated_at, scope, scope_id, name, source_scheme, source_value, destination_scheme -FROM - parameter_values -WHERE - scope = $1 - AND scope_id = $2 -` - -type GetParameterValuesByScopeParams struct { - Scope ParameterScope `db:"scope" json:"scope"` - ScopeID uuid.UUID `db:"scope_id" json:"scope_id"` -} - -func (q *sqlQuerier) GetParameterValuesByScope(ctx context.Context, arg GetParameterValuesByScopeParams) ([]ParameterValue, error) { - rows, err := q.db.QueryContext(ctx, getParameterValuesByScope, arg.Scope, arg.ScopeID) - if err != nil { - return nil, err - } - defer rows.Close() - var items []ParameterValue - for rows.Next() { - var i ParameterValue - if err := rows.Scan( - &i.ID, - &i.CreatedAt, - &i.UpdatedAt, - &i.Scope, - &i.ScopeID, - &i.Name, - &i.SourceScheme, - &i.SourceValue, - &i.DestinationScheme, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - const insertParameterValue = `-- name: InsertParameterValue :one INSERT INTO parameter_values ( @@ -1098,6 +1050,103 @@ func (q *sqlQuerier) InsertParameterValue(ctx context.Context, arg InsertParamet return i, err } +const parameterValue = `-- name: ParameterValue :one +SELECT id, created_at, updated_at, scope, scope_id, name, source_scheme, source_value, destination_scheme FROM + parameter_values +WHERE + id = $1 +` + +func (q *sqlQuerier) ParameterValue(ctx context.Context, id uuid.UUID) (ParameterValue, error) { + row := q.db.QueryRowContext(ctx, parameterValue, id) + var i ParameterValue + err := row.Scan( + &i.ID, + &i.CreatedAt, + &i.UpdatedAt, + &i.Scope, + &i.ScopeID, + &i.Name, + &i.SourceScheme, + &i.SourceValue, + &i.DestinationScheme, + ) + return i, err +} + +const parameterValues = `-- name: ParameterValues :many +SELECT + id, created_at, updated_at, scope, scope_id, name, source_scheme, source_value, destination_scheme +FROM + parameter_values +WHERE + CASE + WHEN $1 :: parameter_scope != '' THEN + scope = $1 + ELSE true + END + AND CASE + WHEN cardinality($2 :: uuid[]) > 0 THEN + scope_id = ANY($2 :: uuid[]) + ELSE true + END + AND CASE + WHEN cardinality($3 :: uuid[]) > 0 THEN + id = ANY($3 :: uuid[]) + ELSE true + END + AND CASE + WHEN cardinality($4 :: text[]) > 0 THEN + "name" = ANY($4 :: text[]) + ELSE true + END +` + +type ParameterValuesParams struct { + Scope ParameterScope `db:"scope" json:"scope"` + ScopeIds []uuid.UUID `db:"scope_ids" json:"scope_ids"` + Ids []uuid.UUID `db:"ids" json:"ids"` + Names []string `db:"names" json:"names"` +} + +func (q *sqlQuerier) ParameterValues(ctx context.Context, arg ParameterValuesParams) ([]ParameterValue, error) { + rows, err := q.db.QueryContext(ctx, parameterValues, + arg.Scope, + pq.Array(arg.ScopeIds), + pq.Array(arg.Ids), + pq.Array(arg.Names), + ) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ParameterValue + for rows.Next() { + var i ParameterValue + if err := rows.Scan( + &i.ID, + &i.CreatedAt, + &i.UpdatedAt, + &i.Scope, + &i.ScopeID, + &i.Name, + &i.SourceScheme, + &i.SourceValue, + &i.DestinationScheme, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getProvisionerDaemonByID = `-- name: GetProvisionerDaemonByID :one SELECT id, created_at, updated_at, name, provisioners diff --git a/coderd/database/queries/parametervalues.sql b/coderd/database/queries/parametervalues.sql index 5a6ea6ece2818..15b99789f51ab 100644 --- a/coderd/database/queries/parametervalues.sql +++ b/coderd/database/queries/parametervalues.sql @@ -1,17 +1,43 @@ +-- name: ParameterValue :one +SELECT * FROM + parameter_values +WHERE + id = $1; + + -- name: DeleteParameterValueByID :exec DELETE FROM parameter_values WHERE id = $1; --- name: GetParameterValuesByScope :many +-- name: ParameterValues :many SELECT * FROM parameter_values WHERE - scope = $1 - AND scope_id = $2; + CASE + WHEN @scope :: parameter_scope != '' THEN + scope = @scope + ELSE true + END + AND CASE + WHEN cardinality(@scope_ids :: uuid[]) > 0 THEN + scope_id = ANY(@scope_ids :: uuid[]) + ELSE true + END + AND CASE + WHEN cardinality(@ids :: uuid[]) > 0 THEN + id = ANY(@ids :: uuid[]) + ELSE true + END + AND CASE + WHEN cardinality(@names :: text[]) > 0 THEN + "name" = ANY(@names :: text[]) + ELSE true + END +; -- name: GetParameterValueByScopeAndName :one SELECT diff --git a/coderd/parameter/compute.go b/coderd/parameter/compute.go index 121cfa9ad4351..ba61ad21629f3 100644 --- a/coderd/parameter/compute.go +++ b/coderd/parameter/compute.go @@ -61,9 +61,9 @@ func Compute(ctx context.Context, db database.Store, scope ComputeScope, options } // Job parameters come second! - err = compute.injectScope(ctx, database.GetParameterValuesByScopeParams{ - Scope: database.ParameterScopeImportJob, - ScopeID: scope.TemplateImportJobID, + err = compute.injectScope(ctx, database.ParameterValuesParams{ + Scope: database.ParameterScopeImportJob, + ScopeIds: []uuid.UUID{scope.TemplateImportJobID}, }) if err != nil { return nil, err @@ -105,9 +105,9 @@ func Compute(ctx context.Context, db database.Store, scope ComputeScope, options if scope.TemplateID.Valid { // Template parameters come third! - err = compute.injectScope(ctx, database.GetParameterValuesByScopeParams{ - Scope: database.ParameterScopeTemplate, - ScopeID: scope.TemplateID.UUID, + err = compute.injectScope(ctx, database.ParameterValuesParams{ + Scope: database.ParameterScopeTemplate, + ScopeIds: []uuid.UUID{scope.TemplateID.UUID}, }) if err != nil { return nil, err @@ -116,9 +116,9 @@ func Compute(ctx context.Context, db database.Store, scope ComputeScope, options if scope.WorkspaceID.Valid { // Workspace parameters come last! - err = compute.injectScope(ctx, database.GetParameterValuesByScopeParams{ - Scope: database.ParameterScopeWorkspace, - ScopeID: scope.WorkspaceID.UUID, + err = compute.injectScope(ctx, database.ParameterValuesParams{ + Scope: database.ParameterScopeWorkspace, + ScopeIds: []uuid.UUID{scope.WorkspaceID.UUID}, }) if err != nil { return nil, err @@ -148,8 +148,8 @@ type compute struct { } // Validates and computes the value for parameters; setting the value on "parameterByName". -func (c *compute) injectScope(ctx context.Context, scopeParams database.GetParameterValuesByScopeParams) error { - scopedParameters, err := c.db.GetParameterValuesByScope(ctx, scopeParams) +func (c *compute) injectScope(ctx context.Context, scopeParams database.ParameterValuesParams) error { + scopedParameters, err := c.db.ParameterValues(ctx, scopeParams) if errors.Is(err, sql.ErrNoRows) { err = nil } diff --git a/coderd/parameters.go b/coderd/parameters.go index eeb8731d2562f..e653dc3d9f0cf 100644 --- a/coderd/parameters.go +++ b/coderd/parameters.go @@ -89,9 +89,9 @@ func (api *API) parameters(rw http.ResponseWriter, r *http.Request) { return } - parameterValues, err := api.Database.GetParameterValuesByScope(r.Context(), database.GetParameterValuesByScopeParams{ - Scope: scope, - ScopeID: scopeID, + parameterValues, err := api.Database.ParameterValues(r.Context(), database.ParameterValuesParams{ + Scope: scope, + ScopeIds: []uuid.UUID{scopeID}, }) if errors.Is(err, sql.ErrNoRows) { err = nil @@ -214,6 +214,8 @@ func (api *API) parameterRBACResource(rw http.ResponseWriter, r *http.Request, s switch scope { case database.ParameterScopeWorkspace: resource, err = api.Database.GetWorkspaceByID(ctx, scopeID) + case database.ParameterScopeImportJob: + resource, err = api.Database.GetTemplateVersionByJobID(ctx, scopeID) case database.ParameterScopeTemplate: resource, err = api.Database.GetTemplateByID(ctx, scopeID) default: @@ -235,12 +237,9 @@ func (api *API) parameterRBACResource(rw http.ResponseWriter, r *http.Request, s } func readScopeAndID(rw http.ResponseWriter, r *http.Request) (database.ParameterScope, uuid.UUID, bool) { - var scope database.ParameterScope - switch chi.URLParam(r, "scope") { - case string(codersdk.ParameterTemplate): - scope = database.ParameterScopeTemplate - case string(codersdk.ParameterWorkspace): - scope = database.ParameterScopeWorkspace + scope := database.ParameterScope(chi.URLParam(r, "scope")) + switch scope { + case database.ParameterScopeTemplate, database.ParameterScopeImportJob, database.ParameterScopeWorkspace: default: httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ Message: fmt.Sprintf("Invalid scope %q.", scope), diff --git a/coderd/templateversions.go b/coderd/templateversions.go index e2b910e49a9f3..dd348ca33b82c 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -547,23 +547,6 @@ func (api *API) patchActiveTemplateVersion(rw http.ResponseWriter, r *http.Reque } err = api.Database.InTx(func(store database.Store) error { - for _, parameterValue := range req.ParameterValues { - _, err = store.InsertParameterValue(r.Context(), database.InsertParameterValueParams{ - ID: uuid.New(), - Name: parameterValue.Name, - CreatedAt: database.Now(), - UpdatedAt: database.Now(), - Scope: database.ParameterScopeImportJob, - ScopeID: version.JobID, - SourceScheme: database.ParameterSourceScheme(parameterValue.SourceScheme), - SourceValue: parameterValue.SourceValue, - DestinationScheme: database.ParameterDestinationScheme(parameterValue.DestinationScheme), - }) - if err != nil { - return xerrors.Errorf("insert parameter value: %w", err) - } - } - err = store.UpdateTemplateActiveVersionByID(r.Context(), database.UpdateTemplateActiveVersionByIDParams{ ID: template.ID, ActiveVersionID: req.ID, @@ -639,7 +622,54 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht var provisionerJob database.ProvisionerJob err = api.Database.InTx(func(db database.Store) error { jobID := uuid.New() + inherits := make([]uuid.UUID, 0) for _, parameterValue := range req.ParameterValues { + if parameterValue.CopyFromParameter != uuid.Nil { + inherits = append(inherits, parameterValue.CopyFromParameter) + } + } + + // Expand inherited params + if len(inherits) > 0 { + if req.TemplateID == uuid.Nil { + return xerrors.Errorf("cannot inherit parameters if template_id is not set") + } + + inheritedParams, err := db.ParameterValues(r.Context(), database.ParameterValuesParams{ + Ids: inherits, + }) + if err != nil { + return xerrors.Errorf("fetch inherited params: %w", err) + } + for _, copy := range inheritedParams { + // This is a bit inefficient, as we make a new db call for each + // param. + version, err := db.GetTemplateVersionByJobID(r.Context(), copy.ScopeID) + if err != nil { + return xerrors.Errorf("fetch template version for param %q: %w", copy.Name, err) + } + if !version.TemplateID.Valid || version.TemplateID.UUID != req.TemplateID { + return xerrors.Errorf("cannot inherit parameters from other templates") + } + if copy.Scope != database.ParameterScopeImportJob { + return xerrors.Errorf("copy parameter scope is %q, must be %q", copy.Scope, database.ParameterScopeImportJob) + } + // Add the copied param to the list to process + req.ParameterValues = append(req.ParameterValues, codersdk.CreateParameterRequest{ + Name: copy.Name, + SourceValue: copy.SourceValue, + SourceScheme: codersdk.ParameterSourceScheme(copy.SourceScheme), + DestinationScheme: codersdk.ParameterDestinationScheme(copy.DestinationScheme), + }) + } + } + + for _, parameterValue := range req.ParameterValues { + if parameterValue.CopyFromParameter != uuid.Nil { + continue + + } + _, err = db.InsertParameterValue(r.Context(), database.InsertParameterValueParams{ ID: uuid.New(), Name: parameterValue.Name, diff --git a/codersdk/parameters.go b/codersdk/parameters.go index d1e8869c59514..3cf5dfbc4898f 100644 --- a/codersdk/parameters.go +++ b/codersdk/parameters.go @@ -14,9 +14,9 @@ import ( type ParameterScope string const ( - ParameterTemplate ParameterScope = "template" - ParameterWorkspace ParameterScope = "workspace" - ParameterScopeImportJob ParameterScope = "import_job" + ParameterTemplate ParameterScope = "template" + ParameterWorkspace ParameterScope = "workspace" + ParameterImportJob ParameterScope = "import_job" ) type ParameterSourceScheme string @@ -78,6 +78,13 @@ type ParameterSchema struct { // CreateParameterRequest is used to create a new parameter value for a scope. type CreateParameterRequest struct { + // CopyFromParameter allows copying the value of another parameter. + // The other param must share the same scope and scopeID for this to + // succeed. + // No other fields are required if using this, as all fields will be copied + // from the other parameter. + CopyFromParameter uuid.UUID `json:"copy_from_parameter,omitempty" validate:"uuid4"` + Name string `json:"name" validate:"required"` SourceValue string `json:"source_value" validate:"required"` SourceScheme ParameterSourceScheme `json:"source_scheme" validate:"oneof=data,required"` diff --git a/codersdk/templates.go b/codersdk/templates.go index a8c3c2f472c80..266de9305c905 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -30,8 +30,7 @@ type Template struct { } type UpdateActiveTemplateVersion struct { - ID uuid.UUID `json:"id" validate:"required"` - ParameterValues []CreateParameterRequest `json:"parameter_values,omitempty"` + ID uuid.UUID `json:"id" validate:"required"` } type UpdateTemplateMeta struct { diff --git a/scripts/develop.sh b/scripts/develop.sh index a46576d121747..ce84eca67ffb5 100755 --- a/scripts/develop.sh +++ b/scripts/develop.sh @@ -24,7 +24,7 @@ export CODER_DEV_ADMIN_PASSWORD=password trap 'kill 0' SIGINT CODERV2_HOST=http://127.0.0.1:3000 INSPECT_XSTATE=true yarn --cwd=./site dev & - go run -tags embed cmd/coder/main.go server --dev --tunnel=true & + CODER_PG_CONNECTION_URL=postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD:-postgres}@localhost:5432/${POSTGRES_DB:-postgres}?sslmode=disable go run -tags embed cmd/coder/main.go server --dev --tunnel=true & # Just a minor sleep to ensure the first user was created to make the member. sleep 2 diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 49ae60058a061..452437c1d6bac 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -49,8 +49,9 @@ export interface CreateOrganizationRequest { readonly name: string } -// From codersdk/parameters.go:79:6 +// From codersdk/parameters.go:80:6 export interface CreateParameterRequest { + readonly copy_from_parameter?: string readonly name: string readonly source_value: string readonly source_scheme: ParameterSourceScheme @@ -160,7 +161,7 @@ export interface Pagination { readonly offset?: number } -// From codersdk/parameters.go:44:6 +// From codersdk/parameters.go:45:6 export interface Parameter { readonly id: string readonly created_at: string @@ -172,7 +173,7 @@ export interface Parameter { readonly destination_scheme: ParameterDestinationScheme } -// From codersdk/parameters.go:55:6 +// From codersdk/parameters.go:56:6 export interface ParameterSchema { readonly id: string readonly created_at: string @@ -495,16 +496,16 @@ export type LogLevel = "debug" | "error" | "info" | "trace" | "warn" // From codersdk/provisionerdaemons.go:16:6 export type LogSource = "provisioner" | "provisioner_daemon" -// From codersdk/parameters.go:28:6 +// From codersdk/parameters.go:29:6 export type ParameterDestinationScheme = "environment_variable" | "none" | "provisioner_variable" // From codersdk/parameters.go:14:6 -export type ParameterScope = "template" | "workspace" +export type ParameterScope = "import_job" | "template" | "workspace" -// From codersdk/parameters.go:21:6 +// From codersdk/parameters.go:22:6 export type ParameterSourceScheme = "data" | "none" -// From codersdk/parameters.go:36:6 +// From codersdk/parameters.go:37:6 export type ParameterTypeSystem = "hcl" | "none" // From codersdk/provisionerdaemons.go:42:6 From e569f0f5a28374562b676688d424609e99bfe9f1 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 15 Jun 2022 18:31:10 -0500 Subject: [PATCH 05/16] Fix name of enum --- cli/parameterslist.go | 6 +++--- codersdk/parameters.go | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cli/parameterslist.go b/cli/parameterslist.go index 8955ba96ca0c3..26859f6c3b29e 100644 --- a/cli/parameterslist.go +++ b/cli/parameterslist.go @@ -46,15 +46,15 @@ func parameterList() *cobra.Command { } scopeID = template.ID - case codersdk.ParameterScopeImportJob, "template_version": - scope = string(codersdk.ParameterScopeImportJob) + case codersdk.ParameterImportJob, "template_version": + scope = string(codersdk.ParameterImportJob) scopeID, err = uuid.Parse(name) if err != nil { return xerrors.Errorf("%q must be a uuid for this scope type", name) } default: return xerrors.Errorf("%q is an unsupported scope, use %v", scope, []codersdk.ParameterScope{ - codersdk.ParameterWorkspace, codersdk.ParameterTemplate, codersdk.ParameterScopeImportJob, + codersdk.ParameterWorkspace, codersdk.ParameterTemplate, codersdk.ParameterImportJob, }) } diff --git a/codersdk/parameters.go b/codersdk/parameters.go index f405d58f844df..1bf46c5cfd1b5 100644 --- a/codersdk/parameters.go +++ b/codersdk/parameters.go @@ -14,9 +14,9 @@ import ( type ParameterScope string const ( - ParameterTemplate ParameterScope = "template" - ParameterWorkspace ParameterScope = "workspace" - ParameterScopeImportJob ParameterScope = "import_job" + ParameterTemplate ParameterScope = "template" + ParameterWorkspace ParameterScope = "workspace" + ParameterImportJob ParameterScope = "import_job" ) type ParameterSourceScheme string From 2e5bf44aa447463406f6d4008d35282ffc2917b5 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 15 Jun 2022 18:34:45 -0500 Subject: [PATCH 06/16] Lint cleanup --- cli/templatecreate.go | 3 +-- cli/templateupdate.go | 8 +++----- coderd/database/databasefake/databasefake.go | 1 - coderd/templateversions.go | 1 - 4 files changed, 4 insertions(+), 9 deletions(-) diff --git a/cli/templatecreate.go b/cli/templatecreate.go index 53d46c724199e..0c93cb1c335ee 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -8,9 +8,8 @@ import ( "strings" "time" - "github.com/google/uuid" - "github.com/briandowns/spinner" + "github.com/google/uuid" "github.com/spf13/cobra" "golang.org/x/xerrors" diff --git a/cli/templateupdate.go b/cli/templateupdate.go index 802115a0ea80d..a0e7958fca1b9 100644 --- a/cli/templateupdate.go +++ b/cli/templateupdate.go @@ -5,14 +5,12 @@ import ( "os" "time" - "golang.org/x/xerrors" - - "github.com/coder/coder/coderd/database" - "github.com/briandowns/spinner" "github.com/spf13/cobra" + "golang.org/x/xerrors" "github.com/coder/coder/cli/cliui" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/codersdk" "github.com/coder/coder/provisionersdk" ) @@ -99,7 +97,7 @@ func templateUpdate() *cobra.Command { return err } - //templateVersion, err := client.CreateTemplateVersion(cmd.Context(), organization.ID, codersdk.CreateTemplateVersionRequest{ + // templateVersion, err := client.CreateTemplateVersion(cmd.Context(), organization.ID, codersdk.CreateTemplateVersionRequest{ // TemplateID: template.ID, // StorageMethod: codersdk.ProvisionerStorageMethodFile, // StorageSource: resp.Hash, diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index aebb703ddf3e2..a4515add17d70 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -110,7 +110,6 @@ func (q *fakeQuerier) ParameterValue(_ context.Context, id uuid.UUID) (database. continue } return parameterValue, nil - } return database.ParameterValue{}, sql.ErrNoRows } diff --git a/coderd/templateversions.go b/coderd/templateversions.go index bb56aafe6986c..3be8d5cb06e3d 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -683,7 +683,6 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht for _, parameterValue := range req.ParameterValues { if parameterValue.CopyFromParameter != uuid.Nil { continue - } _, err = db.InsertParameterValue(r.Context(), database.InsertParameterValueParams{ From e5b90bc3155e6a594636a98ab8e5039e57449b5b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 15 Jun 2022 19:24:55 -0500 Subject: [PATCH 07/16] Enable always prompting of params --- cli/templatecreate.go | 36 ++++++++++--------- cli/templateupdate.go | 38 ++------------------- coderd/database/queries.sql.go | 7 ++-- coderd/database/queries/parametervalues.sql | 7 ++-- coderd/templates.go | 4 +-- 5 files changed, 33 insertions(+), 59 deletions(-) diff --git a/cli/templatecreate.go b/cli/templatecreate.go index 0c93cb1c335ee..e69a46edd959a 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -9,7 +9,6 @@ import ( "time" "github.com/briandowns/spinner" - "github.com/google/uuid" "github.com/spf13/cobra" "golang.org/x/xerrors" @@ -83,7 +82,7 @@ func templateCreate() *cobra.Command { } spin.Stop() - job, parameters, err := createValidTemplateVersion(cmd, createValidTemplateVersionArgs{ + job, _, err := createValidTemplateVersion(cmd, createValidTemplateVersionArgs{ Client: client, Organization: organization, Provisioner: database.ProvisionerType(provisioner), @@ -105,7 +104,6 @@ func templateCreate() *cobra.Command { createReq := codersdk.CreateTemplateRequest{ Name: templateName, VersionID: job.ID, - ParameterValues: parameters, MaxTTLMillis: ptr.Ref(maxTTL.Milliseconds()), MinAutostartIntervalMillis: ptr.Ref(minAutostartInterval.Milliseconds()), } @@ -146,21 +144,28 @@ type createValidTemplateVersionArgs struct { Provisioner database.ProvisionerType FileHash string ParameterFile string - // TemplateID is only required if updating a template's active version. - TemplateID uuid.UUID + // Template is only required if updating a template's active version. + Template *codersdk.Template + // ReuseParams will attempt to reuse params from the Template field + // before prompting the user. Set to false to always prompt for param + // values. + ReuseParams bool } func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVersionArgs, parameters ...codersdk.CreateParameterRequest) (*codersdk.TemplateVersion, []codersdk.CreateParameterRequest, error) { before := time.Now() client := args.Client - version, err := client.CreateTemplateVersion(cmd.Context(), args.Organization.ID, codersdk.CreateTemplateVersionRequest{ + req := codersdk.CreateTemplateVersionRequest{ StorageMethod: codersdk.ProvisionerStorageMethodFile, StorageSource: args.FileHash, Provisioner: codersdk.ProvisionerType(args.Provisioner), ParameterValues: parameters, - TemplateID: args.TemplateID, - }) + } + if args.Template != nil { + req.TemplateID = args.Template.ID + } + version, err := client.CreateTemplateVersion(cmd.Context(), args.Organization.ID, req) if err != nil { return nil, nil, err } @@ -197,15 +202,10 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers // lastParameterValues are pulled from the current active template version if // templateID is provided. This allows pulling params from the last - // version if we are updating template versions. + // version instead of prompting if we are updating template versions. lastParameterValues := make(map[string]codersdk.Parameter) - if args.TemplateID != uuid.Nil { - template, err := client.Template(cmd.Context(), args.TemplateID) - if err != nil { - return nil, nil, xerrors.Errorf("Fetch template: %w", err) - } - - activeVersion, err := client.TemplateVersion(cmd.Context(), template.ActiveVersionID) + if args.ReuseParams && args.Template != nil { + activeVersion, err := client.TemplateVersion(cmd.Context(), args.Template.ActiveVersionID) if err != nil { return nil, nil, xerrors.Errorf("Fetch current active template version: %w", err) } @@ -249,7 +249,9 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers } } for _, parameterSchema := range missingSchemas { - if inherit, ok := lastParameterValues[parameterSchema.Name]; ok { + // If the value is in the file, skip trying to reuse the param + _, fileOk := parameterMapFromFile[parameterSchema.Name] + if inherit, ok := lastParameterValues[parameterSchema.Name]; ok && !fileOk { parameters = append(parameters, codersdk.CreateParameterRequest{ CopyFromParameter: inherit.ID, }) diff --git a/cli/templateupdate.go b/cli/templateupdate.go index a0e7958fca1b9..d5781015ba06a 100644 --- a/cli/templateupdate.go +++ b/cli/templateupdate.go @@ -72,7 +72,8 @@ func templateUpdate() *cobra.Command { Provisioner: database.ProvisionerType(provisioner), FileHash: resp.Hash, ParameterFile: parameterFile, - TemplateID: template.ID, + Template: &template, + ReuseParams: true, }) if err != nil { return err @@ -97,41 +98,6 @@ func templateUpdate() *cobra.Command { return err } - // templateVersion, err := client.CreateTemplateVersion(cmd.Context(), organization.ID, codersdk.CreateTemplateVersionRequest{ - // TemplateID: template.ID, - // StorageMethod: codersdk.ProvisionerStorageMethodFile, - // StorageSource: resp.Hash, - // Provisioner: codersdk.ProvisionerType(provisioner), - //}) - //if err != nil { - // return xerrors.Errorf("create template: %w", err) - //} - //logs, err := client.TemplateVersionLogsAfter(cmd.Context(), templateVersion.ID, before) - //if err != nil { - // return err - //} - //for { - // log, ok := <-logs - // if !ok { - // break - // } - // _, _ = fmt.Printf("%s (%s): %s\n", provisioner, log.Level, log.Output) - //} - //templateVersion, err = client.TemplateVersion(cmd.Context(), templateVersion.ID) - //if err != nil { - // return err - //} - // - //if templateVersion.Job.Status != codersdk.ProvisionerJobSucceeded { - // return xerrors.Errorf("job failed: %s", templateVersion.Job.Error) - //} - // - //err = client.UpdateActiveTemplateVersion(cmd.Context(), template.ID, codersdk.UpdateActiveTemplateVersion{ - // ID: templateVersion.ID, - //}) - //if err != nil { - // return err - //} _, _ = fmt.Printf("Updated version!\n") return nil }, diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 14229e22742c3..88d4cb0db83b5 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1081,8 +1081,11 @@ FROM parameter_values WHERE CASE - WHEN $1 :: parameter_scope != '' THEN - scope = $1 + -- We need to double cast this. First cast is for the sqlc type, + -- the second case is to convert it to text as the empty string + -- is not a valid parameter_scope enum. + WHEN ($1 :: parameter_scope) :: text != '' THEN + scope = $1 :: parameter_scope ELSE true END AND CASE diff --git a/coderd/database/queries/parametervalues.sql b/coderd/database/queries/parametervalues.sql index 15b99789f51ab..853ec7a535a2f 100644 --- a/coderd/database/queries/parametervalues.sql +++ b/coderd/database/queries/parametervalues.sql @@ -18,8 +18,11 @@ FROM parameter_values WHERE CASE - WHEN @scope :: parameter_scope != '' THEN - scope = @scope + -- We need to double cast this. First cast is for the sqlc type, + -- the second case is to convert it to text as the empty string + -- is not a valid parameter_scope enum. + WHEN (@scope :: parameter_scope) :: text != '' THEN + scope = @scope :: parameter_scope ELSE true END AND CASE diff --git a/coderd/templates.go b/coderd/templates.go index 5fc180f16dd99..ed1aade359053 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -217,8 +217,8 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque Name: parameterValue.Name, CreatedAt: database.Now(), UpdatedAt: database.Now(), - Scope: database.ParameterScopeImportJob, - ScopeID: templateVersion.JobID, + Scope: database.ParameterScopeTemplate, + ScopeID: template.ID, SourceScheme: database.ParameterSourceScheme(parameterValue.SourceScheme), SourceValue: parameterValue.SourceValue, DestinationScheme: database.ParameterDestinationScheme(parameterValue.DestinationScheme), From 9d17dac16840a773ca56d65877236711cc4a52c5 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 16 Jun 2022 08:43:30 -0500 Subject: [PATCH 08/16] Fix db query for scope enum --- coderd/database/databasefake/databasefake.go | 7 ++++--- coderd/database/queries.sql.go | 21 +++++++++----------- coderd/database/queries/parametervalues.sql | 11 ++++------ coderd/parameter/compute.go | 8 ++++---- coderd/parameters.go | 2 +- 5 files changed, 22 insertions(+), 27 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index a4515add17d70..1c72b952e9cc0 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -722,10 +722,11 @@ func (q *fakeQuerier) ParameterValues(_ context.Context, arg database.ParameterV parameterValues := make([]database.ParameterValue, 0) for _, parameterValue := range q.parameterValues { - if arg.Scope != "" && parameterValue.Scope != arg.Scope { - continue + if len(arg.Scopes) > 0 { + if !contains(arg.Scopes, parameterValue.Scope) { + continue + } } - if len(arg.ScopeIds) > 0 { if !contains(arg.ScopeIds, parameterValue.ScopeID) { continue diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 88d4cb0db83b5..f791ec34cdd4f 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1080,13 +1080,10 @@ SELECT FROM parameter_values WHERE - CASE - -- We need to double cast this. First cast is for the sqlc type, - -- the second case is to convert it to text as the empty string - -- is not a valid parameter_scope enum. - WHEN ($1 :: parameter_scope) :: text != '' THEN - scope = $1 :: parameter_scope - ELSE true + CASE + WHEN cardinality($1 :: parameter_scope[]) > 0 THEN + scope = ANY($1 :: parameter_scope[]) + ELSE true END AND CASE WHEN cardinality($2 :: uuid[]) > 0 THEN @@ -1106,15 +1103,15 @@ WHERE ` type ParameterValuesParams struct { - Scope ParameterScope `db:"scope" json:"scope"` - ScopeIds []uuid.UUID `db:"scope_ids" json:"scope_ids"` - Ids []uuid.UUID `db:"ids" json:"ids"` - Names []string `db:"names" json:"names"` + Scopes []ParameterScope `db:"scopes" json:"scopes"` + ScopeIds []uuid.UUID `db:"scope_ids" json:"scope_ids"` + Ids []uuid.UUID `db:"ids" json:"ids"` + Names []string `db:"names" json:"names"` } func (q *sqlQuerier) ParameterValues(ctx context.Context, arg ParameterValuesParams) ([]ParameterValue, error) { rows, err := q.db.QueryContext(ctx, parameterValues, - arg.Scope, + pq.Array(arg.Scopes), pq.Array(arg.ScopeIds), pq.Array(arg.Ids), pq.Array(arg.Names), diff --git a/coderd/database/queries/parametervalues.sql b/coderd/database/queries/parametervalues.sql index 853ec7a535a2f..d747725d271a4 100644 --- a/coderd/database/queries/parametervalues.sql +++ b/coderd/database/queries/parametervalues.sql @@ -17,13 +17,10 @@ SELECT FROM parameter_values WHERE - CASE - -- We need to double cast this. First cast is for the sqlc type, - -- the second case is to convert it to text as the empty string - -- is not a valid parameter_scope enum. - WHEN (@scope :: parameter_scope) :: text != '' THEN - scope = @scope :: parameter_scope - ELSE true + CASE + WHEN cardinality(@scopes :: parameter_scope[]) > 0 THEN + scope = ANY(@scopes :: parameter_scope[]) + ELSE true END AND CASE WHEN cardinality(@scope_ids :: uuid[]) > 0 THEN diff --git a/coderd/parameter/compute.go b/coderd/parameter/compute.go index ba61ad21629f3..8a79850b5c99b 100644 --- a/coderd/parameter/compute.go +++ b/coderd/parameter/compute.go @@ -62,7 +62,7 @@ func Compute(ctx context.Context, db database.Store, scope ComputeScope, options // Job parameters come second! err = compute.injectScope(ctx, database.ParameterValuesParams{ - Scope: database.ParameterScopeImportJob, + Scopes: []database.ParameterScope{database.ParameterScopeImportJob}, ScopeIds: []uuid.UUID{scope.TemplateImportJobID}, }) if err != nil { @@ -106,7 +106,7 @@ func Compute(ctx context.Context, db database.Store, scope ComputeScope, options if scope.TemplateID.Valid { // Template parameters come third! err = compute.injectScope(ctx, database.ParameterValuesParams{ - Scope: database.ParameterScopeTemplate, + Scopes: []database.ParameterScope{database.ParameterScopeTemplate}, ScopeIds: []uuid.UUID{scope.TemplateID.UUID}, }) if err != nil { @@ -117,7 +117,7 @@ func Compute(ctx context.Context, db database.Store, scope ComputeScope, options if scope.WorkspaceID.Valid { // Workspace parameters come last! err = compute.injectScope(ctx, database.ParameterValuesParams{ - Scope: database.ParameterScopeWorkspace, + Scopes: []database.ParameterScope{database.ParameterScopeWorkspace}, ScopeIds: []uuid.UUID{scope.WorkspaceID.UUID}, }) if err != nil { @@ -154,7 +154,7 @@ func (c *compute) injectScope(ctx context.Context, scopeParams database.Paramete err = nil } if err != nil { - return xerrors.Errorf("get %s parameters: %w", scopeParams.Scope, err) + return xerrors.Errorf("get %s parameters: %w", scopeParams.Scopes, err) } for _, scopedParameter := range scopedParameters { diff --git a/coderd/parameters.go b/coderd/parameters.go index 1c1217a95ae00..1d22d267fbe2b 100644 --- a/coderd/parameters.go +++ b/coderd/parameters.go @@ -92,7 +92,7 @@ func (api *API) parameters(rw http.ResponseWriter, r *http.Request) { } parameterValues, err := api.Database.ParameterValues(r.Context(), database.ParameterValuesParams{ - Scope: scope, + Scopes: []database.ParameterScope{scope}, ScopeIds: []uuid.UUID{scopeID}, }) if errors.Is(err, sql.ErrNoRows) { From 04098c355904c1b4b65833fa47d770ace75a4cc6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 16 Jun 2022 08:44:48 -0500 Subject: [PATCH 09/16] Update comment --- codersdk/parameters.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codersdk/parameters.go b/codersdk/parameters.go index 1bf46c5cfd1b5..605e550ba911c 100644 --- a/codersdk/parameters.go +++ b/codersdk/parameters.go @@ -79,7 +79,7 @@ type ParameterSchema struct { // CreateParameterRequest is used to create a new parameter value for a scope. type CreateParameterRequest struct { // CopyFromParameter allows copying the value of another parameter. - // The other param must share the same scope and scopeID for this to + // The other param must be related to the same template_id for this to // succeed. // No other fields are required if using this, as all fields will be copied // from the other parameter. From 3422844f9524f80725b0b444a0396a347d6d014f Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 16 Jun 2022 08:56:09 -0500 Subject: [PATCH 10/16] Fix list parameters --- cli/parameterslist.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cli/parameterslist.go b/cli/parameterslist.go index 26859f6c3b29e..58067eab674e6 100644 --- a/cli/parameterslist.go +++ b/cli/parameterslist.go @@ -52,6 +52,14 @@ func parameterList() *cobra.Command { if err != nil { return xerrors.Errorf("%q must be a uuid for this scope type", name) } + + // Could be a template_version id or a job id. Check for the + // version id. + tv, err := client.TemplateVersion(cmd.Context(), scopeID) + if err == nil { + scopeID = tv.Job.ID + } + default: return xerrors.Errorf("%q is an unsupported scope, use %v", scope, []codersdk.ParameterScope{ codersdk.ParameterWorkspace, codersdk.ParameterTemplate, codersdk.ParameterImportJob, From 5f881aaece83f6c4f33cb4142539e4e4611d27f0 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 16 Jun 2022 15:18:14 -0500 Subject: [PATCH 11/16] Rename field --- cli/templatecreate.go | 6 +++--- cli/templateupdate.go | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cli/templatecreate.go b/cli/templatecreate.go index e69a46edd959a..ee5cde306250f 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -146,10 +146,10 @@ type createValidTemplateVersionArgs struct { ParameterFile string // Template is only required if updating a template's active version. Template *codersdk.Template - // ReuseParams will attempt to reuse params from the Template field + // ReuseParameters will attempt to reuse params from the Template field // before prompting the user. Set to false to always prompt for param // values. - ReuseParams bool + ReuseParameters bool } func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVersionArgs, parameters ...codersdk.CreateParameterRequest) (*codersdk.TemplateVersion, []codersdk.CreateParameterRequest, error) { @@ -204,7 +204,7 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers // templateID is provided. This allows pulling params from the last // version instead of prompting if we are updating template versions. lastParameterValues := make(map[string]codersdk.Parameter) - if args.ReuseParams && args.Template != nil { + if args.ReuseParameters && args.Template != nil { activeVersion, err := client.TemplateVersion(cmd.Context(), args.Template.ActiveVersionID) if err != nil { return nil, nil, xerrors.Errorf("Fetch current active template version: %w", err) diff --git a/cli/templateupdate.go b/cli/templateupdate.go index d5781015ba06a..9e8a4ca1212ae 100644 --- a/cli/templateupdate.go +++ b/cli/templateupdate.go @@ -67,13 +67,13 @@ func templateUpdate() *cobra.Command { spin.Stop() job, _, err := createValidTemplateVersion(cmd, createValidTemplateVersionArgs{ - Client: client, - Organization: organization, - Provisioner: database.ProvisionerType(provisioner), - FileHash: resp.Hash, - ParameterFile: parameterFile, - Template: &template, - ReuseParams: true, + Client: client, + Organization: organization, + Provisioner: database.ProvisionerType(provisioner), + FileHash: resp.Hash, + ParameterFile: parameterFile, + Template: &template, + ReuseParameters: true, }) if err != nil { return err From c43aedcbd05bd06bd9c15be668e579b3f2767957 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 17 Jun 2022 09:37:48 -0500 Subject: [PATCH 12/16] Unit test param prompting --- cli/parameterslist.go | 1 - cli/templateupdate_test.go | 180 +++++++++++++++---- coderd/database/databasefake/databasefake.go | 20 +-- coderd/util/slice/slice.go | 10 ++ coderd/util/slice/slice_test.go | 30 ++++ 5 files changed, 189 insertions(+), 52 deletions(-) create mode 100644 coderd/util/slice/slice.go create mode 100644 coderd/util/slice/slice_test.go diff --git a/cli/parameterslist.go b/cli/parameterslist.go index 58067eab674e6..495b48247ff31 100644 --- a/cli/parameterslist.go +++ b/cli/parameterslist.go @@ -45,7 +45,6 @@ func parameterList() *cobra.Command { return xerrors.Errorf("get workspace template: %w", err) } scopeID = template.ID - case codersdk.ParameterImportJob, "template_version": scope = string(codersdk.ParameterImportJob) scopeID, err = uuid.Parse(name) diff --git a/cli/templateupdate_test.go b/cli/templateupdate_test.go index 134e884370733..d85ea774feed2 100644 --- a/cli/templateupdate_test.go +++ b/cli/templateupdate_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -17,47 +18,150 @@ import ( func TestTemplateUpdate(t *testing.T) { t.Parallel() + // NewParameter will: + // 1. Create a template version with 0 params + // 2. Create a new version with 1 param + // 2a. Expects 1 param prompt, fills in value + // 3. Assert 1 param value in new version + // 4. Creates a new version with same param + // 4a. Expects 0 prompts as the param value is carried over + // 5. Assert 1 param value in new version + // 6. Creates a new version with 0 params + // 7. Asset 0 params in new version + t.Run("NewParameter", func(t *testing.T) { + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user := coderdtest.CreateFirstUser(t, client) + // Create initial template version to update + lastActiveVersion := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJob(t, client, lastActiveVersion.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, lastActiveVersion.ID) - client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) - user := coderdtest.CreateFirstUser(t, client) - version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) - _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + // Create new template version with a new parameter + source := clitest.CreateTemplateVersionSource(t, &echo.Responses{ + Parse: createTestParseResponse(), + Provision: echo.ProvisionComplete, + }) + cmd, root := clitest.New(t, "templates", "update", template.Name, "-y", "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho)) + clitest.SetupConfig(t, client, root) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) - // Test the cli command. - source := clitest.CreateTemplateVersionSource(t, &echo.Responses{ - Parse: echo.ParseComplete, - Provision: echo.ProvisionComplete, + execDone := make(chan error) + go func() { + execDone <- cmd.Execute() + }() + + matches := []struct { + match string + write string + }{ + // Expect to be prompted for the new param + {match: "Enter a value:", write: "peter-pan"}, + } + for _, m := range matches { + pty.ExpectMatch(m.match) + pty.WriteLine(m.write) + } + + require.NoError(t, <-execDone) + + // Assert template version changed and we have the new param + latestTV, latestParams := latestTemplateVersion(t, client, template.ID) + assert.NotEqual(t, lastActiveVersion.ID, latestTV.ID) + require.Len(t, latestParams, 1, "expect 1 param") + lastActiveVersion = latestTV + + // Second update of the same source requires no prompt since the params + // are carried over. + cmd, root = clitest.New(t, "templates", "update", template.Name, "-y", "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho)) + clitest.SetupConfig(t, client, root) + go func() { + execDone <- cmd.Execute() + }() + require.NoError(t, <-execDone) + + // Assert template version changed and we have the carried over param + latestTV, latestParams = latestTemplateVersion(t, client, template.ID) + assert.NotEqual(t, lastActiveVersion.ID, latestTV.ID) + require.Len(t, latestParams, 1, "expect 1 param") + lastActiveVersion = latestTV + + // Remove the param + source = clitest.CreateTemplateVersionSource(t, &echo.Responses{ + Parse: echo.ParseComplete, + Provision: echo.ProvisionComplete, + }) + + cmd, root = clitest.New(t, "templates", "update", template.Name, "-y", "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho)) + clitest.SetupConfig(t, client, root) + go func() { + execDone <- cmd.Execute() + }() + require.NoError(t, <-execDone) + // Assert template version changed and the param was removed + latestTV, latestParams = latestTemplateVersion(t, client, template.ID) + assert.NotEqual(t, lastActiveVersion.ID, latestTV.ID) + require.Len(t, latestParams, 0, "expect 0 param") + lastActiveVersion = latestTV }) - cmd, root := clitest.New(t, "templates", "update", template.Name, "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho)) - 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, + + t.Run("OK", func(t *testing.T) { + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + + // Test the cli command. + source := clitest.CreateTemplateVersionSource(t, &echo.Responses{ + Parse: echo.ParseComplete, + Provision: echo.ProvisionComplete, + }) + cmd, root := clitest.New(t, "templates", "update", template.Name, "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho)) + 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) }) +} + +func latestTemplateVersion(t *testing.T, client *codersdk.Client, templateID uuid.UUID) (codersdk.TemplateVersion, []codersdk.Parameter) { + t.Helper() + + ctx := context.Background() + newTemplate, err := client.Template(ctx, templateID) + require.NoError(t, err) + tv, err := client.TemplateVersion(ctx, newTemplate.ActiveVersionID) require.NoError(t, err) - assert.Len(t, templateVersions, 2) - assert.NotEqual(t, template.ActiveVersionID, templateVersions[1].ID) + params, err := client.Parameters(ctx, codersdk.ParameterImportJob, tv.Job.ID) + require.NoError(t, err) + + return tv, params } diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 5077a1ac82e92..dee416cb8d72d 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -8,6 +8,8 @@ import ( "sync" "time" + "github.com/coder/coder/coderd/util/slice" + "github.com/google/uuid" "golang.org/x/exp/slices" @@ -66,6 +68,8 @@ type fakeQuerier struct { workspaceBuilds []database.WorkspaceBuild workspaceApps []database.WorkspaceApp workspaces []database.Workspace + + deploymentID string } // InTx doesn't rollback data properly for in-memory yet. @@ -762,18 +766,18 @@ func (q *fakeQuerier) ParameterValues(_ context.Context, arg database.ParameterV parameterValues := make([]database.ParameterValue, 0) for _, parameterValue := range q.parameterValues { if len(arg.Scopes) > 0 { - if !contains(arg.Scopes, parameterValue.Scope) { + if !slice.Contains(arg.Scopes, parameterValue.Scope) { continue } } if len(arg.ScopeIds) > 0 { - if !contains(arg.ScopeIds, parameterValue.ScopeID) { + if !slice.Contains(arg.ScopeIds, parameterValue.ScopeID) { continue } } if len(arg.Ids) > 0 { - if !contains(arg.Ids, parameterValue.ID) { + if !slice.Contains(arg.Ids, parameterValue.ID) { continue } } @@ -2115,13 +2119,3 @@ func (q *fakeQuerier) GetDeploymentID(_ context.Context) (string, error) { return q.deploymentID, nil } - - -func contains[T comparable](haystack []T, needle T) bool { - for _, hay := range haystack { - if needle == hay { - return true - } - } - return false -} diff --git a/coderd/util/slice/slice.go b/coderd/util/slice/slice.go new file mode 100644 index 0000000000000..dfea2ed26f6c6 --- /dev/null +++ b/coderd/util/slice/slice.go @@ -0,0 +1,10 @@ +package slice + +func Contains[T comparable](haystack []T, needle T) bool { + for _, hay := range haystack { + if needle == hay { + return true + } + } + return false +} diff --git a/coderd/util/slice/slice_test.go b/coderd/util/slice/slice_test.go new file mode 100644 index 0000000000000..ddeee9273c6c5 --- /dev/null +++ b/coderd/util/slice/slice_test.go @@ -0,0 +1,30 @@ +package slice_test + +import ( + "github.com/coder/coder/coderd/util/slice" + "github.com/google/uuid" + "github.com/stretchr/testify/require" + "testing" +) + +func TestContains(t *testing.T) { + testContains(t, []int{1, 2, 3, 4, 5}, []int{1, 2, 3, 4, 5}, []int{0, 6, -1, -2, 100}) + testContains(t, []string{"hello", "world", "foo", "bar", "baz"}, []string{"hello", "world", "baz"}, []string{"not", "words", "in", "set"}) + testContains(t, + []uuid.UUID{uuid.New(), uuid.MustParse("c7c6686d-a93c-4df2-bef9-5f837e9a33d5"), uuid.MustParse("8f3b3e0b-2c3f-46a5-a365-fd5b62bd8818")}, + []uuid.UUID{uuid.MustParse("c7c6686d-a93c-4df2-bef9-5f837e9a33d5")}, + []uuid.UUID{uuid.MustParse("1d00e27d-8de6-46f8-80d5-1da0ca83874a")}, + ) +} + +func testContains[T comparable](t *testing.T, set []T, in []T, out []T) { + for _, e := range set { + require.True(t, slice.Contains(set, e), "elements in set should be in the set") + } + for _, e := range in { + require.True(t, slice.Contains(set, e), "expect element in set") + } + for _, e := range out { + require.False(t, slice.Contains(set, e), "expect element in set") + } +} From fa9b230f09232fcbde0e4a344db77cca524295ac Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 17 Jun 2022 09:55:28 -0500 Subject: [PATCH 13/16] Add "--always-prompt" flag and logging info --- cli/templatecreate.go | 37 ++++++++++++++++++++++++++----------- cli/templateupdate.go | 4 +++- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/cli/templatecreate.go b/cli/templatecreate.go index 83b68d3d4306f..bc360fd99639f 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -229,15 +229,6 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers sort.Slice(parameterSchemas, func(i, j int) bool { return parameterSchemas[i].Name < parameterSchemas[j].Name }) - missingSchemas := make([]codersdk.ParameterSchema, 0) - for _, parameterSchema := range parameterSchemas { - _, ok := valuesBySchemaID[parameterSchema.ID.String()] - if ok { - continue - } - 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") // parameterMapFromFile can be nil if parameter file is not specified var parameterMapFromFile map[string]string @@ -248,15 +239,39 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers return nil, nil, err } } - for _, parameterSchema := range missingSchemas { - // If the value is in the file, skip trying to reuse the param + + // pulled params come from the last template version + pulled := make([]string, 0) + missingSchemas := make([]codersdk.ParameterSchema, 0) + for _, parameterSchema := range parameterSchemas { + _, ok := valuesBySchemaID[parameterSchema.ID.String()] + if ok { + continue + } + + // The file values are handled below. So don't handle them here, + // just check if a value is present in the file. _, fileOk := parameterMapFromFile[parameterSchema.Name] if inherit, ok := lastParameterValues[parameterSchema.Name]; ok && !fileOk { + // If the value is not in the param file, and can be pulled from the last template version, + // then don't mark it as missing. parameters = append(parameters, codersdk.CreateParameterRequest{ CopyFromParameter: inherit.ID, }) + pulled = append(pulled, fmt.Sprintf("%q", parameterSchema.Name)) continue } + + 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.")) + if len(pulled) > 0 { + _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render(fmt.Sprintf("The following parameter values are being pulled from the latest template version: %s.", strings.Join(pulled, ", ")))) + _, _ = fmt.Fprintln(cmd.OutOrStdout(), cliui.Styles.Paragraph.Render("Use \"--always-prompt\" flag to change the values.")) + } + _, _ = fmt.Fprint(cmd.OutOrStdout(), "\r\n") + + for _, parameterSchema := range missingSchemas { parameterValue, err := getParameterValueFromMapOrInput(cmd, parameterMapFromFile, parameterSchema) if err != nil { return nil, nil, err diff --git a/cli/templateupdate.go b/cli/templateupdate.go index 9e8a4ca1212ae..b7a8dbf238099 100644 --- a/cli/templateupdate.go +++ b/cli/templateupdate.go @@ -20,6 +20,7 @@ func templateUpdate() *cobra.Command { directory string provisioner string parameterFile string + alwaysPrompt bool ) cmd := &cobra.Command{ @@ -73,7 +74,7 @@ func templateUpdate() *cobra.Command { FileHash: resp.Hash, ParameterFile: parameterFile, Template: &template, - ReuseParameters: true, + ReuseParameters: !alwaysPrompt, }) if err != nil { return err @@ -107,6 +108,7 @@ func templateUpdate() *cobra.Command { 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.") + cmd.Flags().BoolVar(&alwaysPrompt, "always-prompt", false, "Always prompt all parameters. Does not pull parameter values from active template version") cliui.AllowSkipPrompt(cmd) // This is for testing! err := cmd.Flags().MarkHidden("test.provisioner") From 9438db81173ab04a28faf072a175ca3531b419f5 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 17 Jun 2022 10:14:44 -0500 Subject: [PATCH 14/16] Linting fixes --- cli/templatecreate.go | 2 +- cli/templateupdate_test.go | 2 ++ coderd/database/databasefake/databasefake.go | 3 +-- coderd/templateversions.go | 6 +++--- codersdk/parameters.go | 4 ++-- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/cli/templatecreate.go b/cli/templatecreate.go index bc360fd99639f..6ec0f86b4021e 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -256,7 +256,7 @@ func createValidTemplateVersion(cmd *cobra.Command, args createValidTemplateVers // If the value is not in the param file, and can be pulled from the last template version, // then don't mark it as missing. parameters = append(parameters, codersdk.CreateParameterRequest{ - CopyFromParameter: inherit.ID, + CloneID: inherit.ID, }) pulled = append(pulled, fmt.Sprintf("%q", parameterSchema.Name)) continue diff --git a/cli/templateupdate_test.go b/cli/templateupdate_test.go index d85ea774feed2..71cabc6ad408b 100644 --- a/cli/templateupdate_test.go +++ b/cli/templateupdate_test.go @@ -29,6 +29,7 @@ func TestTemplateUpdate(t *testing.T) { // 6. Creates a new version with 0 params // 7. Asset 0 params in new version t.Run("NewParameter", func(t *testing.T) { + t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) user := coderdtest.CreateFirstUser(t, client) // Create initial template version to update @@ -107,6 +108,7 @@ func TestTemplateUpdate(t *testing.T) { }) t.Run("OK", 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, nil) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index dee416cb8d72d..ada3051085863 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -8,12 +8,11 @@ import ( "sync" "time" - "github.com/coder/coder/coderd/util/slice" - "github.com/google/uuid" "golang.org/x/exp/slices" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/util/slice" ) // New returns an in-memory fake of the database. diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 3be8d5cb06e3d..de327ac7a0bea 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -640,8 +640,8 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht jobID := uuid.New() inherits := make([]uuid.UUID, 0) for _, parameterValue := range req.ParameterValues { - if parameterValue.CopyFromParameter != uuid.Nil { - inherits = append(inherits, parameterValue.CopyFromParameter) + if parameterValue.CloneID != uuid.Nil { + inherits = append(inherits, parameterValue.CloneID) } } @@ -681,7 +681,7 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht } for _, parameterValue := range req.ParameterValues { - if parameterValue.CopyFromParameter != uuid.Nil { + if parameterValue.CloneID != uuid.Nil { continue } diff --git a/codersdk/parameters.go b/codersdk/parameters.go index 605e550ba911c..939d6a7e381b7 100644 --- a/codersdk/parameters.go +++ b/codersdk/parameters.go @@ -78,12 +78,12 @@ type ParameterSchema struct { // CreateParameterRequest is used to create a new parameter value for a scope. type CreateParameterRequest struct { - // CopyFromParameter allows copying the value of another parameter. + // CloneID allows copying the value of another parameter. // The other param must be related to the same template_id for this to // succeed. // No other fields are required if using this, as all fields will be copied // from the other parameter. - CopyFromParameter uuid.UUID `json:"copy_from_parameter,omitempty" validate:"uuid4"` + CloneID uuid.UUID `json:"copy_from_parameter,omitempty" validate:"uuid4"` Name string `json:"name" validate:"required"` SourceValue string `json:"source_value" validate:"required"` From 5c590ae2cd680ddea64ae84e6cd8fba435348d59 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 17 Jun 2022 10:30:08 -0500 Subject: [PATCH 15/16] Linting --- coderd/util/slice/slice_test.go | 17 +++++++++++------ codersdk/parameters.go | 2 +- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/coderd/util/slice/slice_test.go b/coderd/util/slice/slice_test.go index ddeee9273c6c5..1f485b77e3067 100644 --- a/coderd/util/slice/slice_test.go +++ b/coderd/util/slice/slice_test.go @@ -1,23 +1,28 @@ package slice_test import ( - "github.com/coder/coder/coderd/util/slice" + "testing" + "github.com/google/uuid" "github.com/stretchr/testify/require" - "testing" + + "github.com/coder/coder/coderd/util/slice" ) func TestContains(t *testing.T) { - testContains(t, []int{1, 2, 3, 4, 5}, []int{1, 2, 3, 4, 5}, []int{0, 6, -1, -2, 100}) - testContains(t, []string{"hello", "world", "foo", "bar", "baz"}, []string{"hello", "world", "baz"}, []string{"not", "words", "in", "set"}) - testContains(t, + t.Parallel() + + assertSetContains(t, []int{1, 2, 3, 4, 5}, []int{1, 2, 3, 4, 5}, []int{0, 6, -1, -2, 100}) + assertSetContains(t, []string{"hello", "world", "foo", "bar", "baz"}, []string{"hello", "world", "baz"}, []string{"not", "words", "in", "set"}) + assertSetContains(t, []uuid.UUID{uuid.New(), uuid.MustParse("c7c6686d-a93c-4df2-bef9-5f837e9a33d5"), uuid.MustParse("8f3b3e0b-2c3f-46a5-a365-fd5b62bd8818")}, []uuid.UUID{uuid.MustParse("c7c6686d-a93c-4df2-bef9-5f837e9a33d5")}, []uuid.UUID{uuid.MustParse("1d00e27d-8de6-46f8-80d5-1da0ca83874a")}, ) } -func testContains[T comparable](t *testing.T, set []T, in []T, out []T) { +func assertSetContains[T comparable](t *testing.T, set []T, in []T, out []T) { + t.Helper() for _, e := range set { require.True(t, slice.Contains(set, e), "elements in set should be in the set") } diff --git a/codersdk/parameters.go b/codersdk/parameters.go index 939d6a7e381b7..a4943817144be 100644 --- a/codersdk/parameters.go +++ b/codersdk/parameters.go @@ -83,7 +83,7 @@ type CreateParameterRequest struct { // succeed. // No other fields are required if using this, as all fields will be copied // from the other parameter. - CloneID uuid.UUID `json:"copy_from_parameter,omitempty" validate:"uuid4"` + CloneID uuid.UUID `json:"copy_from_parameter,omitempty" validate:""` Name string `json:"name" validate:"required"` SourceValue string `json:"source_value" validate:"required"` From f46b4afe73787724511ce0471979c1f2b742f871 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 17 Jun 2022 10:54:43 -0500 Subject: [PATCH 16/16] Remove excessive prompt --- cli/templateupdate.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/cli/templateupdate.go b/cli/templateupdate.go index b7a8dbf238099..fcbba2b122c05 100644 --- a/cli/templateupdate.go +++ b/cli/templateupdate.go @@ -84,14 +84,6 @@ func templateUpdate() *cobra.Command { return xerrors.Errorf("job failed: %s", job.Job.Status) } - _, err = cliui.Prompt(cmd, cliui.PromptOptions{ - Text: "Confirm create?", - IsConfirm: true, - }) - if err != nil { - return err - } - err = client.UpdateActiveTemplateVersion(cmd.Context(), template.ID, codersdk.UpdateActiveTemplateVersion{ ID: job.ID, })