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

methods for policy chain manipulation #505

Merged
merged 3 commits into from
Nov 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- New policy chains system. This allows users to write custom policies to configure what Apicast can do on each of the Nginx phases [PR #450](https://github.com/3scale/apicast/pull/450)
- Resolver can resolve nginx upstreams [PR #478](https://github.com/3scale/apicast/pull/478)
- Calls 3scale backend with the 'no_body' option enabled. This reduces network traffic in cases where APIcast does not need to parse the response body [PR #483](https://github.com/3scale/apicast/pull/483)
- Methods to modify policy chains [PR #505](https://github.com/3scale/apicast/pull/505)

## Changed

Expand Down
18 changes: 4 additions & 14 deletions gateway/src/apicast/executor.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,14 @@

require('apicast.loader') -- to load code from deprecated paths

local policy_chain = require('apicast.policy_chain')
local PolicyChain = require('apicast.policy_chain')
local policy = require('apicast.policy')
local linked_list = require('apicast.linked_list')

local setmetatable = setmetatable

local _M = { }

local DEFAULT_POLICIES = {
'apicast.policy.load_configuration',
'apicast.policy.find_service',
'apicast.policy.local_chain'
}

local mt = { __index = _M }

-- forward all policy methods to the policy chain
Expand All @@ -29,12 +23,8 @@ for _,phase in policy.phases() do
end
end

local function global_chain()
return policy_chain.build(DEFAULT_POLICIES)
end

function _M.new()
return setmetatable({ policy_chain = global_chain() }, mt)
function _M.new(policy_chain)
return setmetatable({ policy_chain = policy_chain:freeze() }, mt)
end

local function build_context(executor)
Expand Down Expand Up @@ -67,4 +57,4 @@ function _M:context(phase)
return shared_build_context(self)
end

return _M.new()
return _M.new(PolicyChain.default())
54 changes: 48 additions & 6 deletions gateway/src/apicast/policy_chain.lua
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ local error = error
local rawset = rawset
local type = type
local require = require
local insert = table.insert
local noop = function() end

local linked_list = require('apicast.linked_list')
local policy = require('apicast.policy')
local policy_phases = require('apicast.policy').phases

local _M = {

Expand Down Expand Up @@ -49,12 +50,25 @@ function _M.build(modules)
return _M.new(chain)
end


local DEFAULT_POLICIES = {
'apicast.policy.load_configuration',
'apicast.policy.find_service',
'apicast.policy.local_chain'
}

--- Return new policy chain with default policies.
-- @treturn PolicyChain
function _M.default()
return _M.build(DEFAULT_POLICIES)
end

--- Load a module
-- If the module is a string, returns the result of initializing it with the
-- given arguments. Otherwise, this function simply returns the module
-- received.
-- @tparam string/object The module
-- @tparam[opt] params needed to initialize the module
-- @tparam string|table module the module or its name
-- @tparam ?table ... params needed to initialize the module
-- @treturn object The module instantiated
function _M.load(module, ...)
if type(module) == 'string' then
Expand All @@ -71,15 +85,20 @@ function _M.load(module, ...)
end
end

--- Initialize new @{PolicyChain}.
-- @treturn PolicyChain
function _M.new(list)
local chain = list or {}

local self = setmetatable(chain, mt)
chain.config = self:export()
self.frozen = true
return self
end

---------------------
--- @type PolicyChain
-- An instance of @{policy_chain}.

--- Export the shared context of the chain
-- @treturn linked_list The context of the chain. Note: the list returned is
-- read-only.
Expand All @@ -96,6 +115,29 @@ function _M:export()
return chain
end

--- Freeze the policy chain to prevent modifications.
-- After calling this method it won't be possible to insert more policies.
-- @treturn PolicyChain returns self
function _M:freeze()
self.frozen = true
return self
end

--- Insert a policy into the chain
-- @tparam Policy policy the policy to be added to the chain
-- @tparam[opt] int position the position to add the policy to, defaults to last one
-- @treturn int lenght of the chain
-- @error frozen | returned when chain is not modifiable
-- @see freeze
function _M:insert(policy, position)
if self.frozen then
return nil, 'frozen'
else
insert(self, position or #self+1, policy)
return #self
end
end

local function call_chain(phase_name)
return function(self, ...)
for i=1, #self do
Expand All @@ -104,8 +146,8 @@ local function call_chain(phase_name)
end
end

for _,phase in policy.phases() do
for _,phase in policy_phases() do
_M[phase] = call_chain(phase)
end

return _M.build()
return _M.build():freeze()
13 changes: 11 additions & 2 deletions spec/executor_spec.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
local executor = require 'apicast.executor'
local policy_chain = require 'apicast.policy_chain'

describe('executor', function()
local phases = {
'init', 'init_worker',
Expand All @@ -7,8 +10,6 @@ describe('executor', function()
}

it('forwards all the policy methods to the policy chain', function()
local executor = require 'apicast.executor'

-- Policies included by default in the executor
local default_executor_chain = {
require 'apicast.policy.load_configuration',
Expand All @@ -32,4 +33,12 @@ describe('executor', function()
end
end
end)

it('freezes the policy chain', function()
local chain = policy_chain.new({})
assert.falsy(chain.frozen)

executor.new(chain)
assert.truthy(chain.frozen)
end)
end)
75 changes: 54 additions & 21 deletions spec/policy_chain_spec.lua
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
local policy = require 'apicast.policy'
local _M = require 'apicast.policy_chain'

describe('policy_chain', function()
local phases = {
Expand All @@ -9,10 +10,8 @@ describe('policy_chain', function()
}

it('defines a method for each of the nginx phases supported', function()
local policy_chain = require 'apicast.policy_chain'

for _, phase in ipairs(phases) do
assert.equals('function', type(policy_chain[phase]))
assert.equals('function', type(_M[phase]))
end
end)

Expand All @@ -32,8 +31,7 @@ describe('policy_chain', function()
stub(custom_policy_2, 'access')

-- Build the policy chain
local policy_chain = require 'apicast.policy_chain'
local chain = policy_chain.build({ custom_policy_1, custom_policy_2 })
local chain = _M.build({ custom_policy_1, custom_policy_2 })

chain:init()
assert.stub(custom_policy_1.init).was_called()
Expand All @@ -48,7 +46,6 @@ describe('policy_chain', function()

it('uses APIcast as default when no policies are specified', function()
local apicast = require 'apicast.policy.apicast'
local policy_chain = require 'apicast.policy_chain'

-- Stub apicast methods to avoid calling them. We are just interested in
-- knowing whether they were called.
Expand All @@ -57,7 +54,7 @@ describe('policy_chain', function()
end

for _, phase in ipairs(phases) do
policy_chain[phase](policy_chain)
_M[phase](_M)
assert.stub(apicast[phase]).was_called()
end
end)
Expand All @@ -74,32 +71,59 @@ describe('policy_chain', function()
end
end

local policy_chain = require 'apicast.policy_chain'
local sorted_policies = { policies[2], policies[3], policies[1] }
local chain = policy_chain.build(sorted_policies)
local chain = _M.build(sorted_policies)

chain:init()
assert.same({'2', '3', '1'}, execution_order)
end)

it('does not allow to modify phase methods after the chain has been built', function()
local policy_chain = require 'apicast.policy_chain'

for _, phase in ipairs(phases) do
assert.has_error(function()
policy_chain[phase] = function() end
_M[phase] = function() end
end, 'readonly table')
end
end)

it('does not allow to add new methods to the chain after it has been built', function()
local policy_chain = require 'apicast.policy_chain'

assert.has_error(function()
policy_chain['new_phase'] = function() end
_M['new_phase'] = function() end
end, 'readonly table')
end)

describe('.insert', function()

it('adds policy to the end of the chain', function()
local chain = _M.new({ 'one', 'two' })

chain:insert(policy)

assert.equal(policy, chain[3])
assert.equal(3, #chain)
end)

it('adds a policy to specific position', function()
local chain = _M.new({ 'one', 'two'})

chain:insert(policy, 2)
assert.equal(policy, chain[2])
assert.equal('one', chain[1])
assert.equal('two', chain[3])
assert.equal(3, #chain)
end)

it('errors when inserting to frozen chain', function()
local chain = _M.new({}):freeze()

local ok, err = chain:insert(policy, 1)

assert.is_nil(ok)
assert.equal(err, 'frozen')
assert.equal(0, #chain)
end)
end)

describe('.export', function()
it('returns the data exposed by each of its policies', function()
local policy_1 = policy.new('1')
Expand All @@ -108,8 +132,7 @@ describe('policy_chain', function()
local policy_2 = policy.new('2')
policy_2.export = function() return { shared_data_2 = '2' } end

local policy_chain = require 'apicast.policy_chain'
local chain = policy_chain.build({ policy_1, policy_2 })
local chain = _M.build({ policy_1, policy_2 })

local shared_data = chain:export()
assert.equal('1', shared_data['shared_data_1'])
Expand All @@ -120,8 +143,7 @@ describe('policy_chain', function()
local policy_1 = policy.new('1')
policy_1.export = function() return { shared_data = '1' } end

local policy_chain = require 'apicast.policy_chain'
local chain = policy_chain.build({ policy_1 })
local chain = _M.build({ policy_1 })

local shared_data = chain:export()

Expand All @@ -139,12 +161,23 @@ describe('policy_chain', function()
local policy_2 = policy.new('custom_authorizer')
policy_2.export = function() return { shared_data_1 = '2' } end

local policy_chain = require 'apicast.policy_chain'
local chain = policy_chain.build({ policy_1, policy_2 })
local chain = _M.build({ policy_1, policy_2 })

local shared_data = chain:export()
assert.equal('1', shared_data['shared_data_1'])
end)
end)
end)

describe('.default', function()
it('returns a default policy chain', function()
local default = _M.default()

assert(#default > 1, 'has <= 1 policy')
end)

it('returns not frozen chain', function()
assert.falsy(_M.default().frozen)
end)
end)
end)