From 1eac062c169d6cdc48714c61997cff8f9a6d2ee8 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Thu, 4 Apr 2024 11:02:49 -0400 Subject: [PATCH] tproxy: job submission hooks (#20244) Add a constraint on job submission that requires the `consul-cni` plugin fingerprint whenever transparent proxy is used. Add a validation that the `network.dns` cannot be set when transparent proxy is used, unless the `no_dns` flag is set. --- nomad/job_endpoint_hook_connect.go | 6 +++ nomad/job_endpoint_hook_connect_test.go | 20 +++++++++ nomad/job_endpoint_hooks.go | 18 ++++++++- nomad/job_endpoint_hooks_test.go | 54 +++++++++++++++++++++++++ nomad/structs/job.go | 18 +++++++++ nomad/structs/job_test.go | 43 ++++++++++++++++++++ 6 files changed, 158 insertions(+), 1 deletion(-) diff --git a/nomad/job_endpoint_hook_connect.go b/nomad/job_endpoint_hook_connect.go index df0087620e06..d9ab3b3d16b8 100644 --- a/nomad/job_endpoint_hook_connect.go +++ b/nomad/job_endpoint_hook_connect.go @@ -592,6 +592,12 @@ func groupConnectUpstreamsValidate(g *structs.TaskGroup, services []*structs.Ser if tp := service.Connect.SidecarService.Proxy.TransparentProxy; tp != nil { hasTproxy = true + for _, net := range g.Networks { + if !net.DNS.IsZero() && !tp.NoDNS { + return fmt.Errorf( + "Consul Connect transparent proxy cannot be used with network.dns unless no_dns=true") + } + } for _, portLabel := range tp.ExcludeInboundPorts { if !transparentProxyPortLabelValidate(g, portLabel) { return fmt.Errorf( diff --git a/nomad/job_endpoint_hook_connect_test.go b/nomad/job_endpoint_hook_connect_test.go index c741f6e7db79..429309f85e1a 100644 --- a/nomad/job_endpoint_hook_connect_test.go +++ b/nomad/job_endpoint_hook_connect_test.go @@ -681,6 +681,26 @@ func TestJobEndpointConnect_groupConnectUpstreamsValidate(t *testing.T) { }) must.EqError(t, err, `Consul Connect transparent proxy requires there is only one connect block`) }) + + t.Run("Consul Connect transparent proxy DNS not allowed with network.dns", func(t *testing.T) { + tg := &structs.TaskGroup{Name: "group", Networks: []*structs.NetworkResource{{ + DNS: &structs.DNSConfig{Servers: []string{"1.1.1.1"}}, + }}} + err := groupConnectUpstreamsValidate(tg, + []*structs.Service{ + { + Name: "s1", + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + TransparentProxy: &structs.ConsulTransparentProxy{}, + }, + }, + }, + }, + }) + must.EqError(t, err, `Consul Connect transparent proxy cannot be used with network.dns unless no_dns=true`) + }) } func TestJobEndpointConnect_getNamedTaskForNativeService(t *testing.T) { diff --git a/nomad/job_endpoint_hooks.go b/nomad/job_endpoint_hooks.go index 809958c38ffe..dba4c943872f 100644 --- a/nomad/job_endpoint_hooks.go +++ b/nomad/job_endpoint_hooks.go @@ -26,6 +26,7 @@ const ( attrHostLocalCNI = `${attr.plugins.cni.version.host-local}` attrLoopbackCNI = `${attr.plugins.cni.version.loopback}` attrPortMapCNI = `${attr.plugins.cni.version.portmap}` + attrConsulCNI = `${attr.plugins.cni.version.consul-cni}` ) // cniMinVersion is the version expression for the minimum CNI version supported @@ -134,6 +135,14 @@ var ( RTarget: cniMinVersion, Operand: structs.ConstraintSemver, } + + // cniConsulConstraint is an implicit constraint added to jobs making use of + // transparent proxy mode. + cniConsulConstraint = &structs.Constraint{ + LTarget: attrConsulCNI, + RTarget: ">= 1.4.2", + Operand: structs.ConstraintSemver, + } ) type admissionController interface { @@ -250,12 +259,15 @@ func (jobImpliedConstraints) Mutate(j *structs.Job) (*structs.Job, []error, erro bridgeNetworkingTaskGroups := j.RequiredBridgeNetwork() + transparentProxyTaskGroups := j.RequiredTransparentProxy() + // Hot path where none of our things require constraints. // // [UPDATE THIS] if you are adding a new constraint thing! if len(signals) == 0 && len(vaultBlocks) == 0 && nativeServiceDisco.Empty() && len(consulServiceDisco) == 0 && - numaTaskGroups.Empty() && bridgeNetworkingTaskGroups.Empty() { + numaTaskGroups.Empty() && bridgeNetworkingTaskGroups.Empty() && + transparentProxyTaskGroups.Empty() { return j, nil, nil } @@ -320,6 +332,10 @@ func (jobImpliedConstraints) Mutate(j *structs.Job) (*structs.Job, []error, erro mutateConstraint(constraintMatcherLeft, tg, cniLoopbackConstraint) mutateConstraint(constraintMatcherLeft, tg, cniPortMapConstraint) } + + if transparentProxyTaskGroups.Contains(tg.Name) { + mutateConstraint(constraintMatcherLeft, tg, cniConsulConstraint) + } } return j, nil, nil diff --git a/nomad/job_endpoint_hooks_test.go b/nomad/job_endpoint_hooks_test.go index 8b73cc7f9d2f..4fc850e9ca89 100644 --- a/nomad/job_endpoint_hooks_test.go +++ b/nomad/job_endpoint_hooks_test.go @@ -1194,6 +1194,60 @@ func Test_jobImpliedConstraints_Mutate(t *testing.T) { expectedOutputError: nil, name: "task group with bridge network", }, + { + inputJob: &structs.Job{ + Name: "example", + TaskGroups: []*structs.TaskGroup{ + { + Name: "group-with-tproxy", + Services: []*structs.Service{{ + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + TransparentProxy: &structs.ConsulTransparentProxy{}, + }, + }, + }, + }}, + Networks: []*structs.NetworkResource{ + {Mode: "bridge"}, + }, + }, + }, + }, + expectedOutputJob: &structs.Job{ + Name: "example", + TaskGroups: []*structs.TaskGroup{ + { + Name: "group-with-tproxy", + Services: []*structs.Service{{ + Connect: &structs.ConsulConnect{ + SidecarService: &structs.ConsulSidecarService{ + Proxy: &structs.ConsulProxy{ + TransparentProxy: &structs.ConsulTransparentProxy{}, + }, + }, + }, + }}, + Networks: []*structs.NetworkResource{ + {Mode: "bridge"}, + }, + Constraints: []*structs.Constraint{ + consulServiceDiscoveryConstraint, + cniBridgeConstraint, + cniFirewallConstraint, + cniHostLocalConstraint, + cniLoopbackConstraint, + cniPortMapConstraint, + cniConsulConstraint, + }, + }, + }, + }, + expectedOutputWarnings: nil, + expectedOutputError: nil, + name: "task group with tproxy", + }, } for _, tc := range testCases { diff --git a/nomad/structs/job.go b/nomad/structs/job.go index 174359f952fa..e288b62ed3ec 100644 --- a/nomad/structs/job.go +++ b/nomad/structs/job.go @@ -186,3 +186,21 @@ func (j *Job) RequiredBridgeNetwork() set.Collection[string] { } return result } + +// RequiredTransparentProxy identifies which task groups, if any, within the job +// contain Connect blocks using transparent proxy +func (j *Job) RequiredTransparentProxy() set.Collection[string] { + result := set.New[string](len(j.TaskGroups)) + for _, tg := range j.TaskGroups { + for _, service := range tg.Services { + if service.Connect != nil { + if service.Connect.HasTransparentProxy() { + result.Insert(tg.Name) + break // to next TaskGroup + } + } + } + } + + return result +} diff --git a/nomad/structs/job_test.go b/nomad/structs/job_test.go index 2a90eadfeb0b..f46dc2686fa6 100644 --- a/nomad/structs/job_test.go +++ b/nomad/structs/job_test.go @@ -471,3 +471,46 @@ func TestJob_RequiredNUMA(t *testing.T) { }) } } + +func TestJob_RequiredTproxy(t *testing.T) { + job := &Job{ + TaskGroups: []*TaskGroup{ + {Name: "no services"}, + {Name: "services-without-connect", + Services: []*Service{{Name: "foo"}}, + }, + {Name: "services-with-connect-but-no-tproxy", + Services: []*Service{ + {Name: "foo", Connect: &ConsulConnect{}}, + {Name: "bar", Connect: &ConsulConnect{}}}, + }, + {Name: "has-tproxy-1", + Services: []*Service{ + {Name: "foo", Connect: &ConsulConnect{}}, + {Name: "bar", Connect: &ConsulConnect{ + SidecarService: &ConsulSidecarService{ + Proxy: &ConsulProxy{ + TransparentProxy: &ConsulTransparentProxy{}, + }, + }, + }}}, + }, + {Name: "has-tproxy-2", + Services: []*Service{ + {Name: "baz", Connect: &ConsulConnect{ + SidecarService: &ConsulSidecarService{ + Proxy: &ConsulProxy{ + TransparentProxy: &ConsulTransparentProxy{}, + }, + }, + }}}, + }, + }, + } + + expect := []string{"has-tproxy-1", "has-tproxy-2"} + + job.Canonicalize() + result := job.RequiredTransparentProxy() + must.SliceContainsAll(t, expect, result.Slice()) +}