From a4dbd1da4fb0b5df716aeda56f9caf7d2d5175e4 Mon Sep 17 00:00:00 2001 From: Justin Van Patten Date: Thu, 13 Oct 2022 17:13:59 -0700 Subject: [PATCH] Avoid backfilling property deps for Go Two changes: 1. Change the Go SDK to always send a filled-in property dependencies map, which is consistent with all of the other language SDKs. This prevents the engine from backfilling an empty map with explicit dependencies specified in the `pulumi.DependsOn` `ResourceOption`. 2. Stop doing the backfilling in the engine when remote is true, because all clients that support remote already support passing property dependencies, so there's no need to backfill. The backfill behavior was meant for very old clients that didn't support sending property dependencies. --- ...o--avoid-backfilling-property-deps-go.yaml | 4 ++++ pkg/resource/deploy/source_eval.go | 5 +++-- sdk/go/pulumi/rpc.go | 5 +---- sdk/go/pulumi/rpc_test.go | 21 +++++++++++++++++-- 4 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 changelog/pending/20221014--sdk-go--avoid-backfilling-property-deps-go.yaml diff --git a/changelog/pending/20221014--sdk-go--avoid-backfilling-property-deps-go.yaml b/changelog/pending/20221014--sdk-go--avoid-backfilling-property-deps-go.yaml new file mode 100644 index 000000000000..d3b9d8fe0314 --- /dev/null +++ b/changelog/pending/20221014--sdk-go--avoid-backfilling-property-deps-go.yaml @@ -0,0 +1,4 @@ +changes: +- type: fix + scope: sdk/go + description: Avoid backfilling property deps for Go diff --git a/pkg/resource/deploy/source_eval.go b/pkg/resource/deploy/source_eval.go index 5db0f1f9f6e7..07f77f723eff 100644 --- a/pkg/resource/deploy/source_eval.go +++ b/pkg/resource/deploy/source_eval.go @@ -1054,9 +1054,10 @@ func (rm *resmon) RegisterResource(ctx context.Context, } propertyDependencies := make(map[resource.PropertyKey][]resource.URN) - if len(req.GetPropertyDependencies()) == 0 { + if len(req.GetPropertyDependencies()) == 0 && !remote { // If this request did not specify property dependencies, treat each property as depending on every resource - // in the request's dependency list. + // in the request's dependency list. We don't need to do this when remote is true, because all clients that + // support remote already support passing property dependencies, so there's no need to backfill here. for pk := range props { propertyDependencies[pk] = dependencies } diff --git a/sdk/go/pulumi/rpc.go b/sdk/go/pulumi/rpc.go index d7cc5b6b49d8..d72fca04b327 100644 --- a/sdk/go/pulumi/rpc.go +++ b/sdk/go/pulumi/rpc.go @@ -143,12 +143,9 @@ func marshalInputs(props Input) (resource.PropertyMap, map[string][]URN, []URN, } deps.union(allDeps) - if len(allDeps) > 0 { - pdeps[pname] = allDeps.values() - } - if !v.IsNull() || len(allDeps) > 0 { pmap[resource.PropertyKey(pname)] = v + pdeps[pname] = allDeps.values() } return nil } diff --git a/sdk/go/pulumi/rpc_test.go b/sdk/go/pulumi/rpc_test.go index b3bc5ee54485..380d2c1f06f3 100644 --- a/sdk/go/pulumi/rpc_test.go +++ b/sdk/go/pulumi/rpc_test.go @@ -238,7 +238,7 @@ func TestMarshalRoundtrip(t *testing.T) { if assert.Nil(t, err) { assert.Equal(t, reflect.TypeOf(inputs).NumField(), len(resolved)) assert.Equal(t, 10, len(deps)) - assert.Equal(t, 10, len(pdeps)) + assert.Equal(t, 25, len(pdeps)) // Now just unmarshal and ensure the resulting map matches. resV, secret, err := unmarshalPropertyValue(ctx, resource.NewObjectProperty(resolved)) @@ -586,7 +586,7 @@ func TestMarshalRoundtripNestedSecret(t *testing.T) { const resourceFields = 10 assert.Equal(t, reflect.TypeOf(inputs).NumField()-resourceFields, len(resolved)) assert.Equal(t, 0, len(deps)) - assert.Equal(t, 0, len(pdeps)) + assert.Equal(t, 15, len(pdeps)) // Now just unmarshal and ensure the resulting map matches. resV, secret, err := unmarshalPropertyValue(ctx, resource.NewObjectProperty(resolved)) @@ -1757,3 +1757,20 @@ func TestOutputValueMarshallingEnums(t *testing.T) { }) } } + +func TestMarshalInputsPropertyDependencies(t *testing.T) { + t.Parallel() + + pmap, pdeps, deps, err := marshalInputs(testInputs{ + S: String("a string"), + A: Bool(true), + }) + assert.NoError(t, err) + assert.Equal(t, resource.PropertyMap{ + "s": resource.NewStringProperty("a string"), + "a": resource.NewBoolProperty(true), + }, pmap) + assert.Equal(t, []URN{}, deps) + // Expect a non-empty property deps map, even when there aren't any deps. + assert.Equal(t, map[string][]URN{"s": {}, "a": {}}, pdeps) +}