Repository: pulumi/pulumi-terraform-bridge
Pull request #539 (2022-06-03 16:19:00)

Ensure any customDiff detailed changes are not removed from the diff
Fixes: #533

There was an extra iteration around the old values and it was removing
them from the diff map. This extra iteration was removing items from the
diff to ensure they were not state updates. We have changed this logic
to now not remove the diffs if they are forcing a new resource

After this change, we get the correct diff coming back from the provider:

```
Outputs:
  ~ cert-id: "319417831845702948372234168596306827381" => output<string>
```

This is based on the repro code in the original issue

This is what we get back from the TF Diff function:

```
       tfDiff=sdkv2.v2InstanceDiff{
           tf: &terraform.InstanceDiff{
               mu:         sync.Mutex{},
               Attributes: {
                   "ready_for_renewal": &terraform.ResourceAttrDiff{
                       Old:         "false",
                       New:         "true",
                       NewComputed: false,
                       NewRemoved:  false,
                       NewExtra:    nil,
                       RequiresNew: true,
                       Sensitive:   false,
                       Type:        0x0,
                   },
               },
               Destroy:        false,
               DestroyDeposed: false,
               DestroyTainted: false,
               RawConfig:      cty.Value{},
               RawState:       cty.Value{},
               RawPlan:        cty.Value{},
               Meta:           {},
           },
       }
```

We can see the diff before we hit the finalise function:

```
       diff=map[string]*pulumirpc.PropertyDiff{
           "readyForRenewal": &pulumirpc.PropertyDiff{
               Kind:                 1,
               InputDiff:            false,
               XXX_NoUnkeyedLiteral: struct {}{},
               XXX_unrecognized:     nil,
               XXX_sizecache:        0,
           },
       }
```

But in the final loop through the system we delete this diff from the list

