From 3616bc4f92cc0515aa35ee7a681e01ab6bdecf65 Mon Sep 17 00:00:00 2001 From: Eloy Coto Date: Wed, 12 Feb 2020 15:03:29 +0100 Subject: [PATCH] Policy::Caching: issues when can't connect to backend. New HTTP-NG coroutines return 0 when can't connect, so the caching policy fails on the second request used, because the cache status was updated with the an invalid status This changes makes sure that the response code is a valid one, so the caching status will report correctly. Fix: THREESCALE-4471 Reported-by:Jakub Smadis Signed-off-by: Eloy Coto --- CHANGELOG.md | 6 ++++++ gateway/src/apicast/policy/caching/caching.lua | 10 ++++++++-- spec/policy/caching/caching_spec.lua | 10 ++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dbc60c672..180b02385 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Fixed: + +- Check status is bigger than zero on caching policy [THREESCALE-4471](https://issues.jboss.org/browse/THREESCALE-4471) [PR #1163](https://github.com/3scale/APIcast/pull/1163) + +## [3.7.0-alpha1] + ### Added - Now the configuration of the issuer is cached to avoid flip-flop issues when OIDC connectivity fails. [THREESCALE-3809](https://issues.jboss.org/browse/THREESCALE-3809) [PR #1141](https://github.com/3scale/APIcast/pull/1141) diff --git a/gateway/src/apicast/policy/caching/caching.lua b/gateway/src/apicast/policy/caching/caching.lua index 564428b05..d537e51c5 100644 --- a/gateway/src/apicast/policy/caching/caching.lua +++ b/gateway/src/apicast/policy/caching/caching.lua @@ -22,6 +22,12 @@ local _M = policy.new('Caching policy', 'builtin') local new = _M.new +--- On connect issues, the status returned is 0, so need to validate that the +--status is greater than 0 and less than 500 +local function is_valid_status(status) + return status and status > 0 and status < 500 +end + 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) @@ -36,7 +42,7 @@ end local function resilient_handler(cache, cached_key, response, ttl) local status = response.status - if status and status < 500 then + if is_valid_status(status) then ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key, ' status: ', status, ', ttl: ', ttl) @@ -57,7 +63,7 @@ end local function allow_handler(cache, cached_key, response, ttl) local status = response.status - if status and status < 500 then + if is_valid_status(status) then ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key, ' status: ', status, ', ttl: ', ttl) diff --git a/spec/policy/caching/caching_spec.lua b/spec/policy/caching/caching_spec.lua index 6f49e0dde..5410297e0 100644 --- a/spec/policy/caching/caching_spec.lua +++ b/spec/policy/caching/caching_spec.lua @@ -78,6 +78,11 @@ describe('Caching policy', function() cache_handler(cache, 'a_key', { status = 500 }, nil) assert.equals(200, cache:get('a_key')) end) + + it('does not cached connect issues', function() + cache_handler(cache, 'a_key', { status = 0 }, nil) + assert.equals(nil, cache:get('a_key')) + end) end) describe('when configured as allow', function() @@ -97,6 +102,11 @@ describe('Caching policy', function() assert.equals(403, cache:get('a_key')) end) + it('does not cached connect issues', function() + cache_handler(cache, 'a_key', { status = 0 }, nil) + assert.equals(200, cache:get('a_key')) + end) + describe('and backend returns 5XX', function() it('does not invalidate the cache entry if there was a 4XX', function() cache:set('a_key', 403)