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): combine config and json_config fields #11697

Merged
merged 1 commit into from
Oct 11, 2023
Merged

Conversation

flrgh
Copy link
Contributor

@flrgh flrgh commented Oct 5, 2023

Summary

This change combines the config and json_config fields for Wasm filters.

Before, JSON configuration lived in the json_config attribute, and config could only hold strings:

{
  filters = {
    {
      name = "my-filter-with-string-config",
      config = "a=1 b=2",
    },
    {
      name = "my-filter-with-json-config",
      json_config = {
        a = 1,
        b = 2,
      }
    },
  }
}

Now, both string and JSON configurations live in config:

{
  filters = {
    {
      name = "my-filter-with-string-config",
      config = "a=1 b=2",
    },
    {
      name = "my-filter-with-json-config",
      config = {
        a = 1,
        b = 2,
      }
    },
  }
}

The json_config field/attribute was introduced recently and is unreleased, so this change does not break any existing API.

Note: filters may only use JSON in config if they have supplied a JSON schema in a {{filter}}.meta.json file. Filters without a JSON schema may still accept JSON configuration, but it must be serialized, and it will not be validated in any way by the admin API:

{
  filters = {
    {
      name = "my-filter-with-json-config-and-schema",
      config = {
        a = 1,
        b = 2,
      }
    },
    {
      name = "my-filter-with-json-config-but-no-schema",
      config = [[{
        "a": 1,
        "b": 2
      }]]
    },
  }
}

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 - PUT DOCS PR HERE

Full changelog

  • implement json_schema.default fallback schema for dynamic JSON subschemas
  • Remove filter_chain.filters[].config and rename filter_chain.filters[].json_config to filter_chain.filters[].config
  • Create a default/fallback schema for filter_chain.filters[].config of { "type": [ "string", "null" ] }

Issue reference

KAG-2707

@flrgh flrgh force-pushed the feat/wasm-json-config branch from 4189c70 to c61c577 Compare October 5, 2023 22:26
@flrgh flrgh force-pushed the feat/wasm-json-config branch from c61c577 to 5b09011 Compare October 5, 2023 22:27
@flrgh flrgh changed the title WIP: rename filter config/json_config fields WIP: combine filter config/json_config fields Oct 5, 2023
@flrgh flrgh force-pushed the feat/wasm-json-config branch 2 times, most recently from 912f92f to 1e4a488 Compare October 6, 2023 01:45
@flrgh flrgh changed the title WIP: combine filter config/json_config fields feat(wasm): combine config and json_config fields Oct 6, 2023
@flrgh flrgh marked this pull request as ready for review October 6, 2023 01:56
@flrgh flrgh requested review from hishamhm and locao October 6, 2023 02:03
filter.json_config = nil
-- Serialize all JSON configurations up front
--
-- NOTE: there is a subtle difference between a raw, non-JSON filter
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reviewers: we've judged filters that require a JSON-encoded string to be an uncommon use case, compared to, say, an object or an array, which will still work perfectly fine.

@flrgh flrgh force-pushed the feat/wasm-json-config branch from 1e4a488 to a963cb1 Compare October 9, 2023 18:05
@flrgh flrgh added this to the 3.5.0 milestone Oct 10, 2023
@flrgh
Copy link
Contributor Author

flrgh commented Oct 10, 2023

note: changes to Wasm have been given special approval for merge after feature freeze, due to the feature's self-contained nature and current beta/tech preview/unstable status.

@flrgh flrgh force-pushed the feat/wasm-json-config branch from a963cb1 to abd546f Compare October 10, 2023 17:14
@flrgh
Copy link
Contributor Author

flrgh commented Oct 10, 2023

just rebased

This allows JSON filter configuration to live in the same .config
attribute as raw configuration. Filters must have an associated
JSON schema in order to use this feature.
@flrgh flrgh force-pushed the feat/wasm-json-config branch from be69334 to 018c5bf Compare October 11, 2023 17:49
@flrgh flrgh merged commit 064ea58 into master Oct 11, 2023
23 checks passed
@flrgh flrgh deleted the feat/wasm-json-config branch October 11, 2023 18:17
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.

3 participants