feat(kong-ngx-build) add lua-kong-nginx-module patch and build suppor… #26
Conversation
…t and enable it by default for core >= 1.13.6
|
trying to think what a test could look like for this 🤔 As is this looks fine a test would be nice to ensure future us doesn't break anything without noticing |
kong-ngx-build
Outdated
| @@ -254,6 +283,14 @@ main() { | |||
| || fatal "failed to apply patch: $patch_file" | |||
| done | |||
|
|
|||
| if [ $KONG_NGINX_MODULE != 0 ]; then | |||
| for patch_file in $(ls -1 $DOWNLOAD_CACHE/lua-kong-nginx-module/patches/$OPENRESTY_VER/*.patch); do | |||
There was a problem hiding this comment.
so when running this with an OpenResty version for which lua-kong-nginx-module does not have yet a patch, the $(ls ...) will return empty and this will be skipped silently.
If the user explicitly gave a --kong-nginx-module argument and an --openresty version, I think this should fail if the patch for that specific OpenResty version does not exist.
In our specific case, what I could see happening is: if an OpenResty bump happens before a patch is produced, then this would cause openresty-build-tools to happily build without the patch even though the patch was requested. (And things would only fail further down the road when the Kong test suite fails due to the lack of features provided by the patch.)
I think this should be changed so that whenever we enter this branch, we ensure the patch was in fact applied.
There was a problem hiding this comment.
Agreed. Actually since we moved everything into openresty-patches anyway, it should be alright to skip this check for now. We maintain the openresty-patches repo so the user wouldn't accidentally forgot to apply the patch for lua-kong-nginx-module.
We have a card that will address this better in the future as well, that will make sure lua-kong-nginx-module throws a compile error if patches are not applied properly (for all the patches, not only those of lua-kong-nginx-module).
|
@hutchic Tests were added in 46434bd. Thanks @thibaultcha for the hard work getting the test framework started. |
dndx
force-pushed
the
feat/lua-kong-nginx-module
branch
from
46434bd
to
4ede804
Compare
dndx
force-pushed
the
feat/lua-kong-nginx-module
branch
from
4ede804
to
a52e9fd
Compare
…t and enable
it by default for core >= 1.13.6