Re-integrate the "terraform" provider into the main binary #16543
Conversation
This file contains 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
This commit adds the groundwork for supporting module outputs of types
other than string. In order to do so, the state version is increased
from 1 to 2 (though the "public-facing" state version is actually as the
first state file was binary).
Tests are added to ensure that V2 (1) state is upgraded to V3 (2) state,
though no separate read path is required since the V2 JSON will
unmarshal correctly into the V3 structure.
Outputs in a ModuleState are now of type map[string]interface{}, and a
test covers round-tripping string, []string and map[string]string, which
should cover all of the types in question.
Type switches have been added where necessary to deal with the
interface{} value, but they currently default to panicking when the input
is not a string.
As a first example of a real-world data source, the pre-existing terraform_remote_state resource is adapted to be a data source. The original resource is shimmed to wrap the data source for backward compatibility.
For backward compatibility we will continue to support using the data sources that were formerly logical resources as resources for the moment, but we want to warn the user about it since this support is likely to be removed in future. This is done by adding a new "deprecation message" feature to schema.Resource, but for the moment this is done as an internal feature (not usable directly by plugins) so that we can collect additional use-cases and design a more general interface before creating a compatibility constraint.
As a first example of a real-world data source, the pre-existing terraform_remote_state resource is adapted to be a data source. The original resource is shimmed to wrap the data source for backward compatibility.
This commit forward ports the changes made for 0.6.17, in order to store the type and sensitive flag against outputs. It also refactors the logic of the import for V0 to V1 state, and fixes up the call sites of the new format for outputs in V2 state. Finally we fix up tests which did not previously set a state version where one is required.
The work integrated in #6322 silently broke the ability to use remote state correctly. This commit adds a fix for that, making use of the work integrated in #7124. In order to deal with outputs which are complex structures, we use a forked version of the flatmap package - the difference in the version this commit vs the github.com/hashicorp/terraform/flatmap package is that we add in an additional key for map counts which state requires. Because we bypass the normal helper/schema mechanism, this is not set for us. Because of the HIL type checking of maps, values must be of a homogenous type. This is unfortunate, as it means we can no longer refer to outputs as: ${terraform_remote_state.foo.output.outputname} Instead we had to bring them to the top level namespace: ${terraform_remote_state.foo.outputname} This actually does lead to better overall usability - and the BC breakage is made better by the fact that indexing would have broken the original syntax anyway. We also add a real-world test and assert against specific values. Tests which were previously acceptance tests are now run as unit tests, so regression should be identified at a much earlier stage.
I noticed we had two mechanisms for unit test override. One that dropped a sentinel into the env var, and another with a struct member on TestCase. This consolidates the two, using the cleaner struct member internal mechanism and the nicer `resource.UnitTest()` entry point.
- Check for an empty state. If nothing is referenced from that state in the config, there's nothing to do here.
The template resources don't actually need to retain any state, so they are good candidates to be data sources. This includes a few tweaks to the acceptance tests -- now configured to run as unit tests -- since it seems that they have been slightly broken for a while now. In particular, the "update" cases are no longer tested because updating is not a meaningful operation for a data source.
Fixing minor typo to match documentation on https://www.terraform.io/docs/providers/terraform/d/remote_state.html
This is a rework of pull request #6213 submitted by @joshuaspence, adjusted to work with the remote state data source. We also add a deprecation warning for people using the unsupported API, and retain the ability to refer to "_local" as well as "local" for users in a mixed version environment.
* docs/vsphere: Fix code block * docs: Convert `...` to `# ...` to allow `terraform fmt`ing * docs: Trim trailing whitespace * docs: First-pass run of `terraform fmt` on code examples
Add an `environment` field to the terraform remote state data source.
The existence of the attribute is mentioned here already: https://www.terraform.io/docs/state/environments.html
vendor: github.com/hashicorp/terraform/...@v0.10.0
vendor: Remove refs to removed packages
This allows providing default values for keys that might not be set in the retrieved state.
The "terraform" provider was previously split out into its own repository, but that turned out to be a mistake due to how tightly it depends on aspects of Terraform Core. Here we prepare to bring it back into the core repository by reorganizing the directory layout to conform with what's expected there.
|
(the history of the Terraform provider is re-imported in here so we don't flatten it away, which unfortunately causes its entire -- thankfully pretty short! -- history to be listed as the commits for this PR above.) |
|
I have some more work to do here to test the new internal provider exceptions in the provider resolver, but since I ran out of time today I thought I'd share what I have so far to allow review of the general approach. (The tests will be pretty uninteresting, I expect.) |
|
I like the idea of this running in-process too, which also might make trying out pluggable backends easier. |
As part of the 0.10 core/provider split we moved this provider, along with all the others, out into its own repository. In retrospect, the "terraform" provider doesn't really make sense to be separated since it's just a thin wrapper around some core code anyway, and so re-integrating it into core avoids the confusion that results when Terraform Core and the terraform provider have inconsistent versions of the backend code and dependencies. There is no good reason to use a different version of the backend code in the provider than in core, so this new "internal provider" mechanism is stricter than the old one: it's not possible to use an external build of this provider at all, and version constraints for it are rejected as a result. This provider is also run in-process rather than in a child process, since again it's just a very thin wrapper around code that's already running in Terraform core anyway, and so the process barrier between the two does not create enough advantage to warrant the additional complexity.
apparentlymart
force-pushed
the
f-terraform-provider-internal
branch
from
ec901d9
to
d4ee58c
Compare
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
As part of the 0.10 core/provider split we moved this provider, along with all the others, out into its own repository.
In retrospect, the
terraformprovider doesn't really make sense to be separated since it's just a thin wrapper around some core code anyway, and so re-integrating it into core avoids the confusion that results when Terraform Core and theterraformprovider have inconsistent versions of the backend code and dependencies.There is no good reason to use a different version of the backend code in the provider than in core, so this new "internal provider" mechanism is stricter than the old one: it's not possible to use an external build of this provider at all, and version constraints for it are rejected as a result.
This provider is also run in-process rather than in a child process, since again it's just a very thin wrapper around code that's already running in Terraform core anyway, and so the process barrier between the two does not create enough advantage to warrant the additional complexity.