Skip to content

Commit

Permalink
fix: add check for existence of HTTPRoute in KongUpstreamPolicy contr…
Browse files Browse the repository at this point in the history
…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 <czeslavo@gmail.com>
(cherry picked from commit 30a2991)
  • Loading branch information
randmonkey authored and czeslavo committed Apr 3, 2024
1 parent cb6e7ab commit e8b881e
Show file tree
Hide file tree
Showing 9 changed files with 505 additions and 44 deletions.
24 changes: 18 additions & 6 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ 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)
- 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]

Expand Down Expand Up @@ -126,14 +131,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down
36 changes: 21 additions & 15 deletions internal/controllers/configuration/kongupstreampolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
36 changes: 16 additions & 20 deletions internal/manager/controllerdef.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
}),
},
},
{
Expand Down
2 changes: 1 addition & 1 deletion test/envtest/adminapi_discoverer_envtest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit e8b881e

Please sign in to comment.