Repository: istio/istio (go)

Issue:
Socket hang up in AWS VPC
### Bug Description

I have a Node.js application deployed in an AWS EKS cluster and after deploying Istio in the cluster, I noticed sporadic `socket hang up` errors when making outgoing https requests from the Node.js application and keep alive is enabled in the https.Agent for those requests. There is no issue with http requests. The application is registered in Istio and all outgoing traffic is going through the istio-proxy.
I managed to create a very simple script which reproduces the issue consistently:
```JavaScript
const https = require("https");
const agent = new https.Agent({ keepAlive: true, maxSockets: 50 });
const log = (data) => console.log(`[${new Date()}] ${data}`);
const get = async () => {
    return await new Promise((resolve, reject) => {
        log("starting request");
        const req = https.request({ method: "GET", host: "ipv4.icanhazip.com", port: 443, agent }, (res) => {
            res.on("error", reject);
            let body = '';
            res.on("data", (chunk) => { body += chunk.toString(); });
            res.on("end", () => { log(body); resolve() });
        });

        req.on("socket", (s) => {
            s.on("connect", () => {
                log(`socket: ${s.localAddress}:${s.localPort}`);
                log(`remote: ${s.remoteAddress}:${s.remotePort}`);
            });
        });

        req.on("error", reject);
        req.end();
    });
};

const main = async () => {
    await get();
    await new Promise((resolve) => setTimeout(resolve, 6 * 60 * 1000));
    await get();
};

main().then(log).catch(log);
```

Steps to reproduce:
1. Deploy Istio using the Istio operator in EKS
```YAML
apiVersion: install.istio.io/v1alpha1
kind: IstioOperator
metadata:
  name: repro
  namespace: istio-system
spec:
  hub: gcr.io/istio-release
  profile: default
  tag: 1.12.1
  meshConfig:
    accessLogFile: /dev/stdout
    connectTimeout: 30s
    defaultConfig:
      interceptionMode: TPROXY
      terminationDrainDuration: 30s
```
2. Deploy the repro script in EKS:
```YAML
apiVersion: apps/v1
kind: Deployment
metadata:
  name: istio-repro
  labels:
    app: istio-repro
    sidecar.istio.io/inject: "true"
spec:
  selector:
    matchLabels:
      app: istio-repro
  template:
    metadata:
      labels:
        app: istio-repro
        sidecar.istio.io/inject: "true"
    spec:
      containers:
        - name: repro
          image: tmilanov/istio-socket-hang-up-repro:0.1.0
          imagePullPolicy: Always
          command:
            - sleep
            - "1000000000"
          resources:
            limits:
              cpu: 100m
              memory: 128Mi
```
3. Execute the repro script
```bash
kubectl exec $(kubectl get po | grep istio-repro | awk '{print $1}') -c repro -- node repro.js
```

I tested with the steps above in AWS EKS and in Kubernetes cluster deployed locally using Kind. I reproduced the issue in AWS, but I was not able to reproduce it in the local Kubernetes cluster.

I wrote the same script in Golang to test if the problem is in Node.js and I reproduced the issue with the Golang script in AWS (you can run the Golang script by running `kubectl exec $(kubectl get po | grep istio-repro | awk '{print $1}') -c repro -- go run repro.go`)

I tested on EC2 instances with different Linux-based operating systems (Bottlerocket and Ubuntu) using a custom docker image - `tmilanov/istio-socket-hang-up-troubleshoot:0.1.0`. The image contains the scripts for reproducing the issue, envoy with config which only contains the PassthroughCluster and BlackHoleCluster config which Istio generates, iptables script which applies the output iptables config which Istio applies in the istio-init container. I reproduced the issue on both operating systems with this image.

I also tested on EC2 instances deployed in the public subnets of my VPC and I **did not** reproduce the issue. All other tests which reproduced the issue were executed from instances in my private subnets which use VPC NAT gateway when making requests to the internet.

I don't have a lot of experience with iptables and envoy and I'm not sure if the image which I created to test directly on EC2 instances contains the correct config which Istio generates, but if it can help investigating, here are the steps to reproduce using the custom docker image:
1. `docker run -ti --cap-add=NET_ADMIN tmilanov/istio-socket-hang-up-troubleshoot:0.1.0 bash`
2. `./iptables.sh`
3. `./start-envoy.sh`
4. `node repro.js`

