Merged
Conversation
The cloud package intends to implement a new integration for Terraform Cloud/Enterprise. The purpose of this integration is to better support TFC users; it will shed some overly generic UX and architecture, behavior changes that are otherwise backwards incompatible in the remote backend, and technical debt - all of which are vestiges from before Terraform Cloud existed. This initial commit is largely a porting of the existing 'remote' backend, which will serve as an underlying implementation detail and not be a typical user-level backend. This is because to re-implement the literal backend interface is orthogonal to the purpose of this integration, and can always be migrated away from later. As this backend is considered an implementation detail, it will not be registered as a declarable backend. Within these changes it is, for easy of initial development and a clean diff.
This is a replacement declaration for using Terraform Cloud as a remote backend, leaving the literal backend as an implementation detail and not a user-level concept.
With the alternative block introduced in 7bf9b2c7b, this removes the ability to explicitly declare the 'cloud' backend. The literal backend interface is an implementation detail and no longer a user-level concept when using Terraform Cloud.
This restriction is temporary. Overrides should be allowed, but have the added complexity of needing to also override a 'backend' block, so this work is being deferred for now.
These changes allow cloud blocks to be overridden by backend blocks and vice versa; the logic follows the current backend behavior of a block overriding a preceding block in full, with no merges.
This makes the new backend compatible with the new terraform.Context API, which has changed in main.
I missed adding these in the original porting commit from the remote backend, because *.log is added to gitignore.
These changes include additions to fulfill the interface for the client mock, plus moving all that logic (which needn't be duplicated across both the remote and cloud packages) over to the cloud package under a dedicated mock client file.
A mostly cosemetic change; The fields 'workspace' and 'prefix' don't really describe well what they are from a caller, so change these to use a workspaceMapping struct to convey they are for implementing workspace mapping strategies from CLI -> TFC
"NoDefault" is now ambiguous with tags, and it does not imply using a prefix.
The TFC API doesn't behave this way; a search term just signals the substring was found within the name.
Implementing this test was quite a rabbithole, as in order to satisfy backendTestBackendStates() the workspaces returned from backend.Workspaces() must match exactly, and the shortcut taken to test pagination in 3cc5881 created an impossible circumstance that got plastered over with the fact that prefix filtering is done clientside, not by the API as it should be. Tagging does not rely on clientside filtering, and expects that the request made to the TFC API returns exactly those workspaces with the given tags. These changes include a better way to test pagination, wherein we actually create over a page worth of valid workspaces in the mock client and implement a simplified pagination behavior to match how the TFC API actually works.
This changes the 'name' strategy to always align the local configured workspace name and the remote Terraform Cloud workspace, rather than the implicit use of the 'default' unnamed workspace being used instead. What this essentially means is that the Cloud integration does not fully support workspaces when configured for a single TFC workspace (as was the case with the 'remote' backend), but *does* use the backend.Workspaces() interface to allow for normal local behaviors like terraform.workspace to resolve to the correct name. It does this by always setting the local workspace name when the 'name' strategy is used, as a part of initialization. Part of the diff here is exporting all the previously unexported types for mapping workspaces. The command package (and init in particular) needs to be able to handle setting the local workspace in this particular scenario.
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. This is a port of #29645
…ations This is a port of the changes made in #29683
This is an outdated mechanism that isn't used anymore.
The 'tfe' service was appended to with various versions to denote a new 'feature' implemented by a new 'service'. This quickly proved to not be scalable, as adding an entry to the discovery document from every feature is bad. The new mechanism added was checking the TFP-API-Version header on requests for a version, instead. So we'll remove the separation here between different tfe service 'versions' and the separate 'state' service and Just Use TFE, as well as the TFP-API-Version header for all feature versioning., as well as the TFP-API-Version header for all feature versioning.
These changes remove all of the preexisting version checking for individual features, wiping the slate clean with an overall minimum requirement of a future TFP-API-Version 2.5, which at the time of this writing is expected to be TFE v202112-1. It also actually provides that expected TFE version as an actionable error message, rather than generically saying that it isn't supported or using the somewhat opaque API version header.
If you move from the remote backend to the cloud block you will see this error message.
Previously, if the remote TFC/TFE instance doesn't happen to have a tool_version record whose name exactly matches the value of `tfversion.String()`, Terraform would be completely blocked from using the `terraform workspace new` command (when configured with the tags strategy) — the API would give a 422 to the whole create request. This commit changes the StateMgr() function to do the work in two passes; first create the workspace (which should work fine regardless), THEN update the Terraform version and print a warning to the terminal if it fails (which 99% of the time is a benign failure with little impact on your future CLI usage).
Alas, there's not a very good way to test the message we're supposed to print to the console in this situation; we just don't appear to have a mock terminal that the test can read from. But we can at least test that the function returns without erroring under the exact conditions where it was erroring before. Note that the behaviors of mc.Workspaces.Update and UpdateByID were already starting to drift, so I consolidated their actual attribute update logic into a helper function before they drifted much further.
The delete + assign at the end of `Update` and `UpdateByID` are meant to handle renaming a workspace — (remove old name), (insert new name). However, `UpdateByID` was doing (remove new name), (insert new name) and leaving the old name in place. This commit changes it to match `Update` by grabbing the original name off the workspace object _before_ potentially renaming it.
This aligns more with the existing copy for backends
E2E tests including cost estimation should indeed be added, but the default case should be disabled; lots of cycles lost to pointless cost estimates on null and random resources.
When the 'select the exact version if possible' behavior was added, the version check below it was never updated to take the newly updated version in to account, resulting in a failed version check even as the remote workspace updated to the correct version necessary.
This can help differentiate cloud integration API requests from normal remote backend requests
The previous version is a little overdramatic and somewhat accusatory. Just lay it out how it is: TFC needs workspaces to be named.
Contributor
|
Great! Let's do it. |
chrisarcand
added a commit
that referenced
this pull request
Oct 29, 2021
Th3will
pushed a commit
to Th3will/terraform
that referenced
this pull request
Nov 6, 2021
chrisarcand
added
the
cloud
Contributor
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request adds a more native integration for Terraform Cloud and its CLI-driven run workflow.
Some of the purposes of this integration include...
Enhancements! Besides UX improvements, the Cloud integration includes several new features to the CLI-driven run workflow, including per-run variable support using the
-varflag and the ability to map Terraform Cloud workspaces to the current configuration via Workspace Tags.Improving the overall user experience for Terraform Cloud/Enterprise users with actionable error messages and prompts. Our goal is for users to better resolve errors, and more quickly understand what breaks rather than generic responses that point only to backend issues.
Clarifying remote operations and several naming collisions. The CLI-driven run workflow is currently implemented via the 'remote' backend. This backend has long caused confusion with its 'enhanced' abilities to run operations as well as store and lock Terraform state, when compared to 'standard' backends. It also includes several naming collisions which are difficult to explain (e.g.: Originally - and in some present contexts - 'standard' backends were referred to as 'remote backends').
Addressing technical debt. Like any piece of software, changes over time and the need to retain backwards compatibility mean that some cruft occurs. The Cloud integration cleans up and clears out some previous platform mechanisms that are now unused, as well as addresses long-time difficult issues.
Instead of a backend, users declare a special block in the top-level
terraformsettings block to configure Terraform Cloud, then runterraform init:More information will be available in the documentation for this feature, as we draw nearer to Terraform 1.1!