From 7af38480a28b2c33007e9b0aa2e4fd3c5d9d264a Mon Sep 17 00:00:00 2001 From: James Humphries Date: Wed, 19 Feb 2025 15:50:58 +0000 Subject: [PATCH] fix(genconfig): properly quote attribute names when necessary Ensure attribute names are correctly quoted in generated config when they contain characters that make them invalid identifiers (e.g., dots, double quotes). - Use `hclsyntax.ValidIdentifier` to check if an attribute name needs quoting. - Convert invalid identifiers to valid HCL strings using `hclwrite.TokensForValue`. - Add test cases for attributes with dots and double quotes to verify correctness. Signed-off-by: James Humphries --- internal/genconfig/generate_config.go | 9 ++ internal/genconfig/generate_config_test.go | 106 ++++++++++++++++++++- 2 files changed, 111 insertions(+), 4 deletions(-) diff --git a/internal/genconfig/generate_config.go b/internal/genconfig/generate_config.go index 8f4ff32395..048280a9b3 100644 --- a/internal/genconfig/generate_config.go +++ b/internal/genconfig/generate_config.go @@ -12,6 +12,7 @@ import ( "strings" "github.com/hashicorp/hcl/v2" + hclsyntax "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/hashicorp/hcl/v2/hclwrite" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/json" @@ -90,6 +91,10 @@ func writeConfigAttributes(addr addrs.AbsResourceInstance, buf *strings.Builder, } if attrS.Required { buf.WriteString(strings.Repeat(" ", indent)) + // Handle cases where the name should be contained in quotes + if !hclsyntax.ValidIdentifier(name) { + name = string(hclwrite.TokensForValue(cty.StringVal(name)).Bytes()) + } buf.WriteString(fmt.Sprintf("%s = ", name)) tok := hclwrite.TokensForValue(attrS.EmptyValue()) if _, err := tok.WriteTo(buf); err != nil { @@ -104,6 +109,10 @@ func writeConfigAttributes(addr addrs.AbsResourceInstance, buf *strings.Builder, writeAttrTypeConstraint(buf, attrS) } else if attrS.Optional { buf.WriteString(strings.Repeat(" ", indent)) + // Handle cases where the name should be contained in quotes + if !hclsyntax.ValidIdentifier(name) { + name = string(hclwrite.TokensForValue(cty.StringVal(name)).Bytes()) + } buf.WriteString(fmt.Sprintf("%s = ", name)) tok := hclwrite.TokensForValue(attrS.EmptyValue()) if _, err := tok.WriteTo(buf); err != nil { diff --git a/internal/genconfig/generate_config_test.go b/internal/genconfig/generate_config_test.go index a0dda0ef94..2d6169b2f4 100644 --- a/internal/genconfig/generate_config_test.go +++ b/internal/genconfig/generate_config_test.go @@ -82,6 +82,104 @@ resource "tfcoremock_simple_resource" "empty" { list_block { # OPTIONAL block nested_value = null # OPTIONAL string } +}`, + }, + "simple_resource_with_propertyname_containing_a_dot": { + schema: &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "list_block": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "nested_value.json": { + Type: cty.String, + Optional: true, + }, + }, + }, + Nesting: configschema.NestingSingle, + }, + }, + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + }, + "value": { + Type: cty.String, + Optional: true, + }, + }, + }, + addr: addrs.AbsResourceInstance{ + Module: nil, + Resource: addrs.ResourceInstance{ + Resource: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "tfcoremock_simple_resource", + Name: "empty", + }, + Key: nil, + }, + }, + provider: addrs.LocalProviderConfig{ + LocalName: "tfcoremock", + }, + value: cty.NilVal, + expected: ` +resource "tfcoremock_simple_resource" "empty" { + value = null # OPTIONAL string + list_block { # OPTIONAL block + "nested_value.json" = null # OPTIONAL string + } +}`, + }, + "simple_resource_with_propertyname_containing_double_quotes": { + schema: &configschema.Block{ + BlockTypes: map[string]*configschema.NestedBlock{ + "list_block": { + Block: configschema.Block{ + Attributes: map[string]*configschema.Attribute{ + "nested_value\"example": { + Type: cty.String, + Optional: true, + }, + }, + }, + Nesting: configschema.NestingSingle, + }, + }, + Attributes: map[string]*configschema.Attribute{ + "id": { + Type: cty.String, + Computed: true, + }, + "value": { + Type: cty.String, + Optional: true, + }, + }, + }, + addr: addrs.AbsResourceInstance{ + Module: nil, + Resource: addrs.ResourceInstance{ + Resource: addrs.Resource{ + Mode: addrs.ManagedResourceMode, + Type: "tfcoremock_simple_resource", + Name: "empty", + }, + Key: nil, + }, + }, + provider: addrs.LocalProviderConfig{ + LocalName: "tfcoremock", + }, + value: cty.NilVal, + expected: ` +resource "tfcoremock_simple_resource" "empty" { + value = null # OPTIONAL string + list_block { # OPTIONAL block + "nested_value\"example" = null # OPTIONAL string + } }`, }, "simple_resource_with_state": { @@ -670,20 +768,20 @@ resource "tfcoremock_simple_resource" "example" { }, } for name, tc := range tcs { - t.Run(name, func(t *testing.T) { + t.Run(name, func(tt *testing.T) { err := tc.schema.InternalValidate() if err != nil { - t.Fatalf("schema failed InternalValidate: %s", err) + tt.Fatalf("schema failed InternalValidate: %s", err) } contents, diags := GenerateResourceContents(tc.addr, tc.schema, tc.provider, tc.value) if len(diags) > 0 { - t.Errorf("expected no diagnostics but found %s", diags) + tt.Errorf("expected no diagnostics but found %s", diags) } got := WrapResourceContents(tc.addr, contents) want := strings.TrimSpace(tc.expected) if diff := cmp.Diff(got, want); len(diff) > 0 { - t.Errorf("got:\n%s\nwant:\n%s\ndiff:\n%s", got, want, diff) + tt.Errorf("got:\n%s\nwant:\n%s\ndiff:\n%s", got, want, diff) } }) }