Merge branch 'master' into vvm/update_pulumi

internal/testprovider/schema.go

@@ -621,8 +621,8 @@ func ProviderV2() *schemav2.Provider {
 				},
 				Importer: &schemav2.ResourceImporter{
 					StateContext: func(_ context.Context, state *schemav2.ResourceData,
-						_ interface{}) ([]*schemav2.ResourceData, error) {
-
+						_ interface{},
+					) ([]*schemav2.ResourceData, error) {
 						return []*schemav2.ResourceData{state}, nil
 					},
 				},
@@ -868,8 +868,8 @@ func AssertProvider(f func(data *schemav2.ResourceData)) *schemav2.Provider {
 				},
 				Importer: &schemav2.ResourceImporter{
 					StateContext: func(_ context.Context, state *schemav2.ResourceData,
-						_ interface{}) ([]*schemav2.ResourceData, error) {
-
+						_ interface{},
+					) ([]*schemav2.ResourceData, error) {
 						return []*schemav2.ResourceData{state}, nil
 					},
 				},
@@ -911,3 +911,21 @@ func CustomizedDiffProvider(f func(data *schemav2.ResourceData)) *schemav2.Provi
 		},
 	}
 }
+
+func MaxItemsOneProvider() *schemav2.Provider {
+	return &schemav2.Provider{
+		Schema: map[string]*schemav2.Schema{},
+		ResourcesMap: map[string]*schemav2.Resource{
+			"nested_str_res": {
+				Schema: map[string]*schemav2.Schema{
+					"nested_str": {
+						Type:     schemav2.TypeList,
+						MaxItems: 1,
+						Elem:     &schemav2.Schema{Type: schemav2.TypeString},
+						Optional: true,
+					},
+				},
+			},
+		},
+	}
+}

pkg/tfbridge/diff.go

@@ -24,6 +24,7 @@ import (
 	pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go"
 
 	shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim"
+	shimutil "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/util"
 )
 
 // containsComputedValues returns true if the given property value is or contains a computed value.
@@ -102,7 +103,7 @@ func visitPropertyValue(
 				// fill in default values for empty fields (note that this is a property of the field reader, not of
 				// the schema) as it does when computing the hash code for a set element.
 				ctx := &conversionContext{Ctx: ctx}
-				ev, err := ctx.MakeTerraformInput(ep, resource.PropertyValue{}, e, etfs, eps, rawNames)
+				ev, err := ctx.makeTerraformInput(ep, resource.PropertyValue{}, e, etfs, eps, rawNames)
 				if err != nil {
 					return
 				}
@@ -140,7 +141,7 @@ func visitPropertyValue(
 			psflds = ps.Fields
 		}
 
-		rawElementNames := rawNames || useRawNames(tfs)
+		rawElementNames := rawNames || shimutil.IsOfTypeMap(tfs)
 		for k, e := range v.ObjectValue() {
 			var elementPath string
 			if strings.ContainsAny(string(k), `."[]`) {
@@ -261,11 +262,11 @@ func doIgnoreChanges(ctx context.Context, tfs shim.SchemaMap, ps map[string]*Sch
 	}
 	for k, v := range olds {
 		en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false)
-		visitPropertyValue(ctx, en, string(k), v, etf, eps, useRawNames(etf), visitor)
+		visitPropertyValue(ctx, en, string(k), v, etf, eps, shimutil.IsOfTypeMap(etf), visitor)
 	}
 	for k, v := range news {
 		en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false)
-		visitPropertyValue(ctx, en, string(k), v, etf, eps, useRawNames(etf), visitor)
+		visitPropertyValue(ctx, en, string(k), v, etf, eps, shimutil.IsOfTypeMap(etf), visitor)
 	}
 
 	tfDiff.IgnoreChanges(ignoredKeySet)
@@ -298,15 +299,15 @@ func makeDetailedDiff(
 	diff := map[string]*pulumirpc.PropertyDiff{}
 	for k, v := range olds {
 		en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false)
-		makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, forceDiff, etf, eps, false, useRawNames(etf))
+		makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, forceDiff, etf, eps, false, shimutil.IsOfTypeMap(etf))
 	}
 	for k, v := range news {
 		en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false)
-		makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, forceDiff, etf, eps, false, useRawNames(etf))
+		makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, forceDiff, etf, eps, false, shimutil.IsOfTypeMap(etf))
 	}
 	for k, v := range olds {
 		en, etf, eps := getInfoFromPulumiName(k, tfs, ps, false)
-		makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, forceDiff, etf, eps, true, useRawNames(etf))
+		makePropertyDiff(ctx, en, string(k), v, tfDiff, diff, forceDiff, etf, eps, true, shimutil.IsOfTypeMap(etf))
 	}
 
 	changes := pulumirpc.DiffResponse_DIFF_NONE

pkg/tfbridge/provider.go

