From 5ea4382af7c836ccbc8bbcd763691aad0cccd2e2 Mon Sep 17 00:00:00 2001
From: Michael Schurter <mschurter@hashicorp.com>
Date: Thu, 28 Mar 2019 08:59:27 -0700
Subject: [PATCH] api: fix migrate stanza initialization

Fixes Migrate to be initialized like RescheduleStrategy.

Fixes #5477
---
 api/tasks.go                       | 12 ++++----
 command/agent/job_endpoint_test.go | 44 ++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/api/tasks.go b/api/tasks.go
index a55e90f45a1..747711baa3e 100644
--- a/api/tasks.go
+++ b/api/tasks.go
@@ -563,13 +563,11 @@ func (g *TaskGroup) Canonicalize(job *Job) {
 	}
 
 	// Merge with default reschedule policy
-	if *job.Type == "service" {
-		defaultMigrateStrategy := &MigrateStrategy{}
-		defaultMigrateStrategy.Canonicalize()
-		if g.Migrate != nil {
-			defaultMigrateStrategy.Merge(g.Migrate)
-		}
-		g.Migrate = defaultMigrateStrategy
+	if g.Migrate == nil && *job.Type == "service" {
+		g.Migrate = &MigrateStrategy{}
+	}
+	if g.Migrate != nil {
+		g.Migrate.Canonicalize()
 	}
 
 	var defaultRestartPolicy *RestartPolicy
diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go
index acd103c4c90..f93396939b5 100644
--- a/command/agent/job_endpoint_test.go
+++ b/command/agent/job_endpoint_test.go
@@ -15,6 +15,7 @@ import (
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/kr/pretty"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestHTTP_JobsList(t *testing.T) {
@@ -2034,3 +2035,46 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 		t.Fatalf("bad:\n%s", strings.Join(diff, "\n"))
 	}
 }
+
+// TestHTTP_JobValidate_SystemMigrate asserts that a system job with a migrate
+// stanza fails to validate but does not panic (see #5477).
+func TestHTTP_JobValidate_SystemMigrate(t *testing.T) {
+	t.Parallel()
+	httpTest(t, nil, func(s *TestAgent) {
+		// Create the job
+		job := &api.Job{
+			Region:      helper.StringToPtr("global"),
+			Datacenters: []string{"dc1"},
+			ID:          helper.StringToPtr("systemmigrate"),
+			Name:        helper.StringToPtr("systemmigrate"),
+			TaskGroups: []*api.TaskGroup{
+				{Name: helper.StringToPtr("web")},
+			},
+
+			// System job...
+			Type: helper.StringToPtr("system"),
+
+			// ...with an empty migrate stanza
+			Migrate: &api.MigrateStrategy{},
+		}
+
+		args := api.JobValidateRequest{
+			Job:          job,
+			WriteRequest: api.WriteRequest{Region: "global"},
+		}
+		buf := encodeReq(args)
+
+		// Make the HTTP request
+		req, err := http.NewRequest("PUT", "/v1/validate/job", buf)
+		require.NoError(t, err)
+		respW := httptest.NewRecorder()
+
+		// Make the request
+		obj, err := s.Server.ValidateJobRequest(respW, req)
+		require.NoError(t, err)
+
+		// Check the response
+		resp := obj.(structs.JobValidateResponse)
+		require.Contains(t, resp.Error, `Job type "system" does not allow migrate block`)
+	})
+}