[Fleet] Handle Saved Object ID changes (#108959) #119527

hop-dev · 2021-11-23T18:29:46Z

Summary

In 8.0.0 the _id structure of space specific saved objects has changed, this means we now have to use resolve to look up saved objects before getting or deleting them, and we have to use import to create them (instead of bulkCreate).

In Fleet's case this is only relevant to the asset saved objects as they are space-aware.

Using import presented a couple of challenges :

import validates SO references exist before allowing an object to be imported, this has meant a couple of changes:
- We now create index-patterns before we install the package assets
- previously we populated the fields property on an index-pattern saved object based on the installed packages data streams, I have removed this code so that we no longer generate the fields (see implementation details below)
import will only import SO's with importableAndExportable set to true, I have therefore had to add this property to the ml-module and security-rule SO types. Adding visibleInManagement: false means that they do not show in the

Implementation Details

Outstanding issues before merge:

AWS package has a reference to metricbeat-* index pattern on one of its dashboards

Removing `fields` from index patterns

Previously, after installing a package, we extracted all fields from all data streams in the package amd added them to the index-pattern saved object. This was so that on an empty system, dashboards would not show errors for missing fields in the index pattern.

However, as of 7.12, index patterns generate their fields dynamically at runtime, ignoring the fields key on the saved object, meaning these dashboard errors are visible. This has allowed me to remove all field generation code and move the index pattern creation to before the package is installed.

Testing

Mostly manual for now, I have created an issue to automate the manual tests after FF #119621

Checklist

Delete any items that are not applicable to this PR.

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

hop-dev · 2021-11-25T14:07:03Z

@elasticmachine merge upstream

jgowdyelastic

ML changes LGTM

elasticmachine · 2021-11-25T15:28:32Z

Pinging @elastic/fleet (Team:Fleet)

joshdover · 2021-11-29T17:30:03Z

import will only import SO's with importableAndExportable set to true, I have therefore had to add this property to the ml-module and security-rule SO types. Adding visibleInManagement: false means that they do not show in the

I wonder if instead we should add an option to import to bypass this setting for our internal usage like this. I worry about any knock-on effects of making these types importable/exportable when they weren't before. I could be wrong, but I think objects that have visibleInManagement: false but are importable/exportable will still be included in exports when using the "Export All" button. They'll also be included if another object type is exported that has a reference to a ml-module or security-rule.

I don't have any reason to believe that making these types exportable or importable is a bad change, but I think it should be done with full consideration rather than as a side-effect of this work. And maybe you've already done that :) Did we discuss with the ML and Security teams the implications of this change?

EDIT: if we do go down the route of adding an option to import to allow us to bypass the types allowed, we should probably make that a separate PR to Core first.

x-pack/plugins/fleet/server/services/epm/packages/_install_package.ts

x-pack/plugins/fleet/server/services/epm/kibana/assets/install.ts

hop-dev · 2021-11-29T21:35:04Z

HI @joshdover thanks for the review, I did raise this (via @jportner ) in #kibana-core here, Joe was concerned about overloading the import method, so setting the flags was an appealing solution. Maybe me you and Joe need to get together sometime and discuss options, I think import isn't really the ideal function for us, but it does work!

hop-dev · 2021-11-30T18:11:04Z

@joshdover I've done some rough performance testing locally, generally the actual installing of the assets is about the same. However because we no longer install the index patterns and generate the fields as a separate stage, we are seeing some really good performance improvements on the total install time.

Heres my results:

https://gist.github.com/hop-dev/a76c4c6c718e0b014835ee09ba7606df

hop-dev · 2021-12-03T14:44:04Z

@joshdover one other point, it seems visibleInManagement does exclude a saved object from the "export all" option in the UI which is good

joshdover

Looks pretty good, but I did find one issue in manual testing:

The assets tab breaks (empty/broken links) after installing a package in a space and then reinstalling the integration in the default space. Steps to reproduce:

Create a test space and switch to it
Install Apache integration
Switch to default space
Uninstall Apache integration
Install Apache integration
Go to assets tab for Apache integration (still in default space), see broken links

I ran the following test as well to verify the full behavior and everything else worked:

Start Kibana and ES 7.16.0
Create test space
Install nginx 1.1.0 via API in test space
Stop Kibana and ES
Start ES 8.1.0 using same path.data as 7.16.0
Start this branch running from source
Upgrade the nginx package to 1.2.0 in the test space

x-pack/plugins/fleet/server/services/epm/kibana/assets/install.ts

x-pack/plugins/fleet/server/services/epm/packages/_install_package.ts

spong · 2021-12-03T16:23:33Z

