Skip to content

Commit

Permalink
Merge pull request #1145 from Kuadrant/issue-1144
Browse files Browse the repository at this point in the history
`RateLimit.Conditions` need explicit scope
  • Loading branch information
alexsnaps authored Jan 30, 2025
2 parents d963513 + 42d065f commit b9fb42b
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 25 deletions.
2 changes: 1 addition & 1 deletion controllers/limitador_limits_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (r *LimitadorLimitsReconciler) buildLimitadorLimits(ctx context.Context, st
Namespace: limitsNamespace,
MaxValue: maxValue,
Seconds: seconds,
Conditions: []string{fmt.Sprintf("%s == \"1\"", limitIdentifier)},
Conditions: []string{fmt.Sprintf("descriptors[0][\"%s\"] == \"1\"", limitIdentifier)},
Variables: utils.GetEmptySliceIfNil(limit.CountersAsStringList()),
}
})
Expand Down
20 changes: 10 additions & 10 deletions pkg/ratelimit/index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,71 +140,71 @@ func TestIndexToRateLimits(t *testing.T) {

func TestEqualsTo(t *testing.T) {
global_l0 := limitadorv1alpha1.RateLimit{
Conditions: []string{"limit._global___3f2bfd8b == \"1\""},
Conditions: []string{"descriptors[0][\"limit._global___3f2bfd8b\"] == \"1\""},
MaxValue: 3,
Namespace: "default/test-3-gw0-l0",
Seconds: 10,
Variables: []string{},
}
global_l1 := limitadorv1alpha1.RateLimit{
Conditions: []string{"limit._global___3f2bfd8b == \"1\""},
Conditions: []string{"descriptors[0][\"limit._global___3f2bfd8b\"] == \"1\""},
MaxValue: 3,
Namespace: "default/test-3-gw0-l1",
Seconds: 10,
Variables: []string{},
}
global_l2 := limitadorv1alpha1.RateLimit{
Conditions: []string{"limit._global___3f2bfd8b == \"1\""},
Conditions: []string{"descriptors[0][\"limit._global___3f2bfd8b\"] == \"1\""},
MaxValue: 3,
Namespace: "default/test-3-gw0-l2",
Seconds: 10,
Variables: []string{},
}
global_l3 := limitadorv1alpha1.RateLimit{
Conditions: []string{"limit._global___3f2bfd8b == \"1\""},
Conditions: []string{"descriptors[0][\"limit._global___3f2bfd8b\"] == \"1\""},
MaxValue: 3,
Namespace: "default/test-3-gw0-l3",
Seconds: 10,
Variables: []string{},
}
global_l4 := limitadorv1alpha1.RateLimit{
Conditions: []string{"limit._global___3f2bfd8b == \"1\""},
Conditions: []string{"descriptors[0][\"limit._global___3f2bfd8b\"] == \"1\""},
MaxValue: 3,
Namespace: "default/test-3-gw0-l4",
Seconds: 10,
Variables: []string{},
}
global_l5 := limitadorv1alpha1.RateLimit{
Conditions: []string{"limit._global___3f2bfd8b == \"1\""},
Conditions: []string{"descriptors[0][\"limit._global___3f2bfd8b\"] == \"1\""},
MaxValue: 3,
Namespace: "default/test-3-gw0-l5",
Seconds: 10,
Variables: []string{},
}
global_l6 := limitadorv1alpha1.RateLimit{
Conditions: []string{"limit._global___3f2bfd8b == \"1\""},
Conditions: []string{"descriptors[0][\"limit._global___3f2bfd8b\"] == \"1\""},
MaxValue: 3,
Namespace: "default/test-3-gw0-l6",
Seconds: 10,
Variables: []string{},
}

httproute_l0 := limitadorv1alpha1.RateLimit{
Conditions: []string{"limit._httproute_level__ac417cac == \"1\""},
Conditions: []string{"descriptors[0][\"limit._httproute_level__ac417cac\"] == \"1\""},
MaxValue: 3,
Namespace: "default/test-3-gw0-l0",
Seconds: 10,
Variables: []string{},
}
httproute_l1 := limitadorv1alpha1.RateLimit{
Conditions: []string{"limit._httproute_level__e4abd750 == \"1\""},
Conditions: []string{"descriptors[0][\"limit._httproute_level__e4abd750\"] == \"1\""},
MaxValue: 3,
Namespace: "default/test-3-gw0-l1",
Seconds: 10,
Variables: []string{},
}
httproute_l5 := limitadorv1alpha1.RateLimit{
Conditions: []string{"limit._httproute_level__e1d71177 == \"1\""},
Conditions: []string{"descriptors[0][\"limit._httproute_level__e1d71177\"] == \"1\""},
MaxValue: 3,
Namespace: "default/test-3-gw0-l5",
Seconds: 10,
Expand Down
28 changes: 14 additions & 14 deletions tests/common/ratelimitpolicy/ratelimitpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
MaxValue: 1,
Seconds: 3 * 60,
Namespace: controllers.LimitsNamespaceFromRoute(httpRoute),
Conditions: []string{fmt.Sprintf(`%s == "1"`, controllers.LimitNameToLimitadorIdentifier(rlpKey, "l1"))},
Conditions: []string{fmt.Sprintf(`descriptors[0]["%s"] == "1"`, controllers.LimitNameToLimitadorIdentifier(rlpKey, "l1"))},
Variables: []string{},
}))
}).WithContext(ctx).Should(Succeed())
Expand Down Expand Up @@ -339,7 +339,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
MaxValue: 1,
Seconds: 3 * 60,
Namespace: controllers.LimitsNamespaceFromRoute(httpRoute),
Conditions: []string{fmt.Sprintf(`%s == "1"`, controllers.LimitNameToLimitadorIdentifier(rlpKey, "l1"))},
Conditions: []string{fmt.Sprintf(`descriptors[0]["%s"] == "1"`, controllers.LimitNameToLimitadorIdentifier(rlpKey, "l1"))},
Variables: []string{},
}))
}).WithContext(ctx).Should(Succeed())
Expand Down Expand Up @@ -421,7 +421,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
MaxValue: 10,
Seconds: 5,
Namespace: controllers.LimitsNamespaceFromRoute(httpRoute),
Conditions: []string{fmt.Sprintf(`%s == "1"`, controllers.LimitNameToLimitadorIdentifier(routeRLPKey, "l1"))},
Conditions: []string{fmt.Sprintf(`descriptors[0]["%s"] == "1"`, controllers.LimitNameToLimitadorIdentifier(routeRLPKey, "l1"))},
Variables: []string{},
})).WithContext(ctx).Should(Succeed())
})
Expand Down Expand Up @@ -520,7 +520,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
MaxValue: 1,
Seconds: 180,
Namespace: limitsNamespace,
Conditions: []string{fmt.Sprintf(`%s == "1"`, controllers.LimitNameToLimitadorIdentifier(gwRLPKey, "l1"))},
Conditions: []string{fmt.Sprintf(`descriptors[0]["%s"] == "1"`, controllers.LimitNameToLimitadorIdentifier(gwRLPKey, "l1"))},
Variables: []string{},
})).WithContext(ctx).Should(Succeed())

