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()