Skip to content

Commit

Permalink
refactor: use referrer interface, gateway diff and gateway wrapper
Browse files Browse the repository at this point in the history
  • Loading branch information
KevFan committed Sep 18, 2023
1 parent 7c5d722 commit 0c70a9c
Show file tree
Hide file tree
Showing 31 changed files with 1,291 additions and 1,237 deletions.
2 changes: 1 addition & 1 deletion api/v1beta1/authpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,5 +155,5 @@ func (ap *AuthPolicy) Kind() string {
}

func (ap *AuthPolicy) BackReferenceAnnotationName() string {
return "kuadrant.io/authpolicy"
return "kuadrant.io/authpolicies"
}
2 changes: 1 addition & 1 deletion api/v1beta2/ratelimitpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func (r *RateLimitPolicy) Kind() string {
}

func (r *RateLimitPolicy) BackReferenceAnnotationName() string {
return "kuadrant.io/ratelimitpolicy"
return "kuadrant.io/ratelimitpolicies"
}

func init() {
Expand Down
6 changes: 3 additions & 3 deletions controllers/authpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (r *AuthPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Requ
markedForDeletion := ap.GetDeletionTimestamp() != nil

// fetch the target network object
targetNetworkObject, err := r.FetchValidTargetRef(ctx, ap.GetTargetRef(), ap.Namespace)
targetNetworkObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), ap.GetTargetRef(), ap.Namespace)
if err != nil {
if !markedForDeletion {
if apierrors.IsNotFound(err) {
Expand Down Expand Up @@ -134,7 +134,7 @@ func (r *AuthPolicyReconciler) reconcileResources(ctx context.Context, ap *api.A
}

// reconcile based on gateway diffs
gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, ap, targetNetworkObject, &common.KuadrantAuthPolicyRefsConfig{})
gatewayDiffObj, err := reconcilers.ComputeGatewayDiffs(ctx, r.Client(), ap, targetNetworkObject)
if err != nil {
return err
}
Expand All @@ -158,7 +158,7 @@ func (r *AuthPolicyReconciler) reconcileResources(ctx context.Context, ap *api.A

func (r *AuthPolicyReconciler) deleteResources(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error {
// delete based on gateway diffs
gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, ap, targetNetworkObject, &common.KuadrantAuthPolicyRefsConfig{})
gatewayDiffObj, err := reconcilers.ComputeGatewayDiffs(ctx, r.Client(), ap, targetNetworkObject)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions controllers/authpolicy_istio_authorization_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
var KuadrantExtAuthProviderName = env.GetString("AUTH_PROVIDER", "kuadrant-authorization")

// reconcileIstioAuthorizationPolicies translates and reconciles `AuthRules` into an Istio AuthorizationPoilcy containing them.
func (r *AuthPolicyReconciler) reconcileIstioAuthorizationPolicies(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object, gwDiffObj *reconcilers.GatewayDiff) error {
func (r *AuthPolicyReconciler) reconcileIstioAuthorizationPolicies(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object, gwDiffObj *reconcilers.GatewayDiffs) error {
if err := r.deleteIstioAuthorizationPolicies(ctx, ap, gwDiffObj); err != nil {
return err
}
Expand Down Expand Up @@ -58,7 +58,7 @@ func (r *AuthPolicyReconciler) reconcileIstioAuthorizationPolicies(ctx context.C
}

// deleteIstioAuthorizationPolicies deletes IstioAuthorizationPolicies previously created for gateways no longer targeted by the policy (directly or indirectly)
func (r *AuthPolicyReconciler) deleteIstioAuthorizationPolicies(ctx context.Context, ap *api.AuthPolicy, gwDiffObj *reconcilers.GatewayDiff) error {
func (r *AuthPolicyReconciler) deleteIstioAuthorizationPolicies(ctx context.Context, ap *api.AuthPolicy, gwDiffObj *reconcilers.GatewayDiffs) error {
logger, err := logr.FromContext(ctx)
if err != nil {
return err
Expand Down
5 changes: 3 additions & 2 deletions controllers/limitador_cluster_envoyfilter_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"

"github.com/go-logr/logr"
"github.com/kuadrant/kuadrant-operator/api/v1beta2"
limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1"
istioapinetworkingv1alpha3 "istio.io/api/networking/v1alpha3"
istioclientnetworkingv1alpha3 "istio.io/client-go/pkg/apis/networking/v1alpha3"
Expand Down Expand Up @@ -119,8 +120,8 @@ func (r *LimitadorClusterEnvoyFilterReconciler) desiredRateLimitingClusterEnvoyF
},
}

gateway := common.GatewayWrapper{Gateway: gw, PolicyRefsConfig: &common.KuadrantRateLimitPolicyRefsConfig{}}
rlpRefs := gateway.PolicyRefs()
gateway := reconcilers.GatewayWrapper{Gateway: gw, Referrer: &v1beta2.RateLimitPolicy{}}
rlpRefs := common.BackReferencesFromObject(gateway.Gateway, gateway.Referrer)
logger.V(1).Info("desiredRateLimitingClusterEnvoyFilter", "rlpRefs", rlpRefs)

if len(rlpRefs) < 1 {
Expand Down
6 changes: 3 additions & 3 deletions controllers/ratelimitpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl
markedForDeletion := rlp.GetDeletionTimestamp() != nil

// fetch the target network object
targetNetworkObject, err := r.FetchValidTargetRef(ctx, rlp.GetTargetRef(), rlp.Namespace)
targetNetworkObject, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), rlp.GetTargetRef(), rlp.Namespace)
if err != nil {
if !markedForDeletion {
if apierrors.IsNotFound(err) {
Expand Down Expand Up @@ -164,7 +164,7 @@ func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp
}

// reconcile based on gateway diffs
gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, rlp, targetNetworkObject, &common.KuadrantRateLimitPolicyRefsConfig{})
gatewayDiffObj, err := reconcilers.ComputeGatewayDiffs(ctx, r.Client(), rlp, targetNetworkObject)
if err != nil {
return err
}
Expand All @@ -188,7 +188,7 @@ func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp

func (r *RateLimitPolicyReconciler) deleteResources(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, targetNetworkObject client.Object) error {
// delete based on gateway diffs
gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, rlp, targetNetworkObject, &common.KuadrantRateLimitPolicyRefsConfig{})
gatewayDiffObj, err := reconcilers.ComputeGatewayDiffs(ctx, r.Client(), rlp, targetNetworkObject)
if err != nil {
return err
}
Expand Down
6 changes: 3 additions & 3 deletions controllers/ratelimitpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
serialized, err := json.Marshal(refs)
Expect(err).ToNot(HaveOccurred())
Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue(
common.RateLimitPoliciesBackRefAnnotation, string(serialized)))
rlp.BackReferenceAnnotationName(), string(serialized)))
})

It("Creates the correct WasmPlugin for a complex HTTPRoute and a RateLimitPolicy", func() {
Expand Down Expand Up @@ -599,7 +599,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
refs := []client.ObjectKey{rlpKey}
serialized, err := json.Marshal(refs)
Expect(err).ToNot(HaveOccurred())
Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue(common.RateLimitPoliciesBackRefAnnotation, string(serialized)))
Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue(rlp.BackReferenceAnnotationName(), string(serialized)))
})

