From a8028493d84bcf4d03c27051cf5fce2178501364 Mon Sep 17 00:00:00 2001 From: Venelin Date: Wed, 20 Nov 2024 17:41:49 +0000 Subject: [PATCH] Detailed diff handle secrets and outputs --- pkg/pf/tests/diff_secret_test.go | 14 ++--- pkg/tfbridge/detailed_diff.go | 8 +++ pkg/tfbridge/detailed_diff_test.go | 74 ++++++++++++++++++++++++- unstable/propertyvalue/propertyvalue.go | 16 ++++++ 4 files changed, 103 insertions(+), 9 deletions(-) diff --git a/pkg/pf/tests/diff_secret_test.go b/pkg/pf/tests/diff_secret_test.go index af5ea5e321..d6de0e9623 100644 --- a/pkg/pf/tests/diff_secret_test.go +++ b/pkg/pf/tests/diff_secret_test.go @@ -57,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 @@ -202,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 @@ -223,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 @@ -248,7 +249,6 @@ Resources: [id=test-id] [urn=urn:pulumi:test::test::testprovider:index/test:Test::mainRes] ~ key: { - prop1: [secret] ~ prop2: "value2" => "value3" } Resources: @@ -302,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/tfbridge/detailed_diff.go b/pkg/tfbridge/detailed_diff.go index 6566cc7d07..437b399526 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 { @@ -480,6 +481,13 @@ func MakeDetailedDiffV2( ps map[string]*SchemaInfo, priorProps, props, newInputs resource.PropertyMap, ) map[string]*pulumirpc.PropertyDiff { + 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 fbfe92df60..46817a1a8d 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,6 +483,33 @@ func TestBasicDetailedDiff(t *testing.T) { "foo": ComputedVal, }, ) + propertyMapSecretValue1 := resource.NewPropertyMapFromMap( + map[string]interface{}{ + "foo": tt.value1, + }, + ) + propertyMapSecretValue1["foo"] = resource.NewSecretProperty(&resource.Secret{Element: propertyMapValue1["foo"]}) + + propertyMapSecretValue2 := resource.NewPropertyMapFromMap( + map[string]interface{}{ + "foo": tt.value2, + }, + ) + propertyMapSecretValue2["foo"] = resource.NewSecretProperty(&resource.Secret{Element: propertyMapValue2["foo"]}) + + propertyMapOutputValue1 := resource.NewPropertyMapFromMap( + map[string]interface{}{ + "foo": tt.value1, + }, + ) + propertyMapOutputValue1["foo"] = resource.NewOutputProperty(resource.Output{Element: propertyMapValue1["foo"]}) + + propertyMapOutputValue2 := resource.NewPropertyMapFromMap( + map[string]interface{}{ + "foo": tt.value2, + }, + ) + propertyMapOutputValue2["foo"] = resource.NewOutputProperty(resource.Output{Element: propertyMapValue2["foo"]}) t.Run("unchanged", func(t *testing.T) { runDetailedDiffTest(t, propertyMapValue1, propertyMapValue1, tfs, ps, map[string]*pulumirpc.PropertyDiff{}) @@ -566,6 +592,50 @@ 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) { + runDetailedDiffTest(t, propertyMapSecretValue1, propertyMapSecretValue2, tfs, ps, updated()) + }) + + 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) { + runDetailedDiffTest(t, propertyMapOutputValue1, propertyMapOutputValue2, tfs, ps, updated()) + }) }) } } diff --git a/unstable/propertyvalue/propertyvalue.go b/unstable/propertyvalue/propertyvalue.go index 66aadb1283..7a4239ecda 100644 --- a/unstable/propertyvalue/propertyvalue.go +++ b/unstable/propertyvalue/propertyvalue.go @@ -110,6 +110,22 @@ func RemoveSecrets(pv resource.PropertyValue) resource.PropertyValue { return Transform(unsecret, pv) } +func RemoveSecretsAndOutputs(pv resource.PropertyValue) resource.PropertyValue { + return Transform(func(pv resource.PropertyValue) resource.PropertyValue { + if pv.IsSecret() { + return pv.SecretValue().Element + } + if pv.IsOutput() { + o := pv.OutputValue() + if !o.Known { + return resource.NewComputedProperty(resource.Computed{Element: resource.NewStringProperty("")}) + } + return o.Element + } + return pv + }, pv) +} + func extendPath(p resource.PropertyPath, segment any) resource.PropertyPath { rp := make(resource.PropertyPath, len(p)+1) copy(rp, p)