fix(interceptor): clean up direct-pod routing TLS and defaults

- Default DirectPodRouting to true (matching Knative behavior)
- Remove dead forwardingTLSCfg/TLSClientConfig code superseded by
  DialTLSContext
- Soften newTLSDialer to fall back to dial-address hostname when
  context has no server name, instead of hard-failing
- Fix notifyCh comment to reflect actual broadcast semantics
  (ready-state transitions only, not every change)
- Document dual-stack limitation in collectServiceState

Signed-off-by: Mikhail Fedosin <mfedosin@redhat.com>

Author: Fedosin

CHANGELOG.md

@@ -35,7 +35,7 @@ This changelog keeps track of work items that have been completed and are ready
 
 - **General**: Add `staticRoutes` to InterceptorRoute for defining routes that should not trigger autoscaling, such as health checks, redirects, and maintenance pages. Supports `responseMode: WhenUnavailable` (forward to backend when ready, static response otherwise) and `responseMode: Always` (always serve static response) ([#1622](https://github.com/kedacore/http-add-on/issues/1622))
 - **General**: TODO ([#TODO](https://github.com/kedacore/http-add-on/issues/TODO))
-- **Interceptor**: Add `KEDA_HTTP_DIRECT_POD_ROUTING` environment variable (`true` | `false`, default `false`). When enabled, the interceptor routes requests directly to a ready pod IP instead of through the Service ClusterIP, bypassing kube-proxy and other Service-layer features (Service-level NetworkPolicy, session affinity, topology-aware routing). ([#1473](https://github.com/kedacore/http-add-on/issues/1473))
+- **Interceptor**: Add `KEDA_HTTP_DIRECT_POD_ROUTING` environment variable (`true` | `false`, default `true`). When enabled, the interceptor routes requests directly to a ready pod IP instead of through the Service ClusterIP, bypassing kube-proxy and other Service-layer features (Service-level NetworkPolicy, session affinity, topology-aware routing). ([#1473](https://github.com/kedacore/http-add-on/issues/1473))
 
 ### Improvements
 

interceptor/config/serving.go

@@ -67,8 +67,11 @@ type Serving struct {
 	ShutdownDelay time.Duration `env:"KEDA_HTTP_SHUTDOWN_DELAY" envDefault:"5s"`
 	// DirectPodRouting routes requests to a ready pod IP instead of the Service
 	// ClusterIP, bypassing kube-proxy and other Service-layer features
-	// (NetworkPolicy, session affinity, topology-aware routing). Single-stack only.
-	DirectPodRouting bool `env:"KEDA_HTTP_DIRECT_POD_ROUTING" envDefault:"false"`
+	// (NetworkPolicy, session affinity, topology-aware routing).
+	// Note: in dual-stack clusters, endpoint selection does not distinguish
+	// address families, so a pod IPv6 address may be returned in an IPv4-only
+	// environment (or vice versa). Disable this flag if that is a concern.
+	DirectPodRouting bool `env:"KEDA_HTTP_DIRECT_POD_ROUTING" envDefault:"true"`
 }
 
 // MustParseServing parses standard configs and returns the

interceptor/proxy.go

@@ -3,7 +3,6 @@ package main
 import (
 	"context"
 	"crypto/tls"
-	"fmt"
 	"net"
 	"net/http"
 
@@ -52,34 +51,17 @@ func BuildProxyHandler(cfg *ProxyHandlerConfig) http.Handler {
 		}
 	}
 
-	var forwardingTLSCfg *tls.Config
-	if cfg.TLSConfig != nil {
-		forwardingTLSCfg = &tls.Config{
-			RootCAs:            cfg.TLSConfig.RootCAs,
-			Certificates:       cfg.TLSConfig.Certificates,
-			InsecureSkipVerify: cfg.TLSConfig.InsecureSkipVerify, //nolint:gosec // G402: user-configurable
-			MinVersion:         cfg.TLSConfig.MinVersion,
-			MaxVersion:         cfg.TLSConfig.MaxVersion,
-			CipherSuites:       cfg.TLSConfig.CipherSuites,
-			CurvePreferences:   cfg.TLSConfig.CurvePreferences,
-			// Advertise h2 in ALPN explicitly: a custom TLSClientConfig +
-			// DialTLSContext disables net/http's auto-h2, so without this HTTPS
-			// upstreams (incl. gRPC) downgrade to HTTP/1.1 (golang/go#20645).
-			NextProtos: []string{"h2", "http/1.1"},
-		}
-	}
-
 	// Clone DefaultTransport to inherit Go's defaults for IdleConnTimeout, ...
 	baseTransport := http.DefaultTransport.(*http.Transport).Clone()
 	baseTransport.DialContext = dialFunc
 	baseTransport.MaxIdleConns = cfg.Timeouts.MaxIdleConns
 	baseTransport.MaxIdleConnsPerHost = cfg.Timeouts.MaxIdleConnsPerHost
-	baseTransport.TLSClientConfig = forwardingTLSCfg
 
-	// Direct-pod routing rewrites the upstream URL to a pod IP, so SNI must come
-	// from context (the original service hostname) rather than the dial address.
-	if forwardingTLSCfg != nil {
-		baseTransport.DialTLSContext = newTLSDialer(forwardingTLSCfg, dialFunc)
+	// When TLS is enabled, use DialTLSContext to set ServerName per-dial from
+	// context. This is required for direct-pod routing where the URL host is a
+	// pod IP but SNI must remain the original service hostname.
+	if cfg.TLSConfig != nil {
+		baseTransport.DialTLSContext = newTLSDialer(cfg.TLSConfig, dialFunc)
 	}
 
 	// Build handler chain (innermost to outermost)
@@ -119,22 +101,28 @@ func BuildProxyHandler(cfg *ProxyHandlerConfig) http.Handler {
 }
 
 // newTLSDialer dials TCP then wraps the conn in TLS, taking ServerName from
-// context (not the dial address, which direct-pod routing rewrites to a pod IP).
-// The SNI should never be empty here as Routing sets it before any https dial;
-// the empty check is just a safety net so we fail closed instead of dialing
-// without an explicit upstream identity.
-func newTLSDialer(tlsCfg *tls.Config, dial func(ctx context.Context, network, addr string) (net.Conn, error)) func(ctx context.Context, network, addr string) (net.Conn, error) {
+// context (the original service hostname captured by routing middleware). When
+// direct-pod routing rewrites the URL to a pod IP, this ensures SNI stays the
+// service hostname. If the context has no server name (e.g. non-routing paths),
+// falls back to the hostname extracted from the dial address.
+func newTLSDialer(baseCfg *tls.Config, dial func(ctx context.Context, network, addr string) (net.Conn, error)) func(ctx context.Context, network, addr string) (net.Conn, error) {
 	return func(ctx context.Context, network, addr string) (net.Conn, error) {
-		serverName := util.UpstreamServerNameFromContext(ctx)
-		if serverName == "" {
-			return nil, fmt.Errorf("upstream server name missing from context for TLS dial to %s (refusing to dial without explicit SNI)", addr)
-		}
 		conn, err := dial(ctx, network, addr)
 		if err != nil {
 			return nil, err
 		}
-		dialed := tlsCfg.Clone()
-		dialed.ServerName = serverName
-		return tls.Client(conn, dialed), nil
+		tlsCfg := baseCfg.Clone()
+		// Advertise h2 in ALPN explicitly: a custom DialTLSContext disables
+		// net/http's auto-h2, so without this HTTPS upstreams (incl. gRPC)
+		// downgrade to HTTP/1.1 (golang/go#20645).
+		tlsCfg.NextProtos = []string{"h2", "http/1.1"}
+		tlsCfg.ServerName = util.UpstreamServerNameFromContext(ctx)
+		if tlsCfg.ServerName == "" {
+			host, _, err := net.SplitHostPort(addr)
+			if err == nil {
+				tlsCfg.ServerName = host
+			}
+		}
+		return tls.Client(conn, tlsCfg), nil
 	}
 }

interceptor/proxy_test.go

@@ -2,8 +2,14 @@ package main
 
 import (
 	"context"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	crand "crypto/rand"
 	"crypto/tls"
+	"crypto/x509"
+	"encoding/pem"
 	"errors"
+	"math/big"
 	"net"
 	"net/http"
 	"net/http/httptest"
@@ -408,64 +414,25 @@ func (h *proxyTestHarness) doRequest(t *testing.T, method, path, host string) *h
 }
 
 // TestNewTLSDialer_UsesServerNameFromContext verifies the happy path: when
-// Routing has stamped UpstreamServerName, the dialer uses it and proceeds
-// with the underlying TCP dial regardless of cert-verification policy.
+// Routing has stamped UpstreamServerName, the dialer uses it as SNI regardless
+// of what the dial address is (e.g. a pod IP).
 func TestNewTLSDialer_UsesServerNameFromContext(t *testing.T) {
-	for _, insecure := range []bool{false, true} {
-		var dialCalled bool
-		stubDial := func(_ context.Context, _, _ string) (net.Conn, error) {
-			dialCalled = true
-			c, _ := net.Pipe()
-			return c, nil
-		}
-		dialer := newTLSDialer(&tls.Config{InsecureSkipVerify: insecure}, stubDial)
-
-		ctx := util.ContextWithUpstreamServerName(context.Background(), "myservice.default")
-		conn, err := dialer(ctx, "tcp", "10.1.2.3:8443")
-		if err != nil {
-			t.Fatalf("InsecureSkipVerify=%v: unexpected error: %v", insecure, err)
-		}
-		if !dialCalled {
-			t.Errorf("InsecureSkipVerify=%v: dial was not called", insecure)
-		}
-		if conn == nil {
-			t.Errorf("InsecureSkipVerify=%v: returned conn was nil", insecure)
-		} else {
-			_ = conn.Close()
-		}
+	serverName := captureDialerSNI(t, &tls.Config{InsecureSkipVerify: true},
+		util.ContextWithUpstreamServerName(context.Background(), "myservice.default"),
+		"10.1.2.3:8443")
+	if serverName != "myservice.default" {
+		t.Fatalf("ServerName = %q, want %q (should use context, not dial addr)", serverName, "myservice.default")
 	}
 }
 
-// TestNewTLSDialer_HardFailsWhenNoSNI verifies the guardrail: a missing SNI
-// must error out without dialing, so we never open a TLS connection without an
-// explicit upstream identity. The error message must name both the dial address
-// and the reason, so operators don't mistake it for an upstream connectivity
-// failure. This holds regardless of InsecureSkipVerify.
-func TestNewTLSDialer_HardFailsWhenNoSNI(t *testing.T) {
-	for _, skipVerify := range []bool{true, false} {
-		var dialCalled bool
-		stubDial := func(_ context.Context, _, _ string) (net.Conn, error) {
-			dialCalled = true
-			return nil, nil
-		}
-		dialer := newTLSDialer(&tls.Config{InsecureSkipVerify: skipVerify}, stubDial)
-
-		conn, err := dialer(context.Background(), "tcp", "10.1.2.3:8443")
-		if err == nil {
-			t.Fatalf("expected error when SNI is missing (InsecureSkipVerify=%v), got nil", skipVerify)
-		}
-		if dialCalled {
-			t.Errorf("dial must not be called when we hard-fail on missing SNI (InsecureSkipVerify=%v)", skipVerify)
-		}
-		if conn != nil {
-			t.Errorf("returned conn must be nil on error (InsecureSkipVerify=%v)", skipVerify)
-		}
-		if !strings.Contains(err.Error(), "10.1.2.3:8443") {
-			t.Errorf("error %q should name the dial address", err)
-		}
-		if !strings.Contains(err.Error(), "refusing to dial without explicit SNI") {
-			t.Errorf("error %q should explain why we refuse to dial", err)
-		}
+// TestNewTLSDialer_FallsBackToAddrHostname verifies that when the context has
+// no UpstreamServerName, the dialer extracts the hostname from the dial address
+// and uses it as ServerName for SNI.
+func TestNewTLSDialer_FallsBackToAddrHostname(t *testing.T) {
+	serverName := captureDialerSNI(t, &tls.Config{InsecureSkipVerify: true},
+		context.Background(), "my-svc.default:8443")
+	if serverName != "my-svc.default" {
+		t.Fatalf("ServerName = %q, want %q (should fall back to addr hostname)", serverName, "my-svc.default")
 	}
 }
 
@@ -488,3 +455,85 @@ func TestNewTLSDialer_PropagatesDialError(t *testing.T) {
 		t.Error("returned conn must be nil on dial error")
 	}
 }
+
+// captureDialerSNI invokes newTLSDialer with a pipe-backed stub, initiates a
+// TLS handshake, and returns the ServerName the server saw in the ClientHello.
+// We only need the SNI from the first flight, so we abort via pipe close once
+// GetConfigForClient fires.
+func captureDialerSNI(t *testing.T, clientCfg *tls.Config, ctx context.Context, addr string) string {
+	t.Helper()
+	clientEnd, serverEnd := net.Pipe()
+
+	stubDial := func(_ context.Context, _, _ string) (net.Conn, error) {
+		return clientEnd, nil
+	}
+	dialer := newTLSDialer(clientCfg, stubDial)
+
+	sniCh := make(chan string, 1)
+	serverCert := generateSelfSignedCert(t)
+	serverTLSCfg := &tls.Config{
+		Certificates: []tls.Certificate{serverCert},
+		GetConfigForClient: func(hello *tls.ClientHelloInfo) (*tls.Config, error) {
+			sniCh <- hello.ServerName
+			return nil, nil
+		},
+	}
+
+	go func() {
+		srv := tls.Server(serverEnd, serverTLSCfg)
+		_ = srv.Handshake()
+		srv.Close()
+	}()
+
+	conn, err := dialer(ctx, "tcp", addr)
+	if err != nil {
+		t.Fatalf("dialer error: %v", err)
+	}
+
+	// Drive the handshake just far enough for the server to receive ClientHello.
+	go func() {
+		tlsConn := conn.(*tls.Conn)
+		_ = tlsConn.Handshake()
+	}()
+
+	select {
+	case sni := <-sniCh:
+		// Got the SNI, tear down both sides.
+		conn.Close()
+		serverEnd.Close()
+		return sni
+	case <-time.After(5 * time.Second):
+		conn.Close()
+		serverEnd.Close()
+		t.Fatal("timed out waiting for TLS ClientHello")
+		return ""
+	}
+}
+
+func generateSelfSignedCert(t *testing.T) tls.Certificate {
+	t.Helper()
+	key, err := ecdsa.GenerateKey(elliptic.P256(), crand.Reader)
+	if err != nil {
+		t.Fatalf("generate key: %v", err)
+	}
+	template := &x509.Certificate{
+		SerialNumber: big.NewInt(1),
+		NotBefore:    time.Now().Add(-time.Hour),
+		NotAfter:     time.Now().Add(time.Hour),
+	}
+	certDER, err := x509.CreateCertificate(crand.Reader, template, template, &key.PublicKey, key)
+	if err != nil {
+		t.Fatalf("create cert: %v", err)
+	}
+	keyDER, err := x509.MarshalECPrivateKey(key)
+	if err != nil {
+		t.Fatalf("marshal key: %v", err)
+	}
+	certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER})
+	keyPEM := pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: keyDER})
+	cert, err := tls.X509KeyPair(certPEM, keyPEM)
+	if err != nil {
+		t.Fatalf("load keypair: %v", err)
+	}
+	return cert
+}

pkg/k8s/ready_endpoints_cache.go

@@ -41,8 +41,9 @@ type ReadyEndpointsCache struct {
 	// "namespace/service" -> *serviceState
 	states sync.Map
 
-	// Broadcast mechanism: the channel is closed on any change,
-	// then replaced with a fresh one. Waiters select on the channel.
+	// Broadcast mechanism: the channel is closed on a ready-state transition
+	// (ready→not-ready or not-ready→ready), then replaced with a fresh one.
+	// Waiters select on the channel.
 	mu       sync.Mutex
 	notifyCh chan struct{}
 }
@@ -176,7 +177,11 @@ func (c *ReadyEndpointsCache) broadcast() {
 
 // collectServiceState builds an immutable serviceState snapshot. Each candidate
 // pairs a ready pod's address with a port from the same slice, keyed by portName
-// ("" for unnamed). Address families are not distinguished.
+// ("" for unnamed). Address families (IPv4/IPv6) are not distinguished: in
+// dual-stack clusters, candidates from both families are pooled together and
+// pickHost may return an IPv6 address even if the caller can only reach IPv4
+// targets (or vice versa). This is acceptable for single-stack clusters (the
+// common case) and a known limitation for dual-stack deployments.
 func collectServiceState(slices []*discov1.EndpointSlice) *serviceState {
 	// Dedup (ip, port) per portName so overlapping slices don't weight a pod twice.
 	seen := make(map[string]map[endpoint]struct{})