diff --git a/pkg/tests/internal/tfcheck/tf_test.go b/pkg/tests/internal/tfcheck/tf_test.go index 71fd6867c..ff817a148 100644 --- a/pkg/tests/internal/tfcheck/tf_test.go +++ b/pkg/tests/internal/tfcheck/tf_test.go @@ -54,6 +54,67 @@ resource "test_resource" "test" { t.Logf(driver.GetState(t)) } +// TestTfMapMissingElem shows that maps missing Elem types are equivalent to specifying: +// +// Elem: &schema.Schema{Type: schema.TypeString} +// +// Previously, the bridge treated a missing map element type as `schema.TypeAny` instead +// of `schema.TypeString`, which caused provider panics. For example: +// +// - https://github.com/pulumi/pulumi-nomad/issues/389 +func TestTfMapMissingElem(t *testing.T) { + prov := schema.Provider{ + ResourcesMap: map[string]*schema.Resource{ + "test_resource": { + Schema: map[string]*schema.Schema{ + "m": { + Type: schema.TypeMap, + Required: true, + }, + }, + CreateContext: func(ctx context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics { + var errs diag.Diagnostics + if _, ok := d.Get("m").(map[string]any)["string"].(string); !ok { + errs = append(errs, diag.Errorf(`expected m["string"] to be a string`)...) + } + if _, ok := d.Get("m").(map[string]any)["number"].(string); !ok { + errs = append(errs, diag.Errorf(`expected m["number"] to be a string`)...) + } + + d.SetId("test") + return errs + }, + }, + }, + } + + driver := NewTfDriver(t, t.TempDir(), "test", &prov) + + driver.Write(t, ` +resource "test_resource" "test" { + m = { + "string" = "123" + "number" = 123 + } +} +`, + ) + + plan := driver.Plan(t) + t.Logf(driver.Show(t, plan.PlanFile)) + driver.Apply(t, plan) + + t.Logf(driver.GetState(t)) + + newPlan := driver.Plan(t) + + t.Logf(driver.Show(t, plan.PlanFile)) + + driver.Apply(t, newPlan) + + t.Logf(driver.GetState(t)) +} + func TestTfUnknownObjects(t *testing.T) { prov := schema.Provider{ ResourcesMap: map[string]*schema.Resource{ diff --git a/pkg/tfgen/generate.go b/pkg/tfgen/generate.go index 2c7477cfa..43ab851d6 100644 --- a/pkg/tfgen/generate.go +++ b/pkg/tfgen/generate.go @@ -319,7 +319,7 @@ type moduleMember interface { type typeKind int const ( - kindInvalid = iota + kindInvalid typeKind = iota kindBool kindInt kindFloat @@ -330,9 +330,6 @@ const ( kindObject ) -// Avoid an unused warning from varcheck. -var _ = kindInvalid - // propertyType represents a non-resource, non-datasource type. Property types may be simple type propertyType struct { name string @@ -431,6 +428,10 @@ func (g *Generator) makePropertyType(typePath paths.TypePath, switch sch.Type() { case shim.TypeMap: t.kind = kindMap + // TF treats maps without a specified type as map[string]string, so we do the same. + if element == nil || element.kind == kindInvalid { + element = &propertyType{kind: kindString} + } case shim.TypeList: t.kind = kindList case shim.TypeSet: diff --git a/pkg/tfgen/generate_test.go b/pkg/tfgen/generate_test.go index aabede344..afd9937ee 100644 --- a/pkg/tfgen/generate_test.go +++ b/pkg/tfgen/generate_test.go @@ -166,7 +166,7 @@ func Test_makePropertyType(t *testing.T) { t.Run("String", func(t *testing.T) { p, err := g.makePropertyType(path, "obj", strType, nil, false, entityDocs{}) require.NoError(t, err) - assert.Equal(t, typeKind(kindString), p.kind) + assert.Equal(t, kindString, p.kind) }) //TODO: change this test to assert typeKind when implementing @@ -183,8 +183,8 @@ func Test_makePropertyType(t *testing.T) { }).Shim() p, err := g.makePropertyType(path, "obj", strListType, nil, false, entityDocs{}) require.NoError(t, err) - assert.Equal(t, typeKind(kindList), p.kind) - assert.Equal(t, typeKind(kindString), p.element.kind) + assert.Equal(t, kindList, p.kind) + assert.Equal(t, kindString, p.element.kind) }) t.Run("MapString", func(t *testing.T) { @@ -194,8 +194,8 @@ func Test_makePropertyType(t *testing.T) { }).Shim() p, err := g.makePropertyType(path, "obj", strMapType, nil, false, entityDocs{}) require.NoError(t, err) - assert.Equal(t, typeKind(kindMap), p.kind) - assert.Equal(t, typeKind(kindString), p.element.kind) + assert.Equal(t, kindMap, p.kind) + assert.Equal(t, kindString, p.element.kind) }) t.Run("MapUnknown", func(t *testing.T) { @@ -204,8 +204,8 @@ func Test_makePropertyType(t *testing.T) { }).Shim() p, err := g.makePropertyType(path, "obj", unkMapType, nil, false, entityDocs{}) require.NoError(t, err) - assert.Equal(t, typeKind(kindMap), p.kind) - assert.Nil(t, p.element) + assert.Equal(t, kindMap, p.kind) + assert.Equal(t, kindString, p.element.kind) }) t.Run("SingleNestedBlock", func(t *testing.T) { @@ -215,7 +215,7 @@ func Test_makePropertyType(t *testing.T) { }).Shim() p, err := g.makePropertyType(path, "obj", objType, nil, false, entityDocs{}) require.NoError(t, err) - assert.Equal(t, typeKind(kindObject), p.kind) + assert.Equal(t, kindObject, p.kind) assert.Equal(t, "config.prop", p.properties[0].parentPath.String()) }) @@ -226,8 +226,8 @@ func Test_makePropertyType(t *testing.T) { }).Shim() p, err := g.makePropertyType(path, "obj", objType, nil, false, entityDocs{}) require.NoError(t, err) - assert.Equal(t, typeKind(kindList), p.kind) - assert.Equal(t, typeKind(kindObject), p.element.kind) + assert.Equal(t, kindList, p.kind) + assert.Equal(t, kindObject, p.element.kind) assert.Equal(t, "config.prop.$", p.element.properties[0].parentPath.String()) }) @@ -239,7 +239,7 @@ func Test_makePropertyType(t *testing.T) { }).Shim() p, err := g.makePropertyType(path, "obj", objType, nil, false, entityDocs{}) require.NoError(t, err) - assert.Equal(t, typeKind(kindObject), p.kind) + assert.Equal(t, kindObject, p.kind) assert.Equal(t, "config.prop", p.properties[0].parentPath.String()) }) @@ -250,8 +250,8 @@ func Test_makePropertyType(t *testing.T) { }).Shim() p, err := g.makePropertyType(path, "obj", objType, nil, false, entityDocs{}) require.NoError(t, err) - assert.Equal(t, typeKind(kindSet), p.kind) - assert.Equal(t, typeKind(kindObject), p.element.kind) + assert.Equal(t, kindSet, p.kind) + assert.Equal(t, kindObject, p.element.kind) assert.Equal(t, "config.prop.$", p.element.properties[0].parentPath.String()) }) @@ -263,7 +263,7 @@ func Test_makePropertyType(t *testing.T) { }).Shim() p, err := g.makePropertyType(path, "obj", objType, nil, false, entityDocs{}) require.NoError(t, err) - assert.Equal(t, typeKind(kindObject), p.kind) + assert.Equal(t, kindObject, p.kind) assert.Equal(t, "config.prop", p.properties[0].parentPath.String()) }) } diff --git a/pkg/tfgen/test_data/minimuxed-schema.json b/pkg/tfgen/test_data/minimuxed-schema.json index 842335a74..8af31335d 100644 --- a/pkg/tfgen/test_data/minimuxed-schema.json +++ b/pkg/tfgen/test_data/minimuxed-schema.json @@ -49,7 +49,7 @@ "keepers": { "type": "object", "additionalProperties": { - "$ref": "pulumi.json#/Any" + "type": "string" }, "description": "Arbitrary map of values that, when changed, will trigger recreation of resource. See the main provider documentation for\nmore information.\n" }, @@ -79,7 +79,7 @@ "keepers": { "type": "object", "additionalProperties": { - "$ref": "pulumi.json#/Any" + "type": "string" }, "description": "Arbitrary map of values that, when changed, will trigger recreation of resource. See the main provider documentation for\nmore information.\n", "willReplaceOnChanges": true @@ -110,7 +110,7 @@ "keepers": { "type": "object", "additionalProperties": { - "$ref": "pulumi.json#/Any" + "type": "string" }, "description": "Arbitrary map of values that, when changed, will trigger recreation of resource. See the main provider documentation for\nmore information.\n", "willReplaceOnChanges": true diff --git a/pkg/tfgen/test_data/minirandom-schema-csharp.json b/pkg/tfgen/test_data/minirandom-schema-csharp.json index 32b5a863f..33ddff284 100644 --- a/pkg/tfgen/test_data/minirandom-schema-csharp.json +++ b/pkg/tfgen/test_data/minirandom-schema-csharp.json @@ -35,7 +35,7 @@ "keepers": { "type": "object", "additionalProperties": { - "$ref": "pulumi.json#/Any" + "type": "string" }, "description": "Arbitrary map of values that, when changed, will trigger recreation of resource. See the main provider documentation for\nmore information.\n" }, @@ -75,7 +75,7 @@ "keepers": { "type": "object", "additionalProperties": { - "$ref": "pulumi.json#/Any" + "type": "string" }, "description": "Arbitrary map of values that, when changed, will trigger recreation of resource. See the main provider documentation for\nmore information.\n", "willReplaceOnChanges": true @@ -111,7 +111,7 @@ "keepers": { "type": "object", "additionalProperties": { - "$ref": "pulumi.json#/Any" + "type": "string" }, "description": "Arbitrary map of values that, when changed, will trigger recreation of resource. See the main provider documentation for\nmore information.\n", "willReplaceOnChanges": true diff --git a/pkg/tfgen/test_data/regress-minirandom-schema.json b/pkg/tfgen/test_data/regress-minirandom-schema.json index 482f9252a..696e446a3 100644 --- a/pkg/tfgen/test_data/regress-minirandom-schema.json +++ b/pkg/tfgen/test_data/regress-minirandom-schema.json @@ -35,7 +35,7 @@ "keepers": { "type": "object", "additionalProperties": { - "$ref": "pulumi.json#/Any" + "type": "string" }, "description": "Arbitrary map of values that, when changed, will trigger recreation of resource. See the main provider documentation for\nmore information.\n" }, @@ -65,7 +65,7 @@ "keepers": { "type": "object", "additionalProperties": { - "$ref": "pulumi.json#/Any" + "type": "string" }, "description": "Arbitrary map of values that, when changed, will trigger recreation of resource. See the main provider documentation for\nmore information.\n", "willReplaceOnChanges": true @@ -96,7 +96,7 @@ "keepers": { "type": "object", "additionalProperties": { - "$ref": "pulumi.json#/Any" + "type": "string" }, "description": "Arbitrary map of values that, when changed, will trigger recreation of resource. See the main provider documentation for\nmore information.\n", "willReplaceOnChanges": true diff --git a/pkg/tfgen/test_data/test-propagate-language-options.json b/pkg/tfgen/test_data/test-propagate-language-options.json index b506de3d0..465899b58 100644 --- a/pkg/tfgen/test_data/test-propagate-language-options.json +++ b/pkg/tfgen/test_data/test-propagate-language-options.json @@ -52,7 +52,7 @@ "keepers": { "type": "object", "additionalProperties": { - "$ref": "pulumi.json#/Any" + "type": "string" }, "description": "Arbitrary map of values that, when changed, will trigger recreation of resource. See the main provider documentation for\nmore information.\n" }, @@ -82,7 +82,7 @@ "keepers": { "type": "object", "additionalProperties": { - "$ref": "pulumi.json#/Any" + "type": "string" }, "description": "Arbitrary map of values that, when changed, will trigger recreation of resource. See the main provider documentation for\nmore information.\n", "willReplaceOnChanges": true @@ -113,7 +113,7 @@ "keepers": { "type": "object", "additionalProperties": { - "$ref": "pulumi.json#/Any" + "type": "string" }, "description": "Arbitrary map of values that, when changed, will trigger recreation of resource. See the main provider documentation for\nmore information.\n", "willReplaceOnChanges": true