diff --git a/controllers/ratelimitpolicy_controller_test.go b/controllers/ratelimitpolicy_controller_test.go index dbf6950cd..7ae51d0e7 100644 --- a/controllers/ratelimitpolicy_controller_test.go +++ b/controllers/ratelimitpolicy_controller_test.go @@ -201,7 +201,7 @@ var _ = Describe("RateLimitPolicy controller", func() { MaxValue: 1, Seconds: 3 * 60, Namespace: rlptools.LimitsNamespaceFromRLP(rlp), - Conditions: []string{fmt.Sprintf("%s/%s/l1 == \"1\"", testNamespace, rlpName)}, + Conditions: []string{`limit.l1__2804bad6 == "1"`}, Variables: []string{}, })) @@ -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 == "1"`, Value: "1", }, }, @@ -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", }, }, @@ -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", }, }, @@ -547,7 +547,7 @@ var _ = Describe("RateLimitPolicy controller", func() { MaxValue: 1, Seconds: 3 * 60, Namespace: rlptools.LimitsNamespaceFromRLP(rlp), - Conditions: []string{fmt.Sprintf("%s/%s/l1 == \"1\"", testNamespace, rlpName)}, + Conditions: []string{`limit.l1__2804bad6 == "1"`}, Variables: []string{}, })) @@ -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 == "1"`, Value: "1", }, }, @@ -672,7 +672,7 @@ var _ = Describe("RateLimitPolicy controller", func() { MaxValue: 1, Seconds: 3 * 60, Namespace: rlptools.LimitsNamespaceFromRLP(rlp), - Conditions: []string{fmt.Sprintf("%s/%s/l1 == \"1\"", testNamespace, rlpName)}, + Conditions: []string{`limit.l1__2804bad6 == "1"`}, Variables: []string{}, })) diff --git a/pkg/rlptools/utils.go b/pkg/rlptools/utils.go index 02d310e84..ae0163617 100644 --- a/pkg/rlptools/utils.go +++ b/pkg/rlptools/utils.go @@ -1,7 +1,10 @@ package rlptools import ( + "crypto/sha256" + "encoding/hex" "fmt" + "unicode" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" @@ -9,8 +12,27 @@ import ( "github.com/kuadrant/kuadrant-operator/pkg/common" ) -func UniqueLimitName(rlp *kuadrantv1beta2.RateLimitPolicy, limitKey string) string { - return fmt.Sprintf("%s/%s/%s", rlp.GetNamespace(), rlp.GetName(), limitKey) +const ( + LimitadorRateLimitIdentitiferPrefix = "limit." +) + +func LimitNameToLimitadorIdentifier(uniqueLimitName string) string { + identifier := LimitadorRateLimitIdentitiferPrefix + + // sanitize chars that are not allowed in limitador identifiers + for _, c := range uniqueLimitName { + if unicode.IsLetter(c) || unicode.IsDigit(c) || c == '_' { + identifier += string(c) + } else { + identifier += "_" + } + } + + // to avoid breaking the uniqueness of the limit name after sanitization, we add a hash of the original name + hash := sha256.Sum256([]byte(uniqueLimitName)) + identifier += "__" + hex.EncodeToString(hash[:4]) + + return identifier } // LimitadorRateLimitsFromRLP converts rate limits from a Kuadrant RateLimitPolicy into a list of Limitador rate limit @@ -20,14 +42,14 @@ func LimitadorRateLimitsFromRLP(rlp *kuadrantv1beta2.RateLimitPolicy) []limitado rateLimits := make([]limitadorv1alpha1.RateLimit, 0) for limitKey, limit := range rlp.Spec.Limits { - uniqueLimitName := UniqueLimitName(rlp, limitKey) + limitIdentifier := LimitNameToLimitadorIdentifier(limitKey) for _, rate := range limit.Rates { maxValue, seconds := rateToSeconds(rate) rateLimits = append(rateLimits, limitadorv1alpha1.RateLimit{ Namespace: limitsNamespace, MaxValue: maxValue, Seconds: seconds, - Conditions: []string{fmt.Sprintf("%s == \"1\"", uniqueLimitName)}, + Conditions: []string{fmt.Sprintf("%s == \"1\"", limitIdentifier)}, Variables: common.GetEmptySliceIfNil(limit.CountersAsStringList()), }) } diff --git a/pkg/rlptools/utils_test.go b/pkg/rlptools/utils_test.go index d34926938..e4d659f49 100644 --- a/pkg/rlptools/utils_test.go +++ b/pkg/rlptools/utils_test.go @@ -4,6 +4,7 @@ package rlptools import ( "reflect" + "regexp" "testing" limitadorv1alpha1 "github.com/kuadrant/limitador-operator/api/v1alpha1" @@ -320,3 +321,45 @@ func TestConvertRateIntoSeconds(t *testing.T) { }) } } + +func TestLimitNameToLimitadorIdentifier(t *testing.T) { + testCases := []struct { + name string + input string + expected *regexp.Regexp + }{ + { + name: "prepends the limitador limit identifier prefix", + input: "foo", + expected: regexp.MustCompile(`^limit\.foo.+`), + }, + { + name: "sanitizes invalid chars", + input: "my/limit-0", + expected: regexp.MustCompile(`^limit\.my_limit_0.+$`), + }, + { + name: "sanitizes the dot char (.) even though it is a valid char in limitador identifiers", + input: "my.limit", + expected: regexp.MustCompile(`^limit\.my_limit.+$`), + }, + { + name: "appends a hash of the original name to avoid breaking uniqueness", + input: "foo", + expected: regexp.MustCompile(`^.+__2c26b46b$`), + }, + { + name: "empty string", + input: "", + expected: regexp.MustCompile(`^limit.__e3b0c442$`), + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(subT *testing.T) { + identifier := LimitNameToLimitadorIdentifier(tc.input) + if !tc.expected.MatchString(identifier) { + subT.Errorf("identifier does not match, expected(%s), got (%s)", tc.expected, identifier) + } + }) + } +} diff --git a/pkg/rlptools/wasm_utils.go b/pkg/rlptools/wasm_utils.go index 18c4c8c64..f231cf560 100644 --- a/pkg/rlptools/wasm_utils.go +++ b/pkg/rlptools/wasm_utils.go @@ -32,8 +32,8 @@ func WasmRules(rlp *kuadrantv1beta2.RateLimitPolicy, route *gatewayapiv1beta1.HT for limitName, limit := range rlp.Spec.Limits { // 1 RLP limit <---> 1 WASM rule - limitFullName := UniqueLimitName(rlp, limitName) - rule, err := ruleFromLimit(limitFullName, &limit, route) + limitIdentifier := LimitNameToLimitadorIdentifier(limitName) + rule, err := ruleFromLimit(limitIdentifier, &limit, route) if err == nil { rules = append(rules, rule) } @@ -42,7 +42,7 @@ func WasmRules(rlp *kuadrantv1beta2.RateLimitPolicy, route *gatewayapiv1beta1.HT return rules } -func ruleFromLimit(limitFullName string, limit *kuadrantv1beta2.Limit, route *gatewayapiv1beta1.HTTPRoute) (wasm.Rule, error) { +func ruleFromLimit(limitIdentifier string, limit *kuadrantv1beta2.Limit, route *gatewayapiv1beta1.HTTPRoute) (wasm.Rule, error) { rule := wasm.Rule{} if conditions, err := conditionsFromLimit(limit, route); err != nil { @@ -51,7 +51,7 @@ func ruleFromLimit(limitFullName string, limit *kuadrantv1beta2.Limit, route *ga rule.Conditions = conditions } - if data := dataFromLimt(limitFullName, limit); data != nil { + if data := dataFromLimt(limitIdentifier, limit); data != nil { rule.Data = data } @@ -234,13 +234,13 @@ func patternExpresionFromWhen(when kuadrantv1beta2.WhenCondition) wasm.PatternEx } } -func dataFromLimt(limitFullName string, limit *kuadrantv1beta2.Limit) (data []wasm.DataItem) { +func dataFromLimt(limitIdentifier string, limit *kuadrantv1beta2.Limit) (data []wasm.DataItem) { if limit == nil { return } // static key representing the limit - data = append(data, wasm.DataItem{Static: &wasm.StaticSpec{Key: limitFullName, Value: "1"}}) + data = append(data, wasm.DataItem{Static: &wasm.StaticSpec{Key: limitIdentifier, Value: "1"}}) for _, counter := range limit.Counters { data = append(data, wasm.DataItem{Selector: &wasm.SelectorSpec{Selector: counter}}) diff --git a/pkg/rlptools/wasm_utils_test.go b/pkg/rlptools/wasm_utils_test.go index fc6b02dd4..9f3fe5e5a 100644 --- a/pkg/rlptools/wasm_utils_test.go +++ b/pkg/rlptools/wasm_utils_test.go @@ -99,7 +99,7 @@ func TestWasmRules(t *testing.T) { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: "my-app/minimal/50rps", + Key: "limit.50rps__770adfd9", Value: "1", }, }, @@ -149,7 +149,7 @@ func TestWasmRules(t *testing.T) { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: "my-app/my-rlp/50rps-for-selected-hostnames", + Key: "limit.50rps_for_selected_hostnames__5af2c820", Value: "1", }, }, @@ -199,7 +199,7 @@ func TestWasmRules(t *testing.T) { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: "my-app/my-rlp/50rps-for-selected-route", + Key: "limit.50rps_for_selected_route__b6640119", Value: "1", }, }, @@ -248,7 +248,7 @@ func TestWasmRules(t *testing.T) { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: "my-app/my-rlp/50rps-for-selected-path", + Key: "limit.50rps_for_selected_path__4088dcf9", Value: "1", }, }, @@ -289,7 +289,7 @@ func TestWasmRules(t *testing.T) { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: "my-app/my-rlp/50rps", + Key: "limit.50rps__770adfd9", Value: "1", }, }, @@ -312,7 +312,7 @@ func TestWasmRules(t *testing.T) { Data: []wasm.DataItem{ { Static: &wasm.StaticSpec{ - Key: "my-app/my-rlp/50rps-per-username", + Key: "limit.50rps_per_username__f5bebfb8", Value: "1", }, },