diff --git a/cmd/clusterctl/client/repository/template.go b/cmd/clusterctl/client/repository/template.go index d6d7bc26c284..054063d2844d 100644 --- a/cmd/clusterctl/client/repository/template.go +++ b/cmd/clusterctl/client/repository/template.go @@ -30,10 +30,15 @@ import ( // 1. Checks for all the variables in the cluster template YAML file and replace with corresponding config values // 2. Ensure all the cluster objects are deployed in the target namespace. type Template interface { - // Variables required by the template. - // This value is derived by the template YAML. + // Variables used by the template. + // This value is derived from the template YAML. Variables() []string + // Variables used by the template with their default values. If the value is `nil`, there is no + // default and the variable is required. + // This value is derived from the template YAML. + VariableMap() map[string]*string + // TargetNamespace where the template objects will be installed. TargetNamespace() string @@ -47,6 +52,7 @@ type Template interface { // template implements Template. type template struct { variables []string + variableMap map[string]*string targetNamespace string objs []unstructured.Unstructured } @@ -58,6 +64,10 @@ func (t *template) Variables() []string { return t.variables } +func (t *template) VariableMap() map[string]*string { + return t.variableMap +} + func (t *template) TargetNamespace() string { return t.targetNamespace } @@ -86,9 +96,15 @@ func NewTemplate(input TemplateInput) (Template, error) { return nil, err } + variableMap, err := input.Processor.GetVariableMap(input.RawArtifact) + if err != nil { + return nil, err + } + if input.ListVariablesOnly { return &template{ variables: variables, + variableMap: variableMap, targetNamespace: input.TargetNamespace, }, nil } @@ -111,6 +127,7 @@ func NewTemplate(input TemplateInput) (Template, error) { return &template{ variables: variables, + variableMap: variableMap, targetNamespace: input.TargetNamespace, objs: objs, }, nil diff --git a/cmd/clusterctl/client/yamlprocessor/processor.go b/cmd/clusterctl/client/yamlprocessor/processor.go index 62123f6900b8..82be4e9530fe 100644 --- a/cmd/clusterctl/client/yamlprocessor/processor.go +++ b/cmd/clusterctl/client/yamlprocessor/processor.go @@ -25,9 +25,13 @@ type Processor interface { GetTemplateName(version, flavor string) string // GetVariables parses the template blob of bytes and provides a - // list of variables that the template requires. + // list of variables that the template uses. GetVariables([]byte) ([]string, error) + // GetVariables parses the template blob of bytes and provides a + // map of variables that the template uses with their default values. + GetVariableMap([]byte) (map[string]*string, error) + // Process processes the template blob of bytes and will return the final // yaml with values retrieved from the values getter Process([]byte, func(string) (string, error)) ([]byte, error) diff --git a/cmd/clusterctl/client/yamlprocessor/simple_processor.go b/cmd/clusterctl/client/yamlprocessor/simple_processor.go index 6be43e0e4616..dda804df518a 100644 --- a/cmd/clusterctl/client/yamlprocessor/simple_processor.go +++ b/cmd/clusterctl/client/yamlprocessor/simple_processor.go @@ -54,13 +54,10 @@ func (tp *SimpleProcessor) GetTemplateName(_, flavor string) string { // GetVariables returns a list of the variables specified in the yaml. func (tp *SimpleProcessor) GetVariables(rawArtifact []byte) ([]string, error) { - strArtifact := convertLegacyVars(string(rawArtifact)) - - variables, err := inspectVariables(strArtifact) + variables, err := tp.GetVariableMap(rawArtifact) if err != nil { return nil, err } - varNames := make([]string, 0, len(variables)) for k := range variables { varNames = append(varNames, k) @@ -69,6 +66,25 @@ func (tp *SimpleProcessor) GetVariables(rawArtifact []byte) ([]string, error) { return varNames, nil } +// GetVariableMap returns a map of the variables specified in the yaml. +func (tp *SimpleProcessor) GetVariableMap(rawArtifact []byte) (map[string]*string, error) { + strArtifact := convertLegacyVars(string(rawArtifact)) + variables, err := inspectVariables(strArtifact) + if err != nil { + return nil, err + } + varMap := make(map[string]*string, len(variables)) + for k, v := range variables { + if v == "" { + varMap[k] = nil + } else { + v := v + varMap[k] = &v + } + } + return varMap, nil +} + // Process returns the final yaml with all the variables replaced with their // respective values. If there are variables without corresponding values, it // will return the raw yaml along with an error. @@ -82,11 +98,11 @@ func (tp *SimpleProcessor) Process(rawArtifact []byte, variablesClient func(stri var missingVariables []string // keep track of missing variables to return as error later - for name, hasDefault := range variables { + for name, defaultValue := range variables { _, err := variablesClient(name) // add to missingVariables list if the variable does not exist in the // variablesClient AND it does not have a default value - if err != nil && !hasDefault { + if err != nil && len(defaultValue) == 0 { missingVariables = append(missingVariables, name) continue } @@ -122,8 +138,8 @@ func (e *errMissingVariables) Error() string { // inspectVariables parses through the yaml and returns a map of the variable // names and if they have default values. It returns an error if it cannot // parse the yaml. -func inspectVariables(data string) (map[string]bool, error) { - variables := make(map[string]bool) +func inspectVariables(data string) (map[string]string, error) { + variables := make(map[string]string) t, err := parse.Parse(data) if err != nil { return nil, err @@ -134,7 +150,7 @@ func inspectVariables(data string) (map[string]bool, error) { // traverse recursively walks down the root node and tracks the variables // which are FuncNodes and if the variables have default values. -func traverse(root parse.Node, variables map[string]bool) { +func traverse(root parse.Node, variables map[string]string) { switch v := root.(type) { case *parse.ListNode: // iterate through the list node @@ -143,8 +159,19 @@ func traverse(root parse.Node, variables map[string]bool) { } case *parse.FuncNode: if _, ok := variables[v.Param]; !ok { - // if there are args, then the variable has a default value - variables[v.Param] = len(v.Args) > 0 + // Build up a default value string + b := strings.Builder{} + for _, a := range v.Args { + switch w := a.(type) { + case *parse.FuncNode: + b.WriteString(fmt.Sprintf("${%s}", w.Param)) + case *parse.TextNode: + b.WriteString(w.Value) + } + } + // Key the variable name to its default string from the template, + // or to an empty string if it's required (no default). + variables[v.Param] = b.String() } } } diff --git a/cmd/clusterctl/client/yamlprocessor/simple_processor_test.go b/cmd/clusterctl/client/yamlprocessor/simple_processor_test.go index 069257e0e134..9d06d9f89760 100644 --- a/cmd/clusterctl/client/yamlprocessor/simple_processor_test.go +++ b/cmd/clusterctl/client/yamlprocessor/simple_processor_test.go @@ -99,6 +99,72 @@ func TestSimpleProcessor_GetVariables(t *testing.T) { } } +func TestSimpleProcessor_GetVariablesMap(t *testing.T) { + type args struct { + data string + } + def := "default" + aVar := "${A}" + foobar := "foobar" + quotes := `""` + tests := []struct { + name string + args args + want map[string]*string + wantErr bool + }{ + { + name: "variable with different spacing around the name", + args: args{ + data: "yaml with ${A} ${ B} ${ C} ${ D }", + }, + want: map[string]*string{"A": nil, "B": nil, "C": nil, "D": nil}, + }, + { + name: "variables used in many places are grouped", + args: args{ + data: "yaml with ${A } ${A} ${A}", + }, + want: map[string]*string{"A": nil}, + }, + { + name: "variables in multiline texts are processed", + args: args{ + data: "yaml with ${A}\n${B}\n${C}", + }, + want: map[string]*string{"A": nil, "B": nil, "C": nil}, + }, + { + name: "returns error for variables with regex metacharacters", + args: args{ + data: "yaml with ${BA$R}\n${FOO}", + }, + wantErr: true, + }, + { + name: "variables with envsubst functions are properly parsed", + args: args{ + data: `yaml with ${C:=default}\n${B}\n${A=foobar}\n${E=""}\n${D:=${A}}`, + }, + want: map[string]*string{"A": &foobar, "B": nil, "C": &def, "D": &aVar, "E": "es}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + p := NewSimpleProcessor() + actual, err := p.GetVariableMap([]byte(tt.args.data)) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + return + } + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(actual).To(Equal(tt.want)) + }) + } +} + func TestSimpleProcessor_Process(t *testing.T) { type args struct { yaml []byte diff --git a/cmd/clusterctl/cmd/generate_cluster.go b/cmd/clusterctl/cmd/generate_cluster.go index ce4e44eb9840..7ccbac9820c2 100644 --- a/cmd/clusterctl/cmd/generate_cluster.go +++ b/cmd/clusterctl/cmd/generate_cluster.go @@ -182,7 +182,7 @@ func runGenerateClusterTemplate(cmd *cobra.Command, name string) error { } if gc.listVariables { - return printVariablesOutput(template) + return printVariablesOutput(cmd, template) } return printYamlOutput(template) diff --git a/cmd/clusterctl/cmd/util.go b/cmd/clusterctl/cmd/util.go index 8839aa27d6ea..d4fc25aad044 100644 --- a/cmd/clusterctl/cmd/util.go +++ b/cmd/clusterctl/cmd/util.go @@ -20,9 +20,13 @@ import ( "fmt" "os" "path/filepath" + "sort" + "strconv" "strings" + "text/tabwriter" "github.com/pkg/errors" + "github.com/spf13/cobra" "sigs.k8s.io/cluster-api/cmd/clusterctl/client" ) @@ -41,14 +45,67 @@ func printYamlOutput(printer client.YamlPrinter) error { return nil } +func stringPtr(s string) *string { + return &s +} + // printVariablesOutput prints the expected variables in the template to stdout. -func printVariablesOutput(printer client.YamlPrinter) error { - if len(printer.Variables()) > 0 { - fmt.Println("Variables:") - for _, v := range printer.Variables() { +func printVariablesOutput(cmd *cobra.Command, template client.Template) error { + // Decorate the variable map for printing + variableMap := template.VariableMap() + for name, defaultValue := range variableMap { + if defaultValue != nil { + v := *defaultValue + // Add quotes around any unquoted strings + if len(v) > 0 && !strings.HasPrefix(v, "\"") { + v = fmt.Sprintf("\"%s\"", v) + variableMap[name] = &v + } + } + } + // Add variables that are defaulted from clusterctl + controlPlaneMachineCount, err := cmd.Flags().GetInt64("control-plane-machine-count") + if err != nil { + return err + } + variableMap["CONTROL_PLANE_MACHINE_COUNT"] = stringPtr(strconv.FormatInt(controlPlaneMachineCount, 10)) + workerMachineCount, err := cmd.Flags().GetInt64("worker-machine-count") + if err != nil { + return err + } + variableMap["WORKER_MACHINE_COUNT"] = stringPtr(strconv.FormatInt(workerMachineCount, 10)) + variableMap["KUBERNETES_VERSION"] = stringPtr("the value of --kubernetes-version") + variableMap["CLUSTER_NAME"] = stringPtr(" from \"clusterctl config cluster \"") + + // transform variable map into required and optional lists + var requiredVariables []string + var optionalVariables []string + for name, defaultValue := range variableMap { + if defaultValue != nil { + optionalVariables = append(optionalVariables, name) + } else { + requiredVariables = append(requiredVariables, name) + } + } + sort.Strings(requiredVariables) + sort.Strings(optionalVariables) + + if len(requiredVariables) > 0 { + fmt.Println("Required Variables:") + for _, v := range requiredVariables { fmt.Printf(" - %s\n", v) } } + + if len(optionalVariables) > 0 { + fmt.Println("\nOptional Variables:") + w := tabwriter.NewWriter(os.Stdout, 0, 4, 2, ' ', tabwriter.FilterHTML) + for _, v := range optionalVariables { + fmt.Fprintf(w, " - %s\t(defaults to %s)\n", v, *variableMap[v]) + } + w.Flush() + } + fmt.Println() return nil } diff --git a/cmd/clusterctl/internal/test/fake_processor.go b/cmd/clusterctl/internal/test/fake_processor.go index 63d1960ea623..7006fc580448 100644 --- a/cmd/clusterctl/internal/test/fake_processor.go +++ b/cmd/clusterctl/internal/test/fake_processor.go @@ -17,9 +17,10 @@ limitations under the License. package test type FakeProcessor struct { - errGetVariables error - errProcess error - artifactName string + errGetVariables error + errGetVariableMap error + errProcess error + artifactName string } func NewFakeProcessor() *FakeProcessor { @@ -49,6 +50,10 @@ func (fp *FakeProcessor) GetVariables(raw []byte) ([]string, error) { return nil, fp.errGetVariables } +func (fp *FakeProcessor) GetVariableMap(raw []byte) (map[string]*string, error) { + return nil, fp.errGetVariableMap +} + func (fp *FakeProcessor) Process(raw []byte, variablesGetter func(string) (string, error)) ([]byte, error) { return nil, fp.errProcess }