Detailed diff handle secrets and outputs

pkg/pf/tests/diff_secret_test.go

@@ -18,7 +18,6 @@ import (
 )
 
 func TestSecretBasic(t *testing.T) {
-	t.Skip("skipping until #2643")
 	t.Parallel()
 	provBuilder := providerbuilder.NewProvider(
 		providerbuilder.NewProviderArgs{
@@ -58,15 +57,14 @@ resources:
     ~ testprovider:index/test:Test: (update)
         [id=test-id]
         [urn=urn:pulumi:test::test::testprovider:index/test:Test::mainRes]
-        s: [secret]
+      ~ s: [secret] => [secret]
 Resources:
     ~ 1 to update
     1 unchanged
 `).Equal(t, res.StdOut)
 }
 
 func TestSecretSet(t *testing.T) {
-	t.Skip("skipping until #2643")
 	t.Parallel()
 
 	provBuilder := pb.NewProvider(pb.NewProviderArgs{
@@ -160,7 +158,6 @@ Resources:
 }
 
 func TestSecretObjectBlock(t *testing.T) {
-	t.Skip("skipping until #2643")
 	t.Parallel()
 
 	provBuilder := pb.NewProvider(pb.NewProviderArgs{
@@ -205,7 +202,9 @@ resources:
     ~ testprovider:index/test:Test: (update)
         [id=test-id]
         [urn=urn:pulumi:test::test::testprovider:index/test:Test::mainRes]
-        key: [secret]
+      ~ key: {
+          ~ prop1: [secret] => [secret]
+        }
 Resources:
     ~ 1 to update
     1 unchanged
@@ -226,9 +225,8 @@ Resources:
     ~ testprovider:index/test:Test: (update)
         [id=test-id]
         [urn=urn:pulumi:test::test::testprovider:index/test:Test::mainRes]
-        key: {
-            prop1: [secret]
-            prop2: "value2"
+      ~ key: {
+          ~ prop1: [secret] => [secret]
         }
 Resources:
     ~ 1 to update
@@ -251,7 +249,6 @@ Resources:
         [id=test-id]
         [urn=urn:pulumi:test::test::testprovider:index/test:Test::mainRes]
       ~ key: {
-            prop1: [secret]
           ~ prop2: "value2" => "value3"
         }
 Resources:
@@ -262,7 +259,6 @@ Resources:
 }
 
 func TestSecretPulumiSchema(t *testing.T) {
-	t.Skip("skipping until #2643")
 	t.Parallel()
 
 	provBuilder := pb.NewProvider(pb.NewProviderArgs{
@@ -306,7 +302,7 @@ resources:
     ~ testprovider:index/test:Test: (update)
         [id=test-id]
         [urn=urn:pulumi:test::test::testprovider:index/test:Test::mainRes]
-        s: [secret]
+      ~ s: [secret] => [secret]
 Resources:
     ~ 1 to update
     1 unchanged

pkg/pf/tests/provider_diff_test.go

@@ -129,8 +129,6 @@ func TestEmptyTestresDiffWithOptionalComputed(t *testing.T) {
 
 func TestDiffWithSecrets(t *testing.T) {
 	t.Parallel()
-	t.Skip("TODO: secrets")
-
 	server, err := newProviderServer(t, testprovider.RandomProvider())
 	require.NoError(t, err)
 
@@ -175,7 +173,6 @@ func TestDiffWithSecrets(t *testing.T) {
 // See https://github.com/pulumi/pulumi-random/issues/258
 func TestDiffVersionUpgrade(t *testing.T) {
 	t.Parallel()
-	t.Skip("TODO: secrets")
 	server, err := newProviderServer(t, testprovider.RandomProvider())
 	require.NoError(t, err)
 	testCase := `

pkg/pf/tests/util.go

@@ -51,10 +51,11 @@ func bridgedProvider(prov *providerbuilder.Provider) info.Provider {
 	shimProvider := tfbridge.ShimProvider(prov)
 
 	provider := tfbridge0.ProviderInfo{
-		P:            shimProvider,
-		Name:         prov.TypeName,
-		Version:      "0.0.1",
-		MetadataInfo: &tfbridge0.MetadataInfo{},
+		P:                           shimProvider,
+		Name:                        prov.TypeName,
+		Version:                     "0.0.1",
+		MetadataInfo:                &tfbridge0.MetadataInfo{},
+		EnableAccurateBridgePreview: true,
 	}
 
 	provider.MustComputeTokens(tokens.SingleModule(prov.TypeName, "index", tokens.MakeStandard(prov.TypeName)))

pkg/tfbridge/detailed_diff.go

@@ -12,6 +12,7 @@ import (
 
 	shim "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim"
 	"github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/walk"
+	"github.com/pulumi/pulumi-terraform-bridge/v3/unstable/propertyvalue"
 )
 
 func isPresent(val resource.PropertyValue) bool {
@@ -482,6 +483,19 @@ func MakeDetailedDiffV2(
 	ps map[string]*SchemaInfo,
 	priorProps, props, newInputs resource.PropertyMap,
 ) 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
+	// have to deal with the extra noise.
+	// This is safe to do here because the detailed diff we return to the engine
+	// is only represented by paths to the values and not the values themselves.
+	// The engine will then takes care of masking secrets.
+	stripSecretsAndOutputs := func(props resource.PropertyMap) resource.PropertyMap {
+		propsVal := propertyvalue.RemoveSecretsAndOutputs(resource.NewProperty(props))
+		return propsVal.ObjectValue()
+	}
+	priorProps = stripSecretsAndOutputs(priorProps)
+	props = stripSecretsAndOutputs(props)
+	newInputs = stripSecretsAndOutputs(newInputs)
 	differ := detailedDiffer{ctx: ctx, tfs: tfs, ps: ps, newInputs: newInputs}
 	return differ.makeDetailedDiffPropertyMap(priorProps, props)
 }

pkg/tfbridge/detailed_diff_test.go

@@ -299,8 +299,7 @@ func runDetailedDiffTest(
 	expected map[string]*pulumirpc.PropertyDiff,
 ) {
 	t.Helper()
-	differ := detailedDiffer{tfs: tfs, ps: ps, newInputs: new}
-	actual := differ.makeDetailedDiffPropertyMap(old, new)
+	actual := MakeDetailedDiffV2(context.Background(), tfs, ps, old, new, new)
 
 	require.Equal(t, expected, actual)
 }
@@ -484,21 +483,54 @@ func TestBasicDetailedDiff(t *testing.T) {
 					"foo": ComputedVal,
 				},
 			)
+			propertyMapSecretValue1 := resource.NewPropertyMapFromMap(
+				map[string]interface{}{
+					"foo": tt.value1,
+				},
+			)
+			propertyMapSecretValue1["foo"] = resource.NewSecretProperty(
+				&resource.Secret{Element: propertyMapSecretValue1["foo"]})
+
+			propertyMapSecretValue2 := resource.NewPropertyMapFromMap(
+				map[string]interface{}{
+					"foo": tt.value2,
+				},
+			)
+			propertyMapSecretValue2["foo"] = resource.NewSecretProperty(
+				&resource.Secret{Element: propertyMapSecretValue2["foo"]})
+
+			propertyMapOutputValue1 := resource.NewPropertyMapFromMap(
+				map[string]interface{}{
+					"foo": tt.value1,
+				},
+			)
+			propertyMapOutputValue1["foo"] = resource.NewOutputProperty(
+				resource.Output{Element: propertyMapOutputValue1["foo"], Known: true})
+
+			propertyMapOutputValue2 := resource.NewPropertyMapFromMap(
+				map[string]interface{}{
+					"foo": tt.value2,
+				},
+			)
+			propertyMapOutputValue2["foo"] = resource.NewOutputProperty(
+				resource.Output{Element: propertyMapOutputValue2["foo"], Known: true})
+
+			defaultChangePath := "foo"
+			if tt.listLike && tt.objectLike {
+				defaultChangePath = "foo[0].foo"
+			} else if tt.listLike {
+				defaultChangePath = "foo[0]"
+			} else if tt.objectLike {
+				defaultChangePath = "foo.foo"
+			}
 
 			t.Run("unchanged", func(t *testing.T) {
 				runDetailedDiffTest(t, propertyMapValue1, propertyMapValue1, tfs, ps, map[string]*pulumirpc.PropertyDiff{})
 			})
 
 			t.Run("changed non-empty", func(t *testing.T) {
-				expected := make(map[string]*pulumirpc.PropertyDiff)
-				if tt.listLike && tt.objectLike {
-					expected["foo[0].foo"] = &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_UPDATE}
-				} else if tt.listLike {
-					expected["foo[0]"] = &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_UPDATE}
-				} else if tt.objectLike {
-					expected["foo.foo"] = &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_UPDATE}
-				} else {
-					expected["foo"] = &pulumirpc.PropertyDiff{Kind: pulumirpc.PropertyDiff_UPDATE}
+				expected := map[string]*pulumirpc.PropertyDiff{
+					defaultChangePath: {Kind: pulumirpc.PropertyDiff_UPDATE},
 				}
 				runDetailedDiffTest(t, propertyMapValue1, propertyMapValue2, tfs, ps, expected)
 			})
@@ -566,6 +598,56 @@ func TestBasicDetailedDiff(t *testing.T) {
 					runDetailedDiffTest(t, propertyMapNil, propertyMapEmpty, tfs, ps, added())
 				})
 			}
