hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.02k stars 9.47k forks source link

Feature Request: Builtin Function to Sort Semantic Versions #22688

Open vsimon opened 4 years ago

vsimon commented 4 years ago

UPDATE: The PR for this is here: https://github.com/hashicorp/terraform/pull/22689


Hi there, I was wondering if it would be possible to have a builtin function that sorts a list of semantic version strings. Currently lexicographical ordering is available with the 'sort' function.

Current Terraform Version

Terraform v0.12.7

Use-cases

The use case that I'd want to solve with such a function is the following: In a scenario where there can be multiple semantic versions of a sub-resource (a node_pool) but the parent resource (a cluster) contains a min_version field that should be the largest of all the versions (for compatibility reasons).

resource "google_container_cluster" "default" {
  min_master_version = element(sort([var.blue_kubernetes_version, var.green_kubernetes_version]), 1)
}

resource "google_container_node_pool" "blue_pool" {
  version    = var.blue_kubernetes_version
}

resource "google_container_node_pool" "green_pool" {
  version    = var.green_kubernetes_version
}

Proposal

I could see this function being similar to 'sort', taking a list of strings as an argument. The strings themselves must be valid semantic versions or else it would be an error. Also, I could see using blang/semver since its already a dep.

I'd be willing to create a PR if there's interest.

teamterraform commented 4 years ago

Thanks for sharing this use-case, @vsimon!

One thing we'd need to think about for sortsemver that doesn't apply to sort is that semver does not define a total order over all versions: if given the input ["v1.0.0+foo", "v1.0.0+bar"] then the ordering is undefined.

One way we could address that is to make sure the input argument is defined as being a list (rather than a set) and then use a stable sort approach where if two versions have the same precedence then their relative ordering in the input list is retained. That way the caller can in theory apply a separate preference order to those first, before applying the main sort, and have it preserved through this new function.


Separately from that above extra detail, we can think of another semver-related use-case: given a set of versions and a version constraint, return the versions matching that constraint in precedence order.

That seems like a logical extension of what you proposed here:

> semver("~> 1.0.0", ["0.1.0", "1.0.0", "1.2.0"])
[
  "1.2.0",
  "1.0.0",
]

The above both applies the sorting you wanted and also filters out the "0.1.0" entry because it's out of range. If you just wanted the ordering without the filtering, an empty constraint would include all the versions and thus get that result:

> semver("", ["0.1.0", "1.0.0", "1.2.0"])
[
  "1.2.0",
  "1.0.0",
  "0.1.0",
]

Given that version selection hasn't been a commonly-requested feature so far, it's likely better for us to group these two related bits of functionality together into a single function, even if that "empty constraint" situation is arguably overloading it a little, because then the surface area of these features on the rest of Terraform is minimal but we can still meet both of the needs.

What do you think?

vsimon commented 4 years ago

Thanks for the reply.

