Merge pull request #26637 from hashicorp/alisdair/fix-locksfile-unconstrained-versions

internal: Fix lockfile constraint output for 1.2.*

command/providers_test.go

@@ -114,7 +114,7 @@ func TestProviders_modules(t *testing.T) {
 	}
 
 	wantOutput := []string{
-		"provider[registry.terraform.io/hashicorp/foo] 1.0.*", // from required_providers
+		"provider[registry.terraform.io/hashicorp/foo] 1.0.0", // from required_providers
 		"provider[registry.terraform.io/hashicorp/bar] 2.0.0", // from provider config
 		"── module.kiddo",                               // tree node for child module
 		"provider[registry.terraform.io/hashicorp/baz]", // implied by a resource in the child module
@@ -151,7 +151,7 @@ func TestProviders_state(t *testing.T) {
 	}
 
 	wantOutput := []string{
-		"provider[registry.terraform.io/hashicorp/foo] 1.0.*", // from required_providers
+		"provider[registry.terraform.io/hashicorp/foo] 1.0.0", // from required_providers
 		"provider[registry.terraform.io/hashicorp/bar] 2.0.0", // from a provider config block
 		"Providers required by state",                         // header for state providers
 		"provider[registry.terraform.io/hashicorp/baz]",       // from a resouce in state (only)

internal/depsfile/locks_file_test.go

@@ -165,10 +165,12 @@ func TestSaveLocksToFile(t *testing.T) {
 	fooProvider := addrs.MustParseProviderSourceString("test/foo")
 	barProvider := addrs.MustParseProviderSourceString("test/bar")
 	bazProvider := addrs.MustParseProviderSourceString("test/baz")
+	booProvider := addrs.MustParseProviderSourceString("test/boo")
 	oneDotOh := getproviders.MustParseVersion("1.0.0")
 	oneDotTwo := getproviders.MustParseVersion("1.2.0")
 	atLeastOneDotOh := getproviders.MustParseVersionConstraints(">= 1.0.0")
 	pessimisticOneDotOh := getproviders.MustParseVersionConstraints("~> 1")
+	abbreviatedOneDotTwo := getproviders.MustParseVersionConstraints("1.2")
 	hashes := []getproviders.Hash{
 		getproviders.MustParseHash("test:cccccccccccccccccccccccccccccccccccccccccccccccc"),
 		getproviders.MustParseHash("test:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"),
@@ -177,6 +179,7 @@ func TestSaveLocksToFile(t *testing.T) {
 	locks.SetProvider(fooProvider, oneDotOh, atLeastOneDotOh, hashes)
 	locks.SetProvider(barProvider, oneDotTwo, pessimisticOneDotOh, nil)
 	locks.SetProvider(bazProvider, oneDotTwo, nil, nil)
+	locks.SetProvider(booProvider, oneDotTwo, abbreviatedOneDotTwo, nil)
 
 	dir, err := ioutil.TempDir("", "terraform-internal-depsfile-savelockstofile")
 	if err != nil {
@@ -207,6 +210,11 @@ provider "registry.terraform.io/test/baz" {
   version = "1.2.0"
 }
 
+provider "registry.terraform.io/test/boo" {
+  version     = "1.2.0"
+  constraints = "1.2.0"
+}
+
 provider "registry.terraform.io/test/foo" {
   version     = "1.0.0"
   constraints = ">= 1.0.0"

internal/getproviders/types.go

@@ -422,27 +422,34 @@ func VersionConstraintsString(spec VersionConstraints) string {
 			b.WriteString("??? ")
 		}
 
+		// The parser allows writing abbreviated version (such as 2) which
+		// end up being represented in memory with trailing unconstrained parts
+		// (for example 2.*.*). For the purpose of serialization with Ruby
+		// style syntax, these unconstrained parts can all be represented as 0
+		// with no loss of meaning, so we make that conversion here.
+		//
+		// This is possible because we use a different constraint operator to
+		// distinguish between the two types of pessimistic constraint:
+		// minor-only and patch-only. For minor-only constraints, we always
+		// want to display only the major and minor version components, so we
+		// special-case that operator below.
+		//
+		// One final edge case is a minor-only constraint specified with only
+		// the major version, such as ~> 2. We treat this the same as ~> 2.0,
+		// because a major-only pessimistic constraint does not exist: it is
+		// logically identical to >= 2.0.0.
+		boundary := sel.Boundary.ConstrainToZero()
 		if sel.Operator == constraints.OpGreaterThanOrEqualMinorOnly {
 			// The minor-pessimistic syntax uses only two version components.
-			if sel.Boundary.Minor.Unconstrained {
+			fmt.Fprintf(&b, "%s.%s", boundary.Major, boundary.Minor)
-				// The parser allows writing ~> 2, which ends up being
-				// represented in memory as ~> 2.* because the minor
-				// version is unconstrained, but that's not really any
-				// different than saying 2.0 and so we'll prefer that in
-				// our serialization in order to be clearer about how we
-				// understood the version constraint.
-				fmt.Fprintf(&b, "%s.0", sel.Boundary.Major)
-			} else {
-				fmt.Fprintf(&b, "%s.%s", sel.Boundary.Major, sel.Boundary.Minor)
-			}
 		} else {
-			fmt.Fprintf(&b, "%s.%s.%s", sel.Boundary.Major, sel.Boundary.Minor, sel.Boundary.Patch)
+			fmt.Fprintf(&b, "%s.%s.%s", boundary.Major, boundary.Minor, boundary.Patch)
 		}
 		if sel.Boundary.Prerelease != "" {
-			b.WriteString("-" + sel.Boundary.Prerelease)
+			b.WriteString("-" + boundary.Prerelease)
 		}
 		if sel.Boundary.Metadata != "" {
-			b.WriteString("+" + sel.Boundary.Metadata)
+			b.WriteString("+" + boundary.Metadata)
 		}
 	}
 	return b.String()