[events] Retain Secret values. (#9351)

Instead of replacing `Secret` values with the text `[secret]`, retain
the `Secret` value and replace its element with the text `[secret]`.
This allows downstream consumers (e.g. the CLI's diff renderer) to
understand that the contents of the value is secret if necessary.

CHANGELOG_PENDING.md

@@ -6,5 +6,8 @@
 - [cli] Warn users when there are pending operations but proceed with deployment
   [#9293](https://github.com/pulumi/pulumi/pull/9293)
 
+- [cli] Display more useful diffs for secrets that are not primitive values
+  [#9351](https://github.com/pulumi/pulumi/pull/9351)
+
 ### Bug Fixes
 

pkg/engine/diff.go

@@ -466,7 +466,7 @@ func printPropertyValue(
 	indent int, op deploy.StepOp, prefix bool, debug bool) {
 
 	switch {
-	case isPrimitive(v) || v.IsSecret():
+	case isPrimitive(v):
 		printPrimitivePropertyValue(b, v, planning, op)
 	case v.IsArray():
 		arr := v.ArrayValue()
@@ -701,7 +701,7 @@ func printPropertyValueDiff(
 
 func isPrimitive(value resource.PropertyValue) bool {
 	return value.IsNull() || value.IsString() || value.IsNumber() ||
-		value.IsBool() || value.IsComputed() || value.IsOutput()
+		value.IsBool() || value.IsComputed() || value.IsOutput() || value.IsSecret()
 }
 
 func printPrimitivePropertyValue(b io.StringWriter, v resource.PropertyValue, planning bool, op deploy.StepOp) {
@@ -728,6 +728,8 @@ func printPrimitivePropertyValue(b io.StringWriter, v resource.PropertyValue, pl
 		} else {
 			write(b, op, "undefined")
 		}
+	} else if v.IsSecret() {
+		write(b, op, "[secret]")
 	} else {
 		contract.Failf("Unexpected property value kind '%v'", v)
 	}

pkg/engine/events.go

@@ -16,7 +16,6 @@ package engine
 
 import (
 	"bytes"
-	"reflect"
 	"time"
 
 	"github.com/pulumi/pulumi/pkg/v3/resource/deploy"
@@ -337,130 +336,13 @@ func makeStepEventStateMetadata(state *resource.State, debug bool) *StepEventSta
 		ID:         state.ID,
 		Parent:     state.Parent,
 		Protect:    state.Protect,
-		Inputs:     filterPropertyMap(state.Inputs, debug),
-		Outputs:    filterPropertyMap(state.Outputs, debug),
+		Inputs:     filterResourceProperties(state.Inputs, debug),
+		Outputs:    filterResourceProperties(state.Outputs, debug),
 		Provider:   state.Provider,
 		InitErrors: state.InitErrors,
 	}
 }
 
-func filterPropertyMap(propertyMap resource.PropertyMap, debug bool) resource.PropertyMap {
-	mappable := propertyMap.Mappable()
-
-	var filterValue func(v interface{}) interface{}
-
-	filterPropertyValue := func(pv resource.PropertyValue) resource.PropertyValue {
-		return resource.NewPropertyValue(filterValue(pv.Mappable()))
-	}
-
-	// filter values walks unwrapped (i.e. non-PropertyValue) values and applies the filter function
-	// to them recursively.  The only thing the filter actually applies to is strings.
-	//
-	// The return value of this function should have the same type as the input value.
-	filterValue = func(v interface{}) interface{} {
-		if v == nil {
-			return nil
-		}
-
-		// Else, check for some known primitive types.
-		switch t := v.(type) {
-		case bool, int, uint, int32, uint32,
-			int64, uint64, float32, float64:
-			// simple types.  map over as is.
-			return v
-		case string:
-			// have to ensure we filter out secrets.
-			return logging.FilterString(t)
-		case *resource.Asset:
-			text := t.Text
-			if text != "" {
-				// we don't want to include the full text of an asset as we serialize it over as
-				// events.  They represent user files and are thus are unbounded in size.  Instead,
-				// we only include the text if it represents a user's serialized program code, as
-				// that is something we want the receiver to see to display as part of
-				// progress/diffs/etc.
-				if t.IsUserProgramCode() {
-					// also make sure we filter this in case there are any secrets in the code.
-					text = logging.FilterString(resource.MassageIfUserProgramCodeAsset(t, debug).Text)
-				} else {
-					// We need to have some string here so that we preserve that this is a
-					// text-asset
-					text = "<stripped>"
-				}
-			}
-
-			return &resource.Asset{
-				Sig:  t.Sig,
-				Hash: t.Hash,
-				Text: text,
-				Path: t.Path,
-				URI:  t.URI,
-			}
-		case *resource.Archive:
-			return &resource.Archive{
-				Sig:    t.Sig,
-				Hash:   t.Hash,
-				Path:   t.Path,
-				URI:    t.URI,
-				Assets: filterValue(t.Assets).(map[string]interface{}),
-			}
-		case resource.Secret:
-			return "[secret]"
-		case resource.Computed:
-			return resource.Computed{
-				Element: filterPropertyValue(t.Element),
-			}
-		case resource.Output:
-			return resource.Output{
-				Element: filterPropertyValue(t.Element),
-			}
-		case resource.ResourceReference:
-			return resource.ResourceReference{
-				URN:            resource.URN(filterValue(string(t.URN)).(string)),
-				ID:             resource.PropertyValue{V: filterValue(t.ID.V)},
-				PackageVersion: filterValue(t.PackageVersion).(string),
-			}
-		}
-
-		// Next, see if it's an array, slice, pointer or struct, and handle each accordingly.
-		rv := reflect.ValueOf(v)
-		switch rk := rv.Type().Kind(); rk {
-		case reflect.Array, reflect.Slice:
-			// If an array or slice, just create an array out of it.
-			var arr []interface{}
-			for i := 0; i < rv.Len(); i++ {
-				arr = append(arr, filterValue(rv.Index(i).Interface()))
-			}
-			return arr
-		case reflect.Ptr:
-			if rv.IsNil() {
-				return nil
-			}
-
-			v1 := filterValue(rv.Elem().Interface())
-			return &v1
-		case reflect.Map:
-			obj := make(map[string]interface{})
-			for _, key := range rv.MapKeys() {
-				k := key.Interface().(string)
-				v := rv.MapIndex(key).Interface()
-				obj[k] = filterValue(v)
-			}
-			return obj
-		default:
-			contract.Failf("Unrecognized value type: type=%v kind=%v", rv.Type(), rk)
-		}
-
-		return nil
-	}
-
-	return resource.NewPropertyMapFromMapRepl(
-		mappable, nil, /*replk*/
-		func(v interface{}) (resource.PropertyValue, bool) {
-			return resource.NewPropertyValue(filterValue(v)), true
-		})
-}
-
 func (e *eventEmitter) Close() {
 	close(e.ch)
 	<-e.done
@@ -605,3 +487,99 @@ func (e *eventEmitter) diagErrorEvent(d *diag.Diag, prefix, msg string, ephemera
 func (e *eventEmitter) diagWarningEvent(d *diag.Diag, prefix, msg string, ephemeral bool) {
 	diagEvent(e, d, prefix, msg, diag.Warning, ephemeral)
 }
+
+func filterResourceProperties(m resource.PropertyMap, debug bool) resource.PropertyMap {
+	return filterPropertyValue(resource.NewObjectProperty(m), debug).ObjectValue()
+}
+
+func filterPropertyValue(v resource.PropertyValue, debug bool) resource.PropertyValue {
+	switch {
+	case v.IsNull(), v.IsBool(), v.IsNumber():
+		return v
+	case v.IsString():
+		// have to ensure we filter out secrets.
+		return resource.NewStringProperty(logging.FilterString(v.StringValue()))
+	case v.IsAsset():
+		return resource.NewAssetProperty(filterAsset(v.AssetValue(), debug))
+	case v.IsArchive():
+		return resource.NewArchiveProperty(filterArchive(v.ArchiveValue(), debug))
+	case v.IsArray():
+		arr := make([]resource.PropertyValue, len(v.ArrayValue()))
+		for i, v := range v.ArrayValue() {
+			arr[i] = filterPropertyValue(v, debug)
+		}
+		return resource.NewArrayProperty(arr)
+	case v.IsObject():
+		obj := make(resource.PropertyMap, len(v.ObjectValue()))
+		for k, v := range v.ObjectValue() {
+			obj[k] = filterPropertyValue(v, debug)
+		}
+		return resource.NewObjectProperty(obj)
+	case v.IsComputed():
+		return resource.MakeComputed(filterPropertyValue(v.Input().Element, debug))
+	case v.IsOutput():
+		return resource.MakeComputed(filterPropertyValue(v.OutputValue().Element, debug))
+	case v.IsSecret():
+		return resource.MakeSecret(resource.NewStringProperty("[secret]"))
+	case v.IsResourceReference():
+		ref := v.ResourceReferenceValue()
+		return resource.NewResourceReferenceProperty(resource.ResourceReference{
+			URN:            resource.URN(logging.FilterString(string(ref.URN))),
+			ID:             filterPropertyValue(ref.ID, debug),
+			PackageVersion: logging.FilterString(ref.PackageVersion),
+		})
+	default:
+		contract.Failf("unexpected property value type %T", v.V)
+		return resource.PropertyValue{}
+	}
+}
+
+func filterAsset(v *resource.Asset, debug bool) *resource.Asset {
+	if !v.IsText() {
+		return v
+	}
+
+	// we don't want to include the full text of an asset as we serialize it over as
+	// events.  They represent user files and are thus are unbounded in size.  Instead,
+	// we only include the text if it represents a user's serialized program code, as
+	// that is something we want the receiver to see to display as part of
+	// progress/diffs/etc.
+	var text string
+	if v.IsUserProgramCode() {
+		// also make sure we filter this in case there are any secrets in the code.
+		text = logging.FilterString(resource.MassageIfUserProgramCodeAsset(v, debug).Text)
+	} else {
+		// We need to have some string here so that we preserve that this is a
+		// text-asset
+		text = "<contents elided>"
+	}
+
+	return &resource.Asset{
+		Sig:  v.Sig,
+		Hash: v.Hash,
+		Text: text,
+	}
+}
+
+func filterArchive(v *resource.Archive, debug bool) *resource.Archive {
+	if !v.IsAssets() {
+		return v
+	}
+
+	assets := make(map[string]interface{})
+	for k, v := range v.Assets {
+		switch v := v.(type) {
+		case *resource.Asset:
+			assets[k] = filterAsset(v, debug)
+		case *resource.Archive:
+			assets[k] = filterArchive(v, debug)
+		default:
+			contract.Failf("Unrecognized asset map type %T", v)
+		}
+	}
+	return &resource.Archive{
+		Sig:    v.Sig,
+		Hash:   v.Hash,
+		Assets: assets,
+	}
+}

pkg/engine/lifeycletest/pulumi_test.go

@@ -4811,3 +4811,102 @@ func TestPlannedUpdateWithCheckFailure(t *testing.T) {
 		return
 	}
 }
+
+func TestEventSecrets(t *testing.T) {
+	t.Parallel()
+
+	loaders := []*deploytest.ProviderLoader{
+		deploytest.NewProviderLoader("pkgA", semver.MustParse("1.0.0"), func() (plugin.Provider, error) {
+			return &deploytest.Provider{
+				DiffF: func(urn resource.URN, id resource.ID,
+					olds, news resource.PropertyMap, ignoreChanges []string) (plugin.DiffResult, error) {
+
+					diff := olds.Diff(news)
+					if diff == nil {
+						return plugin.DiffResult{Changes: plugin.DiffNone}, nil
+					}
+					detailedDiff := plugin.NewDetailedDiffFromObjectDiff(diff)
+					changedKeys := diff.ChangedKeys()
+
+					return plugin.DiffResult{
+						Changes:      plugin.DiffSome,
+						ChangedKeys:  changedKeys,
+						DetailedDiff: detailedDiff,
+					}, nil
+				},
+
+				UpdateF: func(urn resource.URN, id resource.ID, olds, news resource.PropertyMap, timeout float64,
+					ignoreChanges []string, preview bool) (resource.PropertyMap, resource.Status, error) {
+					return news, resource.StatusOK, nil
+				},
+				CreateF: func(urn resource.URN, inputs resource.PropertyMap, timeout float64,
+					preview bool) (resource.ID, resource.PropertyMap, resource.Status, error) {
+					return resource.ID("id123"), inputs, resource.StatusOK, nil
+				},
+			}, nil
+		}),
+	}
+
+	var inputs resource.PropertyMap
+	program := deploytest.NewLanguageRuntime(func(_ plugin.RunInfo, monitor *deploytest.ResourceMonitor) error {
+		_, _, _, err := monitor.RegisterResource("pkgA:m:typA", "resA", true, deploytest.ResourceOptions{
+			Inputs: inputs,
+		})
+		assert.NoError(t, err)
+		return nil
+	})
+	host := deploytest.NewPluginHost(nil, nil, program, loaders...)
+
+	p := &TestPlan{
+		Options: UpdateOptions{Host: host},
+		Steps: []TestStep{{
+			Op:          Update,
+			SkipPreview: true,
+		}},
+	}
+
+	inputs = resource.PropertyMap{
+		"webhooks": resource.MakeSecret(resource.NewArrayProperty([]resource.PropertyValue{
+			resource.NewObjectProperty(resource.PropertyMap{
+				"clientConfig": resource.NewObjectProperty(resource.PropertyMap{
+					"service": resource.NewStringProperty("foo"),
+				}),
+			}),
+		})),
+	}
+	snap := p.Run(t, nil)
+
+	inputs = resource.PropertyMap{
+		"webhooks": resource.MakeSecret(resource.NewArrayProperty([]resource.PropertyValue{
+			resource.NewObjectProperty(resource.PropertyMap{
+				"clientConfig": resource.NewObjectProperty(resource.PropertyMap{
+					"service": resource.NewStringProperty("bar"),
+				}),
+			}),
+		})),
+	}
+	p.Steps[0].Validate = func(project workspace.Project, target deploy.Target, entries JournalEntries,
+		evts []Event, res result.Result) result.Result {
+
+		for _, e := range evts {
+			var step StepEventMetadata
+			switch e.Type {
+			case ResourcePreEvent:
+				step = e.Payload().(ResourcePreEventPayload).Metadata
+			case ResourceOutputsEvent:
+				step = e.Payload().(ResourceOutputsEventPayload).Metadata
+			default:
+				continue
+			}
+			if step.URN.Name() != "resA" {
+				continue
+			}
+
+			assert.True(t, step.Old.Inputs["webhooks"].IsSecret())
+			assert.True(t, step.Old.Outputs["webhooks"].IsSecret())
+			assert.True(t, step.New.Inputs["webhooks"].IsSecret())
+		}
+		return res
+	}
+	p.Run(t, snap)
+}