Resolve filter index references when importing saved objects #42974

rudolf · 2019-08-08T19:51:07Z

Summary

Fixes #42343 Resolve filter index references when importing saved objects.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
~~Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
~~Documentation was added for features that require explanation or tutorials~~
Unit or functional tests were updated or added to match the most common scenarios
~~This was checked for keyboard-only and screenreader accessibility~~

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

Release notes:
When importing Saved Objects from a JSON file using the Kibana Management UI, Kibana will detect references to missing indexes in these objects and prompt the user to re-map these to any existing index. However, when importing Saved Objects from Kibana < 7.0.0 that contain filters, Kibana did not detect missing index references in these filters. Kibana will now correctly detect missing index references and prompt users to re-map these to existing indexes.

elasticmachine · 2019-08-08T20:54:29Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-08-21T08:37:14Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…olve-filter-imports

elasticmachine · 2019-08-23T21:00:46Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-08-27T17:40:34Z

Pinging @elastic/kibana-platform

mshustov · 2019-08-28T10:30:48Z

...nagement/sections/objects/components/objects_table/components/flyout/__jest__/flyout.test.js

          },
+          _serialize: () => { return { references: [{ id: 'MyIndexPattern*' }, { id: 'filterIndex' }] };},


a bit confusing that _serialize returns an object instead of a string. is it okay?

This mocks the saved_object._serialize() function which returns an object that can be JSON.stringified. https://github.com/rudolf/kibana/blob/fix-42343-resolve-filter-imports/src/legacy/ui/public/saved_objects/saved_object.js#L309

mshustov · 2019-08-28T10:32:23Z

...bana/public/management/sections/objects/components/objects_table/components/flyout/flyout.js

