feat: update Kong Lua tree to 3.0.0-alpha.3 #27

tjasko · 2022-08-26T19:02:29Z

These changes update the Kong Lua tree to 3.0.0-alpha.3 (Kong/kong@e08c0c4).

The new patches that were applied are below:

entities/routes.lua:
- Forcing the Kong router flavor to "traditional", as the ATC router (dubbed as the "expressions" router flavor) cannot be used in goks due to it using the FFI interface.
schema/plugin_loader.lua:
- Reintroducing fallback new_tab() function, as table.new() LuaJIT function is not available for use.
schema/typedefs.lua:
- Updating validate_path_with_regexes() to use go.re2.find(). This may cause problems in the future, due to it utilizing RE2 instead of PCRE.
tools/utils.lua:
- Reintroducing fallback is_array_fast() function, as table.isarray() OpenResty function is not available for use.
- Reintroducing fallback clone() function, as table.clone() OpenResty function is not available for use.

The all-typedefs.lua plugin schema test fixture & the validation tests were also updated to include typedefs.router_path(s) & missing tests for typedefs.paths:

These changes were tested against Koko, and after the expected changes (e.g.: shorthands field being removed in favor of shorthand_fields), all is working as expected. I did have to fake the Kong DP version as while Koko was updated to support a four-digit versioning scheme, go-kong was not.

tjasko · 2022-08-26T19:05:38Z

plugin/testdata/all-typedefs.lua

-          { sources = typedefs.sources },
-          { destinations = typedefs.destinations },
-          { hosts = typedefs.hosts },
-          { paths = typedefs.paths },


See the commit message or PR description for what was altered.

However, part of those changes was removing the typedefs.paths typedef, as it was removed from Kong. In the event users were to use this typedef on plugin schemas, it would cause issues (however they could easily replicate it with the existing typedefs.path typedef).

Are we okay with this removal? We can easily add it back, but should we? This PR is held from merging for this reason. @mikefero stated that he'll look into this a bit further, but figured to get this PR up anyway.

We can't completely get rid off typedefs.path. It is most certainly being used by some plugins.

If we are certain that none of the relevant plugins use this typedef, we can remove it. This includes the custom plugins that matter.

typedefs.paths is not used anywhere in Koko nor is used in any custom plugins. As such, we could technically remove it, however, that still may not be the right thing to do. Hence why I wanted to have this conversation, does that change your viewpoints at all?

I can't make any decisions here, so will defer to the rest of you all.

from the documentation:

The typedefs variable (obtained by requiring kong.db.schema.typedefs) is a table containing a lot of useful type definitions and aliases....
To learn more about schemas, see:
· The source code of typedefs.lua to get an idea of what’s provided there by default.

To me this sounds like most typedefs should stay.

I am on the fence with this. path and paths seemed to be historically for routes and the router and most plugins (and custom plugins I have seen) use string for path fields. If we want to keep backwards compatibility for the small (if not 0) percentage of users using this typedef I think we should alias path and paths to router_path and router_paths.

Also we need to add additional tests for router_path and router_pathssince you are including them here. I noticed the validate functions are callingngx.re.findwhich is not available ingoks` and the aliasing piece for the loader is still being worked on by @omegabytes.

I think cidr is already being handled by ip_or_cidr in the tests, but please verify and create a test for that as well.

@mikefero I am on the fence with this. path and paths seemed to be historically for routes and the router and most plugins (and custom plugins I have seen) use string for path fields. If we want to keep backwards compatibility for the small (if not 0) percentage of users using this typedef I think we should alias path and paths to router_path and router_paths.

I'm also on the fence, as the use of this legacy typedef would likely be quite low; I'm not saying removing it is the right answer either.

@mikefero Also we need to add additional tests for router_path and router_pathssince you are including them here. I noticed the validate functions are callingngx.re.findwhich is not available ingoks` and the aliasing piece for the loader is still being worked on by @omegabytes.

I frankly didn't dig in too much to see if aliasing path & paths to router_path & router_paths would be 100% backwards compatible, but indeed, that's an option & something we can audit.

@mikefero I think cidr is already being handled by ip_or_cidr in the tests, but please verify and create a test for that as well.

After looking a bit more, I see that typedefs.cidr is just an alias for cidr_v4, will remove.

