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

Add new "semver" constraint #6699

Merged
merged 4 commits into from
Nov 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ IMPROVEMENTS:
BUG FIXES:

* core: Ignore `server` config values if `server` is disabled [[GH-6047](https://github.com/hashicorp/nomad/issues/6047)]
* core: Added `semver` constraint for strict Semver 2.0 version comparisons [[GH-6699](https://github.com/hashicorp/nomad/issues/6699)]
* api: Return a 404 if endpoint not found instead of redirecting to /ui/ [[GH-6658](https://github.com/hashicorp/nomad/issues/6658)]
* api: Decompress web socket response body if gzipped on error responses [[GH-6650](https://github.com/hashicorp/nomad/issues/6650)]
* api: Fixed a bug where some FS/Allocation API endpoints didn't return error messages [[GH-6427](https://github.com/hashicorp/nomad/issues/6427)]
Expand Down
1 change: 1 addition & 0 deletions api/constraint.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const (
ConstraintDistinctHosts = "distinct_hosts"
ConstraintRegex = "regexp"
ConstraintVersion = "version"
ConstraintSemver = "semver"
ConstraintSetContains = "set_contains"
ConstraintSetContainsAll = "set_contains_all"
ConstraintSetContainsAny = "set_contains_any"
Expand Down
147 changes: 147 additions & 0 deletions helper/constraints/semver/constraints.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
// semver is a Semver Constraints package copied from
// github.com/hashicorp/go-version @ 2046c9d0f0b03c779670f5186a2a4b2c85493a71
//
// Unlike Constraints in go-version, Semver constraints use Semver 2.0 ordering
// rules and only accept properly formatted Semver versions.
package semver

import (
"fmt"
"regexp"
"strings"

"github.com/hashicorp/go-version"
)

// Constraint represents a single constraint for a version, such as ">=
// 1.0".
type Constraint struct {
f constraintFunc
check *version.Version
original string
}

// Constraints is a slice of constraints. We make a custom type so that
// we can add methods to it.
type Constraints []*Constraint

type constraintFunc func(v, c *version.Version) bool

var constraintOperators map[string]constraintFunc

var constraintRegexp *regexp.Regexp

func init() {
constraintOperators = map[string]constraintFunc{
"": constraintEqual,
"=": constraintEqual,
"!=": constraintNotEqual,
">": constraintGreaterThan,
"<": constraintLessThan,
">=": constraintGreaterThanEqual,
"<=": constraintLessThanEqual,
}

ops := make([]string, 0, len(constraintOperators))
for k := range constraintOperators {
ops = append(ops, regexp.QuoteMeta(k))
}

constraintRegexp = regexp.MustCompile(fmt.Sprintf(
`^\s*(%s)\s*(%s)\s*$`,
strings.Join(ops, "|"),
version.SemverRegexpRaw))
}

// NewConstraint will parse one or more constraints from the given
// constraint string. The string must be a comma-separated list of constraints.
func NewConstraint(v string) (Constraints, error) {
vs := strings.Split(v, ",")
result := make([]*Constraint, len(vs))
for i, single := range vs {
c, err := parseSingle(single)
if err != nil {
return nil, err
}

result[i] = c
}

return Constraints(result), nil
}

// Check tests if a version satisfies all the constraints.
func (cs Constraints) Check(v *version.Version) bool {
for _, c := range cs {
if !c.Check(v) {
return false
}
}

return true
}

// Returns the string format of the constraints
func (cs Constraints) String() string {
csStr := make([]string, len(cs))
for i, c := range cs {
csStr[i] = c.String()
}

return strings.Join(csStr, ",")
}

// Check tests if a constraint is validated by the given version.
func (c *Constraint) Check(v *version.Version) bool {
return c.f(v, c.check)
}

func (c *Constraint) String() string {
return c.original
}

func parseSingle(v string) (*Constraint, error) {
matches := constraintRegexp.FindStringSubmatch(v)
if matches == nil {
return nil, fmt.Errorf("Malformed constraint: %s", v)
}

check, err := version.NewSemver(matches[2])
if err != nil {
return nil, err
}

return &Constraint{
f: constraintOperators[matches[1]],
check: check,
original: v,
}, nil
}

//-------------------------------------------------------------------
// Constraint functions
//-------------------------------------------------------------------

func constraintEqual(v, c *version.Version) bool {
return v.Equal(c)
}

func constraintNotEqual(v, c *version.Version) bool {
return !v.Equal(c)
}

func constraintGreaterThan(v, c *version.Version) bool {
return v.Compare(c) == 1
}

func constraintLessThan(v, c *version.Version) bool {
return v.Compare(c) == -1
}

func constraintGreaterThanEqual(v, c *version.Version) bool {
return v.Compare(c) >= 0
}

func constraintLessThanEqual(v, c *version.Version) bool {
return v.Compare(c) <= 0
}
128 changes: 128 additions & 0 deletions helper/constraints/semver/constraints_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
package semver

import (
"testing"

"github.com/hashicorp/go-version"
)

// This file is a copy of github.com/hashicorp/go-version/constraint_test.go
// with minimal changes to demonstrate differences. Diffing the files should
// illustrate behavior differences in Constraint and version.Constraint.

func TestNewConstraint(t *testing.T) {
cases := []struct {
input string
count int
err bool
}{
{">= 1.2", 1, false},
{"1.0", 1, false},
{">= 1.x", 0, true},
{">= 1.2, < 1.0", 2, false},

// Out of bounds
{"11387778780781445675529500000000000000000", 0, true},

// Semver only
{">= 1.0beta1", 0, true},

// No pessimistic operator
{"~> 1.0", 0, true},
}

for _, tc := range cases {
v, err := NewConstraint(tc.input)
if tc.err && err == nil {
t.Fatalf("expected error for input: %s", tc.input)
} else if !tc.err && err != nil {
t.Fatalf("error for input %s: %s", tc.input, err)
}

if len(v) != tc.count {
t.Fatalf("input: %s\nexpected len: %d\nactual: %d",
tc.input, tc.count, len(v))
}
}
}

func TestConstraintCheck(t *testing.T) {
cases := []struct {
constraint string
version string
check bool
}{
{">= 1.0, < 1.2", "1.1.5", true},
{"< 1.0, < 1.2", "1.1.5", false},
{"= 1.0", "1.1.5", false},
{"= 1.0", "1.0.0", true},
{"1.0", "1.0.0", true},

schmichael marked this conversation as resolved.
Show resolved Hide resolved
// Assert numbers are *not* compared lexically as in #4729
{"> 10", "8", false},

// Pre-releases are ordered according to Semver v2
{"> 2.0", "2.1.0-beta", true},
{"> 2.1.0-a", "2.1.0-beta", true},
{"> 2.1.0-a", "2.1.1-beta", true},
{"> 2.0.0", "2.1.0-beta", true},
{"> 2.1.0-a", "2.1.1", true},
{"> 2.1.0-a", "2.1.1-beta", true},
{"> 2.1.0-a", "2.1.0", true},
{"<= 2.1.0-a", "2.0.0", true},
{">= 0.6.1", "1.3.0-beta1", true},
{"> 1.0-beta1", "1.0-rc1", true},

// Meta components are ignored according to Semver v2
{">= 0.6.1", "1.3.0-beta1+ent", true},
{">= 1.3.0-beta1", "1.3.0-beta1+ent", true},
{"> 1.3.0-beta1+cgo", "1.3.0-beta1+ent", false},
{"= 1.3.0-beta1+cgo", "1.3.0-beta1+ent", true},
}

for _, tc := range cases {
c, err := NewConstraint(tc.constraint)
if err != nil {
t.Fatalf("err: %s", err)
}

v, err := version.NewSemver(tc.version)
if err != nil {
t.Fatalf("err: %s", err)
}

actual := c.Check(v)
expected := tc.check
if actual != expected {
t.Fatalf("Version: %s\nConstraint: %s\nExpected: %#v",
tc.version, tc.constraint, expected)
}
}
}

func TestConstraintsString(t *testing.T) {
cases := []struct {
constraint string
result string
}{
{">= 1.0, < 1.2", ""},
}

for _, tc := range cases {
c, err := NewConstraint(tc.constraint)
if err != nil {
t.Fatalf("err: %s", err)
}

actual := c.String()
expected := tc.result
if expected == "" {
expected = tc.constraint
}

if actual != expected {
t.Fatalf("Constraint: %s\nExpected: %#v\nActual: %s",
tc.constraint, expected, actual)
}
}
}
16 changes: 16 additions & 0 deletions jobspec/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ func parseConstraints(result *[]*api.Constraint, list *ast.ObjectList) error {
"set_contains",
"value",
"version",
"semver",
}
if err := helper.CheckHCLKeys(o.Val, valid); err != nil {
return err
Expand All @@ -159,6 +160,13 @@ func parseConstraints(result *[]*api.Constraint, list *ast.ObjectList) error {
m["RTarget"] = constraint
}

// If "semver" is provided, set the operand
// to "semver" and the value to the "RTarget"
if constraint, ok := m[api.ConstraintSemver]; ok {
m["Operand"] = api.ConstraintSemver
m["RTarget"] = constraint
}

// If "regexp" is provided, set the operand
// to "regexp" and the value to the "RTarget"
if constraint, ok := m[api.ConstraintRegex]; ok {
Expand Down Expand Up @@ -219,6 +227,7 @@ func parseAffinities(result *[]*api.Affinity, list *ast.ObjectList) error {
"set_contains_all",
"value",
"version",
"semver",
"weight",
}
if err := helper.CheckHCLKeys(o.Val, valid); err != nil {
Expand All @@ -241,6 +250,13 @@ func parseAffinities(result *[]*api.Affinity, list *ast.ObjectList) error {
m["RTarget"] = affinity
}

// If "semver" is provided, set the operand
// to "semver" and the value to the "RTarget"
if affinity, ok := m[api.ConstraintSemver]; ok {
m["Operand"] = api.ConstraintSemver
m["RTarget"] = affinity
}

// If "regexp" is provided, set the operand
// to "regexp" and the value to the "RTarget"
if affinity, ok := m[api.ConstraintRegex]; ok {
Expand Down
5 changes: 5 additions & 0 deletions jobspec/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ func TestParse(t *testing.T) {
RTarget: "windows",
Operand: "=",
},
{
LTarget: "${attr.vault.version}",
RTarget: ">= 0.6.1",
Operand: "semver",
},
},

Affinities: []*api.Affinity{
Expand Down
6 changes: 6 additions & 0 deletions jobspec/test-fixtures/basic.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ job "binstore-storagelocker" {
value = "windows"
}

constraint {
attribute = "${attr.vault.version}"
value = ">= 0.6.1"
operator = "semver"
}

affinity {
attribute = "${meta.team}"
value = "mobile"
Expand Down
6 changes: 6 additions & 0 deletions nomad/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -951,6 +951,12 @@ func (j *Job) Allocations(args *structs.JobSpecificRequest,
return structs.ErrPermissionDenied
}

// Ensure JobID is set otherwise everything works and never returns
// allocations which can hide bugs in request code.
if args.JobID == "" {
return fmt.Errorf("missing job ID")
}

// Setup the blocking query
opts := blockingOptions{
queryOpts: &args.QueryOptions,
Expand Down
4 changes: 2 additions & 2 deletions nomad/job_endpoint_hook_connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ var (
connectVersionConstraint = func() *structs.Constraint {
return &structs.Constraint{
LTarget: "${attr.consul.version}",
RTarget: ">= 1.6.0beta1",
Operand: "version",
RTarget: ">= 1.6.0-beta1",
Operand: structs.ConstraintSemver,
}
}
)
Expand Down
2 changes: 1 addition & 1 deletion nomad/job_endpoint_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ var (
vaultConstraint = &structs.Constraint{
LTarget: vaultConstraintLTarget,
RTarget: ">= 0.6.1",
Operand: structs.ConstraintVersion,
Operand: structs.ConstraintSemver,
}
)

Expand Down
Loading