Fail validation if system job has affinities

nomad/structs/structs.go

@@ -2172,13 +2172,18 @@ func (j *Job) Validate() error {
 			mErr.Errors = append(mErr.Errors, outer)
 		}
 	}
-
+	if j.Type == JobTypeSystem {
+		if j.Affinities != nil {
+			mErr.Errors = append(mErr.Errors, fmt.Errorf("System jobs should not have an affinity stanza"))
+		}
+	} else {
 		for idx, affinity := range j.Affinities {
 			if err := affinity.Validate(); err != nil {
 				outer := fmt.Errorf("Affinity %d validation failed: %s", idx+1, err)
 				mErr.Errors = append(mErr.Errors, outer)
 			}
 		}
+	}
 
 	// Check for duplicate task groups
 	taskGroups := make(map[string]int)
@@ -3424,13 +3429,18 @@ func (tg *TaskGroup) Validate(j *Job) error {
 			mErr.Errors = append(mErr.Errors, outer)
 		}
 	}
-
+	if j.Type == JobTypeSystem {
+		if tg.Affinities != nil {
+			mErr.Errors = append(mErr.Errors, fmt.Errorf("System jobs should not have an affinity stanza"))
+		}
+	} else {
 		for idx, affinity := range tg.Affinities {
 			if err := affinity.Validate(); err != nil {
 				outer := fmt.Errorf("Affinity %d validation failed: %s", idx+1, err)
 				mErr.Errors = append(mErr.Errors, outer)
 			}
 		}
+	}
 
 	if tg.RestartPolicy != nil {
 		if err := tg.RestartPolicy.Validate(); err != nil {
@@ -3528,7 +3538,7 @@ func (tg *TaskGroup) Validate(j *Job) error {
 
 	// Validate the tasks
 	for _, task := range tg.Tasks {
-		if err := task.Validate(tg.EphemeralDisk); err != nil {
+		if err := task.Validate(tg.EphemeralDisk, j.Type); err != nil {
 			outer := fmt.Errorf("Task %s validation failed: %v", task.Name, err)
 			mErr.Errors = append(mErr.Errors, outer)
 		}
@@ -4164,7 +4174,7 @@ func (t *Task) GoString() string {
 }
 
 // Validate is used to sanity check a task
-func (t *Task) Validate(ephemeralDisk *EphemeralDisk) error {
+func (t *Task) Validate(ephemeralDisk *EphemeralDisk, jobType string) error {
 	var mErr multierror.Error
 	if t.Name == "" {
 		mErr.Errors = append(mErr.Errors, errors.New("Missing task name"))
@@ -4218,12 +4228,18 @@ func (t *Task) Validate(ephemeralDisk *EphemeralDisk) error {
 		}
 	}
 
+	if jobType == JobTypeSystem {
+		if t.Affinities != nil {
+			mErr.Errors = append(mErr.Errors, fmt.Errorf("System jobs should not have an affinity stanza"))
+		}
+	} else {
 		for idx, affinity := range t.Affinities {
 			if err := affinity.Validate(); err != nil {
 				outer := fmt.Errorf("Affinity %d validation failed: %s", idx+1, err)
 				mErr.Errors = append(mErr.Errors, outer)
 			}
 		}
+	}
 
 	// Validate Services
 	if err := validateServices(t); err != nil {

nomad/structs/structs_test.go

@@ -384,6 +384,27 @@ func TestJob_SystemJob_Validate(t *testing.T) {
 	if err := j.Validate(); err != nil {
 		t.Fatalf("unexpected err: %v", err)
 	}
+
+	// Add affinities at job, task group and task level, that should fail validation
+
+	j.Affinities = []*Affinity{{
+		Operand: "=",
+		LTarget: "${node.datacenter}",
+		RTarget: "dc1",
+	}}
+	j.TaskGroups[0].Affinities = []*Affinity{{
+		Operand: "=",
+		LTarget: "${meta.rack}",
+		RTarget: "r1",
+	}}
+	j.TaskGroups[0].Tasks[0].Affinities = []*Affinity{{
+		Operand: "=",
+		LTarget: "${meta.rack}",
+		RTarget: "r1",
+	}}
+	err = j.Validate()
+	require.NotNil(t, err)
+	require.Contains(t, err.Error(), "System jobs should not have an affinity stanza")
 }
 
 func TestJob_VaultPolicies(t *testing.T) {
@@ -739,7 +760,7 @@ func TestTaskGroup_Validate(t *testing.T) {
 func TestTask_Validate(t *testing.T) {
 	task := &Task{}
 	ephemeralDisk := DefaultEphemeralDisk()
-	err := task.Validate(ephemeralDisk)
+	err := task.Validate(ephemeralDisk, JobTypeBatch)
 	mErr := err.(*multierror.Error)
 	if !strings.Contains(mErr.Errors[0].Error(), "task name") {
 		t.Fatalf("err: %s", err)
@@ -752,7 +773,7 @@ func TestTask_Validate(t *testing.T) {
 	}
 
 	task = &Task{Name: "web/foo"}
-	err = task.Validate(ephemeralDisk)
+	err = task.Validate(ephemeralDisk, JobTypeBatch)
 	mErr = err.(*multierror.Error)
 	if !strings.Contains(mErr.Errors[0].Error(), "slashes") {
 		t.Fatalf("err: %s", err)
@@ -769,7 +790,7 @@ func TestTask_Validate(t *testing.T) {
 		LogConfig: DefaultLogConfig(),
 	}
 	ephemeralDisk.SizeMB = 200
-	err = task.Validate(ephemeralDisk)
+	err = task.Validate(ephemeralDisk, JobTypeBatch)
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
@@ -783,7 +804,7 @@ func TestTask_Validate(t *testing.T) {
 			LTarget: "${meta.rack}",
 		})
 
-	err = task.Validate(ephemeralDisk)
+	err = task.Validate(ephemeralDisk, JobTypeBatch)
 	mErr = err.(*multierror.Error)
 	if !strings.Contains(mErr.Errors[0].Error(), "task level: distinct_hosts") {
 		t.Fatalf("err: %s", err)
@@ -866,7 +887,7 @@ func TestTask_Validate_Services(t *testing.T) {
 		},
 	}
 
-	err := task.Validate(ephemeralDisk)
+	err := task.Validate(ephemeralDisk, JobTypeService)
 	if err == nil {
 		t.Fatal("expected an error")
 	}
@@ -887,7 +908,7 @@ func TestTask_Validate_Services(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	if err = task1.Validate(ephemeralDisk); err != nil {
+	if err = task1.Validate(ephemeralDisk, JobTypeService); err != nil {
 		t.Fatalf("err : %v", err)
 	}
 }
@@ -946,7 +967,7 @@ func TestTask_Validate_Service_AddressMode_Ok(t *testing.T) {
 	for _, service := range cases {
 		task := getTask(service)
 		t.Run(service.Name, func(t *testing.T) {
-			if err := task.Validate(ephemeralDisk); err != nil {
+			if err := task.Validate(ephemeralDisk, JobTypeService); err != nil {
 				t.Fatalf("unexpected err: %v", err)
 			}
 		})
@@ -999,7 +1020,7 @@ func TestTask_Validate_Service_AddressMode_Bad(t *testing.T) {
 	for _, service := range cases {
 		task := getTask(service)
 		t.Run(service.Name, func(t *testing.T) {
-			err := task.Validate(ephemeralDisk)
+			err := task.Validate(ephemeralDisk, JobTypeService)
 			if err == nil {
 				t.Fatalf("expected an error")
 			}
@@ -1320,7 +1341,7 @@ func TestTask_Validate_LogConfig(t *testing.T) {
 		SizeMB: 1,
 	}
 
-	err := task.Validate(ephemeralDisk)
+	err := task.Validate(ephemeralDisk, JobTypeService)
 	mErr := err.(*multierror.Error)
 	if !strings.Contains(mErr.Errors[3].Error(), "log storage") {
 		t.Fatalf("err: %s", err)
@@ -1337,7 +1358,7 @@ func TestTask_Validate_Template(t *testing.T) {
 		SizeMB: 1,
 	}
 
-	err := task.Validate(ephemeralDisk)
+	err := task.Validate(ephemeralDisk, JobTypeService)
 	if !strings.Contains(err.Error(), "Template 1 validation failed") {
 		t.Fatalf("err: %s", err)
 	}
@@ -1350,7 +1371,7 @@ func TestTask_Validate_Template(t *testing.T) {
 	}
 
 	task.Templates = []*Template{good, good}
-	err = task.Validate(ephemeralDisk)
+	err = task.Validate(ephemeralDisk, JobTypeService)
 	if !strings.Contains(err.Error(), "same destination as") {
 		t.Fatalf("err: %s", err)
 	}
@@ -1363,7 +1384,7 @@ func TestTask_Validate_Template(t *testing.T) {
 		},
 	}
 
-	err = task.Validate(ephemeralDisk)
+	err = task.Validate(ephemeralDisk, JobTypeService)
 	if err == nil {
 		t.Fatalf("expected error from Template.Validate")
 	}