Ended up re-introducing typedefs.path per the conversations here.

lua-tree/share/lua/5.1/kong/db/schema/init.lua

hbagdi

A few simple changes:

Move a lot of content from the 2nd commit message into the patch file itself.
Please specify the git sha of the alpha.3 git tag. I do not trust tags to be around all the time.

hbagdi · 2022-08-29T20:09:57Z

patches/lua-tree.patch

+-        end,
+-      } },
+-    },
+   }


Please add this to the patch description like we do for all other changes.

Ah, I did log these things, but it must have got reverted while I was making changes. Will add it back.

hbagdi · 2022-08-29T20:10:13Z

patches/lua-tree.patch

+      end
+      return copy
+  end
+  end


Please document this in the patch description.

Will defer to this comment.

The general comment Reintroducing fallback Lua functions, as the OpenResty tooling is not available for use could be expanded on a bit instead of linking to the Kong Gateway OSS source code. I count three OpenResty table usages that could be added to the bullets under that section; WDYT?

@mikefero The general comment Reintroducing fallback Lua functions, as the OpenResty tooling is not available for use could be expanded on a bit instead of linking to the Kong Gateway OSS source code. I count three OpenResty table usages that could be added to the bullets under that section; WDYT?

Ah, can do. I'll be a bit more specific there.

hbagdi · 2022-08-29T20:10:33Z

patches/lua-tree.patch

+local time_ns = function()
+    return require("socket").gettime() * 10^9
+ end
+ _M.time_ns = time_ns


Please document this in the patch description.

Will defer to this comment.

I mentioned this earlier, but I think we can simply remove time_ns() from util.

@mikefero I mentioned this earlier, but I think we can simply remove time_ns() from util.

Will keep the convo in this conversation.

hbagdi · 2022-08-29T20:15:06Z

lua-tree/share/lua/5.1/kong/tools/utils.lua

+_M.yield = function() return end
+
+local time_ns = function()
+    return require("socket").gettime() * 10^9


Where is the source for this gettime function?
I couldn't grep my way to it.

Ah, it's actually part of a the Lua socket C-lib: https://github.com/lunarmodules/luasocket/blob/5b18e475f38fcf28429b1cc4b17baee3b9793a62/src/timeout.c#L36

Guessing this function is unused during the code execution (however it was previously failing due to the imports before). I'll switch it to use something else.

If nothing for plugin validation is using utils.time_ns() then we can safely remove this function and add that to the patch. It looks like all the imports are localized so removing should be sufficient.

Indeed, nothing is directly using it right now. As it's a global function though, technically speaking couldn't custom plugin schemas use it for "insert any reason"? It wasn't difficult to support, so I was personally leaning to support it. Sure, it doesn't return the exact time in nanoseconds (and technically that could be a security risk if one were using the value for randomness seeding), but for our current use-case of plugin validation, I felt this was okay.

I'm perfectly fine with removing it if that's what we want, however, I'm just thinking about these things from a maintainability perspective and "what if" it's used in the future, because it's a global function after all.

technically speaking couldn't custom plugin schemas use it for "insert any reason"?

Being a util and not built into the PDK I think we can safely assume that no custom plugins will be using this in their custom validations.

I'm just thinking about these things from a maintainability perspective and "what if" it's used in the future, because it's a global function after all.

Fair point, but I think for those what if future uses we can deal with things then or provide them a solution that works within their schema.

You do make some good points there too. I honestly could go either way here. I'll assume we'll just remove the global time_ns() function unless anyone says otherwise. Removing it could break things in a future Kong Lua code update, however, the chances are that are likely rare.

hbagdi · 2022-08-29T20:17:49Z

For verification, how did you generate the first commit?
Did you manually do it or use the script in this directory?

tjasko · 2022-08-29T22:59:48Z

@hbagdi For verification, how did you generate the first commit? Did you manually do it or use the script in this directory?

Had to do it manually as the script is hard-coded to pull from the Luarocks repository. As such, I checked out Kong, built everything myself, and generated the tree from that. Any such automation there should be part of another PR.

tjasko · 2022-08-29T23:03:08Z

@hbagdi Move a lot of content from the 2nd commit message into the patch file itself.

I did actually intend to have it in the patch file itself per this comment, and that's what was in the patch file before. As noted, I'll get that fixed.

