diff --git a/src/limiter/base_limiter.go b/src/limiter/base_limiter.go index 6aec1ac8a..e6d170776 100644 --- a/src/limiter/base_limiter.go +++ b/src/limiter/base_limiter.go @@ -131,11 +131,8 @@ func (this *BaseRateLimiter) GetResponseDescriptorStatus(key string, limitInfo * } } - // If the limit is in ShadowMode, it should be always return OK if isOverLimit && limitInfo.limit.ShadowMode { logger.Debugf("Limit with key %s, is in shadow_mode", limitInfo.limit.FullKey) - responseDescriptorStatus.Code = pb.RateLimitResponse_OK - // Increase shadow mode stats if the limit was actually over the limit this.increaseShadowModeStats(isOverLimitWithLocalCache, limitInfo, hitsAddend) } diff --git a/src/service/ratelimit.go b/src/service/ratelimit.go index 559b49533..af38d19a6 100644 --- a/src/service/ratelimit.go +++ b/src/service/ratelimit.go @@ -51,9 +51,10 @@ type service struct { customHeaderRemainingHeader string customHeaderResetHeader string customHeaderClock utils.TimeSource - globalShadowMode bool - globalQuotaMode bool - responseDynamicMetadataEnabled bool + globalShadowMode bool + globalQuotaMode bool + responseDynamicMetadataEnabled bool + shadowModeExceededHeaderEnabled bool } func (this *service) SetConfig(updateEvent provider.ConfigUpdateEvent, healthyWithAtLeastOneConfigLoad bool) { @@ -90,6 +91,7 @@ func (this *service) SetConfig(updateEvent provider.ConfigUpdateEvent, healthyWi this.globalShadowMode = rlSettings.GlobalShadowMode this.globalQuotaMode = rlSettings.GlobalQuotaMode this.responseDynamicMetadataEnabled = rlSettings.ResponseDynamicMetadata + this.shadowModeExceededHeaderEnabled = rlSettings.ShadowModeExceededHeaderEnabled if rlSettings.RateLimitResponseHeadersEnabled { this.customHeadersEnabled = true @@ -207,6 +209,7 @@ func (this *service) shouldRateLimitWorker( // Track quota mode violations for metadata var quotaModeViolations []int + shadowModeExceeded := false for i, descriptorStatus := range responseDescriptorStatuses { // Keep track of the descriptor closest to hit the ratelimit @@ -225,10 +228,18 @@ func (this *service) shouldRateLimitWorker( } else { response.Statuses[i] = descriptorStatus if descriptorStatus.Code == pb.RateLimitResponse_OVER_LIMIT { + isShadowMode := limitsToCheck[i] != nil && limitsToCheck[i].ShadowMode // Check if this limit is in quota mode (individual or global) isQuotaMode := globalQuotaMode || (limitsToCheck[i] != nil && limitsToCheck[i].QuotaMode) - if isQuotaMode { + if isShadowMode { + shadowModeExceeded = true + response.Statuses[i] = &pb.RateLimitResponse_DescriptorStatus{ + Code: pb.RateLimitResponse_OK, + CurrentLimit: descriptorStatus.CurrentLimit, + LimitRemaining: descriptorStatus.LimitRemaining, + } + } else if isQuotaMode { // In quota mode: track the violation for metadata but keep response as OK quotaModeViolations = append(quotaModeViolations, i) response.Statuses[i] = &pb.RateLimitResponse_DescriptorStatus{ @@ -258,9 +269,19 @@ func (this *service) shouldRateLimitWorker( // If there is a global shadow_mode, it should always return OK if finalCode == pb.RateLimitResponse_OVER_LIMIT && globalShadowMode { finalCode = pb.RateLimitResponse_OK + shadowModeExceeded = true this.stats.GlobalShadowMode.Inc() } + if this.shadowModeExceededHeaderEnabled { + response.RequestHeadersToAdd = []*core.HeaderValue{ + { + Key: "x-ratelimit-exceeded-shadow-mode", + Value: strconv.FormatBool(shadowModeExceeded), + }, + } + } + // If response dynamic data enabled, set dynamic data on response. if this.responseDynamicMetadataEnabled { response.DynamicMetadata = ratelimitToMetadata(request, quotaModeViolations, limitsToCheck) diff --git a/src/settings/settings.go b/src/settings/settings.go index 7ad2d80df..392ce6e01 100644 --- a/src/settings/settings.go +++ b/src/settings/settings.go @@ -208,7 +208,8 @@ type Settings struct { MemcacheTlsSkipHostnameVerification bool `envconfig:"MEMCACHE_TLS_SKIP_HOSTNAME_VERIFICATION" default:"false"` // Should the ratelimiting be running in Global shadow-mode, ie. never report a ratelimit status, unless a rate was provided from envoy as an override - GlobalShadowMode bool `envconfig:"SHADOW_MODE" default:"false"` + GlobalShadowMode bool `envconfig:"SHADOW_MODE" default:"false"` + ShadowModeExceededHeaderEnabled bool `envconfig:"SHADOW_MODE_EXCEEDED_HEADER_ENABLED" default:"false"` // Should the ratelimiting be running in Global quota-mode, ie. set metadata but never report OVER_LIMIT status when quota limits are exceeded GlobalQuotaMode bool `envconfig:"QUOTA_MODE" default:"false"` diff --git a/test/limiter/base_limiter_test.go b/test/limiter/base_limiter_test.go index 7bc404079..1563422f7 100644 --- a/test/limiter/base_limiter_test.go +++ b/test/limiter/base_limiter_test.go @@ -210,8 +210,8 @@ func TestGetResponseStatusOverLimitWithLocalCacheShadowMode(t *testing.T) { limitInfo := limiter.NewRateLimitInfo(limits[0], 2, 6, 4, 5) // As `isOverLimitWithLocalCache` is passed as `true`, immediate response is returned with no checks of the limits. responseStatus := baseRateLimit.GetResponseDescriptorStatus("key", limitInfo, true, 2) - // Limit is reached, but response is still OK due to ShadowMode - assert.Equal(pb.RateLimitResponse_OK, responseStatus.GetCode()) + // Limit is reached, shadow mode code conversion now happens in service layer + assert.Equal(pb.RateLimitResponse_OVER_LIMIT, responseStatus.GetCode()) assert.Equal(uint32(0), responseStatus.GetLimitRemaining()) assert.Equal(limits[0].Limit, responseStatus.GetCurrentLimit()) assert.Equal(uint64(2), limits[0].Stats.OverLimit.Value()) @@ -259,7 +259,8 @@ func TestGetResponseStatusOverLimitShadowMode(t *testing.T) { limits := []*config.RateLimit{config.NewRateLimit(5, pb.RateLimitResponse_RateLimit_SECOND, sm.NewStats("key_value"), false, true, false, "", nil, false)} limitInfo := limiter.NewRateLimitInfo(limits[0], 2, 7, 4, 5) responseStatus := baseRateLimit.GetResponseDescriptorStatus("key", limitInfo, false, 1) - assert.Equal(pb.RateLimitResponse_OK, responseStatus.GetCode()) + // Shadow mode code conversion now happens in service layer + assert.Equal(pb.RateLimitResponse_OVER_LIMIT, responseStatus.GetCode()) assert.Equal(uint32(0), responseStatus.GetLimitRemaining()) assert.Equal(limits[0].Limit, responseStatus.GetCurrentLimit()) result, _ := localCache.Get([]byte("key")) diff --git a/test/redis/fixed_cache_impl_test.go b/test/redis/fixed_cache_impl_test.go index e52abb68d..c84b12e86 100644 --- a/test/redis/fixed_cache_impl_test.go +++ b/test/redis/fixed_cache_impl_test.go @@ -564,10 +564,10 @@ func TestOverLimitWithLocalCacheShadowRule(t *testing.T) { "EXPIRE", "domain_key4_value4_997200", int64(3600)).DoAndReturn(pipeAppend) client.EXPECT().PipeDo(gomock.Any()).Return(nil) - // The result should be OK since limit is in ShadowMode + // Shadow mode code conversion now happens in service layer, cache returns OVER_LIMIT assert.Equal( []*pb.RateLimitResponse_DescriptorStatus{ - {Code: pb.RateLimitResponse_OK, CurrentLimit: limits[0].Limit, LimitRemaining: 0, DurationUntilReset: utils.CalculateReset(&limits[0].Limit.Unit, timeSource)}, + {Code: pb.RateLimitResponse_OVER_LIMIT, CurrentLimit: limits[0].Limit, LimitRemaining: 0, DurationUntilReset: utils.CalculateReset(&limits[0].Limit.Unit, timeSource)}, }, cache.DoLimit(context.Background(), request, limits)) assert.Equal(uint64(3), limits[0].Stats.TotalHits.Value()) @@ -586,10 +586,10 @@ func TestOverLimitWithLocalCacheShadowRule(t *testing.T) { client.EXPECT().PipeAppend(gomock.Any(), gomock.Any(), "EXPIRE", "domain_key4_value4_997200", int64(3600)).Times(0) - // The result should be OK since limit is in ShadowMode + // Shadow mode code conversion now happens in service layer, cache returns OVER_LIMIT assert.Equal( []*pb.RateLimitResponse_DescriptorStatus{ - {Code: pb.RateLimitResponse_OK, CurrentLimit: limits[0].Limit, LimitRemaining: 0, DurationUntilReset: utils.CalculateReset(&limits[0].Limit.Unit, timeSource)}, + {Code: pb.RateLimitResponse_OVER_LIMIT, CurrentLimit: limits[0].Limit, LimitRemaining: 0, DurationUntilReset: utils.CalculateReset(&limits[0].Limit.Unit, timeSource)}, }, cache.DoLimit(context.Background(), request, limits)) diff --git a/test/service/ratelimit_test.go b/test/service/ratelimit_test.go index f22309348..9c0d44e6e 100644 --- a/test/service/ratelimit_test.go +++ b/test/service/ratelimit_test.go @@ -288,8 +288,8 @@ func TestRuleShadowMode(test *testing.T) { t.config.EXPECT().GetLimit(context.Background(), "different-domain", request.Descriptors[1]).Return(limits[1]) t.cache.EXPECT().DoLimit(context.Background(), request, limits).Return( []*pb.RateLimitResponse_DescriptorStatus{ - {Code: pb.RateLimitResponse_OK, CurrentLimit: limits[0].Limit, LimitRemaining: 0}, - {Code: pb.RateLimitResponse_OK, CurrentLimit: nil, LimitRemaining: 0}, + {Code: pb.RateLimitResponse_OVER_LIMIT, CurrentLimit: limits[0].Limit, LimitRemaining: 0}, + {Code: pb.RateLimitResponse_OVER_LIMIT, CurrentLimit: limits[1].Limit, LimitRemaining: 0}, }) response, err := service.ShouldRateLimit(context.Background(), request) t.assert.Equal( @@ -297,7 +297,7 @@ func TestRuleShadowMode(test *testing.T) { OverallCode: pb.RateLimitResponse_OK, Statuses: []*pb.RateLimitResponse_DescriptorStatus{ {Code: pb.RateLimitResponse_OK, CurrentLimit: limits[0].Limit, LimitRemaining: 0}, - {Code: pb.RateLimitResponse_OK, CurrentLimit: nil, LimitRemaining: 0}, + {Code: pb.RateLimitResponse_OK, CurrentLimit: limits[1].Limit, LimitRemaining: 0}, }, }, response) @@ -319,16 +319,10 @@ func TestMixedRuleShadowMode(test *testing.T) { } t.config.EXPECT().GetLimit(context.Background(), "different-domain", request.Descriptors[0]).Return(limits[0]) t.config.EXPECT().GetLimit(context.Background(), "different-domain", request.Descriptors[1]).Return(limits[1]) - testResults := []pb.RateLimitResponse_Code{pb.RateLimitResponse_OVER_LIMIT, pb.RateLimitResponse_OVER_LIMIT} - for i := 0; i < len(limits); i++ { - if limits[i].ShadowMode { - testResults[i] = pb.RateLimitResponse_OK - } - } t.cache.EXPECT().DoLimit(context.Background(), request, limits).Return( []*pb.RateLimitResponse_DescriptorStatus{ - {Code: testResults[0], CurrentLimit: limits[0].Limit, LimitRemaining: 0}, - {Code: testResults[1], CurrentLimit: nil, LimitRemaining: 0}, + {Code: pb.RateLimitResponse_OVER_LIMIT, CurrentLimit: limits[0].Limit, LimitRemaining: 0}, + {Code: pb.RateLimitResponse_OVER_LIMIT, CurrentLimit: nil, LimitRemaining: 0}, }) response, err := service.ShouldRateLimit(context.Background(), request) t.assert.Equal( @@ -345,6 +339,115 @@ func TestMixedRuleShadowMode(test *testing.T) { t.assert.EqualValues(0, t.statStore.NewCounter("global_shadow_mode").Value()) } +func TestShadowModeExceededHeader(test *testing.T) { + os.Setenv("SHADOW_MODE_EXCEEDED_HEADER_ENABLED", "true") + defer os.Unsetenv("SHADOW_MODE_EXCEEDED_HEADER_ENABLED") + + t := commonSetup(test) + defer t.controller.Finish() + service := t.setupBasicService() + + // Test 1: Shadow mode descriptor exceeded - header should be true + request := common.NewRateLimitRequest( + "different-domain", [][][2]string{{{"foo", "bar"}}}, 1) + limits := []*config.RateLimit{ + config.NewRateLimit(10, pb.RateLimitResponse_RateLimit_MINUTE, t.statsManager.NewStats("key"), false, true, false, "", nil, false), + } + t.config.EXPECT().GetLimit(context.Background(), "different-domain", request.Descriptors[0]).Return(limits[0]) + t.cache.EXPECT().DoLimit(context.Background(), request, limits).Return( + []*pb.RateLimitResponse_DescriptorStatus{ + {Code: pb.RateLimitResponse_OVER_LIMIT, CurrentLimit: limits[0].Limit, LimitRemaining: 0}, + }) + response, err := service.ShouldRateLimit(context.Background(), request) + t.assert.Nil(err) + t.assert.Equal(pb.RateLimitResponse_OK, response.OverallCode) + t.assert.Equal(pb.RateLimitResponse_OK, response.Statuses[0].Code) + t.assert.Equal(1, len(response.RequestHeadersToAdd)) + t.assert.Equal("x-ratelimit-exceeded-shadow-mode", response.RequestHeadersToAdd[0].Key) + t.assert.Equal("true", response.RequestHeadersToAdd[0].Value) +} + +func TestShadowModeExceededHeaderNotExceeded(test *testing.T) { + os.Setenv("SHADOW_MODE_EXCEEDED_HEADER_ENABLED", "true") + defer os.Unsetenv("SHADOW_MODE_EXCEEDED_HEADER_ENABLED") + + t := commonSetup(test) + defer t.controller.Finish() + service := t.setupBasicService() + + // Test: Shadow mode descriptor NOT exceeded - header should be false + request := common.NewRateLimitRequest( + "different-domain", [][][2]string{{{"foo", "bar"}}}, 1) + limits := []*config.RateLimit{ + config.NewRateLimit(10, pb.RateLimitResponse_RateLimit_MINUTE, t.statsManager.NewStats("key"), false, true, false, "", nil, false), + } + t.config.EXPECT().GetLimit(context.Background(), "different-domain", request.Descriptors[0]).Return(limits[0]) + t.cache.EXPECT().DoLimit(context.Background(), request, limits).Return( + []*pb.RateLimitResponse_DescriptorStatus{ + {Code: pb.RateLimitResponse_OK, CurrentLimit: limits[0].Limit, LimitRemaining: 5}, + }) + response, err := service.ShouldRateLimit(context.Background(), request) + t.assert.Nil(err) + t.assert.Equal(pb.RateLimitResponse_OK, response.OverallCode) + t.assert.Equal(1, len(response.RequestHeadersToAdd)) + t.assert.Equal("x-ratelimit-exceeded-shadow-mode", response.RequestHeadersToAdd[0].Key) + t.assert.Equal("false", response.RequestHeadersToAdd[0].Value) +} + +func TestShadowModeExceededHeaderDisabled(test *testing.T) { + // Feature flag not set, so no header should be added + t := commonSetup(test) + defer t.controller.Finish() + service := t.setupBasicService() + + request := common.NewRateLimitRequest( + "different-domain", [][][2]string{{{"foo", "bar"}}}, 1) + limits := []*config.RateLimit{ + config.NewRateLimit(10, pb.RateLimitResponse_RateLimit_MINUTE, t.statsManager.NewStats("key"), false, true, false, "", nil, false), + } + t.config.EXPECT().GetLimit(context.Background(), "different-domain", request.Descriptors[0]).Return(limits[0]) + t.cache.EXPECT().DoLimit(context.Background(), request, limits).Return( + []*pb.RateLimitResponse_DescriptorStatus{ + {Code: pb.RateLimitResponse_OVER_LIMIT, CurrentLimit: limits[0].Limit, LimitRemaining: 0}, + }) + response, err := service.ShouldRateLimit(context.Background(), request) + t.assert.Nil(err) + t.assert.Equal(pb.RateLimitResponse_OK, response.OverallCode) + t.assert.Nil(response.RequestHeadersToAdd) +} + +func TestShadowModeExceededHeaderGlobalShadowMode(test *testing.T) { + os.Setenv("SHADOW_MODE", "true") + os.Setenv("SHADOW_MODE_EXCEEDED_HEADER_ENABLED", "true") + defer func() { + os.Unsetenv("SHADOW_MODE") + os.Unsetenv("SHADOW_MODE_EXCEEDED_HEADER_ENABLED") + }() + + t := commonSetup(test) + defer t.controller.Finish() + service := t.setupBasicService() + + // Non-shadow-mode descriptor that is over limit, but global shadow mode converts it + request := common.NewRateLimitRequest( + "different-domain", [][][2]string{{{"foo", "bar"}}}, 1) + limits := []*config.RateLimit{ + config.NewRateLimit(10, pb.RateLimitResponse_RateLimit_MINUTE, t.statsManager.NewStats("key"), false, false, false, "", nil, false), + } + t.config.EXPECT().GetLimit(context.Background(), "different-domain", request.Descriptors[0]).Return(limits[0]) + t.cache.EXPECT().DoLimit(context.Background(), request, limits).Return( + []*pb.RateLimitResponse_DescriptorStatus{ + {Code: pb.RateLimitResponse_OVER_LIMIT, CurrentLimit: limits[0].Limit, LimitRemaining: 0}, + }) + response, err := service.ShouldRateLimit(context.Background(), request) + t.assert.Nil(err) + t.assert.Equal(pb.RateLimitResponse_OK, response.OverallCode) + t.assert.Equal(1, len(response.RequestHeadersToAdd)) + t.assert.Equal("x-ratelimit-exceeded-shadow-mode", response.RequestHeadersToAdd[0].Key) + t.assert.Equal("true", response.RequestHeadersToAdd[0].Value) + t.assert.EqualValues(1, t.statStore.NewCounter("global_shadow_mode").Value()) +} + func TestServiceWithCustomRatelimitHeaders(test *testing.T) { os.Setenv("LIMIT_RESPONSE_HEADERS_ENABLED", "true") os.Setenv("LIMIT_LIMIT_HEADER", "A-Ratelimit-Limit")