diff --git a/.changelog/14433.txt b/.changelog/14433.txt new file mode 100644 index 000000000..25167320c --- /dev/null +++ b/.changelog/14433.txt @@ -0,0 +1,3 @@ +```release-note:bug +checks: If set, use proxy address for automatically added sidecar check instead of service address. +``` diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index 67850f9eb..d380d0d93 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -3764,7 +3764,7 @@ func testAgent_RegisterService_TranslateKeys(t *testing.T, extraHCL string) { fmt.Println("TCP Check:= ", v) } if hasNoCorrectTCPCheck { - t.Fatalf("Did not find the expected TCP Healtcheck '%s' in %#v ", tt.expectedTCPCheckStart, a.checkTCPs) + t.Fatalf("Did not find the expected TCP Healthcheck '%s' in %#v ", tt.expectedTCPCheckStart, a.checkTCPs) } require.Equal(t, sidecarSvc, gotSidecar) }) diff --git a/agent/sidecar_service.go b/agent/sidecar_service.go index e0cb24a0e..a41d73d80 100644 --- a/agent/sidecar_service.go +++ b/agent/sidecar_service.go @@ -127,9 +127,20 @@ func (a *Agent) sidecarServiceFromNodeService(ns *structs.NodeService, token str if err != nil { return nil, nil, "", err } - // Setup default check if none given + // Setup default check if none given. if len(checks) < 1 { - checks = sidecarDefaultChecks(ns.ID, sidecar.Proxy.LocalServiceAddress, sidecar.Port) + // The check should use the sidecar's address because it makes a request to the sidecar. + // If the sidecar's address is empty, we fall back to the address of the local service, as set in + // sidecar.Proxy.LocalServiceAddress, in the hope that the proxy is also accessible on that address + // (which in most cases it is because it's running as a sidecar in the same network). + // We could instead fall back to the address of the service as set by (ns.Address), but I've kept it using + // sidecar.Proxy.LocalServiceAddress so as to not change things too much in the + // process of fixing #14433. + checkAddress := sidecar.Address + if checkAddress == "" { + checkAddress = sidecar.Proxy.LocalServiceAddress + } + checks = sidecarDefaultChecks(ns.ID, checkAddress, sidecar.Port) } return sidecar, checks, token, nil @@ -202,14 +213,11 @@ func (a *Agent) sidecarPortFromServiceID(sidecarCompoundServiceID structs.Servic return sidecarPort, nil } -func sidecarDefaultChecks(serviceID string, localServiceAddress string, port int) []*structs.CheckType { - // Setup default check if none given +func sidecarDefaultChecks(serviceID string, address string, port int) []*structs.CheckType { return []*structs.CheckType{ { - Name: "Connect Sidecar Listening", - // Default to localhost rather than agent/service public IP. The checks - // can always be overridden if a non-loopback IP is needed. - TCP: ipaddr.FormatAddressPort(localServiceAddress, port), + Name: "Connect Sidecar Listening", + TCP: ipaddr.FormatAddressPort(address, port), Interval: 10 * time.Second, }, { diff --git a/agent/sidecar_service_test.go b/agent/sidecar_service_test.go index f095670ff..39ab854a6 100644 --- a/agent/sidecar_service_test.go +++ b/agent/sidecar_service_test.go @@ -215,6 +215,141 @@ func TestAgent_sidecarServiceFromNodeService(t *testing.T) { token: "foo", wantErr: "reserved for internal use", }, + { + name: "uses proxy address for check", + sd: &structs.ServiceDefinition{ + ID: "web1", + Name: "web", + Port: 1111, + Connect: &structs.ServiceConnect{ + SidecarService: &structs.ServiceDefinition{ + Address: "123.123.123.123", + Proxy: &structs.ConnectProxyConfig{ + LocalServiceAddress: "255.255.255.255", + }, + }, + }, + Address: "255.255.255.255", + }, + token: "foo", + wantNS: &structs.NodeService{ + EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(), + Kind: structs.ServiceKindConnectProxy, + ID: "web1-sidecar-proxy", + Service: "web-sidecar-proxy", + Port: 2222, + Address: "123.123.123.123", + LocallyRegisteredAsSidecar: true, + Proxy: structs.ConnectProxyConfig{ + DestinationServiceName: "web", + DestinationServiceID: "web1", + LocalServiceAddress: "255.255.255.255", + LocalServicePort: 1111, + }, + }, + wantChecks: []*structs.CheckType{ + { + Name: "Connect Sidecar Listening", + TCP: "123.123.123.123:2222", + Interval: 10 * time.Second, + }, + { + Name: "Connect Sidecar Aliasing web1", + AliasService: "web1", + }, + }, + wantToken: "foo", + }, + { + name: "uses proxy.local_service_address for check if proxy address is empty", + sd: &structs.ServiceDefinition{ + ID: "web1", + Name: "web", + Port: 1111, + Connect: &structs.ServiceConnect{ + SidecarService: &structs.ServiceDefinition{ + Address: "", // Proxy address empty. + Proxy: &structs.ConnectProxyConfig{ + LocalServiceAddress: "1.2.3.4", + }, + }, + }, + Address: "", // Service address empty. + }, + token: "foo", + wantNS: &structs.NodeService{ + EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(), + Kind: structs.ServiceKindConnectProxy, + ID: "web1-sidecar-proxy", + Service: "web-sidecar-proxy", + Port: 2222, + Address: "", + LocallyRegisteredAsSidecar: true, + Proxy: structs.ConnectProxyConfig{ + DestinationServiceName: "web", + DestinationServiceID: "web1", + LocalServiceAddress: "1.2.3.4", + LocalServicePort: 1111, + }, + }, + wantChecks: []*structs.CheckType{ + { + Name: "Connect Sidecar Listening", + TCP: "1.2.3.4:2222", + Interval: 10 * time.Second, + }, + { + Name: "Connect Sidecar Aliasing web1", + AliasService: "web1", + }, + }, + wantToken: "foo", + }, + { + name: "uses 127.0.0.1 for check if proxy and proxy.local_service_address are empty", + sd: &structs.ServiceDefinition{ + ID: "web1", + Name: "web", + Port: 1111, + Connect: &structs.ServiceConnect{ + SidecarService: &structs.ServiceDefinition{ + Address: "", + Proxy: &structs.ConnectProxyConfig{ + LocalServiceAddress: "", + }, + }, + }, + Address: "", + }, + token: "foo", + wantNS: &structs.NodeService{ + EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(), + Kind: structs.ServiceKindConnectProxy, + ID: "web1-sidecar-proxy", + Service: "web-sidecar-proxy", + Port: 2222, + Address: "", + LocallyRegisteredAsSidecar: true, + Proxy: structs.ConnectProxyConfig{ + DestinationServiceName: "web", + DestinationServiceID: "web1", + LocalServiceAddress: "127.0.0.1", + LocalServicePort: 1111, + }, + }, + wantChecks: []*structs.CheckType{ + { + Name: "Connect Sidecar Listening", + TCP: "127.0.0.1:2222", + Interval: 10 * time.Second, + }, + { + Name: "Connect Sidecar Aliasing web1", + AliasService: "web1", + }, + }, + wantToken: "foo", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {