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

fix: required changes for 3.1 compatibility #36

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
14256e9
temp: ngx.decode_base64url
omegabytes Dec 8, 2022
2f294c1
replace lua resty.openssl.digest with go crypto/sha256
omegabytes Dec 8, 2022
4ac298b
rename pkg crypto to sha256
omegabytes Dec 8, 2022
b6e369b
replace lua resty.string.to_hex with go encoding/hex
omegabytes Dec 8, 2022
788f46c
replace table.nkeys with local fn
omegabytes Dec 9, 2022
86df807
fmt and lint
omegabytes Dec 9, 2022
ff8aeb1
remove log_level from constants
omegabytes Dec 10, 2022
ab748dd
replace table.new with local fn
omegabytes Dec 10, 2022
393daa9
remove/replace resty.* imports
omegabytes Dec 10, 2022
739a308
rework goto statements
omegabytes Dec 10, 2022
001809f
remove ffi imports from node.lua
omegabytes Dec 10, 2022
884c12f
pdk/private/node.lua remove pl imports
omegabytes Dec 10, 2022
23c7b76
remove ngx imports from node and reports
omegabytes Dec 10, 2022
0345f15
override nkeys for router/utils
omegabytes Dec 10, 2022
ea9bcf7
remove ngx.worker.id ref
omegabytes Dec 10, 2022
b47d2e6
just get rid of atc already
omegabytes Dec 10, 2022
33e4b3c
refactor: name go.hex and go.sha256 pkg
omegabytes Dec 12, 2022
9798f25
empty atc and node files
omegabytes Dec 12, 2022
8bd2cdc
remove commented code
omegabytes Dec 12, 2022
a7b66b9
remove debug traceback
omegabytes Dec 12, 2022
74a909b
move new_tab and nkeys to kong.tools.utils
omegabytes Dec 13, 2022
fc044a8
remove node, add newlines
omegabytes Dec 13, 2022
cddf098
revert reports changes
omegabytes Dec 13, 2022
821f5f7
revert router.utils changes
omegabytes Dec 13, 2022
4c25d00
update patchfile
omegabytes Dec 14, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions internal/vm/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ import (
"github.com/kong/goks"
"github.com/kong/goks/internal/fs"
"github.com/kong/goks/lualibs/go/cjson/safe"
"github.com/kong/goks/lualibs/go/hex"
"github.com/kong/goks/lualibs/go/ipmatcher"
"github.com/kong/goks/lualibs/go/jsonschema"
"github.com/kong/goks/lualibs/go/lyaml"
"github.com/kong/goks/lualibs/go/ngx"
"github.com/kong/goks/lualibs/go/rand"
"github.com/kong/goks/lualibs/go/sha256"
"github.com/kong/goks/lualibs/go/uuid"
"github.com/kong/goks/lualibs/go/x509"
json "github.com/layeh/gopher-json"
Expand Down Expand Up @@ -63,6 +65,8 @@ func New(opts Opts) (*VM, error) {
l.PreloadModule("go.jsonschema", jsonschema.Loader)
l.PreloadModule("cjson.safe", safe.Loader)
l.PreloadModule("lyaml", lyaml.Loader)
l.PreloadModule("sha256", sha256.Loader)
l.PreloadModule("hex", hex.Loader)
Copy link
Contributor

Choose a reason for hiding this comment

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

We're using the pattern "go.xxx" for those replacement modules:

Suggested change
l.PreloadModule("sha256", sha256.Loader)
l.PreloadModule("hex", hex.Loader)
l.PreloadModule("go.sha256", sha256.Loader)
l.PreloadModule("go.hex", hex.Loader)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ngx.LoadNgx(l)

if err := setup(l); err != nil {
Expand Down
18 changes: 17 additions & 1 deletion lua-tree/share/lua/5.1/kong/db/schema/init.lua
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
local tablex = require "pl.tablex"
local pretty = require "pl.pretty"
local utils = require "kong.tools.utils"
local nkeys = require "table.nkeys"

local cjson = { array_mt = {} } --- TODO(hbagdi) XXX analyze the impact
local is_reference = function(value) return false end
Expand Down Expand Up @@ -55,6 +54,23 @@ do
end
end

-- `table.nkeys()` is a LuaJIT function and is not available to the Lua VM that goks uses.
local nkeys
do
local ok
ok, nkeys = pcall(require, "table.nkeys")
if not ok then
nkeys = function (tab)
local count = 0
for _, v in pairs(tab) do
if v ~= nil then
count = count + 1
end
end
return count
end
end
end
Copy link
Contributor Author

@omegabytes omegabytes Dec 9, 2022

Choose a reason for hiding this comment

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

This is copypasta from resty/healthecheck.lua. I'm following the pattern of local_tab initialized on L46 of this file. This addresses the error:

lua-tree/share/lua/5.1/kong/db/schema/init.lua:4: module table.nkeys not found:
    no field package.preload['table.nkeys']
    open /table/nkeys.lua: file does not exist
    open lua-tree/share/lua/5.1/table/nkeys.lua: file does not exist
    open lua-tree/share/lua/5.1/table/nkeys/init.lua: file does not exist,

I can't tell if this resolves the issue, however. I get a new error:

Received unexpected error:
lua-tree/share/lua/5.1/kong/constants.lua:222: table index is nil
stack traceback:
    lua-tree/share/lua/5.1/kong/constants.lua:222: in function <lua-tree/share/lua/5.1/kong/constants.lua:0>
    [G]: in function 'require'
    lua-tree/share/lua/5.1/kong/db/schema/typedefs.lua:6: in function <lua-tree/share/lua/5.1/kong/db/schema/typedefs.lua:0>
    [G]: in function 'require'
    setup.lua:6: in main chunk
    [G]: ?

I'm able to validate the method is called. The if block beginning on line 62 is entered but nkeys = function (tab) on line 63 panics. I'm not sure if it's the method call itself or something else. A print statement on line 64 is never executed. Curiously, print statements before the method is called are also not executed?

The stacktrace is also not very illuminating: lua-tree/share/lua/5.1/kong/constants.lua:222: in function <lua-tree/share/lua/5.1/kong/constants.lua:0> is on a different line in kong-ee and points to LOG_LEVELS. This looks new to 3.1, I don't see it in this 3.0 branch.

My guess is either:

  1. This is a new error. The introduction of LOG_LEVELS is causing something to break. What and why is unclear.
  2. The update I made to nkeys somehow breaks this import, or something that processes this import.

Number 1 is the most reasonable, but I don't really understand what table index is nil means in this context and I'm struggling to trace where exactly this is breaking.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really understand what table index is nil means in this context and I'm struggling to trace where exactly this is breaking.

the LOG_LEVELS table, starting in constants.lua:222 has this form:

  LOG_LEVELS = {
    debug = ngx.DEBUG,
    info = ngx.INFO,
    notice = ngx.NOTICE,
    warn = ngx.WARN,
    error = ngx.ERR,
    crit = ngx.CRIT,
    alert = ngx.ALERT,
    emerg = ngx.EMERG,
    [ngx.DEBUG] = "debug",
    [ngx.INFO] = "info",
    [ngx.NOTICE] = "notice",
    [ngx.WARN] = "warn",
    [ngx.ERR] = "error",
    [ngx.CRIT] = "crit",
    [ngx.ALERT] = "alert",
    [ngx.EMERG] = "emerg",
  },

lines 231-238 use the form [key-expression] = value-expression, so it evaluates the ngx.DEBUG, ngx.INFO, etc to get the actual keys (unlike key-name=value-expression, which directly sets the key as a string constant). if any of these is nil, this error will trigger. unfortunately, this aborts the whole (sub) table construction, so we don't know which of these is failing.

Now to find where are those ngx.XXXX values....

Copy link
Contributor

Choose a reason for hiding this comment

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

Now to find where are those ngx.XXXX values....

Can't find them. I think the easiest would be to add to internal/vm/setup.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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


local validation_errors = {
-- general message
Expand Down
35 changes: 4 additions & 31 deletions lua-tree/share/lua/5.1/kong/tools/utils.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1267,7 +1267,7 @@ _M.yield = function() return end
local try_decode_base64
do
local decode_base64 = ngx.decode_base64
local decode_base64url = require "ngx.base64".decode_base64url
local decode_base64url = ngx.decode_base64url
Copy link
Contributor Author

@omegabytes omegabytes Dec 9, 2022

Choose a reason for hiding this comment

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

Addresses:

lua-tree/share/lua/5.1/kong/tools/utils.lua:1270: module ngx.base64 not found:
    no field package.preload['ngx.base64']
    open /ngx/base64.lua: file does not exist
    open lua-tree/share/lua/5.1/ngx/base64.lua: file does not exist
    open lua-tree/share/lua/5.1/ngx/base64/init.lua: file does not exist,

This (and other instances) will need to be added to lua-tree.patch or fixed in kong-ee.

Copy link
Contributor

Choose a reason for hiding this comment

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

there's already lualibs/go/ngx/base64.go, which implements the ngx.decode_base64 function. i guess the URL variant should be added there, so line 1270 could go unmodified.

Copy link
Contributor Author

@omegabytes omegabytes Dec 9, 2022

Choose a reason for hiding this comment

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

Adding a decode_base64url aliased function to lualibs/go/ngx/base64.go was my first approach. However, I noticed that updating the import without the rest of the work also resolved the issue. My guess is there may be a runtime error for calls to ngx.decode_base64url? I'll look at extending lualibs/go/ngx/base64.go and adding some test coverage if I find gaps.

Copy link
Contributor Author

@omegabytes omegabytes Dec 13, 2022

Choose a reason for hiding this comment

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

there's already lualibs/go/ngx/base64.go, which implements the ngx.decode_base64 function. i guess the URL variant should be added there, so line 1270 could go unmodified.

We have encode_base64, no decode functions. Any way I change this will be internally inconsistent. I'd like to leave this change as-is without adding any new methods to the go.ngx package.


local function decode_base64_str(str)
if type(str) == "string" then
Expand Down Expand Up @@ -1299,42 +1299,15 @@ end
_M.try_decode_base64 = try_decode_base64


local sha256_bin
do
local digest = require "resty.openssl.digest"
local sha256_digest

function sha256_bin(key)
local _, bin, err
if not sha256_digest then
sha256_digest, err = digest.new("sha256")
if err then
return nil, err
end
end

bin, err = sha256_digest:final(key)
if err then
sha256_digest = nil
return nil, err
end

_, err = sha256_digest:reset()
if err then
sha256_digest = nil
end

return bin
end
end
local sha256_bin = require "sha256".sha256_bin
Copy link
Contributor Author

@omegabytes omegabytes Dec 9, 2022

Choose a reason for hiding this comment

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

Addresses:

Received unexpected error:
lua-tree/share/lua/5.1/resty/openssl/digest.lua:1: module ffi not found:
    no field package.preload['ffi']
    open /ffi.lua: file does not exist
    open lua-tree/share/lua/5.1/ffi.lua: file does not exist
    open lua-tree/share/lua/5.1/ffi/init.lua: file does not exist,
stack traceback:
    [G]: in function 'require'
    lua-tree/share/lua/5.1/resty/openssl/digest.lua:1: in function <lua-tree/share/lua/5.1/resty/openssl/digest.lua:0>
    [G]: in function 'require'
    lua-tree/share/lua/5.1/kong/tools/utils.lua:1304: in function <lua-tree/share/lua/5.1/kong/tools/utils.lua:0>
    [G]: in function 'require'

This will need to be added to lua-tree.patch.

_M.sha256_bin = sha256_bin


local sha256_hex, sha256_base64, sha256_base64url
do
local to_hex = require "resty.string".to_hex
local to_hex = require "hex".to_hex
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addresses another instance of module ffi not found

local to_base64 = ngx.encode_base64
local to_base64url = require "ngx.base64".encode_base64url
local to_base64url = ngx.encode_base64url

local function sha256_encode(encode_alg, key)
local bin, err = sha256_bin(key)
Expand Down
20 changes: 20 additions & 0 deletions lualibs/go/hex/api.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package hex

import (
"encoding/hex"

lua "github.com/yuin/gopher-lua"
)

// Encode encodes input string into hex bytes. It is a wrapper around
// encoding/hex.Encode and replaces resty.string.to_hex.
// See: https://github.com/openresty/lua-resty-string/blob/master/lib/resty/string.lua
func Encode(l *lua.LState) int {
input := l.CheckString(1)

dst := make([]byte, hex.EncodedLen(len(input)))
hex.Encode(dst, []byte(input))

l.Push(lua.LString(dst))
return 1
}
14 changes: 14 additions & 0 deletions lualibs/go/hex/loader.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package hex

import lua "github.com/yuin/gopher-lua"

func Loader(l *lua.LState) int {
t := l.NewTable()
l.SetFuncs(t, api)
l.Push(t)
return 1
}

var api = map[string]lua.LGFunction{
"to_hex": Encode,
}
16 changes: 16 additions & 0 deletions lualibs/go/sha256/api.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package sha256

import (
"crypto/sha256"

lua "github.com/yuin/gopher-lua"
)

func BinarySha256(l *lua.LState) int {
input := l.CheckString(1)
h := sha256.New()
h.Write([]byte(input))
l.Push(lua.LString(h.Sum(nil)))
h.Reset()
return 1
}
14 changes: 14 additions & 0 deletions lualibs/go/sha256/loader.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package sha256

import lua "github.com/yuin/gopher-lua"

func Loader(l *lua.LState) int {
t := l.NewTable()
l.SetFuncs(t, api)
l.Push(t)
return 1
}

var api = map[string]lua.LGFunction{
"sha256_bin": BinarySha256,
}