Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rlp-v2] do not use gateway name nor host in the RL domain #218

Merged
merged 2 commits into from
Aug 9, 2023
Merged

[rlp-v2] do not use gateway name nor host in the RL domain #218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5fe61cb
fix: do not use gateway name nor host in the RL domain
guicassolato Jul 14, 2023
73b0572
Limit names in the Limitador conditions and RL request data generated…
guicassolato Jul 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions controllers/ratelimitpolicy_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp
return err
}

if err := r.reconcileLimits(ctx, rlp, gatewayDiffObj); err != nil {
if err := r.reconcileLimits(ctx, rlp); err != nil {
return err
}

Expand Down Expand Up @@ -205,7 +205,7 @@ func (r *RateLimitPolicyReconciler) deleteResources(ctx context.Context, rlp *ku
return err
}

if err := r.reconcileLimits(ctx, rlp, gatewayDiffObj); err != nil && !apierrors.IsNotFound(err) {
if err := r.deleteLimits(ctx, rlp); err != nil && !apierrors.IsNotFound(err) {
return err
}

Expand Down
26 changes: 13 additions & 13 deletions controllers/ratelimitpolicy_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ var _ = Describe("RateLimitPolicy controller", func() {
Expect(existingLimitador.Spec.Limits).To(ContainElements(limitadorv1alpha1.RateLimit{
MaxValue: 1,
Seconds: 3 * 60,
Namespace: common.MarshallNamespace(client.ObjectKeyFromObject(gateway), "*.example.com"),
Conditions: []string{fmt.Sprintf("%s/%s/l1 == \"1\"", testNamespace, rlpName)},
Namespace: rlptools.LimitsNamespaceFromRLP(rlp),
Conditions: []string{`limit.l1__2804bad6 == "1"`},
Variables: []string{},
}))

Expand All @@ -227,7 +227,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
RateLimitPolicies: []wasm.RateLimitPolicy{
{
Name: rlpKey.String(),
Domain: fmt.Sprintf("%s/%s#%s", testNamespace, gwName, "*.example.com"),
Domain: rlptools.LimitsNamespaceFromRLP(rlp),
Rules: []wasm.Rule{
{
Conditions: []wasm.Condition{
Expand All @@ -249,7 +249,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
Data: []wasm.DataItem{
{
Static: &wasm.StaticSpec{
Key: fmt.Sprintf("%s/%s/l1", testNamespace, rlpName),
Key: `limit.l1__2804bad6`,
Value: "1",
},
},
Expand Down Expand Up @@ -395,7 +395,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
Expect(existingWASMConfig.RateLimitPolicies).To(HaveLen(1))
wasmRLP := existingWASMConfig.RateLimitPolicies[0]
Expect(wasmRLP.Name).To(Equal(rlpKey.String()))
Expect(wasmRLP.Domain).To(Equal(fmt.Sprintf("%s/%s#%s", testNamespace, gwName, "*.toystore.acme.com")))
Expect(wasmRLP.Domain).To(Equal(rlptools.LimitsNamespaceFromRLP(rlp)))
Expect(wasmRLP.Rules).To(ContainElement(wasm.Rule{ // rule to activate the 'toys' limit defintion
Conditions: []wasm.Condition{
{
Expand Down Expand Up @@ -450,7 +450,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
Data: []wasm.DataItem{
{
Static: &wasm.StaticSpec{
Key: fmt.Sprintf("%s/%s/toys", testNamespace, rlpName),
Key: "limit.toys__3bfcbeee",
Value: "1",
},
},
Expand All @@ -476,7 +476,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
Data: []wasm.DataItem{
{
Static: &wasm.StaticSpec{
Key: fmt.Sprintf("%s/%s/assets", testNamespace, rlpName),
Key: "limit.assets__8bf729ff",
Value: "1",
},
},
Expand Down Expand Up @@ -546,8 +546,8 @@ var _ = Describe("RateLimitPolicy controller", func() {
Expect(existingLimitador.Spec.Limits).To(ContainElements(limitadorv1alpha1.RateLimit{
MaxValue: 1,
Seconds: 3 * 60,
Namespace: common.MarshallNamespace(client.ObjectKeyFromObject(gateway), "*"),
Conditions: []string{fmt.Sprintf("%s/%s/l1 == \"1\"", testNamespace, rlpName)},
Namespace: rlptools.LimitsNamespaceFromRLP(rlp),
Conditions: []string{`limit.l1__2804bad6 == "1"`},
Variables: []string{},
}))

Expand All @@ -573,7 +573,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
RateLimitPolicies: []wasm.RateLimitPolicy{
{
Name: rlpKey.String(),
Domain: fmt.Sprintf("%s/%s#%s", testNamespace, gwName, "*"),
Domain: rlptools.LimitsNamespaceFromRLP(rlp),
Rules: []wasm.Rule{
{
Conditions: []wasm.Condition{
Expand All @@ -595,7 +595,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
Data: []wasm.DataItem{
{
Static: &wasm.StaticSpec{
Key: fmt.Sprintf("%s/%s/l1", testNamespace, rlpName),
Key: `limit.l1__2804bad6`,
Value: "1",
},
},
Expand Down Expand Up @@ -671,8 +671,8 @@ var _ = Describe("RateLimitPolicy controller", func() {
Expect(existingLimitador.Spec.Limits).To(ContainElements(limitadorv1alpha1.RateLimit{
MaxValue: 1,
Seconds: 3 * 60,
Namespace: common.MarshallNamespace(client.ObjectKeyFromObject(gateway), "*"),
Conditions: []string{fmt.Sprintf("%s/%s/l1 == \"1\"", testNamespace, rlpName)},
Namespace: rlptools.LimitsNamespaceFromRLP(rlp),
Conditions: []string{`limit.l1__2804bad6 == "1"`},
Variables: []string{},
}))

Expand Down
196 changes: 59 additions & 137 deletions controllers/ratelimitpolicy_limits.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,189 +2,111 @@ package controllers

import (
"context"
"encoding/json"
"fmt"

"github.com/go-logr/logr"
limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1"
"sigs.k8s.io/controller-runtime/pkg/client"

kuadrantv1beta2 "github.com/kuadrant/kuadrant-operator/api/v1beta2"
"github.com/kuadrant/kuadrant-operator/pkg/common"
"github.com/kuadrant/kuadrant-operator/pkg/reconcilers"
"github.com/kuadrant/kuadrant-operator/pkg/rlptools"
)

func (r *RateLimitPolicyReconciler) reconcileLimits(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, gwDiffObj *reconcilers.GatewayDiff) error {
func (r *RateLimitPolicyReconciler) reconcileLimits(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy) error {
rlpRefs, err := r.GetAllGatewayPolicyRefs(ctx, &common.KuadrantRateLimitPolicyRefsConfig{})
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{})
if err != nil {
return err
}
var rlpRefsWithoutRLP []client.ObjectKey
for _, rlpRef := range rlpRefs {
if rlpRef.Name == rlp.Name && rlpRef.Namespace == rlp.Namespace {
continue
}
rlpRefsWithoutRLP = append(rlpRefsWithoutRLP, rlpRef)
}
return r.reconcileLimitador(ctx, rlp, rlpRefsWithoutRLP)
}

func (r *RateLimitPolicyReconciler) reconcileLimitador(ctx context.Context, rlp *kuadrantv1beta2.RateLimitPolicy, rlpRefs []client.ObjectKey) error {
logger, _ := logr.FromContext(ctx)
logger = logger.WithName("reconcileLimitador").WithValues("rlp refs", common.Map(rlpRefs, func(ref client.ObjectKey) string { return ref.String() }))

rateLimitIndex, err := r.buildRateLimitIndex(ctx, rlpRefs)
if err != nil {
return err
}

logger.V(1).Info("Getting Kuadrant namespace")
// get the current limitador cr for the kuadrant instance so we can compare if it needs to be updated
logger.V(1).Info("get kuadrant namespace")
var kuadrantNamespace string
kuadrantNamespace, isSet := common.GetNamespaceFromPolicy(rlp)
kuadrantNamespace, isSet := common.GetKuadrantNamespaceFromPolicy(rlp)
if !isSet {
var err error
kuadrantNamespace, err = common.GetNamespaceFromPolicyTargetRef(ctx, r.Client(), rlp)
kuadrantNamespace, err = common.GetKuadrantNamespaceFromPolicyTargetRef(ctx, r.Client(), rlp)
if err != nil {
logger.Error(err, "failed to get Kuadrant namespace")
logger.Error(err, "failed to get kuadrant namespace")
return err
}
common.AnnotateObject(rlp, kuadrantNamespace)
err = r.UpdateResource(ctx, rlp)
err = r.UpdateResource(ctx, rlp) // @guicassolato: not sure if this belongs to here
if err != nil {
logger.Error(err, "failed to update policy, re-queuing")
return err
}
}

limitadorKey := client.ObjectKey{Name: common.LimitadorName, Namespace: kuadrantNamespace}
limitador := &limitadorv1alpha1.Limitador{}
err := r.Client().Get(ctx, limitadorKey, limitador)
logger.V(1).Info("reconcileLimits", "get limitador", limitadorKey, "err", err)
err = r.Client().Get(ctx, limitadorKey, limitador)
logger.V(1).Info("get limitador", "limitador", limitadorKey, "err", err)
if err != nil {
return err
}

limitIdx := rlptools.NewLimitadorIndex(limitador, logger)

for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef {
logger.V(1).Info("reconcileLimits: gateway with invalid policy ref", "key", gw.Key())
limitIdx.DeleteGateway(gw.Key())
}

for _, gw := range gwDiffObj.GatewaysWithValidPolicyRef {
logger.V(1).Info("reconcileLimits: gateway with valid policy ref", "rlpRefs", gw.PolicyRefs())

gwLimits, err := r.gatewayLimits(ctx, gw, gw.PolicyRefs())
if err != nil {
return err
}

// delete first to detect when limits have been deleted.
// For instance, gw A has 3 limits
// one limit has been deleted for gwA (coming from a limit deletion in one RLP)
// gw A has now 2 limits
// Deleting the 3 original limits the resulting index will contain only 2 limits as expected
limitIdx.DeleteGateway(gw.Key())
limitIdx.AddGatewayLimits(gw.Key(), gwLimits)
}

for _, gw := range gwDiffObj.GatewaysMissingPolicyRef {
rlpRefs := append(gw.PolicyRefs(), client.ObjectKeyFromObject(rlp))
logger.V(1).Info("reconcileLimits: gateway missing policy ref", "rlpRefs", rlpRefs)

gwLimits, err := r.gatewayLimits(ctx, gw, rlpRefs)
if err != nil {
return err
}

// The gw A had X limits from N RLPs
// now there there are N+1 RLPs
// r.gatewayLimits will compute all the limits for the given gateway with the N+1 RLPs
// the existing limits need to be deleted first,
// otherwise they would be added again and will be duplicated in the index
limitIdx.DeleteGateway(gw.Key())
limitIdx.AddGatewayLimits(gw.Key(), gwLimits)
}

// Build a new index with the original content of limitador to compare with the new limits
originalLimitIndex := rlptools.NewLimitadorIndex(limitador, logger)

if logger.V(1).Enabled() {
jsonData, err := json.MarshalIndent(originalLimitIndex.ToLimits(), "", " ")
if err != nil {
return err
}
logger.V(1).Info("reconcileLimits: original limit index")
logger.V(1).Info(string(jsonData))

jsonData, err = json.MarshalIndent(limitIdx.ToLimits(), "", " ")
if err != nil {
return err
}
logger.V(1).Info("reconcileLimits: new limit index")
logger.V(1).Info(string(jsonData))
// return if limitador is up to date
if rlptools.Equal(rateLimitIndex.ToRateLimits(), limitador.Spec.Limits) {
logger.V(1).Info("limitador is up to date, skipping update")
return nil
}

equalIndexes := originalLimitIndex.Equals(limitIdx)
logger.V(1).Info("reconcileLimits", "equal index", equalIndexes)

if !equalIndexes {
limitador.Spec.Limits = limitIdx.ToLimits()
err := r.UpdateResource(ctx, limitador)
logger.V(1).Info("reconcileLimits: update limitador", "limitador", limitadorKey, "err", err)
if err != nil {
return err
}
// update limitador
limitador.Spec.Limits = rateLimitIndex.ToRateLimits()
err = r.UpdateResource(ctx, limitador)
logger.V(1).Info("update limitador", "limitador", limitadorKey, "err", err)
if err != nil {
return err
}

return nil
}

func (r *RateLimitPolicyReconciler) gatewayLimits(ctx context.Context,
gw common.GatewayWrapper, rlpRefs []client.ObjectKey) (rlptools.LimitsByDomain, error) {
func (r *RateLimitPolicyReconciler) buildRateLimitIndex(ctx context.Context, rlpRefs []client.ObjectKey) (*rlptools.RateLimitIndex, error) {
logger, _ := logr.FromContext(ctx)
logger.V(1).Info("gatewayLimits", "gwKey", gw.Key(), "rlpRefs", rlpRefs)

// Load all rate limit policies
routeRLPList := make([]*kuadrantv1beta2.RateLimitPolicy, 0)
var gwRLP *kuadrantv1beta2.RateLimitPolicy
for _, rlpKey := range rlpRefs {
rlp := &kuadrantv1beta2.RateLimitPolicy{}
err := r.Client().Get(ctx, rlpKey, rlp)
logger.V(1).Info("gatewayLimits", "get rlp", rlpKey, "err", err)
if err != nil {
return nil, err
}
logger = logger.WithName("buildRateLimitIndex").WithValues("ratelimitpolicies", rlpRefs)

if common.IsTargetRefHTTPRoute(rlp.Spec.TargetRef) {
routeRLPList = append(routeRLPList, rlp)
} else if common.IsTargetRefGateway(rlp.Spec.TargetRef) {
if gwRLP == nil {
gwRLP = rlp
} else {
return nil, fmt.Errorf("gatewayLimits: multiple gateway RLP found and only one expected. rlp keys: %v", rlpRefs)
}
}
}

limits := rlptools.LimitsByDomain{}
rateLimitIndex := rlptools.NewRateLimitIndex()

if gwRLP != nil {
if len(gw.Hostnames()) == 0 {
// wildcard domain
limits["*"] = append(limits["*"], rlptools.ReadLimitsFromRLP(gwRLP)...)
} else {
for _, gwHostname := range gw.Hostnames() {
limits[string(gwHostname)] = append(limits[string(gwHostname)], rlptools.ReadLimitsFromRLP(gwRLP)...)
}
for _, rlpKey := range rlpRefs {
if _, ok := rateLimitIndex.Get(rlpKey); ok {
continue
}
}

for _, httpRouteRLP := range routeRLPList {
httpRoute, err := r.FetchValidHTTPRoute(ctx, httpRouteRLP.TargetKey())
rlp := &kuadrantv1beta2.RateLimitPolicy{}
err := r.Client().Get(ctx, rlpKey, rlp)
logger.V(1).Info("get rlp", "ratelimitpolicy", rlpKey, "err", err)
if err != nil {
return nil, err
}

// gateways limits merged with the route level limits
mergedLimits := mergeLimits(httpRouteRLP, gwRLP)
// routeLimits referenced by multiple hostnames
for _, hostname := range httpRoute.Spec.Hostnames {
limits[string(hostname)] = append(limits[string(hostname)], mergedLimits...)
}
}

return limits, nil
}

// merged currently implemented with list append operation
func mergeLimits(routeRLP *kuadrantv1beta2.RateLimitPolicy, gwRLP *kuadrantv1beta2.RateLimitPolicy) []rlptools.Limit {
limits := rlptools.ReadLimitsFromRLP(routeRLP)

if gwRLP == nil {
return limits
rateLimitIndex.Set(rlpKey, rlptools.LimitadorRateLimitsFromRLP(rlp))
}

// add gateway level limits
return append(limits, rlptools.ReadLimitsFromRLP(gwRLP)...)
return rateLimitIndex, nil
}
2 changes: 1 addition & 1 deletion controllers/ratelimitpolicy_wasm_plugins.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func (r *RateLimitPolicyReconciler) wasmPluginConfig(ctx context.Context, gw com

wasmPlugin.RateLimitPolicies = append(wasmPlugin.RateLimitPolicies, wasm.RateLimitPolicy{
Name: rlpKey.String(),
Domain: common.MarshallNamespace(gw.Key(), string(hostnames[0])), // TODO(guicassolato): https://github.com/Kuadrant/kuadrant-operator/issues/201. Meanwhile, we are using the first hostname so it matches at least one set of limit definitions in the Limitador CR
Domain: rlptools.LimitsNamespaceFromRLP(&rlp),
Rules: rules,
Hostnames: common.HostnamesToStrings(hostnames), // we might be listing more hostnames than needed due to route selectors hostnames possibly being more restrictive
Service: common.KuadrantRateLimitClusterName,
Expand Down
5 changes: 0 additions & 5 deletions pkg/common/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,11 +225,6 @@ func UnMarshallLimitNamespace(ns string) (client.ObjectKey, string, error) {
return objKey, domain, nil
}

// MarshallNamespace serializes limit namespace with format "gwNS/gwName#domain"
func MarshallNamespace(gwKey client.ObjectKey, domain string) string {
return fmt.Sprintf("%s/%s#%s", gwKey.Namespace, gwKey.Name, domain)
}

// UnMarshallObjectKey takes a string input and converts it into an ObjectKey struct that
// can be used to access a specific Kubernetes object. The input string is expected to be in the format "namespace/name".
// If the input string does not contain a NamespaceSeparator (typically '/')
Expand Down
Loading