From 69c984391b16fdb44f40b1d72539ddf568c10870 Mon Sep 17 00:00:00 2001 From: Yi Tao Date: Fri, 29 Mar 2024 17:41:09 +0800 Subject: [PATCH 1/4] add HTTPRoute controller for watching HTTPRoute to enqueue KongUpstreamPolicy needing to update ancestor status --- .../kongupstreampolicy_controller.go | 59 +++++++++++++------ internal/manager/controllerdef.go | 30 ++++++---- 2 files changed, 61 insertions(+), 28 deletions(-) diff --git a/internal/controllers/configuration/kongupstreampolicy_controller.go b/internal/controllers/configuration/kongupstreampolicy_controller.go index 0c4b49b740..d6c943eb1c 100644 --- a/internal/controllers/configuration/kongupstreampolicy_controller.go +++ b/internal/controllers/configuration/kongupstreampolicy_controller.go @@ -71,15 +71,6 @@ func (r *KongUpstreamPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error return err } - // Watch for HTTPRoute changes to trigger reconciliation for the KongUpstreamPolicies referenced by the Services - // of the HTTPRoute. - if err := c.Watch( - source.Kind(mgr.GetCache(), &gatewayapi.HTTPRoute{}), - handler.EnqueueRequestsFromMapFunc(r.getUpstreamPoliciesForHTTPRouteServices), - ); err != nil { - return err - } - if r.KongServiceFacadeEnabled { if err := c.Watch( source.Kind(mgr.GetCache(), &incubatorv1alpha1.KongServiceFacade{}), @@ -136,15 +127,6 @@ func (r *KongUpstreamPolicyReconciler) setupIndices(mgr ctrl.Manager) error { return fmt.Errorf("failed to index services on annotation %s: %w", kongv1beta1.KongUpstreamPolicyAnnotationKey, err) } - if err := mgr.GetCache().IndexField( - context.Background(), - &gatewayapi.HTTPRoute{}, - routeBackendRefServiceNameIndexKey, - indexRoutesOnBackendRefServiceName, - ); err != nil { - return fmt.Errorf("failed to index HTTPRoutes on backendReferences: %w", err) - } - if r.KongServiceFacadeEnabled { if err := mgr.GetCache().IndexField( context.Background(), @@ -381,3 +363,44 @@ func (r *KongUpstreamPolicyReconciler) Reconcile(ctx context.Context, req ctrl.R func (r *KongUpstreamPolicyReconciler) SetLogger(l logr.Logger) { r.Log = l } + +type HTTPRouteReconcilerForKongUpstreamPolicy struct { + Reconciler *KongUpstreamPolicyReconciler +} + +func (r *HTTPRouteReconcilerForKongUpstreamPolicy) SetupWithManager(mgr ctrl.Manager) error { + c, err := controller.New("HTTPRouteForKongUpstreamPolicy", mgr, controller.Options{ + Reconciler: r.Reconciler, + LogConstructor: func(_ *reconcile.Request) logr.Logger { + return r.Reconciler.Log + }, + CacheSyncTimeout: r.Reconciler.CacheSyncTimeout, + }) + if err != nil { + return err + } + + if err := mgr.GetCache().IndexField( + context.Background(), + &gatewayapi.HTTPRoute{}, + routeBackendRefServiceNameIndexKey, + indexRoutesOnBackendRefServiceName, + ); err != nil { + return fmt.Errorf("failed to index HTTPRoutes on backendReferences: %w", err) + } + + // Watch for HTTPRoute changes to trigger reconciliation for the KongUpstreamPolicies referenced by the Services + // of the HTTPRoute. + if err := c.Watch( + source.Kind(mgr.GetCache(), &gatewayapi.HTTPRoute{}), + handler.EnqueueRequestsFromMapFunc(r.Reconciler.getUpstreamPoliciesForHTTPRouteServices), + ); err != nil { + return err + } + + return nil +} + +func (r *HTTPRouteReconcilerForKongUpstreamPolicy) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + return r.Reconciler.Reconcile(ctx, req) +} diff --git a/internal/manager/controllerdef.go b/internal/manager/controllerdef.go index ce8a344eb9..8e3113d583 100644 --- a/internal/manager/controllerdef.go +++ b/internal/manager/controllerdef.go @@ -67,6 +67,16 @@ func setupControllers( kongAdminAPIEndpointsNotifier configuration.EndpointsNotifier, adminAPIsDiscoverer configuration.AdminAPIsDiscoverer, ) []ControllerDef { + kongUpstreamPolicyReconciler := &configuration.KongUpstreamPolicyReconciler{ + Client: mgr.GetClient(), + Log: ctrl.LoggerFrom(ctx).WithName("controllers").WithName("KongUpstreamPolicy"), + Scheme: mgr.GetScheme(), + DataplaneClient: dataplaneClient, + CacheSyncTimeout: c.CacheSyncTimeout, + KongServiceFacadeEnabled: featureGates.Enabled(featuregates.KongServiceFacade) && c.KongServiceFacadeEnabled, + StatusQueue: kubernetesStatusQueue, + } + controllers := []ControllerDef{ // --------------------------------------------------------------------------- // Kong Gateway Admin API Service discovery @@ -250,11 +260,17 @@ func setupControllers( // StatusQueue: kubernetesStatusQueue, }, }, + // KongUpstreamPolicy controller without HTTPRoute. { - Enabled: c.KongUpstreamPolicyEnabled, + Enabled: c.KongUpstreamPolicyEnabled, + Controller: kongUpstreamPolicyReconciler, + }, + // HTTPRoute controller for updating KongUpstreamPolicy's ancestor status with services used as backends of HTTPRoutes. + { + Enabled: c.KongUpstreamPolicyEnabled && c.GatewayAPIHTTPRouteController, Controller: &crds.DynamicCRDController{ Manager: mgr, - Log: ctrl.LoggerFrom(ctx).WithName("controllers").WithName("Dynamic/KongUpstreamPolicy"), + Log: ctrl.LoggerFrom(ctx).WithName("controllers").WithName("Dynamic/Gateway"), CacheSyncTimeout: c.CacheSyncTimeout, RequiredCRDs: []schema.GroupVersionResource{ { @@ -263,14 +279,8 @@ func setupControllers( Resource: "httproutes", }, }, - Controller: &configuration.KongUpstreamPolicyReconciler{ - Client: mgr.GetClient(), - Log: ctrl.LoggerFrom(ctx).WithName("controllers").WithName("KongUpstreamPolicy"), - Scheme: mgr.GetScheme(), - DataplaneClient: dataplaneClient, - CacheSyncTimeout: c.CacheSyncTimeout, - KongServiceFacadeEnabled: featureGates.Enabled(featuregates.KongServiceFacade) && c.KongServiceFacadeEnabled, - StatusQueue: kubernetesStatusQueue, + Controller: &configuration.HTTPRouteReconcilerForKongUpstreamPolicy{ + Reconciler: kongUpstreamPolicyReconciler, }, }, }, From ea72a43b5ac8ecc25e2beabfefacd40fc8187c48 Mon Sep 17 00:00:00 2001 From: Yi Tao Date: Mon, 1 Apr 2024 11:19:09 +0800 Subject: [PATCH 2/4] add CHANGELOG --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1eaf2fc706..43a1e29164 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,9 @@ Adding a new version? You'll need three changes: [#5737](https://github.com/Kong/kubernetes-ingress-controller/issues/5737) - Set proper User-Agent for request made to Kong and Konnect. [#5753](https://github.com/Kong/kubernetes-ingress-controller/pull/5753) +- `KongUpstreamPolicy` controller no longer requires existence of `HTTPRoute` CRD + to start. + [#5780](https://github.com/Kong/kubernetes-ingress-controller/pull/5780) ## [3.1.2] From 20b9b6e1f785f057f93aea921d91ba4faf48dc85 Mon Sep 17 00:00:00 2001 From: Yi Tao Date: Wed, 3 Apr 2024 17:33:43 +0800 Subject: [PATCH 3/4] do not use dynamic CRD controller and and envtest cases --- controllers/license/konglicense_controller.go | 2 +- .../kongupstreampolicy_controller.go | 68 ++- .../configuration/kongupstreampolicy_utils.go | 4 + .../kongupstreampolicy_utils_test.go | 1 + internal/manager/controllerdef.go | 48 +- .../adminapi_discoverer_envtest_test.go | 2 +- test/envtest/kongupstreampolicy_test.go | 436 ++++++++++++++++++ test/envtest/setup.go | 2 +- 8 files changed, 487 insertions(+), 76 deletions(-) create mode 100644 test/envtest/kongupstreampolicy_test.go diff --git a/controllers/license/konglicense_controller.go b/controllers/license/konglicense_controller.go index a9f84c5624..0d782f0f6b 100644 --- a/controllers/license/konglicense_controller.go +++ b/controllers/license/konglicense_controller.go @@ -461,7 +461,7 @@ func WrapKongLicenseReconcilerToDynamicCRDController( ) *crds.DynamicCRDController { return &crds.DynamicCRDController{ Manager: mgr, - Log: ctrl.LoggerFrom(ctx).WithName("controllers").WithName("Dynamic/KongUpstreamPolicy"), + Log: ctrl.LoggerFrom(ctx).WithName("controllers").WithName("Dynamic/KongLicense"), CacheSyncTimeout: r.CacheSyncTimeout, RequiredCRDs: []schema.GroupVersionResource{ { diff --git a/internal/controllers/configuration/kongupstreampolicy_controller.go b/internal/controllers/configuration/kongupstreampolicy_controller.go index d6c943eb1c..cacaca898c 100644 --- a/internal/controllers/configuration/kongupstreampolicy_controller.go +++ b/internal/controllers/configuration/kongupstreampolicy_controller.go @@ -44,6 +44,9 @@ type KongUpstreamPolicyReconciler struct { // KongServiceFacadeEnabled determines whether the controller should populate the KongUpstreamPolicy's ancestor // status for KongServiceFacades. KongServiceFacadeEnabled bool + // HTTPRouteEnabled determines whether the controller should populate the KongUpstreamPolicy's + // ancestor status for HTTPRoutes. + HTTPRouteEnabled bool } // SetupWithManager sets up the controller with the Manager. @@ -71,6 +74,17 @@ func (r *KongUpstreamPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error return err } + if r.HTTPRouteEnabled { + // Watch for HTTPRoute changes to trigger reconciliation for the KongUpstreamPolicies referenced by the Services + // of the HTTPRoute. + if err := c.Watch( + source.Kind(mgr.GetCache(), &gatewayapi.HTTPRoute{}), + handler.EnqueueRequestsFromMapFunc(r.getUpstreamPoliciesForHTTPRouteServices), + ); err != nil { + return err + } + } + if r.KongServiceFacadeEnabled { if err := c.Watch( source.Kind(mgr.GetCache(), &incubatorv1alpha1.KongServiceFacade{}), @@ -127,6 +141,17 @@ func (r *KongUpstreamPolicyReconciler) setupIndices(mgr ctrl.Manager) error { return fmt.Errorf("failed to index services on annotation %s: %w", kongv1beta1.KongUpstreamPolicyAnnotationKey, err) } + if r.HTTPRouteEnabled { + if err := mgr.GetCache().IndexField( + context.Background(), + &gatewayapi.HTTPRoute{}, + routeBackendRefServiceNameIndexKey, + indexRoutesOnBackendRefServiceName, + ); err != nil { + return fmt.Errorf("failed to index HTTPRoutes on backendReferences: %w", err) + } + } + if r.KongServiceFacadeEnabled { if err := mgr.GetCache().IndexField( context.Background(), @@ -243,7 +268,7 @@ func (r *KongUpstreamPolicyReconciler) getUpstreamPoliciesForHTTPRouteServices(c if !ok { return nil } - + r.Log.Info("Found HTTPRoute", "name", httpRoute.Name, "namespace", httpRoute.Namespace) var requests []reconcile.Request for _, rule := range httpRoute.Spec.Rules { for _, br := range rule.BackendRefs { @@ -363,44 +388,3 @@ func (r *KongUpstreamPolicyReconciler) Reconcile(ctx context.Context, req ctrl.R func (r *KongUpstreamPolicyReconciler) SetLogger(l logr.Logger) { r.Log = l } - -type HTTPRouteReconcilerForKongUpstreamPolicy struct { - Reconciler *KongUpstreamPolicyReconciler -} - -func (r *HTTPRouteReconcilerForKongUpstreamPolicy) SetupWithManager(mgr ctrl.Manager) error { - c, err := controller.New("HTTPRouteForKongUpstreamPolicy", mgr, controller.Options{ - Reconciler: r.Reconciler, - LogConstructor: func(_ *reconcile.Request) logr.Logger { - return r.Reconciler.Log - }, - CacheSyncTimeout: r.Reconciler.CacheSyncTimeout, - }) - if err != nil { - return err - } - - if err := mgr.GetCache().IndexField( - context.Background(), - &gatewayapi.HTTPRoute{}, - routeBackendRefServiceNameIndexKey, - indexRoutesOnBackendRefServiceName, - ); err != nil { - return fmt.Errorf("failed to index HTTPRoutes on backendReferences: %w", err) - } - - // Watch for HTTPRoute changes to trigger reconciliation for the KongUpstreamPolicies referenced by the Services - // of the HTTPRoute. - if err := c.Watch( - source.Kind(mgr.GetCache(), &gatewayapi.HTTPRoute{}), - handler.EnqueueRequestsFromMapFunc(r.Reconciler.getUpstreamPoliciesForHTTPRouteServices), - ); err != nil { - return err - } - - return nil -} - -func (r *HTTPRouteReconcilerForKongUpstreamPolicy) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - return r.Reconciler.Reconcile(ctx, req) -} diff --git a/internal/controllers/configuration/kongupstreampolicy_utils.go b/internal/controllers/configuration/kongupstreampolicy_utils.go index 3fc1d6eca7..7161765168 100644 --- a/internal/controllers/configuration/kongupstreampolicy_utils.go +++ b/internal/controllers/configuration/kongupstreampolicy_utils.go @@ -212,6 +212,10 @@ func (r *KongUpstreamPolicyReconciler) buildAncestorsStatus( // getConflictedServices returns a set of services that have conflicts. func (r *KongUpstreamPolicyReconciler) getConflictedServices(ctx context.Context, services []corev1.Service) (servicesSet, error) { + // return directly when HTTPRoute is not enabled, as it only check conflicted services in HTTPRoute backends only. + if !r.HTTPRouteEnabled { + return make(servicesSet), nil + } // Prepare a mapping for efficient lookups if a Service uses this KongUpstreamPolicy. upstreamPolicyServices := make(servicesSet) for _, service := range services { diff --git a/internal/controllers/configuration/kongupstreampolicy_utils_test.go b/internal/controllers/configuration/kongupstreampolicy_utils_test.go index bd6c83d0ad..ea46efad89 100644 --- a/internal/controllers/configuration/kongupstreampolicy_utils_test.go +++ b/internal/controllers/configuration/kongupstreampolicy_utils_test.go @@ -698,6 +698,7 @@ func TestEnforceKongUpstreamPolicyStatus(t *testing.T) { Client: fakeClient, DataplaneClient: DataPlaneStatusClientMock{ObjectsConfigured: tc.objectsConfiguredInDataPlane}, KongServiceFacadeEnabled: true, + HTTPRouteEnabled: true, } updated, err := reconciler.enforceKongUpstreamPolicyStatus(context.TODO(), &tc.kongUpstreamPolicy) diff --git a/internal/manager/controllerdef.go b/internal/manager/controllerdef.go index 8e3113d583..22f7ac2779 100644 --- a/internal/manager/controllerdef.go +++ b/internal/manager/controllerdef.go @@ -16,6 +16,7 @@ import ( "github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/crds" "github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/gateway" ctrlref "github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/reference" + "github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/utils" "github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane" "github.com/kong/kubernetes-ingress-controller/v3/internal/manager/featuregates" "github.com/kong/kubernetes-ingress-controller/v3/internal/util/kubernetes/object/status" @@ -67,16 +68,6 @@ func setupControllers( kongAdminAPIEndpointsNotifier configuration.EndpointsNotifier, adminAPIsDiscoverer configuration.AdminAPIsDiscoverer, ) []ControllerDef { - kongUpstreamPolicyReconciler := &configuration.KongUpstreamPolicyReconciler{ - Client: mgr.GetClient(), - Log: ctrl.LoggerFrom(ctx).WithName("controllers").WithName("KongUpstreamPolicy"), - Scheme: mgr.GetScheme(), - DataplaneClient: dataplaneClient, - CacheSyncTimeout: c.CacheSyncTimeout, - KongServiceFacadeEnabled: featureGates.Enabled(featuregates.KongServiceFacade) && c.KongServiceFacadeEnabled, - StatusQueue: kubernetesStatusQueue, - } - controllers := []ControllerDef{ // --------------------------------------------------------------------------- // Kong Gateway Admin API Service discovery @@ -260,28 +251,23 @@ func setupControllers( // StatusQueue: kubernetesStatusQueue, }, }, - // KongUpstreamPolicy controller without HTTPRoute. - { - Enabled: c.KongUpstreamPolicyEnabled, - Controller: kongUpstreamPolicyReconciler, - }, - // HTTPRoute controller for updating KongUpstreamPolicy's ancestor status with services used as backends of HTTPRoutes. + // KongUpstreamPolicy controller. + // When HTTPRoute exists, the controller is enabled to watch HTTPRoutes to set ancestor status of KongUpstreamPolicies. { - Enabled: c.KongUpstreamPolicyEnabled && c.GatewayAPIHTTPRouteController, - Controller: &crds.DynamicCRDController{ - Manager: mgr, - Log: ctrl.LoggerFrom(ctx).WithName("controllers").WithName("Dynamic/Gateway"), - CacheSyncTimeout: c.CacheSyncTimeout, - RequiredCRDs: []schema.GroupVersionResource{ - { - Group: gatewayv1.GroupVersion.Group, - Version: gatewayv1.GroupVersion.Version, - Resource: "httproutes", - }, - }, - Controller: &configuration.HTTPRouteReconcilerForKongUpstreamPolicy{ - Reconciler: kongUpstreamPolicyReconciler, - }, + Enabled: c.KongUpstreamPolicyEnabled, + Controller: &configuration.KongUpstreamPolicyReconciler{ + Client: mgr.GetClient(), + Log: ctrl.LoggerFrom(ctx).WithName("controllers").WithName("KongUpstreamPolicy"), + Scheme: mgr.GetScheme(), + DataplaneClient: dataplaneClient, + CacheSyncTimeout: c.CacheSyncTimeout, + KongServiceFacadeEnabled: featureGates.Enabled(featuregates.KongServiceFacade) && c.KongServiceFacadeEnabled, + StatusQueue: kubernetesStatusQueue, + HTTPRouteEnabled: utils.CRDExists(mgr.GetRESTMapper(), schema.GroupVersionResource{ + Group: gatewayv1.GroupVersion.Group, + Version: gatewayv1.GroupVersion.Version, + Resource: "httproutes", + }), }, }, { diff --git a/test/envtest/adminapi_discoverer_envtest_test.go b/test/envtest/adminapi_discoverer_envtest_test.go index 5215d3e8b9..67f675b42f 100644 --- a/test/envtest/adminapi_discoverer_envtest_test.go +++ b/test/envtest/adminapi_discoverer_envtest_test.go @@ -96,7 +96,7 @@ func TestDiscoverer_GetAdminAPIsForServiceReturnsAllAddressesCorrectlyPagingThro } } -func testPodReference(name, ns string) *corev1.ObjectReference { +func testPodReference(name, ns string) *corev1.ObjectReference { //nolint:unparam return &corev1.ObjectReference{ Kind: "Pod", Namespace: ns, diff --git a/test/envtest/kongupstreampolicy_test.go b/test/envtest/kongupstreampolicy_test.go new file mode 100644 index 0000000000..a3fd276777 --- /dev/null +++ b/test/envtest/kongupstreampolicy_test.go @@ -0,0 +1,436 @@ +//go:build envtest + +package envtest + +import ( + "bytes" + "context" + "fmt" + "io" + "net/http" + "testing" + + "github.com/go-logr/zapr" + gojson "github.com/goccy/go-json" + "github.com/google/uuid" + "github.com/kong/go-database-reconciler/pkg/file" + "github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/generators" + "github.com/samber/lo" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + corev1 "k8s.io/api/core/v1" + discoveryv1 "k8s.io/api/discovery/v1" + netv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8stypes "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/kong/kubernetes-ingress-controller/v3/internal/controllers/gateway" + "github.com/kong/kubernetes-ingress-controller/v3/internal/gatewayapi" + "github.com/kong/kubernetes-ingress-controller/v3/internal/util/builder" + kongv1beta1 "github.com/kong/kubernetes-ingress-controller/v3/pkg/apis/configuration/v1beta1" + "github.com/kong/kubernetes-ingress-controller/v3/test" + "github.com/kong/kubernetes-ingress-controller/v3/test/helpers" +) + +func TestKongUpstreamPolicyWithoutHTTPRoute(t *testing.T) { + t.Parallel() + + scheme := Scheme(t, WithKong) + envcfg := Setup(t, scheme, WithInstallGatewayCRDs(false)) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + ctrlClient := NewControllerClient(t, scheme, envcfg) + ingressClassName := "kongenvtest" + deployIngressClass(ctx, t, ingressClassName, ctrlClient) + + logger := zapr.NewLogger(zap.NewNop()) + ctrl.SetLogger(logger) + + diagPort := helpers.GetFreePort(t) + ns := CreateNamespace(ctx, t, ctrlClient) + RunManager(ctx, t, envcfg, + AdminAPIOptFns(), + WithPublishService(ns.Name), + WithIngressClass(ingressClassName), + WithGatewayFeatureEnabled, + WithGatewayAPIControllers(), + WithProxySyncSeconds(0.10), + WithDiagnosticsServer(diagPort), + ) + + t.Log("verfying that KongUpstreamPolicy works without gateway APIs") + + t.Log("creating a KongUpstreamPolicy") + const KongUpstreamPolicyName = "test-upstream-policy" + kup := &kongv1beta1.KongUpstreamPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: KongUpstreamPolicyName, + Namespace: ns.Name, + }, + Spec: kongv1beta1.KongUpstreamPolicySpec{ + Algorithm: lo.ToPtr("round-robin"), + Slots: lo.ToPtr(32), + }, + } + require.NoError(t, ctrlClient.Create(ctx, kup)) + + t.Log("deploying a minimal HTTP container deployment to test Ingress routes") + container := generators.NewContainer("httpbin", test.HTTPBinImage, test.HTTPBinPort) + deployment := generators.NewDeploymentForContainer(container) + deployment.Spec.Template.Spec.Containers[0].Ports[0].Name = "http" + deployment.Namespace = ns.Name + require.NoError(t, ctrlClient.Create(ctx, deployment)) + + service := generators.NewServiceForDeployment(deployment, corev1.ServiceTypeLoadBalancer) + service.Namespace = ns.Name + service.Annotations = map[string]string{ + kongv1beta1.KongUpstreamPolicyAnnotationKey: KongUpstreamPolicyName, + } + t.Logf("exposing deployment %s via service %s", deployment.Name, service.Name) + require.NoError(t, ctrlClient.Create(ctx, service)) + + pod := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: ns.Name, + Labels: map[string]string{ + "app": "httpbin", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + func() corev1.Container { + c := generators.NewContainer("httpbin", test.HTTPBinImage, test.HTTPBinPort) + c.Ports[0].Name = "http" + return c + }(), + }, + }, + } + require.NoError(t, ctrlClient.Create(ctx, &pod)) + + es := discoveryv1.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: ns.Name, + Labels: map[string]string{ + "kubernetes.io/service-name": service.Name, + }, + }, + AddressType: discoveryv1.AddressTypeIPv4, + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.1"}, + Conditions: discoveryv1.EndpointConditions{ + Ready: lo.ToPtr(true), + Terminating: lo.ToPtr(false), + }, + TargetRef: testPodReference("pod-1", ns.Name), + }, + }, + Ports: builder.NewEndpointPort(80).WithName("http").IntoSlice(), + } + require.NoError(t, ctrlClient.Create(ctx, &es)) + + ingress := &netv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: service.Name, + Namespace: ns.Name, + }, + Spec: netv1.IngressSpec{ + IngressClassName: lo.ToPtr(ingressClassName), + Rules: []netv1.IngressRule{ + { + IngressRuleValue: netv1.IngressRuleValue{ + HTTP: &netv1.HTTPIngressRuleValue{ + Paths: []netv1.HTTPIngressPath{ + { + Path: "/", + PathType: lo.ToPtr(netv1.PathTypePrefix), + Backend: netv1.IngressBackend{ + Service: &netv1.IngressServiceBackend{ + Name: service.Name, + Port: netv1.ServiceBackendPort{ + Name: service.Spec.Ports[0].Name, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + t.Logf("creating ingress %s for service %s", ingress.Name, service.Name) + require.NoError(t, ctrlClient.Create(ctx, ingress)) + + t.Logf("verify that upstream policy is configured in Kong gateway correctly") + require.Eventually(t, func() bool { + resp, err := http.Get(fmt.Sprintf("http://localhost:%d/debug/config/successful", diagPort)) + if err != nil { + t.Logf("WARNING: error while getting config: %v", err) + return false + } + defer resp.Body.Close() + + var ( + config file.Content + buff bytes.Buffer + ) + + if err := gojson.NewDecoder(io.TeeReader(resp.Body, &buff)).Decode(&config); err != nil { + t.Logf("WARNING: error while decoding config: %+v, response: %s", err, buff.String()) + return false + } + + if len(config.Upstreams) != 1 { + t.Logf("WARNING: expected 1 upstream in config: %+v", config) + return false + } + upstream := config.Upstreams[0] + return upstream.Algorithm != nil && *upstream.Algorithm == "round-robin" + }, waitTime, tickTime) + + t.Logf("verify that ancestor status of KongUpstreamPolicy is updated correctly") + err := ctrlClient.Get(ctx, k8stypes.NamespacedName{ + Namespace: ns.Name, + Name: KongUpstreamPolicyName, + }, kup) + require.NoError(t, err) + require.Len(t, kup.Status.Ancestors, 1) + require.Equal(t, "Service", string(*kup.Status.Ancestors[0].AncestorRef.Kind)) + require.Equal(t, ns.Name, string(*kup.Status.Ancestors[0].AncestorRef.Namespace)) + require.Equal(t, service.Name, string(kup.Status.Ancestors[0].AncestorRef.Name)) +} + +func TestKongUpstreamPolicyWithHTTPRoute(t *testing.T) { + t.Parallel() + + scheme := Scheme(t, WithKong, WithGatewayAPI) + envcfg := Setup(t, scheme) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + ctrlClient := NewControllerClient(t, scheme, envcfg) + ingressClassName := "kongenvtest" + deployIngressClass(ctx, t, ingressClassName, ctrlClient) + + logger := zapr.NewLogger(zap.NewNop()) + ctrl.SetLogger(logger) + + diagPort := helpers.GetFreePort(t) + ns := CreateNamespace(ctx, t, ctrlClient) + RunManager(ctx, t, envcfg, + AdminAPIOptFns(), + WithPublishService(ns.Name), + WithIngressClass(ingressClassName), + WithGatewayFeatureEnabled, + WithGatewayAPIControllers(), + WithProxySyncSeconds(0.10), + WithDiagnosticsServer(diagPort), + ) + + t.Log("verfying that KongUpstreamPolicy works without gateway APIs") + + t.Log("creating a KongUpstreamPolicy") + const KongUpstreamPolicyName = "test-upstream-policy" + kup := &kongv1beta1.KongUpstreamPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: KongUpstreamPolicyName, + Namespace: ns.Name, + }, + Spec: kongv1beta1.KongUpstreamPolicySpec{ + Algorithm: lo.ToPtr("round-robin"), + Slots: lo.ToPtr(32), + }, + } + require.NoError(t, ctrlClient.Create(ctx, kup)) + + gwc := gatewayapi.GatewayClass{ + Spec: gatewayapi.GatewayClassSpec{ + ControllerName: gateway.GetControllerName(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Annotations: map[string]string{ + "konghq.com/gatewayclass-unmanaged": "placeholder", + }, + }, + } + require.NoError(t, ctrlClient.Create(ctx, &gwc)) + t.Cleanup(func() { _ = ctrlClient.Delete(ctx, &gwc) }) + + gw := gatewayapi.Gateway{ + Spec: gatewayapi.GatewaySpec{ + GatewayClassName: gatewayapi.ObjectName(gwc.Name), + Listeners: []gatewayapi.Listener{ + { + Name: gatewayapi.SectionName("http"), + Port: gatewayapi.PortNumber(80), + Protocol: gatewayapi.HTTPProtocolType, + AllowedRoutes: &gatewayapi.AllowedRoutes{ + Namespaces: &gatewayapi.RouteNamespaces{ + From: lo.ToPtr(gatewayapi.NamespacesFromAll), + }, + }, + }, + }, + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: uuid.NewString(), + }, + } + require.NoError(t, ctrlClient.Create(ctx, &gw)) + + gwOld := gw.DeepCopy() + gw.Status = gatewayapi.GatewayStatus{ + Addresses: []gatewayapi.GatewayStatusAddress{ + { + Type: lo.ToPtr(gatewayapi.IPAddressType), + Value: "10.0.0.1", + }, + }, + Conditions: []metav1.Condition{ + { + Type: "Programmed", + Status: metav1.ConditionTrue, + Reason: "Programmed", + LastTransitionTime: metav1.Now(), + ObservedGeneration: gw.Generation, + }, + { + Type: "Accepted", + Status: metav1.ConditionTrue, + Reason: "Accepted", + LastTransitionTime: metav1.Now(), + ObservedGeneration: gw.Generation, + }, + }, + Listeners: []gatewayapi.ListenerStatus{ + { + Name: "http", + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionTrue, + Reason: "Accepted", + LastTransitionTime: metav1.Now(), + }, + { + Type: "Programmed", + Status: metav1.ConditionTrue, + Reason: "Programmed", + LastTransitionTime: metav1.Now(), + }, + }, + SupportedKinds: []gatewayapi.RouteGroupKind{ + { + Group: lo.ToPtr(gatewayapi.Group(gatewayv1.GroupVersion.Group)), + Kind: "HTTPRoute", + }, + }, + }, + }, + } + require.NoError(t, ctrlClient.Status().Patch(ctx, &gw, ctrlclient.MergeFrom(gwOld))) + + t.Log("deploying a minimal HTTP container deployment to test HTTPRoutes") + container := generators.NewContainer("httpbin", test.HTTPBinImage, test.HTTPBinPort) + deployment := generators.NewDeploymentForContainer(container) + deployment.Spec.Template.Spec.Containers[0].Ports[0].Name = "http" + deployment.Namespace = ns.Name + require.NoError(t, ctrlClient.Create(ctx, deployment)) + + service := generators.NewServiceForDeployment(deployment, corev1.ServiceTypeLoadBalancer) + service.Namespace = ns.Name + service.Annotations = map[string]string{ + kongv1beta1.KongUpstreamPolicyAnnotationKey: KongUpstreamPolicyName, + } + t.Logf("exposing deployment %s via service %s", deployment.Name, service.Name) + require.NoError(t, ctrlClient.Create(ctx, service)) + + pod := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod-1", + Namespace: ns.Name, + Labels: map[string]string{ + "app": "httpbin", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + func() corev1.Container { + c := generators.NewContainer("httpbin", test.HTTPBinImage, test.HTTPBinPort) + c.Ports[0].Name = "http" + return c + }(), + }, + }, + } + require.NoError(t, ctrlClient.Create(ctx, &pod)) + + es := discoveryv1.EndpointSlice{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: ns.Name, + Labels: map[string]string{ + "kubernetes.io/service-name": service.Name, + }, + }, + AddressType: discoveryv1.AddressTypeIPv4, + Endpoints: []discoveryv1.Endpoint{ + { + Addresses: []string{"10.0.0.1"}, + Conditions: discoveryv1.EndpointConditions{ + Ready: lo.ToPtr(true), + Terminating: lo.ToPtr(false), + }, + TargetRef: testPodReference("pod-1", ns.Name), + }, + }, + Ports: builder.NewEndpointPort(80).WithName("http").IntoSlice(), + } + require.NoError(t, ctrlClient.Create(ctx, &es)) + route := gatewayapi.HTTPRoute{ + TypeMeta: metav1.TypeMeta{ + Kind: "HTTPRoute", + APIVersion: "v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: uuid.NewString(), + }, + Spec: gatewayapi.HTTPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{{ + Name: gatewayapi.ObjectName(gw.Name), + Namespace: lo.ToPtr(gatewayapi.Namespace(ns.Name)), + }}, + }, + Rules: []gatewayapi.HTTPRouteRule{{ + BackendRefs: builder.NewHTTPBackendRef(service.Name).WithNamespace(ns.Name).ToSlice(), + }}, + }, + } + require.NoError(t, ctrlClient.Create(ctx, &route)) + + t.Logf("verifying that the Service as backend of HTTPRoute is added to ancestor status of KongUpstreamPolicy") + require.Eventually(t, func() bool { + err := ctrlClient.Get(ctx, k8stypes.NamespacedName{ + Namespace: ns.Name, + Name: KongUpstreamPolicyName, + }, kup) + require.NoError(t, err) + return lo.ContainsBy(kup.Status.Ancestors, func(ancestorStatus gatewayapi.PolicyAncestorStatus) bool { + return ancestorStatus.AncestorRef.Kind != nil && string(*ancestorStatus.AncestorRef.Kind) == "Service" && + string(ancestorStatus.AncestorRef.Name) == service.Name + }) + }, waitTime, tickTime) +} diff --git a/test/envtest/setup.go b/test/envtest/setup.go index 37d6c155a0..7d34320a96 100644 --- a/test/envtest/setup.go +++ b/test/envtest/setup.go @@ -154,7 +154,7 @@ func installKongCRDs(t *testing.T, scheme *k8sruntime.Scheme, cfg *rest.Config) require.NoError(t, err) } -func deployIngressClass(ctx context.Context, t *testing.T, name string, client ctrlclient.Client) { +func deployIngressClass(ctx context.Context, t *testing.T, name string, client ctrlclient.Client) { //nolint:unparam t.Helper() ingress := &netv1.IngressClass{ From d9c498440bffba14b6aeaa1299f399f9876d5d45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grzegorz=20Burzy=C5=84ski?= Date: Wed, 3 Apr 2024 18:52:04 +0200 Subject: [PATCH 4/4] Apply suggestions from code review --- .../controllers/configuration/kongupstreampolicy_controller.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/controllers/configuration/kongupstreampolicy_controller.go b/internal/controllers/configuration/kongupstreampolicy_controller.go index cacaca898c..9ca17c6119 100644 --- a/internal/controllers/configuration/kongupstreampolicy_controller.go +++ b/internal/controllers/configuration/kongupstreampolicy_controller.go @@ -45,7 +45,7 @@ type KongUpstreamPolicyReconciler struct { // status for KongServiceFacades. KongServiceFacadeEnabled bool // HTTPRouteEnabled determines whether the controller should populate the KongUpstreamPolicy's - // ancestor status for HTTPRoutes. + // ancestor status for Services used in HTTPRoutes. HTTPRouteEnabled bool } @@ -268,7 +268,6 @@ func (r *KongUpstreamPolicyReconciler) getUpstreamPoliciesForHTTPRouteServices(c if !ok { return nil } - r.Log.Info("Found HTTPRoute", "name", httpRoute.Name, "namespace", httpRoute.Namespace) var requests []reconcile.Request for _, rule := range httpRoute.Spec.Rules { for _, br := range rule.BackendRefs {