Support overriding int with string (#1896)

- **Refactor TestOverridingTFSchema for clarity.**
This commit is a pure refactor of the tests, and does not add or remove
any tests.
  

- **Support override int with string**
This conversion was previously handled by by terraform as a best-effort
approach. We
  should handle it in-house for 2 reasons:
  1. To provider errors at the pulumi level.
2. To ensure that providers receive the same inputs as from TF, instead
of relying on TF
  to convert on the wrong type.


Related to #1342

This is a pre-requisite for
pulumi/pulumi-databricks#55.

pkg/tfbridge/schema.go

@@ -384,32 +384,42 @@ func (ctx *conversionContext) makeTerraformInput(
 		v = wrap(v)
 	}
 
+	wrapError := func(v any, err error) (any, error) {
+		if err == nil {
+			return v, nil
+		}
+
+		return v, fmt.Errorf("%s: %w", name, err)
+	}
+
 	// If there is a custom transform for this value, run it before processing the value.
 	if ps != nil && ps.Transform != nil {
 		nv, err := ps.Transform(v)
 		if err != nil {
-			return nil, err
+			return wrapError(nv, err)
 		}
 		v = nv
 	}
 
+	if tfs == nil {
+		tfs = (&schema.Schema{}).Shim()
+	}
+
 	switch {
 	case v.IsNull():
 		return nil, nil
 	case v.IsBool():
-		if tfs != nil && tfs.Type() == shim.TypeString {
+		switch tfs.Type() {
+		case shim.TypeString:
 			if v.BoolValue() {
 				return "true", nil
 			}
 			return "false", nil
+		default:
+			return v.BoolValue(), nil
 		}
-		return v.BoolValue(), nil
 	case v.IsNumber():
-		var typ shim.ValueType
-		if tfs != nil {
-			typ = tfs.Type()
-		}
-		switch typ {
+		switch tfs.Type() {
 		case shim.TypeFloat:
 			return v.NumberValue(), nil
 		case shim.TypeString:
@@ -418,7 +428,12 @@ func (ctx *conversionContext) makeTerraformInput(
 			return int(v.NumberValue()), nil
 		}
 	case v.IsString():
-		return v.StringValue(), nil
+		switch tfs.Type() {
+		case shim.TypeInt:
+			return wrapError(strconv.ParseInt(v.StringValue(), 10, 64))
+		default:
+			return v.StringValue(), nil
+		}
 	case v.IsArray():
 		var oldArr []resource.PropertyValue
 		if old.IsArray() {
@@ -1100,6 +1115,10 @@ func MakeTerraformOutput(
 			v = list
 		}
 
+		if ps == nil {
+			ps = &SchemaInfo{}
+		}
+
 		// We use reflection instead of a type switch so that we can support mapping values whose underlying type is
 		// supported into a Pulumi value, even if they stored as a wrapper type (such as a strongly-typed enum).
 		//
@@ -1111,7 +1130,12 @@ func MakeTerraformOutput(
 		case reflect.Bool:
 			return resource.NewBoolProperty(val.Bool())
 		case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
-			return resource.NewNumberProperty(float64(val.Int()))
+			switch ps.Type {
+			case "string":
+				return resource.NewProperty(strconv.FormatInt(val.Int(), 10))
+			default:
+				return resource.NewNumberProperty(float64(val.Int()))
+			}
 		case reflect.Float32, reflect.Float64:
 			return resource.NewNumberProperty(val.Float())
 		case reflect.String:

pkg/tfbridge/schema_test.go

@@ -1241,92 +1241,164 @@ func TestInvalidAsset(t *testing.T) {
 }
 
 func TestOverridingTFSchema(t *testing.T) {
-	ctx := context.Background()
+	t.Parallel()
 
-	tfInputs := map[string]interface{}{
-		"pulumi_override_tf_string_to_boolean":    MyString("true"),
-		"pulumi_override_tf_string_to_bool":       MyString("true"),
-		"pulumi_empty_tf_override":                MyString("true"),
-		"pulumi_override_tf_string_to_int":        MyString("1"),
-		"pulumi_override_tf_string_to_integer":    MyString("1"),
-		"tf_empty_string_to_pulumi_bool_override": MyString(""),
-	}
-
-	tfSchema := shimv1.NewSchemaMap(map[string]*schemav1.Schema{
-		"pulumi_override_tf_string_to_boolean":    {Type: schemav1.TypeString},
-		"pulumi_override_tf_string_to_bool":       {Type: schemav1.TypeString},
-		"pulumi_empty_tf_override":                {Type: schemav1.TypeString},
-		"pulumi_override_tf_string_to_int":        {Type: schemav1.TypeString},
-		"pulumi_override_tf_string_to_integer":    {Type: schemav1.TypeString},
-		"tf_empty_string_to_pulumi_bool_override": {Type: schemav1.TypeString},
+	const largeNumber int64 = 1<<62 + 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.
+	//
+	// We assert this requirement by checking that the number we use *does not* round
+	// trip through float64.
+	t.Run("number_is_large", func(t *testing.T) {
+		t.Parallel()
+		assert.NotEqual(t, largeNumber, int64(float64(largeNumber)))
 	})
 
-	typeOverrides := map[string]*SchemaInfo{
-		"pulumi_override_tf_string_to_boolean": {
-			Type: "boolean",
+	tests := []struct {
+		name string
+
+		tfSchema *schemav1.Schema
+		info     *SchemaInfo
+
+		tfInput  any
+		tfOutput resource.PropertyValue
+	}{
+		{
+			name: "pulumi_override_tf_string_to_boolean",
+
+			tfSchema: &schemav1.Schema{Type: schemav1.TypeString},
+			info:     &SchemaInfo{Type: "boolean"},
+
+			tfInput:  MyString("true"),
+			tfOutput: resource.NewProperty(true),
 		},
-		"pulumi_override_tf_string_to_bool": {
-			Type: "bool",
+		{
+			name: "pulumi_override_tf_string_to_bool",
+
+			tfSchema: &schemav1.Schema{Type: schemav1.TypeString},
+			info:     &SchemaInfo{Type: "bool"},
+
+			tfInput:  MyString("true"),
+			tfOutput: resource.NewProperty(true),
 		},
-		"pulumi_empty_tf_override": {
-			Type: "",
+		{
+			name: "pulumi_empty_tf_override",
+
+			tfSchema: &schemav1.Schema{Type: schemav1.TypeString},
+			info:     &SchemaInfo{Type: ""},
+
+			tfInput:  MyString("true"),
+			tfOutput: resource.NewProperty("true"),
 		},
-		"pulumi_override_tf_string_to_int": {
-			Type: "int",
+		{
+			name: "pulumi_override_tf_string_to_int",
+
+			tfSchema: &schemav1.Schema{Type: schemav1.TypeString},
+			info:     &SchemaInfo{Type: "int"},
+
+			tfInput:  MyString("1"),
+			tfOutput: resource.NewProperty(1.0),
 		},
-		"pulumi_override_tf_string_to_integer": {
-			Type: "integer",
+		{
+			name: "pulumi_override_tf_string_to_integer",
+
+			tfSchema: &schemav1.Schema{Type: schemav1.TypeString},
+			info:     &SchemaInfo{Type: "integer"},
+
+			tfInput:  MyString("1"),
+			tfOutput: resource.NewProperty(1.0),
 		},
-		"tf_empty_string_to_pulumi_bool_override": {
-			Type:           "boolean",
-			MarkAsOptional: boolPointer(true),
+		{
+			name: "tf_empty_string_to_pulumi_bool_override",
+
+			tfSchema: &schemav1.Schema{Type: schemav1.TypeString},
+			info: &SchemaInfo{
+				Type:           "boolean",
+				MarkAsOptional: True(),
+			},
+
+			tfInput:  MyString(""),
+			tfOutput: resource.NewNullProperty(),
+		},
+		{
+			name: "tf_int_to_pulumi_string",
+
+			tfSchema: &schemav1.Schema{Type: schemav1.TypeInt},
+			info:     &SchemaInfo{Type: "string"},
+
+			tfInput:  largeNumber,
+			tfOutput: resource.NewProperty(strconv.FormatInt(largeNumber, 10)),
 		},
 	}
 
-	tfOutputs := resource.NewPropertyMapFromMap(map[string]interface{}{
-		"pulumiOverrideTfStringToBoolean":   true,
-		"pulumiOverrideTfStringToBool":      true,
-		"pulumiEmptyTfOverride":             "true",
-		"pulumiOverrideTfStringToInt":       1,
-		"pulumiOverrideTfStringToInteger":   1,
-		"tfEmptyStringToPulumiBoolOverride": nil,
-	})
+	const testProp = "prop"
 
-	t.Run("MakeTerraformOutputs", func(t *testing.T) {
-		result := MakeTerraformOutputs(
-			ctx,
-			shimv1.NewProvider(testTFProvider),
-			tfInputs,
-			tfSchema,
-			typeOverrides,
-			nil, /* assets */
-			true,
-		)
-		assert.Equal(t, tfOutputs, result)
-	})
-	t.Run("MakeTerraformInputs", func(t *testing.T) {
-		result, _, err := makeTerraformInputsForConfig(
-			nil,
-			tfOutputs,
-			tfSchema,
-			typeOverrides,
-		)
-		require.NoError(t, err)
-		expected := map[string]interface{}{
-			// SDKv2 Providers have __defaults included.
-			"__defaults": []interface{}{},
-		}
-		for k, v := range tfInputs {
-			// We don't transform nil values because terraform distinguished
-			// between nil and "" values.
-			if s := string(v.(MyString)); s == "" {
-				expected[k] = nil
-			} else {
-				expected[k] = s
-			}
-		}
-		assert.Equal(t, expected, result)
-	})
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			t.Run("MakeTerraformOutputs", func(t *testing.T) {
+				t.Parallel()
+
+				ctx := context.Background()
+				result := MakeTerraformOutputs(
+					ctx,
+					shimv1.NewProvider(testTFProvider),
+					map[string]any{
+						testProp: tt.tfInput,
+					},
+					shimv1.NewSchemaMap(map[string]*schemav1.Schema{
+						testProp: tt.tfSchema,
+					}),
+					map[string]*SchemaInfo{
+						testProp: tt.info,
+					},
+					nil, /* assets */
+					true,
+				)
+				assert.Equal(t, resource.PropertyMap{
+					testProp: tt.tfOutput,
+				}, result)
+			})
+			t.Run("MakeTerraformInputs", func(t *testing.T) {
+				t.Parallel()
+
+				result, _, err := makeTerraformInputsForConfig(
+					nil,
+					resource.PropertyMap{
+						testProp: tt.tfOutput,
+					},
+					shimv1.NewSchemaMap(map[string]*schemav1.Schema{
+						testProp: tt.tfSchema,
+					}),
+					map[string]*SchemaInfo{
+						testProp: tt.info,
+					},
+				)
+				require.NoError(t, err)
+				expected := map[string]any{
+					// SDKv2 Providers have __defaults included.
+					"__defaults": []any{},
+				}
+
+				switch tfInput := tt.tfInput.(type) {
+				// We don't transform nil values because terraform distinguished
+				// between nil and "" values.
+				case MyString:
+					if tfInput == "" {
+						expected[testProp] = nil
+					} else {
+						expected[testProp] = string(tfInput)
+					}
+				default:
+					expected[testProp] = tfInput
+				}
+				assert.Equal(t, expected, result)
+			})
+		})
+	}
 }
 
 func TestArchiveAsAsset(t *testing.T) {