Anyway, will strip it out of the commit.

tjasko · 2022-08-29T23:04:12Z

@hbagdi Please specify the git sha of the alpha.3 git tag. I do not trust tags to be around all the time.

Alright.

tjasko · 2022-08-29T23:52:18Z

lua-tree/share/lua/5.1/kong/tools/utils.lua

-    return tonumber(t.tv_sec) * 1e9 + tonumber(t.tv_nsec)
-  end
+local time_ns = function()
+    return require("os").time() * 10^9


The gopher-lua VM does support this & returns the current Unix epoch in seconds, as expected:
https://github.com/yuin/gopher-lua/blob/658193537a640772633e656f4673334fe1644944/oslib.go#L67

We can just get rid of the function entirely; it doesn't seem to be used. It is only being used in:

kong/pdk/tracing.lua

kong/plugins/opentelemetry/otlp.lua

kong/tracing/instrumentation.lua

kong.tracing.instrumentation is imported in kong/init.lua; however this isn't used directly by goks so none of these should be utilized.

Will defer this conversation to this thread.

hbagdi · 2022-08-30T00:30:09Z

patches/lua-tree.patch

@@ -42,6 +46,8 @@ schema/init.lua:
 - use re_match as UUID match intad of re_find
 - remove log statements
 - ensure custom_entity_checks in schemas return error message
+- Reintroducing fallback Lua functions, as the OpenResty tooling is not available for use.
+  Read more: https://github.com/Kong/kong/commit/15c427e090452396a5f52bf6848d8ad6bbe5e2d1


Secret referencing has been disabled.

Sorry, can you be a bit more specific? Are you saying to add that as a bullet item? I felt that was not needed as this existing bullet item is saying:

goks/patches/lua-tree.patch

Line 40 in c3b4a7a

- removing references to vault due to C lib usage (e.g. referenceable objects)

hbagdi · 2022-08-30T00:31:07Z

The code checks out. Deferring the final review to @javierguerragiraldez and @mikefero.
Both must approve.

javierguerragiraldez · 2022-08-30T03:36:23Z

plugin/testdata/all-typedefs.lua

-          { sources = typedefs.sources },
-          { destinations = typedefs.destinations },
-          { hosts = typedefs.hosts },
-          { paths = typedefs.paths },


from the documentation:

The typedefs variable (obtained by requiring kong.db.schema.typedefs) is a table containing a lot of useful type definitions and aliases....
To learn more about schemas, see:
· The source code of typedefs.lua to get an idea of what’s provided there by default.

To me this sounds like most typedefs should stay.

javierguerragiraldez · 2022-08-30T03:47:17Z

patches/lua-tree.patch

-
-
+ 
+ 


there are several of these lines with whitespace. i think cleaning them would make the diff much easier to read

These whitespaces were generated by the Git patch generation itself, I did not purposefully add these.

Whitespaces in patch files shouldn't be edited as these are part of the source files & how the diff was generated, if this were to be edited, the patch wouldn't be able to be applied without telling it to ignore whitespace (via git apply --reject --whitespace=fix).

right, no need to edit whitespaces (even if only because git is particularly bad in that respect), but this commit adds a lot of them. they weren't there before.

Correct, I assume because the past author of these changes had their editor strip the whitespaces. I didn't edit these manually, I just pasted in the generated patch & rearranged two files as they were previously out of order (and I left it that way).

The way the patch is after these changes is how it actually should have been in the first place.

mikefero · 2022-08-30T10:03:21Z

lua-tree/share/lua/5.1/kong/tools/utils.lua

+_M.yield = function() return end
+
+local time_ns = function()
+    return require("socket").gettime() * 10^9


If nothing for plugin validation is using utils.time_ns() then we can safely remove this function and add that to the patch. It looks like all the imports are localized so removing should be sufficient.

mikefero · 2022-08-30T10:17:57Z

lua-tree/share/lua/5.1/kong/db/schema/entities/routes.lua

-          return true
-        end,
-      } },
-    },


I am on the fence about this change. I understand we need this removal and the requires due to them not being available in gopher-lua, but WDYT about this instead.

Move the local imports into the condition for expression

Force the kong_router_flavor to be traditional and add a comment

Revert the removal of the entity_checks here