+
+			t.Run("secret unchanged", func(t *testing.T) {
+				runDetailedDiffTest(
+					t, propertyMapSecretValue1, propertyMapSecretValue1, tfs, ps, map[string]*pulumirpc.PropertyDiff{})
+			})
+
+			t.Run("value unchanged secretness changed", func(t *testing.T) {
+				runDetailedDiffTest(
+					t, propertyMapValue1, propertyMapSecretValue1, tfs, ps, map[string]*pulumirpc.PropertyDiff{})
+			})
+
+			t.Run("secret added", func(t *testing.T) {
+				runDetailedDiffTest(t, propertyMapNil, propertyMapSecretValue1, tfs, ps, added())
+			})
+
+			t.Run("secret deleted", func(t *testing.T) {
+				runDetailedDiffTest(t, propertyMapSecretValue1, propertyMapNil, tfs, ps, deleted())
+			})
+
+			t.Run("secret changed", func(t *testing.T) {
+				expected := map[string]*pulumirpc.PropertyDiff{
+					defaultChangePath: {Kind: pulumirpc.PropertyDiff_UPDATE},
+				}
+				runDetailedDiffTest(t, propertyMapSecretValue1, propertyMapSecretValue2, tfs, ps, expected)
+			})
+
+			t.Run("output unchanged", func(t *testing.T) {
+				runDetailedDiffTest(
+					t, propertyMapOutputValue1, propertyMapOutputValue1, tfs, ps, map[string]*pulumirpc.PropertyDiff{})
+			})
+
+			t.Run("value unchanged outputness changed", func(t *testing.T) {
+				runDetailedDiffTest(
+					t, propertyMapValue1, propertyMapOutputValue1, tfs, ps, map[string]*pulumirpc.PropertyDiff{})
+			})
+
+			t.Run("output added", func(t *testing.T) {
+				runDetailedDiffTest(t, propertyMapNil, propertyMapOutputValue1, tfs, ps, added())
+			})
+
+			t.Run("output deleted", func(t *testing.T) {
+				runDetailedDiffTest(t, propertyMapOutputValue1, propertyMapNil, tfs, ps, deleted())
+			})
+
+			t.Run("output changed", func(t *testing.T) {
+				expected := map[string]*pulumirpc.PropertyDiff{
+					defaultChangePath: {Kind: pulumirpc.PropertyDiff_UPDATE},
+				}
+				runDetailedDiffTest(t, propertyMapOutputValue1, propertyMapOutputValue2, tfs, ps, expected)
+			})
 		})
 	}
 }
