From 58a5bc74b4fceb35aea59ff637aa9bc188aeb7ad Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Wed, 6 Jul 2022 14:39:35 +0100 Subject: [PATCH 1/4] Ignore existing package hashes for command --- internal/command/providers_lock.go | 51 +++++++- internal/command/providers_lock_test.go | 92 ++++++++------ .../providers-lock/append/.terraform.lock.hcl | 9 ++ .../1.0.0/os_arch/terraform-provider-test | 0 .../testdata/providers-lock/append/main.tf | 7 ++ internal/depsfile/locks.go | 24 ++++ internal/depsfile/locks_test.go | 91 ++++++++++++++ internal/providercache/installer.go | 17 ++- internal/providercache/installer_test.go | 114 ++++++++++++++++++ 9 files changed, 364 insertions(+), 41 deletions(-) create mode 100644 internal/command/testdata/providers-lock/append/.terraform.lock.hcl create mode 100644 internal/command/testdata/providers-lock/append/fs-mirror/registry.terraform.io/hashicorp/test/1.0.0/os_arch/terraform-provider-test create mode 100644 internal/command/testdata/providers-lock/append/main.tf diff --git a/internal/command/providers_lock.go b/internal/command/providers_lock.go index a7243fab5d5a..897f7220fcda 100644 --- a/internal/command/providers_lock.go +++ b/internal/command/providers_lock.go @@ -225,7 +225,7 @@ func (c *ProvidersLockCommand) Run(args []string) int { if keyID != "" { keyID = c.Colorize().Color(fmt.Sprintf(", key ID [reset][bold]%s[reset]", keyID)) } - c.Ui.Output(fmt.Sprintf("- Obtained %s checksums for %s (%s%s)", provider.ForDisplay(), platform, auth, keyID)) + c.Ui.Output(fmt.Sprintf("- Retrieved %s %s for %s (%s%s)", provider.ForDisplay(), version, platform, auth, keyID)) }, } ctx := evts.OnContext(ctx) @@ -233,7 +233,7 @@ func (c *ProvidersLockCommand) Run(args []string) int { dir := providercache.NewDirWithPlatform(tempDir, platform) installer := providercache.NewInstaller(dir, source) - newLocks, err := installer.EnsureProviderVersions(ctx, oldLocks, reqs, providercache.InstallNewProvidersOnly) + newLocks, err := installer.EnsureProviderVersions(ctx, oldLocks, reqs, providercache.InstallNewProvidersForce) if err != nil { diags = diags.Append(tfdiags.Sourceless( tfdiags.Error, @@ -252,6 +252,10 @@ func (c *ProvidersLockCommand) Run(args []string) int { return 1 } + // Track whether we've made any changes to the lock file as part of this + // operation. We can customise the final message based on our actions. + madeAnyChange := false + // We now have a separate updated locks object for each platform. We need // to merge those all together so that the final result has the union of // all of the checksums we saw for each of the providers we've worked on. @@ -270,7 +274,7 @@ func (c *ProvidersLockCommand) Run(args []string) int { constraints = oldLock.VersionConstraints() hashes = append(hashes, oldLock.AllHashes()...) } - for _, platformLocks := range updatedLocks { + for platform, platformLocks := range updatedLocks { platformLock := platformLocks.Provider(provider) if platformLock == nil { continue // weird, but we'll tolerate it to avoid crashing @@ -282,6 +286,39 @@ func (c *ProvidersLockCommand) Run(args []string) int { // platforms here, because the SetProvider method we call below // handles that automatically. hashes = append(hashes, platformLock.AllHashes()...) + + // At this point, we've merged all the hashes for this (provider, platform) + // combo into the combined hashes for this provider. Let's take this + // opportunity to print out a summary for this particular combination. + + if oldLock == nil { + // Then we weren't tracking this provider previously so we can write a simple error message. + madeAnyChange = true + c.Ui.Output( + fmt.Sprintf( + "- Obtained %s checksums for %s; This was a new provider and the checksums for this platform are now tracked in the lock file", + provider.ForDisplay(), + platform)) + continue + } + + if oldLock.ContainsAll(platformLock) { + // Then we found new checksums for this provider and platform that weren't tracked previously. + madeAnyChange = true + c.Ui.Output( + fmt.Sprintf( + "- Obtained %s checksums for %s; Additional checksums for this platform are now tracked in the lock file", + provider.ForDisplay(), + platform)) + continue + } + + // Finally, if we reach here it means we found no new checksums so let's explain that. + c.Ui.Output( + fmt.Sprintf( + "- Obtained %s checksums for %s; All checksums for this platform were already tracked in the lock file", + provider.ForDisplay(), + platform)) } newLocks.SetProvider(provider, version, constraints, hashes) } @@ -294,8 +331,12 @@ func (c *ProvidersLockCommand) Run(args []string) int { return 1 } - c.Ui.Output(c.Colorize().Color("\n[bold][green]Success![reset] [bold]Terraform has updated the lock file.[reset]")) - c.Ui.Output("\nReview the changes in .terraform.lock.hcl and then commit to your\nversion control system to retain the new checksums.\n") + if madeAnyChange { + c.Ui.Output(c.Colorize().Color("\n[bold][green]Success![reset] [bold]Terraform has updated the lock file.[reset]")) + c.Ui.Output("\nReview the changes in .terraform.lock.hcl and then commit to your\nversion control system to retain the new checksums.\n") + } else { + c.Ui.Output(c.Colorize().Color("\n[bold][green]Success![reset] [bold]Terraform has validated the lock file and found no need for changes.[reset]")) + } return 0 } diff --git a/internal/command/providers_lock_test.go b/internal/command/providers_lock_test.go index ef22f51d18cc..6725d2984325 100644 --- a/internal/command/providers_lock_test.go +++ b/internal/command/providers_lock_test.go @@ -33,40 +33,23 @@ func TestProvidersLock(t *testing.T) { // This test depends on the -fs-mirror argument, so we always know what results to expect t.Run("basic", func(t *testing.T) { - td := t.TempDir() - testCopyDir(t, testFixturePath("providers-lock/basic"), td) - defer testChdir(t, td)() - - // Our fixture dir has a generic os_arch dir, which we need to customize - // to the actual OS/arch where this test is running in order to get the - // desired result. - fixtMachineDir := filepath.Join(td, "fs-mirror/registry.terraform.io/hashicorp/test/1.0.0/os_arch") - wantMachineDir := filepath.Join(td, "fs-mirror/registry.terraform.io/hashicorp/test/1.0.0/", fmt.Sprintf("%s_%s", runtime.GOOS, runtime.GOARCH)) - err := os.Rename(fixtMachineDir, wantMachineDir) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - p := testProvider() - ui := new(cli.MockUi) - c := &ProvidersLockCommand{ - Meta: Meta{ - Ui: ui, - testingOverrides: metaOverridesForProvider(p), - }, - } - - args := []string{"-fs-mirror=fs-mirror"} - code := c.Run(args) - if code != 0 { - t.Fatalf("wrong exit code; expected 0, got %d", code) - } + testDirectory := "providers-lock/basic" + expected := `# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. - lockfile, err := os.ReadFile(".terraform.lock.hcl") - if err != nil { - t.Fatal("error reading lockfile") - } +provider "registry.terraform.io/hashicorp/test" { + version = "1.0.0" + hashes = [ + "h1:7MjN4eFisdTv4tlhXH5hL4QQd39Jy4baPhFxwAd/EFE=", + ] +} +` + runProviderLockGenericTest(t, testDirectory, expected) + }) + // This test depends on the -fs-mirror argument, so we always know what results to expect + t.Run("append", func(t *testing.T) { + testDirectory := "providers-lock/append" expected := `# This file is maintained automatically by "terraform init". # Manual edits may be lost in future updates. @@ -74,15 +57,54 @@ provider "registry.terraform.io/hashicorp/test" { version = "1.0.0" hashes = [ "h1:7MjN4eFisdTv4tlhXH5hL4QQd39Jy4baPhFxwAd/EFE=", + "h1:invalid", ] } ` - if string(lockfile) != expected { - t.Fatalf("wrong lockfile content") - } + runProviderLockGenericTest(t, testDirectory, expected) }) } +func runProviderLockGenericTest(t *testing.T, testDirectory, expected string) { + td := t.TempDir() + testCopyDir(t, testFixturePath(testDirectory), td) + defer testChdir(t, td)() + + // Our fixture dir has a generic os_arch dir, which we need to customize + // to the actual OS/arch where this test is running in order to get the + // desired result. + fixtMachineDir := filepath.Join(td, "fs-mirror/registry.terraform.io/hashicorp/test/1.0.0/os_arch") + wantMachineDir := filepath.Join(td, "fs-mirror/registry.terraform.io/hashicorp/test/1.0.0/", fmt.Sprintf("%s_%s", runtime.GOOS, runtime.GOARCH)) + err := os.Rename(fixtMachineDir, wantMachineDir) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + p := testProvider() + ui := new(cli.MockUi) + c := &ProvidersLockCommand{ + Meta: Meta{ + Ui: ui, + testingOverrides: metaOverridesForProvider(p), + }, + } + + args := []string{"-fs-mirror=fs-mirror"} + code := c.Run(args) + if code != 0 { + t.Fatalf("wrong exit code; expected 0, got %d", code) + } + + lockfile, err := os.ReadFile(".terraform.lock.hcl") + if err != nil { + t.Fatal("error reading lockfile") + } + + if string(lockfile) != expected { + t.Fatalf("wrong lockfile content") + } +} + func TestProvidersLock_args(t *testing.T) { t.Run("mirror collision", func(t *testing.T) { diff --git a/internal/command/testdata/providers-lock/append/.terraform.lock.hcl b/internal/command/testdata/providers-lock/append/.terraform.lock.hcl new file mode 100644 index 000000000000..1f711f07ba97 --- /dev/null +++ b/internal/command/testdata/providers-lock/append/.terraform.lock.hcl @@ -0,0 +1,9 @@ +# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/test" { + version = "1.0.0" + hashes = [ + "h1:invalid", + ] +} diff --git a/internal/command/testdata/providers-lock/append/fs-mirror/registry.terraform.io/hashicorp/test/1.0.0/os_arch/terraform-provider-test b/internal/command/testdata/providers-lock/append/fs-mirror/registry.terraform.io/hashicorp/test/1.0.0/os_arch/terraform-provider-test new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/internal/command/testdata/providers-lock/append/main.tf b/internal/command/testdata/providers-lock/append/main.tf new file mode 100644 index 000000000000..41b211f26882 --- /dev/null +++ b/internal/command/testdata/providers-lock/append/main.tf @@ -0,0 +1,7 @@ +terraform { + required_providers { + test = { + source = "hashicorp/test" + } + } +} \ No newline at end of file diff --git a/internal/depsfile/locks.go b/internal/depsfile/locks.go index ebd5a632a617..80f0e984b734 100644 --- a/internal/depsfile/locks.go +++ b/internal/depsfile/locks.go @@ -403,6 +403,30 @@ func (l *ProviderLock) AllHashes() []getproviders.Hash { return l.hashes } +// ContainsAll returns true if the hashes in this ProviderLock contains +// all the hashes in the target. +// +// This function assumes the hashes are in each ProviderLock are sorted. +// If the ProviderLock was created by the NewProviderLock constructor then +// the hashes are guarenteed to be sorted. +func (l *ProviderLock) ContainsAll(target *ProviderLock) bool { + if target == nil || len(target.hashes) == 0 { + return true + } + + targetIndex := 0 + for ix := 0; ix < len(l.hashes); ix++ { + if l.hashes[ix] == target.hashes[targetIndex] { + targetIndex++ + + if targetIndex >= len(target.hashes) { + return true + } + } + } + return false +} + // PreferredHashes returns a filtered version of the AllHashes return value // which includes only the strongest of the availabile hash schemes, in // case legacy hash schemes are deprecated over time but still supported for diff --git a/internal/depsfile/locks_test.go b/internal/depsfile/locks_test.go index e82f9e117b24..c3eb868ed89c 100644 --- a/internal/depsfile/locks_test.go +++ b/internal/depsfile/locks_test.go @@ -216,3 +216,94 @@ func TestLocksProviderSetRemove(t *testing.T) { } } } + +func TestProviderLockContainsAll(t *testing.T) { + provider := addrs.NewDefaultProvider("provider") + v2 := getproviders.MustParseVersion("2.0.0") + v2EqConstraints := getproviders.MustParseVersionConstraints("2.0.0") + + t.Run("non-symmetric", func(t *testing.T) { + target := NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{ + "9r3i9a9QmASqMnQM", + "K43RHM2klOoywtyW", + "swJPXfuCNhJsTM5c", + }) + + original := NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{ + "9r3i9a9QmASqMnQM", + "1ZAChGWUMWn4zmIk", + "K43RHM2klOoywtyW", + "HWjRvIuWZ1LVatnc", + "swJPXfuCNhJsTM5c", + "KwhJK4p/U2dqbKhI", + }) + + if !original.ContainsAll(target) { + t.Fatalf("orginal should contain all hashes in target") + } + if target.ContainsAll(original) { + t.Fatalf("target should not contain all hashes in orginal") + } + }) + + t.Run("symmetric", func(t *testing.T) { + target := NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{ + "9r3i9a9QmASqMnQM", + "K43RHM2klOoywtyW", + "swJPXfuCNhJsTM5c", + }) + + original := NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{ + "9r3i9a9QmASqMnQM", + "K43RHM2klOoywtyW", + "swJPXfuCNhJsTM5c", + }) + + if !original.ContainsAll(target) { + t.Fatalf("orginal should contain all hashes in target") + } + if !target.ContainsAll(original) { + t.Fatalf("target should not contain all hashes in orginal") + } + }) + + t.Run("edge case - null", func(t *testing.T) { + original := NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{ + "9r3i9a9QmASqMnQM", + "K43RHM2klOoywtyW", + "swJPXfuCNhJsTM5c", + }) + + if !original.ContainsAll(nil) { + t.Fatalf("orginal should report true on nil") + } + }) + + t.Run("edge case - empty", func(t *testing.T) { + original := NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{ + "9r3i9a9QmASqMnQM", + "K43RHM2klOoywtyW", + "swJPXfuCNhJsTM5c", + }) + + target := NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{}) + + if !original.ContainsAll(target) { + t.Fatalf("orginal should report true on empty") + } + }) + + t.Run("edge case - original empty", func(t *testing.T) { + original := NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{}) + + target := NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{ + "9r3i9a9QmASqMnQM", + "K43RHM2klOoywtyW", + "swJPXfuCNhJsTM5c", + }) + + if original.ContainsAll(target) { + t.Fatalf("orginal should report false when empty") + } + }) +} diff --git a/internal/providercache/installer.go b/internal/providercache/installer.go index e753a73c04af..588101bbce1e 100644 --- a/internal/providercache/installer.go +++ b/internal/providercache/installer.go @@ -464,7 +464,13 @@ NeedProvider: installTo = i.targetDir linkTo = nil // no linking needed } - authResult, err := installTo.InstallPackage(ctx, meta, preferredHashes) + + allowedHashes := preferredHashes + if mode.forceInstallChecksums() { + allowedHashes = []getproviders.Hash{} + } + + authResult, err := installTo.InstallPackage(ctx, meta, allowedHashes) if err != nil { // TODO: Consider retrying for certain kinds of error that seem // likely to be transient. For now, we just treat all errors equally. @@ -594,6 +600,11 @@ const ( // sets. InstallNewProvidersOnly InstallMode = 'N' + // InstallNewProvidersForce is an InstallMode that follows the same + // logic as InstallNewProvidersOnly except it does not verify existing + // checksums but force installs new checksums for all given providers. + InstallNewProvidersForce InstallMode = 'F' + // InstallUpgrades is an InstallMode that causes the installer to check // all requested providers to see if new versions are available that // are also in the given version sets, even if a suitable version of @@ -605,6 +616,10 @@ func (m InstallMode) forceQueryAllProviders() bool { return m == InstallUpgrades } +func (m InstallMode) forceInstallChecksums() bool { + return m == InstallNewProvidersForce +} + // InstallerError is an error type that may be returned (but is not guaranteed) // from Installer.EnsureProviderVersions to indicate potentially several // separate failed installation outcomes for different providers included in diff --git a/internal/providercache/installer_test.go b/internal/providercache/installer_test.go index 0ca70388833d..fbd4f40294be 100644 --- a/internal/providercache/installer_test.go +++ b/internal/providercache/installer_test.go @@ -1364,6 +1364,120 @@ func TestEnsureProviderVersions(t *testing.T) { } }, }, + "force mode ignores hashes": { + Source: getproviders.NewMockSource( + []getproviders.PackageMeta{ + { + Provider: beepProvider, + Version: getproviders.MustParseVersion("1.0.0"), + TargetPlatform: fakePlatform, + Location: beepProviderDir, + }, + }, + nil, + ), + LockFile: ` + provider "example.com/foo/beep" { + version = "1.0.0" + constraints = ">= 1.0.0" + hashes = [ + "h1:does-not-match", + ] + } + `, + Mode: InstallNewProvidersForce, + Reqs: getproviders.Requirements{ + beepProvider: getproviders.MustParseVersionConstraints(">= 1.0.0"), + }, + Check: func(t *testing.T, dir *Dir, locks *depsfile.Locks) { + if allCached := dir.AllAvailablePackages(); len(allCached) != 1 { + t.Errorf("wrong number of cache directory entries; want only one\n%s", spew.Sdump(allCached)) + } + if allLocked := locks.AllProviders(); len(allLocked) != 1 { + t.Errorf("wrong number of provider lock entries; want only one\n%s", spew.Sdump(allLocked)) + } + + gotLock := locks.Provider(beepProvider) + wantLock := depsfile.NewProviderLock( + beepProvider, + getproviders.MustParseVersion("1.0.0"), + getproviders.MustParseVersionConstraints(">= 1.0.0"), + []getproviders.Hash{beepProviderHash, "h1:does-not-match"}, + ) + if diff := cmp.Diff(wantLock, gotLock, depsfile.ProviderLockComparer); diff != "" { + t.Errorf("wrong lock entry\n%s", diff) + } + + gotEntry := dir.ProviderLatestVersion(beepProvider) + wantEntry := &CachedProvider{ + Provider: beepProvider, + Version: getproviders.MustParseVersion("1.0.0"), + PackageDir: filepath.Join(dir.BasePath(), "example.com/foo/beep/1.0.0/bleep_bloop"), + } + if diff := cmp.Diff(wantEntry, gotEntry); diff != "" { + t.Errorf("wrong cache entry\n%s", diff) + } + }, + WantEvents: func(inst *Installer, dir *Dir) map[addrs.Provider][]*testInstallerEventLogItem { + return map[addrs.Provider][]*testInstallerEventLogItem{ + noProvider: { + { + Event: "PendingProviders", + Args: map[addrs.Provider]getproviders.VersionConstraints{ + beepProvider: getproviders.MustParseVersionConstraints(">= 1.0.0"), + }, + }, + { + Event: "ProvidersFetched", + Args: map[addrs.Provider]*getproviders.PackageAuthenticationResult{ + beepProvider: nil, + }, + }, + }, + beepProvider: { + { + Event: "QueryPackagesBegin", + Provider: beepProvider, + Args: struct { + Constraints string + Locked bool + }{">= 1.0.0", true}, + }, + { + Event: "QueryPackagesSuccess", + Provider: beepProvider, + Args: "1.0.0", + }, + { + Event: "FetchPackageMeta", + Provider: beepProvider, + Args: "1.0.0", + }, + { + Event: "FetchPackageBegin", + Provider: beepProvider, + Args: struct { + Version string + Location getproviders.PackageLocation + }{"1.0.0", beepProviderDir}, + }, + { + Event: "FetchPackageSuccess", + Provider: beepProvider, + Args: struct { + Version string + LocalDir string + AuthResult string + }{ + "1.0.0", + filepath.Join(dir.BasePath(), "example.com/foo/beep/1.0.0/bleep_bloop"), + "unauthenticated", + }, + }, + }, + } + }, + }, } ctx := context.Background() From acb8f5bff23883d87d2e4da8e5e4250bb89fcff1 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Wed, 6 Jul 2022 15:30:21 +0100 Subject: [PATCH 2/4] missing new line --- internal/command/testdata/providers-lock/append/main.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/command/testdata/providers-lock/append/main.tf b/internal/command/testdata/providers-lock/append/main.tf index 41b211f26882..d3de379059f4 100644 --- a/internal/command/testdata/providers-lock/append/main.tf +++ b/internal/command/testdata/providers-lock/append/main.tf @@ -4,4 +4,4 @@ terraform { source = "hashicorp/test" } } -} \ No newline at end of file +} From 95e487e9a35f2a9e3f300e20428438ccb081ae11 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Thu, 7 Jul 2022 16:05:24 +0100 Subject: [PATCH 3/4] Fix incorrect logic when deciding change message --- internal/command/providers_lock.go | 58 +++++++++++++----- internal/command/providers_lock_test.go | 81 +++++++++++++++++++++++++ internal/depsfile/locks.go | 2 +- internal/depsfile/locks_test.go | 8 +-- 4 files changed, 128 insertions(+), 21 deletions(-) diff --git a/internal/command/providers_lock.go b/internal/command/providers_lock.go index 897f7220fcda..7dcf20db11b1 100644 --- a/internal/command/providers_lock.go +++ b/internal/command/providers_lock.go @@ -13,6 +13,14 @@ import ( "github.com/hashicorp/terraform/internal/tfdiags" ) +type providersLockChangeType string + +const ( + providersLockChangeTypeNoChange providersLockChangeType = "providersLockChangeTypeNoChange" + providersLockChangeTypeNewProvider providersLockChangeType = "providersLockChangeTypeNewProvider" + providersLockChangeTypeNewHashes providersLockChangeType = "providersLockChangeTypeNewHashes" +) + // ProvidersLockCommand is a Command implementation that implements the // "terraform providers lock" command, which creates or updates the current // configuration's dependency lock file using information from upstream @@ -290,35 +298,28 @@ func (c *ProvidersLockCommand) Run(args []string) int { // At this point, we've merged all the hashes for this (provider, platform) // combo into the combined hashes for this provider. Let's take this // opportunity to print out a summary for this particular combination. - - if oldLock == nil { - // Then we weren't tracking this provider previously so we can write a simple error message. + switch providersLockCalculateChangeType(oldLock, platformLock) { + case providersLockChangeTypeNewProvider: madeAnyChange = true c.Ui.Output( fmt.Sprintf( "- Obtained %s checksums for %s; This was a new provider and the checksums for this platform are now tracked in the lock file", provider.ForDisplay(), platform)) - continue - } - - if oldLock.ContainsAll(platformLock) { - // Then we found new checksums for this provider and platform that weren't tracked previously. + case providersLockChangeTypeNewHashes: madeAnyChange = true c.Ui.Output( fmt.Sprintf( "- Obtained %s checksums for %s; Additional checksums for this platform are now tracked in the lock file", provider.ForDisplay(), platform)) - continue + case providersLockChangeTypeNoChange: + c.Ui.Output( + fmt.Sprintf( + "- Obtained %s checksums for %s; All checksums for this platform were already tracked in the lock file", + provider.ForDisplay(), + platform)) } - - // Finally, if we reach here it means we found no new checksums so let's explain that. - c.Ui.Output( - fmt.Sprintf( - "- Obtained %s checksums for %s; All checksums for this platform were already tracked in the lock file", - provider.ForDisplay(), - platform)) } newLocks.SetProvider(provider, version, constraints, hashes) } @@ -398,3 +399,28 @@ Options: set of target platforms. ` } + +// providersLockCalculateChangeType works out whether there is any difference +// between oldLock and newLock and returns a variable the main function can use +// to decide on which message to print. +// +// One assumption made here that is not obvious without the context from the +// main function is that while platformLock contains the lock information for a +// single platform after the current run, oldLock contains the combined +// information of all platforms from when the versions were last checked. A +// simple equality check is not sufficient for deciding on change as we expect +// that oldLock will be a superset of platformLock if no new hashes have been +// found. +// +// We've separated this function out so we can write unit tests around the +// logic. This function assumes the platformLock is not nil, as the main +// function explicitly checks this before calling this function. +func providersLockCalculateChangeType(oldLock *depsfile.ProviderLock, platformLock *depsfile.ProviderLock) providersLockChangeType { + if oldLock == nil { + return providersLockChangeTypeNewProvider + } + if oldLock.ContainsAll(platformLock) { + return providersLockChangeTypeNoChange + } + return providersLockChangeTypeNewHashes +} diff --git a/internal/command/providers_lock_test.go b/internal/command/providers_lock_test.go index 6725d2984325..e17695f1ed98 100644 --- a/internal/command/providers_lock_test.go +++ b/internal/command/providers_lock_test.go @@ -2,6 +2,9 @@ package command import ( "fmt" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/depsfile" + "github.com/hashicorp/terraform/internal/getproviders" "os" "path/filepath" "runtime" @@ -173,3 +176,81 @@ func TestProvidersLock_args(t *testing.T) { } }) } + +func TestProvidersLockCalculateChangeType(t *testing.T) { + provider := addrs.NewDefaultProvider("provider") + v2 := getproviders.MustParseVersion("2.0.0") + v2EqConstraints := getproviders.MustParseVersionConstraints("2.0.0") + + t.Run("oldLock == nil", func(t *testing.T) { + platformLock := depsfile.NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{ + "9r3i9a9QmASqMnQM", + "K43RHM2klOoywtyW", + "swJPXfuCNhJsTM5c", + }) + + if ct := providersLockCalculateChangeType(nil, platformLock); ct != providersLockChangeTypeNewProvider { + t.Fatalf("output was %s but should be %s", ct, providersLockChangeTypeNewProvider) + } + }) + + t.Run("oldLock == platformLock", func(t *testing.T) { + platformLock := depsfile.NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{ + "9r3i9a9QmASqMnQM", + "K43RHM2klOoywtyW", + "swJPXfuCNhJsTM5c", + }) + + oldLock := depsfile.NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{ + "9r3i9a9QmASqMnQM", + "K43RHM2klOoywtyW", + "swJPXfuCNhJsTM5c", + }) + + if ct := providersLockCalculateChangeType(oldLock, platformLock); ct != providersLockChangeTypeNoChange { + t.Fatalf("output was %s but should be %s", ct, providersLockChangeTypeNoChange) + } + }) + + t.Run("oldLock > platformLock", func(t *testing.T) { + platformLock := depsfile.NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{ + "9r3i9a9QmASqMnQM", + "K43RHM2klOoywtyW", + "swJPXfuCNhJsTM5c", + }) + + oldLock := depsfile.NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{ + "9r3i9a9QmASqMnQM", + "1ZAChGWUMWn4zmIk", + "K43RHM2klOoywtyW", + "HWjRvIuWZ1LVatnc", + "swJPXfuCNhJsTM5c", + "KwhJK4p/U2dqbKhI", + }) + + if ct := providersLockCalculateChangeType(oldLock, platformLock); ct != providersLockChangeTypeNoChange { + t.Fatalf("output was %s but should be %s", ct, providersLockChangeTypeNoChange) + } + }) + + t.Run("oldLock < platformLock", func(t *testing.T) { + platformLock := depsfile.NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{ + "9r3i9a9QmASqMnQM", + "1ZAChGWUMWn4zmIk", + "K43RHM2klOoywtyW", + "HWjRvIuWZ1LVatnc", + "swJPXfuCNhJsTM5c", + "KwhJK4p/U2dqbKhI", + }) + + oldLock := depsfile.NewProviderLock(provider, v2, v2EqConstraints, []getproviders.Hash{ + "9r3i9a9QmASqMnQM", + "K43RHM2klOoywtyW", + "swJPXfuCNhJsTM5c", + }) + + if ct := providersLockCalculateChangeType(oldLock, platformLock); ct != providersLockChangeTypeNewHashes { + t.Fatalf("output was %s but should be %s", ct, providersLockChangeTypeNoChange) + } + }) +} diff --git a/internal/depsfile/locks.go b/internal/depsfile/locks.go index 80f0e984b734..4997c4858f2b 100644 --- a/internal/depsfile/locks.go +++ b/internal/depsfile/locks.go @@ -408,7 +408,7 @@ func (l *ProviderLock) AllHashes() []getproviders.Hash { // // This function assumes the hashes are in each ProviderLock are sorted. // If the ProviderLock was created by the NewProviderLock constructor then -// the hashes are guarenteed to be sorted. +// the hashes are guaranteed to be sorted. func (l *ProviderLock) ContainsAll(target *ProviderLock) bool { if target == nil || len(target.hashes) == 0 { return true diff --git a/internal/depsfile/locks_test.go b/internal/depsfile/locks_test.go index c3eb868ed89c..ce84d2e2e003 100644 --- a/internal/depsfile/locks_test.go +++ b/internal/depsfile/locks_test.go @@ -239,10 +239,10 @@ func TestProviderLockContainsAll(t *testing.T) { }) if !original.ContainsAll(target) { - t.Fatalf("orginal should contain all hashes in target") + t.Errorf("orginal should contain all hashes in target") } if target.ContainsAll(original) { - t.Fatalf("target should not contain all hashes in orginal") + t.Errorf("target should not contain all hashes in orginal") } }) @@ -260,10 +260,10 @@ func TestProviderLockContainsAll(t *testing.T) { }) if !original.ContainsAll(target) { - t.Fatalf("orginal should contain all hashes in target") + t.Errorf("orginal should contain all hashes in target") } if !target.ContainsAll(original) { - t.Fatalf("target should not contain all hashes in orginal") + t.Errorf("target should not contain all hashes in orginal") } }) From d50713995e4239cddfc2287066ae23c7b90aea77 Mon Sep 17 00:00:00 2001 From: Liam Cervante Date: Thu, 7 Jul 2022 17:00:37 +0100 Subject: [PATCH 4/4] fix imports --- internal/command/providers_lock_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/command/providers_lock_test.go b/internal/command/providers_lock_test.go index e17695f1ed98..5ba792b7e62b 100644 --- a/internal/command/providers_lock_test.go +++ b/internal/command/providers_lock_test.go @@ -2,15 +2,15 @@ package command import ( "fmt" - "github.com/hashicorp/terraform/internal/addrs" - "github.com/hashicorp/terraform/internal/depsfile" - "github.com/hashicorp/terraform/internal/getproviders" "os" "path/filepath" "runtime" "strings" "testing" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/depsfile" + "github.com/hashicorp/terraform/internal/getproviders" "github.com/mitchellh/cli" )