lang/eval: Apply attr-as-nested-block fixup in EvalBlock #20837
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…butes This preprocessing step allows users to use nested block syntax to specify elements of an attribute that is defined as being a list or set of an object type. This restores part of the unintended flexibility permitted in Terraform v0.11 so that we can work around a few tricky edges where provider implementations were relying on Terraform's failure to validate this in earlier versions. For any body that is pre-processed using this new helper, we will recognize when a configuration author uses nested block syntax with a name that is specified in the schema as an attribute of a suitable type and tweak the schema just in time before decoding to expect that usage and then fix up the result on the way out to conform to the original schema. Achieving this requires an abstraction inversion because only Terraform's high-level schema has enough information to decide how to rewrite the incoming low-level schema. We must therefore here implement HCL's lowest-level API interface in terms of the higher-level abstractions of hcldec and Terraform's configschema. Because of the abstraction inversion this fixup mechanism cannot be used generally for arbitrary HCL bodies but we can use it carefully inside the lang package where its own API can guarantee the necessary invariants for this to work.
For any block content we evaluate dynamically via this API, we'll make a special allowance for users to optionally write members of a list attribute instead as a sequence of nested blocks, thus allowing some existing provider features that were assuming this capability to continue to support it after v0.12. This should not be used for any new provider features, and should ideally be eventually phased out so that there aren't two similar-but-slightly-different syntaxes for saying the same thing.
apparentlymart
force-pushed
the
f-hcl2-block-to-attr
branch
from
e7721fd
to
829abbb
Compare
jbardin
approved these changes
Mar 27, 2019
This gives us an extra hook in the dynblock variables analysis that should allow us to also make it subject also to the lang/blocktoattr fixup, to ensure we'll find all the references in spite of these various pre-processing wrappers.
Because we handle FixUpBlockAttrs after dynamic block expansion, when resolving variables we unfortunately need to consider the possibility of both dynamic block expansion _and_ the block attrs fixup. To accommodate this we have a variant of dynblock.VariablesHCLDec that instead walks using the configschema.Block representation of the schema and applies the same opportunistic schema rewrite used by FixUpBlockAttrs at each body encountered during the walk.
For compatibility with documented patterns from existing providers we are now allowing (via a pre-processing step) any attribute whose type is a list-of-object or set-of-object type to optionally be assigned using one or more blocks whose type is the attribute name. The pre-processing functionality was implemented in previous commits but we were not correctly detecting references within these blocks that are, from the perspective of the primary schema, invalid. Now we'll use an alternative implementation of variable detection that is able to apply the same schema rewriting technique we used to implement the transform and thus can find all of the references as if they were already in their final locations.
jbardin
approved these changes
Mar 28, 2019
|
Travis-CI is down right now, so the automatic CI check here won't be completing for a long time. Instead, I ran a similar set of test steps locally to approximate that so we can make progress here. |
This was referenced Apr 25, 2019
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For any block content we evaluate dynamically via this API, we'll make a special allowance for users to optionally write members of a list attribute instead as a sequence of nested blocks, thus allowing some
existing provider features that were assuming this capability to continue to support it after v0.12.
This should not be used for any new provider features, and subsequent work we will define a new pattern that allows the same or better expressiveness without relying on distinguishing between two configurations that appear logically equivalent.
This provides a path forward for the few cases in existing providers where a nested block type was declared as
Optional+Computedin the schema and the provider logic then distinguished between unset (no blocks at all) vs. explicitly empty by relying on the Terraform v0.11 bug that allowed attribute syntax to be used to define blocks in some cases:For this specific confluence of assumptions only, we can set the schema
ConfigModefornested_objectto beSchemaConfigModeAttr, ensure thatSkipCoreTypeCheckremains false (because this fixup depends on having accurate type information in Terraform Core), and then continue to show in the resource type documentation the use of nested block syntax even though the schema actually describes the name as being an attribute.So far this does not include a corresponding fixup for the
terraform 0.12upgradetool, so it will attempt to migrate use of these attributes to attribute syntax. Before v0.12.0 final release we'll decide on a suitable heuristic for the upgrade tool to recognize when this workaround seems to be in use in the provider and make it prefer to use nested block syntax in that scenario, in spite of what the resource type schema seems to require.