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.
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>
There is no special reason to do this; we just typically adopt the latest
minor release of the Go toolchain for each new minor release of
Terraform CLI so that we can make use of its new library and language
features gradually over the subsequent patch releases.
Adopting early will give us more time to exercise this and catch any
wrinkles before the Terraform CLI v1.2 release.
We previously used to throw an error denoting where in the configuration the attribute was missing or invalid.
Considering that organization can be now be omitted from the configuration, our previous error message will be
improperly formatted. This commit also updates the message to mention `TF_ORGANIZATION` as a valid substitute if
organization is missing or invalid in the configuration.
The initial rough implementation contained a bug where it would
incorrectly return a NilVal in some cases.
Improve the heuristics here to insert null values more precisely when
parent objects change to or from null. We also check for dynamic types
changing, in which case the entire object must be taken when we can't
match the individual attribute values.
TF_ORGANIZATION will serve as a fallback for configuring the organization in the `cloud`
block. This is the first step to make it easier for users wanting to configure Terraform
programmatically.
The existing description for the '-' symbol as use in format() stated that the result would padded spaces to the left. When tested in via 'terraform console' using format("%-10.1f", 3) the result was "3.0 "
Terraform v1.1.7
