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.

Sign up for GitHub

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

Jump to bottom

[cherry release/3.4.x] cherry-picking important fixes to 34x #12403

Merged
merged 8 commits into from
Mar 8, 2024
Merged

[cherry release/3.4.x] cherry-picking important fixes to 34x #12403

merged 8 commits into from
Mar 8, 2024

Conversation

windmgc
Copy link
Member

@windmgc windmgc commented Jan 24, 2024 •
edited
Loading

Summary

This PR cherry-picks some of the important fixes to release/3.4.x so that we can get alignment between release/3.4.x and next/3.4.x.x

Contains the following PRs:
#11613
#11566
#11859
#12003
#11926
#11464
#11414
#11437

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

@windmgc windmgc marked this pull request as ready for review January 24, 2024 03:04
@VicYP VicYP requested a review from fffonion January 24, 2024 03:08
@kikito kikito requested a review from chobits January 24, 2024 10:08
samugi
samugi approved these changes Jan 24, 2024
View reviewed changes
Copy link
Member

@samugi samugi left a comment

Choose a reason for hiding this comment

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

Reviewed:

  • fix(rate-limiting): counters accuracy with redis policy & sync_rate
  • fix(plugin): RL instances sync to the same DB at same rate

lgtm

@tzssangglass
Copy link
Member

Reviewed:

  • fix(pdk): use deep copies of Route, Service, and Consumer objects when log serialize

LGTM

@locao
Copy link
Contributor

locao commented Mar 5, 2024

@windmgc it seems you already have enough approvals but the branch has conflicts, can you review them, please?

