-
Notifications
You must be signed in to change notification settings - Fork 170
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
Conditional policy #812
Merged
Merged
Conditional policy #812
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
d3f37c0
Add conditional policy
davidor fe54dad
spec/policy: add specs for Conditional policy
davidor a43f38f
spec/policy/conditional: add initial specs for the engine
davidor 18b72fa
policy/conditional: add first version of Engine
davidor e6f2187
t: add tests for the conditional policy
davidor f516d80
CHANGELOG: add conditional policy
davidor File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
26 changes: 26 additions & 0 deletions
26
gateway/src/apicast/policy/conditional/apicast-policy.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
{ | ||
"$schema": "http://apicast.io/policy-v1/schema#manifest#", | ||
"name": "Conditional", | ||
"summary": "Executes a policy chain conditionally", | ||
"description": [ | ||
"Evaluates a condition, and when it's true, it calls its policy chain." | ||
], | ||
"version": "builtin", | ||
"configuration": { | ||
"type": "object", | ||
"properties": { | ||
"condition": { | ||
"description": "condition to be evaluated", | ||
"type": "string" | ||
}, | ||
"policy_chain": { | ||
"description": "the policy chain to execute when the condition is true", | ||
"type": "array", | ||
"items": { | ||
"type": "object" | ||
} | ||
} | ||
}, | ||
"required": ["condition"] | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
local policy = require('apicast.policy') | ||
local policy_phases = require('apicast.policy').phases | ||
local PolicyChain = require('apicast.policy_chain') | ||
local Engine = require('apicast.policy.conditional.engine') | ||
|
||
local _M = policy.new('Conditional policy') | ||
|
||
local new = _M.new | ||
|
||
local function build_policy_chain(chain) | ||
if not chain then return {} end | ||
|
||
local policies = {} | ||
|
||
for i=1, #chain do | ||
policies[i] = PolicyChain.load_policy( | ||
chain[i].name, | ||
chain[i].version, | ||
chain[i].configuration | ||
) | ||
end | ||
|
||
return PolicyChain.new(policies) | ||
end | ||
|
||
function _M.new(config) | ||
local self = new(config) | ||
self.condition = config.condition or true | ||
self.policy_chain = build_policy_chain(config.policy_chain) | ||
return self | ||
end | ||
|
||
local function condition_is_true(condition) | ||
return Engine.evaluate(condition) | ||
end | ||
|
||
function _M:export() | ||
return self.policy_chain:export() | ||
end | ||
|
||
-- Forward policy phases to chain | ||
for _, phase in policy_phases() do | ||
_M[phase] = function(self, context) | ||
if condition_is_true(self.condition) then | ||
ngx.log(ngx.DEBUG, 'Condition met in conditional policy') | ||
self.policy_chain[phase](self.policy_chain, context) | ||
else | ||
ngx.log(ngx.DEBUG, 'Condition not met in conditional policy') | ||
end | ||
end | ||
end | ||
|
||
-- To avoid calling init and init_worker more than once in the policies | ||
_M.init = function() end | ||
_M.init_worker = function() end | ||
|
||
return _M |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
local _M = {} | ||
|
||
local value_of = { | ||
request_method = function() return ngx.req.get_method() end, | ||
request_host = function() return ngx.var.host end, | ||
request_path = function() return ngx.var.uri end | ||
} | ||
|
||
function _M.evaluate(expression) | ||
local match_attr = ngx.re.match(expression, [[^([\w]+)$]], 'oj') | ||
|
||
if match_attr then | ||
return value_of[match_attr[1]]() | ||
end | ||
|
||
local match_attr_and_value = ngx.re.match(expression, [[^([\w]+) == "([\w/]+)"$]], 'oj') | ||
|
||
if not match_attr_and_value then | ||
return nil, 'Error while parsing the condition' | ||
end | ||
|
||
local entity = match_attr_and_value[1] | ||
local value = match_attr_and_value[2] | ||
|
||
return value_of[entity]() == value | ||
end | ||
|
||
return _M |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
return require('conditional') |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
local ConditionalPolicy = require('apicast.policy.conditional') | ||
local Policy = require('apicast.policy') | ||
local PolicyChain = require('apicast.policy_chain') | ||
local Engine = require('apicast.policy.conditional.engine') | ||
|
||
describe('Conditional policy', function() | ||
local test_policy_chain | ||
local context | ||
local condition = "request_path == '/some_path'" | ||
|
||
before_each(function() | ||
test_policy_chain = PolicyChain.build({}) | ||
|
||
for _, phase in Policy.phases() do | ||
test_policy_chain[phase] = spy.new(function() end) | ||
end | ||
|
||
context = {} | ||
end) | ||
|
||
describe('when the condition is true', function() | ||
before_each(function() | ||
stub(Engine, 'evaluate').returns(true) | ||
end) | ||
|
||
it('forwards the policy phases (except init and init_worker) to the chain', function() | ||
local conditional = ConditionalPolicy.new({ condition = condition }) | ||
|
||
-- .new() will try to load the chain, set it here to avoid that and | ||
-- control which one to use. | ||
conditional.policy_chain = test_policy_chain | ||
|
||
for _, phase in Policy.phases() do | ||
if phase ~= 'init' and phase ~= 'init_worker' then | ||
conditional[phase](conditional, context) | ||
|
||
assert.spy(test_policy_chain[phase]).was_called(1) | ||
assert.spy(test_policy_chain[phase]).was_called_with( | ||
test_policy_chain, | ||
context | ||
) | ||
end | ||
end | ||
end) | ||
end) | ||
|
||
describe('when the condition is false', function() | ||
before_each(function() | ||
stub(Engine, 'evaluate').returns(false) | ||
end) | ||
|
||
it('does not forward the policy phases to the chain', function() | ||
local conditional = ConditionalPolicy.new({ condition = condition }) | ||
conditional.policy_chain = test_policy_chain | ||
|
||
for _, phase in Policy.phases() do | ||
conditional[phase](conditional, context) | ||
|
||
assert.spy(test_policy_chain[phase]).was_not_called() | ||
end | ||
end) | ||
end) | ||
|
||
describe('.export', function() | ||
it('forwards the method to the policy chain', function() | ||
local exported_by_chain = { a = 1, b = 2 } | ||
|
||
stub(test_policy_chain, 'export').returns(exported_by_chain) | ||
|
||
local conditional = ConditionalPolicy.new({ condition = condition }) | ||
conditional.policy_chain = test_policy_chain | ||
|
||
assert.same(exported_by_chain, conditional:export()) | ||
end) | ||
end) | ||
end) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
local Engine = require('apicast.policy.conditional.engine') | ||
|
||
describe('Engine', function() | ||
describe('.evaluate', function() | ||
it('evaluates "request_method"', function() | ||
stub(ngx.req, 'get_method', function () return 'GET' end) | ||
|
||
assert.equals('GET', Engine.evaluate('request_method')) | ||
end) | ||
|
||
it('evaluates "request_host"', function() | ||
ngx.var = { host = 'localhost' } | ||
|
||
assert.equals('localhost', Engine.evaluate('request_host')) | ||
end) | ||
|
||
it('evaluates "request_path"', function() | ||
ngx.var = { uri = '/some_path' } | ||
|
||
assert.equals('/some_path', Engine.evaluate('request_path')) | ||
end) | ||
|
||
it('evaluates "=="', function() | ||
stub(ngx.req, 'get_method', function () return 'GET' end) | ||
|
||
assert.is_true(Engine.evaluate('request_method == "GET"')) | ||
assert.is_false(Engine.evaluate('request_method == "POST"')) | ||
end) | ||
|
||
it('returns nil for expressions that cannot be evaluated', function() | ||
assert.is_nil(Engine.evaluate('request_method <> "GET"')) | ||
end) | ||
end) | ||
end) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
use lib 't'; | ||
use Test::APIcast::Blackbox 'no_plan'; | ||
|
||
run_tests(); | ||
|
||
__DATA__ | ||
|
||
=== TEST 1: Conditional policy calls its chain when the condition is true | ||
In order to test this, we define a conditional policy that only runs the | ||
phase_logger policy when the request path is /log. | ||
We know that the policy outputs "running phase: some_phase" for each of the | ||
phases it runs, so we can use that to verify it was executed. | ||
--- configuration | ||
{ | ||
"services": [ | ||
{ | ||
"id": 42, | ||
"proxy": { | ||
"policy_chain": [ | ||
{ | ||
"name": "apicast.policy.conditional", | ||
"configuration": { | ||
"condition": "request_path == \"/log\"", | ||
"policy_chain": [ | ||
{ | ||
"name": "apicast.policy.phase_logger" | ||
} | ||
] | ||
} | ||
}, | ||
{ | ||
"name": "apicast.policy.echo" | ||
} | ||
] | ||
} | ||
} | ||
] | ||
} | ||
--- request | ||
GET /log | ||
--- response_body | ||
GET /log HTTP/1.1 | ||
--- error_code: 200 | ||
--- no_error_log | ||
[error] | ||
--- error_log chomp | ||
running phase: rewrite | ||
|
||
=== TEST 2: Conditional policy does not call its chain when the condition is false | ||
In order to test this, we define a conditional policy that only runs the | ||
phase_logger policy when the request path is /log. | ||
We know that the policy outputs "running phase: some_phase" for each of the | ||
phases it runs, so we can use that to verify that it was not executed. | ||
--- configuration | ||
{ | ||
"services": [ | ||
{ | ||
"id": 42, | ||
"proxy": { | ||
"policy_chain": [ | ||
{ | ||
"name": "apicast.policy.conditional", | ||
"configuration": { | ||
"condition": "request_path == \"/log\"", | ||
"policy_chain": [ | ||
{ | ||
"name": "apicast.policy.phase_logger" | ||
} | ||
] | ||
} | ||
}, | ||
{ | ||
"name": "apicast.policy.echo" | ||
} | ||
] | ||
} | ||
} | ||
] | ||
} | ||
--- request | ||
GET / | ||
--- response_body | ||
GET / HTTP/1.1 | ||
--- error_code: 200 | ||
--- no_error_log | ||
[error] | ||
running phase: rewrite |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is not exactly what we need.