-- ATC router is incompatible with goks local kong_router_flavor = "traditional" if kong_router_flavor == "expressions" then local atc = require("kong.router.atc") local router = require("resty.router.router") return {

My reasoning here is less patch and if the defaults change out from underneath us and we are not ready then this catch will ensure nothing breaks.

As local r = router.new(s) uses the FFI interface (due to it using atc-router), importing resty.router.router wouldn't work as intended as such. Am I missing something?

Agreed it wouldn't work, but it should never hit that condition which means those ATC imports will never get performed either. We could also just remove everything in here for the ATC router, remove the if block and keep only the else block. WDYT?

Sounds good with me. I'll just assume if we want to support the ATC router typedef, it'll be part of another effort then.

mikefero · 2022-08-30T10:20:40Z

lua-tree/share/lua/5.1/kong/tools/utils.lua

-    return tonumber(t.tv_sec) * 1e9 + tonumber(t.tv_nsec)
-  end
+local time_ns = function()
+    return require("os").time() * 10^9


We can just get rid of the function entirely; it doesn't seem to be used. It is only being used in:

kong/pdk/tracing.lua

kong/plugins/opentelemetry/otlp.lua

kong/tracing/instrumentation.lua

kong.tracing.instrumentation is imported in kong/init.lua; however this isn't used directly by goks so none of these should be utilized.

mikefero · 2022-08-30T10:37:18Z

patches/lua-tree.patch

+  end
+end
+
+


This localized function makes sense and it is only used if a field is referenceable. Maybe in the patch comments you can indicate a little more under Reintroducing fallback Lua functions, as the OpenResty tooling is not available for use. Also could we add a small blurb as to why we are adding this function in the code?

-- 'table.new' is an OpenResty function and is not available to goks local new_tab

something like that; please rephrase to make more coherent.

I can be a bit more specific within the patch file as mentioned here. Was there anything specific you wanted to call out other than what functions have been added back & to what files?

As for adding in comments into the code itself, sure thing.

I think just listing out the OpenResty functionality that you are re-implementing is good enough; just something to catch our eyes and help the next person looking at this.

mikefero · 2022-08-30T10:41:35Z

patches/lua-tree.patch

+      end
+      return copy
+  end
+  end


The general comment Reintroducing fallback Lua functions, as the OpenResty tooling is not available for use could be expanded on a bit instead of linking to the Kong Gateway OSS source code. I count three OpenResty table usages that could be added to the bullets under that section; WDYT?

mikefero · 2022-08-30T10:42:11Z

patches/lua-tree.patch

+local time_ns = function()
+    return require("socket").gettime() * 10^9
+ end
+ _M.time_ns = time_ns


I mentioned this earlier, but I think we can simply remove time_ns() from util.

mikefero · 2022-08-30T10:50:55Z

plugin/testdata/all-typedefs.lua

-          { sources = typedefs.sources },
-          { destinations = typedefs.destinations },
-          { hosts = typedefs.hosts },
-          { paths = typedefs.paths },


I am on the fence with this. path and paths seemed to be historically for routes and the router and most plugins (and custom plugins I have seen) use string for path fields. If we want to keep backwards compatibility for the small (if not 0) percentage of users using this typedef I think we should alias path and paths to router_path and router_paths.

Also we need to add additional tests for router_path and router_pathssince you are including them here. I noticed the validate functions are callingngx.re.findwhich is not available ingoks` and the aliasing piece for the loader is still being worked on by @omegabytes.

I think cidr is already being handled by ip_or_cidr in the tests, but please verify and create a test for that as well.

javierguerragiraldez · 2022-08-30T20:27:52Z

I generally agree on @mikefero comments about removing impossible conditions or making the "fallback" functions as simple as possible. OTOH, these are very small details and I don't think affect maintainability at all, so I don't want to waste more of everybody's time on that.

about removing typedefs, I do think that most of them should stay; even if the verification is currently weak. that could be progressively improved while maintaining source compatibility with Kong CP.

javierguerragiraldez · 2022-09-01T00:55:24Z

patches/lua-tree.patch

@@ -42,6 +47,7 @@ schema/init.lua:
 - use re_match as UUID match intad of re_find
 - remove log statements
 - ensure custom_entity_checks in schemas return error message
+- Reintroducing fallback `new_tab()` function, as `table.new()` OpenResty function is not available for use.


nit: table.new is luajit, not openresty

Ah derp, will fix the comment. To be fair, it's slightly confusing as OpenResty maintains its own fork of LuaJIT.

javierguerragiraldez · 2022-09-01T01:00:42Z

patches/lua-tree.patch

@@ -52,6 +58,11 @@ schema/typedefs.lua:
 - use go.re2::match instead of string.match
 - fix tag validation; switch from decimal ASCII code to hex
 - fix validate_path regex due to go.re2::match
+- No-oping `kong.tools.utils.yield()` function.
+- Removing `time_ns()` function, as it requires the FFI interface & is currently unused by goks.
+- Adding back the `paths` typedef as it was removed in 3.0, as we're wanting to keep it around for use in goks.


now i'm confused, was paths removed from Kong?

(nit, "we're wanting" sounds unnatural... at least to non-native ears)

Yes, typedefs.paths was removed from Kong 3.0 in favor of typedefs.router_paths.

Regarding the grammatical comment, "we're wanting" is perfectly valid English, as "we're" is a contraction for "we are". To your point, English will always be English with its weird oddities. "we're" & "were" may or not be pronounced the same way depending on the accent, but the typical phonetic spelling of "we're" is "weer" & "were" is "wuhr".

on paths, as defined in Kong:

pre-3.0:

path : string starting with /; used on services and upstreams.

paths: array of "path" elements; used on routes.

3.0 and on:

path : string starting with /; used on services and upstreams.

router_path : string starting with / or ~/

router_paths: array or "router_path" elements; used on routes.

so my confusion was because the plural form was removed(/replaced), but the singular is still there. and yes, since we have different backwards compatibility requirements we should keep it.

(on the grammatical thing, of course i do know about contractions (took a while, but i've grown used to the british style 🤣 ) it's the gerund form that seems a bit out of phase with the indefinite tense typical in docs)

so my confusion was because the plural form was removed(/replaced), but the singular is still there. and yes, since we have different backwards compatibility requirements we should keep it.

Exactly, that's what I noticed as well. I'm unsure why the gateway team decided to keep typedefs.path around, but perhaps there is a reason. Apologize for the confusion.

it's the gerund form that seems a bit out of phase with the indefinite tense typical in docs)

Ah, I see, I thought you were commenting on how it sounds vs. the tense. Yes, you're right, the verb form there wasn't grammatically proper. I've re-phrased it.

Used Kong/kong@e08c0c4.

mikefero · 2022-09-01T19:15:39Z

plugin/testdata/router-path-test.lua

+local typedefs = require "kong.db.schema.typedefs"
+
+local schema = {
+  name = "path-test",


Let's keep the plugin name consistent with the filename. I understand that there is the alias, but it isn't an exact alias of router_path. You should be able to update the function signature to pass in the plugin name or to use the basename of the filename.

Suggested change

name = "path-test",

name = "route-path-test",

I've made this change, but called it "router-path-test" to be consistent with the typedef naming.

mikefero · 2022-09-01T19:22:51Z

plugin/validator_test.go

+		t.Run(path.name, func(t *testing.T) {
+			err := v.Validate(fmt.Sprintf(pathTestPlugin, path.path))
+			require.Error(t, err)
+			assert.JSONEq(t, path.expectedErr, err.Error())


Could we write a couple tests for this for typedefs.router_paths? I know this is just an array of typedefs.router_path, but it will allow for complete typedef coverage. The easiest would be all of the good router paths in one test, then a mixture of good and bad for failure test; WDYT?

There are no existing tests for typedefs.paths, so I was just matching what was already done. With that said, yes we can update the code to support running tests for those types.

Edit: There are some very thorough tests here now. 😄

GGabriele · 2022-09-07T10:33:31Z

patches/lua-tree.patch

 +local cjson        = { array_mt = {} } --- TODO(hbagdi) XXX analyze the impact
-+local uuid         = require "go.uuid".generate


Is the removal of local uuid = require "go.uuid".generate expected? I see the "docstring" entry has not been removed.

It is expected. I did not modify this patch manually, it was all done after updating the Lua tree & just simply replacing the existing patches for the files.

I just forgot to remove the docstring comment from the patch file. I'll go ahead and do that, nice catch.

Removed here:
de4b149

GGabriele · 2022-09-07T13:05:19Z

plugin/validator_paths_test.go

+	shouldSkip := true
+	for _, p := range t.scopedToPlugins {
+		if p == pluginName {
+			shouldSkip = false


Can't we just return false here to avoid keep looping?

Blerg, yea we can. This wasn't originally in a function & decided to move it into one, and didn't refactor it.

Done! 7a0259e

GGabriele · 2022-09-07T13:07:36Z

plugin/validator_paths_test.go

@@ -0,0 +1,297 @@
+package plugin


Very nice!!!

GGabriele · 2022-09-07T13:11:43Z

Two questions:

once luarocks 3.0.0-0 is released, is the plan to update the proper value here to be able to automatically regenerate the tree, or do you plan to do something different?
for verification, have we tried mounting this in kong/koko and run some manual test there?

tjasko · 2022-09-07T20:17:38Z

@GGabriele once luarocks 3.0.0-0 is released, is the plan to update the proper value here to be able to automatically regenerate the tree, or do you plan to do something different?

For now, that's the plan, until we can introduce better automation here. I didn't want to increase the scope of work of this too dramatically, as such automation there is a bit easier said than done.

@GGabriele for verification, have we tried mounting this in kong/koko and run some manual test there?

As notated in the PR description, yes this was tested in Koko itself as much as I possibly can.

Used Kong/kong@e08c0c4.

Introduces tests for: - `typedefs.router_path` - `typedefs.router_paths` - `typedefs.paths`

mikefero

Changes looks great!!! There is one small issue as 3.0.0 has been recently tagged causing a patch issue with utils.lua. The patch will still work, but creates fuzz apply due to changes between 3.0.0-alpha.3 and this new tag; 3.0.0-alpha applies cleanly.

No need to worry about this now, we can tackle that when luarock is available.

tjasko added the hold/do-not-merge label Aug 26, 2022

tjasko requested a review from a team as a code owner August 26, 2022 19:02

tjasko commented Aug 26, 2022

View reviewed changes

hbagdi requested review from mikefero and javierguerragiraldez August 29, 2022 19:07

tjasko commented Aug 29, 2022

View reviewed changes

lua-tree/share/lua/5.1/kong/db/schema/init.lua Outdated Show resolved Hide resolved

tjasko force-pushed the feat/update-lua-tree-to-3.0.0-alpha.3 branch from 03d32b4 to 7e6e20c Compare August 29, 2022 19:58

hbagdi requested changes Aug 29, 2022

View reviewed changes

tjasko force-pushed the feat/update-lua-tree-to-3.0.0-alpha.3 branch from 7e6e20c to 1a7efa2 Compare August 29, 2022 23:36

feat: updating Kong Lua tree to 3.0.0-alpha.3 (e08c0c4)

a1f1696

tjasko force-pushed the feat/update-lua-tree-to-3.0.0-alpha.3 branch 2 times, most recently from c78a29d to ff0ad4b Compare August 29, 2022 23:49

tjasko commented Aug 29, 2022

View reviewed changes

hbagdi reviewed Aug 30, 2022

View reviewed changes

javierguerragiraldez reviewed Aug 30, 2022

View reviewed changes

mikefero reviewed Aug 30, 2022

View reviewed changes

tjasko force-pushed the feat/update-lua-tree-to-3.0.0-alpha.3 branch 2 times, most recently from 77e2c34 to 7870c4a Compare August 31, 2022 20:05

tjasko removed the hold/do-not-merge label Aug 31, 2022

tjasko requested review from mikefero, javierguerragiraldez and hbagdi August 31, 2022 23:48

javierguerragiraldez reviewed Sep 1, 2022

View reviewed changes

feat: applying existing & new Lua patches to Kong 3.0.0-alpha.3

48270f0

Used Kong/kong@e08c0c4.

mikefero reviewed Sep 1, 2022

View reviewed changes

mikefero mentioned this pull request Sep 2, 2022

feat: feat: policy default to local for rate limiting plugins #28

Closed

tjasko force-pushed the feat/update-lua-tree-to-3.0.0-alpha.3 branch from f387d53 to dc1410c Compare September 3, 2022 02:06

tjasko requested review from mikefero and javierguerragiraldez September 3, 2022 02:07

tjasko force-pushed the feat/update-lua-tree-to-3.0.0-alpha.3 branch 2 times, most recently from d0c9f6b to d69830e Compare September 3, 2022 02:10

tjasko added the hold/do-not-merge label Sep 3, 2022

tjasko force-pushed the feat/update-lua-tree-to-3.0.0-alpha.3 branch 3 times, most recently from be9cbf3 to 8e5bc67 Compare September 3, 2022 02:17

GGabriele reviewed Sep 7, 2022

View reviewed changes

plugin/validator_paths_test.go

@@ -0,0 +1,297 @@

package plugin

Copy link

Contributor

GGabriele Sep 7, 2022

Choose a reason for hiding this comment

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

Very nice!!!

tjasko reacted with heart emoji

tjasko force-pushed the feat/update-lua-tree-to-3.0.0-alpha.3 branch from 8e5bc67 to 1c289a6 Compare September 8, 2022 00:52

tjasko requested a review from GGabriele September 8, 2022 00:53

tjasko force-pushed the feat/update-lua-tree-to-3.0.0-alpha.3 branch from 1c289a6 to 7a0259e Compare September 8, 2022 00:55

GGabriele previously approved these changes Sep 8, 2022

View reviewed changes

This was referenced Sep 8, 2022

feat: override socket.url with patched.url for all imports #25

Merged

feat: alias glaure.match as ngx.re.match #26

Merged

tjasko added 2 commits September 8, 2022 16:02

feat: updating Kong Lua tree patch file for 3.0.0-alpha.3 changes

1a4e71f

Used Kong/kong@e08c0c4.

tests: updating typedefs validation tests

2919b47

Introduces tests for: - `typedefs.router_path` - `typedefs.router_paths` - `typedefs.paths`

tjasko dismissed GGabriele’s stale review via 2919b47 September 8, 2022 21:03

tjasko force-pushed the feat/update-lua-tree-to-3.0.0-alpha.3 branch from 7a0259e to 2919b47 Compare September 8, 2022 21:03

mikefero reviewed Sep 8, 2022

View reviewed changes

mikefero approved these changes Sep 8, 2022

View reviewed changes

tjasko merged commit 3b0830e into main Sep 8, 2022

tjasko deleted the feat/update-lua-tree-to-3.0.0-alpha.3 branch September 8, 2022 21:13

		+local cjson = { array_mt = {} } --- TODO(hbagdi) XXX analyze the impact
		+local uuid = require "go.uuid".generate

feat: update Kong Lua tree to 3.0.0-alpha.3 #27

feat: update Kong Lua tree to 3.0.0-alpha.3 #27

Conversation

tjasko commented Aug 26, 2022 • edited Loading

tjasko Aug 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjasko Aug 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjasko Aug 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hbagdi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjasko Aug 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjasko Aug 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hbagdi commented Aug 29, 2022

tjasko commented Aug 29, 2022

tjasko commented Aug 29, 2022 • edited Loading

tjasko commented Aug 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjasko Aug 30, 2022 • edited Loading

Choose a reason for hiding this comment

hbagdi commented Aug 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjasko Aug 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjasko Aug 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javierguerragiraldez commented Aug 30, 2022

Choose a reason for hiding this comment

tjasko Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjasko Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

javierguerragiraldez Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjasko Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjasko Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

tjasko commented Aug 26, 2022 •

edited

Loading

tjasko Aug 26, 2022 •

edited

Loading

tjasko Aug 29, 2022 •

edited

Loading

tjasko Aug 30, 2022 •

edited

Loading

tjasko Aug 30, 2022 •

edited

Loading

tjasko Aug 30, 2022 •

edited

Loading

tjasko commented Aug 29, 2022 •

edited

Loading

tjasko Aug 30, 2022 •

edited

Loading

tjasko Aug 30, 2022 •

edited

Loading

tjasko Aug 30, 2022 •

edited

Loading

tjasko Sep 1, 2022 •

edited

Loading

tjasko Sep 1, 2022 •

edited

Loading

javierguerragiraldez Sep 1, 2022 •

edited

Loading

tjasko Sep 1, 2022 •

edited

Loading

tjasko Sep 1, 2022 •

edited

Loading