Expand All @@ -532,7 +532,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
MaxValue: 10,
Seconds: 5,
Namespace: limitsNamespace,
Conditions: []string{fmt.Sprintf(`%s == "1"`, controllers.LimitNameToLimitadorIdentifier(routeRLPKey, "route"))},
Conditions: []string{fmt.Sprintf(`descriptors[0]["%s"] == "1"`, controllers.LimitNameToLimitadorIdentifier(routeRLPKey, "route"))},
Variables: []string{},
})).WithContext(ctx).Should(Succeed())
}, testTimeOut)
Expand All @@ -557,7 +557,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
MaxValue: 1,
Seconds: 180,
Namespace: controllers.LimitsNamespaceFromRoute(httpRoute),
Conditions: []string{fmt.Sprintf(`%s == "1"`, controllers.LimitNameToLimitadorIdentifier(gwRLPKey, "l1"))},
Conditions: []string{fmt.Sprintf(`descriptors[0]["%s"] == "1"`, controllers.LimitNameToLimitadorIdentifier(gwRLPKey, "l1"))},
Variables: []string{},
})).WithContext(ctx).Should(Succeed())
}, testTimeOut)
Expand Down Expand Up @@ -588,7 +588,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
MaxValue: 10,
Seconds: 5,
Namespace: limitsNamespace,
Conditions: []string{fmt.Sprintf(`%s == "1"`, controllers.LimitNameToLimitadorIdentifier(routeRLPKey, "route"))},
Conditions: []string{fmt.Sprintf(`descriptors[0]["%s"] == "1"`, controllers.LimitNameToLimitadorIdentifier(routeRLPKey, "route"))},
Variables: []string{},
})).WithContext(ctx).Should(Succeed())

Expand All @@ -610,7 +610,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
MaxValue: 1,
Seconds: 180,
Namespace: limitsNamespace,
Conditions: []string{fmt.Sprintf(`%s == "1"`, controllers.LimitNameToLimitadorIdentifier(gwRLPKey, "l1"))},
Conditions: []string{fmt.Sprintf(`descriptors[0]["%s"] == "1"`, controllers.LimitNameToLimitadorIdentifier(gwRLPKey, "l1"))},
Variables: []string{},
})).WithContext(ctx).Should(Succeed())
}, testTimeOut)
Expand All @@ -637,7 +637,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
MaxValue: 1,
Seconds: 180,
Namespace: limitsNamespace,
Conditions: []string{fmt.Sprintf(`%s == "1"`, controllers.LimitNameToLimitadorIdentifier(gwRLPKey, "l1"))},
Conditions: []string{fmt.Sprintf(`descriptors[0]["%s"] == "1"`, controllers.LimitNameToLimitadorIdentifier(gwRLPKey, "l1"))},
Variables: []string{},
})).WithContext(ctx).Should(Succeed())

Expand All @@ -659,7 +659,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
MaxValue: 10,
Seconds: 5,
Namespace: limitsNamespace,
Conditions: []string{fmt.Sprintf(`%s == "1"`, controllers.LimitNameToLimitadorIdentifier(routeRLPKey, "route"))},
Conditions: []string{fmt.Sprintf(`descriptors[0]["%s"] == "1"`, controllers.LimitNameToLimitadorIdentifier(routeRLPKey, "route"))},
Variables: []string{},
})).WithContext(ctx).Should(Succeed())
}, testTimeOut)
Expand Down Expand Up @@ -850,28 +850,28 @@ var _ = Describe("RateLimitPolicy controller", func() {
MaxValue: 1000,
Seconds: 1,
Namespace: controllers.LimitsNamespaceFromRoute(targetedRoute),
Conditions: []string{fmt.Sprintf(`%s == "1"`, limitIdentifierGwA)},
Conditions: []string{fmt.Sprintf(`descriptors[0]["%s"] == "1"`, limitIdentifierGwA)},
Variables: []string{},
},
limitadorv1alpha1.RateLimit{
MaxValue: 100,
Seconds: 1,
Namespace: controllers.LimitsNamespaceFromRoute(targetedRoute),
Conditions: []string{fmt.Sprintf(`%s == "1"`, limitIdentifierGwB)},
Conditions: []string{fmt.Sprintf(`descriptors[0]["%s"] == "1"`, limitIdentifierGwB)},
Variables: []string{},
},
limitadorv1alpha1.RateLimit{ // FIXME(@guicassolato): we need to create one limit definition per gateway × route combination, not one per gateway × policy combination
MaxValue: 1000,
Seconds: 1,
Namespace: controllers.LimitsNamespaceFromRoute(untargetedRoute),
Conditions: []string{fmt.Sprintf(`%s == "1"`, limitIdentifierGwA)},
Conditions: []string{fmt.Sprintf(`descriptors[0]["%s"] == "1"`, limitIdentifierGwA)},
Variables: []string{},
},
limitadorv1alpha1.RateLimit{
MaxValue: 100,
Seconds: 1,
Namespace: controllers.LimitsNamespaceFromRoute(untargetedRoute),
Conditions: []string{fmt.Sprintf(`%s == "1"`, limitIdentifierGwB)},
Conditions: []string{fmt.Sprintf(`descriptors[0]["%s"] == "1"`, limitIdentifierGwB)},
Variables: []string{},
},
)).WithContext(ctx).Should(Succeed())
Expand Down

0 comments on commit b9fb42b

Please sign in to comment.