We previously had two tests of how the module installer responds to
cancellation (e.g. SIGINT) which were flakey because they tried to rely
on the cancellation being detected at some arbitrary point before the
module installer attempted to make a request, which isn't guaranteed in
practice because our interrupt mechanism only aims to cause OpenTofu to
exit "soon", with no guarantee about how much ongoing progress it will
make before it does.
To make these tests more robust, we'll now instead tell the module
installer to install from a real HTTP server that is intentionally designed
to stall the client by accepting its request but then just leaving the
connection open without responding.
This means that we can now test the more realistic situation of the cancel
signal being triggered after a slow request is already in progress, and
be sure that we're definitely sending the cancel signal at a moment that
matches that intention.
This is similar to a strategy we previously took to improve the reliability
of the tests for cancellation of the _provider_ installer, in
TestInit_cancelProviders. However, our provider installer version of this
used an intentionally-stalling implementation of getproviders.Source
instead of running a real server because the provider installer is designed
to support configurable installation methods, while the module installer
is not: its policy about what module source types are accepted is
hard-coded in package getproviders, at least for now.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Earlier work started to reshape this API to follow the dependency inversion
style, but didn't go so far as treating the package fetcher as an argument
because so far it hasn't offered any customizable policy anyway.
In future commits we will be introducing some policy arguments for the
package fetcher, and so this is some preparation work where we move the
responsibility for calling getmodules.NewPackageFetcher() out into the
caller of initwd.NewModuleInstaller().
This changes the API consumed by a bunch of unit testing helpers, so
splitting this out into its own commit should hopefully make future
commits more focused. The module installer now explicitly supports being
instantiated without a registry client or a remote package fetcher and
will in that case return an error if it's asked to install from anywhere
other than local relative directories. Most of our existing tests are
comfortable running under that constraint and so will not need any further
work in later commits that will change the signature of
getmodules.NewPackageFetcher.
However, a couple tests in package initwd _itself_ were making use of the
esoteric legacy support for treating an absolute filesystem path as a funny
sort of remote source, and so for now those will instantiate their own
package fetcher. Future commits that change the NewPackageFetcher signature
will need to offer a concession for those two tests.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
* Rename module name from "github.com/hashicorp/terraform" to "github.com/placeholderplaceholderplaceholder/opentf".
Signed-off-by: Jakub Martin <kubam@spacelift.io>
* Gofmt.
Signed-off-by: Jakub Martin <kubam@spacelift.io>
* Regenerate protobuf.
Signed-off-by: Jakub Martin <kubam@spacelift.io>
* Fix comments.
Signed-off-by: Jakub Martin <kubam@spacelift.io>
* Undo issue and pull request link changes.
Signed-off-by: Jakub Martin <kubam@spacelift.io>
* Undo comment changes.
Signed-off-by: Jakub Martin <kubam@spacelift.io>
* Fix comment.
Signed-off-by: Jakub Martin <kubam@spacelift.io>
* Undo some link changes.
Signed-off-by: Jakub Martin <kubam@spacelift.io>
* make generate && make protobuf
Signed-off-by: Jakub Martin <kubam@spacelift.io>
---------
Signed-off-by: Jakub Martin <kubam@spacelift.io>
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.
Go 1.19's "fmt" has some awareness of the new doc comment formatting
conventions and adjusts the presentation of the source comments to make
it clearer how godoc would interpret them. Therefore this commit includes
various updates made by "go fmt" to acheve that.
In line with our usual convention that we make stylistic/grammar/spelling
tweaks typically only when we're "in the area" changing something else
anyway, I also took this opportunity to review most of the comments that
this updated to see if there were any other opportunities to improve them.
* Add golden JSON test for Terraform plan
* Add data source to golden JSON plan
* Move output comparison code into shared helper function
* Add note for maintainer to contact TFC when UI changes
UI changes may potentially impact the behavior of structured run output
on TFC.
* Add test_data_source to other mock providers
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>
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.
We introduced this experiment to gather feedback, and the feedback we saw
led to us deciding to do another round of design work before we move
forward with something to meet this use-case.
In addition to being experimental, this has only been included in alpha
releases so far, and so on both counts it is not protected by the
Terraform v1.0 Compatibility Promises.
In historical versions of Terraform the responsibility to check this was
inside the terraform.NewContext function, along with various other
assorted concerns that made that function particularly complicated.
More recently, we reduced the responsibility of the "terraform" package
only to instantiating particular named plugins, assuming that its caller
is responsible for selecting appropriate versions of any providers that
_are_ external. However, until this commit we were just assuming that
"terraform init" had correctly selected appropriate plugins and recorded
them in the lock file, and so nothing was dealing with the problem of
ensuring that there haven't been any changes to the lock file or config
since the most recent "terraform init" which would cause us to need to
re-evaluate those decisions.
Part of the game here is to slightly extend the role of the dependency
locks object to also carry information about a subset of provider
addresses whose lock entries we're intentionally disregarding as part of
the various little edge-case features we have for overridding providers:
dev_overrides, "unmanaged providers", and the testing overrides in our
own unit tests. This is an in-memory-only annotation, never included in
the serialized plan files on disk.
I had originally intended to create a new package to encapsulate all of
this plugin-selection logic, including both the version constraint
checking here and also the handling of the provider factory functions, but
as an interim step I've just made version constraint consistency checks
the responsibility of the backend/local package, which means that we'll
always catch problems as part of preparing for local operations, while
not imposing these additional checks on commands that _don't_ run local
operations, such as "terraform apply" when in remote operations mode.
