From 7ab914491bb3c8006cb0ea5b1b8e1b26e6542789 Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Thu, 11 Jun 2020 16:34:16 -0700
Subject: [PATCH] configs: Don't panic if new version constraint parser raises
 an error

The new provider installer code is using a new version constraint parser
because it produces better error messages than the one we were using
before. However, it has some cases where it returns errors that the old
parser (which was entirely regex-match-based) didn't catch.

In the long run we should consistently use the new parser everywhere, but
until then we'll avoid panicking then the two disagree, by returning
diagnostic messages instead of using MustParseVersionConstraints.

For now, we only hit these error cases if the user enters something that
the old parser allows but the new parser does not.
---
 configs/config.go | 40 ++++++++++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/configs/config.go b/configs/config.go
index fadc78c5f..84df43771 100644
--- a/configs/config.go
+++ b/configs/config.go
@@ -236,11 +236,22 @@ func (c *Config) addProviderRequirements(reqs getproviders.Requirements) hcl.Dia
 		}
 		// The model of version constraints in this package is still the
 		// old one using a different upstream module to represent versions,
-		// so we'll need to shim that out here for now. We assume this
-		// will always succeed because these constraints already succeeded
-		// parsing with the other constraint parser, which uses the same
-		// syntax.
-		constraints := getproviders.MustParseVersionConstraints(providerReqs.Requirement.Required.String())
+		// so we'll need to shim that out here for now. The two parsers
+		// don't exactly agree in practice 🙄 so this might produce new errors.
+		// TODO: Use the new parser throughout this package so we can get the
+		// better error messages it produces in more situations.
+		constraints, err := getproviders.ParseVersionConstraints(providerReqs.Requirement.Required.String())
+		if err != nil {
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Invalid version constraint",
+				// The errors returned by ParseVersionConstraint already include
+				// the section of input that was incorrect, so we don't need to
+				// include that here.
+				Detail:  fmt.Sprintf("Incorrect version constraint syntax: %s.", err.Error()),
+				Subject: providerReqs.Requirement.DeclRange.Ptr(),
+			})
+		}
 		reqs[fqn] = append(reqs[fqn], constraints...)
 	}
 	// Each resource in the configuration creates an *implicit* provider
@@ -272,7 +283,24 @@ func (c *Config) addProviderRequirements(reqs getproviders.Requirements) hcl.Dia
 			reqs[fqn] = nil
 		}
 		if provider.Version.Required != nil {
-			constraints := getproviders.MustParseVersionConstraints(provider.Version.Required.String())
+			// The model of version constraints in this package is still the
+			// old one using a different upstream module to represent versions,
+			// so we'll need to shim that out here for now. The two parsers
+			// don't exactly agree in practice 🙄 so this might produce new errors.
+			// TODO: Use the new parser throughout this package so we can get the
+			// better error messages it produces in more situations.
+			constraints, err := getproviders.ParseVersionConstraints(provider.Version.Required.String())
+			if err != nil {
+				diags = diags.Append(&hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary:  "Invalid version constraint",
+					// The errors returned by ParseVersionConstraint already include
+					// the section of input that was incorrect, so we don't need to
+					// include that here.
+					Detail:  fmt.Sprintf("Incorrect version constraint syntax: %s.", err.Error()),
+					Subject: provider.Version.DeclRange.Ptr(),
+				})
+			}
 			reqs[fqn] = append(reqs[fqn], constraints...)
 		}
 	}