Merge pull request #2807 from hashicorp/f-validate-ports

Prevent port conflicts

CHANGELOG.md

@@ -59,6 +59,7 @@ BUG FIXES:
  * driver/exec: Properly set file/dir ownership in chroots [GH-2552]
  * driver/docker: Fix panic in Docker driver on Windows [GH-2614]
  * driver/rkt: Fix env var interpolation [GH-2777]
+ * jobspec/validation: Prevent static port conflicts [GH-2807]
  * server: Reject non-TLS clients when TLS enabled [GH-2525]
  * server: Fix a panic in plan evaluation with partial failures and all_at_once
    set [GH-2544]

nomad/structs/structs.go

@@ -2531,8 +2531,10 @@ func (tg *TaskGroup) Validate(j *Job) error {
 		}
 	}
 
-	// Check for duplicate tasks and that there is only leader task if any
+	// Check for duplicate tasks, that there is only leader task if any,
+	// and no duplicated static ports
 	tasks := make(map[string]int)
+	staticPorts := make(map[int]string)
 	leaderTasks := 0
 	for idx, task := range tg.Tasks {
 		if task.Name == "" {
@@ -2546,6 +2548,21 @@ func (tg *TaskGroup) Validate(j *Job) error {
 		if task.Leader {
 			leaderTasks++
 		}
+
+		if task.Resources == nil {
+			continue
+		}
+
+		for _, net := range task.Resources.Networks {
+			for _, port := range net.ReservedPorts {
+				if other, ok := staticPorts[port.Value]; ok {
+					err := fmt.Errorf("Static port %d already reserved by %s", port.Value, other)
+					mErr.Errors = append(mErr.Errors, err)
+				} else {
+					staticPorts[port.Value] = fmt.Sprintf("%s:%s", task.Name, port.Label)
+				}
+			}
+		}
 	}
 
 	if leaderTasks > 1 {

nomad/structs/structs_test.go

@@ -814,6 +814,59 @@ func TestTaskGroup_Validate(t *testing.T) {
 		t.Fatalf("err: %s", err)
 	}
 
+	tg = &TaskGroup{
+		Tasks: []*Task{
+			&Task{
+				Name: "task-a",
+				Resources: &Resources{
+					Networks: []*NetworkResource{
+						&NetworkResource{
+							ReservedPorts: []Port{{Label: "foo", Value: 123}},
+						},
+					},
+				},
+			},
+			&Task{
+				Name: "task-b",
+				Resources: &Resources{
+					Networks: []*NetworkResource{
+						&NetworkResource{
+							ReservedPorts: []Port{{Label: "foo", Value: 123}},
+						},
+					},
+				},
+			},
+		},
+	}
+	err = tg.Validate(&Job{})
+	expected := `Static port 123 already reserved by task-a:foo`
+	if !strings.Contains(err.Error(), expected) {
+		t.Errorf("expected %s but found: %v", expected, err)
+	}
+
+	tg = &TaskGroup{
+		Tasks: []*Task{
+			&Task{
+				Name: "task-a",
+				Resources: &Resources{
+					Networks: []*NetworkResource{
+						&NetworkResource{
+							ReservedPorts: []Port{
+								{Label: "foo", Value: 123},
+								{Label: "bar", Value: 123},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	err = tg.Validate(&Job{})
+	expected = `Static port 123 already reserved by task-a:foo`
+	if !strings.Contains(err.Error(), expected) {
+		t.Errorf("expected %s but found: %v", expected, err)
+	}
+
 	tg = &TaskGroup{
 		Name:  "web",
 		Count: 1,