* Add new `AssertNoDiagnostics` and `AssertDiagnosticCount` test helpers in tfdiags package
* Add new `AssertNoErrors` and `LogDiagnostics` test helpers in tfdiags package
* Refactor existing tests to use new helpers
* Refactor more recent tests
* Revert change - wrong function replaced
* Refactor to account for function no longer returning a boolean
* Fix comment
We've been gradually chipping away at how much we use go-getter for source
packages, because it's generally been a bit of a nightmare and sharing it
with other codebases means that any time someone wants to change something
we end up needing to find some way to prevent it breaking Terraform's
compatibility promises.
Here we make one further step: Terraform owns the "detectors" idea that
deals with source address normalization, and now always produces
fully-qualified addresses for go-getter to chew on only for the getting
and decompressing steps.
Retaining go-getter for the actual getting part is helpful because we can
then benefit from security fixes upstream, but Terraform owning the first
layer of parsing means that we can fix in place the definition of what
"module source address" syntax means, and thus we can avoid having
everything in this codebase indirectly depend on go-getter just because it
wants to parse module source addresses.
Now only the module installer actually depends indirectly on go-getter,
which finally disconnects go-getter's subtree from all of the remote state
backend dependency graphs.
Our package addrs ends up getting imported from just about ever other
package in Terraform, because it contains the types we use to talk about
various different kinds of objects. Therefore we typically try to keep its
transitive dependency graph relatively small, because anything it depends
on becomes an indirect dependency of nearly everything else.
A while back we moved the module source address models into package addrs,
which also brought with them the code for parsing strings to produce those
addresses. Unfortunately, remote module source addresses are defined using
the external dependency go-getter, which is pretty heavy itself and also
brings with it numerous other external dependencies such as the AWS SDK,
the Google Cloud Platform SDK, etc.
Since only relatively few packages actually need to _parse_ source
addresses -- with most either not caring about them at all or only
consuming addresses that were already parsed by someone else -- we'll
move the parser functions into their own package, while keeping the
resulting address types in package addrs.
This does still retain the package addrs dependency on external module
github.com/hashicorp/terraform-registry-address, which is not ideal but
that one at least has a relatively shallow dependency subgraph, so there's
not so much urgency to tidy that one.
We install remote modules prior to showing any validation errors during init
so that we can show errors about the core version requirement before we do
anything else. Unfortunately, this means that we don't validate module names
until after remote modules have been installed, which may cause unexpected
problems if we can't convert the module name into a valid path.
When initializing the working directory from a module, some use cases
may still want the module source even when the configuration cannot be
loaded due to validation errors.
Isolate the installation errors during the initialization sequence, so
we can report them if there is a problem fetching the actual module
source. Once we have the module, convert configuration errors to
warnings so the cli initialization can proceed.
This is a mostly mechanical refactor with a handful of changes which
are necessary due to the semantic difference between earlyconfig and
configs.
When parsing root and descendant modules in the module installer, we now
check the core version requirements inline. If the Terraform version is
incompatible, we drop any other module loader diagnostics. This ensures
that future language additions don't clutter the output and confuse the
user.
We also add two new checks during the module load process:
* Don't try to load a module with a `nil` source address. This is a
necessary change due to the move away from earlyconfig.
* Don't try to load a module with a blank name (i.e. `module ""`).
Because our module loading manifest uses the stringified module path
as its map key, this causes a collision with the root module, and a
later panic. This is the bug which triggered this refactor in the
first place.
* Use the apparentlymart/go-versions library to parse module constraints
* goimports
* Update comments, and parse versions carefully
* add acceptance tests to verify behaviour of partial matches
* goimports
* refactor: Use tfaddr for provider address parsing
* refactor: Use tfaddr for module address parsing
* deps: introduce hashicorp/terraform-registry-address
This commit replaces `ioutil.TempDir` with `t.TempDir` in tests. The
directory created by `t.TempDir` is automatically removed when the test
and all its subtests complete.
Prior to this commit, temporary directory created using `ioutil.TempDir`
needs to be removed manually by calling `os.RemoveAll`, which is omitted
in some tests. The error handling boilerplate e.g.
defer func() {
if err := os.RemoveAll(dir); err != nil {
t.Fatal(err)
}
}
is also tedious, but `t.TempDir` handles this for us nicely.
Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Previously we ended up losing all of the error message detail produced by
the registry address parser, because we treated any registry address
failure as cause to parse the address as a go-getter-style remote address
instead.
That led to terrible feedback in the situation where the user _was_
trying to write a module address but it was invalid in some way.
Although we can't really tighten this up in the default case due to our
compatibility promises, it's never been valid to use the "version"
argument with anything other than a registry address and so as a
compromise here we'll use the presence of "version" as a heuristic for
user intent to parse the source address as a registry address, and thus
we can return a registry-address-specific error message in that case and
thus give more direct feedback about what was wrong.
This unfortunately won't help someone trying to install from the registry
_without_ a version constraint, but I didn't want to let perfect be the
enemy of the good here, particularly since we recommend using version
constraints with registry modules anyway; indeed, that's one of the main
benefits of using a registry rather than a remote source directly.
Earlier work to make "terraform init" interruptible made the getproviders
package context-aware in order to allow provider installation to be cancelled.
Here we make a similar change for module installation, which is now also
cancellable with SIGINT. This involves plumbing context through initwd and
getmodules. Functions which can make network requests now include a context
parameter whose cancellation cancels those requests.
Since the module installation code is shared, "terraform get" is now
also interruptible during module installation.
Previously we had a separation between ModuleSourceRemote and
ModulePackage as a way to represent within the type system that there's an
important difference between a module source address and a package address,
because module packages often contain multiple modules and so a
ModuleSourceRemote combines a ModulePackage with a subdirectory to
represent one specific module.
This commit applies that same strategy to ModuleSourceRegistry, creating
a new type ModuleRegistryPackage to represent the different sort of
package that we use for registry modules. Again, the main goal here is
to try to reflect the conceptual modelling more directly in the type
system so that we can more easily verify that uses of these different
address types are correct.
To make use of that, I've also lightly reworked initwd's module installer
to use addrs.ModuleRegistryPackage directly, instead of a string
representation thereof. This was in response to some earlier commits where
I found myself accidentally mixing up package addresses and source
addresses in the installRegistryModule method; with this new organization
those bugs would've been caught at compile time, rather than only at
unit and integration testing time.
While in the area anyway, I also took this opportunity to fix some
historical confusing names of fields in initwd.ModuleInstaller, to be
clearer that they are only for registry packages and not for all module
source address types.
We have some tests in this package that install real modules from the real
registry at registry.terraform.io. Those tests were written at an earlier
time when the registry's behavior was to return the URL of a .tar.gz
archive generated automatically by GitHub, which included an extra level
of subdirectory that would then be reflected in the paths to the local
copies of these modules.
GitHub started rate limiting those tar archives in a way that Terraform's
module installer couldn't authenticate to, and so the registry switched
to returning direct git repository URLs instead, which don't have that
extra subdirectory and so the local paths on disk now end up being a
little different, because the actual module directories are at a different
subdirectory of the package.
It's been a long while since we gave close attention to the codepaths for
module source address parsing and external module package installation.
Due to their age, these codepaths often diverged from our modern practices
such as representing address types in the addrs package, and encapsulating
package installation details only in a particular location.
In particular, this refactor makes source address parsing a separate step
from module installation, which therefore makes the result of that parsing
available to other Terraform subsystems which work with the configuration
representation objects.
This also presented the opportunity to better encapsulate our use of
go-getter into a new package "getmodules" (echoing "getproviders"), which
is intended to be the only part of Terraform that directly interacts with
go-getter.
This is largely just a refactor of the existing functionality into a new
code organization, but there is one notable change in behavior here: the
source address parsing now happens during configuration loading rather
than module installation, which may cause errors about invalid addresses
to be returned in different situations than before. That counts as
backward compatible because we only promise to remain compatible with
configurations that are _valid_, which means that they can be initialized,
planned, and applied without any errors. This doesn't introduce any new
error cases, and instead just makes a pre-existing error case be detected
earlier.
Our module registry client is still using its own special module address
type from registry/regsrc for now, with a small shim from the new
addrs.ModuleSourceRegistry type. Hopefully in a later commit we'll also
rework the registry client to work with the new address type, but this
commit is already big enough as it is.
Our module installer has a somewhat-informal idea of a "module package",
which is some external thing we can go fetch in order to add one or more
modules to the current configuration. Our documentation doesn't talk much
about it because most users seem to have found the distinction between
external and local modules pretty intuitive without us throwing a lot of
funny terminology at them, but there are some situations where the
distinction between a module and a module package are material to the
end-user.
One such situation is when using an absolute rather than relative
filesystem path: we treat that as an external package in order to make the
resulting working directory theoretically "portable" (although users can
do various other things to defeat that), and so Terraform will copy the
directory into .terraform/modules in the same way as it would download and
extract a remote archive package or clone a git repository.
A consequence of this, though, is that any relative paths called from
inside a module loaded from an absolute path will fail if they try to
traverse upward into the parent directory, because at runtime we're
actually running from a copy of the directory that's been taking out of
its original context.
A similar sort of situation can occur in a truly remote module package if
the author accidentally writes a "../" source path that traverses up out
of the package root, and so this commit introduces a special error message
for both situations that tries to be a bit clearer about there being a
package boundary and use that to explain why installation failed.
We would ideally have made escaping local references like that illegal in
the first place, but sadly we did not and so when we rebuilt the module
installer for Terraform v0.12 we ended up keeping the previous behavior of
just trying it and letting it succeed if there happened to somehow be a
matching directory at the given path, in order to remain compatible with
situations that had worked by coincidence rather than intention. For that
same reason, I've implemented this as a replacement error message we will
return only if local module installation was going to fail anyway, and
thus it only modifies the error message for some existing error situations
rather than introducing new error situations.
This also includes some light updates to the documentation to say a little
more about how Terraform treats absolute paths, though aiming not to get
too much into the weeds about module packages since it's something that
most users can get away with never knowing.
This is part of a general effort to move all of Terraform's non-library
package surface under internal in order to reinforce that these are for
internal use within Terraform only.
If you were previously importing packages under this prefix into an
external codebase, you could pin to an earlier release tag as an interim
solution until you've make a plan to achieve the same functionality some
other way.
This is part of a general effort to move all of Terraform's non-library
package surface under internal in order to reinforce that these are for
internal use within Terraform only.
If you were previously importing packages under this prefix into an
external codebase, you could pin to an earlier release tag as an interim
solution until you've make a plan to achieve the same functionality some
other way.
This is part of a general effort to move all of Terraform's non-library
package surface under internal in order to reinforce that these are for
internal use within Terraform only.
If you were previously importing packages under this prefix into an
external codebase, you could pin to an earlier release tag as an interim
solution until you've make a plan to achieve the same functionality some
other way.
This is part of a general effort to move all of Terraform's non-library
package surface under internal in order to reinforce that these are for
internal use within Terraform only.
If you were previously importing packages under this prefix into an
external codebase, you could pin to an earlier release tag as an interim
solution until you've make a plan to achieve the same functionality some
other way.
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.
helper/copy CopyDir was used heavily in tests. It differes from
internal/copydir in a few ways, the main one being that it creates the
dst directory while the internal version expected the dst to exist
(there are other differences, which is why I did not just switch tests
to using internal's CopyDir).
I moved the CopyDir func from helper/copy into command_test.go; I could
also have moved it into internal/copy and named it something like
CreateDirAndCopy so if that seems like a better option please let me
know.
helper/copy/CopyFile was used in a couple of spots so I moved it into
internal, at which point I thought it made more sense to rename the
package copy (instead of copydir).
There's also a `go mod tidy` included.
* internal/initwd: fix panics with relative submodules in DirFromModule
There were two related issues here:
1. panic with any local module with submodules
1. panic with a relative directory that was above the workdir ("../")
The first panic was caused by the local installer looking up the root
module with the (nonexistant) key "root.", instead of "".
The second panic was caused by the installer trying to determine the
relative path from ".". This was fixed by detecting "." as the source
path and using the absolute path for the call to filepath.Rel.
Added test cases for both panics and updated the existing e2e tests with
the correct install paths.
Relying on the early config for provider requirements was necessary in
Terraform 0.12, to allow the 0.12upgrade command to run after init
installs providers.
However in 0.13, the same restrictions do not apply, and the detection
of provider requirements has changed. As a result, the early config
loader gives incorrect provider requirements in some circumstances,
such as those in the new test in this commit.
Therefore we are changing the init command to use the requirements found
by the full configuration loader. This also means that we can remove the
internal initwd CheckCoreVersionRequirements function.
We've previously been copying this function around so it could remain
unexported while being used in various packages. However, it's a
non-trivial function with lots of specific assumptions built into it, so
here we'll put it somewhere that other packages can depend on it _and_
document the assumptions it seems to be making for future reference.
As a bonus, this now uses os.SameFile to detect when two paths point to
the same physical file, instead of the slightly buggy local implementation
we had before which only worked on Unix systems and did not correctly
handle when the paths were on different physical devices.
The copy of the function I extracted here is the one from internal/initwd,
so this commit also includes the removal of that unexported version and
updating the callers in that package to use at at this new location.
