Skip to content

Commit

Permalink
Fix implicit conversion error (#1945)
Browse files Browse the repository at this point in the history 
Fixes #1940

The tf-plugin-sdk type checks input to int fields and only allows
explicit ints and strings, not int64 and similar:


https://github.com/hashicorp/terraform-plugin-sdk/blob/1f499688ebd9420768f501d4ed622a51b2135ced/helper/schema/schema.go#L2269

This corrects the type there and adds tests for implicitly converting
all primitive types.
  • Loading branch information
@VenelinMartinov
VenelinMartinov committed May 9, 2024
1 parent fa1796a commit b3ecbf8
Show file tree
Hide file tree
Showing 3 changed files with 289 additions and 5 deletions.
267 changes: 267 additions & 0 deletions pkg/tfbridge/provider_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4013,3 +4013,270 @@ func TestProviderMetaPlanResourceChangeNoError(t *testing.T) {
}`)
})
}

func TestStringValForOtherProperty(t *testing.T) {
const largeNumber int64 = 1<<62 + 1

p := &schemav2.Provider{
Schema: map[string]*schemav2.Schema{},
ResourcesMap: map[string]*schemav2.Resource{
"res": {
Schema: map[string]*schemav2.Schema{
"int_prop": {
Optional: true,
Type: schema.TypeInt,
},
"float_prop": {
Optional: true,
Type: schema.TypeFloat,
},
"bool_prop": {
Optional: true,
Type: schema.TypeBool,
},
"nested_int": {
Optional: true,
Type: schema.TypeList,
Elem: &schemav2.Schema{
Type: schema.TypeInt,
},
},
"nested_float": {
Optional: true,
Type: schema.TypeList,
Elem: &schemav2.Schema{
Type: schema.TypeFloat,
},
},
"nested_bool": {
Optional: true,
Type: schema.TypeList,
Elem: &schemav2.Schema{
Type: schema.TypeBool,
},
},
},
},
},
}
shimProv := shimv2.NewProvider(p)
provider := &Provider{
tf: shimProv,
config: shimv2.NewSchemaMap(p.Schema),
info: ProviderInfo{P: shimProv},
resources: map[tokens.Type]Resource{
"Res": {
TF: shimv2.NewResource(p.ResourcesMap["res"]),
TFName: "res",
Schema: &ResourceInfo{},
},
},
}

t.Run("String value for int property", func(t *testing.T) {
testutils.Replay(t, provider, `
{
"method": "/pulumirpc.ResourceProvider/Check",
"request": {
"urn": "urn:pulumi:dev::teststack::Res::exres",
"olds": {
},
"news": {
"__defaults": [],
"intProp": "80"
},
"randomSeed": "zjSL8IMF68r5aLLepOpsIT53uBTbkDryYFDnHQHkjko="
},
"response": {
"inputs": {
"__defaults": [],
"intProp": 80
}
}
}`)
})

t.Run("String value for large int property", func(t *testing.T) {
testutils.Replay(t, provider, fmt.Sprintf(`
{
"method": "/pulumirpc.ResourceProvider/Check",
"request": {
"urn": "urn:pulumi:dev::teststack::Res::exres",
"olds": {
},
"news": {
"__defaults": [],
"intProp": "%d"
},
"randomSeed": "zjSL8IMF68r5aLLepOpsIT53uBTbkDryYFDnHQHkjko="
},
"response": {
"inputs": {
"__defaults": [],
"intProp": %d
}
}
}`, largeNumber, largeNumber))
})

t.Run("String value for bool property", func(t *testing.T) {
testutils.Replay(t, provider, `
{
"method": "/pulumirpc.ResourceProvider/Check",
"request": {
"urn": "urn:pulumi:dev::teststack::Res::exres",
"olds": {
},
"news": {
"__defaults": [],
"boolProp": "true"
},
"randomSeed": "zjSL8IMF68r5aLLepOpsIT53uBTbkDryYFDnHQHkjko="
},
"response": {
"inputs": {
"__defaults": [],
"boolProp": true
}
}
}`)
})

t.Run("String num value for bool property", func(t *testing.T) {
testutils.Replay(t, provider, `
{
"method": "/pulumirpc.ResourceProvider/Check",
"request": {
"urn": "urn:pulumi:dev::teststack::Res::exres",
"olds": {
},
"news": {
"__defaults": [],
"boolProp": "1"
},
"randomSeed": "zjSL8IMF68r5aLLepOpsIT53uBTbkDryYFDnHQHkjko="
},
"response": {
"inputs": {
"__defaults": [],
"boolProp": true
}
}
}`)
})

t.Run("String value for float property", func(t *testing.T) {
testutils.Replay(t, provider, `
{
"method": "/pulumirpc.ResourceProvider/Check",
"request": {
"urn": "urn:pulumi:dev::teststack::Res::exres",
"olds": {
},
"news": {
"__defaults": [],
"floatProp": "8.2"
},
"randomSeed": "zjSL8IMF68r5aLLepOpsIT53uBTbkDryYFDnHQHkjko="
},
"response": {
"inputs": {
"__defaults": [],
"floatProp": 8.2
}
}
}`)
})

t.Run("String value for nested int property", func(t *testing.T) {
testutils.Replay(t, provider, `
{
"method": "/pulumirpc.ResourceProvider/Check",
"request": {
"urn": "urn:pulumi:dev::teststack::Res::exres",
"olds": {
},
"news": {
"__defaults": [],
"nestedInts": ["80"]
},
"randomSeed": "zjSL8IMF68r5aLLepOpsIT53uBTbkDryYFDnHQHkjko="
},
"response": {
"inputs": {
"__defaults": [],
"nestedInts": [80]
}
}
}`)
})

t.Run("String value for nested float property", func(t *testing.T) {
testutils.Replay(t, provider, `
{
"method": "/pulumirpc.ResourceProvider/Check",
"request": {
"urn": "urn:pulumi:dev::teststack::Res::exres",
"olds": {
},
"news": {
"__defaults": [],
"nestedFloats": ["8.2"]
},
"randomSeed": "zjSL8IMF68r5aLLepOpsIT53uBTbkDryYFDnHQHkjko="
},
"response": {
"inputs": {
"__defaults": [],
"nestedFloats": [8.2]
}
}
}`)
})

t.Run("String value for nested bool property", func(t *testing.T) {
testutils.Replay(t, provider, `
{
"method": "/pulumirpc.ResourceProvider/Check",
"request": {
"urn": "urn:pulumi:dev::teststack::Res::exres",
"olds": {
},
"news": {
"__defaults": [],
"nestedBools": ["true"]
},
"randomSeed": "zjSL8IMF68r5aLLepOpsIT53uBTbkDryYFDnHQHkjko="
},
"response": {
"inputs": {
"__defaults": [],
"nestedBools": [true]
}
}
}`)
})

t.Run("String num value for nested bool property", func(t *testing.T) {
testutils.Replay(t, provider, `
{
"method": "/pulumirpc.ResourceProvider/Check",
"request": {
"urn": "urn:pulumi:dev::teststack::Res::exres",
"olds": {
},
"news": {
"__defaults": [],
"nestedBools": ["1"]
},
"randomSeed": "zjSL8IMF68r5aLLepOpsIT53uBTbkDryYFDnHQHkjko="
},
"response": {
"inputs": {
"__defaults": [],
"nestedBools": [true]
}
}
}`)
})
}
4 changes: 3 additions & 1 deletion pkg/tfbridge/schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,9 @@ func (ctx *conversionContext) makeTerraformInput(
case v.IsString():
switch tfs.Type() {
case shim.TypeInt:
return wrapError(strconv.ParseInt(v.StringValue(), 10, 64))
v, err := wrapError(strconv.ParseInt(v.StringValue(), 10, 64))
// The plugin sdk asserts against the type - need this to be an int.
return int(v.(int64)), err
default:
return v.StringValue(), nil
}
Expand Down
23 changes: 19 additions & 4 deletions pkg/tfbridge/schema_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1245,6 +1245,8 @@ func TestOverridingTFSchema(t *testing.T) {

const largeNumber int64 = 1<<62 + 1

const notSoLargeNumber int64 = 1<<50 + 1

// We need to assert that when both the Pulumi type (String) and the Terraform
// type (Int) are large enough to hold a large number, we never round trip it
// through a smaller type like a float64.
Expand All @@ -1256,6 +1258,11 @@ func TestOverridingTFSchema(t *testing.T) {
assert.NotEqual(t, largeNumber, int64(float64(largeNumber)))
})

t.Run("number_is_not_so_large", func(t *testing.T) {
t.Parallel()
assert.Equal(t, notSoLargeNumber, int64(float64(notSoLargeNumber)))
})

tests := []struct {
name string

Expand Down Expand Up @@ -1322,13 +1329,20 @@ func TestOverridingTFSchema(t *testing.T) {
tfInput: MyString(""),
tfOutput: resource.NewNullProperty(),
},
{
name: "tf_mid_int_to_pulumi_string",
tfSchema: &schemav1.Schema{Type: schemav1.TypeInt},
info: &SchemaInfo{Type: "string"},
tfInput: int(notSoLargeNumber),
tfOutput: resource.NewProperty(strconv.FormatInt(notSoLargeNumber, 10)),
},
{
name: "tf_int_to_pulumi_string",

tfSchema: &schemav1.Schema{Type: schemav1.TypeInt},
info: &SchemaInfo{Type: "string"},

tfInput: largeNumber,
tfInput: int(largeNumber),
tfOutput: resource.NewProperty(strconv.FormatInt(largeNumber, 10)),
},
}
Expand Down Expand Up @@ -2624,7 +2638,6 @@ func TestExtractDefaultIntegerInputs(t *testing.T) {
}

func TestExtractSchemaInputsNestedMaxItemsOne(t *testing.T) {

provider := func(info *ResourceInfo) *Provider {
if info == nil {
info = &ResourceInfo{}
Expand Down Expand Up @@ -2682,12 +2695,14 @@ func TestExtractSchemaInputsNestedMaxItemsOne(t *testing.T) {
set(d, "list_object", []any{
map[string]any{
"field1": false,
"list_scalar": []any{1}},
"list_scalar": []any{1},
},
})
set(d, "list_object_maxitems", []any{
map[string]any{
"field1": true,
"list_scalar": []any{2}},
"list_scalar": []any{2},
},
})
return nil
}
Expand Down

0 comments on commit b3ecbf8

Please sign in to comment.