@@ -612,6 +694,16 @@ func TestDetailedDiffObject(t *testing.T) {
 		},
 	)
 
+	propertyMapWithSecrets := resource.PropertyMap{
+		resource.PropertyKey("foo"): resource.NewPropertyValue(
+			resource.PropertyMap{
+				resource.PropertyKey("prop1"): resource.NewSecretProperty(
+					&resource.Secret{Element: resource.NewStringProperty("val1")}),
+				resource.PropertyKey("prop2"): resource.NewStringProperty("qux"),
+			},
+		),
+	}
+
 	t.Run("unchanged", func(t *testing.T) {
 		runDetailedDiffTest(t, propertyMapProp1Val1, propertyMapProp1Val1, tfs, ps, map[string]*pulumirpc.PropertyDiff{})
 	})
@@ -653,6 +745,18 @@ func TestDetailedDiffObject(t *testing.T) {
 			"foo.prop2": {Kind: pulumirpc.PropertyDiff_ADD},
 		})
 	})
+
+	t.Run("secret added", func(t *testing.T) {
+		runDetailedDiffTest(t, propertyMapProp2, propertyMapWithSecrets, tfs, ps, map[string]*pulumirpc.PropertyDiff{
+			"foo.prop1": {Kind: pulumirpc.PropertyDiff_ADD},
+		})
+	})
+
+	t.Run("secret deleted", func(t *testing.T) {
+		runDetailedDiffTest(t, propertyMapWithSecrets, propertyMapProp2, tfs, ps, map[string]*pulumirpc.PropertyDiff{
+			"foo.prop1": {Kind: pulumirpc.PropertyDiff_DELETE},
+		})
+	})
 }
 
 func TestDetailedDiffList(t *testing.T) {
@@ -822,6 +926,22 @@ func TestDetailedDiffSet(t *testing.T) {
 		},
 	)
 
