Skip to content

Commit

Permalink
tproxy: job submission hooks (#20244)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tgross committed Apr 5, 2024
1 parent d1f3a72 commit 8b6d6e4
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 1 deletion.
6 changes: 6 additions & 0 deletions nomad/job_endpoint_hook_connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
20 changes: 20 additions & 0 deletions nomad/job_endpoint_hook_connect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
18 changes: 17 additions & 1 deletion nomad/job_endpoint_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down
54 changes: 54 additions & 0 deletions nomad/job_endpoint_hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 18 additions & 0 deletions nomad/structs/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,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
}
43 changes: 43 additions & 0 deletions nomad/structs/job_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

0 comments on commit 8b6d6e4

Please sign in to comment.