From baa72ce235c40da70df1c796fb5ef44fdd15dd38 Mon Sep 17 00:00:00 2001 From: Nick Fagerlund Date: Tue, 12 Oct 2021 17:59:56 -0700 Subject: [PATCH] Simplify logic flow: everything is a constraint Explicit version strings are actually also version constraints! And the special comparisons we were doing to allow a range of compatible versions can also be expressed as version constraints. Bonus: also simplify the way we handle version check errors, by composing the messages inline and only extracting the repetitive parts into a function. --- internal/cloud/backend.go | 102 ++++++++++++++++----------------- internal/cloud/backend_test.go | 6 +- internal/cloud/errors.go | 39 ++----------- 3 files changed, 60 insertions(+), 87 deletions(-) diff --git a/internal/cloud/backend.go b/internal/cloud/backend.go index 5fdd2b12c..08dafb275 100644 --- a/internal/cloud/backend.go +++ b/internal/cloud/backend.go @@ -781,66 +781,66 @@ func (b *Cloud) VerifyWorkspaceTerraformVersion(workspaceName string) tfdiags.Di return nil } - // Even if ignoring version conflicts, it may still be useful to call this - // method and warn the user about a mismatch between the local and remote - // Terraform versions. - remoteVersion, err := version.NewSemver(workspace.TerraformVersion) + remoteConstraint, err := version.NewConstraint(workspace.TerraformVersion) if err != nil { - log.Printf("[DEBUG] Invalid Terraform version (%s); will try to parse as version constraint", workspace.TerraformVersion) - } - if remoteVersion == nil { - // If it's not a valid version, it might be a valid version constraint: - remoteConstraint, err := version.NewConstraint(workspace.TerraformVersion) - if err != nil { - diags = diags.Append(terraformInvalidVersionOrConstraint(b.ignoreVersionConflict, workspace.TerraformVersion)) - return diags - } - - // Avoiding tfversion.SemVer because it omits the prerelease prefix, and we - // want constraints like `~> 1.2.0-beta1` to be possible. - fullTfversion := version.Must(version.NewSemver(tfversion.String())) - - // If it's a constraint, we only ensure that the local version meets it. - // This can result in both false positives and false negatives, but in the - // most common case (~> x.y.z) it's useful enough. - if remoteConstraint.Check(fullTfversion) { - return diags - } - - diags = diags.Append(terraformMismatchDiagnostic(b.ignoreVersionConflict, b.organization, workspace, tfversion.String())) + message := fmt.Sprintf( + "The remote workspace specified an invalid Terraform version or constraint (%s), "+ + "and it isn't possible to determine whether the local Terraform version (%s) is compatible.", + workspace.TerraformVersion, + tfversion.String(), + ) + diags = diags.Append(incompatibleWorkspaceTerraformVersion(message, b.ignoreVersionConflict)) return diags } - v014 := version.Must(version.NewSemver("0.14.0")) - if tfversion.SemVer.LessThan(v014) || remoteVersion.LessThan(v014) { - // Versions of Terraform prior to 0.14.0 will refuse to load state files - // written by a newer version of Terraform, even if it is only a patch - // level difference. As a result we require an exact match. - if tfversion.SemVer.Equal(remoteVersion) { - return diags - } - } - if tfversion.SemVer.GreaterThanOrEqual(v014) && remoteVersion.GreaterThanOrEqual(v014) { - // Versions of Terraform after 0.14.0 should be compatible with each - // other. At the time this code was written, the only constraints we - // are aware of are: - // - // - 0.14.0 is guaranteed to be compatible with versions up to but not - // including 1.2.0 + // If the workspace has a literal Terraform version, see if we can use a + // looser version constraint. + remoteVersion, _ := version.NewSemver(workspace.TerraformVersion) + if remoteVersion != nil { + v014 := version.Must(version.NewSemver("0.14.0")) v120 := version.Must(version.NewSemver("1.2.0")) - if tfversion.SemVer.LessThan(v120) && remoteVersion.LessThan(v120) { - return diags + + // Versions from 0.14 through the early 1.x series should be compatible + // (though we don't know about 1.2 yet). + if remoteVersion.GreaterThanOrEqual(v014) && remoteVersion.LessThan(v120) { + early1xCompatible, err := version.NewConstraint(fmt.Sprintf(">= 0.14.0, < %s", v120.String())) + if err != nil { + panic(err) + } + remoteConstraint = early1xCompatible } - // - Any new Terraform state version will require at least minor patch - // increment, so x.y.* will always be compatible with each other - tfvs := tfversion.SemVer.Segments64() - rwvs := remoteVersion.Segments64() - if len(tfvs) == 3 && len(rwvs) == 3 && tfvs[0] == rwvs[0] && tfvs[1] == rwvs[1] { - return diags + + // Any future new state format will require at least a minor version + // increment, so x.y.* will always be compatible with each other. + if remoteVersion.GreaterThanOrEqual(v120) { + rwvs := remoteVersion.Segments64() + if len(rwvs) >= 3 { + // ~> x.y.0 + minorVersionCompatible, err := version.NewConstraint(fmt.Sprintf("~> %d.%d.0", rwvs[0], rwvs[1])) + if err != nil { + panic(err) + } + remoteConstraint = minorVersionCompatible + } } } - diags = diags.Append(terraformMismatchDiagnostic(b.ignoreVersionConflict, b.organization, workspace, tfversion.String())) + // Re-parsing tfversion.String because tfversion.SemVer omits the prerelease + // prefix, and we want to allow constraints like `~> 1.2.0-beta1`. + fullTfversion := version.Must(version.NewSemver(tfversion.String())) + + if remoteConstraint.Check(fullTfversion) { + return diags + } + + message := fmt.Sprintf( + "The local Terraform version (%s) does not meet the version requirements for remote workspace %s/%s (%s).", + tfversion.String(), + b.organization, + workspace.Name, + workspace.TerraformVersion, + ) + diags = diags.Append(incompatibleWorkspaceTerraformVersion(message, b.ignoreVersionConflict)) return diags } diff --git a/internal/cloud/backend_test.go b/internal/cloud/backend_test.go index 38e4db8d6..cd2995887 100644 --- a/internal/cloud/backend_test.go +++ b/internal/cloud/backend_test.go @@ -656,7 +656,7 @@ func TestCloud_VerifyWorkspaceTerraformVersion(t *testing.T) { if len(diags) != 1 { t.Fatal("expected diag, but none returned") } - if got := diags.Err().Error(); !strings.Contains(got, "Terraform version mismatch") { + if got := diags.Err().Error(); !strings.Contains(got, "Incompatible Terraform version") { t.Fatalf("unexpected error: %s", got) } } else { @@ -705,7 +705,7 @@ func TestCloud_VerifyWorkspaceTerraformVersion_workspaceErrors(t *testing.T) { if len(diags) != 1 { t.Fatal("expected diag, but none returned") } - if got := diags.Err().Error(); !strings.Contains(got, "Terraform version error: The remote workspace specified") { + if got := diags.Err().Error(); !strings.Contains(got, "Incompatible Terraform version: The remote workspace specified") { t.Fatalf("unexpected error: %s", got) } } @@ -757,7 +757,7 @@ func TestCloud_VerifyWorkspaceTerraformVersion_ignoreFlagSet(t *testing.T) { if got, want := diags[0].Severity(), tfdiags.Warning; got != want { t.Errorf("wrong severity: got %#v, want %#v", got, want) } - if got, want := diags[0].Description().Summary, "Terraform version mismatch"; got != want { + if got, want := diags[0].Description().Summary, "Incompatible Terraform version"; got != want { t.Errorf("wrong summary: got %s, want %s", got, want) } wantDetail := "The local Terraform version (0.14.0) does not meet the version requirements for remote workspace hashicorp/app-prod (0.13.5)." diff --git a/internal/cloud/errors.go b/internal/cloud/errors.go index b51f44338..58494f70e 100644 --- a/internal/cloud/errors.go +++ b/internal/cloud/errors.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - tfe "github.com/hashicorp/go-tfe" "github.com/hashicorp/terraform/internal/tfdiags" "github.com/zclconf/go-cty/cty" ) @@ -32,41 +31,15 @@ var ( ) ) -func terraformMismatchDiagnostic(ignoreVersionConflict bool, organization string, workspace *tfe.Workspace, tfversion string) tfdiags.Diagnostic { +const ignoreRemoteVersionHelp = "If you're sure you want to upgrade the state, you can force Terraform to continue using the -ignore-remote-version flag. This may result in an unusable workspace." + +func incompatibleWorkspaceTerraformVersion(message string, ignoreVersionConflict bool) tfdiags.Diagnostic { severity := tfdiags.Error + suggestion := ignoreRemoteVersionHelp if ignoreVersionConflict { severity = tfdiags.Warning - } - - suggestion := "If you're sure you want to upgrade the state, you can force Terraform to continue using the -ignore-remote-version flag. This may result in an unusable workspace." - if ignoreVersionConflict { suggestion = "" } - - description := fmt.Sprintf( - "The local Terraform version (%s) does not meet the version requirements for remote workspace %s/%s (%s).\n\n%s", - tfversion, - organization, - workspace.Name, - workspace.TerraformVersion, - suggestion, - ) - description = strings.TrimSpace(description) - return tfdiags.Sourceless(severity, "Terraform version mismatch", description) -} - -func terraformInvalidVersionOrConstraint(ignoreVersionConflict bool, tfversion string) tfdiags.Diagnostic { - severity := tfdiags.Error - if ignoreVersionConflict { - severity = tfdiags.Warning - } - - suggestion := "If you're sure you want to upgrade the state, you can force Terraform to continue using the -ignore-remote-version flag. This may result in an unusable workspace." - if ignoreVersionConflict { - suggestion = "" - } - - description := fmt.Sprintf("The remote workspace specified an invalid Terraform version or version constraint: %s\n\n%s", tfversion, suggestion) - description = strings.TrimSpace(description) - return tfdiags.Sourceless(severity, "Terraform version error", description) + description := strings.TrimSpace(fmt.Sprintf("%s\n\n%s", message, suggestion)) + return tfdiags.Sourceless(severity, "Incompatible Terraform version", description) }