From 3f4bed06d51eb7bb019bbd7d020eea717d724964 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Fri, 23 Apr 2021 16:22:17 -0400 Subject: [PATCH] api: Ignore User provided ParentID (#10424) ParentID is an internal field that Nomad sets for dispatched or parameterized jobs. Job submitters should not be able to set it directly, as that messes up children tracking. Fixes #10422 . It specifically stops the scheduler from honoring the ParentID. The reason failure and why the scheduler didn't schedule that job once it was created is very interesting and requires follow up with a more technical issue. --- command/agent/job_endpoint.go | 1 - command/agent/job_endpoint_test.go | 64 +++++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 6c3c4cd4020..a5b3cf4b1ff 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -783,7 +783,6 @@ func ApiJobToStructJob(job *api.Job) *structs.Job { Region: *job.Region, Namespace: *job.Namespace, ID: *job.ID, - ParentID: *job.ParentID, Name: *job.Name, Type: *job.Type, Priority: *job.Priority, diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 43f395a5f4f..d5219e48394 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -221,6 +221,68 @@ func TestHTTP_JobsRegister(t *testing.T) { }) } +func TestHTTP_JobsRegister_IgnoresParentID(t *testing.T) { + t.Parallel() + httpTest(t, nil, func(s *TestAgent) { + // Create the job + job := MockJob() + parentID := "somebadparentid" + job.ParentID = &parentID + args := api.JobRegisterRequest{ + Job: job, + WriteRequest: api.WriteRequest{Region: "global"}, + } + buf := encodeReq(args) + + // Make the HTTP request + req, err := http.NewRequest("PUT", "/v1/jobs", buf) + require.NoError(t, err) + respW := httptest.NewRecorder() + + // Make the request + obj, err := s.Server.JobsRequest(respW, req) + require.NoError(t, err) + + // Check the response + reg := obj.(structs.JobRegisterResponse) + require.NotEmpty(t, reg.EvalID) + + // Check for the index + require.NotEmpty(t, respW.HeaderMap.Get("X-Nomad-Index")) + + // Check the job is registered + getReq := structs.JobSpecificRequest{ + JobID: *job.ID, + QueryOptions: structs.QueryOptions{ + Region: "global", + Namespace: structs.DefaultNamespace, + }, + } + var getResp structs.SingleJobResponse + err = s.Agent.RPC("Job.GetJob", &getReq, &getResp) + require.NoError(t, err) + + require.NotNil(t, getResp.Job) + require.Equal(t, *job.ID, getResp.Job.ID) + require.Empty(t, getResp.Job.ParentID) + + // check the eval exists + evalReq := structs.EvalSpecificRequest{ + EvalID: reg.EvalID, + QueryOptions: structs.QueryOptions{ + Region: "global", + Namespace: structs.DefaultNamespace, + }, + } + var evalResp structs.SingleEvalResponse + err = s.Agent.RPC("Eval.GetEval", &evalReq, &evalResp) + require.NoError(t, err) + + require.NotNil(t, evalResp.Eval) + require.Equal(t, reg.EvalID, evalResp.Eval.ID) + }) +} + // Test that ACL token is properly threaded through to the RPC endpoint func TestHTTP_JobsRegister_ACL(t *testing.T) { t.Parallel() @@ -2167,7 +2229,6 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { Namespace: "foo", VaultNamespace: "ghi789", ID: "foo", - ParentID: "lol", Name: "name", Type: "service", Priority: 50, @@ -2655,7 +2716,6 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { Region: "global", Namespace: "foo", ID: "foo", - ParentID: "lol", Name: "name", Type: "system", Priority: 50,