fix(controller): Allow wildcard resources with specific actions in rate limit config (#6550)
* Wildcard API rate limit added * changes updated * reviwed changes added
This commit is contained in:
1 parent
f735676657
commit
c1283c2875
3 files changed
+156
-1
No files matched your search
@@ -137,6 +137,25 @@ func TestController_initializeRateLimiter(t *testing.T) {
|
||||
false,
|
||||
nil,
|
||||
},
|
||||
{
|
||||
"wildcardReadList",
|
||||
&config.Config{
|
||||
Controller: &config.Controller{
|
||||
ApiRateLimits: ratelimit.Configs{
|
||||
{
|
||||
Resources: []string{"*"},
|
||||
Actions: []string{"read", "list"},
|
||||
Per: "total",
|
||||
Limit: 100,
|
||||
Period: time.Minute,
|
||||
},
|
||||
},
|
||||
ApiRateLimiterMaxQuotas: ratelimit.DefaultLimiterMaxQuotas(),
|
||||
},
|
||||
},
|
||||
false,
|
||||
nil,
|
||||
},
|
||||
{
|
||||
"invalid",
|
||||
&config.Config{
|
||||
|
||||
@@ -196,8 +196,9 @@ func (c Configs) Limits(ctx context.Context) ([]rate.Limit, error) {
|
||||
|
||||
for _, cc := range c {
|
||||
var resourceSet []resource.Type
|
||||
wildcardResources := len(cc.Resources) == 1 && cc.Resources[0] == resource.All.String()
|
||||
switch {
|
||||
case len(cc.Resources) == 1 && cc.Resources[0] == resource.All.String():
|
||||
case wildcardResources:
|
||||
resourceSet = allResources
|
||||
default:
|
||||
for _, r := range cc.Resources {
|
||||
@@ -251,6 +252,18 @@ func (c Configs) Limits(ctx context.Context) ([]rate.Limit, error) {
|
||||
for _, aStr := range cc.Actions {
|
||||
a, ok := validActionMap[aStr]
|
||||
if !ok {
|
||||
if wildcardResources {
|
||||
// For wildcard resources, verify the action exists in the action system
|
||||
// before skipping. If it doesn't exist at all, return an error.
|
||||
if _, exists := action.Map[aStr]; !exists {
|
||||
return nil, errors.New(ctx, errors.InvalidConfiguration, op, "", errors.WithMsg("action %s not valid for resource %s", aStr, resource.All.String()))
|
||||
}
|
||||
// When using wildcard resources (*), skip actions that aren't valid for
|
||||
// the current resource type rather than returning an error. This allows
|
||||
// specifying rate limits like "read,list" for all resources, even though
|
||||
// not all resources support both actions (e.g., sessions don't support "read").
|
||||
continue
|
||||
}
|
||||
return nil, errors.New(ctx, errors.InvalidConfiguration, op, "", errors.WithMsg("action %s not valid for resource %s", aStr, res.String()))
|
||||
}
|
||||
key := fmt.Sprintf("%s:%s:%s", res.String(), a.String(), rate.LimitPer(cc.Per))
|
||||
|
||||
@@ -572,6 +572,101 @@ func TestConfigsLimits(t *testing.T) {
|
||||
}(),
|
||||
nil,
|
||||
},
|
||||
{
|
||||
"all-resources-multiple-actions",
|
||||
Configs{
|
||||
{
|
||||
Resources: []string{"*"},
|
||||
Actions: []string{"list", "read"},
|
||||
Per: "total",
|
||||
Limit: 10,
|
||||
Period: time.Minute,
|
||||
},
|
||||
{
|
||||
Resources: []string{"*"},
|
||||
Actions: []string{"list", "read"},
|
||||
Per: "ip-address",
|
||||
Limit: 5,
|
||||
Period: time.Minute,
|
||||
},
|
||||
{
|
||||
Resources: []string{"*"},
|
||||
Actions: []string{"list", "read"},
|
||||
Per: "auth-token",
|
||||
Limit: 1,
|
||||
Period: time.Minute,
|
||||
},
|
||||
},
|
||||
func() []rate.Limit {
|
||||
limits := make([]rate.Limit, 0, len(resource.Map)*len(action.Map))
|
||||
for _, res := range resource.Map {
|
||||
switch res {
|
||||
case resource.Unknown, resource.All, resource.Controller:
|
||||
continue
|
||||
}
|
||||
validActions, err := action.ActionSetForResource(res)
|
||||
require.NoError(t, err)
|
||||
for a := range validActions {
|
||||
switch a {
|
||||
case action.Unknown, action.All:
|
||||
continue
|
||||
case action.List, action.Read:
|
||||
limits = append(
|
||||
limits,
|
||||
&rate.Limited{
|
||||
Resource: res.String(),
|
||||
Action: a.String(),
|
||||
Per: rate.LimitPerTotal,
|
||||
MaxRequests: 10,
|
||||
Period: time.Minute,
|
||||
},
|
||||
&rate.Limited{
|
||||
Resource: res.String(),
|
||||
Action: a.String(),
|
||||
Per: rate.LimitPerIPAddress,
|
||||
MaxRequests: 5,
|
||||
Period: time.Minute,
|
||||
},
|
||||
&rate.Limited{
|
||||
Resource: res.String(),
|
||||
Action: a.String(),
|
||||
Per: rate.LimitPerAuthToken,
|
||||
MaxRequests: 1,
|
||||
Period: time.Minute,
|
||||
},
|
||||
)
|
||||
default:
|
||||
limits = append(
|
||||
limits,
|
||||
&rate.Limited{
|
||||
Resource: res.String(),
|
||||
Action: a.String(),
|
||||
Per: rate.LimitPerTotal,
|
||||
MaxRequests: DefaultInTotalRequestLimit,
|
||||
Period: DefaultPeriod,
|
||||
},
|
||||
&rate.Limited{
|
||||
Resource: res.String(),
|
||||
Action: a.String(),
|
||||
Per: rate.LimitPerIPAddress,
|
||||
MaxRequests: DefaultIpAddressRequestLimit,
|
||||
Period: DefaultPeriod,
|
||||
},
|
||||
&rate.Limited{
|
||||
Resource: res.String(),
|
||||
Action: a.String(),
|
||||
Per: rate.LimitPerAuthToken,
|
||||
MaxRequests: DefaultAuthTokenRequestLimit,
|
||||
Period: DefaultPeriod,
|
||||
},
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
return limits
|
||||
}(),
|
||||
nil,
|
||||
},
|
||||
{
|
||||
"order-matters",
|
||||
Configs{
|
||||
@@ -764,6 +859,34 @@ func TestConfigsLimits(t *testing.T) {
|
||||
nil,
|
||||
fmt.Errorf("ratelimit.(Configs).Limits: action foo not valid for resource session: configuration issue: error #5000"),
|
||||
},
|
||||
{
|
||||
"invalid-action-with-wildcard-resource",
|
||||
Configs{
|
||||
{
|
||||
Resources: []string{"*"},
|
||||
Actions: []string{"foo"},
|
||||
Per: "total",
|
||||
Limit: 10,
|
||||
Period: time.Minute,
|
||||
},
|
||||
},
|
||||
nil,
|
||||
fmt.Errorf("ratelimit.(Configs).Limits: action foo not valid for resource *: configuration issue: error #5000"),
|
||||
},
|
||||
{
|
||||
"typo-in-action-with-wildcard-resource",
|
||||
Configs{
|
||||
{
|
||||
Resources: []string{"*"},
|
||||
Actions: []string{"reaad"}, // typo: should be "read"
|
||||
Per: "total",
|
||||
Limit: 10,
|
||||
Period: time.Minute,
|
||||
},
|
||||
},
|
||||
nil,
|
||||
fmt.Errorf("ratelimit.(Configs).Limits: action reaad not valid for resource *: configuration issue: error #5000"),
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
|
||||
Reference in new issue
Block a user