diff --git a/src/client_cmd/main.go b/src/client_cmd/main.go index a3750f99..63e87b8f 100644 --- a/src/client_cmd/main.go +++ b/src/client_cmd/main.go @@ -13,6 +13,7 @@ import ( "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/credentials" + "google.golang.org/grpc/credentials/insecure" "github.com/envoyproxy/ratelimit/src/utils" @@ -95,7 +96,7 @@ func main() { tlsConfig := utils.TlsConfigFromFiles(*grpcTlsCertFile, *grpcTlsKeyFile, *grpcServerTlsCACert, utils.ServerCA, false) dialOptions = append(dialOptions, grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig))) } else { - dialOptions = append(dialOptions, grpc.WithInsecure()) + dialOptions = append(dialOptions, grpc.WithTransportCredentials(insecure.NewCredentials())) } conn, err := grpc.Dial(*dialString, dialOptions...) if err != nil { @@ -106,9 +107,8 @@ func main() { defer conn.Close() c := pb.NewRateLimitServiceClient(conn) desc := make([]*pb_struct.RateLimitDescriptor, len(descriptorsValue.descriptors)) - for i, v := range descriptorsValue.descriptors { - desc[i] = v - } + copy(desc, descriptorsValue.descriptors) + response, err := c.ShouldRateLimit( context.Background(), &pb.RateLimitRequest{ diff --git a/src/config/config_impl.go b/src/config/config_impl.go index 1a537951..2bdbf9d4 100644 --- a/src/config/config_impl.go +++ b/src/config/config_impl.go @@ -151,7 +151,7 @@ func (this *rateLimitDescriptor) loadDescriptors(config RateLimitConfigToLoad, p if validUnit { panic(newRateLimitConfigError( config.Name, - fmt.Sprintf("should not specify rate limit unit when unlimited"))) + "should not specify rate limit unit when unlimited")) } } else if !validUnit { panic(newRateLimitConfigError( @@ -234,7 +234,7 @@ func validateYamlKeys(fileName string, config_map map[interface{}]interface{}) { // the yaml's keys we don't panic here. case nil: default: - errorText := fmt.Sprintf("error checking config") + errorText := "error checking config" logger.Debugf(errorText) panic(newRateLimitConfigError(fileName, errorText)) } diff --git a/src/config_check_cmd/main.go b/src/config_check_cmd/main.go index dc313c31..8f83a724 100644 --- a/src/config_check_cmd/main.go +++ b/src/config_check_cmd/main.go @@ -3,7 +3,6 @@ package main import ( "flag" "fmt" - "io/ioutil" "os" "path/filepath" @@ -36,7 +35,7 @@ func main() { fmt.Printf("checking rate limit configs...\n") fmt.Printf("loading config directory: %s\n", *configDirectory) - files, err := ioutil.ReadDir(*configDirectory) + files, err := os.ReadDir(*configDirectory) if err != nil { fmt.Printf("error opening directory %s: %s\n", *configDirectory, err.Error()) os.Exit(1) @@ -46,7 +45,7 @@ func main() { for _, file := range files { finalPath := filepath.Join(*configDirectory, file.Name()) fmt.Printf("opening config file: %s\n", finalPath) - bytes, err := ioutil.ReadFile(finalPath) + bytes, err := os.ReadFile(finalPath) if err != nil { fmt.Printf("error reading file %s: %s\n", finalPath, err.Error()) os.Exit(1) diff --git a/src/limiter/base_limiter.go b/src/limiter/base_limiter.go index d8a6b7cb..b76366cf 100644 --- a/src/limiter/base_limiter.go +++ b/src/limiter/base_limiter.go @@ -73,10 +73,7 @@ func (this *BaseRateLimiter) IsOverLimitWithLocalCache(key string) bool { func (this *BaseRateLimiter) IsOverLimitThresholdReached(limitInfo *LimitInfo) bool { limitInfo.overLimitThreshold = limitInfo.limit.Limit.RequestsPerUnit - if limitInfo.limitAfterIncrease > limitInfo.overLimitThreshold { - return true - } - return false + return limitInfo.limitAfterIncrease > limitInfo.overLimitThreshold } // Generates response descriptor status based on cache key, over the limit with local cache, over the limit and diff --git a/src/redis/cache_impl.go b/src/redis/cache_impl.go index 9ad9731e..0b0a45b4 100644 --- a/src/redis/cache_impl.go +++ b/src/redis/cache_impl.go @@ -18,8 +18,8 @@ func NewRateLimiterCacheImplFromSettings(s settings.Settings, localCache *freeca perSecondPool = NewClientImpl(srv.Scope().Scope("redis_per_second_pool"), s.RedisPerSecondTls, s.RedisPerSecondAuth, s.RedisPerSecondSocketType, s.RedisPerSecondType, s.RedisPerSecondUrl, s.RedisPerSecondPoolSize, s.RedisPerSecondPipelineWindow, s.RedisPerSecondPipelineLimit, s.RedisTlsConfig, s.RedisHealthCheckActiveConnection, srv) } - var otherPool Client - otherPool = NewClientImpl(srv.Scope().Scope("redis_pool"), s.RedisTls, s.RedisAuth, s.RedisSocketType, s.RedisType, s.RedisUrl, s.RedisPoolSize, + + otherPool := NewClientImpl(srv.Scope().Scope("redis_pool"), s.RedisTls, s.RedisAuth, s.RedisSocketType, s.RedisType, s.RedisUrl, s.RedisPoolSize, s.RedisPipelineWindow, s.RedisPipelineLimit, s.RedisTlsConfig, s.RedisHealthCheckActiveConnection, srv) return NewFixedRateLimitCacheImpl( diff --git a/src/redis/driver_impl.go b/src/redis/driver_impl.go index 436458ca..f68addda 100644 --- a/src/redis/driver_impl.go +++ b/src/redis/driver_impl.go @@ -119,7 +119,7 @@ func NewClientImpl(scope stats.Scope, useTls bool, auth, redisSocketType, redisT client, err = poolFunc(redisSocketType, url) case "cluster": urls := strings.Split(url, ",") - if implicitPipelining == false { + if !implicitPipelining { panic(RedisError("Implicit Pipelining must be enabled to work with Redis Cluster Mode. Set values for REDIS_PIPELINE_WINDOW or REDIS_PIPELINE_LIMIT to enable implicit pipelining")) } logger.Warnf("Creating cluster with urls %v", urls) diff --git a/src/server/health.go b/src/server/health.go index 9af7d2dc..7a2f9cb7 100644 --- a/src/server/health.go +++ b/src/server/health.go @@ -33,7 +33,7 @@ const ( func areAllComponentsHealthy(healthMap map[string]bool) bool { allComponentsHealthy := true for _, value := range healthMap { - if value == false { + if !value { allComponentsHealthy = false break } diff --git a/src/server/server_impl.go b/src/server/server_impl.go index dbadbe1e..85c636b7 100644 --- a/src/server/server_impl.go +++ b/src/server/server_impl.go @@ -1,7 +1,6 @@ package server import ( - "bytes" "context" "expvar" "fmt" @@ -18,13 +17,13 @@ import ( "google.golang.org/grpc/credentials" "google.golang.org/grpc/keepalive" + "google.golang.org/protobuf/encoding/protojson" "github.com/envoyproxy/ratelimit/src/provider" "github.com/envoyproxy/ratelimit/src/stats" "github.com/coocood/freecache" pb "github.com/envoyproxy/go-control-plane/envoy/service/ratelimit/v3" - "github.com/golang/protobuf/jsonpb" "github.com/gorilla/mux" reuseport "github.com/kavu/go_reuseport" "github.com/lyft/goruntime/loader" @@ -79,24 +78,28 @@ func (server *server) AddDebugHttpEndpoint(path string, help string, handler htt // example usage from cURL with domain "dummy" and descriptor "perday": // echo '{"domain": "dummy", "descriptors": [{"entries": [{"key": "perday"}]}]}' | curl -vvvXPOST --data @/dev/stdin localhost:8080/json func NewJsonHandler(svc pb.RateLimitServiceServer) func(http.ResponseWriter, *http.Request) { - // Default options include enums as strings and no identation. - m := &jsonpb.Marshaler{} - return func(writer http.ResponseWriter, request *http.Request) { var req pb.RateLimitRequest ctx := context.Background() - if err := jsonpb.Unmarshal(request.Body, &req); err != nil { + body, err := io.ReadAll(request.Body) + if err != nil { + logger.Warnf("error: %s", err.Error()) + writeHttpStatus(writer, http.StatusBadRequest) + return + } + + if err := protojson.Unmarshal(body, &req); err != nil { logger.Warnf("error: %s", err.Error()) - http.Error(writer, err.Error(), http.StatusBadRequest) + writeHttpStatus(writer, http.StatusBadRequest) return } resp, err := svc.ShouldRateLimit(ctx, &req) if err != nil { logger.Warnf("error: %s", err.Error()) - http.Error(writer, err.Error(), http.StatusBadRequest) + writeHttpStatus(writer, http.StatusBadRequest) return } @@ -109,12 +112,16 @@ func NewJsonHandler(svc pb.RateLimitServiceServer) func(http.ResponseWriter, *ht defer span.End() logger.Debugf("resp:%s", resp) + if resp == nil { + logger.Error("nil response") + writeHttpStatus(writer, http.StatusInternalServerError) + return + } - buf := bytes.NewBuffer(nil) - err = m.Marshal(buf, resp) + jsonResp, err := protojson.Marshal(resp) if err != nil { logger.Errorf("error marshaling proto3 to json: %s", err.Error()) - http.Error(writer, "error marshaling proto3 to json: "+err.Error(), http.StatusInternalServerError) + writeHttpStatus(writer, http.StatusInternalServerError) return } @@ -124,10 +131,14 @@ func NewJsonHandler(svc pb.RateLimitServiceServer) func(http.ResponseWriter, *ht } else if resp.OverallCode == pb.RateLimitResponse_OVER_LIMIT { writer.WriteHeader(http.StatusTooManyRequests) } - writer.Write(buf.Bytes()) + writer.Write(jsonResp) } } +func writeHttpStatus(writer http.ResponseWriter, code int) { + http.Error(writer, http.StatusText(code), code) +} + func getProviderImpl(s settings.Settings, statsManager stats.Manager, rootStore gostats.Store) provider.RateLimitConfigProvider { switch s.ConfigType { case "FILE": diff --git a/src/stats/manager.go b/src/stats/manager.go index cab7dc33..ba83fd51 100644 --- a/src/stats/manager.go +++ b/src/stats/manager.go @@ -2,7 +2,6 @@ package stats import ( gostats "github.com/lyft/gostats" - stats "github.com/lyft/gostats" ) // Manager is the interface that wraps initialization of stat structures. @@ -17,7 +16,7 @@ type Manager interface { // Multiple calls to this method are idempotent. NewServiceStats() ServiceStats // Returns the stats.Store wrapped by the Manager. - GetStatsStore() stats.Store + GetStatsStore() gostats.Store } type ManagerImpl struct { diff --git a/test/common/common.go b/test/common/common.go index 0fec1ae1..1062a899 100644 --- a/test/common/common.go +++ b/test/common/common.go @@ -12,8 +12,8 @@ import ( "testing" "time" - "github.com/golang/protobuf/proto" "github.com/stretchr/testify/assert" + "google.golang.org/protobuf/proto" pb_struct "github.com/envoyproxy/go-control-plane/envoy/extensions/common/ratelimit/v3" pb "github.com/envoyproxy/go-control-plane/envoy/service/ratelimit/v3" @@ -111,7 +111,7 @@ func startCacheProcess(ctx context.Context, command string, args []string, port if err1 != nil || err2 != nil { cancel() - return nil, fmt.Errorf("Problem starting %s subprocess: %v / %v", command, err1, err2) + return nil, fmt.Errorf("problem starting %s subprocess: %v / %v", command, err1, err2) } // You'd think cmd.Stdout = os.Stdout would make more sense here, but @@ -127,13 +127,13 @@ func startCacheProcess(ctx context.Context, command string, args []string, port err := cmd.Start() if err != nil { cancel() - return nil, fmt.Errorf("Problem starting %s subprocess: %v", command, err) + return nil, fmt.Errorf("problem starting %s subprocess: %v", command, err) } err = WaitForTcpPort(ctx, port, 1*time.Second) if err != nil { cancel() - return nil, fmt.Errorf("Timed out waiting for %s to start up and accept connections: %v", command, err) + return nil, fmt.Errorf("timed out waiting for %s to start up and accept connections: %v", command, err) } return func() { diff --git a/test/config/config_test.go b/test/config/config_test.go index ed131110..3ed3984b 100644 --- a/test/config/config_test.go +++ b/test/config/config_test.go @@ -1,7 +1,8 @@ package config_test import ( - "io/ioutil" + "context" + "os" "testing" "github.com/envoyproxy/ratelimit/test/common" @@ -17,7 +18,7 @@ import ( ) func loadFile(path string) []config.RateLimitConfigToLoad { - contents, err := ioutil.ReadFile(path) + contents, err := os.ReadFile(path) if err != nil { panic(err) } @@ -31,39 +32,39 @@ func TestBasicConfig(t *testing.T) { rlConfig := config.NewRateLimitConfigImpl(loadFile("basic_config.yaml"), mockstats.NewMockStatManager(stats), false) rlConfig.Dump() assert.Equal(rlConfig.IsEmptyDomains(), false) - assert.Nil(rlConfig.GetLimit(nil, "foo_domain", &pb_struct.RateLimitDescriptor{})) - assert.Nil(rlConfig.GetLimit(nil, "test-domain", &pb_struct.RateLimitDescriptor{})) + assert.Nil(rlConfig.GetLimit(context.TODO(), "foo_domain", &pb_struct.RateLimitDescriptor{})) + assert.Nil(rlConfig.GetLimit(context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{})) rl := rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key1", Value: "something"}}, }) assert.Nil(rl) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key1", Value: "value1"}}, }) assert.Nil(rl) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key2", Value: "value2"}, {Key: "subkey", Value: "subvalue"}}, }) assert.Nil(rl) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key5", Value: "value5"}, {Key: "subkey5", Value: "subvalue"}}, }) assert.Nil(rl) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key1", Value: "value1"}, {Key: "subkey1", Value: "something"}}, }) @@ -79,7 +80,7 @@ func TestBasicConfig(t *testing.T) { assert.EqualValues(1, stats.NewCounter("test-domain.key1_value1.subkey1.within_limit").Value()) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key1", Value: "value1"}, {Key: "subkey1", Value: "subvalue1"}}, }) @@ -99,7 +100,7 @@ func TestBasicConfig(t *testing.T) { 1, stats.NewCounter("test-domain.key1_value1.subkey1_subvalue1.within_limit").Value()) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key2", Value: "something"}}, }) @@ -115,7 +116,7 @@ func TestBasicConfig(t *testing.T) { assert.EqualValues(1, stats.NewCounter("test-domain.key2.within_limit").Value()) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key2", Value: "value2"}}, }) @@ -131,14 +132,14 @@ func TestBasicConfig(t *testing.T) { assert.EqualValues(1, stats.NewCounter("test-domain.key2_value2.within_limit").Value()) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key2", Value: "value3"}}, }) assert.Nil(rl) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key3", Value: "foo"}}, }) @@ -154,7 +155,7 @@ func TestBasicConfig(t *testing.T) { assert.EqualValues(1, stats.NewCounter("test-domain.key3.within_limit").Value()) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key4", Value: "foo"}}, }) @@ -170,7 +171,7 @@ func TestBasicConfig(t *testing.T) { assert.EqualValues(1, stats.NewCounter("test-domain.key4.within_limit").Value()) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key6", Value: "foo"}}, }) @@ -183,7 +184,7 @@ func TestBasicConfig(t *testing.T) { // A value for the key with detailed_metric: true // should also generate a stat with the value included rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key7", Value: "unspecified_value"}}, }) @@ -201,7 +202,7 @@ func TestBasicConfig(t *testing.T) { // Another value for the key with detailed_metric: true // should also generate a stat with the value included rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key7", Value: "another_value"}}, }) @@ -226,11 +227,11 @@ func TestDomainMerge(t *testing.T) { rlConfig := config.NewRateLimitConfigImpl(files, mockstats.NewMockStatManager(stats), true) rlConfig.Dump() - assert.Nil(rlConfig.GetLimit(nil, "foo_domain", &pb_struct.RateLimitDescriptor{})) - assert.Nil(rlConfig.GetLimit(nil, "test-domain", &pb_struct.RateLimitDescriptor{})) + assert.Nil(rlConfig.GetLimit(context.TODO(), "foo_domain", &pb_struct.RateLimitDescriptor{})) + assert.Nil(rlConfig.GetLimit(context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{})) rl := rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key1", Value: "value1"}}, }) @@ -238,7 +239,7 @@ func TestDomainMerge(t *testing.T) { assert.EqualValues(10, rl.Limit.RequestsPerUnit) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key2", Value: "value2"}}, }) @@ -252,13 +253,13 @@ func TestConfigLimitOverride(t *testing.T) { rlConfig := config.NewRateLimitConfigImpl(loadFile("basic_config.yaml"), mockstats.NewMockStatManager(stats), false) rlConfig.Dump() // No matching domain - assert.Nil(rlConfig.GetLimit(nil, "foo_domain", &pb_struct.RateLimitDescriptor{ + assert.Nil(rlConfig.GetLimit(context.TODO(), "foo_domain", &pb_struct.RateLimitDescriptor{ Limit: &pb_struct.RateLimitDescriptor_RateLimitOverride{ RequestsPerUnit: 10, Unit: pb_type.RateLimitUnit_DAY, }, })) rl := rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key1", Value: "value1"}, {Key: "subkey1", Value: "something"}}, Limit: &pb_struct.RateLimitDescriptor_RateLimitOverride{ @@ -281,7 +282,7 @@ func TestConfigLimitOverride(t *testing.T) { // Change in override value doesn't erase stats rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key1", Value: "value1"}, {Key: "subkey1", Value: "something"}}, Limit: &pb_struct.RateLimitDescriptor_RateLimitOverride{ @@ -304,7 +305,7 @@ func TestConfigLimitOverride(t *testing.T) { // Different value creates a different counter rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key1", Value: "value1"}, {Key: "subkey1", Value: "something_else"}}, Limit: &pb_struct.RateLimitDescriptor_RateLimitOverride{ @@ -498,7 +499,7 @@ func TestShadowModeConfig(t *testing.T) { rlConfig.Dump() rl := rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key1", Value: "value1"}, {Key: "subkey1", Value: "something"}}, }) @@ -515,7 +516,7 @@ func TestShadowModeConfig(t *testing.T) { assert.EqualValues(0, stats.NewCounter("test-domain.key1_value1.subkey1.shadow_mode").Value()) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key1", Value: "value1"}, {Key: "subkey1", Value: "subvalue1"}}, }) @@ -533,7 +534,7 @@ func TestShadowModeConfig(t *testing.T) { assert.EqualValues(1, stats.NewCounter("test-domain.key1_value1.subkey1_subvalue1.shadow_mode").Value()) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key2", Value: "something"}}, }) @@ -550,7 +551,7 @@ func TestShadowModeConfig(t *testing.T) { assert.EqualValues(1, stats.NewCounter("test-domain.key2.shadow_mode").Value()) rl = rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "key2", Value: "value2"}}, }) @@ -574,12 +575,12 @@ func TestWildcardConfig(t *testing.T) { // Baseline to show wildcard works like no value withoutVal1 := rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "noVal", Value: "foo1"}}, }) withoutVal2 := rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "noVal", Value: "foo2"}}, }) @@ -588,17 +589,17 @@ func TestWildcardConfig(t *testing.T) { // Matches multiple wildcard values and results are equal wildcard1 := rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "wild", Value: "foo1"}}, }) wildcard2 := rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "wild", Value: "foo2"}}, }) wildcard3 := rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "nestedWild", Value: "val1"}, {Key: "wild", Value: "goo2"}}, }) @@ -608,7 +609,7 @@ func TestWildcardConfig(t *testing.T) { // Doesn't match non-matching values noMatch := rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "wild", Value: "bar"}}, }) @@ -616,7 +617,7 @@ func TestWildcardConfig(t *testing.T) { // Non-wildcard values don't eager match eager := rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "noWild", Value: "foo1"}}, }) @@ -624,7 +625,7 @@ func TestWildcardConfig(t *testing.T) { // Wildcard in the middle of value is not supported. midWildcard := rlConfig.GetLimit( - nil, "test-domain", + context.TODO(), "test-domain", &pb_struct.RateLimitDescriptor{ Entries: []*pb_struct.RateLimitDescriptor_Entry{{Key: "midWildcard", Value: "barab"}}, }) diff --git a/test/memcached/stats_collecting_client_test.go b/test/memcached/stats_collecting_client_test.go index d2e2b5cd..5d9849ae 100644 --- a/test/memcached/stats_collecting_client_test.go +++ b/test/memcached/stats_collecting_client_test.go @@ -135,7 +135,7 @@ func TestStats_Increment(t *testing.T) { expectedErr := errors.New("expectedError") fakeSink.Reset() client.EXPECT().Increment("foo", uint64(5)).Return(uint64(0), expectedErr) - newValue, err = sc.Increment("foo", 5) + _, err = sc.Increment("foo", 5) statsStore.Flush() assert.Equal(expectedErr, err) @@ -145,7 +145,7 @@ func TestStats_Increment(t *testing.T) { fakeSink.Reset() client.EXPECT().Increment("foo", uint64(5)).Return(uint64(0), memcache.ErrCacheMiss) - newValue, err = sc.Increment("foo", 5) + _, err = sc.Increment("foo", 5) statsStore.Flush() assert.Equal(memcache.ErrCacheMiss, err) diff --git a/test/server/health_test.go b/test/server/health_test.go index fd0de4bd..a2238a5c 100644 --- a/test/server/health_test.go +++ b/test/server/health_test.go @@ -25,11 +25,11 @@ func TestHealthCheck(t *testing.T) { r, _ := http.NewRequest("GET", "http://1.2.3.4/healthcheck", nil) hc.ServeHTTP(recorder, r) - if 200 != recorder.Code { + if recorder.Code != 200 { t.Errorf("expected code 200 actual %d", recorder.Code) } - if "OK" != recorder.Body.String() { + if recorder.Body.String() != "OK" { t.Errorf("expected body 'OK', got '%s'", recorder.Body.String()) } @@ -43,7 +43,7 @@ func TestHealthCheck(t *testing.T) { r, _ = http.NewRequest("GET", "http://1.2.3.4/healthcheck", nil) hc.ServeHTTP(recorder, r) - if 500 != recorder.Code { + if recorder.Code != 500 { t.Errorf("expected code 500 actual %d", recorder.Code) } @@ -57,11 +57,11 @@ func TestHealthCheck(t *testing.T) { r, _ = http.NewRequest("GET", "http://1.2.3.4/healthcheck", nil) hc.ServeHTTP(recorder, r) - if 200 != recorder.Code { + if recorder.Code != 200 { t.Errorf("expected code 200 actual %d", recorder.Code) } - if "OK" != recorder.Body.String() { + if recorder.Body.String() != "OK" { t.Errorf("expected body 'OK', got '%s'", recorder.Body.String()) } } @@ -76,7 +76,7 @@ func TestHealthyWithAtLeastOneConfigLoaded(t *testing.T) { r, _ := http.NewRequest("GET", "http://1.2.3.4/healthcheck", nil) hc.ServeHTTP(recorder, r) - if 500 != recorder.Code { + if recorder.Code != 500 { t.Errorf("expected code 500 actual %d", recorder.Code) } @@ -90,11 +90,11 @@ func TestHealthyWithAtLeastOneConfigLoaded(t *testing.T) { r, _ = http.NewRequest("GET", "http://1.2.3.4/healthcheck", nil) hc.ServeHTTP(recorder, r) - if 200 != recorder.Code { + if recorder.Code != 200 { t.Errorf("expected code 200 actual %d", recorder.Code) } - if "OK" != recorder.Body.String() { + if recorder.Body.String() != "OK" { t.Errorf("expected body 'OK', got '%s'", recorder.Body.String()) } @@ -108,7 +108,7 @@ func TestHealthyWithAtLeastOneConfigLoaded(t *testing.T) { r, _ = http.NewRequest("GET", "http://1.2.3.4/healthcheck", nil) hc.ServeHTTP(recorder, r) - if 500 != recorder.Code { + if recorder.Code != 500 { t.Errorf("expected code 500 actual %d", recorder.Code) } @@ -122,11 +122,11 @@ func TestHealthyWithAtLeastOneConfigLoaded(t *testing.T) { r, _ = http.NewRequest("GET", "http://1.2.3.4/healthcheck", nil) hc.ServeHTTP(recorder, r) - if 200 != recorder.Code { + if recorder.Code != 200 { t.Errorf("expected code 200 actual %d", recorder.Code) } - if "OK" != recorder.Body.String() { + if recorder.Body.String() != "OK" { t.Errorf("expected body 'OK', got '%s'", recorder.Body.String()) } } diff --git a/test/server/server_impl_test.go b/test/server/server_impl_test.go index 235ba2a5..9896b798 100644 --- a/test/server/server_impl_test.go +++ b/test/server/server_impl_test.go @@ -3,14 +3,14 @@ package server_test import ( "context" "fmt" - "io/ioutil" + "io" "net/http" "net/http/httptest" "strings" "testing" - "github.com/golang/protobuf/proto" "github.com/stretchr/testify/mock" + "google.golang.org/protobuf/proto" pb "github.com/envoyproxy/go-control-plane/envoy/service/ratelimit/v3" @@ -36,7 +36,7 @@ func assertHttpResponse(t *testing.T, handler(w, req) resp := w.Result() - actualBody, _ := ioutil.ReadAll(resp.Body) + actualBody, _ := io.ReadAll(resp.Body) assert.Equal(expectedContentType, resp.Header.Get("Content-Type")) assert.Equal(expectedStatusCode, resp.StatusCode) assert.Equal(expectedResponseBody, string(actualBody)) @@ -55,10 +55,10 @@ func TestJsonHandler(t *testing.T) { }) // Missing request body - assertHttpResponse(t, handler, "", 400, "text/plain; charset=utf-8", "EOF\n") + assertHttpResponse(t, handler, "", 400, "text/plain; charset=utf-8", "Bad Request\n") // Request body is not valid json - assertHttpResponse(t, handler, "}", 400, "text/plain; charset=utf-8", "invalid character '}' looking for beginning of value\n") + assertHttpResponse(t, handler, "}", 400, "text/plain; charset=utf-8", "Bad Request\n") // Unknown response code rls.EXPECT().ShouldRateLimit(context.Background(), requestMatcher).Return(&pb.RateLimitResponse{}, nil) @@ -66,11 +66,11 @@ func TestJsonHandler(t *testing.T) { // ratelimit service error rls.EXPECT().ShouldRateLimit(context.Background(), requestMatcher).Return(nil, fmt.Errorf("some error")) - assertHttpResponse(t, handler, `{"domain": "foo"}`, 400, "text/plain; charset=utf-8", "some error\n") + assertHttpResponse(t, handler, `{"domain": "foo"}`, 400, "text/plain; charset=utf-8", "Bad Request\n") // json unmarshaling error rls.EXPECT().ShouldRateLimit(context.Background(), requestMatcher).Return(nil, nil) - assertHttpResponse(t, handler, `{"domain": "foo"}`, 500, "text/plain; charset=utf-8", "error marshaling proto3 to json: Marshal called with nil\n") + assertHttpResponse(t, handler, `{"domain": "foo"}`, 500, "text/plain; charset=utf-8", "Internal Server Error\n") // successful request, not rate limited rls.EXPECT().ShouldRateLimit(context.Background(), requestMatcher).Return(&pb.RateLimitResponse{