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

feat(wasmx): DB entities and admin API #10317

Merged
merged 1 commit into from
Mar 2, 2023
Merged

Conversation

flrgh
Copy link
Contributor

@flrgh flrgh commented Feb 17, 2023

This is still WIP and not ready for a final review, but feel free to share feedback in the meantime.

For the sake of simplicity, this implementation bundles the entire filter chain array into a JSON-serialized array. We can break filter entities off into their own table/entity with a FK relationship if need be (should decide on this before release).

changes

  • new wasm_filter_chains entity
    • schema
    • migrations
    • DB tests
    • admin API tests

todo

  • filter registration/metadata/schema integration
    • validate wasm_filter_chains.filters[].name against known filters
    • validate wasm_filter_chains.filters[].config

@hbagdi
Copy link
Member

hbagdi commented Feb 21, 2023

Were we not going for a single plugin that takes in the lists of filters to execute (and their configurations) to start small?

@hishamhm
Copy link
Contributor

@hbagdi No, we ultimately reversed on that decision because the execution flow between Lua plugins and Wasm filters (each happening on their own Nginx module) imposes some constraints that wouldn't be correctly represented if Wasm filters were encapsulated by a plugin.

@thibaultcha
Copy link
Member

thibaultcha commented Feb 24, 2023

Were we not going for a single plugin that takes in the lists of filters to execute (and their configurations) to start small?

That was the 2022 Tech Preview model. The product decision we made was to not depend on a Lua Plugin, but incorporate wasm filters as part of the core.

Copy link
Member

@thibaultcha thibaultcha left a comment

Choose a reason for hiding this comment

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

LGTM so far.

@hbagdi hbagdi added the version-compatibility Incompatibility with older data plane versions label Mar 1, 2023
@flrgh flrgh marked this pull request as ready for review March 1, 2023 22:56
@flrgh
Copy link
Contributor Author

flrgh commented Mar 1, 2023

I've been banging my head on the wall about filter config schema validation for a minute now and still don't have it fully functional and sane, so I'm punting on that for the moment. I'd rather we just get this PR merged as-is so that everyone is unblocked.

@locao locao merged commit c94e225 into feat/wasmx Mar 2, 2023
@locao locao deleted the feat/wasmx-data-model branch March 2, 2023 15:22
locao pushed a commit that referenced this pull request Mar 3, 2023
locao pushed a commit that referenced this pull request Mar 30, 2023
flrgh added a commit that referenced this pull request Apr 12, 2023
flrgh added a commit that referenced this pull request May 4, 2023
flrgh added a commit that referenced this pull request May 23, 2023
flrgh added a commit that referenced this pull request Jun 7, 2023
locao pushed a commit that referenced this pull request Jul 13, 2023
locao pushed a commit that referenced this pull request Jul 14, 2023
flrgh added a commit that referenced this pull request Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants