Return null marker for caching even when no row matches the requirements

[shashankmehra]

Summary

When loading a plugin's configuration, if the database query returns some rows but none of the returned rows match in api_id and consumer_id, the function returns nil. In which case the query result isnt cached. This happens for global plugins:
When plugins are first loaded get_or_set is called on global plugins like plugins:basic-auth. If there exists no global entry for this plugin, but there is a api/consumer specific entry some rows are returned. Even though rows are returned, none of them will match resulting in the function returning nil and the result not being cached. On every request to kong, kong tries to get_or_set plugins:basic-auth but fails bringing down throughput. This also causes kong to start returning 500 on every request when database is down.

Full changelog

• Hotfix: Make sure plugin iterator call back returns non-nil response for proper caching

Issues resolved

I found no reported issue related to this.

[Tieske]

Thx! Good catch. Question; have you checked whether the inserted sentinel value will be properly invalidated when an actual entry is created for it?

[shashankmehra]

I did the following test -
Setup: Two kong machines in a cluster using postgres: K1 and K2

1. There is already a plugin entry added plugins:custom_plugin:<api_id>. But plugins:custom_plugin is "nil" cached.
2. Using Admin API of K1 I added a global plugin entry for custom_plugin which should have overwritten plugins:custom_plugin.
3. Using Admin API of K2 I fetched the list of plugins and the result had the entry for global custom_plugin.

[Tieske]

The code here; https://github.com/Mashape/kong/blob/master/spec/02-integration/04-core/02-hooks_spec.lua#L271-L487

Does not have any tests for invalidating plugins upon 'adding'. That needs fixing. If we insert a sentinel value for a database-miss, we must make sure that it gets invalidated when an entry is added later. Might be that the behaviour is ok right now, but we need the tests to make sure it stays that way.

[Tieske]

superseded by #1914