Skip to content

Commit

Permalink
server: handle invalid jobs in expose handler hook (#10154)
Browse files Browse the repository at this point in the history
The expose handler hook must handle if the submitted job is invalid. Without this validation, the rpc handler panics on invalid input.

Co-authored-by: Tim Gross <tgross@hashicorp.com>
  • Loading branch information
Mahmood Ali and tgross committed Mar 10, 2021
1 parent a0aef18 commit b0ca1fd
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 0 deletions.
9 changes: 9 additions & 0 deletions nomad/job_endpoint_hook_expose_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ func (jobExposeCheckHook) Mutate(job *structs.Job) (_ *structs.Job, warnings []e
for _, s := range tg.Services {
for _, c := range s.Checks {
if c.Expose {
// TG isn't validated yet, but validation
// may depend on mutation results.
// Do basic validation here and skip mutation,
// so Validate can return a meaningful error
// messages
if !s.Connect.HasSidecar() {
continue
}

if exposePath, err := exposePathForCheck(tg, s, c); err != nil {
return nil, nil, err
} else if exposePath != nil {
Expand Down
2 changes: 2 additions & 0 deletions nomad/job_endpoint_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ type jobValidator interface {
}

func (j *Job) admissionControllers(job *structs.Job) (out *structs.Job, warnings []error, err error) {
// Mutators run first before validators, so validators view the final rendered job.
// So, mutators must handle invalid jobs.
out, warnings, err = j.admissionMutators(job)
if err != nil {
return nil, nil, err
Expand Down
51 changes: 51 additions & 0 deletions nomad/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,57 @@ func TestJobEndpoint_Register_ConnectWithSidecarTask(t *testing.T) {

}

func TestJobEndpoint_Register_Connect_ValidatesWithoutSidecarTask(t *testing.T) {
t.Parallel()

s1, cleanupS1 := TestServer(t, func(c *Config) {
c.NumSchedulers = 0 // Prevent automatic dequeue
})
defer cleanupS1()
codec := rpcClient(t, s1)
testutil.WaitForLeader(t, s1.RPC)

// Create the register request
job := mock.Job()
job.TaskGroups[0].Networks = structs.Networks{
{
Mode: "bridge",
},
}
job.TaskGroups[0].Tasks[0].Services = nil
job.TaskGroups[0].Services = []*structs.Service{
{
Name: "backend",
PortLabel: "8080",
Connect: &structs.ConsulConnect{
SidecarService: nil,
},
Checks: []*structs.ServiceCheck{{
Name: "exposed_no_sidecar",
Type: "http",
Expose: true,
Path: "/health",
Interval: 10 * time.Second,
Timeout: 2 * time.Second,
}},
},
}

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)
require.Error(t, err)
require.Contains(t, err.Error(), "exposed_no_sidecar requires use of sidecar_proxy")
}

// TestJobEndpoint_Register_Connect_AllowUnauthenticatedFalse asserts that a job
// submission fails allow_unauthenticated is false, and either an invalid or no
// operator Consul token is provided.
Expand Down

0 comments on commit b0ca1fd

Please sign in to comment.