[backport release/3.1.x] fix: add check for existence of HTTPRoute in…

… KongUpstreamPolicy controller (#5806) * fix: add check for existence of HTTPRoute in KongUpstreamPolicy controller (#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]> (cherry picked from commit 30a2991) * Update CHANGELOG.md --------- Co-authored-by: Tao Yi <[email protected]>
Kong · Apr 3, 2024 · 9af1396 · 9af1396
1 parent cb6e7ab
commit 9af1396
Show file tree

Hide file tree

Showing 9 changed files with 503 additions and 44 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -92,6 +92,9 @@ Adding a new version? You'll need three changes:
   [#5658](https://github.com/Kong/kubernetes-ingress-controller/issues/5658) 
 - Do not require `rsa_public_key` field in credential `Secret`s when working with jwt HMAC credentials.
   [#5737](https://github.com/Kong/kubernetes-ingress-controller/issues/5737)
+- `KongUpstreamPolicy` controller no longer requires existence of `HTTPRoute` CRD
+  to start.
+  [#5780](https://github.com/Kong/kubernetes-ingress-controller/pull/5780)
 
 ## [3.1.2]
 
@@ -126,14 +129,21 @@ Adding a new version? You'll need three changes:
 
 ### Highlights
 
-- 🔒 Kong Gateway's [secret vaults](kong-vault) now become a first-class citizen for Kubernetes users
-  thanks to the new `KongVault` CRD.
-- 🔒 Providing an Enterprise license to KIC-managed Kong Gateways becomes much easier thanks to the new `KongLicense` CRD
-  which is used to dynamically provision all the replicas with the latest license found in the cluster.
-- ✨ Populating a single field of `KongPlugin`'s configuration with use of a Kubernetes Secret becomes possible thanks 
-  to the new `KongPlugin`'s `configPatches` field.
+- 🔒 Kong Gateway's [secret vaults][kong-vault] now become a first-class citizen for Kubernetes users thanks to the new
+  `KongVault` CRD. _See [Kong Vault guide][vault-guide] and [CRDs reference][crds-ref] for more details._
+- 🔒 Providing an Enterprise license to KIC-managed Kong Gateways becomes much easier thanks to the new `KongLicense`
+  CRD which is used to dynamically provision all the replicas with the latest license found in the cluster. _See
+  [Enterprise License][license-guide] and [CRDs reference][crds-ref] for more details._
+- ✨ Populating a single field of `KongPlugin`'s configuration with use of a Kubernetes Secret becomes possible thanks
+  to the new `KongPlugin`'s `configPatches` field. _See [Using Kubernetes Secrets in Plugins][secrets-in-plugins-guide]
+  and [CRDs reference][crds-ref] for more details._
 - 🔒 All sensitive information stored in the cluster is now sanitized while sending configuration to Konnect.
 
+[crds-ref]: https://docs.konghq.com/kubernetes-ingress-controller/latest/reference/custom-resources/
+[vault-guide]: https://docs.konghq.com/kubernetes-ingress-controller/latest/guides/security/kong-vault/
+[license-guide]: https://docs.konghq.com/kubernetes-ingress-controller/latest/license/
+[secrets-in-plugins-guide]: https://docs.konghq.com/kubernetes-ingress-controller/latest/guides/security/plugin-secrets/
+
 ### Added
 
 - New CRD `KongVault` to represent a custom Kong vault for storing sensitive

diff --git a/internal/controllers/configuration/konglicense_controller.go b/internal/controllers/configuration/konglicense_controller.go
@@ -366,7 +366,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,