Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Terraform provider version constraint resolution for NOT prerelease #33452

Closed
jskirde opened this issue Jun 30, 2023 · 6 comments · Fixed by #35181
Closed

Terraform provider version constraint resolution for NOT prerelease #33452

jskirde opened this issue Jun 30, 2023 · 6 comments · Fixed by #35181
Labels
bug confirmed a Terraform Core team member has reproduced this issue upstream v1.5 Issues (primarily bugs) reported against v1.5 releases

Comments

@jskirde
Copy link

jskirde commented Jun 30, 2023

Terraform Version

Terraform v1.5.2
on windows_amd64

Terraform Configuration Files

Terraform is able to select a pre-release provider for a module even if it's explicitly denied using a version constraint.
This means that if any one module enforces a pre-release version and other modules use pessimistic or greater than version constraints, after all the constraints are AND'ed together, a pre-release version will be selected.

terraform {
  required_providers {
    myprovider= {
      source  = "mydomain.com/namespace/myprovider"
      version = "~>2.0, !=2.0.1-alpha1, !=2.0.1-alpha2"
    }
  }
}

output


### Debug Output

$ terraform init -upgrade

Initializing the backend...
Upgrading modules...

  • my-module in ....\modules\terraform-myprovider-my-module

Initializing provider plugins...

  • Finding mydomain.com/namespace/myprovider versions matching ""~> 2.0, !=2.0.1-alpha1, != 2.0.1-alpha2, 2.0.1-alpha2"...
  • Installing mydomain.com/namespace/myprovider v2.0.1-alpha2...
  • Installed mydomain.com/namespace/myprovider v2.0.1-alpha2 (unauthenticated)

Terraform has created a lock file .terraform.lock.hcl to record the provider
selections it made above. Include this file in your version control repository
so that Terraform can guarantee to make the same selections by default when
you run "terraform init" in the future.


### Expected Behavior

We expected that we should be able to deny a specific pre-release version of a provider in the provider version constraint in our modules. In the case where we combine a module that explicitly requires a pre-release provider and another module that explicitly denies this provider, an error is appropriate and expected.

The documentation is ambiguous in this regard as the != "not equals" operator is an "exact" operator but is not listed here:
https://developer.hashicorp.com/terraform/language/expressions/version-constraints
- A prerelease version is a version number that contains a suffix introduced by a dash, like 1.2.0-beta. A prerelease version can be selected only by an exact version constraint (the = operator or no operator). Prerelease versions do not match inexact operators such as >=, ~>, etc.

### Actual Behavior

Terraform selects the pre-release provider despite the constraint to the contrary.

### Steps to Reproduce

1. Have a pre-release provider version available (in this case 2.0.1-alpha1)
2. Define a provider constraint like:

version = "2.0.1-alpha1, !=2.0.1-alpha1"

3. Expect an error as constraint not resolvable.


### Additional Context

_No response_

### References

_No response_
@jskirde jskirde added bug new new issue not yet triaged labels Jun 30, 2023
@apparentlymart
Copy link
Member

Thanks for reporting this @jskirde.

There is logic in Terraform's version resolution to ignore prerelease versions unless they are named exactly, so writing just ~> 2.0 without the exclusions should prevent selecting these two alpha releases.

I wonder then if the logic for that special rule isn't taking into account the negation. It works by searching the version constraints for any exact version references that are prereleases, and so perhaps it's incorrectly classifying these negated terms as "exact", since they do specify an exact version even though it's negated.

This version handle belongs to an upstream library rather than to Terraform itself, but I happen to be the maintainer of that library so I'll be able to arrange for a fix to be released so far Terraform can use it, if the problem does turn out to be there.

@apparentlymart apparentlymart added the v1.5 Issues (primarily bugs) reported against v1.5 releases label Jun 30, 2023
@apparentlymart
Copy link
Member

I tried quickly adding a test case to the upstream apparentlymart/go-versions that resembles this situation, with both a lower bound and a negation:

		{
			MustMakeSet(MeetingConstraintsStringRuby(">= 2.0.0, != 2.0.0-beta1, != 2.0.0-beta2")),
			MustParseVersion("2.0.0-beta2"),
			false,
		},

This table-based test asks whether the version given in the second field is matched by the constraints given in the first field. In this case I would expect the answer to be false both because the 2.0.0-beta2 is a prerelease and should therefore be excluded unless explicitly selected and because it's been explicitly excluded.

This test passed, so unfortunately I think that disproves my theory that this might just be a flaw in the logic for deciding whether a particular prerelease version is "explicitly selected". Something else seems to be going on here.

@apparentlymart
Copy link
Member

Looking back again at what you reported, I see this in the terraform init output:

Finding mydomain.com/namespace/myprovider versions matching "~> 2.0, !=2.0.1-alpha1, != 2.0.1-alpha2, 2.0.1-alpha2"...

