From de4c7be95736e84689f6305b2a848cc3ab3eedfc Mon Sep 17 00:00:00 2001 From: Steve Sloka Date: Tue, 8 Jun 2021 13:00:07 -0700 Subject: [PATCH] handle tls route gateway.listener modes (#3748) Fixup issue where the listener protocol wasn't checked for type=tls. Also, adds logic to validate the TLS.Mode is valid. Signed-off-by: Steve Sloka --- internal/dag/builder_test.go | 274 +++++++++++++++++++++++++- internal/dag/gatewayapi_processor.go | 37 +++- test/e2e/gateway/008_tlsroute_test.go | 5 +- test/e2e/gateway/gateway_test.go | 55 +++++- 4 files changed, 354 insertions(+), 17 deletions(-) diff --git a/internal/dag/builder_test.go b/internal/dag/builder_test.go index b44ab8a9282..dfc4012a86f 100644 --- a/internal/dag/builder_test.go +++ b/internal/dag/builder_test.go @@ -214,7 +214,51 @@ func TestDAGInsertGatewayAPI(t *testing.T) { Spec: gatewayapi_v1alpha1.GatewaySpec{ Listeners: []gatewayapi_v1alpha1.Listener{{ Port: 80, - Protocol: gatewayapi_v1alpha1.HTTPProtocolType, + Protocol: gatewayapi_v1alpha1.TLSProtocolType, + Routes: gatewayapi_v1alpha1.RouteBindingSelector{ + Kind: KindTLSRoute, + Namespaces: &gatewayapi_v1alpha1.RouteNamespaces{ + From: routeSelectTypePtr(gatewayapi_v1alpha1.RouteSelectAll), + }, + }, + }}, + }, + } + + gatewayTLSRouteModePassthrough := &gatewayapi_v1alpha1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "contour", + Namespace: "projectcontour", + }, + Spec: gatewayapi_v1alpha1.GatewaySpec{ + Listeners: []gatewayapi_v1alpha1.Listener{{ + Port: 80, + Protocol: gatewayapi_v1alpha1.TLSProtocolType, + TLS: &gatewayapi_v1alpha1.GatewayTLSConfig{ + Mode: tlsModeTypePtr(gatewayapi_v1alpha1.TLSModePassthrough), + }, + Routes: gatewayapi_v1alpha1.RouteBindingSelector{ + Kind: KindTLSRoute, + Namespaces: &gatewayapi_v1alpha1.RouteNamespaces{ + From: routeSelectTypePtr(gatewayapi_v1alpha1.RouteSelectAll), + }, + }, + }}, + }, + } + + gatewayTLSRouteModeTerminate := &gatewayapi_v1alpha1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "contour", + Namespace: "projectcontour", + }, + Spec: gatewayapi_v1alpha1.GatewaySpec{ + Listeners: []gatewayapi_v1alpha1.Listener{{ + Port: 80, + Protocol: gatewayapi_v1alpha1.TLSProtocolType, + TLS: &gatewayapi_v1alpha1.GatewayTLSConfig{ + Mode: tlsModeTypePtr(gatewayapi_v1alpha1.TLSModeTerminate), + }, Routes: gatewayapi_v1alpha1.RouteBindingSelector{ Kind: KindTLSRoute, Namespaces: &gatewayapi_v1alpha1.RouteNamespaces{ @@ -252,7 +296,7 @@ func TestDAGInsertGatewayAPI(t *testing.T) { Spec: gatewayapi_v1alpha1.GatewaySpec{ Listeners: []gatewayapi_v1alpha1.Listener{{ Port: 80, - Protocol: gatewayapi_v1alpha1.HTTPProtocolType, + Protocol: gatewayapi_v1alpha1.TLSProtocolType, Routes: gatewayapi_v1alpha1.RouteBindingSelector{ Kind: KindTLSRoute, Namespaces: &gatewayapi_v1alpha1.RouteNamespaces{ @@ -926,6 +970,228 @@ func TestDAGInsertGatewayAPI(t *testing.T) { }, want: listeners(), }, + "TLSRoute with TLS.Mode=Passthrough is valid": { + gateway: gatewayTLSRouteModePassthrough, + objs: []interface{}{ + &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "custom", + Labels: map[string]string{ + "app": "contour", + "type": "controller", + }, + }, + }, + kuardServiceCustomNs, + &gatewayapi_v1alpha1.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "basic", + Namespace: "custom", + }, + Spec: gatewayapi_v1alpha1.TLSRouteSpec{ + Gateways: &gatewayapi_v1alpha1.RouteGateways{ + Allow: gatewayAllowTypePtr(gatewayapi_v1alpha1.GatewayAllowAll), + }, + Rules: []gatewayapi_v1alpha1.TLSRouteRule{{ + Matches: []gatewayapi_v1alpha1.TLSRouteMatch{{ + SNIs: []gatewayapi_v1alpha1.Hostname{ + "test.projectcontour.io", + }, + }}, + ForwardTo: tcpRouteForwardTo("kuard", 8080, 0), + }}, + }, + }, + }, + want: listeners( + &Listener{ + Port: 443, + VirtualHosts: virtualhosts( + &SecureVirtualHost{ + VirtualHost: VirtualHost{ + Name: "test.projectcontour.io", + ListenerName: "ingress_https", + }, + TCPProxy: &TCPProxy{ + Clusters: clusters(service(kuardServiceCustomNs)), + }, + }, + ), + }, + ), + }, + "TLSRoute with TLS.Mode=Passthrough is invalid if certificateRef is specified": { + gateway: &gatewayapi_v1alpha1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "contour", + Namespace: "projectcontour", + }, + Spec: gatewayapi_v1alpha1.GatewaySpec{ + Listeners: []gatewayapi_v1alpha1.Listener{{ + Port: 80, + Protocol: gatewayapi_v1alpha1.TLSProtocolType, + TLS: &gatewayapi_v1alpha1.GatewayTLSConfig{ + Mode: tlsModeTypePtr(gatewayapi_v1alpha1.TLSModePassthrough), + CertificateRef: &gatewayapi_v1alpha1.LocalObjectReference{ + Group: "core", + Kind: "Secret", + Name: sec1.Name, + }, + }, + Routes: gatewayapi_v1alpha1.RouteBindingSelector{ + Kind: KindTLSRoute, + Namespaces: &gatewayapi_v1alpha1.RouteNamespaces{ + From: routeSelectTypePtr(gatewayapi_v1alpha1.RouteSelectAll), + }, + }, + }}, + }, + }, + objs: []interface{}{ + kuardService, + &gatewayapi_v1alpha1.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "basic", + Namespace: "projectcontour", + }, + Spec: gatewayapi_v1alpha1.TLSRouteSpec{ + Gateways: &gatewayapi_v1alpha1.RouteGateways{ + Allow: gatewayAllowTypePtr(gatewayapi_v1alpha1.GatewayAllowAll), + }, + Rules: []gatewayapi_v1alpha1.TLSRouteRule{{ + Matches: []gatewayapi_v1alpha1.TLSRouteMatch{{ + SNIs: []gatewayapi_v1alpha1.Hostname{ + "test.projectcontour.io", + }, + }}, + ForwardTo: tcpRouteForwardTo("kuard", 8080, 0), + }}, + }, + }, + }, + want: listeners(), + }, + "TLSRoute with TLS.Mode=Terminate is invalid when TLS is not defined": { + gateway: gatewayTLSRouteModeTerminate, + objs: []interface{}{ + &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "custom", + Labels: map[string]string{ + "app": "contour", + "type": "controller", + }, + }, + }, + kuardServiceCustomNs, + &gatewayapi_v1alpha1.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "basic", + Namespace: "custom", + }, + Spec: gatewayapi_v1alpha1.TLSRouteSpec{ + Gateways: &gatewayapi_v1alpha1.RouteGateways{ + Allow: gatewayAllowTypePtr(gatewayapi_v1alpha1.GatewayAllowAll), + }, + Rules: []gatewayapi_v1alpha1.TLSRouteRule{{ + Matches: []gatewayapi_v1alpha1.TLSRouteMatch{{ + SNIs: []gatewayapi_v1alpha1.Hostname{ + "test.projectcontour.io", + }, + }}, + ForwardTo: tcpRouteForwardTo("kuard", 8080, 0), + }}, + }, + }, + }, + want: listeners(), + }, + "TLSRoute with invalid listener protocol of HTTP": { + gateway: &gatewayapi_v1alpha1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "contour", + Namespace: "projectcontour", + }, + Spec: gatewayapi_v1alpha1.GatewaySpec{ + Listeners: []gatewayapi_v1alpha1.Listener{{ + Port: 80, + Protocol: gatewayapi_v1alpha1.HTTPProtocolType, + TLS: &gatewayapi_v1alpha1.GatewayTLSConfig{ + Mode: tlsModeTypePtr(gatewayapi_v1alpha1.TLSModePassthrough), + CertificateRef: &gatewayapi_v1alpha1.LocalObjectReference{ + Group: "core", + Kind: "Secret", + Name: sec1.Name, + }, + }, + Routes: gatewayapi_v1alpha1.RouteBindingSelector{ + Kind: KindTLSRoute, + Namespaces: &gatewayapi_v1alpha1.RouteNamespaces{ + From: routeSelectTypePtr(gatewayapi_v1alpha1.RouteSelectAll), + }, + }, + }}, + }, + }, + objs: []interface{}{ + kuardService, + &gatewayapi_v1alpha1.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "basic", + Namespace: "projectcontour", + }, + Spec: gatewayapi_v1alpha1.TLSRouteSpec{ + Gateways: &gatewayapi_v1alpha1.RouteGateways{ + Allow: gatewayAllowTypePtr(gatewayapi_v1alpha1.GatewayAllowAll), + }, + Rules: []gatewayapi_v1alpha1.TLSRouteRule{{ + Matches: []gatewayapi_v1alpha1.TLSRouteMatch{{ + SNIs: []gatewayapi_v1alpha1.Hostname{ + "test.projectcontour.io", + }, + }}, + ForwardTo: tcpRouteForwardTo("kuard", 8080, 0), + }}, + }, + }, + }, + want: listeners(), + }, + "TLSRoute with invalid listener kind": { + gateway: gatewayNoSelector, + objs: []interface{}{ + &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "custom", + Labels: map[string]string{ + "app": "contour", + "type": "controller", + }, + }, + }, + kuardServiceCustomNs, + &gatewayapi_v1alpha1.TLSRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "basic", + Namespace: "custom", + }, + Spec: gatewayapi_v1alpha1.TLSRouteSpec{ + Gateways: &gatewayapi_v1alpha1.RouteGateways{ + Allow: gatewayAllowTypePtr(gatewayapi_v1alpha1.GatewayAllowAll), + }, + Rules: []gatewayapi_v1alpha1.TLSRouteRule{{ + Matches: []gatewayapi_v1alpha1.TLSRouteMatch{{ + SNIs: []gatewayapi_v1alpha1.Hostname{ + "test.projectcontour.io", + }, + }}, + ForwardTo: tcpRouteForwardTo("kuard", 8080, 0), + }}, + }, + }, + }, + want: listeners(), + }, // TLSRoute is in a different namespace than the Gateway, // but will be allowed since "All" is set. "TLSRoute: RouteGateways with GatewayAllowType: All": { @@ -10187,3 +10453,7 @@ func withMirror(r *Route, mirror *Service) *Route { func routeSelectTypePtr(rst gatewayapi_v1alpha1.RouteSelectType) *gatewayapi_v1alpha1.RouteSelectType { return &rst } + +func tlsModeTypePtr(mode gatewayapi_v1alpha1.TLSModeType) *gatewayapi_v1alpha1.TLSModeType { + return &mode +} diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go index 55325c0026d..e78d89a5994 100644 --- a/internal/dag/gatewayapi_processor.go +++ b/internal/dag/gatewayapi_processor.go @@ -78,7 +78,7 @@ func (p *GatewayAPIProcessor) Run(dag *DAG, source *KubernetesCache) { var matchingTLSRoutes []*gatewayapi_v1alpha1.TLSRoute var listenerSecret *Secret - // Validate the Kind on the selector is a supported type. + // Validate the Protocol on the selector is a supported type. switch listener.Protocol { case gatewayapi_v1alpha1.HTTPSProtocolType: // Validate that if protocol is type HTTPS, that TLS is defined. @@ -93,7 +93,27 @@ func (p *GatewayAPIProcessor) Run(dag *DAG, source *KubernetesCache) { // routes to be bound to this listener since it can't serve TLS traffic. continue } - case gatewayapi_v1alpha1.HTTPProtocolType, gatewayapi_v1alpha1.TLSProtocolType: + case gatewayapi_v1alpha1.TLSProtocolType: + + if tlsConfig := listener.TLS; tlsConfig != nil { + if tlsConfig.Mode != nil { + switch *tlsConfig.Mode { + case gatewayapi_v1alpha1.TLSModeTerminate: + // Check for TLS on the Gateway. + if listenerSecret = p.validGatewayTLS(listener); listenerSecret == nil { + // If TLS was configured on the Listener, but it's invalid, don't allow any + // routes to be bound to this listener since it can't serve TLS traffic. + continue + } + case gatewayapi_v1alpha1.TLSModePassthrough: + if listener.TLS.CertificateRef != nil { + p.Errorf("Listener.TLS cannot be defined when TLS Mode is %q.", tlsConfig.Mode) + continue + } + } + } + } + case gatewayapi_v1alpha1.HTTPProtocolType: break default: p.Errorf("Listener.Protocol %q is not supported.", listener.Protocol) @@ -157,6 +177,13 @@ func (p *GatewayAPIProcessor) Run(dag *DAG, source *KubernetesCache) { } } case KindTLSRoute: + + // Validate the listener protocol is type=TLS. + if listener.Protocol != gatewayapi_v1alpha1.TLSProtocolType { + p.Errorf("invalid listener protocol %q for Kind: TLSRoute", listener.Protocol) + continue + } + for _, route := range p.source.tlsroutes { // Filter the TLSRoutes that match the gateway which Contour is configured to watch. // RouteBindingSelector defines a schema for associating routes with the Gateway. @@ -216,12 +243,6 @@ func (p *GatewayAPIProcessor) validGatewayTLS(listener gatewayapi_v1alpha1.Liste return nil } - // Validate the correct protocol is specified. - if listener.Protocol != gatewayapi_v1alpha1.HTTPSProtocolType { - p.Errorf("Spec.VirtualHost.Protocol %q is not valid.", listener.Protocol) - return nil - } - // Validate a v1.Secret is referenced which can be kind: secret & group: core. // ref: https://github.com/kubernetes-sigs/gateway-api/pull/562 if !isSecretRef(listener.TLS.CertificateRef) { diff --git a/test/e2e/gateway/008_tlsroute_test.go b/test/e2e/gateway/008_tlsroute_test.go index c0f4ca9ddc7..46dd92f0437 100644 --- a/test/e2e/gateway/008_tlsroute_test.go +++ b/test/e2e/gateway/008_tlsroute_test.go @@ -27,9 +27,8 @@ import ( gatewayv1alpha1 "sigs.k8s.io/gateway-api/apis/v1alpha1" ) -func testTLSRoute(fx *e2e.Framework) { +func testTLSRoutePassthrough(fx *e2e.Framework, namespace string) { t := fx.T() - namespace := "gateway-008-tlsroute" fx.CreateNamespace(namespace) defer fx.DeleteNamespace(namespace) @@ -37,6 +36,7 @@ func testTLSRoute(fx *e2e.Framework) { fx.Fixtures.EchoSecure.Deploy(namespace, "echo") fx.Certs.CreateSelfSignedCert(namespace, "backend-server-cert", "backend-server-cert", "tlsroute.gatewayapi.projectcontour.io") + // TLSRoute that doesn't define the termination type. route := &gatewayv1alpha1.TLSRoute{ ObjectMeta: metav1.ObjectMeta{ Namespace: namespace, @@ -99,5 +99,4 @@ func testTLSRoute(fx *e2e.Framework) { }) assert.Truef(t, ok, "expected 200 response code, got %d", res.StatusCode) assert.Equal(t, "echo", fx.GetEchoResponseBody(res.Body).Service) - } diff --git a/test/e2e/gateway/gateway_test.go b/test/e2e/gateway/gateway_test.go index 0cc285e4857..69e659916f6 100644 --- a/test/e2e/gateway/gateway_test.go +++ b/test/e2e/gateway/gateway_test.go @@ -110,7 +110,7 @@ var _ = Describe("Gateway API", func() { }) }) - Describe("TLS Gateway", func() { + Describe("HTTPRoute: TLS Gateway", func() { var gateway *gatewayv1alpha1.Gateway var cleanupCert func() @@ -171,12 +171,12 @@ var _ = Describe("Gateway API", func() { cleanupCert() }) - It("004-tls-gateway", func() { + It("004-httproute-tls-gateway", func() { testTLSGateway(f) }) }) - Describe("TLSRoute Gateway", func() { + Describe("TLSRoute: Gateway", func() { var gateway *gatewayv1alpha1.Gateway BeforeEach(func() { @@ -212,7 +212,50 @@ var _ = Describe("Gateway API", func() { }) It("008-tlsroute", func() { - testTLSRoute(f) + testTLSRoutePassthrough(f, "gateway-008-tlsroute") + }) + }) + + Describe("TLSRoute Gateway: Mode: Passthrough", func() { + var gateway *gatewayv1alpha1.Gateway + + BeforeEach(func() { + gateway = &gatewayv1alpha1.Gateway{ + // Namespace and name need to match what's + // configured in the Contour config file. + ObjectMeta: metav1.ObjectMeta{ + Namespace: "projectcontour", + Name: "contour", + }, + Spec: gatewayv1alpha1.GatewaySpec{ + GatewayClassName: "contour-class", + Listeners: []gatewayv1alpha1.Listener{ + { + Protocol: gatewayv1alpha1.TLSProtocolType, + Port: gatewayv1alpha1.PortNumber(443), + TLS: &gatewayv1alpha1.GatewayTLSConfig{ + Mode: tlsModeTypePtr(gatewayv1alpha1.TLSModePassthrough), + }, + Routes: gatewayv1alpha1.RouteBindingSelector{ + Kind: "TLSRoute", + Namespaces: &gatewayv1alpha1.RouteNamespaces{ + From: routeSelectTypePtr(gatewayv1alpha1.RouteSelectAll), + }, + }, + }, + }, + }, + } + + require.NoError(f.T(), f.Client.Create(context.TODO(), gateway)) + }) + + AfterEach(func() { + require.NoError(f.T(), f.Client.Delete(context.TODO(), gateway)) + }) + + It("008-tlsroute-mode-passthrough", func() { + testTLSRoutePassthrough(f, "gateway-008-tlsroute-mode-passthrough") }) }) @@ -308,6 +351,10 @@ func gatewayAllowTypePtr(val gatewayv1alpha1.GatewayAllowType) *gatewayv1alpha1. return &val } +func tlsModeTypePtr(mode gatewayv1alpha1.TLSModeType) *gatewayv1alpha1.TLSModeType { + return &mode +} + // httpRouteAdmitted returns true if the route has a .status.conditions // entry of "Admitted: true". func httpRouteAdmitted(route *gatewayv1alpha1.HTTPRoute) bool {