Skip to content

Commit

Permalink
Policy::Caching: issues when can't connect to backend.
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Eloy Coto <[email protected]>
  • Loading branch information
eloycoto committed Feb 18, 2020
1 parent 9ac0e2d commit 3616bc4
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 2 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 8 additions & 2 deletions gateway/src/apicast/policy/caching/caching.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)

Expand All @@ -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)

Expand Down
10 changes: 10 additions & 0 deletions spec/policy/caching/caching_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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)
Expand Down

0 comments on commit 3616bc4

Please sign in to comment.