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

Loading partially overlapping data from multiple bundles results incorrect state #4998

Closed
sirpi opened this issue Aug 10, 2022 · 1 comment · Fixed by #5005
Closed

Loading partially overlapping data from multiple bundles results incorrect state #4998

sirpi opened this issue Aug 10, 2022 · 1 comment · Fixed by #5005
Assignees
Labels
bug investigating Issues being actively investigated

Comments

@sirpi
Copy link

sirpi commented Aug 10, 2022

Short description

When OPA 0.43.0 loads two bundles (via config or discovery) whose paths are partially overlapping (e.g. store/a and store/b)
then final state contains only store/a or store/b, not both
Reproduceable on Windows or Kubernetes, with the latest version: 0.43.0 (while 0.42.2 works fine)

Steps To Reproduce

  1. Create two very simple bundles, only containing manifest and data:

a.tar.gz

  • .manifest:
    { "revision": "a", "roots": ["store/a"] }
  • store/a/data.json:
    { "v1" : {}}

b.tar.gz

  • .manifest:
    { "revision": "b", "roots": ["store/b"] }
  • store/b/data.json:
    { "v1" : {} }
  1. load them by config or by discovery, e.g. with the following config.yml:
services:
- name: storage-service
  ...
bundles:
  a:
    resource: "/bundles/a"
    service: storage-service
  b:
    resource: "/bundles/b"
    service: storage-service
  1. validate the data via REST:
    GET localhost:8181/v1/data

the result contains exactly one of the data, depending on the loading order:

{
  "result": {
    "store": {
      "a": {
        "v1": {}
      }
    }
  }
}

when the loading order is b and then a

{"level":"info","msg":"Starting bundle loader.","name":"a","plugin":"bundle","time":"2022-08-10T16:03:01+02:00"}
{"level":"info","msg":"Starting bundle loader.","name":"b","plugin":"bundle","time":"2022-08-10T16:03:01+02:00"}
{"level":"info","msg":"Bundle loaded and activated successfully. Etag updated to \"d7afa6a214d03f32ea740f60e8bc0e03\".","name":"b","plugin":"bundle","time":"2022-08-10T16:03:03+02:00"}
{"level":"info","msg":"Bundle loaded and activated successfully. Etag updated to \"8a5f55ef77291df643eda67b872ef684\".","name":"a","plugin":"bundle","time":"2022-08-10T16:03:04+02:00"}

OR

{
  "result": {
    "store": {
      "b": {
        "v1": {}
      }
    }
  }
}

when the loading order is a and then b

{"level":"info","msg":"Starting bundle loader.","name":"a","plugin":"bundle","time":"2022-08-10T16:04:11+02:00"}
{"level":"info","msg":"Starting bundle loader.","name":"b","plugin":"bundle","time":"2022-08-10T16:04:11+02:00"}
{"level":"info","msg":"Bundle loaded and activated successfully. Etag updated to \"8a5f55ef77291df643eda67b872ef684\".","name":"a","plugin":"bundle","time":"2022-08-10T16:04:13+02:00"}
{"level":"info","msg":"Bundle loaded and activated successfully. Etag updated to \"d7afa6a214d03f32ea740f60e8bc0e03\".","name":"b","plugin":"bundle","time":"2022-08-10T16:04:13+02:00"}

Similarly, if there are multiple (more than two) bundles containing data, only the one which has been loaded last remains and the others are removed from the json tree.

Expected behavior

on 0.42.2 the above REST call works as intended and returns the following structure:

{
  "result": {
    "store": {
      "a": {
        "v1": {}
      },
      "b": {
        "v1": {}
      }
    }
  }
}

The result is also correct even on 0.43.0 if the either

  • the bundles are loaded by command line parameters:
    opa run -s --skip-verify -b a.tar.gz -b b.tar.gz
  • or the paths are totally independent, e.g. store1/a and store2/b

Additional context

N/A

@sirpi sirpi added the bug label Aug 10, 2022
@ashutosh-narkar ashutosh-narkar added the investigating Issues being actively investigated label Aug 10, 2022
@ashutosh-narkar ashutosh-narkar self-assigned this Aug 10, 2022
@ashutosh-narkar
Copy link
Member

Thanks for reporting the issue @sirpi. We're able to repro it and will have a fix soon.

ashutosh-narkar added a commit to ashutosh-narkar/opa that referenced this issue Aug 17, 2022
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 added a commit that referenced this issue Aug 17, 2022
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug investigating Issues being actively investigated
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants