Skip to content

Commit 83e84e5

Browse files
Liam Cervanteapparentlymart
andauthored
terraform init: add warning and guidance when lock file is incomplete (#31399)
* terraform init: add warning and guidance when lock file is incomplete * make the provider list in the warning deterministic * create installer event for tracking provider lock hashes (#31406) * create installer event for tracking provider lock hashes * address comments * fix tests * improve error message * Update internal/command/init.go Co-authored-by: Martin Atkins <mart@degeneration.co.uk> Co-authored-by: Martin Atkins <mart@degeneration.co.uk>
1 parent 2247288 commit 83e84e5

File tree

6 files changed

+233
-38
lines changed

6 files changed

+233
-38
lines changed

internal/command/init.go

Lines changed: 73 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"context"
55
"fmt"
66
"log"
7+
"reflect"
8+
"sort"
79
"strings"
810

911
"github.com/hashicorp/hcl/v2"
@@ -548,6 +550,11 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State,
548550
ctx, done := c.InterruptibleContext()
549551
defer done()
550552

553+
// We want to print out a nice warning if we don't manage to pull
554+
// checksums for all our providers. This is tracked via callbacks
555+
// and incomplete providers are stored here for later analysis.
556+
var incompleteProviders []string
557+
551558
// Because we're currently just streaming a series of events sequentially
552559
// into the terminal, we're showing only a subset of the events to keep
553560
// things relatively concise. Later it'd be nice to have a progress UI
@@ -789,6 +796,41 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State,
789796

790797
c.Ui.Info(fmt.Sprintf("- Installed %s v%s (%s%s)", provider.ForDisplay(), version, authResult, keyID))
791798
},
799+
ProvidersLockUpdated: func(provider addrs.Provider, version getproviders.Version, localHashes []getproviders.Hash, signedHashes []getproviders.Hash, priorHashes []getproviders.Hash) {
800+
// We're going to use this opportunity to track if we have any
801+
// "incomplete" installs of providers. An incomplete install is
802+
// when we are only going to write the local hashes into our lock
803+
// file which means a `terraform init` command will fail in future
804+
// when used on machines of a different architecture.
805+
//
806+
// We want to print a warning about this.
807+
808+
if len(signedHashes) > 0 {
809+
// If we have any signedHashes hashes then we don't worry - as
810+
// we know we retrieved all available hashes for this version
811+
// anyway.
812+
return
813+
}
814+
815+
// If local hashes and prior hashes are exactly the same then
816+
// it means we didn't record any signed hashes previously, and
817+
// we know we're not adding any extra in now (because we already
818+
// checked the signedHashes), so that's a problem.
819+
//
820+
// In the actual check here, if we have any priorHashes and those
821+
// hashes are not the same as the local hashes then we're going to
822+
// accept that this provider has been configured correctly.
823+
if len(priorHashes) > 0 && !reflect.DeepEqual(localHashes, priorHashes) {
824+
return
825+
}
826+
827+
// Now, either signedHashes is empty, or priorHashes is exactly the
828+
// same as our localHashes which means we never retrieved the
829+
// signedHashes previously.
830+
//
831+
// Either way, this is bad. Let's complain/warn.
832+
incompleteProviders = append(incompleteProviders, provider.ForDisplay())
833+
},
792834
ProvidersFetched: func(authResults map[addrs.Provider]*getproviders.PackageAuthenticationResult) {
793835
thirdPartySigned := false
794836
for _, authResult := range authResults {
@@ -803,18 +845,6 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State,
803845
"https://www.terraform.io/docs/cli/plugins/signing.html"))
804846
}
805847
},
806-
HashPackageFailure: func(provider addrs.Provider, version getproviders.Version, err error) {
807-
diags = diags.Append(tfdiags.Sourceless(
808-
tfdiags.Error,
809-
"Failed to validate installed provider",
810-
fmt.Sprintf(
811-
"Validating provider %s v%s failed: %s",
812-
provider.ForDisplay(),
813-
version,
814-
err,
815-
),
816-
))
817-
},
818848
}
819849
ctx = evts.OnContext(ctx)
820850

