kemko/nomad
1
0
Fork 0
mirror of https://github.com/kemko/nomad.git synced 2026-01-05 09:55:44 +03:00
Code Issues Packages Projects Releases Wiki Activity

consul/connect: Validate uniqueness of Connect upstreams within task group

Browse Source 
This PR adds validation during job submission that Connect proxy upstreams
within a task group are using different listener addresses. Otherwise, a
duplicate envoy listener will be created and not be able to bind.

Closes #7833
This commit is contained in:
Seth Hoenig
2021-06-18 16:50:53 -05:00
parent 6dcada4346
commit adcbcc129b
3 changed files with 144 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -2,6 +2,7 @@
IMPROVEMENTS:
* cli: Added `-monitor` flag to `deployment status` command and automatically monitor deployments from `job run` command. [[GH-10661](https://github.com/hashicorp/nomad/pull/10661)]
* consul/connect: Validate Connect service upstream address uniqueness within task group [[GH-7833](https://github.com/hashicorp/nomad/issues/7833)]
* docker: Tasks using `network.mode = "bridge"` that don't set their `network_mode` will receive a `/etc/hosts` file that includes the pause container's hostname and any `extra_hosts`. [[GH-10766](https://github.com/hashicorp/nomad/issues/10766)]
BUG FIXES:

39
nomad/job_endpoint_hook_connect.go
View File

@@ -127,10 +127,8 @@ func (jobConnectHook) Validate(job *structs.Job) ([]error, error) {
var warnings []error
for _, g := range job.TaskGroups {
if w, err := groupConnectValidate(g); err != nil {
if err := groupConnectValidate(g); err != nil {
return nil, err
} else if w != nil {
warnings = append(warnings, w...)
}
}
@@ -480,24 +478,49 @@ func newConnectSidecarTask(service string) *structs.Task {
}
}
func groupConnectValidate(g *structs.TaskGroup) (warnings []error, err error) {
func groupConnectValidate(g *structs.TaskGroup) error {
for _, s := range g.Services {
switch {
case s.Connect.HasSidecar():
if err := groupConnectSidecarValidate(g, s); err != nil {
return nil, err
return err
}
case s.Connect.IsNative():
if err := groupConnectNativeValidate(g, s); err != nil {
return nil, err
return err
}
case s.Connect.IsGateway():
if err := groupConnectGatewayValidate(g); err != nil {
return nil, err
return err
}
}
}
return nil, nil
if err := groupConnectUpstreamsValidate(g.Name, g.Services); err != nil {
return err
}
return nil
}
func groupConnectUpstreamsValidate(group string, services []*structs.Service) error {
listeners := make(map[string]string) // address -> service
for _, service := range services {
if service.Connect.HasSidecar() && service.Connect.SidecarService.Proxy != nil {
for _, up := range service.Connect.SidecarService.Proxy.Upstreams {
listener := fmt.Sprintf("%s:%d", up.LocalBindAddress, up.LocalBindPort)
if s, exists := listeners[listener]; exists {
return fmt.Errorf(
"Consul Connect services %q and %q in group %q using same address for upstreams (%s)",
service.Name, s, group, listener,
)
}
listeners[listener] = service.Name
}
}
}
return nil
}
func groupConnectSidecarValidate(g *structs.TaskGroup, s *structs.Service) error {

117
nomad/job_endpoint_hook_connect_test.go
View File

@@ -348,7 +348,7 @@ func TestJobEndpointConnect_groupConnectSidecarValidate(t *testing.T) {
// group and service name validation
t.Run("non-connect service contains uppercase characters", func(t *testing.T) {
_, err := groupConnectValidate(&structs.TaskGroup{
err := groupConnectValidate(&structs.TaskGroup{
Name: "group",
Networks: structs.Networks{{Mode: "bridge"}},
Services: []*structs.Service{{
@@ -359,7 +359,7 @@ func TestJobEndpointConnect_groupConnectSidecarValidate(t *testing.T) {
})
t.Run("connect service contains uppercase characters", func(t *testing.T) {
_, err := groupConnectValidate(&structs.TaskGroup{
err := groupConnectValidate(&structs.TaskGroup{
Name: "group",
Networks: structs.Networks{{Mode: "bridge"}},
Services: []*structs.Service{{
@@ -370,7 +370,7 @@ func TestJobEndpointConnect_groupConnectSidecarValidate(t *testing.T) {
})
t.Run("non-connect group contains uppercase characters", func(t *testing.T) {
_, err := groupConnectValidate(&structs.TaskGroup{
err := groupConnectValidate(&structs.TaskGroup{
Name: "Other-Group",
Networks: structs.Networks{{Mode: "bridge"}},
Services: []*structs.Service{{
@@ -381,7 +381,7 @@ func TestJobEndpointConnect_groupConnectSidecarValidate(t *testing.T) {
})
t.Run("connect-group contains uppercase characters", func(t *testing.T) {
_, err := groupConnectValidate(&structs.TaskGroup{
err := groupConnectValidate(&structs.TaskGroup{
Name: "Connect-Group",
Networks: structs.Networks{{Mode: "bridge"}},
Services: []*structs.Service{{
@@ -392,7 +392,7 @@ func TestJobEndpointConnect_groupConnectSidecarValidate(t *testing.T) {
})
t.Run("connect group and service lowercase", func(t *testing.T) {
_, err := groupConnectValidate(&structs.TaskGroup{
err := groupConnectValidate(&structs.TaskGroup{
Name: "connect-group",
Networks: structs.Networks{{Mode: "bridge"}},
Services: []*structs.Service{{
@@ -401,6 +401,113 @@ func TestJobEndpointConnect_groupConnectSidecarValidate(t *testing.T) {
})
require.NoError(t, err)
})
t.Run("connect group overlap upstreams", func(t *testing.T) {
s1 := makeService("s1")
s2 := makeService("s2")
s1.Connect.SidecarService.Proxy = &structs.ConsulProxy{
Upstreams: []structs.ConsulUpstream{{
LocalBindPort: 8999,
}},
}
s2.Connect.SidecarService.Proxy = &structs.ConsulProxy{
Upstreams: []structs.ConsulUpstream{{
LocalBindPort: 8999,
}},
}
err := groupConnectValidate(&structs.TaskGroup{
Name: "connect-group",
Networks: structs.Networks{{Mode: "bridge"}},
Services: []*structs.Service{s1, s2},
})
require.EqualError(t, err, `Consul Connect services "s2" and "s1" in group "connect-group" using same address for upstreams (:8999)`)
})
}
func TestJobEndpointConnect_groupConnectUpstreamsValidate(t *testing.T) {
t.Run("no connect services", func(t *testing.T) {
err := groupConnectUpstreamsValidate("group",
[]*structs.Service{{Name: "s1"}, {Name: "s2"}})
require.NoError(t, err)
})
t.Run("connect services no overlap", func(t *testing.T) {
err := groupConnectUpstreamsValidate("group",
[]*structs.Service{
{
Name: "s1",
Connect: &structs.ConsulConnect{
SidecarService: &structs.ConsulSidecarService{
Proxy: &structs.ConsulProxy{
Upstreams: []structs.ConsulUpstream{{
LocalBindAddress: "127.0.0.1",
LocalBindPort: 9001,
}, {
LocalBindAddress: "127.0.0.1",
LocalBindPort: 9002,
}},
},
},
},
},
{
Name: "s2",
Connect: &structs.ConsulConnect{
SidecarService: &structs.ConsulSidecarService{
Proxy: &structs.ConsulProxy{
Upstreams: []structs.ConsulUpstream{{
LocalBindAddress: "10.0.0.1",
LocalBindPort: 9001,
}, {
LocalBindAddress: "127.0.0.1",
LocalBindPort: 9003,
}},
},
},
},
},
})
require.NoError(t, err)
})
t.Run("connect services overlap port", func(t *testing.T) {
err := groupConnectUpstreamsValidate("group",
[]*structs.Service{
{
Name: "s1",
Connect: &structs.ConsulConnect{
SidecarService: &structs.ConsulSidecarService{
Proxy: &structs.ConsulProxy{
Upstreams: []structs.ConsulUpstream{{
LocalBindAddress: "127.0.0.1",
LocalBindPort: 9001,
}, {
LocalBindAddress: "127.0.0.1",
LocalBindPort: 9002,
}},
},
},
},
},
{
Name: "s2",
Connect: &structs.ConsulConnect{
SidecarService: &structs.ConsulSidecarService{
Proxy: &structs.ConsulProxy{
Upstreams: []structs.ConsulUpstream{{
LocalBindAddress: "127.0.0.1",
LocalBindPort: 9002,
}, {
LocalBindAddress: "127.0.0.1",
LocalBindPort: 9003,
}},
},
},
},
},
})
require.EqualError(t, err, `Consul Connect services "s2" and "s1" in group "group" using same address for upstreams (127.0.0.1:9002)`)
})
}
func TestJobEndpointConnect_getNamedTaskForNativeService(t *testing.T) {