From 9576a5b2d8774e0a3d5fc5daeca44229188284e2 Mon Sep 17 00:00:00 2001 From: Alisdair McDiarmid Date: Mon, 19 Oct 2020 16:37:00 -0400 Subject: [PATCH] internal: Fix lockfile constraint output for 1.2.* If a configuration requires a partial provider version (with some parts unspecified), Terraform considers this as a constrained-to-zero version. For example, a version constraint of 1.2 will result in an attempt to install version 1.2.0, even if 1.2.1 is available. When writing the dependency locks file, we previously would write 1.2.*, as this is the in-memory representation of 1.2. This would then cause an error on re-reading the locks file, as this is not a valid constraint format. Instead, we now explicitly convert the constraint to its zero-filled representation before writing the locks file. This ensures that it correctly round-trips. Because this change is made in getproviders.VersionConstraintsString, it also affects the output of the providers sub-command. --- command/providers_test.go | 4 ++-- internal/depsfile/locks_file_test.go | 8 +++++++ internal/getproviders/types.go | 35 +++++++++++++++++----------- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/command/providers_test.go b/command/providers_test.go index 04bac7014..d11bda79e 100644 --- a/command/providers_test.go +++ b/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) diff --git a/internal/depsfile/locks_file_test.go b/internal/depsfile/locks_file_test.go index cdd6e6a03..6019a0715 100644 --- a/internal/depsfile/locks_file_test.go +++ b/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" diff --git a/internal/getproviders/types.go b/internal/getproviders/types.go index c3b93b36f..0f8bc7d7b 100644 --- a/internal/getproviders/types.go +++ b/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 { - // 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) - } + fmt.Fprintf(&b, "%s.%s", boundary.Major, 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()