connect: enable automatic expose paths for individual group service checks

Part of #6120

Building on the support for enabling connect proxy paths in #7323, this change
adds the ability to configure the 'service.check.expose' flag on group-level
service check definitions for services that are connect-enabled. This is a slight
deviation from the "magic" that Consul provides. With Consul, the 'expose' flag
exists on the connect.proxy stanza, which will then auto-generate expose paths
for every HTTP and gRPC service check associated with that connect-enabled
service.

A first attempt at providing similar magic for Nomad's Consul Connect integration
followed that pattern exactly, as seen in #7396. However, on reviewing the PR
we realized having the `expose` flag on the proxy stanza inseperably ties together
the automatic path generation with every HTTP/gRPC defined on the service. This
makes sense in Consul's context, because a service definition is reasonably
associated with a single "task". With Nomad's group level service definitions
however, there is a reasonable expectation that a service definition is more
abstractly representative of multiple services within the task group. In this
case, one would want to define checks of that service which concretely make HTTP
or gRPC requests to different underlying tasks. Such a model is not possible
with the course `proxy.expose` flag.

Instead, we now have the flag made available within the check definitions themselves.
By making the expose feature resolute to each check, it is possible to have
some HTTP/gRPC checks which make use of the envoy exposed paths, as well as
some HTTP/gRPC checks which make use of some orthongonal port-mapping to do
checks on some other task (or even some other bound port of the same task)
within the task group.

Given this example,

group "server-group" {
  network {
    mode = "bridge"
    port "forchecks" {
      to = -1
    }
  }

  service {
    name = "myserver"
    port = 2000

    connect {
      sidecar_service {
      }
    }

    check {
      name     = "mycheck-myserver"
      type     = "http"
      port     = "forchecks"
      interval = "3s"
      timeout  = "2s"
      method   = "GET"
      path     = "/classic/responder/health"
      expose   = true
    }
  }
}

Nomad will automatically inject (via job endpoint mutator) the
extrapolated expose path configuration, i.e.

expose {
  path {
    path            = "/classic/responder/health"
    protocol        = "http"
    local_path_port = 2000
    listener_port   = "forchecks"
  }
}

Documentation is coming in #7440 (needs updating, doing next)

Modifications to the `countdash` examples in https://github.com/hashicorp/demo-consul-101/pull/6
which will make the examples in the documentation actually runnable.

Will add some e2e tests based on the above when it becomes available.

api/services.go

