From bdb14e773ca8bfc01e830d4f2e87ffef162da8fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Ma=C5=82ek?= Date: Tue, 21 Mar 2023 20:34:31 +0100 Subject: [PATCH] tests: add envtests unit test for HTTPRouteReconciler --- .../kongadminapi_controller_envtest_test.go | 4 +- .../gateway/dataplane_mock_test.go | 29 ++ .../gateway/httproute_controller.go | 12 +- .../httproute_controller_envtest_test.go | 345 ++++++++++++++++++ 4 files changed, 386 insertions(+), 4 deletions(-) create mode 100644 internal/controllers/gateway/dataplane_mock_test.go create mode 100644 internal/controllers/gateway/httproute_controller_envtest_test.go diff --git a/internal/controllers/configuration/kongadminapi_controller_envtest_test.go b/internal/controllers/configuration/kongadminapi_controller_envtest_test.go index 1a55db5ddd..b409e75581 100644 --- a/internal/controllers/configuration/kongadminapi_controller_envtest_test.go +++ b/internal/controllers/configuration/kongadminapi_controller_envtest_test.go @@ -60,7 +60,7 @@ func startKongAdminAPIServiceReconciler(ctx context.Context, t *testing.T, clien mgr, err := ctrl.NewManager(cfg, ctrl.Options{ Logger: logrusr.New(logrus.New()), - Scheme: scheme.Scheme, + Scheme: client.Scheme(), SyncPeriod: lo.ToPtr(2 * time.Second), MetricsBindAddress: "0", }) @@ -106,7 +106,7 @@ func TestKongAdminAPIController(t *testing.T) { // In tests below we use a deferred cancel to stop the manager and not wait // for its timeout. - cfg := envtest.Setup(t) + cfg := envtest.Setup(t, scheme.Scheme) client, err := ctrlclient.New(cfg, ctrlclient.Options{}) require.NoError(t, err) diff --git a/internal/controllers/gateway/dataplane_mock_test.go b/internal/controllers/gateway/dataplane_mock_test.go new file mode 100644 index 0000000000..7efe49ff69 --- /dev/null +++ b/internal/controllers/gateway/dataplane_mock_test.go @@ -0,0 +1,29 @@ +package gateway + +import ( + "sigs.k8s.io/controller-runtime/pkg/client" + + k8sobj "github.com/kong/kubernetes-ingress-controller/v2/internal/util/kubernetes/object" +) + +type DataplaneMock struct { + KubernetesObjectReportsEnabled bool + // Mapping namespace to name to status + ObjectsStatuses map[string]map[string]k8sobj.ConfigurationStatus +} + +func (d DataplaneMock) UpdateObject(obj client.Object) error { + return nil +} + +func (d DataplaneMock) DeleteObject(obj client.Object) error { + return nil +} + +func (d DataplaneMock) AreKubernetesObjectReportsEnabled() bool { + return d.KubernetesObjectReportsEnabled +} + +func (d DataplaneMock) KubernetesObjectConfigurationStatus(obj client.Object) k8sobj.ConfigurationStatus { + return d.ObjectsStatuses[obj.GetNamespace()][obj.GetName()] +} diff --git a/internal/controllers/gateway/httproute_controller.go b/internal/controllers/gateway/httproute_controller.go index e2225999fe..480f1dd1df 100644 --- a/internal/controllers/gateway/httproute_controller.go +++ b/internal/controllers/gateway/httproute_controller.go @@ -23,7 +23,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" - "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane" "github.com/kong/kubernetes-ingress-controller/v2/internal/util" k8sobj "github.com/kong/kubernetes-ingress-controller/v2/internal/util/kubernetes/object" ) @@ -32,13 +31,22 @@ import ( // HTTPRoute Controller - HTTPRouteReconciler // ----------------------------------------------------------------------------- +// TODO: This can probably be useful in other reconcilers as well. +type DataPlane interface { + UpdateObject(obj client.Object) error + DeleteObject(obj client.Object) error + + AreKubernetesObjectReportsEnabled() bool + KubernetesObjectConfigurationStatus(obj client.Object) k8sobj.ConfigurationStatus +} + // HTTPRouteReconciler reconciles an HTTPRoute object. type HTTPRouteReconciler struct { client.Client Log logr.Logger Scheme *runtime.Scheme - DataplaneClient *dataplane.KongClient + DataplaneClient DataPlane // If EnableReferenceGrant is true, we will check for ReferenceGrant if backend in another // namespace is in backendRefs. // If it is false, referencing backend in different namespace will be rejected. diff --git a/internal/controllers/gateway/httproute_controller_envtest_test.go b/internal/controllers/gateway/httproute_controller_envtest_test.go new file mode 100644 index 0000000000..82aa382890 --- /dev/null +++ b/internal/controllers/gateway/httproute_controller_envtest_test.go @@ -0,0 +1,345 @@ +//go:build envtest +// +build envtest + +package gateway_test + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/bombsimon/logrusr/v2" + "github.com/google/uuid" + "github.com/samber/lo" + "github.com/sirupsen/logrus" + "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/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + + "github.com/kong/kubernetes-ingress-controller/v2/internal/controllers/gateway" + "github.com/kong/kubernetes-ingress-controller/v2/test/envtest" +) + +func init() { + if err := gatewayv1beta1.Install(scheme.Scheme); err != nil { + panic(err) + } +} + +func startHTTPRouteReconciler(ctx context.Context, t *testing.T, client ctrlclient.Client, cfg *rest.Config, reconciler *gateway.HTTPRouteReconciler) { + mgr, err := ctrl.NewManager(cfg, ctrl.Options{ + Logger: logrusr.New(logrus.New()), + Scheme: client.Scheme(), + MetricsBindAddress: "0", + }) + require.NoError(t, err) + + reconciler.Log = mgr.GetLogger() + + require.NoError(t, reconciler.SetupWithManager(mgr)) + + // This wait group makes it so that we wait for manager to exit. + // This way we get clean test logs not mixing between tests. + wg := sync.WaitGroup{} + wg.Add(1) + go func() { + defer wg.Done() + assert.NoError(t, mgr.Start(ctx)) + }() + t.Cleanup(func() { wg.Wait() }) + + return +} + +func TestHTTPRouteReconcilerProperlyReactsToReferenceGrant(t *testing.T) { + t.Parallel() + + const ( + waitDuration = 10 * time.Second + tickDuration = 10 * time.Millisecond + ) + + // In tests below we use a deferred cancel to stop the manager and not wait + // for its timeout. + + cfg := envtest.Setup(t, scheme.Scheme) + client, err := ctrlclient.New(cfg, ctrlclient.Options{ + Scheme: scheme.Scheme, + }) + require.NoError(t, err) + + testcases := []struct { + name string + reconciler *gateway.HTTPRouteReconciler + }{ + { + name: "with KubernetesObjectsReportsEnabled set to true", + reconciler: &gateway.HTTPRouteReconciler{ + DataplaneClient: func() gateway.DataPlane { + return gateway.DataplaneMock{ + KubernetesObjectReportsEnabled: true, + } + }(), + Client: client, + EnableReferenceGrant: true, + }, + }, + { + name: "with KubernetesObjectsReportsEnabled set to false", + reconciler: &gateway.HTTPRouteReconciler{ + DataplaneClient: func() gateway.DataPlane { + return gateway.DataplaneMock{ + KubernetesObjectReportsEnabled: false, + } + }(), + Client: client, + EnableReferenceGrant: true, + }, + }, + } + + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + // TODO figure out why making the test parallel passes even without the ReferenceGrant watch. + // t.Parallel() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + ns := envtest.CreateNamespace(ctx, t, client) + nsRoute := envtest.CreateNamespace(ctx, t, client) + + svc := corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: "backend-1", + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "http", + Protocol: corev1.ProtocolTCP, + Port: 80, + TargetPort: intstr.FromInt(80), + }, + }, + }, + } + require.NoError(t, client.Create(ctx, &svc)) + + startHTTPRouteReconciler(ctx, t, client, cfg, tc.reconciler) + + gwc := gatewayv1beta1.GatewayClass{ + Spec: gatewayv1beta1.GatewayClassSpec{ + ControllerName: gateway.GetControllerName(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: uuid.NewString(), + Annotations: map[string]string{ + "konghq.com/gatewayclass-unmanaged": "placeholder", + }, + }, + } + require.NoError(t, client.Create(ctx, &gwc)) + + gw := gatewayv1beta1.Gateway{ + Spec: gatewayv1beta1.GatewaySpec{ + GatewayClassName: gatewayv1beta1.ObjectName(gwc.Name), + Listeners: []gatewayv1beta1.Listener{ + { + Name: gatewayv1beta1.SectionName("http"), + Port: gatewayv1beta1.PortNumber(80), + Protocol: gatewayv1beta1.HTTPProtocolType, + AllowedRoutes: &gatewayv1beta1.AllowedRoutes{ + Namespaces: &gatewayv1beta1.RouteNamespaces{ + From: lo.ToPtr(gatewayv1beta1.NamespacesFromAll), + }, + }, + }, + }, + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: uuid.NewString(), + }, + } + require.NoError(t, client.Create(ctx, &gw)) + + gwOld := gw.DeepCopy() + gw.Status = gatewayv1beta1.GatewayStatus{ + Addresses: []gatewayv1beta1.GatewayAddress{ + { + Type: lo.ToPtr(gatewayv1beta1.IPAddressType), + Value: "10.0.0.1", + }, + }, + Conditions: []metav1.Condition{ + { + Type: "Ready", + Status: metav1.ConditionTrue, + Reason: "Ready", + LastTransitionTime: metav1.Now(), + ObservedGeneration: gw.Generation, + }, + { + Type: "Accepted", + Status: metav1.ConditionTrue, + Reason: "Accepted", + LastTransitionTime: metav1.Now(), + ObservedGeneration: gw.Generation, + }, + { + Type: "Programmed", + Status: metav1.ConditionTrue, + Reason: "Programmed", + LastTransitionTime: metav1.Now(), + ObservedGeneration: gw.Generation, + }, + }, + Listeners: []gatewayv1beta1.ListenerStatus{ + { + Name: gatewayv1beta1.SectionName("http"), + Conditions: []metav1.Condition{ + { + Type: "Accepted", + Status: metav1.ConditionTrue, + Reason: "Accepted", + LastTransitionTime: metav1.Now(), + }, + }, + SupportedKinds: []gatewayv1beta1.RouteGroupKind{ + { + Group: lo.ToPtr(gatewayv1beta1.Group(gatewayv1beta1.GroupVersion.Group)), + Kind: "HTTPRoute", + }, + }, + }, + }, + } + require.NoError(t, client.Status().Patch(ctx, &gw, ctrlclient.MergeFrom(gwOld))) + + route := gatewayv1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: nsRoute.Name, + Name: uuid.NewString(), + }, + Spec: gatewayv1beta1.HTTPRouteSpec{ + CommonRouteSpec: gatewayv1beta1.CommonRouteSpec{ + ParentRefs: []gatewayv1beta1.ParentReference{{ + Name: gatewayv1beta1.ObjectName(gw.Name), + Namespace: lo.ToPtr(gatewayv1beta1.Namespace(ns.Name)), + }}, + }, + Rules: []gatewayv1beta1.HTTPRouteRule{{ + BackendRefs: []gatewayv1beta1.HTTPBackendRef{{ + BackendRef: gatewayv1beta1.BackendRef{ + BackendObjectReference: gatewayv1beta1.BackendObjectReference{ + Name: gatewayv1beta1.ObjectName("backend-1"), + Namespace: lo.ToPtr(gatewayv1beta1.Namespace(ns.Name)), + }, + }, + }}, + }}, + }, + } + require.NoError(t, client.Create(ctx, &route)) + + t.Logf("verifying that HTTPRoute has ResolvedRefs set to Status False and Reason RefNotPermitted") + assert.Eventually(t, + func() bool { + err = client.Get(ctx, ctrlclient.ObjectKey{Namespace: nsRoute.Name, Name: route.Name}, &route) + if err != nil { + t.Logf("failed to get HTTPRoute: %v", err) + return false + } + + return lo.ContainsBy(route.Status.Parents, func(p gateway.RouteParentStatus) bool { + resolvedRefs := lo.ContainsBy(p.Conditions, func(c metav1.Condition) bool { + return c.Type == "ResolvedRefs" && c.Status == "False" && c.Reason == "RefNotPermitted" + }) + + return resolvedRefs + }) + }, + waitDuration, tickDuration) + + rg := gatewayv1beta1.ReferenceGrant{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: uuid.NewString(), + }, + Spec: gatewayv1beta1.ReferenceGrantSpec{ + From: []gatewayv1beta1.ReferenceGrantFrom{ + { + Group: gatewayv1beta1.Group(gatewayv1beta1.GroupVersion.Group), + Kind: "HTTPRoute", + Namespace: gatewayv1beta1.Namespace(nsRoute.Name), + }, + }, + To: []gatewayv1beta1.ReferenceGrantTo{ + { + Group: "", + Kind: "Service", + }, + }, + }, + } + require.NoError(t, client.Create(ctx, &rg)) + t.Logf("verifying that HTTPRoute gets accepted by HTTPRouteReconciler after relevant ReferenceGrant gets created") + assert.Eventually(t, + func() bool { + err = client.Get(ctx, ctrlclient.ObjectKey{Namespace: nsRoute.Name, Name: route.Name}, &route) + if err != nil { + t.Logf("failed to get HTTPRoute: %v", err) + return false + } + + return lo.ContainsBy(route.Status.Parents, func(p gateway.RouteParentStatus) bool { + resolvedRefs := lo.ContainsBy(p.Conditions, func(c metav1.Condition) bool { + return c.Type == "ResolvedRefs" && c.Status == "True" && c.Reason == "ResolvedRefs" + }) + + accepted := lo.ContainsBy(p.Conditions, func(c metav1.Condition) bool { + return c.Type == "Accepted" && c.Status == "True" && c.Reason == "Accepted" + }) + + // TODO: Programmed can't be easily checked in here because we'd need to rely on + // dataplane client having an updated kubernetesObjectReportsFilter and that will + // require some work. + return resolvedRefs && accepted + }) + }, + waitDuration, tickDuration) + + require.NoError(t, client.Delete(ctx, &rg)) + t.Logf("verifying that HTTPRoute gets its ResolvedRefs condition to Status False and Reason RefNotPermitted when relevant ReferenceGrant gets deleted") + assert.Eventually(t, + func() bool { + err = client.Get(ctx, ctrlclient.ObjectKey{Namespace: nsRoute.Name, Name: route.Name}, &route) + if err != nil { + t.Logf("failed to get HTTPRoute: %v", err) + return false + } + + return lo.ContainsBy(route.Status.Parents, func(p gateway.RouteParentStatus) bool { + resolvedRefs := lo.ContainsBy(p.Conditions, func(c metav1.Condition) bool { + return c.Type == "ResolvedRefs" && c.Status == "False" && c.Reason == "RefNotPermitted" + }) + + return resolvedRefs + }) + }, + waitDuration, tickDuration) + }) + } +}