From eb351b860e3ed1d7ccd4f5e742241c8ecd18e44b Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Tue, 9 Jan 2018 17:21:46 +0100 Subject: [PATCH 01/10] Add caching policy This is based on the current CacheHandler module. Many parts of the code are duplicated, but this duplication will disappear once we split the current Apicast policy into smaller ones. This does not break the current Apicast cache. Users can still use the APICAST_BACKEND_CACHE_HANDLER to configure the cache. --- gateway/src/apicast/policy/caching/policy.lua | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 gateway/src/apicast/policy/caching/policy.lua diff --git a/gateway/src/apicast/policy/caching/policy.lua b/gateway/src/apicast/policy/caching/policy.lua new file mode 100644 index 000000000..d14916175 --- /dev/null +++ b/gateway/src/apicast/policy/caching/policy.lua @@ -0,0 +1,74 @@ +--- Caching policy +-- Configures a cache for the authentication calls against the 3scale backend. +-- The 3scale backend can authorize (status code = 200) and deny (status code = +-- 4xx) calls. When it fails, it returns a 5xx code. +-- This policy support two kinds of caching: +-- - Strict: it only caches authorized calls. Denied and failed calls +-- invalidate the cache entry. +-- - Resilient: caches authorized and denied calls. Failed calls do not +-- invalidate the cache. This allows us to authorize and deny calls +-- according to the result of the last request made even when backend is +-- down. + +local policy = require('apicast.policy') +local _M = policy.new('Caching policy') + +local new = _M.new + +local function strict_handler(cache, cached_key, response, ttl) + if response.status == 200 then + ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key, ', ttl: ', ttl) + cache:set(cached_key, 200, ttl or 0) + else + ngx.log(ngx.NOTICE, 'apicast cache delete key: ', cached_key, + ' cause status ', response.status) + cache:delete(cached_key) + end +end + +local function resilient_handler(cache, cached_key, response, ttl) + local status = response.status + + if status and status < 500 then + ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key, + ' status: ', status, ', ttl: ', ttl) + + cache:set(cached_key, status, ttl or 0) + end +end + +local handlers = { + resilient = resilient_handler, + strict = strict_handler +} + +local function handler(config) + if not config.caching_type then + ngx.log(ngx.ERR, 'Caching type not specified, falling back to strict.') + return handlers.strict + end + + local res = handlers[config.caching_type] + + if not res then + ngx.log(ngx.ERR, 'Invalid caching type, falling back to strict.') + res = handlers.strict + end + + return res +end + +--- Initialize a Caching policy. +-- @tparam[opt] table config +-- @field caching_type Caching type (strict, resilient) +function _M.new(config) + local self = new() + self.cache_handler = handler(config or {}) + return self +end + +function _M:rewrite(context) + context.cache_handler = self.cache_handler +end + +return _M From fb3215a8ea930df376f3ef3f5af1c2c349e44d90 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Tue, 9 Jan 2018 17:25:39 +0100 Subject: [PATCH 02/10] policy/caching: add json schema --- gateway/src/apicast/policy/caching/schema.json | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 gateway/src/apicast/policy/caching/schema.json diff --git a/gateway/src/apicast/policy/caching/schema.json b/gateway/src/apicast/policy/caching/schema.json new file mode 100644 index 000000000..a979c5a0e --- /dev/null +++ b/gateway/src/apicast/policy/caching/schema.json @@ -0,0 +1,11 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "Caching policy configuration", + "type": "object", + "properties": { + "exit": { + "type": "caching_type", + "enum": ["resilient", "strict"] + } + } +} From c110c0c2c952422ba1a0715e17446baa385a02a9 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Tue, 9 Jan 2018 17:26:00 +0100 Subject: [PATCH 03/10] t: add tests for caching policy --- t/apicast-policy-caching.t | 121 +++++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 t/apicast-policy-caching.t diff --git a/t/apicast-policy-caching.t b/t/apicast-policy-caching.t new file mode 100644 index 000000000..9b1e52e57 --- /dev/null +++ b/t/apicast-policy-caching.t @@ -0,0 +1,121 @@ +use lib 't'; +use Test::APIcast::Blackbox 'no_plan'; + +repeat_each(1); +run_tests(); + +__DATA__ + +=== TEST 1: Caching policy configured as resilient +When the cache is configured as 'resilient', cache entries are not deleted when +backend returns a 500 error. This means that if we get a 200, and then +backend fails and starts returning 500, we will still have the 200 cached +and we'll continue authorizing requests. +In order to test this, we configure our backend so the first request returns +200, and all the others 502. +--- configuration +{ + "services": [ + { + "id": 42, + "backend_version": 1, + "backend_authentication_type": "service_token", + "backend_authentication_value": "token-value", + "proxy": { + "policy_chain": [ + { + "name": "apicast.policy.caching", + "configuration": { "caching_type": "resilient" } + }, + { + "name": "apicast.policy.apicast" + } + ], + "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/", + "proxy_rules": [ + { "pattern": "/", "http_method": "GET", "metric_system_name": "hits", "delta": 2 } + ] + } + } + ] +} +--- backend + location /transactions/authrep.xml { + content_by_lua_block { + local test_counter = ngx.shared.test_counter or 0 + if test_counter == 0 then + ngx.shared.test_counter = test_counter + 1 + ngx.exit(200) + else + ngx.shared.test_counter = test_counter + 1 + ngx.exit(502) + end + } + } +--- upstream + location / { + echo 'yay, api backend'; + } +--- request eval +["GET /test?user_key=foo", "GET /foo?user_key=foo", "GET /?user_key=foo"] +--- response_body eval +["yay, api backend\x{0a}", "yay, api backend\x{0a}", "yay, api backend\x{0a}"] +--- error_code eval +[ 200, 200, 200 ] + +=== TEST 2: Caching policy configured as strict +When the cache is configured as 'strict', entries are removed when backend +denies the authorization with a 4xx or when it fails with a 5xx. +In order to test this, we use a backend that returns 200 on the first call, and +502 on the rest. We need to test that the first call is authorized, the +second is too because it will be cached, and the third will not be authorized +because the cache was cleared in the second call. +--- configuration +{ + "services": [ + { + "id": 42, + "backend_version": 1, + "backend_authentication_type": "service_token", + "backend_authentication_value": "token-value", + "proxy": { + "policy_chain": [ + { + "name": "apicast.policy.caching", + "configuration": { "caching_type": "strict" } + }, + { + "name": "apicast.policy.apicast" + } + ], + "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/", + "proxy_rules": [ + { "pattern": "/", "http_method": "GET", "metric_system_name": "hits", "delta": 2 } + ] + } + } + ] +} +--- backend + location /transactions/authrep.xml { + content_by_lua_block { + local test_counter = ngx.shared.test_counter or 0 + if test_counter == 0 then + ngx.shared.test_counter = test_counter + 1 + ngx.exit(200) + else + ngx.shared.test_counter = test_counter + 1 + ngx.exit(502) + end + } + } +--- upstream + location / { + echo 'yay, api backend'; + } +--- request eval +["GET /test?user_key=foo", "GET /foo?user_key=foo", "GET /?user_key=foo"] +--- response_body eval +["yay, api backend\x{0a}", "yay, api backend\x{0a}", "Authentication failed"] +--- error_code eval +[ 200, 200, 403 ] From add21123c1c4221c03fc4642cd965429b5ff1fb9 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Sun, 14 Jan 2018 20:30:56 +0100 Subject: [PATCH 04/10] spec: add tests for caching policy --- spec/policy/caching/policy_spec.lua | 69 +++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 spec/policy/caching/policy_spec.lua diff --git a/spec/policy/caching/policy_spec.lua b/spec/policy/caching/policy_spec.lua new file mode 100644 index 000000000..f55253ef2 --- /dev/null +++ b/spec/policy/caching/policy_spec.lua @@ -0,0 +1,69 @@ +local resty_lrucache = require('resty.lrucache') + +describe('policy', function() + describe('.export', function() + describe('when configured as strict', function() + local caching_policy + local cache + local ctx -- the caching policy will add the handler here + + before_each(function() + local config = { caching_type = 'strict' } + caching_policy = require('apicast.policy.caching').new(config) + ctx = { } + caching_policy:rewrite(ctx) + cache = resty_lrucache.new(1) + end) + + it('caches authorized requests', function() + ctx.cache_handler(cache, 'a_key', { status = 200 }, nil) + assert.equals(200, cache:get('a_key')) + end) + + it('clears the cache entry for a request when it is denied', function() + cache:set('a_key', 200) + + ctx.cache_handler(cache, 'a_key', { status = 403 }, nil) + assert.is_nil(cache:get('a_key')) + end) + + it('clears the cache entry for a request when it fails', function() + cache:set('a_key', 200) + + ctx.cache_handler(cache, 'a_key', { status = 500 }, nil) + assert.is_nil(cache:get('a_key')) + end) + end) + + describe('when configured as resilient', function() + local caching_policy + local cache + local ctx -- the caching policy will add the handler here + + before_each(function() + local config = { caching_type = 'resilient' } + caching_policy = require('apicast.policy.caching').new(config) + ctx = { } + caching_policy:rewrite(ctx) + cache = resty_lrucache.new(1) + end) + + it('caches authorized requests', function() + ctx.cache_handler(cache, 'a_key', { status = 200 }, nil) + assert.equals(200, cache:get('a_key')) + end) + + it('caches denied requests', function() + ctx.cache_handler(cache, 'a_key', { status = 403 }, nil) + assert.equals(403, cache:get('a_key')) + end) + + it('does not clear the cache entry for a request when it fails', function() + cache:set('a_key', 200) + + ctx.cache_handler(cache, 'a_key', { status = 500 }, nil) + assert.equals(200, cache:get('a_key')) + end) + end) + end) +end) From 7e2aaef02eb9f5e22c4b89ff364721899add5c64 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Mon, 15 Jan 2018 10:32:14 +0100 Subject: [PATCH 05/10] policy/caching: add 'none' cache type --- gateway/src/apicast/policy/caching/policy.lua | 18 ++++++++++++------ gateway/src/apicast/policy/caching/schema.json | 2 +- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/gateway/src/apicast/policy/caching/policy.lua b/gateway/src/apicast/policy/caching/policy.lua index d14916175..2329e4cd7 100644 --- a/gateway/src/apicast/policy/caching/policy.lua +++ b/gateway/src/apicast/policy/caching/policy.lua @@ -2,13 +2,14 @@ -- Configures a cache for the authentication calls against the 3scale backend. -- The 3scale backend can authorize (status code = 200) and deny (status code = -- 4xx) calls. When it fails, it returns a 5xx code. --- This policy support two kinds of caching: +-- This policy support three kinds of caching: -- - Strict: it only caches authorized calls. Denied and failed calls -- invalidate the cache entry. -- - Resilient: caches authorized and denied calls. Failed calls do not -- invalidate the cache. This allows us to authorize and deny calls -- according to the result of the last request made even when backend is -- down. +-- - None: disables caching. local policy = require('apicast.policy') local _M = policy.new('Caching policy') @@ -37,22 +38,27 @@ local function resilient_handler(cache, cached_key, response, ttl) end end +local function disabled_cache_handler() + ngx.log(ngx.DEBUG, 'Caching is disabled. Skipping cache handler.') +end + local handlers = { resilient = resilient_handler, - strict = strict_handler + strict = strict_handler, + none = disabled_cache_handler } local function handler(config) if not config.caching_type then - ngx.log(ngx.ERR, 'Caching type not specified, falling back to strict.') - return handlers.strict + ngx.log(ngx.ERR, 'Caching type not specified. Disabling cache.') + return handlers.none end local res = handlers[config.caching_type] if not res then - ngx.log(ngx.ERR, 'Invalid caching type, falling back to strict.') - res = handlers.strict + ngx.log(ngx.ERR, 'Invalid caching type. Disabling cache.') + res = handlers.none end return res diff --git a/gateway/src/apicast/policy/caching/schema.json b/gateway/src/apicast/policy/caching/schema.json index a979c5a0e..0f1b5bb8a 100644 --- a/gateway/src/apicast/policy/caching/schema.json +++ b/gateway/src/apicast/policy/caching/schema.json @@ -5,7 +5,7 @@ "properties": { "exit": { "type": "caching_type", - "enum": ["resilient", "strict"] + "enum": ["resilient", "strict", "none"] } } } From 661b9df04ee0b38a11dcdbe6712177f30374c6a3 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Mon, 15 Jan 2018 10:32:22 +0100 Subject: [PATCH 06/10] policy/spec: add tests with disabled cache --- spec/policy/caching/policy_spec.lua | 44 ++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/spec/policy/caching/policy_spec.lua b/spec/policy/caching/policy_spec.lua index f55253ef2..1199d6b84 100644 --- a/spec/policy/caching/policy_spec.lua +++ b/spec/policy/caching/policy_spec.lua @@ -1,7 +1,30 @@ local resty_lrucache = require('resty.lrucache') describe('policy', function() - describe('.export', function() + describe('.new', function() + local cache = resty_lrucache.new(1) + + it('disables caching when caching type is not specified', function() + local caching_policy = require('apicast.policy.caching').new({}) + local ctx = {} + caching_policy:rewrite(ctx) + + ctx.cache_handler(cache, 'a_key', { status = 200 }, nil) + assert.is_nil(cache:get('a_key')) + end) + + it('disables caching when invalid caching type is specified', function() + local config = { caching_type = 'invalid_caching_type' } + local caching_policy = require('apicast.policy.caching').new(config) + local ctx = {} + caching_policy:rewrite(ctx) + + ctx.cache_handler(cache, 'a_key', { status = 200 }, nil) + assert.is_nil(cache:get('a_key')) + end) + end) + + describe('.access', function() describe('when configured as strict', function() local caching_policy local cache @@ -65,5 +88,24 @@ describe('policy', function() assert.equals(200, cache:get('a_key')) end) end) + + describe('when disabled', function() + local caching_policy + local cache + local ctx + + setup(function() + local config = { caching_type = 'none' } + caching_policy = require('apicast.policy.caching').new(config) + ctx = {} + caching_policy:rewrite(ctx) + cache = resty_lrucache.new(1) + end) + + it('does not cache anything', function() + ctx.cache_handler(cache, 'a_key', { status = 200 }, nil) + assert.is_nil(cache:get('a_key')) + end) + end) end) end) From 0fd95c5dce4d22ba3a7230d4b8b9ecaedd24f323 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Mon, 15 Jan 2018 10:32:25 +0100 Subject: [PATCH 07/10] t/apicast-policy-caching: test disabled cache --- t/apicast-policy-caching.t | 56 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/t/apicast-policy-caching.t b/t/apicast-policy-caching.t index 9b1e52e57..472ca3ed3 100644 --- a/t/apicast-policy-caching.t +++ b/t/apicast-policy-caching.t @@ -119,3 +119,59 @@ because the cache was cleared in the second call. ["yay, api backend\x{0a}", "yay, api backend\x{0a}", "Authentication failed"] --- error_code eval [ 200, 200, 403 ] + +=== TEST 3: Caching disabled +When the cache is configured as 'none', all the authorizations are performed +synchronously. +In order to test this, we configure our backend to authorize even requests, and +deny the odd ones. We need to check that we got a 200 in even requests and an +auth error in the odd ones. +--- configuration +{ + "services": [ + { + "id": 42, + "backend_version": 1, + "backend_authentication_type": "service_token", + "backend_authentication_value": "token-value", + "proxy": { + "policy_chain": [ + { + "name": "apicast.policy.caching", + "configuration": { "caching_type": "none" } + }, + { + "name": "apicast.policy.apicast" + } + ], + "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/", + "proxy_rules": [ + { "pattern": "/", "http_method": "GET", "metric_system_name": "hits", "delta": 2 } + ] + } + } + ] +} +--- backend + location /transactions/authrep.xml { + content_by_lua_block { + local test_counter = ngx.shared.test_counter or 0 + if test_counter % 2 == 0 then + ngx.shared.test_counter = test_counter + 1 + ngx.exit(200) + else + ngx.shared.test_counter = test_counter + 1 + ngx.exit(502) + end + } + } +--- upstream + location / { + echo 'yay, api backend'; + } +--- request eval +["GET /?user_key=foo", "GET /?user_key=foo", "GET /?user_key=foo", "GET /?user_key=foo"] +--- response_body eval +["yay, api backend\x{0a}", "Authentication failed", "yay, api backend\x{0a}", "Authentication failed"] +--- error_code eval +[ 200, 403, 200, 403 ] From 3b00ece01019a232b70d3c4413a70595ac3cd1e6 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Mon, 15 Jan 2018 12:22:25 +0100 Subject: [PATCH 08/10] CHANGELOG: add caching policy entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9e3e7fad..034fc30b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - URL rewriting policy [PR #529](https://github.com/3scale/apicast/pull/529) - Liquid template can find files in current folder too [PR #533](https://github.com/3scale/apicast/pull/533) - `bin/apicast` respects `APICAST_OPENRESTY_BINARY` and `TEST_NGINX_BINARY` environment [PR #540](https://github.com/3scale/apicast/pull/540) +- Caching policy [PR #546](https://github.com/3scale/apicast/pull/546) ## Fixed From 4cf52d428d3eeac53ae871644992ea0dada46345 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Mon, 15 Jan 2018 15:04:27 +0100 Subject: [PATCH 09/10] cache_handler: remove empty log part --- gateway/src/apicast/backend/cache_handler.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gateway/src/apicast/backend/cache_handler.lua b/gateway/src/apicast/backend/cache_handler.lua index 475a3b79c..b3aab6bac 100644 --- a/gateway/src/apicast/backend/cache_handler.lua +++ b/gateway/src/apicast/backend/cache_handler.lua @@ -40,7 +40,7 @@ function _M.handlers.strict(cache, cached_key, response, ttl) -- so to not write the cache twice lets write it just in authorize if fetch_cached_key(cached_key) ~= cached_key then - ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key, ', ttl: ', ttl, ' sub: ') + ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key, ', ttl: ', ttl) cache:set(cached_key, 200, ttl or 0) end else From 2016d41eb4046b6892c8a7d10e1c804db10ff0d1 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 17 Jan 2018 12:13:39 +0100 Subject: [PATCH 10/10] policy/apicast: override proxy's cache_handler if received via context --- gateway/src/apicast/policy/apicast/policy.lua | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/gateway/src/apicast/policy/apicast/policy.lua b/gateway/src/apicast/policy/apicast/policy.lua index da6e45294..ade7dba36 100644 --- a/gateway/src/apicast/policy/apicast/policy.lua +++ b/gateway/src/apicast/policy/apicast/policy.lua @@ -45,6 +45,11 @@ function _M:rewrite(context) -- because the module is reloaded and has to be configured again local p = context.proxy + + if context.cache_handler then + p.cache_handler = context.cache_handler + end + p.set_upstream(context.service) ngx.ctx.proxy = p end