fix: add check for existence of HTTPRoute in KongUpstreamPolicy contr…

…oller (#5780) * add HTTPRoute controller for watching HTTPRoute to enqueue KongUpstreamPolicy needing to update ancestor status * add CHANGELOG * do not use dynamic CRD controller and and envtest cases * Apply suggestions from code review --------- Co-authored-by: Grzegorz Burzyński <[email protected]>
Kong · Apr 3, 2024 · 30a2991 · 30a2991
1 parent 9b5e378
commit 30a2991
Show file tree

Hide file tree

Showing 9 changed files with 484 additions and 38 deletions.
diff --git 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]
 

diff --git 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
@@ -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 Services used in HTTPRoutes.
+	HTTPRouteEnabled bool
 }
 
 // SetupWithManager sets up the controller with the Manager.
@@ -71,13 +74,15 @@ 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.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 {
@@ -136,13 +141,15 @@ 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.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 {
@@ -261,7 +268,6 @@ func (r *KongUpstreamPolicyReconciler) getUpstreamPoliciesForHTTPRouteServices(c
 	if !ok {
 		return nil
 	}
-
 	var requests []reconcile.Request
 	for _, rule := range httpRoute.Spec.Rules {
 		for _, br := range rule.BackendRefs {

diff --git 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
@@ -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
@@ -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"
@@ -250,28 +251,23 @@ func setupControllers(
 				// StatusQueue:       kubernetesStatusQueue,
 			},
 		},
+		// KongUpstreamPolicy controller.
+		// When HTTPRoute exists, the controller is enabled to watch HTTPRoutes to set ancestor status of KongUpstreamPolicies.
 		{
 			Enabled: c.KongUpstreamPolicyEnabled,
-			Controller: &crds.DynamicCRDController{
-				Manager:          mgr,
-				Log:              ctrl.LoggerFrom(ctx).WithName("controllers").WithName("Dynamic/KongUpstreamPolicy"),
-				CacheSyncTimeout: c.CacheSyncTimeout,
-				RequiredCRDs: []schema.GroupVersionResource{
-					{
-						Group:    gatewayv1.GroupVersion.Group,
-						Version:  gatewayv1.GroupVersion.Version,
-						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.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
@@ -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,