Handle secrets in update diffs

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - [Fix multi-type resolutions](https://github.com/pulumi/pulumi-aws-native/pull/1383) includes fixes for inline role policies
 - [Include "id" resource properties as "awsId"](https://github.com/pulumi/pulumi-aws-native/pull/1387)
+- [Fix updates with secret values](https://github.com/pulumi/pulumi-aws-native/pull/1397)
 
 ## 0.98.0 (2024-02-29)
 

examples/examples_nodejs_test.go

@@ -37,6 +37,13 @@ func TestUpdate(t *testing.T) {
 				{
 					Dir:      filepath.Join(getCwd(t), "update", "step2"),
 					Additive: true,
+					ExtraRuntimeValidation: func(t *testing.T, stackInfo integration.RuntimeValidationStackInfo) {
+						// Check that the name of the updated secret is correct.
+						secretValue := stackInfo.Outputs["secretValue"].(string)
+						if secretValue != "secretbuzz" {
+							t.Errorf("Expected secretValue to be 'secretbuzz', got %q (%[1]T)", secretValue)
+						}
+					},
 				},
 			},
 		})

examples/lambda-update/step1/index.ts

@@ -53,7 +53,7 @@ export const layerCode = new aws.s3.BucketObject(
       ),
     }),
   },
-  { ignoreChanges: ["source"] }
+  { ignoreChanges: ["source", "versionId"] }
 );
 
 const graphqlPublicRole = new aws.iam.Role("lambda-role", {

examples/update/step1/index.ts

@@ -1,13 +1,18 @@
 // Copyright 2016-2021, Pulumi Corporation.
 
+import * as pulumi from "@pulumi/pulumi";
 import * as aws from "@pulumi/aws-native";
 
-const logGroup = new aws.s3.Bucket("bucket", {
+const bucket = new aws.s3.Bucket("bucket", {
   tags: [
     {
       key: "foo",
       value: "bar",
     },
+    {
+      key: "secretfoo",
+      value: pulumi.secret("secretbar"),
+    },
   ],
 });
 

examples/update/step2/index.ts

@@ -1,13 +1,18 @@
 // Copyright 2016-2021, Pulumi Corporation.
 
+import * as pulumi from "@pulumi/pulumi";
 import * as aws from "@pulumi/aws-native";
 
-const logGroup = new aws.s3.Bucket("bucket", {
+const bucket = new aws.s3.Bucket("bucket", {
   tags: [
     {
       key: "foo",
       value: "buzz", // <-- this value has changed
     },
+    {
+      key: "secretfoo",
+      value: pulumi.secret("secretbuzz"), // <-- this value has changed
+    },
   ],
 });
 
@@ -41,3 +46,5 @@ const func = new aws.lambda.Function("function", {
   `,
   },
 });
+
+export const secretValue = bucket.tags.apply((tags) => tags!.find((t) => t.key === "secretfoo")?.value);

provider/go.mod

@@ -161,6 +161,7 @@ require (
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/pulumi/appdash v0.0.0-20231130102222-75f619a67231 // indirect
 	github.com/pulumi/esc v0.6.2 // indirect
+	github.com/pulumi/pulumi-go-provider v0.14.1-0.20240312115843-96cd54a979b1 // indirect
 	github.com/rivo/uniseg v0.4.4 // indirect
 	github.com/rogpeppe/go-internal v1.11.0 // indirect
 	github.com/ryanuber/go-glob v1.0.0 // indirect
@@ -182,7 +183,7 @@ require (
 	github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect
 	github.com/xeipuuv/gojsonschema v1.2.0 // indirect
 	go.opencensus.io v0.24.0 // indirect
-	go.uber.org/atomic v1.9.0 // indirect
+	go.uber.org/atomic v1.10.0 // indirect
 	gocloud.dev v0.36.0 // indirect
 	gocloud.dev/secrets/hashivault v0.27.0 // indirect
 	golang.org/x/crypto v0.18.0 // indirect

provider/go.sum

@@ -1519,6 +1519,8 @@ github.com/pulumi/jsschema v0.0.0-20240222154951-2ba83b66ea54 h1:bKq2LqtQOtab1Zu
 github.com/pulumi/jsschema v0.0.0-20240222154951-2ba83b66ea54/go.mod h1:nXH0b0s3hX5wMv1UlCz9n/O6832x4dPIVl7t5aonm0Q=
 github.com/pulumi/providertest v0.0.10 h1:bx77G0JYPO2Alf/SHRP05XpAYMrboKJkMIVkbFclVhI=
 github.com/pulumi/providertest v0.0.10/go.mod h1:HsxjVsytcMIuNj19w1lT2W0QXY0oReXl1+h6eD2JXP8=
+github.com/pulumi/pulumi-go-provider v0.14.1-0.20240312115843-96cd54a979b1 h1:qs1UQ2DyGmx7pixzw3WC9iPT0Cnajbw+etx2fyaMerg=
+github.com/pulumi/pulumi-go-provider v0.14.1-0.20240312115843-96cd54a979b1/go.mod h1:fpmTbPgwdzd4OtUcgE0wYRNzqEY5r88T/c3J1UM/lpA=
 github.com/pulumi/pulumi/pkg/v3 v3.107.0 h1:HRyIl1c9ur0PVQW+GuFL1APBEuGa/fQQMp3F+WluxW8=
 github.com/pulumi/pulumi/pkg/v3 v3.107.0/go.mod h1:7edfZu4FlrXdIn4339tJ+SQX5VKGqbFntmpc8cai0Zg=
 github.com/pulumi/pulumi/sdk/v3 v3.107.0 h1:bef+ayh9+4KkAqXih4EjlHfQXRY24NWPwWBIQhBxTjg=
@@ -1796,6 +1798,8 @@ go.uber.org/atomic v1.6.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ=
 go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
 go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE=
 go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
+go.uber.org/atomic v1.10.0 h1:9qC72Qh0+3MqyJbAn8YU5xVq1frD8bn3JtD2oXtafVQ=
+go.uber.org/atomic v1.10.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0=
 go.uber.org/automaxprocs v1.5.1/go.mod h1:BF4eumQw0P9GtnuxxovUd06vwm1o18oMzFtK66vU6XU=
 go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A=
 go.uber.org/goleak v1.1.11/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ=

provider/pkg/provider/provider.go

@@ -54,6 +54,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/pulumi/pulumi-aws-native/provider/pkg/schema"
 	"github.com/pulumi/pulumi-aws-native/provider/pkg/version"
+	"github.com/pulumi/pulumi-go-provider/resourcex"
 	"github.com/pulumi/pulumi/pkg/v3/resource/provider"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/diag"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
@@ -811,7 +812,7 @@ func (p *cfnProvider) Create(ctx context.Context, req *pulumirpc.CreateRequest)
 	switch resourceToken {
 	case schema.ExtensionResourceToken:
 		// For a custom resource, both CF type and inputs shape are defined explicitly in the SDK.
-		inputsMap := inputs.Mappable()
+		inputsMap := resourcex.Decode(inputs)
 		if v, has := inputsMap["type"].(string); has {
 			cfType = v
 		} else {
@@ -830,7 +831,7 @@ func (p *cfnProvider) Create(ctx context.Context, req *pulumirpc.CreateRequest)
 		cfType = spec.CfType
 
 		// Convert SDK inputs to CFN payload.
-		payload, err = schema.SdkToCfn(&spec, p.resourceMap.Types, inputs.MapRepl(nil, mapReplStripSecrets))
+		payload, err = schema.SdkToCfn(&spec, p.resourceMap.Types, resourcex.Decode(inputs))
 		if err != nil {
 			return nil, fmt.Errorf("Failed to convert value for %s: %w", resourceToken, err)
 		}
@@ -1316,14 +1317,6 @@ func hasFinished(pi *types.ProgressEvent) (bool, error) {
 	}
 }
 
-func mapReplStripSecrets(v resource.PropertyValue) (interface{}, bool) {
-	if v.IsSecret() {
-		return v.SecretValue().Element.MapRepl(nil, mapReplStripSecrets), true
-	}
-
-	return nil, false
-}
-
 // diffState extracts old and new inputs and calculates a diff between them.
 func (p *cfnProvider) diffState(olds *pbstruct.Struct, newInputs resource.PropertyMap, label string) (*resource.ObjectDiff, error) {
 	oldState, err := plugin.UnmarshalProperties(olds, plugin.MarshalOptions{

provider/pkg/schema/convert.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/mattbaird/jsonpatch"
+	"github.com/pulumi/pulumi-go-provider/resourcex"
 	pschema "github.com/pulumi/pulumi/pkg/v3/codegen/schema"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
 )
@@ -209,6 +210,8 @@ func (c *sdkToCfnConverter) diffToPatch(diff *resource.ObjectDiff) ([]jsonpatch.
 func (c *sdkToCfnConverter) valueToPatch(opName, propName string, prop pschema.PropertySpec, value resource.PropertyValue) (*jsonpatch.JsonPatchOperation, error) {
 	op := jsonpatch.NewPatch(opName, "/"+propName, nil)
 	switch {
+	case value.IsSecret():
+		return c.valueToPatch(opName, propName, prop, value.SecretValue().Element)
 	case value.IsNumber() && prop.Type == "integer":
 		i := int32(value.NumberValue())
 		op.Value = i
@@ -219,8 +222,7 @@ func (c *sdkToCfnConverter) valueToPatch(opName, propName string, prop pschema.P
 	case value.IsString():
 		op.Value = value.StringValue()
 	default:
-		sdkObj := value.MapRepl(nil, nil)
-		cfnObj, err := c.sdkTypedValueToCfn(&prop.TypeSpec, sdkObj)
+		cfnObj, err := c.sdkTypedValueToCfn(&prop.TypeSpec, resourcex.DecodeValue(value))
 		if err != nil {
 			return nil, err
 		}

provider/pkg/schema/convert_test.go

@@ -83,47 +83,83 @@ func TestSdkToCfnOneOf(t *testing.T) {
 }
 
 func TestDiffToPatch(t *testing.T) {
-	diff := resource.ObjectDiff{
-		Updates: map[resource.PropertyKey]resource.ValueDiff{
-			"desiredCount":         {New: resource.NewNumberProperty(2)},
-			"enableEcsManagedTags": {New: resource.NewBoolProperty(true)},
-			"loadBalancers": {
-				New: resource.NewArrayProperty([]resource.PropertyValue{
-					resource.NewObjectProperty(resource.NewPropertyMapFromMap(map[string]interface{}{
-						"containerName": "my-app",
-						"containerPort": 80,
-					})),
-				}),
-			},
-		},
-		Adds: map[resource.PropertyKey]resource.PropertyValue{
-			"launchType": resource.NewStringProperty("FARGATE"),
-		},
-		Deletes: map[resource.PropertyKey]resource.PropertyValue{
-			"platformVersion": resource.NewStringProperty("LATEST"),
-		},
+	test := func(t *testing.T, diff resource.ObjectDiff, expected []jsonpatch.JsonPatchOperation) {
+		res := sampleSchema.Resources["aws-native:ecs:Service"]
+		actual, err := DiffToPatch(&res, sampleSchema.Types, &diff)
+		assert.NoError(t, err)
+		sort.SliceStable(actual, func(i, j int) bool {
+			return actual[i].Path < actual[j].Path
+		})
+		assert.Equal(t, expected, actual)
+
 	}
-	two := int32(2)
-	expected := []jsonpatch.JsonPatchOperation{
-		jsonpatch.NewPatch("replace", "/DesiredCount", two),
-		jsonpatch.NewPatch("replace", "/EnableECSManagedTags", true),
-		jsonpatch.NewPatch("add", "/LaunchType", "FARGATE"),
-		jsonpatch.NewPatch("replace", "/LoadBalancers",
-			[]interface{}{
-				map[string]interface{}{
-					"ContainerName": "my-app",
-					"ContainerPort": 80.,
+
+	t.Run("add-delete-replace", func(t *testing.T) {
+		diff := resource.ObjectDiff{
+			Updates: map[resource.PropertyKey]resource.ValueDiff{
+				"desiredCount":         {New: resource.NewNumberProperty(2)},
+				"enableEcsManagedTags": {New: resource.NewBoolProperty(true)},
+				"loadBalancers": {
+					New: resource.NewArrayProperty([]resource.PropertyValue{
+						resource.NewObjectProperty(resource.PropertyMap{
+							resource.PropertyKey("containerName"): resource.NewStringProperty("my-app"),
+							resource.PropertyKey("containerPort"): resource.NewNumberProperty(80),
+						}),
+					}),
 				},
-			}),
-		jsonpatch.NewPatch("remove", "/PlatformVersion", nil),
-	}
-	res := sampleSchema.Resources["aws-native:ecs:Service"]
-	actual, err := DiffToPatch(&res, sampleSchema.Types, &diff)
-	assert.NoError(t, err)
-	sort.SliceStable(actual, func(i, j int) bool {
-		return actual[i].Path < actual[j].Path
+			},
+			Adds: map[resource.PropertyKey]resource.PropertyValue{
+				"launchType": resource.NewStringProperty("FARGATE"),
+			},
+			Deletes: map[resource.PropertyKey]resource.PropertyValue{
+				"platformVersion": resource.NewStringProperty("LATEST"),
+			},
+		}
+		two := int32(2)
+		expected := []jsonpatch.JsonPatchOperation{
+			jsonpatch.NewPatch("replace", "/DesiredCount", two),
+			jsonpatch.NewPatch("replace", "/EnableECSManagedTags", true),
+			jsonpatch.NewPatch("add", "/LaunchType", "FARGATE"),
+			jsonpatch.NewPatch("replace", "/LoadBalancers",
+				[]interface{}{
+					map[string]interface{}{
+						"ContainerName": "my-app",
+						"ContainerPort": 80.,
+					},
+				}),
+			jsonpatch.NewPatch("remove", "/PlatformVersion", nil),
+		}
+		test(t, diff, expected)
+	})
+
+	t.Run("secrets", func(t *testing.T) {
+		diff := resource.ObjectDiff{
+			Updates: map[resource.PropertyKey]resource.ValueDiff{
+				"cluster": {New: resource.MakeSecret(resource.NewStringProperty("mycluster"))},
+				"loadBalancers": {
+					New: resource.NewArrayProperty([]resource.PropertyValue{
+						resource.NewObjectProperty(resource.PropertyMap{
+							resource.PropertyKey("targetGroupArn"): resource.MakeSecret(resource.NewStringProperty("arn:mytargetgroup")),
+						}),
+					}),
+				},
+			},
+			Adds: map[resource.PropertyKey]resource.PropertyValue{
+				"schedulingStrategy": resource.MakeSecret(resource.NewStringProperty("REPLICA")),
+			},
+		}
+		expected := []jsonpatch.JsonPatchOperation{
+			jsonpatch.NewPatch("replace", "/Cluster", "mycluster"),
+			jsonpatch.NewPatch("replace", "/LoadBalancers",
+				[]interface{}{
+					map[string]interface{}{
+						"TargetGroupArn": "arn:mytargetgroup",
+					},
+				}),
+			jsonpatch.NewPatch("add", "/SchedulingStrategy", "REPLICA"),
+		}
+		test(t, diff, expected)
 	})
-	assert.Equal(t, expected, actual)
 }
 
 var cfnPayload = map[string]interface{}{