@@ -874,6 +904,22 @@ func (c *InitCommand) getProviders(config *configs.Config, state *states.State,
874904
return true, false, diags
875905
}
876906

907+
// Jump in here and add a warning if any of the providers are incomplete.
908+
if len(incompleteProviders) > 0 {
909+
// We don't really care about the order here, we just want the
910+
// output to be deterministic.
911+
sort.Slice(incompleteProviders, func(i, j int) bool {
912+
return incompleteProviders[i] < incompleteProviders[j]
913+
})
914+
diags = diags.Append(tfdiags.Sourceless(
915+
tfdiags.Warning,
916+
incompleteLockFileInformationHeader,
917+
fmt.Sprintf(
918+
incompleteLockFileInformationBody,
919+
strings.Join(incompleteProviders, "\n - "),
920+
getproviders.CurrentPlatform.String())))
921+
}
922+
877923
if previousLocks.Empty() {
878924
// A change from empty to non-empty is special because it suggests
879925
// we're running "terraform init" for the first time against a
@@ -1195,3 +1241,18 @@ Alternatively, upgrade to the latest version of Terraform for compatibility with
11951241

11961242
// No version of the provider is compatible.
11971243
const errProviderVersionIncompatible = `No compatible versions of provider %s were found.`
1244+
1245+
// incompleteLockFileInformationHeader is the summary displayed to users when
1246+
// the lock file has only recorded local hashes.
1247+
const incompleteLockFileInformationHeader = `Incomplete lock file information for providers`
1248+
1249+
// incompleteLockFileInformationBody is the body of text displayed to users when
1250+
// the lock file has only recorded local hashes.
1251+
const incompleteLockFileInformationBody = `Due to your customized provider installation methods, Terraform was forced to calculate lock file checksums locally for the following providers:
1252+
- %s
1253+
1254+
The current .terraform.lock.hcl file only includes checksums for %s, so Terraform running on another platform will fail to install these providers.
1255+
1256+
To calculate additional checksums for another platform, run:
1257+
terraform providers lock -platform=linux_amd64
1258+
(where linux_amd64 is the platform to generate)`

internal/command/init_test.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,7 +1644,7 @@ func TestInit_providerSource(t *testing.T) {
16441644
})
16451645
defer close()
16461646

1647-
ui := new(cli.MockUi)
1647+
ui := cli.NewMockUi()
16481648
view, _ := testView(t)
16491649
m := Meta{
16501650
testingOverrides: metaOverridesForProvider(testProvider()),
@@ -1726,13 +1726,16 @@ func TestInit_providerSource(t *testing.T) {
17261726
},
17271727
),
17281728
}
1729+
17291730
if diff := cmp.Diff(gotProviderLocks, wantProviderLocks, depsfile.ProviderLockComparer); diff != "" {
17301731
t.Errorf("wrong version selections after upgrade\n%s", diff)
17311732
}
17321733

1733-
outputStr := ui.OutputWriter.String()
1734-
if want := "Installed hashicorp/test v1.2.3 (verified checksum)"; !strings.Contains(outputStr, want) {
1735-
t.Fatalf("unexpected output: %s\nexpected to include %q", outputStr, want)
1734+
if got, want := ui.OutputWriter.String(), "Installed hashicorp/test v1.2.3 (verified checksum)"; !strings.Contains(got, want) {
1735+
t.Fatalf("unexpected output: %s\nexpected to include %q", got, want)
1736+
}
1737+
if got, want := ui.ErrorWriter.String(), "\n - hashicorp/source\n - hashicorp/test\n - hashicorp/test-beta"; !strings.Contains(got, want) {
1738+
t.Fatalf("wrong error message\nshould contain: %s\ngot:\n%s", want, got)
17361739
}
17371740
}
17381741