bungle and others added 8 commits March 7, 2024 17:44
@bungle @windmgc
chore(deps): bump resty.aws from 1.3.2 to 1.3.5 (#11613)
b9c9e45 
- fix: lazily initialize structures to avoid c-boundary errors on require
  [87](Kong/lua-resty-aws#87)

- fix: remove more module-level uses of config.global
  [83](Kong/lua-resty-aws#83)

- fix: don't invoke region detection code on the module toplevel and advise against trying to.
  [81](Kong/lua-resty-aws#81)

Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
@sabertobihwy @windmgc
fix(pdk): use deep copies of Route, Service, and Consumer objects whe…
4878605 
…n log serialize (#11566)

In the function of `kong.log.serialize`, the lifecycle of
three table `ctx.route`, `ctx.service` and `ctx.authenticated_consumer`
is across request. Modifying the sub-items in the current request of
the table will affect the next request, resulting in unexpected behavior
in Kong.

Fix FTI-5357

---------

Signed-off-by: sabertobihwy <sabertobihwy@gmail.com>
@samugi @windmgc
fix(rate-limiting): counters accuracy with redis policy & sync_rate (#…
0e908c7 
…11859)

* fix(rate-limiting): redis async updates

When the periodic sync to redis feature is turned on, using the
`sync_rate` configuration option, keys are incremented by steps of 2
instead of 1 for requests that arrive after the `sync_rate` interval
has expired.

This happens because after each sync, the key is loaded again from redis
and also incremented atomically (see: #10559)
however the next call to `increment` also adds 1 to its value, so
the key is incremented by 2 every time it's loaded from redis.

This fix sets a negative delta for the key when
`conf.sync_rate ~= SYNC_RATE_REALTIME` and the key was loaded from redis
in order to invalidate the next call to `increment`.

Includes a small code refactor
@StarlightIbuki @samugi @windmgc
fix(plugin): RL instances sync to the same DB at same rate (#12003)
f9dc649 
All rate-limiting plugin instance syncs with the same plugin config, that is the very first config got hit by a request, and they all sync with the same rate. Even a config update won't change the DB to be synced.

The timer will sync not just the same instance's counters but all counters in the same DB. This is a compromise given the emergency and we prefer simplicity over correctness for this behavior.

Full changelog
- The counter table is split with DB;
- Timers are created when a request hits;
- The sync_rate is guaranteed with limited running timers and timer delay
- Cover the case in the integration test by "with_sync_rate"

Fix KAG-2904

Co-authored-by: samugi <samuele@konghq.com>
@bungle @windmgc
fix(db): pg store connection called without self
3c004ab 
### Summary

The PR #11480 introduced a bug that calls `store_connection`
without passing `self`. This fixes that.

Signed-off-by: Aapo Talvensaari <aapo.talvensaari@gmail.com>
@chobits @windmgc
fix(db/declarative): fix TTL not working in DB-less and Hybrid mode (#…
f8e23b2 
…11464)

* fix(declarative): fix TTL not working in DB-less and Hybrid mode

* Previously, in DB-less and Hybrid mode, the ttl/updated_at fields were
  not copied from the original entities to the flattened entities. As a
  result, the entities were loaded without the TTL field.
* Additionally, for loading the TTL field, the "off" DB strategy
  (lmdb) did not properly filter expired items, nor returned right
  TTL value for DAO.

FTI-4512

* fix coding style
* fix coding style: improved function name
* added test case: hybrid mode for key-auth
* fix test case warnings
* fixed test case consumer domain
* export ttl as absolute value
* delete unused defination
* move ttl-fixing logic into row_to_entity()
* still use pg to caculate relative value
* clean code
* add changelog entry
* fixed test cases
* fixed test cases warning
* fixed test failure
* fix test case issue: ttl expiration
* fix test case: unsed local variable
* add an entry in CHANGELOG.md
* fix changelog scope
* remove release-related information in CHANGELOG.md
* fix test case: sleep before attempting unnecessary requests
* sleep before attempting unnecessary requests
* decrease the ttl to expedite the case's execution
* fix CHANGELOG typo
* fix the tense problem of changelog entry
* add export options for "page_*_for_export" sql statement
* fix warning: setting non-standard global variable
* fix error reporting: options is nil
* fix an issue where the off strategy returned the expired entity
* run ttl processing before schema:process_auto_fields()
@oowl @windmgc
fix(cache): make acl entities group cache warmup (#11414)
af322f9 
kong acl plugin will need to get all consumer groups by customer id (because consumer group and acl plugin config some complex connection), these entities can not be warmed up by Kong, The "warm up" mechanism mainly ensures that all entities are loaded into the cache and enables more precise SQL queries (based on cache key definitions) https://github.com/Kong/kong/blob/master/kong/plugins/acl/daos.lua#L8. However, for scenarios where it is necessary to iterate through all entities under a consumer ID, caching is not possible. https://github.com/Kong/kong/blob/master/kong/plugins/acl/groups.lua#L45

So this PR tries a patch warmup mechanism to support acls entities config.

FTI-5275
@oowl @windmgc
fix(core): fix response status code is not real upstream status when …
edf3ed6 
…using kong.response function (#11437)

When kong uses kong_buffered_http and upstream timeout, the return status code needs to return 504 (which return by ngx.location.capture), not 502.

FTI-5320
@windmgc windmgc force-pushed the cherry-pick-important-fixes-34x branch from ed37f2c to edf3ed6 Compare March 7, 2024 09:45
@windmgc
Copy link
Member Author

windmgc commented Mar 7, 2024 •
edited
Loading

@locao Done! It can be REBASE merged when all CI passed

@windmgc windmgc merged commit 7095210 into release/3.4.x Mar 8, 2024
24 checks passed
@windmgc windmgc deleted the cherry-pick-important-fixes-34x branch March 8, 2024 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chobits chobits chobits approved these changes

@fffonion fffonion fffonion approved these changes

@samugi samugi samugi approved these changes

@tzssangglass tzssangglass tzssangglass approved these changes

@hanshuebner hanshuebner Awaiting requested review from hanshuebner

@bungle bungle Awaiting requested review from bungle

@chronolaw chronolaw Awaiting requested review from chronolaw

Assignees

@windmgc windmgc

Labels
core/db core/logs core/wasm Everything relevant to [proxy-]wasm plugins/acl plugins/rate-limiting schema-change-noteworthy size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@windmgc @tzssangglass @locao @chobits @fffonion @samugi @bungle @sabertobihwy @StarlightIbuki @oowl