1212 Commits (96e8bd0c3e8a2340944cf0affdf006b1acbcbae9)

Author	SHA1	Message	Date
James Bardin	62a8b9ef1d	Merge pull request #32207 from hashicorp/jbardin/destroy-plan-state ... Ensure destroy plan contains valid state values	3 years ago
James Bardin	b5168eb6f4	Merge pull request #32208 from hashicorp/jbardin/pre-desstroy-refresh ... Make the pre-destroy refresh a full plan	3 years ago
James Bardin	242b8a726c	Merge pull request #32206 from hashicorp/jbardin/communicator-size ... fix typo in scp upload size check	3 years ago
James Bardin	3db3ed03fb	ensure destroy plan contains valid state values ... Some prior refactors left the detroyPlan method a bit confusing, and ran into a case where the previous run state could be returned as nil. Get rid of the no longer used pendingPlan value, and track the prior and prev states directly, making sure we always have a value for both.	3 years ago
James Bardin	3ea704ef81	Make the pre-destroy refresh a full plan ... In order to complete the terraform destroy command, a refresh must first be done to update state and remove any instances which have already been deleted externally. This was being done with a refresh plan, which will avoid any condition evaluations and avoid planning new instances. That however can fail due to invalid references from resources that are already missing from the state. A new plan type to handle the concept of the pre-destroy-refresh is needed here, which should probably be incorporated directly into the destroy plan, just like the original refresh walk was incorporated into the normal planning process. That however is major refactoring that is not appropriate for a patch release. Instead we make two discrete changes here to prevent blocking a destroy plan. The first is to use a normal plan to refresh, which will enable evaluation because missing and inconsistent instances will be planned for creation and updates, allowing them to be evaluated. That is not optimal of course, but does revert to the method used by previous Terraform releases until a better method can be implemented. The second change is adding a preDestroyRefresh flag to the planning process. This is checked in any location which evalCheckRules is called, and lets us change the diagnosticSeverity of the output to only be warnings, matching the behavior of a normal refresh plan.	3 years ago
James Bardin	8ba8d5aec4	fix typo in upload size check ... The scp upload size check had a typo preventing files from reporting their size, causing an extra temp file to be created.	3 years ago
Liam Cervante	0c7fda1906	Update HCL and go-cty to fix optional and default attributes (#32178) ... * Add test cases to verify all the default and optional issues are fixed * actually commit all the tests * update go-cty * Update hcl	3 years ago
Martin Atkins	d0a35c60a7	providercache: Ignore lock-mismatching global cache entries ... When we originally introduced the trust-on-first-use checksum locking mechanism in v0.14, we had to make some tricky decisions about how it should interact with the pre-existing optional read-through global cache of provider packages: The global cache essentially conflicts with the checksum locking because if the needed provider is already in the cache then Terraform skips installing the provider from upstream and therefore misses the opportunity to capture the signed checksums published by the provider developer. We can't use the signed checksums to verify a cache entry because the origin registry protocol is still using the legacy ziphash scheme and that is only usable for the original zipped provider packages and not for the unpacked-layout cache directory. Therefore we decided to prioritize the existing cache directory behavior at the expense of the lock file behavior, making Terraform produce an incomplete lock file in that case. Now that we've had some real-world experience with the lock file mechanism, we can see that the chosen compromise was not ideal because it causes "terraform init" to behave significantly differently in its lock file update behavior depending on whether or not a particular provider is already cached. By robbing Terraform of its opportunity to fetch the official checksums, Terraform must generate a lock file that is inherently non-portable, which is problematic for any team which works with the same Terraform configuration on multiple different platforms. This change addresses that problem by essentially flipping the decision so that we'll prioritize the lock file behavior over the provider cache behavior. Now a global cache entry is eligible for use if and only if the lock file already contains a checksum that matches the cache entry. This means that the first time a particular configuration sees a new provider it will always be fetched from the configured installation source (typically the origin registry) and record the checksums from that source. On subsequent installs of the same provider version already locked, Terraform will then consider the cache entry to be eligible and skip re-downloading the same package. This intentionally makes the global cache mechanism subordinate to the lock file mechanism: the lock file must be populated in order for the global cache to be effective. For those who have many separate configurations which all refer to the same provider version, they will need to re-download the provider once for each configuration in order to gather the information needed to populate the lock file, whereas before they would have only downloaded it for the _first_ configuration using that provider. This should therefore remove the most significant cause of folks ending up with incomplete lock files that don't work for colleagues using other platforms, and the expense of bypassing the cache for the first use of each new package with each new configuration. This tradeoff seems reasonable because otherwise such users would inevitably need to run "terraform providers lock" separately anyway, and that command _always_ bypasses the cache. Although this change does decrease the hit rate of the cache, if we subtract the never-cached downloads caused by "terraform providers lock" then this is a net benefit overall, and does the right thing by default without the need to run a separate command.	4 years ago
Brandon Croft	be5984d664	Merge pull request #32004 from hashicorp/brandonc/nested_attr_sensitive ... fix: don't reveal nested attributes with sensitive schema	4 years ago
James Bardin	1100eae89f	use UIMode instead of 0 changes to detect refresh	4 years ago
James Bardin	cccfa5e4af	Merge pull request #32111 from hashicorp/jbardin/refresh-only-data-read ... don't plan data source reads during refresh-only	4 years ago
Liam Cervante	6521355ba5	Convert variable types before applying defaults (#32027) ... * Convert variable types before applying defaults * revert change to unrelated test * Add another test case to verify behaviour * update go-cty * Update internal/terraform/eval_variable.go Co-authored-by: alisdair <alisdair@users.noreply.github.com> Co-authored-by: alisdair <alisdair@users.noreply.github.com>	4 years ago
Graham Davison	6663cde619	Merge pull request #23965 from tpaschalis/disallow-s3-backend-key-trailing-slash ... S3 Backend : Bucket key should not contain trailing slash	4 years ago
James Bardin	efd77159dd	use key data from plan method for apply	4 years ago
James Bardin	ffe2e3935e	avoid re-writing state for noop applies ... We need to avoid re-writing the state for every NoOp apply. We may still be evaluating the instance to account for any side-effects in the condition checks, however the state of the instance has not changes. Re-writing the state is a non-current operation, which may require encoding a fairly large instance state and re-serializing the entire state blob, so it is best avoided if possible.	4 years ago
James Bardin	eb88ccbc7b	only add NoOp nodes with conditions ... ONly add NoOp changes to the apply graph if they have conditions which need to be evaluated.	4 years ago
James Bardin	19152e7ba5	fix log mesage	4 years ago
James Bardin	eae246cfb5	normalize empty CheckResults fields in stateV4 ... Ensure that empty check results are normalized in state serialization to prevent unexpected state changes from being written. Because there is no consistent empty, null and omit_empty usage for state structs, there's no good way to create a test which will fail for future additions.	4 years ago
James Bardin	fa4c652013	changes are mutated during apply	4 years ago
James Bardin	b61c02da05	don't lose checks from refresh-only plan ... If there are no changes, then there is no reason to create an apply graph since all objects are known. We however do need the walk to match the expected state structure. This is probably only cleanup of empty nested modules and outputs, but some investigation is needed before making the full change. For now we can store the checks from the plan directly into the new state, since the apply walk overwrote the results we had already.	4 years ago
Paschalis Tsilias	4cb355f3d6	S3 Backend : Bucket key should not contain trailing slash	4 years ago
Tom Bamford	ff68c8d129	Merge pull request #31966 from kschu91/feature/azure-generic-oidc ... backend/azurerm: add support for generic OIDC authentication	4 years ago
James Bardin	300ad02937	don't plan data source reads during refresh-only ... When executing a refresh-only plan, it is not valid to plan a data source read. If the data source config is not known during planning, the only valid update would be the prior state, if there is any.	4 years ago
Brandon Croft	bd744ad4e9	Unify all sensitive value plan output as "(sensitive value)" ... Previously, there was mixed usage of "(sensitive)" and "(sensitive value)" and even though it was more common to see "(sensitive)", the thought is that it's a value we are hiding rather than describing something already shown.	4 years ago
Brandon Croft	076fccd8e7	fix: don't reveal nested attributes with sensitive schema	4 years ago
James Bardin	92c8c76684	fix variable name	4 years ago
James Bardin	a44c859c86	update UIMode comment	4 years ago
James Bardin	ac99cd6051	remove extra import line	4 years ago
James Bardin	28d5a5bf63	NoOp nodes should not have destroy edges ... NoOp changes should not participate in a destroy sequence, but because they are included as normal update nodes the usual connections were still being made.	4 years ago
James Bardin	586401aeea	make naming consistent	4 years ago
James Bardin	8a4883fd13	don't eval checks on destroy	4 years ago
James Bardin	8d11c7f524	the destroy refresh plan should be refresh-only ... Refreshing for a destroy should use the refresh-only plan to avoid planning new objects or evaluating conditions. This should also be skipped if there is no state, since there would be nothing to refresh.	4 years ago
James Bardin	08081097cb	check console with preconditions	4 years ago
James Bardin	a0723442b9	test for incorrectly evaluated outputs	4 years ago
James Bardin	333bdecf39	checks must be registered during eval	4 years ago
James Bardin	1eb22fd94a	fix output transformer names ... The removeRootOutputs field was not strictly used for that purpose, and was also copied to another DestroyPlan field.	4 years ago
James Bardin	47b6386348	remove IsFullDestroy workaround ... IsFullDestroy was a workaround during apply to detect when the change set was created by a destroy plan. This no longer works correctly, and we need to fall back to the UIMode set in the plan.	4 years ago
James Bardin	bcb792ee00	complete the root output expansion ... Not all root output instances were going through proper expansion when destroy operations were involved, leading to cases where they would be evaluated even though the expected result was only to remove them from the state. Normally destroy nodes stand alone in the graph, and do not produce references to other nodes. Because root output nodes were replaced by expansion nodes, these were being connected via normal references, even in the case where we were working with a destroy graph.	4 years ago
James Bardin	8a24d73d15	outputs should not be checked during destroy ... Module output may need to be evaluated during destroy in order to possibly be used by providers. The final state however is that all objects are destroyed, so preconditions should not be evaluated.	4 years ago
James Bardin	71837d187b	walkDestroy op in apply graph	4 years ago
James Bardin	5085ccdfbd	fix error message	4 years ago
Martin Atkins	4bc1696fd1	core: Simplify our idea of "root node" and require it for DynamicExpand ... The graph walking mechanism is specified as requiring a graph with a single root, which in practice means there's exactly one node in the graph which doesn't have any dependencies. However, we previously weren't verifying that invariant is true for subgraphs returned from DynamicExpand. It was working anyway, but it's not ideal to be relying on a behavior that isn't guaranteed by our underlying infrastructure. We also previously had the RootTransformer being a bit clever and trying to avoid adding a new node if there is already only a single graph with no dependencies. That special case isn't particularly valuable since there's no harm in turning a one-node graph into a two-node graph with an explicit separate root node, and doing that allows us to assume that the root node is always present and is always exactly terraform.rootNode. Many existing DynamicExpand implementations were not producing valid graphs and were previously getting away with it. All of them now produce properly-rooted graphs that should pass validation, and we will guarantee that with an explicit check of the DynamicExpand return value before we try to walk that subgraph. For good measure we also verify that the root node is exactly terraform.rootNode, even though that isn't strictly required by our graph walker, just to help us catch potential future bugs where a DynamicExpand implementation neglects to add our singleton root node.	4 years ago
Brian Flad	8c93420270	docs/plugin-protocol: Add notes about missing configuration in ReadResource and UpgradeResourceState request messages (#31998) ... This opts to inline document these intentional design decisions in the protocol definition as a catch-all for it not being documented elsewhere. Protocol Buffers files updated via: ```shell make protobuf ```	4 years ago
Tom Bamford	82c0b440dc	Fix default values	4 years ago
James Bardin	3779dbc2af	noop orphan change has nothing to apply ... An orphaned resource which plans as a NoOp change will have no config. This is not an error, but there is nothing to do since there are also no checks to validate. We still leave the change in the plan to keep the plan as complete as possible, noting all possible changes. Preventing the node from being added to the graph is awkward, because the config is attached separately from the diff transformer. This should not pose any problems however, because there is no longer any state or config linking the instance to any dependencies in the graph.	4 years ago
Sarah French	89ef27d3b4	Add Private Service Connect endpoint support to GCS backend (#31967) ... * Add support for `storage_custom_endpoint` in `gcs` backend * Add documentation for new `storage_custom_endpoint` endpoint * Empty commit to trigger Vercel deployment	4 years ago
Brandon Croft	bc1436af53	Merge pull request #31821 from glennsarti/gs/TF-707-add-pre-apply ... Add support for pre-apply task results in the cloud backend	4 years ago
Kevin Schu	5842950291	Added support for generic OIDC authentication (eg. Gitlab) and ensure documentation is more clear about it´s usage. oidc_request_url and oidc_request_token are meant to be used for Github Actions only.	4 years ago
Nick Fagerlund	ccd7bd017e	Clarify some comments in internal/dag ... When reading this code to check Terraform's graph sorting behavior, I got very confused about the direction of traversal for several methods. Although some of these methods would also probably benefit from renames, this commit only updates their doc comments to use the same directional terminology that we use in the `Edge` interface (source/target).	4 years ago
James Bardin	041d9d3eec	unknown evaluation of missing instances in import ... Because import does not yet plan new instances as part of the import process, we can end up evaluating references to resources which have no state at all. The fallback for this situation could result in slightly better values during import. The count and for_each values were technically incorrect, since the length is not known to be zero, and the single instance does have a concrete type which we can return.	4 years ago