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.

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

providercache: Discard lock entries for unused providers #30192

Merged
merged 1 commit into from
Dec 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions internal/command/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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",
Expand All @@ -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",
Expand Down
3 changes: 3 additions & 0 deletions internal/command/testdata/init-provider-now-unused/main.tf
Original file line number Diff line number Diff line change
@@ -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.
17 changes: 17 additions & 0 deletions internal/depsfile/locks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
78 changes: 78 additions & 0 deletions internal/depsfile/locks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}
}
}
16 changes: 14 additions & 2 deletions internal/providercache/installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
// locks file reflects what the installer has selected.
// We'll add, replace, or remove locks in here during our work so that the
// final locks file reflects what the installer has selected.
locks = locks.DeepCopy()

if cb := evts.PendingProviders; cb != nil {
Expand Down Expand Up @@ -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,
Expand Down
124 changes: 124 additions & 0 deletions internal/providercache/installer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
Expand Down
52 changes: 52 additions & 0 deletions website/docs/language/files/dependency-lock.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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.