29063 Commits (5ff2495ffaeb58b045815e47beffa2cb33b08b76)

 

All Branches   

Search

Author	SHA1	Message	Date
Martin Atkins	f60d55d6ad	core: Emit only one warning for move collisions in destroy-plan mode ... Our current implementation of destroy planning includes secretly running a normal plan first, in order to get its effect of refreshing the state. Previously our warning about colliding moves would betray that implementation detail because we'd return it from both of our planning operations here and thus show the message twice. That would also have happened in theory for any other warnings emitted by both plan operations, but it's the move collision warning that made it immediately visible. We'll now only return warnings from the initial plan if we're also returning errors from that plan, and thus the warnings of both plans can never mix together into the same diags and thus we'll avoid duplicating any warnings. This does mean that we'd lose any warnings which might hypothetically emerge only from the hidden normal plan and not from the subsequent destroy plan, but we'll accept that as an okay tradeoff here because those warnings are likely to not be super relevant to the destroy case anyway, or else we'd emit them from the destroy-plan walk too.	5 years ago
James Bardin	372814e49a	Merge pull request #29659 from hashicorp/jbardin/really-nested-within ... refactoring: exhaustive NestedWithin checks	5 years ago
James Bardin	cac1f5c264	refactoring: exhaustive NestedWithin checks ... When checking dependencies between statements, we need to check all combinations of To and From addresses.	5 years ago
Alisdair McDiarmid	e09bad76ff	build: Add exhaustive switch statement lint ... For now, only check the JSON views package, since this was the instance that most recently tripped us up. There are a few dozen failures elsewhere in Terraform which would need to be addressed before expanding this to other packages.	5 years ago
Alisdair McDiarmid	a742d7ee88	Merge pull request #29649 from hashicorp/alisdair/json-ui-exhaustiveness ... json-output: Add change reasons to explain deletes	5 years ago
Alisdair McDiarmid	b699391d04	json-output: Add change reasons to explain deletes ... The extra feedback information for why resource instance deletion is planned is now included in the streaming JSON UI output. We also add an explicit case for no-op actions to switch statements in this package to ensure exhaustiveness, for future linting.	5 years ago
James Bardin	8ce2254ad6	Merge pull request #29647 from hashicorp/jbardin/test-temp-cleanup ... temp path clean for some backend tests	5 years ago
James Bardin	c8cd0b1e74	Merge pull request #29624 from hashicorp/jbardin/no-block-to-attr ... skip FixUpBlockAttrs when we can detect types from a new SDK	5 years ago
Alisdair McDiarmid	50fac6afd8	Merge pull request #29645 from hashicorp/alisdair/verify-remote-terraform-version-update ... backend/remote: Support interop from 0.14 to 1.1	5 years ago
Alisdair McDiarmid	57318ef561	backend/remote: Support interop from 0.14 to 1.1 ... The previous conservative guarantee that we would not make backwards incompatible changes to the state file format until at least Terraform 1.1 can now be extended. Terraform 0.14 through 1.1 will be able to interoperably use state files, so we can update the remote backend version compatibility check accordingly.	5 years ago
Alisdair McDiarmid	f1e9d88ddc	Merge pull request #29640 from hashicorp/alisdair/fix-refresh-only-with-orphans ... core: Fix refresh-only interaction with orphans	5 years ago
Martin Atkins	5d620303eb	Update CHANGELOG.md	5 years ago
Martin Atkins	0f76e3a4e1	Update CHANGELOG.md	5 years ago
Martin Atkins	d97ef10bb8	core: Don't return other errors if move statements are invalid ... Because our validation rules depend on some dynamic results produced by actually running the plan, we deal with moves in a "backwards" order where we try to apply them first -- ignoring anything strange we might find -- and then validate the original statements only after planning. An unfortunate consequence of that approach is that when the move statements are invalid it's likely that move execution will not fully complete, and so the generated plan is likely to be incorrect and might well include errors resulting from the unresolved moves. To mitigate that, here we let any move validation errors supersede all other diagnostics that the plan phase might've generated, in the hope that it'll help the user focus on fixing the incorrect move statements without creating confusing by reporting errors that only appeared as a quick of how Terraform worked around the invalid move statements earlier.	5 years ago
Martin Atkins	1bff623fd9	core: Report a warning if any moves get blocked ... In most cases Terraform will be able to automatically fully resolve all of the pending move statements before creating a plan, but there are some edge cases where we can end up wanting to move one object to a location where another object is already declared. One relatively-obvious example is if someone uses "terraform state mv" in order to create a set of resource instance bindings that could never have arising in normal Terraform use. A less obvious example arises from the interactions between moves at different levels of granularity. If we are both moving a module to a new address and moving a resource into an instance of the new module at the same time, the old module might well have already had a resource of the same name and so the resource move will be unresolvable. In these situations Terraform will move the objects as far as possible, but because it's never valid for a move "from" address to still be declared in the configuration Terraform will inevitably always plan to destroy the objects that didn't find a final home. To give some additional explanation for that result, here we'll add a warning which describes what happened. This is not a particularly actionable warning because we don't really have enough information to guess what the user intended, but we do at least prompt that they might be able to use the "terraform state" family of subcommands to repair the ambiguous situation before planning, if they want a different result than what Terraform proposed.	5 years ago
Martin Atkins	04f9e7148c	command/format: Include deletion reasons in plan report ... The core runtime is now able to specify a reason for some situations when Terraform plans to delete a resource instance. This commit makes that information visible in the human-oriented UI. A previous commit already made the underlying data informing these new hints visible as part of the machine-oriented (JSON) plan output. This also removes the bold formatting from the existing "has moved to" hints, because subjectively it seemed like the result was emphasizing too many parts of the output and thus somewhat defeating the benefit of the emphasis in trying to create additional visual hierarchy for sighted users running Terraform in a terminal. Now only the first line containing the main action statement will be in bold, and all of the parenthesized follow-up notes will be unformatted.	5 years ago
Martin Atkins	a1a713cf28	core: Report ActionReasons when we plan to delete "orphans" ... There are a few different reasons why a resource instance tracked in the prior state might be considered an "orphan", but previously we reported them all identically in the planned changes. In order to help users understand the reason for a surprising planned delete, we'll now try to specify an additional reason for the planned deletion, covering all of the main reasons why that could happen. This commit only introduces the new detail to the plans.Changes result, though it also incidentally exposes it as part of the JSON plan result in order to keep that working without returning errors in these new cases. We'll expose this information in the human-oriented UI output in a subsequent commit.	5 years ago
Martin Atkins	7b99861b1c	refactoring: Don't implicitly move for resources with for_each ... Our previous rule for implicitly moving from IntKey(0) to NoKey would apply that move even when the current resource configuration uses for_each, because we were only considering whether "count" were set. Previously this was relatively harmless because the resource instance in question would end up planned for deletion anyway: neither an IntKey nor a NoKey are valid keys for for_each. Now that we're going to be announcing these moves explicitly in the UI, it would be confusing to see Terraform report that IntKey moved to NoKey in a situation where the config changed from count to for_each, so to address that we'll only generate the implied statement if neither repetition argument is set.	5 years ago
James Bardin	9c078c27cf	temp path clean for some backend tests	5 years ago
Alisdair McDiarmid	ceb580ec40	core: Fix refresh-only interaction with orphans ... When planning in refresh-only mode, we must not remove orphaned resources due to changed count or for_each values from the planned state. This was previously happening because we failed to pass through the plan's skip-plan-changes flag to the instance orphan node.	5 years ago
Chris Arcand	db6ca866fc	Merge pull request #29627 from hashicorp/auto-select-single-workspace ... Auto select single workspace when necessary	5 years ago
Chris Arcand	171cdbbf93	command: Clean up testInputResponseMap before failing on unused answers ... If you don't, the unused answers will persist in the package-level var and bleed in to other tests.	5 years ago
Chris Arcand	60bc7aa05d	command: Auto-select single workspace if necessary ... When initializing a backend, if the currently selected workspace does not exist, the user is prompted to select from the list of workspaces the backend provides. Instead, we should automatically select the only workspace available _if_ that's all that's there. Although with being a nice bit of polish, this enables future improvments with Terraform Cloud in potentially removing the implicit depenency on always using the 'default' workspace when the current configuration is mapped to a single TFC workspace.	5 years ago
James Bardin	8706a18c4b	refine the skipFixup heuristic ... We can also rule out some attribute types as indicating something other than the legacy SDK. - Tuple types were not generated at all. - There were no single objects types, the convention was to use a block list or set of length 1. - Maps of objects were not possible to generate, since named blocks were not implemented. - Nested collections were not supported, but when they were generated they would have primitive types.	5 years ago
hc-github-team-tf-core	cd1d30ea95	Cleanup after v1.1.0-alpha20210922 release	5 years ago
hc-github-team-tf-core	d14bbbb6f2	Release v1.1.0-alpha20210922	5 years ago
Martin Atkins	3f1c15c792	Upgrade to Go 1.17.1	5 years ago
James Bardin	6b4e73af48	skip the blocktoattr fixup with nested types ... If structural types are being used, we can be assured that the legacy SDK SchemaConfigModeAttr is not being used, and the fixup is not needed. This prevents inadvertent mapping of blocks to structural attributes, and allows us to skip the fixup overhead when possible.	5 years ago
Martin Atkins	83f0376673	refactoring: ApplyMoves new return type ... When we originally stubbed ApplyMoves we didn't know yet how exactly we'd be using the result, so we made it a double-indexed map allowing looking up moves in both directions. However, in practice we only actually need to look up old addresses by new addresses, and so this commit first removes the double indexing so that each move is only represented by one element in the map. We also need to describe situations where a move was blocked, because in a future commit we'll generate some warnings in those cases. Therefore ApplyMoves now returns a MoveResults object which contains both a map of changes and a map of blocks. The map of blocks isn't used yet as of this commit, but we'll use it in a later commit to produce warnings within the "terraform" package.	5 years ago
Martin Atkins	d054102d38	addrs: AbsResource.UniqueKey distinct from AbsResourceInstance.UniqueKey ... The whole point of UniqueKey is to deal with the fact that we have some distinct address types which have an identical string representation, but unfortunately that fact caused us to not notice that we'd incorrectly made AbsResource.UniqueKey return a no-key instance UniqueKey instead of its own distinct unique key type.	5 years ago
Chris Arcand	08a86b6c13	Merge pull request #29621 from hashicorp/error-on-unused-test-answers ... command: Ensure all answers were used in command.testInputResponseMap	5 years ago
Chris Arcand	8684a85e26	command: Ensure all answers were used in command.testInputResponseMap ... Remove answers from testInputResponse as they are given, and raise an error during cleanup if any answers remain unused. This enables tests to ensure that the expected mock answers are actually used in a test; previously, an entire branch of code including an input sequence could be omitted and the test(s) would not fail. The only test that had unused answers in this map is one leftover from legacy state migrations, a prompt that was removed in 7c93b2e5e6	5 years ago
Paddy	4fe75bead3	Remove panic debugging information (#29512) ... It was out of date. Fixes #1113.	5 years ago
Laura Pacilio	8f09e27597	Merge pull request #29567 from drasko95/patch-1 ... Fix a documentation typo	5 years ago
Alisdair McDiarmid	5386d6c5b8	Merge pull request #29603 from hashicorp/alisdair/moved-json-output ... json-output: Add support for config-driven move to plan JSON and streaming JSON UI	5 years ago
Alisdair McDiarmid	b59b057591	json-output: Config-driven move support in JSON UI ... Add previous address information to the `planned_change` and `resource_drift` messages for the streaming JSON UI output of plan and apply operations. Here we also add a "move" action value to the `change` object of these messages, to represent a move-only operation. As part of this work we also simplify this code to use the plan's DriftedResources values instead of recomputing the drift from state.	5 years ago
Alisdair McDiarmid	78c4a8c461	json-output: Previous address for resource changes ... Configuration-driven moves are represented in the plan file by setting the resource's `PrevRunAddr` to a different value than its `Addr`. For JSON plan output, we here add a new field to resource changes, `previous_address`, which is present and non-empty only if the resource is planned to be moved. Like the CLI UI, refresh-only plans will include move-only changes in the resource drift JSON output. In normal plan mode, these are elided to avoid redundancy with planned changes.	5 years ago
Martin Atkins	78705f4f10	Update CHANGELOG.md	5 years ago
Martin Atkins	f0034beb33	core: refactoring.ImpliedMoveStatements replaces NodeCountBoundary ... Going back a long time we've had a special magic behavior which tries to recognize a situation where a module author either added or removed the "count" argument from a resource that already has instances, and to silently rename the zeroth or no-key instance so that we don't plan to destroy and recreate the associated object. Now we have a more general idea of "move statements", and specifically the idea of "implied" move statements which replicates the same heuristic we used to use for this behavior, we can treat this magic renaming rule as just another "move statement", special only in that Terraform generates it automatically rather than it being written out explicitly in the configuration. In return for wiring that in, we can now remove altogether the NodeCountBoundary graph node type and its associated graph transformer, CountBoundaryTransformer. We handle moves as a preprocessing step before building the plan graph, so we no longer need to include any special nodes in the graph to deal with that situation. The test updates here are mainly for the graph builders themselves, to acknowledge that indeed we're no longer inserting the NodeCountBoundary vertices. The vertices that NodeCountBoundary previously depended on now become dependencies of the special "root" vertex, although in many cases here we don't see that explicitly because of the transitive reduction algorithm, which notices when there's already an equivalent indirect dependency chain and removes the redundant edge. We already have plenty of test coverage for these "count boundary" cases in the context tests whose names start with TestContext2Plan_count and TestContext2Apply_resourceCount, all of which continued to pass here without any modification and so are not visible in the diff. The test functions particularly relevant to this situation are: - TestContext2Plan_countIncreaseFromNotSet - TestContext2Plan_countDecreaseToOne - TestContext2Plan_countOneIndex - TestContext2Apply_countDecreaseToOneCorrupted The last of those in particular deals with the situation where we have both a no-key instance _and_ a zero-key instance in the prior state, which is interesting here because to exercises an intentional interaction between refactoring.ImpliedMoveStatements and refactoring.ApplyMoves, where we intentionally generate an implied move statement that produces a collision and then expect ApplyMoves to deal with it in the same way as it would deal with all other collisions, and thus ensure we handle both the explicit and implied collisions in the same way. This does affect some UI-level tests, because a nice side-effect of this new treatment of this old feature is that we can now report explicitly in the UI that we're assigning new addresses to these objects, whereas before we just said nothing and hoped the user would just guess what had happened and why they therefore weren't seeing a diff. The backend/local plan tests actually had a pre-existing bug where they were using a state with a different instance key than the config called for but getting away with it because we'd previously silently fix it up. That's still fixed up, but now done with an explicit mention in the UI and so I made the state consistent with the configuration here so that the tests would be able to recognize _real_ differences where present, as opposed to the errant difference caused by that inconsistency.	5 years ago
Martin Atkins	ee9e346039	refactoring: ApplyMoves skips moving when destination address occupied ... Per our rule that the content of the state can never make a move statement invalid, our behavior for two objects trying to occupy the same address will be to just ignore that and let the object already at the address take priority. For the moment this is silent from an end-user perspective and appears only in our internal logs. However, I'm hoping that our future planned adjustment to the interface of this function will include some way to allow reporting these collisions in some end-user-visible way, either as a separate warning per collision or as a single warning that collects together all of the collisions into a single message somehow. This situation can arise both because the previous run state already contained an object at the target address of a move and because more than one move ends up trying to target the same location. In the latter case, which one "wins" is decided by our depth-first traversal order, which is in turn derived from our chaining and nesting rules and is therefore arbitrary but deterministic.	5 years ago
Martin Atkins	ef5a1c9cfe	refactoring: ImpliedMoveStatements function ... This new function complements the existing function FindMoveStatements by potentially generating additional "implied" move statements that aren't written explicit in the configuration but that we'll infer by comparing the configuration and te previous run state. The goal here is to infer only enough to replicate the effect of the "count boundary fixup" graph node (terraform.NodeCountBoundary) that we currently use to deal with this concern of preserving the zero-instance when switching between "count" and not "count". This is just dead code for now. A subsequent commit will introduce this into the "terraform" package while also removing terraform.NodeCountBoundary, thus achieving the same effect as before but in a way that'll get reported in the UI as a move, using the same language that we'd use for an explicit move statement.	5 years ago
Martin Atkins	7f99a8802e	addrs: MoveEndpointInModule.SelectsResource ... This is similar to the existing SelectsModule method, returning true if the reciever selects either a particular resource as a whole or any of the instances of that resource. We don't need this test in the normal case, but we will need it in a subsequent commit when we'll be possibly generating _implied_ move statements between instances of resources, but only if there aren't explicit move statements mentioning those resources already.	5 years ago
Alisdair McDiarmid	2b6a1be18f	Merge pull request #29597 from hashicorp/alisdair/move-refresh-ui ... cli: Improved plan UI for move-only changes	5 years ago
Alisdair McDiarmid	638784b195	cli: Omit move-only drift, except for refresh-only ... The set of drifted resources now includes move-only changes, where the object value is identical but a move has been executed. In normal operation, we previousl displayed these moves twice: once as part of drift output, and once as part of planned changes. As of this commit we omit move-only changes from drift display, except for refresh-only plans. This fixes the redundant output.	5 years ago
Alisdair McDiarmid	61b2d8e3fe	core: Plan drift includes move-only changes ... Previously, drifted resources included only updates and deletes. To correctly display the full changes which would result as part of a refresh-only apply, the drifted resources must also include move-only changes.	5 years ago
Alisdair McDiarmid	9a7bbdab6f	Fix terraform add test failure due to bad merge	5 years ago
Alisdair McDiarmid	d425c26d77	Merge pull request #29589 from hashicorp/alisdair/planfile-drifted-resources ... core: Compute resource drift during plan phase, store in plan file	5 years ago
Alisdair McDiarmid	1f57b7a8bd	Merge pull request #29235 from magodo/terraform_add_output_append ... `terraform add`: `-out` option append to existing config & optionally check resource existance	5 years ago
Laura Pacilio	dfbef12a6c	Merge pull request #29598 from hashicorp/laura-add-mrui-to-sidebar ... Add Machine-Readable UI to sidebar	5 years ago
Laura Pacilio	a8e5b6a4ad	Fix alphabetical order of sidebar	5 years ago