Skip to content

Commit

Permalink
Merge pull request #704 from Hitachi/improve-rate-limit-policy
Browse files Browse the repository at this point in the history 
Improve rate limit policy
  • Loading branch information
@mikz
mikz authored May 14, 2018
2 parents 356be50 + fcf5c35 commit 5ec4848
Show file tree
Hide file tree
Showing 5 changed files with 209 additions and 163 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Changed

- `THREESCALE_PORTAL_ENDPOINT` and `THREESCALE_CONFIG_FILE` are not required anymore [PR #702](https://github.com/3scale/apicast/pull/702)
- The `scope` of the Rate Limit policy is `service` by default [PR #704](https://github.com/3scale/apicast/pull/704)

## [3.2.0-rc2] - 2018-05-11

Expand Down
142 changes: 43 additions & 99 deletions gateway/src/apicast/policy/rate_limit/apicast-policy.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,37 +6,52 @@
"version": "builtin",
"configuration": {
"type": "object",
"definitions": {
"key": {
"$id": "#/definitions/key",
"description": "The key corresponding to the limiter object",
"type": "object",
"properties": {
"name": {
"type": "string",
"description": "The name of the key, must be unique in the scope"
},
"scope": {
"type": "string",
"description": "Scope of the key",
"default": "service",
"oneOf": [{
"enum": ["global"],
"description": "Global scope, affecting to all services"
}, {
"enum": ["service"],
"description": "Service scope, affecting to one service"
}]
}
}
},
"error_handling": {
"$id": "#/definitions/error_handling",
"type": "string",
"description": "How to handle an error",
"default": "exit",
"oneOf": [{
"enum": ["exit"],
"description": "Respond with an error"
}, {
"enum": ["log"],
"description": "Let the request go through and only output logs"
}]
}
},
"properties": {
"connection_limiters": {
"type": "array",
"items": {
"type": "object",
"properties": {
"key": {
"description": "The key corresponding to the limiter object",
"type": "object",
"properties": {
"name": {
"type": "string",
"description": "The name of the key, must be unique in the scope"
},
"scope": {
"type": "string",
"description": "Scope of the key",
"default": "global",
"oneOf": [{
"enum": ["global"],
"description": "Global scope, affecting to all services"
}, {
"enum": ["service"],
"description": "Service scope, affecting to one service"
}]
},
"service_name": {
"type": "string",
"description": "Name of service, necessary for service scope"
}
}
"$ref": "#/definitions/key"
},
"conn": {
"type": "integer",
Expand All @@ -62,30 +77,7 @@
"type": "object",
"properties": {
"key": {
"description": "The key corresponding to the limiter object",
"type": "object",
"properties": {
"name": {
"type": "string",
"description": "The name of the key, must be unique in the scope"
},
"scope": {
"type": "string",
"description": "Scope of the key",
"default": "global",
"oneOf": [{
"enum": ["global"],
"description": "Global scope, affecting to all services"
}, {
"enum": ["service"],
"description": "Service scope, affecting to one service"
}]
},
"service_name": {
"type": "string",
"description": "Name of service, necessary for service scope"
}
}
"$ref": "#/definitions/key"
},
"rate": {
"type": "integer",
Expand All @@ -106,30 +98,7 @@
"type": "object",
"properties": {
"key": {
"description": "The key corresponding to the limiter object",
"type": "object",
"properties": {
"name": {
"type": "string",
"description": "The name of the key, must be unique in the scope"
},
"scope": {
"type": "string",
"description": "Scope of the key",
"default": "global",
"oneOf": [{
"enum": ["global"],
"description": "Global scope, affecting to all services"
}, {
"enum": ["service"],
"description": "Service scope, affecting to one service"
}]
},
"service_name": {
"type": "string",
"description": "Name of service, necessary for service scope"
}
}
"$ref": "#/definitions/key"
},
"count": {
"type": "integer",
Expand Down Expand Up @@ -157,16 +126,7 @@
"default": 429
},
"error_handling": {
"type": "string",
"description": "How to handle an error",
"default": "exit",
"oneOf": [{
"enum": ["exit"],
"description": "Respond with an error"
}, {
"enum": ["log"],
"description": "Let the request go through and only output logs"
}]
"$ref": "#/definitions/error_handling"
}
}
},
Expand All @@ -179,23 +139,7 @@
"default": 500
},
"error_handling": {
"type": "string",
"description": "How to handle an error",
"default": "exit",
"oneOf": [
{
"enum": [
"exit"
],
"description": "Respond with an error"
},
{
"enum": [
"log"
],
"description": "Let the request go through and only output logs"
}
]
"$ref": "#/definitions/error_handling"
}
}
}
Expand Down
24 changes: 13 additions & 11 deletions gateway/src/apicast/policy/rate_limit/rate_limit.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ local shdict_key = 'limiter'
local insert = table.insert
local ipairs = ipairs
local unpack = table.unpack
local format = string.format
local concat = table.concat

local new = _M.new

Expand Down Expand Up @@ -108,7 +110,7 @@ local function init_error_settings(limits_exceeded_error, configuration_error)
return error_settings
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 = {}

Expand All @@ -125,10 +127,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 = limiter.key.service_name.."_"..type.."_"..limiter.key.name
if limiter.key.scope == "global" then
key = format("%s_%s", type, limiter.key.name)
else
key = type.."_"..limiter.key.name
key = format("%s_%s_%s", service_id, type, limiter.key.name)
end

insert(res_keys, key)
Expand All @@ -150,7 +152,7 @@ function _M.new(config)
return self
end

function _M:access()
function _M:access(context)
local red
if self.redis_url then
local rederr
Expand All @@ -163,13 +165,13 @@ function _M:access()
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 }
Expand Down Expand Up @@ -205,8 +207,8 @@ function _M:access()

for i, lim in ipairs(limiters) do
if lim.is_committed and lim:is_committed() then
table.insert(connections_committed, lim)
table.insert(keys_committed, keys[i])
insert(connections_committed, lim)
insert(keys_committed, keys[i])
end
end

Expand All @@ -217,7 +219,7 @@ function _M:access()
end

if delay > 0 then
ngx.log(ngx.WARN, 'need to delay by: ', delay, 's, states: ', table.concat(states, ", "))
ngx.log(ngx.WARN, 'need to delay by: ', delay, 's, states: ', concat(states, ", "))
ngx.sleep(delay)
end

Expand Down
Loading

0 comments on commit 5ec4848

Please sign in to comment.