Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(deps) bump OpenResty compatibility #2489

Merged
merged 4 commits into from
Jul 25, 2017
Merged

Conversation

thibaultcha
Copy link
Member

@thibaultcha thibaultcha commented May 2, 2017

Summary

Bump OpenResty compatibility to 1.11.2.3 and fix a few issues along the way, especially regarding the recent LuaJIT string hashing updates.

Full changelog

  • bump OpenResty compat up to 1.11.2.3
  • update JWT tests fixtures after OR 1.11.2.3 bump
  • hard-code plugins priorities as of 1.11.2.2 to the best of my knowledge

TODO

  • waiting on a release from Lapis to ensure routes prioritization according to the number of variables, for the upstreams/{upstream}/targets/active route.

@thibaultcha thibaultcha changed the title Chore/deps bump openresty chore(deps) bump OpenResty compatibility May 2, 2017
@thibaultcha thibaultcha added the pr/wip A work in progress PR opened to receive feedback label May 2, 2017
@thibaultcha thibaultcha modified the milestone: 0.11.0 May 19, 2017
@thibaultcha
Copy link
Member Author

This is still waiting for a Lapis release. Lapis is not getting a release because apparently it needs to be stable on cqueues or something first. Yet it is currently *broken on OpenResty 1.11.2.2 for now, due to some LuaJIT/lua-cjson incompatibilities.

At this point, I think we need to consider forking it and uploading the current master branch of Lapis onto our own LuaRocks mashape-lapis rock.

@thibaultcha thibaultcha self-assigned this May 30, 2017
local prefix = ""
local len

for pref in str:gmatch("\n(%s+)") do
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly inconsistent usage of the string library... I could update that.

@thibaultcha thibaultcha modified the milestones: 0.11.0rc2, 0.11.0 Jul 6, 2017
@thibaultcha thibaultcha force-pushed the chore/deps-bump-openresty branch from 4fbc402 to d67de02 Compare July 12, 2017 20:45
@thibaultcha thibaultcha added pr/status/please review pr/wip A work in progress PR opened to receive feedback and removed pr/wip A work in progress PR opened to receive feedback pr/status/please review labels Jul 13, 2017
@Tieske
Copy link
Member

Tieske commented Jul 18, 2017

since Penlight got an update, should we reenable 52 compatibility? see also #2375 (comment)

@thibaultcha thibaultcha force-pushed the chore/deps-bump-openresty branch 3 times, most recently from 32c895c to 9ea7728 Compare July 24, 2017 21:00
@thibaultcha thibaultcha removed the pr/wip A work in progress PR opened to receive feedback label Jul 24, 2017
@thibaultcha
Copy link
Member Author

since Penlight got an update, should we reenable 52 compatibility? see also #2375 (comment)

Yes - although I think this should be part of another PR (I will submit it soon after merging this one)

OpenResty 1.11.2.4 bumps its LuaJIT version, and among the changes in
there, the hashing of Lua strings has seen an update in OpenResty's
LuaJIT of 1.11.2.3. Now, `next()` might not yield results in the same
order as it used to, hence `json.encode()` isn't either, hence some JWT
tests are broken, due to the base64-encoding being different than it
used to (different JSON value to encode because order of keys differs).

Ideally, we should think about building a "safe" iterator (consistent),
maybe with `__pairs` once we enable Lua 5.2 compat. This is to be
emphasized once Kong starts forging JWT tokens, if ever, as the
iteration order might be different accross platforms, or after such a
change from LuaJIT.
Due to the change in OpenResty's LuaJIT introduced in 1.11.2.3 (new
hashing for string interning), the iteration order of `pairs()` has
changed. Since we should not rely on it anyways, this hard-codes the
order of execution of plugins as of Kong 0.10.1, to ensure
backwards-compatibility.

* hard-code `PRIORITY` fields as per 0.10.1 order
* new unit tests to ensure all plugins have their unique priority
* LuaSocket/lua_pack: ensure LuaJIT v2.1 compat
* Lapis has been forked to https://github.com/Mashape/lapis and a new
  release has been drafted from Lapis `v1.6.0`.
  This Kong-specific release (`v1.6.0.1`) removes lua-cjson (the
  non-OpenResty module) and luaossl from its list of dependencies, as they
  are not compatible with Kong and/or OpenResty 1.11.2.3+
@thibaultcha thibaultcha force-pushed the chore/deps-bump-openresty branch from 9ea7728 to b4d9dd7 Compare July 25, 2017 00:16
@thibaultcha thibaultcha merged commit 1d19a61 into next Jul 25, 2017
@thibaultcha thibaultcha deleted the chore/deps-bump-openresty branch July 25, 2017 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants