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.
hashicorp · Apr 5, 2024 · 8b6d6e4 · 8b6d6e4
1 parent d1f3a72
commit 8b6d6e4
Show file tree

Hide file tree

Showing 6 changed files with 158 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
+}
diff --git 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())
+}