-
Notifications
You must be signed in to change notification settings - Fork 9.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
0.10 Core/Provider split work #15208
Merged
Merged
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
apparentlymart
force-pushed
the
0.10-plugins
branch
from
June 9, 2017 21:02
38c885b
to
5627b4d
Compare
With forthcoming support for versioned plugins we need to be able to answer questions like what versions of plugins are currently installed, what's the newest version of a given plugin available, etc. PluginMetaSet gives us a building block for this sort of plugin version wrangling.
For now this supports both our old and new directory layouts, so we can preserve compatibility with existing configurations until a future major release where support for the old paths will be removed. Currently this allows both styles in all given directories, which means we support more variants than we actually intend to but this is accepted to keep the interface simple such that we can easily remove the legacy exception later. The documentation will reflect only the subset of path layouts that we actually intend to support.
This will be used later to verify that installed plugins match what's available on the releases server.
These new methods ClientConfig and Client provide the bridge into the main plugin infrastructure by configuring and instantiating (respectively) a client object for the referenced plugin. This stops short of getting the proxy object from the client since that then requires referencing the interface for the plugin kind, which would then create a dependency on the main terraform package which we'd rather avoid here. It'll be the responsibility of the caller in the command package to do the final wiring to get a provider instance out of a provider plugin client.
The .terraformrc file allows the user to override the executable location for certain plugins. This mechanism allows us to retain that behavior for a deprecation period by treating such executables as an unversioned plugin for the given name and excluding all other candidates for that name, thus ensuring that the override will "win". Users must eventually transition away from using this mechanism and use vendor directories instead, because these unversioned overrides will never match for a provider referenced with non-zero version constraints.
Previously we did plugin discovery in the main package, but as we move towards versioned plugins we need more information available in order to resolve plugins, so we move this responsibility into the command package itself. For the moment this is just preserving the existing behavior as long as there are only internal and unversioned plugins present. This is the final state for provisioners in 0.10, since we don't want to support versioned provisioners yet. For providers this is just a checkpoint along the way, since further work is required to apply version constraints from configuration and support additional plugin search directories. The automatic plugin discovery behavior is not desirable for tests because we want to mock the plugins there, so we add a new backdoor for the tests to use to skip the plugin discovery and just provide their own mock implementations. Most of this diff is thus noisy rework of the tests to use this new mechanism.
In future we will support version constraints on providers, so we're reserving this attribute name that is currently not used by any builtin providers. For now using this will produce an error, since the rest of Terraform (outside of the config parser) doesn't currently have this notion and we don't want people to start trying to use it until its behavior is fully defined and implemented. It may be used by third-party providers, so this is a breaking change worth warning about in CHANGELOG but one whose impact should be small. Any third-party providers using this name should migrate to using a new attribute name instead moving forward.
Having this as a method of PluginMeta felt most natural, but unfortunately that means that discovery must depend on plugin and plugin in turn depends on core Terraform, thus making the discovery package hard to use without creating dependency cycles. To resolve this, we invert the dependency and make the plugin package be responsible for instantiating clients given a meta, using a top-level function.
We now accept syntactically-valid version constraints on provider blocks, though we still don't actually do anything with them.
Previously the logic for inferring a provider type from a resource name was buried a utility function in the 'terraform' package. Instead here we lift it up into the 'config' package where we can make broader use of it and where it's easier to discover.
The semver library we were using doesn't have support for a "pessimistic constraint" where e.g. the user wants to accept only minor or patch version upgrades. This is important for providers since users will generally want to pin their dependencies to not inadvertantly accept breaking changes. So here we switch to hashicorp's home-grown go-version library, which has the ~> constraint operator for this sort of constraint. Given how much the old version object was already intruding into the interface and creating dependency noise in callers, this also now wraps the "raw" go-version objects in package-local structs, thus keeping the details encapsulated and allowing callers to deal just with this package's own types.
As we add support for versioned providers, it's getting more complex to track the dependencies of each module and of the configuration as a whole, so this new package is intended to give us some room to model that nicely as a building block for the various aspects of dependency management. This package is not responsible for *building* the dependency data structure, since that requires knowledge of core Terraform and that would create cyclic package dependencies. A later change will add some logic in Terraform to create a Module tree based on the combination of a given configuration and state, returning an instance of this package's Module type. The Module.PluginRequirements method flattens the provider-oriented requirements into a set of plugin-oriented requirements (flattening any provider aliases) giving us what we need to work with the plugin/discovery package to find matching installed plugins. Other later uses of this package will include selecting plugin archives to auto-install from releases.hashicorp.com as part of "terraform init", where the module-oriented level of abstraction here should be useful for giving users good, specific feedback when constraints cannot be met. A "reason" is tracked for each provider dependency with the intent that this would later drive a UI for users to see and understand why a given dependency is present, to aid in debugging sticky issues with dependency resolution.
Various implied dependencies will accept all versions, so this is convenient for populating dependency data structures for such implied dependencies.
Compares two modules for deep equality. This is primarily provided to enable easy testing of code that constructs module tree structures.
This new private function takes a configuration tree and a state structure and finds all of the explicit and implied provider dependencies represented, returning them as a moduledeps.Module tree structure. It annotates each dependency with a "reason", which is intended to be useful to a user trying to figure out where a particular dependency is coming from, though we don't yet have any UI to view this. Nothing calls this yet, but a subsequent commit will use the result of this to produce a constraint-conforming map of provider factories during context initialization.
Previously the Type of a ResourceState was generally ignored, but we're now starting to use it to figure out which providers are needed to support the resources in state so our tests need to set it accurately in order to get the expected result.
The previous error was very generic, making it hard to quickly tell from the test output that the error was during context initialization.
Previously having a config was mutually exclusive with running an import, but we need to provide a config so that the provider is declared, or else we can't actually complete the import in the future world where providers are installed dynamically based on their declarations.
Currently this doesn't matter much, but we're about to start checking the availability of providers early on and so we need to use the correct name for the mock set of providers we use in command tests, which includes only a provider named "test". Without this change, the "push" tests will begin failing once we start verifying this, since there's no "aws" provider available in the test context.
ResourceProviderResolver is an extra level of indirection before we get to a map[string]ResourceProviderFactory, which accepts a map of version constraints and uses it to choose from potentially-many available versions of each provider to produce a single ResourceProviderFactory for each one requested. As of this commit the ResourceProviderResolver interface is not used. In a future commit the ContextOpts.Providers map will be replaced with a resolver instance, with the creation of the factory delayed until the version constraints have been resolved.
Previously the set of providers was fixed early on in the command package processing. In order to be version-aware we need to defer this work until later, so this interface exists so we can hold on to the possibly-many versions of plugins we have available and then later, once we've finished determining the provider dependencies, select the appropriate version of each provider to produce the final set of providers to use. This commit establishes the use of this new mechanism, and thus populates the provider factory map with only the providers that result from the dependency resolution process. This disables support for internal provider plugins, though the mechanisms for building and launching these are still here vestigially, to be cleaned up in a subsequent commit. This also adds a new awkward quirk to the "terraform import" workflow where one can't import a resource from a provider that isn't already mentioned (implicitly or explicitly) in config. We will do some UX work in subsequent commits to make this behavior better. This breaks many tests due to the change in interface, but to keep this particular diff reasonably easy to read the test fixes are split into a separate commit.
Rather than providing an already-resolved map of plugins to core, we now provide a "provider resolver" which knows how to resolve a set of provider dependencies, to be determined later, and produce that map. This requires the context to be instantiated in a different way, so this very noisy diff is a mostly-mechanical update of all of the existing places where contexts get created for testing, using some adapted versions of the pre-existing utilities for passing in mock providers.
We're going to use config to determine provider dependencies, so we need to always provide a config when instantiating a context or we'll end up loading no providers at all. We previously had a test for running "terraform import -config=''" to disable the config entirely, but this test is now removed because it makes no sense. The actual functionality its testing still remains for now, but it will be removed in a subsequent commit when we start requiring that a resource to be imported must already exist in configuration.
This is a generally-useful utility for computing dependency trees, so no reason to restrict it to just the terraform package.
We can filter the allowed versions and sort them before checking the protocol version, that way we can just return the first one found reducing network requests.
Previously we were using the "semver" library to parse version constraints, but we switched over to go-version and encapsulated it inside our own plugin/discovery package to reduce dependency sprawl in the code. This particular situation was missed when updating references to the new path, which meant that our validation code disagreed with the rest of the code about what is considered a valid version constraint string. By using the correct function, we ensure that we catch early any invalid versions.
When running "terraform init" with providers that are unconstrained, we will now produce information to help the user update configuration to constrain for the particular providers that were chosen, to prevent inadvertently drifting onto a newer major release that might contain breaking changes. A ~> constraint is used here because pinning to a single specific version is expected to create dependency hell when using child modules. By using this constraint mode, which allows minor version upgrades, we avoid the need for users to constantly adjust version constraints across many modules, but make major version upgrades still be opt-in. Any constraint at all in the configuration will prevent the display of these suggestions, so users are free to use stronger or weaker constraints if desired, ignoring the recommendation.
The expected type was changed in the mainline code but the tests were not updated to match.
This was added with the idea of using it to override the SHA256 hashes to match those hypothetically stored in a plan, but we already have a mechanism elsewhere for populating context fields from plan fields, so this is not actually necessary.
The information stored in a plan is tightly coupled to the Terraform core and provider plugins that were used to create it, since we have no mechanism to "upgrade" a plan to reflect schema changes and so mismatching versions are likely to lead to the "diffs didn't match during apply" error. To allow us to catch this early and return an error message that _doesn't_ say it's a bug in Terraform, we'll remember the Terraform version and plugin binaries that created a particular plan and then require that those match when loading the plan in order to apply it. The planFormatVersion is increased here so that plan files produced by earlier Terraform versions _without_ this information won't be accepted by this new version, and also that older versions won't try to process plans created by newer versions.
Each provider plugin will take at least a few seconds to download, so providing feedback about each one should make users feel less like Terraform has hung. Ideally we'd show ongoing progress during the download, but that's not possible without re-working go-getter, so we'll accept this as an interim solution for now.
It's now required to run "terraform init" to get the provider plugins installed, so we need to mention that in the intro guide.
This form of "terraform init" is vestigial at this point and being phased out in 0.10. Something similar may return in a later version for installing modules from a more formal module library, but for now we are advising to use git manually to simplify the UX for "terraform init".
"environment" is a very overloaded term, so here we prefer to use the term "working directory" to talk about a local directory where operations are executed on a given Terraform configuration.
Further iteration of this will undoubtedly be needed before the final release, but this is a start.
ConstrainVersions was documented as returning nil, but it was instead returning an empty set. Use the Count() method to check for nil or empty. Add test to verify failed constraints will show up as missing.
Provide log-form message when a provider isn't found, along with the desired constraints.
Last minute change to the location of the binaries
Names are case-insensitive, using lowercase by default.
It might not just be for providers, and it's in the plugins dir, so lock.json seems descriptive enough.
apparentlymart
force-pushed
the
0.10-plugins
branch
from
June 9, 2017 21:04
38c885b
to
d1c50ef
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
Apr 11, 2020
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.
This stuff was already reviewed as it went into the 0.10-plugins branch, so this PR is mostly just a formality, but going to create this to let Travis run the tests and give us one more chance to look over it and make sure everything looks sensible.
We're going to playtest this a bit before merging, once the split provider releases have reached a good state and we're able to end-to-end run this.