From d49179b1a639261f582ffb98178f88948c9be744 Mon Sep 17 00:00:00 2001 From: 70335265 Date: Mon, 14 May 2018 15:11:17 +0900 Subject: [PATCH] change default scope to service and use service_id in the context --- .../policy/rate_limit/apicast-policy.json | 18 +-- .../apicast/policy/rate_limit/rate_limit.lua | 30 ++--- spec/policy/rate_limit/rate_limit_spec.lua | 103 +++++++++------ t/apicast-policy-rate-limit.t | 122 ++++++++++++++---- 4 files changed, 180 insertions(+), 93 deletions(-) diff --git a/gateway/src/apicast/policy/rate_limit/apicast-policy.json b/gateway/src/apicast/policy/rate_limit/apicast-policy.json index 836f1348f..d0e650f13 100644 --- a/gateway/src/apicast/policy/rate_limit/apicast-policy.json +++ b/gateway/src/apicast/policy/rate_limit/apicast-policy.json @@ -23,7 +23,7 @@ "scope": { "type": "string", "description": "Scope of the key", - "default": "global", + "default": "service", "oneOf": [{ "enum": ["global"], "description": "Global scope, affecting to all services" @@ -31,10 +31,6 @@ "enum": ["service"], "description": "Service scope, affecting to one service" }] - }, - "service_name": { - "type": "string", - "description": "Name of service, necessary for service scope" } } }, @@ -72,7 +68,7 @@ "scope": { "type": "string", "description": "Scope of the key", - "default": "global", + "default": "service", "oneOf": [{ "enum": ["global"], "description": "Global scope, affecting to all services" @@ -80,10 +76,6 @@ "enum": ["service"], "description": "Service scope, affecting to one service" }] - }, - "service_name": { - "type": "string", - "description": "Name of service, necessary for service scope" } } }, @@ -116,7 +108,7 @@ "scope": { "type": "string", "description": "Scope of the key", - "default": "global", + "default": "service", "oneOf": [{ "enum": ["global"], "description": "Global scope, affecting to all services" @@ -124,10 +116,6 @@ "enum": ["service"], "description": "Service scope, affecting to one service" }] - }, - "service_name": { - "type": "string", - "description": "Name of service, necessary for service scope" } } }, diff --git a/gateway/src/apicast/policy/rate_limit/rate_limit.lua b/gateway/src/apicast/policy/rate_limit/rate_limit.lua index a9e8bf0ee..ec2ce37a4 100644 --- a/gateway/src/apicast/policy/rate_limit/rate_limit.lua +++ b/gateway/src/apicast/policy/rate_limit/rate_limit.lua @@ -113,13 +113,13 @@ local function init_error_settings(limits_exceeded_error, configuration_error) return error_settings end -local function initialize_redis_records(type, redis, seed, limiters) +local function initialize_redis_records(type, redis, seed, limiters, service_id) for _, limiter in ipairs(limiters) do local key - if limiter.key.scope == "service" then - key = format("%s_%s_%s", limiter.key.service_name, type, limiter.key.name) - else + if limiter.key.scope == "global" then key = format("%s_%s", type, limiter.key.name) + else + key = format("%s_%s_%s", service_id, type, limiter.key.name) end local seed_key = format("%s_seed", key) @@ -147,7 +147,7 @@ local function initialize_redis_records(type, redis, seed, limiters) end end -local function build_limiters_and_keys(type, limiters, redis, error_settings) +local function build_limiters_and_keys(type, limiters, redis, error_settings, service_id) local res_limiters = {} local res_keys = {} @@ -164,10 +164,10 @@ local function build_limiters_and_keys(type, limiters, redis, error_settings) insert(res_limiters, lim) local key - if limiter.key.scope == "service" then - key = format("%s_%s_%s", limiter.key.service_name, type, limiter.key.name) - else + if limiter.key.scope == "global" then key = format("%s_%s", type, limiter.key.name) + else + key = format("%s_%s_%s", service_id, type, limiter.key.name) end insert(res_keys, key) @@ -190,7 +190,7 @@ function _M.new(config) return self end -function _M:access() +function _M:access(context) local red if self.redis_url then local rederr @@ -201,19 +201,19 @@ function _M:access() return end - initialize_redis_records('connections', red, self.seed, self.connection_limiters) - initialize_redis_records('leaky_bucket', red, self.seed, self.leaky_bucket_limiters) - initialize_redis_records('fixed_window', red, self.seed, self.fixed_window_limiters) + initialize_redis_records('connections', red, self.seed, self.connection_limiters, context.service.id) + initialize_redis_records('leaky_bucket', red, self.seed, self.leaky_bucket_limiters, context.service.id) + initialize_redis_records('fixed_window', red, self.seed, self.fixed_window_limiters, context.service.id) end local conn_limiters, conn_keys = build_limiters_and_keys( - 'connections', self.connection_limiters, red, self.error_settings) + 'connections', self.connection_limiters, red, self.error_settings, context.service.id) local leaky_bucket_limiters, leaky_bucket_keys = build_limiters_and_keys( - 'leaky_bucket', self.leaky_bucket_limiters, red, self.error_settings) + 'leaky_bucket', self.leaky_bucket_limiters, red, self.error_settings, context.service.id) local fixed_window_limiters, fixed_window_keys = build_limiters_and_keys( - 'fixed_window', self.fixed_window_limiters, red, self.error_settings) + 'fixed_window', self.fixed_window_limiters, red, self.error_settings, context.service.id) local limiters = {} local limiter_groups = { conn_limiters, leaky_bucket_limiters, fixed_window_limiters } diff --git a/spec/policy/rate_limit/rate_limit_spec.lua b/spec/policy/rate_limit/rate_limit_spec.lua index 17a6edd41..038bd4e16 100644 --- a/spec/policy/rate_limit/rate_limit_spec.lua +++ b/spec/policy/rate_limit/rate_limit_spec.lua @@ -40,6 +40,7 @@ local redis_port = env.get('TEST_NGINX_REDIS_PORT') or 6379 describe('Rate limit policy', function() local ngx_exit_spy local ngx_sleep_spy + local context setup(function() ngx_exit_spy = spy.on(ngx, 'exit') @@ -50,108 +51,113 @@ describe('Rate limit policy', function() local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) redis:select(1) - redis:del('connections_test1', 'leaky_bucket_test2', 'bank_A_leaky_bucket_test4') + redis:del('connections_test1', 'leaky_bucket_test2', '5_leaky_bucket_test4') redis:del('fixed_window_test3', 'fixed_window_test3_count', 'fixed_window_test3_window') redis:del('connections_test1_seed', 'leaky_bucket_test2_seed') - redis:del('fixed_window_test3_seed', 'bank_A_leaky_bucket_test4_seed') + redis:del('fixed_window_test3_seed', '5_leaky_bucket_test4_seed') init_val() + context = { + service = { + id = 5 + } + } end) describe('.access', function() it('success with multiple limiters', function() local config = { connection_limiters = { - { key = { name = 'test1' }, conn = 20, burst = 10, delay = 0.5 } + { key = { name = 'test1', scope = 'global' }, conn = 20, burst = 10, delay = 0.5 } }, leaky_bucket_limiters = { - { key = { name = 'test2' }, rate = 18, burst = 9 } + { key = { name = 'test2', scope = 'global' }, rate = 18, burst = 9 } }, fixed_window_limiters = { - { key = {name = 'test3'}, count = 10, window = 10 } + { key = { name = 'test3', scope = 'global' }, count = 10, window = 10 } }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } local rate_limit_policy = RateLimitPolicy.new(config) - rate_limit_policy:access() + rate_limit_policy:access(context) end) it('no redis url', function() local config = { connection_limiters = { - { key = { name = 'test1' }, conn = 20, burst = 10, delay = 0.5 } + { key = { name = 'test1', scope = 'global' }, conn = 20, burst = 10, delay = 0.5 } }, leaky_bucket_limiters = { - { key = { name = 'test2' }, rate = 18, burst = 9 } + { key = { name = 'test2', scope = 'global' }, rate = 18, burst = 9 } }, fixed_window_limiters = { - { key = { name = 'test3' }, count = 10, window = 10 } + { key = { name = 'test3', scope = 'global' }, count = 10, window = 10 } } } local rate_limit_policy = RateLimitPolicy.new(config) - rate_limit_policy:access() + rate_limit_policy:access(context) end) it('invalid redis url', function() local config = { connection_limiters = { - { key = { name = 'test1' }, conn = 20, burst = 10, delay = 0.5 } + { key = { name = 'test1', scope = 'global' }, conn = 20, burst = 10, delay = 0.5 } }, redis_url = 'redis://invalidhost:'..redis_port..'/1' } local rate_limit_policy = RateLimitPolicy.new(config) - rate_limit_policy:access() + rate_limit_policy:access(context) assert.spy(ngx_exit_spy).was_called_with(500) end) it('rejected (conn)', function() local config = { connection_limiters = { - { key = { name = 'test1' }, conn = 1, burst = 0, delay = 0.5 } + { key = { name = 'test1', scope = 'global' }, conn = 1, burst = 0, delay = 0.5 } }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } local rate_limit_policy = RateLimitPolicy.new(config) - rate_limit_policy:access() - rate_limit_policy:access() + rate_limit_policy:access(context) + rate_limit_policy:access(context) assert.spy(ngx_exit_spy).was_called_with(429) end) it('rejected (req)', function() local config = { leaky_bucket_limiters = { - { key = { name = 'test2' }, rate = 1, burst = 0 } + { key = { name = 'test2', scope = 'global' }, rate = 1, burst = 0 } }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } local rate_limit_policy = RateLimitPolicy.new(config) - rate_limit_policy:access() - rate_limit_policy:access() + rate_limit_policy:access(context) + rate_limit_policy:access(context) assert.spy(ngx_exit_spy).was_called_with(429) end) it('rejected (count)', function() local config = { fixed_window_limiters = { - { key = { name = 'test3' }, count = 1, window = 10 } + { key = { name = 'test3', scope = 'global' }, count = 1, window = 10 } }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } local rate_limit_policy = RateLimitPolicy.new(config) - rate_limit_policy:access() - rate_limit_policy:access() + rate_limit_policy:access(context) + rate_limit_policy:access(context) assert.spy(ngx_exit_spy).was_called_with(429) end) it('delay (conn)', function() local config = { connection_limiters = { - { key = {name = 'test1'}, conn = 1, burst = 1, delay = 2 } + { key = { name = 'test1', scope = 'global' }, conn = 1, burst = 1, delay = 2 } }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } local rate_limit_policy = RateLimitPolicy.new(config) - rate_limit_policy:access() - rate_limit_policy:access() + rate_limit_policy:access(context) + rate_limit_policy:access(context) assert.spy(ngx_sleep_spy).was_called_with(match.is_gt(0.001)) end) @@ -163,8 +169,8 @@ describe('Rate limit policy', function() redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } local rate_limit_policy = RateLimitPolicy.new(config) - rate_limit_policy:access() - rate_limit_policy:access() + rate_limit_policy:access(context) + rate_limit_policy:access(context) assert.spy(ngx_sleep_spy).was_called_with(match.is_gt(0.001)) end) @@ -172,7 +178,24 @@ describe('Rate limit policy', function() local config = { leaky_bucket_limiters = { { - key = { name = 'test4', scope = 'service', service_name = 'bank_A' }, + key = { name = 'test4', scope = 'service' }, + rate = 1, + burst = 1 + } + }, + redis_url = 'redis://'..redis_host..':'..redis_port..'/1' + } + local rate_limit_policy = RateLimitPolicy.new(config) + rate_limit_policy:access(context) + rate_limit_policy:access(context) + assert.spy(ngx_sleep_spy).was_called_with(match.is_gt(0.001)) + end) + + it('delay (req) default service scope', function() + local config = { + leaky_bucket_limiters = { + { + key = { name = 'test4' }, rate = 1, burst = 1 } @@ -180,33 +203,33 @@ describe('Rate limit policy', function() redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } local rate_limit_policy = RateLimitPolicy.new(config) - rate_limit_policy:access() - rate_limit_policy:access() + rate_limit_policy:access(context) + rate_limit_policy:access(context) assert.spy(ngx_sleep_spy).was_called_with(match.is_gt(0.001)) end) it('initialize redis records', function() local config = { connection_limiters = { - {key = {name = 'test1'}, conn = 20, burst = 10, delay = 0.5} + {key = { name = 'test1', scope = 'global' }, conn = 20, burst = 10, delay = 0.5} }, fixed_window_limiters = { - {key = {name = 'test3'}, count = 10, window = 10} + {key = { name = 'test3', scope = 'global' }, count = 10, window = 10} }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } local config2 = { connection_limiters = { - {key = {name = 'test1'}, conn = 20, burst = 10, delay = 0.5} + {key = { name = 'test1', scope = 'global' }, conn = 20, burst = 10, delay = 0.5} }, fixed_window_limiters = { - {key = {name = 'test3'}, count = 15, window = 10} + {key = { name = 'test3', scope = 'global' }, count = 15, window = 10} }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } local rate_limit_policy = RateLimitPolicy.new(config) - rate_limit_policy:access() + rate_limit_policy:access(context) local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) @@ -222,7 +245,7 @@ describe('Rate limit policy', function() ngx.sleep(1) rate_limit_policy = RateLimitPolicy.new(config2) - rate_limit_policy:access() + rate_limit_policy:access(context) conn = redis:get('connections_test1') fixed_window = redis:get('fixed_window_test3') @@ -237,13 +260,13 @@ describe('Rate limit policy', function() it('do not initialize redis records', function() local config = { fixed_window_limiters = { - {key = {name = 'test3'}, count = 10, window = 10} + {key = { name = 'test3', scope = 'global' }, count = 10, window = 10} }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } local rate_limit_policy = RateLimitPolicy.new(config) - rate_limit_policy:access() + rate_limit_policy:access(context) local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) @@ -257,7 +280,7 @@ describe('Rate limit policy', function() ngx.sleep(1) rate_limit_policy = RateLimitPolicy.new(config) - rate_limit_policy:access() + rate_limit_policy:access(context) fixed_window = redis:get('fixed_window_test3') count = redis:get('fixed_window_test3_count') @@ -272,12 +295,12 @@ describe('Rate limit policy', function() it('success in leaving', function() local config = { connection_limiters = { - { key = {name = 'test1'}, conn = 20, burst = 10, delay = 0.5 } + { key = { name = 'test1', scope = 'global' }, conn = 20, burst = 10, delay = 0.5 } }, redis_url = 'redis://'..redis_host..':'..redis_port..'/1' } local rate_limit_policy = RateLimitPolicy.new(config) - rate_limit_policy:access() + rate_limit_policy:access(context) rate_limit_policy:log() end) end) diff --git a/t/apicast-policy-rate-limit.t b/t/apicast-policy-rate-limit.t index f00e5de57..5547f016e 100644 --- a/t/apicast-policy-rate-limit.t +++ b/t/apicast-policy-rate-limit.t @@ -30,7 +30,7 @@ Return 200 code. configuration = { connection_limiters = { { - key = {name = "test1", scope = "service", service_name = "service_C"}, + key = {name = "test1", scope = "service"}, conn = 1, burst = 1, delay = 2 @@ -44,7 +44,7 @@ Return 200 code. configuration = { connection_limiters = { { - key = {name = "test1", scope = "service", service_name = "service_C"}, + key = {name = "test1", scope = "service"}, conn = 1, burst = 1, delay = 2 @@ -77,7 +77,83 @@ Return 200 code. local redis = require('resty.redis'):new() redis:connect(redis_host, redis_port) redis:select(1) - redis:del("service_C_connections_test1", "service_C_connections_test1_seed") + redis:del("42_connections_test1", "42_connections_test1_seed") + } + } + +--- pipelined_requests eval +["GET /flush_redis","GET /"] +--- error_code eval +[200, 200] + +=== TEST 2: Delay (conn) default service scope. +Return 200 code. +--- http_config + include $TEST_NGINX_UPSTREAM_CONFIG; + lua_package_path "$TEST_NGINX_LUA_PATH"; + + init_by_lua_block { + require "resty.core" + ngx.shared.limiter:flush_all() + require('apicast.configuration_loader').mock({ + services = { + { + id = 42, + proxy = { + policy_chain = { + { + name = "apicast.policy.rate_limit", + configuration = { + connection_limiters = { + { + key = {name = "test2"}, + conn = 1, + burst = 1, + delay = 2 + } + }, + redis_url = "redis://$TEST_NGINX_REDIS_HOST:$TEST_NGINX_REDIS_PORT/1" + } + }, + { + name = "apicast.policy.rate_limit", + configuration = { + connection_limiters = { + { + key = {name = "test2"}, + conn = 1, + burst = 1, + delay = 2 + } + }, + redis_url = "redis://$TEST_NGINX_REDIS_HOST:$TEST_NGINX_REDIS_PORT/1" + } + } + } + } + } + } + }) + } + lua_shared_dict limiter 1m; + +--- config + include $TEST_NGINX_APICAST_CONFIG; + resolver $TEST_NGINX_RESOLVER; + + location /transactions/authrep.xml { + content_by_lua_block { ngx.exit(200) } + } + + location /flush_redis { + content_by_lua_block { + local env = require('resty.env') + local redis_host = "$TEST_NGINX_REDIS_HOST" or '127.0.0.1' + local redis_port = "$TEST_NGINX_REDIS_PORT" or 6379 + local redis = require('resty.redis'):new() + redis:connect(redis_host, redis_port) + redis:select(1) + redis:del("42_connections_test2", "42_connections_test2_seed") } } @@ -106,7 +182,7 @@ Return 500 code. configuration = { connection_limiters = { { - key = {name = "test3"}, + key = {name = "test3", scope = "global"}, conn = 20, burst = 10, delay = 0.5 @@ -150,7 +226,7 @@ Return 200 code. configuration = { connection_limiters = { { - key = {name = "test4"}, + key = {name = "test4", scope = "global"}, conn = 1, burst = 0, delay = 2 @@ -165,7 +241,7 @@ Return 200 code. configuration = { connection_limiters = { { - key = {name = "test4"}, + key = {name = "test4", scope = "global"}, conn = 1, burst = 0, delay = 2 @@ -224,7 +300,7 @@ Return 200 code. configuration = { connection_limiters = { { - key = {name = "test5"}, + key = {name = "test5", scope = "global"}, conn = 20, burst = 10, delay = 0.5 @@ -269,14 +345,14 @@ Return 200 code. configuration = { leaky_bucket_limiters = { { - key = {name = "test6_1"}, + key = {name = "test6_1", scope = "global"}, rate = 20, burst = 10 } }, connection_limiters = { { - key = {name = "test6_2"}, + key = {name = "test6_2", scope = "global"}, conn = 20, burst = 10, delay = 0.5 @@ -284,7 +360,7 @@ Return 200 code. }, fixed_window_limiters = { { - key = {name = "test6_3"}, + key = {name = "test6_3", scope = "global"}, count = 20, window = 10 } @@ -345,7 +421,7 @@ Return 429 code. configuration = { connection_limiters = { { - key = {name = "test7"}, + key = {name = "test7", scope = "global"}, conn = 1, burst = 0, delay = 2 @@ -359,7 +435,7 @@ Return 429 code. configuration = { connection_limiters = { { - key = {name = "test7"}, + key = {name = "test7", scope = "global"}, conn = 1, burst = 0, delay = 2 @@ -419,7 +495,7 @@ Return 503 code. configuration = { leaky_bucket_limiters = { { - key = {name = "test8"}, + key = {name = "test8", scope = "global"}, rate = 1, burst = 0 } @@ -539,7 +615,7 @@ Return 200 code. configuration = { connection_limiters = { { - key = {name = "test10"}, + key = {name = "test10", scope = "global"}, conn = 1, burst = 1, delay = 2 @@ -553,7 +629,7 @@ Return 200 code. configuration = { connection_limiters = { { - key = {name = "test10"}, + key = {name = "test10", scope = "global"}, conn = 1, burst = 1, delay = 2 @@ -615,7 +691,7 @@ Return 200 code. configuration = { leaky_bucket_limiters = { { - key = {name = "test11"}, + key = {name = "test11", scope = "global"}, rate = 1, burst = 1 } @@ -676,7 +752,7 @@ Return 429 code. configuration = { connection_limiters = { { - key = {name = "test12"}, + key = {name = "test12", scope = "global"}, conn = 1, burst = 0, delay = 2 @@ -689,7 +765,7 @@ Return 429 code. configuration = { connection_limiters = { { - key = {name = "test12"}, + key = {name = "test12", scope = "global"}, conn = 1, burst = 0, delay = 2 @@ -734,7 +810,7 @@ Return 429 code. configuration = { leaky_bucket_limiters = { { - key = {name = "test13"}, + key = {name = "test13", scope = "global"}, rate = 1, burst = 0 } @@ -778,7 +854,7 @@ Return 429 code. configuration = { fixed_window_limiters = { { - key = {name = "test14"}, + key = {name = "test14", scope = "global"}, count = 1, window = 10 } @@ -821,7 +897,7 @@ Return 200 code. configuration = { connection_limiters = { { - key = {name = "test15"}, + key = {name = "test15", scope = "global"}, conn = 1, burst = 1, delay = 2 @@ -834,7 +910,7 @@ Return 200 code. configuration = { connection_limiters = { { - key = {name = "test15"}, + key = {name = "test15", scope = "global"}, conn = 1, burst = 1, delay = 2 @@ -883,7 +959,7 @@ Return 200 code. configuration = { leaky_bucket_limiters = { { - key = {name = "test16"}, + key = {name = "test16", scope = "global"}, rate = 1, burst = 1 }