Skip to content

Commit

Permalink
Merge #11021
Browse files Browse the repository at this point in the history
11021: Avoid backfilling property deps for Go r=justinvp a=justinvp

Two changes:

## 1. Go SDK

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`.

**Node.js:**

https://github.com/pulumi/pulumi/blob/a4dbd1da4fb0b5df716aeda56f9caf7d2d5175e4/sdk/nodejs/runtime/rpc.ts#L140-L143

**Python:**

https://github.com/pulumi/pulumi/blob/a4dbd1da4fb0b5df716aeda56f9caf7d2d5175e4/sdk/python/lib/pulumi/runtime/rpc.py#L211-L223

**.NET:**

https://github.com/pulumi/pulumi/blob/a4dbd1da4fb0b5df716aeda56f9caf7d2d5175e4/sdk/dotnet/Pulumi/Deployment/Deployment_Serialization.cs#L79-L83

**Java:**

https://github.com/pulumi/pulumi-java/blob/33e2d98ad96890937f5374e7b6699fc518edfc19/sdk/java/pulumi/src/main/java/com/pulumi/serialization/internal/PropertiesSerializer.java#L67-L72

## 2. Engine

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.

Fixes #10951


Co-authored-by: Justin Van Patten <jvp@justinvp.com>
  • Loading branch information
bors[bot] and justinvp committed Oct 14, 2022
2 parents 69e3bf9 + a4dbd1d commit 761d1c7
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 8 deletions.
@@ -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
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
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
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 761d1c7

Please sign in to comment.