Skip to content

fix: Merge extra on update instead of replacing#787

Open
mashawes wants to merge 2 commits into
openstack-experimental:mainfrom
mashawes:extra-merge
Open

fix: Merge extra on update instead of replacing#787
mashawes wants to merge 2 commits into
openstack-experimental:mainfrom
mashawes:extra-merge

Conversation

@mashawes

Copy link
Copy Markdown
Collaborator

Fixes update operations so the extra JSON field is merged with the existing value instead of being fully replaced, matching Python Keystone behavior

  • Adds a shared merge_extra helper in crates/core/src/db.rs: the currently stored extra object is used as the base, keys in the update overwrite or add entries, and a key whose new value is null unsets that key. This preserves properties the caller did not resend while still allowing individual properties to be cleared.
  • Wires the helper into all four driver update operations that handle extra: service, region, and endpoint in the catalog-sql driver, and user in the identity-sql driver. Each now reads the existing row's extra and merges rather than overwrites.
  • Adds unit tests for merge_extra covering preserve, overwrite, add, null-unset, no-op null, and empty/missing existing blobs, plus an integration test under tests/integration/src/catalog/service/update.rs that creates a service with extra and verifies an update preserves an untouched key, adds a new one, and unsets a key via null.

Closes #777

Note: this contribution was developed with AI assistance.

@gtema gtema left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The change on it's own is clean, there is however one critical issue: provider sends one value for update (which is also logged in traces), but drivers persist a different value. It also opens doors for situations that for 1 provider one drivers merges the data, while the second one misses it and overwrites the data. The same is valid across providers.

It must be responsibility of the provider to prepare the data, while the backend driver is only responsible for persistence.

@mashawes mashawes requested a review from gtema June 12, 2026 15:02
@mashawes mashawes self-assigned this Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

update_XXX treatment of "extra" field cause full reset of unchanged properties

2 participants