Skip to content

refactor(cli): load template variables #11234

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 15, 2023
Merged

refactor(cli): load template variables #11234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af67cc2
refactor(cli): loading template variables
mtojek Dec 15, 2023
2e5184c
cleanup
mtojek Dec 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 21 additions & 47 deletions cli/templatecreate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (
"fmt"
"io"
"net/http"
"os"
"path/filepath"
"strings"
"time"
"unicode/utf8"
Expand All @@ -27,7 +25,7 @@ func (r *RootCmd) templateCreate() *clibase.Cmd {
provisioner string
provisionerTags []string
variablesFile string
variables []string
commandLineVariables []string
disableEveryone bool
requireActiveVersion bool

Expand Down Expand Up @@ -129,15 +127,21 @@ func (r *RootCmd) templateCreate() *clibase.Cmd {
return err
}

userVariableValues, err := ParseUserVariableValues(
variablesFile,
commandLineVariables)
if err != nil {
return err
}

job, err := createValidTemplateVersion(inv, createValidTemplateVersionArgs{
Message: message,
Client: client,
Organization: organization,
Provisioner: codersdk.ProvisionerType(provisioner),
FileID: resp.ID,
ProvisionerTags: tags,
VariablesFile: variablesFile,
Variables: variables,
Message: message,
Client: client,
Organization: organization,
Provisioner: codersdk.ProvisionerType(provisioner),
FileID: resp.ID,
ProvisionerTags: tags,
UserVariableValues: userVariableValues,
})
if err != nil {
return err
Expand Down Expand Up @@ -197,12 +201,12 @@ func (r *RootCmd) templateCreate() *clibase.Cmd {
{
Flag: "variable",
Description: "Specify a set of values for Terraform-managed variables.",
Value: clibase.StringArrayOf(&variables),
Value: clibase.StringArrayOf(&commandLineVariables),
},
{
Flag: "var",
Description: "Alias of --variable.",
Value: clibase.StringArrayOf(&variables),
Value: clibase.StringArrayOf(&commandLineVariables),
},
{
Flag: "provisioner-tag",
Expand Down Expand Up @@ -267,40 +271,27 @@ type createValidTemplateVersionArgs struct {
Provisioner codersdk.ProvisionerType
FileID uuid.UUID

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
// before prompting the user. Set to false to always prompt for param
// values.
ReuseParameters bool
ProvisionerTags map[string]string
ReuseParameters bool
ProvisionerTags map[string]string
UserVariableValues []codersdk.VariableValue
}

func createValidTemplateVersion(inv *clibase.Invocation, args createValidTemplateVersionArgs) (*codersdk.TemplateVersion, error) {
client := args.Client

variableValues, err := loadVariableValuesFromFile(args.VariablesFile)
if err != nil {
return nil, err
}

variableValuesFromKeyValues, err := loadVariableValuesFromOptions(args.Variables)
if err != nil {
return nil, err
}
variableValues = append(variableValues, variableValuesFromKeyValues...)

req := codersdk.CreateTemplateVersionRequest{
Name: args.Name,
Message: args.Message,
StorageMethod: codersdk.ProvisionerStorageMethodFile,
FileID: args.FileID,
Provisioner: args.Provisioner,
ProvisionerTags: args.ProvisionerTags,
UserVariableValues: variableValues,
UserVariableValues: args.UserVariableValues,
}
if args.Template != nil {
req.TemplateID = args.Template.ID
Expand Down Expand Up @@ -364,23 +355,6 @@ func createValidTemplateVersion(inv *clibase.Invocation, args createValidTemplat
return &version, nil
}

// prettyDirectoryPath returns a prettified path when inside the users
// home directory. Falls back to dir if the users home directory cannot
// discerned. This function calls filepath.Clean on the result.
func prettyDirectoryPath(dir string) string {
dir = filepath.Clean(dir)
homeDir, err := os.UserHomeDir()
if err != nil {
return dir
}
prettyDir := dir
if strings.HasPrefix(prettyDir, homeDir) {
prettyDir = strings.TrimPrefix(prettyDir, homeDir)
prettyDir = "~" + prettyDir
}
return prettyDir
}

func ParseProvisionerTags(rawTags []string) (map[string]string, error) {
tags := map[string]string{}
for _, rawTag := range rawTags {
Expand Down
64 changes: 44 additions & 20 deletions cli/templatepush.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bufio"
"fmt"
"io"
"os"
"path/filepath"
"strings"
"time"
Expand Down Expand Up @@ -155,16 +156,16 @@ func (pf *templateUploadFlags) templateName(args []string) (string, error) {

func (r *RootCmd) templatePush() *clibase.Cmd {
var (
versionName string
provisioner string
workdir string
variablesFile string
variables []string
alwaysPrompt bool
provisionerTags []string
uploadFlags templateUploadFlags
activate bool
create bool
versionName string
provisioner string
workdir string
variablesFile string
commandLineVariables []string
alwaysPrompt bool
provisionerTags []string
uploadFlags templateUploadFlags
activate bool
create bool
)
client := new(codersdk.Client)
cmd := &clibase.Cmd{
Expand Down Expand Up @@ -213,15 +214,21 @@ func (r *RootCmd) templatePush() *clibase.Cmd {
return err
}

userVariableValues, err := ParseUserVariableValues(
variablesFile,
commandLineVariables)
if err != nil {
return err
}

args := createValidTemplateVersionArgs{
Message: message,
Client: client,
Organization: organization,
Provisioner: codersdk.ProvisionerType(provisioner),
FileID: resp.ID,
ProvisionerTags: tags,
VariablesFile: variablesFile,
Variables: variables,
Message: message,
Client: client,
Organization: organization,
Provisioner: codersdk.ProvisionerType(provisioner),
FileID: resp.ID,
ProvisionerTags: tags,
UserVariableValues: userVariableValues,
}

if !createTemplate {
Expand Down Expand Up @@ -291,12 +298,12 @@ func (r *RootCmd) templatePush() *clibase.Cmd {
{
Flag: "variable",
Description: "Specify a set of values for Terraform-managed variables.",
Value: clibase.StringArrayOf(&variables),
Value: clibase.StringArrayOf(&commandLineVariables),
},
{
Flag: "var",
Description: "Alias of --variable.",
Value: clibase.StringArrayOf(&variables),
Value: clibase.StringArrayOf(&commandLineVariables),
},
{
Flag: "provisioner-tag",
Expand Down Expand Up @@ -330,3 +337,20 @@ func (r *RootCmd) templatePush() *clibase.Cmd {
cmd.Options = append(cmd.Options, uploadFlags.options()...)
return cmd
}

// prettyDirectoryPath returns a prettified path when inside the users
// home directory. Falls back to dir if the users home directory cannot
// discerned. This function calls filepath.Clean on the result.
func prettyDirectoryPath(dir string) string {
dir = filepath.Clean(dir)
homeDir, err := os.UserHomeDir()
if err != nil {
return dir
}
prettyDir := dir
if strings.HasPrefix(prettyDir, homeDir) {
prettyDir = strings.TrimPrefix(prettyDir, homeDir)
prettyDir = "~" + prettyDir
}
return prettyDir
}
35 changes: 33 additions & 2 deletions cli/templatevariables.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,21 @@ import (
"github.com/coder/coder/v2/codersdk"
)

func loadVariableValuesFromFile(variablesFile string) ([]codersdk.VariableValue, error) {
func ParseUserVariableValues(variablesFile string, commandLineVariables []string) ([]codersdk.VariableValue, error) {
fromFile, err := parseVariableValuesFromFile(variablesFile)
if err != nil {
return nil, err
}

fromCommandLine, err := parseVariableValuesFromCommandLine(commandLineVariables)
if err != nil {
return nil, err
}

return combineVariableValues(fromFile, fromCommandLine), nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This raises an interesting question - what should the order of precedence be between values from files and values specified directly by CLI?
My hunch is that CLI variables should override those specified in files, and looking at combineVariableValues below that seems to be what would happen here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My hunch is that CLI variables should override those specified in files, and looking at combineVariableValues below that seems to be what would happen here.

Yeah, that is my idea too. I described the order of precedence here.

}

func parseVariableValuesFromFile(variablesFile string) ([]codersdk.VariableValue, error) {
var values []codersdk.VariableValue
if variablesFile == "" {
return values, nil
Expand Down Expand Up @@ -50,7 +64,7 @@ func createVariablesMapFromFile(variablesFile string) (map[string]string, error)
return variablesMap, nil
}

func loadVariableValuesFromOptions(variables []string) ([]codersdk.VariableValue, error) {
func parseVariableValuesFromCommandLine(variables []string) ([]codersdk.VariableValue, error) {
var values []codersdk.VariableValue
for _, keyValue := range variables {
split := strings.SplitN(keyValue, "=", 2)
Expand All @@ -65,3 +79,20 @@ func loadVariableValuesFromOptions(variables []string) ([]codersdk.VariableValue
}
return values, nil
}

func combineVariableValues(valuesSets ...[]codersdk.VariableValue) []codersdk.VariableValue {
combinedValues := make(map[string]string)

for _, values := range valuesSets {
for _, v := range values {
combinedValues[v.Name] = v.Value
}
}

var result []codersdk.VariableValue
for name, value := range combinedValues {
result = append(result, codersdk.VariableValue{Name: name, Value: value})
}

return result
}