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

proposal: Enhance Envoy's Lua filter #13307

Closed
nic-chen opened this issue Sep 29, 2020 · 18 comments
Closed

proposal: Enhance Envoy's Lua filter #13307

nic-chen opened this issue Sep 29, 2020 · 18 comments
Labels
area/lua stale stalebot believes this issue/PR has not been touched recently

Comments

@nic-chen
Copy link
Contributor

nic-chen commented Sep 29, 2020

Background

Envoy is a great open source project. Developers can use c++, WASM and Lua to implement plugins.

I come from the Apache APISIX community and am a PMC member of APISIX. Apache APISIX is an API gateway based on Nginx and Lua. We are very willing to join the Envoy community to participate in the development of Envoy Lua to make Envoy even more powerful.

Goals

Apache APISIX plugins run directly in Envoy Lua filter without modify Envoy

Design

We don't need to modify Envoy, just some optimizations, which we believe have been taken into account by the Envoy community based on what is in the Lua filter documentation (https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/lua_filter#overview True global support may be added via an API in the future. ).

We mask platform differences at the plugin layer, and all interfaces that need to be used are abstracted in the underlying framework, which we call apisix.core, so that all plugins can run both on Envoy and Apache APISIX.

The specific architecture is as follows:

image

image

Plan

We can do this in four steps.

  1. Simulate APISIX.core for Envoy and run a simple plugin. Apisix has a redirect plugin, which can redirect client request URI. We have implemented it to run on Envoy without modifying the code of redirect. We could run it as follows:

    git clone https://github.com/api7/envoy-apisix.git
    cd envoy/compose
    docker-compose up -d

    And test it by:

    curl 127.0.0.1:8000/ -i
    
    HTTP/1.1 302 Found
    location: /redirected/path
    server: envoy
    content-length: 4
    date: Tue, 29 Sep 2020 09:25:24 GMT

    We could see it had redirect to the specified path with status 302.

  2. The Lua script can read the plugin configuration written in envoy.yaml. We think reading plugin configuration from metadata should be a feasible solution(to be determined).

  3. The Lua filter of Envoy supports context throughout its lifecycle so that envoy_on_request and envoy_on_response can work well together and synchronize data. For example:

    function envoy_on_request(request_handle, context)
        -- Do something.
    end
    -- Called on the response path.
    function envoy_on_response(response_handle, context)
        -- Do something.
    end

    The context in envoy_on_request and envoy_on_response are the same table object.

  4. When Envoy initializes the Luajit VM, it can load and execute the specified Lua script for initializing Lua global variables and so on.

In 3 and 4, the parts of Envoy that need to be changed, it would be great if the Envoy community could help with this so we can do it simultaneously.

Follow-up optimizations

  • The Lua filter of Envoy supports context (currently implemented using global variables).

  • When initialize the Luajit VM, it can load and execute the specified Lua script to initialize Lua global variables and so on.

Example

filter

name: envoy.filters.http.lua.redirect
  typed_config:
    "@type": type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua
  source_codes:
    apisix_init.lua:
      filename: /apisix/init.lua

config

metadata:
  filter_metadata:
    envoy.filters.http.lua.redirect:
      plugins: 
      - name: redirect
        conf:
          ret_code: 302
          uri: /redirected/path

Envoy community, what do you think of the proposal?
Hope to hear your feedback, thanks.

@nic-chen nic-chen added enhancement Feature requests. Not bugs or questions. triage Issue requires triage labels Sep 29, 2020
@nic-chen
Copy link
Contributor Author

cc @membphis @moonming

@mattklein123 mattklein123 added area/lua and removed enhancement Feature requests. Not bugs or questions. triage Issue requires triage labels Sep 29, 2020
@mattklein123
Copy link
Member

See also #12724. Is this basically the same issue or is there something different about it? In general we have no objection to having Envoy support existing Lua frameworks (it would be great) but would like to understand the changes to the Envoy core/filter that would be required.

@nic-chen
Copy link
Contributor Author

@mattklein123

Thanks for the reply !

I had read pr #12724. It looks like a similar issue, but I guess they need more modifications to envoy.

Our implementation does not need to modify envoy.

