Skip to content
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 82 commits into from Jun 9, 2017
Merged

0.10 Core/Provider split work #15208

merged 82 commits into from Jun 9, 2017

Conversation

apparentlymart
Copy link
Member

@apparentlymart apparentlymart commented Jun 9, 2017

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.

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.
jbardin and others added 22 commits June 9, 2017 14:03
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.
@ghost
Copy link

ghost commented Apr 11, 2020

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.

@hashicorp hashicorp 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants