Add a fallback for detailed diff replace decisions to ensure detailed…

… diff is presentation-only (#2757)

This change adds a fallback for the detailed diff replace decision. This
ensures that the detailed diff is presentation only.

If we fail to identify the reason for a replace in the detailed diff
calculation we mark it against `__meta`, similar to what we did before:
https://github.com/pulumi/pulumi-terraform-bridge/blob/a952164c556a86f46dac2ac34e915143cfd7abd8/pkg/tfbridge/provider.go#L1262
If we incorrectly identify a non-existent replace we demote it to an
update/create/delete.

This is flagged behind the Accurate Previews flag.

I've stood up
#2747 again as it
got accidentally merged into another branch.

fixes #2674
fixes #2726

pkg/pf/tfbridge/provider_diff.go

@@ -135,7 +135,9 @@ func (p *provider) DiffWithContext(
 	}
 
 	if providerOpts.enableAccurateBridgePreview {
-		pluginDetailedDiff, err := calculateDetailedDiff(ctx, &rh, priorState, plannedStateValue, checkedInputs)
+		replaceOverride := len(replaceKeys) > 0
+		pluginDetailedDiff, err := calculateDetailedDiff(
+			ctx, &rh, priorState, plannedStateValue, checkedInputs, &replaceOverride)
 		if err != nil {
 			return plugin.DiffResult{}, err
 		}
@@ -148,7 +150,7 @@ func (p *provider) DiffWithContext(
 
 func calculateDetailedDiff(
 	ctx context.Context, rh *resourceHandle, priorState *upgradedResourceState,
-	plannedStateValue tftypes.Value, checkedInputs resource.PropertyMap,
+	plannedStateValue tftypes.Value, checkedInputs resource.PropertyMap, replaceOverride *bool,
 ) (map[string]plugin.PropertyDiff, error) {
 	priorProps, err := convert.DecodePropertyMap(ctx, rh.decoder, priorState.state.Value)
 	if err != nil {
@@ -167,6 +169,7 @@ func calculateDetailedDiff(
 		priorProps,
 		props,
 		checkedInputs,
+		replaceOverride,
 	)
 
 	pluginDetailedDiff := make(map[string]plugin.PropertyDiff, len(detailedDiff))

pkg/tests/detailed_diff_unknown_test.go

@@ -900,7 +900,7 @@ func TestUnknownCollectionForceNewDetailedDiff(t *testing.T) {
 			runTest(t, program2, autogold.Expect(`
     + prov:index/aux:Aux: (create)
         [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes]
-    +-prov:index/test:Test: (replace)
+    ~ prov:index/test:Test: (update)
         [id=newid]
         [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes]
       ~ tests: [
@@ -917,7 +917,7 @@ func TestUnknownCollectionForceNewDetailedDiff(t *testing.T) {
 			runTest(t, program2, autogold.Expect(`
     + prov:index/aux:Aux: (create)
         [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes]
-    +-prov:index/test:Test: (replace)
+    ~ prov:index/test:Test: (update)
         [id=newid]
         [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes]
       - tests: [
@@ -1026,7 +1026,7 @@ func TestUnknownCollectionForceNewDetailedDiff(t *testing.T) {
 			runTest(t, program2, autogold.Expect(`
     + prov:index/aux:Aux: (create)
         [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes]
-    +-prov:index/test:Test: (replace)
+    ~ prov:index/test:Test: (update)
         [id=newid]
         [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes]
       ~ tests: [
@@ -1043,7 +1043,7 @@ func TestUnknownCollectionForceNewDetailedDiff(t *testing.T) {
 			runTest(t, program2, autogold.Expect(`
     + prov:index/aux:Aux: (create)
         [urn=urn:pulumi:test::test::prov:index/aux:Aux::auxRes]
-    +-prov:index/test:Test: (replace)
+    ~ prov:index/test:Test: (update)
         [id=newid]
         [urn=urn:pulumi:test::test::prov:index/test:Test::mainRes]
       - tests: [

pkg/tfbridge/detailed_diff.go

@@ -57,6 +57,18 @@ func isTypeShapeMismatched(val resource.PropertyValue, propType shim.ValueType)
 	}
 }
 
+func containsReplace(m map[string]*pulumirpc.PropertyDiff) bool {
+	for _, v := range m {
+		switch v.GetKind() {
+		case pulumirpc.PropertyDiff_UPDATE_REPLACE,
+			pulumirpc.PropertyDiff_ADD_REPLACE,
+			pulumirpc.PropertyDiff_DELETE_REPLACE:
+			return true
+		}
+	}
+	return false
+}
+
 func promoteToReplace(diff *pulumirpc.PropertyDiff) *pulumirpc.PropertyDiff {
 	if diff == nil {
 		return nil
@@ -75,6 +87,23 @@ func promoteToReplace(diff *pulumirpc.PropertyDiff) *pulumirpc.PropertyDiff {
 	}
 }
 
+func demoteToNoReplace(diff *pulumirpc.PropertyDiff) *pulumirpc.PropertyDiff {
+	if diff == nil {
+		return nil
+	}
+	kind := diff.GetKind()
+	switch kind {
+	case pulumirpc.PropertyDiff_ADD_REPLACE:
+		return &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_ADD}
+	case pulumirpc.PropertyDiff_DELETE_REPLACE:
+		return &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_DELETE}
+	case pulumirpc.PropertyDiff_UPDATE_REPLACE:
+		return &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_UPDATE}
+	default:
+		return diff
+	}
+}
+
 type baseDiff string
 
 const (
@@ -477,11 +506,17 @@ func (differ detailedDiffer) makeDetailedDiffPropertyMap(
 
 // MakeDetailedDiffV2 is the main entry point for calculating the detailed diff.
 // This is an internal function that should not be used outside of the pulumi-terraform-bridge.
+//
+// The `replaceOverride` parameter is used to override the replace behavior of the detailed diff.
+// If true, the diff will be overridden to return a replace.
+// If false, the diff will be overridden to not return a replace.
+// If nil, the detailed diff will be returned as is.
 func MakeDetailedDiffV2(
 	ctx context.Context,
 	tfs shim.SchemaMap,
 	ps map[string]*SchemaInfo,
 	priorProps, props, newInputs resource.PropertyMap,
+	replaceOverride *bool,
 ) map[string]*pulumirpc.PropertyDiff {
 	// Strip secrets and outputs from the properties before calculating the diff.
 	// This allows the rest of the algorithm to focus on the actual changes and not
@@ -497,7 +532,25 @@ func MakeDetailedDiffV2(
 	props = stripSecretsAndOutputs(props)
 	newInputs = stripSecretsAndOutputs(newInputs)
 	differ := detailedDiffer{ctx: ctx, tfs: tfs, ps: ps, newInputs: newInputs}
-	return differ.makeDetailedDiffPropertyMap(priorProps, props)
+	res := differ.makeDetailedDiffPropertyMap(priorProps, props)
+
+	if replaceOverride != nil {
+		if *replaceOverride {
+			// We need to make sure there is a replace.
+			if !containsReplace(res) {
+				// We use the internal __meta property to trigger a replace when we have failed to
+				// determine the correct detailed diff for it.
+				res[metaKey] = &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_UPDATE_REPLACE}
+			}
+		} else {
+			// There is an override for no replaces, so ensure we don't have any.
+			for k, v := range res {
+				res[k] = demoteToNoReplace(v)
+			}
+		}
+	}
+
+	return res
 }
 
 func makeDetailedDiffV2(
@@ -511,6 +564,7 @@ func makeDetailedDiffV2(
 	assets AssetTable,
 	supportsSecrets bool,
 	newInputs resource.PropertyMap,
+	replaceOverride *bool,
 ) (map[string]*pulumirpc.PropertyDiff, error) {
 	// We need to compare the new and olds after all transformations have been applied.
 	// ex. state upgrades, implementation-specific normalizations etc.
@@ -532,5 +586,5 @@ func makeDetailedDiffV2(
 		return nil, err
 	}
 
-	return MakeDetailedDiffV2(ctx, tfs, ps, priorProps, props, newInputs), nil
+	return MakeDetailedDiffV2(ctx, tfs, ps, priorProps, props, newInputs, replaceOverride), nil
 }

pkg/tfbridge/detailed_diff_test.go

@@ -299,7 +299,7 @@ func runDetailedDiffTest(
 	expected map[string]*pulumirpc.PropertyDiff,
 ) {
 	t.Helper()
-	actual := MakeDetailedDiffV2(context.Background(), tfs, ps, old, new, new)
+	actual := MakeDetailedDiffV2(context.Background(), tfs, ps, old, new, new, nil)
 
 	require.Equal(t, expected, actual)
 }
@@ -2788,3 +2788,95 @@ func TestDetailedDiffSetHashPanicCaught(t *testing.T) {
 
 	require.Contains(t, buf.String(), "Failed to calculate preview for element in foo")
 }
+
+func TestDetailedDiffReplaceOverrideFalse(t *testing.T) {
+	t.Parallel()
+
+	old := resource.NewPropertyMapFromMap(map[string]interface{}{
+		"foo": "bar",
+	})
+	new := resource.NewPropertyMapFromMap(map[string]interface{}{
+		"foo": "baz",
+	})
+
+	tfs := shimv2.NewSchemaMap(map[string]*schema.Schema{
+		"foo": {
+			Type:     schema.TypeString,
+			Optional: true,
+			ForceNew: true,
+		},
+	})
+
+	actual := MakeDetailedDiffV2(context.Background(), tfs, nil, old, new, new, ref(false))
+	require.Equal(t, actual, map[string]*pulumirpc.PropertyDiff{
+		"foo": {Kind: pulumirpc.PropertyDiff_UPDATE},
+	})
+}
+
+func TestDetailedDiffReplaceOverrideTrue(t *testing.T) {
+	t.Parallel()
+
+	old := resource.NewPropertyMapFromMap(map[string]interface{}{
+		"foo": "bar",
+	})
+	new := resource.NewPropertyMapFromMap(map[string]interface{}{
+		"foo": "baz",
+	})
+
+	tfs := shimv2.NewSchemaMap(map[string]*schema.Schema{
+		"foo": {
+			Type:     schema.TypeString,
+			Optional: true,
+		},
+	})
+
+	actual := MakeDetailedDiffV2(context.Background(), tfs, nil, old, new, new, ref(true))
+	require.Equal(t, actual, map[string]*pulumirpc.PropertyDiff{
+		"foo":    {Kind: pulumirpc.PropertyDiff_UPDATE},
+		"__meta": {Kind: pulumirpc.PropertyDiff_UPDATE_REPLACE},
+	})
+}
+
+func TestDemoteToNoReplace(t *testing.T) {
+	t.Parallel()
+
+	diff := &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_ADD_REPLACE}
+	require.Equal(t, demoteToNoReplace(diff), &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_ADD})
+
+	diff = &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_DELETE_REPLACE}
+	require.Equal(t, demoteToNoReplace(diff), &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_DELETE})
+
+	diff = &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_UPDATE_REPLACE}
+	require.Equal(t, demoteToNoReplace(diff), &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_UPDATE})
+
+	diff = &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_ADD}
+	require.Equal(t, demoteToNoReplace(diff), &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_ADD})
+
+	diff = &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_DELETE}
+	require.Equal(t, demoteToNoReplace(diff), &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_DELETE})
+
+	diff = &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_UPDATE}
+	require.Equal(t, demoteToNoReplace(diff), &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_UPDATE})
+}
+
+func TestContainsReplace(t *testing.T) {
+	t.Parallel()
+
+	require.True(t, containsReplace(map[string]*pulumirpc.PropertyDiff{
+		"foo": {Kind: pulumirpc.PropertyDiff_UPDATE_REPLACE},
+	}))
+
+	require.True(t, containsReplace(map[string]*pulumirpc.PropertyDiff{
+		"foo": {Kind: pulumirpc.PropertyDiff_ADD_REPLACE},
+	}))
+
+	require.True(t, containsReplace(map[string]*pulumirpc.PropertyDiff{
+		"foo": {Kind: pulumirpc.PropertyDiff_DELETE_REPLACE},
+	}))
+
+	require.False(t, containsReplace(map[string]*pulumirpc.PropertyDiff{
+		"foo": {Kind: pulumirpc.PropertyDiff_UPDATE},
+	}))
+
+	require.False(t, containsReplace(map[string]*pulumirpc.PropertyDiff{}))
+}

pkg/tfbridge/provider.go

@@ -1177,8 +1177,9 @@ func (p *Provider) Diff(ctx context.Context, req *pulumirpc.DiffRequest) (*pulum
 			changes = pulumirpc.DiffResponse_DIFF_SOME
 		}
 
+		replaceDecision := diff.RequiresNew()
 		detailedDiff, err = makeDetailedDiffV2(
-			ctx, schema, fields, res.TF, p.tf, state, diff, assets, p.supportsSecrets, news)
+			ctx, schema, fields, res.TF, p.tf, state, diff, assets, p.supportsSecrets, news, &replaceDecision)
 		if err != nil {
 			return nil, err
 		}