Data sources should not require reading the previous versions. While we
previously skipped the decoding if it were to fail, this removes the
need for any prior state at all.
The only place where the prior state was functionally used was in the
destroy path. Because a data source destroy is only for cleanup purposes
to clean out the state using the same code paths as a managed resource,
we can substitute the prior state in the change change with a null value
to maintain the same behavior.
After data source handling was moved from a separate refresh phase into
the planning phase, reading the existing state was only used for
informational purposes. This had been reduced to reporting warnings when
the provider returned an unexpected value to try and help locate legacy
provider bugs, but any actual issues located from those warnings were
very few and far between.
Because the prior state cannot be reliably decoded when faced with
incompatible provider schema upgrades, and there is no longer any
significant reason to try and get the prior state at all, we can skip
the process entirely.
Data sources do not have state migrations, so there may be no way to
decode the prior state when faced with incompatible type changes.
Because prior state is only informational to the plan, and its existence
should not effect the planning process, we can skip decoding when faced
with errors.
When rendering diffs for resources which use nested attribute types, we
must cope with collections backing those attributes which are entirely
sensitive. The most common way this will be seen is through sensitive
values being present in sets, which will result in the entire set being
marked sensitive.
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>
The "push" event is only for pushes to branches within the same repository.
Since external contributors make commits in their own repositories rather than directly in the target repository, we also need the pull_request event to react to opening a pull request and to pushing new code to an external branch associated with a pull request.
Since internal pull requests would in principle trigger both "push" _and_ "pull_request", we also constrain the push event only to the branches we typically release from, on the assumption that all other branches will merge into those via pull requests. This avoids redundantly running the same checks in response to two events at the same time.
TF_WORKSPACE can now be used for your cloud configuration, effectively serving as an alternative
to setting the name attribute in your workspaces configuration.
In order to include condition block results in the JSON plan output, we
must store them in the plan and its serialization.
Terraform can evaluate condition blocks multiple times, so we must be
able to update the result. Accordingly, the plan.Conditions object is a
map with keys representing the condition block's address. Condition
blocks are not referenceable in any other context, so this address form
cannot be used anywhere in the configuration.
The commit includes a new test case for the JSON output of a
refresh-only plan, which is currently the only way for a failing
condition result to be rendered through this path.
We will henceforth use the "checks.yml" GitHub Actions workflow instead of
CircleCI, because we're standardizing on using GitHub Actions for all of
our automation in this repository so that everything is in a consistent
language and we have as few external dependencies as possible.
The checks.yml workflow alone does not actually replace everything this
CircleCI configuration did. Reworking things for GitHub Actions was a good
opportunity to revisit the cost/benefit of the various steps here and my
conclusions were:
- Unit tests and consistency checks give the best signal about the
correctness of new code, with broad coverage over all of our packages.
These are the most important things we want to run before reviewing a
pull request, although our unit test run is currently relatively slow
and would probably be worth optimizing in future commits.
- Our existing build.yml workflow already runs the E2E tests across
various platforms and so I considered removing those but elected to keep
the same single-platform (Linux) E2E test run in the pre-review checks
because in practice those tests are typically faster than the full
unit test run anyway and so they don't delay a green check result and
can serve as a reasonable proxy for whether the cross-platform E2E tests
will all succeed when we eventually check in build.yml, after merge.
- We've long had a special exception to our usual rule of not running
acceptance tests in CI specifically for the Consul backend. In practice
the Consul backend is essentially "done" and doesn't change much, so
I don't think the cost of installing and launching Consul just to test
that one backend has sufficient benefit to preserve. Our unit tests do
still exercise all of the generic backend machinery via the inmem and
local backends, and in the event that someone does make changes to the
Consul backend they can still run the acceptance tests locally as we'd
expect for a change to any other backend.
- We previously included jobs to run "go build" across various different
platforms. Although that can occasionally help catch platform-specific
issues, most code in Terraform is platform-agnostic and so it's rare
to encounter single-platform build errors. These jobs were typically
the long pole for completion of the CI checks before and so I've removed
them here in favor of relying on similar checks already happening inside
the build.yml workflow, which runs only after a PR is merged. This does
increase the risk of a platform-specific error landing in a release
branch before we catch it, but since platform-specific problems are
rare this feels like a reasonable tradeoff. Anyone working on
explicitly-platform-specific code in Terraform should typically test
locally on the relevant platform anyway, and so catching these with our
build step is a last gate just to make sure mistakes don't end up in
production releases.
Building our protobuf files requires the protoc tool, which takes a little
while to download and install. Hopefully downloading it out of the GitHub
Actions cache will make it a little faster in the common case where we're
still using the same version as the previous run.
Originally we had this running as part of the Unit Tests job because most
of the checks are relatively fast. However, the addition of the protobuf
generation check made that no longer be true because it needs to download
and run protoc.
With that extra overhead it now _is_ productive to spend the time booting
and installing Go on third worker, as long as GitHub Actions continues
to let us run all three of these jobs concurrently most of the time.
goenv was making things more complicated than needed since it's really
designed to help with interactive use in a shell more than automated use
like this.
Instead, we'll follow the same strategy that our build.yml was doing of
just reading the .go-version file directly and then using the official
actions/setup-go action to do the actual installation. The key advantage
here is that Go ends up installed in a way where just running "go" will
do the right thing, and we no longer need to fuss with shims and
version-based path prefixes.
Rather than duplicating the logic from build.yml, instead it's factored
out into a separate composite action which both build.yml and checks.yml
will now share, in case we want to change the Go version selection
methodology in the future.
This factors out the "Install Go" step into a separate composite action
which we can then call from both of the jobs that need it.
We can't factor out the "Cache Go Modules" because GitHub doesn't allow
composite actions to refer to other external actions.
This is intended to eventually replace the CircleCI-based checks we use
as part of the PR process in this repository. We're already using GitHub
Actions for various other processes in this repository, so this change is
motivated by consistency of having all of our automation running in the
same system and written in the same language.
This is not a complete replacement for our CircleCI workflow yet, and
probably won't ever be because the CircleCI workflow contains some steps
that are arguably redundant with other processes we follow elsewhere.
However, the CircleCI workflow remains for now and won't be removed until
we're satisfied that enough of it is replicated by this GitHub Actions
workflow.
When rendering a diff, we should quote object attribute names if the
string representation is not a valid identifier. While this is not
strictly necessary, it makes the diff output more closely resemble the
configuration language, which is less confusing.
This commit applies to both top-level schema attributes and any object
value attributes. We use a simplistic "%q" Go format string to quote the
strings, which is not strictly identical to HCL's quoting requirements,
but is the pattern used elsewhere in HCL and Terraform.
Co-Authored-By: Katy Moe <katy@katy.moe>
Co-authored-by: Alisdair McDiarmid <alisdair@users.noreply.github.com>
