Skip to content

Commit

Permalink
Merge pull request #8882 from hashicorp/b-expose-panic
Browse files Browse the repository at this point in the history
consul/connect: validate group network on expose port injection
  • Loading branch information
shoenig authored and Mahmood Ali committed Sep 15, 2020
1 parent 34c9d87 commit 75b50d9
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
8 changes: 7 additions & 1 deletion nomad/job_endpoint_hook_expose_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func serviceUsesConnectEnvoy(s *structs.Service) bool {
}

// A non-nil connect.sidecar_task stanza implies the sidecar task is being
// overridden (i.e. the default Envoy is not being uesd).
// overridden (i.e. the default Envoy is not being used).
if s.Connect.SidecarTask != nil {
return false
}
Expand Down Expand Up @@ -199,6 +199,12 @@ func exposePathForCheck(tg *structs.TaskGroup, s *structs.Service, check *struct
return nil, nil
}

// Borrow some of the validation before we start manipulating the group
// network, which needs to exist once.
if err := tgValidateUseOfBridgeMode(tg); err != nil {
return nil, err
}

// If the check is exposable but doesn't have a port label set build
// a port with a generated label, add it to the group's Dynamic ports
// and set the check port label to the generated label.
Expand Down
24 changes: 24 additions & 0 deletions nomad/job_endpoint_hook_expose_check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,30 @@ func TestJobExposeCheckHook_exposePathForCheck(t *testing.T) {
ListenerPort: tg.Networks[0].DynamicPorts[0].Label,
}, ePath)
})

t.Run("missing network with no service check port label", func(t *testing.T) {
// this test ensures we do not try to manipulate the group network
// to inject an expose port if the group network does not exist
c := &structs.ServiceCheck{
Name: "check1",
Type: "http",
Path: "/health",
PortLabel: "", // not set
Expose: true, // will require a service check port label
}
s := &structs.Service{
Name: "service1",
Checks: []*structs.ServiceCheck{c},
}
tg := &structs.TaskGroup{
Name: "group1",
Services: []*structs.Service{s},
Networks: nil, // not set, should cause validation error
}
ePath, err := exposePathForCheck(tg, s, c)
require.EqualError(t, err, `group "group1" must specify one bridge network for exposing service check(s)`)
require.Nil(t, ePath)
})
}

func TestJobExposeCheckHook_containsExposePath(t *testing.T) {
Expand Down

0 comments on commit 75b50d9

Please sign in to comment.