fix panic when Connect mesh gateway doesn't have a proxy block (#11257)

Co-authored-by: Michael Schurter <mschurter@hashicorp.com>

.changelog/11257.txt

@@ -0,0 +1,3 @@
+```release-note:security
+consul/connect: Fixed a bug causing the Nomad agent to panic if a mesh gateway was registered without a `proxy` block.
+```

command/agent/consul/service_client.go

@@ -1100,26 +1100,32 @@ func (c *ServiceClient) serviceRegs(ops *operations, service *structs.Service, w
 		kind = api.ServiceKindIngressGateway
 	case service.Connect.IsTerminating():
 		kind = api.ServiceKindTerminatingGateway
-		// set the default port if bridge / default listener set
+
-		if defaultBind, exists := service.Connect.Gateway.Proxy.EnvoyGatewayBindAddresses["default"]; exists {
+		if proxy := service.Connect.Gateway.Proxy; proxy != nil {
-			portLabel := envoy.PortLabel(structs.ConnectTerminatingPrefix, service.Name, "")
+			// set the default port if bridge / default listener set
-			if dynPort, ok := workload.Ports.Get(portLabel); ok {
+			if defaultBind, exists := proxy.EnvoyGatewayBindAddresses["default"]; exists {
-				defaultBind.Port = dynPort.Value
+				portLabel := envoy.PortLabel(structs.ConnectTerminatingPrefix, service.Name, "")
+				if dynPort, ok := workload.Ports.Get(portLabel); ok {
+					defaultBind.Port = dynPort.Value
+				}
 			}
 		}
 	case service.Connect.IsMesh():
 		kind = api.ServiceKindMeshGateway
-		// wan uses the service port label, which is typically on a discrete host_network
+
-		if wanBind, exists := service.Connect.Gateway.Proxy.EnvoyGatewayBindAddresses["wan"]; exists {
+		if proxy := service.Connect.Gateway.Proxy; proxy != nil {
-			if wanPort, ok := workload.Ports.Get(service.PortLabel); ok {
+			// wan uses the service port label, which is typically on a discrete host_network
-				wanBind.Port = wanPort.Value
+			if wanBind, exists := proxy.EnvoyGatewayBindAddresses["wan"]; exists {
+				if wanPort, ok := workload.Ports.Get(service.PortLabel); ok {
+					wanBind.Port = wanPort.Value
+				}
 			}
-		}
+			// lan uses a nomad generated dynamic port on the default network
-		// lan uses a nomad generated dynamic port on the default network
+			if lanBind, exists := proxy.EnvoyGatewayBindAddresses["lan"]; exists {
-		if lanBind, exists := service.Connect.Gateway.Proxy.EnvoyGatewayBindAddresses["lan"]; exists {
+				portLabel := envoy.PortLabel(structs.ConnectMeshPrefix, service.Name, "lan")
-			portLabel := envoy.PortLabel(structs.ConnectMeshPrefix, service.Name, "lan")
+				if dynPort, ok := workload.Ports.Get(portLabel); ok {
-			if dynPort, ok := workload.Ports.Get(portLabel); ok {
+					lanBind.Port = dynPort.Value
-				lanBind.Port = dynPort.Value
+				}
 			}
 		}
 	}

nomad/job_endpoint_hook_connect.go

@@ -279,16 +279,10 @@ func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error {
 			// a name of an injected gateway task
 			service.Name = env.ReplaceEnv(service.Name)
 
-			// detect whether the group is in host networking mode, which will
+			// Generate a proxy configuration, if one is not provided, that is
-			// require tweaking the default gateway task config
+			// most appropriate for the network mode being used.
-			netHost := g.Networks[0].Mode == "host"
+			netMode := g.Networks[0].Mode
-
+			service.Connect.Gateway.Proxy = gatewayProxy(service.Connect.Gateway, netMode)
-			if !netHost && service.Connect.IsGateway() {
-				// Modify the gateway proxy service configuration to automatically
-				// do the correct envoy bind address plumbing when inside a net
-				// namespace, but only if things are not explicitly configured.
-				service.Connect.Gateway.Proxy = gatewayProxyForBridge(service.Connect.Gateway)
-			}
 
 			// Inject a port whether bridge or host network (if not already set).
 			// This port is accessed by the magic of Connect plumbing so it seems
@@ -317,6 +311,10 @@ func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error {
 			// inject the gateway task only if it does not yet already exist
 			if !hasGatewayTaskForService(g, service.Name) {
 				prefix := service.Connect.Gateway.Prefix()
+
+				// detect whether the group is in host networking mode, which will
+				// require tweaking the default gateway task config
+				netHost := netMode == "host"
 				task := newConnectGatewayTask(prefix, service.Name, netHost)
 				g.Tasks = append(g.Tasks, task)
 
@@ -356,10 +354,10 @@ func gatewayProxyIsDefault(proxy *structs.ConsulGatewayProxy) bool {
 	return false
 }
 
-// gatewayProxyForBridge scans an existing gateway proxy configuration and tweaks
+// gatewayProxy scans an existing gateway proxy configuration and tweaks it
-// it given an associated configuration entry so that it works as intended from
+// given an associated configuration entry so that it works as intended with
-// inside a network namespace.
+// the network mode specified.
-func gatewayProxyForBridge(gateway *structs.ConsulGateway) *structs.ConsulGatewayProxy {
+func gatewayProxy(gateway *structs.ConsulGateway, mode string) *structs.ConsulGatewayProxy {
 	if gateway == nil {
 		return nil
 	}
@@ -383,40 +381,42 @@ func gatewayProxyForBridge(gateway *structs.ConsulGateway) *structs.ConsulGatewa
 		proxy.ConnectTimeout = helper.TimeToPtr(defaultConnectTimeout)
 	}
 
-	// magically configure bind address(es) for bridge networking, per gateway type
+	if mode == "bridge" {
-	// non-default configuration is gated above
+		// magically configure bind address(es) for bridge networking, per gateway type
-	switch {
+		// non-default configuration is gated above
-	case gateway.Ingress != nil:
+		switch {
-		proxy.EnvoyGatewayNoDefaultBind = true
+		case gateway.Ingress != nil:
-		proxy.EnvoyGatewayBindTaggedAddresses = false
+			proxy.EnvoyGatewayNoDefaultBind = true
-		proxy.EnvoyGatewayBindAddresses = gatewayBindAddressesIngress(gateway.Ingress)
+			proxy.EnvoyGatewayBindTaggedAddresses = false
-	case gateway.Terminating != nil:
+			proxy.EnvoyGatewayBindAddresses = gatewayBindAddressesIngressForBridge(gateway.Ingress)
-		proxy.EnvoyGatewayNoDefaultBind = true
+		case gateway.Terminating != nil:
-		proxy.EnvoyGatewayBindTaggedAddresses = false
+			proxy.EnvoyGatewayNoDefaultBind = true
-		proxy.EnvoyGatewayBindAddresses = map[string]*structs.ConsulGatewayBindAddress{
+			proxy.EnvoyGatewayBindTaggedAddresses = false
-			"default": {
+			proxy.EnvoyGatewayBindAddresses = map[string]*structs.ConsulGatewayBindAddress{
-				Address: "0.0.0.0",
+				"default": {
-				Port:    -1, // filled in later with dynamic port
+					Address: "0.0.0.0",
-			}}
+					Port:    -1, // filled in later with dynamic port
-	case gateway.Mesh != nil:
+				}}
-		proxy.EnvoyGatewayNoDefaultBind = true
+		case gateway.Mesh != nil:
-		proxy.EnvoyGatewayBindTaggedAddresses = false
+			proxy.EnvoyGatewayNoDefaultBind = true
-		proxy.EnvoyGatewayBindAddresses = map[string]*structs.ConsulGatewayBindAddress{
+			proxy.EnvoyGatewayBindTaggedAddresses = false
-			"wan": {
+			proxy.EnvoyGatewayBindAddresses = map[string]*structs.ConsulGatewayBindAddress{
-				Address: "0.0.0.0",
+				"wan": {
-				Port:    -1, // filled in later with configured port
+					Address: "0.0.0.0",
-			},
+					Port:    -1, // filled in later with configured port
-			"lan": {
+				},
-				Address: "0.0.0.0",
+				"lan": {
-				Port:    -1, // filled in later with generated port
+					Address: "0.0.0.0",
-			},
+					Port:    -1, // filled in later with generated port
+				},
+			}
 		}
 	}
 
 	return proxy
 }
 
-func gatewayBindAddressesIngress(ingress *structs.ConsulIngressConfigEntry) map[string]*structs.ConsulGatewayBindAddress {
+func gatewayBindAddressesIngressForBridge(ingress *structs.ConsulIngressConfigEntry) map[string]*structs.ConsulGatewayBindAddress {
 	if ingress == nil || len(ingress.Listeners) == 0 {
 		return make(map[string]*structs.ConsulGatewayBindAddress)
 	}

nomad/job_endpoint_hook_connect_test.go

@@ -112,7 +112,7 @@ func TestJobEndpointConnect_groupConnectHook(t *testing.T) {
 	require.Exactly(t, tgExp, job.TaskGroups[0])
 }
 
-func TestJobEndpointConnect_groupConnectHook_IngressGateway(t *testing.T) {
+func TestJobEndpointConnect_groupConnectHook_IngressGateway_BridgeNetwork(t *testing.T) {
 	t.Parallel()
 
 	// Test that the connect ingress gateway task is inserted if a gateway service
@@ -135,7 +135,39 @@ func TestJobEndpointConnect_groupConnectHook_IngressGateway(t *testing.T) {
 	expTG.Networks[0].Canonicalize()
 
 	// rewrite the service gateway proxy configuration
-	expTG.Services[0].Connect.Gateway.Proxy = gatewayProxyForBridge(expTG.Services[0].Connect.Gateway)
+	expTG.Services[0].Connect.Gateway.Proxy = gatewayProxy(expTG.Services[0].Connect.Gateway, "bridge")
+
+	require.NoError(t, groupConnectHook(job, job.TaskGroups[0]))
+	require.Exactly(t, expTG, job.TaskGroups[0])
+
+	// Test that the hook is idempotent
+	require.NoError(t, groupConnectHook(job, job.TaskGroups[0]))
+	require.Exactly(t, expTG, job.TaskGroups[0])
+}
+
+func TestJobEndpointConnect_groupConnectHook_IngressGateway_HostNetwork(t *testing.T) {
+	t.Parallel()
+
+	// Test that the connect ingress gateway task is inserted if a gateway service
+	// exists. In host network mode, the default values are used.
+	job := mock.ConnectIngressGatewayJob("host", false)
+	job.Meta = map[string]string{
+		"gateway_name": "my-gateway",
+	}
+	job.TaskGroups[0].Services[0].Name = "${NOMAD_META_gateway_name}"
+
+	// setup expectations
+	expTG := job.TaskGroups[0].Copy()
+	expTG.Tasks = []*structs.Task{
+		// inject the gateway task
+		newConnectGatewayTask(structs.ConnectIngressPrefix, "my-gateway", true),
+	}
+	expTG.Services[0].Name = "my-gateway"
+	expTG.Tasks[0].Canonicalize(job, expTG)
+	expTG.Networks[0].Canonicalize()
+
+	// rewrite the service gateway proxy configuration
+	expTG.Services[0].Connect.Gateway.Proxy = gatewayProxy(expTG.Services[0].Connect.Gateway, "host")
 
 	require.NoError(t, groupConnectHook(job, job.TaskGroups[0]))
 	require.Exactly(t, expTG, job.TaskGroups[0])
@@ -204,7 +236,7 @@ func TestJobEndpointConnect_groupConnectHook_IngressGateway_CustomTask(t *testin
 	expTG.Networks[0].Canonicalize()
 
 	// rewrite the service gateway proxy configuration
-	expTG.Services[0].Connect.Gateway.Proxy = gatewayProxyForBridge(expTG.Services[0].Connect.Gateway)
+	expTG.Services[0].Connect.Gateway.Proxy = gatewayProxy(expTG.Services[0].Connect.Gateway, "bridge")
 
 	require.NoError(t, groupConnectHook(job, job.TaskGroups[0]))
 	require.Exactly(t, expTG, job.TaskGroups[0])
@@ -237,7 +269,7 @@ func TestJobEndpointConnect_groupConnectHook_TerminatingGateway(t *testing.T) {
 	expTG.Networks[0].Canonicalize()
 
 	// rewrite the service gateway proxy configuration
-	expTG.Services[0].Connect.Gateway.Proxy = gatewayProxyForBridge(expTG.Services[0].Connect.Gateway)
+	expTG.Services[0].Connect.Gateway.Proxy = gatewayProxy(expTG.Services[0].Connect.Gateway, "bridge")
 
 	require.NoError(t, groupConnectHook(job, job.TaskGroups[0]))
 	require.Exactly(t, expTG, job.TaskGroups[0])
@@ -278,7 +310,7 @@ func TestJobEndpointConnect_groupConnectHook_MeshGateway(t *testing.T) {
 	expTG.Networks[0].Canonicalize()
 
 	// rewrite the service gateway proxy configuration
-	expTG.Services[0].Connect.Gateway.Proxy = gatewayProxyForBridge(expTG.Services[0].Connect.Gateway)
+	expTG.Services[0].Connect.Gateway.Proxy = gatewayProxy(expTG.Services[0].Connect.Gateway, "bridge")
 
 	require.NoError(t, groupConnectHook(job, job.TaskGroups[0]))
 	require.Exactly(t, expTG, job.TaskGroups[0])
@@ -692,22 +724,22 @@ func TestJobEndpointConnect_gatewayProxyIsDefault(t *testing.T) {
 	})
 }
 
-func TestJobEndpointConnect_gatewayBindAddresses(t *testing.T) {
+func TestJobEndpointConnect_gatewayBindAddressesForBridge(t *testing.T) {
 	t.Parallel()
 
 	t.Run("nil", func(t *testing.T) {
 
-		result := gatewayBindAddressesIngress(nil)
+		result := gatewayBindAddressesIngressForBridge(nil)
 		require.Empty(t, result)
 	})
 
 	t.Run("no listeners", func(t *testing.T) {
-		result := gatewayBindAddressesIngress(&structs.ConsulIngressConfigEntry{Listeners: nil})
+		result := gatewayBindAddressesIngressForBridge(&structs.ConsulIngressConfigEntry{Listeners: nil})
 		require.Empty(t, result)
 	})
 
 	t.Run("simple", func(t *testing.T) {
-		result := gatewayBindAddressesIngress(&structs.ConsulIngressConfigEntry{
+		result := gatewayBindAddressesIngressForBridge(&structs.ConsulIngressConfigEntry{
 			Listeners: []*structs.ConsulIngressListener{{
 				Port:     3000,
 				Protocol: "tcp",
@@ -725,7 +757,7 @@ func TestJobEndpointConnect_gatewayBindAddresses(t *testing.T) {
 	})
 
 	t.Run("complex", func(t *testing.T) {
-		result := gatewayBindAddressesIngress(&structs.ConsulIngressConfigEntry{
+		result := gatewayBindAddressesIngressForBridge(&structs.ConsulIngressConfigEntry{
 			Listeners: []*structs.ConsulIngressListener{{
 				Port:     3000,
 				Protocol: "tcp",
@@ -759,16 +791,16 @@ func TestJobEndpointConnect_gatewayBindAddresses(t *testing.T) {
 	})
 }
 
-func TestJobEndpointConnect_gatewayProxyForBridge(t *testing.T) {
+func TestJobEndpointConnect_gatewayProxy(t *testing.T) {
 	t.Parallel()
 
 	t.Run("nil", func(t *testing.T) {
-		result := gatewayProxyForBridge(nil)
+		result := gatewayProxy(nil, "bridge")
 		require.Nil(t, result)
 	})
 
 	t.Run("nil proxy", func(t *testing.T) {
-		result := gatewayProxyForBridge(&structs.ConsulGateway{
+		result := gatewayProxy(&structs.ConsulGateway{
 			Ingress: &structs.ConsulIngressConfigEntry{
 				Listeners: []*structs.ConsulIngressListener{{
 					Port:     3000,
@@ -778,7 +810,7 @@ func TestJobEndpointConnect_gatewayProxyForBridge(t *testing.T) {
 					}},
 				}},
 			},
-		})
+		}, "bridge")
 		require.Equal(t, &structs.ConsulGatewayProxy{
 			ConnectTimeout:                  helper.TimeToPtr(defaultConnectTimeout),
 			EnvoyGatewayNoDefaultBind:       true,
@@ -792,7 +824,7 @@ func TestJobEndpointConnect_gatewayProxyForBridge(t *testing.T) {
 	})
 
 	t.Run("ingress set defaults", func(t *testing.T) {
-		result := gatewayProxyForBridge(&structs.ConsulGateway{
+		result := gatewayProxy(&structs.ConsulGateway{
 			Proxy: &structs.ConsulGatewayProxy{
 				ConnectTimeout: helper.TimeToPtr(2 * time.Second),
 				Config:         map[string]interface{}{"foo": 1},
@@ -806,7 +838,7 @@ func TestJobEndpointConnect_gatewayProxyForBridge(t *testing.T) {
 					}},
 				}},
 			},
-		})
+		}, "bridge")
 		require.Equal(t, &structs.ConsulGatewayProxy{
 			ConnectTimeout:                  helper.TimeToPtr(2 * time.Second),
 			Config:                          map[string]interface{}{"foo": 1},
@@ -821,7 +853,7 @@ func TestJobEndpointConnect_gatewayProxyForBridge(t *testing.T) {
 	})
 
 	t.Run("ingress leave as-is", func(t *testing.T) {
-		result := gatewayProxyForBridge(&structs.ConsulGateway{
+		result := gatewayProxy(&structs.ConsulGateway{
 			Proxy: &structs.ConsulGatewayProxy{
 				Config:                          map[string]interface{}{"foo": 1},
 				EnvoyGatewayBindTaggedAddresses: true,
@@ -835,7 +867,7 @@ func TestJobEndpointConnect_gatewayProxyForBridge(t *testing.T) {
 					}},
 				}},
 			},
-		})
+		}, "bridge")
 		require.Equal(t, &structs.ConsulGatewayProxy{
 			ConnectTimeout:                  nil,
 			Config:                          map[string]interface{}{"foo": 1},
@@ -846,7 +878,7 @@ func TestJobEndpointConnect_gatewayProxyForBridge(t *testing.T) {
 	})
 
 	t.Run("terminating set defaults", func(t *testing.T) {
-		result := gatewayProxyForBridge(&structs.ConsulGateway{
+		result := gatewayProxy(&structs.ConsulGateway{
 			Proxy: &structs.ConsulGatewayProxy{
 				ConnectTimeout:        helper.TimeToPtr(2 * time.Second),
 				EnvoyDNSDiscoveryType: "STRICT_DNS",
@@ -860,7 +892,7 @@ func TestJobEndpointConnect_gatewayProxyForBridge(t *testing.T) {
 					SNI:      "",
 				}},
 			},
-		})
+		}, "bridge")
 		require.Equal(t, &structs.ConsulGatewayProxy{
 			ConnectTimeout:                  helper.TimeToPtr(2 * time.Second),
 			EnvoyGatewayNoDefaultBind:       true,
@@ -876,7 +908,7 @@ func TestJobEndpointConnect_gatewayProxyForBridge(t *testing.T) {
 	})
 
 	t.Run("terminating leave as-is", func(t *testing.T) {
-		result := gatewayProxyForBridge(&structs.ConsulGateway{
+		result := gatewayProxy(&structs.ConsulGateway{
 			Proxy: &structs.ConsulGatewayProxy{
 				Config:                          map[string]interface{}{"foo": 1},
 				EnvoyGatewayBindTaggedAddresses: true,
@@ -886,7 +918,7 @@ func TestJobEndpointConnect_gatewayProxyForBridge(t *testing.T) {
 					Name: "service1",
 				}},
 			},
-		})
+		}, "bridge")
 		require.Equal(t, &structs.ConsulGatewayProxy{
 			ConnectTimeout:                  nil,
 			Config:                          map[string]interface{}{"foo": 1},
@@ -896,15 +928,15 @@ func TestJobEndpointConnect_gatewayProxyForBridge(t *testing.T) {
 		}, result)
 	})
 
-	t.Run("mesh set defaults", func(t *testing.T) {
+	t.Run("mesh set defaults in bridge", func(t *testing.T) {
-		result := gatewayProxyForBridge(&structs.ConsulGateway{
+		result := gatewayProxy(&structs.ConsulGateway{
 			Proxy: &structs.ConsulGatewayProxy{
 				ConnectTimeout: helper.TimeToPtr(2 * time.Second),
 			},
 			Mesh: &structs.ConsulMeshConfigEntry{
 				// nothing
 			},
-		})
+		}, "bridge")
 		require.Equal(t, &structs.ConsulGatewayProxy{
 			ConnectTimeout:                  helper.TimeToPtr(2 * time.Second),
 			EnvoyGatewayNoDefaultBind:       true,
@@ -922,8 +954,22 @@ func TestJobEndpointConnect_gatewayProxyForBridge(t *testing.T) {
 		}, result)
 	})
 
+	t.Run("mesh set defaults in host", func(t *testing.T) {
+		result := gatewayProxy(&structs.ConsulGateway{
+			Proxy: &structs.ConsulGatewayProxy{
+				ConnectTimeout: helper.TimeToPtr(2 * time.Second),
+			},
+			Mesh: &structs.ConsulMeshConfigEntry{
+				// nothing
+			},
+		}, "host")
+		require.Equal(t, &structs.ConsulGatewayProxy{
+			ConnectTimeout: helper.TimeToPtr(2 * time.Second),
+		}, result)
+	})
+
 	t.Run("mesh leave as-is", func(t *testing.T) {
-		result := gatewayProxyForBridge(&structs.ConsulGateway{
+		result := gatewayProxy(&structs.ConsulGateway{
 			Proxy: &structs.ConsulGatewayProxy{
 				Config:                          map[string]interface{}{"foo": 1},
 				EnvoyGatewayBindTaggedAddresses: true,
@@ -931,7 +977,7 @@ func TestJobEndpointConnect_gatewayProxyForBridge(t *testing.T) {
 			Mesh: &structs.ConsulMeshConfigEntry{
 				// nothing
 			},
-		})
+		}, "bridge")
 		require.Equal(t, &structs.ConsulGatewayProxy{
 			ConnectTimeout:                  nil,
 			Config:                          map[string]interface{}{"foo": 1},