Skip to content

Commit

Permalink
Avoid backfilling property deps for Go
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
justinvp committed Oct 14, 2022
1 parent f713611 commit a4dbd1d
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
changes:
- type: fix
scope: sdk/go
description: Avoid backfilling property deps for Go
5 changes: 3 additions & 2 deletions pkg/resource/deploy/source_eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 1 addition & 4 deletions sdk/go/pulumi/rpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
21 changes: 19 additions & 2 deletions sdk/go/pulumi/rpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)
}

0 comments on commit a4dbd1d

Please sign in to comment.