diff --git a/common/authorization/default_authorizer_test.go b/common/authorization/default_authorizer_test.go index 4973c1db48e..ea9f11e57a5 100644 --- a/common/authorization/default_authorizer_test.go +++ b/common/authorization/default_authorizer_test.go @@ -32,6 +32,8 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + healthpb "google.golang.org/grpc/health/grpc_health_v1" + "go.temporal.io/server/common/config" ) @@ -79,7 +81,7 @@ var ( Namespace: testNamespace, } targetGrpcHealthCheck = CallTarget{ - APIName: "/grpc.health.v1.Health/Check", + APIName: healthpb.Health_Check_FullMethodName, Namespace: "", } targetGetSystemInfo = CallTarget{ diff --git a/common/authorization/frontend_api.go b/common/authorization/frontend_api.go index 73b05128098..a132a5e8139 100644 --- a/common/authorization/frontend_api.go +++ b/common/authorization/frontend_api.go @@ -24,10 +24,14 @@ package authorization -import "go.temporal.io/server/common/api" +import ( + healthpb "google.golang.org/grpc/health/grpc_health_v1" + + "go.temporal.io/server/common/api" +) var healthCheckAPI = map[string]struct{}{ - "/grpc.health.v1.Health/Check": {}, + healthpb.Health_Check_FullMethodName: {}, "/temporal.api.workflowservice.v1.WorkflowService/GetSystemInfo": {}, } diff --git a/common/rpc/interceptor/rate_limit.go b/common/rpc/interceptor/rate_limit.go index 7cac9ac1e81..b9585538e04 100644 --- a/common/rpc/interceptor/rate_limit.go +++ b/common/rpc/interceptor/rate_limit.go @@ -91,6 +91,11 @@ func (i *RateLimitInterceptor) Allow( token = RateLimitDefaultToken } + // we don't want to apply rate limiter if a method is configured with 0 tokens. + if token < 1 { + return nil + } + if !i.rateLimiter.Allow(time.Now().UTC(), quotas.NewRequest( methodName, token, diff --git a/common/rpc/interceptor/rate_limit_test.go b/common/rpc/interceptor/rate_limit_test.go new file mode 100644 index 00000000000..ec864e39941 --- /dev/null +++ b/common/rpc/interceptor/rate_limit_test.go @@ -0,0 +1,105 @@ +// The MIT License +// +// Copyright (c) 2020 Temporal Technologies Inc. All rights reserved. +// +// Copyright (c) 2020 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package interceptor + +import ( + "context" + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + "google.golang.org/grpc" + + "go.temporal.io/server/common/quotas" +) + +type ( + // rateLimitInterceptorSuite struct { + rateLimitInterceptorSuite struct { + suite.Suite + *require.Assertions + + controller *gomock.Controller + mockRateLimiter *quotas.MockRequestRateLimiter + } +) + +func TestRateLimitInterceptorSuite(t *testing.T) { + suite.Run(t, &rateLimitInterceptorSuite{}) +} + +func (s *rateLimitInterceptorSuite) SetupTest() { + s.Assertions = require.New(s.T()) + s.controller = gomock.NewController(s.T()) + s.mockRateLimiter = quotas.NewMockRequestRateLimiter(s.controller) +} + +func (s *rateLimitInterceptorSuite) TestInterceptWithTokenConfig() { + methodName := "TEST/METHOD" + interceptor := NewRateLimitInterceptor(s.mockRateLimiter, map[string]int{methodName: 0}) + // mock rate limiter should not be called. + s.mockRateLimiter.EXPECT().Allow(gomock.Any(), gomock.Any()).MaxTimes(0).Return(false) + + handlerCalled := false + handler := func(ctx context.Context, req any) (any, error) { + handlerCalled = true + return nil, nil + } + _, err := interceptor.Intercept(context.Background(), nil, &grpc.UnaryServerInfo{FullMethod: methodName}, handler) + s.NoError(err) + s.True(handlerCalled) +} + +func (s *rateLimitInterceptorSuite) TestInterceptWithNoTokenConfig() { + interceptor := NewRateLimitInterceptor(s.mockRateLimiter, nil) + // mock rate limiter is set to blocking. + s.mockRateLimiter.EXPECT().Allow(gomock.Any(), gomock.Any()).MaxTimes(1).Return(false) + + handlerCalled := false + handler := func(ctx context.Context, req any) (any, error) { + handlerCalled = true + return nil, nil + } + _, err := interceptor.Intercept(context.Background(), nil, &grpc.UnaryServerInfo{}, handler) + s.Error(err) + s.False(handlerCalled) +} + +func (s *rateLimitInterceptorSuite) TestInterceptWithNonZeroTokenConfig() { + methodName := "TEST/METHOD" + interceptor := NewRateLimitInterceptor(s.mockRateLimiter, map[string]int{methodName: 100}) + // mock rate limiter is set to non-blocking. + s.mockRateLimiter.EXPECT().Allow(gomock.Any(), gomock.Any()).MaxTimes(1).Return(true) + + handlerCalled := false + handler := func(ctx context.Context, req any) (any, error) { + handlerCalled = true + return nil, nil + } + _, err := interceptor.Intercept(context.Background(), nil, &grpc.UnaryServerInfo{FullMethod: methodName}, handler) + s.NoError(err) + s.True(handlerCalled) +} diff --git a/service/frontend/fx.go b/service/frontend/fx.go index a45b420f5a4..9524cf6ed14 100644 --- a/service/frontend/fx.go +++ b/service/frontend/fx.go @@ -71,6 +71,7 @@ import ( "go.temporal.io/server/service/frontend/configs" "go.temporal.io/server/service/history/tasks" "go.temporal.io/server/service/worker/scheduler" + healthpb "google.golang.org/grpc/health/grpc_health_v1" ) type ( @@ -395,7 +396,9 @@ func RateLimitInterceptorProvider( quotas.NewDefaultIncomingRateBurst(namespaceReplicationInducingRateFn), serviceConfig.OperatorRPSRatio, ), - map[string]int{}, + map[string]int{ + healthpb.Health_Check_FullMethodName: 0, // exclude health check requests from rate limiting. + }, ) } diff --git a/service/history/fx.go b/service/history/fx.go index 46be9b50417..af14352a6f5 100644 --- a/service/history/fx.go +++ b/service/history/fx.go @@ -61,6 +61,7 @@ import ( "go.temporal.io/server/service/history/shard" "go.temporal.io/server/service/history/workflow" "go.temporal.io/server/service/history/workflow/cache" + healthpb "google.golang.org/grpc/health/grpc_health_v1" "go.temporal.io/server/components/callbacks" "go.temporal.io/server/components/nexusoperations" @@ -211,7 +212,9 @@ func RateLimitInterceptorProvider( ) *interceptor.RateLimitInterceptor { return interceptor.NewRateLimitInterceptor( configs.NewPriorityRateLimiter(func() float64 { return float64(serviceConfig.RPS()) }, serviceConfig.OperatorRPSRatio), - map[string]int{}, + map[string]int{ + healthpb.Health_Check_FullMethodName: 0, // exclude health check requests from rate limiting. + }, ) } diff --git a/service/matching/fx.go b/service/matching/fx.go index 071e0b42305..dfcda610664 100644 --- a/service/matching/fx.go +++ b/service/matching/fx.go @@ -45,6 +45,7 @@ import ( "go.temporal.io/server/common/searchattribute" "go.temporal.io/server/service" "go.temporal.io/server/service/matching/configs" + healthpb "google.golang.org/grpc/health/grpc_health_v1" ) var Module = fx.Options( @@ -101,7 +102,9 @@ func RateLimitInterceptorProvider( ) *interceptor.RateLimitInterceptor { return interceptor.NewRateLimitInterceptor( configs.NewPriorityRateLimiter(func() float64 { return float64(serviceConfig.RPS()) }, serviceConfig.OperatorRPSRatio), - map[string]int{}, + map[string]int{ + healthpb.Health_Check_FullMethodName: 0, // exclude health check requests from rate limiting. + }, ) }