Exclude gRPC health check requests from rate limiting in history & ma…

…tching service (#6036)

## What changed?
The rate limit interceptor will now ignore gRPC health check requests
and doesn't apply any rate limiters.

## Why?
Health check requests should never be rate limited.

## How did you test it?
Added unit tests to assert interceptor behavior.

## Potential risks
N/A

## Documentation
N/A

## Is hotfix candidate?
no

---------

Co-authored-by: Tim Deeb-Swihart <409226+tdeebswihart@users.noreply.github.com>
Co-authored-by: David Reiss <david@temporal.io>

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{

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": {},
 }
 

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,

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)
+}

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.
+		},
 	)
 }
 

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.
+		},
 	)
 }
 

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.
+		},
 	)
 }