From c1283c2875d3165f49e670dc81c778729dfe307e Mon Sep 17 00:00:00 2001 From: Abhishek Manjegowda Date: Fri, 17 Apr 2026 12:01:13 +0530 Subject: [PATCH] fix(controller): Allow wildcard resources with specific actions in rate limit config (#6550) * Wildcard API rate limit added * changes updated * reviwed changes added --- .../daemon/controller/rate_limiter_test.go | 19 +++ internal/ratelimit/config.go | 15 ++- internal/ratelimit/config_test.go | 123 ++++++++++++++++++ 3 files changed, 156 insertions(+), 1 deletion(-) diff --git a/internal/daemon/controller/rate_limiter_test.go b/internal/daemon/controller/rate_limiter_test.go index f6511ba87..54cc974f8 100644 --- a/internal/daemon/controller/rate_limiter_test.go +++ b/internal/daemon/controller/rate_limiter_test.go @@ -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{ diff --git a/internal/ratelimit/config.go b/internal/ratelimit/config.go index b39a1aee6..5643fea39 100644 --- a/internal/ratelimit/config.go +++ b/internal/ratelimit/config.go @@ -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)) diff --git a/internal/ratelimit/config_test.go b/internal/ratelimit/config_test.go index 501c47324..b710a63a2 100644 --- a/internal/ratelimit/config_test.go +++ b/internal/ratelimit/config_test.go @@ -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 {