core: add semver constraint

The existing version constraint uses logic optimized for package
managers, not schedulers, when checking prereleases:

- 1.3.0-beta1 will *not* satisfy ">= 0.6.1"
- 1.7.0-rc1 will *not* satisfy ">= 1.6.0-beta1"

This is due to package managers wishing to favor final releases over
prereleases.

In a scheduler versions more often represent the earliest release all
required features/APIs are available in a system. Whether the constraint
or the version being evaluated are prereleases has no impact on
ordering.

This commit adds a new constraint - `semver` - which will use Semver
v2.0 ordering when evaluating constraints. Given the above examples:

- 1.3.0-beta1 satisfies ">= 0.6.1" using `semver`
- 1.7.0-rc1 satisfies ">= 1.6.0-beta1" using `semver`

Since existing jobspecs may rely on the old behavior, a new constraint
was added and the implicit Consul Connect and Vault constraints were
updated to use it.

api/constraint.go

@@ -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"

helper/constraints/semver/constraints.go

@@ -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
+}

helper/constraints/semver/constraints_test.go

@@ -0,0 +1,125 @@
+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},
+
+		// 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)
+		}
+	}
+}

jobspec/parse.go

@@ -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
@@ -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 {
@@ -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 {
@@ -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 {

jobspec/parse_test.go

@@ -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{

jobspec/test-fixtures/basic.hcl

@@ -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"

nomad/job_endpoint.go

@@ -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,

nomad/job_endpoint_hook_connect.go

@@ -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,
 		}
 	}
 )

nomad/job_endpoint_hooks.go

@@ -21,7 +21,7 @@ var (
 	vaultConstraint = &structs.Constraint{
 		LTarget: vaultConstraintLTarget,
 		RTarget: ">= 0.6.1",
-		Operand: structs.ConstraintVersion,
+		Operand: structs.ConstraintSemver,
 	}
 )