Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

terraform init: add warning and guidance when lock file is incomplete #31399

Merged
merged 5 commits into from Jul 20, 2022
Merged

terraform init: add warning and guidance when lock file is incomplete #31399

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d08b485
terraform init: add warning and guidance when lock file is incomplete
liamcervante Jul 7, 2022
d19d6e1
make the provider list in the warning deterministic
liamcervante Jul 7, 2022
2695db4
create installer event for tracking provider lock hashes (#31406)
liamcervante Jul 12, 2022
f9fcbc7
improve error message
liamcervante Jul 12, 2022
b77fffd
Update internal/command/init.go
liamcervante Jul 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 27 additions & 0 deletions internal/command/init.go
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"log"
"sort"
"strings"

"github.com/hashicorp/hcl/v2"
Expand Down Expand Up @@ -874,6 +875,32 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State,
return true, false, diags
}

// Jump in here and add a warning if any of the providers we've validated
// have only a single checksum. This usually means we didn't manage to
// pull checksums for all architectures remotely and only generated the
// checksum locally for the current architecture. There is a simple fix
// for this so terraform can print out some help.

var incompleteProviders []string
for provider, locks := range newLocks.AllProviders() {
if len(locks.AllHashes()) == 1 {
incompleteProviders = append(incompleteProviders, provider.ForDisplay())
}
}

if len(incompleteProviders) > 0 {
// We don't really care about the order here, we just want the output to be deterministic.
sort.Slice(incompleteProviders, func(i, j int) bool {
return incompleteProviders[i] < incompleteProviders[j]
})
diags = diags.Append(tfdiags.Sourceless(
tfdiags.Warning,
"Incomplete validation for providers",
fmt.Sprintf(
"Terraform has only been able to record incomplete checksums for some providers (%s) so you may encounter problems when the lock file is used on machines with a different architecture. This can normally be fixed by running `terraform providers lock -platform=os_arch` for all target architectures.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some copyedits of this warning message which are admittedly subjective but my idea here is firstly to make this "sound like" the Terraform UI voice (which isn't defined strongly, but I think you'll get a sense for it over time) and also to give a little more information about what Terraform just did:

Warning: Incomplete lock file information for providers

Due to your customized provider installation methods, Terraform was forced
to calculate lock file checksums locally for the following providers:
  - hashicorp/aws
  - hashicorp/null
  - somethingelse/somethingelse

The current .terraform.lock.hcl file only includes checksums for
darwin_arm64, so Terraform running on another platform will fail to
install these providers.

To calculate additional checksums for another platform, run:
  terraform providers lock -platform=linux_amd64
(where linux_amd64 is the platform to generate)

My thought process as I wrote this was:

  • Subjectively, feels nicer to enumerate the providers as a "bullet list", rather than as a comma-separated list, because the list could potentially be quite long and contain long strings like foo.example.com/bleep-bloop/bar-baz that probably won't word-wrap nicely.

  • State that this is caused by the customized installation methods, which hopefully hints at a potential way to avoid this problem altogether (use the default settings) but without implying that it's wrong to use customized settings, since there are of course various good reasons to do so.

  • Mention the lock file name specifically so it's easier to see that we're talking about the same file that Terraform presumably also mentioned elsewhere in this output.

  • Mention the platform the user is currently using, honestly mainly just to give another example of what a platform string might look like, since this might be the user's first exposure to them.

  • Show the copy-pasteable command line on a line of its own, to avoid readers being confused about whether the delimiters are important. (This is our typical style with other error and warning messages which include commands to run).

    I also kinda arbitrarily chose to use linux_amd64 as a "real example" of a platform string, rather than the "os_arch" placeholder, in the hope of making it easier to see what to do by showing another actual platform string. I chose linux_amd64 in recognition that a typical way this problem manifests is someone writing a configuration originally on a Windows or macOS workstation but then trying to run it "for real" in automation running in Linux containers.

  • Note that our diagnostic renderer in the CLI will automatically wrap any line of text that has no leading whitespace, assuming it's a paragraph, but will not wrap lines which start with leading whitespace. In the above example, that applies to both the "bullet points" for the provider addresses and to the copy-pasteable command line at the end.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! All makes sense to me.

strings.Join(incompleteProviders, ", "))))
}

if previousLocks.Empty() {
// A change from empty to non-empty is special because it suggests
// we're running "terraform init" for the first time against a
Expand Down
12 changes: 8 additions & 4 deletions internal/command/init_test.go
View file
Open in desktop
Expand Up @@ -1644,7 +1644,7 @@ func TestInit_providerSource(t *testing.T) {
})
defer close()

ui := new(cli.MockUi)
ui := cli.NewMockUi()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you've discovered another example of the historical mistake of not properly initializing the mock UI! This does still come up from time to time, and indeed this is the way we typically fix it. (By which I mean: this probably won't be the last time you make a change like this!)

view, _ := testView(t)
m := Meta{
testingOverrides: metaOverridesForProvider(testProvider()),
Expand Down Expand Up @@ -1726,13 +1726,17 @@ func TestInit_providerSource(t *testing.T) {
},
),
}

if diff := cmp.Diff(gotProviderLocks, wantProviderLocks, depsfile.ProviderLockComparer); diff != "" {
t.Errorf("wrong version selections after upgrade\n%s", diff)
}

outputStr := ui.OutputWriter.String()
if want := "Installed hashicorp/test v1.2.3 (verified checksum)"; !strings.Contains(outputStr, want) {
t.Fatalf("unexpected output: %s\nexpected to include %q", outputStr, want)
if got, want := ui.OutputWriter.String(), "Installed hashicorp/test v1.2.3 (verified checksum)"; !strings.Contains(got, want) {
t.Fatalf("unexpected output: %s\nexpected to include %q", got, want)
}

if got, want := ui.ErrorWriter.String(), "(hashicorp/source, hashicorp/test, hashicorp/test-beta)"; !strings.Contains(got, want) {
t.Fatalf("wrong error message\nshould contain: %s\ngot:\n%s", want, got)
}
}

Expand Down