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(plugin)(correlationId) increase priority order for plugin to mat… #13581

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

Conversation

andrewgkew
Copy link
Contributor

@andrewgkew andrewgkew commented Aug 27, 2024

Summary

Increase priority order for plugin to match EE so it can be used in other/custom plugins

Checklist

Issue reference

Fix #[issue number]

@github-actions github-actions bot added plugins/correlation-id cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Aug 27, 2024
@team-eng-enablement team-eng-enablement added the author/community PRs from the open-source community (not Kong Inc) label Aug 27, 2024
@jeremyjpj0916
Copy link
Contributor

I patch my existing correlation id plugin in our own runtimes to execute earliest too as it makes sense to be able to capture that in other http logging plugins etc.

nowNick and others added 13 commits September 5, 2024 10:26
…eir values mismatch (Kong#13565)

* fix(*): reject config if both deprecated and new field defined and their values mismatch

This commit adds a validation to deprecated fields
that checks if in case both new field and old field were defined -
their values must match.

Note that if one of the fields is null then the validation passes even
if the other is not null.

KAG-5262

* fixup! fix(*): reject config if both deprecated and new field defined and their values mismatch

PR fixes

* fixup! fix(*): reject config if both deprecated and new field defined and their values mismatch

PR fixes 2
add an action to enable executing the `/prdiff <other-pr-url>` command,
to get the diff between the current PR's changes and the other PR's.

This gives a quick overview of the differences between two PRs and is
meant to facilitate the process of reviewing cherry-picks.
### Summary

Adds libada and lua-resty-ada as a dependency.

This is needed for:
Kong#12758

But it may be great for many other uses too.

The `lua-resty-ada` LuaJIT FFI bindings can be found here:
https://github.com/bungle/lua-resty-ada

Signed-off-by: Aapo Talvensaari <[email protected]>
…t_redis_connection()` are mistaken (Kong#13613)

* fix(rate-limiting-plugin): fix a bug where the return values from
`get_redis_connnection()` are mistaken

* fix changelog
It is one of the serial refactors of helpers.lua
KAG-5236

The function `kong_exec()` needs an upvalue `conf`, which must be reloaded each time.
It is one of the serial refactors of helpers.lua
This function (reload_module) may be used by test cases outside.
It is one of the serial refactors of helpers.lua

KAG-5242
…ong#13610)

This PR modifies the `cache_key` function of the vault entity to always generate a cache key without workspace id.

Vault entity is workspace-able, but our secret rotation timer always run without workspace settings(thus the default workspace is being used), so during secret rotation, the code https://github.com/Kong/kong/blob/4e38b965b922f57febe8652fb96b7d74aeab591a/kong/pdk/vault.lua#L620-L621 will generate a duplicate vault cache with default workspace id for each non-default workspace vault entity, and those cache will never be refreshed. The result of this issue is that when you update a vault entity's configuration inside a non-default workspace, it will never take effect in the secret rotation.

Since the prefix of vault entity is unique across workspaces, it should be safe to only use one cache key without workspace id, so that the correct cache is used during secret rotation.

FTI-6152
A small PR for adding a null handling code for the multiValueHeaders field. Also contains a small refactor to put isBase64Encode check into the validate function to keep consistency with other fields

FTI-6168
…lugins iterator (Kong#13602)

the workspace id is needed in some cases, for example in the configure handler.

https://konghq.atlassian.net/browse/FTI-6200
@bungle
Copy link
Member

bungle commented Sep 5, 2024

Can we do this in non-major release, as it is a breaking change (though having this difference does make moving from CE to EE an vice-versa as a potentially breaking too)? Personally I am fine with that.

@gszr
Copy link
Member

gszr commented Sep 5, 2024

Can we do this in non-major release, as it is a breaking change (though having this difference does make moving from CE to EE an vice-versa as a potentially breaking too)? Personally I am fine with that.

Generally, changing plugins ordering is a breaking change and should be avoided in a non-major; especially if the change decreases the priority - ie, its side effects cease being visible to plugins it previously was visible. In this particular case, this does not happen: it expands the list of plugins that have access to the id. Also, there's no change in functionality. I would say it's okay to merge in a minor or even a patch.

@andrewgkew
Copy link
Contributor Author

@gszr - when do you think this will make it in a version of CE?

@gszr
Copy link
Member

gszr commented Sep 10, 2024

@gszr - when do you think this will make it in a version of CE?

@andrewgkew This will likely be included in 3.9, which is planned for late December.

@dndx
Copy link
Member

dndx commented Sep 10, 2024

I think @bungle made a good point, technically this is a breaking change, but if the EE version already increased the priority and we have not heard much complaints, it should be enough proof that this user probably doesn't notice the difference. Wouldn't consistency be more of a consideration here?

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.