internal/providercache/installer.go

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -385,14 +385,14 @@ NeedProvider:
385385
// calculated from the package we just linked, which allows
386386
// the lock file to gradually transition to recording newer hash
387387
// schemes when they become available.
388-
var newHashes []getproviders.Hash
388+
var priorHashes []getproviders.Hash
389389
if lock != nil && lock.Version() == version {
390390
// If the version we're installing is identical to the
391391
// one we previously locked then we'll keep all of the
392392
// hashes we saved previously and add to it. Otherwise
393393
// we'll be starting fresh, because each version has its
394394
// own set of packages and thus its own hashes.
395-
newHashes = append(newHashes, preferredHashes...)
395+
priorHashes = append(priorHashes, preferredHashes...)
396396

397397
// NOTE: The behavior here is unfortunate when a particular
398398
// provider version was already cached on the first time
@@ -423,8 +423,17 @@ NeedProvider:
423423
// The hashes slice gets deduplicated in the lock file
424424
// implementation, so we don't worry about potentially
425425
// creating a duplicate here.
426+
var newHashes []getproviders.Hash
427+
newHashes = append(newHashes, priorHashes...)
426428
newHashes = append(newHashes, newHash)
427429
locks.SetProvider(provider, version, reqs[provider], newHashes)
430+
if cb := evts.ProvidersLockUpdated; cb != nil {
431+
// We want to ensure that newHash and priorHashes are
432+
// sorted. newHash is a single value, so it's definitely
433+
// sorted. priorHashes are pulled from the lock file, so
434+
// are also already sorted.
435+
cb(provider, version, []getproviders.Hash{newHash}, nil, priorHashes)
436+
}
428437

429438
if cb := evts.LinkFromCacheSuccess; cb != nil {
430439
cb(provider, version, new.PackageDir)
@@ -530,14 +539,14 @@ NeedProvider:
530539
// The hashes slice gets deduplicated in the lock file
531540
// implementation, so we don't worry about potentially
532541
// creating duplicates here.
533-
var newHashes []getproviders.Hash
542+
var priorHashes []getproviders.Hash
534543
if lock != nil && lock.Version() == version {
535544
// If the version we're installing is identical to the
536545
// one we previously locked then we'll keep all of the
537546
// hashes we saved previously and add to it. Otherwise
538547
// we'll be starting fresh, because each version has its
539548
// own set of packages and thus its own hashes.
540-
newHashes = append(newHashes, preferredHashes...)
549+
priorHashes = append(priorHashes, preferredHashes...)
541550
}
542551
newHash, err := new.Hash()
543552
if err != nil {
@@ -548,15 +557,32 @@ NeedProvider:
548557
}
549558
continue
550559
}
551-
newHashes = append(newHashes, newHash)
560+
561+
var signedHashes []getproviders.Hash
552562
if authResult.SignedByAnyParty() {
553563
// We'll trust new hashes from upstream only if they were verified
554564
// as signed by a suitable key. Otherwise, we'd record only
555565
// a new hash we just calculated ourselves from the bytes on disk,
556566
// and so the hashes would cover only the current platform.
557-
newHashes = append(newHashes, meta.AcceptableHashes()...)
567+
signedHashes = append(signedHashes, meta.AcceptableHashes()...)
558568
}
569+
570+
var newHashes []getproviders.Hash
571+
newHashes = append(newHashes, newHash)
572+
newHashes = append(newHashes, priorHashes...)
573+
newHashes = append(newHashes, signedHashes...)
574+
559575
locks.SetProvider(provider, version, reqs[provider], newHashes)
576+
if cb := evts.ProvidersLockUpdated; cb != nil {
577+
// newHash and priorHashes are already sorted.
578+
// But we do need to sort signedHashes so we can reason about it
579+
// sensibly.
580+
sort.Slice(signedHashes, func(i, j int) bool {
581+
return string(signedHashes[i]) < string(signedHashes[j])
582+
})
583+
584+
cb(provider, version, []getproviders.Hash{newHash}, signedHashes, priorHashes)
585+
}
560586

561587
if cb := evts.FetchPackageSuccess; cb != nil {
562588
cb(provider, version, new.PackageDir, authResult)

internal/providercache/installer_events.go

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,15 +106,28 @@ type InstallerEvents struct {
106106
FetchPackageSuccess func(provider addrs.Provider, version getproviders.Version, localDir string, authResult *getproviders.PackageAuthenticationResult)
107107
FetchPackageFailure func(provider addrs.Provider, version getproviders.Version, err error)
108108

109+
// The ProvidersLockUpdated event is called whenever the lock file will be
110+
// updated. It provides the following information:
111+
//
112+
// - `localHashes`: Hashes computed on the local system by analyzing
113+
// files on disk.
114+
// - `signedHashes`: Hashes signed by the private key that the origin
115+
// registry claims is the owner of this provider.
116+
// - `priorHashes`: Hashes already present in the lock file before we
117+
// made any changes.
118+
//
119+
// The final lock file will be updated with a union of all the provided
120+
// hashes. It not just likely, but expected that there will be duplicates
121+
// shared between all three collections of hashes i.e. the local hash and
122+
// remote hashes could already be in the cached hashes.
123+
//
124+
// In addition, we place a guarantee that the hash slices will be ordered
125+
// in the same manner enforced by the lock file within NewProviderLock.
126+
ProvidersLockUpdated func(provider addrs.Provider, version getproviders.Version, localHashes []getproviders.Hash, signedHashes []getproviders.Hash, priorHashes []getproviders.Hash)
127+
109128
// The ProvidersFetched event is called after all fetch operations if at
110129
// least one provider was fetched successfully.
111130
ProvidersFetched func(authResults map[addrs.Provider]*getproviders.PackageAuthenticationResult)
112-
113-
// HashPackageFailure is called if the installer is unable to determine
114-
// the hash of the contents of an installed package after installation.
115-
// In that case, the selection will not be recorded in the target cache
116-
// directory's lock file.
117-
HashPackageFailure func(provider addrs.Provider, version getproviders.Version, err error)
118131
}
119132

120133
// OnContext produces a context with all of the same behaviors as the given

internal/providercache/installer_events_test.go

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -164,20 +164,22 @@ func installerLogEventsForTests(into chan<- *testInstallerEventLogItem) *Install
164164
}{version.String(), err.Error()},
165165
}
166166
},
167-
ProvidersFetched: func(authResults map[addrs.Provider]*getproviders.PackageAuthenticationResult) {
167+
ProvidersLockUpdated: func(provider addrs.Provider, version getproviders.Version, localHashes []getproviders.Hash, signedHashes []getproviders.Hash, priorHashes []getproviders.Hash) {
168168
into <- &testInstallerEventLogItem{
169-
Event: "ProvidersFetched",
170-
Args: authResults,
171-
}
172-
},
173-
HashPackageFailure: func(provider addrs.Provider, version getproviders.Version, err error) {
174-
into <- &testInstallerEventLogItem{
175-
Event: "HashPackageFailure",
169+
Event: "ProvidersLockUpdated",
176170
Provider: provider,
177171
Args: struct {
178172
Version string
179-
Error string
180-
}{version.String(), err.Error()},
173+
Local []getproviders.Hash
174+
Signed []getproviders.Hash
175+
Prior []getproviders.Hash
176+
}{version.String(), localHashes, signedHashes, priorHashes},
177+
}
178+
},
179+
ProvidersFetched: func(authResults map[addrs.Provider]*getproviders.PackageAuthenticationResult) {
180+
into <- &testInstallerEventLogItem{
181+
Event: "ProvidersFetched",
182+
Args: authResults,
181183
}
182184
},
183185
}

0 commit comments

Comments
 (0)