Exiting plugins (access phase): which PDK function should I use?


#1

Hi everyone,

I’m migrating to version 0.14.1 and thus also trying to use the PDK for my custom plugins. I’m a bit confused when looking at the exit path (in the access phase only) of the plugins bundled in Kong as:

When applicable, they have adopted the new “delayed response” pattern (e.g. AWS Lambda plugin and Request Termination plugin), but they are not yet using the Kong PDK kong.response.exit function

My understanding is that, for my nominal use cases (in case everything goes fine in my plugin :stuck_out_tongue: ), instead of duplicating the code found in AWS Lambda and Request Termination plugins (aka the flush and send methods), I can directly use the kong.response.exit PDK function, and everything is done in a single line of code for me: great!

However, what about the error cases of my plugin, where I used to call the legacy/non-public kong.tools.responses.send_HTTP_* function ? I don’t see any of the bundled plugins replacing such calls with the “delayed response” pattern (i.e. they are still using the kong.tools.responses.send_HTTP_* function) => is there any reason for that?

Should I stay with this non-public function or should I use kong.response.exit function for error cases as well ?

Thanks for your help !


#2

Hi,

I can directly use the kong.response.exit PDK function, and everything is done in a single line of code for me: great!

Indeed. Yay! :smiley:

what about the error cases of my plugin, where I used to call the legacy/non-public kong.tools.responses.send_HTTP_* function ?

Why not calling kong.response.exit(4xx)? What is the difference you are seeing between the nominal and non-nominal use cases?

I don’t see any of the bundled plugins replacing such calls with the “delayed response” pattern […] is there any reason for that?

We did not have time to update all of our plugins to use the PDK. Btw, we would very much welcome contributions helping us in this transition!

I advise that you give the PDK a try (i.e. kong.response.exit and other niceties from https://docs.konghq.com/0.14.x/pdk); we hope that it covers most of your use cases. If not, please let us know :slight_smile:

Best,


#3

Why not calling kong.response.exit(4xx)? What is the difference you are seeing between the nominal and non-nominal use cases?

Well, one returns a data and the other one not… but you’re right, that the same !
So, I used kong.response.exit everywhere, and it is working fine… except in the piece of code corresponding to a plugin api: in this case, I’m receiving an exception

2018/09/11 12:54:10 [error] 39#0: *569 lua coroutine: runtime error: /usr/local/share/lua/5.1/kong/pdk/private/phases.lua:64: no phase in kong.ctx.core.phase
stack traceback:
coroutine 0:
[C]: in function 'error'
/usr/local/share/lua/5.1/kong/pdk/private/phases.lua:64: in function 'check_phase'
/usr/local/share/lua/5.1/kong/pdk/response.lua:544: in function </usr/local/share/lua/5.1/kong/pdk/response.lua:543>
coroutine 1:
[C]: in function 'resume'
/usr/local/share/lua/5.1/lapis/application.lua:393: in function 'handler'
/usr/local/share/lua/5.1/lapis/application.lua:130: in function 'resolve'
/usr/local/share/lua/5.1/lapis/application.lua:161: in function </usr/local/share/lua/5.1/lapis/application.lua:159>
[C]: in function 'xpcall'
/usr/local/share/lua/5.1/lapis/application.lua:159: in function 'dispatch'
/usr/local/share/lua/5.1/lapis/nginx.lua:215: in function 'serve_admin_api'
content_by_lua(nginx-kong.conf:246):2: in function <content_by_lua(nginx-kong.conf:246):1>, client: 10.129.2.1, server: kong_admin

So for this use case, I’m still using return helpers.responses.send_HTTP_OK() instead.

Regarding the PDK usage in general, I tried to replace all references to ngx. inside the code of my plugins. OK for for getting data from the request and setting on the response; OK for setting data on the upstream request and getting data from the upstream response; here are the ngx. usages that remain in my code:

  • The HTTP error codes: ngx.OK, ngx.HTTP_UNAUTHORIZED…, because I still prefer using constants instead of raw numbers.
  • ngx.redirect for redirecting the incoming call
  • ngx.var.<proprietary variable>: ok, that’s fully proprietary variables; I’m not expecting the PDK to give me a shortcut to that :stuck_out_tongue:
  • And then some other misc functions that are most likely not eligible to be provided in the PDK: ngx.encode_args (I don’t know if it would be better for me to use the function present in kong.utils.lua…), ngx.decode_base64, ngx.time, ngx.re

#4

Well, one returns a data and the other one not…

Would you mind clarifying what you meant by this?

I’m receiving an exception

Are you calling the PDK from an api.lua file (i.e. from the Admin API)? If so, that is unfortunately not yet supported by the PDK, sorry.

here are the ngx. usages that remain in my code:

We do have plans to add more utilities to the PDK toolbelt, but some operations will remain the domain of the lower-level ngx_lua API. For examples:

  • status codes constants could be added to the PDK (good point)
  • ngx.redirect could become part of the PDK (hopwever, it is low-level enough that we may not add it)
  • ngx.var will likely not be part of the PDK
  • utility functions such as form-encoded serialization, base64 encoding, JSON encoding, timer scheduling, and a PCRE, are all on the roadmap to be included (and standardized) by the PDK

#5

Would you mind clarifying what you meant by this?

Well, I just wanted to say that in my nominal cases, response are including a body while in error case, they are not.

Are you calling the PDK from an api.lua file

Yes

We do have plans to add more utilities to the PDK toolbelt, but some operations will remain the domain of the lower-level ngx_lua API. For examples: […]

Your proposals look good to me !

Note also that I’m also using kong.dao.errors in the self_check function of the Schema files, to report errors. Might be interesting to have a PDK stuff for that as well.


#6

@thibaultcha, why not as well having getters on kong.service.request of the PDK (get_scheme, get_path, get_query…)?
Today, there are only setters, which forces plugins to use, for example, ngx.var.upstream_uri to know what is the expected full upstream path.


#7

@pamiel Good point! We’ll keep this in mind. We would also welcome PRs contributing those getters, if you want to give it a try (can be implemented gradually).