apache / openwhisk-apigateway

Apache OpenWhisk API Gateway service for exposing actions as REST interfaces.
https://openwhisk.apache.org/
Apache License 2.0
64 stars 45 forks source link

Enable overriding the backend url to enable standalone mode #347

Closed chetanmeh closed 5 years ago

chetanmeh commented 5 years ago

This PR enables running api gateway with standalone server (apache/incubator-openwhisk#4571).

It also fixes current broken build by updating the opm version

Implementation Details

Api Gateway uses the backend url as computed by makeWebActionBackendUrl in createApi call. This backend url is based on url passed by wsk api command

{"apidoc":{"namespace":"_","gatewayBasePath":"/hello","gatewayPath":"/world","gatewayMethod":"GET","id":"API:_:/hello","action":{"name":"hello","namespace":"_","backendMethod":"GET","backendUrl":"http://localhost:3233/api/v1/web/_/default/hello.http","authkey":"23bc46b1-71f6-4ed5-8c54-816aa4f8c502:123zO3xZCLrMN6v2BKK1dXYFpXlPkccOFqm12CdAsMgRU4VrNZ9lyGVCGuMDGIwP"}}}

With standalone case this url becomes http://localhost:3233 and is used as is by api gateway routing logic. So to enable this flow to work the backendRouting logic now exposes a env variable BACKEND_HOST. If this variable is found to be set then it supersedes the backend url as specified in createApi call

chetanmeh commented 5 years ago

@mrutkows @dgrove-oss The build is failing due to scancode issue reporting lots of trailing whitespace in code from downloaded lua modules. Anyway to fix scancode logic to exclude those files?

dgrove-oss commented 5 years ago

@chetanmeh -- we can reorder travis to do the scancode before it installs lua. Do the scancode in preinstall stage. Do you want to do it as part of this PR, or do you want me to do a separate PR just for that?

chetanmeh commented 5 years ago

@dgrove-oss Please review the travis and build related changes once

mhamann commented 5 years ago

@chetanmeh can you write some tests for this? we need to make sure that this doesn't break production use cases. I'm assuming it does not, but there are no test assertions proving that.

rabbah commented 5 years ago

a visual inspection shows all the changes are guarded by if backendOverride ~= nil then whose value is derived from a new environment variable, making this low risk in general.

of course, adding tests ensures this new feature isn't broken in the future.

mhamann commented 5 years ago

Positive and negative tests are both needed (proving that both cases work as intended).

While I agree that the code looks correct, I've been bitten plenty of times by code that looked fine visually, but had some nasty side-effect nobody noticed.

chetanmeh commented 5 years ago

@mhamann Added few tests in #350. Please review