In this issue, I have one question and two optimization suggestions for better performance (I believe these two optimizations are also beneficial to other users who use Lua to develop filters).

For clarity, I will divide the question and optimizations into three replies.

@nic-chen
Copy link
Contributor Author

The question is

Is it a recommend way that Using metadata as configuration for APISIX plugins ?

@nic-chen
Copy link
Contributor Author

nic-chen commented Sep 30, 2020

Optimization 1:

The Lua filter of Envoy supports context throughout its lifecycle so that envoy_on_request and envoy_on_response can work well together and synchronize data.

For example:

    function envoy_on_request(request_handle, context)
        context.foo = "bar"
        -- Do something.
    end
    -- Called on the response path.
    function envoy_on_response(response_handle, context)
        print(context.foo) -- should be bar
        -- Do something.
    end

The context in envoy_on_request and envoy_on_response are the same table object.

@nic-chen
Copy link
Contributor Author

Optimization 2:

When Envoy initializes the Luajit VM, it can load and execute specified Lua script(s) for initializing Lua global variables and so on.

Thanks!

@membphis
Copy link

The Lua filter of Envoy supports context throughout its lifecycle so that envoy_on_request and envoy_on_response can work well together and synchronize data.

another style, it seems good.

function envoy_on_request(request_handle)
    local context = envoy.ctx   -- a table for each request
    context.foo = "bar"
    -- Do something
end

function envoy_on_response(response_handle)
    local context = envoy.ctx
    print(context.foo) -- should be "bar"
    -- Do something
end

@mattklein123
Copy link
Member

Is it a recommend way that Using metadata as configuration for APISIX plugins ?

I think this is fine, but happy to discuss a different method if you want to suggest something.

Otherwise the other suggestions seem fine to me. cc @dio for comment.

Thanks!

@dio
Copy link
Member

dio commented Oct 1, 2020

  1. Using metadata as a way to convey config seems fine.

  2. Re: context API

I think inside Envoy this is done either through FilterState or dynamic metadata. The current Lua HTTP filter has dynamic metadata API:

          - name: envoy.filters.http.lua
            typed_config:
              "@type": type.googleapis.com/envoy.config.filter.http.lua.v2.Lua
              inline_code: |
                function envoy_on_request(request_handle)
                  request_handle:streamInfo():dynamicMetadata():set("context", "foo", "bar")
                end
                function envoy_on_response(response_handle)
                  local foo = response_handle:streamInfo():dynamicMetadata():get("context")["foo"]
                  response_handle:logInfo(foo)
                end

To achieve the context API think we can evolve from this to make it "clear" and easy to follow.

  1. Re: init script

I think that will be useful. FWIW we have a couple of ideas to add "global" utilities #12626 (comment).

@nic-chen @membphis To push this forward, can we have a separate issue for each item (and treat this issue as the ☂️ one)? Thank you!

@membphis
Copy link

membphis commented Oct 1, 2020

2. Re: context API
I think inside Envoy this is done either through FilterState or dynamic metadata.

This is a feasible way, we will try it later, thanks.

To push this forward, can we have a separate issue for each item (and treat this issue as the ☂️ one)? Thank you!

got it ^_^

@nic-chen
Copy link
Contributor Author

nic-chen commented Oct 1, 2020

@dio @mattklein123

Thank you for your replies and provide us with a nice solution !

So we can continue to move forward. In this process, there may be other issues that need your help, thank you in advance.

For the last item, I had create a new issue #13347 to push it forward.

@github-actions
Copy link

github-actions bot commented Dec 9, 2020

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Dec 9, 2020
@nic-chen
Copy link
Contributor Author

we are still working on it.

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Dec 10, 2020
@github-actions
Copy link

github-actions bot commented Jan 9, 2021

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Jan 9, 2021
@moonming
Copy link

@nic-chen Any progress?

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Jan 10, 2021
@nic-chen
Copy link
Contributor Author

@nic-chen Any progress?

we have supported plugins:

And more plugins are in process.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Feb 22, 2021
@github-actions
Copy link

github-actions bot commented Mar 2, 2021

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

@github-actions github-actions bot closed this as completed Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/lua stale stalebot believes this issue/PR has not been touched recently
Projects
None yet
Development

No branches or pull requests

5 participants