From 8410263caa8a42fd8e028bc0cb6dc26a04c14724 Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Tue, 26 May 2026 15:41:27 +0100 Subject: [PATCH 1/9] test: Add tests defining how provider installation behaviour is affected by providers that are either reattached or dev_overrides (#38633) * test: Add test coverage to provider installer logic defining how providers being unmanaged/reattached or dev_overrides impacts the provider installation process These tests show that the two methods of overriding a provider behave differently. This is due to how reattached/unmanaged providers are skipped during the provider installation process whereas development overrides are removed from the provider requirements passed into the installation logic. This results in Terraform treating dev_override providers similarly to a provider that's been completely removed from a configuration. I believe this is a bug and will fix in a future commit, but these tests help to highlight the current problem. --- .../command/e2etest/provider_plugin_test.go | 595 ++++++++++++++++++ internal/command/meta_providers_test.go | 208 ++++++ 2 files changed, 803 insertions(+) create mode 100644 internal/command/meta_providers_test.go diff --git a/internal/command/e2etest/provider_plugin_test.go b/internal/command/e2etest/provider_plugin_test.go index e0cf9ca8a5..b23e6deddf 100644 --- a/internal/command/e2etest/provider_plugin_test.go +++ b/internal/command/e2etest/provider_plugin_test.go @@ -4,13 +4,29 @@ package e2etest import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" "os" "path/filepath" + "slices" "strings" "testing" + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/go-hclog" + "github.com/hashicorp/go-plugin" + + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/e2e" "github.com/hashicorp/terraform/internal/getproviders" + "github.com/hashicorp/terraform/internal/getproviders/providerreqs" + "github.com/hashicorp/terraform/internal/grpcwrap" + tfplugin "github.com/hashicorp/terraform/internal/plugin6" + simple "github.com/hashicorp/terraform/internal/provider-simple-v6" + proto "github.com/hashicorp/terraform/internal/tfplugin6" ) // TestProviderProtocols verifies that Terraform can execute provider plugins @@ -88,3 +104,582 @@ func TestProviderProtocols(t *testing.T) { t.Fatalf("wrong destroy output\nstdout:%s\nstderr:%s", stdout, stderr) } } + +// TestProviderInstall_dev_override verifies provider plugin installation behaviour +// when a dev_override is in use. +func TestProviderInstall_dev_override(t *testing.T) { + if !canRunGoBuild { + // We're running in a separate-build-then-run context, so we can't + // currently execute this test which depends on being able to build + // new executable at runtime. + // + // (See the comment on canRunGoBuild's declaration for more information.) + t.Skip("can't run without building a new provider executable") + } + + fixturePath := "testdata/provider-plugin" // Reused + + // In temp dir create a plugin cache to be used in the test cases. + // The cache is supplied to commands using the -plugin-dir init flag. + // There are 4 providers total: + // - simple provider with versions 1.0.0 and 2.0.0 available + // - simple6 provider with versions 1.0.0 and 2.0.0 available + td := t.TempDir() + providerVersionOld := "1.0.0" + providerVersionNew := "2.0.0" + platform := getproviders.CurrentPlatform.String() + absolutePathToCache := filepath.Join(td, "cache") + simple5Provider := filepath.Join(td, "terraform-provider-simple") + simple5ProviderExe := e2e.GoBuild("github.com/hashicorp/terraform/internal/provider-simple/main", simple5Provider) + for _, v := range []string{providerVersionOld, providerVersionNew} { + dir := filepath.Join(absolutePathToCache, "registry.terraform.io/hashicorp", "simple", v, platform) + if err := os.MkdirAll(dir, os.ModePerm); err != nil { + t.Fatal(err) + } + // Create an executable copy of the simple5ProviderExe file per version in the cache dir + data, err := os.ReadFile(simple5ProviderExe) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "terraform-provider-simple"), data, 0755); err != nil { + t.Fatal(err) + } + } + simple6Provider := filepath.Join(td, "terraform-provider-simple6") + simple6ProviderExe := e2e.GoBuild("github.com/hashicorp/terraform/internal/provider-simple-v6/main", simple6Provider) + for _, v := range []string{providerVersionOld, providerVersionNew} { + dir := filepath.Join(absolutePathToCache, "registry.terraform.io/hashicorp", "simple6", v, platform) + if err := os.MkdirAll(dir, os.ModePerm); err != nil { + t.Fatal(err) + } + // Create an executable copy of the simple6ProviderExe file per version in the cache dir + data, err := os.ReadFile(simple6ProviderExe) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "terraform-provider-simple6"), data, 0755); err != nil { + t.Fatal(err) + } + } + + // Get hashes of 3 of the 4 providers + // These are used when creating or asserting against lock files. + var simple5v1_0_0Hash providerreqs.Hash + var simple6v1_0_0Hash providerreqs.Hash + var simple5v2_0_0Hash providerreqs.Hash + var err error + loc := getproviders.PackageLocalDir(filepath.Join(absolutePathToCache, "registry.terraform.io/hashicorp", "simple", providerVersionOld, platform)) + if simple5v1_0_0Hash, err = getproviders.PackageHash(loc); err != nil { + t.Fatal(err) + } + loc = getproviders.PackageLocalDir(filepath.Join(absolutePathToCache, "registry.terraform.io/hashicorp", "simple", providerVersionNew, platform)) + if simple5v2_0_0Hash, err = getproviders.PackageHash(loc); err != nil { + t.Fatal(err) + } + loc = getproviders.PackageLocalDir(filepath.Join(absolutePathToCache, "registry.terraform.io/hashicorp", "simple6", providerVersionOld, platform)) + if simple6v1_0_0Hash, err = getproviders.PackageHash(loc); err != nil { + t.Fatal(err) + } + + // Set up a reused CLI configuration file that sets simple6 as a dev_override, + // Tests will use this via the TF_CLI_CONFIG_FILE environment variable. + cliCfg := fmt.Sprintf(`provider_installation { + + dev_overrides { + "hashicorp/simple6" = "%s" + } + + # For all other providers, install them directly from their origin provider + # registries as normal. If you omit this, Terraform will _only_ use + # the dev_overrides block, and so no other providers will be available. + direct {} +} +`, filepath.Join(absolutePathToCache, "registry.terraform.io/hashicorp", "simple6", providerVersionOld, platform)) + cliConfigFilePath := filepath.Join(td, "dev_override.tfrc") + if err := os.WriteFile(cliConfigFilePath, []byte(cliCfg), 0644); err != nil { + t.Fatalf("err: %s", err) + } + + t.Run("dev_override not installed during init when provider not present in dependency lock file", func(t *testing.T) { + terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") + tf := e2e.NewBinary(t, terraformBin, fixturePath) + + // There is no lock file present at start + lockFile := filepath.Join(tf.WorkDir(), ".terraform.lock.hcl") + _, err := os.Stat(lockFile) + if err == nil { + t.Fatal("expected error due to file not existing, got no error") + } + if !os.IsNotExist(err) { + t.Fatalf("expected error due to file not existing, got different error: %s", err) + } + + // The simple6 provider is a dev_override + tf.AddEnv("TF_CLI_CONFIG_FILE=" + cliConfigFilePath) + + // The init process should succeed. + stdout, stderr, err := tf.Run("init", fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) + if err != nil { + t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) + } + + // Lockfile is created + // simple provider is installed using the latest, 2.0.0 version, + // but the dev_override simple6 provider is not added to the lockfile. + buf, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("unexpected error accessing lock file: %s", err) + } + buf = bytes.TrimSpace(buf) + + expectedLockFileContent := fmt.Sprintf(`# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/simple" { + version = "2.0.0" + hashes = [ + "%s", + ] +}`, + simple5v2_0_0Hash, + ) + if diff := cmp.Diff(expectedLockFileContent, string(buf)); diff != "" { + t.Errorf("unexpected difference in lock file content: %s", diff) + } + }) + + t.Run("dev_override causes provider to be removed from dependency lock file during init", func(t *testing.T) { + terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") + tf := e2e.NewBinary(t, terraformBin, fixturePath) + + // Lockfile contains both simple and simple6 providers already + priorLockFile := fmt.Sprintf(`# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/simple" { + version = "1.0.0" + hashes = [ + "%s", + ] +} + +provider "registry.terraform.io/hashicorp/simple6" { + version = "1.0.0" + hashes = [ + "%s", + ] +}`, + simple5v1_0_0Hash, + simple6v1_0_0Hash, + ) + lockFile := filepath.Join(tf.WorkDir(), ".terraform.lock.hcl") + if err := os.WriteFile(lockFile, []byte(priorLockFile), 0644); err != nil { + t.Fatalf("error writing prior lock file: %s", err) + } + + // The simple6 provider is a dev_override + tf.AddEnv("TF_CLI_CONFIG_FILE=" + cliConfigFilePath) + + // The init process should succeed. + stdout, stderr, err := tf.Run("init", fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) + if err != nil { + t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) + } + + // Lockfile has been altered to remove the simple6 provider + buf, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("unexpected error accessing lock file: %s", err) + } + buf = bytes.TrimSpace(buf) + expectedLockFile := fmt.Sprintf(`# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/simple" { + version = "1.0.0" + hashes = [ + "%s", + ] +}`, + simple5v1_0_0Hash, + ) + if diff := cmp.Diff(expectedLockFile, string(buf)); diff != "" { + t.Fatalf("unexpected difference in lock file content: %s", diff) + } + }) + + t.Run("dev_override also causes provider to be removed from dependency lock file during init -upgrade", func(t *testing.T) { + terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") + tf := e2e.NewBinary(t, terraformBin, fixturePath) + + // Lockfile contains both simple and simple6 providers already + priorLockFile := fmt.Sprintf(`# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/simple" { + version = "1.0.0" + hashes = [ + "%s", + ] +} + +provider "registry.terraform.io/hashicorp/simple6" { + version = "1.0.0" + hashes = [ + "%s", + ] +}`, + simple5v1_0_0Hash, + simple6v1_0_0Hash, + ) + lockFile := filepath.Join(tf.WorkDir(), ".terraform.lock.hcl") + if err := os.WriteFile(lockFile, []byte(priorLockFile), 0644); err != nil { + t.Fatalf("error writing prior lock file: %s", err) + } + + // The simple6 provider is a dev_override + tf.AddEnv("TF_CLI_CONFIG_FILE=" + cliConfigFilePath) + + // The init -upgrade process should succeed. + stdout, stderr, err := tf.Run("init", "-upgrade", fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) + if err != nil { + t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) + } + + // Lockfile shows evidence of upgrade process + // simple provider is upgraded to the newer 2.0.0 version, + // but the dev_override simple6 provider is removed from the lockfile. + buf, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("unexpected error accessing lock file: %s", err) + } + buf = bytes.TrimSpace(buf) + + expectedLockFileContent := fmt.Sprintf(`# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/simple" { + version = "2.0.0" + hashes = [ + "%s", + ] +}`, + simple5v2_0_0Hash, + ) + if diff := cmp.Diff(expectedLockFileContent, string(buf)); diff != "" { + t.Errorf("unexpected difference in lock file content: %s", diff) + } + }) +} + +// TestProviderInstall_reattached verifies provider plugin installation behaviour +// when a reattached/unmanaged provider is in use. +func TestProviderInstall_reattached(t *testing.T) { + if !canRunGoBuild { + // We're running in a separate-build-then-run context, so we can't + // currently execute this test which depends on being able to build + // new executable at runtime. + // + // (See the comment on canRunGoBuild's declaration for more information.) + t.Skip("can't run without building a new provider executable") + } + + fixturePath := "testdata/provider-plugin" // Reused + + // In temp dir create a plugin cache to be used in the test cases. + // The cache is supplied to commands using the -plugin-dir init flag. + // There are 4 providers total: + // - simple provider with versions 1.0.0 and 2.0.0 available + // - simple6 provider with versions 1.0.0 and 2.0.0 available + td := t.TempDir() + providerVersionOld := "1.0.0" + providerVersionNew := "2.0.0" + platform := getproviders.CurrentPlatform.String() + absolutePathToCache := filepath.Join(td, "cache") + simple5Provider := filepath.Join(td, "terraform-provider-simple") + simple5ProviderExe := e2e.GoBuild("github.com/hashicorp/terraform/internal/provider-simple/main", simple5Provider) + for _, v := range []string{providerVersionOld, providerVersionNew} { + dir := filepath.Join(absolutePathToCache, "registry.terraform.io/hashicorp", "simple", v, platform) + if err := os.MkdirAll(dir, os.ModePerm); err != nil { + t.Fatal(err) + } + // Create an executable copy of the simple5ProviderExe file per version in the cache dir + data, err := os.ReadFile(simple5ProviderExe) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "terraform-provider-simple"), data, 0755); err != nil { + t.Fatal(err) + } + } + simple6Provider := filepath.Join(td, "terraform-provider-simple6") + simple6ProviderExe := e2e.GoBuild("github.com/hashicorp/terraform/internal/provider-simple-v6/main", simple6Provider) + for _, v := range []string{providerVersionOld, providerVersionNew} { + dir := filepath.Join(absolutePathToCache, "registry.terraform.io/hashicorp", "simple6", v, platform) + if err := os.MkdirAll(dir, os.ModePerm); err != nil { + t.Fatal(err) + } + // Create an executable copy of the simple6ProviderExe file per version in the cache dir + data, err := os.ReadFile(simple6ProviderExe) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "terraform-provider-simple6"), data, 0755); err != nil { + t.Fatal(err) + } + } + + // Get hashes of 3 of the 4 providers + // These are used when creating or asserting against lock files. + var simple5v1_0_0Hash providerreqs.Hash + var simple6v1_0_0Hash providerreqs.Hash + var simple5v2_0_0Hash providerreqs.Hash + var err error + loc := getproviders.PackageLocalDir(filepath.Join(absolutePathToCache, "registry.terraform.io/hashicorp", "simple", providerVersionOld, platform)) + if simple5v1_0_0Hash, err = getproviders.PackageHash(loc); err != nil { + t.Fatal(err) + } + loc = getproviders.PackageLocalDir(filepath.Join(absolutePathToCache, "registry.terraform.io/hashicorp", "simple", providerVersionNew, platform)) + if simple5v2_0_0Hash, err = getproviders.PackageHash(loc); err != nil { + t.Fatal(err) + } + loc = getproviders.PackageLocalDir(filepath.Join(absolutePathToCache, "registry.terraform.io/hashicorp", "simple6", providerVersionOld, platform)) + if simple6v1_0_0Hash, err = getproviders.PackageHash(loc); err != nil { + t.Fatal(err) + } + + // Launch a separate simple6 provider process to be re-used as a reattached provider. + // Tests will use this via the TF_REATTACH_PROVIDERS environment variable. + reattachConfig := reattachedProviderForTest(t, addrs.NewDefaultProvider("simple6"), 6) + + t.Run("reattached provider not installed when provider not present in dependency lock file", func(t *testing.T) { + terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") + tf := e2e.NewBinary(t, terraformBin, fixturePath) + + // There is no lock file present at start + lockFile := filepath.Join(tf.WorkDir(), ".terraform.lock.hcl") + _, err := os.Stat(lockFile) + if err == nil { + t.Fatal("expected error due to file not existing, got no error") + } + if !os.IsNotExist(err) { + t.Fatalf("expected error due to file not existing, got different error: %s", err) + } + + // The simple6 provider is reattached/unmanaged + tf.AddEnv("TF_REATTACH_PROVIDERS=" + reattachConfig) + + // The init process should succeed. + stdout, stderr, err := tf.Run("init", fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) + if err != nil { + t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) + } + + // Lock file should have been created + buf, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("unexpected error accessing lock file: %s", err) + } + buf = bytes.TrimSpace(buf) + + // We expect the lock file to not contain the simple6 provider that's being reattached/unmanaged, + // because that provider is skipped during the installation process. + // The simple (v5) provider is installed as usual, pulling in the latest version. + expectedLockFileContent := fmt.Sprintf(`# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/simple" { + version = "2.0.0" + hashes = [ + "%s", + ] +}`, simple5v2_0_0Hash) + + if diff := cmp.Diff(expectedLockFileContent, string(buf)); diff != "" { + t.Errorf("unexpected difference in lock file content: %s", diff) + } + }) + + t.Run("reattached providers do NOT cause provider to be removed from dependency lock file during init", func(t *testing.T) { + terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") + tf := e2e.NewBinary(t, terraformBin, fixturePath) + + // Lockfile contains both simple and simple6 providers already + priorLockFile := fmt.Sprintf(`# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/simple" { + version = "1.0.0" + hashes = [ + "%s", + ] +} + +provider "registry.terraform.io/hashicorp/simple6" { + version = "1.0.0" + hashes = [ + "%s", + ] +}`, + simple5v1_0_0Hash, + simple6v1_0_0Hash, + ) + lockFile := filepath.Join(tf.WorkDir(), ".terraform.lock.hcl") + if err := os.WriteFile(lockFile, []byte(priorLockFile), 0644); err != nil { + t.Fatalf("error writing prior lock file: %s", err) + } + + // The simple6 provider is reattached/unmanaged + tf.AddEnv("TF_REATTACH_PROVIDERS=" + reattachConfig) + + // The init process should succeed. + stdout, stderr, err := tf.Run("init", fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) + if err != nil { + t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) + } + + // Lockfile is unchanged despite use of a reattached/unmanaged simple6 provider + buf, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("unexpected error accessing lock file: %s", err) + } + buf = bytes.TrimSpace(buf) + if diff := cmp.Diff(priorLockFile, string(buf)); diff != "" { + t.Fatalf("unexpected difference in lock file content: %s", diff) + } + }) + + t.Run("reattached providers are unchanged in the dependency lock file during init -upgrade", func(t *testing.T) { + terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") + tf := e2e.NewBinary(t, terraformBin, fixturePath) + + // Lockfile contains both simple and simple6 providers already at older version 1.0.0 + priorLockFile := fmt.Sprintf(`# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/simple" { + version = "1.0.0" + hashes = [ + "%s", + ] +} + +provider "registry.terraform.io/hashicorp/simple6" { + version = "1.0.0" + hashes = [ + "%s", + ] +}`, + simple5v1_0_0Hash, + simple6v1_0_0Hash, + ) + lockFile := filepath.Join(tf.WorkDir(), ".terraform.lock.hcl") + if err := os.WriteFile(lockFile, []byte(priorLockFile), 0644); err != nil { + t.Fatalf("error writing prior lock file: %s", err) + } + + // The simple6 provider is reattached/unmanaged + tf.AddEnv("TF_REATTACH_PROVIDERS=" + reattachConfig) + + // The init -upgrade process should succeed. + stdout, stderr, err := tf.Run("init", "-upgrade", fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) + if err != nil { + t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) + } + + // Lockfile shows evidence of upgrade process + // simple provider is upgraded to the newer 2.0.0 version, + // but the reattached simple6 provider is unchanged due to being reattached. + buf, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("unexpected error accessing lock file: %s", err) + } + buf = bytes.TrimSpace(buf) + + expectedLockFileContent := fmt.Sprintf(`# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/simple" { + version = "2.0.0" + hashes = [ + "%s", + ] +} + +provider "registry.terraform.io/hashicorp/simple6" { + version = "1.0.0" + hashes = [ + "%s", + ] +}`, + simple5v2_0_0Hash, + simple6v1_0_0Hash, + ) + if diff := cmp.Diff(expectedLockFileContent, string(buf)); diff != "" { + t.Errorf("unexpected difference in lock file content: %s", diff) + } + }) +} + +// reattachedProviderForTest launches a provider process and returns a reattach config string +// that can be used as the value for the TF_REATTACH_PROVIDERS environment variable in tests. +// Cleanup of the provider process is handled internally. +func reattachedProviderForTest(t *testing.T, provider addrs.Provider, protocol int) string { + t.Helper() + if !slices.Contains([]int{5, 6}, protocol) { + t.Fatalf("test setup tried to create a provider using protocol version %d, which is unsupported. Choose between 5 and 6.", protocol) + } + + reattachCh := make(chan *plugin.ReattachConfig) + closeCh := make(chan struct{}) + server := &providerServer{ + ProviderServer: grpcwrap.Provider6(simple.Provider()), + } + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(func() { + cancel() + <-closeCh + }) + go plugin.Serve(&plugin.ServeConfig{ + Logger: hclog.New(&hclog.LoggerOptions{ + Name: "plugintest", + Level: hclog.Trace, + Output: io.Discard, + }), + Test: &plugin.ServeTestConfig{ + Context: ctx, + ReattachConfigCh: reattachCh, + CloseCh: closeCh, + }, + GRPCServer: plugin.DefaultGRPCServer, + VersionedPlugins: map[int]plugin.PluginSet{ + 6: { + "provider": &tfplugin.GRPCProviderPlugin{ + GRPCProvider: func() proto.ProviderServer { + return server + }, + }, + }, + }, + }) + config := <-reattachCh + if config == nil { + t.Fatalf("no reattach config received") + } + reattachStr, err := json.Marshal(map[string]reattachConfig{ + provider.String(): { + Protocol: string(config.Protocol), + ProtocolVersion: 6, + Pid: config.Pid, + Test: true, + Addr: reattachConfigAddr{ + Network: config.Addr.Network(), + String: config.Addr.String(), + }, + }, + }) + if err != nil { + t.Fatal(err) + } + return string(reattachStr) +} diff --git a/internal/command/meta_providers_test.go b/internal/command/meta_providers_test.go new file mode 100644 index 0000000000..de04620a07 --- /dev/null +++ b/internal/command/meta_providers_test.go @@ -0,0 +1,208 @@ +// Copyright IBM Corp. 2014, 2026 +// SPDX-License-Identifier: BUSL-1.1 + +package command + +import ( + "slices" + "testing" + + "github.com/apparentlymart/go-versions/versions" + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/go-plugin" + "github.com/hashicorp/terraform/internal/addrs" + "github.com/hashicorp/terraform/internal/depsfile" + "github.com/hashicorp/terraform/internal/getproviders" + "github.com/hashicorp/terraform/internal/getproviders/providerreqs" + "github.com/hashicorp/terraform/internal/providercache" +) + +// Test the impacts of dev_overrides and reattached/unmanaged providers on the provider installation process. +// The locks returned from EnsureProviderVersions are what's saved to the dependency lock file, so we are interested +// in how the pre-existing locks and how providers are overidden impacts the locks returned from that installation process. +func TestEnsureProviderVersions_devOverrideAndReattachedProviders(t *testing.T) { + providerSource := newMockProviderSource(t, map[string][]string{ + "provider-a": {"1.0.0", "2.0.0", "3.0.0", "4.0.0"}, + "provider-b": {"1.0.0", "2.0.0", "3.0.0", "4.0.0"}, + "provider-c": {"1.0.0", "2.0.0", "3.0.0", "4.0.0"}, + "provider-d": {"1.0.0", "2.0.0", "3.0.0", "4.0.0"}, + }) + + providerA := addrs.NewDefaultProvider("provider-a") + providerB := addrs.NewDefaultProvider("provider-b") + providerC := addrs.NewDefaultProvider("provider-c") + providerD := addrs.NewDefaultProvider("provider-d") + + // In all test cases the imagined config required providers A through D at specific versions. + reqs := providerreqs.Requirements{ + providerA: providerreqs.MustParseVersionConstraints("1.0.0"), + providerB: providerreqs.MustParseVersionConstraints("2.0.0"), + providerC: providerreqs.MustParseVersionConstraints("3.0.0"), + providerD: providerreqs.MustParseVersionConstraints("4.0.0"), + } + + // Some tests are installing providers for the first time, and prior locks include only A. + priorLocksJustA := depsfile.NewLocks() + priorLocksJustA.SetProvider( + providerA, + versions.MustParseVersion("1.0.0"), + providerreqs.MustParseVersionConstraints("1.0.0"), + nil, // no hashes needed for this test + ) + + // Other tests are performing an install after all providers (A-D) have already been added to the dependency lock file. + priorLocksABCD := depsfile.NewLocks() + priorLocksABCD.SetProvider( + providerA, + versions.MustParseVersion("1.0.0"), + providerreqs.MustParseVersionConstraints("1.0.0"), + nil, // no hashes needed for this test + ) + priorLocksABCD.SetProvider( + providerB, + versions.MustParseVersion("2.0.0"), + providerreqs.MustParseVersionConstraints("2.0.0"), + nil, // no hashes needed for this test + ) + priorLocksABCD.SetProvider( + providerC, + versions.MustParseVersion("3.0.0"), + providerreqs.MustParseVersionConstraints("3.0.0"), + nil, // no hashes needed for this test + ) + priorLocksABCD.SetProvider( + providerD, + versions.MustParseVersion("4.0.0"), + providerreqs.MustParseVersionConstraints("4.0.0"), + nil, // no hashes needed for this test + ) + + cases := map[string]struct { + providerDevOverrides map[addrs.Provider]getproviders.PackageLocalDir + unmanagedProviders map[addrs.Provider]*plugin.ReattachConfig + + priorLocks *depsfile.Locks + expectedProviderTypesInLocks []string + }{ + "no overrides or unmanaged providers": { + providerDevOverrides: map[addrs.Provider]getproviders.PackageLocalDir{}, + unmanagedProviders: map[addrs.Provider]*plugin.ReattachConfig{}, + priorLocks: priorLocksJustA, // Prior locks contain only provider A. + expectedProviderTypesInLocks: []string{ + // All required providers are installed as expected. + providerA.ForDisplay(), + providerB.ForDisplay(), + providerC.ForDisplay(), + providerD.ForDisplay(), + }, + }, + "reattachment present at first-time installation of provider": { + providerDevOverrides: map[addrs.Provider]getproviders.PackageLocalDir{}, + unmanagedProviders: map[addrs.Provider]*plugin.ReattachConfig{ + providerD: { + Protocol: "grpc", + ProtocolVersion: 6, + Pid: 1234567890, + Test: true, + Addr: nil, + }, + }, + priorLocks: priorLocksJustA, // Prior locks contain only provider A. + expectedProviderTypesInLocks: []string{ + providerA.ForDisplay(), + providerB.ForDisplay(), // New + providerC.ForDisplay(), // New + // D is not installed due to being reattached/unmanaged + }, + }, + "reattachment present at subsequent installation of provider": { + providerDevOverrides: map[addrs.Provider]getproviders.PackageLocalDir{}, + unmanagedProviders: map[addrs.Provider]*plugin.ReattachConfig{ + providerD: { + Protocol: "grpc", + ProtocolVersion: 6, + Pid: 1234567890, + Test: true, + Addr: nil, + }, + }, + priorLocks: priorLocksABCD, // Prior locks include the provider that's being reattached/unmanaged. + expectedProviderTypesInLocks: []string{ + providerA.ForDisplay(), + providerB.ForDisplay(), + providerC.ForDisplay(), + providerD.ForDisplay(), // Pre-existing lock for D is expected to be unaffected by use of reattachment/unmanaged. + }, + }, + "dev override present at first-time installation of provider": { + providerDevOverrides: map[addrs.Provider]getproviders.PackageLocalDir{ + providerD: "/path/to/local/provider-d", + }, + unmanagedProviders: map[addrs.Provider]*plugin.ReattachConfig{}, + priorLocks: priorLocksJustA, // Prior locks contain only provider A. + expectedProviderTypesInLocks: []string{ + providerA.ForDisplay(), + providerB.ForDisplay(), + providerC.ForDisplay(), + providerD.ForDisplay(), // D is installed despite being dev overridden + }, + }, + + "dev override present at subsequent installation of provider": { + providerDevOverrides: map[addrs.Provider]getproviders.PackageLocalDir{ + providerD: "/path/to/local/provider-d", + }, + unmanagedProviders: map[addrs.Provider]*plugin.ReattachConfig{}, + priorLocks: priorLocksABCD, // Prior locks include the provider that's being dev overridden. + expectedProviderTypesInLocks: []string{ + providerA.ForDisplay(), + providerB.ForDisplay(), + providerC.ForDisplay(), + providerD.ForDisplay(), // Pre-existing lock for D is expected to be unaffected by use of dev_override. + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + // Temp dir needed because provider installation process writes to filesystem. + td := t.TempDir() + t.Chdir(td) + + meta := Meta{ + ProviderSource: providerSource, + + ProviderDevOverrides: tc.providerDevOverrides, + UnmanagedProviders: tc.unmanagedProviders, + } + + inst := meta.providerInstaller() + if inst == nil { + t.Fatal("expected installer, got nil") + } + + // We cannot make assertions about internals of the installer resulting from (Meta).providerInstaller(), + // but we can make assertions on outputs from using the installer. Arguably this is more informative. + + ctx := t.Context() + locks, err := inst.EnsureProviderVersions(ctx, tc.priorLocks, reqs, providercache.InstallNewProvidersOnly) + if err != nil { + t.Fatal(err) + } + if locks == nil { + t.Fatal("expected locks, got nil") + } + + var gotProviderTypes []string + for addr := range locks.AllProviders() { + gotProviderTypes = append(gotProviderTypes, addr.ForDisplay()) + } + + slices.Sort(tc.expectedProviderTypesInLocks) + slices.Sort(gotProviderTypes) + if diff := cmp.Diff(tc.expectedProviderTypesInLocks, gotProviderTypes); diff != "" { + t.Errorf("unexpected difference in expected provider types in locks: %s", diff) + } + }) + } +} From 95db2b28ee84946c7d4660bb979303bb1680f233 Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Tue, 26 May 2026 15:58:11 +0100 Subject: [PATCH 2/9] Refactor E2E tests to use new reattached provider test helper (#38643) * test: Update reattachedProviderForTest to return the provider server used, so tests can choose to make assertions against it. Refactor relevant E2E tests to use new reattachedProviderForTest helper function. --- .../e2etest/pluggable_state_store_test.go | 62 +--------- internal/command/e2etest/primary_test.go | 112 +---------------- .../command/e2etest/provider_plugin_test.go | 6 +- internal/command/e2etest/unmanaged_test.go | 114 +----------------- 4 files changed, 10 insertions(+), 284 deletions(-) diff --git a/internal/command/e2etest/pluggable_state_store_test.go b/internal/command/e2etest/pluggable_state_store_test.go index 208a8001fb..f9c389e917 100644 --- a/internal/command/e2etest/pluggable_state_store_test.go +++ b/internal/command/e2etest/pluggable_state_store_test.go @@ -5,11 +5,8 @@ package e2etest import ( "bytes" - "context" - "encoding/json" "errors" "fmt" - "io" "os" "path" "path/filepath" @@ -17,18 +14,12 @@ import ( "testing" "github.com/google/go-cmp/cmp" - "github.com/hashicorp/go-hclog" - "github.com/hashicorp/go-plugin" "github.com/hashicorp/go-version" "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/e2e" "github.com/hashicorp/terraform/internal/getproviders" - "github.com/hashicorp/terraform/internal/grpcwrap" - tfplugin "github.com/hashicorp/terraform/internal/plugin6" - simple "github.com/hashicorp/terraform/internal/provider-simple-v6" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/states/statefile" - proto "github.com/hashicorp/terraform/internal/tfplugin6" ) // Test that users can do the full init-plan-apply workflow with pluggable state storage @@ -51,55 +42,7 @@ func TestPrimary_stateStore_unmanaged_separatePlan(t *testing.T) { terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") tf := e2e.NewBinary(t, terraformBin, fixturePath) - reattachCh := make(chan *plugin.ReattachConfig) - closeCh := make(chan struct{}) - provider := &providerServer{ - ProviderServer: grpcwrap.Provider6(simple.Provider()), - } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - go plugin.Serve(&plugin.ServeConfig{ - Logger: hclog.New(&hclog.LoggerOptions{ - Name: "plugintest", - Level: hclog.Trace, - Output: io.Discard, - }), - Test: &plugin.ServeTestConfig{ - Context: ctx, - ReattachConfigCh: reattachCh, - CloseCh: closeCh, - }, - GRPCServer: plugin.DefaultGRPCServer, - VersionedPlugins: map[int]plugin.PluginSet{ - 6: { - "provider": &tfplugin.GRPCProviderPlugin{ - GRPCProvider: func() proto.ProviderServer { - return provider - }, - }, - }, - }, - }) - config := <-reattachCh - if config == nil { - t.Fatalf("no reattach config received") - } - reattachStr, err := json.Marshal(map[string]reattachConfig{ - "hashicorp/simple6": { - Protocol: string(config.Protocol), - ProtocolVersion: 6, - Pid: config.Pid, - Test: true, - Addr: reattachConfigAddr{ - Network: config.Addr.Network(), - String: config.Addr.String(), - }, - }, - }) - if err != nil { - t.Fatal(err) - } - + reattachStr, provider := reattachedProviderForTest(t, addrs.NewDefaultProvider("simple6"), 6) tf.AddEnv("TF_REATTACH_PROVIDERS=" + string(reattachStr)) // Required for the local state files to be written to the temp directory, @@ -165,9 +108,6 @@ func TestPrimary_stateStore_unmanaged_separatePlan(t *testing.T) { if err != nil { t.Fatalf("unexpected destroy error: %s\nstderr:\n%s\nstdout:\n%s", err, stderr, stdout) } - - cancel() - <-closeCh } // Tests using `terraform workspace` commands in combination with pluggable state storage. diff --git a/internal/command/e2etest/primary_test.go b/internal/command/e2etest/primary_test.go index 06b2db53d3..b475e9ffb5 100644 --- a/internal/command/e2etest/primary_test.go +++ b/internal/command/e2etest/primary_test.go @@ -4,10 +4,7 @@ package e2etest import ( - "context" - "encoding/json" "fmt" - "io" "os" "path/filepath" "reflect" @@ -16,18 +13,13 @@ import ( "testing" "github.com/davecgh/go-spew/spew" - "github.com/hashicorp/go-hclog" - "github.com/hashicorp/go-plugin" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/command" "github.com/hashicorp/terraform/internal/command/clistate" "github.com/hashicorp/terraform/internal/e2e" "github.com/hashicorp/terraform/internal/getproviders" - "github.com/hashicorp/terraform/internal/grpcwrap" "github.com/hashicorp/terraform/internal/plans" - tfplugin "github.com/hashicorp/terraform/internal/plugin6" - simple "github.com/hashicorp/terraform/internal/provider-simple-v6" "github.com/hashicorp/terraform/internal/states/statefile" - proto "github.com/hashicorp/terraform/internal/tfplugin6" "github.com/zclconf/go-cty/cty" ) @@ -422,56 +414,7 @@ func TestPrimary_stateStore_swapProviderSupplyMode_betweenInitAndPlanApply(t *te terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") tf := e2e.NewBinary(t, terraformBin, fixturePath) - reattachCh := make(chan *plugin.ReattachConfig) - closeCh := make(chan struct{}) - provider := &providerServer{ - ProviderServer: grpcwrap.Provider6(simple.Provider()), - } - ctx, cancel := context.WithCancel(context.Background()) - t.Cleanup(cancel) - - go plugin.Serve(&plugin.ServeConfig{ - Logger: hclog.New(&hclog.LoggerOptions{ - Name: "plugintest", - Level: hclog.Trace, - Output: io.Discard, - }), - Test: &plugin.ServeTestConfig{ - Context: ctx, - ReattachConfigCh: reattachCh, - CloseCh: closeCh, - }, - GRPCServer: plugin.DefaultGRPCServer, - VersionedPlugins: map[int]plugin.PluginSet{ - 6: { - "provider": &tfplugin.GRPCProviderPlugin{ - GRPCProvider: func() proto.ProviderServer { - return provider - }, - }, - }, - }, - }) - config := <-reattachCh - if config == nil { - t.Fatalf("no reattach config received") - } - reattachStr, err := json.Marshal(map[string]reattachConfig{ - "hashicorp/simple6": { - Protocol: string(config.Protocol), - ProtocolVersion: 6, - Pid: config.Pid, - Test: true, - Addr: reattachConfigAddr{ - Network: config.Addr.Network(), - String: config.Addr.String(), - }, - }, - }) - if err != nil { - t.Fatal(err) - } - + reattachStr, _ := reattachedProviderForTest(t, addrs.NewDefaultProvider("simple6"), 6) tf.AddEnv("TF_REATTACH_PROVIDERS=" + string(reattachStr)) //// INIT - using reattached provider. @@ -758,56 +701,7 @@ func TestPrimary_stateStore_swapProviderSupplyMode_betweenSuccessiveInits(t *tes terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") tf := e2e.NewBinary(t, terraformBin, fixturePath) - reattachCh := make(chan *plugin.ReattachConfig) - closeCh := make(chan struct{}) - provider := &providerServer{ - ProviderServer: grpcwrap.Provider6(simple.Provider()), - } - ctx, cancel := context.WithCancel(context.Background()) - t.Cleanup(cancel) - - go plugin.Serve(&plugin.ServeConfig{ - Logger: hclog.New(&hclog.LoggerOptions{ - Name: "plugintest", - Level: hclog.Trace, - Output: io.Discard, - }), - Test: &plugin.ServeTestConfig{ - Context: ctx, - ReattachConfigCh: reattachCh, - CloseCh: closeCh, - }, - GRPCServer: plugin.DefaultGRPCServer, - VersionedPlugins: map[int]plugin.PluginSet{ - 6: { - "provider": &tfplugin.GRPCProviderPlugin{ - GRPCProvider: func() proto.ProviderServer { - return provider - }, - }, - }, - }, - }) - config := <-reattachCh - if config == nil { - t.Fatalf("no reattach config received") - } - reattachStr, err := json.Marshal(map[string]reattachConfig{ - "hashicorp/simple6": { - Protocol: string(config.Protocol), - ProtocolVersion: 6, - Pid: config.Pid, - Test: true, - Addr: reattachConfigAddr{ - Network: config.Addr.Network(), - String: config.Addr.String(), - }, - }, - }) - if err != nil { - t.Fatal(err) - } - + reattachStr, _ := reattachedProviderForTest(t, addrs.NewDefaultProvider("simple6"), 6) tf.AddEnv("TF_REATTACH_PROVIDERS=" + string(reattachStr)) //// INIT 1 - using reattached provider. diff --git a/internal/command/e2etest/provider_plugin_test.go b/internal/command/e2etest/provider_plugin_test.go index b23e6deddf..44fe5b1d3f 100644 --- a/internal/command/e2etest/provider_plugin_test.go +++ b/internal/command/e2etest/provider_plugin_test.go @@ -450,7 +450,7 @@ func TestProviderInstall_reattached(t *testing.T) { // Launch a separate simple6 provider process to be re-used as a reattached provider. // Tests will use this via the TF_REATTACH_PROVIDERS environment variable. - reattachConfig := reattachedProviderForTest(t, addrs.NewDefaultProvider("simple6"), 6) + reattachConfig, _ := reattachedProviderForTest(t, addrs.NewDefaultProvider("simple6"), 6) t.Run("reattached provider not installed when provider not present in dependency lock file", func(t *testing.T) { terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") @@ -624,7 +624,7 @@ provider "registry.terraform.io/hashicorp/simple6" { // reattachedProviderForTest launches a provider process and returns a reattach config string // that can be used as the value for the TF_REATTACH_PROVIDERS environment variable in tests. // Cleanup of the provider process is handled internally. -func reattachedProviderForTest(t *testing.T, provider addrs.Provider, protocol int) string { +func reattachedProviderForTest(t *testing.T, provider addrs.Provider, protocol int) (string, *providerServer) { t.Helper() if !slices.Contains([]int{5, 6}, protocol) { t.Fatalf("test setup tried to create a provider using protocol version %d, which is unsupported. Choose between 5 and 6.", protocol) @@ -681,5 +681,5 @@ func reattachedProviderForTest(t *testing.T, provider addrs.Provider, protocol i if err != nil { t.Fatal(err) } - return string(reattachStr) + return string(reattachStr), server } diff --git a/internal/command/e2etest/unmanaged_test.go b/internal/command/e2etest/unmanaged_test.go index ea984d8889..f81f51957d 100644 --- a/internal/command/e2etest/unmanaged_test.go +++ b/internal/command/e2etest/unmanaged_test.go @@ -5,21 +5,13 @@ package e2etest import ( "context" - "encoding/json" - "io" "path/filepath" "strings" "sync" "testing" - "github.com/hashicorp/go-hclog" - "github.com/hashicorp/go-plugin" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/e2e" - "github.com/hashicorp/terraform/internal/grpcwrap" - tfplugin5 "github.com/hashicorp/terraform/internal/plugin" - tfplugin "github.com/hashicorp/terraform/internal/plugin6" - simple5 "github.com/hashicorp/terraform/internal/provider-simple" - simple "github.com/hashicorp/terraform/internal/provider-simple-v6" proto5 "github.com/hashicorp/terraform/internal/tfplugin5" proto "github.com/hashicorp/terraform/internal/tfplugin6" ) @@ -234,55 +226,7 @@ func TestUnmanagedSeparatePlan(t *testing.T) { fixturePath := filepath.Join("testdata", "test-provider") tf := e2e.NewBinary(t, terraformBin, fixturePath) - reattachCh := make(chan *plugin.ReattachConfig) - closeCh := make(chan struct{}) - provider := &providerServer{ - ProviderServer: grpcwrap.Provider6(simple.Provider()), - } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - go plugin.Serve(&plugin.ServeConfig{ - Logger: hclog.New(&hclog.LoggerOptions{ - Name: "plugintest", - Level: hclog.Trace, - Output: io.Discard, - }), - Test: &plugin.ServeTestConfig{ - Context: ctx, - ReattachConfigCh: reattachCh, - CloseCh: closeCh, - }, - GRPCServer: plugin.DefaultGRPCServer, - VersionedPlugins: map[int]plugin.PluginSet{ - 6: { - "provider": &tfplugin.GRPCProviderPlugin{ - GRPCProvider: func() proto.ProviderServer { - return provider - }, - }, - }, - }, - }) - config := <-reattachCh - if config == nil { - t.Fatalf("no reattach config received") - } - reattachStr, err := json.Marshal(map[string]reattachConfig{ - "hashicorp/test": { - Protocol: string(config.Protocol), - ProtocolVersion: 6, - Pid: config.Pid, - Test: true, - Addr: reattachConfigAddr{ - Network: config.Addr.Network(), - String: config.Addr.String(), - }, - }, - }) - if err != nil { - t.Fatal(err) - } - + reattachStr, provider := reattachedProviderForTest(t, addrs.NewDefaultProvider("test"), 6) tf.AddEnv("TF_REATTACH_PROVIDERS=" + string(reattachStr)) //// INIT @@ -329,8 +273,6 @@ func TestUnmanagedSeparatePlan(t *testing.T) { if !provider.ApplyResourceChangeCalled() { t.Error("ApplyResourceChange (destroy) not called on in-process provider") } - cancel() - <-closeCh } func TestUnmanagedSeparatePlan_proto5(t *testing.T) { @@ -339,55 +281,7 @@ func TestUnmanagedSeparatePlan_proto5(t *testing.T) { fixturePath := filepath.Join("testdata", "test-provider") tf := e2e.NewBinary(t, terraformBin, fixturePath) - reattachCh := make(chan *plugin.ReattachConfig) - closeCh := make(chan struct{}) - provider := &providerServer5{ - ProviderServer: grpcwrap.Provider(simple5.Provider()), - } - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - go plugin.Serve(&plugin.ServeConfig{ - Logger: hclog.New(&hclog.LoggerOptions{ - Name: "plugintest", - Level: hclog.Trace, - Output: io.Discard, - }), - Test: &plugin.ServeTestConfig{ - Context: ctx, - ReattachConfigCh: reattachCh, - CloseCh: closeCh, - }, - GRPCServer: plugin.DefaultGRPCServer, - VersionedPlugins: map[int]plugin.PluginSet{ - 5: { - "provider": &tfplugin5.GRPCProviderPlugin{ - GRPCProvider: func() proto5.ProviderServer { - return provider - }, - }, - }, - }, - }) - config := <-reattachCh - if config == nil { - t.Fatalf("no reattach config received") - } - reattachStr, err := json.Marshal(map[string]reattachConfig{ - "hashicorp/test": { - Protocol: string(config.Protocol), - ProtocolVersion: 5, - Pid: config.Pid, - Test: true, - Addr: reattachConfigAddr{ - Network: config.Addr.Network(), - String: config.Addr.String(), - }, - }, - }) - if err != nil { - t.Fatal(err) - } - + reattachStr, provider := reattachedProviderForTest(t, addrs.NewDefaultProvider("test"), 5) // protocol 5 tf.AddEnv("TF_REATTACH_PROVIDERS=" + string(reattachStr)) //// INIT @@ -434,6 +328,4 @@ func TestUnmanagedSeparatePlan_proto5(t *testing.T) { if !provider.ApplyResourceChangeCalled() { t.Error("ApplyResourceChange (destroy) not called on in-process provider") } - cancel() - <-closeCh } From 3b4c2f24270b8feb5dd5ea33bf98cb8a352cb351 Mon Sep 17 00:00:00 2001 From: Samsondeen <40821565+dsa0x@users.noreply.github.com> Date: Wed, 27 May 2026 14:55:48 +0200 Subject: [PATCH 3/9] Fix flaky ordering test (#38650) --- internal/command/test_test.go | 43 ++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/internal/command/test_test.go b/internal/command/test_test.go index ae9f96f121..a802db37ab 100644 --- a/internal/command/test_test.go +++ b/internal/command/test_test.go @@ -1343,30 +1343,41 @@ func TestTest_Parallel(t *testing.T) { // Split the log into lines lines := strings.Split(output, "\n") - // Find the positions of "test_d", "test_c", "test_setup" in the log output - var testDIndex, testCIndex, testSetupIndex int + // Find first positions in the log output + testSetupIndex, testBIndex, testCIndex, testDIndex := -1, -1, -1, -1 for i, line := range lines { - if strings.Contains(line, "run \"setup\"") { - testSetupIndex = i - } else if strings.Contains(line, "run \"test_d\"") { - testDIndex = i - } else if strings.Contains(line, "run \"test_c\"") { - testCIndex = i + switch { + case strings.Contains(line, `run "setup"`): + if testSetupIndex == -1 { + testSetupIndex = i + } + case strings.Contains(line, `run "test_b"`): + if testBIndex == -1 { + testBIndex = i + } + case strings.Contains(line, `run "test_c"`): + if testCIndex == -1 { + testCIndex = i + } + case strings.Contains(line, `run "test_d"`): + if testDIndex == -1 { + testDIndex = i + } } } - if testDIndex == 0 || testCIndex == 0 || testSetupIndex == 0 { - t.Fatalf("test_d, test_c, or test_setup not found in the log output") - } - // Ensure "test_d" appears before "test_c", because test_d has no dependencies, - // and would therefore run in parallel to much earlier tests which test_c depends on. - if testDIndex > testCIndex { - t.Errorf("test_d appears after test_c in the log output") + if testSetupIndex == -1 || testBIndex == -1 || testCIndex == -1 || testDIndex == -1 { + t.Fatalf("test_setup, test_b, test_c, or test_d not found in the log output") } // Ensure "test_d" appears after "test_setup", because they have the same state key if testDIndex < testSetupIndex { - t.Errorf("test_d appears before test_setup in the log output") + t.Errorf("invalid ordering: test_d appears before test_setup in the log output") + } + + // Ensure "test_c" appears after "test_b", because they have the same state key + if testCIndex < testBIndex { + t.Errorf("invalid ordering: test_c appears before test_b in the log output") } } From 81053e9d7015ae4955a0e1214fc6e4856166915a Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Wed, 27 May 2026 14:16:03 +0100 Subject: [PATCH 4/9] fix: Block commands from using an invalid workspace name. (#38594) We're blocking commands from using an invalid workspace name, as this mitigates some risks if the invalid name attempts to cause path traversal. A workspace is selected if its name is in the .terraform/environment file, and something other than Terraform could change the contents of that file. To fix this, we expect users to use Terraform to update that file via Terraform using the `workspace select` command. This will allow the user to select or create a valid workspace instead, while also removing the bad data from the file without expecting the user to know about that file (it's an implementation detail, not a public API!). --- .changes/v1.16/BUG FIXES-20260515-135704.yaml | 5 + internal/command/cloud.go | 3 +- internal/command/meta.go | 34 +++++-- internal/command/meta_test.go | 16 ++- internal/command/workspace_command_test.go | 98 +++++++++++++++++++ internal/command/workspace_list.go | 7 +- internal/command/workspace_new.go | 6 +- internal/command/workspace_select.go | 6 +- internal/stacks/stackmigrate/meta.go | 25 +++-- internal/stacks/stackmigrate/meta_test.go | 11 ++- 10 files changed, 187 insertions(+), 24 deletions(-) create mode 100644 .changes/v1.16/BUG FIXES-20260515-135704.yaml diff --git a/.changes/v1.16/BUG FIXES-20260515-135704.yaml b/.changes/v1.16/BUG FIXES-20260515-135704.yaml new file mode 100644 index 0000000000..704174c593 --- /dev/null +++ b/.changes/v1.16/BUG FIXES-20260515-135704.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'workspace: Terraform will now error if an invalid workspace name becomes selected due to actions performed out-of-band' +time: 2026-05-15T13:57:04.484645+01:00 +custom: + Issue: "38594" diff --git a/internal/command/cloud.go b/internal/command/cloud.go index d9d039fbc7..4fb6f8c582 100644 --- a/internal/command/cloud.go +++ b/internal/command/cloud.go @@ -172,7 +172,8 @@ func (c *CloudCommand) discoverAndConfigure() tfdiags.Diagnostics { currentWorkspace, err := c.Workspace() if err != nil { - // The only possible error here is "you set TF_WORKSPACE badly" + // The only possible errors here are "you set TF_WORKSPACE badly", + // or "someone other than Terraform CLI tampered with your .terraform/environment file". return diags.Append(tfdiags.Sourceless( tfdiags.Error, "Bad current workspace", diff --git a/internal/command/meta.go b/internal/command/meta.go index 8454b2f150..cb8fbaf1af 100644 --- a/internal/command/meta.go +++ b/internal/command/meta.go @@ -201,6 +201,10 @@ type Meta struct { // Override certain behavior for tests within this package testingOverrides *testingOverrides + // Overrides checking the validity of the workspace name set in .terraform/environment (not ENVs). + // This is to enable commands used to recover from invalid workspaces to run without the error blocking them. + bypassWorkspaceNameValidityCheck bool + //---------------------------------------------------------- // Private: do not set these //---------------------------------------------------------- @@ -764,10 +768,12 @@ var errInvalidWorkspaceNameEnvVar = fmt.Errorf("Invalid workspace name set using // Workspace returns the name of the currently configured workspace, corresponding // to the desired named state. +// +// Workspace names are validated via use of the `WorkspaceOverridden` method. func (m *Meta) Workspace() (string, error) { - current, overridden := m.WorkspaceOverridden() - if overridden && !validWorkspaceName(current) { - return "", errInvalidWorkspaceNameEnvVar + current, _, err := m.WorkspaceOverridden() + if err != nil { + return "", err } return current, nil } @@ -775,23 +781,37 @@ func (m *Meta) Workspace() (string, error) { // WorkspaceOverridden returns the name of the currently configured workspace, // corresponding to the desired named state, as well as a bool saying whether // this was set via the TF_WORKSPACE environment variable. -func (m *Meta) WorkspaceOverridden() (string, bool) { +// +// The method also validates the workspace name. If it's invalid, an error is +// returned alongside an empty string. The returned boolean will still reflect +// whether the workspace is set by ENV or not. +func (m *Meta) WorkspaceOverridden() (string, bool, error) { if envVar := os.Getenv(WorkspaceNameEnvVar); envVar != "" { - return envVar, true + if !validWorkspaceName(envVar) { + // Protect against invalid workspace names set via ENV. + return "", true, errInvalidWorkspaceNameEnvVar + } + return envVar, true, nil } - envData, err := ioutil.ReadFile(filepath.Join(m.DataDir(), local.DefaultWorkspaceFile)) + envData, err := os.ReadFile(filepath.Join(m.DataDir(), local.DefaultWorkspaceFile)) current := string(bytes.TrimSpace(envData)) if current == "" { current = backend.DefaultStateName } + if !m.bypassWorkspaceNameValidityCheck && !validWorkspaceName(current) { + // This check is active in every command that uses a backend. + // It is selectively disabled in commands that are recommended for recovering from an invalid workspace. + err := fmt.Errorf("Invalid workspace name: The selected workspace described in %q has an invalid name. This suggests that the file contents were edited by something other than Terraform. To select a different, valid workspace name use commands `terraform workspace select` or `terraform workspace select -or-create`.", filepath.Join(m.DataDir(), local.DefaultWorkspaceFile)) + return "", false, err + } if err != nil && !os.IsNotExist(err) { // always return the default if we can't get a workspace name log.Printf("[ERROR] failed to read current workspace: %s", err) } - return current, false + return current, false, nil } // SetWorkspace saves the given name as the current workspace in the local diff --git a/internal/command/meta_test.go b/internal/command/meta_test.go index 185686ac04..86dd1cc6ab 100644 --- a/internal/command/meta_test.go +++ b/internal/command/meta_test.go @@ -259,6 +259,7 @@ func TestMeta_StatePersistInterval(t *testing.T) { }) } +// Invalid workspace names provided via ENV are validated. func TestMeta_Workspace_override(t *testing.T) { m := new(Meta) @@ -296,7 +297,6 @@ func TestMeta_Workspace_override(t *testing.T) { func TestMeta_Workspace_invalidSelected(t *testing.T) { td := t.TempDir() - os.MkdirAll(td, 0755) t.Chdir(td) // this is an invalid workspace name @@ -311,13 +311,25 @@ func TestMeta_Workspace_invalidSelected(t *testing.T) { if err := os.MkdirAll(DefaultDataDir, 0755); err != nil { t.Fatal(err) } - if err := ioutil.WriteFile(filepath.Join(DefaultDataDir, local.DefaultWorkspaceFile), []byte(workspace), 0644); err != nil { + if err := os.WriteFile(filepath.Join(DefaultDataDir, local.DefaultWorkspaceFile), []byte(workspace), 0644); err != nil { t.Fatal(err) } m := new(Meta) + // Normally, errors are returned when selecting an invalid workspace. ws, err := m.Workspace() + if ws != "" { + t.Errorf("Unexpected workspace\n got: %s\nwant: %s\n", ws, workspace) + } + if err == nil { + t.Errorf("Expected error but got none") + } + + // But it is possible to select an invalid workspace, enabling some + // commands to interact with and correct the issue. + m.bypassWorkspaceNameValidityCheck = true + ws, err = m.Workspace() if ws != workspace { t.Errorf("Unexpected workspace\n got: %s\nwant: %s\n", ws, workspace) } diff --git a/internal/command/workspace_command_test.go b/internal/command/workspace_command_test.go index faa6bf1003..1164629a24 100644 --- a/internal/command/workspace_command_test.go +++ b/internal/command/workspace_command_test.go @@ -1564,3 +1564,101 @@ func TestWorkspace_list_jsonOutput(t *testing.T) { t.Fatalf("expected stderr to be empty, but got: %s", output.Stderr()) } } + +// Show how a user can recover from the .terraform/environment file being edited out-of-band +// and containing an invalid workspace name. In particular this scenario shows that the path +// traversal attempt is blocked and that users can select a different workspace to recover. +func TestInvalidWorkspaceSelectedOutOfBand(t *testing.T) { + td := t.TempDir() + t.Chdir(td) + + // Create some config + cfg := `output "greeting" { + value = "hello" +}` + + if err := os.WriteFile("main.tf", []byte(cfg), 0600); err != nil { + t.Fatal(err) + } + + // Initialize the working directory + ui := new(cli.MockUi) + view, done := testView(t) + meta := Meta{ + Ui: ui, + View: view, + } + initCmd := &InitCommand{ + Meta: meta, + } + if code := initCmd.Run(nil); code != 0 { + t.Fatalf("unexpected non-zero exit code: %d\n output: %s", code, done(t).All()) + } + + // Make a custom workspace. + customWorkspace := "custom" + ui = new(cli.MockUi) + view, done = testView(t) + meta.Ui = ui + meta.View = view + newCmd := &WorkspaceNewCommand{ + Meta: meta, + } + if code := newCmd.Run([]string{customWorkspace}); code != 0 { + t.Fatalf("unexpected non-zero exit code: %d\n output: %s", code, done(t).All()) + } + + // Manually edit the .terraform/environment file to have an invalid workspace name. + invalidWorkspace := "../invalid" + path := filepath.Join(meta.DataDir(), local.DefaultWorkspaceFile) + if err := os.WriteFile(path, []byte(invalidWorkspace), 0600); err != nil { + t.Fatal(err) + } + + expectedError := "Invalid workspace name" + + // Errors block users from performing init with the invalid workspace selected. + ui = new(cli.MockUi) + view, done = testView(t) + meta.Ui = ui + meta.View = view + initCmd = &InitCommand{ + Meta: meta, + } + if code := initCmd.Run(nil); code != 1 { + t.Fatalf("expected exit code 1, got %d\n output: %s", code, done(t).All()) + } + if !strings.Contains(done(t).All(), expectedError) { + t.Fatalf("expected error message %q, got %q", expectedError, done(t).All()) + } + + // Errors block users from performing apply with the invalid workspace selected. + ui = new(cli.MockUi) + view, done = testView(t) + meta.Ui = ui + meta.View = view + applyCmd := &ApplyCommand{ + Meta: meta, + } + if code := applyCmd.Run(nil); code != 1 { + t.Fatalf("expected exit code 1, got %d\n output: %s", code, done(t).All()) + } + if !strings.Contains(done(t).All(), expectedError) { + t.Fatalf("expected error message %q, got %q", expectedError, done(t).All()) + } + + // Users can select a different workspace to recover from the issue + ui = new(cli.MockUi) + view, _ = testView(t) + meta.Ui = ui + meta.View = view + selectCmd := &WorkspaceSelectCommand{ + Meta: meta, + } + if code := selectCmd.Run([]string{"default"}); code != 0 { + t.Fatalf("expected exit code 0, got %d\n output: %s", code, ui.ErrorWriter) + } + + // In this scenario the invalid workspace name only exists in the .terraform/environment file. + // Therefore there's no 'bad' workspace that really exists and needs to be deleted. +} diff --git a/internal/command/workspace_list.go b/internal/command/workspace_list.go index ca62958c9f..6b82f41d8b 100644 --- a/internal/command/workspace_list.go +++ b/internal/command/workspace_list.go @@ -75,7 +75,12 @@ func (c *WorkspaceListCommand) Run(rawArgs []string) int { return 1 } - env, isOverridden := c.WorkspaceOverridden() + env, isOverridden, err := c.WorkspaceOverridden() + if err != nil { + diags = diags.Append(err) + view.List("", nil, diags) + return 1 + } if isOverridden { warn := tfdiags.Sourceless( diff --git a/internal/command/workspace_new.go b/internal/command/workspace_new.go index 7049837f7b..1ed859d14e 100644 --- a/internal/command/workspace_new.go +++ b/internal/command/workspace_new.go @@ -44,7 +44,11 @@ func (c *WorkspaceNewCommand) Run(rawArgs []string) int { // You can't ask to create a workspace when you're overriding the // workspace name to be something different. - if current, isOverridden := c.WorkspaceOverridden(); current != workspace && isOverridden { + // + // Any errors about the ENV's value we can ignore as we're erroring + // already due to it being set. + current, isOverridden, _ := c.WorkspaceOverridden() + if current != workspace && isOverridden { c.Ui.Error(envIsOverriddenNewError) return 1 } diff --git a/internal/command/workspace_select.go b/internal/command/workspace_select.go index dcb731ddea..6be3ec4fe5 100644 --- a/internal/command/workspace_select.go +++ b/internal/command/workspace_select.go @@ -34,13 +34,17 @@ func (c *WorkspaceSelectCommand) Run(rawArgs []string) int { } // Block selecting a workspace if an environment variable will override the new selection anyway. - current, isOverridden := c.WorkspaceOverridden() + // + // We ignore errors raised about the value of the override ENV, or the value of the currently selected + // workspace; this command should help users recover from those errors, not block them. + current, isOverridden, _ := c.WorkspaceOverridden() if isOverridden { c.Ui.Error(envIsOverriddenSelectError) return 1 } // Load the backend + c.bypassWorkspaceNameValidityCheck = true // allow selecting a new workspace when the current one is invalid. configPath := c.WorkingDir.RootModuleDir() b, diags := c.backend(configPath, args.ViewType) if diags.HasErrors() { diff --git a/internal/stacks/stackmigrate/meta.go b/internal/stacks/stackmigrate/meta.go index b4a5bf7f5d..97c8cd9f0c 100644 --- a/internal/stacks/stackmigrate/meta.go +++ b/internal/stacks/stackmigrate/meta.go @@ -6,7 +6,6 @@ package stackmigrate import ( "bytes" "fmt" - "io/ioutil" "log" "net/url" "os" @@ -33,9 +32,9 @@ var errInvalidWorkspaceNameEnvVar = fmt.Errorf("Invalid workspace name set using // Workspace returns the name of the currently configured workspace, corresponding // to the desired named state. func (m *Meta) Workspace() (string, error) { - current, overridden := m.WorkspaceOverridden() - if overridden && !validWorkspaceName(current) { - return "", errInvalidWorkspaceNameEnvVar + current, _, err := m.WorkspaceOverridden() + if err != nil { + return "", err } return current, nil } @@ -43,23 +42,33 @@ func (m *Meta) Workspace() (string, error) { // WorkspaceOverridden returns the name of the currently configured workspace, // corresponding to the desired named state, as well as a bool saying whether // this was set via the TF_WORKSPACE environment variable. -func (m *Meta) WorkspaceOverridden() (string, bool) { +func (m *Meta) WorkspaceOverridden() (string, bool, error) { if envVar := os.Getenv(WorkspaceNameEnvVar); envVar != "" { - return envVar, true + if !validWorkspaceName(envVar) { + // Protect against invalid workspace names set via ENV. + return "", true, errInvalidWorkspaceNameEnvVar + } + return envVar, true, nil } - envData, err := ioutil.ReadFile(filepath.Join(m.DataDir(), local.DefaultWorkspaceFile)) + envData, err := os.ReadFile(filepath.Join(m.DataDir(), local.DefaultWorkspaceFile)) current := string(bytes.TrimSpace(envData)) if current == "" { current = backend.DefaultStateName } + if !validWorkspaceName(current) { + // This check is active in every command that uses a backend. + // It is selectively disabled in commands that are recommended for recovering from an invalid workspace. + err := fmt.Errorf("Invalid workspace name: The selected workspace described in %q has an invalid name. This suggests that the file contents were edited by something other than Terraform. To select a different, valid workspace name use commands `terraform workspace select` or `terraform workspace select -or-create`.", filepath.Join(m.DataDir(), local.DefaultWorkspaceFile)) + return "", false, err + } if err != nil && !os.IsNotExist(err) { // always return the default if we can't get a workspace name log.Printf("[ERROR] failed to read current workspace: %s", err) } - return current, false + return current, false, nil } // fixupMissingWorkingDir is a compensation for various existing tests which diff --git a/internal/stacks/stackmigrate/meta_test.go b/internal/stacks/stackmigrate/meta_test.go index 39cce6c029..f970f174da 100644 --- a/internal/stacks/stackmigrate/meta_test.go +++ b/internal/stacks/stackmigrate/meta_test.go @@ -78,11 +78,16 @@ func TestMeta_Workspace_invalidSelected(t *testing.T) { m := new(Meta) + // Normally, errors are returned when selecting an invalid workspace. ws, err := m.Workspace() - if ws != workspace { + if ws != "" { t.Errorf("Unexpected workspace\n got: %s\nwant: %s\n", ws, workspace) } - if err != nil { - t.Errorf("Unexpected error: %s", err) + if err == nil { + t.Errorf("Expected error but got none") } + + // The Meta in the command package has a field `bypassWorkspaceNameValidityCheck` that allows + // a selected workspace to be invalid in specific commands (i.e. enough to enable users to recover from an invalid value). + // This can be implemented in Stacks when/if needed. } From 234ef96aecbd971fe3062eca53337e95b4cd200f Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Wed, 27 May 2026 16:06:40 +0100 Subject: [PATCH 5/9] init: Make both provider override methods (dev_overrides and reattaching providers) behave same in the context of provider download (#38634) This PR makes a few related changes: 1. Construct provider cache Installers so they have knowledge about dev_override providers Prior to this change, providercache.Installer variables were made in the command package with knowledge about reattached/unmanaged providers, but not with knowledge about dev_override providers. Now both sets of overridden providers are present in the installer and can affect installation logic. 2. Make the provider installation logic skip any dev_override providers from being installed from the Registry (or other configured sources) This means that if a provider is first added to a config while someone uses a dev_override an init command will not add that provider to the lock file. If the overridden provider is already in the lock file then the lock will be unchanged. An edge case, that already exists for unmanaged providers, is that if a dev_override is in play while an init -upgrade command is used, only the providers that aren't overridden or unmanaged will be upgraded This change is coupled to another change in the PR described below. 3. Fix the provider download process (in context of init) so that dev_overrides are not removed from the provider requirements. This reverts a change in https://github.com/hashicorp/terraform/pull/37884. The original motivation of that PR was to address a situation where the provider supplied by dev_overrides isn't published to the Registry yet. The config may need to include an entry in required_providers for that provider, which means that init would always fail due to the provider being unavailable in the Registry for download. The prior commit (bfc08b5d96) changed installer logic to skip dev_override providers, which cancels out this commit's changes; dev_override providers will remain in the provider requirements passed to installation logic, but that logic will now ignore them. As a consequence of no longer removing these provider requirements we will retain any pre-existing locks for the provider through the init process. 4. The `providers locks` command will now warn users about any dev_overrides in effect, as these will stop provider locks from being downloaded. --- .changes/v1.16/BUG FIXES-20260522-180848.yaml | 5 ++ .changes/v1.16/NOTES-20260526-164738.yaml | 5 ++ .../e2etest/pluggable_state_store_test.go | 4 +- internal/command/e2etest/primary_test.go | 20 +++---- .../command/e2etest/provider_plugin_test.go | 57 +++++++++---------- internal/command/init.go | 2 - internal/command/meta_providers.go | 49 ++++++++++------ internal/command/meta_providers_test.go | 2 +- internal/command/providers_lock.go | 5 ++ internal/providercache/installer.go | 19 +++++++ 10 files changed, 107 insertions(+), 61 deletions(-) create mode 100644 .changes/v1.16/BUG FIXES-20260522-180848.yaml create mode 100644 .changes/v1.16/NOTES-20260526-164738.yaml diff --git a/.changes/v1.16/BUG FIXES-20260522-180848.yaml b/.changes/v1.16/BUG FIXES-20260522-180848.yaml new file mode 100644 index 0000000000..c457ee40a2 --- /dev/null +++ b/.changes/v1.16/BUG FIXES-20260522-180848.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'init: Stop removing locks from the dependency lock file corresponding to providers configured as a dev_override' +time: 2026-05-22T18:08:48.823127+01:00 +custom: + Issue: "38634" diff --git a/.changes/v1.16/NOTES-20260526-164738.yaml b/.changes/v1.16/NOTES-20260526-164738.yaml new file mode 100644 index 0000000000..9e59c889c2 --- /dev/null +++ b/.changes/v1.16/NOTES-20260526-164738.yaml @@ -0,0 +1,5 @@ +kind: NOTES +body: 'providers: The `providers locks` command will now warn users about any dev_overrides in effect, as these will stop provider locks from being downloaded.' +time: 2026-05-26T16:47:38.947615+01:00 +custom: + Issue: "38634" diff --git a/internal/command/e2etest/pluggable_state_store_test.go b/internal/command/e2etest/pluggable_state_store_test.go index f9c389e917..73cef31ba2 100644 --- a/internal/command/e2etest/pluggable_state_store_test.go +++ b/internal/command/e2etest/pluggable_state_store_test.go @@ -23,9 +23,9 @@ import ( ) // Test that users can do the full init-plan-apply workflow with pluggable state storage -// when the state storage provider is reattached/unmanaged by Terraform. +// when the state storage provider is unmanaged by Terraform. // As well as ensuring that the state store can be initialised ok, this tests that -// the state store's details can be stored in the plan file despite the fact it's reattached. +// the state store's details can be stored in the plan file despite the fact it's unmanaged. func TestPrimary_stateStore_unmanaged_separatePlan(t *testing.T) { if !canRunGoBuild { // We're running in a separate-build-then-run context, so we can't diff --git a/internal/command/e2etest/primary_test.go b/internal/command/e2etest/primary_test.go index b475e9ffb5..66f0f58eb0 100644 --- a/internal/command/e2etest/primary_test.go +++ b/internal/command/e2etest/primary_test.go @@ -396,9 +396,9 @@ func TestPrimary_stateStore_swapProviderSupplyMode_betweenInitAndPlanApply(t *te // that change doesn't impact the hash of the state store. The hash is impacted by the Version data, and all unmanaged // providers used for PSS will have null version data. // - // In contrast, swapping between a managed provider and any of reattached/dev_override/builtin WILL trigger a hash mismatch + // In contrast, swapping between a managed provider and any of unmanaged/dev_override/builtin WILL trigger a hash mismatch // because the version data will change. - t.Run("users are NOT prompted to migrate state if an unmanaged provider used for PSS provider swaps supply mode (e.g. swap from reattached to dev_override) between init and plan+apply", func(t *testing.T) { + t.Run("users are NOT prompted to migrate state if an unmanaged provider used for PSS provider swaps supply mode (e.g. swap from unmanaged to dev_override) between init and plan+apply", func(t *testing.T) { if !canRunGoBuild { // We're running in a separate-build-then-run context, so we can't // currently execute this test which depends on being able to build @@ -417,13 +417,13 @@ func TestPrimary_stateStore_swapProviderSupplyMode_betweenInitAndPlanApply(t *te reattachStr, _ := reattachedProviderForTest(t, addrs.NewDefaultProvider("simple6"), 6) tf.AddEnv("TF_REATTACH_PROVIDERS=" + string(reattachStr)) - //// INIT - using reattached provider. + //// INIT - using unmanaged provider. _, stderr, err := tf.Run("init", "-enable-pluggable-state-storage-experiment=true", "-no-color") if err != nil { t.Fatalf("unexpected init error: %s\nstderr:\n%s", err, stderr) } - // Assert backend state file says the provider is a reattached + // Assert backend state file says the provider is unmanaged statePath := filepath.Join(tf.WorkDir(), ".terraform", command.DefaultStateFilename) sMgr := &clistate.LocalState{Path: statePath} if err := sMgr.RefreshState(); err != nil { @@ -439,7 +439,7 @@ func TestPrimary_stateStore_swapProviderSupplyMode_betweenInitAndPlanApply(t *te //// PLAN - using same provider but supplied via dev_override instead of reattach config. - // No longer using reattached providers. + // No longer using unmanaged providers. tf.RemoveEnv("TF_REATTACH_PROVIDERS") // Build the provider binary and direct Terraform to use it via dev_override, which should cause Terraform to treat it as a dev_override in a CLI configuration file. @@ -683,9 +683,9 @@ func TestPrimary_stateStore_swapProviderSupplyMode_betweenSuccessiveInits(t *tes // that change doesn't impact the hash of the state store. The hash is impacted by the Version data, and all unmanaged // providers used for PSS will have null version data. // - // In contrast, swapping between a managed provider and any of reattached/dev_override/builtin WILL trigger a hash mismatch + // In contrast, swapping between a managed provider and any of unmanaged/dev_override/builtin WILL trigger a hash mismatch // because the version data will change. - t.Run("users are NOT prompted to migrate state if an unmanaged provider used for PSS provider swaps supply mode (e.g. swap from reattached to dev_override) between init and plan+apply", func(t *testing.T) { + t.Run("users are NOT prompted to migrate state if an unmanaged provider used for PSS provider swaps supply mode (e.g. swap from unmanaged to dev_override) between init and plan+apply", func(t *testing.T) { if !canRunGoBuild { // We're running in a separate-build-then-run context, so we can't // currently execute this test which depends on being able to build @@ -704,13 +704,13 @@ func TestPrimary_stateStore_swapProviderSupplyMode_betweenSuccessiveInits(t *tes reattachStr, _ := reattachedProviderForTest(t, addrs.NewDefaultProvider("simple6"), 6) tf.AddEnv("TF_REATTACH_PROVIDERS=" + string(reattachStr)) - //// INIT 1 - using reattached provider. + //// INIT 1 - using unmanaged provider. _, stderr, err := tf.Run("init", "-enable-pluggable-state-storage-experiment=true", "-no-color") if err != nil { t.Fatalf("unexpected init error: %s\nstderr:\n%s", err, stderr) } - // Assert backend state file says the provider is a reattached + // Assert backend state file says the provider is unmanaged statePath := filepath.Join(tf.WorkDir(), ".terraform", command.DefaultStateFilename) sMgr := &clistate.LocalState{Path: statePath} if err := sMgr.RefreshState(); err != nil { @@ -726,7 +726,7 @@ func TestPrimary_stateStore_swapProviderSupplyMode_betweenSuccessiveInits(t *tes //// INIT 2 - using same provider but supplied via dev_override instead of reattach config. - // No longer using reattached providers. + // No longer using unmanaged providers. tf.RemoveEnv("TF_REATTACH_PROVIDERS") // Build the provider binary and direct Terraform to use it via dev_override, which should cause Terraform to treat it as a dev_override in a CLI configuration file. diff --git a/internal/command/e2etest/provider_plugin_test.go b/internal/command/e2etest/provider_plugin_test.go index 44fe5b1d3f..513804d5d0 100644 --- a/internal/command/e2etest/provider_plugin_test.go +++ b/internal/command/e2etest/provider_plugin_test.go @@ -248,7 +248,7 @@ provider "registry.terraform.io/hashicorp/simple" { } }) - t.Run("dev_override causes provider to be removed from dependency lock file during init", func(t *testing.T) { + t.Run("dev_override providers are still represented in the dependency lock file after init", func(t *testing.T) { terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") tf := e2e.NewBinary(t, terraformBin, fixturePath) @@ -286,29 +286,18 @@ provider "registry.terraform.io/hashicorp/simple6" { t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) } - // Lockfile has been altered to remove the simple6 provider + // Lockfile is unchanged despite use of a dev_override simple6 provider buf, err := os.ReadFile(lockFile) if err != nil { t.Fatalf("unexpected error accessing lock file: %s", err) } buf = bytes.TrimSpace(buf) - expectedLockFile := fmt.Sprintf(`# This file is maintained automatically by "terraform init". -# Manual edits may be lost in future updates. - -provider "registry.terraform.io/hashicorp/simple" { - version = "1.0.0" - hashes = [ - "%s", - ] -}`, - simple5v1_0_0Hash, - ) - if diff := cmp.Diff(expectedLockFile, string(buf)); diff != "" { + if diff := cmp.Diff(priorLockFile, string(buf)); diff != "" { t.Fatalf("unexpected difference in lock file content: %s", diff) } }) - t.Run("dev_override also causes provider to be removed from dependency lock file during init -upgrade", func(t *testing.T) { + t.Run("dev_override providers are unchanged in the dependency lock file during init -upgrade", func(t *testing.T) { terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") tf := e2e.NewBinary(t, terraformBin, fixturePath) @@ -363,8 +352,16 @@ provider "registry.terraform.io/hashicorp/simple" { hashes = [ "%s", ] +} + +provider "registry.terraform.io/hashicorp/simple6" { + version = "1.0.0" + hashes = [ + "%s", + ] }`, simple5v2_0_0Hash, + simple6v1_0_0Hash, // not upgraded to 2.0.0 ) if diff := cmp.Diff(expectedLockFileContent, string(buf)); diff != "" { t.Errorf("unexpected difference in lock file content: %s", diff) @@ -372,9 +369,9 @@ provider "registry.terraform.io/hashicorp/simple" { }) } -// TestProviderInstall_reattached verifies provider plugin installation behaviour -// when a reattached/unmanaged provider is in use. -func TestProviderInstall_reattached(t *testing.T) { +// TestProviderInstall_unmanaged verifies provider plugin installation behaviour +// when an unmanaged provider is in use. +func TestProviderInstall_unmanaged(t *testing.T) { if !canRunGoBuild { // We're running in a separate-build-then-run context, so we can't // currently execute this test which depends on being able to build @@ -448,11 +445,11 @@ func TestProviderInstall_reattached(t *testing.T) { t.Fatal(err) } - // Launch a separate simple6 provider process to be re-used as a reattached provider. + // Launch a separate simple6 provider process to be re-used as an unmanaged provider. // Tests will use this via the TF_REATTACH_PROVIDERS environment variable. reattachConfig, _ := reattachedProviderForTest(t, addrs.NewDefaultProvider("simple6"), 6) - t.Run("reattached provider not installed when provider not present in dependency lock file", func(t *testing.T) { + t.Run("unmanaged provider not installed when provider not present in dependency lock file", func(t *testing.T) { terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") tf := e2e.NewBinary(t, terraformBin, fixturePath) @@ -466,7 +463,7 @@ func TestProviderInstall_reattached(t *testing.T) { t.Fatalf("expected error due to file not existing, got different error: %s", err) } - // The simple6 provider is reattached/unmanaged + // The simple6 provider is unmanaged tf.AddEnv("TF_REATTACH_PROVIDERS=" + reattachConfig) // The init process should succeed. @@ -482,7 +479,7 @@ func TestProviderInstall_reattached(t *testing.T) { } buf = bytes.TrimSpace(buf) - // We expect the lock file to not contain the simple6 provider that's being reattached/unmanaged, + // We expect the lock file to not contain the simple6 provider that's being unmanaged, // because that provider is skipped during the installation process. // The simple (v5) provider is installed as usual, pulling in the latest version. expectedLockFileContent := fmt.Sprintf(`# This file is maintained automatically by "terraform init". @@ -500,7 +497,7 @@ provider "registry.terraform.io/hashicorp/simple" { } }) - t.Run("reattached providers do NOT cause provider to be removed from dependency lock file during init", func(t *testing.T) { + t.Run("unmanaged providers are still represented in the dependency lock file after init", func(t *testing.T) { terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") tf := e2e.NewBinary(t, terraformBin, fixturePath) @@ -529,7 +526,7 @@ provider "registry.terraform.io/hashicorp/simple6" { t.Fatalf("error writing prior lock file: %s", err) } - // The simple6 provider is reattached/unmanaged + // The simple6 provider is unmanaged tf.AddEnv("TF_REATTACH_PROVIDERS=" + reattachConfig) // The init process should succeed. @@ -538,7 +535,7 @@ provider "registry.terraform.io/hashicorp/simple6" { t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) } - // Lockfile is unchanged despite use of a reattached/unmanaged simple6 provider + // Lockfile is unchanged despite use of an unmanaged simple6 provider buf, err := os.ReadFile(lockFile) if err != nil { t.Fatalf("unexpected error accessing lock file: %s", err) @@ -549,7 +546,7 @@ provider "registry.terraform.io/hashicorp/simple6" { } }) - t.Run("reattached providers are unchanged in the dependency lock file during init -upgrade", func(t *testing.T) { + t.Run("unmanaged providers are unchanged in the dependency lock file during init -upgrade", func(t *testing.T) { terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") tf := e2e.NewBinary(t, terraformBin, fixturePath) @@ -578,7 +575,7 @@ provider "registry.terraform.io/hashicorp/simple6" { t.Fatalf("error writing prior lock file: %s", err) } - // The simple6 provider is reattached/unmanaged + // The simple6 provider is unmanaged tf.AddEnv("TF_REATTACH_PROVIDERS=" + reattachConfig) // The init -upgrade process should succeed. @@ -589,7 +586,7 @@ provider "registry.terraform.io/hashicorp/simple6" { // Lockfile shows evidence of upgrade process // simple provider is upgraded to the newer 2.0.0 version, - // but the reattached simple6 provider is unchanged due to being reattached. + // but the unmanaged simple6 provider is unchanged due to being unmanaged. buf, err := os.ReadFile(lockFile) if err != nil { t.Fatalf("unexpected error accessing lock file: %s", err) @@ -613,7 +610,7 @@ provider "registry.terraform.io/hashicorp/simple6" { ] }`, simple5v2_0_0Hash, - simple6v1_0_0Hash, + simple6v1_0_0Hash, // not upgraded to 2.0.0 ) if diff := cmp.Diff(expectedLockFileContent, string(buf)); diff != "" { t.Errorf("unexpected difference in lock file content: %s", diff) @@ -621,7 +618,7 @@ provider "registry.terraform.io/hashicorp/simple6" { }) } -// reattachedProviderForTest launches a provider process and returns a reattach config string +// reattachConfigForTest launches a provider process and returns a reattach config string // that can be used as the value for the TF_REATTACH_PROVIDERS environment variable in tests. // Cleanup of the provider process is handled internally. func reattachedProviderForTest(t *testing.T, provider addrs.Provider, protocol int) (string, *providerServer) { diff --git a/internal/command/init.go b/internal/command/init.go index fdfe291cf3..e8b3885af3 100644 --- a/internal/command/init.go +++ b/internal/command/init.go @@ -401,8 +401,6 @@ func (c *InitCommand) getProvidersFromConfig(ctx context.Context, config *config return false, nil, SafeInitActionInvalid, nil, diags } - reqs = c.removeDevOverrides(reqs) - for providerAddr := range reqs { if providerAddr.IsLegacy() { diags = diags.Append(tfdiags.Sourceless( diff --git a/internal/command/meta_providers.go b/internal/command/meta_providers.go index d194115d54..66621156ce 100644 --- a/internal/command/meta_providers.go +++ b/internal/command/meta_providers.go @@ -18,7 +18,6 @@ import ( builtinProviders "github.com/hashicorp/terraform/internal/builtin/providers" "github.com/hashicorp/terraform/internal/depsfile" "github.com/hashicorp/terraform/internal/getproviders" - "github.com/hashicorp/terraform/internal/getproviders/providerreqs" "github.com/hashicorp/terraform/internal/logging" tfplugin "github.com/hashicorp/terraform/internal/plugin" tfplugin6 "github.com/hashicorp/terraform/internal/plugin6" @@ -74,11 +73,21 @@ func (m *Meta) providerInstallerCustomSource(source getproviders.Source) *provid builtinProviderTypes = append(builtinProviderTypes, ty) } inst.SetBuiltInProviderTypes(builtinProviderTypes) + + // Overridden providers consist of both: + // 1. reattached providers + // 2. development override providers unmanagedProviderTypes := make(map[addrs.Provider]struct{}, len(m.UnmanagedProviders)) for ty := range m.UnmanagedProviders { unmanagedProviderTypes[ty] = struct{}{} } inst.SetUnmanagedProviderTypes(unmanagedProviderTypes) + devOverrideProviderTypes := make(map[addrs.Provider]struct{}, len(m.ProviderDevOverrides)) + for ty := range m.ProviderDevOverrides { + devOverrideProviderTypes[ty] = struct{}{} + } + inst.SetDevOverrideTypes(devOverrideProviderTypes) + return inst } @@ -187,6 +196,29 @@ func (m *Meta) providerDevOverrideInitWarnings() tfdiags.Diagnostics { } } +// providerDevOverrideProviderLockWarnings is just like providerDevOverrideInitWarnings +// except the diagnostic is written with a message specific to the `providers lock` command. +// Similarly, diags will only be returned if there is 1+ dev_overrides in effect, and no error +// diags will be returned. +func (m *Meta) providerDevOverrideProvidersLockWarnings() tfdiags.Diagnostics { + if len(m.ProviderDevOverrides) == 0 { + return nil + } + var detailMsg strings.Builder + detailMsg.WriteString("The following provider development overrides are set in the CLI configuration:\n") + for addr, path := range m.ProviderDevOverrides { + detailMsg.WriteString(fmt.Sprintf(" - %s in %s\n", addr.ForDisplay(), path)) + } + detailMsg.WriteString("\nThese provider locks will not be recorded because the provider is overwritten. If this is unintentional please re-run without the development overrides set.") + return tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Warning, + "Provider development overrides are in effect", + detailMsg.String(), + ), + } +} + func (m *Meta) isProviderDevOverride(pAddr addrs.Provider) bool { if len(m.ProviderDevOverrides) == 0 { return false @@ -195,21 +227,6 @@ func (m *Meta) isProviderDevOverride(pAddr addrs.Provider) bool { return overridden } -func (m *Meta) removeDevOverrides(reqs providerreqs.Requirements) providerreqs.Requirements { - // Deep copy the requirements to avoid mutating the input - copiedReqs := make(providerreqs.Requirements) - for provider, versions := range reqs { - // Only copy if the provider is not overridden - if _, overridden := m.ProviderDevOverrides[provider]; !overridden { - copiedVersions := make(providerreqs.VersionConstraints, len(versions)) - copy(copiedVersions, versions) - copiedReqs[provider] = copiedVersions - } - } - - return copiedReqs -} - // providerDevOverrideRuntimeWarnings returns a diagnostics that contains at // least one warning if and only if there is at least one provider development // override in effect. If not, the result is always empty. The result never diff --git a/internal/command/meta_providers_test.go b/internal/command/meta_providers_test.go index de04620a07..0b50488985 100644 --- a/internal/command/meta_providers_test.go +++ b/internal/command/meta_providers_test.go @@ -144,7 +144,7 @@ func TestEnsureProviderVersions_devOverrideAndReattachedProviders(t *testing.T) providerA.ForDisplay(), providerB.ForDisplay(), providerC.ForDisplay(), - providerD.ForDisplay(), // D is installed despite being dev overridden + // D is not installed due to being dev override }, }, diff --git a/internal/command/providers_lock.go b/internal/command/providers_lock.go index 4430565dd0..09cde2f3ba 100644 --- a/internal/command/providers_lock.go +++ b/internal/command/providers_lock.go @@ -195,6 +195,11 @@ func (c *ProvidersLockCommand) Run(args []string) int { // merge all of the generated locks together at the end. updatedLocks := map[getproviders.Platform]*depsfile.Locks{} selectedVersions := map[addrs.Provider]getproviders.Version{} + + // Warn users about any development overrides in effect; they will block + // locks being obtained for the overridden providers. + c.showDiagnostics(c.providerDevOverrideProvidersLockWarnings()) + for _, platform := range platforms { tempDir, err := ioutil.TempDir("", "terraform-providers-lock") if err != nil { diff --git a/internal/providercache/installer.go b/internal/providercache/installer.go index d375e90ca5..936430ca53 100644 --- a/internal/providercache/installer.go +++ b/internal/providercache/installer.go @@ -58,6 +58,12 @@ type Installer struct { // lifecycle for, and therefore does not need to worry about the // installation of. unmanagedProviderTypes map[addrs.Provider]struct{} + + // devOverrideTypes is a set of provider addresses that should be + // considered implemented. Binaries of these providers are supplied + // from the users machine via CLI configuration, so Terraform does + // not need to worry about installing them. + devOverrideTypes map[addrs.Provider]struct{} } // NewInstaller constructs and returns a new installer with the given target @@ -161,6 +167,15 @@ func (i *Installer) SetUnmanagedProviderTypes(types map[addrs.Provider]struct{}) i.unmanagedProviderTypes = types } +// SetDevOverrideTypes tells the receiver to consider the providers +// indicated by the passed addrs.Providers as dev overrides. Terraform should not +// try to install these providers and record their versions in the dependency lock +// file; the binaries supplied via CLI configuration have no version information +// available. +func (i *Installer) SetDevOverrideTypes(types map[addrs.Provider]struct{}) { + i.devOverrideTypes = types +} + // EnsureProviderVersions compares the given provider requirements with what // is already available in the installer's target directory and then takes // appropriate installation actions to ensure that suitable packages @@ -237,6 +252,10 @@ func (i *Installer) EnsureProviderVersions(ctx context.Context, locks *depsfile. // unmanaged providers do not require installation continue } + if _, ok := i.devOverrideTypes[provider]; ok { + // development override providers do not require installation + continue + } acceptableVersions := versions.MeetingConstraints(versionConstraints) if !mode.forceQueryAllProviders() { // If we're not forcing potential changes of version then an From dcfb4828cc2d46c579d51320a179aea7d604522a Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Wed, 27 May 2026 17:22:25 +0100 Subject: [PATCH 6/9] PSS: Allow users of Terraform in automation to trust the state storage provider when initialising a state store for the first time (#38461) Terraform cannot prompt users to establish trust in a newly downloaded provider used for state storage if input is disabled. This commit's changes enable people using Terraform in automation (defined as `-input=false` being set) to establish trust for a provider in the same situation. We expect users to follow this workflow: 1. Practitioner initialises a Terraform project with minimal config describing how they'd like to use PSS in other production configurations. This is done manually, so they get an interactive prompt for approval and create a lock file describing the downloaded provider. 2. Practitioner copies that .terraform.lock.hcl artefact to a location that can be used in their automation environment. 3. Projects being initialised with PSS for the first time (no prior state) in automation are initialised using the command terraform init -state-provider-lock=. The `-state-provider-lock-file` flag is expected to be a path to a .terraform.lock.hcl file. If the flag is missing it defaults to a path for the project's own lock file. The lock file is expected to describe the provider used for state storage (can be among other locks present in the file) and the version must match the requirements of the config used. Use of the lock file supplied via the flag is at the same point that a user would otherwise interactively approve a provider. If the lock file is insufficient to establish trust it'll be similar to a user declining a prompt to trust a provider. --- internal/command/arguments/init.go | 55 +++ internal/command/arguments/init_test.go | 254 +++++++++- internal/command/init.go | 26 +- internal/command/init_run.go | 78 ++- internal/command/init_test.go | 629 +++++++++++++++++++----- internal/command/views/init.go | 55 ++- 6 files changed, 902 insertions(+), 195 deletions(-) diff --git a/internal/command/arguments/init.go b/internal/command/arguments/init.go index fa49b1adf4..d4f4032253 100644 --- a/internal/command/arguments/init.go +++ b/internal/command/arguments/init.go @@ -6,6 +6,7 @@ package arguments import ( "fmt" "os" + "path/filepath" "time" "github.com/hashicorp/terraform/internal/tfdiags" @@ -76,6 +77,8 @@ type Init struct { Args []string + StateStoreProviderLockFile string + // The -enable-pluggable-state-storage-experiment flag is used in control flow logic in the init command. // TODO(SarahFrench/radeksimko): Remove this once the feature is no longer // experimental @@ -113,6 +116,7 @@ func ParseInit(args []string, experimentsEnabled bool) (*Init, tfdiags.Diagnosti cmdFlags.BoolVar(&init.Json, "json", false, "json") cmdFlags.Var(&init.BackendConfig, "backend-config", "") cmdFlags.Var(&init.PluginPath, "plugin-dir", "plugin directory") + cmdFlags.StringVar(&init.StateStoreProviderLockFile, "state-provider-lock-file", "", "path to the dependency lock file used to establish trust in the provider used for state storage. This flag can only be supplied when input is disabled. Defaults to the working directory's .terraform.lock.hcl file.") // Used for enabling experimental code that's invoked before configuration is parsed. cmdFlags.BoolVar(&init.EnablePssExperiment, "enable-pluggable-state-storage-experiment", false, "Enable the pluggable state storage experiment") @@ -138,6 +142,57 @@ func ParseInit(args []string, experimentsEnabled bool) (*Init, tfdiags.Diagnosti "Terraform cannot use the -enable-pluggable-state-storage-experiment flag (or TF_ENABLE_PLUGGABLE_STATE_STORAGE environment variable) unless experiments are enabled.", )) } + if init.StateStoreProviderLockFile != "" { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Cannot use -state-provider-lock-file flag without experiments enabled", + "Terraform cannot use the -state-provider-lock-file flag unless experiments are enabled.", + )) + } + } + + if !init.EnablePssExperiment && init.StateStoreProviderLockFile != "" { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Cannot use -state-provider-lock-file flag unless the pluggable state storage experiment is enabled", + "Terraform cannot use the -state-provider-lock-file flag unless the pluggable state storage experiment is enabled. Add the -enable-pluggable-state-storage-experiment flag to your command.", + )) + } + + if init.StateStoreProviderLockFile != "" { + if init.InputEnabled { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Cannot use -state-provider-lock-file flag when input is enabled", + "The -state-provider-lock-file flag is only intended to be used when input is disabled. Either remove the flag or add -input=false to your command.", + )) + } + if init.Upgrade { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "The -state-provider-lock-file and -upgrade options are mutually-exclusive", + "The -upgrade flag causes Terraform to ignore prior dependency locks, so it cannot be used in conjunction with the -state-provider-lock-file flag. Remove one flag and try again.", + )) + } + + // Validate that the path uses the expected file name: .terraform.lock.hcl + srcFilename := filepath.Base(init.StateStoreProviderLockFile) + if srcFilename != lockFileName { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid -state-provider-lock-file value", + fmt.Sprintf("Expected lock file name to be %s, got: %s", lockFileName, srcFilename), + )) + } + + // Validate that the file exists + if _, err := os.Stat(init.StateStoreProviderLockFile); os.IsNotExist(err) { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Lock file supplied by -state-provider-lock-file does not exist", + fmt.Sprintf("Terraform cannot find the dependency lock file at %s. Please ensure the file exists and the path is correct.", init.StateStoreProviderLockFile), + )) + } } if init.MigrateState && init.Json { diff --git a/internal/command/arguments/init_test.go b/internal/command/arguments/init_test.go index bb15ab07c7..a2bba958ee 100644 --- a/internal/command/arguments/init_test.go +++ b/internal/command/arguments/init_test.go @@ -4,6 +4,8 @@ package arguments import ( + "fmt" + "os" "strings" "testing" "time" @@ -40,10 +42,12 @@ func TestParseInit_basicValid(t *testing.T) { FlagName: "-backend-config", Items: &flagNameValue, }, - Vars: &Vars{}, - InputEnabled: true, - CompactWarnings: false, - TargetFlags: nil, + Vars: &Vars{}, + InputEnabled: true, + CompactWarnings: false, + TargetFlags: nil, + EnablePssExperiment: false, + StateStoreProviderLockFile: "", }, }, "setting multiple options": { @@ -74,11 +78,13 @@ func TestParseInit_basicValid(t *testing.T) { FlagName: "-backend-config", Items: &flagNameValue, }, - Vars: &Vars{}, - InputEnabled: true, - Args: []string{}, - CompactWarnings: true, - TargetFlags: nil, + Vars: &Vars{}, + InputEnabled: true, + Args: []string{}, + CompactWarnings: true, + TargetFlags: nil, + EnablePssExperiment: false, + StateStoreProviderLockFile: "", }, }, "with cloud option": { @@ -103,11 +109,13 @@ func TestParseInit_basicValid(t *testing.T) { FlagName: "-backend-config", Items: &[]FlagNameValue{{Name: "-backend-config", Value: "backend.config"}}, }, - Vars: &Vars{}, - InputEnabled: false, - Args: []string{}, - CompactWarnings: false, - TargetFlags: []string{"foo_bar.baz"}, + Vars: &Vars{}, + InputEnabled: false, + Args: []string{}, + CompactWarnings: false, + TargetFlags: []string{"foo_bar.baz"}, + EnablePssExperiment: false, + StateStoreProviderLockFile: "", }, }, } @@ -129,6 +137,212 @@ func TestParseInit_basicValid(t *testing.T) { } } +func TestParseInit_stateProviderLockFile(t *testing.T) { + td := t.TempDir() + t.Chdir(td) + + // Create a dummy lock file to test with + lockFileName := ".terraform.lock.hcl" + if err := os.WriteFile(lockFileName, []byte("content doesn't matter!"), 0600); err != nil { + t.Fatalf("unable to create dependency lock file: %v", err) + } + + // Create an existing file with an incorrect file name + invalidLockFileName := "foobar.lock.hcl" + if err := os.WriteFile(invalidLockFileName, []byte("content doesn't matter!"), 0600); err != nil { + t.Fatalf("unable to create dependency lock file: %v", err) + } + + var flagNameValue []FlagNameValue + testCases := map[string]struct { + args []string + expectErr bool + want *Init + }{ + "error when -state-provider-lock-file is used alongside -input=true": { + []string{"-state-provider-lock-file=foobar.lock.hcl", "-upgrade", "-input=true", "-enable-pluggable-state-storage-experiment"}, + true, + &Init{ + FromModule: "", + Lockfile: "", + TestsDirectory: "tests", + ViewType: ViewHuman, + Backend: true, + Cloud: true, + Get: true, + ForceInitCopy: false, + StateLock: true, + StateLockTimeout: 0, + Reconfigure: false, + MigrateState: false, + Upgrade: true, + Json: false, + IgnoreRemoteVersion: false, + BackendConfig: FlagNameValueSlice{ + FlagName: "-backend-config", + Items: &flagNameValue, + }, + Vars: &Vars{}, + InputEnabled: true, + Args: []string{}, + CompactWarnings: false, + TargetFlags: nil, + EnablePssExperiment: true, + StateStoreProviderLockFile: "foobar.lock.hcl", + }, + }, + "error when -state-provider-lock-file is used alongside -upgrade": { + []string{"-state-provider-lock-file=foobar.lock.hcl", "-upgrade", "-input=false", "-enable-pluggable-state-storage-experiment"}, + true, + &Init{ + FromModule: "", + Lockfile: "", + TestsDirectory: "tests", + ViewType: ViewHuman, + Backend: true, + Cloud: true, + Get: true, + ForceInitCopy: false, + StateLock: true, + StateLockTimeout: 0, + Reconfigure: false, + MigrateState: false, + Upgrade: true, + Json: false, + IgnoreRemoteVersion: false, + BackendConfig: FlagNameValueSlice{ + FlagName: "-backend-config", + Items: &flagNameValue, + }, + Vars: &Vars{}, + InputEnabled: false, + Args: []string{}, + CompactWarnings: false, + TargetFlags: nil, + EnablePssExperiment: true, + StateStoreProviderLockFile: "foobar.lock.hcl", + }, + }, + "error when -state-provider-lock-file references a non-existent file": { + []string{"-state-provider-lock-file=nonexistent.lock.hcl", "-input=false", "-enable-pluggable-state-storage-experiment"}, + true, + &Init{ + FromModule: "", + Lockfile: "", + TestsDirectory: "tests", + ViewType: ViewHuman, + Backend: true, + Cloud: true, + Get: true, + ForceInitCopy: false, + StateLock: true, + StateLockTimeout: 0, + Reconfigure: false, + MigrateState: false, + Upgrade: false, + Json: false, + IgnoreRemoteVersion: false, + BackendConfig: FlagNameValueSlice{ + FlagName: "-backend-config", + Items: &flagNameValue, + }, + Vars: &Vars{}, + InputEnabled: false, + Args: []string{}, + CompactWarnings: false, + TargetFlags: nil, + EnablePssExperiment: true, + StateStoreProviderLockFile: "nonexistent.lock.hcl", + }, + }, + "error when -state-provider-lock-file references a file that is not called .terraform.lock.hcl": { + []string{fmt.Sprintf("-state-provider-lock-file=%s", invalidLockFileName), "-input=false", "-enable-pluggable-state-storage-experiment"}, + true, + &Init{ + FromModule: "", + Lockfile: "", + TestsDirectory: "tests", + ViewType: ViewHuman, + Backend: true, + Cloud: true, + Get: true, + ForceInitCopy: false, + StateLock: true, + StateLockTimeout: 0, + Reconfigure: false, + MigrateState: false, + Upgrade: false, + Json: false, + IgnoreRemoteVersion: false, + BackendConfig: FlagNameValueSlice{ + FlagName: "-backend-config", + Items: &flagNameValue, + }, + Vars: &Vars{}, + InputEnabled: false, + Args: []string{}, + CompactWarnings: false, + TargetFlags: nil, + EnablePssExperiment: true, + StateStoreProviderLockFile: invalidLockFileName, + }, + }, + "valid when -state-provider-lock-file references a file that exists": { + []string{fmt.Sprintf("-state-provider-lock-file=%s", lockFileName), "-input=false", "-enable-pluggable-state-storage-experiment"}, + false, + &Init{ + FromModule: "", + Lockfile: "", + TestsDirectory: "tests", + ViewType: ViewHuman, + Backend: true, + Cloud: true, + Get: true, + ForceInitCopy: false, + StateLock: true, + StateLockTimeout: 0, + Reconfigure: false, + MigrateState: false, + Upgrade: false, + Json: false, + IgnoreRemoteVersion: false, + BackendConfig: FlagNameValueSlice{ + FlagName: "-backend-config", + Items: &flagNameValue, + }, + Vars: &Vars{}, + InputEnabled: false, + Args: []string{}, + CompactWarnings: false, + TargetFlags: nil, + EnablePssExperiment: true, + StateStoreProviderLockFile: lockFileName, + }, + }, + } + + cmpOpts := cmpopts.IgnoreUnexported(Vars{}) + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + experimentsEnabled := true + got, diags := ParseInit(tc.args, experimentsEnabled) + if len(diags) > 0 { + if !tc.expectErr { + t.Fatalf("unexpected diags: %v", diags) + } + } + if tc.expectErr && len(diags) == 0 { + t.Fatal("expected error diags but got none") + } + + if diff := cmp.Diff(tc.want, got, cmpOpts); diff != "" { + t.Errorf("unexpected result\n%s", diff) + } + }) + } +} + func TestParseInit_invalid(t *testing.T) { testCases := map[string]struct { args []string @@ -173,7 +387,7 @@ func TestParseInit_invalid(t *testing.T) { t.Fatalf("wrong diags\n got: %s\nwant: %s", got, want) } if got.ViewType != tc.wantViewType { - t.Fatalf("wrong view type, got %#v, want %#v", got.ViewType, ViewHuman) + t.Fatalf("wrong view type, got %#v, want %#v", got.ViewType, tc.wantViewType) } }) } @@ -198,6 +412,16 @@ func TestParseInit_experimentalFlags(t *testing.T) { experimentsEnabled: false, wantErr: "Cannot use -enable-pluggable-state-storage-experiment flag without experiments enabled", }, + "error: -state-provider-lock-file and experiments are disabled": { + args: []string{"-state-provider-lock-file=.terraform.lock.hcl"}, + experimentsEnabled: false, + wantErr: "Cannot use -state-provider-lock-file flag without experiments enabled", + }, + "error: -state-provider-lock-file and -enable-pluggable-state-storage-experiment isn't set": { + args: []string{"-state-provider-lock-file=.terraform.lock.hcl"}, + experimentsEnabled: true, + wantErr: "Cannot use -state-provider-lock-file flag unless the pluggable state storage experiment is enabled", + }, } for name, tc := range testCases { diff --git a/internal/command/init.go b/internal/command/init.go index e8b3885af3..ff5a64a4f7 100644 --- a/internal/command/init.go +++ b/internal/command/init.go @@ -366,10 +366,10 @@ the backend configuration is present and valid. type SafeInitAction rune const ( - SafeInitActionInvalid SafeInitAction = 0 - SafeInitActionProceed SafeInitAction = 'P' - SafeInitActionPromptForInput SafeInitAction = 'I' - SafeInitActionNotRelevant SafeInitAction = 'N' // For when a state store isn't in use at all! + SafeInitActionInvalid SafeInitAction = 0 + SafeInitActionProceed SafeInitAction = 'P' + SafeInitActionRequireApproval SafeInitAction = 'I' + SafeInitActionNotRelevant SafeInitAction = 'N' // For when a state store isn't in use at all! ) // getProvidersFromConfig determines what providers are required by the given configuration data. @@ -378,7 +378,7 @@ const ( // The dependency lock file itself isn't updated here. // // Calling code is responsible for validating inputs to this method, e.g. mutually exclusive flags. -func (c *InitCommand) getProvidersFromConfig(ctx context.Context, config *configs.Config, upgrade bool, pluginDirs []string, flagLockfile string, view views.Init) (output bool, resultingLocks *depsfile.Locks, safeInitAction SafeInitAction, authResult *getproviders.PackageAuthenticationResult, diags tfdiags.Diagnostics) { +func (c *InitCommand) getProvidersFromConfig(ctx context.Context, config *configs.Config, previousLocks *depsfile.Locks, upgrade bool, pluginDirs []string, flagLockfile string, view views.Init) (output bool, resultingLocks *depsfile.Locks, safeInitAction SafeInitAction, authResult *getproviders.PackageAuthenticationResult, diags tfdiags.Diagnostics) { if config == nil { return false, nil, SafeInitActionNotRelevant, nil, diags } @@ -505,13 +505,6 @@ func (c *InitCommand) getProvidersFromConfig(ctx context.Context, config *config mode = providercache.InstallUpgrades } - // Previous locks from dep locks file are needed so we don't re-download any providers - previousLocks, moreDiags := c.lockedDependencies() - diags = diags.Append(moreDiags) - if diags.HasErrors() { - return false, nil, SafeInitActionInvalid, nil, diags - } - // Determine which required providers are already downloaded, and download any // new providers or newer versions of providers configLocks, err := inst.EnsureProviderVersions(ctx, previousLocks, reqs, mode) @@ -557,7 +550,7 @@ func (c *InitCommand) getProvidersFromConfig(ctx context.Context, config *config safeInitAction = SafeInitActionProceed case getproviders.PackageHTTPURL: log.Printf("[DEBUG] init (getProvidersFromConfig): the state storage provider %s (%q) is downloaded via HTTP, so we consider it potentially unsafe.", config.Module.StateStore.ProviderAddr.Type, config.Module.StateStore.ProviderAddr) - safeInitAction = SafeInitActionPromptForInput + safeInitAction = SafeInitActionRequireApproval default: panic(fmt.Sprintf("init (getProvidersFromConfig): unexpected provider location type for state storage provider %q: %T", config.Module.StateStore.ProviderAddr, location)) } @@ -1003,6 +996,13 @@ Options: -enable-pluggable-state-storage-experiment [EXPERIMENTAL] A flag to enable an alternative init command that allows use of pluggable state storage. Only usable with experiments enabled. + + -state-provider-lock-file [EXPERIMENTAL] + Specifies a lock file Terraform should use to establish trust in + a provider before initializing a state store for the first time. + Only usable when input is disabled through -input=false. + Only usable with experiments enabled and the + -enable-pluggable-state-storage-experiment flag present. ` return strings.TrimSpace(helpText) } diff --git a/internal/command/init_run.go b/internal/command/init_run.go index 228a954164..22c95c9056 100644 --- a/internal/command/init_run.go +++ b/internal/command/init_run.go @@ -214,7 +214,49 @@ func (c *InitCommand) run(initArgs *arguments.Init, view views.Init) int { previousLocks, moreDiags := c.lockedDependencies() diags = diags.Append(moreDiags) - configProvidersOutput, configLocks, safeInitAction, stateStoreProviderAuthResult, configProviderDiags := c.getProvidersFromConfig(ctx, config, initArgs.Upgrade, initArgs.PluginPath, initArgs.Lockfile, view) + // If -state-provider-lock-file is set, we'll use that to obtain a new lock used for the state store provider + // This will be 'upserted': it may be that the previous locks don't contain the provider being added. potentially due to being empty, or contain a different version. + // The lock added will be used in the first step of provider download. + // + // We leave `previousLocks` unchanged so it can be used to accurately detect changes to the locks when the lock file is updated later. + alteredPreviousLocks := previousLocks.DeepCopy() + if initArgs.StateStoreProviderLockFile != "" { + stateStoreLocks, lockDiags := depsfile.LoadLocksFromFile(initArgs.StateStoreProviderLockFile) + if lockDiags.HasErrors() { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Error loading -state-provider-lock-file lock file", + fmt.Sprintf("Terraform experienced an error loading the file at %q: %s", initArgs.StateStoreProviderLockFile, lockDiags.Err()), + )) + view.Diagnostics(diags) + return 1 + } + diags = diags.Append(lockDiags) // capture any warnings + + lock := stateStoreLocks.Provider(config.Module.StateStore.ProviderAddr) + if lock == nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "State store provider not found in -state-provider-lock-file dependency lock file", + fmt.Sprintf("Terraform could not find the state store provider %q (%s) in the dependency lock file %q provided via the -state-provider-lock-file flag. Please ensure the lock file contains a lock for the state store provider and try again.", + config.Module.StateStore.ProviderAddr.Type, + config.Module.StateStore.ProviderAddr.ForDisplay(), + initArgs.StateStoreProviderLockFile, + ), + )) + view.Diagnostics(diags) + return 1 + } + + // Overwrite or add the state store provider lock to the other locks for this project + alteredPreviousLocks.SetProvider( + lock.Provider(), + lock.Version(), + lock.VersionConstraints(), + lock.PreferredHashes(), + ) + } + configProvidersOutput, configLocks, safeInitAction, stateStoreProviderAuthResult, configProviderDiags := c.getProvidersFromConfig(ctx, config, alteredPreviousLocks, initArgs.Upgrade, initArgs.PluginPath, initArgs.Lockfile, view) diags = diags.Append(configProviderDiags) if configProviderDiags.HasErrors() { view.Diagnostics(diags) @@ -224,21 +266,37 @@ func (c *InitCommand) run(initArgs *arguments.Init, view views.Init) int { header = true } - // Prompt the user about trusting the provider used for state storage. // Course of action depends on the safeInitAction returned from getProvidersFromConfig switch safeInitAction { case SafeInitActionNotRelevant: // do nothing; security features aren't relevant. case SafeInitActionProceed: // do nothing; provider is already trusted and there's no need to notify the user. - case SafeInitActionPromptForInput: - diags = diags.Append(c.promptStateStorageProviderApproval(config.Module.StateStore.ProviderAddr, configLocks, stateStoreProviderAuthResult)) - if diags.HasErrors() { - view.Output(views.StateStoreProviderRejectedMessage) - view.Diagnostics(diags) - return 1 + case SafeInitActionRequireApproval: + if c.input { + // Prompt the user about trusting the provider used for state storage. + diags = diags.Append(c.promptStateStorageProviderApproval(config.Module.StateStore.ProviderAddr, configLocks, stateStoreProviderAuthResult)) + if diags.HasErrors() { + view.Output(views.StateStoreProviderInteractiveRejectedMessage) + view.Diagnostics(diags) + return 1 + } + view.Output(views.StateStoreProviderInteractiveApprovedMessage) + } else { + // Confirm that a lock was used to control download. + // Note: we have to wait and do that here because at this point we know the provider was downloaded from a source that requires additional info about trust. + if alteredPreviousLocks.Provider(config.Module.StateStore.ProviderAddr) == nil { + // No lock was provided for the state store provider either through pre-existing locks or through the -state-provider-lock-file flag. + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Missing lock for state store provider", + "Terraform is initializing a state store for the first time in a non-interactive mode. In this scenario Terraform needs a pre-existing dependency lock for the state store provider to be present in the working directory's dependency lock file, or present in another file supplied via the -state-provider-lock-file flag. No lock was found for the state store provider. Please re-run the command using the -state-provider-lock-file flag.", + )) + view.Diagnostics(diags) + return 1 + } + view.Output(views.StateStoreProviderAutomationApprovedMessage) } - view.Output(views.StateStoreProviderApprovedMessage) default: // Handle SafeInitActionInvalid or unexpected action types panic(fmt.Sprintf("When installing providers described in the config Terraform couldn't determine what 'safe init' action should be taken and returned action type %T. This is a bug in Terraform and should be reported.", safeInitAction)) @@ -248,7 +306,7 @@ func (c *InitCommand) run(initArgs *arguments.Init, view views.Init) int { // Unless users choose to reconfigure, they must upgrade the state store provider separately using `terraform state migrate -upgrade`. if initArgs.Upgrade && !initArgs.Reconfigure && config.Module.StateStore != nil { pAddr := config.Module.StateStore.ProviderAddr - old := previousLocks.Provider(pAddr) + old := alteredPreviousLocks.Provider(pAddr) new := configLocks.Provider(pAddr) if old == nil || new == nil { panic(fmt.Sprintf(`Unexpected missing provider lock for %s during init -upgrade: diff --git a/internal/command/init_test.go b/internal/command/init_test.go index f3c1b5b003..5e433d6e71 100644 --- a/internal/command/init_test.go +++ b/internal/command/init_test.go @@ -3908,9 +3908,299 @@ func TestInit_testsWithModule(t *testing.T) { } } -// Testing init's behaviors with `state_store` when run in an empty working directory -func TestInit_stateStore_newWorkingDir(t *testing.T) { - t.Run("no need to interactively approve a state store provider installed from local archive", func(t *testing.T) { +// Testing init's basic behaviors with `state_store` when run in an empty working directory: backend state file creation, behaviour based on the selected workspace +func TestInit_stateStore_newWorkingDir_basic(t *testing.T) { + t.Run("the init command creates a backend state file, and the default workspace is not made by default", func(t *testing.T) { + // Create a temporary, uninitialized working directory with configuration including a state store + td := t.TempDir() + testCopyDir(t, testFixturePath("init-with-state-store"), td) + t.Chdir(td) + + mockProvider := mockPluggableStateStorageProvider() + mockProviderAddress := addrs.NewDefaultProvider("test") + providerSource := newMockProviderSource(t, map[string][]string{ + // The test fixture config has no version constraints, so the latest version will + // be used; below is the 'latest' version in the test world. + "hashicorp/test": {"1.2.3"}, + }) + + ui := new(cli.MockUi) + view, done := testView(t) + meta := Meta{ + Ui: ui, + View: view, + AllowExperimentalFeatures: true, + testingOverrides: &testingOverrides{ + Providers: map[addrs.Provider]providers.Factory{ + mockProviderAddress: providers.FactoryFixed(mockProvider), + }, + }, + ProviderSource: providerSource, + } + c := &InitCommand{ + Meta: meta, + } + + args := []string{"-enable-pluggable-state-storage-experiment=true"} + code := c.Run(args) + testOutput := done(t) + if code != 0 { + t.Fatalf("expected code 0 exit code, got %d, output: \n%s", code, testOutput.All()) + } + + // Check output + output := testOutput.All() + expectedOutputs := []string{ + "Initializing the state store...", + "Terraform has been successfully initialized!", + } + for _, expected := range expectedOutputs { + if !strings.Contains(output, expected) { + t.Fatalf("expected output to include %q, but got':\n %s", expected, output) + } + } + + // Assert the default workspace was not created + if _, exists := mockProvider.MockStates[backend.DefaultStateName]; exists { + t.Fatal("expected the default workspace to not be created during init, but it exists") + } + + // Assert contents of the backend state file + statePath := filepath.Join(meta.DataDir(), DefaultStateFilename) + sMgr := &clistate.LocalState{Path: statePath} + if err := sMgr.RefreshState(); err != nil { + t.Fatal("Failed to load state:", err) + } + s := sMgr.State() + if s == nil { + t.Fatal("expected backend state file to be created, but there isn't one") + } + v1_2_3, _ := version.NewVersion("1.2.3") + expectedState := &workdir.StateStoreConfigState{ + Type: "test_store", + ConfigRaw: []byte("{\n \"value\": \"foobar\"\n }"), + Hash: uint64(4158988729), + ProviderSupplyMode: getproviders.ManagedByTerraform, + Provider: &workdir.ProviderConfigState{ + Version: v1_2_3, + Source: &tfaddr.Provider{ + Hostname: tfaddr.DefaultProviderRegistryHost, + Namespace: "hashicorp", + Type: "test", + }, + ConfigRaw: []byte("{\n \"region\": null\n }"), + }, + } + if diff := cmp.Diff(s.StateStore, expectedState); diff != "" { + t.Fatalf("unexpected diff in backend state file's description of state store:\n%s", diff) + } + }) + + // This scenario would be rare, but protecting against it is easy and avoids assumptions. + t.Run("error when a custom workspace is selected but no workspaces exist", func(t *testing.T) { + // Create a temporary, uninitialized working directory with configuration including a state store + td := t.TempDir() + testCopyDir(t, testFixturePath("init-with-state-store"), td) + t.Chdir(td) + + // Select a custom workspace (which will not exist) + customWorkspace := "my-custom-workspace" + t.Setenv(WorkspaceNameEnvVar, customWorkspace) + + mockProvider := mockPluggableStateStorageProvider() + mockProviderAddress := addrs.NewDefaultProvider("test") + providerSource := newMockProviderSource(t, map[string][]string{ + "hashicorp/test": {"1.0.0"}, + }) + + ui := new(cli.MockUi) + view, done := testView(t) + meta := Meta{ + Ui: ui, + View: view, + AllowExperimentalFeatures: true, + testingOverrides: &testingOverrides{ + Providers: map[addrs.Provider]providers.Factory{ + mockProviderAddress: providers.FactoryFixed(mockProvider), + }, + }, + ProviderSource: providerSource, + } + c := &InitCommand{ + Meta: meta, + } + + args := []string{"-enable-pluggable-state-storage-experiment=true"} + code := c.Run(args) + testOutput := done(t) + if code != 1 { + t.Fatalf("expected code 1 exit code, got %d, output: \n%s", code, testOutput.All()) + } + + // Check output + output := testOutput.All() + expectedOutputs := []string{ + fmt.Sprintf("Workspace %q has not been created yet", customWorkspace), + fmt.Sprintf("To create the custom workspace %q use the command `terraform workspace new %s`", customWorkspace, customWorkspace), + } + for _, expected := range expectedOutputs { + if !strings.Contains(cleanString(output), expected) { + t.Fatalf("expected output to include %q, but got':\n %s", expected, cleanString(output)) + } + } + + // Assert no workspaces exist + if len(mockProvider.MockStates) != 0 { + t.Fatalf("expected no workspaces, but got: %#v", mockProvider.MockStates) + } + + // Assert no backend state file made due to the error + statePath := filepath.Join(meta.DataDir(), DefaultStateFilename) + _, err := os.Stat(statePath) + if pathErr, ok := err.(*os.PathError); !ok || !os.IsNotExist(pathErr.Err) { + t.Fatalf("expected backend state file to not be created, but it exists") + } + }) + + // Test what happens when the selected workspace doesn't exist, but there are other workspaces available. + // + // When input is disabled (in automation, etc) Terraform cannot prompts the user to select an alternative. + // Instead, an error is returned. + t.Run("error when input is disabled and the selected workspace doesn't exist and other custom workspaces do exist.", func(t *testing.T) { + // Create a temporary, uninitialized working directory with configuration including a state store + td := t.TempDir() + testCopyDir(t, testFixturePath("init-with-state-store"), td) + t.Chdir(td) + + mockProvider := mockPluggableStateStorageProvider() + mockProvider.GetStatesResponse = &providers.GetStatesResponse{ + States: []string{ + "foobar1", + "foobar2", + // Force provider to report workspaces exist + // But default workspace doesn't exist + }, + } + + mockProviderAddress := addrs.NewDefaultProvider("test") + providerSource := newMockProviderSource(t, map[string][]string{ + "hashicorp/test": {"1.0.0"}, + }) + + ui := new(cli.MockUi) + view, done := testView(t) + meta := Meta{ + Ui: ui, + View: view, + AllowExperimentalFeatures: true, + testingOverrides: &testingOverrides{ + Providers: map[addrs.Provider]providers.Factory{ + mockProviderAddress: providers.FactoryFixed(mockProvider), + }, + }, + ProviderSource: providerSource, + } + c := &InitCommand{ + Meta: meta, + } + + // If input is disabled users receive an error about the missing workspace + args := []string{ + "-enable-pluggable-state-storage-experiment=true", + "-input=false", + } + code := c.Run(args) + testOutput := done(t) + if code != 1 { + t.Fatalf("expected code 1 exit code, got %d, output: \n%s", code, testOutput.All()) + } + output := testOutput.All() + expectedOutput := "Failed to select a workspace: Currently selected workspace \"default\" does not exist" + if !strings.Contains(cleanString(output), expectedOutput) { + t.Fatalf("expected output to include %q, but got':\n %s", expectedOutput, cleanString(output)) + } + statePath := filepath.Join(meta.DataDir(), DefaultStateFilename) + _, err := os.Stat(statePath) + if _, ok := err.(*os.PathError); !ok { + if err == nil { + t.Fatalf("expected backend state file to not be created, but it exists") + } + + t.Fatalf("unexpected error: %s", err) + } + }) + + // Test what happens when the selected workspace doesn't exist, but there are other workspaces available. + // + // When input is enabled Terraform prompts the user to select an alternative. + t.Run("if the selected workspace doesn't exist and other custom workspaces do exist, Terraform prompts the user to select a workspace .", func(t *testing.T) { + // Create a temporary, uninitialized working directory with configuration including a state store + td := t.TempDir() + testCopyDir(t, testFixturePath("init-with-state-store"), td) + t.Chdir(td) + + mockProvider := mockPluggableStateStorageProvider() + mockProvider.GetStatesResponse = &providers.GetStatesResponse{ + States: []string{ + "foobar1", + "foobar2", + // Force provider to report workspaces exist + // But default workspace doesn't exist + }, + } + + mockProviderAddress := addrs.NewDefaultProvider("test") + providerSource := newMockProviderSource(t, map[string][]string{ + "hashicorp/test": {"1.0.0"}, + }) + + // Allow the test to respond to the prompt to pick an + // existing workspace, given the selected one doesn't exist. + _ = testInputMap(t, map[string]string{ + "select-workspace": "1", // foobar1 in numbered list + }) + + ui := new(cli.MockUi) + view, done := testView(t) + meta := Meta{ + Ui: ui, + View: view, + AllowExperimentalFeatures: true, + testingOverrides: &testingOverrides{ + Providers: map[addrs.Provider]providers.Factory{ + mockProviderAddress: providers.FactoryFixed(mockProvider), + }, + }, + ProviderSource: providerSource, + } + c := &InitCommand{ + Meta: meta, + } + + args := []string{ + "-enable-pluggable-state-storage-experiment=true", + } + code := c.Run(args) + testOutput := done(t) + if code != 0 { + t.Fatalf("expected code 0 exit code, got %d, output: \n%s", code, testOutput.All()) + } + + // The init command should have caused the selected workspace to change, based on the input + // provided by the user. + currentWorkspace, err := c.Meta.Workspace() + if err != nil { + t.Fatal(err) + } + if currentWorkspace != "foobar1" { + t.Fatalf("expected init command to alter the selected workspace from 'default' to 'foobar1', but got: %s", currentWorkspace) + } + }) +} + +// Testing init's behaviors when approving a new state store provider when a workspace is initialized for the first time. +func TestInit_stateStore_newWorkingDir_interactiveProviderApproval(t *testing.T) { + t.Run("users do not need to approve trusting a state store provider if it's installed from local archive", func(t *testing.T) { // Create a temporary, uninitialized working directory with configuration including a state store td := t.TempDir() testCopyDir(t, testFixturePath("init-with-state-store"), td) @@ -4314,19 +4604,25 @@ func TestInit_stateStore_newWorkingDir(t *testing.T) { } } }) +} - t.Run("the init command creates a backend state file, and the default workspace is not made by default", func(t *testing.T) { +// Testing init's behaviors when, in automation, we're approving a new state store provider when a workspace is initialized for the first time. +func TestInit_stateStore_newWorkingDir_inAutomationProviderApproval(t *testing.T) { + t.Run("users do not need to approve trusting a state store provider if it's installed from local archive", func(t *testing.T) { // Create a temporary, uninitialized working directory with configuration including a state store td := t.TempDir() testCopyDir(t, testFixturePath("init-with-state-store"), td) t.Chdir(td) - mockProvider := mockPluggableStateStorageProvider() - mockProviderAddress := addrs.NewDefaultProvider("test") - providerSource := newMockProviderSource(t, map[string][]string{ + // This mock provider source makes Terraform think the provider is coming from a local archive, + // so security checks are skipped. + source := newMockProviderSource(t, map[string][]string{ "hashicorp/test": {"1.2.3"}, }) + mockProvider := mockPluggableStateStorageProvider() + mockProviderAddress := addrs.NewDefaultProvider("test") + ui := new(cli.MockUi) view, done := testView(t) meta := Meta{ @@ -4338,13 +4634,16 @@ func TestInit_stateStore_newWorkingDir(t *testing.T) { mockProviderAddress: providers.FactoryFixed(mockProvider), }, }, - ProviderSource: providerSource, + ProviderSource: source, } c := &InitCommand{ Meta: meta, } - args := []string{"-enable-pluggable-state-storage-experiment=true"} + args := []string{ + "-enable-pluggable-state-storage-experiment=true", + "-input=false", // Simulate running in automation where input is disabled + } code := c.Run(args) testOutput := done(t) if code != 0 { @@ -4363,58 +4662,28 @@ func TestInit_stateStore_newWorkingDir(t *testing.T) { } } - // Assert the default workspace was not created - if _, exists := mockProvider.MockStates[backend.DefaultStateName]; exists { - t.Fatal("expected the default workspace to not be created during init, but it exists") - } - - // Assert contents of the backend state file - statePath := filepath.Join(meta.DataDir(), DefaultStateFilename) - sMgr := &clistate.LocalState{Path: statePath} - if err := sMgr.RefreshState(); err != nil { - t.Fatal("Failed to load state:", err) - } - s := sMgr.State() - if s == nil { - t.Fatal("expected backend state file to be created, but there isn't one") - } - v1_2_3, _ := version.NewVersion("1.2.3") - expectedState := &workdir.StateStoreConfigState{ - Type: "test_store", - ConfigRaw: []byte("{\n \"value\": \"foobar\"\n }"), - Hash: uint64(4158988729), - ProviderSupplyMode: getproviders.ManagedByTerraform, - Provider: &workdir.ProviderConfigState{ - Version: v1_2_3, - Source: &tfaddr.Provider{ - Hostname: tfaddr.DefaultProviderRegistryHost, - Namespace: "hashicorp", - Type: "test", - }, - ConfigRaw: []byte("{\n \"region\": null\n }"), - }, - } - if diff := cmp.Diff(s.StateStore, expectedState); diff != "" { - t.Fatalf("unexpected diff in backend state file's description of state store:\n%s", diff) + // Assert the dependency lock file was created + lockFile := filepath.Join(td, ".terraform.lock.hcl") + _, err := os.Stat(lockFile) + if os.IsNotExist(err) { + t.Fatal("expected dependency lock file to exist, but it doesn't") } }) - // This scenario would be rare, but protecting against it is easy and avoids assumptions. - t.Run("if a custom workspace is selected but no workspaces exist an error is returned", func(t *testing.T) { + t.Run("users approve trusting a state store provider downloaded via HTTP by supplying locks via -state-provider-lock flag", func(t *testing.T) { // Create a temporary, uninitialized working directory with configuration including a state store td := t.TempDir() testCopyDir(t, testFixturePath("init-with-state-store"), td) t.Chdir(td) - // Select a custom workspace (which will not exist) - customWorkspace := "my-custom-workspace" - t.Setenv(WorkspaceNameEnvVar, customWorkspace) - - mockProvider := mockPluggableStateStorageProvider() + // Set up mock provider source that mocks out hashicorp/test via HTTP. + // This stops Terraform auto-approving the provider installation. mockProviderAddress := addrs.NewDefaultProvider("test") - providerSource := newMockProviderSource(t, map[string][]string{ - "hashicorp/test": {"1.0.0"}, + expectedVersion := "1.2.3" + source := newMockProviderSourceUsingTestHttpServer(t, map[string][]string{ + "hashicorp/test": {expectedVersion, "9.9.9"}, // Extra version - expected version is downloaded, not the latest }) + mockProvider := mockPluggableStateStorageProvider() ui := new(cli.MockUi) view, done := testView(t) @@ -4427,68 +4696,156 @@ func TestInit_stateStore_newWorkingDir(t *testing.T) { mockProviderAddress: providers.FactoryFixed(mockProvider), }, }, - ProviderSource: providerSource, + ProviderSource: source, } c := &InitCommand{ Meta: meta, } - args := []string{"-enable-pluggable-state-storage-experiment=true"} + // Create supplemental lock file to be used with the -state-provider-lock flag + // To avoid this being confused with the lock file in the working directory, + // this is made in a second temp directory away from other files in this test. + td2 := t.TempDir() + lockFileName := filepath.Join(td2, ".terraform.lock.hcl") + suppliedLockFileVersion := getproviders.MustParseVersion(expectedVersion) + locks := depsfile.NewLocks() + locks.SetProvider( + mockProviderAddress, + suppliedLockFileVersion, + getproviders.MustParseVersionConstraints("> 1.0.0"), + []getproviders.Hash{ + getproviders.HashScheme1.New("wlbEC2mChQZ2hhgUhl6SeVLPP7fMqOFUZAQhQ9GIIno="), + }, + ) + depsfile.SaveLocksToFile(locks, lockFileName) + + args := []string{ + "-enable-pluggable-state-storage-experiment=true", + "-input=false", // Simulate running in automation where input is disabled + fmt.Sprintf("-state-provider-lock-file=%s", lockFileName), + } code := c.Run(args) testOutput := done(t) - if code != 1 { - t.Fatalf("expected code 1 exit code, got %d, output: \n%s", code, testOutput.All()) + if code != 0 { + t.Fatalf("expected code 0 exit code, got %d, output: \n%s", code, testOutput.All()) } - // Check output + // Check output via view output := testOutput.All() expectedOutputs := []string{ - fmt.Sprintf("Workspace %q has not been created yet", customWorkspace), - fmt.Sprintf("To create the custom workspace %q use the command `terraform workspace new %s`", customWorkspace, customWorkspace), + "Initializing the state store...", + "The state store provider was approved automatically", + "Terraform has been successfully initialized!", } for _, expected := range expectedOutputs { - if !strings.Contains(cleanString(output), expected) { - t.Fatalf("expected output to include %q, but got':\n %s", expected, cleanString(output)) + if !strings.Contains(output, expected) { + t.Fatalf("expected output to include %q, but got':\n %s", expected, output) } } - // Assert no workspaces exist - if len(mockProvider.MockStates) != 0 { - t.Fatalf("expected no workspaces, but got: %#v", mockProvider.MockStates) + // Assert the dependency lock file was created + // and it contains the state store provider version described by the -state-provider-lock-file flag + lockFile := filepath.Join(td, ".terraform.lock.hcl") + locks, lockDiags := depsfile.LoadLocksFromFile(lockFile) + if lockDiags.HasErrors() { + t.Fatal("expected dependency lock file to exist, but got errors loading it:", lockDiags.Err()) } - - // Assert no backend state file made due to the error - statePath := filepath.Join(meta.DataDir(), DefaultStateFilename) - _, err := os.Stat(statePath) - if pathErr, ok := err.(*os.PathError); !ok || !os.IsNotExist(pathErr.Err) { - t.Fatalf("expected backend state file to not be created, but it exists") + gotLock := locks.Provider(mockProviderAddress) + if gotLock == nil { + t.Fatalf("expected dependency lock file to contain the state store provider %s, but it doesn't", mockProviderAddress.ForDisplay()) + } + if !gotLock.Version().Same(suppliedLockFileVersion) { + t.Fatalf("expected dependency lock file to contain version %s for provider %s that was supplied via the -state-provider-lock-file flag, but got version %s", suppliedLockFileVersion, mockProviderAddress.ForDisplay(), gotLock.Version()) } }) - // Test what happens when the selected workspace doesn't exist, but there are other workspaces available. - // - // When input is disabled (in automation, etc) Terraform cannot prompts the user to select an alternative. - // Instead, an error is returned. - t.Run("init: returns an error when input is disabled and the selected workspace doesn't exist and other custom workspaces do exist.", func(t *testing.T) { + t.Run("a state store provider downloaded via HTTP can be automatically approved if it already exists in the .terraform.lock.hcl file", func(t *testing.T) { // Create a temporary, uninitialized working directory with configuration including a state store td := t.TempDir() testCopyDir(t, testFixturePath("init-with-state-store"), td) t.Chdir(td) + // Set up mock provider source that mocks out hashicorp/test via HTTP. + // This stops Terraform auto-approving the provider installation. + mockProviderAddress := addrs.NewDefaultProvider("test") + expectedVersion := "1.2.3" + source := newMockProviderSourceUsingTestHttpServer(t, map[string][]string{ + "hashicorp/test": {expectedVersion, "9.9.9"}, // Extra version - expected version is downloaded, not the latest + }) mockProvider := mockPluggableStateStorageProvider() - mockProvider.GetStatesResponse = &providers.GetStatesResponse{ - States: []string{ - "foobar1", - "foobar2", - // Force provider to report workspaces exist - // But default workspace doesn't exist + + ui := new(cli.MockUi) + view, done := testView(t) + meta := Meta{ + Ui: ui, + View: view, + AllowExperimentalFeatures: true, + testingOverrides: &testingOverrides{ + Providers: map[addrs.Provider]providers.Factory{ + mockProviderAddress: providers.FactoryFixed(mockProvider), + }, + }, + ProviderSource: source, + } + c := &InitCommand{ + Meta: meta, + } + + // Create a local .terraform.lock.hcl file that already contains the state store provider version + lockFileName := ".terraform.lock.hcl" + suppliedLockFileVersion := getproviders.MustParseVersion(expectedVersion) + locks := depsfile.NewLocks() + locks.SetProvider( + mockProviderAddress, + suppliedLockFileVersion, + getproviders.MustParseVersionConstraints("> 1.0.0"), + []getproviders.Hash{ + getproviders.HashScheme1.New("wlbEC2mChQZ2hhgUhl6SeVLPP7fMqOFUZAQhQ9GIIno="), }, + ) + depsfile.SaveLocksToFile(locks, lockFileName) + + args := []string{ + "-enable-pluggable-state-storage-experiment=true", + "-input=false", // Simulate running in automation where input is disabled + // -state-provider-lock-file flag isn't used; this test shows it can fall back to using the .terraform.lock.hcl file in the working directory + } + code := c.Run(args) + testOutput := done(t) + if code != 0 { + t.Fatalf("expected code 0 exit code, got %d, output: \n%s", code, testOutput.All()) + } + + // Check output via view + output := testOutput.All() + expectedOutputs := []string{ + "Initializing the state store...", + "The state store provider was approved automatically", + "Terraform has been successfully initialized!", + } + for _, expected := range expectedOutputs { + if !strings.Contains(output, expected) { + t.Fatalf("expected output to include %q, but got':\n %s", expected, output) + } } + // No need for assertions about the dependency lock file + // as it was created during test setup. + }) + + t.Run("error if the lock file supplied by the -state-provider-lock-file flag doesn't contain the state store provider", func(t *testing.T) { + // Create a temporary, uninitialized working directory with configuration including a state store + td := t.TempDir() + testCopyDir(t, testFixturePath("init-with-state-store"), td) + t.Chdir(td) + // Set up mock provider source that mocks out hashicorp/test via HTTP. + // This stops Terraform auto-approving the provider installation. mockProviderAddress := addrs.NewDefaultProvider("test") - providerSource := newMockProviderSource(t, map[string][]string{ - "hashicorp/test": {"1.0.0"}, + expectedVersion := "1.2.3" + source := newMockProviderSourceUsingTestHttpServer(t, map[string][]string{ + "hashicorp/test": {expectedVersion}, }) + mockProvider := mockPluggableStateStorageProvider() ui := new(cli.MockUi) view, done := testView(t) @@ -4501,67 +4858,68 @@ func TestInit_stateStore_newWorkingDir(t *testing.T) { mockProviderAddress: providers.FactoryFixed(mockProvider), }, }, - ProviderSource: providerSource, + ProviderSource: source, } c := &InitCommand{ Meta: meta, } - // If input is disabled users receive an error about the missing workspace + // Create supplemental lock file to be used with the -state-provider-lock flag + // To avoid this being confused with the lock file in the working directory, + // this is made in a second temp directory away from other files in this test. + td2 := t.TempDir() + lockFileName := filepath.Join(td2, ".terraform.lock.hcl") + + // It DOESNT contain the state store provider hashicorp/test though, causing an error. + locks := depsfile.NewLocks() + locks.SetProvider( + addrs.NewDefaultProvider("notusedprovider"), + getproviders.MustParseVersion("9.9.9"), + getproviders.MustParseVersionConstraints("> 1.0.0"), + []getproviders.Hash{ + getproviders.HashScheme1.New("wlbEC2mChQZ2hhgUhl6SeVLPP7fMqOFUZAQhQ9GIIno="), + }, + ) + depsfile.SaveLocksToFile(locks, lockFileName) + args := []string{ "-enable-pluggable-state-storage-experiment=true", - "-input=false", + "-input=false", // Simulate running in automation where input is disabled + fmt.Sprintf("-state-provider-lock-file=%s", lockFileName), } code := c.Run(args) testOutput := done(t) if code != 1 { t.Fatalf("expected code 1 exit code, got %d, output: \n%s", code, testOutput.All()) } - output := testOutput.All() - expectedOutput := "Failed to select a workspace: Currently selected workspace \"default\" does not exist" - if !strings.Contains(cleanString(output), expectedOutput) { - t.Fatalf("expected output to include %q, but got':\n %s", expectedOutput, cleanString(output)) + + // Check output via view + output := cleanString(testOutput.All()) + expectedOutputs := []string{ + "Error: State store provider not found in -state-provider-lock-file dependency lock file", + fmt.Sprintf("Terraform could not find the state store provider \"test\" (hashicorp/test) in the dependency lock file \"%s\" provided via the -state-provider-lock-file flag", lockFileName), } - statePath := filepath.Join(meta.DataDir(), DefaultStateFilename) - _, err := os.Stat(statePath) - if _, ok := err.(*os.PathError); !ok { - if err == nil { - t.Fatalf("expected backend state file to not be created, but it exists") + for _, expected := range expectedOutputs { + if !strings.Contains(output, expected) { + t.Fatalf("expected output to include %q, but got':\n %s", expected, output) } - - t.Fatalf("unexpected error: %s", err) } }) - // Test what happens when the selected workspace doesn't exist, but there are other workspaces available. - // - // When input is enabled Terraform prompts the user to select an alternative. - t.Run("init: prompts user to select a workspace if the selected workspace doesn't exist and other custom workspaces do exist.", func(t *testing.T) { + t.Run("error if the state store lock is supplied by neither a pre-existing lock nor the -state-provider-lock-file flag", func(t *testing.T) { // Create a temporary, uninitialized working directory with configuration including a state store td := t.TempDir() testCopyDir(t, testFixturePath("init-with-state-store"), td) t.Chdir(td) - mockProvider := mockPluggableStateStorageProvider() - mockProvider.GetStatesResponse = &providers.GetStatesResponse{ - States: []string{ - "foobar1", - "foobar2", - // Force provider to report workspaces exist - // But default workspace doesn't exist - }, - } - + // Set up mock provider source that mocks out hashicorp/test via HTTP. + // This stops Terraform auto-approving the provider installation. mockProviderAddress := addrs.NewDefaultProvider("test") - providerSource := newMockProviderSource(t, map[string][]string{ - "hashicorp/test": {"1.0.0"}, - }) - - // Allow the test to respond to the prompt to pick an - // existing workspace, given the selected one doesn't exist. - _ = testInputMap(t, map[string]string{ - "select-workspace": "1", // foobar1 in numbered list + expectedVersion := "1.2.3" + source := newMockProviderSourceUsingTestHttpServer(t, map[string][]string{ + "hashicorp/test": {expectedVersion}, }) + mockProvider := mockPluggableStateStorageProvider() ui := new(cli.MockUi) view, done := testView(t) @@ -4574,35 +4932,42 @@ func TestInit_stateStore_newWorkingDir(t *testing.T) { mockProviderAddress: providers.FactoryFixed(mockProvider), }, }, - ProviderSource: providerSource, + ProviderSource: source, } c := &InitCommand{ Meta: meta, } + // Confirm the .terraform.lock.hcl file doesn't exist before the test runs + // Therefore this isn't an adequate fallback source of locks for the state store provider, causing an error. + _, err := os.Stat(filepath.Join(td, ".terraform.lock.hcl")) + if !os.IsNotExist(err) { + t.Fatal("expected .terraform.lock.hcl file to not exist, but got an unrelated error:", err) + } + args := []string{ "-enable-pluggable-state-storage-experiment=true", + "-input=false", // Simulate running in automation where input is disabled + //-state-provider-lock-file is not used, and there's no .terraform.lock.hcl file, so no locks are supplied for the state store provider } code := c.Run(args) testOutput := done(t) - if code != 0 { - t.Fatalf("expected code 0 exit code, got %d, output: \n%s", code, testOutput.All()) + if code != 1 { + t.Fatalf("expected code 1 exit code, got %d, output: \n%s", code, testOutput.All()) } - // The init command should have caused the selected workspace to change, based on the input - // provided by the user. - currentWorkspace, err := c.Meta.Workspace() - if err != nil { - t.Fatal(err) + // Check output via view + output := cleanString(testOutput.All()) + expectedOutputs := []string{ + "Error: Missing lock for state store provider", + "Terraform is initializing a state store for the first time in a non-interactive mode. In this scenario Terraform needs a pre-existing dependency lock for the state store provider to be present in the working directory's dependency lock file, or present in another file supplied via the -state-provider-lock-file flag. No lock was found for the state store provider. Please re-run the command using the -state-provider-lock-file flag.", } - if currentWorkspace != "foobar1" { - t.Fatalf("expected init command to alter the selected workspace from 'default' to 'foobar1', but got: %s", currentWorkspace) + for _, expected := range expectedOutputs { + if !strings.Contains(output, expected) { + t.Fatalf("expected output to include %q, but got':\n %s", expected, output) + } } }) - - // TODO(SarahFrench/radeksimko): Add test cases below: - // 1) "during a non-init command, the command ends in with an error telling the user to run an init command" - // >>> Currently this is handled at a lower level in `internal/command/meta_backend_test.go` } // Test a scenario where a state store is introduced via `init -reconfigure` in a working directory diff --git a/internal/command/views/init.go b/internal/command/views/init.go index dcc6a9bc9e..d1ffd97d7a 100644 --- a/internal/command/views/init.go +++ b/internal/command/views/init.go @@ -199,13 +199,17 @@ var MessageRegistry map[InitMessageCode]InitMessage = map[InitMessageCode]InitMe HumanValue: "\n[reset][bold]Initializing the state store...", JSONValue: "Initializing the state store...", }, - "state_store_provider_approved_message": { - HumanValue: "\n[reset][bold]The state store provider was approved.", - JSONValue: "The state store provider was approved.", + "state_store_provider_interactive_approved_message": { + HumanValue: "\n[reset][bold]The state store provider was approved by the user.", + JSONValue: "The state store provider was approved by the user.", }, - "state_store_provider_rejected_message": { - HumanValue: "\n[reset][bold]The state store provider was rejected.", - JSONValue: "The state store provider was rejected.", + "state_store_provider_interactive_rejected_message": { + HumanValue: "\n[reset][bold]The state store provider was rejected by the user.", + JSONValue: "The state store provider was rejected by the user.", + }, + "state_store_provider_automation_approved_message": { + HumanValue: "\n[reset][bold]The state store provider was approved automatically.", + JSONValue: "The state store provider was approved automatically.", }, "dependencies_lock_changes_info": { HumanValue: dependenciesLockChangesInfo, @@ -335,25 +339,26 @@ const ( // Following message codes are used and documented EXTERNALLY // Keep docs/internals/machine-readable-ui.mdx up to date with // this list when making changes here. - CopyingConfigurationMessage InitMessageCode = "copying_configuration_message" - EmptyMessage InitMessageCode = "empty_message" - OutputInitEmptyMessage InitMessageCode = "output_init_empty_message" - OutputInitSuccessMessage InitMessageCode = "output_init_success_message" - OutputInitSuccessCloudMessage InitMessageCode = "output_init_success_cloud_message" - OutputInitSuccessCLIMessage InitMessageCode = "output_init_success_cli_message" - OutputInitSuccessCLICloudMessage InitMessageCode = "output_init_success_cli_cloud_message" - UpgradingModulesMessage InitMessageCode = "upgrading_modules_message" - InitializingTerraformCloudMessage InitMessageCode = "initializing_terraform_cloud_message" - InitializingModulesMessage InitMessageCode = "initializing_modules_message" - InitializingBackendMessage InitMessageCode = "initializing_backend_message" - InitializingStateStoreMessage InitMessageCode = "initializing_state_store_message" - StateStoreProviderApprovedMessage InitMessageCode = "state_store_provider_approved_message" - StateStoreProviderRejectedMessage InitMessageCode = "state_store_provider_rejected_message" - InitializingProviderPluginFromConfigMessage InitMessageCode = "initializing_provider_plugin_from_config_message" - InitializingProviderPluginFromStateMessage InitMessageCode = "initializing_provider_plugin_from_state_message" - ReusingVersionIdentifiedFromConfig InitMessageCode = "reusing_version_during_state_provider_init" - LockInfo InitMessageCode = "lock_info" - DependenciesLockChangesInfo InitMessageCode = "dependencies_lock_changes_info" + CopyingConfigurationMessage InitMessageCode = "copying_configuration_message" + EmptyMessage InitMessageCode = "empty_message" + OutputInitEmptyMessage InitMessageCode = "output_init_empty_message" + OutputInitSuccessMessage InitMessageCode = "output_init_success_message" + OutputInitSuccessCloudMessage InitMessageCode = "output_init_success_cloud_message" + OutputInitSuccessCLIMessage InitMessageCode = "output_init_success_cli_message" + OutputInitSuccessCLICloudMessage InitMessageCode = "output_init_success_cli_cloud_message" + UpgradingModulesMessage InitMessageCode = "upgrading_modules_message" + InitializingTerraformCloudMessage InitMessageCode = "initializing_terraform_cloud_message" + InitializingModulesMessage InitMessageCode = "initializing_modules_message" + InitializingBackendMessage InitMessageCode = "initializing_backend_message" + InitializingStateStoreMessage InitMessageCode = "initializing_state_store_message" + StateStoreProviderInteractiveApprovedMessage InitMessageCode = "state_store_provider_interactive_approved_message" + StateStoreProviderInteractiveRejectedMessage InitMessageCode = "state_store_provider_interactive_rejected_message" + StateStoreProviderAutomationApprovedMessage InitMessageCode = "state_store_provider_automation_approved_message" + InitializingProviderPluginFromConfigMessage InitMessageCode = "initializing_provider_plugin_from_config_message" + InitializingProviderPluginFromStateMessage InitMessageCode = "initializing_provider_plugin_from_state_message" + ReusingVersionIdentifiedFromConfig InitMessageCode = "reusing_version_during_state_provider_init" + LockInfo InitMessageCode = "lock_info" + DependenciesLockChangesInfo InitMessageCode = "dependencies_lock_changes_info" //// Message codes below are ONLY used INTERNALLY (for now) From cf0aae24f4cc8456c4cbcc3d21eac124992a8c68 Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Thu, 28 May 2026 10:37:39 +0100 Subject: [PATCH 7/9] fix: Stop our anti-`init -upgrade` check panicking when an overridden provider is in use. (#38635) Prior to this commit we wanted to validate the provider upgrade process using data that was only populated during backend initialisation (as that was the only place using that data previously). Now we enable the provider supply mode data to be available before backends are initialised. After making that available, we can update the check after getProvidersFromConfig that protects against the provider version being changed outside of a deliberate state migration process. If the state store provider is overridden then the upgrade process doesn't impact that provider anyway, so the check can be skipped. Also, this protects against errors if there was ever a built in provider supplying a state store implementation. --- internal/command/e2etest/init_test.go | 97 ++++++++++++- .../.terraform.lock.hcl | 6 + .../.terraform/terraform.tfstate | 17 +++ .../main.tf | 21 +++ .../outputs.tf | 3 + .../states/default/terraform.tfstate | 40 ++++++ internal/command/init.go | 10 -- internal/command/init_run.go | 21 ++- internal/command/init_test.go | 130 ++++++++++++++++++ internal/command/meta_backend.go | 11 +- internal/command/meta_config.go | 13 ++ 11 files changed, 346 insertions(+), 23 deletions(-) create mode 100644 internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/.terraform.lock.hcl create mode 100644 internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/.terraform/terraform.tfstate create mode 100644 internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/main.tf create mode 100644 internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/outputs.tf create mode 100644 internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/states/default/terraform.tfstate diff --git a/internal/command/e2etest/init_test.go b/internal/command/e2etest/init_test.go index 57ceec6214..dc2c0afdc8 100644 --- a/internal/command/e2etest/init_test.go +++ b/internal/command/e2etest/init_test.go @@ -14,7 +14,9 @@ import ( "github.com/google/go-cmp/cmp" + "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/e2e" + "github.com/hashicorp/terraform/internal/getproviders" ) func TestInitProviders(t *testing.T) { @@ -55,7 +57,6 @@ func TestInitProviders(t *testing.T) { if !strings.Contains(stdout, "Terraform has created a lock file") { t.Errorf("lock file notification is missing from output:\n%s", stdout) } - } func TestInitProvidersInternal(t *testing.T) { @@ -140,7 +141,6 @@ func TestInitProvidersVendored(t *testing.T) { if !strings.Contains(stdout, "- Installing hashicorp/null v1.0.0+local") { t.Errorf("provider download message is missing from output:\n%s", stdout) } - } func TestInitProvidersLocalOnly(t *testing.T) { @@ -425,7 +425,100 @@ func TestInitProviderWarnings(t *testing.T) { if !strings.Contains(stdout, "This provider is archived and no longer needed.") { t.Errorf("expected warning message is missing from output:\n%s", stdout) } +} + +// This is a regression test asserting that `terraform init -upgrade` doesn't error +// when the state storage provider is unmanaged by Terraform. The check to see if the +// state storage provider was affected by the upgrade process should be skipped when +// the provider is not managed by Terraform. Previously the check wasn't skipped and +// panicked due to how the provider was supplied. +// +// See the TestInit_getUpgradePlugins integration test for similar testing when using a +// dev_override provider. +func TestInitStateStoreUsingUnmanagedProvider(t *testing.T) { + if !canRunGoBuild { + // We're running in a separate-build-then-run context, so we can't + // currently execute this test which depends on being able to build + // new executable at runtime. + // + // (See the comment on canRunGoBuild's declaration for more information.) + t.Skip("can't run without building a new provider executable") + } + + // In temp dir create a plugin cache to be used in the test cases. + // The cache is supplied to commands using the -plugin-dir init flag. + // There are 2 versions of the simple6 provider: 0.0.1 and 2.0.0 + // This enables us to test an upgrade scenario where a newer provider version is available. + td := t.TempDir() + providerVersionOld := "0.0.1" + providerVersionNew := "2.0.0" + platform := getproviders.CurrentPlatform.String() + absolutePathToCache := filepath.Join(td, "cache") + simple6Provider := filepath.Join(td, "terraform-provider-simple6") + simple6ProviderExe := e2e.GoBuild("github.com/hashicorp/terraform/internal/provider-simple-v6/main", simple6Provider) + for _, v := range []string{providerVersionOld, providerVersionNew} { + dir := filepath.Join(absolutePathToCache, "registry.terraform.io/hashicorp", "simple6", v, platform) + if err := os.MkdirAll(dir, os.ModePerm); err != nil { + t.Fatal(err) + } + // Create an executable copy of the simple6ProviderExe file per version in the cache dir + data, err := os.ReadFile(simple6ProviderExe) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "terraform-provider-simple6"), data, 0755); err != nil { + t.Fatal(err) + } + } + t.Setenv(e2e.TestExperimentFlag, "true") + terraformBin := e2e.GoBuild("github.com/hashicorp/terraform", "terraform") + fixturePath := filepath.Join("testdata", "initialized-directory-with-state-store-unmanaged") + tf := e2e.NewBinary(t, terraformBin, fixturePath) + + // Assert the existing lockfile describes the older version of the provider. + lockFile := tf.Path(".terraform.lock.hcl") + buf, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("unexpected error accessing lock file: %s", err) + } + buf = bytes.TrimSpace(buf) + + expectedLockFileContent := fmt.Sprintf(`# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/simple6" { + version = "%s" +}`, providerVersionOld) + if diff := cmp.Diff(expectedLockFileContent, string(buf)); diff != "" { + t.Errorf("unexpected difference in lock file content: %s", diff) + } + + // The simple6 provider is unmanaged + reattachConfig, _ := reattachedProviderForTest(t, addrs.NewDefaultProvider("simple6"), 6) + tf.AddEnv("TF_REATTACH_PROVIDERS=" + reattachConfig) + + // The init -upgrade process should succeed. + stdout, stderr, err := tf.Run( + "init", + "-upgrade", + "-enable-pluggable-state-storage-experiment", + fmt.Sprintf("-plugin-dir=%s", absolutePathToCache), + ) + if err != nil { + t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) + } + + // Lockfile should be unchanged because the only provider in the config is unmanaged. + buf, err = os.ReadFile(lockFile) + if err != nil { + t.Fatalf("unexpected error accessing lock file: %s", err) + } + buf = bytes.TrimSpace(buf) + + if diff := cmp.Diff(expectedLockFileContent, string(buf)); diff != "" { + t.Errorf("unexpected difference in lock file content: %s", diff) + } } // emptyConfigFileForTests creates a blank .terraformrc file in the requested diff --git a/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/.terraform.lock.hcl b/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/.terraform.lock.hcl new file mode 100644 index 0000000000..7a0db0a25a --- /dev/null +++ b/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/.terraform.lock.hcl @@ -0,0 +1,6 @@ +# This file is maintained automatically by "terraform init". +# Manual edits may be lost in future updates. + +provider "registry.terraform.io/hashicorp/simple6" { + version = "0.0.1" +} diff --git a/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/.terraform/terraform.tfstate b/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/.terraform/terraform.tfstate new file mode 100644 index 0000000000..009467c3c7 --- /dev/null +++ b/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/.terraform/terraform.tfstate @@ -0,0 +1,17 @@ +{ + "version": 3, + "terraform_version": "1.15.0", + "state_store": { + "type": "simple6_fs", + "provider": { + "version": "0.0.1", + "source": "registry.terraform.io/hashicorp/simple6", + "config": {} + }, + "config": { + "workspace_dir": "states" + }, + "hash": 3942813381, + "provider_supply_mode": "reattached" + } +} \ No newline at end of file diff --git a/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/main.tf b/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/main.tf new file mode 100644 index 0000000000..f20f555669 --- /dev/null +++ b/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/main.tf @@ -0,0 +1,21 @@ +terraform { + required_providers { + simple6 = { + source = "registry.terraform.io/hashicorp/simple6" + } + } + + state_store "simple6_fs" { + provider "simple6" {} + + workspace_dir = "states" + } +} + +variable "name" { + default = "world" +} + +resource "terraform_data" "my-data" { + input = "hello ${var.name}" +} diff --git a/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/outputs.tf b/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/outputs.tf new file mode 100644 index 0000000000..b650136fa1 --- /dev/null +++ b/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/outputs.tf @@ -0,0 +1,3 @@ +output "greeting" { + value = resource.terraform_data.my-data.output +} diff --git a/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/states/default/terraform.tfstate b/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/states/default/terraform.tfstate new file mode 100644 index 0000000000..4feaaed87a --- /dev/null +++ b/internal/command/e2etest/testdata/initialized-directory-with-state-store-unmanaged/states/default/terraform.tfstate @@ -0,0 +1,40 @@ +{ + "version": 4, + "terraform_version": "1.15.0", + "serial": 1, + "lineage": "9e13d881-e480-7a63-d47a-b4f5224e6743", + "outputs": { + "greeting": { + "value": "hello world", + "type": "string" + } + }, + "resources": [ + { + "mode": "managed", + "type": "terraform_data", + "name": "my-data", + "provider": "provider[\"terraform.io/builtin/terraform\"]", + "instances": [ + { + "schema_version": 0, + "attributes": { + "id": "d71fb368-2ba1-fb4c-5bd9-6a2b7f05d60c", + "input": { + "value": "hello world", + "type": "string" + }, + "output": { + "value": "hello world", + "type": "string" + }, + "triggers_replace": null + }, + "sensitive_attributes": [], + "identity_schema_version": 0 + } + ] + } + ], + "check_results": null +} \ No newline at end of file diff --git a/internal/command/init.go b/internal/command/init.go index ff5a64a4f7..e8b26ce095 100644 --- a/internal/command/init.go +++ b/internal/command/init.go @@ -8,7 +8,6 @@ import ( "fmt" "log" "maps" - "os" "reflect" "slices" "sort" @@ -32,7 +31,6 @@ import ( "github.com/hashicorp/terraform/internal/didyoumean" "github.com/hashicorp/terraform/internal/getproviders" "github.com/hashicorp/terraform/internal/getproviders/providerreqs" - "github.com/hashicorp/terraform/internal/getproviders/reattach" "github.com/hashicorp/terraform/internal/providercache" "github.com/hashicorp/terraform/internal/states" "github.com/hashicorp/terraform/internal/tfdiags" @@ -233,14 +231,6 @@ func (c *InitCommand) initBackend(ctx context.Context, root *configs.Module, ini } } - // Annotate state_store config representation with info about how the provider - // is supplied to Terraform. - isReattached, err := reattach.IsProviderReattached(root.StateStore.ProviderAddr, os.Getenv("TF_REATTACH_PROVIDERS")) - if err != nil { - panic(fmt.Sprintf("Unable to determine if provider %s is reattached while initializing the state store. This is a bug in Terraform and should be reported: %v", root.StateStore.ProviderAddr.ForDisplay(), err)) - } - root.StateStore.ProviderSupplyMode = getproviders.DetermineProviderSupplyMode(c.Meta.isProviderDevOverride(root.StateStore.ProviderAddr), isReattached, root.StateStore.ProviderAddr.IsBuiltIn()) - opts = &BackendOpts{ StateStoreConfig: root.StateStore, Locks: configLocks, diff --git a/internal/command/init_run.go b/internal/command/init_run.go index 22c95c9056..5636e8a6d0 100644 --- a/internal/command/init_run.go +++ b/internal/command/init_run.go @@ -167,6 +167,13 @@ func (c *InitCommand) run(initArgs *arguments.Init, view views.Init) int { return 1 } + if rootModEarly.StateStore != nil { // We know rootModEarly is not nil. + rootModEarly.StateStore.ProviderSupplyMode = c.Meta.getProviderSupplyModeForStateStore(rootModEarly) + if rootModEarly.StateStore.ProviderSupplyMode == getproviders.Unset { + panic("unset provider supply mode for state store") + } + } + if initArgs.Get { modsOutput, modsAbort, modsDiags := c.getModules(ctx, path, initArgs.TestsDirectory, rootModEarly, initArgs.Upgrade, view) diags = diags.Append(modsDiags) @@ -182,6 +189,12 @@ func (c *InitCommand) run(initArgs *arguments.Init, view views.Init) int { // With all of the modules (hopefully) installed, we can now try to load the // whole configuration tree. config, confDiags := c.loadConfigWithTests(path, initArgs.TestsDirectory) + if config != nil && config.Module != nil && config.Module.StateStore != nil { + config.Module.StateStore.ProviderSupplyMode = c.Meta.getProviderSupplyModeForStateStore(config.Module) + if config.Module.StateStore.ProviderSupplyMode == getproviders.Unset { + panic("unset provider supply mode for state store") + } + } // configDiags will be handled after: // - the version constraint check has happened // - and, the backend/state_store is initialised @@ -304,7 +317,13 @@ func (c *InitCommand) run(initArgs *arguments.Init, view views.Init) int { // The init command is not allowed to upgrade the provider used for state storage (unless we're reconfiguring the state store). // Unless users choose to reconfigure, they must upgrade the state store provider separately using `terraform state migrate -upgrade`. - if initArgs.Upgrade && !initArgs.Reconfigure && config.Module.StateStore != nil { + // We only check to see if the state store provider was upgraded if the provider supply mode is ManagedByTerraform; providers overridden + // using dev_override or unmanaged providers blocks any upgrade process impacting that provider. + // For more context, see: https://github.com/hashicorp/terraform/pull/38633 + if initArgs.Upgrade && + !initArgs.Reconfigure && + config.Module.StateStore != nil && + config.Module.StateStore.ProviderSupplyMode == getproviders.ManagedByTerraform { pAddr := config.Module.StateStore.ProviderAddr old := alteredPreviousLocks.Provider(pAddr) new := configLocks.Provider(pAddr) diff --git a/internal/command/init_test.go b/internal/command/init_test.go index 5e433d6e71..3715622f95 100644 --- a/internal/command/init_test.go +++ b/internal/command/init_test.go @@ -2610,6 +2610,136 @@ terraform { } }) + // A dev_override setting stops a provider being upgraded. That behaviour is tested elsewhere. + // This test is a regression test asserting that checks ensuring the state storage provider wasn't + // upgraded during `init -upgrade` doesn't panic when the state storage provider is a dev_override. + // + // An equivalent test for this scenario where the state storage provider is unmanaged is implemented + // as an E2E test, as that's the only place unmanaged providers can be used in tests. + t.Run("no errors if `init -upgrade` is run while the state store provider is a dev_override ", func(t *testing.T) { + // Create a temporary working directory and copy in test fixtures + td := t.TempDir() + t.Chdir(td) + + // Configuration uses a state store and has other provider requirements. + cfg := ` +terraform { + + required_providers { + test = { + source = "hashicorp/test" + version = "> 1.0.0" + } + } + state_store "test_store" { + provider "test" { + } + + value = "foobar" + } +}` + if err := os.WriteFile("main.tf", []byte(cfg), 0644); err != nil { + t.Fatalf("failed to write main.tf: %s", err) + } + + providerSource := newMockProviderSource(t, map[string][]string{ + // config requires > 1.0.0 + "test": {"1.2.3", "9.9.9"}, + }) + + // Mock provider to act as "hashicorp/test" + mockProvider := mockPluggableStateStorageProvider() + mockProviderAddress := addrs.NewDefaultProvider("test") + ui := new(cli.MockUi) + view, done := testView(t) + m := Meta{ + testingOverrides: metaOverridesForProvider(mockProvider), + Ui: ui, + View: view, + ProviderSource: providerSource, + AllowExperimentalFeatures: true, + + // THIS ALLOWS THE TEST TO MIMIC A SCENARIO WHERE THE PROVIDER IS A DEV OVERRIDE. + // The mock is still accessed via testingOverrides, but Terraform believes that it's a dev override due to + // its presence in the ProviderDevOverrides map. + ProviderDevOverrides: map[addrs.Provider]getproviders.PackageLocalDir{ + mockProviderAddress: ".", + }, + } + + // Make Terraform believe that we already have version 1.2.3 installed. + installFakeProviderPackages(t, &m, map[string][]string{ + "test": {"1.2.3"}, + }) + // Create a dependency lock file describing the hashicorp/test provider at version 1.2.3, to simulate a previous init with that version. + locks := depsfile.NewLocks() + locks.SetProvider( + addrs.NewDefaultProvider("test"), + getproviders.MustParseVersion("1.2.3"), + getproviders.MustParseVersionConstraints("> 1.0.0"), + []getproviders.Hash{ + getproviders.HashScheme1.New("wlbEC2mChQZ2hhgUhl6SeVLPP7fMqOFUZAQhQ9GIIno="), + }, + ) + if err := depsfile.SaveLocksToFile(locks, ".terraform.lock.hcl"); err != nil { + t.Fatalf("failed to write provider locks file: %s", err) + } + + c := &InitCommand{ + Meta: m, + } + + args := []string{ + "-upgrade=true", + "-enable-pluggable-state-storage-experiment", + } + code := c.Run(args) + if code != 0 { + t.Fatalf("expected code 0, but got code %d. \nOutput:\n%s", code, done(t).All()) + } + + // Assert that no providers were upgraded. + // + // Unlike other test scenarios, "test" v9.9.9 will not be installed in the cache. + // This is because the installation process skips installing providers that are + // dev_overrides. That logic is used in both upgrade and non-upgrade operations. + cacheDir := m.providerLocalCacheDir() + gotPackages := cacheDir.AllAvailablePackages() + wantPackages := map[addrs.Provider][]providercache.CachedProvider{ + mockProviderAddress: { + // No v9.9.9 entry + { + Provider: mockProviderAddress, + Version: getproviders.MustParseVersion("1.2.3"), + PackageDir: expectedPackageInstallPath("test", "1.2.3", false), + }, + }, + } + if diff := cmp.Diff(wantPackages, gotPackages); diff != "" { + t.Errorf("wrong cache directory contents after upgrade\n%s", diff) + } + + // The provider locks should not have changed. + locks, err := m.lockedDependencies() + if err != nil { + t.Fatalf("failed to get locked dependencies: %s", err) + } + gotProviderLocks := locks.AllProviders() + wantProviderLocks := map[addrs.Provider]*depsfile.ProviderLock{ + mockProviderAddress: depsfile.NewProviderLock( + mockProviderAddress, + getproviders.MustParseVersion("1.2.3"), + getproviders.MustParseVersionConstraints("> 1.0.0"), + []getproviders.Hash{ + getproviders.HashScheme1.New("wlbEC2mChQZ2hhgUhl6SeVLPP7fMqOFUZAQhQ9GIIno="), + }, + ), + } + if diff := cmp.Diff(gotProviderLocks, wantProviderLocks, depsfile.ProviderLockComparer); diff != "" { + t.Errorf("wrong version selections after upgrade\n%s", diff) + } + }) + t.Run("`init -upgrade -reconfigure` can be used to upgrade the state store provider", func(t *testing.T) { // Create a temporary working directory and copy in test fixtures td := t.TempDir() diff --git a/internal/command/meta_backend.go b/internal/command/meta_backend.go index ddfdba42cd..d20e805af9 100644 --- a/internal/command/meta_backend.go +++ b/internal/command/meta_backend.go @@ -14,7 +14,6 @@ import ( "fmt" "log" "maps" - "os" "path/filepath" "slices" "strconv" @@ -42,7 +41,6 @@ import ( "github.com/hashicorp/terraform/internal/didyoumean" "github.com/hashicorp/terraform/internal/getproviders" "github.com/hashicorp/terraform/internal/getproviders/providerreqs" - "github.com/hashicorp/terraform/internal/getproviders/reattach" "github.com/hashicorp/terraform/internal/plans" "github.com/hashicorp/terraform/internal/providers" "github.com/hashicorp/terraform/internal/states/statemgr" @@ -2094,19 +2092,12 @@ func (m *Meta) backend(configPath string, viewType arguments.ViewType) (backendr ViewType: viewType, } case root.StateStore != nil: - // Annotate state_store config representation with info about how the provider - // is supplied to Terraform. - isReattached, err := reattach.IsProviderReattached(root.StateStore.ProviderAddr, os.Getenv("TF_REATTACH_PROVIDERS")) - if err != nil { - panic(fmt.Sprintf("Unable to determine if provider %s is reattached while initializing the state store. This is a bug in Terraform and should be reported: %v", root.StateStore.ProviderAddr.ForDisplay(), err)) - } - root.StateStore.ProviderSupplyMode = getproviders.DetermineProviderSupplyMode(m.isProviderDevOverride(root.StateStore.ProviderAddr), isReattached, root.StateStore.ProviderAddr.IsBuiltIn()) - // Check the provider for state storage is present, either via the dependency lock file or // supplied via developer overrides, reattach config, or being built-in. // // Remember, the (Meta).backend method is used for non-init commands, so we expect dependency locks // to be present or for the provider to be otherwise available, e.g. via reattach config. + root.StateStore.ProviderSupplyMode = m.getProviderSupplyModeForStateStore(root) depsDiags := root.StateStore.VerifyDependencySelection(locks, root.ProviderRequirements, root.StateStore.ProviderSupplyMode) diags = diags.Append(depsDiags) if depsDiags.HasErrors() { diff --git a/internal/command/meta_config.go b/internal/command/meta_config.go index c476378ff1..7872adfac0 100644 --- a/internal/command/meta_config.go +++ b/internal/command/meta_config.go @@ -22,6 +22,8 @@ import ( "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/configs/configload" "github.com/hashicorp/terraform/internal/configs/configschema" + "github.com/hashicorp/terraform/internal/getproviders" + "github.com/hashicorp/terraform/internal/getproviders/reattach" "github.com/hashicorp/terraform/internal/initwd" "github.com/hashicorp/terraform/internal/registry" "github.com/hashicorp/terraform/internal/terraform" @@ -484,6 +486,17 @@ func (m *Meta) registryClient() *registry.Client { return registry.NewClient(m.Services, nil) } +func (m *Meta) getProviderSupplyModeForStateStore(config *configs.Module) getproviders.ProviderSupplyMode { + if config == nil || config.StateStore == nil { + return getproviders.Unset + } + isReattached, err := reattach.IsProviderReattached(config.StateStore.ProviderAddr, os.Getenv("TF_REATTACH_PROVIDERS")) + if err != nil { + panic(fmt.Sprintf("Unable to determine if provider %s is reattached while initializing the state store. This is a bug in Terraform and should be reported: %v", config.StateStore.ProviderAddr.ForDisplay(), err)) + } + return getproviders.DetermineProviderSupplyMode(m.isProviderDevOverride(config.StateStore.ProviderAddr), isReattached, config.StateStore.ProviderAddr.IsBuiltIn()) +} + // configValueFromCLI parses a configuration value that was provided in a // context in the CLI where only strings can be provided, such as on the // command line or in an environment variable, and returns the resulting From cf395905e7c16adb0072e352ae689a3ea287b6e4 Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Thu, 28 May 2026 16:08:06 +0100 Subject: [PATCH 8/9] Fix: Remove erroneous warning about dev overrides in the providers lock command (#38655) In the `init` command we skip downloading any providers that are overridden or aren't managed by Terraform, but during `providers lock` the installer is not configured with knowledge about those affected providers so they aren't similarly skipped. Compare how the init installer is created (https://github.com/hashicorp/terraform/blob/cf0aae24f4cc8456c4cbcc3d21eac124992a8c68/internal/command/meta_providers.go#L63-L92) to how the providers lock installer is created (https://github.com/hashicorp/terraform/blob/cf0aae24f4cc8456c4cbcc3d21eac124992a8c68/internal/command/providers_lock.go#L258-L265). Therefore we don't need to warn about the effect of dev_overrides as they have no effect. --- .changes/v1.16/NOTES-20260526-164738.yaml | 5 ----- internal/command/meta_providers.go | 23 ----------------------- internal/command/providers_lock.go | 5 ----- 3 files changed, 33 deletions(-) delete mode 100644 .changes/v1.16/NOTES-20260526-164738.yaml diff --git a/.changes/v1.16/NOTES-20260526-164738.yaml b/.changes/v1.16/NOTES-20260526-164738.yaml deleted file mode 100644 index 9e59c889c2..0000000000 --- a/.changes/v1.16/NOTES-20260526-164738.yaml +++ /dev/null @@ -1,5 +0,0 @@ -kind: NOTES -body: 'providers: The `providers locks` command will now warn users about any dev_overrides in effect, as these will stop provider locks from being downloaded.' -time: 2026-05-26T16:47:38.947615+01:00 -custom: - Issue: "38634" diff --git a/internal/command/meta_providers.go b/internal/command/meta_providers.go index 66621156ce..978f0d26cb 100644 --- a/internal/command/meta_providers.go +++ b/internal/command/meta_providers.go @@ -196,29 +196,6 @@ func (m *Meta) providerDevOverrideInitWarnings() tfdiags.Diagnostics { } } -// providerDevOverrideProviderLockWarnings is just like providerDevOverrideInitWarnings -// except the diagnostic is written with a message specific to the `providers lock` command. -// Similarly, diags will only be returned if there is 1+ dev_overrides in effect, and no error -// diags will be returned. -func (m *Meta) providerDevOverrideProvidersLockWarnings() tfdiags.Diagnostics { - if len(m.ProviderDevOverrides) == 0 { - return nil - } - var detailMsg strings.Builder - detailMsg.WriteString("The following provider development overrides are set in the CLI configuration:\n") - for addr, path := range m.ProviderDevOverrides { - detailMsg.WriteString(fmt.Sprintf(" - %s in %s\n", addr.ForDisplay(), path)) - } - detailMsg.WriteString("\nThese provider locks will not be recorded because the provider is overwritten. If this is unintentional please re-run without the development overrides set.") - return tfdiags.Diagnostics{ - tfdiags.Sourceless( - tfdiags.Warning, - "Provider development overrides are in effect", - detailMsg.String(), - ), - } -} - func (m *Meta) isProviderDevOverride(pAddr addrs.Provider) bool { if len(m.ProviderDevOverrides) == 0 { return false diff --git a/internal/command/providers_lock.go b/internal/command/providers_lock.go index 09cde2f3ba..4430565dd0 100644 --- a/internal/command/providers_lock.go +++ b/internal/command/providers_lock.go @@ -195,11 +195,6 @@ func (c *ProvidersLockCommand) Run(args []string) int { // merge all of the generated locks together at the end. updatedLocks := map[getproviders.Platform]*depsfile.Locks{} selectedVersions := map[addrs.Provider]getproviders.Version{} - - // Warn users about any development overrides in effect; they will block - // locks being obtained for the overridden providers. - c.showDiagnostics(c.providerDevOverrideProvidersLockWarnings()) - for _, platform := range platforms { tempDir, err := ioutil.TempDir("", "terraform-providers-lock") if err != nil { From 61a0333f9651c07883b98c92509ecb12051e56d9 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Thu, 28 May 2026 17:30:03 +0100 Subject: [PATCH 9/9] feat: Add warnings to the `init` command if unmanaged providers will influence provider installation When developer overrides warnings were first added in #26605 we warned users about the potential impacts of dev_overrides on resource state. If users performed apply using a dev override in a project that wasn't for testing purposes their state could become incompatible with published providers and require manual edits to state to reverse the effects. Not good. Warnings were added to init, apply, and eventually to plan. Later, in #27514, the warning in init was made more specific than warnings in plan/apply and just instructed users to not run init when overrides are used. This was based on the assumption that the provider would not be used during init in any way. This also was intended as a way for users to avoid confusing errors when their config includes an unpublished provider in required_providers, supply that provider via overrides, and then experience installation errors during init. Recently, in #37884, dev_overrides were made more compatible with init, avoiding the unpublished provider installation errors from happening. Therefore the old guidance of 'don't run init when using dev_overrides' became outdated. At that time the dev_override warning was updated to stop instructing users to not perform init while a dev_override is present, and instead the warning tells users about the overrides' potential effects on provider download. Given that latest change, this PR adds a warning to init highlighting the same effects of unmanaged providers on the provider download process. Unmanaged providers have the same impact as overridden providers. --- .changes/v1.16/BUG FIXES-20260528-171432.yaml | 5 ++ .../command/e2etest/provider_plugin_test.go | 53 ++++++++++++++++--- internal/command/init.go | 16 ++++-- internal/command/meta_providers.go | 32 +++++++++++ internal/command/meta_providers_test.go | 42 +++++++++++++++ 5 files changed, 137 insertions(+), 11 deletions(-) create mode 100644 .changes/v1.16/BUG FIXES-20260528-171432.yaml diff --git a/.changes/v1.16/BUG FIXES-20260528-171432.yaml b/.changes/v1.16/BUG FIXES-20260528-171432.yaml new file mode 100644 index 0000000000..3b2a7b985b --- /dev/null +++ b/.changes/v1.16/BUG FIXES-20260528-171432.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'init: Add warnings when unmanaged providers are in use and will impact provider installation processes.' +time: 2026-05-28T17:14:32.881514+01:00 +custom: + Issue: "38656" diff --git a/internal/command/e2etest/provider_plugin_test.go b/internal/command/e2etest/provider_plugin_test.go index 513804d5d0..bcb93cffc8 100644 --- a/internal/command/e2etest/provider_plugin_test.go +++ b/internal/command/e2etest/provider_plugin_test.go @@ -218,10 +218,17 @@ func TestProviderInstall_dev_override(t *testing.T) { tf.AddEnv("TF_CLI_CONFIG_FILE=" + cliConfigFilePath) // The init process should succeed. - stdout, stderr, err := tf.Run("init", fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) + stdout, stderr, err := tf.Run( + "init", + "-no-color", + fmt.Sprintf("-plugin-dir=%s", absolutePathToCache), + ) if err != nil { t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) } + if got, want := stdout, "Warning: Provider development overrides are in effect"; !strings.Contains(got, want) { + t.Fatalf("stdout doesn't include the warning about overridden providers\nwant: %s\n%s", want, got) + } // Lockfile is created // simple provider is installed using the latest, 2.0.0 version, @@ -281,10 +288,17 @@ provider "registry.terraform.io/hashicorp/simple6" { tf.AddEnv("TF_CLI_CONFIG_FILE=" + cliConfigFilePath) // The init process should succeed. - stdout, stderr, err := tf.Run("init", fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) + stdout, stderr, err := tf.Run( + "init", + "-no-color", + fmt.Sprintf("-plugin-dir=%s", absolutePathToCache), + ) if err != nil { t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) } + if got, want := stdout, "Warning: Provider development overrides are in effect"; !strings.Contains(got, want) { + t.Fatalf("stdout doesn't include the warning about overridden providers\nwant: %s\n%s", want, got) + } // Lockfile is unchanged despite use of a dev_override simple6 provider buf, err := os.ReadFile(lockFile) @@ -330,10 +344,18 @@ provider "registry.terraform.io/hashicorp/simple6" { tf.AddEnv("TF_CLI_CONFIG_FILE=" + cliConfigFilePath) // The init -upgrade process should succeed. - stdout, stderr, err := tf.Run("init", "-upgrade", fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) + stdout, stderr, err := tf.Run( + "init", + "-no-color", + "-upgrade", + fmt.Sprintf("-plugin-dir=%s", absolutePathToCache), + ) if err != nil { t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) } + if got, want := stdout, "Warning: Provider development overrides are in effect"; !strings.Contains(got, want) { + t.Fatalf("stdout doesn't include the warning about overridden providers\nwant: %s\n%s", want, got) + } // Lockfile shows evidence of upgrade process // simple provider is upgraded to the newer 2.0.0 version, @@ -467,10 +489,16 @@ func TestProviderInstall_unmanaged(t *testing.T) { tf.AddEnv("TF_REATTACH_PROVIDERS=" + reattachConfig) // The init process should succeed. - stdout, stderr, err := tf.Run("init", fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) + stdout, stderr, err := tf.Run( + "init", + "-no-color", + fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) if err != nil { t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) } + if got, want := stdout, "Warning: Unmanaged providers are in effect"; !strings.Contains(got, want) { + t.Fatalf("stdout doesn't include the warning about unmanaged providers\nwant: %s\n%s", want, got) + } // Lock file should have been created buf, err := os.ReadFile(lockFile) @@ -530,10 +558,16 @@ provider "registry.terraform.io/hashicorp/simple6" { tf.AddEnv("TF_REATTACH_PROVIDERS=" + reattachConfig) // The init process should succeed. - stdout, stderr, err := tf.Run("init", fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) + stdout, stderr, err := tf.Run( + "init", + "-no-color", + fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) if err != nil { t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) } + if got, want := stdout, "Warning: Unmanaged providers are in effect"; !strings.Contains(got, want) { + t.Fatalf("stdout doesn't include the warning about unmanaged providers\nwant: %s\n%s", want, got) + } // Lockfile is unchanged despite use of an unmanaged simple6 provider buf, err := os.ReadFile(lockFile) @@ -579,10 +613,17 @@ provider "registry.terraform.io/hashicorp/simple6" { tf.AddEnv("TF_REATTACH_PROVIDERS=" + reattachConfig) // The init -upgrade process should succeed. - stdout, stderr, err := tf.Run("init", "-upgrade", fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) + stdout, stderr, err := tf.Run( + "init", + "-upgrade", + "-no-color", + fmt.Sprintf("-plugin-dir=%s", absolutePathToCache)) if err != nil { t.Fatalf("unexpected error: %s\nstdout: %s\nstderr: %s", err, stdout, stderr) } + if got, want := stdout, "Warning: Unmanaged providers are in effect"; !strings.Contains(got, want) { + t.Fatalf("stdout doesn't include the warning about unmanaged providers\nwant: %s\n%s", want, got) + } // Lockfile shows evidence of upgrade process // simple provider is upgraded to the newer 2.0.0 version, diff --git a/internal/command/init.go b/internal/command/init.go index e8b26ce095..0dc5b2f13f 100644 --- a/internal/command/init.go +++ b/internal/command/init.go @@ -376,13 +376,19 @@ func (c *InitCommand) getProvidersFromConfig(ctx context.Context, config *config ctx, span := tracer.Start(ctx, "install providers from config") defer span.End() - // Dev overrides cause the result of "terraform init" to be irrelevant for - // any overridden providers, so we'll warn about it to avoid later - // confusion when Terraform ends up using a different provider than the - // lock file called for. + // Dev overrides and unmanaged providers change the installation process in "terraform init"; + // overridden and unmanaged providers are skipped during installation. + // This means that impacted providers won't be downloaded from external sources nor added + // to the dependency lock file if they aren't already recorded there. Similarly, any attempt + // to upgrade providers will not affect providers impacted by overrides or unmanaged providers. // - // This warning is only added here to avoid duplication; not raised in getProvidersFromState. + // So, we'll warn users about it to avoid later confusion when Terraform ends up using + // a different provider than the lock file called for, or doesn't make expected changes + // to the lock file. + // + // This warning is only shown once, here, to avoid duplication; not raised in getProvidersFromState. diags = diags.Append(c.providerDevOverrideInitWarnings()) + diags = diags.Append(c.providerUnmanagedInitWarnings()) // Collect the provider dependencies from the configuration. reqs, hclDiags := config.ProviderRequirements() diff --git a/internal/command/meta_providers.go b/internal/command/meta_providers.go index 978f0d26cb..5b34bf3e59 100644 --- a/internal/command/meta_providers.go +++ b/internal/command/meta_providers.go @@ -10,6 +10,7 @@ import ( "log" "os" "os/exec" + "sort" "strings" "github.com/hashicorp/go-plugin" @@ -196,6 +197,37 @@ func (m *Meta) providerDevOverrideInitWarnings() tfdiags.Diagnostics { } } +// providerUnmanagedInitWarnings returns diagnostics containing at least one +// warning if and only if there is at least one unmanaged provider in effect +// via TF_REATTACH_PROVIDERS. +func (m *Meta) providerUnmanagedInitWarnings() tfdiags.Diagnostics { + if len(m.UnmanagedProviders) == 0 { + return nil + } + + var detailMsg strings.Builder + detailMsg.WriteString("The following unmanaged providers are set via the TF_REATTACH_PROVIDERS environment variable:\n") + + providerAddresses := make([]string, 0, len(m.UnmanagedProviders)) + for providerAddr := range m.UnmanagedProviders { + providerAddresses = append(providerAddresses, providerAddr.ForDisplay()) + } + sort.Strings(providerAddresses) // Enable deterministic ordering. + + for _, providerAddr := range providerAddresses { + detailMsg.WriteString(fmt.Sprintf(" - %s\n", providerAddr)) + } + detailMsg.WriteString("\nThese providers will not be installed as part of init, nor init -upgrade. Their entries in the dependency lock file will be left unchanged, if present. If this is unintentional please re-run without TF_REATTACH_PROVIDERS set.") + + return tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Warning, + "Unmanaged providers are in effect", + detailMsg.String(), + ), + } +} + func (m *Meta) isProviderDevOverride(pAddr addrs.Provider) bool { if len(m.ProviderDevOverrides) == 0 { return false diff --git a/internal/command/meta_providers_test.go b/internal/command/meta_providers_test.go index 0b50488985..96b201b767 100644 --- a/internal/command/meta_providers_test.go +++ b/internal/command/meta_providers_test.go @@ -17,6 +17,48 @@ import ( "github.com/hashicorp/terraform/internal/providercache" ) +func TestProviderUnmanagedWarnings(t *testing.T) { + t.Run("no warnings with no unmanaged providers", func(t *testing.T) { + meta := Meta{} + + diags := meta.providerUnmanagedInitWarnings() + if diags != nil { + t.Fatalf("expected no diagnostics, got %#v", diags) + } + }) + + t.Run("warnings with unmanaged providers present", func(t *testing.T) { + providerA := addrs.NewDefaultProvider("provider-a") + providerB := addrs.NewDefaultProvider("provider-b") + meta := Meta{ + UnmanagedProviders: map[addrs.Provider]*plugin.ReattachConfig{ + providerB: {}, + providerA: {}, + }, + } + + diags := meta.providerUnmanagedInitWarnings() + if diags == nil { + t.Fatal("expected diagnostics, got nil") + } + if diags.HasErrors() { + t.Fatalf("expected only warning diagnostics, got errors: %s", diags.ErrWithWarnings()) + } + + got := diags.ErrWithWarnings().Error() + // Output is deterministic so assert exact contents + want := `Unmanaged providers are in effect: The following unmanaged providers are set via the TF_REATTACH_PROVIDERS environment variable: + - hashicorp/provider-a + - hashicorp/provider-b + +These providers will not be installed as part of init, nor init -upgrade. Their entries in the dependency lock file will be left unchanged, if present. If this is unintentional please re-run without TF_REATTACH_PROVIDERS set.` + + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("unexpected difference in warning message: %s", diff) + } + }) +} + // Test the impacts of dev_overrides and reattached/unmanaged providers on the provider installation process. // The locks returned from EnsureProviderVersions are what's saved to the dependency lock file, so we are interested // in how the pre-existing locks and how providers are overidden impacts the locks returned from that installation process.