Commit Graph

831 Commits

Author SHA1 Message Date
Theo Chupp d15a2bc024
fix: local variables should not be overridden by remote variables during `terraform import` (#29972)
* fix: local variables should not be overridden by remote variables during `terraform import`

* chore: applied the same fix in the 'internal/cloud' package

* backport changes from cloud package to remote package

Co-authored-by: Alisdair McDiarmid <alisdair@users.noreply.github.com>
Co-authored-by: uturunku1 <luces.huayhuaca@gmail.com>
2022-03-15 14:42:11 -07:00
Alisdair McDiarmid b5cfc0bb8b core: Fix sensitive variable validation errors
Variable validation error message expressions which generated sensitive
values would previously crash. This commit updates the logic to align
with preconditions and postconditions, eliding sensitive error message
values and adding a separate diagnostic explaining why.
2022-03-11 13:45:04 -05:00
Alisdair McDiarmid 6db174e210 core: Fix crash for sensitive values in conditions
Precondition and postcondition blocks which evaluated expressions
resulting in sensitive values would previously crash. This commit fixes
the crashes, and adds an additional diagnostic if the error message
expression produces a sensitive value (which we also elide).
2022-03-11 13:45:04 -05:00
Alisdair McDiarmid 6cd0876596
Merge pull request #30658 from hashicorp/alisdair/preconditions-postconditions-refresh-only
core: Eval pre/postconditions in refresh-only mode
2022-03-11 13:44:51 -05:00
Alisdair McDiarmid a103c65140 core: Eval pre/postconditions in refresh-only mode
Evaluate precondition and postcondition blocks in refresh-only mode, but
report any failures as warnings instead of errors. This ensures that any
deviation from the contract defined by condition blocks is reported as
early as possible, without preventing the completion of a state refresh
operation.

Prior to this commit, Terraform evaluated output preconditions and data
source pre/postconditions as normal in refresh-only mode, while managed
resource pre/postconditions were not evaluated at all. This omission
could lead to confusing partial condition errors, or failure to detect
undesired changes which would otherwise cause resources to become
invalid.

Reporting the failures as errors also meant that changes retrieved
during refresh could cause the refresh operation to fail. This is also
undesirable, as the primary purpose of the operation is to update local
state. Precondition/postcondition checks are still valuable here, but
should be informative rather than blocking.
2022-03-11 13:32:40 -05:00
James Bardin 45e2a410f7
Merge pull request #30656 from hashicorp/jbardin/always-validate
Always validate the graph
2022-03-11 10:37:30 -05:00
James Bardin b1de94a176 make sure CBD test graphs are valid
The graphs used for the CBD tests wouldn't validate because they skipped
adding the root module node. Re add the root module transformer and
transitive reduction transformer to the build steps, and match the new
reduced output in the test fixtures.
2022-03-11 10:20:50 -05:00
James Bardin 0bc69d64ec always validate all graphs
Complete the removal of the Validate option for graph building. There is
no case where we want to allow an invalid graph, as the primary reason
for validation is to ensure we have no cycles, and we can't walk a graph
with cycles. The only code which specifically relied on there being no
validation was a test to ensure the Validate flag prevented it.
2022-03-11 10:20:50 -05:00
Alisdair McDiarmid 2ee64dc7e0
Merge pull request #30645 from hashicorp/alisdair/preconditions-postconditions-expanded-resources
core: Fix expanded condition block validation
2022-03-11 10:12:08 -05:00
Alisdair McDiarmid ef0d859af7 core: Refactor stub repetition data generation 2022-03-10 13:52:48 -05:00
Alisdair McDiarmid ad995322e1 core: Fix expanded condition block validation
The previous precondition/postcondition block validation implementation
failed if the enclosing resource was expanded. This commit fixes this by
generating appropriate placeholder instance data for the resource,
depending on whether `count` or `for_each` is used.
2022-03-10 13:47:17 -05:00
Martin Atkins 1879a39d2d configs: Refined error messages for mismatched provider passing
This set of diagnostic messages is under a number of unusual constraints
that make them tough to get right:
 - They are discussing a couple finicky concepts which authors are
   likely to be encountering for the first time in these error messages:
   the idea of "local names" for providers, the relationship between those
   and provider source addresses, and additional ("aliased") provider
   configurations.
 - They are reporting concerns that span across a module call boundary,
   and so need to take care to be clear about whether they are talking
   about a problem in the caller or a problem in the callee.
 - Some of them are effectively deprecation warnings for features that
   might be in use by a third-party module that the user doesn't control,
   in which case they have no recourse to address them aside from opening
   a feature request with the upstream module maintainer.
 - Terraform has, for backward-compatibility reasons, a lot of implied
   default behaviors regarding providers and provider configurations,
   and these errors can arise in situations where Terraform's assumptions
   don't match the author's intent, and so we need to be careful to
   explain what Terraform assumed in order to make the messages
   understandable.

After seeing some confusion with these messages in the community, and
being somewhat confused by some of them myself, I decided to try to edit
them a bit for consistency of terminology (both between the messages and
with terminology in our docs), being explicit about caller vs. callee
by naming them in the messages, and making explicit what would otherwise
be implicit with regard to the correspondences between provider source
addresses and local names.

My assumed audience for all of these messages is the author of the caller
module, because it's the caller who is responsible for creating the
relationship between caller and callee. As much as possible I tried to
make the messages include specific actions for that author to take to
quiet the warning or fix the error, but some of the warnings are only
fixable by the callee's maintainer and so those messages are, in effect,
a suggestion to send a request to the author to stop using a deprecated
feature.

I think these new messages are also not ideal by any means, because it's
just tough to pack so much information into concise messages while being
clear and consistent, but I hope at least this will give users seeing
these messages enough context to infer what's going on, possibly with the
help of our documentation.

I intentionally didn't change which cases Terraform will return warnings
or errors -- only the message texts -- although I did highlight in a
comment in one of the tests that what it is a asserting seems a bit
suspicious to me. I don't intend to address that here; instead, I intend
that note to be something to refer to if we later see a bug report that
calls that behavior into question.

This does actually silence some _unrelated_ warnings and errors in cases
where a provider block has an invalid provider local name as its label,
because our other functions for dealing with provider addresses are
written to panic if given invalid addresses under the assumption that
earlier code will have guarded against that. Doing this allowed for the
provider configuration validation logic to safely include more information
about the configuration as helpful context, without risking tripping over
known-invalid configuration and panicking in the process.
2022-03-10 10:05:56 -08:00
James Bardin 05a10f06d1 remove PreDiff and PostDiff hook calls
PreDiff and PostDiff hooks were designed to be called immediately before
and after the PlanResourceChange calls to the provider. Probably due to
the confusing legacy naming of the hooks, these were scattered about the
nodes involved with planning, causing the hooks to be called in a number
of places where they were designed, including data sources and destroy
plans. Since these hooks are not used at all any longer anyway, we can
removed the extra calls with no effect.

If we choose in the future to call PlanResourceChange for resource
destroy plans, the hooks can be re-inserted (even though they currently
are unused) into the new code path which must diverge from the current
combined path of managed and data sources.
2022-03-08 13:48:41 -05:00
James Bardin dc668dff38 ensure UI hooks are called for data sources
The UI hooks for data source reads were missed during planning. Move the
hook calls to immediatley before and after the ReadDataSource calls to
ensure they are called during both plan and apply.
2022-03-08 13:06:30 -05:00
Alisdair McDiarmid 45d0c04707 core: Add fallback for JSON syntax error messages
Custom variable validations specified using JSON syntax would always
parse error messages as string literals, even if they included template
expressions. We need to be as backwards compatible with this behaviour
as possible, which results in this complex fallback logic. More detail
about this in the extensive code comments.
2022-03-04 15:39:31 -05:00
Alisdair McDiarmid b59bffada6 core: Evaluate pre/postconditions during validate
During the validation walk, we attempt to proactively evaluate check
rule condition and error message expressions. This will help catch some
errors as early as possible.

At present, resource values in the validation walk are of dynamic type.
This means that any references to resources will cause validation to be
delayed, rather than presenting useful errors. Validation may still
catch other errors, and any future changes which cause better type
propagation will result in better validation too.
2022-03-04 15:39:31 -05:00
Alisdair McDiarmid b06fe04621 core: Check rule error message expressions
Error messages for preconditions, postconditions, and custom variable
validations have until now been string literals. This commit changes
this to treat the field as an HCL expression, which must evaluate to a
string. Most commonly this will either be a string literal or a template
expression.

When the check rule condition is evaluated, we also evaluate the error
message. This means that the error message should always evaluate to a
string value, even if the condition passes. If it does not, this will
result in an error diagnostic.

If the condition fails, and the error message also fails to evaluate, we
fall back to a default error message. This means that the check rule
failure will still be reported, alongside diagnostics explaining why the
custom error message failed to render.

As part of this change, we also necessarily remove the heuristic about
the error message format. This guidance can be readded in future as part
of a configuration hint system.
2022-03-04 15:35:39 -05:00
Sebastian Rivera afb956d745
Merge pull request #30141 from hashicorp/preapply-runtasks-clioutput
Cloud run tasks (post-plan only) CLI integration
2022-02-25 15:46:46 -05:00
Sebastian Rivera 52c5f9f6b7 Updated for latest go-tfe run task changes 2022-02-25 15:32:16 -05:00
Sebastian Rivera 126d6df088 Added run task support for post plan run stage, removed pre apply
This commit stems from the change to make post plan the default run task stage, at the
time of this commit's writing! Since pre apply is under internal revision, we have removed
the block that polls the pre apply stage until the team decides to re-add support for pre apply
run tasks.
2022-02-24 14:06:57 -05:00
uturunku1 383da4893b use new enum string for task stages 2022-02-24 14:06:57 -05:00
Brandon Croft aa0dda81b4 Fall back to reading latest run without task_stages
Older versions of TFE will not allow "task_stages" as an include parameter. In this case, fall back to reading the Run without additional options.
2022-02-24 14:06:57 -05:00
uturunku1 a9da859ee5 rename variables to something more descriptive 2022-02-24 14:03:02 -05:00
uturunku1 77946af472 pull latest changes from go-tfe branch and use use new field name that previously was incorrectly named TaskStage 2022-02-24 14:03:00 -05:00
uturunku1 8090b23db7 delete unused function 2022-02-24 14:02:37 -05:00
Brandon Croft 0b8bb29a61 [cloud] refactor integration context and add code documentation 2022-02-24 14:02:37 -05:00
Brandon Croft 791c36c504 [cloud] report run tasks by name instead of assuming pre_apply 2022-02-24 14:02:37 -05:00
uturunku1 6b5da4d43c [cloud] run tasks output formatting 2022-02-24 14:02:37 -05:00
Brandon Croft 2938ec43fa [cloud] handle unreachable run tasks 2022-02-24 14:02:37 -05:00
uturunku1 3e9ae69a12 [cloud] run tasks integration
This change will await the completion of pre-apply run tasks if they
exist on a run and then report the results.

It also adds an abstraction when interacting with cloud integrations such
as policy checking and cost estimation that simplify and unify output,
although I did not go so far as to refactor those callers to use it yet.
2022-02-24 14:02:35 -05:00
Alisdair McDiarmid 70db495d00
Merge pull request #30525 from nozaq/provider-config-default
jsonconfig: fix keys for default providers
2022-02-22 16:13:41 -05:00
Alisdair McDiarmid 3e4d6b252f jsonplan: Improve performance for deep objects
When calculating the unknown values for JSON plan output, we would
previously recursively call the `unknownAsBool` function on the current
sub-tree twice, if any values were unknown. This was wasteful, but not
noticeable for normal Terraform resource shapes.

However for deeper nested object values, such as Kubernetes manifests,
this was a severe performance problem, causing `terraform show -json` to
take several hours to render a plan.

This commit reuses the already calculated unknown value for the subtree,
and adds benchmark coverage to demonstrate the improvement.
2022-02-18 17:00:18 -05:00
nozaq 3c32f7a56c
jsonconfig: add implicitly created provider configs 2022-02-19 01:55:09 +09:00
nozaq 0ce040405c
jsonconfig: fix provider mappings with same names 2022-02-19 00:44:48 +09:00
kmoe 161374725c
Warn when ignore_changes includes a Computed attribute (#30517)
* ignore_changes attributes must exist in schema

Add a test verifying that attempting to add a nonexistent attribute to
ignore_changes throws an error.

* ignore_changes cannot be used with Computed attrs

Return a warning if a Computed attribute is present in ignore_changes,
unless the attribute is also Optional.

ignore_changes on a non-Optional Computed attribute is a no-op, so the user
likely did not want to set this in config.
An Optional Computed attribute, however, is still subject to ignore_changes
behaviour, since it is possible to make changes in the configuration that
Terraform must ignore.
2022-02-18 10:38:29 +00:00
nozaq 7f4019e1f6
jsonconfig: fix keys for default providers
Fixes provider config keys to reflect implicit provider inheritance.
2022-02-16 01:38:57 +09:00
Pedro Belém aed7162e9a
cli: Fix state mv exit code for missing resource (#29839) 2022-02-14 15:20:03 -05:00
Alisdair McDiarmid d8018270a7
Merge pull request #30138 from hashicorp/alisdair/json-module-call-providers-mapping
jsonconfig: Improve provider configuration output
2022-02-10 10:33:47 -05:00
Alisdair McDiarmid 843c50e8ce lang: Further limit the console-only type function
This commit introduces a capsule type, `TypeType`, which is used to
extricate type information from the console-only `type` function. In
combination with the `TypeType` mark, this allows us to restrict the use
of this function to top-level display of a value's type. Any other use
of `type()` will result in an error diagnostic.
2022-02-10 06:12:58 -05:00
Alisdair McDiarmid 903d6f1055 lang: Remove use of marks.Raw in tests
These instances of marks.Raw usage were semantically only testing the
properties of combining multiple marks. Testing this with an arbitrary
value for the mark is just as valid and clearer.
2022-02-09 17:43:54 -05:00
Alisdair McDiarmid 691b98b612 cli: Prevent overuse of console-only type function
The console-only `type` function allows interrogation of any value's
type.  An implementation quirk is that we use a cty.Mark to allow the
console to display this type information without the usual HCL quoting.
For example:

> type("boop")
string

instead of:

> type("boop")
"string"

Because these marks can propagate when used in complex expressions,
using the type function as part of a complex expression could result in
this "print as raw" mark being attached to a collection. When this
happened, it would result in a crash when we tried to iterate over a
marked value.

The `type` function was never intended to be used in this way, which is
why its use is limited to the console command. Its purpose was as a
pseudo-builtin, used only at the top level to display the type of a
given value.

This commit goes some way to preventing the use of the `type` function
in complex expressions, by refusing to display any non-string value
which was marked by `type`, or contains a sub-value which was so marked.
2022-02-09 17:43:54 -05:00
Alisdair McDiarmid fe8183c4af json: Increment JSON plan format version
The JSON plan configuration data now includes a `full_name` field for
providers. This addition warrants a backwards compatible increment to
the version number.
2022-02-07 15:06:05 -05:00
Alisdair McDiarmid ddc81a204f json: Disregard format version in tests
Instead of manually updating every JSON output test fixture when we
change the format version, disregard any differences when testing.
2022-02-07 15:05:58 -05:00
Alisdair McDiarmid f5b90f84a8 jsonconfig: Improve provider configuration output
When rendering configuration as JSON, we have a single map of provider
configurations at the top level, since these are globally applicable.
Each resource has an opaque key into this map which points at the
configuration data for the provider.

This commit fixes two bugs in this implementation:

- Resources in non-root modules had an invalid provider config key,
  which meant that there was never a valid reference to the provider
  config block. These keys were prefixed with the local module name
  instead of the path to the module. This is now corrected.

- Modules with passed provider configs would point to either an empty
  provider config block or one which is not present at all. This has
  been fixed so that these resources point to the provider config block
  from the calling module (or wherever up the module tree it was
  originally defined).

We also add a "full_name" key-value pair to the provider config block,
with the entire fully-qualified provider name including hostname and
namespace.
2022-02-07 15:05:58 -05:00
Alisdair McDiarmid 639eb5212f core: Remove unused PlanOpts.Validate
This vestigial field was written to but never read.
2022-02-03 14:16:25 -05:00
Alisdair McDiarmid 7ded73f266 configs: Validate pre/postcondition self-refs
Preconditions and postconditions for resources and data sources may not
refer to the address of the containing resource or data source. This
commit adds a parse-time validation for this rule.
2022-02-03 09:37:22 -05:00
Alisdair McDiarmid cdae6d4396 core: Add context tests for pre/post conditions 2022-01-31 15:38:26 -05:00
Alisdair McDiarmid a95ad997e1 core: Document postconditions as valid use of self
This is not currently gated by the experiment only because it is awkward
to do so in the context of evaluationStateData, which doesn't have any
concept of experiments at the moment.
2022-01-31 14:34:35 -05:00
Martin Atkins 5573868cd0 core: Check pre- and postconditions for resources and output values
If the configuration contains preconditions and/or postconditions for any
objects, we'll check them during evaluation of those objects and generate
errors if any do not pass.

The handling of post-conditions is particularly interesting here because
we intentionally evaluate them _after_ we've committed our record of the
resulting side-effects to the state/plan, with the intent that future
plans against the same object will keep failing until the problem is
addressed either by changing the object so it would pass the precondition
or changing the precondition to accept the current object. That then
avoids the need for us to proactively taint managed resources whose
postconditions fail, as we would for provisioner failures: instead, we can
leave the resolution approach up to the user to decide.

Co-authored-by: Alisdair McDiarmid <alisdair@users.noreply.github.com>
2022-01-31 14:02:53 -05:00
Martin Atkins c827c049fe terraform: Precondition and postcondition blocks generate dependencies
If a resource or output value has a precondition or postcondition rule
then anything the condition depends on is a dependency of the object,
because the condition rules will be evaluated as part of visiting the
relevant graph node.
2022-01-28 11:00:29 -05:00