+        return { doc, obj: obj._serialize() };
+      }).forEach(({ doc, obj }) =>
+        obj.references.forEach(ref => {
+          byId[ref.id] = byId[ref.id] != null ? byId[ref.id].concat({ doc, obj }) : [{ doc, obj }];


why not add conflictedIndexPatternsById as a part of resolveSavedObjects api? this logic is not related to the component.

it makes a lot of sense to move it, but when I do a lot of tests break :( because there's so much business logic in _serialize() it's not so easy to stub out.

mshustov · 2019-08-28T10:42:33Z

src/legacy/core_plugins/kibana/public/management/sections/objects/lib/resolve_saved_objects.js

+      }
+
+      resolution = resolutions.find(({ oldId }) => oldId === filter.meta.index);
+      return resolution ? ({ ...filter, ...{ meta: { ...filter.meta, index: resolution.newId } } }) : filter;


also not super clear why management changes saved object internal content and mutate it

Yeah I'm not sure if this was implemented before we had migrations, but we're essentially performing migrations in the frontend which is rather brittle :( We use this code path when importing JSON files, but newer exports are made in NDJSON and if you importing NDJSON we'll do all the migrations server-side with a much cleaner architecture. Unfortunately JSON support will probably have to stay till 8.0.0.

mshustov · 2019-08-28T10:48:20Z

src/legacy/ui/public/saved_objects/saved_object.js

@@ -326,7 +326,7 @@ export function SavedObjectProvider(Promise, Private, confirmModalPromise, index
          references.push({
            name: refName,
            type: 'index-pattern',
-            id: indexId,
+            id: indexId || searchSourceFields.index,


is it a necessary change? anyway we read id later https://github.com/elastic/kibana/pull/42974/files#diff-e8fcd698a977c19072c4c82d0d4eb40bR149
probably we should throw if id is not present?

According to the source code comments when hydrateIndexPattern() is called index can be either a string or an IndexPattern, but after the call it's converted to an IndexPattern. So I think this can happen if a Saved Object is created and then _serialize() is called before hydrateIndexPattern is called. This might happen here https://github.com/elastic/kibana/pull/42974/files#diff-28408d72ee9bcd7cf7c986c55a9212d7R279

But I'll take a deeper look and also refactor this a bit (if it's necessary).

const indexId = typeof(searchSourceFields.index) == 'string' : searchSourceFields.index : searchSourceFields.index.id;

My comment above wasn't 100% accurate...

group by ID https://github.com/elastic/kibana/pull/42974/files#diff-28408d72ee9bcd7cf7c986c55a9212d7L276 used to use obj.searchSource.getOwnField('index') (a string) which would only group by searchSource indexes and not other referenced indexes. To get access to the other referenced indexes I need to call _serialize(). When serialize() is called searchSource.getFields('index') is a string because these index patterns don't exist so they never get resolved to an IndexPattern object in hydrateIndexPattern().

I think this behaviour change makes sense, when a saved object is serialized (which normally happens before saving) we should persist the id of referenced indexes even if these don't exist in Elasticsearch.

mikecote

Took a quick look at the code (excluding tests) and it looks good to me.

…ndexes

elasticmachine · 2019-08-29T14:57:42Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…#42974) * Resolve filter index references when importing saved objects * Tests for filter index resolution * saved_object.js test for serialization of non-existing searchSource indexes

…#44480) * Resolve filter index references when importing saved objects * Tests for filter index resolution * saved_object.js test for serialization of non-existing searchSource indexes

…#44481) * Resolve filter index references when importing saved objects * Tests for filter index resolution * saved_object.js test for serialization of non-existing searchSource indexes

…#44482) * Resolve filter index references when importing saved objects * Tests for filter index resolution * saved_object.js test for serialization of non-existing searchSource indexes

Resolve filter index references when importing saved objects

b813139

Tests for filter index resolution

3994473

rudolf marked this pull request as ready for review August 23, 2019 19:40

rudolf requested a review from a team as a code owner August 23, 2019 19:40

rudolf added the release_note:fix label Aug 23, 2019

Merge branch 'master' of github.com:elastic/kibana into fix-42343-res…

c9828ab

…olve-filter-imports

rudolf added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Aug 27, 2019

mshustov reviewed Aug 28, 2019

View reviewed changes

rudolf requested review from chrisronline and mikecote August 28, 2019 13:26

mikecote reviewed Aug 29, 2019

View reviewed changes

rudolf added 2 commits August 29, 2019 15:17

Merge branch 'master' into fix-42343-resolve-filter-imports

932bcbf

saved_object.js test for serialization of non-existing searchSource i…

39cf885

…ndexes

mshustov approved these changes Aug 29, 2019

View reviewed changes

rudolf added v7.4.0 v7.5.0 labels Aug 30, 2019

rudolf merged commit 901a54e into elastic:master Aug 30, 2019

rudolf deleted the fix-42343-resolve-filter-imports branch August 30, 2019 10:11

rudolf added the v7.3.2 label Aug 30, 2019

rudolf mentioned this pull request Aug 30, 2019

[7.x] Resolve filter index references when importing saved objects (#42974) #44480

Merged

rudolf mentioned this pull request Aug 30, 2019

[7.4] Resolve filter index references when importing saved objects (#42974) #44481

Merged

rudolf mentioned this pull request Aug 30, 2019

[7.3] Resolve filter index references when importing saved objects (#42974) #44482

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resolve filter index references when importing saved objects #42974

Resolve filter index references when importing saved objects #42974

rudolf commented Aug 8, 2019 •

edited

Loading

elasticmachine commented Aug 8, 2019

elasticmachine commented Aug 21, 2019

elasticmachine commented Aug 23, 2019

elasticmachine commented Aug 27, 2019

mshustov Aug 28, 2019

rudolf Aug 28, 2019

mshustov Aug 28, 2019

rudolf Aug 29, 2019

mshustov Aug 28, 2019

rudolf Aug 28, 2019

mshustov Aug 28, 2019 •

edited

Loading

rudolf Aug 28, 2019

rudolf Aug 29, 2019

mikecote left a comment

elasticmachine commented Aug 29, 2019

		},
		_serialize: () => { return { references: [{ id: 'MyIndexPattern*' }, { id: 'filterIndex' }] };},

Resolve filter index references when importing saved objects #42974

Resolve filter index references when importing saved objects #42974

Conversation

rudolf commented Aug 8, 2019 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Aug 8, 2019

💔 Build Failed

elasticmachine commented Aug 21, 2019

💚 Build Succeeded

elasticmachine commented Aug 23, 2019

💚 Build Succeeded

elasticmachine commented Aug 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mshustov Aug 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikecote left a comment

Choose a reason for hiding this comment

elasticmachine commented Aug 29, 2019

💚 Build Succeeded

rudolf commented Aug 8, 2019 •

edited

Loading

mshustov Aug 28, 2019 •

edited

Loading