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(wasm): add wasm plugin interface #13843

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat(wasm): add wasm plugin interface #13843

wants to merge 2 commits into from

Conversation

flrgh
Copy link
Contributor

@flrgh flrgh commented Nov 7, 2024

Summary

This adds a new interface to wasm filters, allowing users to configure them in the same manner as a plugin entity.

From the admin API/database/plugin entity side of things, inspiration was taken from the implementation of plugin servers, so there are parallels with regards to how filters and their schemas are discovered. Some refactoring was done so that the filter Lua schema could be reused between both the filter_chain entity and this new plugin schema interface.

This feature works in concert with the existing filter chain model. On the runloop side, we iterate over plugin entities to discover the wasm filters and create proxy-wasm filter chains on-the-fly. When an existing filter_chain entity exists for a service/route, we append filters to it.

Checklist

  • The Pull Request has tests
  • A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
  • There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com

Issue reference

KAG-5616

@github-actions github-actions bot added core/proxy core/db core/configuration schema-change-noteworthy cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/wasm Everything relevant to [proxy-]wasm labels Nov 7, 2024
@flrgh flrgh marked this pull request as ready for review November 7, 2024 17:48
@flrgh flrgh requested review from brentos and locao November 7, 2024 17:48
Comment on lines 1190 to 1196
k = tostring(k)
local field = self.fields[k]
local subschema_field = subschema_fields[k]

elseif not json_schema.optional then
errors[k] = validation_errors.JSON_PARENT_KEY_MISSING:format(k, parent_key)
end
end
if field and field.type == "json"
or (subschema_field and subschema_field.type == "json")
then
errors[k] = validate_json_field(subschema_field or field, k, input)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The plugins entity makes use of subschemas to extend its schema/entity validation. Prior to this PR, it wasn't possible to validate json-typed fields that were members of a subschema, so that is the reason for this change.

@flrgh flrgh force-pushed the feat/wasm-plugins branch 3 times, most recently from 51b12ed to 9e4e054 Compare November 8, 2024 00:51
Copy link
Contributor

@locao locao left a comment

Choose a reason for hiding this comment

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

What do you think about the dbless tests?

}
end

for _, strategy in helpers.each_strategy({ "postgres" }) do
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have a set of tests for dbless? I'd like to see a filter being added and removed from a route/service. I think that's a case we could miss when upgrading the wasm module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tests in spec/02-integration/20-wasm/09-filter-meta_spec.lua are already generic over db/dbless modes, so this round I opted to expand them to cover the route+filter plugin creation and request use case.

What do you think? Enough, or add more dbless tests? Happy to add more, but I'll need to refactor the tests in this file to some degree to separate the admin API tests (e.g. POST /plugins) from the proxy request behavioral tests.

kong/conf_loader/init.lua Outdated Show resolved Hide resolved
Comment on lines +1080 to +1081
if t1 == "record" and t2 == "json" then
return true
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a change we must communicate to other control planes devs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this schema file out of entities dir?

Copy link
Contributor Author

@flrgh flrgh Nov 12, 2024

Choose a reason for hiding this comment

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

This schema for wasm filter objects/records originally lived in kong/db/schema/entities/filter_chains.lua. To make it reusable for both kong/db/schema/entities/filter_chains.lua and kong/runloop/wasm/plugins.lua, it was factored out into its own file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee core/configuration core/db core/proxy core/wasm Everything relevant to [proxy-]wasm schema-change-noteworthy size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants