providercache: Discard lock entries for unused providers

Previously we would only ever add new lock entries or update existing
ones. However, it's possible that over time a module may _cease_ using
a particular provider, at which point we ought to remove it from the lock
file so that operations won't fail when seeing that the provider cache
directory is inconsistent with the lock file.

Now the provider installer (EnsureProviderVersions) will remove any lock
file entries that relate to providers not included in the given
requirements, which therefore makes the resulting lock file properly match
the set of packages the installer wrote into the cache.

This does potentially mean that someone could inadvertently defeat the
lock by removing a provider dependency, running "terraform init", then
undoing that removal, and finally running "terraform init" again. However,
that seems relatively unlikely compared to the likelihood of removing
a provider and keeping it removed, and in the event it _did_ happen the
changes to the lock entry for that provider would be visible in the diff
of the provider lock file as usual, and so could be noticed in code
review just as for any other change to dependencies.

internal/command/init_test.go

@@ -2203,6 +2203,11 @@ provider "registry.terraform.io/hashicorp/test" {
     "zh:e919b507a91e23a00da5c2c4d0b64bcc7900b68d43b3951ac0f6e5d80387fbdc",
   ]
 }
+`)
+
+	emptyUpdatedLockFile := strings.TrimSpace(`
+# This file is maintained automatically by "terraform init".
+# Manual edits may be lost in future updates.
 `)
 
 	cases := []struct {
@@ -2223,6 +2228,15 @@ provider "registry.terraform.io/hashicorp/test" {
 			ok:        true,
 			want:      updatedLockFile,
 		},
+		{
+			desc:      "unused provider",
+			fixture:   "init-provider-now-unused",
+			providers: map[string][]string{"test": {"1.2.3"}},
+			input:     inputLockFile,
+			args:      []string{},
+			ok:        true,
+			want:      emptyUpdatedLockFile,
+		},
 		{
 			desc:      "readonly",
 			fixture:   "init-provider-lock-file",
@@ -2232,6 +2246,15 @@ provider "registry.terraform.io/hashicorp/test" {
 			ok:        true,
 			want:      inputLockFile,
 		},
+		{
+			desc:      "unused provider readonly",
+			fixture:   "init-provider-now-unused",
+			providers: map[string][]string{"test": {"1.2.3"}},
+			input:     inputLockFile,
+			args:      []string{"-lockfile=readonly"},
+			ok:        false,
+			want:      inputLockFile,
+		},
 		{
 			desc:      "conflict",
 			fixture:   "init-provider-lock-file",

internal/command/testdata/init-provider-now-unused/main.tf

@@ -0,0 +1,3 @@
+# Intentionally blank, but intended to be used in a test case which
+# uses an input lock file which already had an entry for the hashicorp/test
+# provider, and should therefore detect it as no longer used.

internal/depsfile/locks.go

@@ -98,6 +98,23 @@ func (l *Locks) SetProvider(addr addrs.Provider, version getproviders.Version, c
 	return new
 }
 
+// RemoveProvider removes any existing lock file entry for the given provider.
+//
+// If the given provider did not already have a lock entry, RemoveProvider is
+// a no-op.
+//
+// Only lockable providers can be passed to this method. If you pass a
+// non-lockable provider address then this function will panic. Use
+// function ProviderIsLockable to determine whether a particular provider
+// should participate in the version locking mechanism.
+func (l *Locks) RemoveProvider(addr addrs.Provider) {
+	if !ProviderIsLockable(addr) {
+		panic(fmt.Sprintf("Locks.RemoveProvider with non-lockable provider %s", addr))
+	}
+
+	delete(l.providers, addr)
+}
+
 // SetProviderOverridden records that this particular Terraform process will
 // not pay attention to the recorded lock entry for the given provider, and
 // will instead access that provider's functionality in some other special

internal/depsfile/locks_test.go

@@ -3,6 +3,7 @@ package depsfile
 import (
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/getproviders"
 )
@@ -138,3 +139,80 @@ func TestLocksEqualProviderAddress(t *testing.T) {
 		equalProviderAddressBothWays(t, a, b)
 	})
 }
+
+func TestLocksProviderSetRemove(t *testing.T) {
+	beepProvider := addrs.NewDefaultProvider("beep")
+	boopProvider := addrs.NewDefaultProvider("boop")
+	v2 := getproviders.MustParseVersion("2.0.0")
+	v2EqConstraints := getproviders.MustParseVersionConstraints("2.0.0")
+	v2GtConstraints := getproviders.MustParseVersionConstraints(">= 2.0.0")
+	hash := getproviders.HashScheme("test").New("1")
+
+	locks := NewLocks()
+	if got, want := len(locks.AllProviders()), 0; got != want {
+		t.Fatalf("fresh locks object already has providers")
+	}
+
+	locks.SetProvider(boopProvider, v2, v2EqConstraints, []getproviders.Hash{hash})
+	{
+		got := locks.AllProviders()
+		want := map[addrs.Provider]*ProviderLock{
+			boopProvider: {
+				addr:               boopProvider,
+				version:            v2,
+				versionConstraints: v2EqConstraints,
+				hashes:             []getproviders.Hash{hash},
+			},
+		}
+		if diff := cmp.Diff(want, got, ProviderLockComparer); diff != "" {
+			t.Fatalf("wrong providers after SetProvider boop\n%s", diff)
+		}
+	}
+
+	locks.SetProvider(beepProvider, v2, v2GtConstraints, []getproviders.Hash{hash})
+	{
+		got := locks.AllProviders()
+		want := map[addrs.Provider]*ProviderLock{
+			boopProvider: {
+				addr:               boopProvider,
+				version:            v2,
+				versionConstraints: v2EqConstraints,
+				hashes:             []getproviders.Hash{hash},
+			},
+			beepProvider: {
+				addr:               beepProvider,
+				version:            v2,
+				versionConstraints: v2GtConstraints,
+				hashes:             []getproviders.Hash{hash},
+			},
+		}
+		if diff := cmp.Diff(want, got, ProviderLockComparer); diff != "" {
+			t.Fatalf("wrong providers after SetProvider beep\n%s", diff)
+		}
+	}
+
+	locks.RemoveProvider(boopProvider)
+	{
+		got := locks.AllProviders()
+		want := map[addrs.Provider]*ProviderLock{
+			beepProvider: {
+				addr:               beepProvider,
+				version:            v2,
+				versionConstraints: v2GtConstraints,
+				hashes:             []getproviders.Hash{hash},
+			},
+		}
+		if diff := cmp.Diff(want, got, ProviderLockComparer); diff != "" {
+			t.Fatalf("wrong providers after RemoveProvider boop\n%s", diff)
+		}
+	}
+
+	locks.RemoveProvider(beepProvider)
+	{
+		got := locks.AllProviders()
+		want := map[addrs.Provider]*ProviderLock{}
+		if diff := cmp.Diff(want, got, ProviderLockComparer); diff != "" {
+			t.Fatalf("wrong providers after RemoveProvider beep\n%s", diff)
+		}
+	}
+}

internal/providercache/installer.go

@@ -162,8 +162,8 @@ func (i *Installer) EnsureProviderVersions(ctx context.Context, locks *depsfile.
 
 	// We'll work with a copy of the given locks, so we can modify it and
 	// return the updated locks without affecting the caller's object.
-	// We'll add or replace locks in here during our work so that the final
+	// We'll add, replace, or remove locks in here during our work so that the
-	// locks file reflects what the installer has selected.
+	// final locks file reflects what the installer has selected.
 	locks = locks.DeepCopy()
 
 	if cb := evts.PendingProviders; cb != nil {
@@ -562,6 +562,18 @@ NeedProvider:
 		cb(authResults)
 	}
 
+	// Finally, if the lock structure contains locks for any providers that
+	// are no longer needed by this configuration, we'll remove them. This
+	// is important because we will not have installed those providers
+	// above and so a lock file still containing them would make the working
+	// directory invalid: not every provider in the lock file is available
+	// for use.
+	for providerAddr := range locks.AllProviders() {
+		if _, ok := reqs[providerAddr]; !ok {
+			locks.RemoveProvider(providerAddr)
+		}
+	}
+
 	if len(errs) > 0 {
 		return locks, InstallerError{
 			ProviderErrors: errs,

internal/providercache/installer_test.go

@@ -824,6 +824,130 @@ func TestEnsureProviderVersions(t *testing.T) {
 				}
 			},
 		},
+		"remove no-longer-needed provider from lock file": {
+			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:2y06Ykj0FRneZfGCTxI9wRTori8iB7ZL5kQ6YyEnh84=",
+					]
+				}
+				provider "example.com/foo/obsolete" {
+					version     = "2.0.0"
+					constraints = ">= 2.0.0"
+					hashes = [
+						"no:irrelevant",
+					]
+				}
+			`,
+			Mode: InstallNewProvidersOnly,
+			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{"h1:2y06Ykj0FRneZfGCTxI9wRTori8iB7ZL5kQ6YyEnh84="},
+				)
+				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,
+							},
+						},
+					},
+					// Note: intentionally no entries for example.com/foo/obsolete
+					// here, because it's no longer needed and therefore not
+					// installed.
+					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",
+							},
+						},
+					},
+				}
+			},
+		},
 		"failed install of a non-existing built-in provider": {
 			Source: getproviders.NewMockSource(
 				[]getproviders.PackageMeta{},

website/docs/language/files/dependency-lock.mdx

@@ -363,3 +363,55 @@ both `zh:` and `h1:` checksums for each of them in the lock file, thus avoiding
 the case where Terraform will learn about a `h1:` equivalent only at a later
 time. See the `terraform providers lock` documentation for more information on
 this command.
+
+### Providers that are no longer required
+
+If you remove the last dependency on a particular provider from your
+configuration, then `terraform init` will remove any existing lock file entry
+for that provider.
+
+```diff
+--- .terraform.lock.hcl	2020-10-07 16:12:07.539570634 -0700
++++ .terraform.lock.hcl	2020-10-07 16:12:15.267487237 -0700
+@@ -6,26 +6,6 @@
+   ]
+ }
+
+-provider "registry.terraform.io/hashicorp/azurerm" {
+-  version     = "2.30.0"
+-  constraints = "~> 2.12"
+-  hashes = [
+-    "h1:FJwsuowaG5CIdZ0WQyFZH9r6kIJeRKts9+GcRsTz1+Y=",
+-    "h1:c/ntSXrDYM1mUir2KufijYebPcwKqS9CRGd3duDSGfY=",
+-    "h1:yre4Ph76g9H84MbuhZ2z5MuldjSA4FsrX6538O7PCcY=",
+-    "zh:04f0a50bb2ba92f3bea6f0a9e549ace5a4c13ef0cbb6975494cac0ef7d4acb43",
+-    "zh:2082e12548ebcdd6fd73580e83f626ed4ed13f8cdfd51205d8696ffe54f30734",
+-    "zh:246bcc449e9a92679fb30f3c0a77f05513886565e2dcc66b16c4486f51533064",
+-    "zh:24de3930625ac9014594d79bfa42d600eca65e9022b9668b54bfd0d924e21d14",
+-    "zh:2a22893a576ff6f268d9bf81cf4a56406f7ba79f77826f6df51ee787f6d2840a",
+-    "zh:2b27485e19c2aaa9f15f29c4cff46154a9720647610171e30fc6c18ddc42ec28",
+-    "zh:435f24ce1fb2b63f7f02aa3c84ac29c5757cd29ec4d297ed0618423387fe7bd4",
+-    "zh:7d99725923de5240ff8b34b5510569aa4ebdc0bdb27b7bac2aa911a8037a3893",
+-    "zh:7e3b5d0af3b7411dd9dc65ec9ab6caee8c191aee0fa7f20fc4f51716e67f50c0",
+-    "zh:da0af4552bef5a29b88f6a0718253f3bf71ce471c959816eb7602b0dadb469ca",
+-  ]
+-}
+-
+ provider "registry.terraform.io/newrelic/newrelic" {
+   version     = "2.1.2"
+   constraints = "~> 2.1.1"
+```
+
+If you add a new requirement for the same provider at a later date and run
+`terraform init` again, Terraform will treat it as if it were
+[an entirely new provider](#dependency-on-a-new-provider)
+and so will not necessarily select the same version that was previously
+selected and will not be able to verify that the checksums remained unchanged.
+
+-> **Note:** In Terraform v1.0 and earlier, `terraform init` does not
+automatically remove now-unneeded providers from the lock file, and instead
+just ignores them. If you removed a provider dependency while using an
+earlier version of Terraform and then upgraded to Terraform v1.1 or later
+then you may see the error "missing or corrupted provider plugins", referring to
+the stale lock file entries. If so, run `terraform init` with the new Terraform
+version to tidy those unneeded entries and then retry the previous operation.