Creation of Ingress resource with ssl-passthrough=true causes TLS configuration to be lost

[adriansuarez]

Description

When an Ingress resource is created that has annotation ingress.kubernetes.io/ssl-passthrough: "true", this causes the configured TLS key needed to terminate HTTPS connections for all non-ssl-passthrough resources to be removed from haproxy.cfg, breaking HTTPS L7 load balancing completely and causing all HTTPS requests to be downgraded to HTTP.

Reproduction case

1. Install haproxytech/kubernetes-ingress with version 1.8.0 or higher of the Ingress controller using the default settings. The default settings of the Helm charts generates a TLS key used to terminate HTTPS connections.
2. Wait about 10 seconds and capture haproxy.cfg:

kubectl exec "$HAPROXY_POD" -- cat /etc/haproxy/haproxy.cfg > haproxy-before.cfg

3. Create a Kubernetes service and Ingress resource that has ingress.kubernetes.io/ssl-passthrough: "true":

kubectl create service clusterip test --tcp=1234
kubectl apply -f - <<EOF
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: test
  annotations:
    ingress.kubernetes.io/ssl-passthrough: "true"
spec:
  rules:
  - host: "test.local"
    http:
      paths:
      - path: /
        pathType: Prefix
        backend:
          service:
            name: test
            port:
              number: 1234
EOF

4. Wait about 10 seconds and capture haproxy.cfg again:

kubectl exec "$HAPROXY_POD" -- cat /etc/haproxy/haproxy.cfg > haproxy-after.cfg

5. If there are any Ingress resources that define L7 rules (i.e. not having ingress.kubernetes.io/ssl-passthrough: "true"), they will not longer work with HTTPS.
6. Comparing haproxy-before.cfg and haproxy-after.cfg reveals the problem. The TLS configuration is no longer present in the bind entries of frontend https:

$ diff -u haproxy-before.cfg haproxy-after.cfg 
--- haproxy-before.cfg	2022-08-09 09:25:20.502046000 -0400
+++ haproxy-after.cfg	2022-08-09 09:25:38.127643000 -0400
@@ -1,4 +1,4 @@
-# _version=8
+# _version=9
 # HAProxy Technologies
 # https://www.haproxy.com/
 # this file is not meant to be changed directly
@@ -60,8 +60,8 @@
 
 frontend https
   mode http
-  bind 0.0.0.0:443 name v4 crt /etc/haproxy/certs/frontend ssl alpn h2,http/1.1
-  bind :::443 name v6 crt /etc/haproxy/certs/frontend ssl alpn h2,http/1.1
+  bind 127.0.0.1:443 name v4 accept-proxy
+  bind ::1:443 name v6 v4v6 accept-proxy
   http-request set-var(txn.base) base
   http-request set-var(txn.path) path
   http-request set-var(txn.host) req.hdr(Host),field(1,:),lower
...

Expected behavior

I would expect that HTTPS still works even if there are Ingress resources that have ingress.kubernetes.io/ssl-passthrough: "true", since those resources should only be used if they match the SNI attribute of the TLS ClientHello message. Otherwise, L7 load balancing should be used like normal to route traffic based on HTTP request parameters, with the Ingress controller terminating the TLS connection.

Environment

minikube: 1.24.0 kubernetes: 1.22.3 haproxytech/helm-charts: 1.22.4 haproxytech/kubernetes-ingress: reproduced with 1.8.0, 1.8.2, and 1.8.4 (1.7.11 and below do not seem to have this issue)

[ivanmatmati]

Hi @adriansuarez , thanks for reporting, I'll have a look as soon as possible.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[adriansuarez]

@ivanmatmati @oktalz I noticed that the "stale" label has been automatically applied to this issue. Hopefully it doesn't fall through the cracks, since this makes ssl-passthrough unusable for us in newer versions (we've pinned the version of the HAProxy Ingress at 1.7.11).

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[adriansuarez]

@ivanmatmati, @oktalz: I see that this was closed as stale. Has there been a fix of this that wasn't explicitly linked to this issue?