Merge pull request #6687 from hashicorp/f-override-vault-constraint

vault: allow overriding implicit vault constraint

CHANGELOG.md

@@ -21,6 +21,7 @@ BUG FIXES:
  * nomad: Multiple connect enabled services in the same taskgroup failed to
    register [[GH-6646](https://github.com/hashicorp/nomad/issues/6646)]
  * scheduler: Changes to devices in resource stanza should cause rescheduling [[GH-6644](https://github.com/hashicorp/nomad/issues/6644)]
+ * vault: Allow overriding implicit Vault version constraint [[GH-6687](https://github.com/hashicorp/nomad/issues/6687)]
  * vault: Supported Vault auth role's new field, `token_period` [[GH-6574](https://github.com/hashicorp/nomad/issues/6574)]
 
 ## 0.10.1 (November 4, 2019)

nomad/job_endpoint.go

@@ -33,13 +33,6 @@ const (
 )
 
 var (
-	// vaultConstraint is the implicit constraint added to jobs requesting a
-	// Vault token
-	vaultConstraint = &structs.Constraint{
-		LTarget: "${attr.vault.version}",
-		RTarget: ">= 0.6.1",
-		Operand: structs.ConstraintVersion,
-	}
 
 	// allowRescheduleTransition is the transition that allows failed
 	// allocations to be force rescheduled. We create a one off

nomad/job_endpoint_hooks.go

@@ -8,6 +8,23 @@ import (
 	"github.com/hashicorp/nomad/nomad/structs"
 )
 
+const (
+	// vaultConstraintLTarget is the lefthand side of the Vault constraint
+	// injected when Vault policies are used. If an existing constraint
+	// with this target exists it overrides the injected constraint.
+	vaultConstraintLTarget = "${attr.vault.version}"
+)
+
+var (
+	// vaultConstraint is the implicit constraint added to jobs requesting a
+	// Vault token
+	vaultConstraint = &structs.Constraint{
+		LTarget: vaultConstraintLTarget,
+		RTarget: ">= 0.6.1",
+		Operand: structs.ConstraintVersion,
+	}
+)
+
 type admissionController interface {
 	Name() string
 }
@@ -112,7 +129,7 @@ func (jobImpliedConstraints) Mutate(j *structs.Job) (*structs.Job, []error, erro
 		return j, nil, nil
 	}
 
-	// Add Vault constraints
+	// Add Vault constraints if no Vault constraint exists
 	for _, tg := range j.TaskGroups {
 		_, ok := policies[tg.Name]
 		if !ok {
@@ -122,7 +139,7 @@ func (jobImpliedConstraints) Mutate(j *structs.Job) (*structs.Job, []error, erro
 
 		found := false
 		for _, c := range tg.Constraints {
-			if c.Equals(vaultConstraint) {
+			if c.LTarget == vaultConstraintLTarget {
 				found = true
 				break
 			}

nomad/job_endpoint_test.go

@@ -847,6 +847,8 @@ func TestJobEndpoint_Register_EnforceIndex(t *testing.T) {
 	}
 }
 
+// TestJobEndpoint_Register_Vault_Disabled asserts that submitting a job that
+// uses Vault when Vault is *disabled* results in an error.
 func TestJobEndpoint_Register_Vault_Disabled(t *testing.T) {
 	t.Parallel()
 	s1 := TestServer(t, func(c *Config) {
@@ -880,6 +882,9 @@ func TestJobEndpoint_Register_Vault_Disabled(t *testing.T) {
 	}
 }
 
+// TestJobEndpoint_Register_Vault_AllowUnauthenticated asserts submitting a job
+// with a Vault policy but without a Vault token is *succeeds* if
+// allow_unauthenticated=true.
 func TestJobEndpoint_Register_Vault_AllowUnauthenticated(t *testing.T) {
 	t.Parallel()
 	s1 := TestServer(t, func(c *Config) {
@@ -933,6 +938,64 @@ func TestJobEndpoint_Register_Vault_AllowUnauthenticated(t *testing.T) {
 	}
 }
 
+// TestJobEndpoint_Register_Vault_OverrideConstraint asserts that job
+// submitters can specify their own Vault constraint to override the
+// automatically injected one.
+func TestJobEndpoint_Register_Vault_OverrideConstraint(t *testing.T) {
+	t.Parallel()
+	s1 := TestServer(t, func(c *Config) {
+		c.NumSchedulers = 0 // Prevent automatic dequeue
+	})
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	testutil.WaitForLeader(t, s1.RPC)
+
+	// Enable vault and allow authenticated
+	tr := true
+	s1.config.VaultConfig.Enabled = &tr
+	s1.config.VaultConfig.AllowUnauthenticated = &tr
+
+	// Replace the Vault Client on the server
+	s1.vault = &TestVaultClient{}
+
+	// Create the register request with a job asking for a vault policy
+	job := mock.Job()
+	job.TaskGroups[0].Tasks[0].Vault = &structs.Vault{
+		Policies:   []string{"foo"},
+		ChangeMode: structs.VaultChangeModeRestart,
+	}
+	job.TaskGroups[0].Tasks[0].Constraints = []*structs.Constraint{
+		{
+			LTarget: "${attr.vault.version}",
+			Operand: "is_set",
+		},
+	}
+	req := &structs.JobRegisterRequest{
+		Job: job,
+		WriteRequest: structs.WriteRequest{
+			Region:    "global",
+			Namespace: job.Namespace,
+		},
+	}
+
+	// Fetch the response
+	var resp structs.JobRegisterResponse
+	err := msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp)
+
+	// Check for the job in the FSM
+	state := s1.fsm.State()
+	ws := memdb.NewWatchSet()
+	out, err := state.JobByID(ws, job.Namespace, job.ID)
+	require.NoError(t, err)
+	require.NotNil(t, out)
+	require.Equal(t, resp.JobModifyIndex, out.CreateIndex)
+
+	// Assert constraint was not overridden by the server
+	outConstraints := out.TaskGroups[0].Tasks[0].Constraints
+	require.Len(t, outConstraints, 1)
+	require.True(t, job.TaskGroups[0].Tasks[0].Constraints[0].Equals(outConstraints[0]))
+}
+
 func TestJobEndpoint_Register_Vault_NoToken(t *testing.T) {
 	t.Parallel()
 	s1 := TestServer(t, func(c *Config) {