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

[Multiple DataSource] Do not support import data source object to Local cluster when not enable data source #6395

Merged
merged 21 commits into from
Apr 16, 2024

Conversation

yujin-emma
Copy link
Contributor

@yujin-emma yujin-emma commented Apr 10, 2024

Description

Transfer saved object from a data source to local cluster can cause troubles, when a cluster disable Multiple Datasource feature, there is no data source concept, any saved objects containing data source information will fail to work, we should not support import saved object from a data source to cluster that is not enabled Multiple Datasource

Issues Resolved

#6115

Screenshot

Testing the changes

When not enable data source

Test import fail when import data-source obj only

single-ds

Test import fail all when import exported obj from MDS cluster

ds-exported

Test import fail all when mixed non-MDS exported objs and MDS exported objs in a single import call

mixed-ds-exported

Test import success when import non-MDS exported objs

non-ds

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

yujin-emma and others added 5 commits April 11, 2024 00:23
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
@yujin-emma
Copy link
Contributor Author

@yujin-emma
Copy link
Contributor Author

yujin-emma and others added 5 commits April 15, 2024 04:41
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Copy link

codecov bot commented Apr 16, 2024

Codecov Report

Attention: Patch coverage is 12.50000% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 45.14%. Comparing base (e785fd3) to head (7d1d90a).
Report is 8 commits behind head on main.

❗ Current head 7d1d90a differs from pull request most recent head 0aef7c1. Consider uploading reports for the commit 0aef7c1 to get more accurate results

Files Patch % Lines
...erver/saved_objects/import/import_saved_objects.ts 0.00% 9 Missing ⚠️
.../server/saved_objects/import/validate_object_id.ts 33.33% 4 Missing ⚠️
src/core/server/saved_objects/routes/import.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6395      +/-   ##
==========================================
- Coverage   49.55%   45.14%   -4.42%     
==========================================
  Files        2670     1654    -1016     
  Lines       54290    33563   -20727     
  Branches     8878     6378    -2500     
==========================================
- Hits        26906    15151   -11755     
+ Misses      25720    17242    -8478     
+ Partials     1664     1170     -494     
Flag Coverage Δ
Windows_1 ?
Windows_3 45.14% <12.50%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ZilongX ZilongX merged commit 9735599 into opensearch-project:main Apr 16, 2024
72 checks passed
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-6395-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 97355995c89a78bf45ebbdcbe3364fa8cc6eafe6
# Push it to GitHub
git push --set-upstream origin backport/backport-6395-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-6395-to-2.x.

@@ -64,6 +64,7 @@ export const registerImportRoute = (router: IRouter, config: SavedObjectConfig)
workspaces: schema.maybe(
schema.oneOf([schema.string(), schema.arrayOf(schema.string())])
),
dataSourceEnabled: schema.maybe(schema.boolean({ defaultValue: false })),
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this dataSourceEnabled option determined by the saved object service itself, passing as parameter doesn't looks right to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, @zengyan-amazon something here might confuse me. The dataSourceEnabled is originally from DataSourcePluginSetup props, it is from data source service and pass as service field, UI component will pass it as parameter to import api, UI got it from saved_object_management plugin set up (code)[https://github.com/opensearch-project/OpenSearch-Dashboards/blob/fb76ee9beee3a98d9b7c0783df072abe745acd7a/src/plugins/saved_objects_management/public/plugin.ts#L110], import api from core is used in this plugin, but does not have plugin setup, I do not know how to get the dataSourceEnabled from the saved object client from the core, could you please point me some example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Issue to track here: #6557

@BionIT
Copy link
Collaborator

BionIT commented Jun 3, 2024

@yujin-emma backport failed, can you fix?

yujin-emma added a commit to yujin-emma/OpenSearch-Dashboards that referenced this pull request Jun 5, 2024
…object to Local cluster when not enable data source

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
zhongnansu pushed a commit to yujin-emma/OpenSearch-Dashboards that referenced this pull request Jun 6, 2024
…object to Local cluster when not enable data source

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
yujin-emma added a commit to yujin-emma/OpenSearch-Dashboards that referenced this pull request Jun 6, 2024
…object to Local cluster when not enable data source

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
BionIT pushed a commit that referenced this pull request Jun 6, 2024
…a source object to Local cluster when not enable data source (#6913)

* [Backport #6395] Do not support import data source object to Local cluster when not enable data source

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

* Update CHANGELOG.md

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

* fix: integration test failure

Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

---------

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Signed-off-by: SuZhou-Joe <suzhou@amazon.com>
Co-authored-by: SuZhou-Joe <suzhou@amazon.com>
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.

6 participants