Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract MappingRulesMatcher and Usage modules #580

Merged
merged 11 commits into from
Feb 7, 2018
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Extract `mapping_rule` module from the `configuration` module [PR #571](https://github.com/3scale/apicast/pull/571)
- Renamed `apicast/policy/policy.lua` to `apicast/policy.lua` [PR #569](https://github.com/3scale/apicast/pull/569)
- Sandbox loading policies [PR #566](https://github.com/3scale/apicast/pull/566)
- Extracted `usage` and `mapping_rules_matcher` modules so they can be used from policies [PR #580](https://github.com/3scale/apicast/pull/580)

## [3.2.0-alpha2] - 2017-11-30

Expand Down
32 changes: 3 additions & 29 deletions gateway/src/apicast/configuration/service.lua
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ local rawget = rawget
local lower = string.lower
local gsub = string.gsub
local select = select
local concat = table.concat
local insert = table.insert
local re = require 'ngx.re'

local http_authorization = require 'resty.http_authorization'

local oauth = require('apicast.oauth')
local mapping_rules_matcher = require('apicast.mapping_rules_matcher')

local _M = { }
local mt = { __index = _M }
Expand Down Expand Up @@ -163,21 +162,6 @@ function backend_version_credentials.version_oauth(config)
return setmetatable({ access_token, access_token = access_token }, credentials_oauth_mt)
end

local function set_or_inc(t, name, delta)
return (t[name] or 0) + (delta or 0)
end

local function check_rule(req, rule, usage_t, matched_rules, params)
if rule:matches(req.method, req.path, req.args) then
local system_name = rule.system_name
local value = set_or_inc(usage_t, system_name, rule.delta)

usage_t[system_name] = value
params['usage[' .. system_name .. ']'] = value
insert(matched_rules, rule.pattern)
end
end

local function get_auth_params(method)
local params = ngx.req.get_uri_args()

Expand Down Expand Up @@ -238,22 +222,12 @@ function _M:oauth()
end

local function extract_usage_v2(config, method, path)
local usage_t = {}
local matched_rules = {}
local params = {}
local rules = config.rules

local args = get_auth_params(method)

ngx.log(ngx.DEBUG, '[mapping] service ', config.id, ' has ', #rules, ' rules')

for i = 1, #rules do
check_rule({path=path, method=method, args=args}, rules[i], usage_t, matched_rules, params)
end

-- if there was no match, usage is set to nil and it will respond a 404, this
-- behavior can be changed
return usage_t, concat(matched_rules, ", "), params
local args = get_auth_params(method)
return mapping_rules_matcher.get_usage_from_matches(method, path, args, rules)
end

-- Deprecated
Expand Down
53 changes: 53 additions & 0 deletions gateway/src/apicast/mapping_rules_matcher.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
--- Mapping rules matcher
-- @module mapping_rules_matcher
-- Matches a request against a set of mapping rules and calculates the usage
-- that needs to be authorized and reported according to the rules that match.

local ipairs = ipairs
local insert = table.insert
local Usage = require('apicast.usage')

local _M = {}

--- Calculate usage from matching mapping rules.
-- Matches a request against a set of mapping rules and returns the resulting
-- usage and the matched rules.
-- @tparam string method HTTP method.
-- @tparam string uri URI.
-- @tparam table args Request arguments.
-- @tparam table rules Mapping rules to be matched.
-- @treturn Usage Calculated usage.
-- @treturn table Matched rules.
function _M.get_usage_from_matches(method, uri, args, rules)
local usage = Usage.new()
local matched_rules = {}

for _, rule in ipairs(rules) do
if rule:matches(method, uri, args) then
-- Some rules have no delta. Send 0 in that case.
usage:add(rule.system_name, rule.delta or 0)
insert(matched_rules, rule)
end
end

return usage, matched_rules
end

--- Check if there is a mapping rule that matches.
-- @tparam string method HTTP method.
-- @tparam string uri URI.
-- @tparam table args Request arguments.
-- @tparam table rules Mapping rules to be matched.
-- @treturn boolean Whether there is a match.
-- @treturn integer|nil Index of the first matched rule.
function _M.matches(method, uri, args, rules)
for i, rule in ipairs(rules) do
if rule:matches(method, uri, args) then
return true, i
end
end

return false
end

return _M
2 changes: 1 addition & 1 deletion gateway/src/apicast/policy/apicast/policy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function _M:rewrite(context)
ngx.ctx.service = service

-- it is possible that proxy:rewrite will terminate the request
p:rewrite(service)
p:rewrite(service, context)
end

p.set_upstream(service)
Expand Down
13 changes: 8 additions & 5 deletions gateway/src/apicast/policy/find_service/policy.lua
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
local next = next

local policy = require('apicast.policy')
local _M = policy.new('Find Service Policy')
local configuration_store = require 'apicast.configuration_store'
local mapping_rules_matcher = require 'apicast.mapping_rules_matcher'
local new = _M.new

local function find_service_strict(configuration, host)
Expand Down Expand Up @@ -39,10 +38,14 @@ local function find_service_cascade(configuration, host)
if hosts[h] == host then
local name = service.system_name or service.id
ngx.log(ngx.DEBUG, 'service ', name, ' matched host ', hosts[h])
local usage, matched_patterns = service:get_usage(method, uri)

if next(usage) and matched_patterns ~= '' then
ngx.log(ngx.DEBUG, 'service ', name, ' matched patterns ', matched_patterns)
local matches = mapping_rules_matcher.matches(method, uri, {}, service.rules)
-- matches() also returns the index of the first rule that matched.
-- As a future optimization, in the part of the code that calculates
-- the usage, we could use this to avoid trying to match again all the
-- rules before the one that matched.

if matches then
found = service
break
end
Expand Down
56 changes: 47 additions & 9 deletions gateway/src/apicast/proxy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ local custom_config = env.get('APICAST_CUSTOM_CONFIG')
local util = require('apicast.util')
local resty_lrucache = require('resty.lrucache')
local backend_cache_handler = require('apicast.backend.cache_handler')
local Usage = require('apicast.usage')

local resty_url = require 'resty.url'

Expand All @@ -21,6 +22,7 @@ local concat = table.concat
local gsub = string.gsub
local tonumber = tonumber
local setmetatable = setmetatable
local ipairs = ipairs
local encode_args = ngx.encode_args
local resty_resolver = require 'resty.resolver'
local semaphore = require('ngx.semaphore')
Expand Down Expand Up @@ -126,10 +128,37 @@ local function output_debug_headers(service, usage, credentials)
end
end

-- Converts a usage to the format expected by the 3scale backend client.
local function format_usage(usage)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it could be a method of usage.lua? Like self.format.
It would also facilitate its testing.

Copy link
Contributor Author

@davidor davidor Feb 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather leave the responsibility of dealing with this to the backend client.

The reason is that, otherwise, we'll need to include a similar method on each of our 'model' classes that need to be sent in the params of a request.

In a future refactor, this format_usage() will be moved somewhere else.

local res = {}

local usage_metrics = usage.metrics
local usage_deltas = usage.deltas

for _, metric in ipairs(usage_metrics) do
local delta = usage_deltas[metric]
res['usage[' .. metric .. ']'] = delta
end

return res
end

local function matched_patterns(matched_rules)
local patterns = {}

for _, rule in ipairs(matched_rules) do
insert(patterns, rule.pattern)
end

return patterns
end

function _M:authorize(service, usage, credentials, ttl)
if not usage or not credentials then return nil, 'missing usage or credentials' end

local encoded_usage = encode_args(usage)
local formatted_usage = format_usage(usage)

local encoded_usage = encode_args(formatted_usage)
if encoded_usage == '' then
return error_no_match(service)
end
Expand All @@ -152,7 +181,7 @@ function _M:authorize(service, usage, credentials, ttl)
ngx.var.cached_key = nil

local backend = assert(backend_client:new(service, http_ng_ngx), 'missing backend')
local res = backend:authrep(usage, credentials)
local res = backend:authrep(formatted_usage, credentials)

local authorized, rejection_reason = self:handle_backend_response(cached_key, res, ttl)
if not authorized then
Expand Down Expand Up @@ -228,7 +257,7 @@ local function handle_oauth(service)
return oauth
end

function _M:rewrite(service)
function _M:rewrite(service, context)
service = _M.set_service(service or ngx.ctx.service)

-- handle_oauth can terminate the request
Expand All @@ -243,7 +272,7 @@ function _M:rewrite(service)
return error_no_credentials(service)
end

local _, matched_patterns, usage_params = service:get_usage(ngx.req.get_method(), ngx.var.uri)
local usage, matched_rules = service:get_usage(ngx.req.get_method(), ngx.var.uri)
local cached_key = { service.id }

-- remove integer keys for serialization
Expand All @@ -264,15 +293,22 @@ function _M:rewrite(service)
local var = ngx.var

-- save those tables in context so they can be used in the backend client
ctx.usage = usage_params
context.usage = context.usage or Usage.new()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be provided by export, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Unfortunately, there's a bug in export(). It does not work correctly on service-level policies.
Something we'll need to fix soon :)

context.usage:merge(usage)

ctx.usage = usage
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now both proxy and the Apicast policy depend on ctx and context. We'll need to change this to make this cleaner.

Copy link
Contributor

@mikz mikz Feb 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So ctx.usage will be the formatted one and context.usage will be our Usage object for now, right?

Both ctx.usage and context.usage are going to be instance of Usage.

ctx.credentials = credentials
ctx.matched_patterns = matched_patterns

self.credentials = credentials
self.usage = usage_params
self.usage = usage

var.cached_key = concat(cached_key, ':')

if debug_header_enabled(service) then
local patterns = matched_patterns(matched_rules)
ctx.matched_patterns = concat(patterns, ', ')
end

local ttl

if self.oauth then
Expand Down Expand Up @@ -319,7 +355,8 @@ end

local function capture_post_action(self, cached_key, service)
local backend = assert(backend_client:new(service, http_ng_ngx), 'missing backend')
local res = backend:authrep(self.usage, self.credentials, response_codes_data())
local formatted_usage = format_usage(self.usage)
local res = backend:authrep(formatted_usage, self.credentials, response_codes_data())

self:handle_backend_response(cached_key, res)
end
Expand All @@ -332,7 +369,8 @@ local function timer_post_action(self, cached_key, service)
if ok then
-- TODO: try to do this in different phase and use semaphore to limit number of background threads
-- TODO: Also it is possible to use sets in shared memory to enqueue work
ngx.timer.at(0, post_action, self, cached_key, backend, self.usage, self.credentials, response_codes_data())
local formatted_usage = format_usage(self.usage)
ngx.timer.at(0, post_action, self, cached_key, backend, formatted_usage, self.credentials, response_codes_data())
else
ngx.log(ngx.ERR, 'failed to acquire timer: ', err)
return capture_post_action(self, cached_key, service)
Expand Down
64 changes: 64 additions & 0 deletions gateway/src/apicast/usage.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
--- Usage
-- @module usage
-- Usage to be authorized and reported against 3scale's backend.

local setmetatable = setmetatable
local ipairs = ipairs
local insert = table.insert

local _M = {}

local mt = { __index = _M }

--- Initialize a usage.
-- @return usage New usage.
function _M.new()
local self = setmetatable({}, mt)

-- table where the keys are metrics and the values their deltas.
self.deltas = {}

-- table that contains the metrics that have a delta associated.
-- It's useful to iterate over the deltas without using '.pairs'.
-- That's what we are doing in .merge().
-- We want to avoid using '.pairs' because is not jitted, '.ipairs' is.
self.metrics = {}

return self
end

--- Add a metric usage.
-- Increases the usage of the given metric by the given value. If the metric
-- is not in the usage, it will be included.
-- Note that this mutates self.
-- @tparam string metric Metric.
-- @tparam integer value Value.
function _M:add(metric, value)
if self.deltas[metric] then
self.deltas[metric] = self.deltas[metric] + value
else
self.deltas[metric] = value
insert(self.metrics, metric)
end
end

--- Merge usages
-- Merges two usages. This means that:
--
-- 1) When a metric appears in both usages, its delta is updated in self by
-- adding the two values.
-- 2) When a metric does not appear in self, it is added in self.
--
-- Note that this mutates self.
-- @tparam another_usage Usage Usage.
function _M:merge(another_usage)
local another_usage_metrics = another_usage.metrics
local another_usage_deltas = another_usage.deltas

for _, metric in ipairs(another_usage_metrics) do
local delta = another_usage_deltas[metric]
self:add(metric, delta)
end
end

return _M
Loading