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

…ntation-only
pulumi · Dec 16, 2024 · cebe68f · cebe68f
1 parent b1c0b54
commit cebe68f
Show file tree

Hide file tree

Showing 4 changed files with 108 additions and 6 deletions.
diff --git a/pkg/pf/tfbridge/provider_diff.go b/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))

diff --git a/pkg/tfbridge/detailed_diff.go b/pkg/tfbridge/detailed_diff.go
@@ -57,6 +57,21 @@ func isTypeShapeMismatched(val resource.PropertyValue, propType shim.ValueType)
 	}
 }
 
+func containsReplace(m map[string]*pulumirpc.PropertyDiff) bool {
+	for _, v := range m {
+		if v.GetKind() == pulumirpc.PropertyDiff_UPDATE_REPLACE {
+			return true
+		}
+		if v.GetKind() == pulumirpc.PropertyDiff_ADD_REPLACE {
+			return true
+		}
+		if v.GetKind() == pulumirpc.PropertyDiff_DELETE_REPLACE {
+			return true
+		}
+	}
+	return false
+}
+
 func promoteToReplace(diff *pulumirpc.PropertyDiff) *pulumirpc.PropertyDiff {
 	if diff == nil {
 		return nil
@@ -75,6 +90,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 (
@@ -482,6 +514,7 @@ func MakeDetailedDiffV2(
 	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 +530,23 @@ 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 containsReplace(res) && !*replaceOverride {
+			for k, v := range res {
+				res[k] = demoteToNoReplace(v)
+			}
+		}
+
+		if !containsReplace(res) && *replaceOverride {
+			// We use the internal __meta property to trigger a replace when we have failed to
+			// determine the correct detailed diff for it.
+			res["__meta"] = &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_UPDATE_REPLACE}
+		}
+	}
+
+	return res
 }
 
 func makeDetailedDiffV2(
@@ -511,6 +560,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 +582,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
 }
diff --git a/pkg/tfbridge/detailed_diff_test.go b/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,51 @@ 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},
+	})
+}
diff --git a/pkg/tfbridge/provider.go b/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
 		}