-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Disable fields added from unknown saved object types #70951
Merged
Merged
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
rudolf
added
Team:Core
Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Feature:Saved Objects
v8.0.0
v7.9.0
labels
Jul 7, 2020
Pinging @elastic/kibana-platform (Team:Platform) |
rudolf
force-pushed
the
so-mappings-cleanup
branch
from
July 7, 2020 14:33
4fd945c
to
1d6cfc9
Compare
pgayvallet
reviewed
Jul 8, 2020
src/core/server/saved_objects/migrations/core/migration_context.ts
Outdated
Show resolved
Hide resolved
💔 Build Failed
Failed CI StepsBuild metrics
History
To update your PR or re-run it, just comment with: |
pgayvallet
approved these changes
Jul 9, 2020
Type check failure comes from master (and has since been fixed) so ignoring. |
rudolf
added a commit
to rudolf/kibana
that referenced
this pull request
Jul 9, 2020
* Allow disabled: false SO field mappings * Disable fields for unknown SO types * Update everyone else's docs ;) * Address review comments * Add unit tests for disableUnknownTypeMappingFields()
rudolf
added a commit
that referenced
this pull request
Jul 9, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Feature:Saved Objects
release_note:skip
Skip the PR/issue when compiling release notes
Team:Core
Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
v7.9.0
v8.0.0
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.
Summary
Fixes #67086 so that we don't need workarounds to remove old type mappings like https://github.com/aaronjcaldwell/kibana/blob/8a04349b3c5b5c6cb852cf4cef83b4bb9884e8d3/x-pack/plugins/maps/server/saved_objects/maps_telemetry.ts#L8-L23
When upgrading from 7.7.1 -> 7.8.0 -> master this PR removes 123 fields, but 122 of these are because
security_solution
is disabled by default in master but was enabled in 7.8 (as the siem plugin).Limitations
Setting
dynamic: false
on the top-level index mappings would remove a lot of validation provided by ES so until Saved Objects allow plugins to define their own validation that's not really an option. Since we always retain the documents belonging to unknown saved objects types, we need to keep the minimal mappings to be able to store these. This means we can only disable the fields underneath an unknown type's mappings withdynamic: false
.Although plugins could use a "core field datatype" like
text
for their mappings from Legacy this is unlikely and most plugins used anobject
data type.I don't think this has ever been done, but should we for instance deprecate the
namespace
field (type: 'keyword'
) in favour ofnamespaces
then this PR won't clean up that field.It's also important to note that although we clean up all the fields of a unknown type, the top-level field for that type will still count towards the field count limit.
As an example, the following mapping:
Would add fields
a
,b
andc
that count towards the field count limit (even though onlyc
is added to the index). We can disable everything belowa
but withdynamic: false
but we would still be left with one fielda
.So this would remove one of the two fields left over by
server.uuid
from #66963 and potentially leave onemaps-telemetry
field over from #69995 when users upgrade from older versions.We will eventually completely solve this problem in #66056 by refusing to upgrade with unknown data so then we don't need any mappings for unknown types hanging around.
Checklist
Delete any items that are not applicable to this PR.
For maintainers