We have a few different .proto files in this repository that all need to
get recompiled into .pb.go files each time we change them, but we were
previously handling that with some scripts that just assumed that protoc
and the relevant plugins were already installed on the system somewhere,
at the right versions.
In practice we've been constantly flopping between different versions of
these tools due to folks having different versions installed in their
development environments. In particular, the state of the .pb.go files
in the prior commit wasn't reproducible by any single version of the tools
because they've all slightly diverged from one another.
In the interests of being more consistent here and avoiding accidental
inconsistencies, we'll now centralize the protocol buffer compile steps
all into a single tool that knows how to fetch and install the expected
versions of the various tools we need and then run those tools with the
right options to get a stable result.
If we want to upgrade to either a newer protoc or a newer protoc-gen-go
in future then we'll do that in a central location and update all of the
.pb.go files at the same time, so that we're always consistently tracking
the same version of protocol buffers everywhere.
While doing this I attempted to keep as close as possible to the toolchain
we'd most recently used, but since they were not consistent with each
other they've now all changed which version numbers they record at minimum,
and the planproto stub in particular now also has a slightly different
descriptor serialization but is otherwise offering the same API.
CanChainFrom needs to be able to handle move statements from different
relative modules, re-implementing with addrs.anyKey
Add the anyKey InstanceKey value to the addrs package to simplify module
path comparison. This allows all combinations of module path
representation to be normalized into a ModuleInstance which can be
compared directly, rather than dealing with multiple levels of different
prefix types.
The -force-copy flag to init should automatically migrate state.
Previously this was not applied to one case: when migrating from
a backend with multiple workspaces to another backend supporting
multiple workspaces. I believe this was an oversight so this commit
fixes that.
We're aware of several quirks of this command's current design, which
result from some existing architectural limitations that we can't address
immediately.
However, we do still want to make this command available in its current
capacity as an incremental improvement, so as a compromise we'll document
it as experimental. Our intent here is to exclude it from the
Terraform 1.0 Compatibility Promises so that we can have the space to
continue to improve the design as other parts of the overall Terraform
system gain new capabilities.
We don't currently have any concrete plan for this command to be
stabilized and subject to compatibility promises. That decision will
follow from ongoing discussions with other teams whose systems may need to
change in order to support the final design of "terraform add".
The code adopted from block diffs was not set to handle null and unknown
values, as those are not allowed for blocks.
We also revert the change to formatting nested object types as single
attributes, because the attribute formatter cannot handle sensitive
values from the schema. This presents some awkward syntax for diffs for
now, but should suffice until the entire formatter can be refactored to
better handle these new nested types.
Go 1.17 includes a breaking change to both net.ParseIP and net.ParseCIDR
functions to reject IPv4 address octets written with leading zeros.
Our use of these functions as part of the various CIDR functions in the
Terraform language doesn't have the same security concerns that the Go
team had in evaluating this change to the standard library, and so we
can't justify an exception to our v1.0 compatibility promises on the same
sort of security grounds that the Go team used to justify their
compatibility exception.
For that reason, we'll now use our own fork of the Go library functions
which has the new check disabled in order to preserve the prior behavior.
We're taking this path, rather than pre-normalizing the IP address before
calling into the standard library, because an additional normalization
layer would be entirely new code and additional complexity, whereas this
fork is relatively minor in terms of code size and avoids any significant
changes to our own calls to these functions.
Thanks to the Kubernetes team for their prior work on carving out a subset
of the "net" package for their similar backward-compatibility concern.
Our "ipaddr" package here is a lightly-modified fork of their fork, with
only the comments changed to talk about Terraform instead of Kubernetes.
This fork is not intended for use in any other future feature
implementations, because they wouldn't be subject to the same
compatibility constraints as our existing functions. We will use these
forked implementations for new callers only if consistency with the
behavior of the existing functions is a key requirement.
This includes the addition of the new "//go:build" comment form in addition
to the legacy "// +build" notation, as produced by gofmt to ensure
consistent behavior between Go versions. The new directives are all
equivalent to what was present before, so there's no change in behavior.
Go 1.17 continues to use the Unicode 13 tables as in Go 1.16, so this
upgrade does not require also upgrading our Unicode-related dependencies.
This upgrade includes the following breaking changes which will also
appear as breaking changes for Terraform users, but that are consistent
with the Terraform v1.0 compatibility promises.
- On MacOS, Terraform now requires macOS 10.13 High Sierra or later.
This upgrade also includes the following breaking changes which will
appear as breaking changes for Terraform users that are inconsistent with
our compatibility promises, but have justified exceptions as follows:
- cidrsubnet, cidrhost, and cidrnetmask will now reject IPv4 CIDR
addresses whose decimal components have leading zeros, where previously
they would just silently ignore those leading zeros.
This is a security-motivated exception to our compatibility promises,
because some external systems interpret zero-prefixed octets as octal
numbers rather than decimal, and thus the previous lenient parsing could
lead to a different interpretation of the address between systems, and
thus potentially allow bypassing policy when configuring firewall rules
etc.
This upgrade also includes the following breaking changes which could
_potentially_ appear as breaking changes for Terraform users, but that do
not in practice for the reasons given:
- The Go net/url package no longer allows query strings with pairs
separated by semicolons instead of ampersands. This primarily affects
HTTP servers written in Go, and Terraform includes a special temporary
HTTP server as part of its implementation of OAuth for "terraform login",
but that server only needs to accept URLs created by Terraform itself
and Terraform does not generate any URLs that would be rejected.
Add implementations of CanChainFrom and NestedWithin for
MoveEndpointInModule.
CanChainFrom allows the linking of move statements of the same address,
which means the prior destination address must equal the following
source address. If the destination and source addresses are of different
types, they must be covered by NestedWithin rather than CanChainFrom.
NestedWithin checks if the destination contains the source address. Any
matching types would be covered by CanChainFrom.
Extend the outputs JSON log message to support an `action` field (and
make the `type` and `value` fields optional). This allows us to emit a
useful output change summary as part of the plan, bringing the JSON log
output into parity with the text output.
While we do have access to the before/after values in the output
changes, attempting to wedge those into a structured log message is not
appropriate. That level of detail can be extracted from the JSON plan
output from `terraform show -json`.
This is a first pass at implementing refactoring.ValidateMoves, covering
the main validation rules.
This is not yet complete. A couple situations not yet covered are
represented by commented test cases in TestValidateMoves, although that
isn't necessarily comprehensive. We'll do a further pass of filling this
out with any other subtleties before we ship this feature.
As of this commit, refactoring.ValidateMoves doesn't actually do anything
yet (always returns nil) but the goal here is to wire in the set of all
declared instances so that refactoring.ValidateMoves will then have all
of the information it needs to encapsulate our validation rules.
The actual implementation of refactoring.ValidateMoves will follow in
subsequent commits.
In order to precisely implement the validation rules for "moved"
statements we need to be able to test whether particular instances were
declared in the configuration.
The instance expander is the source of record for which instances we
decided while creating a plan, but it's API is far more involved than what
our validation rules need, so this new AllInstances method returns a
wrapper object with a more straightforward API that provides read-only
access to just the question of whether particular instances got registered
in the expander already.
This API covers all three of the kinds of objects that move statements can
refer to. It includes module calls and resources, even though they aren't
_themselves_ "instances" in the sense we usually mean, because the module
instance addresses they are contained within _are_ instances and so we
need to take their dynamic instance keys into account when answering these
queries.
All of our MoveDestination methods have the common problem of deciding
whether the receiver is even potentially in the scope of a particular
MoveEndpointInModule, which requires that the receiver belong to an
instance of the module where the move statement was found.
Previously we had this logic inline in all three cases, but now we'll
factor it out into a shared helper function.
At first it seemed like there ought to be more factoring possible for
the AbsResource vs. AbsResourceInstance implementations, since textually
they look very similar, but in practice they only look similar because
those two types have a lot of method names in common, but the Go compiler
sees them as completely distinct and thus we must write the same logic
out twice. I did try some further refactoring to address that but it
made the resulting code significantly more complicated and, by my
judgement, harder to follow. Consequently I decided that a little
duplication was okay and warranted here because this logic is already
quite fiddly to read through and isn't likely to change significantly once
released (due to backward-compatibility promises).
Previously our MoveDestination methods only honored move statements whose
endpoints were module calls, module instances, or resources.
Now we'll additionally handle when the endpoints are individual resource
instances. This situation only applies to
AbsResourceInstance.MoveDestination because no other objects can be
contained inside of a resource instance.
This completes all of the MoveDestination cases for all supported move
statement types and moveable object types.
Previously our MoveDestination methods only honored move statements whose
endpoints were module calls or module instances.
Now we'll additionally handle when the endpoints are whole resource
addresses. This includes both renaming resource blocks and moving resource
blocks into or out of child modules.
This doesn't yet include endpoints that are specific resource _instances_,
which will follow in a subsequent commit. For the moment that situation
will always indicate a non-match.
This is a subset of the MoveDestination behavior for AbsResource and
AbsResourceInstance which deals with source and destination addresses that
refer to module calls or module instances.
They both work by delegating to ModuleInstance.MoveDestination and then
applying the same resource or resource instance address to the
newly-chosen module instance address, thus ensuring that when we move
a module we also move all of the resources inside that module in the same
way.
This doesn't yet include support for moving between specific resource or
resource instance addresses; that'll follow later. This commit should have
enough logic to support moving between module names and module instance
keys, including any module calls or resources nested within.
This method encapsulates the move-processing rules for applying move
statements to ModuleInstance addresses. It honors both module call moves
and module instance moves by trying to find a subsequence of the input
that matches the "from" endpoint and then, if found, replacing it with
the "to" endpoint while preserving the prefix and suffix around the match,
if any.
* configs/configschema: extend block.AttributeByPath to descend into Objects
This commit adds a recursive Object.AttributeByPath function which will step through Attributes with NestedTypes. If an Attribute without a NestedType is encountered while there is still more to the path, it will return nil.
Martin Atkins