Skip to content

Commit

Permalink
Detailed diff handle secrets and outputs (#2643)
Browse files Browse the repository at this point in the history 
This change adds secret and output handling to the detailed diff v2
code. Secrets and outputs are not necessary for the algorithm so we just
strip both from the inputs.

fixes #2526
fixes #2651
  • Loading branch information
@VenelinMartinov
VenelinMartinov authored Nov 26, 2024
1 parent 264a002 commit 9319c4b
Show file tree
Hide file tree
Showing 5 changed files with 189 additions and 29 deletions.
18 changes: 7 additions & 11 deletions pkg/pf/tests/diff_secret_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
)

func TestSecretBasic(t *testing.T) {
t.Skip("skipping until #2643")
t.Parallel()
provBuilder := providerbuilder.NewProvider(
providerbuilder.NewProviderArgs{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -160,7 +158,6 @@ Resources:
}

func TestSecretObjectBlock(t *testing.T) {
t.Skip("skipping until #2643")
t.Parallel()

provBuilder := pb.NewProvider(pb.NewProviderArgs{
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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:
Expand All @@ -262,7 +259,6 @@ Resources:
}

func TestSecretPulumiSchema(t *testing.T) {
t.Skip("skipping until #2643")
t.Parallel()

provBuilder := pb.NewProvider(pb.NewProviderArgs{
Expand Down Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions pkg/pf/tests/provider_diff_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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 := `
Expand Down
9 changes: 5 additions & 4 deletions pkg/pf/tests/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Expand Down
14 changes: 14 additions & 0 deletions pkg/tfbridge/detailed_diff.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
174 changes: 163 additions & 11 deletions pkg/tfbridge/detailed_diff_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
})
Expand Down Expand Up @@ -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)
})
})
}
}
Expand Down Expand Up @@ -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{})
})
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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{})
})
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 9319c4b

Please sign in to comment.