umegaya / lua-aws

pure-lua implementation of aws REST APIs
122 stars 35 forks source link

s3 putObject is not supported in lua-resty-http #76

Open CriztianiX opened 4 years ago

CriztianiX commented 4 years ago

Hello, I'm returning to use the library in a new project!. So, using lua-resty-http as preferred_engine, s3 putOject returns:

core.lua:159: api_log(): s3:v[2006-03-01]:PutObject:error:...al/share/lua/5.1/lua-aws/engines/http/lua-resty-http.lua:24: Method not supported: PUT @ stack traceback:

CriztianiX commented 4 years ago

https://github.com/umegaya/lua-aws/blob/master/lua-aws/engines/http/lua-resty-http.lua#L21

    elseif req.method == 'PUT' then
        opts[ 'body' ] = req.body

Works as expected! There are some reason for not add this line?

umegaya commented 4 years ago

@CriztianiX thanks for investigation👍 actually, lua-resty-http.lua is contributed code and I don't fully know author's intension here. but according to the logs you provided, your fix got a point.

I will add the fix, but problem is master branch in my environment, s3 test with resty (test/s3.lua) which contains putObject call succeed as following without your fix:

$ make shell
docker run --rm -ti -v `pwd`:/project -w /project -e AWS_ACCESS_KEY=**** -e AWS_SECRET_KEY=**** -e AWS_DEFAULT_REGION=ap-northeast-1 -e EC2_URL= umegaya/lua-aws-test bash
root@68390fa56b11:/project# resty test/s3.lua
....
s3-putobjectETag"33a49d53a0e13e096b397b692a64b68c"
s3-putobjectETag"33a49d53a0e13e096b397b692a64b68c"
...
root@68390fa56b11:/project# echo $?
0
root@68390fa56b11:/project# cat lua-aws/engines/http/lua-resty-http.lua
return function( req )

    local lua_resty_http = require 'resty.http'
    local httpc = lua_resty_http.new()

    local u = require 'lua-aws.util'
    u.fill_header( req )

    local url = req.protocol .. "://" .. req.host .. ":" .. req.port .. req.path

    local opts = {
        -- To enable SSL certificate verification, set the lua_ssl_trusted_certificate line in nginx.conf
        -- and then remove the following line (see https://github.com/pintsized/lua-resty-http/issues/42):
        ssl_verify = false,

        method = req.method,
        body = req.body,
        headers = req.headers
    }
    if req.method == 'GET' then
    elseif req.method == 'POST' then
        opts[ 'body' ] = req.body
    else
        error( 'Method not supported: ' .. req.method )
    end

    local res, err = httpc:request_uri( url, opts )
    if not res then error( err ) end

    return {
        status = res.status,
        body = res.body,
        headers = res.headers,
    }

end

if you have docker in your host, can you try to run resty test/s3.lua and check it succeeds? maybe you need to runmake image to generate umegaya/lua-aws-test image.

and if test/s3.lua succeed in your host too, can you share minimum example to cause Method not supported error as test case of the fix?

CriztianiX commented 4 years ago

Ok, i'll try, but. what version of resty are you running?

My enviroment:

root@b31f9e737113 opt]# resty -V       
resty 0.23
nginx version: openresty/1.15.8.3
built by gcc 8.3.1 20190311 (Red Hat 8.3.1-3) (GCC) 
built with OpenSSL 1.1.0l  10 Sep 2019
lua-resty-http
   0.15-0 (installed) - /usr/local/openresty/luajit/lib/luarocks/rocks-5.1
umegaya commented 4 years ago

this is my one (docker image umegaya/lua-aws-test) seems a bit older than yours.

resty 0.23
nginx version: openresty/1.15.8.1
built by gcc 7.4.0 (Ubuntu 7.4.0-1ubuntu1~18.04)
built with OpenSSL 1.1.0j  20 Nov 2018
TLS SNI support enabled
configure arguments: --prefix=/usr/local/openresty/nginx --with-cc-opt=-O2 --add-module=../ngx_devel_kit-0.3.1rc1 --add-module=../echo-nginx-module-0.61 --add-module=../xss-nginx-module-0.06 --add-module=../ngx_coolkit-0.2 --add-module=../set-misc-nginx-module-0.32 --add-module=../form-input-nginx-module-0.12 --add-module=../encrypted-session-nginx-module-0.08 --add-module=../srcache-nginx-module-0.31 --add-module=../ngx_lua-0.10.15 --add-module=../ngx_lua_upstream-0.07 --add-module=../headers-more-nginx-module-0.33 --add-module=../array-var-nginx-module-0.05 --add-module=../memc-nginx-module-0.19 --add-module=../redis2-nginx-module-0.15 --add-module=../redis-nginx-module-0.3.7 --add-module=../rds-json-nginx-module-0.15 --add-module=../rds-csv-nginx-module-0.09 --add-module=../ngx_stream_lua-0.0.7 --with-ld-opt=-Wl,-rpath,/usr/local/openresty/luajit/lib --with-openssl=/tmp/openssl-1.1.0j --with-pcre=/tmp/pcre-8.42 --with-file-aio --with-http_addition_module --with-http_auth_request_module --with-http_dav_module --with-http_flv_module --with-http_geoip_module=dynamic --with-http_gunzip_module --with-http_gzip_static_module --with-http_image_filter_module=dynamic --with-http_mp4_module --with-http_random_index_module --with-http_realip_module --with-http_secure_link_module --with-http_slice_module --with-http_ssl_module --with-http_stub_status_module --with-http_sub_module --with-http_v2_module --with-http_xslt_module=dynamic --with-ipv6 --with-mail --with-mail_ssl_module --with-md5-asm --with-pcre-jit --with-sha1-asm --with-stream --with-stream_ssl_module --with-threads --with-stream --with-stream_ssl_preread_module
umegaya commented 4 years ago

current latest openresty:bionic uses 1.17.8.1, I will try update my docker image to this version

umegaya commented 4 years ago

@CriztianiX I can replicate error with 1.17.8.1 thank you for pointing out resty version! older version, test/s3.lua actually uses luasocket, not resty-http

umegaya commented 4 years ago

@CriztianiX s3 problem should fixed with https://github.com/umegaya/lua-aws/pull/77 but it cannot be merged now because lua-resty-http does not seem to work with some endpoint of aws (eg. kinesis.getRecords). as it seems to related with deep internal of nginx/lua-nginx-module, I post issue https://github.com/openresty/resty-cli/issues/57 . if you are urgent (and not interested in kinesis), please take commit from above pull request. thanks!

CriztianiX commented 4 years ago

@umegaya thanks! I will take this commit!