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

fix(dbless): perform uniqueness checks on unique fields #11199

Merged
merged 7 commits into from
Jul 10, 2023

Conversation

hishamhm
Copy link
Contributor

@hishamhm hishamhm commented Jul 10, 2023

Summary

DB-less mode wasn't performing proper uniqueness checks on declarative config inputs. This is especially problematic on
primary and endpoint keys. This would cause entities to be silently dropped: in the event of conflicting keys, the last one lexically present in the input file would take precedence.

For other unique fields, when using Hybrid mode you would eventually get a database error as the DB would have the uniqueness constraint set, but for pure DB-less you would get inconsistent data.

Checklist

  • The Pull Request has tests
  • There's an entry in the CHANGELOG

Full changelog

  • Declarative config now performs proper uniqueness checks against its inputs: previously, it would silently drop entries with conflicting primary/endpoint keys, or accept conflicting unique fields silently.

flrgh and others added 5 commits July 10, 2023 10:14
Further changes in the DB-less UUID generation algorithm
may change this value. (Ooh, foreshadowing!)

Signed-off-by: Hisham Muhammad <hisham@gobolinux.org>
DB-less mode wasn't performing proper uniqueness checks on
primary and endpoint keys.

This would cause entities to be silently dropped: in the event
of conflicting keys, the last one lexically present in the input
file would take precedence.

The uniqueness check for primary keys needs to happen here,
at an early stage where the maps `by_id` and `by_key` are built,
otherwise entities are silently lost.

Other uniqueness checks can happen later, at the flattening stage.
See the following commit.
This will be useful later when we trigger uniqueness checks
to fields of flattened entities.
@hishamhm hishamhm force-pushed the fix/dbless-uniqueness branch from bfe963b to 82e6b99 Compare July 10, 2023 13:15
@hishamhm hishamhm requested a review from locao July 10, 2023 13:23
@hishamhm hishamhm added bug and removed bug labels Jul 10, 2023
hishamhm and others added 2 commits July 10, 2023 10:26
This would cause uniqueness constraints to be missed if two
entities have distinct endpoint keys but their cache keys rely
on the uniqueness of other fields. When using a database, this
would be usually be caught by explicit DB constraints, but
here we subsume the cache key uniqueness constraint to the
generated UUID. This has the nice property that it saves us
from performing an extra explicit check for the uniqueness of
composite cache keys, since id's are now being properly
checked for uniqueness.

Co-authored-by: Michael Martin <michael.martin@konghq.com>
@hishamhm hishamhm force-pushed the fix/dbless-uniqueness branch from 82e6b99 to 2b2e4c4 Compare July 10, 2023 13:26
@locao locao merged commit 9ff7af1 into master Jul 10, 2023
@locao locao deleted the fix/dbless-uniqueness branch July 10, 2023 14:40
@hishamhm hishamhm restored the fix/dbless-uniqueness branch July 11, 2023 14:07
@outsinre
Copy link
Contributor

outsinre commented Jul 17, 2023

This PR seems break an EE function: https://github.com/Kong/kong-ee/pull/6023#discussion_r1265101739

New Fix #11236

@kong-rob kong-rob deleted the fix/dbless-uniqueness branch January 29, 2025 20:53
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.

4 participants