From d41fc8d51751996e289b9d291bc644f7b17110ed Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Thu, 18 Jul 2019 17:35:47 -0700
Subject: [PATCH] states/statefile: additional context for some v3 upgrade
 error messages

Some of our errors returned here were lacking context about what part of
the file was problematic, which led to some useless error reporting for
some real-world situations that this upgrade process doesn't seem to be
catching.

Here we add additional context to those error cases, as a step towards
tracking down exactly which upgrade cases are missing here so that we can
potentially fix them in a subsequent release.
---
 states/statefile/version3_upgrade.go | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/states/statefile/version3_upgrade.go b/states/statefile/version3_upgrade.go
index 2cbe8a53c..fbec5477c 100644
--- a/states/statefile/version3_upgrade.go
+++ b/states/statefile/version3_upgrade.go
@@ -79,7 +79,7 @@ func upgradeStateV3ToV4(old *stateV3) (*stateV4, error) {
 				case addrs.DataResourceMode:
 					modeStr = "data"
 				default:
-					return nil, fmt.Errorf("state contains resource %s with an unsupported resource mode", resAddr)
+					return nil, fmt.Errorf("state contains resource %s with an unsupported resource mode %#v", resAddr, resAddr.Mode)
 				}
 
 				// In state versions prior to 4 we allowed each instance of a
@@ -98,7 +98,7 @@ func upgradeStateV3ToV4(old *stateV3) (*stateV4, error) {
 					var diags tfdiags.Diagnostics
 					providerAddr, diags = addrs.ParseAbsProviderConfigStr(oldProviderAddr)
 					if diags.HasErrors() {
-						return nil, diags.Err()
+						return nil, fmt.Errorf("invalid provider config reference %q for %s: %s", oldProviderAddr, instAddr, diags.Err())
 					}
 				} else {
 					// Smells like an old-style module-local provider address,
@@ -109,7 +109,7 @@ func upgradeStateV3ToV4(old *stateV3) (*stateV4, error) {
 					if oldProviderAddr != "" {
 						localAddr, diags := addrs.ParseProviderConfigCompactStr(oldProviderAddr)
 						if diags.HasErrors() {
-							return nil, diags.Err()
+							return nil, fmt.Errorf("invalid legacy provider config reference %q for %s: %s", oldProviderAddr, instAddr, diags.Err())
 						}
 						providerAddr = localAddr.Absolute(moduleAddr)
 					} else {
@@ -272,7 +272,7 @@ func upgradeInstanceObjectV3ToV4(rsOld *resourceStateV2, isOld *instanceStateV2,
 		instKeyRaw = string(tk)
 	default:
 		if instKeyRaw != nil {
-			return nil, fmt.Errorf("insupported instance key: %#v", instKey)
+			return nil, fmt.Errorf("unsupported instance key: %#v", instKey)
 		}
 	}
 
@@ -301,7 +301,11 @@ func upgradeInstanceObjectV3ToV4(rsOld *resourceStateV2, isOld *instanceStateV2,
 
 	dependencies := make([]string, len(rsOld.Dependencies))
 	for i, v := range rsOld.Dependencies {
-		dependencies[i] = parseLegacyDependency(v)
+		depStr, err := parseLegacyDependency(v)
+		if err != nil {
+			return nil, fmt.Errorf("invalid dependency reference %q: %s", v, err)
+		}
+		dependencies[i] = depStr
 	}
 
 	return &instanceObjectStateV4{
@@ -414,7 +418,7 @@ func simplifyImpliedValueType(ty cty.Type) cty.Type {
 	}
 }
 
-func parseLegacyDependency(s string) string {
+func parseLegacyDependency(s string) (string, error) {
 	parts := strings.Split(s, ".")
 	ret := parts[0]
 	for _, part := range parts[1:] {
@@ -427,5 +431,14 @@ func parseLegacyDependency(s string) string {
 		}
 		ret = ret + "." + part
 	}
-	return ret
+
+	// The result must parse as a reference, or else we'll create an invalid
+	// state file.
+	var diags tfdiags.Diagnostics
+	_, diags = addrs.ParseRefStr(ret)
+	if diags.HasErrors() {
+		return "", diags.Err()
+	}
+
+	return ret, nil
 }