storage: Handle multi-bundle data with overlapping roots #5005
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ashutosh-narkar
force-pushed
the
root-overlap-fix
branch
from
5795bb0
to
a7dc478
Compare
srenatus
reviewed
Aug 15, 2022
| // BasePaths indicates the top-level paths where write operations will be performed in this transaction. | ||
| BasePaths []string | ||
|
|
||
| // RootOverwrite is deprecated. Use BasePaths instead. | ||
| RootOverwrite bool |
There was a problem hiding this comment.
Would code using the TransactionParams before now break because BasePaths doesn't equal ""? Or do we handle that backwards-compatibly via RootOverwrite? 🤔
There was a problem hiding this comment.
Good point. We can support backwards-compatibly via RootOverwrite for now and then remove it in a future release. Updated the code.
ashutosh-narkar
force-pushed
the
root-overlap-fix
branch
3 times, most recently
from
8fa094c
to
b7c2b0e
Compare
If the bundles being activated share a manifest root prefix, it would result in overwriting the bundle data based on the activation order. This happened since the truncate call writes data to the store based on the top-level keys in the data. When multiple bundles with overlapping bundle root prefixes are being activated as part of the same txn, adding data to the store by iterating over the top-level keys in the data object would result in an unintended overwrite. The truncate call would be able to properly write data if it had knowledge of the bundle roots. This commit passes the bundle roots to the truncate call to assist in writing data to the store. Fixes: open-policy-agent#4998 Signed-off-by: Ashutosh Narkar <[email protected]>
ashutosh-narkar
force-pushed
the
root-overlap-fix
branch
from
b7c2b0e
to
552b58d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the bundles being activated share a manifest root prefix, it
would result in overwriting the bundle data based on the activation
order. This happened since the truncate call writes data to the
store based on the top-level keys in the data. When multiple
bundles with overlapping bundle root prefixes are being activated
as part of the same txn, adding data to the store by iterating
over the top-level keys in the data object would result in an unintended
overwrite. The truncate call would be able to properly write
data if it had knowledge of the bundle roots. This commit passes
the bundle roots to the truncate call to assist in writing data
to the store.
Fixes: #4998
Signed-off-by: Ashutosh Narkar [email protected]