Use import provider instead of guess (#2336)

Signed-off-by: Christian Mesh <christianmesh1@gmail.com>

CHANGELOG.md

@@ -21,6 +21,7 @@ BUG FIXES:
 - Fixed an issue where an invalid provider name in the `provider_meta` block would crash OpenTofu rather than report an error ([#2347](https://github.com/opentofu/opentofu/pull/2347))
 - When assigning an empty map to a variable that is declared as a map of an object type with at least one optional attribute, OpenTofu will no longer create a subtly-broken value. ([#2371](https://github.com/opentofu/opentofu/pull/2371))
 - The `format` and `formatlist` functions can now accept `null` as one of the arguments without causing problems during the apply phase. Previously these functions would incorrectly return an unknown value when given `null` and so could cause a failure during the apply phase where no unknown values are allowed. ([#2371](https://github.com/opentofu/opentofu/pull/2371))
+- Provider used in import is correctly identified. ([#2336](https://github.com/opentofu/opentofu/pull/2336))
 
 ## Previous Releases
 

internal/configs/config.go

@@ -431,19 +431,6 @@ func (c *Config) addProviderRequirements(reqs getproviders.Requirements, recurse
 		}
 		reqs[fqn] = nil
 	}
-	for _, i := range c.Module.Import {
-		implied, err := addrs.ParseProviderPart(i.StaticTo.Resource.ImpliedProvider())
-		if err == nil {
-			provider := c.Module.ImpliedProviderForUnqualifiedType(implied)
-			if _, exists := reqs[provider]; exists {
-				// Explicit dependency already present
-				continue
-			}
-			reqs[provider] = nil
-		}
-		// We don't return a diagnostic here, because the invalid address will
-		// have been caught elsewhere.
-	}
 
 	// Import blocks that are generating config may also have a custom provider
 	// meta argument. Like the provider meta argument used in resource blocks,
@@ -454,6 +441,14 @@ func (c *Config) addProviderRequirements(reqs getproviders.Requirements, recurse
 	// this will be because the user has written explicit provider arguments
 	// that don't agree and we'll get them to fix it.
 	for _, i := range c.Module.Import {
+		// Add the import's declared or implicit provider
+		fqn := i.Provider
+		if _, exists := reqs[fqn]; !exists {
+			reqs[fqn] = nil
+		}
+
+		// TODO: This should probably be moved to provider_validation.go so that
+		// import providers can be properly validated across modules (root -> children)
 		if len(i.StaticTo.Module) > 0 {
 			// All provider information for imports into modules should come
 			// from the module block, so we don't need to load anything for
@@ -479,7 +474,7 @@ func (c *Config) addProviderRequirements(reqs getproviders.Requirements, recurse
 					diags = append(diags, &hcl.Diagnostic{
 						Severity: hcl.DiagError,
 						Summary:  "Invalid import provider argument",
-						Detail:   "The provider argument can only be specified in import blocks that will generate configuration.\n\nUse the provider argument in the target resource block to configure the provider for a resource with explicit provider configuration.",
+						Detail:   "The provider argument in the target resource block must be specified and match the import block.",
 						Subject:  i.ProviderDeclRange.Ptr(),
 					})
 					continue
@@ -499,27 +494,13 @@ func (c *Config) addProviderRequirements(reqs getproviders.Requirements, recurse
 					diags = append(diags, &hcl.Diagnostic{
 						Severity: hcl.DiagError,
 						Summary:  "Invalid import provider argument",
-						Detail:   "The provider argument can only be specified in import blocks that will generate configuration.\n\nUse the provider argument in the target resource block to configure the provider for a resource with explicit provider configuration.",
+						Detail:   "The provider argument in the target resource block must match the import block.",
 						Subject:  i.ProviderDeclRange.Ptr(),
 					})
 					continue
 				}
 			}
-
-			// All the provider information should come from the target resource
-			// which has already been processed, so skip the rest of this
-			// processing.
-			continue
 		}
-
-		// Otherwise we are generating config for the resource being imported,
-		// so all the provider information must come from this import block.
-		fqn := i.Provider
-		if _, exists := reqs[fqn]; exists {
-			// Explicit dependency already present
-			continue
-		}
-		reqs[fqn] = nil
 	}
 
 	// "provider" block can also contain version constraints

internal/configs/config_test.go

@@ -149,6 +149,8 @@ func TestConfigProviderRequirements(t *testing.T) {
 	nullProvider := addrs.NewDefaultProvider("null")
 	randomProvider := addrs.NewDefaultProvider("random")
 	impliedProvider := addrs.NewDefaultProvider("implied")
+	importimpliedProvider := addrs.NewDefaultProvider("importimplied")
+	importexplicitProvider := addrs.NewDefaultProvider("importexplicit")
 	terraformProvider := addrs.NewBuiltInProvider("terraform")
 	configuredProvider := addrs.NewDefaultProvider("configured")
 	grandchildProvider := addrs.NewDefaultProvider("grandchild")
@@ -157,14 +159,16 @@ func TestConfigProviderRequirements(t *testing.T) {
 	assertNoDiagnostics(t, diags)
 	want := getproviders.Requirements{
 		// the nullProvider constraints from the two modules are merged
-		nullProvider:       getproviders.MustParseVersionConstraints("~> 2.0.0, 2.0.1"),
-		randomProvider:     getproviders.MustParseVersionConstraints("~> 1.2.0"),
-		tlsProvider:        getproviders.MustParseVersionConstraints("~> 3.0"),
-		configuredProvider: getproviders.MustParseVersionConstraints("~> 1.4"),
-		impliedProvider:    nil,
-		happycloudProvider: nil,
-		terraformProvider:  nil,
-		grandchildProvider: nil,
+		nullProvider:           getproviders.MustParseVersionConstraints("~> 2.0.0, 2.0.1"),
+		randomProvider:         getproviders.MustParseVersionConstraints("~> 1.2.0"),
+		tlsProvider:            getproviders.MustParseVersionConstraints("~> 3.0"),
+		configuredProvider:     getproviders.MustParseVersionConstraints("~> 1.4"),
+		impliedProvider:        nil,
+		importimpliedProvider:  nil,
+		importexplicitProvider: nil,
+		happycloudProvider:     nil,
+		terraformProvider:      nil,
+		grandchildProvider:     nil,
 	}
 
 	if diff := cmp.Diff(want, got); diff != "" {
@@ -230,6 +234,8 @@ func TestConfigProviderRequirementsShallow(t *testing.T) {
 	nullProvider := addrs.NewDefaultProvider("null")
 	randomProvider := addrs.NewDefaultProvider("random")
 	impliedProvider := addrs.NewDefaultProvider("implied")
+	importimpliedProvider := addrs.NewDefaultProvider("importimplied")
+	importexplicitProvider := addrs.NewDefaultProvider("importexplicit")
 	terraformProvider := addrs.NewBuiltInProvider("terraform")
 	configuredProvider := addrs.NewDefaultProvider("configured")
 
@@ -237,12 +243,14 @@ func TestConfigProviderRequirementsShallow(t *testing.T) {
 	assertNoDiagnostics(t, diags)
 	want := getproviders.Requirements{
 		// the nullProvider constraint is only from the root module
-		nullProvider:       getproviders.MustParseVersionConstraints("~> 2.0.0"),
-		randomProvider:     getproviders.MustParseVersionConstraints("~> 1.2.0"),
-		tlsProvider:        getproviders.MustParseVersionConstraints("~> 3.0"),
-		configuredProvider: getproviders.MustParseVersionConstraints("~> 1.4"),
-		impliedProvider:    nil,
-		terraformProvider:  nil,
+		nullProvider:           getproviders.MustParseVersionConstraints("~> 2.0.0"),
+		randomProvider:         getproviders.MustParseVersionConstraints("~> 1.2.0"),
+		tlsProvider:            getproviders.MustParseVersionConstraints("~> 3.0"),
+		configuredProvider:     getproviders.MustParseVersionConstraints("~> 1.4"),
+		impliedProvider:        nil,
+		importimpliedProvider:  nil,
+		importexplicitProvider: nil,
+		terraformProvider:      nil,
 	}
 
 	if diff := cmp.Diff(want, got); diff != "" {
@@ -301,6 +309,8 @@ func TestConfigProviderRequirementsByModule(t *testing.T) {
 	nullProvider := addrs.NewDefaultProvider("null")
 	randomProvider := addrs.NewDefaultProvider("random")
 	impliedProvider := addrs.NewDefaultProvider("implied")
+	importimpliedProvider := addrs.NewDefaultProvider("importimplied")
+	importexplicitProvider := addrs.NewDefaultProvider("importexplicit")
 	terraformProvider := addrs.NewBuiltInProvider("terraform")
 	configuredProvider := addrs.NewDefaultProvider("configured")
 	grandchildProvider := addrs.NewDefaultProvider("grandchild")
@@ -313,12 +323,14 @@ func TestConfigProviderRequirementsByModule(t *testing.T) {
 		SourceDir:  "testdata/provider-reqs",
 		Requirements: getproviders.Requirements{
 			// Only the root module's version is present here
-			nullProvider:       getproviders.MustParseVersionConstraints("~> 2.0.0"),
-			randomProvider:     getproviders.MustParseVersionConstraints("~> 1.2.0"),
-			tlsProvider:        getproviders.MustParseVersionConstraints("~> 3.0"),
-			configuredProvider: getproviders.MustParseVersionConstraints("~> 1.4"),
-			impliedProvider:    nil,
-			terraformProvider:  nil,
+			nullProvider:           getproviders.MustParseVersionConstraints("~> 2.0.0"),
+			randomProvider:         getproviders.MustParseVersionConstraints("~> 1.2.0"),
+			tlsProvider:            getproviders.MustParseVersionConstraints("~> 3.0"),
+			configuredProvider:     getproviders.MustParseVersionConstraints("~> 1.4"),
+			impliedProvider:        nil,
+			importimpliedProvider:  nil,
+			importexplicitProvider: nil,
+			terraformProvider:      nil,
 		},
 		Children: map[string]*ModuleRequirements{
 			"kinder": {
@@ -433,6 +445,8 @@ func TestVerifyDependencySelections(t *testing.T) {
 	nullProvider := addrs.NewDefaultProvider("null")
 	randomProvider := addrs.NewDefaultProvider("random")
 	impliedProvider := addrs.NewDefaultProvider("implied")
+	importimpliedProvider := addrs.NewDefaultProvider("importimplied")
+	importexplicitProvider := addrs.NewDefaultProvider("importexplicit")
 	configuredProvider := addrs.NewDefaultProvider("configured")
 	grandchildProvider := addrs.NewDefaultProvider("grandchild")
 
@@ -448,6 +462,8 @@ func TestVerifyDependencySelections(t *testing.T) {
 				`provider registry.opentofu.org/hashicorp/configured: required by this configuration but no version is selected`,
 				`provider registry.opentofu.org/hashicorp/grandchild: required by this configuration but no version is selected`,
 				`provider registry.opentofu.org/hashicorp/implied: required by this configuration but no version is selected`,
+				`provider registry.opentofu.org/hashicorp/importexplicit: required by this configuration but no version is selected`,
+				`provider registry.opentofu.org/hashicorp/importimplied: required by this configuration but no version is selected`,
 				`provider registry.opentofu.org/hashicorp/null: required by this configuration but no version is selected`,
 				`provider registry.opentofu.org/hashicorp/random: required by this configuration but no version is selected`,
 				`provider registry.opentofu.org/hashicorp/tls: required by this configuration but no version is selected`,
@@ -459,6 +475,8 @@ func TestVerifyDependencySelections(t *testing.T) {
 				locks.SetProvider(configuredProvider, getproviders.MustParseVersion("1.4.0"), nil, nil)
 				locks.SetProvider(grandchildProvider, getproviders.MustParseVersion("0.1.0"), nil, nil)
 				locks.SetProvider(impliedProvider, getproviders.MustParseVersion("0.2.0"), nil, nil)
+				locks.SetProvider(importimpliedProvider, getproviders.MustParseVersion("0.2.0"), nil, nil)
+				locks.SetProvider(importexplicitProvider, getproviders.MustParseVersion("0.2.0"), nil, nil)
 				locks.SetProvider(nullProvider, getproviders.MustParseVersion("2.0.1"), nil, nil)
 				locks.SetProvider(randomProvider, getproviders.MustParseVersion("1.2.2"), nil, nil)
 				locks.SetProvider(tlsProvider, getproviders.MustParseVersion("3.0.1"), nil, nil)
@@ -471,6 +489,8 @@ func TestVerifyDependencySelections(t *testing.T) {
 				locks.SetProvider(configuredProvider, getproviders.MustParseVersion("1.4.0"), nil, nil)
 				locks.SetProvider(grandchildProvider, getproviders.MustParseVersion("0.1.0"), nil, nil)
 				locks.SetProvider(impliedProvider, getproviders.MustParseVersion("0.2.0"), nil, nil)
+				locks.SetProvider(importimpliedProvider, getproviders.MustParseVersion("0.2.0"), nil, nil)
+				locks.SetProvider(importexplicitProvider, getproviders.MustParseVersion("0.2.0"), nil, nil)
 				locks.SetProvider(nullProvider, getproviders.MustParseVersion("3.0.0"), nil, nil)
 				locks.SetProvider(randomProvider, getproviders.MustParseVersion("1.2.2"), nil, nil)
 				locks.SetProvider(tlsProvider, getproviders.MustParseVersion("3.0.1"), nil, nil)
@@ -490,6 +510,8 @@ func TestVerifyDependencySelections(t *testing.T) {
 				locks.SetProvider(configuredProvider, getproviders.MustParseVersion("1.4.0"), nil, nil)
 				locks.SetProvider(grandchildProvider, getproviders.MustParseVersion("0.1.0"), nil, nil)
 				locks.SetProvider(impliedProvider, getproviders.MustParseVersion("0.2.0"), nil, nil)
+				locks.SetProvider(importimpliedProvider, getproviders.MustParseVersion("0.2.0"), nil, nil)
+				locks.SetProvider(importexplicitProvider, getproviders.MustParseVersion("0.2.0"), nil, nil)
 				locks.SetProvider(randomProvider, getproviders.MustParseVersion("1.2.2"), nil, nil)
 				locks.SetProvider(tlsProvider, getproviders.MustParseVersion("3.0.1"), nil, nil)
 				locks.SetProvider(happycloudProvider, getproviders.MustParseVersion("0.0.1"), nil, nil)
@@ -507,6 +529,8 @@ func TestVerifyDependencySelections(t *testing.T) {
 				`provider registry.opentofu.org/hashicorp/configured: required by this configuration but no version is selected`,
 				`provider registry.opentofu.org/hashicorp/grandchild: required by this configuration but no version is selected`,
 				`provider registry.opentofu.org/hashicorp/implied: required by this configuration but no version is selected`,
+				`provider registry.opentofu.org/hashicorp/importexplicit: required by this configuration but no version is selected`,
+				`provider registry.opentofu.org/hashicorp/importimplied: required by this configuration but no version is selected`,
 				`provider registry.opentofu.org/hashicorp/null: required by this configuration but no version is selected`,
 				`provider registry.opentofu.org/hashicorp/random: required by this configuration but no version is selected`,
 				`provider registry.opentofu.org/hashicorp/tls: required by this configuration but no version is selected`,
@@ -588,9 +612,7 @@ func TestConfigImportProviderClashesWithResources(t *testing.T) {
 
 	diags = cfg.addProviderRequirements(getproviders.Requirements{}, true, false)
 	assertExactDiagnostics(t, diags, []string{
-		`testdata/invalid-import-files/import-and-resource-clash.tf:9,3-19: Invalid import provider argument; The provider argument can only be specified in import blocks that will generate configuration.
-
-Use the provider argument in the target resource block to configure the provider for a resource with explicit provider configuration.`,
+		`testdata/invalid-import-files/import-and-resource-clash.tf:9,3-19: Invalid import provider argument; The provider argument in the target resource block must match the import block.`,
 	})
 }
 
@@ -600,9 +622,7 @@ func TestConfigImportProviderWithNoResourceProvider(t *testing.T) {
 
 	diags = cfg.addProviderRequirements(getproviders.Requirements{}, true, false)
 	assertExactDiagnostics(t, diags, []string{
-		`testdata/invalid-import-files/import-and-no-resource.tf:5,3-19: Invalid import provider argument; The provider argument can only be specified in import blocks that will generate configuration.
-
-Use the provider argument in the target resource block to configure the provider for a resource with explicit provider configuration.`,
+		`testdata/invalid-import-files/import-and-no-resource.tf:5,3-19: Invalid import provider argument; The provider argument in the target resource block must be specified and match the import block.`,
 	})
 }
 

internal/configs/parser_test.go

@@ -180,13 +180,13 @@ func assertExactDiagnostics(t *testing.T, diags hcl.Diagnostics, want []string)
 	bad := false
 	for got := range gotDiags {
 		if _, exists := wantDiags[got]; !exists {
-			t.Errorf("unexpected diagnostic: %s", got)
+			t.Errorf("unexpected diagnostic: \n%s", got)
 			bad = true
 		}
 	}
 	for want := range wantDiags {
 		if _, exists := gotDiags[want]; !exists {
-			t.Errorf("missing expected diagnostic: %s", want)
+			t.Errorf("missing expected diagnostic: \n%s", want)
 			bad = true
 		}
 	}

internal/configs/testdata/provider-reqs/provider-reqs-root.tf

@@ -32,3 +32,15 @@ data "terraform_remote_state" "bar" {
 provider "configured" {
   version = "~> 1.4"
 }
+
+# Import using implied provider
+import {
+  to = importimplied.targetA
+  id = "ii"
+}
+
+import {
+  to = importimplied.targetB
+  id = "ie"
+  provider = importexplicit
+}