Commit Graph

15 Commits

Author SHA1 Message Date
Martin Atkins 37b1413ab3 core: Handle root and child module input variables consistently
Previously we had a significant discrepancy between these two situations:
we wrote the raw root module variables directly into the EvalContext and
then applied type conversions only at expression evaluation time, while
for child modules we converted and validated the values while visiting
the variable graph node and wrote only the _final_ value into the
EvalContext.

This confusion seems to have been the root cause for #29899, where
validation rules for root module variables were being applied at the wrong
point in the process, prior to type conversion.

To fix that bug and also make similar mistakes less likely in the future,
I've made the root module variable handling more like the child module
variable handling in the following ways:
 - The "raw value" (exactly as given by the user) lives only in the graph
   node representing the variable, which mirrors how the _expression_
   for a child module variable lives in its graph node. This means that
   the flow for the two is the same except that there's no expression
   evaluation step for root module variables, because they arrive as
   constant values from the caller.
 - The set of variable values in the EvalContext is always only "final"
   values, after type conversion is complete. That in turn means we no
   longer need to do "just in time" conversion in
   evaluationStateData.GetInputVariable, and can just return the value
   exactly as stored, which is consistent with how we handle all other
   references between objects.

This diff is noisier than I'd like because of how much it takes to wire
a new argument (the raw variable values) through to the plan graph builder,
but those changes are pretty mechanical and the interesting logic lives
inside the plan graph builder itself, in NodeRootVariable, and
the shared helper functions in eval_variable.go.

While here I also took the opportunity to fix a historical API wart in
EvalContext, where SetModuleCallArguments was built to take a set of
variable values all at once but our current caller always calls with only
one at a time. That is now just SetModuleCallArgument singular, to match
with the new SetRootModuleArgument to deal with root module variables.
2022-01-10 12:26:54 -08:00
James Bardin 71b9682e8c ensure there is always a valid return value 2021-12-14 18:02:57 -05:00
James Bardin d469e86331 revert 6b8b0617
Revert the evaluation change from #29862.
While returning a dynamic value for all expanded resources during
validation is not optimal, trying to work around this using unknown maps
and lists is causing other undesirable behaviors during evaluation.
2021-12-14 17:58:10 -05:00
James Bardin 647d36062c we don't need an abs provider address
We only lookup providers by provider type to get the schema, so there's
no reason to generate anything more specific.
2021-11-02 15:38:53 -04:00
James Bardin 6b8b0617bb generate precise resource types during validate
Allow `GetResource` to return correct types values during validation,
rather than relying on `cty.DynamicVal` as a placeholder. This allows
other dependent expressions to be more correctly evaluated.
2021-11-02 15:38:36 -04:00
James Bardin b71b393cf6 move nullable check to variable input evaluation 2021-11-01 09:02:32 -04:00
James Bardin f0a64eb456 configs: explicitly nullable variable values
The current behavior of module input variables is to allow users to
override a default by assigning `null`, which works contrary to the
behavior of resource attributes, and prevents explicitly accepting a
default when the input must be defined in the configuration.

Add a new variable attribute called `nullable` will allow explicitly
defining when a variable can be set to null or not. The current default
behavior is that of `nullable=true`.

Setting `nullable=false` in a variable block indicates that the variable
value can never be null. This either requires a non-null input value, or
a non-null default value. In the case of the latter, we also opt-in to
the new behavior of a `null` input value taking the default rather than
overriding it.

In a future language edition where we make `nullable=false` the default,
setting `nullable=true` will allow the legacy behavior of `null`
overriding a default value. The only future configuration in which this
would be required even if the legacy behavior were not desired is when
setting an optional+nullable value. In that case `default=null` would
also be needed and we could therefor imply `nullable=true` without
requiring it in the configuration.
2021-10-29 13:59:46 -04:00
Chris Arcand 172200808b Correct terraform.env deprecation message typo 2021-09-13 14:21:26 -05:00
James Bardin 8ed9a270e5
Merge pull request #29559 from hashicorp/jbardin/optional-attrs
Prevent `ObjectWithOptionalAttrs` from escaping
2021-09-13 08:58:11 -04:00
James Bardin 7f26531d4f variable types should always be populated 2021-09-13 08:51:32 -04:00
James Bardin 53a73a8ab6 configs: add ConstraintType to config.Variable
In order to handle optional attributes, the Variable type needs to keep
track of the type constraint for decoding and conversion, as well as the
concrete type for creating values and type comparison.

Since the Type field is referenced throughout the codebase, and for
future refactoring if the handling of optional attributes changes
significantly, the constraint is now loaded into an entirely new field
called ConstraintType. This prevents types containing
ObjectWithOptionalAttrs from escaping the decode/conversion codepaths
into the rest of the codebase.
2021-09-13 08:51:32 -04:00
Martin Atkins 343279110a core: Graph walk loads plugin schemas opportunistically
Previously our graph walker expected to recieve a data structure
containing schemas for all of the provider and provisioner plugins used in
the configuration and state. That made sense back when
terraform.NewContext was responsible for loading all of the schemas before
taking any other action, but it no longer has that responsiblity.

Instead, we'll now make sure that the "contextPlugins" object reaches all
of the locations where we need schema -- many of which already had access
to that object anyway -- and then load the needed schemas just in time.

The contextPlugins object memoizes schema lookups, so we can safely call
it many times with the same provider address or provisioner type name and
know that it'll still only load each distinct plugin once per Context
object.

As of this commit, the Context.Schemas method is now a public interface
only and not used by logic in the "terraform" package at all. However,
that does leave us in a rather tenuous situation of relying on the fact
that all practical users of terraform.Context end up calling "Schemas" at
some point in order to verify that we have all of the expected versions
of plugins. That's a non-obvious implicit dependency, and so in subsequent
commits we'll gradually move all responsibility for verifying plugin
versions into the caller of terraform.NewContext, which'll heal a
long-standing architectural wart whereby the caller is responsible for
installing and locating the plugin executables but not for verifying that
what's installed is conforming to the current configuration and dependency
lock file.
2021-09-10 14:56:49 -07:00
James Bardin d9dfd451ea update to use typed sensitive marks 2021-06-25 12:49:07 -04:00
James Bardin 2c493e38c7 marks package
marks.Sensitive
2021-06-25 12:35:51 -04:00
Martin Atkins 36d0a50427 Move terraform/ to internal/terraform/
This is part of a general effort to move all of Terraform's non-library
package surface under internal in order to reinforce that these are for
internal use within Terraform only.

If you were previously importing packages under this prefix into an
external codebase, you could pin to an earlier release tag as an interim
solution until you've make a plan to achieve the same functionality some
other way.
2021-05-17 14:09:07 -07:00