From 222240e8e816064b0cf286226b7bb07632532cda Mon Sep 17 00:00:00 2001 From: Mattia Lavacca Date: Wed, 22 Mar 2023 19:00:03 +0100 Subject: [PATCH 01/10] test(conformance): tls conformance test enabled Signed-off-by: Mattia Lavacca --- internal/controllers/gateway/gateway_controller.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/internal/controllers/gateway/gateway_controller.go b/internal/controllers/gateway/gateway_controller.go index e33de1fbae..76cd244291 100644 --- a/internal/controllers/gateway/gateway_controller.go +++ b/internal/controllers/gateway/gateway_controller.go @@ -749,6 +749,7 @@ func (r *GatewayReconciler) determineListenersFromDataPlane( // are configured for them in the data-plane. upgradedListeners := make([]Listener, 0, len(listeners)) for _, listener := range listeners { + newListeners := make([]Listener, 0) if streamListener, ok := streamListenersMap[portMapper[int(listener.Port)]]; ok { if streamListener.SSL { listener.Protocol = gatewayv1beta1.TLSProtocolType @@ -757,6 +758,7 @@ func (r *GatewayReconciler) determineListenersFromDataPlane( {Group: &gatewayV1beta1Group, Kind: (Kind)("TLSRoute")}, }, } + newListeners = append(newListeners, listener) } } if proxyListener, ok := proxyListenersMap[portMapper[int(listener.Port)]]; ok { @@ -767,6 +769,12 @@ func (r *GatewayReconciler) determineListenersFromDataPlane( {Group: &gatewayV1beta1Group, Kind: (Kind)("HTTPRoute")}, }, } + tlsListener := listener + tlsListener.Protocol = gatewayv1beta1.TLSProtocolType + tlsListener.AllowedRoutes.Kinds = append(tlsListener.AllowedRoutes.Kinds, + gatewayv1beta1.RouteGroupKind{Group: &gatewayV1beta1Group, Kind: (Kind)("TLSRoute")}) + newListeners = append(newListeners, listener) + newListeners = append(newListeners, tlsListener) } else { listener.Protocol = gatewayv1beta1.HTTPProtocolType listener.AllowedRoutes = &gatewayv1beta1.AllowedRoutes{ @@ -774,9 +782,10 @@ func (r *GatewayReconciler) determineListenersFromDataPlane( {Group: &gatewayV1beta1Group, Kind: (Kind)("HTTPRoute")}, }, } + newListeners = append(newListeners, listener) } } - upgradedListeners = append(upgradedListeners, listener) + upgradedListeners = append(upgradedListeners, newListeners...) } return upgradedListeners, nil From b91f28a5eeabf440dd0a7b772250175f7a69ca24 Mon Sep 17 00:00:00 2001 From: Mattia Lavacca Date: Wed, 26 Jul 2023 17:14:22 +0200 Subject: [PATCH 02/10] workarounds and hardcodings to make it work Signed-off-by: Mattia Lavacca --- .../controllers/gateway/gateway_controller.go | 40 ++++++++++++++++--- internal/manager/config.go | 4 ++ internal/manager/controllerdef.go | 1 + test/conformance/gateway_conformance_test.go | 19 ++++----- 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/internal/controllers/gateway/gateway_controller.go b/internal/controllers/gateway/gateway_controller.go index 76cd244291..806060a304 100644 --- a/internal/controllers/gateway/gateway_controller.go +++ b/internal/controllers/gateway/gateway_controller.go @@ -62,6 +62,7 @@ type GatewayReconciler struct { //nolint:revive PublishServiceRef k8stypes.NamespacedName PublishServiceUDPRef mo.Option[k8stypes.NamespacedName] + PublishServiceTLSRef mo.Option[k8stypes.NamespacedName] // If enableReferenceGrant is true, controller will watch ReferenceGrants // to invalidate or allow cross-namespace TLSConfigs in gateways. @@ -451,6 +452,11 @@ func (r *GatewayReconciler) reconcileUnmanagedGateway(ctx context.Context, log l services = append(services, udpRef.String()) } + // TLS service is optional. + if tlsRef, ok := r.PublishServiceTLSRef.Get(); ok { + services = append(services, tlsRef.String()) + } + servicesAnnotation := strings.Join(services, ",") debug(log, gateway, fmt.Sprintf("no unmanaged annotation, setting it to proxy services %s", services)) if gateway.Annotations == nil { @@ -461,13 +467,16 @@ func (r *GatewayReconciler) reconcileUnmanagedGateway(ctx context.Context, log l } serviceRefs := strings.Split(annotations.ExtractUnmanagedGatewayClassMode(gateway.Annotations), ",") + // validation check of the Gateway to ensure that the publish service is actually available // in the cluster. If it is not the object will be requeued until it exists (or is otherwise retrievable). debug(log, gateway, "gathering the gateway publish service") // this will also be done by the validating webhook, this is a fallback + var gatewayServices []*corev1.Service for _, ref := range serviceRefs { r.Log.V(util.DebugLevel).Info("determining service for ref", "ref", ref) - svc, err := r.determineServiceForGateway(ctx, ref) + + svc, err := r.determineServiceForGateway(ctx, ref, gateway.Spec.Listeners) if err != nil { log.Error(err, "could not determine service for gateway", "namespace", gateway.Namespace, "name", gateway.Name) return ctrl.Result{Requeue: true}, err @@ -609,20 +618,39 @@ func init() { // determineServiceForGateway provides the "publish service" (aka the proxy Service) object which // will be used to populate unmanaged gateways. -func (r *GatewayReconciler) determineServiceForGateway(ctx context.Context, ref string) (*corev1.Service, error) { +func (r *GatewayReconciler) determineServiceForGateway(ctx context.Context, ref string, listeners []gatewayv1beta1.Listener) (*corev1.Service, error) { // currently the gateway controller ONLY supports service references that correspond with the --publish-service // provided to the controller manager via flags when operating on unmanaged gateways. This constraint may // be loosened in later iterations if there is need. + protocols := map[gatewayv1beta1.ProtocolType]interface{}{} + for _, l := range listeners { + protocols[l.Protocol] = nil + } + var name k8stypes.NamespacedName switch { case ref == r.PublishServiceRef.String(): - name = r.PublishServiceRef + if _, ok := protocols[gatewayv1beta1.HTTPProtocolType]; ok { + name = r.PublishServiceRef + } + if _, ok := protocols[gatewayv1beta1.HTTPSProtocolType]; ok { + name = r.PublishServiceRef + } + if _, ok := protocols[gatewayv1beta1.TCPProtocolType]; ok { + name = r.PublishServiceRef + } case r.PublishServiceUDPRef.IsPresent() && ref == r.PublishServiceUDPRef.MustGet().String(): - name = r.PublishServiceUDPRef.MustGet() + if _, ok := protocols[gatewayv1beta1.UDPProtocolType]; ok { + name = r.PublishServiceUDPRef.MustGet() + } + case r.PublishServiceTLSRef.IsPresent() && ref == r.PublishServiceTLSRef.MustGet().String(): + if _, ok := protocols[gatewayv1beta1.TLSProtocolType]; ok { + name = r.PublishServiceTLSRef.MustGet() + } default: - return nil, fmt.Errorf("service ref %s did not match controller manager ref %s or %s", - ref, r.PublishServiceRef.String(), r.PublishServiceUDPRef.OrEmpty()) + return nil, fmt.Errorf("service ref %s did not match controller manager ref %s, %s or %s", + ref, r.PublishServiceRef.String(), r.PublishServiceUDPRef.OrEmpty(), r.PublishServiceTLSRef.OrEmpty()) } // retrieve the service for the kong gateway diff --git a/internal/manager/config.go b/internal/manager/config.go index 1fcc4672dd..378ac357ec 100644 --- a/internal/manager/config.go +++ b/internal/manager/config.go @@ -85,6 +85,7 @@ type Config struct { // Ingress status PublishServiceUDP OptionalNamespacedName + PublishServiceTLS OptionalNamespacedName PublishService OptionalNamespacedName PublishStatusAddress []string PublishStatusAddressUDP []string @@ -209,6 +210,9 @@ func (c *Config) FlagSet() *pflag.FlagSet { flagSet.Var(flags.NewValidatedValue(&c.PublishServiceUDP, namespacedNameFromFlagValue, nnTypeNameOverride), "publish-service-udp", `Service fronting UDP routing resources in `+ `"namespace/name" format. The controller will update UDP route status information with this Service's `+ `endpoints. If omitted, the same Service will be used for both TCP and UDP routes.`) + flagSet.Var(flags.NewValidatedValue(&c.PublishServiceTLS, namespacedNameFromFlagValue, nnTypeNameOverride), "publish-service-tls", `Service fronting TLS routing resources in `+ + `"namespace/name" format. The controller will update TLS route status information with this Service's `+ + `endpoints. If omitted, the same Service will be used for both TCP and UDP routes.`) flagSet.StringSliceVar(&c.PublishStatusAddressUDP, "publish-status-address-udp", []string{}, `User-provided address CSV, for use in lieu of "publish-service-udp" when that Service lacks useful address information.`) diff --git a/internal/manager/controllerdef.go b/internal/manager/controllerdef.go index da8bb72857..53f8dfcf5b 100644 --- a/internal/manager/controllerdef.go +++ b/internal/manager/controllerdef.go @@ -361,6 +361,7 @@ func setupControllers( DataplaneClient: dataplaneClient, PublishServiceRef: c.PublishService.OrEmpty(), PublishServiceUDPRef: c.PublishServiceUDP, + PublishServiceTLSRef: c.PublishServiceTLS, WatchNamespaces: c.WatchNamespaces, CacheSyncTimeout: c.CacheSyncTimeout, ReferenceIndexers: referenceIndexers, diff --git a/test/conformance/gateway_conformance_test.go b/test/conformance/gateway_conformance_test.go index 897651767e..d736f16776 100644 --- a/test/conformance/gateway_conformance_test.go +++ b/test/conformance/gateway_conformance_test.go @@ -75,7 +75,7 @@ var skippedTestsForTraditionalRoutes = []string{ // https://github.com/Kong/kubernetes-ingress-controller/issues/3680 tests.GatewayClassObservedGenerationBump.ShortName, // https://github.com/Kong/kubernetes-ingress-controller/issues/3678 - tests.TLSRouteSimpleSameNamespace.ShortName, + //tests.TLSRouteSimpleSameNamespace.ShortName, // https://github.com/Kong/kubernetes-ingress-controller/issues/3679 tests.HTTPRouteQueryParamMatching.ShortName, // https://github.com/Kong/kubernetes-ingress-controller/issues/3681 @@ -120,6 +120,7 @@ func TestGatewayConformance(t *testing.T) { "--debug-log-reduce-redundancy", featureGateFlag, "--anonymous-reports=false", + "--publish-service-tls=kong-system/ingress-controller-kong-tls-proxy", } require.NoError(t, testutils.DeployControllerManagerForCluster(ctx, globalDeprecatedLogger, globalLogger, env.Cluster(), args...)) @@ -147,14 +148,14 @@ func TestGatewayConformance(t *testing.T) { skippedTests = skippedTestsForExpressionRoutes } cSuite := suite.New(suite.Options{ - Client: client, - GatewayClassName: gatewayClass.Name, - Debug: showDebug, - CleanupBaseResources: shouldCleanup, - EnableAllSupportedFeatures: enableAllSupportedFeatures, - ExemptFeatures: exemptFeatures, - BaseManifests: conformanceTestsBaseManifests, - SkipTests: skippedTests, + Client: client, + GatewayClassName: gatewayClass.Name, + Debug: showDebug, + CleanupBaseResources: shouldCleanup, + ExemptFeatures: exemptFeatures, + BaseManifests: conformanceTestsBaseManifests, + SkipTests: skippedTests, + SupportedFeatures: suite.TLSCoreFeatures, }) cSuite.Setup(t) // To work with individual tests only, you can disable the normal Run call and construct a slice containing a From 65eb785f7bd7bd072f328cc04b4df8aaea8c2fc4 Mon Sep 17 00:00:00 2001 From: Mattia Lavacca Date: Wed, 26 Jul 2023 18:18:14 +0200 Subject: [PATCH 03/10] services and protocols took into account Signed-off-by: Mattia Lavacca --- .../controllers/gateway/gateway_controller.go | 41 ++++++++----------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/internal/controllers/gateway/gateway_controller.go b/internal/controllers/gateway/gateway_controller.go index 806060a304..b9c7b9f3ae 100644 --- a/internal/controllers/gateway/gateway_controller.go +++ b/internal/controllers/gateway/gateway_controller.go @@ -476,13 +476,15 @@ func (r *GatewayReconciler) reconcileUnmanagedGateway(ctx context.Context, log l for _, ref := range serviceRefs { r.Log.V(util.DebugLevel).Info("determining service for ref", "ref", ref) - svc, err := r.determineServiceForGateway(ctx, ref, gateway.Spec.Listeners) - if err != nil { - log.Error(err, "could not determine service for gateway", "namespace", gateway.Namespace, "name", gateway.Name) - return ctrl.Result{Requeue: true}, err - } - if svc != nil { - gatewayServices = append(gatewayServices, svc) + for _, l := range gateway.Spec.Listeners { + svc, err := r.determineServiceForGateway(ctx, ref, l.Protocol) + if err != nil { + log.Error(err, "could not determine service for gateway", "namespace", gateway.Namespace, "name", gateway.Name) + return ctrl.Result{Requeue: true}, err + } + if svc != nil { + gatewayServices = append(gatewayServices, svc) + } } } @@ -618,34 +620,25 @@ func init() { // determineServiceForGateway provides the "publish service" (aka the proxy Service) object which // will be used to populate unmanaged gateways. -func (r *GatewayReconciler) determineServiceForGateway(ctx context.Context, ref string, listeners []gatewayv1beta1.Listener) (*corev1.Service, error) { +func (r *GatewayReconciler) determineServiceForGateway(ctx context.Context, ref string, protocol gatewayv1beta1.ProtocolType) (*corev1.Service, error) { // currently the gateway controller ONLY supports service references that correspond with the --publish-service // provided to the controller manager via flags when operating on unmanaged gateways. This constraint may // be loosened in later iterations if there is need. - protocols := map[gatewayv1beta1.ProtocolType]interface{}{} - for _, l := range listeners { - protocols[l.Protocol] = nil - } - var name k8stypes.NamespacedName switch { case ref == r.PublishServiceRef.String(): - if _, ok := protocols[gatewayv1beta1.HTTPProtocolType]; ok { - name = r.PublishServiceRef - } - if _, ok := protocols[gatewayv1beta1.HTTPSProtocolType]; ok { - name = r.PublishServiceRef - } - if _, ok := protocols[gatewayv1beta1.TCPProtocolType]; ok { + if protocol == gatewayv1beta1.HTTPProtocolType || protocol == gatewayv1beta1.HTTPSProtocolType || protocol == gatewayv1beta1.TCPProtocolType { name = r.PublishServiceRef } - case r.PublishServiceUDPRef.IsPresent() && ref == r.PublishServiceUDPRef.MustGet().String(): - if _, ok := protocols[gatewayv1beta1.UDPProtocolType]; ok { + fallthrough + case r.PublishServiceUDPRef.IsPresent(): + if protocol == gatewayv1beta1.UDPProtocolType { name = r.PublishServiceUDPRef.MustGet() } - case r.PublishServiceTLSRef.IsPresent() && ref == r.PublishServiceTLSRef.MustGet().String(): - if _, ok := protocols[gatewayv1beta1.TLSProtocolType]; ok { + fallthrough + case r.PublishServiceTLSRef.IsPresent(): + if protocol == gatewayv1beta1.TLSProtocolType { name = r.PublishServiceTLSRef.MustGet() } default: From 0bef20c642330259cd8aee14102af6f30990daa2 Mon Sep 17 00:00:00 2001 From: Mattia Lavacca Date: Fri, 28 Jul 2023 12:08:24 +0200 Subject: [PATCH 04/10] tls service creation Signed-off-by: Mattia Lavacca --- test/conformance/gateway_conformance_test.go | 29 +++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/test/conformance/gateway_conformance_test.go b/test/conformance/gateway_conformance_test.go index d736f16776..bc5b133e66 100644 --- a/test/conformance/gateway_conformance_test.go +++ b/test/conformance/gateway_conformance_test.go @@ -9,7 +9,9 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" @@ -101,6 +103,31 @@ func TestGatewayConformance(t *testing.T) { require.NoError(t, gatewayv1alpha2.AddToScheme(client.Scheme())) require.NoError(t, gatewayv1beta1.AddToScheme(client.Scheme())) + t.Log("creating tls service for gateway conformance tests") + tlsService := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tls-proxy-" + uuid.NewString(), + Namespace: "kong-system", + }, + Spec: corev1.ServiceSpec{ + Selector: map[string]string{ + "app.kubernetes.io/component": "app", + "app.kubernetes.io/instance": "ingress-controller", + "app.kubernetes.io/name": "kong", + }, + Type: corev1.ServiceTypeLoadBalancer, + Ports: []corev1.ServicePort{ + { + Port: 443, + TargetPort: intstr.FromInt(8899), + Protocol: corev1.ProtocolTCP, + }, + }, + }, + } + require.NoError(t, client.Create(ctx, tlsService)) + t.Cleanup(func() { assert.NoError(t, client.Delete(ctx, tlsService)) }) + featureGateFlag := fmt.Sprintf("--feature-gates=%s", consts.DefaultFeatureGates) if expressionRoutesEnabled() { t.Log("expression routes enabled") @@ -120,7 +147,7 @@ func TestGatewayConformance(t *testing.T) { "--debug-log-reduce-redundancy", featureGateFlag, "--anonymous-reports=false", - "--publish-service-tls=kong-system/ingress-controller-kong-tls-proxy", + fmt.Sprintf("--publish-service-tls=%s/%s", tlsService.Namespace, tlsService.Name), } require.NoError(t, testutils.DeployControllerManagerForCluster(ctx, globalDeprecatedLogger, globalLogger, env.Cluster(), args...)) From 5e46c3c70f11081a01a4f1a99014bee67d410a98 Mon Sep 17 00:00:00 2001 From: Mattia Lavacca Date: Tue, 1 Aug 2023 10:10:53 +0200 Subject: [PATCH 05/10] all features enabled Signed-off-by: Mattia Lavacca --- test/conformance/gateway_conformance_test.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/test/conformance/gateway_conformance_test.go b/test/conformance/gateway_conformance_test.go index bc5b133e66..6abc0ba0a9 100644 --- a/test/conformance/gateway_conformance_test.go +++ b/test/conformance/gateway_conformance_test.go @@ -76,8 +76,6 @@ var skippedTestsForTraditionalRoutes = []string{ tests.HTTPRouteRedirectPortAndScheme.ShortName, // https://github.com/Kong/kubernetes-ingress-controller/issues/3680 tests.GatewayClassObservedGenerationBump.ShortName, - // https://github.com/Kong/kubernetes-ingress-controller/issues/3678 - //tests.TLSRouteSimpleSameNamespace.ShortName, // https://github.com/Kong/kubernetes-ingress-controller/issues/3679 tests.HTTPRouteQueryParamMatching.ShortName, // https://github.com/Kong/kubernetes-ingress-controller/issues/3681 @@ -175,14 +173,14 @@ func TestGatewayConformance(t *testing.T) { skippedTests = skippedTestsForExpressionRoutes } cSuite := suite.New(suite.Options{ - Client: client, - GatewayClassName: gatewayClass.Name, - Debug: showDebug, - CleanupBaseResources: shouldCleanup, - ExemptFeatures: exemptFeatures, - BaseManifests: conformanceTestsBaseManifests, - SkipTests: skippedTests, - SupportedFeatures: suite.TLSCoreFeatures, + Client: client, + GatewayClassName: gatewayClass.Name, + Debug: showDebug, + CleanupBaseResources: shouldCleanup, + ExemptFeatures: exemptFeatures, + BaseManifests: conformanceTestsBaseManifests, + SkipTests: skippedTests, + EnableAllSupportedFeatures: true, }) cSuite.Setup(t) // To work with individual tests only, you can disable the normal Run call and construct a slice containing a From 3189e4443ebb23263a030aa897eaafab2d3e9d09 Mon Sep 17 00:00:00 2001 From: Mattia Lavacca Date: Tue, 1 Aug 2023 10:49:52 +0200 Subject: [PATCH 06/10] chore: PR polished Signed-off-by: Mattia Lavacca --- CHANGELOG.md | 24 +++++++++++-------- .../controllers/gateway/gateway_controller.go | 11 +-------- .../dataplane/parser/translate_tlsroute.go | 2 +- internal/manager/config.go | 2 +- test/conformance/gateway_conformance_test.go | 4 +++- 5 files changed, 20 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f77ba9378..a865f8d5f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -138,7 +138,7 @@ Adding a new version? You'll need three changes: [#4211](https://github.com/Kong/kubernetes-ingress-controller/pull/4211) - Assign priorities to routes translated from Ingresses when parser translate them to expression based Kong routes. The assigning method is basically the - same as in Kong gateway's `traditional_compatible` router, except that + same as in Kong gateway's `traditional_compatible` router, except that `regex_priority` field in Kong traditional route is not supported. This method is adopted to keep the compatibility with traditional router on maximum effort. @@ -148,7 +148,7 @@ Adding a new version? You'll need three changes: [specification on priorities of matches in `HTTPRoute`][httproute-specification]. [#4296](https://github.com/Kong/kubernetes-ingress-controller/pull/4296) [#4434](https://github.com/Kong/kubernetes-ingress-controller/pull/4434) -- Assign priorities to routes translated from GRPCRoutes when the parser translates +- Assign priorities to routes translated from GRPCRoutes when the parser translates them to expression based Kong routes. The priority order follows the [specification on match priorities in GRPCRoute][grpcroute-specification]. [#4364](https://github.com/Kong/kubernetes-ingress-controller/pull/4364) @@ -165,10 +165,11 @@ Adding a new version? You'll need three changes: in terms of accepting data-plane traffic, but are ready to accept configuration updates. The controller will now send configuration to such Gateways and will actively monitor their readiness for accepting configuration updates. - [#4368](https://github.com/Kong/kubernetes-ingress-controller/pull/4368 -- `KongConsumer`, `KongConsumerGroup` `KongPlugin`, and `KongClusterPlugin` CRDs were extended with - `Status.Conditions` field. It will contain the `Programmed` condition describing - whether an object was successfully translated into Kong entities and sent to Kong. + [#4368](https://github.com/Kong/kubernetes-ingress-controller/pull/4368) +- `KongConsumer`, `KongConsumerGroup` `KongPlugin`, and `KongClusterPlugin` CRDs + were extended with `Status.Conditions` field. It will contain the `Programmed` + condition describing whether an object was successfully translated into Kong + entities and sent to Kong. [#4409](https://github.com/Kong/kubernetes-ingress-controller/pull/4409) [#4412](https://github.com/Kong/kubernetes-ingress-controller/pull/4412) [#4423](https://github.com/Kong/kubernetes-ingress-controller/pull/4423) @@ -182,6 +183,9 @@ Adding a new version? You'll need three changes: in the `Programmed` condition of the object being set to `False` and an event being emitted. [#4428](https://github.com/Kong/kubernetes-ingress-controller/pull/4428) +- A new `--publish-service-tls` flag has been added to expose Kong TLS stream port + (default value is 8899) through a service using a different port. + [#3797](https://github.com/Kong/kubernetes-ingress-controller/pull/3797) ### Changed @@ -200,11 +204,11 @@ Adding a new version? You'll need three changes: - Changed the Gateway's readiness probe in all-in-one manifests from `/status` to `/status/ready`. Gateways will be considered ready only after an initial configuration is applied by the controller. - [#4368](https://github.com/Kong/kubernetes-ingress-controller/pull/4368 -- When translating to expression based Kong routes, annotations to specify + [#4368](https://github.com/Kong/kubernetes-ingress-controller/pull/4368) +- When translating to expression based Kong routes, annotations to specify protocols are translated to `protocols` field of the result Kong route, - instead of putting the conditions to match protocols inside expressions. - [#4422](https://github.com/Kong/kubernetes-ingress-controller/pull/4422) + instead of putting the conditions to match protocols inside expressions. + [#4422](https://github.com/Kong/kubernetes-ingress-controller/pull/4422) ### Fixed diff --git a/internal/controllers/gateway/gateway_controller.go b/internal/controllers/gateway/gateway_controller.go index b9c7b9f3ae..c0e461c997 100644 --- a/internal/controllers/gateway/gateway_controller.go +++ b/internal/controllers/gateway/gateway_controller.go @@ -770,7 +770,6 @@ func (r *GatewayReconciler) determineListenersFromDataPlane( // are configured for them in the data-plane. upgradedListeners := make([]Listener, 0, len(listeners)) for _, listener := range listeners { - newListeners := make([]Listener, 0) if streamListener, ok := streamListenersMap[portMapper[int(listener.Port)]]; ok { if streamListener.SSL { listener.Protocol = gatewayv1beta1.TLSProtocolType @@ -779,7 +778,6 @@ func (r *GatewayReconciler) determineListenersFromDataPlane( {Group: &gatewayV1beta1Group, Kind: (Kind)("TLSRoute")}, }, } - newListeners = append(newListeners, listener) } } if proxyListener, ok := proxyListenersMap[portMapper[int(listener.Port)]]; ok { @@ -790,12 +788,6 @@ func (r *GatewayReconciler) determineListenersFromDataPlane( {Group: &gatewayV1beta1Group, Kind: (Kind)("HTTPRoute")}, }, } - tlsListener := listener - tlsListener.Protocol = gatewayv1beta1.TLSProtocolType - tlsListener.AllowedRoutes.Kinds = append(tlsListener.AllowedRoutes.Kinds, - gatewayv1beta1.RouteGroupKind{Group: &gatewayV1beta1Group, Kind: (Kind)("TLSRoute")}) - newListeners = append(newListeners, listener) - newListeners = append(newListeners, tlsListener) } else { listener.Protocol = gatewayv1beta1.HTTPProtocolType listener.AllowedRoutes = &gatewayv1beta1.AllowedRoutes{ @@ -803,10 +795,9 @@ func (r *GatewayReconciler) determineListenersFromDataPlane( {Group: &gatewayV1beta1Group, Kind: (Kind)("HTTPRoute")}, }, } - newListeners = append(newListeners, listener) } } - upgradedListeners = append(upgradedListeners, newListeners...) + upgradedListeners = append(upgradedListeners, listener) } return upgradedListeners, nil diff --git a/internal/dataplane/parser/translate_tlsroute.go b/internal/dataplane/parser/translate_tlsroute.go index 9a5c9d9129..07cd7c08a5 100644 --- a/internal/dataplane/parser/translate_tlsroute.go +++ b/internal/dataplane/parser/translate_tlsroute.go @@ -136,7 +136,7 @@ func (p *Parser) isTLSRoutePassthrough(tlsroute *gatewayv1alpha2.TLSRoute) (bool // If any of the gateway's listeners is configured to passthrough // TLS requests, we return true. for _, listener := range gateway.Spec.Listeners { - if parentRef.SectionName == nil || listener.Name == *parentRef.SectionName { + if listener.Name == *parentRef.SectionName { if listener.TLS != nil && listener.TLS.Mode != nil && *listener.TLS.Mode == gatewayv1beta1.TLSModePassthrough { return true, nil diff --git a/internal/manager/config.go b/internal/manager/config.go index 378ac357ec..bebd39c487 100644 --- a/internal/manager/config.go +++ b/internal/manager/config.go @@ -212,7 +212,7 @@ func (c *Config) FlagSet() *pflag.FlagSet { `endpoints. If omitted, the same Service will be used for both TCP and UDP routes.`) flagSet.Var(flags.NewValidatedValue(&c.PublishServiceTLS, namespacedNameFromFlagValue, nnTypeNameOverride), "publish-service-tls", `Service fronting TLS routing resources in `+ `"namespace/name" format. The controller will update TLS route status information with this Service's `+ - `endpoints. If omitted, the same Service will be used for both TCP and UDP routes.`) + `endpoints. If omitted, the same Service will be used for both HTTP and TLS routes.`) flagSet.StringSliceVar(&c.PublishStatusAddressUDP, "publish-status-address-udp", []string{}, `User-provided address CSV, for use in lieu of "publish-service-udp" when that Service lacks useful address information.`) diff --git a/test/conformance/gateway_conformance_test.go b/test/conformance/gateway_conformance_test.go index 6abc0ba0a9..e67200463a 100644 --- a/test/conformance/gateway_conformance_test.go +++ b/test/conformance/gateway_conformance_test.go @@ -41,7 +41,7 @@ var skippedTestsForExpressionRoutes = []string{ tests.HTTPRouteRedirectPortAndScheme.ShortName, // https://github.com/Kong/kubernetes-ingress-controller/issues/3680 tests.GatewayClassObservedGenerationBump.ShortName, - // https://github.com/Kong/kubernetes-ingress-controller/issues/3678 + // https://github.com/Kong/kubernetes-ingress-controller/issues/4312 tests.TLSRouteSimpleSameNamespace.ShortName, // https://github.com/Kong/kubernetes-ingress-controller/issues/3679 tests.HTTPRouteQueryParamMatching.ShortName, @@ -101,6 +101,8 @@ func TestGatewayConformance(t *testing.T) { require.NoError(t, gatewayv1alpha2.AddToScheme(client.Scheme())) require.NoError(t, gatewayv1beta1.AddToScheme(client.Scheme())) + // This service creation is a temporary solution, intended to be replaced by + // https://github.com/Kong/charts/issues/848 t.Log("creating tls service for gateway conformance tests") tlsService := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ From 268a3970eea4c55ce5e675c7724e5d0615f909eb Mon Sep 17 00:00:00 2001 From: Mattia Lavacca Date: Tue, 1 Aug 2023 12:00:51 +0200 Subject: [PATCH 07/10] fix conformance tests Signed-off-by: Mattia Lavacca --- internal/controllers/gateway/gateway_controller.go | 8 ++++---- test/conformance/gateway_conformance_test.go | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/internal/controllers/gateway/gateway_controller.go b/internal/controllers/gateway/gateway_controller.go index c0e461c997..16aae95c7d 100644 --- a/internal/controllers/gateway/gateway_controller.go +++ b/internal/controllers/gateway/gateway_controller.go @@ -631,13 +631,11 @@ func (r *GatewayReconciler) determineServiceForGateway(ctx context.Context, ref if protocol == gatewayv1beta1.HTTPProtocolType || protocol == gatewayv1beta1.HTTPSProtocolType || protocol == gatewayv1beta1.TCPProtocolType { name = r.PublishServiceRef } - fallthrough - case r.PublishServiceUDPRef.IsPresent(): + case r.PublishServiceUDPRef.IsPresent() && ref == r.PublishServiceUDPRef.MustGet().String(): if protocol == gatewayv1beta1.UDPProtocolType { name = r.PublishServiceUDPRef.MustGet() } - fallthrough - case r.PublishServiceTLSRef.IsPresent(): + case r.PublishServiceTLSRef.IsPresent() && ref == r.PublishServiceTLSRef.MustGet().String(): if protocol == gatewayv1beta1.TLSProtocolType { name = r.PublishServiceTLSRef.MustGet() } @@ -778,6 +776,8 @@ func (r *GatewayReconciler) determineListenersFromDataPlane( {Group: &gatewayV1beta1Group, Kind: (Kind)("TLSRoute")}, }, } + upgradedListeners = append(upgradedListeners, listener) + continue } } if proxyListener, ok := proxyListenersMap[portMapper[int(listener.Port)]]; ok { diff --git a/test/conformance/gateway_conformance_test.go b/test/conformance/gateway_conformance_test.go index e67200463a..742034e559 100644 --- a/test/conformance/gateway_conformance_test.go +++ b/test/conformance/gateway_conformance_test.go @@ -182,7 +182,8 @@ func TestGatewayConformance(t *testing.T) { ExemptFeatures: exemptFeatures, BaseManifests: conformanceTestsBaseManifests, SkipTests: skippedTests, - EnableAllSupportedFeatures: true, + EnableAllSupportedFeatures: false, + SupportedFeatures: suite.TLSCoreFeatures, }) cSuite.Setup(t) // To work with individual tests only, you can disable the normal Run call and construct a slice containing a From 55a0de41874df20ea230df9e3efc02a83673b835 Mon Sep 17 00:00:00 2001 From: Mattia Lavacca Date: Tue, 1 Aug 2023 12:55:33 +0200 Subject: [PATCH 08/10] parent-ref check re-enabled Signed-off-by: Mattia Lavacca --- internal/dataplane/parser/translate_tlsroute.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/dataplane/parser/translate_tlsroute.go b/internal/dataplane/parser/translate_tlsroute.go index 07cd7c08a5..9a5c9d9129 100644 --- a/internal/dataplane/parser/translate_tlsroute.go +++ b/internal/dataplane/parser/translate_tlsroute.go @@ -136,7 +136,7 @@ func (p *Parser) isTLSRoutePassthrough(tlsroute *gatewayv1alpha2.TLSRoute) (bool // If any of the gateway's listeners is configured to passthrough // TLS requests, we return true. for _, listener := range gateway.Spec.Listeners { - if listener.Name == *parentRef.SectionName { + if parentRef.SectionName == nil || listener.Name == *parentRef.SectionName { if listener.TLS != nil && listener.TLS.Mode != nil && *listener.TLS.Mode == gatewayv1beta1.TLSModePassthrough { return true, nil From a38a4f1f1c48d339c658f2bf0e29f0cf10baf8cf Mon Sep 17 00:00:00 2001 From: Mattia Lavacca Date: Tue, 1 Aug 2023 17:19:00 +0200 Subject: [PATCH 09/10] integration tests fixed Signed-off-by: Mattia Lavacca --- internal/controllers/gateway/gateway_controller.go | 5 ++++- test/conformance/gateway_conformance_test.go | 7 ++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/controllers/gateway/gateway_controller.go b/internal/controllers/gateway/gateway_controller.go index 16aae95c7d..990c48d342 100644 --- a/internal/controllers/gateway/gateway_controller.go +++ b/internal/controllers/gateway/gateway_controller.go @@ -628,7 +628,10 @@ func (r *GatewayReconciler) determineServiceForGateway(ctx context.Context, ref var name k8stypes.NamespacedName switch { case ref == r.PublishServiceRef.String(): - if protocol == gatewayv1beta1.HTTPProtocolType || protocol == gatewayv1beta1.HTTPSProtocolType || protocol == gatewayv1beta1.TCPProtocolType { + if protocol == gatewayv1beta1.HTTPProtocolType || + protocol == gatewayv1beta1.HTTPSProtocolType || + protocol == gatewayv1beta1.TCPProtocolType || + r.PublishServiceTLSRef.IsAbsent() && protocol == TLSProtocolType { name = r.PublishServiceRef } case r.PublishServiceUDPRef.IsPresent() && ref == r.PublishServiceUDPRef.MustGet().String(): diff --git a/test/conformance/gateway_conformance_test.go b/test/conformance/gateway_conformance_test.go index 742034e559..48e79172ee 100644 --- a/test/conformance/gateway_conformance_test.go +++ b/test/conformance/gateway_conformance_test.go @@ -4,6 +4,7 @@ package conformance import ( "fmt" + "strings" "testing" "github.com/google/uuid" @@ -104,9 +105,10 @@ func TestGatewayConformance(t *testing.T) { // This service creation is a temporary solution, intended to be replaced by // https://github.com/Kong/charts/issues/848 t.Log("creating tls service for gateway conformance tests") + svcNameSuffix, _, _ := strings.Cut(uuid.NewString(), "-") tlsService := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: "tls-proxy-" + uuid.NewString(), + Name: "ingress-controller-kong-tls-proxy-" + svcNameSuffix, Namespace: "kong-system", }, Spec: corev1.ServiceSpec{ @@ -179,11 +181,10 @@ func TestGatewayConformance(t *testing.T) { GatewayClassName: gatewayClass.Name, Debug: showDebug, CleanupBaseResources: shouldCleanup, + EnableAllSupportedFeatures: enableAllSupportedFeatures, ExemptFeatures: exemptFeatures, BaseManifests: conformanceTestsBaseManifests, SkipTests: skippedTests, - EnableAllSupportedFeatures: false, - SupportedFeatures: suite.TLSCoreFeatures, }) cSuite.Setup(t) // To work with individual tests only, you can disable the normal Run call and construct a slice containing a From 7839338e0e4baa7c433af5b17bafb61bbda491ce Mon Sep 17 00:00:00 2001 From: Mattia Lavacca Date: Thu, 3 Aug 2023 11:42:17 +0200 Subject: [PATCH 10/10] Update internal/manager/config.go Co-authored-by: Travis Raines <571832+rainest@users.noreply.github.com> --- internal/manager/config.go | 6 +++--- test/conformance/gateway_conformance_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/manager/config.go b/internal/manager/config.go index bebd39c487..9db9fe2341 100644 --- a/internal/manager/config.go +++ b/internal/manager/config.go @@ -210,9 +210,9 @@ func (c *Config) FlagSet() *pflag.FlagSet { flagSet.Var(flags.NewValidatedValue(&c.PublishServiceUDP, namespacedNameFromFlagValue, nnTypeNameOverride), "publish-service-udp", `Service fronting UDP routing resources in `+ `"namespace/name" format. The controller will update UDP route status information with this Service's `+ `endpoints. If omitted, the same Service will be used for both TCP and UDP routes.`) - flagSet.Var(flags.NewValidatedValue(&c.PublishServiceTLS, namespacedNameFromFlagValue, nnTypeNameOverride), "publish-service-tls", `Service fronting TLS routing resources in `+ - `"namespace/name" format. The controller will update TLS route status information with this Service's `+ - `endpoints. If omitted, the same Service will be used for both HTTP and TLS routes.`) + flagSet.Var(flags.NewValidatedValue(&c.PublishServiceTLS, namespacedNameFromFlagValue, nnTypeNameOverride), "publish-service-tls", `Optional Service for TLS Gateway Listeners in `+ + `"namespace/name" format. When omitted, TLS Listeners use the --publish-service Service.`+ + `Only necessary if TLS and HTTPS Listeners share the same port.`) flagSet.StringSliceVar(&c.PublishStatusAddressUDP, "publish-status-address-udp", []string{}, `User-provided address CSV, for use in lieu of "publish-service-udp" when that Service lacks useful address information.`) diff --git a/test/conformance/gateway_conformance_test.go b/test/conformance/gateway_conformance_test.go index 48e79172ee..a48f5d7da0 100644 --- a/test/conformance/gateway_conformance_test.go +++ b/test/conformance/gateway_conformance_test.go @@ -109,7 +109,7 @@ func TestGatewayConformance(t *testing.T) { tlsService := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "ingress-controller-kong-tls-proxy-" + svcNameSuffix, - Namespace: "kong-system", + Namespace: "kong", }, Spec: corev1.ServiceSpec{ Selector: map[string]string{