@@ -81,6 +81,7 @@ type ServiceCheck struct {
 	Path          string
 	Protocol      string
 	PortLabel     string `mapstructure:"port"`
+	Expose        bool
 	AddressMode   string `mapstructure:"address_mode"`
 	Interval      time.Duration
 	Timeout       time.Duration
@@ -183,7 +184,6 @@ type ConsulUpstream struct {
 
 type ConsulExposeConfig struct {
 	Path []*ConsulExposePath `mapstructure:"path"`
-	// todo(shoenig): add magic for 'checks' option
 }
 
 type ConsulExposePath struct {

command/agent/job_endpoint.go

@@ -1151,6 +1151,7 @@ func ApiServicesToStructs(in []*api.Service) []*structs.Service {
 					Path:          check.Path,
 					Protocol:      check.Protocol,
 					PortLabel:     check.PortLabel,
+					Expose:        check.Expose,
 					AddressMode:   check.AddressMode,
 					Interval:      check.Interval,
 					Timeout:       check.Timeout,

jobspec/parse_service.go

@@ -405,7 +405,6 @@ func parseProxy(o *ast.ObjectItem) (*api.ConsulProxy, error) {
 func parseExpose(eo *ast.ObjectItem) (*api.ConsulExposeConfig, error) {
 	valid := []string{
 		"path", // an array of path blocks
-		// todo(shoenig) checks boolean
 	}
 
 	if err := helper.CheckHCLKeys(eo.Val, valid); err != nil {
@@ -514,6 +513,7 @@ func parseChecks(service *api.Service, checkObjs *ast.ObjectList) error {
 			"path",
 			"protocol",
 			"port",
+			"expose",
 			"command",
 			"args",
 			"initial_status",

jobspec/parse_test.go

@@ -1150,6 +1150,32 @@ func TestParse(t *testing.T) {
 			},
 			false,
 		},
+		{
+			"tg-service-check-expose.hcl",
+			&api.Job{
+				ID:   helper.StringToPtr("group_service_proxy_expose"),
+				Name: helper.StringToPtr("group_service_proxy_expose"),
+				TaskGroups: []*api.TaskGroup{{
+					Name: helper.StringToPtr("group"),
+					Services: []*api.Service{{
+						Name: "example",
+						Connect: &api.ConsulConnect{
+							SidecarService: &api.ConsulSidecarService{
+								Proxy: &api.ConsulProxy{},
+							},
+						},
+						Checks: []api.ServiceCheck{{
+							Name:   "example-check1",
+							Expose: true,
+						}, {
+							Name:   "example-check2",
+							Expose: false,
+						}},
+					}},
+				}},
+			},
+			false,
+		},
 		{
 			"tg-service-enable-tag-override.hcl",
 			&api.Job{

jobspec/test-fixtures/tg-service-check-expose.hcl

@@ -0,0 +1,23 @@
+job "group_service_proxy_expose" {
+  group "group" {
+    service {
+      name = "example"
+      connect {
+        sidecar_service {
+          proxy {
+          }
+        }
+      }
+
+      check {
+        name = "example-check1"
+        expose = true
+      }
+
+      check {
+        name = "example-check2"
+        expose = false
+      }
+    }
+  }
+}

nomad/job_endpoint.go

@@ -61,10 +61,12 @@ func NewJobEndpoints(s *Server) *Job {
 		mutators: []jobMutator{
 			jobCanonicalizer{},
 			jobConnectHook{},
+			jobExposeCheckHook{},
 			jobImpliedConstraints{},
 		},
 		validators: []jobValidator{
 			jobConnectHook{},
+			jobExposeCheckHook{},
 			jobValidate{},
 		},
 	}

nomad/job_endpoint_hook_expose_check.go

@@ -0,0 +1,228 @@
+package nomad
+
+import (
+	"strconv"
+	"strings"
+
+	"github.com/hashicorp/nomad/nomad/structs"
+	"github.com/pkg/errors"
+)
+
+type jobExposeCheckHook struct{}
+
+func (jobExposeCheckHook) Name() string {
+	return "expose-check"
+}
+
+// Mutate will scan every task group for group-services which have checks defined
+// that have the Expose field configured, and generate expose path configurations
+// extrapolated from those check definitions.
+func (jobExposeCheckHook) Mutate(job *structs.Job) (_ *structs.Job, warnings []error, err error) {
+	for _, tg := range job.TaskGroups {
+		for _, s := range tg.Services {
+			for _, c := range s.Checks {
+				if c.Expose {
+					if exposePath, err := exposePathForCheck(tg, s, c); err != nil {
+						return nil, nil, err
+					} else if exposePath != nil {
+						serviceExposeConfig := serviceExposeConfig(s)
+						// insert only if not already present - required for job
+						// updates which would otherwise create duplicates
+						if !containsExposePath(serviceExposeConfig.Paths, *exposePath) {
+							serviceExposeConfig.Paths = append(
+								serviceExposeConfig.Paths, *exposePath,
+							)
+						}
+					}
+				}
+			}
+		}
+	}
+	return job, nil, nil
+}
+
+// Validate will ensure:
+// - The job contains valid network configuration for each task group in which
+//	an expose path is configured. The network must be of type bridge mode.
+// - The check Expose field is configured only for connect-enabled group-services.
+func (jobExposeCheckHook) Validate(job *structs.Job) (warnings []error, err error) {
+	for _, tg := range job.TaskGroups {
+		// Make sure any group that contains a group-service that enables expose
+		// is configured with one network that is in "bridge" mode. This check
+		// is being done independently of the preceding Connect task injection
+		// hook, because at some point in the future Connect will not require the
+		// use of network namespaces, whereas the use of "expose" does not make
+		// sense without the use of network namespace.
+		if err := tgValidateUseOfBridgeMode(tg); err != nil {
+			return nil, err
+		}
+		// Make sure any group-service that contains a check that enables expose
+		// is connect-enabled and does not specify a custom sidecar task. We only
+		// support the expose feature when using the built-in Envoy integration.
+		if err := tgValidateUseOfCheckExpose(tg); err != nil {
+			return nil, err
+		}
+	}
+	return nil, nil
+}
+
+// serviceExposeConfig digs through s to extract the connect sidecar service proxy
+// expose configuration. It is not required of the user to provide this, so it
+// is created on demand here as needed in the case where any service check exposes
+// itself.
+//
+// The service, connect, and sidecar_service are assumed not to be nil, as they
+// are enforced in previous hooks / validation.
+func serviceExposeConfig(s *structs.Service) *structs.ConsulExposeConfig {
+	if s.Connect.SidecarService.Proxy == nil {
+		s.Connect.SidecarService.Proxy = new(structs.ConsulProxy)
+	}
+	if s.Connect.SidecarService.Proxy.Expose == nil {
+		s.Connect.SidecarService.Proxy.Expose = new(structs.ConsulExposeConfig)
+	}
+	return s.Connect.SidecarService.Proxy.Expose
+}
+
+// containsExposePath returns true if path is contained in paths.
+func containsExposePath(paths []structs.ConsulExposePath, path structs.ConsulExposePath) bool {
+	for _, p := range paths {
+		if p == path {
+			return true
+		}
+	}
+	return false
+}
+
+// tgValidateUseOfCheckExpose ensures that any service check in tg making use
+// of the expose field is within an appropriate context to do so. The check must
+// be a group level check, and must use the builtin envoy proxy.
+func tgValidateUseOfCheckExpose(tg *structs.TaskGroup) error {
+	// validation for group services (which must use built-in connect proxy)
+	for _, s := range tg.Services {
+		for _, check := range s.Checks {
+			if check.Expose && !serviceUsesConnectEnvoy(s) {
+				return errors.Errorf(
+					"exposed service check %s->%s->%s requires use of Nomad's builtin Connect proxy",
+					tg.Name, s.Name, check.Name,
+				)
+			}
+		}
+	}
+
+	// validation for task services (which must not be configured to use Expose)
+	for _, t := range tg.Tasks {
+		for _, s := range t.Services {
+			for _, check := range s.Checks {
+				if check.Expose {
+					return errors.Errorf(
+						"exposed service check %s[%s]->%s->%s is not a task-group service",
+						tg.Name, t.Name, s.Name, check.Name,
+					)
+				}
+			}
+		}
+	}
+	return nil
+}
+
+// tgValidateUseOfBridgeMode ensures there is exactly 1 network configured for
+// the task group, and that it makes use of "bridge" mode (i.e. enables network
+// namespaces).
+func tgValidateUseOfBridgeMode(tg *structs.TaskGroup) error {
+	if tgUsesExposeCheck(tg) {
+		if len(tg.Networks) != 1 {
+			return errors.Errorf("group %q must specify one bridge network for exposing service check(s)", tg.Name)
+		}
+		if tg.Networks[0].Mode != "bridge" {
+			return errors.Errorf("group %q must use bridge network for exposing service check(s)", tg.Name)
+		}
+	}
+	return nil
+}
+
+// tgUsesExposeCheck returns true if any group service in the task group makes
+// use of the expose field.
+func tgUsesExposeCheck(tg *structs.TaskGroup) bool {
+	for _, s := range tg.Services {
+		for _, check := range s.Checks {
+			if check.Expose {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// serviceUsesConnectEnvoy returns true if the service is going to end up using
+// the built-in envoy proxy.
+//
+// This implementation is kind of reading tea leaves - firstly Connect
+// must be enabled, and second the sidecar_task must not be overridden. If these
+// conditions are met, the preceding connect hook will have injected a Connect
+// sidecar task, the configuration of which is interpolated at runtime.
+func serviceUsesConnectEnvoy(s *structs.Service) bool {
+	// A non-nil connect stanza implies this service isn't connect enabled in
+	// the first place.
+	if s.Connect == nil {
+		return false
+	}
+
+	// A non-nil connect.sidecar_task stanza implies the sidecar task is being
+	// overridden (i.e. the default Envoy is not being uesd).
+	if s.Connect.SidecarTask != nil {
+		return false
+	}
+
+	return true
+}
+
+// checkIsExposable returns true if check is qualified for automatic generation
+// of connect proxy expose path configuration based on configured consul checks.
+// To qualify, the check must be of type "http" or "grpc", and must have a Path
+// configured.
+func checkIsExposable(check *structs.ServiceCheck) bool {
+	switch strings.ToLower(check.Type) {
+	case "grpc", "http":
+		return strings.HasPrefix(check.Path, "/")
+	default:
+		return false
+	}
+}
+
+// exposePathForCheck extrapolates the necessary expose path configuration for
+// the given consul service check. If the check is not compatible, nil is
+// returned.
+func exposePathForCheck(tg *structs.TaskGroup, s *structs.Service, check *structs.ServiceCheck) (*structs.ConsulExposePath, error) {
+	if !checkIsExposable(check) {
+		return nil, nil
+	}
+
+	// Determine the local service port (i.e. what port the service is actually
+	// listening to inside the network namespace).
+	//
+	// Similar logic exists in getAddress of client.go which is used for
+	// creating check & service registration objects.
+	//
+	// The difference here is the address is predestined to be localhost since
+	// it is binding inside the namespace.
+	var port int
+	if _, port = tg.Networks.Port(s.PortLabel); port <= 0 { // try looking up by port label
+		if port, _ = strconv.Atoi(s.PortLabel); port <= 0 { // then try direct port value
+			return nil, errors.Errorf(
+				"unable to determine local service port for service check %s->%s->%s",
+				tg.Name, s.Name, check.Name,
+			)
+		}
+	}
+
+	// The Path, Protocol, and PortLabel are just copied over from the service
+	// check definition. It is required that the user configure their own port
+	// mapping for each check, including setting the 'to = -1' sentinel value
+	// enabling the network namespace pass-through.
+	return &structs.ConsulExposePath{
+		Path:          check.Path,
+		Protocol:      check.Protocol,
+		LocalPathPort: port,
+		ListenerPort:  check.PortLabel,
+	}, nil
+}

nomad/job_endpoint_hook_expose_check_test.go

@@ -0,0 +1,598 @@
+package nomad
+
+import (
+	"testing"
+
+	"github.com/hashicorp/nomad/nomad/structs"
+	"github.com/stretchr/testify/require"
+)
+
+func TestJobExposeCheckHook_Name(t *testing.T) {
+	t.Parallel()
+
+	require.Equal(t, "expose-check", new(jobExposeCheckHook).Name())
+}
+
+func TestJobExposeCheckHook_serviceUsesConnectEnvoy(t *testing.T) {
+	t.Parallel()
+
+	t.Run("connect is nil", func(t *testing.T) {
+		require.False(t, serviceUsesConnectEnvoy(&structs.Service{
+			Connect: nil,
+		}))
+	})
+
+	t.Run("sidecar-task is overridden", func(t *testing.T) {
+		require.False(t, serviceUsesConnectEnvoy(&structs.Service{
+			Connect: &structs.ConsulConnect{
+				SidecarTask: &structs.SidecarTask{
+					Name: "my-sidecar",
+				},
+			},
+		}))
+	})
+
+	t.Run("sidecar-task is nil", func(t *testing.T) {
+		require.True(t, serviceUsesConnectEnvoy(&structs.Service{
+			Connect: &structs.ConsulConnect{
+				SidecarTask: nil,
+			},
+		}))
+	})
+}
+
+func TestJobExposeCheckHook_tgUsesExposeCheck(t *testing.T) {
+	t.Parallel()
+
+	t.Run("no check.expose", func(t *testing.T) {
+		require.False(t, tgUsesExposeCheck(&structs.TaskGroup{
+			Services: []*structs.Service{{
+				Checks: []*structs.ServiceCheck{{
+					Expose: false,
+				}},
+			}},
+		}))
+	})
+
+	t.Run("with check.expose", func(t *testing.T) {
+		require.True(t, tgUsesExposeCheck(&structs.TaskGroup{
+			Services: []*structs.Service{{
+				Checks: []*structs.ServiceCheck{{
+					Expose: false,
+				}, {
+					Expose: true,
+				}},
+			}},
+		}))
+	})
+}
+
+func TestJobExposeCheckHook_tgValidateUseOfBridgeMode(t *testing.T) {
+	t.Parallel()
+
+	s1 := &structs.Service{
+		Name: "s1",
+		Checks: []*structs.ServiceCheck{{
+			Name:      "s1-check1",
+			Type:      "http",
+			PortLabel: "health",
+			Expose:    true,
+		}},
+	}
+
+	t.Run("no networks but no use of expose", func(t *testing.T) {
+		require.Nil(t, tgValidateUseOfBridgeMode(&structs.TaskGroup{
+			Networks: make(structs.Networks, 0),
+		}))
+	})
+
+	t.Run("no networks and uses expose", func(t *testing.T) {
+		require.EqualError(t, tgValidateUseOfBridgeMode(&structs.TaskGroup{
+			Name:     "g1",
+			Networks: make(structs.Networks, 0),
+			Services: []*structs.Service{s1},
+		}), `group "g1" must specify one bridge network for exposing service check(s)`)
+	})
+
+	t.Run("non-bridge network and uses expose", func(t *testing.T) {
+		require.EqualError(t, tgValidateUseOfBridgeMode(&structs.TaskGroup{
+			Name: "g1",
+			Networks: structs.Networks{{
+				Mode: "host",
+			}},
+			Services: []*structs.Service{s1},
+		}), `group "g1" must use bridge network for exposing service check(s)`)
+	})
+
+	t.Run("bridge network uses expose", func(t *testing.T) {
+		require.Nil(t, tgValidateUseOfBridgeMode(&structs.TaskGroup{
+			Name: "g1",
+			Networks: structs.Networks{{
+				Mode: "bridge",
+			}},
+			Services: []*structs.Service{s1},
+		}))
+	})
+}
+
+func TestJobExposeCheckHook_tgValidateUseOfCheckExpose(t *testing.T) {
+	t.Parallel()
+
+	withCustomProxyTask := &structs.Service{
+		Name: "s1",
+		Connect: &structs.ConsulConnect{
+			SidecarTask: &structs.SidecarTask{Name: "custom"},
+		},
+		Checks: []*structs.ServiceCheck{{
+			Name:      "s1-check1",
+			Type:      "http",
+			PortLabel: "health",
+			Expose:    true,
+		}},
+	}
+
+	t.Run("group-service uses custom proxy", func(t *testing.T) {
+		require.EqualError(t, tgValidateUseOfCheckExpose(&structs.TaskGroup{
+			Name:     "g1",
+			Services: []*structs.Service{withCustomProxyTask},
+		}), `exposed service check g1->s1->s1-check1 requires use of Nomad's builtin Connect proxy`)
+	})
+
+	t.Run("group-service uses custom proxy but no expose", func(t *testing.T) {
+		withCustomProxyTaskNoExpose := &(*withCustomProxyTask)
+		withCustomProxyTask.Checks[0].Expose = false
+		require.Nil(t, tgValidateUseOfCheckExpose(&structs.TaskGroup{
+			Name:     "g1",
+			Services: []*structs.Service{withCustomProxyTaskNoExpose},
+		}))
+	})
+
+	t.Run("task-service sets expose", func(t *testing.T) {
+		require.EqualError(t, tgValidateUseOfCheckExpose(&structs.TaskGroup{
+			Name: "g1",
+			Tasks: []*structs.Task{{
+				Name: "t1",
+				Services: []*structs.Service{{
+					Name: "s2",
+					Checks: []*structs.ServiceCheck{{
+						Name:   "check1",
+						Type:   "http",
+						Expose: true,
+					}},
+				}},
+			}},
+		}), `exposed service check g1[t1]->s2->check1 is not a task-group service`)
+	})
+}
+
+func TestJobExposeCheckHook_Validate(t *testing.T) {
+	s1 := &structs.Service{
+		Name: "s1",
+		Checks: []*structs.ServiceCheck{{
+			Name:   "s1-check1",
+			Type:   "http",
+			Expose: true,
+		}},
+	}
+
+	t.Run("double network", func(t *testing.T) {
+		warnings, err := new(jobExposeCheckHook).Validate(&structs.Job{
+			TaskGroups: []*structs.TaskGroup{{
+				Name: "g1",
+				Networks: structs.Networks{{
+					Mode: "bridge",
+				}, {
+					Mode: "bridge",
+				}},
+				Services: []*structs.Service{s1},
+			}},
+		})
+		require.Empty(t, warnings)
+		require.EqualError(t, err, `group "g1" must specify one bridge network for exposing service check(s)`)
+	})
+
+	t.Run("expose in service check", func(t *testing.T) {
+		warnings, err := new(jobExposeCheckHook).Validate(&structs.Job{
+			TaskGroups: []*structs.TaskGroup{{
+				Name: "g1",
+				Networks: structs.Networks{{
+					Mode: "bridge",
+				}},
+				Tasks: []*structs.Task{{
+					Name: "t1",
+					Services: []*structs.Service{{
+						Name: "s2",
+						Checks: []*structs.ServiceCheck{{
+							Name:   "s2-check1",
+							Type:   "http",
+							Expose: true,
+						}},
+					}},
+				}},
+			}},
+		})
+		require.Empty(t, warnings)
+		require.EqualError(t, err, `exposed service check g1[t1]->s2->s2-check1 is not a task-group service`)
+	})
+
+	t.Run("ok", func(t *testing.T) {
+		warnings, err := new(jobExposeCheckHook).Validate(&structs.Job{
+			TaskGroups: []*structs.TaskGroup{{
+				Name: "g1",
+				Networks: structs.Networks{{
+					Mode: "bridge",
+				}},
+				Services: []*structs.Service{{
+					Name:    "s1",
+					Connect: &structs.ConsulConnect{},
+					Checks: []*structs.ServiceCheck{{
+						Name:   "check1",
+						Type:   "http",
+						Expose: true,
+					}},
+				}},
+				Tasks: []*structs.Task{{
+					Name: "t1",
+					Services: []*structs.Service{{
+						Name: "s2",
+						Checks: []*structs.ServiceCheck{{
+							Name:   "s2-check1",
+							Type:   "http",
+							Expose: false,
+						}},
+					}},
+				}},
+			}},
+		})
+		require.Empty(t, warnings)
+		require.Nil(t, err)
+	})
+}
+
+func TestJobExposeCheckHook_exposePathForCheck(t *testing.T) {
+	t.Parallel()
+
+	t.Run("not expose compatible", func(t *testing.T) {
+		c := &structs.ServiceCheck{
+			Type: "tcp", // not expose compatible
+		}
+		s := &structs.Service{
+			Checks: []*structs.ServiceCheck{c},
+		}
+		ePath, err := exposePathForCheck(&structs.TaskGroup{
+			Services: []*structs.Service{s},
+		}, s, c)
+		require.NoError(t, err)
+		require.Nil(t, ePath)
+	})
+
+	t.Run("direct port", func(t *testing.T) {
+		c := &structs.ServiceCheck{
+			Name:      "check1",
+			Type:      "http",
+			Path:      "/health",
+			PortLabel: "hcPort",
+		}
+		s := &structs.Service{
+			Name:      "service1",
+			PortLabel: "4000",
+			Checks:    []*structs.ServiceCheck{c},
+		}
+		ePath, err := exposePathForCheck(&structs.TaskGroup{
+			Name:     "group1",
+			Services: []*structs.Service{s},
+		}, s, c)
+		require.NoError(t, err)
+		require.Equal(t, &structs.ConsulExposePath{
+			Path:          "/health",
+			Protocol:      "", // often blank, consul does the Right Thing
+			LocalPathPort: 4000,
+			ListenerPort:  "hcPort",
+		}, ePath)
+	})
+
+	t.Run("labeled port", func(t *testing.T) {
+		c := &structs.ServiceCheck{
+			Name:      "check1",
+			Type:      "http",
+			Path:      "/health",
+			PortLabel: "hcPort",
+		}
+		s := &structs.Service{
+			Name:      "service1",
+			PortLabel: "sPort", // port label indirection
+			Checks:    []*structs.ServiceCheck{c},
+		}
+		ePath, err := exposePathForCheck(&structs.TaskGroup{
+			Name:     "group1",
+			Services: []*structs.Service{s},
+			Networks: structs.Networks{{
+				Mode: "bridge",
+				DynamicPorts: []structs.Port{
+					{Label: "sPort", Value: 4000},
+				},
+			}},
+		}, s, c)
+		require.NoError(t, err)
+		require.Equal(t, &structs.ConsulExposePath{
+			Path:          "/health",
+			Protocol:      "",
+			LocalPathPort: 4000,
+			ListenerPort:  "hcPort",
+		}, ePath)
+	})
+
+	t.Run("missing port", func(t *testing.T) {
+		c := &structs.ServiceCheck{
+			Name:      "check1",
+			Type:      "http",
+			Path:      "/health",
+			PortLabel: "hcPort",
+		}
+		s := &structs.Service{
+			Name:      "service1",
+			PortLabel: "sPort", // port label indirection
+			Checks:    []*structs.ServiceCheck{c},
+		}
+		_, err := exposePathForCheck(&structs.TaskGroup{
+			Name:     "group1",
+			Services: []*structs.Service{s},
+			Networks: structs.Networks{{
+				Mode:         "bridge",
+				DynamicPorts: []structs.Port{
+					// service declares "sPort", but does not exist
+				},
+			}},
+		}, s, c)
+		require.EqualError(t, err, `unable to determine local service port for service check group1->service1->check1`)
+	})
+}
+
+func TestJobExposeCheckHook_containsExposePath(t *testing.T) {
+	t.Parallel()
+
+	t.Run("contains path", func(t *testing.T) {
+		require.True(t, containsExposePath([]structs.ConsulExposePath{{
+			Path:          "/v2/health",
+			Protocol:      "grpc",
+			LocalPathPort: 8080,
+			ListenerPort:  "v2Port",
+		}, {
+			Path:          "/health",
+			Protocol:      "http",
+			LocalPathPort: 8080,
+			ListenerPort:  "hcPort",
+		}}, structs.ConsulExposePath{
+			Path:          "/health",
+			Protocol:      "http",
+			LocalPathPort: 8080,
+			ListenerPort:  "hcPort",
+		}))
+	})
+
+	t.Run("no such path", func(t *testing.T) {
+		require.False(t, containsExposePath([]structs.ConsulExposePath{{
+			Path:          "/v2/health",
+			Protocol:      "grpc",
+			LocalPathPort: 8080,
+			ListenerPort:  "v2Port",
+		}, {
+			Path:          "/health",
+			Protocol:      "http",
+			LocalPathPort: 8080,
+			ListenerPort:  "hcPort",
+		}}, structs.ConsulExposePath{
+			Path:          "/v3/health",
+			Protocol:      "http",
+			LocalPathPort: 8080,
+			ListenerPort:  "hcPort",
+		}))
+	})
+}
+
+func TestJobExposeCheckHook_serviceExposeConfig(t *testing.T) {
+	t.Parallel()
+
+	t.Run("proxy is nil", func(t *testing.T) {
+		require.NotNil(t, serviceExposeConfig(&structs.Service{
+			Connect: &structs.ConsulConnect{
+				SidecarService: &structs.ConsulSidecarService{},
+			},
+		}))
+	})
+
+	t.Run("expose is nil", func(t *testing.T) {
+		require.NotNil(t, serviceExposeConfig(&structs.Service{
+			Connect: &structs.ConsulConnect{
+				SidecarService: &structs.ConsulSidecarService{
+					Proxy: &structs.ConsulProxy{},
+				},
+			},
+		}))
+	})
+
+	t.Run("expose pre-existing", func(t *testing.T) {
+		exposeConfig := serviceExposeConfig(&structs.Service{
+			Connect: &structs.ConsulConnect{
+				SidecarService: &structs.ConsulSidecarService{
+					Proxy: &structs.ConsulProxy{
+						Expose: &structs.ConsulExposeConfig{
+							Paths: []structs.ConsulExposePath{{
+								Path: "/health",
+							}},
+						},
+					},
+				},
+			},
+		})
+		require.NotNil(t, exposeConfig)
+		require.Equal(t, []structs.ConsulExposePath{{
+			Path: "/health",
+		}}, exposeConfig.Paths)
+	})
+
+	t.Run("append to paths is safe", func(t *testing.T) {
+		// double check that serviceExposeConfig(s).Paths can be appended to
+		// from a derived pointer without fear of the original underlying array
+		// pointer being lost
+
+		s := &structs.Service{
+			Connect: &structs.ConsulConnect{
+				SidecarService: &structs.ConsulSidecarService{
+					Proxy: &structs.ConsulProxy{
+						Expose: &structs.ConsulExposeConfig{
+							Paths: []structs.ConsulExposePath{{
+								Path: "/one",
+							}},
+						},
+					},
+				},
+			},
+		}
+
+		exposeConfig := serviceExposeConfig(s)
+		exposeConfig.Paths = append(exposeConfig.Paths,
+			structs.ConsulExposePath{Path: "/two"},
+			structs.ConsulExposePath{Path: "/three"},
+			structs.ConsulExposePath{Path: "/four"},
+			structs.ConsulExposePath{Path: "/five"},
+			structs.ConsulExposePath{Path: "/six"},
+			structs.ConsulExposePath{Path: "/seven"},
+			structs.ConsulExposePath{Path: "/eight"},
+			structs.ConsulExposePath{Path: "/nine"},
+		)
+
+		// works, because exposeConfig.Paths gets re-assigned into exposeConfig
+		// which is a pointer, meaning the field is modified also from the
+		// service struct's perspective
+		require.Equal(t, 9, len(s.Connect.SidecarService.Proxy.Expose.Paths))
+	})
+}
+
+func TestJobExposeCheckHook_checkIsExposable(t *testing.T) {
+	t.Parallel()
+
+	t.Run("grpc", func(t *testing.T) {
+		require.True(t, checkIsExposable(&structs.ServiceCheck{
+			Type: "grpc",
+			Path: "/health",
+		}))
+		require.True(t, checkIsExposable(&structs.ServiceCheck{
+			Type: "gRPC",
+			Path: "/health",
+		}))
+	})
+
+	t.Run("http", func(t *testing.T) {
+		require.True(t, checkIsExposable(&structs.ServiceCheck{
+			Type: "http",
+			Path: "/health",
+		}))
+		require.True(t, checkIsExposable(&structs.ServiceCheck{
+			Type: "HTTP",
+			Path: "/health",
+		}))
+	})
+
+	t.Run("tcp", func(t *testing.T) {
+		require.False(t, checkIsExposable(&structs.ServiceCheck{
+			Type: "tcp",
+			Path: "/health",
+		}))
+	})
+
+	t.Run("no path slash prefix", func(t *testing.T) {
+		require.False(t, checkIsExposable(&structs.ServiceCheck{
+			Type: "http",
+			Path: "health",
+		}))
+	})
+}
+
+func TestJobExposeCheckHook_Mutate(t *testing.T) {
+	t.Parallel()
+
+	t.Run("typical", func(t *testing.T) {
+		result, warnings, err := new(jobExposeCheckHook).Mutate(&structs.Job{
+			TaskGroups: []*structs.TaskGroup{{
+				Name: "group0",
+				Networks: structs.Networks{{
+					Mode: "host",
+				}},
+			}, {
+				Name: "group1",
+				Networks: structs.Networks{{
+					Mode: "bridge",
+				}},
+				Services: []*structs.Service{{
+					Name:      "service1",
+					PortLabel: "8000",
+					Checks: []*structs.ServiceCheck{{
+						Name:      "check1",
+						Type:      "tcp",
+						PortLabel: "8100",
+					}, {
+						Name:      "check2",
+						Type:      "http",
+						PortLabel: "health",
+						Path:      "/health",
+						Expose:    true,
+					}, {
+						Name:      "check3",
+						Type:      "grpc",
+						PortLabel: "health",
+						Path:      "/v2/health",
+						Expose:    true,
+					}},
+					Connect: &structs.ConsulConnect{
+						SidecarService: &structs.ConsulSidecarService{
+							Proxy: &structs.ConsulProxy{
+								Expose: &structs.ConsulExposeConfig{
+									Paths: []structs.ConsulExposePath{{
+										Path:          "/pre-existing",
+										Protocol:      "http",
+										LocalPathPort: 9000,
+										ListenerPort:  "otherPort",
+									}}}}}}}, {
+					Name:      "service2",
+					PortLabel: "3000",
+					Checks: []*structs.ServiceCheck{{
+						Name:      "check1",
+						Type:      "grpc",
+						Protocol:  "http2",
+						Path:      "/ok",
+						PortLabel: "health",
+						Expose:    true,
+					}},
+					Connect: &structs.ConsulConnect{
+						SidecarService: &structs.ConsulSidecarService{
+							Proxy: &structs.ConsulProxy{},
+						},
+					},
+				}}}},
+		})
+
+		require.NoError(t, err)
+		require.Empty(t, warnings)
+		require.Equal(t, []structs.ConsulExposePath{{
+			Path:          "/pre-existing",
+			LocalPathPort: 9000,
+			Protocol:      "http",
+			ListenerPort:  "otherPort",
+		}, {
+			Path:          "/health",
+			LocalPathPort: 8000,
+			ListenerPort:  "health",
+		}, {
+			Path:          "/v2/health",
+			LocalPathPort: 8000,
+			ListenerPort:  "health",
+		}}, result.TaskGroups[1].Services[0].Connect.SidecarService.Proxy.Expose.Paths)
+		require.Equal(t, []structs.ConsulExposePath{{
+			Path:          "/ok",
+			LocalPathPort: 3000,
+			Protocol:      "http2",
+			ListenerPort:  "health",
+		}}, result.TaskGroups[1].Services[1].Connect.SidecarService.Proxy.Expose.Paths)
+	})
+}

nomad/job_endpoint_test.go

@@ -121,19 +121,15 @@ func TestJobEndpoint_Register_Connect(t *testing.T) {
 
 	// Create the register request
 	job := mock.Job()
-	job.TaskGroups[0].Networks = structs.Networks{
-		{
-			Mode: "bridge",
-		},
-	}
-	job.TaskGroups[0].Services = []*structs.Service{
-		{
-			Name:      "backend",
-			PortLabel: "8080",
-			Connect: &structs.ConsulConnect{
-				SidecarService: &structs.ConsulSidecarService{},
-			},
-		},
+	job.TaskGroups[0].Networks = structs.Networks{{
+		Mode: "bridge",
+	}}
+	job.TaskGroups[0].Services = []*structs.Service{{
+		Name:      "backend",
+		PortLabel: "8080",
+		Connect: &structs.ConsulConnect{
+			SidecarService: &structs.ConsulSidecarService{},
+		}},
 	}
 	req := &structs.JobRegisterRequest{
 		Job: job,
@@ -178,7 +174,118 @@ func TestJobEndpoint_Register_Connect(t *testing.T) {
 
 	require.Len(out.TaskGroups[0].Tasks, 2)
 	require.Exactly(sidecarTask, out.TaskGroups[0].Tasks[1])
+}
 
+func TestJobEndpoint_Register_ConnectExposeCheck(t *testing.T) {
+	t.Parallel()
+	r := require.New(t)
+
+	s1, cleanupS1 := TestServer(t, func(c *Config) {
+		c.NumSchedulers = 0 // Prevent automatic dequeue
+	})
+	defer cleanupS1()
+	codec := rpcClient(t, s1)
+	testutil.WaitForLeader(t, s1.RPC)
+
+	// Setup the job we are going to register
+	job := mock.Job()
+	job.TaskGroups[0].Networks = structs.Networks{{
+		Mode: "bridge",
+		DynamicPorts: []structs.Port{{
+			Label: "hcPort",
+			To:    -1,
+		}, {
+			Label: "v2Port",
+			To:    -1,
+		}},
+	}}
+	job.TaskGroups[0].Services = []*structs.Service{{
+		Name:      "backend",
+		PortLabel: "8080",
+		Checks: []*structs.ServiceCheck{{
+			Name:      "check1",
+			Type:      "http",
+			Protocol:  "http",
+			Path:      "/health",
+			Expose:    true,
+			PortLabel: "hcPort",
+			Interval:  1 * time.Second,
+			Timeout:   1 * time.Second,
+		}, {
+			Name:     "check2",
+			Type:     "script",
+			Command:  "/bin/true",
+			Interval: 1 * time.Second,
+			Timeout:  1 * time.Second,
+		}, {
+			Name:      "check3",
+			Type:      "grpc",
+			Protocol:  "grpc",
+			Path:      "/v2/health",
+			Expose:    true,
+			PortLabel: "v2Port",
+			Interval:  1 * time.Second,
+			Timeout:   1 * time.Second,
+		}},
+		Connect: &structs.ConsulConnect{
+			SidecarService: &structs.ConsulSidecarService{}},
+	}}
+
+	// Create the register request
+	req := &structs.JobRegisterRequest{
+		Job: job,
+		WriteRequest: structs.WriteRequest{
+			Region:    "global",
+			Namespace: job.Namespace,
+		},
+	}
+
+	// Fetch the response
+	var resp structs.JobRegisterResponse
+	r.NoError(msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp))
+	r.NotZero(resp.Index)
+
+	// Check for the node in the FSM
+	state := s1.fsm.State()
+	ws := memdb.NewWatchSet()
+	out, err := state.JobByID(ws, job.Namespace, job.ID)
+	r.NoError(err)
+	r.NotNil(out)
+	r.Equal(resp.JobModifyIndex, out.CreateIndex)
+
+	// Check that the new expose paths got created
+	r.Len(out.TaskGroups[0].Services[0].Connect.SidecarService.Proxy.Expose.Paths, 2)
+	httpPath := out.TaskGroups[0].Services[0].Connect.SidecarService.Proxy.Expose.Paths[0]
+	r.Equal(structs.ConsulExposePath{
+		Path:          "/health",
+		Protocol:      "http",
+		LocalPathPort: 8080,
+		ListenerPort:  "hcPort",
+	}, httpPath)
+	grpcPath := out.TaskGroups[0].Services[0].Connect.SidecarService.Proxy.Expose.Paths[1]
+	r.Equal(structs.ConsulExposePath{
+		Path:          "/v2/health",
+		Protocol:      "grpc",
+		LocalPathPort: 8080,
+		ListenerPort:  "v2Port",
+	}, grpcPath)
+
+	// make sure round tripping does not create duplicate expose paths
+	out.Meta["test"] = "abc"
+	req.Job = out
+	r.NoError(msgpackrpc.CallWithCodec(codec, "Job.Register", req, &resp))
+	r.NotZero(resp.Index)
+
+	// Check for the new node in the FSM
+	state = s1.fsm.State()
+	ws = memdb.NewWatchSet()
+	out, err = state.JobByID(ws, job.Namespace, job.ID)
+	r.NoError(err)
+	r.NotNil(out)
+	r.Equal(resp.JobModifyIndex, out.CreateIndex)
+
+	// make sure we are not re-adding what has already been added
+	r.Len(out.TaskGroups[0].Services[0].Connect.SidecarService.Proxy.Expose.Paths, 2)
 }
 
 func TestJobEndpoint_Register_ConnectWithSidecarTask(t *testing.T) {

nomad/structs/diff_test.go

@@ -2454,6 +2454,7 @@ func TestTaskGroupDiff(t *testing.T) {
 								Args:     []string{"foo"},
 								Path:     "foo",
 								Protocol: "http",
+								Expose:   true,
 								Interval: 1 * time.Second,
 								Timeout:  1 * time.Second,
 							},
@@ -2486,6 +2487,7 @@ func TestTaskGroupDiff(t *testing.T) {
 								Command:  "bar",
 								Path:     "bar",
 								Protocol: "tcp",
+								Expose:   false,
 								Interval: 2 * time.Second,
 								Timeout:  2 * time.Second,
 								Header: map[string][]string{
@@ -2548,7 +2550,6 @@ func TestTaskGroupDiff(t *testing.T) {
 							},
 						},
 						Objects: []*ObjectDiff{
-
 							{
 								Type: DiffTypeEdited,
 								Name: "Check",
@@ -2565,6 +2566,12 @@ func TestTaskGroupDiff(t *testing.T) {
 										Old:  "foo",
 										New:  "bar",
 									},
+									{
+										Type: DiffTypeEdited,
+										Name: "Expose",
+										Old:  "true",
+										New:  "false",
+									},
 									{
 										Type: DiffTypeNone,
 										Name: "GRPCService",
@@ -2619,6 +2626,7 @@ func TestTaskGroupDiff(t *testing.T) {
 										Old:  "http",
 										New:  "tcp",
 									},
+
 									{
 										Type: DiffTypeNone,
 										Name: "TLSSkipVerify",
@@ -4864,6 +4872,12 @@ func TestTaskDiff(t *testing.T) {
 										Old:  "",
 										New:  "foo",
 									},
+									{
+										Type: DiffTypeAdded,
+										Name: "Expose",
+										Old:  "",
+										New:  "false",
+									},
 									{
 										Type: DiffTypeAdded,
 										Name: "GRPCUseTLS",
@@ -4924,6 +4938,12 @@ func TestTaskDiff(t *testing.T) {
 										Old:  "foo",
 										New:  "",
 									},
+									{
+										Type: DiffTypeDeleted,
+										Name: "Expose",
+										Old:  "false",
+										New:  "",
+									},
 									{
 										Type: DiffTypeDeleted,
 										Name: "GRPCUseTLS",
@@ -5092,6 +5112,12 @@ func TestTaskDiff(t *testing.T) {
 										Old:  "foo",
 										New:  "foo",
 									},
+									{
+										Type: DiffTypeNone,
+										Name: "Expose",
+										Old:  "false",
+										New:  "false",
+									},
 									{
 										Type: DiffTypeNone,
 										Name: "GRPCService",

nomad/structs/services.go

@@ -47,6 +47,7 @@ type ServiceCheck struct {
 	Path          string              // path of the health check url for http type check
 	Protocol      string              // Protocol to use if check is http, defaults to http
 	PortLabel     string              // The port to use for tcp/http checks
+	Expose        bool                // Whether to have Envoy expose the check path (connect-enabled group-services only)
 	AddressMode   string              // 'host' to use host ip:port or 'driver' to use driver's
 	Interval      time.Duration       // Interval of the check
 	Timeout       time.Duration       // Timeout of the response from the check before consul fails the check
@@ -136,6 +137,10 @@ func (sc *ServiceCheck) Equals(o *ServiceCheck) bool {
 		return false
 	}
 
+	if sc.Expose != o.Expose {
+		return false
+	}
+
 	if sc.Protocol != o.Protocol {
 		return false
 	}
@@ -180,18 +185,19 @@ func (sc *ServiceCheck) Canonicalize(serviceName string) {
 // validate a Service's ServiceCheck
 func (sc *ServiceCheck) validate() error {
 	// Validate Type
-	switch strings.ToLower(sc.Type) {
+	checkType := strings.ToLower(sc.Type)
+	switch checkType {
 	case ServiceCheckGRPC:
 	case ServiceCheckTCP:
 	case ServiceCheckHTTP:
 		if sc.Path == "" {
 			return fmt.Errorf("http type must have a valid http path")
 		}
-		url, err := url.Parse(sc.Path)
+		checkPath, err := url.Parse(sc.Path)
 		if err != nil {
 			return fmt.Errorf("http type must have a valid http path")
 		}
-		if url.IsAbs() {
+		if checkPath.IsAbs() {
 			return fmt.Errorf("http type must have a relative http path")
 		}
 
@@ -238,6 +244,19 @@ func (sc *ServiceCheck) validate() error {
 		return fmt.Errorf("invalid address_mode %q", sc.AddressMode)
 	}
 
+	// Note that we cannot completely validate the Expose field yet - we do not
+	// know whether this ServiceCheck belongs to a connect-enabled group-service.
+	// Instead, such validation will happen in a job admission controller.
+	if sc.Expose {
+		// We can however immediately ensure expose is configured only for HTTP
+		// and gRPC checks.
+		switch checkType {
+		case ServiceCheckGRPC, ServiceCheckHTTP: // ok
+		default:
+			return fmt.Errorf("expose may only be set on HTTP or gRPC checks")
+		}
+	}
+
 	return sc.CheckRestart.Validate()
 }
 
@@ -263,47 +282,56 @@ func (sc *ServiceCheck) TriggersRestarts() bool {
 // called.
 func (sc *ServiceCheck) Hash(serviceID string) string {
 	h := sha1.New()
-	io.WriteString(h, serviceID)
-	io.WriteString(h, sc.Name)
-	io.WriteString(h, sc.Type)
-	io.WriteString(h, sc.Command)
-	io.WriteString(h, strings.Join(sc.Args, ""))
-	io.WriteString(h, sc.Path)
-	io.WriteString(h, sc.Protocol)
-	io.WriteString(h, sc.PortLabel)
-	io.WriteString(h, sc.Interval.String())
-	io.WriteString(h, sc.Timeout.String())
-	io.WriteString(h, sc.Method)
-	// Only include TLSSkipVerify if set to maintain ID stability with Nomad <0.6
-	if sc.TLSSkipVerify {
-		io.WriteString(h, "true")
-	}
+	hashString(h, serviceID)
+	hashString(h, sc.Name)
+	hashString(h, sc.Type)
+	hashString(h, sc.Command)
+	hashString(h, strings.Join(sc.Args, ""))
+	hashString(h, sc.Path)
+	hashString(h, sc.Protocol)
+	hashString(h, sc.PortLabel)
+	hashBool(h, sc.Expose, "Expose")
+	hashString(h, sc.Interval.String())
+	hashString(h, sc.Timeout.String())
+	hashString(h, sc.Method)
 
-	// Since map iteration order isn't stable we need to write k/v pairs to
-	// a slice and sort it before hashing.
-	if len(sc.Header) > 0 {
-		headers := make([]string, 0, len(sc.Header))
-		for k, v := range sc.Header {
+	// use name "true" to maintain ID stability
+	hashBool(h, sc.TLSSkipVerify, "true")
+
+	// maintain artisanal map hashing to maintain ID stability
+	hashHeader(h, sc.Header)
+
+	// Only include AddressMode if set to maintain ID stability with Nomad <0.7.1
+	hashStringIfNonEmpty(h, sc.AddressMode)
+
+	// Only include gRPC if set to maintain ID stability with Nomad <0.8.4
+	hashStringIfNonEmpty(h, sc.GRPCService)
+
+	// use name "true" to maintain ID stability
+	hashBool(h, sc.GRPCUseTLS, "true")
+
+	// maintain use of hex (i.e. not b32) to maintain ID stability
+	return fmt.Sprintf("%x", h.Sum(nil))
+}
+
+func hashStringIfNonEmpty(h hash.Hash, s string) {
+	if len(s) > 0 {
+		hashString(h, s)
+	}
+}
+
+func hashHeader(h hash.Hash, m map[string][]string) {
+	// maintain backwards compatibility for ID stability
+	// using the %v formatter on a map with string keys produces consistent
+	// output, but our existing format here is incompatible
+	if len(m) > 0 {
+		headers := make([]string, 0, len(m))
+		for k, v := range m {
 			headers = append(headers, k+strings.Join(v, ""))
 		}
 		sort.Strings(headers)
-		io.WriteString(h, strings.Join(headers, ""))
+		hashString(h, strings.Join(headers, ""))
 	}
-
-	// Only include AddressMode if set to maintain ID stability with Nomad <0.7.1
-	if len(sc.AddressMode) > 0 {
-		io.WriteString(h, sc.AddressMode)
-	}
-
-	// Only include GRPC if set to maintain ID stability with Nomad <0.8.4
-	if sc.GRPCService != "" {
-		io.WriteString(h, sc.GRPCService)
-	}
-	if sc.GRPCUseTLS {
-		io.WriteString(h, "true")
-	}
-
-	return fmt.Sprintf("%x", h.Sum(nil))
 }
 
 const (

nomad/structs/structs_test.go

@@ -1487,6 +1487,26 @@ func TestTask_Validate_Service_Check(t *testing.T) {
 	if !strings.Contains(err.Error(), "relative http path") {
 		t.Fatalf("err: %v", err)
 	}
+
+	t.Run("check expose", func(t *testing.T) {
+		t.Run("type http", func(t *testing.T) {
+			require.NoError(t, (&ServiceCheck{
+				Type:     ServiceCheckHTTP,
+				Interval: 1 * time.Second,
+				Timeout:  1 * time.Second,
+				Path:     "/health",
+				Expose:   true,
+			}).validate())
+		})
+		t.Run("type tcp", func(t *testing.T) {
+			require.EqualError(t, (&ServiceCheck{
+				Type:     ServiceCheckTCP,
+				Interval: 1 * time.Second,
+				Timeout:  1 * time.Second,
+				Expose:   true,
+			}).validate(), "expose may only be set on HTTP or gRPC checks")
+		})
+	})
 }
 
 // TestTask_Validate_Service_Check_AddressMode asserts that checks do not