Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Various fixes for "terraform validate" command #18318

Merged
merged 3 commits into from
Jun 26, 2018
Merged

Various fixes for "terraform validate" command #18318

merged 3 commits into from
Jun 26, 2018

Commits on Jun 22, 2018

  1. command: validate must set values for root variables 

    Since the intent of the validate command is to check config validity
regardless of context (input variables, state, etc), we use unknown values
of the requested type here, which will then allow us to complete type
checking against the specified types of the variables without assuming
any particular values.
    @apparentlymart
    apparentlymart committed Jun 22, 2018
    Configuration menu
    Copy the full SHA
    1535851 View commit details
    Browse the repository at this point in the history

  2. core: Handle count.index evaluation more explicitly 

    Previously we had the evaluate methods accept directly an
addrs.InstanceKey and had our evaluator infer a suitable value for
count.index for it, but that prevents us from setting the index to be
unknown in the validation scenario where we may not be able to predict
the number of instances yet but we still want to be able to check that
the configuration block is type-safe for all possible count values.

To achieve this, we separate the concern of deciding on a value for
count.index from the concern of evaluating it, which then allows for
other implementations of this in future. For the purpose of this commit
there is no change in behavior, with the count.index value being populated
whenever the instance key is a number.

This commit does a little more groundwork for the future implementation
of the for_each feature (which'll support each.key and each.value) but
still doesn't yet implement it, leaving it just stubbed out for the
moment.
    @apparentlymart
    apparentlymart committed Jun 22, 2018
    Configuration menu
    Copy the full SHA
    89de9f9 View commit details
    Browse the repository at this point in the history

  3. core: Don't DynamicExpand during validate 

    Previously we would attempt to DynamicExpand during the validate walk and
then validate each expanded instance separately. However, this meant that
we would not be able to validate the contents of a block where count = 0
or if count is not yet known.

Here we instead do a more static validation pass against the resource
configuration itself, setting count.index to cty.UnknownVal(cty.Number) so
we can type-check everything inside the block as being correct regardless
of the final count.

This is another step towards repairing the "validate" command for our
changed assumptions in a world where we have a more sophisticated type
checker.

This doesn't yet address the remaining problem that the expression
evaluator can't, with the current state structures, distinguish between
a completed resource with count = 0 and a resource that doesn't exist
at all (during validate), and so we'll still get errors if an expression
elsewhere in configuration refers to a dynamic index of a resource with
"count" set. That's a pre-existing condition that's no longer being masked
by _this_ problem, but can't be addressed until we've introduced the new
state types (states.State, etc) and thus we _can_ distinguish these two
situations. That will therefore be addressed in a later commit.
    @apparentlymart
    apparentlymart committed Jun 22, 2018
    Configuration menu
    Copy the full SHA
    b2aaa7d View commit details
    Browse the repository at this point in the history