Use proxy address for default check

When a sidecar proxy is registered, a check is automatically added. Previously, the address this check used was the underlying service's address instead of the proxy's address, even though the check is testing if the proxy is up. This worked in most cases because the proxy ran on the same IP as the underlying service but it's not guaranteed and so the proper default address should be the proxy's address.
hashicorp · Sep 1, 2022 · 5ef875e · 5ef875e
1 parent d037ccf
commit 5ef875e
Show file tree

Hide file tree

Showing 3 changed files with 142 additions and 6 deletions.
diff --git a/.changelog/14433.txt b/.changelog/14433.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+checks: Use proxy address for automatically added sidecar check instead of service address.
+```
diff --git a/agent/sidecar_service.go b/agent/sidecar_service.go
@@ -129,7 +129,7 @@ func (a *Agent) sidecarServiceFromNodeService(ns *structs.NodeService, token str
 	}
 	// Setup default check if none given
 	if len(checks) < 1 {
-		checks = sidecarDefaultChecks(ns.ID, sidecar.Proxy.LocalServiceAddress, sidecar.Port)
+		checks = sidecarDefaultChecks(ns.ID, sidecar.Address, sidecar.Port)
 	}
 
 	return sidecar, checks, token, nil
@@ -202,14 +202,16 @@ func (a *Agent) sidecarPortFromServiceID(sidecarCompoundServiceID structs.Servic
 	return sidecarPort, nil
 }
 
-func sidecarDefaultChecks(serviceID string, localServiceAddress string, port int) []*structs.CheckType {
+func sidecarDefaultChecks(serviceID string, proxyAddress string, port int) []*structs.CheckType {
+	if proxyAddress == "" {
+		proxyAddress = "127.0.0.1"
+	}
+
 	// Setup default check if none given
 	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(proxyAddress, port),
 			Interval: 10 * time.Second,
 		},
 		{

diff --git a/agent/sidecar_service_test.go b/agent/sidecar_service_test.go
@@ -215,6 +215,137 @@ 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 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:   &structs.ConnectProxyConfig{},
+					},
+				},
+				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:                    "255.255.255.255",
+				LocallyRegisteredAsSidecar: true,
+				Proxy: structs.ConnectProxyConfig{
+					DestinationServiceName: "web",
+					DestinationServiceID:   "web1",
+					LocalServiceAddress:    "127.0.0.1",
+					LocalServicePort:       1111,
+				},
+			},
+			wantChecks: []*structs.CheckType{
+				{
+					Name:     "Connect Sidecar Listening",
+					TCP:      "255.255.255.255: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 service addresses are empty",
+			sd: &structs.ServiceDefinition{
+				ID:   "web1",
+				Name: "web",
+				Port: 1111,
+				Connect: &structs.ServiceConnect{
+					SidecarService: &structs.ServiceDefinition{
+						Address: "",
+						Proxy:   &structs.ConnectProxyConfig{},
+					},
+				},
+				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) {