From 9195a6238c17913b0df50cfb947037ce2456d4f9 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Wed, 14 May 2025 23:50:29 -0400 Subject: [PATCH 1/2] Improve support for maps and slices --- pkg/generators/terraform_generator.go | 108 +++-- pkg/generators/terraform_generator_test.go | 498 +++++++-------------- 2 files changed, 229 insertions(+), 377 deletions(-) diff --git a/pkg/generators/terraform_generator.go b/pkg/generators/terraform_generator.go index 2b8074820..00d0696cc 100644 --- a/pkg/generators/terraform_generator.go +++ b/pkg/generators/terraform_generator.go @@ -531,8 +531,8 @@ func (g *TerraformGenerator) parseVariablesFile(variablesTfPath string, protecte // Helper Functions // ============================================================================= -// addTfvarsHeader adds a header comment to the tfvars file indicating Windsor CLI management. -// It includes the module source if provided. +// addTfvarsHeader adds a Windsor CLI management header to the tfvars file body. +// It includes a module source comment if provided, ensuring users are aware of CLI management and module provenance. func addTfvarsHeader(body *hclwrite.Body, source string) { windsorHeaderToken := "Managed by Windsor CLI:" headerComment := fmt.Sprintf("# %s This file is partially managed by the windsor CLI. Your changes will not be overwritten.", windsorHeaderToken) @@ -546,8 +546,9 @@ func addTfvarsHeader(body *hclwrite.Body, source string) { } } -// writeComponentValues writes component-provided values to the tfvars file. -// It processes all variables in the order they appear in variables.tf. +// writeComponentValues writes all component-provided or default variable values to the tfvars file body. +// It comments out default values and descriptions for unset variables, and writes explicit values for set variables. +// Handles sensitive variables and preserves variable order from variables.tf. func writeComponentValues(body *hclwrite.Body, values map[string]any, protectedValues map[string]bool, variables []VariableInfo) { for _, info := range variables { if protectedValues[info.Name] { @@ -556,7 +557,6 @@ func writeComponentValues(body *hclwrite.Body, values map[string]any, protectedV body.AppendNewline() - // Write description if available if info.Description != "" { body.AppendUnstructuredTokens(hclwrite.Tokens{ {Type: hclsyntax.TokenComment, Bytes: []byte("# " + info.Description)}, @@ -564,13 +564,11 @@ func writeComponentValues(body *hclwrite.Body, values map[string]any, protectedV body.AppendNewline() } - // If value is provided in component values, use it if val, exists := values[info.Name]; exists { - writeVariable(body, info.Name, val, []VariableInfo{}) // Pass empty variables to avoid duplicate description + writeVariable(body, info.Name, val, []VariableInfo{}) continue } - // For sensitive values, write them as commented sensitive if info.Sensitive { body.AppendUnstructuredTokens(hclwrite.Tokens{ {Type: hclsyntax.TokenComment, Bytes: []byte(fmt.Sprintf("# %s = \"(sensitive)\"", info.Name))}, @@ -579,45 +577,42 @@ func writeComponentValues(body *hclwrite.Body, values map[string]any, protectedV continue } - // Comment out the default value or null - defaultStr := "null" if info.Default != nil { defaultVal := convertToCtyValue(info.Default) if !defaultVal.IsNull() { + var rendered string if defaultVal.Type().IsObjectType() || defaultVal.Type().IsMapType() { - // For objects/maps, format with proper indentation and comment each line var mapStr strings.Builder - mapStr.WriteString(fmt.Sprintf("# %s = {\n", info.Name)) - it := defaultVal.ElementIterator() - for it.Next() { - k, v := it.Element() - mapStr.WriteString(fmt.Sprintf("# %s = %s\n", k.AsString(), formatValue(convertFromCtyValue(v)))) - } - mapStr.WriteString("# }") + mapStr.WriteString(fmt.Sprintf("%s = %s", info.Name, formatValue(convertFromCtyValue(defaultVal)))) + rendered = mapStr.String() + } else { + rendered = fmt.Sprintf("%s = %s", info.Name, string(hclwrite.TokensForValue(defaultVal).Bytes())) + } + for _, line := range strings.Split(rendered, "\n") { body.AppendUnstructuredTokens(hclwrite.Tokens{ - {Type: hclsyntax.TokenComment, Bytes: []byte(mapStr.String())}, + {Type: hclsyntax.TokenComment, Bytes: []byte("# " + line)}, }) body.AppendNewline() - continue - } else { - defaultStr = string(hclwrite.TokensForValue(defaultVal).Bytes()) } + continue } } + // If no default, just comment null body.AppendUnstructuredTokens(hclwrite.Tokens{ - {Type: hclsyntax.TokenComment, Bytes: []byte(fmt.Sprintf("# %s = %s", info.Name, defaultStr))}, + {Type: hclsyntax.TokenComment, Bytes: []byte(fmt.Sprintf("# %s = null", info.Name))}, }) body.AppendNewline() } } -// writeDefaultValues writes default values from variables.tf to the tfvars file. -// This is now just an alias for writeComponentValues with empty values +// writeDefaultValues writes only the default values from variables.tf to the tfvars file body. +// This is an alias for writeComponentValues with no explicit values, ensuring all defaults are commented. func writeDefaultValues(body *hclwrite.Body, variables []VariableInfo, componentValues map[string]any) { writeComponentValues(body, componentValues, map[string]bool{}, variables) } -// writeHeredoc writes a variable value as a heredoc. +// writeHeredoc writes a multi-line string value as a heredoc assignment in the tfvars file body. +// Used for YAML or other multi-line string values to preserve formatting. func writeHeredoc(body *hclwrite.Body, name string, content string) { tokens := hclwrite.Tokens{ {Type: hclsyntax.TokenOHeredoc, Bytes: []byte("<") { - t.Errorf("missing nil value: %s = ", k) - } - case string: - if val == "" { - if !strings.Contains(output, k+` = ""`) { - t.Errorf("missing empty string: %s = \"\"", k) - } - } else { - if !strings.Contains(output, k+` = "`+val+`"`) { - t.Errorf("missing string value: %s = \"%s\"", k, val) - } - } - case bool: - if !strings.Contains(output, k+" = "+fmt.Sprintf("%v", val)) { - t.Errorf("missing boolean value: %s = %v", k, val) - } - case int: - if !strings.Contains(output, k+" = "+fmt.Sprintf("%v", val)) { - t.Errorf("missing integer value: %s = %v", k, val) - } - } - } -} - -func TestTerraformGenerator_writeShimOutputsTf(t *testing.T) { - setup := func(t *testing.T) (*TerraformGenerator, *Mocks) { - mocks := setupMocks(t) - generator := NewTerraformGenerator(mocks.Injector) - generator.shims = mocks.Shims - if err := generator.Initialize(); err != nil { - t.Fatalf("failed to initialize TerraformGenerator: %v", err) - } - return generator, mocks - } - - t.Run("Success", func(t *testing.T) { - // Given a TerraformGenerator with mocks - generator, mocks := setup(t) - - // And Stat is mocked to return success for outputs.tf - mocks.Shims.Stat = func(path string) (fs.FileInfo, error) { - if strings.HasSuffix(path, "outputs.tf") { - return nil, nil - } - return nil, os.ErrNotExist - } - - // And ReadFile is mocked to return content for outputs.tf - mocks.Shims.ReadFile = func(path string) ([]byte, error) { - if strings.HasSuffix(path, "outputs.tf") { - return []byte(`output "test" { - description = "Test output" - value = "test" -}`), nil - } - return nil, fmt.Errorf("unexpected file read: %s", path) - } - - // When writeShimOutputsTf is called - err := generator.writeShimOutputsTf("test_dir", "test_path") - - // Then no error should occur - if err != nil { - t.Errorf("expected no error, got %v", err) +func Test_formatValue(t *testing.T) { + t.Run("EmptyArray", func(t *testing.T) { + result := formatValue([]any{}) + if result != "[]" { + t.Errorf("expected [] got %q", result) } }) - - t.Run("OutputsFileDoesNotExist", func(t *testing.T) { - // Given a TerraformGenerator with mocks - generator, mocks := setup(t) - - // And Stat is mocked to return not exist for outputs.tf - mocks.Shims.Stat = func(path string) (fs.FileInfo, error) { - return nil, os.ErrNotExist + t.Run("EmptyMap", func(t *testing.T) { + result := formatValue(map[string]any{}) + if result != "{}" { + t.Errorf("expected {} got %q", result) } - - // When writeShimOutputsTf is called - err := generator.writeShimOutputsTf("test_dir", "test_path") - - // Then no error should occur - if err != nil { - t.Errorf("expected no error, got %v", err) + }) + t.Run("NilValue", func(t *testing.T) { + result := formatValue(nil) + if result != "null" { + t.Errorf("expected null got %q", result) } }) - - t.Run("ErrorReadingOutputs", func(t *testing.T) { - // Given a TerraformGenerator with mocks - generator, mocks := setup(t) - - // And Stat is mocked to return success for outputs.tf - mocks.Shims.Stat = func(path string) (fs.FileInfo, error) { - if strings.HasSuffix(path, "outputs.tf") { - return nil, nil - } - return nil, os.ErrNotExist + t.Run("ComplexNestedObject", func(t *testing.T) { + input := map[string]any{ + "node_groups": map[string]any{ + "default": map[string]any{ + "instance_types": []any{"t3.medium"}, + "min_size": 1, + "max_size": 3, + "desired_size": 2, + }, + }, } - - // And ReadFile is mocked to return an error for outputs.tf - mocks.Shims.ReadFile = func(path string) ([]byte, error) { - if strings.HasSuffix(path, "outputs.tf") { - return nil, fmt.Errorf("mock error reading outputs.tf") - } - return nil, fmt.Errorf("unexpected file read: %s", path) + expected := `{ + node_groups = { + default = { + desired_size = 2 + instance_types = ["t3.medium"] + max_size = 3 + min_size = 1 + } + } +}` + result := formatValue(input) + if result != expected { + t.Errorf("expected %q, got %q", expected, result) } - - // When writeShimOutputsTf is called - err := generator.writeShimOutputsTf("test_dir", "test_path") - - // Then an error should be returned - if err == nil { - t.Fatalf("expected an error, got nil") + }) + t.Run("EmptyAddons", func(t *testing.T) { + input := map[string]any{ + "addons": map[string]any{ + "vpc-cni": map[string]any{}, + "aws-efs-csi-driver": map[string]any{}, + "aws-ebs-csi-driver": map[string]any{}, + "eks-pod-identity-agent": map[string]any{}, + "coredns": map[string]any{}, + "external-dns": map[string]any{}, + }, } - - // And the error should match the expected error - expectedError := "failed to read outputs.tf: mock error reading outputs.tf" - if err.Error() != expectedError { - t.Errorf("expected error %s, got %s", expectedError, err.Error()) + expected := `{ + addons = { + aws-ebs-csi-driver = {} + aws-efs-csi-driver = {} + coredns = {} + eks-pod-identity-agent = {} + external-dns = {} + vpc-cni = {} + } +}` + result := formatValue(input) + if result != expected { + t.Errorf("expected %q, got %q", expected, result) } }) +} - t.Run("ErrorParsingOutputs", func(t *testing.T) { - // Given a TerraformGenerator with mocks - generator, mocks := setup(t) - - // And Stat is mocked to return success for outputs.tf - mocks.Shims.Stat = func(path string) (fs.FileInfo, error) { - if strings.HasSuffix(path, "outputs.tf") { - return nil, nil - } - return nil, os.ErrNotExist +func Test_writeVariable(t *testing.T) { + t.Run("ComplexObject_node_groups", func(t *testing.T) { + file := hclwrite.NewEmptyFile() + writeVariable(file.Body(), "node_groups", map[string]any{ + "node_groups": map[string]any{ + "default": map[string]any{ + "instance_types": []any{"t3.medium"}, + "min_size": 1, + "max_size": 3, + "desired_size": 2, + }, + }, + }, nil) + expected := "node_groups = {\n node_groups = {\n default = {\n desired_size = 2\n instance_types = [\"t3.medium\"]\n max_size = 3\n min_size = 1\n }\n }\n}" + result := strings.TrimSpace(string(file.Bytes())) + if result != expected { + t.Errorf("expected\n%s\ngot\n%s", expected, result) } - - // And ReadFile is mocked to return invalid HCL for outputs.tf - mocks.Shims.ReadFile = func(path string) ([]byte, error) { - if strings.HasSuffix(path, "outputs.tf") { - return []byte(`invalid hcl`), nil - } - return nil, fmt.Errorf("unexpected file read: %s", path) + }) + t.Run("ComplexObject_addons", func(t *testing.T) { + file := hclwrite.NewEmptyFile() + writeVariable(file.Body(), "addons", map[string]any{ + "addons": map[string]any{ + "vpc-cni": map[string]any{}, + "aws-efs-csi-driver": map[string]any{}, + "aws-ebs-csi-driver": map[string]any{}, + "eks-pod-identity-agent": map[string]any{}, + "coredns": map[string]any{}, + "external-dns": map[string]any{}, + }, + }, nil) + expected := "addons = {\n addons = {\n aws-ebs-csi-driver = {}\n aws-efs-csi-driver = {}\n coredns = {}\n eks-pod-identity-agent = {}\n external-dns = {}\n vpc-cni = {}\n }\n}" + result := strings.TrimSpace(string(file.Bytes())) + if result != expected { + t.Errorf("expected\n%s\ngot\n%s", expected, result) } - - // When writeShimOutputsTf is called - err := generator.writeShimOutputsTf("test_dir", "test_path") - - // Then an error should be returned - if err == nil { - t.Fatalf("expected an error, got nil") + }) + t.Run("ObjectAssignment_no_heredoc", func(t *testing.T) { + file := hclwrite.NewEmptyFile() + value := map[string]any{ + "default": map[string]any{ + "desired_size": 2, + "instance_types": []any{"t3.medium"}, + "max_size": 3, + "min_size": 1, + }, } - - // And the error should indicate parsing failure - expectedError := "failed to parse outputs.tf" - if !strings.Contains(err.Error(), expectedError) { - t.Errorf("expected error containing %q, got %q", expectedError, err.Error()) + writeVariable(file.Body(), "node_groups", value, nil) + expected := "node_groups = {\n default = {\n desired_size = 2\n instance_types = [\"t3.medium\"]\n max_size = 3\n min_size = 1\n }\n}" + result := strings.TrimSpace(string(file.Bytes())) + if result != expected { + t.Errorf("expected\n%s\ngot\n%s", expected, result) } }) +} - t.Run("ErrorWritingOutputs", func(t *testing.T) { - // Given a TerraformGenerator with mocks - generator, mocks := setup(t) - - // And Stat is mocked to return success for outputs.tf - mocks.Shims.Stat = func(path string) (fs.FileInfo, error) { - if strings.HasSuffix(path, "outputs.tf") { - return nil, nil - } - return nil, os.ErrNotExist +func Test_writeComponentValues(t *testing.T) { + t.Run("ComplexDefaults_NodeGroups", func(t *testing.T) { + file := hclwrite.NewEmptyFile() + variable := VariableInfo{ + Name: "node_groups", + Description: "Map of EKS managed node group definitions to create.", + Default: map[string]any{ + "default": map[string]any{ + "instance_types": []any{"t3.medium"}, + "min_size": 1, + "max_size": 3, + "desired_size": 2, + }, + }, } - - // And ReadFile is mocked to return content for outputs.tf - mocks.Shims.ReadFile = func(path string) ([]byte, error) { - if strings.HasSuffix(path, "outputs.tf") { - return []byte(`output "test" { - description = "Test output" - value = "test" -}`), nil + writeComponentValues(file.Body(), map[string]any{}, map[string]bool{}, []VariableInfo{variable}) + expected := ` +# Map of EKS managed node group definitions to create. +# node_groups = { +# default = { +# desired_size = 2 +# instance_types = ["t3.medium"] +# max_size = 3 +# min_size = 1 +# } +# }` + result := string(file.Bytes()) + // Check that every line is commented + for _, line := range strings.Split(result, "\n") { + if strings.TrimSpace(line) == "" { + continue } - return nil, fmt.Errorf("unexpected file read: %s", path) - } - - // And WriteFile is mocked to return an error for outputs.tf - mocks.Shims.WriteFile = func(path string, _ []byte, _ fs.FileMode) error { - if strings.HasSuffix(path, "outputs.tf") { - return fmt.Errorf("mock error writing outputs.tf") + if !strings.HasPrefix(line, "#") { + t.Errorf("uncommented line found: %q", line) } - return nil - } - - // When writeShimOutputsTf is called - err := generator.writeShimOutputsTf("test_dir", "test_path") - - // Then an error should be returned - if err == nil { - t.Fatalf("expected an error, got nil") } - - // And the error should match the expected error - expectedError := "failed to write shim outputs.tf: mock error writing outputs.tf" - if err.Error() != expectedError { - t.Errorf("expected error %s, got %s", expectedError, err.Error()) + // Check that the output matches expected ignoring leading/trailing whitespace + if strings.TrimSpace(result) != strings.TrimSpace(expected) { + t.Errorf("expected\n%s\ngot\n%s", expected, result) } }) - - t.Run("NoValidOutputBlocks", func(t *testing.T) { - // Given a TerraformGenerator with mocks - generator, mocks := setup(t) - - // And Stat is mocked to return success for outputs.tf - mocks.Shims.Stat = func(path string) (fs.FileInfo, error) { - if strings.HasSuffix(path, "outputs.tf") { - return nil, nil - } - return nil, os.ErrNotExist - } - - // And ReadFile is mocked to return content with no valid output blocks - mocks.Shims.ReadFile = func(path string) ([]byte, error) { - if strings.HasSuffix(path, "outputs.tf") { - return []byte(`# No output blocks here`), nil - } - return nil, fmt.Errorf("unexpected file read: %s", path) + t.Run("ComplexDefaults_EmptyAddons", func(t *testing.T) { + file := hclwrite.NewEmptyFile() + variable := VariableInfo{ + Name: "addons", + Description: "Map of EKS add-ons", + Default: map[string]any{ + "vpc-cni": map[string]any{}, + "aws-efs-csi-driver": map[string]any{}, + "aws-ebs-csi-driver": map[string]any{}, + "eks-pod-identity-agent": map[string]any{}, + "coredns": map[string]any{}, + "external-dns": map[string]any{}, + }, } - - // When writeShimOutputsTf is called - err := generator.writeShimOutputsTf("test_dir", "test_path") - - // Then no error should occur - if err != nil { - t.Errorf("expected no error, got %v", err) + writeComponentValues(file.Body(), map[string]any{}, map[string]bool{}, []VariableInfo{variable}) + expected := "\n# Map of EKS add-ons\n# addons = {\n# aws-ebs-csi-driver = {}\n# aws-efs-csi-driver = {}\n# coredns = {}\n# eks-pod-identity-agent = {}\n# external-dns = {}\n# vpc-cni = {}\n# }\n" + result := string(file.Bytes()) + if result != expected { + t.Errorf("expected %q, got %q", expected, result) } }) } From 8b4c4d2d3501d1c64be002b4776f9f2c61c949b2 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy Date: Thu, 15 May 2025 00:04:53 -0400 Subject: [PATCH 2/2] Test fix --- pkg/generators/terraform_generator_test.go | 84 +++++++--------------- 1 file changed, 26 insertions(+), 58 deletions(-) diff --git a/pkg/generators/terraform_generator_test.go b/pkg/generators/terraform_generator_test.go index 82a7fd2ca..e438983e0 100644 --- a/pkg/generators/terraform_generator_test.go +++ b/pkg/generators/terraform_generator_test.go @@ -446,12 +446,12 @@ last line`, }, }, expected: `{ - string = "value" - number = 42 bool = true nested = { key = "value" } + number = 42 + string = "value" }`, checkMap: true, }, @@ -462,70 +462,38 @@ last line`, file := hclwrite.NewEmptyFile() writeVariable(file.Body(), "test_var", tt.value, nil) - // Extract the heredoc content content := string(file.Bytes()) - lines := strings.Split(content, "\n") - var actualLines []string - inHeredoc := false - for _, line := range lines { - if strings.Contains(line, "<