-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
feat: add openfunction plugin #7634
Conversation
apisix/plugins/openfunction.lua
Outdated
end | ||
|
||
|
||
function _M.access(conf, ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we reuse code base in https://github.com/apache/apisix/blob/master/apisix/plugins/serverless/init.lua?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And code in init.lua, you can refer to azure-function for the detail.
ci/init-plugin-test-service.sh
Outdated
} | ||
|
||
set_container_registry_secret() { | ||
REGISTRY_SERVER=https://index.docker.io/v1/ REGISTRY_USER=apisixtestaccount123 REGISTRY_PASSWORD=apisixtestaccount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use this? We can push the docker images through kind load docker-image
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The part of the openfuction building lifecycle, pushing the image to a registry, needs it.Will it be better to build locally to avoid this part?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should avoid exposing your secrets here. By the way, is this for building openfunction itself or the function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure,for the function.
I 'll avoid it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a lighter way (using less runtime memory) to run these services? I'm worried that running these services will again fill up the memory of the server where github action is running CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My plan is to just run the function locally in docker with the image I created by openfunction platform previously,which doesn't need to install many services. I'll update it after test.
|
||
## Enabling the Plugin | ||
|
||
Before configuring the Plugin, you need to have OpenFunction running. The example below shows OpenFunction installed in Helm: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also give a link to the installation page of OpenFunction.
Co-authored-by: Alex Zhang <tokers@apache.org>
Co-authored-by: Alex Zhang <tokers@apache.org>
@jackkkkklee Please make the CI pass. Thanks! |
apisix/plugins/openfunction.lua
Outdated
local function request_processor(conf, ctx, params) | ||
local headers = params.headers or {} | ||
-- setting authorization headers if not already set | ||
if not headers["Authorization"] and conf.authorization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the header here need to be lower-case? And we need a test case that the user-specific Authorization header has higher priority.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reminding.
Lower-case for headers is needed if you mean the local param 'headers', or you mean the header of docs above?
Test case added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The local param headers
.
Co-authored-by: 罗泽轩 <spacewanderlzx@gmail.com>
Co-authored-by: 罗泽轩 <spacewanderlzx@gmail.com>
apisix/plugins/openfunction.lua
Outdated
local function request_processor(conf, ctx, params) | ||
local headers = params.headers or {} | ||
-- setting authorization headers if not already set | ||
if not headers["Authorization"] and conf.authorization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The local param headers
.
ci/init-plugin-test-service.sh
Outdated
@@ -41,3 +41,13 @@ docker exec -i rmqnamesrv /home/rocketmq/rocketmq-4.6.0/bin/mqadmin updateTopic | |||
|
|||
# prepare vault kv engine | |||
docker exec -i vault sh -c "VAULT_TOKEN='root' VAULT_ADDR='http://0.0.0.0:8200' vault secrets enable -path=kv -version=1 kv" | |||
|
|||
# prepare openfunction env | |||
docker pull apisixtestaccount123/sample-go-func:v1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if OpenFunction releases some sample functions 🤔.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They do, but the functions just print the URI of request, which doesn't meet my test cases(e.g. test body, test headers). Therefore, I just fork and change their functions a little bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we expose the Dockerfile for apisixtestaccount123/sample-go-func:v1
?
My concern is that when we want to make updates based on this image later, then it will be difficult for us to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ‘ll add local function building process latter,then we just need to update the code of repositories.
Co-authored-by: Fei Han <97138894+hf400159@users.noreply.github.com>
Co-authored-by: Fei Han <97138894+hf400159@users.noreply.github.com>
Co-authored-by: Fei Han <97138894+hf400159@users.noreply.github.com>
Co-authored-by: Fei Han <97138894+hf400159@users.noreply.github.com>
Co-authored-by: Fei Han <97138894+hf400159@users.noreply.github.com>
Co-authored-by: Fei Han <97138894+hf400159@users.noreply.github.com>
Co-authored-by: Fei Han <97138894+hf400159@users.noreply.github.com>
Co-authored-by: Fei Han <97138894+hf400159@users.noreply.github.com>
Co-authored-by: Fei Han <97138894+hf400159@users.noreply.github.com>
Co-authored-by: Fei Han <97138894+hf400159@users.noreply.github.com>
@tokers Could you please rerun ”CI / build (ubuntu-20.04, linux_openresty, t/node t/pubsub t/router t/script t/stream-node t/utils t/w... (pull_request) Failing after 17m...“? |
done |
ci/init-plugin-test-service.sh
Outdated
@@ -41,3 +41,4 @@ docker exec -i rmqnamesrv /home/rocketmq/rocketmq-4.6.0/bin/mqadmin updateTopic | |||
|
|||
# prepare vault kv engine | |||
docker exec -i vault sh -c "VAULT_TOKEN='root' VAULT_ADDR='http://0.0.0.0:8200' vault secrets enable -path=kv -version=1 kv" | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to touch this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No,we don't . I just run the function image in the dockerfile . And I ’ll discard the change in this file .
Co-authored-by: 罗泽轩 <spacewanderlzx@gmail.com>
� Conflicts: � ci/pod/docker-compose.plugin.yml � docs/en/latest/config.json � docs/zh/latest/config.json
* upstream/master: (214 commits) feat: set constants.apisix_lua_home for used by plugins (apache#7893) fix: response-rewrite plugin might cause Apache AIPSIX hanging (apache#7836) test: sleep 1 second in t/cli/test_upstream_mtls.sh (apache#7889) fix: reload once when log rotate (apache#7869) change: move etcd conf under deployment (apache#7860) fix: plugin metadata missing v3 adapter call (apache#7877) docs: add ClickHouse and Elasticsearch loggers. (apache#7848) docs(plugin): refactor limit-conn.md (apache#7857) feat: call `destroy` method when Nginx exits (apache#7866) feat: Add ability to inject headers via prefix to otel traces (apache#7822) docs(plugin): refactor proxy-cache.md (apache#7858) fix: don't enable passive healthcheck by default (apache#7850) feat: add openfunction plugin (apache#7634) fix(zipkin): send trace IDs with a reject sampling decision (apache#7833) fix: Change opentelemetry's span kind to server (apache#7830) docs(hmac-auth): additional details for generating signing_string (apache#7816) docs: correct the test-nginx description (apache#7818) docs: add docs of workflow plugin (apache#7813) docs(FAQ): add how to detect APISIX alive status (apache#7812) feat: add elasticsearch-logger (apache#7643) ...
Co-authored-by: Alex Zhang <tokers@apache.org> Co-authored-by: 罗泽轩 <spacewanderlzx@gmail.com> Co-authored-by: Fei Han <97138894+hf400159@users.noreply.github.com> Co-authored-by: LCW <Lcw769193516@163.com>
Description
support openfunction serverless platform
implement proposal #7404
Checklist