The changes in the previous commit confirmed that this test was passing
only as a false-positive when running on Windows, because the test was
previously only checking that the provisioner was stopped shortly after
asking it to stop, but that wasn't accounting for the possibility that it
stopped due to an unrelated error.
Windows Command Interpreter does not support semicolon as a command
separator, so on Windows we need to use an ampersand instead.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Previously this test was just assuming that the provisioner run would
succeed and only requiring that it run for more than 50ms before exiting.
That meant that it could potentially false-positive succeed if the
provisioner happened to return an error but take more than 50ms to do so.
Now we'll test for failure before we ask the provisioner to stop, which
narrows the false-positive window. This still isn't completely robust
because we don't have any way to test whether the provisioner failed due
to being canceled or for some other reason. The error message returned on
cancellation varies depending on what state the provisioner was in when
it got the cancellation message, so it's not currently feasible to write
a robust check that would definitely distinguish between the expected error
vs. unexpected errors.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Previously we were using a mixture of old and new, with our code generation
using the plugin from the old github.com/golang/protobuf library but
our callers using the modern google.golang.org/protobuf . We were also
using pretty ancient version of protoc.
This brings us up to the current latest releases and consistently using
the new Go protobuf library. There have been some notable changes to these
tools in the meantime:
Previously the protoc-gen-go plugin handled grpc by having its own
additional level of Go-specific "plugins" of which the gRPC codegen was
an example.
Now the protobuf generator and the gRPC generator are separate plugins
handled directly by protoc, which means the command line arguments are
a different shape and the gRPC stubs get generated in a separate file
from the main protobuf messages, rather than all being in one .pb.go file
as before.The results are otherwise similar, though.
The grpc codegen now also defaults to requiring that implementations embed
the generated "unimplemented" server, which is an implementation of each
service where the methods just immediately return the "unimplemented"
error. This is not super important for us because we maintain the generated
interfaces and their implementations together in the same repository
anyway, but adding the "unimplemented" server embeds was not a big change
and so seems better to follow the prevailing convention.
Using these new versions means that we could in principle now switch to
using protobuf edition 2024 and the new "sealed" style for Go code
generation, but this commit does not include any such changes and focuses
only on getting things upgraded with as few other changes as possible. We
can discuss using different codegen style later and deal with that in
separate commits.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
This upstream library (which I wrote, independently of my work on OpenTofu)
came about because "go-spew" tended to produce unreadable representations
of certain types commonly used in OpenTofu, whereas "go-dump" is really
just a pretty-printer for whatever a type might produce when formatted
using the %#v verb in package fmt.
Over time the uses of this seem to have decreased only to some leftover
situations where we wanted to pretty-print a cty.Value in a test, but
we already depend on go-cty-debug that has a more specialized
implementation of that behavior and so switching the few remaining callers
over to that allows us to remove one dependency.
(And, FWIW, that upstream dependency is effectively unmaintained; I don't
know of any callers of it other than OpenTofu itself, and after merging
this even OpenTofu won't depend on it anymore.)
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
Previously, the source snippet was only showing the last defined
meta-argument. To have a better context of which meta-arguments
are being used, we start to show from the first one defined
until the last one.
Signed-off-by: Diogenes Fernandes <diofeher@gmail.com>
This uses the same auth package as the newly-rewritten Azure State
Backend, so many of the properties and environment variables are the
same. I have put this through both the compliance test as well as built
the binary and run some end-to-end tests, and found that it
appropriately uses the Azure key as expected.
Signed-off-by: Larry Bordowitz <laurence.bordowitz@gmail.com>
From some more practical testing of this I realized that usually the first
thing I want to know after seeing this warning is what the object literal
was being assigned to and what else was also defined inside it, and so
this sets the diagnostic's "context" to include the whole containing
object literal so that the source snippet in the diagnostic message is more
immediately useful, without having to cross-reference to the source code
in a separate text editor.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
This generalizes the previously-added lint-like check for when an object
constructor is used to define an input variable and it contains a
definition for an attribute that isn't part of the target type, so that
now it also works for various nested structures that commonly arise in
real-world configurations.
Because this is now considerably more complicated I factored it out into
a new package called "lint" which could potentially grow to include other
similar "technically valid but probably a mistake" situations in future,
but for now it just introduced an opportunity to produce similar warning
messages for ignored attribute definitions in the default value for an
input variable.
It seems to me that there is actually no useful reason to include an
unexpected attribute definition in either of these two cases: that
attribute will never appear as part of any expression that any other part
of the configuration can use. Therefore I considered making these be
treated as errors rather than warnings, but turning something that was
previously valid into an error is risky so I'm suggesting that we start
with these as warnings and then consider upgrading them to errors in a
later release if we don't hear of anyone reporting a false-positive that
was _somehow_ actually useful. (I find that very unlikely, but still...)
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
We intentionally allow assigning object types with a superset of the
attributes included in an input variable's object type constraints because
it makes it possible to assign a whole object for which only some of the
attributes are relevant for one input variable but a different subset might
be relevant when the object value is used in a different part of the
configuration.
However, when the variable is defined using an object literal expression
there is no possible way an unexpected attribute could be useful in a
different part of the configuration, and so that's very very likely to be
a mistake rather than intentional. Therefore we'll generate a "linter-like"
warning in that case to help the author notice their mistake without
introducing any new "strict-mode" language features, or other complexity
that would be harder to maintain and evolve over time.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
This introduces the concept of "backend aliases", which are alternative
names that can be used to refer to a given backend.
Each backend type has one canonical name and zero or more alias names. The
"backend" block in the root module can specify either a canonical backend
type or an alias, but internally OpenTofu will always track the backend
type using its canonical name.
In particular, the following are all true when the configuration specifies
an alias instead of a canonical backend type:
- The "tofu init" output includes a brief extra message saying which
backend type OpenTofu actually used, because that is the name that we'd
prioritize in our documentation and so an operator can use the canonical
type to find the relevant docs when needed.
- The .terraform/terraform.tfstate file that tracks the working directory's
currently-initialized backend settings always uses the canonical backend
type, and so it's possible to freely switch between aliases and canonical
without "tofu init" thinking that a state migration might be needed.
- Plan files similarly use the canonical backend type to track which
backend was active when the plan was created, which doesn't have any
significant user-facing purpose, but is consistent with the previous
point since the settings in the plan file effectively substitute for
the .terraform/terraform.tfstate file when applying a saved plan.
- The terraform_remote_state data source in the provider
terraform.io/builtin/terraform accepts both canonical and alias in its
backend type argument, treating both as equivalent for the purpose of
fetching the state snapshot for the configured workspace.
The primary motivation for this new facility is to allow the planned
"oracle_oci" backend to have an alias "oci" to allow writing configurations
that are cross-compatible with HashiCorp Terraform, since that software
has chosen to have unqualified OCI mean Oracle's system, whereas OpenTofu
has previously established that unqualified OCI means "Open Container
Initiative" in our ecosystem.
In particular, this design makes it possible in principle to bring an
existing Terraform configuration specifying backend "oci" over to OpenTofu
without modifications, and then to optionally switch it to specifying
backend "oracle-oci" at a later time without a spurious prompt to migrate
state snapshots to the same physical location where they are already
stored.
This commit doesn't actually introduce any aliases and therefore doesn't
have any tests for the new mechanism because our backend system uses a
global table that isn't friendly to mocking for testing purposes. I've
tested this manually using a placeholder alias to have confidence that it
works, and I expect that a subsequent commit introducing the new
"oracle_oci" backend will also introduce its "oci" alias and will include
tests that cover use of the alias and migration from the alias to the
canonical name and vice-versa.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
These were previously settable only via environment variables. These are
now handled as part of CLI Configuration and so also settable in a new
"registry_protocols" block in a CLI configuration file, with the
environment variables now treated as if they are an additional virtual
configuration file containing the corresponding settings.
This handles our settings in our modern style where package cliconfig is
responsible for deciding the configuration and then package main reacts
to that configuration without being aware of how it is decided.
Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