In particular, notice that this final merged constraint contains both != 2.0.1-alpha2 and 2.0.1-alpha2. I assume that the extra 2.0.1-alpha2 clause has come from another module in your configuration, not included in the question.

As you noted at the end, Terraform should've treated this as a contradiction and either matched nothing at all (because a version can't be both 2.0.1-alpha2 and not 2.0.1-alpha2 at the same time) or returned an error during version constraint parsing saying that there's a contradiction.

It seems like the bug is that the positive 2.0.1-alpha2 constraint effectively superseded the negative != 2.0.1-alpha2. I'll try a test case for that and see where that leads.

@apparentlymart
Copy link
Member

Okay! I was able to reproduce this variant in the upstream test suite:

		{
			MustMakeSet(MeetingConstraintsStringRuby(">= 2.0.0, != 2.0.0-beta1, != 2.0.0-beta2, 2.0.0-beta2")),
			MustParseVersion("2.0.0-beta2"),
			false,
		},

This test fails because the version set membership test returns true instead of false, claiming that 2.0.0-beta2 is included by these version constraints.

@apparentlymart apparentlymart added upstream confirmed a Terraform Core team member has reproduced this issue and removed new new issue not yet triaged labels Jun 30, 2023
@apparentlymart
Copy link
Member

apparentlymart commented Jun 30, 2023

It seems like the minimum reproduction case here is:

  • A positive and negative constraint for the same exact version, thereby creating a contradiction.
  • The specified version must be a prerelease.

If either of the above don't hold then the test passes. Here's the revised test case:

		{
			MustMakeSet(MeetingConstraintsStringRuby("!= 2.0.0-beta1, 2.0.0-beta1")),
			MustParseVersion("2.0.0-beta1"),
			false, // the constraint is contradictory, so includes nothing
		},
--- FAIL: TestSetHas (0.00s)
    --- FAIL: TestSetHas/versions.Union(versions.Only(versions.MustParseVersion("2.0.0-beta1")),_versions.Intersection(versions.Released,_(versions.All).Subtract(versions.Only(versions.MustParseVersion("2.0.0-beta1"))),_versions.Only(versions.MustParseVersion("2.0.0-beta1")))) (0.00s)
        set_test.go:348: wrong result
            set:     versions.Union(versions.Only(versions.MustParseVersion("2.0.0-beta1")), versions.Intersection(versions.Released, (versions.All).Subtract(versions.Only(versions.MustParseVersion("2.0.0-beta1"))), versions.Only(versions.MustParseVersion("2.0.0-beta1"))))
            version: versions.MustParseVersion("2.0.0-beta1")
            got:     true
            want:    false

For readability, here's a more concise view of how the library is interpreting this version constraint:

union(
  2.0.0-beta1,
  intersection(
    non-prerelease versions,
    all possible versions - 2.0.0-beta1,
  )
)

That interpretation seems flawed: the outer union combines a set containing only 2.0.0-beta1 with a set containing everything except that version, thereby effectively reproducing a set with all versions allowed again. This particular combination of a negation and a prerelease seems to require special handling beyond just set operations, so that the negation can overrule the outermost union.

With that said, I am also kinda curious as to why it chose "union" here since I would have intuitively expected these version sets to be combined with "intersection", since the comma operator in this syntax means "AND", not "OR". Perhaps the root cause is there.

@apparentlymart
Copy link
Member

apparentlymart commented Jun 30, 2023

Further poking confirms that it does seem to be the special case for prerelease versions that's somehow causing the problem, because there's already a test in the parsing code for this trivial contradiction case:

		{
			`1.0.0, != 1.0.0`, // degenerate empty set
			Intersection(
				Released,
				Only(MustParseVersion(`1.0.0`)),
				All.Subtract(Only(MustParseVersion(`1.0.0`))),
			),
		},

This behaves as I would've expected, producing an empty set by applying intersection to both "1.0.0" and "not 1.0.0", which have no versions in common.

The equivalent of this with prereleases makes a similar odd result as in the previous comment:

`1.0.0-beta1, != 1.0.0-beta1`

versions.Union(
    versions.Only(versions.MustParseVersion("1.0.0-beta1")),
    versions.Intersection(
        versions.Released,
        versions.Only(versions.MustParseVersion("1.0.0-beta1")),
        (versions.All).Subtract(versions.Only(versions.MustParseVersion("1.0.0-beta1"))),
    )
)

So it seems like the logic that tries to carve out the exception for allowing prereleases while lowering the constraint into its constituent operators is where the problem lies here.

Since this bug is with what is arguably an edge case (it's rare for version constraints to contain negations at all, let alone ones that are prereleases and are contradicted) and the correct behavior here would've been a failure anyway (so this isn't blocking any valid configuration from working) I'm going to leave this context here for now in the hope of addressing this in future, but I unfortunately have to divert to other planned work for today.

Thanks for reporting this, and sorry for the incorrect behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug confirmed a Terraform Core team member has reproduced this issue upstream v1.5 Issues (primarily bugs) reported against v1.5 releases
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants