Skip to content
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.

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

Infer Map[string] when the Map's element type is unspecified #2316

Merged
merged 1 commit into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
61 changes: 61 additions & 0 deletions pkg/tests/internal/tfcheck/tf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
9 changes: 5 additions & 4 deletions pkg/tfgen/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ type moduleMember interface {
type typeKind int

const (
kindInvalid = iota
kindInvalid typeKind = iota
kindBool
kindInt
kindFloat
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down
28 changes: 14 additions & 14 deletions pkg/tfgen/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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())
})

Expand All @@ -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())
})

Expand All @@ -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())
})

Expand All @@ -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())
})

Expand All @@ -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())
})
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/tfgen/test_data/minimuxed-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions pkg/tfgen/test_data/minirandom-schema-csharp.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions pkg/tfgen/test_data/regress-minirandom-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions pkg/tfgen/test_data/test-propagate-language-options.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down