From 6da47443fd79df3d14e0b25cb0df8237a7f0a563 Mon Sep 17 00:00:00 2001 From: Timothy Messier Date: Mon, 9 Oct 2023 19:39:49 +0000 Subject: [PATCH] feat(ratelimit): Create rate limits from configs --- internal/cmd/config/config.go | 2 +- internal/cmd/config/config_load_test.go | 10 +- internal/cmd/config/config_test.go | 4 +- internal/daemon/controller/testing.go | 2 + internal/ratelimit/config.go | 204 +++++++++- internal/ratelimit/config_test.go | 473 ++++++++++++++++++++++++ 6 files changed, 684 insertions(+), 11 deletions(-) create mode 100644 internal/ratelimit/config_test.go diff --git a/internal/cmd/config/config.go b/internal/cmd/config/config.go index 391f0747da..b630f992b2 100644 --- a/internal/cmd/config/config.go +++ b/internal/cmd/config/config.go @@ -693,7 +693,7 @@ func Parse(d string) (*Config, error) { } if result.Controller.ApiRateLimiterMaxEntries <= 0 { - result.Controller.ApiRateLimiterMaxEntries = ratelimit.DefaultLimiterMaxEntries + result.Controller.ApiRateLimiterMaxEntries = ratelimit.DefaultLimiterMaxEntries() } } diff --git a/internal/cmd/config/config_load_test.go b/internal/cmd/config/config_load_test.go index 614489c35b..ac76012dda 100644 --- a/internal/cmd/config/config_load_test.go +++ b/internal/cmd/config/config_load_test.go @@ -419,7 +419,7 @@ func TestLoad(t *testing.T) { WorkerStatusGracePeriodDuration: 0, LivenessTimeToStaleDuration: 0, ApiRateLimits: make(ratelimit.Configs, 0), - ApiRateLimiterMaxEntries: ratelimit.DefaultLimiterMaxEntries, + ApiRateLimiterMaxEntries: ratelimit.DefaultLimiterMaxEntries(), }, DevController: false, DevUiPassthroughDir: "", @@ -844,7 +844,7 @@ func TestLoad(t *testing.T) { WorkerStatusGracePeriodDuration: 0, LivenessTimeToStaleDuration: 0, ApiRateLimits: make(ratelimit.Configs, 0), - ApiRateLimiterMaxEntries: ratelimit.DefaultLimiterMaxEntries, + ApiRateLimiterMaxEntries: ratelimit.DefaultLimiterMaxEntries(), }, DevController: false, DevUiPassthroughDir: "", @@ -1283,7 +1283,7 @@ func TestLoad(t *testing.T) { Unlimited: false, }, }, - ApiRateLimiterMaxEntries: ratelimit.DefaultLimiterMaxEntries, + ApiRateLimiterMaxEntries: ratelimit.DefaultLimiterMaxEntries(), }, DevController: false, DevUiPassthroughDir: "", @@ -1703,7 +1703,7 @@ func TestLoad(t *testing.T) { WorkerStatusGracePeriodDuration: 0, LivenessTimeToStaleDuration: 0, ApiRateLimits: make(ratelimit.Configs, 0), - ApiRateLimiterMaxEntries: ratelimit.DefaultLimiterMaxEntries, + ApiRateLimiterMaxEntries: ratelimit.DefaultLimiterMaxEntries(), }, DevController: false, DevUiPassthroughDir: "", @@ -1794,7 +1794,7 @@ func TestLoad(t *testing.T) { WorkerStatusGracePeriodDuration: 0, LivenessTimeToStaleDuration: 0, ApiRateLimits: make(ratelimit.Configs, 0), - ApiRateLimiterMaxEntries: ratelimit.DefaultLimiterMaxEntries, + ApiRateLimiterMaxEntries: ratelimit.DefaultLimiterMaxEntries(), }, DevController: false, DevUiPassthroughDir: "", diff --git a/internal/cmd/config/config_test.go b/internal/cmd/config/config_test.go index 466cc71661..845edeb0f5 100644 --- a/internal/cmd/config/config_test.go +++ b/internal/cmd/config/config_test.go @@ -113,7 +113,7 @@ func TestDevController(t *testing.T) { Name: "dev-controller", Description: "A default controller created in dev mode", ApiRateLimits: make(ratelimit.Configs, 0), - ApiRateLimiterMaxEntries: ratelimit.DefaultLimiterMaxEntries, + ApiRateLimiterMaxEntries: ratelimit.DefaultLimiterMaxEntries(), }, DevController: true, } @@ -488,7 +488,7 @@ func TestDevCombined(t *testing.T) { Name: "dev-controller", Description: "A default controller created in dev mode", ApiRateLimits: make(ratelimit.Configs, 0), - ApiRateLimiterMaxEntries: ratelimit.DefaultLimiterMaxEntries, + ApiRateLimiterMaxEntries: ratelimit.DefaultLimiterMaxEntries(), }, DevController: true, Worker: &Worker{ diff --git a/internal/daemon/controller/testing.go b/internal/daemon/controller/testing.go index 40c29fee8d..ac3bb7d229 100644 --- a/internal/daemon/controller/testing.go +++ b/internal/daemon/controller/testing.go @@ -32,6 +32,7 @@ import ( "github.com/hashicorp/boundary/internal/host/plugin" "github.com/hashicorp/boundary/internal/iam" "github.com/hashicorp/boundary/internal/kms" + "github.com/hashicorp/boundary/internal/ratelimit" "github.com/hashicorp/boundary/internal/scheduler" "github.com/hashicorp/boundary/internal/server" "github.com/hashicorp/boundary/internal/session" @@ -656,6 +657,7 @@ func TestControllerConfig(t testing.TB, ctx context.Context, tc *TestController, } } opts.Config.Controller.Scheduler.JobRunIntervalDuration = opts.SchedulerRunJobInterval + opts.Config.Controller.ApiRateLimiterMaxEntries = ratelimit.DefaultLimiterMaxEntries() if opts.EnableEventing { opts.Config.Eventing = &event.EventerConfig{ diff --git a/internal/ratelimit/config.go b/internal/ratelimit/config.go index 97c5207082..dd8cc11625 100644 --- a/internal/ratelimit/config.go +++ b/internal/ratelimit/config.go @@ -4,15 +4,90 @@ package ratelimit import ( + "context" + "fmt" + "sync" "time" + + "github.com/hashicorp/boundary/internal/errors" + "github.com/hashicorp/boundary/internal/types/action" + "github.com/hashicorp/boundary/internal/types/resource" + "github.com/hashicorp/go-rate" + + // Imported to register all actions for all resources + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/accounts" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/authmethods" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/authtokens" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/credentiallibraries" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/credentials" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/credentialstores" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/groups" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/health" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/host_catalogs" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/host_sets" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/hosts" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/managed_groups" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/roles" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/scopes" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/session_recordings" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/sessions" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/storage_buckets" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/targets" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/users" + _ "github.com/hashicorp/boundary/internal/daemon/controller/handlers/workers" ) +// Defaults used when creating default rate.Limits. const ( - // DefaultLimiterMaxEntries is the default maximum number of quotas that - // can be tracked by the rate limiter. - DefaultLimiterMaxEntries = 16384 // TODO: pick a meaningful default value + DefaultRequestLimit = 3000 + DefaultPeriod = time.Second * 30 + DefaultListRequestLimit = 60 + DefaultListPeriod = time.Second * 30 ) +// defaultLimiterMaxEntries is the default maximum number of quotas that +// can be tracked by the rate limiter. +// This is determined at initialization time based on the number of endpoints. +var defaultLimiterMaxEntries int + +var initDefaultLimiterMaxEntries sync.Once + +// DefaultLimiterMaxEntries returns the default maximum number of quotas that +// can be tracked by the rate limiter. +func DefaultLimiterMaxEntries() int { + initDefaultLimiterMaxEntries.Do(func() { + // Calculate the default max number of quotas that the rate limiter can + // store. This is calculated based on the number of endpoints and a + // static number of quotas per endpoint. This seems like a reasonable + // way to determine a sane default value. However, it should be noted + // that this total is shared across all endpoints, and some endpoints + // will be used more frequently than others. + + const quotasPerInTotal = 1 + const quotasPerIpAddress = 1000 + const quotasPerAuthToken = 1000 + + var endpointCount int + for _, res := range resource.Map { + switch res { + case resource.Unknown, resource.All, resource.Controller: + continue + } + + actions, err := action.ActionSetForResource(res) + if err != nil { + panic(fmt.Sprintf("No actions registered for resource %q", res.String())) + } + endpointCount += len(actions) + } + + defaultLimiterMaxEntries = (endpointCount * quotasPerInTotal) + + (endpointCount * quotasPerAuthToken) + + (endpointCount * quotasPerIpAddress) + }) + return defaultLimiterMaxEntries +} + // Config is used to configure rate limits. Each config is used to specify // the maximum number of requests that can be made in a time period for the // corresponding resources and actions. @@ -28,3 +103,126 @@ type Config struct { // Configs is an ordered set of Config. type Configs []*Config + +// Limits creates a slice of rate.Limit from the Configs. This will enumerate +// every combination of resource+action, defining a Limit for each. +func (c Configs) Limits(ctx context.Context) ([]*rate.Limit, error) { + const op = "ratelimit.(Configs).Limits" + + defaults := make(map[string]*rate.Limit, len(resource.Map)*len(action.Map)) + + allResources := make([]resource.Type, 0, len(resource.Map)) + for _, res := range resource.Map { + switch res { + case resource.Unknown, resource.All, resource.Controller: + continue + } + allResources = append(allResources, res) + } + + for _, res := range allResources { + validActions, err := action.ActionSetForResource(res) + if err != nil { + // This shouldn't be possible, since we should have encountered + // this error during init and panicked already. If for some reason + // that was not the case, it seems like a good idea to panic here. + panic(fmt.Sprintf("No actions registered for resource %q", res.String())) + } + + for a := range validActions { + key := fmt.Sprintf("%s:%s:%s", res.String(), a.String(), rate.LimitPerTotal) + switch a { + case action.List: + defaults[key] = &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultListRequestLimit, + Period: DefaultListPeriod, + } + default: + defaults[key] = &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultRequestLimit, + Period: DefaultPeriod, + } + } + } + } + + for _, cc := range c { + var resourceSet []resource.Type + switch { + case len(cc.Resources) == 1 && cc.Resources[0] == resource.All.String(): + resourceSet = allResources + default: + for _, r := range cc.Resources { + rr, ok := resource.Map[r] + if !ok { + return nil, errors.New(ctx, errors.InvalidConfiguration, op, "", errors.WithMsg("unknown resource %s", r)) + } + resourceSet = append(resourceSet, rr) + } + } + + switch { + case len(cc.Actions) == 1 && cc.Actions[0] == action.All.String(): + for _, res := range resourceSet { + validActions, err := action.ActionSetForResource(res) + if err != nil { + return nil, err + } + for a := range validActions { + key := fmt.Sprintf("%s:%s:%s", res.String(), a.String(), rate.LimitPerTotal) + + defaults[key] = &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPer(cc.Per), + Unlimited: cc.Unlimited, + MaxRequests: uint64(cc.Limit), + Period: cc.Period, + } + } + } + default: + for _, res := range resourceSet { + validActions, err := action.ActionSetForResource(res) + if err != nil { + return nil, err + } + validActionMap := make(map[string]action.Type, len(validActions)) + for a := range validActions { + validActionMap[a.String()] = a + } + + for _, aStr := range cc.Actions { + a, ok := validActionMap[aStr] + if !ok { + 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.LimitPerTotal) + + defaults[key] = &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPer(cc.Per), + Unlimited: cc.Unlimited, + MaxRequests: uint64(cc.Limit), + Period: cc.Period, + } + } + } + } + } + + limits := make([]*rate.Limit, 0, len(defaults)) + for _, v := range defaults { + limits = append(limits, v) + } + return limits, nil +} diff --git a/internal/ratelimit/config_test.go b/internal/ratelimit/config_test.go new file mode 100644 index 0000000000..d5374f6bbb --- /dev/null +++ b/internal/ratelimit/config_test.go @@ -0,0 +1,473 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package ratelimit + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/hashicorp/boundary/internal/types/action" + "github.com/hashicorp/boundary/internal/types/resource" + "github.com/hashicorp/go-rate" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestConfigsLimits(t *testing.T) { + ctx := context.Background() + cases := []struct { + name string + configs Configs + want []*rate.Limit + wantErr error + }{ + { + "empty", + Configs{}, + 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: + limits = append(limits, &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultListRequestLimit, + Period: DefaultListPeriod, + }) + default: + limits = append(limits, &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultRequestLimit, + Period: DefaultPeriod, + }) + } + } + } + return limits + }(), + nil, + }, + { + "single-config-all-resources-all-actions", + Configs{ + { + Resources: []string{"*"}, + Actions: []string{"*"}, + Per: "total", + Limit: 20, + Period: time.Minute * 5, + }, + }, + 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 + default: + limits = append(limits, &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: 20, + Period: time.Minute * 5, + }) + } + } + } + return limits + }(), + nil, + }, + { + "broad-config-with-specific-override", + Configs{ + { + Resources: []string{"*"}, + Actions: []string{"*"}, + Per: "total", + Limit: 20, + Period: time.Minute * 5, + }, + { + Resources: []string{"target"}, + Actions: []string{"list"}, + Per: "total", + Limit: 10, + 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: + if res == resource.Target { + limits = append(limits, &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }) + continue + } + fallthrough + default: + limits = append(limits, &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: 20, + Period: time.Minute * 5, + }) + } + } + } + return limits + }(), + nil, + }, + { + "multiple-resources", + Configs{ + { + Resources: []string{"target", "session"}, + Actions: []string{"*"}, + Per: "total", + Limit: 10, + 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 + case resource.Target, resource.Session: + validActions, err := action.ActionSetForResource(res) + require.NoError(t, err) + for a := range validActions { + switch a { + case action.Unknown, action.All: + continue + default: + limits = append(limits, &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }) + } + } + default: + validActions, err := action.ActionSetForResource(res) + require.NoError(t, err) + for a := range validActions { + switch a { + case action.Unknown, action.All: + continue + case action.List: + limits = append(limits, &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultListRequestLimit, + Period: DefaultListPeriod, + }) + default: + limits = append(limits, &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultRequestLimit, + Period: DefaultPeriod, + }) + } + } + } + } + return limits + }(), + nil, + }, + { + "multiple-actions", + Configs{ + { + Resources: []string{"target", "session"}, + Actions: []string{"list", "read"}, + Per: "total", + Limit: 10, + 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 + case resource.Target, resource.Session: + 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.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: 10, + Period: time.Minute, + }) + default: + limits = append(limits, &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultRequestLimit, + Period: DefaultPeriod, + }) + } + } + default: + validActions, err := action.ActionSetForResource(res) + require.NoError(t, err) + for a := range validActions { + switch a { + case action.Unknown, action.All: + continue + case action.List: + limits = append(limits, &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultListRequestLimit, + Period: DefaultListPeriod, + }) + default: + limits = append(limits, &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: DefaultRequestLimit, + Period: DefaultPeriod, + }) + } + } + } + } + return limits + }(), + nil, + }, + { + "order-matters", + Configs{ + // This one is overridden by the second config that is more broad. + { + Resources: []string{"target", "session"}, + Actions: []string{"*"}, + Per: "total", + Limit: 10, + Period: time.Minute, + }, + { + Resources: []string{"*"}, + Actions: []string{"*"}, + Per: "total", + Limit: 20, + Period: time.Minute * 5, + }, + }, + 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 + default: + limits = append(limits, &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: false, + MaxRequests: 20, + Period: time.Minute * 5, + }) + } + } + } + return limits + }(), + nil, + }, + { + "no-limits", + Configs{ + { + Resources: []string{"*"}, + Actions: []string{"*"}, + Per: "total", + Unlimited: true, + }, + }, + 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 + default: + limits = append(limits, &rate.Limit{ + Resource: res.String(), + Action: a.String(), + Per: rate.LimitPerTotal, + Unlimited: true, + }) + } + } + } + return limits + }(), + nil, + }, + { + "invalid-resource", + Configs{ + { + Resources: []string{"foo"}, + Actions: []string{"*"}, + Per: "total", + Limit: 10, + Period: time.Minute, + }, + }, + nil, + fmt.Errorf("ratelimit.(Configs).Limits: unknown resource foo: configuration issue: error #5000"), + }, + { + "invalid-action-for-resource", + Configs{ + { + Resources: []string{"session"}, + Actions: []string{"authorize-session"}, + Per: "total", + Limit: 10, + Period: time.Minute, + }, + }, + nil, + fmt.Errorf("ratelimit.(Configs).Limits: action authorize-session not valid for resource session: configuration issue: error #5000"), + }, + { + "invalid-action", + Configs{ + { + Resources: []string{"session"}, + Actions: []string{"foo"}, + Per: "total", + Limit: 10, + Period: time.Minute, + }, + }, + nil, + fmt.Errorf("ratelimit.(Configs).Limits: action foo not valid for resource session: configuration issue: error #5000"), + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := tc.configs.Limits(ctx) + if tc.wantErr != nil { + require.EqualError(t, err, tc.wantErr.Error()) + return + } + require.NoError(t, err) + assert.ElementsMatch(t, tc.want, got) + }) + } +} + +func TestDefaulLimiterMaxEntries(t *testing.T) { + var want int + + var endpointCount int + for _, res := range resource.Map { + switch res { + case resource.Unknown, resource.All, resource.Controller: + continue + } + + actions, err := action.ActionSetForResource(res) + require.NoError(t, err) + endpointCount += len(actions) + } + want = endpointCount*2000 + endpointCount + + got := DefaultLimiterMaxEntries() + assert.Equal(t, want, got) +}