diff --git a/.github/workflows/apisix-conformance-test.yml b/.github/workflows/apisix-conformance-test.yml index dbff2fba..cd9bd03d 100644 --- a/.github/workflows/apisix-conformance-test.yml +++ b/.github/workflows/apisix-conformance-test.yml @@ -32,6 +32,9 @@ concurrency: permissions: pull-requests: write +env: + ADC_VERSION: dev + jobs: conformance-test: env: @@ -70,7 +73,6 @@ jobs: ARCH: amd64 ENABLE_PROXY: "false" BASE_IMAGE_TAG: "debug" - # ADC_VERSION: "dev" run: | echo "building images..." make build-image diff --git a/Makefile b/Makefile index 73cf9284..260793e4 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ KIND_NAME ?= apisix-ingress-cluster KIND_NODE_IMAGE ?= kindest/node:v1.32.2@sha256:f226345927d7e348497136874b6d207e0b32cc52154ad8323129352923a3142f DASHBOARD_VERSION ?= dev -ADC_VERSION ?= 0.29.0 +ADC_VERSION ?= 0.31.0 DIR := $(shell pwd) diff --git a/api/adc/types.go b/api/adc/types.go index c7c67019..bae1a373 100644 --- a/api/adc/types.go +++ b/api/adc/types.go @@ -181,7 +181,18 @@ type StreamRoute struct { RemoteAddr string `json:"remote_addr,omitempty"` ServerAddr string `json:"server_addr,omitempty"` ServerPort int32 `json:"server_port,omitempty"` - SNI string `json:"sni,omitempty"` + // SNI and SNIs are the singular and plural forms of the same match; APISIX + // rejects a stream route carrying both, so only one is ever set. + SNI string `json:"sni,omitempty"` + SNIs []string `json:"snis,omitempty"` + // TLSPassthrough forwards the TLS stream to the upstream untouched instead + // of terminating it on the gateway. + // + // A listen carrying tls_passthrough alone passes every connection through + // and never reads this; a listen carrying tls alone always terminates. It + // decides only on a listen configured with both, where the matched stream + // route picks between the terminating and the prereading server. + TLSPassthrough *bool `json:"tls_passthrough,omitempty"` } // +k8s:deepcopy-gen=true diff --git a/api/adc/zz_generated.deepcopy.go b/api/adc/zz_generated.deepcopy.go index fc11631f..6e413a2f 100644 --- a/api/adc/zz_generated.deepcopy.go +++ b/api/adc/zz_generated.deepcopy.go @@ -567,6 +567,16 @@ func (in *StreamRoute) DeepCopyInto(out *StreamRoute) { *out = *in in.Metadata.DeepCopyInto(&out.Metadata) out.Plugins = in.Plugins.DeepCopy() + if in.SNIs != nil { + in, out := &in.SNIs, &out.SNIs + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.TLSPassthrough != nil { + in, out := &in.TLSPassthrough, &out.TLSPassthrough + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StreamRoute. diff --git a/docs/en/latest/concepts/gateway-api.md b/docs/en/latest/concepts/gateway-api.md index 27f00358..c78db3df 100644 --- a/docs/en/latest/concepts/gateway-api.md +++ b/docs/en/latest/concepts/gateway-api.md @@ -85,6 +85,6 @@ The fields below are specified in the Gateway API specification but are either p | `spec.listeners[].port` | Not supported* | The configuration is required but ignored. This is due to limitations in the data plane: it cannot dynamically open new ports. Since the Ingress Controller does not manage the data plane deployment, it cannot automatically update the configuration or restart the data plane to apply port changes. | | `spec.listeners[].tls.certificateRefs[].group` | Partially supported | Only `""` is supported; other group values cause validation failure. | | `spec.listeners[].tls.certificateRefs[].kind` | Partially supported | Only `Secret` is supported. | -| `spec.listeners[].tls.mode` | Partially supported | `Terminate` is implemented; `Passthrough` is effectively unsupported for Gateway listeners. | +| `spec.listeners[].tls.mode` | Partially supported | `Terminate` and `Passthrough` are both implemented. A `Passthrough` listener needs APISIX to be listening on that port with [`tls_passthrough`](https://apisix.apache.org/docs/apisix/stream-proxy/); the controller cannot open data plane ports itself. A single port cannot mix the two modes. Within one Gateway, listeners that disagree on `tls.mode` for one port are reported `Accepted=False` with `ProtocolConflict` and carry no routes, and a route attached to two Gateways that disagree about the same port falls back to `Terminate`, because a physical stream listen has a single mode. Two Gateways that disagree are each valid on their own, so when different routes attach to each of them no conflict is reported and every route is translated for the mode its own listener declares — only one of which the listen can honour. The controller does not configure the stream listen, so it cannot detect or resolve that: declare one `tls.mode` per port across Gateways that share a data plane. | | `spec.listeners[].tls.frontendValidation` | Partially supported | Enables downstream (client) mTLS. `caCertificateRefs` may reference a `ConfigMap` (Gateway API Core support) or a `Secret` (implementation-specific) holding the CA certificate under the `ca.crt` key; clients are then required to present a certificate signed by one of the referenced CAs. | | `spec.addresses` | Not supported | Controller does not read or act on `spec.addresses`. | diff --git a/internal/adc/translator/l4route_test.go b/internal/adc/translator/l4route_test.go index 5ac129da..dac18a91 100644 --- a/internal/adc/translator/l4route_test.go +++ b/internal/adc/translator/l4route_test.go @@ -291,13 +291,20 @@ func TestTranslateTLSRouteWithL4RoutePolicy(t *testing.T) { require.NoError(t, err) require.Len(t, result.Services, 1) - // Verify stream routes are created per SNI hostname - if len(tt.hostnames) > 0 { - assert.Len(t, result.Services[0].StreamRoutes, len(tt.hostnames)) + // One stream route carries every hostname: the singular sni for a + // single one, the plural snis beyond that. APISIX rejects both at once. + require.Len(t, result.Services[0].StreamRoutes, 1) + switch len(tt.hostnames) { + case 1: + assert.Equal(t, tt.hostnames[0], result.Services[0].StreamRoutes[0].SNI) + assert.Empty(t, result.Services[0].StreamRoutes[0].SNIs) + default: + assert.Equal(t, tt.hostnames, result.Services[0].StreamRoutes[0].SNIs) + assert.Empty(t, result.Services[0].StreamRoutes[0].SNI) } // Plugins are attached at the stream_route level so the APISIX stream proxy - // applies them; with multiple SNIs each stream_route carries its own copy. + // applies them. require.NotEmpty(t, result.Services[0].StreamRoutes) plugins := result.Services[0].StreamRoutes[0].Plugins if tt.wantNoPlugins { diff --git a/internal/adc/translator/tlsroute.go b/internal/adc/translator/tlsroute.go index 2faa16b6..7d584f51 100644 --- a/internal/adc/translator/tlsroute.go +++ b/internal/adc/translator/tlsroute.go @@ -20,11 +20,13 @@ package translator import ( "fmt" + "k8s.io/utils/ptr" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" adctypes "github.com/apache/apisix-ingress-controller/api/adc" apiv2 "github.com/apache/apisix-ingress-controller/api/v2" "github.com/apache/apisix-ingress-controller/internal/controller/label" + cutils "github.com/apache/apisix-ingress-controller/internal/controller/utils" "github.com/apache/apisix-ingress-controller/internal/id" "github.com/apache/apisix-ingress-controller/internal/provider" "github.com/apache/apisix-ingress-controller/internal/types" @@ -34,10 +36,6 @@ func (t *Translator) TranslateTLSRoute(tctx *provider.TranslateContext, tlsRoute result := &TranslateResult{} rules := tlsRoute.Spec.Rules labels := label.GenLabel(tlsRoute) - hosts := make([]string, 0, len(tlsRoute.Spec.Hostnames)) - for _, hostname := range tlsRoute.Spec.Hostnames { - hosts = append(hosts, string(hostname)) - } for ruleIndex, rule := range rules { service := adctypes.NewDefaultService() service.Labels = labels @@ -143,18 +141,125 @@ func (t *Translator) TranslateTLSRoute(tctx *provider.TranslateContext, tlsRoute } } - for _, host := range hosts { - streamRoutes, err := t.buildL4StreamRoutes(tctx, tlsRoute.Namespace, tlsRoute.Name, ruleIndex, "TLS", "TLSRoute", labels) - if err != nil { - return nil, err + // One set of stream routes per rule, each carrying the SNIs of its own + // port, rather than one set per hostname: buildL4StreamRoutes names them by + // rule and port only, so a per hostname loop produced N sets sharing one ID + // and only the last survived. + streamRoutes, err := t.buildL4StreamRoutes(tctx, tlsRoute.Namespace, tlsRoute.Name, ruleIndex, "TLS", "TLSRoute", labels) + if err != nil { + return nil, err + } + for _, streamRoute := range streamRoutes { + snis := tlsRouteSNIsForPort(tctx.Listeners, tlsRoute.Spec.Hostnames, streamRoute.ServerPort) + if len(snis) == 0 { + // Nothing for the stream route to match on. The listeners a route + // attaches to already intersect its hostnames, so this is a guard + // against emitting a route that would answer every SNI instead. + continue } - for _, streamRoute := range streamRoutes { - streamRoute.SNI = host + // A single SNI keeps using the singular form: it is what every + // APISIX version understands, and snis only earns its place once + // there is more than one to match. + if len(snis) == 1 { + streamRoute.SNI = snis[0] + } else { + streamRoute.SNIs = snis + } + if tlsPassthroughOnPort(tctx.Listeners, streamRoute.ServerPort) { + streamRoute.TLSPassthrough = ptr.To(true) } - service.StreamRoutes = append(service.StreamRoutes, streamRoutes...) + service.StreamRoutes = append(service.StreamRoutes, streamRoute) } result.Services = append(result.Services, service) } return result, nil } + +// tlsRouteSNIsForPort returns the SNIs the stream route bound to port matches on. +// +// The set is derived from the listeners on that port alone. Handing every port +// the route-wide union instead would let a port answer a hostname its own +// listener never accepted, and would cost a port its catch-all as soon as a +// listener on another port named a hostname. port 0 means the stream route +// carries no server_port match, so every matched listener applies. +// +// A TLSRoute without hostnames matches everything its listeners accept, so it +// falls back to the hostnames of the listeners on the port and, when one of them +// carries no hostname either, to the catch-all "*". Emitting nothing - which is +// what the per hostname loop used to do - left such a route attached but +// unserved. +func tlsRouteSNIsForPort(listeners []gatewayv1.Listener, routeHostnames []gatewayv1.Hostname, port int32) []string { + matched := make([]gatewayv1.Listener, 0, len(listeners)) + for _, listener := range listeners { + if listener.Protocol != gatewayv1.TLSProtocolType { + continue + } + if port != 0 && listener.Port != port { + continue + } + matched = append(matched, listener) + } + + // No listener to narrow against: keep what the route itself asks for. + if len(matched) == 0 { + if len(routeHostnames) == 0 { + return []string{"*"} + } + return hostnamesToSNIs(routeHostnames) + } + + if len(routeHostnames) == 0 { + hostnames, matchAnyHost := cutils.UnionHostnames(matched) + if matchAnyHost { + return []string{"*"} + } + return hostnamesToSNIs(cutils.DedupHostnames(hostnames)) + } + + snis := make([]gatewayv1.Hostname, 0, len(routeHostnames)) + for _, hostname := range routeHostnames { + snis = append(snis, cutils.NarrowHostname(matched, hostname)...) + } + return hostnamesToSNIs(cutils.DedupHostnames(snis)) +} + +func hostnamesToSNIs(hostnames []gatewayv1.Hostname) []string { + snis := make([]string, 0, len(hostnames)) + for _, hostname := range hostnames { + snis = append(snis, string(hostname)) + } + return snis +} + +// tlsPassthroughOnPort reports whether the stream routes bound to port must +// forward the connection untouched instead of having the gateway terminate it. +// port 0 means the StreamRoute carries no server_port match, so every matched +// listener applies. +// +// Every TLS listener this route matched on the port has to agree. Within one +// Gateway a port carrying both modes is already reported ProtocolConflict and +// attaches no routes; a route attached to two Gateways that disagree falls back +// to terminating rather than to a guess, since the physical stream listen has a +// single mode. +// +// Two *different* routes attached to two disagreeing Gateways are each +// translated for the mode their own listener declares, and only one of those is +// the mode the listen was configured with. The controller does not configure the +// stream listen, so it can neither detect that nor resolve it. +func tlsPassthroughOnPort(listeners []gatewayv1.Listener, port int32) bool { + matched := false + for _, listener := range listeners { + if listener.Protocol != gatewayv1.TLSProtocolType { + continue + } + if port != 0 && listener.Port != port { + continue + } + if listener.TLS == nil || listener.TLS.Mode == nil || *listener.TLS.Mode != gatewayv1.TLSModePassthrough { + return false + } + matched = true + } + return matched +} diff --git a/internal/adc/translator/tlsroute_test.go b/internal/adc/translator/tlsroute_test.go new file mode 100644 index 00000000..b207da8b --- /dev/null +++ b/internal/adc/translator/tlsroute_test.go @@ -0,0 +1,275 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package translator + +import ( + "context" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/apache/apisix-ingress-controller/internal/controller/config" + "github.com/apache/apisix-ingress-controller/internal/provider" +) + +func tlsModeListener(name string, port int32, mode *gatewayv1.TLSModeType, hostname string) gatewayv1.Listener { + listener := gatewayv1.Listener{ + Name: gatewayv1.SectionName(name), + Protocol: gatewayv1.TLSProtocolType, + Port: port, + TLS: &gatewayv1.ListenerTLSConfig{Mode: mode}, + } + if hostname != "" { + listener.Hostname = ptr.To(gatewayv1.Hostname(hostname)) + } + return listener +} + +func translateTLSRoute(t *testing.T, listeners []gatewayv1.Listener, explicit bool, hostnames ...string) *TranslateResult { + t.Helper() + + // listener_port_match_mode defaults to off; auto is what the e2e and + // conformance runs use, and what makes the per-port fan-out observable. + translator := NewTranslator(logr.Discard(), config.ListenerPortMatchModeAuto) + tctx := provider.NewDefaultTranslateContext(context.Background()) + tctx.Listeners = listeners + tctx.HasExplicitListenerMatch = explicit + + specHostnames := make([]gatewayv1.Hostname, 0, len(hostnames)) + for _, hostname := range hostnames { + specHostnames = append(specHostnames, gatewayv1.Hostname(hostname)) + } + + result, err := translator.TranslateTLSRoute(tctx, &gatewayv1.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{Name: "my-tls", Namespace: "default"}, + Spec: gatewayv1.TLSRouteSpec{ + Hostnames: specHostnames, + Rules: []gatewayv1.TLSRouteRule{ + {BackendRefs: []gatewayv1.BackendRef{}}, + }, + }, + }) + require.NoError(t, err) + require.Len(t, result.Services, 1) + return result +} + +func TestTranslateTLSRouteTLSPassthrough(t *testing.T) { + terminate := gatewayv1.TLSModeTerminate + passthrough := gatewayv1.TLSModePassthrough + + for _, tc := range []struct { + name string + listeners []gatewayv1.Listener + explicit bool + want []*bool + }{ + { + name: "passthrough listener asks the data plane to pass the stream through", + listeners: []gatewayv1.Listener{tlsModeListener("tls", 9110, &passthrough, "")}, + explicit: true, + want: []*bool{ptr.To(true)}, + }, + { + name: "terminate listener leaves the flag off", + listeners: []gatewayv1.Listener{tlsModeListener("tls", 9110, &terminate, "")}, + explicit: true, + want: []*bool{nil}, + }, + { + // tls.mode is optional and defaults to Terminate. + name: "omitted mode is terminate", + listeners: []gatewayv1.Listener{tlsModeListener("tls", 9110, nil, "")}, + explicit: true, + want: []*bool{nil}, + }, + { + name: "each port carries the mode of its own listener", + listeners: []gatewayv1.Listener{ + tlsModeListener("terminate", 9110, &terminate, ""), + tlsModeListener("passthrough", 9120, &passthrough, ""), + }, + want: []*bool{nil, ptr.To(true)}, + }, + { + // A physical stream listen has one mode, so a port whose matched + // listeners disagree cannot be served both ways; terminating is + // what the gateway did before passthrough existed. + name: "listeners disagreeing on one port fall back to terminate", + listeners: []gatewayv1.Listener{ + tlsModeListener("a", 9110, &passthrough, ""), + tlsModeListener("b", 9110, &terminate, ""), + }, + explicit: true, + want: []*bool{nil}, + }, + { + // No listener port to pin to: the single portless StreamRoute takes + // the mode every matched listener agrees on. + name: "portless stream route still follows the listener mode", + listeners: []gatewayv1.Listener{tlsModeListener("tls", 9110, &passthrough, "")}, + want: []*bool{ptr.To(true)}, + }, + { + name: "no matched listener leaves the flag off", + listeners: nil, + want: []*bool{nil}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + result := translateTLSRoute(t, tc.listeners, tc.explicit, "example.com") + + streamRoutes := result.Services[0].StreamRoutes + require.Len(t, streamRoutes, len(tc.want)) + for i, want := range tc.want { + assert.Equal(t, want, streamRoutes[i].TLSPassthrough, "stream route %d", i) + } + }) + } +} + +func TestTranslateTLSRouteSNIs(t *testing.T) { + passthrough := gatewayv1.TLSModePassthrough + + t.Run("a single hostname uses the singular sni", func(t *testing.T) { + result := translateTLSRoute(t, nil, false, "example.com") + + streamRoutes := result.Services[0].StreamRoutes + require.Len(t, streamRoutes, 1) + assert.Equal(t, "example.com", streamRoutes[0].SNI) + assert.Empty(t, streamRoutes[0].SNIs) + }) + + t.Run("several hostnames share one stream route", func(t *testing.T) { + result := translateTLSRoute(t, nil, false, "a.example.com", "b.example.com") + + // Regression: every hostname used to produce its own StreamRoute under + // one name, so they collided on a single id and only the last survived. + streamRoutes := result.Services[0].StreamRoutes + require.Len(t, streamRoutes, 1) + assert.Equal(t, []string{"a.example.com", "b.example.com"}, streamRoutes[0].SNIs) + assert.Empty(t, streamRoutes[0].SNI) + }) + + t.Run("no hostname falls back to the listener hostnames", func(t *testing.T) { + result := translateTLSRoute(t, []gatewayv1.Listener{ + tlsModeListener("tls", 9110, &passthrough, "*.example.com"), + }, true) + + streamRoutes := result.Services[0].StreamRoutes + require.Len(t, streamRoutes, 1) + assert.Equal(t, "*.example.com", streamRoutes[0].SNI) + }) + + t.Run("no hostname anywhere falls back to the catch-all", func(t *testing.T) { + result := translateTLSRoute(t, []gatewayv1.Listener{ + tlsModeListener("tls", 9110, &passthrough, ""), + }, true) + + streamRoutes := result.Services[0].StreamRoutes + require.Len(t, streamRoutes, 1) + assert.Equal(t, "*", streamRoutes[0].SNI) + }) +} + +func TestTranslateTLSRouteSNIsPerPort(t *testing.T) { + passthrough := gatewayv1.TLSModePassthrough + + snisByPort := func(result *TranslateResult) map[int32][]string { + byPort := make(map[int32][]string) + for _, streamRoute := range result.Services[0].StreamRoutes { + snis := streamRoute.SNIs + if streamRoute.SNI != "" { + snis = []string{streamRoute.SNI} + } + byPort[streamRoute.ServerPort] = snis + } + return byPort + } + + // The route-wide union went to every port, so each port also answered the + // hostname only the other port's listener accepted. + t.Run("a hostname-less route takes the hostnames of its own port", func(t *testing.T) { + result := translateTLSRoute(t, []gatewayv1.Listener{ + tlsModeListener("a", 9110, &passthrough, "a.example.com"), + tlsModeListener("b", 9120, &passthrough, "b.example.com"), + }, true) + + assert.Equal(t, map[int32][]string{ + 9110: {"a.example.com"}, + 9120: {"b.example.com"}, + }, snisByPort(result)) + }) + + // A listener carrying no hostname accepts every name. Collecting hostnames + // route-wide cost that port its catch-all as soon as a listener on another + // port named a hostname. + t.Run("a port whose listener names no hostname keeps the catch-all", func(t *testing.T) { + result := translateTLSRoute(t, []gatewayv1.Listener{ + tlsModeListener("named", 9110, &passthrough, "a.example.com"), + tlsModeListener("any", 9120, &passthrough, ""), + }, true) + + assert.Equal(t, map[int32][]string{ + 9110: {"a.example.com"}, + 9120: {"*"}, + }, snisByPort(result)) + }) + + t.Run("route hostnames are narrowed to the listeners of each port", func(t *testing.T) { + result := translateTLSRoute(t, []gatewayv1.Listener{ + tlsModeListener("a", 9110, &passthrough, "a.example.com"), + tlsModeListener("b", 9120, &passthrough, "b.example.com"), + }, true, "*.example.com") + + assert.Equal(t, map[int32][]string{ + 9110: {"a.example.com"}, + 9120: {"b.example.com"}, + }, snisByPort(result)) + }) + + // Both listeners on the port are covered by the route's wildcard, so the + // route answers both names rather than only the first intersection. + t.Run("one port with several listeners keeps every intersection", func(t *testing.T) { + result := translateTLSRoute(t, []gatewayv1.Listener{ + tlsModeListener("a", 9110, &passthrough, "a.example.com"), + tlsModeListener("b", 9110, &passthrough, "b.example.com"), + }, true, "*.example.com") + + assert.Equal(t, map[int32][]string{ + 9110: {"a.example.com", "b.example.com"}, + }, snisByPort(result)) + }) + + // A listener hostname that covers the route hostname accepts it whole, so + // the route keeps its own name instead of the listener's. + t.Run("a wildcard listener keeps the route hostname", func(t *testing.T) { + result := translateTLSRoute(t, []gatewayv1.Listener{ + tlsModeListener("wildcard", 9110, &passthrough, "*.example.com"), + }, true, "a.example.com") + + assert.Equal(t, map[int32][]string{ + 9110: {"a.example.com"}, + }, snisByPort(result)) + }) +} diff --git a/internal/controller/listener_utils_test.go b/internal/controller/listener_utils_test.go index 9fa4cc9a..b852a94d 100644 --- a/internal/controller/listener_utils_test.go +++ b/internal/controller/listener_utils_test.go @@ -20,14 +20,19 @@ package controller import ( "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + cutils "github.com/apache/apisix-ingress-controller/internal/controller/utils" ) func listener(name string, port gatewayv1.PortNumber, hostname string) gatewayv1.Listener { l := gatewayv1.Listener{ - Name: gatewayv1.SectionName(name), - Port: port, + Name: gatewayv1.SectionName(name), + Protocol: gatewayv1.HTTPProtocolType, + Port: port, } if hostname != "" { h := gatewayv1.Hostname(hostname) @@ -35,19 +40,43 @@ func listener(name string, port gatewayv1.PortNumber, hostname string) gatewayv1 } return l } +func TestAppendListeners(t *testing.T) { + listenerA := gatewayv1.Listener{Name: "a", Port: 80} + listenerB := gatewayv1.Listener{Name: "b", Port: 81} + listenerA2 := gatewayv1.Listener{Name: "a", Port: 82} + listenerA3 := gatewayv1.Listener{Name: "a", Port: 80} -func TestAppendListenersKeepsSameNameOnDifferentPorts(t *testing.T) { - // Listener names are unique per Gateway only, and this slice spans every - // Gateway a route attaches to. - got := appendListeners(nil, - listener("http", 80, ""), - listener("http", 8080, ""), - listener("http", 80, ""), - ) + tests := []struct { + name string + target []gatewayv1.Listener + source []gatewayv1.Listener + expected []gatewayv1.Listener + }{ + { + name: "empty target, add listeners", + target: nil, + source: []gatewayv1.Listener{listenerA, listenerB}, + expected: []gatewayv1.Listener{listenerA, listenerB}, + }, + { + name: "preserves same listener names from different gateways", + target: []gatewayv1.Listener{listenerA}, + source: []gatewayv1.Listener{listenerA, listenerB}, + expected: []gatewayv1.Listener{listenerA, listenerA, listenerB}, + }, + { + name: "preserves all listeners when names collide", + target: []gatewayv1.Listener{listenerA}, + source: []gatewayv1.Listener{listenerB, listenerA2, listenerA3}, + expected: []gatewayv1.Listener{listenerA, listenerB, listenerA2, listenerA3}, + }, + } - require.Len(t, got, 2) - require.Equal(t, gatewayv1.PortNumber(80), got[0].Port) - require.Equal(t, gatewayv1.PortNumber(8080), got[1].Port) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.expected, appendListeners(tt.target, tt.source...)) + }) + } } func TestListenersForGatewayContext(t *testing.T) { @@ -89,11 +118,11 @@ func TestListenersForGatewayContext(t *testing.T) { }) } -// TestGetMinimumHostnameIntersectionUsesMatchedListeners pins the intersection to -// the listeners the parentRef actually selected. A parentRef selecting by port -// leaves ListenerName empty, so filtering by name alone would let a route keep a +// TestNarrowHostnameUsesMatchedListeners pins the intersection to the listeners +// the parentRef actually selected. A parentRef selecting by port leaves +// ListenerName empty, so filtering by name alone would let a route keep a // hostname that only a listener on a different port serves. -func TestGetMinimumHostnameIntersectionUsesMatchedListeners(t *testing.T) { +func TestNarrowHostnameUsesMatchedListeners(t *testing.T) { gateway := &gatewayv1.Gateway{} gateway.Spec.Listeners = []gatewayv1.Listener{ listener("http-a", 80, "a.example.com"), @@ -104,14 +133,214 @@ func TestGetMinimumHostnameIntersectionUsesMatchedListeners(t *testing.T) { Listeners: []gatewayv1.Listener{gateway.Spec.Listeners[0]}, }} - require.Equal(t, gatewayv1.Hostname("a.example.com"), - getMinimumHostnameIntersection(portSelected, "a.example.com")) - require.Empty(t, getMinimumHostnameIntersection(portSelected, "b.example.com"), + require.Equal(t, []gatewayv1.Hostname{"a.example.com"}, + cutils.NarrowHostname(hostnameEffectiveListeners(portSelected), "a.example.com")) + require.Empty(t, cutils.NarrowHostname(hostnameEffectiveListeners(portSelected), "b.example.com"), "a hostname served only by the untargeted listener must not survive") // Without a matched listener the whole spec is still considered, so both // hostnames intersect as before. whole := []RouteParentRefContext{{Gateway: gateway}} - require.Equal(t, gatewayv1.Hostname("b.example.com"), - getMinimumHostnameIntersection(whole, "b.example.com")) + require.Equal(t, []gatewayv1.Hostname{"b.example.com"}, + cutils.NarrowHostname(hostnameEffectiveListeners(whole), "b.example.com")) +} + +// The Gateway API conformance case TLSRouteHostnameIntersection turns on this: +// four Gateways whose TLS listeners carry different hostnames all resolve to one +// physical stream listen, so a TLSRoute keeping its own hostname verbatim serves +// SNIs its listener never accepted and steals them from the route whose listener +// did. +func TestFilterTLSRouteHostnames(t *testing.T) { + exact := gatewayv1.Hostname("abc.example.com") + moreSpecificWildcard := gatewayv1.Hostname("*.example.com") + lessSpecificWildcard := gatewayv1.Hostname("*.com") + + tlsListener := func(name string, hostname *gatewayv1.Hostname) gatewayv1.Listener { + return gatewayv1.Listener{ + Name: gatewayv1.SectionName(name), + Protocol: gatewayv1.TLSProtocolType, + Port: 443, + Hostname: hostname, + } + } + route := func(hostnames ...gatewayv1.Hostname) *gatewayv1.TLSRoute { + return &gatewayv1.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: gatewayv1.TLSRouteSpec{Hostnames: hostnames}, + } + } + + for _, tc := range []struct { + name string + listener *gatewayv1.Hostname + hostnames []gatewayv1.Hostname + want []gatewayv1.Hostname + wantErr bool + }{ + { + name: "a wildcard route narrows to the exact listener hostname", + listener: &exact, + hostnames: []gatewayv1.Hostname{moreSpecificWildcard}, + want: []gatewayv1.Hostname{exact}, + }, + { + name: "a broader wildcard route narrows to the listener wildcard", + listener: &moreSpecificWildcard, + hostnames: []gatewayv1.Hostname{lessSpecificWildcard}, + want: []gatewayv1.Hostname{moreSpecificWildcard}, + }, + { + name: "an exact route under a listener wildcard keeps its own hostname", + listener: &moreSpecificWildcard, + hostnames: []gatewayv1.Hostname{exact}, + want: []gatewayv1.Hostname{exact}, + }, + { + name: "a listener without a hostname leaves the route alone", + listener: nil, + hostnames: []gatewayv1.Hostname{lessSpecificWildcard}, + want: []gatewayv1.Hostname{lessSpecificWildcard}, + }, + { + name: "a route without hostnames takes the listener hostname", + listener: &moreSpecificWildcard, + hostnames: nil, + want: []gatewayv1.Hostname{moreSpecificWildcard}, + }, + { + name: "a route without hostnames under a hostname-less listener matches anything", + listener: nil, + hostnames: nil, + want: nil, + }, + { + name: "no intersection is rejected", + listener: &exact, + hostnames: []gatewayv1.Hostname{gatewayv1.Hostname("other.example.net")}, + wantErr: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + gateways := []RouteParentRefContext{ + {Listeners: []gatewayv1.Listener{tlsListener("tls", tc.listener)}}, + } + + filtered, err := filterTLSRouteHostnames(gateways, route(tc.hostnames...).DeepCopy()) + if tc.wantErr { + require.ErrorIs(t, err, ErrNoMatchingListenerHostname) + return + } + require.NoError(t, err) + require.Equal(t, tc.want, filtered.Spec.Hostnames) + }) + } +} + +// A rejected parentRef carries no matched listener, so listenersForGatewayContext +// falls back to every listener on its Gateway. Without filtering, a hostname only +// that Gateway accepts survives the intersection and the TLS translator serves it +// as an SNI through the parent that was accepted. +func TestIntersectRouteHostnamesIgnoresRejectedParents(t *testing.T) { + acceptedHost := gatewayv1.Hostname("a.example.com") + rejectedHost := gatewayv1.Hostname("b.example.com") + + gatewayWith := func(hostname gatewayv1.Hostname) *gatewayv1.Gateway { + h := hostname + return &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{Name: string(hostname), Namespace: "default"}, + Spec: gatewayv1.GatewaySpec{ + Listeners: []gatewayv1.Listener{{ + Name: "tls", + Port: 443, + // Has to be a protocol isListenerHostnameEffective accepts, or + // the union branch skips the listener and reads as "no hostnames". + Protocol: gatewayv1.TLSProtocolType, + Hostname: &h, + }}, + }, + } + } + condition := func(status metav1.ConditionStatus) []metav1.Condition { + return []metav1.Condition{{ + Type: string(gatewayv1.RouteConditionAccepted), + Status: status, + }} + } + + accepted := gatewayWith(acceptedHost) + rejected := gatewayWith(rejectedHost) + gateways := []RouteParentRefContext{ + { + Gateway: accepted, + Listeners: accepted.Spec.Listeners, + Conditions: condition(metav1.ConditionTrue), + }, + { + // No matched listener, as ParseRouteParentRefs leaves it. + Gateway: rejected, + Conditions: condition(metav1.ConditionFalse), + }, + } + + got, err := intersectRouteHostnames(gateways, []gatewayv1.Hostname{acceptedHost, rejectedHost}) + require.NoError(t, err) + require.Equal(t, []gatewayv1.Hostname{acceptedHost}, got) + + // A route without hostnames takes the union, which must not be widened either. + got, err = intersectRouteHostnames(gateways, nil) + require.NoError(t, err) + require.Equal(t, []gatewayv1.Hostname{acceptedHost}, got) +} + +// A route hostname that covers several listener hostnames intersects with every +// one of them. Keeping only the first left the names the other listeners accept +// unserved. +func TestIntersectRouteHostnamesKeepsEveryIntersection(t *testing.T) { + aHostname := gatewayv1.Hostname("a.example.com") + bHostname := gatewayv1.Hostname("b.example.com") + gateways := []RouteParentRefContext{ + { + Listeners: []gatewayv1.Listener{ + {Name: "a", Protocol: gatewayv1.TLSProtocolType, Port: gatewayv1.PortNumber(443), Hostname: &aHostname}, + {Name: "b", Protocol: gatewayv1.TLSProtocolType, Port: gatewayv1.PortNumber(443), Hostname: &bHostname}, + }, + Conditions: []metav1.Condition{ + { + Type: string(gatewayv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }, + }, + }, + } + + got, err := intersectRouteHostnames(gateways, []gatewayv1.Hostname{"*.example.com"}) + require.NoError(t, err) + assert.Equal(t, []gatewayv1.Hostname{aHostname, bHostname}, got) +} + +// The same hostname contributed by two listeners is one hostname: listeners are +// aggregated across Gateways without de-duplication. +func TestIntersectRouteHostnamesDedups(t *testing.T) { + hostname := gatewayv1.Hostname("a.example.com") + listener := gatewayv1.Listener{ + Name: "a", Protocol: gatewayv1.TLSProtocolType, Port: gatewayv1.PortNumber(443), Hostname: &hostname, + } + accepted := []metav1.Condition{ + { + Type: string(gatewayv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }, + } + gateways := []RouteParentRefContext{ + {Listeners: []gatewayv1.Listener{listener}, Conditions: accepted}, + {Listeners: []gatewayv1.Listener{listener}, Conditions: accepted}, + } + + got, err := intersectRouteHostnames(gateways, []gatewayv1.Hostname{"*.example.com"}) + require.NoError(t, err) + assert.Equal(t, []gatewayv1.Hostname{hostname}, got) + + got, err = intersectRouteHostnames(gateways, nil) + require.NoError(t, err) + assert.Equal(t, []gatewayv1.Hostname{hostname}, got) } diff --git a/internal/controller/tlsroute_controller.go b/internal/controller/tlsroute_controller.go index f1b1792f..1346c3ef 100644 --- a/internal/controller/tlsroute_controller.go +++ b/internal/controller/tlsroute_controller.go @@ -318,7 +318,8 @@ func (r *TLSRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c acceptStatus.msg = err.Error() } // Populate the matched listeners so the translator can derive the - // StreamRoute server_port from the listener the route attaches to. + // StreamRoute server_port and its tls.mode from the listener the route + // attaches to. if len(gateway.Listeners) > 0 { tctx.Listeners = appendListeners(tctx.Listeners, gateway.Listeners...) } else if gateway.Listener != nil { @@ -348,12 +349,26 @@ func (r *TLSRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c if r.supportsL4RoutePolicy { ProcessL4RoutePolicy(r.Client, r.Log, tctx, tr.Namespace, tr.Name, types.KindTLSRoute) } + + // The hostnames become the SNIs the stream routes match on, so they have to be + // narrowed to what the listeners accept first. Every Gateway shares one physical + // stream listen, so a route keeping its own broader hostname does not merely + // over-serve: it takes that name from the route whose listener does accept it. + filteredTLSRoute, hostnameErr := filterTLSRouteHostnames(gateways, tr.DeepCopy()) + if hostnameErr != nil { + acceptStatus.status = false + acceptStatus.msg = hostnameErr.Error() + } // Translate and sync before writing Accepted=True. A translation or // buildConfig failure from Provider.Update used to leave a green status // and only requeue, matching neither Ingress nor ApisixGlobalRule. var syncErr error - if isRouteAccepted(gateways) { - if syncErr = r.Provider.Update(ctx, tctx, tr); syncErr != nil { + if isRouteAccepted(gateways) && hostnameErr == nil { + routeToUpdate := tr + if filteredTLSRoute != nil { + routeToUpdate = filteredTLSRoute + } + if syncErr = r.Provider.Update(ctx, tctx, routeToUpdate); syncErr != nil { acceptStatus.status = false acceptStatus.msg = syncErr.Error() } @@ -386,7 +401,7 @@ func (r *TLSRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c }), }) UpdateStatus(r.Updater, r.Log, tctx) - if isRouteAccepted(gateways) { + if isRouteAccepted(gateways) && hostnameErr == nil { return ctrl.Result{}, syncErr } diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 13550995..732a7414 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -51,6 +51,7 @@ import ( apiv2 "github.com/apache/apisix-ingress-controller/api/v2" "github.com/apache/apisix-ingress-controller/internal/controller/config" "github.com/apache/apisix-ingress-controller/internal/controller/indexer" + cutils "github.com/apache/apisix-ingress-controller/internal/controller/utils" "github.com/apache/apisix-ingress-controller/internal/provider" sslutils "github.com/apache/apisix-ingress-controller/internal/ssl" "github.com/apache/apisix-ingress-controller/internal/types" @@ -725,7 +726,7 @@ func listenerHostnameIntersectWithRouteHostnames(listener gatewayv1.Listener, ho // iterate over all the hostnames and check that at least one intersect with the listener hostname for _, hostname := range hostnames { - if HostnamesIntersect(string(*listener.Hostname), string(hostname)) { + if cutils.HostnamesIntersect(string(*listener.Hostname), string(hostname)) { return true } } @@ -733,57 +734,6 @@ func listenerHostnameIntersectWithRouteHostnames(listener gatewayv1.Listener, ho return false } -func HostnamesIntersect(a, b string) bool { - return HostnamesMatch(a, b) || HostnamesMatch(b, a) -} - -// HostnamesMatch checks that the hostnameB matches the hostnameA. HostnameA is treated as mask -// to be checked against the hostnameB. -func HostnamesMatch(hostnameA, hostnameB string) bool { - // the hostnames are in the form of "foo.bar.com"; split them - // in a slice of substrings - hostnameALabels := strings.Split(hostnameA, ".") - hostnameBLabels := strings.Split(hostnameB, ".") - - var a, b int - var wildcard bool - - // iterate over the parts of both the hostnames - for a, b = 0, 0; a < len(hostnameALabels) && b < len(hostnameBLabels); a, b = a+1, b+1 { - var matchFound bool - - // if the current part of B is a wildcard, we need to find the first - // A part that matches with the following B part - if wildcard { - for ; b < len(hostnameBLabels); b++ { - if hostnameALabels[a] == hostnameBLabels[b] { - matchFound = true - break - } - } - } - - // if no match was found, the hostnames don't match - if wildcard && !matchFound { - return false - } - - // check if at least on of the current parts are a wildcard; if so, continue - if hostnameALabels[a] == "*" { - wildcard = true - continue - } - // reset the wildcard variables - wildcard = false - - // if the current a part is different from the b part, the hostnames are incompatible - if hostnameALabels[a] != hostnameBLabels[b] { - return false - } - } - return len(hostnameBLabels)-b == len(hostnameALabels)-a -} - func routeMatchesListenerAllowedRoutes( ctx context.Context, mgrc client.Client, @@ -1409,51 +1359,71 @@ func FullTypeName(a any) string { // filterHostnames accepts a list of gateways and an HTTPRoute, and returns a copy of the HTTPRoute with only the hostnames that match the listener hostnames of the gateways. // If the HTTPRoute hostnames do not intersect with the listener hostnames of the gateways, it returns an ErrNoMatchingListenerHostname error. func filterHostnames(gateways []RouteParentRefContext, httpRoute *gatewayv1.HTTPRoute) (*gatewayv1.HTTPRoute, error) { - filteredHostnames := make([]gatewayv1.Hostname, 0) + hostnames, err := intersectRouteHostnames(gateways, httpRoute.Spec.Hostnames) + if err != nil { + return httpRoute, err + } + httpRoute.Spec.Hostnames = hostnames + return httpRoute, nil +} - // If the HTTPRoute does not specify hostnames, we use the union of the listener hostnames of all supported gateways - // If any supported listener does not specify a hostname, the HTTPRoute hostnames remain empty to match any hostname - if len(httpRoute.Spec.Hostnames) == 0 { - hostnames, matchAnyHost := getUnionOfGatewayHostnames(gateways) +// filterTLSRouteHostnames is filterHostnames for a TLSRoute. Its hostnames become +// the SNIs the stream routes match on, so a route left carrying its own broader +// hostname would serve names the listener it attached to never accepted - and, on +// a single shared stream listen, would take them from the route that should have. +func filterTLSRouteHostnames(gateways []RouteParentRefContext, tlsRoute *gatewayv1.TLSRoute) (*gatewayv1.TLSRoute, error) { + hostnames, err := intersectRouteHostnames(gateways, tlsRoute.Spec.Hostnames) + if err != nil { + return tlsRoute, err + } + tlsRoute.Spec.Hostnames = hostnames + return tlsRoute, nil +} + +// intersectRouteHostnames narrows a route's hostnames to what the listeners it +// attached to actually accept. +// +// A route without hostnames takes the union of the listener hostnames, and stays +// empty - matching any hostname - as soon as one supported listener carries no +// hostname of its own. Otherwise every hostname is replaced by its intersections +// with the listener hostnames, and a route that intersects with none is +// ErrNoMatchingListenerHostname. +func intersectRouteHostnames(gateways []RouteParentRefContext, routeHostnames []gatewayv1.Hostname) ([]gatewayv1.Hostname, error) { + listeners := hostnameEffectiveListeners(acceptedParentRefs(gateways)) + + if len(routeHostnames) == 0 { + hostnames, matchAnyHost := cutils.UnionHostnames(listeners) if matchAnyHost { - return httpRoute, nil - } - filteredHostnames = hostnames - } else { - // If the HTTPRoute specifies hostnames, we need to find the intersection with the gateway listener hostnames - for _, hostname := range httpRoute.Spec.Hostnames { - if hostnameMatching := getMinimumHostnameIntersection(gateways, hostname); hostnameMatching != "" { - filteredHostnames = append(filteredHostnames, hostnameMatching) - } - } - if len(filteredHostnames) == 0 { - return httpRoute, ErrNoMatchingListenerHostname + return routeHostnames, nil } + return cutils.DedupHostnames(hostnames), nil } - httpRoute.Spec.Hostnames = filteredHostnames - return httpRoute, nil + filteredHostnames := make([]gatewayv1.Hostname, 0, len(routeHostnames)) + for _, hostname := range routeHostnames { + filteredHostnames = append(filteredHostnames, cutils.NarrowHostname(listeners, hostname)...) + } + filteredHostnames = cutils.DedupHostnames(filteredHostnames) + if len(filteredHostnames) == 0 { + return nil, ErrNoMatchingListenerHostname + } + return filteredHostnames, nil } -// getUnionOfGatewayHostnames returns the union of the hostnames specified in all supported gateways -// The second return value indicates whether any listener can match any hostname -func getUnionOfGatewayHostnames(gateways []RouteParentRefContext) ([]gatewayv1.Hostname, bool) { - hostnames := make([]gatewayv1.Hostname, 0) - +// hostnameEffectiveListeners flattens the listeners the route attached to, +// keeping only those a hostname can be matched against. +func hostnameEffectiveListeners(gateways []RouteParentRefContext) []gatewayv1.Listener { + listeners := make([]gatewayv1.Listener, 0, len(gateways)) for _, gateway := range gateways { for _, listener := range listenersForGatewayContext(gateway) { // Only consider listeners that can effectively configure hostnames (HTTP, HTTPS, or TLS). if !isListenerHostnameEffective(listener) { continue } - if listener.Hostname == nil { - return nil, true - } - hostnames = append(hostnames, *listener.Hostname) + listeners = append(listeners, listener) } } - - return hostnames, false + return listeners } // listenersForGatewayContext returns the listeners relevant for hostname resolution. @@ -1478,55 +1448,11 @@ func listenersForGatewayContext(gateway RouteParentRefContext) []gatewayv1.Liste return gateway.Gateway.Spec.Listeners } -// appendListeners appends listeners to the slice, avoiding duplicates by port+name. -func appendListeners(existing []gatewayv1.Listener, toAdd ...gatewayv1.Listener) []gatewayv1.Listener { - // Listener names are only unique within a Gateway, while this slice is - // aggregated across every Gateway the route attaches to, so the name alone - // would collapse distinct listeners such as two "http" on different ports. - type listenerKey struct { - name string - port gatewayv1.PortNumber - } - seen := make(map[listenerKey]struct{}, len(existing)) - for _, l := range existing { - seen[listenerKey{name: string(l.Name), port: l.Port}] = struct{}{} - } - for _, l := range toAdd { - key := listenerKey{name: string(l.Name), port: l.Port} - if _, ok := seen[key]; !ok { - existing = append(existing, l) - seen[key] = struct{}{} - } - } - return existing -} - -// getMinimumHostnameIntersection returns the smallest intersection hostname -// - If the listener hostname is empty, return the HTTPRoute hostname -// - If the listener hostname is a wildcard of the HTTPRoute hostname, return the HTTPRoute hostname -// - If the HTTPRoute hostname is a wildcard of the listener hostname, return the listener hostname -// - If the HTTPRoute hostname and listener hostname are the same, return it -// - If none of the above, return an empty string -func getMinimumHostnameIntersection(gateways []RouteParentRefContext, hostname gatewayv1.Hostname) gatewayv1.Hostname { - for _, gateway := range gateways { - // Only the listeners the parentRef actually matched: a parentRef selecting - // by port alone leaves ListenerName empty, and intersecting against the - // whole spec would keep a hostname served only by a listener on another - // port. - for _, listener := range listenersForGatewayContext(gateway) { - if listener.Hostname == nil || *listener.Hostname == "" { - return hostname - } - if HostnamesMatch(string(*listener.Hostname), string(hostname)) { - return hostname - } - if HostnamesMatch(string(hostname), string(*listener.Hostname)) { - return *listener.Hostname - } - } - } - - return "" +// appendListeners appends listeners without de-duplication. +// Route translation aggregates listeners across multiple Gateways, and listener +// names are only unique within a single Gateway. +func appendListeners(target []gatewayv1.Listener, source ...gatewayv1.Listener) []gatewayv1.Listener { + return append(target, source...) } // isListenerHostnameEffective checks if a listener can specify a hostname to match the hostname in the request @@ -1537,6 +1463,33 @@ func isListenerHostnameEffective(listener gatewayv1.Listener) bool { listener.Protocol == gatewayv1.TLSProtocolType } +// acceptedParentRefs keeps only the contexts whose parentRef was accepted. +// +// ParseRouteParentRefs returns a context per parentRef, accepted or not, and a +// rejected one carries no matched listener at all - so listenersForGatewayContext +// falls back to every listener on its Gateway. A hostname that only such a +// Gateway accepts would then survive the intersection and be served through the +// parentRef that was accepted. Only an accepted parent may widen the set. +// +// When nothing was accepted the contexts are returned unchanged: the route is +// not translated in that case, and narrowing here would only change which +// reason its status reports. +func acceptedParentRefs(gateways []RouteParentRefContext) []RouteParentRefContext { + accepted := make([]RouteParentRefContext, 0, len(gateways)) + for _, gateway := range gateways { + for _, condition := range gateway.Conditions { + if condition.Type == string(gatewayv1.RouteConditionAccepted) && condition.Status == metav1.ConditionTrue { + accepted = append(accepted, gateway) + break + } + } + } + if len(accepted) == 0 { + return gateways + } + return accepted +} + func isRouteAccepted(gateways []RouteParentRefContext) bool { for _, gateway := range gateways { for _, condition := range gateway.Conditions { diff --git a/internal/controller/utils/hostname.go b/internal/controller/utils/hostname.go new file mode 100644 index 00000000..18b7bd4f --- /dev/null +++ b/internal/controller/utils/hostname.go @@ -0,0 +1,130 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package utils + +import ( + "strings" + + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +func HostnamesIntersect(a, b string) bool { + return HostnamesMatch(a, b) || HostnamesMatch(b, a) +} + +// HostnamesMatch checks that the hostnameB matches the hostnameA. HostnameA is treated as mask +// to be checked against the hostnameB. +func HostnamesMatch(hostnameA, hostnameB string) bool { + // the hostnames are in the form of "foo.bar.com"; split them + // in a slice of substrings + hostnameALabels := strings.Split(hostnameA, ".") + hostnameBLabels := strings.Split(hostnameB, ".") + + var a, b int + var wildcard bool + + // iterate over the parts of both the hostnames + for a, b = 0, 0; a < len(hostnameALabels) && b < len(hostnameBLabels); a, b = a+1, b+1 { + var matchFound bool + + // if the current part of B is a wildcard, we need to find the first + // A part that matches with the following B part + if wildcard { + for ; b < len(hostnameBLabels); b++ { + if hostnameALabels[a] == hostnameBLabels[b] { + matchFound = true + break + } + } + } + + // if no match was found, the hostnames don't match + if wildcard && !matchFound { + return false + } + + // check if at least on of the current parts are a wildcard; if so, continue + if hostnameALabels[a] == "*" { + wildcard = true + continue + } + // reset the wildcard variables + wildcard = false + + // if the current a part is different from the b part, the hostnames are incompatible + if hostnameALabels[a] != hostnameBLabels[b] { + return false + } + } + return len(hostnameBLabels)-b == len(hostnameALabels)-a +} + +// NarrowHostname returns every intersection of routeHostname with the hostnames +// of the given listeners. +// +// A listener carrying no hostname, or one whose hostname is a mask covering +// routeHostname, accepts routeHostname whole and is then the only intersection. +// Otherwise each listener hostname that routeHostname covers is an intersection +// of its own: keeping only the first - which is what looking up a single +// "smallest intersection" does - silently drops the traffic the remaining +// listeners accept. +// +// An empty result means the listeners accept none of routeHostname. +func NarrowHostname(listeners []gatewayv1.Listener, routeHostname gatewayv1.Hostname) []gatewayv1.Hostname { + narrowed := make([]gatewayv1.Hostname, 0, len(listeners)) + for _, listener := range listeners { + if listener.Hostname == nil || *listener.Hostname == "" || + HostnamesMatch(string(*listener.Hostname), string(routeHostname)) { + return []gatewayv1.Hostname{routeHostname} + } + if HostnamesMatch(string(routeHostname), string(*listener.Hostname)) { + narrowed = append(narrowed, *listener.Hostname) + } + } + return narrowed +} + +// UnionHostnames returns the union of the listener hostnames. The second return +// value reports that a listener carries no hostname of its own and therefore +// accepts every hostname, which makes the union meaningless. +func UnionHostnames(listeners []gatewayv1.Listener) ([]gatewayv1.Hostname, bool) { + hostnames := make([]gatewayv1.Hostname, 0, len(listeners)) + for _, listener := range listeners { + if listener.Hostname == nil || *listener.Hostname == "" { + return nil, true + } + hostnames = append(hostnames, *listener.Hostname) + } + return hostnames, false +} + +// DedupHostnames drops repeated hostnames, keeping the first occurrence. +// Listeners are aggregated across Gateways, so the same hostname can be +// contributed more than once. +func DedupHostnames(hostnames []gatewayv1.Hostname) []gatewayv1.Hostname { + deduped := make([]gatewayv1.Hostname, 0, len(hostnames)) + seen := make(map[gatewayv1.Hostname]struct{}, len(hostnames)) + for _, hostname := range hostnames { + if _, ok := seen[hostname]; ok { + continue + } + seen[hostname] = struct{}{} + deduped = append(deduped, hostname) + } + return deduped +} diff --git a/test/conformance/api7ee/conformance_test.go b/test/conformance/api7ee/conformance_test.go index a9572a9f..3645d8a7 100644 --- a/test/conformance/api7ee/conformance_test.go +++ b/test/conformance/api7ee/conformance_test.go @@ -31,6 +31,23 @@ var skippedTestsForSSL = []string{ tests.HTTPRouteRedirectPortAndScheme.ShortName, } +// The TLSRoute tests all pin their listener to mode: Passthrough, which the +// API7 gateway serves since api7/api7-ee-3-gateway#2173. One of them cannot +// pass here for the same reason it cannot on the APISIX provider, recorded in +// test/conformance/conformance_test.go: every Gateway resolves to one data +// plane address and one physical stream listen, so their SNI namespaces are +// shared and a name no listener on the addressed Gateway accepts is still +// answered by a sibling Gateway's route. +var skippedTestsForKnownGaps = []string{ + tests.TLSRouteHostnameIntersection.ShortName, + // Terminate mode itself is covered by TLSRouteListenerTerminateSupportedKinds + // and by the e2e TLSRoute suite. This provisional test additionally requires a + // standalone Gateway with no GatewayProxy attached to reach Accepted=True, and + // a stream proxy listening on the port it picks; neither holds here, so the + // Gateway is rejected with "gateway proxy not found" before any traffic flows. + tests.TLSRouteTerminateSimpleSameNamespace.ShortName, +} + // TODO: HTTPRoute hostname intersection and listener hostname matching func TestGatewayAPIConformance(t *testing.T) { @@ -39,6 +56,7 @@ func TestGatewayAPIConformance(t *testing.T) { opts.CleanupBaseResources = true opts.GatewayClassName = gatewayClassName opts.SkipTests = append(opts.SkipTests, skippedTestsForSSL...) + opts.SkipTests = append(opts.SkipTests, skippedTestsForKnownGaps...) opts.Implementation = conformancev1.Implementation{ Organization: "APISIX", Project: "apisix-ingress-controller", diff --git a/test/conformance/api7ee/suite_test.go b/test/conformance/api7ee/suite_test.go index 2812d1af..0d303b80 100644 --- a/test/conformance/api7ee/suite_test.go +++ b/test/conformance/api7ee/suite_test.go @@ -151,6 +151,11 @@ func TestMain(m *testing.M) { ServiceType: "LoadBalancer", ServiceHTTPPort: 80, ServiceHTTPSPort: 443, + // The TLSRoute tests pin their Gateway listener to port 443 in + // Passthrough mode and dial the Gateway address there, so 443 has to + // reach the stream tls_passthrough listen rather than the HTTP ssl + // listen. One port cannot serve both. + ServiceHTTPSTargetPort: 9120, }) if len(svc.Status.LoadBalancer.Ingress) == 0 { diff --git a/test/conformance/conformance_test.go b/test/conformance/conformance_test.go index cf606fc6..223a0a73 100644 --- a/test/conformance/conformance_test.go +++ b/test/conformance/conformance_test.go @@ -31,17 +31,6 @@ var skippedTestsForSSL = []string{ tests.HTTPRouteRedirectPortAndScheme.ShortName, } -// APISIX terminates TLS on its stream proxy and matches stream routes by SNI, -// which implements TLSRoute in Terminate mode (declared via the -// TLSRouteModeTerminate feature) but never forwards the encrypted stream -// untouched. Every test below pins its listener to mode: Passthrough. -var skippedTestsForTLSPassthrough = []string{ - tests.TLSRouteSimpleSameNamespace.ShortName, - tests.TLSRouteHostnameIntersection.ShortName, - tests.TLSRouteInvalidBackendRefNonexistent.ShortName, - tests.TLSRouteInvalidBackendRefUnknownKind.ShortName, -} - // Known gaps tracked for follow-up. These are genuine feature gaps rather than // architectural limits, so they are expected to shrink over time. var skippedTestsForKnownGaps = []string{ @@ -69,6 +58,18 @@ var skippedTestsForKnownGaps = []string{ // A single HTTPRoute attached to several Gateways is not served from each // parent independently. tests.HTTPRouteMultipleGateways.ShortName, + + // The same limitation for TLSRoute, and not something the translator can + // fix. The test stands four Gateways up on port 443 with different listener + // hostnames; every Gateway resolves to the one data plane address and the one + // physical stream listen, so their SNI namespaces are shared. The Gateway + // whose listener carries no hostname keeps its route's "*.com" verbatim - + // correctly, and its own subtest depends on it - which then also answers + // "non.matching.com" on the address of the Gateway that should have rejected + // it. Which Gateway a connection was addressed to is not on the wire, so + // there is nothing left to discriminate on. Every other assertion in this + // test passes, including the hostname intersections themselves. + tests.TLSRouteHostnameIntersection.ShortName, } func TestGatewayAPIConformance(t *testing.T) { @@ -77,7 +78,6 @@ func TestGatewayAPIConformance(t *testing.T) { opts.CleanupBaseResources = true opts.GatewayClassName = gatewayClassName opts.SkipTests = append(opts.SkipTests, skippedTestsForSSL...) - opts.SkipTests = append(opts.SkipTests, skippedTestsForTLSPassthrough...) opts.SkipTests = append(opts.SkipTests, skippedTestsForKnownGaps...) opts.Implementation = conformancev1.Implementation{ Organization: "APISIX", diff --git a/test/conformance/suite_test.go b/test/conformance/suite_test.go index e7ab91b1..64783c31 100644 --- a/test/conformance/suite_test.go +++ b/test/conformance/suite_test.go @@ -154,6 +154,13 @@ func TestMain(m *testing.M) { ServiceType: "LoadBalancer", ServiceHTTPPort: 80, ServiceHTTPSPort: 443, + // The TLSRoute tests pin their Gateway listener to port 443 in + // Passthrough mode and dial the Gateway address there, so 443 has to + // reach APISIX's stream tls_passthrough listen rather than its HTTP ssl + // listen. One port cannot serve both, and the HTTPRoute tests that would + // want HTTP-over-TLS on 443 are skipped for unrelated reasons + // (skippedTestsForSSL). + ServiceHTTPSTargetPort: 9120, }) svc := s.GetDataplaneService() diff --git a/test/e2e/framework/api7_gateway.go b/test/e2e/framework/api7_gateway.go index fc5ccdb1..7662e7d4 100644 --- a/test/e2e/framework/api7_gateway.go +++ b/test/e2e/framework/api7_gateway.go @@ -59,7 +59,10 @@ type API7DeployOptions struct { ServiceType string ServiceHTTPPort int ServiceHTTPSPort int - Replicas *int + // ServiceHTTPSTargetPort redirects the HTTPS service port at the data + // plane's stream tls_passthrough listen instead of its HTTP ssl listen. + ServiceHTTPSTargetPort int + Replicas *int } func (f *Framework) DeployGateway(opts *API7DeployOptions) *corev1.Service { @@ -75,6 +78,10 @@ func (f *Framework) DeployGateway(opts *API7DeployOptions) *corev1.Service { opts.ServiceHTTPSPort = 443 } + if opts.ServiceHTTPSTargetPort == 0 { + opts.ServiceHTTPSTargetPort = 9443 + } + dpCert := f.GetDataplaneCertificates(opts.GatewayGroupID) f.applySSLSecret(opts.Namespace, diff --git a/test/e2e/framework/manifests/apisix.yaml b/test/e2e/framework/manifests/apisix.yaml index 30289194..d36074f5 100644 --- a/test/e2e/framework/manifests/apisix.yaml +++ b/test/e2e/framework/manifests/apisix.yaml @@ -49,6 +49,11 @@ data: - 9100 - addr: 9110 tls: true + # TLS passthrough: the SNI is read out of the prereaded ClientHello + # and the stream reaches the backend untouched. A listen is either + # terminating or prereading, never both, so this needs its own port. + - addr: 9120 + tls_passthrough: true udp: # UDP proxy port list - 9200 discovery: @@ -119,6 +124,9 @@ spec: - name: tls containerPort: 9110 protocol: TCP + - name: tls-passthrough + containerPort: 9120 + protocol: TCP volumeMounts: - name: config-writable mountPath: /usr/local/apisix/conf @@ -151,7 +159,11 @@ spec: - port: {{ .ServiceHTTPSPort }} name: https protocol: TCP - targetPort: 9443 + # Normally APISIX's HTTP ssl listen. The conformance run redirects it to + # the stream tls_passthrough listen instead: a Gateway API TLSRoute + # Passthrough listener is fixed at 443, and one port cannot serve both + # HTTP-over-TLS and an untouched stream. + targetPort: {{ .ServiceHTTPSTargetPort }} - port: 9180 name: admin protocol: TCP @@ -168,6 +180,10 @@ spec: port: 9110 protocol: TCP targetPort: 9110 + - name: tls-passthrough + port: 9120 + protocol: TCP + targetPort: 9120 selector: app.kubernetes.io/name: apisix type: {{ .ServiceType | default "NodePort" }} diff --git a/test/e2e/framework/manifests/dp.yaml b/test/e2e/framework/manifests/dp.yaml index 7608c9de..ee431b79 100644 --- a/test/e2e/framework/manifests/dp.yaml +++ b/test/e2e/framework/manifests/dp.yaml @@ -36,6 +36,11 @@ data: - 9100 - addr: 9110 tls: true + # TLS passthrough: the SNI is read out of the prereaded ClientHello + # and the stream reaches the backend untouched. A listen is either + # terminating or prereading, never both, so this needs its own port. + - addr: 9120 + tls_passthrough: true udp: - 9200 nginx_config: @@ -227,6 +232,9 @@ spec: - containerPort: 9110 name: tls protocol: TCP + - containerPort: 9120 + name: tls-passthrough + protocol: TCP readinessProbe: failureThreshold: 10 initialDelaySeconds: 3 @@ -279,7 +287,11 @@ spec: - name: https port: {{ .ServiceHTTPSPort }} protocol: TCP - targetPort: 9443 + # Normally the HTTP ssl listen. The conformance run redirects it to the + # stream tls_passthrough listen instead: a Gateway API TLSRoute Passthrough + # listener is fixed at 443, and one port cannot serve both HTTP-over-TLS + # and an untouched stream. + targetPort: {{ .ServiceHTTPSTargetPort }} - name: control-api port: 9090 protocol: TCP @@ -295,6 +307,10 @@ spec: port: 9110 protocol: TCP targetPort: 9110 + - name: tls-passthrough + port: 9120 + protocol: TCP + targetPort: 9120 selector: app.kubernetes.io/instance: api7ee3 app.kubernetes.io/name: apisix diff --git a/test/e2e/gatewayapi/tlsroute.go b/test/e2e/gatewayapi/tlsroute.go index 08d4e4a3..a6361ac8 100644 --- a/test/e2e/gatewayapi/tlsroute.go +++ b/test/e2e/gatewayapi/tlsroute.go @@ -20,10 +20,13 @@ package gatewayapi import ( "fmt" "net/http" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "k8s.io/utils/ptr" + "github.com/apache/apisix-ingress-controller/test/e2e/framework" "github.com/apache/apisix-ingress-controller/test/e2e/scaffold" ) @@ -114,4 +117,87 @@ spec: }).Should(ContainSubstring("EOF"), "should get EOF after deleting TLSRoute") }) }) + + Context("TLSRoute Passthrough", func() { + // The certificate the e2e nginx serves, and the CA that signed it. + const backendSNI = "server.example.com" + + var passthroughGateway = ` +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: tls-passthrough-gateway +spec: + gatewayClassName: %s + listeners: + - name: passthrough + protocol: TLS + # Must equal APISIX's physical stream_proxy listen configured with + # tls_passthrough (see the e2e apisix manifest): the controller runs with + # listener_port_match_mode=auto, so the port reaches the stream route as a + # server_port match and has to be one the data plane actually accepts on. + port: 9120 + hostname: server.example.com + tls: + mode: Passthrough + infrastructure: + parametersRef: + group: apisix.apache.org + kind: GatewayProxy + name: apisix-proxy-config +` + + var passthroughRoute = ` +apiVersion: gateway.networking.k8s.io/v1 +kind: TLSRoute +metadata: + name: tls-passthrough-route +spec: + parentRefs: + - name: tls-passthrough-gateway + sectionName: passthrough + hostnames: ["server.example.com"] + rules: + - backendRefs: + - name: nginx + port: 443 +` + + BeforeEach(func() { + By("create GatewayProxy") + Expect(s.CreateResourceFromString(s.GetGatewayProxySpec())).NotTo(HaveOccurred(), "creating GatewayProxy") + + By("create GatewayClass") + Expect(s.CreateResourceFromString(s.GetGatewayClassYaml())).NotTo(HaveOccurred(), "creating GatewayClass") + + By("create Gateway with a Passthrough listener") + Expect(s.CreateResourceFromString(fmt.Sprintf(passthroughGateway, s.Namespace()))).NotTo(HaveOccurred(), "creating Gateway") + + By("deploy the TLS backend") + s.DeployNginx(framework.NginxOptions{ + Namespace: s.Namespace(), + Replicas: ptr.To(int32(1)), + }) + }) + + It("forwards the stream to the backend that owns the certificate", func() { + s.ResourceApplied("TLSRoute", "tls-passthrough-route", passthroughRoute, 1) + + // The client verifies the served chain against the backend's own CA. + // The gateway holds no certificate for this listener - Passthrough + // takes no certificateRefs - so a chain that validates here can only + // have come from nginx, which is what passthrough means. + s.RequestAssert(&scaffold.RequestAssert{ + Client: s.NewAPISIXClientWithTLSPassthrough(backendSNI, []byte(framework.TestCACert)), + Method: http.MethodGet, + Path: "/", + Checks: []scaffold.ResponseCheckFunc{ + scaffold.WithExpectedStatus(http.StatusOK), + scaffold.WithExpectedBodyContains("Hello, World!"), + }, + Timeout: time.Minute * 3, + Interval: time.Second * 2, + }) + }) + }) }) diff --git a/test/e2e/scaffold/apisix_deployer.go b/test/e2e/scaffold/apisix_deployer.go index 1edc6536..a08b3885 100644 --- a/test/e2e/scaffold/apisix_deployer.go +++ b/test/e2e/scaffold/apisix_deployer.go @@ -43,6 +43,11 @@ type APISIXDeployOptions struct { ServiceType string ServiceHTTPPort int ServiceHTTPSPort int + // ServiceHTTPSTargetPort is the container port the HTTPS service port + // forwards to: APISIX's HTTP ssl listen (9443) by default, or its stream + // tls_passthrough listen (9120). See the manifest for why the two cannot + // share a port. + ServiceHTTPSTargetPort int ConfigProvider string Replicas *int @@ -175,11 +180,12 @@ func (s *APISIXDeployer) AfterEach() { func (s *APISIXDeployer) DeployDataplane(deployOpts DeployDataplaneOptions) { opts := APISIXDeployOptions{ - Namespace: s.namespace, - AdminKey: s.runtimeOpts.APISIXAdminAPIKey, - ServiceHTTPPort: 9080, - ServiceHTTPSPort: 9443, - Replicas: ptr.To(1), + Namespace: s.namespace, + AdminKey: s.runtimeOpts.APISIXAdminAPIKey, + ServiceHTTPPort: 9080, + ServiceHTTPSPort: 9443, + ServiceHTTPSTargetPort: 9443, + Replicas: ptr.To(1), } if deployOpts.Namespace != "" { @@ -194,6 +200,9 @@ func (s *APISIXDeployer) DeployDataplane(deployOpts DeployDataplaneOptions) { if deployOpts.ServiceHTTPSPort != 0 { opts.ServiceHTTPSPort = deployOpts.ServiceHTTPSPort } + if deployOpts.ServiceHTTPSTargetPort != 0 { + opts.ServiceHTTPSTargetPort = deployOpts.ServiceHTTPSTargetPort + } if deployOpts.AdminKey != "" { opts.AdminKey = deployOpts.AdminKey } @@ -412,10 +421,11 @@ func (s *APISIXDeployer) CreateAdditionalGatewayWithOptions(namePrefix string, o // Deploy dataplane for this additional gateway o := APISIXDeployOptions{ - Namespace: additionalNS, - AdminKey: adminKey, - ServiceHTTPPort: 9080, - ServiceHTTPSPort: 9443, + Namespace: additionalNS, + AdminKey: adminKey, + ServiceHTTPPort: 9080, + ServiceHTTPSPort: 9443, + ServiceHTTPSTargetPort: 9443, } if opts.Namespace != "" { o.Namespace = opts.Namespace @@ -429,6 +439,9 @@ func (s *APISIXDeployer) CreateAdditionalGatewayWithOptions(namePrefix string, o if opts.ServiceHTTPSPort != 0 { o.ServiceHTTPSPort = opts.ServiceHTTPSPort } + if opts.ServiceHTTPSTargetPort != 0 { + o.ServiceHTTPSTargetPort = opts.ServiceHTTPSTargetPort + } if opts.ProviderType != "" { if opts.ProviderType == framework.ProviderTypeAPISIX { o.ConfigProvider = framework.ConfigProviderTypeEtcd diff --git a/test/e2e/scaffold/apisix_prewarm.go b/test/e2e/scaffold/apisix_prewarm.go index af913b64..8135100f 100644 --- a/test/e2e/scaffold/apisix_prewarm.go +++ b/test/e2e/scaffold/apisix_prewarm.go @@ -166,13 +166,14 @@ func provisionDataplane(t *bgTestingT, env *pooledEnv, _ Options) (*corev1.Servi } deployOpts := APISIXDeployOptions{ - Namespace: env.namespace, - AdminKey: env.adminKey, - ServiceName: serviceName, - ServiceHTTPPort: 9080, - ServiceHTTPSPort: 9443, - ConfigProvider: configProvider, - Replicas: ptr.To(1), + Namespace: env.namespace, + AdminKey: env.adminKey, + ServiceName: serviceName, + ServiceHTTPPort: 9080, + ServiceHTTPSPort: 9443, + ServiceHTTPSTargetPort: 9443, + ConfigProvider: configProvider, + Replicas: ptr.To(1), } buf := bytes.NewBuffer(nil) if err := framework.APISIXStandaloneTpl.Execute(buf, &deployOpts); err != nil { diff --git a/test/e2e/scaffold/deployer.go b/test/e2e/scaffold/deployer.go index d9ddb2f6..ad412768 100644 --- a/test/e2e/scaffold/deployer.go +++ b/test/e2e/scaffold/deployer.go @@ -45,7 +45,10 @@ type DeployDataplaneOptions struct { SkipCreateTunnels bool ServiceHTTPPort int ServiceHTTPSPort int - Replicas *int - AdminKey string - ProviderType string + // ServiceHTTPSTargetPort redirects the HTTPS service port at the data + // plane's stream tls_passthrough listen instead of its HTTP ssl listen. + ServiceHTTPSTargetPort int + Replicas *int + AdminKey string + ProviderType string } diff --git a/test/e2e/scaffold/scaffold.go b/test/e2e/scaffold/scaffold.go index db765d8f..20f19289 100644 --- a/test/e2e/scaffold/scaffold.go +++ b/test/e2e/scaffold/scaffold.go @@ -20,11 +20,14 @@ package scaffold import ( "context" "crypto/tls" + "crypto/x509" "fmt" + "net" "net/http" "net/url" "os" "strings" + "sync" "time" "github.com/api7/gopkg/pkg/log" @@ -95,6 +98,9 @@ type Tunnels struct { TCP Tunnel HTTP2 Tunnel TLS Tunnel + // TLSPassthrough reaches the stream listen that prereads the SNI and + // forwards the connection without terminating it. + TLSPassthrough Tunnel } func (t *Tunnels) Close() { @@ -118,6 +124,10 @@ func (t *Tunnels) Close() { t.safeClose(t.TLS.Close) t.TLS = nil } + if t.TLSPassthrough != nil { + t.safeClose(t.TLSPassthrough.Close) + t.TLSPassthrough = nil + } } func (t *Tunnels) safeClose(close func()) { @@ -305,6 +315,176 @@ func (s *Scaffold) NewAPISIXClientWithTCPProxy() *httpexpect.Expect { }) } +// tlsPassthroughPortName is the data plane Service port that forwards to the +// APISIX stream listen configured with tls_passthrough. +const tlsPassthroughPortName = "tls-passthrough" + +// NewAPISIXClientWithTLSPassthrough dials the data plane's TLS passthrough +// stream listen with sni and verifies the served chain against caCert. +// +// Verifying is the point: the gateway holds no certificate for a passthrough +// listener, so a chain that validates against the backend's own CA can only +// have come from the backend itself. +func (s *Scaffold) NewAPISIXClientWithTLSPassthrough(sni string, caCert []byte) *httpexpect.Expect { + pool := x509.NewCertPool() + Expect(pool.AppendCertsFromPEM(caCert)).To(BeTrue(), "parsing CA certificate") + + forward := s.newTLSPassthroughForward(&tls.Config{ServerName: sni, RootCAs: pool}) + + u := url.URL{ + Scheme: apiv2.SchemeHTTPS, + Host: forward.endpoint(), + } + return httpexpect.WithConfig(httpexpect.Config{ + BaseURL: u.String(), + Client: &http.Client{ + Transport: &http.Transport{ + // Every request dials afresh, so a forwarder that died since the + // last one is noticed and replaced rather than reused. + DisableKeepAlives: true, + DialTLSContext: forward.dialTLSContext, + }, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + }, + Reporter: httpexpect.NewAssertReporter( + httpexpect.NewAssertReporter(s.GinkgoT), + ), + }) +} + +// tlsPassthroughForward owns the port-forward to the data plane's +// tls_passthrough stream port and replaces it whenever it stops working. +// +// Two things make this port different from the others the scaffold forwards. +// +// A tls_passthrough listen prereads the ClientHello to pick a stream route, and +// resets a connection no route matches. The port-forward treats that reset as +// fatal and exits, taking the local listener with it - every later dial then +// gets "connection refused" rather than anything describing the cause. A +// handshake that succeeded once does not rule this out: the data plane workers +// load a new stream route independently, so the next connection can still land +// on one that has not. So the forward is verified with a real handshake for the +// SNI, never a bare TCP dial, and any failed dial rebuilds it instead of +// leaving every later retry to hit a forwarder that is already gone. +// +// It is also forwarded on demand rather than alongside the HTTP tunnels when the +// data plane is deployed: APISIX opens its stream listens later than its HTTP +// ones, and only this one spec dials this port, so the rest of the suite would +// pay for a forward it never uses. +type tlsPassthroughForward struct { + s *Scaffold + opts k8s.KubectlOptions + service string + port int + tlsConfig *tls.Config + + mu sync.Mutex + tunnel *k8s.Tunnel +} + +func (s *Scaffold) newTLSPassthroughForward(tlsConfig *tls.Config) *tlsPassthroughForward { + svc := s.dataplaneService + Expect(svc).NotTo(BeNil(), "data plane service") + + var port int + for _, p := range svc.Spec.Ports { + if p.Name == tlsPassthroughPortName { + port = int(p.Port) + } + } + Expect(port).NotTo(BeZero(), "data plane service has no %s port", tlsPassthroughPortName) + + // Copy the scaffold's options rather than building fresh ones: a run that + // configures a kubeconfig or context would otherwise be ignored here while + // every other dataplane tunnel honours it. + f := &tlsPassthroughForward{ + s: s, + opts: *s.kubectlOptions, + service: svc.Name, + port: port, + tlsConfig: tlsConfig, + } + f.opts.Namespace = svc.Namespace + + Eventually(func() error { + t, err := f.open() + if err != nil { + return err + } + f.replace(t) + return nil + }).WithTimeout(time.Minute*2).WithPolling(time.Second*2). + Should(Succeed(), "forwarding the tls passthrough port") + return f +} + +// open forwards the port and proves the forward with a handshake, since +// ForwardPortE returns before the forward has proxied anything. +func (f *tlsPassthroughForward) open() (*k8s.Tunnel, error) { + t := k8s.NewTunnel(&f.opts, k8s.ResourceTypeService, f.service, 0, f.port) + if err := t.ForwardPortE(f.s.t); err != nil { + return nil, err + } + conn, err := f.handshake(context.Background(), t.Endpoint()) + if err != nil { + t.Close() + return nil, err + } + _ = conn.Close() + return t, nil +} + +func (f *tlsPassthroughForward) handshake(ctx context.Context, endpoint string) (*tls.Conn, error) { + d := &tls.Dialer{ + NetDialer: &net.Dialer{Timeout: 10 * time.Second}, + Config: f.tlsConfig.Clone(), + } + conn, err := d.DialContext(ctx, "tcp", endpoint) + if err != nil { + return nil, err + } + return conn.(*tls.Conn), nil +} + +// replace installs t as the current forward and closes the one it supersedes. +// Tunnels.Close only knows about the tunnel stored there, so a superseded +// forward would otherwise keep running until the process exits. +func (f *tlsPassthroughForward) replace(t *k8s.Tunnel) { + if f.tunnel != nil { + f.tunnel.Close() + } else if previous := f.s.apisixTunnels.TLSPassthrough; previous != nil { + previous.Close() + } + f.tunnel = t + f.s.apisixTunnels.TLSPassthrough = t +} + +func (f *tlsPassthroughForward) endpoint() string { + f.mu.Lock() + defer f.mu.Unlock() + return f.tunnel.Endpoint() +} + +// dialTLSContext ignores addr: the request URL names whichever local port the +// forward had when the client was built, and that port changes on a rebuild. +func (f *tlsPassthroughForward) dialTLSContext(ctx context.Context, _, _ string) (net.Conn, error) { + f.mu.Lock() + defer f.mu.Unlock() + + conn, err := f.handshake(ctx, f.tunnel.Endpoint()) + if err == nil { + return conn, nil + } + t, rebuildErr := f.open() + if rebuildErr != nil { + return nil, fmt.Errorf("%w; rebuilding the tls passthrough forward: %v", err, rebuildErr) + } + f.replace(t) + return f.handshake(ctx, t.Endpoint()) +} + func (s *Scaffold) NewAPISIXClientWithTLSProxy(host string) *httpexpect.Expect { u := url.URL{ Scheme: apiv2.SchemeHTTPS, @@ -478,6 +658,9 @@ func (s *Scaffold) createDataplaneTunnels( tunnels.HTTP2 = http2Tunnel } + // The TLS passthrough tunnel is deliberately not forwarded here. See + // tlsPassthroughForward. + return tunnels, nil }