For defining a total ordering specifically for the build-metadata case, do you think this would be in-itself defining an ordering precedence outside of the spec when the spec specifically calls out that build metadata (characters following the '+' identifier) must be ignored when determining version precedence? (https://semver.org/#spec-item-10)

10. Build metadata MAY be denoted by appending a plus sign and a series of dot 
separated identifiers immediately following the patch or pre-release version. 
Identifiers MUST comprise only ASCII alphanumerics and hyphen [0-9A-Za-z-]. 
Identifiers MUST NOT be empty. Build metadata MUST be ignored when determining 
version precedence. Thus two versions that differ only in the build metadata, 
have the same precedence. Examples: 1.0.0-alpha+001, 1.0.0+20130313144700, 
1.0.0-beta+exp.sha.5114f85.

Other major, minor, patch and pre-release identifiers do have a spec-defined precedence which spec-compliant libraries probably all implement correctly. I guess what I'm saying should we even be defining behavior that has been called out to be ignored?

Secondly, I agree that a version constraint argument would be useful. The blank string empty constraint is a reasonable empty filter. At a glance, one small problem currently might be that the blang/semver library might be lacking in supporting a variety of constraints (see blang/semver issues 41, 45, and 54. There seems to be some work done in forks but nothing has been merged upstream). The Masterminds/semver package seems more featured in supported version constraints. Would it be OK to add a dep if this might be a relevant package to use?

Also, I was curious about why there were two places where functions are defined (config/interpolate_funcs.go, and lang/functions.go). Is one of them considered "legacy" and should not get new functions? They both aren't in parity with their respective function sets. The doc templates seem to be based on the config/interpolate_funcs set, is this true? Are there doc templates for the lang/functions?

teamterraform commented 4 years ago

Hi @vsimon,

The problem is that in order to produce a flat list we need to decide on some ordering for the build metadata, whether it be to order them lexically, or to preserve the order given in the input, or something else. Otherwise we'd need to have some way to indicate that two versions in the input are equivalent, which would add a lot of extra complication for what is hopefully an edge-case.

On more reflection, it seems like preserving the input order in that case is probably the easiest behavior to reason about: if a caller gives a list containing version numbers that only differ in build metadata, then the result would be identical to the input. That could be achieved by using sort.SliceStable, which will preserve the ordering of items with equal precedence automatically.


If blang/semver's constraint implementation isn't sufficient, we'd prefer to use hashicorp/go-version here, since that's also already in Terraform's dependency set and is what Terraform uses to implement version constraints for providers and modules.

It supports ~> constraints as well as the usual >=, <, etc constraints, so should be rich enough to meet the use-case we were describing previously.

go-version is not technically a strict implementation of semver, but it does have a function ParseSemver which constrains the input to be a valid semver version. Its LessThan method should be sufficient for sorting.


Finally, on the subject of the function implementations: the ones in config/interpolate_funcs.go are legacy code left over from Terraform 0.11. They're not deleted yet because the config package is still being used by some testing callers and so deleting it is blocked for the moment while that cleanup is going on. The table in lang/functions.go is the one used by current versions of Terraform, and that's also what the Terraform 0.12 section of the documentation is covering.

sc250024 commented 4 years ago

Yes please!

vsimon commented 4 years ago

Hello, it's been awhile but I've updated this aged PR to address:

  1. Using hashicorp/go-version and its NewSemver function for parsing.
  2. Adding a version constraint. Constraints are defined and supported by https://godoc.org/github.com/hashicorp/go-version#Constraint
  3. Using sort.Stable (and by extension go-version's LessThan function)
  4. Updating tests and documentation
vsimon commented 3 years ago

ping @teamterraform https://github.com/hashicorp/terraform/pull/22689

superbobdthm commented 3 years ago

Have a couple of uses for this.. Would love to see it merged, if possible.

vsimon commented 3 years ago

thanks @superbobdthm but unfortunately it's been super difficult getting a maintainer's attention. Maybe more upvotes?

ping @teamterraform @apparentlymart for #22689 please 🙏

peay commented 3 years ago

A use case I have for this is to check that another state that a workspace depends on via Terraform remote state has the right version.

With an assertion and @vsimon's addition in the PR, it would allow a component A to require that another component B that it depends on be deployed at a compatible version, and block deployment otherwise. Some of that can be mimicked using string manipulations (e.g., regexes), but that's not great.

Any chance that this'll be merged? PR has been open for a long time now. @teamterraform

vsimon commented 3 years ago

Rebased the PR again, can anyone please take a look? @teamterraform

entscheidungsproblem commented 2 years ago

I've found a workaround that can be used in the meantime. This can compare two semantic versions and indicate which value is larger.

locals {
  version_a = split(".", "1.2.3")
  version_b = split(".", "2.0.1")
  version-test = [
    for i, j in reverse(range(length(local.version_a)))
    : signum(local.version_b[i] - local.version_a[i]) * pow(10, j)
  ]
  # The version-compare value is -1, 0, or 1 if version_a is greater than, equal to or less than version_b respectively
  version-compare = signum(sum(local.version-test))
  a-less-than-b   = 1 == local.version-compare
}
liad5h commented 1 year ago

In case this is still relevant, I implemented this for kubernetes version comparison:


data "aws_eks_cluster" "cluster" {
  name = var.cluster_name
}

locals {
  kubernetes_1_24 = [1,24]
  cluster_version = [ for i in split(".", data.aws_eks_cluster.cluster.version): tonumber(i) ]
  cluster_1_24_or_higher = alltrue(
    [
      for index, num in local.cluster_version:
            (signum(num - local.kubernetes_1_24[index]) >= 0)
    ]
  )
}
marcel-puchol-jt commented 1 year ago

If you want to also allow different version sizes (e.g. 14.6 compared with 14) an improvement for https://github.com/hashicorp/terraform/issues/22688#issuecomment-1196805096 (thanks for the code) is:

  version_engine = split(".", "14.7.1")
  default_version = split(".", "14.6")

  # The version_engine_with_default_version_comparison value is -1 (version_engine < default_version), 0 (version_engine = default_version), or 1 (version_engine > default_version)
  version_engine_with_default_version_comparison = signum(sum([
    for i, j in reverse(range(max(length(local.version_engine), length(local.default_version))))
    : signum(try(local.version_engine[i], 0) - try(local.default_version[i], 0)) * pow(10, j)
  ]))
tmccombs commented 1 year ago

Not sure if this should be a separate feature request, but I would just like to test if a version matches a version range.

Something like:


my_local = version_matches(">= 7.0", var.version) ? "thing-for-higher-versions" : "thing-for-lower-versions"
HenriBlacksmith commented 7 months ago

Any updates on this PR?

Being able to leverage hashicorp/go-version in Terraform code as suggested in https://github.com/hashicorp/terraform/issues/22688#issuecomment-1525986571 would help a lot to perform some validations in Terraform modules.

It would be really nice if something can be added (looks like this PR has been opened for a really long time)

mloskot commented 7 months ago

Or, copy this feature request to https://github.com/opentofu/opentofu/issues/

crw commented 5 months ago

Thank you for your continued interest in this issue.

Terraform version 1.8 launches with support of provider-defined functions. It is now possible to implement your own functions! We would love to see this implemented as a provider-defined function.

Please see the provider-defined functions documentation to learn how to implement functions in your providers. If you are new to provider development, learn how to create a new provider with the Terraform Plugin Framework. If you have any questions, please visit the Terraform Plugin Development category in our official forum.

We hope this feature unblocks future function development and provides more flexibility for the Terraform community. Thank you for your continued support of Terraform!

mloskot commented 5 months ago

@crw Although this does make sense, I personally fear that without a common framework in place that scans, tests, and stamps providers with "Hashicorp Conformance Tested", this approach will lead to the grand mess of NodeJS and alike.

HenriBlacksmith commented 5 months ago

@crw Although this does make sense, I personally fear that without a common framework in place that scans, tests, and stamps providers with "Hashicorp Conformance Tested", this approach will lead to the grand mess of NodeJS and alike.

I see this as more HashiCorp will use that to move some deeper and specific functions into optional providers.

I agree that the DIY option should be avoided when possible

I expect an optional functions library from HashiCorp including those version checks for instance