From 0f2503e4ea5decb444440b94caa2a09e4b87b300 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 3 Mar 2026 17:42:42 -0500 Subject: [PATCH] detect non-legacy schemas We already had a heuristic for flagging non-legacy schemas in the blocktoattr fixup code, but now we need to extend that to computed blocks. Adapt these checks to the block schema type itself since we want to use the check outside of the old blocktoattr package. --- internal/configs/configschema/schema.go | 51 ++++++++++++++++++- internal/lang/blocktoattr/fixup.go | 43 +--------------- internal/plans/objchange/normalize_obj.go | 11 ++-- .../node_resource_abstract_instance.go | 15 +++--- 4 files changed, 67 insertions(+), 53 deletions(-) diff --git a/internal/configs/configschema/schema.go b/internal/configs/configschema/schema.go index 529f65d242..db86052588 100644 --- a/internal/configs/configschema/schema.go +++ b/internal/configs/configschema/schema.go @@ -47,6 +47,55 @@ type Block struct { Computed bool } +// AssertNoLegacyBehavior indicates that we are certain the schema is not relying on +// any legacy SDK behavior. We do this by checking for options which did not +// exist at the time of the legacy SDK, like structural attributes, certain +// complex types, or computed blocks. +func (b Block) AssertNoLegacyBehavior() bool { + if b.Computed { + return true + } + + // check for non-legacy attribute types + for _, attr := range b.Attributes { + // A nested type cannot come from the SDK + if attr.NestedType != nil { + return true + } + ty := attr.Type + + // Lists and sets of objects could be generated by + // SchemaConfigModeAttr, but some other combinations can be ruled out. + + // Tuples and objects could not be generated at all. + if ty.IsTupleType() || ty.IsObjectType() { + return true + } + + // A map of objects was not possible. + if ty.IsMapType() && ty.ElementType().IsObjectType() { + return true + } + + // Nested collections were not really supported, but could be generated + // with string types (though we conservatively limit this to primitive types) + if ty.IsCollectionType() { + ety := ty.ElementType() + if ety.IsCollectionType() && !ety.ElementType().IsPrimitiveType() { + return true + } + } + } + + for _, nb := range b.BlockTypes { + if nb.Block.AssertNoLegacyBehavior() { + return true + } + } + + return false +} + // Attribute represents a configuration attribute, within a block. type Attribute struct { // Type is a type specification that the attribute's value must conform to. @@ -92,7 +141,7 @@ type Attribute struct { // This is valid only when Deprecated is true. DeprecationMessage string - // WriteOnly, if set to true, indicates that the attribute is not presisted + // WriteOnly, if set to true, indicates that the attribute is not persisted // in the state. WriteOnly bool } diff --git a/internal/lang/blocktoattr/fixup.go b/internal/lang/blocktoattr/fixup.go index 85c9a59895..61fc4bedfe 100644 --- a/internal/lang/blocktoattr/fixup.go +++ b/internal/lang/blocktoattr/fixup.go @@ -37,7 +37,7 @@ func FixUpBlockAttrs(body hcl.Body, schema *configschema.Block) hcl.Body { schema = &configschema.Block{} } - if skipFixup(schema) { + if schema.AssertNoLegacyBehavior() { // we don't have any context for the resource name or type, but // hopefully this could help locate the evaluation in the logs if there // were a problem @@ -52,47 +52,6 @@ func FixUpBlockAttrs(body hcl.Body, schema *configschema.Block) hcl.Body { } } -// skipFixup detects any use of Attribute.NestedType, or Types which could not -// be generate by the legacy SDK when taking SchemaConfigModeAttr into account. -func skipFixup(schema *configschema.Block) bool { - for _, attr := range schema.Attributes { - if attr.NestedType != nil { - return true - } - ty := attr.Type - - // Lists and sets of objects could be generated by - // SchemaConfigModeAttr, but some other combinations can be ruled out. - - // Tuples and objects could not be generated at all. - if ty.IsTupleType() || ty.IsObjectType() { - return true - } - - // A map of objects was not possible. - if ty.IsMapType() && ty.ElementType().IsObjectType() { - return true - } - - // Nested collections were not really supported, but could be generated - // with string types (though we conservatively limit this to primitive types) - if ty.IsCollectionType() { - ety := ty.ElementType() - if ety.IsCollectionType() && !ety.ElementType().IsPrimitiveType() { - return true - } - } - } - - for _, block := range schema.BlockTypes { - if skipFixup(&block.Block) { - return true - } - } - - return false -} - type fixupBody struct { original hcl.Body schema *configschema.Block diff --git a/internal/plans/objchange/normalize_obj.go b/internal/plans/objchange/normalize_obj.go index 4bb952a1dd..bbfd77e6c4 100644 --- a/internal/plans/objchange/normalize_obj.go +++ b/internal/plans/objchange/normalize_obj.go @@ -8,19 +8,22 @@ import ( "github.com/zclconf/go-cty/cty" ) -// NormalizeObjectFromLegacySDK takes an object that may have been generated -// by the legacy Terraform SDK (i.e. returned from a provider with the +// NormalizeObjectFromLegacySDK takes an object that may have been generated by +// the legacy Terraform SDK (i.e. returned from a provider with the // LegacyTypeSystem opt-out set) and does its best to normalize it for the // assumptions we would normally enforce if the provider had not opted out. // // In particular, this function guarantees that a value representing a nested -// block will never itself be unknown or null, instead representing that as -// a non-null value that may contain null/unknown values. +// block will never itself be unknown or null, instead representing that as a +// non-null value that may contain null/unknown values. // // The input value must still conform to the implied type of the given schema, // or else this function may produce garbage results or panic. This is usually // okay because type consistency is enforced when deserializing the value // returned from the provider over the RPC wire protocol anyway. +// +// This is clearly incompatible with computed blocks in protocol v6, so must not +// be called for anything using features from v6 or greater. func NormalizeObjectFromLegacySDK(val cty.Value, schema *configschema.Block) cty.Value { val, valMarks := val.UnmarkDeepWithPaths() val = normalizeObjectFromLegacySDK(val, schema) diff --git a/internal/terraform/node_resource_abstract_instance.go b/internal/terraform/node_resource_abstract_instance.go index a2bccb0bbe..49cfb3aaaf 100644 --- a/internal/terraform/node_resource_abstract_instance.go +++ b/internal/terraform/node_resource_abstract_instance.go @@ -755,12 +755,15 @@ func (n *NodeAbstractResourceInstance) refresh(ctx EvalContext, deposedKey state return state, deferred, diags } - newState := objchange.NormalizeObjectFromLegacySDK(resp.NewState, schema.Body) - if !newState.RawEquals(resp.NewState) { - // We had to fix up this object in some way, and we still need to - // accept any changes for compatibility, so all we can do is log a - // warning about the change. - log.Printf("[WARN] Provider %q produced an invalid new value containing null blocks for %q during refresh\n", n.ResolvedProvider.Provider, n.Addr) + newState := resp.NewState + if !schema.Body.AssertNoLegacyBehavior() { + newState = objchange.NormalizeObjectFromLegacySDK(resp.NewState, schema.Body) + if !newState.RawEquals(resp.NewState) { + // We had to fix up this object in some way, and we still need to + // accept any changes for compatibility, so all we can do is log a + // warning about the change. + log.Printf("[WARN] Provider %q produced an invalid new value containing null blocks for %q during refresh\n", n.ResolvedProvider.Provider, n.Addr) + } } ret := state.DeepCopy()