github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/00bf8e2..e6f3126) with merge commit 287c7aaaaeb31d894f00cb2b93ed60cbbbd494c1
github-actions[bot]: Diff for [pulumi-gcp](https://github.com/pulumi-bot/pulumi-gcp/compare/cbe75e5e8..acdafc346) with merge commit 287c7aaaaeb31d894f00cb2b93ed60cbbbd494c1
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/c4a1a0a..bde427d) with merge commit ba52a84e8a6caaa519b6d2ac03fc2642dec9bb79
github-actions[bot]: Diff for [pulumi-gcp](https://github.com/pulumi-bot/pulumi-gcp/compare/3b735b972..803b61a0c) with merge commit ba52a84e8a6caaa519b6d2ac03fc2642dec9bb79
stack72: The failures here are not related to this PR - it's due to tier 1 providers now having updated deps

Patch:
diff --git a/pkg/tfbridge/diff.go b/pkg/tfbridge/diff.go
index 5fa73ec6..55ee0b0d 100644
--- a/pkg/tfbridge/diff.go
+++ b/pkg/tfbridge/diff.go
@@ -196,7 +196,8 @@ func makePropertyDiff(name, path string, v resource.PropertyValue, tfDiff shim.I
 			// These diffs are typically changes to output properties that we don't care about.
 			if finalize {
 				if hasOtherDiff &&
-					(other.Kind == pulumirpc.PropertyDiff_ADD || other.Kind == pulumirpc.PropertyDiff_ADD_REPLACE) {
+					(other.Kind == pulumirpc.PropertyDiff_ADD || other.Kind == pulumirpc.PropertyDiff_ADD_REPLACE) &&
+					!d.RequiresNew {
 					delete(diff, path)
 				}
 				return false


Test patch:
diff --git a/pkg/tfbridge/diff_test.go b/pkg/tfbridge/diff_test.go
index 3fb44b56..10b013c5 100644
--- a/pkg/tfbridge/diff_test.go
+++ b/pkg/tfbridge/diff_test.go
@@ -1,14 +1,17 @@
 package tfbridge
 
 import (
+	"context"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
+	v2Schema "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
 	pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go"
 	"github.com/stretchr/testify/assert"
 
 	shimv1 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v1"
+	shimv2 "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2"
 )
 
 type DiffKind = pulumirpc.PropertyDiff_Kind
@@ -24,6 +27,122 @@ const (
 
 var computedValue = resource.Computed{Element: resource.NewStringProperty("")}
 
+func TestCustomizeDiff(t *testing.T) {
+	inputsMap := resource.NewPropertyMapFromMap(map[string]interface{}{
+		"prop": "foo",
+	})
+	stateMap := resource.NewPropertyMapFromMap(map[string]interface{}{
+		"prop": "foo",
+		"outp": false,
+	})
+
+	// Pulumi Schema
+	info := map[string]*SchemaInfo{}
+	tfs := map[string]*v2Schema.Schema{
+		"prop": {Type: v2Schema.TypeString},
+		"outp": {Type: v2Schema.TypeBool, Computed: true},
+	}
+	// Terraform schema
+	sch := shimv2.NewSchemaMap(tfs)
+
+	// ignores
+	var ignores []string
+
+	t.Run("CustomDiffCausesAddReplace", func(t *testing.T) {
+		// expected diff
+		expected := map[string]DiffKind{
+			"outp": AR,
+		}
+
+		// Fake up a TF resource and a TF provider.
+		customDiffRes := &v2Schema.Resource{
+			Schema: tfs,
+			CustomizeDiff: func(_ context.Context, d *v2Schema.ResourceDiff, _ interface{}) error {
+				var err error
+				err = d.SetNew("outp", true)
+				if err != nil {
+					return err
+				}
+				err = d.ForceNew("outp")
+				if err != nil {
+					return err
+				}
+				return err
+			},
+		}
+		provider := shimv2.NewProvider(&v2Schema.Provider{
+			ResourcesMap: map[string]*v2Schema.Resource{
+				"resource": customDiffRes,
+			},
+		})
+
+		// Convert the inputs and state to TF config and resource attributes.
+		r := Resource{
+			TF:     shimv2.NewResource(customDiffRes),
+			Schema: &ResourceInfo{Fields: info},
+		}
+		tfState, err := MakeTerraformState(r, "id", stateMap)
+		assert.NoError(t, err)
+
+		config, _, err := MakeTerraformConfig(&Provider{tf: provider}, inputsMap, sch, info)
+		assert.NoError(t, err)
+
+		tfDiff, err := provider.Diff("resource", tfState, config)
+		assert.NoError(t, err)
+
+		// ProcessIgnoreChanges
+		doIgnoreChanges(sch, info, stateMap, inputsMap, ignores, tfDiff)
+
+		// Convert the diff to a detailed diff and check the result.
+		diff := makeDetailedDiff(sch, info, stateMap, inputsMap, tfDiff)
+		expectedDiff := map[string]*pulumirpc.PropertyDiff{}
+		for k, v := range expected {
+			expectedDiff[k] = &pulumirpc.PropertyDiff{Kind: v}
+		}
+		assert.Equal(t, expectedDiff, diff)
+	})
+
+	t.Run("NoCustomDiffCausesNoDiff", func(t *testing.T) {
+		// expected diff
+		expected := map[string]DiffKind{}
+
+		// Fake up a TF resource and a TF provider.
+		noCustomDiffRes := &v2Schema.Resource{
+			Schema: tfs,
+		}
+		provider := shimv2.NewProvider(&v2Schema.Provider{
+			ResourcesMap: map[string]*v2Schema.Resource{
+				"resource": noCustomDiffRes,
+			},
+		})
+
+		// Convert the inputs and state to TF config and resource attributes.
+		r := Resource{
+			TF:     shimv2.NewResource(noCustomDiffRes),
+			Schema: &ResourceInfo{Fields: info},
+		}
+		tfState, err := MakeTerraformState(r, "id", stateMap)
+		assert.NoError(t, err)
+
+		config, _, err := MakeTerraformConfig(&Provider{tf: provider}, inputsMap, sch, info)
+		assert.NoError(t, err)
+
+		tfDiff, err := provider.Diff("resource", tfState, config)
+		assert.NoError(t, err)
+
+		// ProcessIgnoreChanges
+		doIgnoreChanges(sch, info, stateMap, inputsMap, ignores, tfDiff)
+
+		// Convert the diff to a detailed diff and check the result.
+		diff := makeDetailedDiff(sch, info, stateMap, inputsMap, tfDiff)
+		expectedDiff := map[string]*pulumirpc.PropertyDiff{}
+		for k, v := range expected {
+			expectedDiff[k] = &pulumirpc.PropertyDiff{Kind: v}
+		}
+		assert.Equal(t, expectedDiff, diff)
+	})
+}
+
 func diffTest(t *testing.T, tfs map[string]*schema.Schema, info map[string]*SchemaInfo,
 	inputs, state map[string]interface{}, expected map[string]DiffKind, ignoreChanges ...string) {
 
@@ -80,6 +199,23 @@ func diffTest(t *testing.T, tfs map[string]*schema.Schema, info map[string]*Sche
 	assert.Equal(t, map[string]*pulumirpc.PropertyDiff{}, diff)
 }
 
+func TestCustomDiffProducesReplace(t *testing.T) {
+	diffTest(t,
+		map[string]*schema.Schema{
+			"prop": {Type: schema.TypeString},
+			"outp": {Type: schema.TypeString, Computed: true},
+		},
+		map[string]*SchemaInfo{},
+		map[string]interface{}{
+			"prop": "foo",
+		},
+		map[string]interface{}{
+			"prop": "foo",
+			"outp": "bar",
+		},
+		map[string]DiffKind{})
+}
+
 func TestEmptyDiff(t *testing.T) {
 	diffTest(t,
 		map[string]*schema.Schema{