From 58bcc2e9bb89c9dc892479fccaec43062d000bcc Mon Sep 17 00:00:00 2001
From: Kristin Laemmert <mildwonkey@users.noreply.github.com>
Date: Mon, 8 Jun 2020 08:27:36 -0400
Subject: [PATCH] addrs: detect builtin provider when parsing legacy provider
 string (#25154)

* addrs: detect builtin provider when parsing legacy provider string

The ParseLegacyAbsProviderConfig was not detecting builtin providers
("terraform"), which caused issues for all users with 0.12 state and
the "terraform_remote_state" data source. Since "terraform" is the only
built-in provider this adds a very simple check to the parser so it
properly returns the builtin FQN.

* add tests to the addrs package
---
 addrs/provider_config.go                      |  9 ++++-
 addrs/provider_config_test.go                 | 37 +++++++++++++++++++
 .../roundtrip/v4-legacy-simple.in.tfstate     | 14 +++++++
 .../roundtrip/v4-legacy-simple.out.tfstate    | 14 +++++++
 states/statefile/version4.go                  |  3 +-
 5 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/addrs/provider_config.go b/addrs/provider_config.go
index f83ce8d48..5b1d2e1e0 100644
--- a/addrs/provider_config.go
+++ b/addrs/provider_config.go
@@ -279,9 +279,14 @@ func ParseLegacyAbsProviderConfig(traversal hcl.Traversal) (AbsProviderConfig, t
 		return ret, diags
 	}
 
-	// We always assume legacy-style providers in legacy state
+	// We always assume legacy-style providers in legacy state ...
 	if tt, ok := remain[1].(hcl.TraverseAttr); ok {
-		ret.Provider = NewLegacyProvider(tt.Name)
+		// ... unless it's the builtin "terraform" provider, a special case.
+		if tt.Name == "terraform" {
+			ret.Provider = NewBuiltInProvider(tt.Name)
+		} else {
+			ret.Provider = NewLegacyProvider(tt.Name)
+		}
 	} else {
 		diags = diags.Append(&hcl.Diagnostic{
 			Severity: hcl.DiagError,
diff --git a/addrs/provider_config_test.go b/addrs/provider_config_test.go
index deba9e713..9ceb2a43a 100644
--- a/addrs/provider_config_test.go
+++ b/addrs/provider_config_test.go
@@ -1,6 +1,7 @@
 package addrs
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/go-test/deep"
@@ -241,3 +242,39 @@ func TestAbsProviderConfigLegacyString(t *testing.T) {
 		}
 	}
 }
+
+func TestParseLegacyAbsProviderConfigStr(t *testing.T) {
+	tests := []struct {
+		Config string
+		Want   AbsProviderConfig
+	}{
+		{
+			`provider.foo`,
+			AbsProviderConfig{
+				Module:   RootModule,
+				Provider: NewLegacyProvider("foo"),
+			},
+		},
+		{
+			`module.child_module.provider.foo`,
+			AbsProviderConfig{
+				Module:   RootModule.Child("child_module"),
+				Provider: NewLegacyProvider("foo"),
+			},
+		},
+		{
+			`provider.terraform`,
+			AbsProviderConfig{
+				Module:   RootModule,
+				Provider: NewBuiltInProvider("terraform"),
+			},
+		},
+	}
+
+	for _, test := range tests {
+		got, _ := ParseLegacyAbsProviderConfigStr(test.Config)
+		if !reflect.DeepEqual(got, test.Want) {
+			t.Errorf("wrong result. Got %s, want %s\n", got, test.Want)
+		}
+	}
+}
diff --git a/states/statefile/testdata/roundtrip/v4-legacy-simple.in.tfstate b/states/statefile/testdata/roundtrip/v4-legacy-simple.in.tfstate
index 2924215a9..946c4f82a 100644
--- a/states/statefile/testdata/roundtrip/v4-legacy-simple.in.tfstate
+++ b/states/statefile/testdata/roundtrip/v4-legacy-simple.in.tfstate
@@ -10,6 +10,20 @@
         }
     },
     "resources": [
+        {
+            "mode": "data",
+            "type": "terraform_remote_state",
+            "name": "random",
+            "provider": "provider.terraform",
+            "instances": [
+                {
+                    "schema_version": 0,
+                    "attributes_flat": {
+                        "backend": "remote"
+                    }
+                }
+            ]
+        },
         {
             "mode": "managed",
             "type": "null_resource",
diff --git a/states/statefile/testdata/roundtrip/v4-legacy-simple.out.tfstate b/states/statefile/testdata/roundtrip/v4-legacy-simple.out.tfstate
index 5d3c0af9f..0c19a64a3 100644
--- a/states/statefile/testdata/roundtrip/v4-legacy-simple.out.tfstate
+++ b/states/statefile/testdata/roundtrip/v4-legacy-simple.out.tfstate
@@ -10,6 +10,20 @@
         }
     },
     "resources": [
+        {
+            "mode": "data",
+            "type": "terraform_remote_state",
+            "name": "random",
+            "provider": "provider[\"terraform.io/builtin/terraform\"]",
+            "instances": [
+                {
+                    "schema_version": 0,
+                    "attributes_flat": {
+                        "backend": "remote"
+                    }
+                }
+            ]
+        },
         {
             "mode": "managed",
             "type": "null_resource",
diff --git a/states/statefile/version4.go b/states/statefile/version4.go
index 04701f49d..ee63fb3d7 100644
--- a/states/statefile/version4.go
+++ b/states/statefile/version4.go
@@ -87,7 +87,8 @@ func prepareStateV4(sV4 *stateV4) (*File, tfdiags.Diagnostics) {
 			// If ParseAbsProviderConfigStr returns an error, the state may have
 			// been written before Provider FQNs were introduced and the
 			// AbsProviderConfig string format will need normalization. If so,
-			// we assume it is a default (hashicorp) provider.
+			// we treat it like a legacy provider (namespace "-") and let the
+			// provider installer handle detecting the FQN.
 			var legacyAddrDiags tfdiags.Diagnostics
 			providerAddr, legacyAddrDiags = addrs.ParseLegacyAbsProviderConfigStr(rsV4.ProviderConfig)
 			if legacyAddrDiags.HasErrors() {