From f7564080d2c3174c27b8384d43914c1f273d038e Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Wed, 25 May 2022 15:05:15 -0500 Subject: [PATCH] connect: enable setting connect upstream destination namespace --- api/consul.go | 23 +++++++++---------- api/consul_test.go | 33 +++++++++++++++------------- command/agent/consul/connect.go | 11 +++++----- command/agent/consul/connect_test.go | 18 ++++++++------- command/agent/job_endpoint.go | 11 +++++----- command/agent/job_endpoint_test.go | 22 ++++++++++--------- nomad/structs/diff_test.go | 15 +++++++++---- nomad/structs/services.go | 17 +++++++++----- nomad/structs/services_test.go | 33 +++++++++++++++++++++------- 9 files changed, 112 insertions(+), 71 deletions(-) diff --git a/api/consul.go b/api/consul.go index 9a11187c08cb..93959ba26fc2 100644 --- a/api/consul.go +++ b/api/consul.go @@ -75,7 +75,6 @@ func (css *ConsulSidecarService) Canonicalize() { css.Proxy.Canonicalize() } - // SidecarTask represents a subset of Task fields that can be set to override // the fields of the Task generated for the sidecar type SidecarTask struct { @@ -196,11 +195,12 @@ func (c *ConsulMeshGateway) Copy() *ConsulMeshGateway { // ConsulUpstream represents a Consul Connect upstream jobspec stanza. type ConsulUpstream struct { - DestinationName string `mapstructure:"destination_name" hcl:"destination_name,optional"` - LocalBindPort int `mapstructure:"local_bind_port" hcl:"local_bind_port,optional"` - Datacenter string `mapstructure:"datacenter" hcl:"datacenter,optional"` - LocalBindAddress string `mapstructure:"local_bind_address" hcl:"local_bind_address,optional"` - MeshGateway *ConsulMeshGateway `mapstructure:"mesh_gateway" hcl:"mesh_gateway,block"` + DestinationName string `mapstructure:"destination_name" hcl:"destination_name,optional"` + DestinationNamespace string `mapstructure:"destination_namespace" hcl:"destination_namespace,optional"` + LocalBindPort int `mapstructure:"local_bind_port" hcl:"local_bind_port,optional"` + Datacenter string `mapstructure:"datacenter" hcl:"datacenter,optional"` + LocalBindAddress string `mapstructure:"local_bind_address" hcl:"local_bind_address,optional"` + MeshGateway *ConsulMeshGateway `mapstructure:"mesh_gateway" hcl:"mesh_gateway,block"` } func (cu *ConsulUpstream) Copy() *ConsulUpstream { @@ -208,11 +208,12 @@ func (cu *ConsulUpstream) Copy() *ConsulUpstream { return nil } return &ConsulUpstream{ - DestinationName: cu.DestinationName, - LocalBindPort: cu.LocalBindPort, - Datacenter: cu.Datacenter, - LocalBindAddress: cu.LocalBindAddress, - MeshGateway: cu.MeshGateway.Copy(), + DestinationName: cu.DestinationName, + DestinationNamespace: cu.DestinationNamespace, + LocalBindPort: cu.LocalBindPort, + Datacenter: cu.Datacenter, + LocalBindAddress: cu.LocalBindAddress, + MeshGateway: cu.MeshGateway.Copy(), } } diff --git a/api/consul_test.go b/api/consul_test.go index 0c32c4c8168d..0a4c6781cfa8 100644 --- a/api/consul_test.go +++ b/api/consul_test.go @@ -163,11 +163,12 @@ func TestConsulUpstream_Copy(t *testing.T) { t.Run("complete upstream", func(t *testing.T) { cu := &ConsulUpstream{ - DestinationName: "dest1", - Datacenter: "dc2", - LocalBindPort: 2000, - LocalBindAddress: "10.0.0.1", - MeshGateway: &ConsulMeshGateway{Mode: "remote"}, + DestinationName: "dest1", + DestinationNamespace: "ns2", + Datacenter: "dc2", + LocalBindPort: 2000, + LocalBindAddress: "10.0.0.1", + MeshGateway: &ConsulMeshGateway{Mode: "remote"}, } result := cu.Copy() require.Equal(t, cu, result) @@ -185,19 +186,21 @@ func TestConsulUpstream_Canonicalize(t *testing.T) { t.Run("complete", func(t *testing.T) { cu := &ConsulUpstream{ - DestinationName: "dest1", - Datacenter: "dc2", - LocalBindPort: 2000, - LocalBindAddress: "10.0.0.1", - MeshGateway: &ConsulMeshGateway{Mode: ""}, + DestinationName: "dest1", + DestinationNamespace: "ns2", + Datacenter: "dc2", + LocalBindPort: 2000, + LocalBindAddress: "10.0.0.1", + MeshGateway: &ConsulMeshGateway{Mode: ""}, } cu.Canonicalize() require.Equal(t, &ConsulUpstream{ - DestinationName: "dest1", - Datacenter: "dc2", - LocalBindPort: 2000, - LocalBindAddress: "10.0.0.1", - MeshGateway: &ConsulMeshGateway{Mode: ""}, + DestinationName: "dest1", + DestinationNamespace: "ns2", + Datacenter: "dc2", + LocalBindPort: 2000, + LocalBindAddress: "10.0.0.1", + MeshGateway: &ConsulMeshGateway{Mode: ""}, }, cu) }) } diff --git a/command/agent/consul/connect.go b/command/agent/consul/connect.go index e70b728d4dbf..c92ff922f5b6 100644 --- a/command/agent/consul/connect.go +++ b/command/agent/consul/connect.go @@ -195,11 +195,12 @@ func connectUpstreams(in []structs.ConsulUpstream) []api.Upstream { upstreams := make([]api.Upstream, len(in)) for i, upstream := range in { upstreams[i] = api.Upstream{ - DestinationName: upstream.DestinationName, - LocalBindPort: upstream.LocalBindPort, - Datacenter: upstream.Datacenter, - LocalBindAddress: upstream.LocalBindAddress, - MeshGateway: connectMeshGateway(upstream.MeshGateway), + DestinationName: upstream.DestinationName, + DestinationNamespace: upstream.DestinationNamespace, + LocalBindPort: upstream.LocalBindPort, + Datacenter: upstream.Datacenter, + LocalBindAddress: upstream.LocalBindAddress, + MeshGateway: connectMeshGateway(upstream.MeshGateway), } } return upstreams diff --git a/command/agent/consul/connect_test.go b/command/agent/consul/connect_test.go index 89bebd2306f5..5e4ac6c3e6b8 100644 --- a/command/agent/consul/connect_test.go +++ b/command/agent/consul/connect_test.go @@ -360,19 +360,21 @@ func TestConnect_connectUpstreams(t *testing.T) { DestinationName: "foo", LocalBindPort: 8000, }, { - DestinationName: "bar", - LocalBindPort: 9000, - Datacenter: "dc2", - LocalBindAddress: "127.0.0.2", + DestinationName: "bar", + DestinationNamespace: "ns2", + LocalBindPort: 9000, + Datacenter: "dc2", + LocalBindAddress: "127.0.0.2", }}, connectUpstreams([]structs.ConsulUpstream{{ DestinationName: "foo", LocalBindPort: 8000, }, { - DestinationName: "bar", - LocalBindPort: 9000, - Datacenter: "dc2", - LocalBindAddress: "127.0.0.2", + DestinationName: "bar", + DestinationNamespace: "ns2", + LocalBindPort: 9000, + Datacenter: "dc2", + LocalBindAddress: "127.0.0.2", }}), ) }) diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 46739207d486..ead803e243c6 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1628,11 +1628,12 @@ func apiUpstreamsToStructs(in []*api.ConsulUpstream) []structs.ConsulUpstream { upstreams := make([]structs.ConsulUpstream, len(in)) for i, upstream := range in { upstreams[i] = structs.ConsulUpstream{ - DestinationName: upstream.DestinationName, - LocalBindPort: upstream.LocalBindPort, - Datacenter: upstream.Datacenter, - LocalBindAddress: upstream.LocalBindAddress, - MeshGateway: apiMeshGatewayToStructs(upstream.MeshGateway), + DestinationName: upstream.DestinationName, + DestinationNamespace: upstream.DestinationNamespace, + LocalBindPort: upstream.LocalBindPort, + Datacenter: upstream.Datacenter, + LocalBindAddress: upstream.LocalBindAddress, + MeshGateway: apiMeshGatewayToStructs(upstream.MeshGateway), } } return upstreams diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index c89bcf5c2ab4..97e01fbfa609 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -3679,17 +3679,19 @@ func TestConversion_apiUpstreamsToStructs(t *testing.T) { require.Nil(t, apiUpstreamsToStructs(nil)) require.Nil(t, apiUpstreamsToStructs(make([]*api.ConsulUpstream, 0))) require.Equal(t, []structs.ConsulUpstream{{ - DestinationName: "upstream", - LocalBindPort: 8000, - Datacenter: "dc2", - LocalBindAddress: "127.0.0.2", - MeshGateway: &structs.ConsulMeshGateway{Mode: "local"}, + DestinationName: "upstream", + DestinationNamespace: "ns2", + LocalBindPort: 8000, + Datacenter: "dc2", + LocalBindAddress: "127.0.0.2", + MeshGateway: &structs.ConsulMeshGateway{Mode: "local"}, }}, apiUpstreamsToStructs([]*api.ConsulUpstream{{ - DestinationName: "upstream", - LocalBindPort: 8000, - Datacenter: "dc2", - LocalBindAddress: "127.0.0.2", - MeshGateway: &api.ConsulMeshGateway{Mode: "local"}, + DestinationName: "upstream", + DestinationNamespace: "ns2", + LocalBindPort: 8000, + Datacenter: "dc2", + LocalBindAddress: "127.0.0.2", + MeshGateway: &api.ConsulMeshGateway{Mode: "local"}, }})) } diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 9b327af294e5..8cb208cb7e07 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -2773,10 +2773,11 @@ func TestTaskGroupDiff(t *testing.T) { LocalServicePort: 8080, Upstreams: []ConsulUpstream{ { - DestinationName: "foo", - LocalBindPort: 8000, - Datacenter: "dc2", - LocalBindAddress: "127.0.0.2", + DestinationName: "foo", + DestinationNamespace: "ns2", + LocalBindPort: 8000, + Datacenter: "dc2", + LocalBindAddress: "127.0.0.2", MeshGateway: &ConsulMeshGateway{ Mode: "remote", }, @@ -3101,6 +3102,12 @@ func TestTaskGroupDiff(t *testing.T) { Old: "", New: "foo", }, + { + Type: DiffTypeAdded, + Name: "DestinationNamespace", + Old: "", + New: "ns2", + }, { Type: DiffTypeAdded, Name: "LocalBindAddress", diff --git a/nomad/structs/services.go b/nomad/structs/services.go index c0643e05d39e..7d0bfba237ca 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -725,6 +725,7 @@ func hashConnect(h hash.Hash, connect *ConsulConnect) { hashConfig(h, p.Config) for _, upstream := range p.Upstreams { hashString(h, upstream.DestinationName) + hashString(h, upstream.DestinationNamespace) hashString(h, strconv.Itoa(upstream.LocalBindPort)) hashStringIfNonEmpty(h, upstream.Datacenter) hashStringIfNonEmpty(h, upstream.LocalBindAddress) @@ -1363,6 +1364,9 @@ type ConsulUpstream struct { // DestinationName is the name of the upstream service. DestinationName string + // DestinationNamespace is the namespace of the upstream service. + DestinationNamespace string + // LocalBindPort is the port the proxy will receive connections for the // upstream on. LocalBindPort int @@ -1403,11 +1407,12 @@ func (u *ConsulUpstream) Copy() *ConsulUpstream { } return &ConsulUpstream{ - DestinationName: u.DestinationName, - LocalBindPort: u.LocalBindPort, - Datacenter: u.Datacenter, - LocalBindAddress: u.LocalBindAddress, - MeshGateway: u.MeshGateway.Copy(), + DestinationName: u.DestinationName, + DestinationNamespace: u.DestinationNamespace, + LocalBindPort: u.LocalBindPort, + Datacenter: u.Datacenter, + LocalBindAddress: u.LocalBindAddress, + MeshGateway: u.MeshGateway.Copy(), } } @@ -1420,6 +1425,8 @@ func (u *ConsulUpstream) Equals(o *ConsulUpstream) bool { switch { case u.DestinationName != o.DestinationName: return false + case u.DestinationNamespace != o.DestinationNamespace: + return false case u.LocalBindPort != o.LocalBindPort: return false case u.Datacenter != o.Datacenter: diff --git a/nomad/structs/services_test.go b/nomad/structs/services_test.go index 39a733613123..2ee2f62d38c6 100644 --- a/nomad/structs/services_test.go +++ b/nomad/structs/services_test.go @@ -209,8 +209,9 @@ func TestService_Hash(t *testing.T) { LocalServicePort: 24000, Config: map[string]interface{}{"foo": "bar"}, Upstreams: []ConsulUpstream{{ - DestinationName: "upstream1", - LocalBindPort: 29000, + DestinationName: "upstream1", + DestinationNamespace: "ns2", + LocalBindPort: 29000, }}, }, }, @@ -291,11 +292,15 @@ func TestService_Hash(t *testing.T) { try(t, func(s *svc) { s.Connect.SidecarService.Proxy.Config = map[string]interface{}{"foo": "baz"} }) }) - t.Run("mod connect sidecar proxy upstream dest name", func(t *testing.T) { + t.Run("mod connect sidecar proxy upstream destination name", func(t *testing.T) { try(t, func(s *svc) { s.Connect.SidecarService.Proxy.Upstreams[0].DestinationName = "dest2" }) }) - t.Run("mod connect sidecar proxy upstream dest local bind port", func(t *testing.T) { + t.Run("mod connect sidecar proxy upstream destination namespace", func(t *testing.T) { + try(t, func(s *svc) { s.Connect.SidecarService.Proxy.Upstreams[0].DestinationNamespace = "ns3" }) + }) + + t.Run("mod connect sidecar proxy upstream destination local bind port", func(t *testing.T) { try(t, func(s *svc) { s.Connect.SidecarService.Proxy.Upstreams[0].LocalBindPort = 29999 }) }) } @@ -331,12 +336,14 @@ func TestConsulConnect_CopyEquals(t *testing.T) { LocalServicePort: 8080, Upstreams: []ConsulUpstream{ { - DestinationName: "up1", - LocalBindPort: 9002, + DestinationName: "up1", + DestinationNamespace: "ns2", + LocalBindPort: 9002, }, { - DestinationName: "up2", - LocalBindPort: 9003, + DestinationName: "up2", + DestinationNamespace: "ns2", + LocalBindPort: 9003, }, }, Config: map[string]interface{}{ @@ -530,6 +537,16 @@ func TestConsulUpstream_upstreamEquals(t *testing.T) { require.False(t, upstreamsEquals(a, b)) }) + t.Run("different namespace", func(t *testing.T) { + a := []ConsulUpstream{up("foo", 8000)} + a[0].DestinationNamespace = "ns1" + + b := []ConsulUpstream{up("foo", 8000)} + b[0].DestinationNamespace = "ns2" + + require.False(t, upstreamsEquals(a, b)) + }) + t.Run("different mesh_gateway", func(t *testing.T) { a := []ConsulUpstream{{DestinationName: "foo", MeshGateway: &ConsulMeshGateway{Mode: "local"}}} b := []ConsulUpstream{{DestinationName: "foo", MeshGateway: &ConsulMeshGateway{Mode: "remote"}}}