Not sure if this PR has had any interaction here (been a bit since I've tested this workflow), but Assets tab is empty for the Prebuilt Security Detection Rules integration:

Is this intended? Only way to see the number of assets installed seems to be clicking through to Settings and Uninstall and the counts are displayed in the modal:

spong

Checked out, tested locally, and everything appears to be working as expected with the Prebuilt Security Detection Rules integration. Was able to verify installation of the integration, and also confirmed that when installing rules from within the Security UI the SO's from the integration were used. Also verified assets weren't showing up in the SOM either, so all good there too!

Security changes LGTM! 👍 Thanks! 🙂

We were checking for an error before the import was complete.

joshdover

Changes LGTM. As mentioned offline, we're still wanting to confirm the root cause of the bug identified in #119527 (review). We currently suspect it will be fixed by the fixing the uninstall step across Spaces which will be fixed as part of #74353

kibana-ci · 2021-12-06T13:12:05Z

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`fleet`	643.2KB	643.4KB	+182.0B

History

💔 Build #11250 failed c9f3ddf
💚 Build #11017 succeeded 6db67bd
💚 Build #10727 succeeded ce573d2
💚 Build #10300 succeeded 8a5a664
💛 Build #10017 was flaky 8e2e1ee
💚 Build #9926 succeeded 208f9e5

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @hop-dev

* Do not add fields to index patterns * remove redundant tests * install index patterns before package install * update remove comment * use import to create package assets Here I have also moved to importing all assets at once. This is essential when importing to ensure that all saved objects references are imported at once. There is also an efficiencey improvement. * Import index patterns * use resolve when deleting index patterns * fix: asset type validation * add option to override supported import types * make ml-module importable * Revert "add option to override supported import types" This reverts commit 1f48e6e. * remove hidden: false from ml-module * use resolve when deleting assets * make security-rule SO type importable * use bulkResolve to get package assets * fix tests * fix 'multiple' tests * remove unused function * create index patterns at the same time as other assets * remove unused test * Fix integration tests We were checking for an error before the import was complete. * tidy for PR * add missing test assets * do not attempt to delete missing assets * resolve any reference errors that occur on import * await installKibanaAssets immediately * show assets not found when assets installed in a different space * fix delete asset check on force upgrade * add comment about reference errors * remove a couple of appContextService dependencies

kibanamachine · 2021-12-06T13:14:58Z

💚 Backport successful

Status	Branch	Result
✅	8.0

This backport PR will be merged automatically after passing CI.

* Do not add fields to index patterns * remove redundant tests * install index patterns before package install * update remove comment * use import to create package assets Here I have also moved to importing all assets at once. This is essential when importing to ensure that all saved objects references are imported at once. There is also an efficiencey improvement. * Import index patterns * use resolve when deleting index patterns * fix: asset type validation * add option to override supported import types * make ml-module importable * Revert "add option to override supported import types" This reverts commit 1f48e6e. * remove hidden: false from ml-module * use resolve when deleting assets * make security-rule SO type importable * use bulkResolve to get package assets * fix tests * fix 'multiple' tests * remove unused function * create index patterns at the same time as other assets * remove unused test * Fix integration tests We were checking for an error before the import was complete. * tidy for PR * add missing test assets * do not attempt to delete missing assets * resolve any reference errors that occur on import * await installKibanaAssets immediately * show assets not found when assets installed in a different space * fix delete asset check on force upgrade * add comment about reference errors * remove a couple of appContextService dependencies Co-authored-by: Mark Hopkin <[email protected]>

* Do not add fields to index patterns * remove redundant tests * install index patterns before package install * update remove comment * use import to create package assets Here I have also moved to importing all assets at once. This is essential when importing to ensure that all saved objects references are imported at once. There is also an efficiencey improvement. * Import index patterns * use resolve when deleting index patterns * fix: asset type validation * add option to override supported import types * make ml-module importable * Revert "add option to override supported import types" This reverts commit 1f48e6e. * remove hidden: false from ml-module * use resolve when deleting assets * make security-rule SO type importable * use bulkResolve to get package assets * fix tests * fix 'multiple' tests * remove unused function * create index patterns at the same time as other assets * remove unused test * Fix integration tests We were checking for an error before the import was complete. * tidy for PR * add missing test assets * do not attempt to delete missing assets * resolve any reference errors that occur on import * await installKibanaAssets immediately * show assets not found when assets installed in a different space * fix delete asset check on force upgrade * add comment about reference errors * remove a couple of appContextService dependencies

hop-dev mentioned this pull request Nov 24, 2021

[Fleet] [Testing] Add automated tests for space ID changes. #119621

Closed

12 tasks

hop-dev force-pushed the 108959-install-index-patterns-before-assets branch 2 times, most recently from 37e3285 to b742df6 Compare November 25, 2021 12:09

hop-dev marked this pull request as ready for review November 25, 2021 14:28

hop-dev requested review from a team as code owners November 25, 2021 14:28

jgowdyelastic approved these changes Nov 25, 2021

View reviewed changes

botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Nov 25, 2021

hop-dev changed the title ~~[Fleet] [Draft] Handle Saved Object ID changes (#108959)~~ [Fleet] Handle Saved Object ID changes (#108959) Nov 29, 2021

hop-dev force-pushed the 108959-install-index-patterns-before-assets branch from 218f6da to edf56dd Compare November 29, 2021 10:37

hop-dev added auto-backport Deprecated - use backport:version if exact versions are needed release_note:fix v8.0.0 v8.1.0 labels Nov 29, 2021

joshdover self-requested a review November 29, 2021 12:10

hop-dev self-assigned this Nov 29, 2021

joshdover reviewed Nov 29, 2021

View reviewed changes

x-pack/plugins/fleet/server/services/epm/packages/_install_package.ts Outdated Show resolved Hide resolved

x-pack/plugins/fleet/server/services/epm/kibana/assets/install.ts Show resolved Hide resolved

hop-dev force-pushed the 108959-install-index-patterns-before-assets branch 2 times, most recently from 8a5a664 to ce573d2 Compare December 2, 2021 17:41

joshdover reviewed Dec 3, 2021

View reviewed changes

x-pack/plugins/fleet/server/services/epm/kibana/assets/install.ts Outdated Show resolved Hide resolved

x-pack/plugins/fleet/server/services/epm/kibana/assets/install.ts Show resolved Hide resolved

x-pack/plugins/fleet/server/services/epm/packages/_install_package.ts Show resolved Hide resolved

hop-dev force-pushed the 108959-install-index-patterns-before-assets branch from 8382974 to 6db67bd Compare December 3, 2021 16:32

spong approved these changes Dec 3, 2021

View reviewed changes

hop-dev added 13 commits December 6, 2021 10:51

fix 'multiple' tests

a88f9f4

remove unused function

688d1ae

create index patterns at the same time as other assets

5c36e70

remove unused test

c7724af

Fix integration tests

a44ca95

We were checking for an error before the import was complete.

tidy for PR

e2991f1

add missing test assets

1ed4148

do not attempt to delete missing assets

7ed9f6b

resolve any reference errors that occur on import

529828c

await installKibanaAssets immediately

63d66cc

show assets not found when assets installed in a different space

37d9e21

fix delete asset check on force upgrade

2e29b05

add comment about reference errors

e772263

hop-dev force-pushed the 108959-install-index-patterns-before-assets branch from 1087909 to c9f3ddf Compare December 6, 2021 10:51

remove a couple of appContextService dependencies

5945b38

hop-dev force-pushed the 108959-install-index-patterns-before-assets branch from c9f3ddf to 5945b38 Compare December 6, 2021 11:34

joshdover approved these changes Dec 6, 2021

View reviewed changes

hop-dev enabled auto-merge (squash) December 6, 2021 12:05

hop-dev merged commit 7e3c3d1 into elastic:main Dec 6, 2021

kibanamachine mentioned this pull request Dec 6, 2021

[8.0] [Fleet] Handle Saved Object ID changes (#108959) (#119527) #120481

Merged

hop-dev deleted the 108959-install-index-patterns-before-assets branch December 6, 2021 13:53

spong mentioned this pull request Dec 6, 2021

[Security Solution] Handle rule deprecations within Prebuilt Rule upgrade workflow #118942

Open

This was referenced Feb 7, 2022

[Ingest Manager] Optimize creation of index patterns #67760

Closed

[EPM] log duplicate fields in kibana when creating index patterns #60952

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fleet] Handle Saved Object ID changes (#108959) #119527

[Fleet] Handle Saved Object ID changes (#108959) #119527

hop-dev commented Nov 23, 2021 •

edited

Loading

hop-dev commented Nov 25, 2021

jgowdyelastic left a comment

elasticmachine commented Nov 25, 2021

joshdover commented Nov 29, 2021 •

edited

Loading

hop-dev commented Nov 29, 2021

hop-dev commented Nov 30, 2021 •

edited

Loading

hop-dev commented Dec 3, 2021

joshdover left a comment

spong commented Dec 3, 2021

spong left a comment

joshdover left a comment

kibana-ci commented Dec 6, 2021

kibanamachine commented Dec 6, 2021

[Fleet] Handle Saved Object ID changes (#108959) #119527

[Fleet] Handle Saved Object ID changes (#108959) #119527

Conversation

hop-dev commented Nov 23, 2021 • edited Loading

Summary

Implementation Details

Removing fields from index patterns

Testing

Checklist

hop-dev commented Nov 25, 2021

jgowdyelastic left a comment

Choose a reason for hiding this comment

elasticmachine commented Nov 25, 2021

joshdover commented Nov 29, 2021 • edited Loading

hop-dev commented Nov 29, 2021

hop-dev commented Nov 30, 2021 • edited Loading

hop-dev commented Dec 3, 2021

joshdover left a comment

Choose a reason for hiding this comment

spong commented Dec 3, 2021

spong left a comment

Choose a reason for hiding this comment

joshdover left a comment

Choose a reason for hiding this comment

kibana-ci commented Dec 6, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

History

kibanamachine commented Dec 6, 2021

💚 Backport successful

hop-dev commented Nov 23, 2021 •

edited

Loading

Removing `fields` from index patterns

joshdover commented Nov 29, 2021 •

edited

Loading

hop-dev commented Nov 30, 2021 •

edited

Loading