Repository: pulumi/pulumi-terraform-bridge
Pull request #1064 (2023-05-01 21:11:31)

Cleanup Empty AutoAliasing Objects
This PR modifies token mapping to not add extra fields for otherwise empty objects. This is necessary because `AutoName` sets a full `tfbridge.SchemaInfo`, it does not modify one.

github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/712e984..93ab911) with merge commit 16a4ff2d13ed95d14c1e04700ed56f5257b7a6a9
github-actions[bot]: Diff for [pulumi-gcp](https://github.com/pulumi-bot/pulumi-gcp/compare/2961762aa..29b7a04dd) with merge commit 16a4ff2d13ed95d14c1e04700ed56f5257b7a6a9
github-actions[bot]: Diff for [pulumi-azure](https://github.com/pulumi-bot/pulumi-azure/compare/126c5e19c..79d24223b) with merge commit 16a4ff2d13ed95d14c1e04700ed56f5257b7a6a9
t0yv0: How does this connect to the user problem prompting this PR, would it be possible to elaborate? The tests are at a level of indirection, no elucidating new tests are added,  and it's not obvious how the problem is being fixed.

```
I believe you will recover old behavior by putting p.SetAutonaming before the call to x.AutoAliasing.
```

After this PR do we believe that the order of p.SetAutonaming and x.AutoAliasing does not matter anymore? 

Does editing the existing test not weaken the MaxItems one test strength?

I'm going to be able to review tomorrow morning but it's not an obvious fix to review in a tired state.
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/0fd6a6d..76317aa) with merge commit dd58726ff1f7b2c7984a83c6e646082772e81173
github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/ddc4f59..6ee416a) with merge commit dd58726ff1f7b2c7984a83c6e646082772e81173
github-actions[bot]: Diff for [pulumi-gcp](https://github.com/pulumi-bot/pulumi-gcp/compare/2102a0bb6..ff74e1c84) with merge commit dd58726ff1f7b2c7984a83c6e646082772e81173
github-actions[bot]: Diff for [pulumi-azure](https://github.com/pulumi-bot/pulumi-azure/compare/bcaf991c1..8d5a1fe5d) with merge commit dd58726ff1f7b2c7984a83c6e646082772e81173
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/9d3cfd3..d7e09e4) with merge commit 1a05aa09a3a4e1d3e9307b49040bbffe19c11936
github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/23bd3ef..aee81c4) with merge commit 1a05aa09a3a4e1d3e9307b49040bbffe19c11936
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/987640e..53178cb) with merge commit 4c501add6f52ef9816184cf04a405929a8252fec
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/e19b1a9..ee38171) with merge commit 5ea397d613d36c35049b8945835e80fd991e5f6a
github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/b0c7b54..af27762) with merge commit 4c501add6f52ef9816184cf04a405929a8252fec
github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/9e16e9f..e042ab0) with merge commit 5ea397d613d36c35049b8945835e80fd991e5f6a
github-actions[bot]: Diff for [pulumi-gcp](https://github.com/pulumi-bot/pulumi-gcp/compare/ac8f22681..e5f910a65) with merge commit 1a05aa09a3a4e1d3e9307b49040bbffe19c11936
iwahbe: > How does this connect to the user problem prompting this PR, would it be possible to elaborate? The tests are at a level of indirection, no elucidating new tests are added, and it's not obvious how the problem is being fixed.
> 
> ```
> I believe you will recover old behavior by putting p.SetAutonaming before the call to x.AutoAliasing.
> ```
> 
> After this PR do we believe that the order of p.SetAutonaming and x.AutoAliasing does not matter anymore?
> 
> Does editing the existing test not weaken the MaxItems one test strength?
> 
> I'm going to be able to review tomorrow morning but it's not an obvious fix to review in a tired state.

After this PR we do believe that `p.SetAutonaming` and `x.AutoAliasing` should be callable in any order.[^1] We want this property to avoid confusion.

The test was more specific than necessary. Specifically, it was testing for a property that was not necessary for the desired behavior that we want to change:

```patch
- assert.Nil(t, info.Resources["pkg_r1"].Fields["f2"].MaxItemsOne)
+ assert.Nil(t, info.Resources["pkg_r1"].Fields["f2"])
```

Previously, `info.Resources["pkg_r1"].Fields["f2"]` was equal to `&tfbridge.SchemaInfo{}`, and we only cared that `info.Resources["pkg_r1"].Fields["f2"].MaxItemsOne` was `nil`. We are tightening the requirements to ensure that the full thing is nil. 

What we actually cared about is this property (expressed in TS style pseudo-code):
```
info?.Resources?["pkg_r1"]?.Fields?["f2"]?.MaxItemsOne == nil
```

Because of how painful it is to express this in go, I abbreviated to asserting `MaxItemsOne` was `nil`.

[^1]: I believe that `SetAutonaming` will break at runtime when it is applied to a non-string field, so I'm discounting the possibility of a list type with auto-naming called `name`.
github-actions[bot]: Diff for [pulumi-azure](https://github.com/pulumi-bot/pulumi-azure/compare/d463a9615..01eb5bc56) with merge commit 1a05aa09a3a4e1d3e9307b49040bbffe19c11936
github-actions[bot]: Diff for [pulumi-gcp](https://github.com/pulumi-bot/pulumi-gcp/compare/950caadc4..d411a31b2) with merge commit 4c501add6f52ef9816184cf04a405929a8252fec
github-actions[bot]: Diff for [pulumi-gcp](https://github.com/pulumi-bot/pulumi-gcp/compare/22446d41c..3351fc995) with merge commit 5ea397d613d36c35049b8945835e80fd991e5f6a
github-actions[bot]: Diff for [pulumi-azure](https://github.com/pulumi-bot/pulumi-azure/compare/e632883ef..c08f36ea8) with merge commit 4c501add6f52ef9816184cf04a405929a8252fec
github-actions[bot]: Diff for [pulumi-azure](https://github.com/pulumi-bot/pulumi-azure/compare/a60319dd4..f3b4ad0f8) with merge commit 5ea397d613d36c35049b8945835e80fd991e5f6a
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/20a81ac..efa310a) with merge commit 78dbc7bba7febd8289590eebeba6e588f61a11b2
github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/77fc6b8..950ab0d) with merge commit 78dbc7bba7febd8289590eebeba6e588f61a11b2
github-actions[bot]: Diff for [pulumi-gcp](https://github.com/pulumi-bot/pulumi-gcp/compare/3744b5a6a..bd165d7ba) with merge commit 78dbc7bba7febd8289590eebeba6e588f61a11b2
github-actions[bot]: Diff for [pulumi-azure](https://github.com/pulumi-bot/pulumi-azure/compare/90e0c83b0..0ac187c66) with merge commit 78dbc7bba7febd8289590eebeba6e588f61a11b2
t0yv0: > Because of how painful it is to express this in go, I abbreviated to asserting MaxItemsOne was nil.

Yes that's true. Yes. I think we try not to care anywhere about the different flavors of "missing", not to attach semantics to nil vs missing key here, so what could work is getter functions that isolate the caller from nils, like this:

```
assert.Nil(t, info.Resource("pkg_r1").Field("f2").MaxItemsOne)
```

But perhaps for another day.
t0yv0: Well this is interesting, it works around the immediate problem but looks a tad error prone. I'm a bit worried about this quirk we found in SetAutonaming behavior, and perhaps there's still conditions where the order of application matters.

Consider this:

- autoaliasing runs and injects a MaxItems=1 specification, creating a SchemaInfo in ResourceInfo.Fields
- then autonaming runs and finds that whoops, the user set ResourceInfo.Fields["spock"] so I'm going to skip it

To the end user this looks like we accidentally disabled auto naming.
  
Could we do better? Could we enforce the order so SetAutonaming is always before aliasing?
github-actions[bot]: Diff for [pulumi-random](https://github.com/pulumi-bot/pulumi-random/compare/aaa8618..187ac50) with merge commit 07fcd2fcf692ba708f63b9d790c9e0f497ecf7b4
github-actions[bot]: Diff for [pulumi-azuread](https://github.com/pulumi-bot/pulumi-azuread/compare/ea70eac..2700d13) with merge commit 07fcd2fcf692ba708f63b9d790c9e0f497ecf7b4
github-actions[bot]: Diff for [pulumi-gcp](https://github.com/pulumi-bot/pulumi-gcp/compare/327e13b36..c32304699) with merge commit 07fcd2fcf692ba708f63b9d790c9e0f497ecf7b4
iwahbe: > Well this is interesting, it works around the immediate problem but looks a tad error prone. I'm a bit worried about this quirk we found in SetAutonaming behavior, and perhaps there's still conditions where the order of application matters.


Unfortunately, I think the quirk is here to stay. I've seen plenty of `tk: { Fields: map[string]*tfbridge.SchemaInfo{ "name": {Name: "name" } } }`. AFAIK, this exists only to disable auto-naming. 

AutoNaming should have been more selective by checking if `*SchemaInfo.Defaults` was `nil` instead of `*SchemaInfo`, but I think that change needs to wait for bridge v4.0.0.

> 
> Consider this:
> 
> * autoaliasing runs and injects a MaxItems=1 specification, creating a SchemaInfo in ResourceInfo.Fields
> * then autonaming runs and finds that whoops, the user set ResourceInfo.Fields["spock"] so I'm going to skip it
> 
> To the end user this looks like we accidentally disabled auto naming.

If `AutoAliasing` saw a resource and injected `ResourceInfo["spock"].MaxItemsOne = True()`, then `SetAutonames` would not be affected, since it only effects fields called `"name"`.

If `AutoAliasing` saw a resource and injected `ResourceInfo["name"].MaxItemsOne = True()`, it would imply that `ResourceMap.Get(resourceName).Schema().Get("name").Type` is [either a `List`, `Map` or `Set` ](https://github.com/pulumi/pulumi-terraform-bridge/blob/7747fa11af1899be0b17ec3889662972b2442567/pkg/tfbridge/x/tokens.go#L732)(something that `MaxItemsOne` makes sense on). If that is the case, then `SetAutonaming` would be invalid, since it assumes that the name field is of type `String`.

The end user will never observe `AutoAliasing` disabling a valid auto name.

> Could we do better? Could we enforce the order so SetAutonaming is always before aliasing?

We could enforce the ordering, but I don't think we should.
1. We impose accidental complexity on our users. Since we can't order enforce in the type system, this is a runtime panic when developing a provider. We should avoid this where possible.
2. This cuts down the size of `bridge-metadata.json` to only keep the data it needs. This makes diffs much more meaningful.
github-actions[bot]: Diff for [pulumi-azure](https://github.com/pulumi-bot/pulumi-azure/compare/7b084c104..6b80febc6) with merge commit 07fcd2fcf692ba708f63b9d790c9e0f497ecf7b4

Patch:
diff --git a/pf/internal/schemashim/block_schema.go b/pf/internal/schemashim/block_schema.go
index 94813f41..9208589f 100644
--- a/pf/internal/schemashim/block_schema.go
+++ b/pf/internal/schemashim/block_schema.go
@@ -30,7 +30,6 @@ type blockSchema struct {
 }
 
 func newBlockSchema(key string, block pfutils.Block) *blockSchema {
-	fmt.Println("newBlockSchema", key, block)
 	return &blockSchema{key, block}
 }
 
diff --git a/pf/tfbridge/main.go b/pf/tfbridge/main.go
index 0e33b102..9cb0f1d3 100644
--- a/pf/tfbridge/main.go
+++ b/pf/tfbridge/main.go
@@ -162,7 +162,7 @@ func MakeMuxedServer(
 			case *schemashim.SchemaOnlyProvider:
 				m.Servers = append(m.Servers, muxer.Endpoint{
 					Server: func(host *rprovider.HostClient) (pulumirpc.ResourceProviderServer, error) {
-						return newProviderServer(ctx, host, ProviderInfo{
+						return NewProviderServer(ctx, host, ProviderInfo{
 							ProviderInfo: info,
 							NewProvider:  prov.PfProvider,
 						}, ProviderMetadata{PackageSchema: schema})
diff --git a/pf/tfbridge/provider.go b/pf/tfbridge/provider.go
index 4b78e71f..bea65d1b 100644
--- a/pf/tfbridge/provider.go
+++ b/pf/tfbridge/provider.go
@@ -148,7 +148,8 @@ func newProviderWithContext(ctx context.Context, info ProviderInfo,
 	}, nil
 }
 
-func newProviderServer(
+// Internal. The signature of this function can change between major releases. Exposed to facilitate testing.
+func NewProviderServer(
 	ctx context.Context,
 	logSink logutils.LogSink,
 	info ProviderInfo,
diff --git a/pf/tfbridge/serve.go b/pf/tfbridge/serve.go
index fd29317d..a3f8987a 100644
--- a/pf/tfbridge/serve.go
+++ b/pf/tfbridge/serve.go
@@ -23,6 +23,6 @@ import (
 
 func serve(ctx context.Context, pkg string, prov ProviderInfo, meta ProviderMetadata) error {
 	return rprovider.Main(pkg, func(host *rprovider.HostClient) (pulumirpc.ResourceProviderServer, error) {
-		return newProviderServer(ctx, host, prov, meta)
+		return NewProviderServer(ctx, host, prov, meta)
 	})
 }
diff --git a/pkg/tfbridge/x/tokens.go b/pkg/tfbridge/x/tokens.go
index 5a9445f4..a8b17ac0 100644
--- a/pkg/tfbridge/x/tokens.go
+++ b/pkg/tfbridge/x/tokens.go
@@ -650,61 +650,147 @@ func aliasResource(
 		return
 	}
 
-	safeRange := func(r shim.Resource, f func(string, shim.Schema)) {
+	// When walking the schema tree for a resource, we create mirroring trees in
+	// *fieldHistory and *b.SchemaInfo. To avoid polluting either tree (and
+	// interfering with other actions such as SetAutonaming), we clean up the paths
+	// that we created but did not store any information into.
+	//
+	// For example, consider the schema for a field of type `Object{ Key1:
+	// List[String] }`.  The schema tree for this field looks like this:
+	//
+	//     Object:
+	//       Fields:
+	//         Key1:
+	//           List:
+	//             Elem:
+	//               String
+	//
+	// When we walk the tree, we create an almost identical history tree:
+	//
+	//     Object:
+	//       Fields:
+	//         Key1:
+	//           List:
+	//             MaxItemsOne: false
+	//             Elem:
+	//               String
+	//
+	// We stored the additional piece of information `MaxItemsOne: false`. We need to
+	// keep enough of the tree to maintain that information, but no more. We can
+	// discard the unnecessary `Elem: String`.
+	//
+	// This keeps the tree as clean as possible for other processes which expect a
+	// `nil` element when making changes. Since other processes (like SetAutonaming)
+	// act on edge nodes (like our String), this allows us to inter-operate with them
+	// without interference.
+
+	// walk traverses a generic shim.Schema recursively, applying fieldHistory to
+	// SchemaInfo and vise versa as necessary to avoid breaking changes in the
+	// resulting sdk.
+	//
+	// We declare walk ahead of its implementation so that (1) walkResource and walk
+	// can be mutually recursive and (2) walk can be self-recursive.
+	var walk func(shim.Schema, *fieldHistory, *b.SchemaInfo) (bool, bool)
+
+	// walkResource traverses a shim.Resource, applying walk to each field in the resource.
+	walkResource := func(r shim.Resource, hist *map[string]*fieldHistory, info *map[string]*b.SchemaInfo) (bool, bool) {
 		if r == nil {
-			return
+			return hist != nil, info != nil
 		}
 		m := r.Schema()
 		if m == nil {
-			return
+			return hist != nil, info != nil
 		}
+
+		var rHasH, rHasI bool
+
 		m.Range(func(k string, v shim.Schema) bool {
-			f(k, v)
+			h, hasH := getNonNil(hist, k)
+			i, hasI := getNonNil(info, k)
+			fieldHasHist, fieldHasInfo := walk(v, h, i)
+
+			hasH = hasH || fieldHasHist
+			hasI = hasI || fieldHasInfo
+
+			if !hasH {
+				delete(*hist, k)
+			}
+			if !hasI {
+				delete(*info, k)
+			}
+
+			rHasH = rHasH || hasH
+			rHasI = rHasI || hasI
+
 			return true
 		})
+
+		return rHasH, rHasI
 	}
 
-	var walk func(*fieldHistory, *b.SchemaInfo, shim.Schema)
-	walk = func(h *fieldHistory, info *b.SchemaInfo, schema shim.Schema) {
+	walk = func(schema shim.Schema, h *fieldHistory, info *b.SchemaInfo) (hasH bool, hasI bool) {
 		if schema == nil || (schema.Type() != shim.TypeList && schema.Type() != shim.TypeSet) {
 			// MaxItemsOne does not apply, so do nothing
 		} else if info.MaxItemsOne != nil {
 			// The user has overwritten the value, so we will just record that.
 			h.MaxItemsOne = info.MaxItemsOne
+			hasH = true
 		} else if h.MaxItemsOne != nil {
 			// If we have a previous value in the history, we keep it as is.
 			info.MaxItemsOne = h.MaxItemsOne
+			hasI = true
 		} else {
 			// There is no history for this value, so we bake it into the
 			// alias history.
 			h.MaxItemsOne = b.BoolRef(b.IsMaxItemsOne(schema, info))
+			hasH = true
 		}
-		e := schema.Elem()
-		switch e := e.(type) {
-		case shim.Resource:
+
+		// Ensure that the h.Elem and info.Elem fields are non-nil so they can be
+		// safely recursed on.
+		//
+		// If the .Elem existed before this function, we mark it as unsafe to cleanup.
+		var hasElemH, hasElemI bool
+		populateElem := func() {
 			if h.Elem == nil {
 				h.Elem = &fieldHistory{}
+			} else {
+				hasElemH = true
 			}
 			if info.Elem == nil {
 				info.Elem = &b.SchemaInfo{}
+			} else {
+				hasElemI = true
 			}
-
-			safeRange(e, func(k string, v shim.Schema) {
-				walk(
-					getNonNil(&h.Elem.Fields, k),
-					getNonNil(&info.Elem.Fields, k),
-					v,
-				)
-			})
-		case shim.Schema:
-			if h.Elem == nil {
-				h.Elem = &fieldHistory{}
+		}
+		// Cleanup after we have walked a .Elem value.
+		//
+		// If the .Elem field was created in populateElem and the field was not
+		// changed, we then delete the field.
+		cleanupElem := func(elemHist, elemInfo bool) {
+			hasElemH = hasElemH || elemHist
+			hasElemI = hasElemI || elemInfo
+			if !hasElemH {
+				h.Elem = nil
 			}
-			if info.Elem == nil {
-				info.Elem = &b.SchemaInfo{}
+			if !hasElemI {
+				info.Elem = nil
 			}
-			walk(h.Elem, info.Elem, e)
 		}
+
+		e := schema.Elem()
+		switch e := e.(type) {
+		case shim.Resource:
+			populateElem()
+			eHasH, eHasI := walkResource(e, &h.Elem.Fields, &info.Elem.Fields)
+			cleanupElem(eHasH, eHasI)
+		case shim.Schema:
+			populateElem()
+			eHasH, eHasI := walk(e, h.Elem, info.Elem)
+			cleanupElem(eHasH, eHasI)
+		}
+
+		return hasH || hasElemH, hasI || hasElemI
 	}
 
 	// If we are behind the major version, reset the fields and the major version.
@@ -713,27 +799,24 @@ func aliasResource(
 		hist[tfToken].Fields = nil
 	}
 
-	safeRange(res, func(k string, v shim.Schema) {
-		walk(
-			getNonNil(&hist[tfToken].Fields, k),
-			getNonNil(&computed.Fields, k),
-			v,
-		)
-	})
+	walkResource(res, &hist[tfToken].Fields, &computed.Fields)
 }
 
-func getNonNil[K comparable, V any](m *map[K]*V, key K) *V {
+func getNonNil[K comparable, V any](m *map[K]*V, key K) (_ *V, alreadyThere bool) {
 	contract.Assertf(m != nil, "Cannot restore map if ptr is nil")
 	if *m == nil {
 		*m = map[K]*V{}
 	}
 	v := (*m)[key]
+
 	if v == nil {
 		var new V
 		v = &new
 		(*m)[key] = v
+	} else {
+		alreadyThere = true
 	}
-	return v
+	return v, alreadyThere
 }
 
 func aliasOrRenameResource(


Test patch:
diff --git a/pf/tests/provider_configure_test.go b/pf/tests/provider_configure_test.go
index 7e27b1ca..b6e98eb1 100644
--- a/pf/tests/provider_configure_test.go
+++ b/pf/tests/provider_configure_test.go
@@ -33,7 +33,6 @@ func TestConfigure(t *testing.T) {
       }
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     }
diff --git a/pf/tests/provider_read_test.go b/pf/tests/provider_read_test.go
index 2f1c674b..2cacb580 100644
--- a/pf/tests/provider_read_test.go
+++ b/pf/tests/provider_read_test.go
@@ -44,7 +44,6 @@ func TestReadFromRefresh(t *testing.T) {
 	    "acceptResources": true
 	  },
 	  "response": {
-	    "acceptSecrets": true,
 	    "supportsPreview": true,
 	    "acceptResources": true
 	  },
diff --git a/pf/tests/testdata/genrandom/random-delete-update.json b/pf/tests/testdata/genrandom/random-delete-update.json
index 2284dc36..b16282ba 100644
--- a/pf/tests/testdata/genrandom/random-delete-update.json
+++ b/pf/tests/testdata/genrandom/random-delete-update.json
@@ -53,7 +53,6 @@
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
diff --git a/pf/tests/testdata/genrandom/random-empty-preview.json b/pf/tests/testdata/genrandom/random-empty-preview.json
index ce8bceb4..402a36d9 100644
--- a/pf/tests/testdata/genrandom/random-empty-preview.json
+++ b/pf/tests/testdata/genrandom/random-empty-preview.json
@@ -53,7 +53,6 @@
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
diff --git a/pf/tests/testdata/genrandom/random-empty-update.json b/pf/tests/testdata/genrandom/random-empty-update.json
index e12f1a84..7ea9f8ad 100644
--- a/pf/tests/testdata/genrandom/random-empty-update.json
+++ b/pf/tests/testdata/genrandom/random-empty-update.json
@@ -53,7 +53,6 @@
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
diff --git a/pf/tests/testdata/genrandom/random-initial-preview.json b/pf/tests/testdata/genrandom/random-initial-preview.json
index 23ad2a4b..b43645a1 100644
--- a/pf/tests/testdata/genrandom/random-initial-preview.json
+++ b/pf/tests/testdata/genrandom/random-initial-preview.json
@@ -115,7 +115,6 @@
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
diff --git a/pf/tests/testdata/genrandom/random-initial-update.json b/pf/tests/testdata/genrandom/random-initial-update.json
index d3754320..3336e212 100644
--- a/pf/tests/testdata/genrandom/random-initial-update.json
+++ b/pf/tests/testdata/genrandom/random-initial-update.json
@@ -115,7 +115,6 @@
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
diff --git a/pf/tests/testdata/genrandom/random-replace-preview.json b/pf/tests/testdata/genrandom/random-replace-preview.json
index 48f1a575..60a1f694 100644
--- a/pf/tests/testdata/genrandom/random-replace-preview.json
+++ b/pf/tests/testdata/genrandom/random-replace-preview.json
@@ -53,7 +53,6 @@
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
diff --git a/pf/tests/testdata/genrandom/random-replace-update.json b/pf/tests/testdata/genrandom/random-replace-update.json
index 68749647..5a84df59 100644
--- a/pf/tests/testdata/genrandom/random-replace-update.json
+++ b/pf/tests/testdata/genrandom/random-replace-update.json
@@ -53,7 +53,6 @@
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
diff --git a/pf/tests/testdata/updateprogram.json b/pf/tests/testdata/updateprogram.json
index 09267947..0d7e2797 100644
--- a/pf/tests/testdata/updateprogram.json
+++ b/pf/tests/testdata/updateprogram.json
@@ -120,7 +120,6 @@
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
@@ -370,7 +369,6 @@
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
@@ -553,7 +551,6 @@
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
@@ -834,11 +831,9 @@
     "method": "/pulumirpc.ResourceProvider/Configure",
     "request": {
       "args": {},
-      "acceptSecrets": true,
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
@@ -1091,7 +1086,6 @@
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
@@ -1193,7 +1187,6 @@
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
@@ -1518,7 +1511,6 @@
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
@@ -1810,7 +1802,6 @@
       "acceptResources": true
     },
     "response": {
-      "acceptSecrets": true,
       "supportsPreview": true,
       "acceptResources": true
     },
diff --git a/pf/tests/util.go b/pf/tests/util.go
index 4259ec65..6973cce9 100644
--- a/pf/tests/util.go
+++ b/pf/tests/util.go
@@ -20,7 +20,6 @@ import (
 
 	"github.com/stretchr/testify/require"
 
-	"github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin"
 	pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go"
 
 	"github.com/pulumi/pulumi-terraform-bridge/pf/tfbridge"
@@ -30,9 +29,9 @@ import (
 func newProviderServer(t *testing.T, info tfbridge.ProviderInfo) pulumirpc.ResourceProviderServer {
 	ctx := context.Background()
 	meta := genMetadata(t, info)
-	p, err := tfbridge.NewProvider(ctx, info, meta)
+	srv, err := tfbridge.NewProviderServer(ctx, nil, info, meta)
 	require.NoError(t, err)
-	return plugin.NewProviderServer(p)
+	return srv
 }
 
 func newMuxedProviderServer(t *testing.T, info tfbridge0.ProviderInfo) pulumirpc.ResourceProviderServer {
diff --git a/pkg/tfbridge/x/token_test.go b/pkg/tfbridge/x/token_test.go
index 14316191..2bf5124f 100644
--- a/pkg/tfbridge/x/token_test.go
+++ b/pkg/tfbridge/x/token_test.go
@@ -514,6 +514,7 @@ func TestMaxItemsOneAliasing(t *testing.T) {
 					"pkg_r1": (&schema.Resource{Schema: schema.SchemaMap{
 						"f1": Schema{MaxItemsOne: f1},
 						"f2": Schema{MaxItemsOne: f2},
+						"f3": Schema{typ: shim.TypeString},
 					}}).Shim(),
 				},
 			}).Shim(),
@@ -652,8 +653,8 @@ func TestMaxItemsOneAliasingExpiring(t *testing.T) {
 	err = AutoAliasing(info, metadata)
 	require.NoError(t, err)
 
-	assert.Nil(t, info.Resources["pkg_r1"].Fields["f1"].MaxItemsOne)
-	assert.Nil(t, info.Resources["pkg_r1"].Fields["f2"].MaxItemsOne)
+	assert.Nil(t, info.Resources["pkg_r1"].Fields["f1"])
+	assert.Nil(t, info.Resources["pkg_r1"].Fields["f2"])
 	assert.Equal(t, `{
     "auto-aliasing": {
         "resources": {
@@ -746,9 +747,98 @@ func TestMaxItemsOneAliasingNested(t *testing.T) {
 	assert.False(t, *info.Resources["pkg_r1"].Fields["f2"].Elem.Fields["n2"].MaxItemsOne)
 }
 
+// (*ProviderInfo).SetAutonaming skips fields that have a SchemaInfo already defined in
+// their resource's ResourceInfo.Fields. We need to make sure that unless we mark a field
+// as `MaxItemsOne: nonNil` for some non-nil value, we don't leave that field entry behind
+// since that will disable SetAutonaming.
+func TestMaxItemsOneAliasingWithAutoNaming(t *testing.T) {
+	provider := func() *tfbridge.ProviderInfo {
+		prov := &tfbridge.ProviderInfo{
+			P: (&schema.Provider{
+				ResourcesMap: schema.ResourceMap{
+					"pkg_r1": (&schema.Resource{Schema: schema.SchemaMap{
+						"name":      Schema{typ: shim.TypeString},
+						"nest_list": Schema{elem: Schema{typ: shim.TypeBool}},
+						"nest_flat": Schema{
+							elem:        Schema{typ: shim.TypeBool},
+							MaxItemsOne: true,
+						},
+						"override_list": Schema{elem: Schema{typ: shim.TypeBool}},
+						"override_flat": Schema{
+							elem:        Schema{typ: shim.TypeInt},
+							MaxItemsOne: true,
+						},
+					}}).Shim(),
+				},
+			}).Shim(),
+		}
+		err := ComputeDefaults(prov, TokensSingleModule("pkg_", "index", MakeStandardToken("pkg")))
+		require.NoError(t, err)
+		return prov
+	}
+
+	assertExpected := func(t *testing.T, p *tfbridge.ProviderInfo, hist *metadata.Data) {
+		r := p.Resources["pkg_r1"]
+		assert.True(t, r.Fields["name"].Default.AutoNamed)
+
+		assert.Nil(t, r.Fields["nest_list"])
+		assert.Nil(t, r.Fields["override_list"])
+
+		t.Log(string(hist.Marshal()))
+		assert.JSONEq(t, `{
+                "auto-aliasing": {
+                    "resources": {
+                        "pkg_r1": {
+                            "current": "pkg:index/r1:R1",
+                            "fields": {
+                                "nest_flat": {
+                                    "maxItemsOne": true
+                                },
+                                "nest_list": {
+                                    "maxItemsOne": false
+                                },
+                                "override_flat": {
+                                    "maxItemsOne": true
+                                },
+                                "override_list": {
+                                    "maxItemsOne": false
+                                }
+                            }
+                        }
+                    },
+                    "datasources": {}
+                }
+            }`, string(hist.Marshal()))
+	}
+
+	t.Run("auto-named-then-aliased", func(t *testing.T) {
+		p := provider()
+
+		info, err := metadata.New(nil)
+		require.NoError(t, err)
+		p.SetAutonaming(24, "-")
+		err = AutoAliasing(p, info)
+		require.NoError(t, err)
+
+		assertExpected(t, p, info)
+	})
+
+	t.Run("auto-aliased-then-named", func(t *testing.T) {
+		p := provider()
+		info, err := metadata.New(nil)
+		require.NoError(t, err)
+		err = AutoAliasing(p, info)
+		require.NoError(t, err)
+		p.SetAutonaming(24, "-")
+
+		assertExpected(t, p, info)
+	})
+}
+
 type Schema struct {
 	shim.Schema
 	MaxItemsOne bool
+	typ         shim.ValueType
 	elem        any
 }
 
@@ -760,7 +850,13 @@ func (s Schema) MaxItems() int {
 }
 
 func (s Schema) Type() shim.ValueType {
-	return shim.TypeList
+	if s.typ == shim.TypeInvalid {
+		return shim.TypeList
+	}
+	return s.typ
 }
 
+func (s Schema) Optional() bool { return true }
+func (s Schema) Required() bool { return false }
+
 func (s Schema) Elem() any { return s.elem }