@@ -34,6 +34,7 @@ import (
 	"golang.org/x/net/context"
 	"google.golang.org/grpc/codes"
 
+	"github.com/pulumi/pulumi-terraform-bridge/v3/unstable/propertyvalue"
 	"github.com/pulumi/pulumi/pkg/v3/resource/provider"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/diag"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
@@ -678,6 +679,31 @@ func (p *Provider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (*pul
 	return &pulumirpc.CheckResponse{Inputs: minputs, Failures: failures}, nil
 }
 
+// For properties with MaxItemsOne, where the state is still an array
+// (i.e. from a previous version without MaxItemsOne)
+// we need to mark them for update manually in order to correct the state
+// from an array to a flat type.
+// The diff is otherwise ignored since MakeTerraformInputs won't touch
+// the type if it in the right shape.
+func markWronglyTypedMaxItemsOneStateDiff(
+	schema shim.SchemaMap, info map[string]*SchemaInfo, olds resource.PropertyMap,
+) bool {
+	res := False()
+	tr := func(pulumiPath resource.PropertyPath, localValue resource.PropertyValue) (resource.PropertyValue, error) {
+		schemaPath := PropertyPathToSchemaPath(pulumiPath, schema, info)
+		localSchema, info, err := LookupSchemas(schemaPath, schema, info)
+		contract.IgnoreError(err)
+		if IsMaxItemsOne(localSchema, info) && localValue.IsArray() {
+			glog.V(9).Infof("Found type mismatch for %s, flagging for update.", pulumiPath)
+			*res = true
+		}
+		return localValue, nil // don't change just visit
+	}
+	_, err := propertyvalue.TransformPropertyValue(make(resource.PropertyPath, 0), tr, resource.NewObjectProperty(olds))
+	contract.AssertNoErrorf(err, "markWronglyTypedMaxItemsOneStateDiff should not return errors!")
+	return *res
+}
+
 // Diff checks what impacts a hypothetical update will have on the resource's properties.
 func (p *Provider) Diff(ctx context.Context, req *pulumirpc.DiffRequest) (*pulumirpc.DiffResponse, error) {
 	ctx = p.loggingContext(ctx, resource.URN(req.GetUrn()))
@@ -799,6 +825,12 @@ func (p *Provider) Diff(ctx context.Context, req *pulumirpc.DiffRequest) (*pulum
 		changes = pulumirpc.DiffResponse_DIFF_SOME
 	}
 
+	if changes == pulumirpc.DiffResponse_DIFF_NONE &&
+		markWronglyTypedMaxItemsOneStateDiff(res.TF.Schema(), res.Schema.Fields, olds) {
+
+		changes = pulumirpc.DiffResponse_DIFF_SOME
+	}
+
 	return &pulumirpc.DiffResponse{
 		Changes:             changes,
 		Replaces:            replaces,

pkg/tfbridge/provider_test.go

@@ -2133,3 +2133,107 @@ func TestTransformFromState(t *testing.T) {
 		}`)
 	})
 }
+
+// This emulates the situation where we migrate from a state without maxItemsOne
+// which would make the property a list
+// into a state with maxItemsOne, which would flatten the type.
+// https://github.com/pulumi/pulumi-aws/issues/3092
+func TestMaxItemOneWrongStateDiff(t *testing.T) {
+	p := testprovider.MaxItemsOneProvider()
+	provider := &Provider{
+		tf:     shimv2.NewProvider(p),
+		config: shimv2.NewSchemaMap(p.Schema),
+		resources: map[tokens.Type]Resource{
+			"NestedStrRes": {
+				TF:     shimv2.NewResource(p.ResourcesMap["nested_str_res"]),
+				TFName: "nested_str_res",
+				Schema: &ResourceInfo{
+					Tok:    "NestedStrRes",
+					Fields: map[string]*SchemaInfo{},
+				},
+			},
+		},
+	}
+	t.Run("DiffListAndVal", func(t *testing.T) {
+		testutils.Replay(t, provider, `
+		{
+			"method": "/pulumirpc.ResourceProvider/Diff",
+			"request": {
+				"urn": "urn:pulumi:dev::teststack::NestedStrRes::exres",
+				"id": "0",
+				"olds": {
+					"nested_str": []
+				},
+				"news": {
+					"nested_str": ""
+				}
+			},
+			"response": {
+				"changes": "DIFF_SOME",
+				"hasDetailedDiff": true
+			}
+		}`)
+	})
+	t.Run("DiffListAndValNonEmpty", func(t *testing.T) {
+		testutils.Replay(t, provider, `
+		{
+			"method": "/pulumirpc.ResourceProvider/Diff",
+			"request": {
+				"urn": "urn:pulumi:dev::teststack::NestedStrRes::exres",
+				"id": "0",
+				"olds": {
+					"nested_str": ["val"]
+				},
+				"news": {
+					"nested_str": "val"
+				}
+			},
+			"response": {
+				"changes": "DIFF_SOME",
+				"hasDetailedDiff": true
+			}
+		}`)
+	})
+
+	// Also check that we don't produce spurious diffs when not necessary.
+	t.Run("DiffValAndValEmpty", func(t *testing.T) {
+		testutils.Replay(t, provider, `
+		{
+			"method": "/pulumirpc.ResourceProvider/Diff",
+			"request": {
+				"urn": "urn:pulumi:dev::teststack::NestedStrRes::exres",
+				"id": "0",
+				"olds": {
+					"nested_str": ""
+				},
+				"news": {
+					"nested_str": ""
+				}
+			},
+			"response": {
+				"changes": "DIFF_NONE",
+				"hasDetailedDiff": true
+			}
+		}`)
+	})
+	t.Run("DiffValAndValNonempty", func(t *testing.T) {
+		testutils.Replay(t, provider, `
+		{
+			"method": "/pulumirpc.ResourceProvider/Diff",
+			"request": {
+				"urn": "urn:pulumi:dev::teststack::NestedStrRes::exres",
+				"id": "0",
+				"olds": {
+					"nested_str": "val"
+				},
+				"news": {
+					"nested_str": "val"
+				}
+			},
+			"response": {
+				"changes": "DIFF_NONE",
+				"hasDetailedDiff": true
+			}
+		}`)
+	})
+}