Ignore existing package hashes for providers lock command (#31389)

* Ignore existing package hashes for  command

* missing new line

* Fix incorrect logic when deciding change message

* fix imports

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
@@ -225,15 +233,15 @@ 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)
 
 		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 +260,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 +282,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 +294,32 @@ 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.
+			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))
+			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))
+			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))
+			}
 		}
 		newLocks.SetProvider(provider, version, constraints, hashes)
 	}
@@ -294,8 +332,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
 }
 
@@ -357,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
+}

internal/command/providers_lock_test.go

@@ -8,6 +8,9 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/hashicorp/terraform/internal/addrs"
+	"github.com/hashicorp/terraform/internal/depsfile"
+	"github.com/hashicorp/terraform/internal/getproviders"
 	"github.com/mitchellh/cli"
 )
 
@@ -33,56 +36,78 @@ 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.
 
 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) {
@@ -151,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)
+		}
+	})
+}

internal/command/testdata/providers-lock/append/main.tf

@@ -0,0 +1,7 @@
+terraform {
+    required_providers {
+        test = {
+            source = "hashicorp/test"
+        }
+    }
+}

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 guaranteed 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