terraform test: Disallow version constraints within test files (#35161)

internal/command/init_test.go

@@ -25,6 +25,8 @@ import (
 	"github.com/hashicorp/terraform/internal/depsfile"
 	"github.com/hashicorp/terraform/internal/getproviders"
 	"github.com/hashicorp/terraform/internal/providercache"
+	"github.com/hashicorp/terraform/internal/providers"
+	testing_provider "github.com/hashicorp/terraform/internal/providers/testing"
 	"github.com/hashicorp/terraform/internal/states"
 	"github.com/hashicorp/terraform/internal/states/statefile"
 	"github.com/hashicorp/terraform/internal/states/statemgr"
@@ -2831,6 +2833,44 @@ func TestInit_invalidSyntaxBackendAttribute(t *testing.T) {
 	}
 }
 
+func TestInit_testsWithExternalProviders(t *testing.T) {
+	td := t.TempDir()
+	testCopyDir(t, testFixturePath("init-with-tests-external-providers"), td)
+	defer testChdir(t, td)()
+
+	providerSource, close := newMockProviderSource(t, map[string][]string{
+		"hashicorp/testing": {"1.0.0"},
+		"testing/configure": {"1.0.0"},
+	})
+	defer close()
+
+	hashicorpTestingProviderAddress := addrs.NewDefaultProvider("testing")
+	hashicorpTestingProvider := new(testing_provider.MockProvider)
+	testingConfigureProviderAddress := addrs.NewProvider(addrs.DefaultProviderRegistryHost, "testing", "configure")
+	testingConfigureProvider := new(testing_provider.MockProvider)
+
+	ui := new(cli.MockUi)
+	view, done := testView(t)
+	c := &InitCommand{
+		Meta: Meta{
+			testingOverrides: &testingOverrides{
+				Providers: map[addrs.Provider]providers.Factory{
+					hashicorpTestingProviderAddress: providers.FactoryFixed(hashicorpTestingProvider),
+					testingConfigureProviderAddress: providers.FactoryFixed(testingConfigureProvider),
+				},
+			},
+			Ui:             ui,
+			View:           view,
+			ProviderSource: providerSource,
+		},
+	}
+
+	var args []string
+	if code := c.Run(args); code != 0 {
+		t.Fatalf("bad: \n%s", done(t).All())
+	}
+}
+
 func TestInit_tests(t *testing.T) {
 	// Create a temporary working directory that is empty
 	td := t.TempDir()

internal/command/providers_test.go

@@ -192,7 +192,6 @@ func TestProviders_tests(t *testing.T) {
 	}
 
 	wantOutput := []string{
-		"provider[registry.terraform.io/hashicorp/foo]",
 		"test.main",
 		"provider[registry.terraform.io/hashicorp/bar]",
 	}

internal/command/testdata/init-with-tests-external-providers/main.tf

@@ -0,0 +1,3 @@
+resource "testing_instance" "baz" {
+  ami = "baz"
+}

internal/command/testdata/init-with-tests-external-providers/main.tftest.hcl

@@ -0,0 +1,25 @@
+
+// configure is not a "hashicorp" provider, so it won't be able to load
+// this using the default behaviour. Terraform will need to look into the setup
+// module to find the provider configuration.
+provider "configure" {}
+
+// testing is a "hashicorp" provider, so it can load this using the defaults
+// even though not required provider block providers a definition for it.
+provider "testing" {}
+
+run "setup" {
+  module {
+    source = "./setup"
+  }
+
+  providers = {
+    configure = configure
+  }
+}
+
+run "test" {
+  providers = {
+    testing = testing
+  }
+}

internal/command/testdata/init-with-tests-external-providers/setup/main.tf

@@ -0,0 +1,11 @@
+terraform {
+  required_providers {
+    configure = {
+      source = "testing/configure"
+    }
+  }
+}
+
+resource "configure_instance" "baz" {
+  ami = "baz"
+}

internal/command/testdata/providers/tests/main.tftest.hcl

@@ -1,3 +1,5 @@
+// This won't actually show up in the providers list, as nothing is actually
+// using it.
 provider "foo" {
 
 }

internal/configs/config.go

@@ -384,10 +384,6 @@ func (c *Config) ProviderRequirementsByModule() (*ModuleRequirements, hcl.Diagno
 			Runs:         make(map[string]*ModuleRequirements),
 		}
 
-		for _, provider := range test.Providers {
-			diags = append(diags, c.addProviderRequirementsFromProviderBlock(testReqs.Requirements, provider)...)
-		}
-
 		for _, run := range test.Runs {
 			if run.ConfigUnderTest == nil {
 				continue
@@ -556,20 +552,13 @@ func (c *Config) addProviderRequirements(reqs providerreqs.Requirements, recurse
 
 	// We may have provider blocks and required_providers set in some testing
 	// files.
-	if tests {
+	if tests && recurse {
 		for _, file := range c.Module.Tests {
-			for _, provider := range file.Providers {
-				moreDiags := c.addProviderRequirementsFromProviderBlock(reqs, provider)
-				diags = append(diags, moreDiags...)
-			}
-
-			if recurse {
-				// Then we'll also look for requirements in testing modules.
-				for _, run := range file.Runs {
-					if run.ConfigUnderTest != nil {
-						moreDiags := run.ConfigUnderTest.addProviderRequirements(reqs, true, false)
-						diags = append(diags, moreDiags...)
-					}
+			// Then we'll also look for requirements in testing modules.
+			for _, run := range file.Runs {
+				if run.ConfigUnderTest != nil {
+					moreDiags := run.ConfigUnderTest.addProviderRequirements(reqs, true, false)
+					diags = append(diags, moreDiags...)
 				}
 			}
 		}

internal/configs/config_test.go

@@ -165,12 +165,7 @@ func TestConfigProviderRequirements(t *testing.T) {
 
 func TestConfigProviderRequirementsInclTests(t *testing.T) {
 	cfg, diags := testNestedModuleConfigFromDirWithTests(t, "testdata/provider-reqs-with-tests")
-	// TODO: Version Constraint Deprecation.
-	// Once we've removed the version argument from provider configuration
-	// blocks, this can go back to expected 0 diagnostics.
-	// assertNoDiagnostics(t, diags)
-	assertDiagnosticCount(t, diags, 1)
-	assertDiagnosticSummary(t, diags, "Version constraints inside provider configuration blocks are deprecated")
+	assertDiagnosticCount(t, diags, 0)
 
 	tlsProvider := addrs.NewProvider(
 		addrs.DefaultProviderRegistryHost,
@@ -189,7 +184,7 @@ func TestConfigProviderRequirementsInclTests(t *testing.T) {
 		nullProvider:       providerreqs.MustParseVersionConstraints("~> 2.0.0"),
 		randomProvider:     providerreqs.MustParseVersionConstraints("~> 1.2.0"),
 		tlsProvider:        providerreqs.MustParseVersionConstraints("~> 3.0"),
-		configuredProvider: providerreqs.MustParseVersionConstraints("~> 1.4"),
+		configuredProvider: nil,
 		impliedProvider:    nil,
 		terraformProvider:  nil,
 	}
@@ -243,12 +238,7 @@ func TestConfigProviderRequirementsShallow(t *testing.T) {
 
 func TestConfigProviderRequirementsShallowInclTests(t *testing.T) {
 	cfg, diags := testNestedModuleConfigFromDirWithTests(t, "testdata/provider-reqs-with-tests")
-	// TODO: Version Constraint Deprecation.
-	// Once we've removed the version argument from provider configuration
-	// blocks, this can go back to expected 0 diagnostics.
-	// assertNoDiagnostics(t, diags)
-	assertDiagnosticCount(t, diags, 1)
-	assertDiagnosticSummary(t, diags, "Version constraints inside provider configuration blocks are deprecated")
+	assertDiagnosticCount(t, diags, 0)
 
 	tlsProvider := addrs.NewProvider(
 		addrs.DefaultProviderRegistryHost,
@@ -256,15 +246,13 @@ func TestConfigProviderRequirementsShallowInclTests(t *testing.T) {
 	)
 	impliedProvider := addrs.NewDefaultProvider("implied")
 	terraformProvider := addrs.NewBuiltInProvider("terraform")
-	configuredProvider := addrs.NewDefaultProvider("configured")
 
 	got, diags := cfg.ProviderRequirementsShallow()
 	assertNoDiagnostics(t, diags)
 	want := providerreqs.Requirements{
-		tlsProvider:        providerreqs.MustParseVersionConstraints("~> 3.0"),
-		configuredProvider: providerreqs.MustParseVersionConstraints("~> 1.4"),
-		impliedProvider:    nil,
-		terraformProvider:  nil,
+		tlsProvider:       providerreqs.MustParseVersionConstraints("~> 3.0"),
+		impliedProvider:   nil,
+		terraformProvider: nil,
 	}
 
 	if diff := cmp.Diff(want, got); diff != "" {
@@ -346,12 +334,7 @@ func TestConfigProviderRequirementsByModule(t *testing.T) {
 
 func TestConfigProviderRequirementsByModuleInclTests(t *testing.T) {
 	cfg, diags := testNestedModuleConfigFromDirWithTests(t, "testdata/provider-reqs-with-tests")
-	// TODO: Version Constraint Deprecation.
-	// Once we've removed the version argument from provider configuration
-	// blocks, this can go back to expected 0 diagnostics.
-	// assertNoDiagnostics(t, diags)
-	assertDiagnosticCount(t, diags, 1)
-	assertDiagnosticSummary(t, diags, "Version constraints inside provider configuration blocks are deprecated")
+	assertDiagnosticCount(t, diags, 0)
 
 	tlsProvider := addrs.NewProvider(
 		addrs.DefaultProviderRegistryHost,
@@ -378,17 +361,16 @@ func TestConfigProviderRequirementsByModuleInclTests(t *testing.T) {
 		Children: make(map[string]*ModuleRequirements),
 		Tests: map[string]*TestFileModuleRequirements{
 			"provider-reqs-root.tftest.hcl": {
-				Requirements: providerreqs.Requirements{
-					configuredProvider: providerreqs.MustParseVersionConstraints("~> 1.4"),
-				},
+				Requirements: providerreqs.Requirements{},
 				Runs: map[string]*ModuleRequirements{
 					"setup": {
 						Name:       "setup",
 						SourceAddr: addrs.ModuleSourceLocal("./setup"),
 						SourceDir:  "testdata/provider-reqs-with-tests/setup",
 						Requirements: providerreqs.Requirements{
-							nullProvider:   providerreqs.MustParseVersionConstraints("~> 2.0.0"),
-							randomProvider: providerreqs.MustParseVersionConstraints("~> 1.2.0"),
+							nullProvider:       providerreqs.MustParseVersionConstraints("~> 2.0.0"),
+							randomProvider:     providerreqs.MustParseVersionConstraints("~> 1.2.0"),
+							configuredProvider: nil,
 						},
 						Children: make(map[string]*ModuleRequirements),
 						Tests:    make(map[string]*TestFileModuleRequirements),

internal/configs/parser_config.go

@@ -147,7 +147,7 @@ func parseConfigFile(body hcl.Body, diags hcl.Diagnostics, override, allowExperi
 			})
 
 		case "provider":
-			cfg, cfgDiags := decodeProviderBlock(block)
+			cfg, cfgDiags := decodeProviderBlock(block, false)
 			diags = append(diags, cfgDiags...)
 			if cfg != nil {
 				file.ProviderConfigs = append(file.ProviderConfigs, cfg)

internal/configs/provider.go

@@ -48,7 +48,7 @@ type Provider struct {
 	MockDataExternalSource string
 }
 
-func decodeProviderBlock(block *hcl.Block) (*Provider, hcl.Diagnostics) {
+func decodeProviderBlock(block *hcl.Block, testFile bool) (*Provider, hcl.Diagnostics) {
 	var diags hcl.Diagnostics
 
 	content, config, moreDiags := block.Body.PartialContent(providerBlockSchema)
@@ -92,15 +92,24 @@ func decodeProviderBlock(block *hcl.Block) (*Provider, hcl.Diagnostics) {
 	}
 
 	if attr, exists := content.Attributes["version"]; exists {
-		diags = append(diags, &hcl.Diagnostic{
-			Severity: hcl.DiagWarning,
-			Summary:  "Version constraints inside provider configuration blocks are deprecated",
-			Detail:   "Terraform 0.13 and earlier allowed provider version constraints inside the provider configuration block, but that is now deprecated and will be removed in a future version of Terraform. To silence this warning, move the provider version constraint into the required_providers block.",
-			Subject:  attr.Expr.Range().Ptr(),
-		})
-		var versionDiags hcl.Diagnostics
-		provider.Version, versionDiags = decodeVersionConstraint(attr)
-		diags = append(diags, versionDiags...)
+		if testFile {
+			diags = append(diags, &hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Version constraints are not allowed in test files",
+				Detail:   "Version constraints inside provider configuration blocks are not allowed in test files. To silence this error, move the provider version constraint into the required_providers block of the configuration that uses this provider.",
+				Subject:  attr.Expr.Range().Ptr(),
+			})
+		} else {
+			diags = append(diags, &hcl.Diagnostic{
+				Severity: hcl.DiagWarning,
+				Summary:  "Version constraints inside provider configuration blocks are deprecated",
+				Detail:   "Terraform 0.13 and earlier allowed provider version constraints inside the provider configuration block, but that is now deprecated and will be removed in a future version of Terraform. To silence this warning, move the provider version constraint into the required_providers block.",
+				Subject:  attr.Expr.Range().Ptr(),
+			})
+			var versionDiags hcl.Diagnostics
+			provider.Version, versionDiags = decodeVersionConstraint(attr)
+			diags = append(diags, versionDiags...)
+		}
 	}
 
 	// Reserved attribute names

internal/configs/test_file.go

@@ -337,7 +337,7 @@ func loadTestFile(body hcl.Body) (*TestFile, hcl.Diagnostics) {
 				tf.Variables[v.Name] = v.Expr
 			}
 		case "provider":
-			provider, providerDiags := decodeProviderBlock(block)
+			provider, providerDiags := decodeProviderBlock(block, true)
 			diags = append(diags, providerDiags...)
 			if provider != nil {
 				key := provider.moduleUniqueKey()

internal/configs/testdata/provider-reqs-with-tests/provider-reqs-root.tftest.hcl

@@ -1,8 +1,6 @@
-# There is no provider in required_providers called "configured", so the version
-# constraint should come from this configuration block.
-provider "configured" {
-  version = "~> 1.4"
-}
+# There is no provider in required_providers called "configured", so we won't
+# have a version constraint for it.
+provider "configured" {}
 
 run "setup" {
   module {

internal/configs/testdata/provider-reqs-with-tests/setup/setup.tf

@@ -6,3 +6,5 @@ terraform {
     }
   }
 }
+
+resource "configured_resource" "resource" {}

internal/moduletest/config/config.go

@@ -87,7 +87,6 @@ func TransformConfigForTest(config *configs.Config, run *moduletest.Run, file *m
 				NameRange:  ref.InChild.NameRange,
 				Alias:      ref.InChild.Alias,
 				AliasRange: ref.InChild.AliasRange,
-				Version:    testProvider.Version,
 				Config: &hcltest.ProviderConfig{
 					Original:            testProvider.Config,
 					VariableCache:       variableCaches.GetCache(run.Name, config),
@@ -114,7 +113,6 @@ func TransformConfigForTest(config *configs.Config, run *moduletest.Run, file *m
 				NameRange:  provider.NameRange,
 				Alias:      provider.Alias,
 				AliasRange: provider.AliasRange,
-				Version:    provider.Version,
 				Config: &hcltest.ProviderConfig{
 					Original:            provider.Config,
 					VariableCache:       variableCaches.GetCache(run.Name, config),