diff --git a/pkg/pf/tests/diff_secret_test.go b/pkg/pf/tests/diff_secret_test.go index 75fa89d83..d6de0e962 100644 --- a/pkg/pf/tests/diff_secret_test.go +++ b/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,7 +57,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 @@ -66,7 +65,6 @@ Resources: } 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 diff --git a/pkg/pf/tests/provider_diff_test.go b/pkg/pf/tests/provider_diff_test.go index c3158eefc..c551823a0 100644 --- a/pkg/pf/tests/provider_diff_test.go +++ b/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 := ` diff --git a/pkg/pf/tests/util.go b/pkg/pf/tests/util.go index 044c662b4..7eac4e7b2 100644 --- a/pkg/pf/tests/util.go +++ b/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))) diff --git a/pkg/tfbridge/detailed_diff.go b/pkg/tfbridge/detailed_diff.go index a50e8563b..e241e90ee 100644 --- a/pkg/tfbridge/detailed_diff.go +++ b/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) } diff --git a/pkg/tfbridge/detailed_diff_test.go b/pkg/tfbridge/detailed_diff_test.go index fbfe92df6..2130d0ce8 100644 --- a/pkg/tfbridge/detailed_diff_test.go +++ b/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) {