My workaround is to use `traffic.sidecar.istio.io/excludeOutboundPorts` and exclude port 443 but I'd appreciate if someone with more experience could take a look at this issue.

### Version

```prose
istioctl version:
client version: 1.11.3
control plane version: 1.12.1
data plane version: 1.12.1 (5 proxies)

kubectl version --short:
Client Version: v1.20.0
Server Version: v1.20.7-eks-d88609
```


### Additional Information

_No response_

Pull request:
Fix mesh tcp keepalive setting.
fix #36499 

currently mesh config tcp keepalive is ignored if there is no destination rule for a cluster.

Patch:
diff --git a/pilot/pkg/networking/core/v1alpha3/cluster.go b/pilot/pkg/networking/core/v1alpha3/cluster.go
index ebeb22743e..3f982e6b30 100644
--- a/pilot/pkg/networking/core/v1alpha3/cluster.go
+++ b/pilot/pkg/networking/core/v1alpha3/cluster.go
@@ -593,10 +593,9 @@ type upgradeTuple struct {
 	override    networking.ConnectionPoolSettings_HTTPSettings_H2UpgradePolicy
 }
 
-func applyTCPKeepalive(mesh *meshconfig.MeshConfig, c *cluster.Cluster, settings *networking.ConnectionPoolSettings) {
+func applyTCPKeepalive(mesh *meshconfig.MeshConfig, c *cluster.Cluster, tcp *networking.ConnectionPoolSettings_TCPSettings) {
 	// Apply Keepalive config only if it is configured in mesh config or in destination rule.
-	if mesh.TcpKeepalive != nil || settings.Tcp.TcpKeepalive != nil {
-
+	if mesh.TcpKeepalive != nil || (tcp != nil && tcp.TcpKeepalive != nil) {
 		// Start with empty tcp_keepalive, which would set SO_KEEPALIVE on the socket with OS default values.
 		c.UpstreamConnectionOptions = &cluster.UpstreamConnectionOptions{
 			TcpKeepalive: &core.TcpKeepalive{},
@@ -608,8 +607,8 @@ func applyTCPKeepalive(mesh *meshconfig.MeshConfig, c *cluster.Cluster, settings
 		}
 
 		// Apply/Override individual attributes with DestinationRule TCP keepalive if set.
-		if settings.Tcp.TcpKeepalive != nil {
-			setKeepAliveSettings(c, settings.Tcp.TcpKeepalive)
+		if tcp != nil && tcp.TcpKeepalive != nil {
+			setKeepAliveSettings(c, tcp.TcpKeepalive)
 		}
 	}
 }
diff --git a/pilot/pkg/networking/core/v1alpha3/cluster_builder.go b/pilot/pkg/networking/core/v1alpha3/cluster_builder.go
index b460ec02fb..60a6707411 100644
--- a/pilot/pkg/networking/core/v1alpha3/cluster_builder.go
+++ b/pilot/pkg/networking/core/v1alpha3/cluster_builder.go
@@ -919,16 +919,16 @@ func (cb *ClusterBuilder) applyConnectionPool(mesh *meshconfig.MeshConfig, mc *M
 	}
 
 	cb.applyDefaultConnectionPool(mc.cluster)
-	if settings.Tcp != nil {
-		if settings.Tcp.ConnectTimeout != nil {
+	if settings.Tcp != nil || mesh.TcpKeepalive != nil {
+		if settings.Tcp != nil && settings.Tcp.ConnectTimeout != nil {
 			mc.cluster.ConnectTimeout = gogo.DurationToProtoDuration(settings.Tcp.ConnectTimeout)
 		}
 
-		if settings.Tcp.MaxConnections > 0 {
+		if settings.Tcp != nil && settings.Tcp.MaxConnections > 0 {
 			threshold.MaxConnections = &wrappers.UInt32Value{Value: uint32(settings.Tcp.MaxConnections)}
 		}
 
-		applyTCPKeepalive(mesh, mc.cluster, settings)
+		applyTCPKeepalive(mesh, mc.cluster, settings.Tcp)
 	}
 
 	mc.cluster.CircuitBreakers = &cluster.CircuitBreakers{
diff --git a/releasenotes/notes/36499.yaml b/releasenotes/notes/36499.yaml
new file mode 100644
index 0000000000..d97be859b1
--- /dev/null
+++ b/releasenotes/notes/36499.yaml
@@ -0,0 +1,8 @@
+apiVersion: release-notes/v2
+kind: bug-fix
+area: traffic-management
+issue:
+  - https://github.com/istio/istio/issues/36499
+releaseNotes:
+  - |
+    **Fixed** an issue where TcpKeepalive setting at mesh config is not honored.
diff --git a/samples/addons/extras/zipkin.yaml b/samples/addons/extras/zipkin.yaml
index 516b8e8fa9..10dcd0d88a 100644
--- a/samples/addons/extras/zipkin.yaml
+++ b/samples/addons/extras/zipkin.yaml
@@ -18,7 +18,7 @@ spec:
     spec:
       containers:
         - name: zipkin
-          image: openzipkin/zipkin-slim:2.23.0
+          image: openzipkin/zipkin-slim:2.23.14
           env:
             - name: STORAGE_METHOD
               value: "mem"


Test patch:
diff --git a/pilot/pkg/networking/core/v1alpha3/cluster_builder_test.go b/pilot/pkg/networking/core/v1alpha3/cluster_builder_test.go
index dc95f3731c..801a51ee38 100644
--- a/pilot/pkg/networking/core/v1alpha3/cluster_builder_test.go
+++ b/pilot/pkg/networking/core/v1alpha3/cluster_builder_test.go
@@ -27,6 +27,7 @@ import (
 	core "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
 	endpoint "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
 	tls "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3"
+	"github.com/gogo/protobuf/types"
 	"github.com/google/go-cmp/cmp"
 	"google.golang.org/protobuf/testing/protocmp"
 	"google.golang.org/protobuf/types/known/durationpb"
@@ -3062,3 +3063,67 @@ func TestApplyDestinationRuleOSCACert(t *testing.T) {
 		})
 	}
 }
+
+func TestApplyTCPKeepalive(t *testing.T) {
+	cases := []struct {
+		name           string
+		mesh           *meshconfig.MeshConfig
+		connectionPool *networking.ConnectionPoolSettings
+		wantConnOpts   *cluster.UpstreamConnectionOptions
+	}{
+		{
+			name:           "no tcp alive",
+			mesh:           &meshconfig.MeshConfig{},
+			connectionPool: &networking.ConnectionPoolSettings{},
+			wantConnOpts:   nil,
+		},
+		{
+			name: "destination rule tcp alive",
+			mesh: &meshconfig.MeshConfig{},
+			connectionPool: &networking.ConnectionPoolSettings{
+				Tcp: &networking.ConnectionPoolSettings_TCPSettings{
+					TcpKeepalive: &networking.ConnectionPoolSettings_TCPSettings_TcpKeepalive{
+						Time: &types.Duration{Seconds: 10},
+					},
+				},
+			},
+			wantConnOpts: &cluster.UpstreamConnectionOptions{
+				TcpKeepalive: &core.TcpKeepalive{
+					KeepaliveTime: &wrappers.UInt32Value{Value: uint32(10)},
+				},
+			},
+		},
+		{
+			name: "mesh tcp alive",
+			mesh: &meshconfig.MeshConfig{
+				TcpKeepalive: &networking.ConnectionPoolSettings_TCPSettings_TcpKeepalive{
+					Time: &types.Duration{Seconds: 10},
+				},
+			},
+			connectionPool: &networking.ConnectionPoolSettings{},
+			wantConnOpts: &cluster.UpstreamConnectionOptions{
+				TcpKeepalive: &core.TcpKeepalive{
+					KeepaliveTime: &wrappers.UInt32Value{Value: uint32(10)},
+				},
+			},
+		},
+	}
+
+	for _, tt := range cases {
+		t.Run(tt.name, func(t *testing.T) {
+			cg := NewConfigGenTest(t, TestOptions{})
+			proxy := cg.SetupProxy(nil)
+			cb := NewClusterBuilder(proxy, &model.PushRequest{Push: cg.PushContext()}, nil)
+			mc := &MutableCluster{
+				cluster: &cluster.Cluster{Name: "foo", ClusterDiscoveryType: &cluster.Cluster_Type{Type: cluster.Cluster_EDS}},
+			}
+
+			cb.applyConnectionPool(tt.mesh, mc, tt.connectionPool)
+
+			if !reflect.DeepEqual(tt.wantConnOpts, mc.cluster.UpstreamConnectionOptions) {
+				t.Errorf("unexpected tcp keepalive settings, want %v, got %v", tt.wantConnOpts,
+					mc.cluster.UpstreamConnectionOptions)
+			}
+		})
+	}
+}