+	propertyMapWithSecrets := resource.NewPropertyMapFromMap(
+		map[string]interface{}{
+			"foo": []interface{}{resource.NewSecretProperty(
+				&resource.Secret{Element: resource.NewStringProperty("val1")}), "val2"},
+		},
+	)
+
+	propertyMapWithSecretsAndOutputs := resource.NewPropertyMapFromMap(
+		map[string]interface{}{
+			"foo": []interface{}{
+				resource.NewSecretProperty(&resource.Secret{Element: resource.NewStringProperty("val1")}),
+				resource.NewOutputProperty(resource.Output{Element: resource.NewStringProperty("val2")}),
+			},
+		},
+	)
+
 	t.Run("unchanged", func(t *testing.T) {
 		runDetailedDiffTest(t, propertyMapVal1, propertyMapVal1, tfs, ps, map[string]*pulumirpc.PropertyDiff{})
 	})
@@ -870,6 +990,38 @@ func TestDetailedDiffSet(t *testing.T) {
 			"foo[1]": {Kind: pulumirpc.PropertyDiff_ADD},
 		})
 	})
+
+	t.Run("secret added", func(t *testing.T) {
+		runDetailedDiffTest(t, propertyMapVal2, propertyMapWithSecrets, tfs, ps, map[string]*pulumirpc.PropertyDiff{
+			"foo[0]": {Kind: pulumirpc.PropertyDiff_ADD},
+		})
+	})
+
+	t.Run("secret and output added", func(t *testing.T) {
+		runDetailedDiffTest(
+			t, propertyMapEmpty, propertyMapWithSecretsAndOutputs, tfs, ps, map[string]*pulumirpc.PropertyDiff{
+				"foo[0]": {Kind: pulumirpc.PropertyDiff_ADD},
+				"foo[1]": {Kind: pulumirpc.PropertyDiff_ADD},
+			})
+	})
+
+	t.Run("secret removed", func(t *testing.T) {
+		runDetailedDiffTest(t, propertyMapWithSecrets, propertyMapVal2, tfs, ps, map[string]*pulumirpc.PropertyDiff{
+			"foo[0]": {Kind: pulumirpc.PropertyDiff_DELETE},
+		})
+	})
+
+	t.Run("output removed", func(t *testing.T) {
+		runDetailedDiffTest(t, propertyMapWithSecretsAndOutputs, propertyMapVal1, tfs, ps, map[string]*pulumirpc.PropertyDiff{
+			"foo[1]": {Kind: pulumirpc.PropertyDiff_DELETE},
+		})
+	})
+
+	t.Run("secretness and outputness changed", func(t *testing.T) {
+		runDetailedDiffTest(t, propertyMapWithSecretsAndOutputs, propertyMapBoth, tfs, ps, map[string]*pulumirpc.PropertyDiff{
+			"foo[1]": {Kind: pulumirpc.PropertyDiff_UPDATE},
+		})
+	})
 }
 
 func TestDetailedDiffTFForceNewPlain(t *testing.T) {