diff --git a/nomad/job_endpoint_hook_expose_check.go b/nomad/job_endpoint_hook_expose_check.go index ad60f706a466..7b2202438a3e 100644 --- a/nomad/job_endpoint_hook_expose_check.go +++ b/nomad/job_endpoint_hook_expose_check.go @@ -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 { diff --git a/nomad/job_endpoint_hooks.go b/nomad/job_endpoint_hooks.go index a7594ea91f76..043367ea0a89 100644 --- a/nomad/job_endpoint_hooks.go +++ b/nomad/job_endpoint_hooks.go @@ -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 diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index 62bad2c33eb6..82dace1c4340 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -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.