Adds ability to target specific phase for serverless functions

[nijikokun]

Implements #20

Now you can specifically target and run functions in the header_filter or body_filter phase, or any other phase that Kong Plugins support.

Usage:

Setup

$ curl -i -X POST http://localhost:8001/services/ \
       --data "name=plugin-testing" \
       --data "url=http://httpbin.org/headers"
$ curl -i -X POST http://localhost:8001/services/plugin-testing/routes \
       --data "paths[]=/test"

Check for server header

$ curl -i -X GET http://localhost:8000/test  

Save as custom-fn.lua

kong.response.clear_header("server")

Add pre-function for header_filter phase

$  curl -i -X POST http://localhost:8001/services/plugin-testing/plugins \
       -F "name=pre-function" \
       -F "config.functions=@custom-fn.lua" \
       -F "config.phase=header_filters"

No more server header

$ curl -i -X GET http://localhost:8000/test  

[Tieske]

can't we specify the phase on a per-function basis? Now all the functions run in the same phase.

It's harder to do backward compatible though.

[nijikokun]

Much harder to do backwards compatibility. I like the idea though.

[Tieske]

Maybe not; if we add it as a second array in the config. Then all entries in the old array can be copied over to the new array under "access" entry.

Mark the arrays in the schema as either one or the other can be provided.

In some future version we drop support for the old array.

[hishamhm]

@nijikokun if you'd like to give this a shot for Kong 2.0, now it's a window of opportunity for API changes if you think it's worth it for greater flexibility.

[aalmazanarbs]

Are you planning to merge this changes for kong 1.x? We need to execute custom Lua function in header_filter or body_filter phase

[hishamhm]

@eskerda You mentioned you had a rebased version of this PR? Any chance you could incorporate the changes here?

[eskerda]

@hishamhm This is the rebased version. Trying to allocate some time to look at this CI failure.

The other changes proposed are at https://github.com/Kong/kong-plugin-serverless-functions/tree/feat/multi-phase-take-two

Probably we want to incorporate a nicer version of e7ff2d5 and most definitely we do not want aebc8b3

[eskerda]

@nijikokun (cannot assign you for review it seems) cleaned up the changes from https://github.com/Kong/kong-plugin-serverless-functions/tree/feat/multi-phase-take-two

• includes e7ff2d5 cleaned up
• Did not include any of the api overloading wizardy from aebc8b3
• adds tests for both syntaxes. The way I did it is a bit overkill (it dupes all tests for each syntax), but unless there are strong feelings against it, is better than not testing it. I would be in favor of deprecating the old syntax for a later release

@Tieske since this is an old PR. Some context:

can't we specify the phase on a per-function basis? Now all the functions run in the same phase.
It's harder to do backward compatible though.

this new syntax I think is compatible with both old syntax and adds support for targeting multiple phases in the same config

[eskerda]

@Tieske :

• removed config.phase and defaulted config.functions to run on the access phase
• made config.functions and config.access mutually exclusive
• using config.functions throws a deprecation warning on the logs
• added a couple of more tests regarding that

I really didn't find a nicer (easy) way to rework the specs making sure that both old and new features work. Since we are going to deprecate config.functions at some point, we will be able to remove that part from the specs. For that, I think that the current overkill solution and breaking the indentation is good, plus it will be a matter of just removing these lines and doing small changes. WDYT?

[Tieske]

I think we can still cleanup the core of the handler to be cleaner code. Need to think about that.

[eskerda]

I think we can still cleanup the core of the handler to be cleaner code. Need to think about that.

True. I think I will give the handler core a few more spins

[Tieske]

there is a bigger underlying problem with the whole plugin. If a configured function does an early exit, then it will never be cached, which will make the plugin be recompiled and executed on every invocation.

In case you use a plugin with upvalues, it means even that the upvalues will be lost. The way I see it there are 2 options:

• drop support for upvalues
• drop support for the original format with upvalues

The 2 cannot be combined properly, I think, but it's hard to reason about.

[Tieske]

I added some config files and rebased it on master. Also fixed the bad test that displays the behaviour I described (bug in the test failed it to show).

[Tieske]

see #27 for the fix (complete rewrite btw)