From 36aa58512caea591a396c5cb23591f223c0105e7 Mon Sep 17 00:00:00 2001 From: Mark Anderson Date: Mon, 3 May 2021 22:34:24 -0700 Subject: [PATCH] More review fixups Signed-off-by: Mark Anderson --- agent/config/testdata/full-config.json | 6 +-- agent/structs/structs.go | 2 +- agent/xds/endpoints_test.go | 54 -------------------------- agent/xds/listeners.go | 8 ++-- agent/xds/listeners_test.go | 5 --- connect/proxy/config.go | 2 +- go.mod | 1 - go.sum | 4 -- proto/pbservice/service.gen.go | 2 + proto/pbservice/service.proto | 2 +- 10 files changed, 12 insertions(+), 74 deletions(-) diff --git a/agent/config/testdata/full-config.json b/agent/config/testdata/full-config.json index 273d35c40d71..6dd73a01cfbe 100644 --- a/agent/config/testdata/full-config.json +++ b/agent/config/testdata/full-config.json @@ -358,6 +358,7 @@ "address": "cOlSOhbp", "token": "msy7iWER", "port": 24237, + "socket_path": "/tmp/rc78ap", "weights": { "passing": 100, "warning": 1 @@ -592,9 +593,8 @@ "destination_type": "prepared_query", "local_bind_address": "127.24.88.0", "local_bind_port": 11884, - "local_bind_socket_path": "/foo/bar/upstream" - "local_bind_socket_path": "/foo/bar/upstream" - + "local_bind_socket_path": "/foo/bar/upstream", + "local_bind_socket_mode": "0600" } ] } diff --git a/agent/structs/structs.go b/agent/structs/structs.go index f446dc3f20bb..08cb7be139fb 100644 --- a/agent/structs/structs.go +++ b/agent/structs/structs.go @@ -1001,7 +1001,7 @@ type NodeService struct { TaggedAddresses map[string]ServiceAddress `json:",omitempty"` Meta map[string]string Port int `json:",omitempty"` - SocketPath string `json:",omitempty"` // This might be integrated into Address somehow, but not sure about the ergonomics. Only one of (address,port) or socketpath can be defined. + SocketPath string `json:",omitempty"` // TODO This might be integrated into Address somehow, but not sure about the ergonomics. Only one of (address,port) or socketpath can be defined. Weights *Weights EnableTagOverride bool diff --git a/agent/xds/endpoints_test.go b/agent/xds/endpoints_test.go index 5cd0a062e851..9bdb181136da 100644 --- a/agent/xds/endpoints_test.go +++ b/agent/xds/endpoints_test.go @@ -72,31 +72,6 @@ func Test_makeLoadAssignment(t *testing.T) { }, }, }, - structs.CheckServiceNode{ - Node: &structs.Node{ - ID: "node3-id", - Node: "node3", - - Datacenter: "dc1", - }, - Service: &structs.NodeService{ - Service: "web", - SocketPath: "/tmp/valid_socket_path", - }, - Checks: structs.HealthChecks{ - &structs.HealthCheck{ - Node: "node3", - CheckID: "serfHealth", - Status: "passing", - }, - &structs.HealthCheck{ - Node: "node", - ServiceID: "web", - CheckID: "web:check", - Status: "passing", - }, - }, - }, } testWeightedCheckServiceNodesRaw, err := copystructure.Copy(testCheckServiceNodes) @@ -165,14 +140,6 @@ func Test_makeLoadAssignment(t *testing.T) { HealthStatus: envoy_core_v3.HealthStatus_HEALTHY, LoadBalancingWeight: makeUint32Value(1), }, - { - HostIdentifier: &envoy_endpoint_v3.LbEndpoint_Endpoint{ - Endpoint: &envoy_endpoint_v3.Endpoint{ - Address: makePipeAddress("/tmp/valid_socket_path", 0), - }}, - HealthStatus: envoy_core_v3.HealthStatus_HEALTHY, - LoadBalancingWeight: makeUint32Value(1), - }, }, }}, }, @@ -203,14 +170,6 @@ func Test_makeLoadAssignment(t *testing.T) { HealthStatus: envoy_core_v3.HealthStatus_HEALTHY, LoadBalancingWeight: makeUint32Value(5), }, - { - HostIdentifier: &envoy_endpoint_v3.LbEndpoint_Endpoint{ - Endpoint: &envoy_endpoint_v3.Endpoint{ - Address: makePipeAddress("/tmp/valid_socket_path", 0), - }}, - HealthStatus: envoy_core_v3.HealthStatus_HEALTHY, - LoadBalancingWeight: makeUint32Value(1), - }, }, }}, }, @@ -241,14 +200,6 @@ func Test_makeLoadAssignment(t *testing.T) { HealthStatus: envoy_core_v3.HealthStatus_UNHEALTHY, LoadBalancingWeight: makeUint32Value(1), }, - { - HostIdentifier: &envoy_endpoint_v3.LbEndpoint_Endpoint{ - Endpoint: &envoy_endpoint_v3.Endpoint{ - Address: makePipeAddress("/tmp/valid_socket_path", 0), - }}, - HealthStatus: envoy_core_v3.HealthStatus_HEALTHY, - LoadBalancingWeight: makeUint32Value(1), - }, }, }}, }, @@ -378,11 +329,6 @@ func TestEndpointsFromSnapshot(t *testing.T) { } }, }, - { - name: "downstream-pipe", - create: proxycfg.TestConfigSnapshot, - setup: nil, // Default snapshot - }, { name: "splitter-with-resolver-redirect", create: proxycfg.TestConfigSnapshotDiscoveryChain_SplitterWithResolverRedirectMultiDC, diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index 35f59f0604b6..966a7b4b1cbc 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -104,7 +104,6 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. // Generate the upstream listeners for when they are explicitly set with a local bind port or socket path if outboundListener == nil || (upstreamCfg != nil && upstreamCfg.HasLocalPortOrSocket()) { - filterChain, err := s.makeUpstreamFilterChainForDiscoveryChain( id, "", @@ -548,7 +547,6 @@ func makePortListener(name, addr string, port int, trafficDirection envoy_core_v } func makePortListenerWithDefault(name, addr string, port int, trafficDirection envoy_core_v3.TrafficDirection) *envoy_listener_v3.Listener { - if addr == "" { addr = "127.0.0.1" } @@ -557,7 +555,10 @@ func makePortListenerWithDefault(name, addr string, port int, trafficDirection e func makePipeListener(name, path string, mode_str string, trafficDirection envoy_core_v3.TrafficDirection) *envoy_listener_v3.Listener { // We've already validated this, so it should not fail. - mode, _ := strconv.ParseUint(mode_str, 0, 32) + mode, err := strconv.ParseUint(mode_str, 0, 32) + if err != nil { + mode = 0 + } return &envoy_listener_v3.Listener{ Name: fmt.Sprintf("%s:%s", name, path), Address: makePipeAddress(path, uint32(mode)), @@ -1295,7 +1296,6 @@ func (s *ResourceGenerator) makeUpstreamListenerForDiscoveryChain( upstreamID := u.Identifier() l := makePortListenerWithDefault(upstreamID, address, u.LocalBindPort, envoy_core_v3.TrafficDirection_OUTBOUND) - cfg := s.getAndModifyUpstreamConfigForListener(upstreamID, u, chain) if cfg.EnvoyListenerJSON != "" { return makeListenerFromUserConfig(cfg.EnvoyListenerJSON) diff --git a/agent/xds/listeners_test.go b/agent/xds/listeners_test.go index 54c9e13e131a..e72d74f57589 100644 --- a/agent/xds/listeners_test.go +++ b/agent/xds/listeners_test.go @@ -347,11 +347,6 @@ func TestListenersFromSnapshot(t *testing.T) { create: proxycfg.TestConfigSnapshotIngressGateway, setup: nil, }, - // { - // name: "ingress-gateway-unix-domain-socket", - // create: proxycfg.TestConfigSnapshotIngressGatewayWithUDS, - // setup: nil, - // }, { name: "ingress-gateway-bind-addrs", create: proxycfg.TestConfigSnapshotIngressGateway, diff --git a/connect/proxy/config.go b/connect/proxy/config.go index 332ace94ca49..f4016f0ea9fc 100644 --- a/connect/proxy/config.go +++ b/connect/proxy/config.go @@ -249,7 +249,7 @@ func (w *AgentConfigWatcher) handler(blockVal watch.BlockingParamVal, cfg.PublicListener.BindAddress = resp.Address cfg.PublicListener.BindPort = resp.Port if resp.Proxy.LocalServiceSocketPath != "" { - panic(fmt.Sprintf("Unhandled unix domain socket config %+v %+v", resp.Proxy, cfg.PublicListener)) + w.logger.Error("Unhandled unix domain socket config %+v %+v", resp.Proxy, cfg.PublicListener) } cfg.PublicListener.LocalServiceAddress = ipaddr.FormatAddressPort( resp.Proxy.LocalServiceAddress, resp.Proxy.LocalServicePort) diff --git a/go.mod b/go.mod index 160471b70387..96beaa7168c7 100644 --- a/go.mod +++ b/go.mod @@ -52,7 +52,6 @@ require ( github.com/hashicorp/mdns v1.0.4 // indirect github.com/hashicorp/memberlist v0.2.3 github.com/hashicorp/net-rpc-msgpackrpc v0.0.0-20151116020338-a14192a58a69 - github.com/hashicorp/protoc-gen-go-binary v0.0.0-20200316174223-e4c249d51dbb // indirect github.com/hashicorp/raft v1.2.0 github.com/hashicorp/raft-autopilot v0.1.2 github.com/hashicorp/raft-boltdb v0.0.0-20171010151810-6e5ba93211ea diff --git a/go.sum b/go.sum index 217e013f94ee..c7fbf655d0d1 100644 --- a/go.sum +++ b/go.sum @@ -209,8 +209,6 @@ github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7/go.mod h1:Fecb github.com/grpc-ecosystem/go-grpc-middleware v1.0.0/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= -github.com/grpc-ecosystem/grpc-gateway v1.11.2 h1:bUDfHRK8aKGdya+msYJHffDwNxB8Eileyl7Jf2qqYjI= -github.com/grpc-ecosystem/grpc-gateway v1.11.2/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/go-bexpr v0.1.2 h1:ijMXI4qERbzxbCnkxmfUtwMyjrrk3y+Vt0MxojNCbBs= @@ -280,8 +278,6 @@ github.com/hashicorp/memberlist v0.2.3 h1:BwZa5IjREr75J0am7nblP+X5i95Rmp8EEbMI5v github.com/hashicorp/memberlist v0.2.3/go.mod h1:MS2lj3INKhZjWNqd3N0m3J+Jxf3DAOnAH9VT3Sh9MUE= github.com/hashicorp/net-rpc-msgpackrpc v0.0.0-20151116020338-a14192a58a69 h1:lc3c72qGlIMDqQpQH82Y4vaglRMMFdJbziYWriR4UcE= github.com/hashicorp/net-rpc-msgpackrpc v0.0.0-20151116020338-a14192a58a69/go.mod h1:/z+jUGRBlwVpUZfjute9jWaF6/HuhjuFQuL1YXzVD1Q= -github.com/hashicorp/protoc-gen-go-binary v0.0.0-20200316174223-e4c249d51dbb h1:biFUxNgHvoaob2aLWZu+WQfR/reXDRnHiNMAmdoW/As= -github.com/hashicorp/protoc-gen-go-binary v0.0.0-20200316174223-e4c249d51dbb/go.mod h1:U8XDK1AA+O3hXAqyooDBH1XKBq1pRvHALl4lEq941Ag= github.com/hashicorp/raft v1.1.1/go.mod h1:vPAJM8Asw6u8LxC3eJCUZmRP/E4QmUGE1R7g7k8sG/8= github.com/hashicorp/raft v1.2.0 h1:mHzHIrF0S91d3A7RPBvuqkgB4d/7oFJZyvf1Q4m7GA0= github.com/hashicorp/raft v1.2.0/go.mod h1:vPAJM8Asw6u8LxC3eJCUZmRP/E4QmUGE1R7g7k8sG/8= diff --git a/proto/pbservice/service.gen.go b/proto/pbservice/service.gen.go index a8984503c3d9..942f64ede09c 100644 --- a/proto/pbservice/service.gen.go +++ b/proto/pbservice/service.gen.go @@ -10,6 +10,7 @@ func ConnectProxyConfigToStructs(s ConnectProxyConfig) structs.ConnectProxyConfi t.DestinationServiceID = s.DestinationServiceID t.LocalServiceAddress = s.LocalServiceAddress t.LocalServicePort = int(s.LocalServicePort) + t.LocalServiceSocketPath = s.LocalServiceSocketPath t.Mode = s.Mode t.Config = ProtobufTypesStructToMapStringInterface(s.Config) t.Upstreams = UpstreamsToStructs(s.Upstreams) @@ -24,6 +25,7 @@ func NewConnectProxyConfigFromStructs(t structs.ConnectProxyConfig) ConnectProxy s.DestinationServiceID = t.DestinationServiceID s.LocalServiceAddress = t.LocalServiceAddress s.LocalServicePort = int32(t.LocalServicePort) + s.LocalServiceSocketPath = t.LocalServiceSocketPath s.Mode = t.Mode s.Config = MapStringInterfaceToProtobufTypesStruct(t.Config) s.Upstreams = NewUpstreamsFromStructs(t.Upstreams) diff --git a/proto/pbservice/service.proto b/proto/pbservice/service.proto index 1fcee0523564..4c601ac56b8a 100644 --- a/proto/pbservice/service.proto +++ b/proto/pbservice/service.proto @@ -239,7 +239,7 @@ message ServiceDefinition { map Meta = 6; // mog: func-to=int func-from=int32 int32 Port = 7; - // Path for socket + // Path for socket string SocketPath = 18; CheckType Check = 8 [(gogoproto.nullable) = false]; // mog: func-to=CheckTypesToStructs func-from=NewCheckTypesFromStructs