It("Creates all the resources for a basic Gateway and RateLimitPolicy when missing a HTTPRoute attached to the Gateway", func() {
Expand Down Expand Up @@ -675,7 +675,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
refs := []client.ObjectKey{rlpKey}
serialized, err := json.Marshal(refs)
Expect(err).ToNot(HaveOccurred())
Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue(common.RateLimitPoliciesBackRefAnnotation, string(serialized)))
Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue(rlp.BackReferenceAnnotationName(), string(serialized)))
})
})
})
Expand Down
4 changes: 2 additions & 2 deletions controllers/ratelimitpolicy_limits.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ import (
)

func (r *RateLimitPolicyReconciler) reconcileLimits(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy) error {
rlpRefs, err := r.GetAllGatewayPolicyRefs(ctx, &common.KuadrantRateLimitPolicyRefsConfig{})
rlpRefs, err := r.GetAllGatewayPolicyRefs(ctx, rlp)
if err != nil {
return err
}
return r.reconcileLimitador(ctx, rlp, append(rlpRefs, client.ObjectKeyFromObject(rlp)))
}

func (r *RateLimitPolicyReconciler) deleteLimits(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy) error {
rlpRefs, err := r.GetAllGatewayPolicyRefs(ctx, &common.KuadrantRateLimitPolicyRefsConfig{})
rlpRefs, err := r.GetAllGatewayPolicyRefs(ctx, rlp)
if err != nil {
return err
}
Expand Down
21 changes: 12 additions & 9 deletions controllers/ratelimitpolicy_wasm_plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ import (
"github.com/kuadrant/kuadrant-operator/pkg/rlptools/wasm"
)

func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, gwDiffObj *reconcilers.GatewayDiff) error {
func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, gwDiffObj *reconcilers.GatewayDiffs) error {
logger, _ := logr.FromContext(ctx)

for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef {
logger.V(1).Info("reconcileWASMPluginConf: gateway with invalid policy ref", "gw key", gw.Key())
rlpRefs := gw.PolicyRefs()
rlpRefs := common.BackReferencesFromObject(gw.Gateway, gw.Referrer)
rlpKey := client.ObjectKeyFromObject(rlp)
// Remove the RLP key from the reference list. Only if it exists (it should)
if refID := common.FindObjectKey(rlpRefs, rlpKey); refID != len(rlpRefs) {
Expand All @@ -43,7 +43,7 @@ func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context,

for _, gw := range gwDiffObj.GatewaysWithValidPolicyRef {
logger.V(1).Info("reconcileWASMPluginConf: gateway with valid policy ref", "gw key", gw.Key())
wp, err := r.gatewayWASMPlugin(ctx, gw, gw.PolicyRefs())
wp, err := r.gatewayWASMPlugin(ctx, gw, common.BackReferencesFromObject(gw.Gateway, gw.Referrer))
if err != nil {
return err
}
Expand All @@ -55,11 +55,11 @@ func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context,

for _, gw := range gwDiffObj.GatewaysMissingPolicyRef {
logger.V(1).Info("reconcileWASMPluginConf: gateway missing policy ref", "gw key", gw.Key())
rlpRefs := gw.PolicyRefs()
rlpRefs := common.BackReferencesFromObject(gw.Gateway, gw.Referrer)
rlpKey := client.ObjectKeyFromObject(rlp)
// Add the RLP key to the reference list. Only if it does not exist (it should not)
if !slices.Contains(rlpRefs, rlpKey) {
rlpRefs = append(gw.PolicyRefs(), rlpKey)
rlpRefs = append(common.BackReferencesFromObject(gw.Gateway, gw.Referrer), rlpKey)
}
wp, err := r.gatewayWASMPlugin(ctx, gw, rlpRefs)
if err != nil {
Expand All @@ -73,7 +73,7 @@ func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context,
return nil
}

func (r *RateLimitPolicyReconciler) gatewayWASMPlugin(ctx context.Context, gw common.GatewayWrapper, rlpRefs []client.ObjectKey) (*istioclientgoextensionv1alpha1.WasmPlugin, error) {
func (r *RateLimitPolicyReconciler) gatewayWASMPlugin(ctx context.Context, gw reconcilers.GatewayWrapper, rlpRefs []client.ObjectKey) (*istioclientgoextensionv1alpha1.WasmPlugin, error) {
logger, _ := logr.FromContext(ctx)
logger.V(1).Info("gatewayWASMPlugin", "gwKey", gw.Key(), "rlpRefs", rlpRefs)

Expand Down Expand Up @@ -121,7 +121,7 @@ func (r *RateLimitPolicyReconciler) gatewayWASMPlugin(ctx context.Context, gw co
}

// returns nil when there is no rate limit policy to apply
func (r *RateLimitPolicyReconciler) wasmPluginConfig(ctx context.Context, gw common.GatewayWrapper, rlpRefs []client.ObjectKey) (*wasm.Plugin, error) {
func (r *RateLimitPolicyReconciler) wasmPluginConfig(ctx context.Context, gw reconcilers.GatewayWrapper, rlpRefs []client.ObjectKey) (*wasm.Plugin, error) {
logger, _ := logr.FromContext(ctx)
logger = logger.WithName("wasmPluginConfig").WithValues("gateway", gw.Key())

Expand All @@ -145,11 +145,14 @@ func (r *RateLimitPolicyReconciler) wasmPluginConfig(ctx context.Context, gw com

// target ref is a HTTPRoute
if common.IsTargetRefHTTPRoute(rlp.Spec.TargetRef) {
route, err := r.FetchValidHTTPRoute(ctx, rlp.TargetKey())
route, err := reconcilers.FetchTargetRefObject(ctx, r.Client(), rlp.GetTargetRef(), rlp.Namespace)
if err != nil {
return nil, err
}
rlps[rlpKey.String()] = &store{rlp: *rlp, route: *route}
// Should only be HTTPRoute in this if block
httpRoute, _ := route.(*gatewayapiv1beta1.HTTPRoute)

rlps[rlpKey.String()] = &store{rlp: *rlp, route: *httpRoute}
routeKeys[client.ObjectKeyFromObject(route).String()] = struct{}{}
continue
}
Expand Down
14 changes: 6 additions & 8 deletions pkg/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,12 @@ import (

// TODO: move the const to a proper place, or get it from config
const (
KuadrantRateLimitClusterName = "kuadrant-rate-limiting-service"
RateLimitPoliciesBackRefAnnotation = "kuadrant.io/ratelimitpolicies"
RateLimitPolicyBackRefAnnotation = "kuadrant.io/ratelimitpolicy"
AuthPoliciesBackRefAnnotation = "kuadrant.io/authpolicies"
AuthPolicyBackRefAnnotation = "kuadrant.io/authpolicy"
KuadrantNamespaceLabel = "kuadrant.io/namespace"
NamespaceSeparator = '/'
LimitadorName = "limitador"
KuadrantRateLimitClusterName = "kuadrant-rate-limiting-service"
RateLimitPolicyBackRefAnnotation = "kuadrant.io/ratelimitpolicy"
AuthPolicyBackRefAnnotation = "kuadrant.io/authpolicy"
KuadrantNamespaceLabel = "kuadrant.io/namespace"
NamespaceSeparator = '/'
LimitadorName = "limitador"
)

type KuadrantPolicy interface {
Expand Down
Loading

0 comments on commit 0c70a9c

Please sign in to comment.