Add is_valid and compare SemVer built-ins

[charlieegan3]

I recently wrote a gist to compare Semantic Version strings as a proof of concept for something I was working on. Given the domains that OPA is commonly used in, I thought this might be a useful built-in. Especially after learning that it was hardly trivial in Rego alone (see gist).

Our use case was comparing installed software with recommended/insecure versions of the same applications.

Here is an example of what the functions look like:

> semver.is_valid("1.0.0")
true
> semver.is_valid(1)
false
> semver.compare("1.0.0", "2.0.0")
-1
> semver.compare("2.0.0", "1.0.0")
1
> semver.compare("1.0.0", "1.0.0")
0
> semver.compare("1.0.0", "1")
1 error occurred: semver.compare("1.0.0", "1"): eval_builtin_error: semver.compare: operand 2: string "1" is not a valid SemVer
> semver.compare("1.0.0", 1)
1 error occurred: 1:1: rego_type_error: semver.compare: invalid argument(s)
        have: (string, number)
        want: (string, string, number)

I apologise in advance for not raising an issue first to discuss the idea - I found myself with some time today and took the opportunity. I won't be offended if you feel this is too specific 😸

[patrick-east]

For the builtin definitions and implementation this looks great. The only thing I am hesitant about is taking the dependency on golang/x/mod. Thats a lot of code just for them, and we try very hard to keep the dependency requirements for OPA as small as possible to make it easier to embed in other go applications.

I would suggest that we grab something minimal and laser focused on semvers like https://github.com/coreos/go-semver/blob/master/semver/semver.go plus its supporting helpers/tests as needed: https://github.com/coreos/go-semver/tree/master/semver . Then add them to a new package like ./internal/semver in OPA (so no new golang module dependency, we're hard vendoring them as an internal package). Preserve the original licensing info and all that, we did something similar for ./internal/jwx as a reference.

The reasoning is that this type of helper library doesn't change much, if ever. Semvers are a pretty static thing, the spec isn't constantly changing and that code shouldn't need updates frequently. It's worth the extra cost of maintaining an internal fork to prevent any new dependencies on OPA for cases like this.

Edit: To clarify, it doesn't have to be that one I linked, we can even take the implementation from golang/x/mod if its small and assuming the license is compatible.

[charlieegan3]

Hey Patrick, thanks for the review - makes sense. I will have a look at making that change over the next day or two.

[charlieegan3]

I've had another go at this. I opted to use the coreos package since it uses the same license.

In cdec628302ef71971686d4f1d593de20216f1e5f I opted to remove unused parts of the vendored package rather than explaining what they did to please the linter.

If you'd rather not have these edits, and think that the go mod license is compatible then vendoring the go mod semver package might fare better with the linter and require fewer changes to the vendored code.

Let me know what you'd prefer.

[patrick-east]

In cdec628 I opted to remove unused parts of the vendored package rather than explaining what they did to please the linter.

Perfect! 😄 No reason for us to keep code around that isn't used anyway.

Just one nit for the new files and then assuming the CI checks are all good go ahead and squash the changes.

[patrick-east]

nit: Please add the OPA copyright header to these new files in topdown

[charlieegan3]

Oops, should be all present and correct now.

[charlieegan3]

Hopefully ready to go. I've squashed the commits too.

I also made a last minute update to the docs. The functions in the coreos package don't expect the v prefix in version strings. I think this is probably my preference anyway. The prefix v was only there because of the go package's implementation anyway.