138 Commits (f-exprstress)

Author	SHA1	Message	Date
Omar Ismail	e9c7f37b8c	Enable --auto-approve for Policy checks on Remote Backend (Terraform Cloud) (#27804) ... * Fix auto-approve for soft-policy * Update error handling * update testing string for consistency	5 years ago
Alisdair McDiarmid	7c0ec0107f	backend: Replace ShowDiagnostics with view.Diagnostics	5 years ago
Alisdair McDiarmid	02eb283ad4	backend/remote: Fix broken state lock retry ... When using the -lock-timeout option with the remote backend configured in local operations mode, Terraform would fail to retry acquiring the lock. This was caused by the lock error message having a missing Info field, which the state manager requires to be present in order to attempt retries.	5 years ago
Alisdair McDiarmid	68558ccd54	backend/local: Replace CLI with view instance ... This commit extracts the remaining UI logic from the local backend, and removes access to the direct CLI output. This is replaced with an instance of a `views.Operation` interface, which codifies the current requirements for the local backend to interact with the user. The exception to this at present is interactivity: approving a plan still depends on the `UIIn` field for the backend. This is out of scope for this commit and can be revisited separately, at which time the `UIOut` field can also be removed. Changes in support of this: - Some instances of direct error output have been replaced with diagnostics, most notably in the emergency state backup handler. This requires reformatting the error messages to allow the diagnostic renderer to line-wrap them; - The "in-automation" logic has moved out of the backend and into the view implementation; - The plan, apply, refresh, and import commands instantiate a view and set it on the `backend.Operation` struct, as these are the only code paths which call the `local.Operation()` method that requires it; - The show command requires the plan rendering code which is now in the views package, so there is a stub implementation of a `views.Show` interface there. Other refactoring work in support of migrating these commands to the common views code structure will come in follow-up PRs, at which point we will be able to remove the UI instances from the unit tests for those commands.	5 years ago
Alisdair McDiarmid	2d1976bbda	clistate: Update clistate.Locker for command views ... The clistate package includes a Locker interface which provides a simple way for the local backend to lock and unlock state, while providing feedback to the user if there is a delay while waiting for the lock. Prior to this commit, the backend was responsible for initializing the Locker, passing through direct access to the cli.Ui instance. This structure prevented commands from implementing different implementations of the state locker UI. In this commit, we: - Move the responsibility of creating the appropriate Locker to the source of the Operation; - Add the ability to set the context for a Locker via a WithContext method; - Replace the Locker's cli.Ui and Colorize members with a StateLocker view; - Implement views.StateLocker for human-readable UI; - Update the Locker interface to return detailed diagnostics instead of errors, reducing its direct interactions with UI; - Add a Timeout() method on Locker to allow the remote backend to continue to misuse the -lock-timeout flag to cancel pending runs. When an Operation is created, the StateLocker field must now be populated with an implementation of Locker. For situations where locking is disabled, this can be a no-op locker. This change has no significant effect on the operation of Terraform, with the exception of slightly different formatting of errors when state locking or unlocking fails.	5 years ago
Alisdair McDiarmid	536c80da23	backend: Add per-operation diagnostic rendering ... The enhanced backends (local and remote) need to be able to render diagnostics during operations. Prior to this commit, this functionality was supported with a per-backend `ShowDiagnostics` function pointer. In order to allow users of these backends to control how diagnostics are rendered, this commit moves that function pointer to the `Operation` type. This means that a diagnostic renderer is configured for each operation, rather than once per backend initialization. Some secondary consequences of this change: - The `ReportResult` method on the backend is now moved to the `Operation` type, as it needs to access the `ShowDiagnostics` callback (and nothing else from the backend); - Tests which assumed that diagnostics would be written to the backend's `cli.Ui` instance are migrated to using a new record/playback diags helper function; - Apply, plan, and refresh commands now pass a pointer to the `Meta` struct's `showDiagnostics` method. This commit should not change how Terraform works, and is refactoring in preparation for more changes which move UI code out of the backend.	5 years ago
Omar Ismail	4ac095b63f	only print horizontal rule at first attempt	5 years ago
Omar Ismail	d96c90b6af	Cost Estimation CLI output unnecessary	5 years ago
Alisdair McDiarmid	b2ba650c21	cli: Remove deprecated destroy -force flag ... This dramatically simplifies the logic around auto-approve, which is nice. Also add test coverage for the manual approve step, for both apply and destroy, answering both yes and no.	5 years ago
Alisdair McDiarmid	5ca118b4e6	cli: Move resource count code to command package ... CountHook is an implementation of terraform.Hook which is used to calculate how many resources were added, changed, or destroyed during an apply. This hook was previously injected in the local backend code, which means that the apply command code has no access to these counts. This commit moves the CountHook code into the command package, and removes an unused instance of the hook in the plan code path. The goal here is moving UI code into the command package.	5 years ago
James Bardin	3bc7d77230	update MockProvider usage	5 years ago
Alisdair McDiarmid	7e09cd1228	backend/remote: No version check for local ops ... If the remote backend is connected to a Terraform Cloud workspace in local operations mode, we disable the version check, as the remote Terraform version is meaningless.	5 years ago
Alisdair McDiarmid	619c6727ef	backend/remote: No version check for operations ... Terraform remote version conflicts are not a concern for operations. We are in one of three states: - Running remotely, in which case the local version is irrelevant; - Workspace configured for local operations, in which case the remote version is meaningless; - Forcing local operations with a remote backend, which should only happen in the Terraform Cloud worker, in which case the Terraform versions by definition match. This commit therefore disables the version check for operations (plan and apply), which has the consequence of disabling it in Terraform Cloud and Enterprise runs. In turn this enables Terraform Enterprise runs with bundles which have a version that doesn't exactly match the bundled Terraform version.	5 years ago
Alisdair McDiarmid	6e0d8cde91	backend/remote: Fix for "latest" workspace version ... Terraform Cloud/Enterprise support a pseudo-version of "latest" for the configured workspace Terraform version. If this is chosen, we abandon the attempt to verify the versions are compatible, as the meaning of "latest" cannot be predicted. This affects both the StateMgr check (used for commands which execute remotely) and the full version check (for local commands).	6 years ago
James Bardin	96436f526a	unused err value and nil context ... Incorrect err assignment had no effect. Never use a nil context.	6 years ago
Alisdair McDiarmid	c5c1f31db3	backend: Validate remote backend Terraform version ... When using the enhanced remote backend, a subset of all Terraform operations are supported. Of these, only plan and apply can be executed on the remote infrastructure (e.g. Terraform Cloud). Other operations run locally and use the remote backend for state storage. This causes problems when the local version of Terraform does not match the configured version from the remote workspace. If the two versions are incompatible, an `import` or `state mv` operation can cause the remote workspace to be unusable until a manual fix is applied. To prevent this from happening accidentally, this commit introduces a check that the local Terraform version and the configured remote workspace Terraform version are compatible. This check is skipped for commands which do not write state, and can also be disabled by the use of a new command-line flag, `-ignore-remote-version`. Terraform version compatibility is defined as: - For all releases before 0.14.0, local must exactly equal remote, as two different versions cannot share state; - 0.14.0 to 1.0.x are compatible, as we will not change the state version number until at least Terraform 1.1.0; - Versions after 1.1.0 must have the same major and minor versions, as we will not change the state version number in a patch release. If the two versions are incompatible, a diagnostic is displayed, advising that the error can be suppressed with `-ignore-remote-version`. When this flag is used, the diagnostic is still displayed, but as a warning instead of an error. Commands which will not write state can assert this fact by calling the helper `meta.ignoreRemoteBackendVersionConflict`, which will disable the checks. Those which can write state should instead call the helper `meta.remoteBackendVersionCheck`, which will return diagnostics for display. In addition to these explicit paths for managing the version check, we have an implicit check in the remote backend's state manager initialization method. Both of the above helpers will disable this check. This fallback is in place to ensure that future code paths which access state cannot accidentally skip the remote version check.	6 years ago
Alisdair McDiarmid	eee57280f6	backend: Faster remote backend tests ... The remote backend tests spent most of their execution time sleeping in various polling and backoff waits. This is unnecessary when testing against a mock server, so reduce all of these delays when under test to much lower values. Only one remaining test has an artificial delay: verifying the discovery of services against an unknown hostname. This times out at DNS resolution, which is more difficult to fix than seems worth it at this time.	6 years ago
Paul Thrasher	fc8a76a2a3	Stop printing tfce error message log	6 years ago
Paul Thrasher	960042ad42	Merge pull request #26618 from hashicorp/pault/tfce-continue-on-error ... Allow runs to continue on tfce error	6 years ago
James Bardin	0b31ffa587	use a single log writer ... Use a single log writer instance for all std library logging. Setup the std log writer in the logging package, and remove boilerplate from test packages.	6 years ago
James Bardin	6ca477f042	move helper/logging to internal ... remove a dead code file too	6 years ago
Paul Thrasher	d380f5873c	Allow runs to continue on tfce error ... A cost estimation error does not actually stop a run, so the run was continuing in the background after the cli exits, causing confusion. This change matches the UI behavior.	6 years ago
James Bardin	5e9425b562	unreachable	6 years ago
Kristin Laemmert	d76cfc8c0c	Merge pull request #26440 from hashicorp/mildwonkey/remove-older-state-vs ... refactor tests to use modern states.State in favor of terraform.State where possible	6 years ago
Kristin Laemmert	479655ad47	refactor tests to use modern states.State in favor of terraform.State where possible	6 years ago
James Bardin	ab6d6f99ae	fix races in remote backend mock	6 years ago
Kristin Laemmert	6621501ae3	state: remove deprecated state package (#25490) ... Most of the state package has been deprecated by the states package. This PR replaces all the references to the old state package that can be done simply - the low-hanging fruit. * states: move state.Locker to statemgr The state.Locker interface was a wrapper around a statemgr.Full, so moving this was relatively straightforward. * command: remove unnecessary use of state package for writing local terraform state files * move state.LocalState into terraform package state.LocalState is responsible for managing terraform.States, so it made sense (to me) to move it into the terraform package. * slight change of heart: move state.LocalState into clistate instead of terraform	6 years ago
Kristin Laemmert	86e9ba3d65	* backend/local: push responsibility for unlocking state into individual operations ... * unlock the state if Context() has an error, exactly as backend/remote does today * terraform console and terraform import will exit before unlocking state in case of error in Context() * responsibility for unlocking state in the local backend is pushed down the stack, out of backend.go and into each individual state operation * add tests confirming that state is not locked after apply and plan * backend/local: add checks that the state is unlocked after operations This adds tests to plan, apply and refresh which validate that the state is unlocked after all operations, regardless of exit status. I've also added specific tests that force Context() to fail during each operation to verify that locking behavior specifically.	6 years ago
Kristin Laemmert	604e65bb62	Revert "backend/local: release lock if there is an error in Context() (#25427)" ... This reverts commit 1ba0d615e7.	6 years ago
Kristin Laemmert	1ba0d615e7	backend/local: release lock if there is an error in Context() (#25427) ... * command/console: return in case of errors before trying to unlock remote state The remote backend `Context` would exit without an active lock if there was an error, while the local backend `Context` exited *with* a lock. This caused a problem in `terraform console`, which would call unlock regardless of error status. This commit makes the local and remote backend consistently unlock the state incase of error, and updates terraform console to check for errors before trying to unlock the state. * adding tests for remote and local backends	6 years ago
James Bardin	35d232481f	prevent panic in remote backend retry ... Ensure that the *http.Response is not nil before checking the status.	6 years ago
Kristin Laemmert	e6cf6cd758	backend/remote: do not panic if PrepareConfig or Configure receive null (#25135) ... * backend/remote: do not panic if PrepareConfig or Configure receive null objects If a user cancels (ctrl-c) terraform init while it is requesting missing configuration options for the remote backend, the PrepareConfig and Configure functions would receive a null cty.Value which would result in panics. This PR adds a check for null objects to the two functions in question. Fixes #23992	6 years ago
CJ Horton	e1dcae17b7	add sad path tests for the TFP API version check	6 years ago
CJ Horton	68f199fc76	replace status string with go-tfe enum	6 years ago
Martin Atkins	8e1615a802	backend/remote: Handle cost estimation skipped due to targeting ... The remote server might choose to skip running cost estimation for a targeted plan, in which case we'll show a note about it in the UI and then move on, rather than returning an "invalid status" error. This new status isn't yet available in the go-tfe library as a constant, so for now we have the string directly in our switch statement. This is a pragmatic way to expedite getting the "critical path" of this feature in place without blocking on changes to ancillary codebases. A subsequent commit should switch this over to tfe.CostEstimateSkippedDueToTargeting once that's available in a go-tfe release.	6 years ago
CJ Horton	0eea4e7c62	prevent targeting for unsupported API versions	6 years ago
Mike Nomitch	650a272a1d	Change message sent to remote backend if -target used	6 years ago
Martin Atkins	16f1f3b739	backend/remote: Support -target on plan and apply ... Previously we did not allow -target to be used with the remote backend because there was no way to send the targets to Terraform Cloud/Enterprise via the API. There is now an attribute in the request for creating a plan that allows us to send target addresses, so we'll remove that restriction and copy the given target addresses into the API request.	6 years ago
Martin Atkins	db4f3f8bc5	vendor: go get github.com/hashicorp/go-tfe@v0.8.0 ... This includes a new TargetAddrs field on both Run and RunCreateOptions which we'll use to send resource addresses that were specified using -target on the CLI command line when using the remote backend. There were some unrelated upstream breaking changes compared to the last version we had vendored, so this commit also includes some changes to the backend/remote package to work with this new API, which now requires the remote backend to be aware of the remote system's opaque workspace id.	6 years ago
Pam Selle	60b3815af4	Merge pull request #24696 from hashicorp/leetrout/remote-state-force-push ... Add support for force pushing with the remote backend	6 years ago
Lee Trout	cb0e20ca2b	Add support for force pushing with the remote backend ... Both differing serials and lineage protections should be bypassed with the -force flag (in addition to resources). Compared to other backends we aren’t just shipping over the state bytes in a simple payload during the persistence phase of the push command and the force flag added to the Go TFE client needs to be specified at that time. To prevent changing every method signature of PersistState of the remote client I added an optional interface that provides a hook to flag the Client as operating in a force push context. Changing the method signature would be more explicit at the cost of not being used anywhere else currently or the optional interface pattern could be applied to the state itself so it could be upgraded to support PersistState(force bool) only when needed. Prior to this only the resources of the state were checked for changes not the lineage or the serial. To bring this in line with documented behavior noted above those attributes also have a “read” counterpart just like state has. These are now checked along with state to determine if the state as a whole is unchanged. Tests were altered to table driven test format and testing was expanded to include WriteStateForMigration and its interaction with a ClientForcePusher type.	6 years ago
Kritonas	71395a0656	Fixed minor grammatical errors	6 years ago
Ryan Uber	105fcb3cac	backend/remote: move reading policy check logs earlier	6 years ago
Ryan Uber	93906e1ad8	backend/remote: display cost estimate and policy check whenever available	6 years ago
Alisdair McDiarmid	f15e58adf8	Improve remote backend missing token error ... Prompt the user to run terraform login to generate and store a token for the configured remote backend.	6 years ago
Robert Tillery	af77d1d22c	backend/remote: Filter environment variables when loading context (#23358) ... * backend/remote: Filter environment variables when loading context Following up on #23122, the remote system (Terraform Cloud or Enterprise) serves environment and Terraform variables using a single type of object. We only should load Terraform variables into the Terraform context. Fixes https://github.com/hashicorp/terraform/issues/23283.	7 years ago
Martin Atkins	8f27409007	backend/remote: Support HCL variable values in local operations ... For remote operations, the remote system (Terraform Cloud or Enterprise) writes the stored variable values into a .tfvars file before running the remote copy of Terraform CLI. By contrast, for operations that only run locally (like "terraform import"), we fetch the stored variable values from the remote API and add them into the set of available variables directly as part of creating the local execution context. Previously in the local-only case we were assuming that all stored variables are strings, which isn't true: the Terraform Cloud/Enterprise UI allows users to specify that a particular variable is given as an HCL expression, in which case the correct behavior is to parse and evaluate the expression to obtain the final value. This also addresses a related issue whereby previously we were forcing all sensitive values to be represented as a special string "<sensitive>". That leads to type checking errors for any variable specified as having a type other than string, so instead here we use an unknown value as a placeholder so that type checking can pass. Unpopulated sensitive values may cause errors downstream though, so we'll also produce a warning for each of them to let the user know that those variables are not available for local-only operations. It's a warning rather than an error so that operations that don't rely on known values for those variables can potentially complete successfully. This can potentially produce errors in situations that would've been silently ignored before: if a remote variable is marked as being HCL syntax but is not valid HCL then it will now fail parsing at this early stage, whereas previously it would've just passed through as a string and failed only if the operation tried to interpret it as a non-string. However, in situations like these the remote operations like "terraform plan" would already have been failing with an equivalent error message anyway, so it's unlikely that any existing workspace that is being used for routine operations would have such a broken configuration.	7 years ago
Martin Atkins	9f9f22091e	backend/remote: Unlock workspace if Context fails ... Previously any error case in the Context method would cause us to leave the remote workspace locked on exit, requiring manual action to unlock it.	7 years ago
Martin Atkins	b10f058cbb	backend/remote: Only load variables if we're going to use them ... Some commands don't use variables at all or use them in a way that doesn't require them to all be fully valid and consistent. For those, we don't want to fetch variable values from the remote system and try to validate them because that's wasteful and likely to cause unnecessary error messages. Furthermore, the variables endpoint in Terraform Cloud and Enterprise only works for personal access tokens, so it's important that we don't assume we can _always_ use it. If we do, then we'll see problems when commands are run inside Terraform Cloud and Enterprise remote execution contexts, where the variables map always comes back as empty.	7 years ago
Martin Atkins	a8d01e3940	backend/remote: Report invalid variables only remotely ... The remote backend uses backend.ParseVariableValues locally only to decide if the user seems to be trying to use -var or -var-file options locally, since those are not supported for the remote backend. Other than detecting those, we don't actually have any need to use the results of backend.ParseVariableValues, and so it's better for us to ignore any errors it produces itself and prefer to just send a potentially-invalid request to the remote system and let the remote system be responsible for validating it. This then avoids issues caused by the fact that when remote operations are in use the local system does not have all of the required context: it can't see which environment variables will be set in the remote execution context nor which variables the remote system will set using its own generated -var-file based on the workspace stored variables.	7 years ago