-
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
[Fleet] Use default component templates from Elasticsearch #163731
[Fleet] Use default component templates from Elasticsearch #163731
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
elastic/elasticsearch#99975 has just been merged so this should be unblocked now. @jpountz last chance for objections. |
x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/fleet/server/services/epm/elasticsearch/template/default_settings.ts
Outdated
Show resolved
Hide resolved
Thanks for the ping, @felixbarny. I'll get this updated soon |
…-ref HEAD~1..HEAD --fix'
5445274
to
b3b0064
Compare
Pinging @elastic/fleet (Team:Fleet) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one minor comment
x-pack/plugins/fleet/server/services/epm/elasticsearch/template/install.ts
Outdated
Show resolved
Hide resolved
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
@@ -242,7 +251,7 @@ const updateIndexTemplate = async ( | |||
aliases: template?.aliases, | |||
}, | |||
_meta, | |||
composed_of: composedOf, | |||
composed_of: composedOf.filter((ct) => ct !== STACK_COMPONENT_TEMPLATE_LOGS_SETTINGS), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elastic/kibana-cloud-security-posture Not basing the CSP transform index on the logs@settings
template was necessary due to the logs@settings
template on serverless specifying a default lifecycle. I could not find a way to override this from the index template, so instead I decided to filter this out for now and just manually specify the settings that were used before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think we should remove this special initialization of the transform and index template and use the dedicated feature provided by Fleet for setting up transforms which would not have this issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshdover, you right, we have task for that:
codec?: string; | ||
mapping?: { | ||
ignore_malformed: boolean; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshdover, what those 2 new attributes are using for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These attributes were already being supplied previously by Fleet in the index template you were copying. With this PR, they're not longer added directly to the index template and instead are sourced from the logs@settings
component template.
Since I'm filtering that component template out as mentioned above, I added these settings back explicitly to ensure that this index gets the same settings as it did before this change. I'll leave it to your team to decide if you need these settings or not, but they generally should help reduce storage costs and not drop any data.
@@ -242,7 +251,7 @@ const updateIndexTemplate = async ( | |||
aliases: template?.aliases, | |||
}, | |||
_meta, | |||
composed_of: composedOf, | |||
composed_of: composedOf.filter((ct) => ct !== STACK_COMPONENT_TEMPLATE_LOGS_SETTINGS), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshdover, you right, we have task for that:
…63731) ## Summary Fixes elastic#163141 Fixes elastic#160288 Blocked by: - elastic/elasticsearch#98535 This switches where integrations installed by EPM get their default index settings from to use the [source-of-truth component templates supplied by Elasticsearch](https://github.com/elastic/elasticsearch/tree/main/x-pack/plugin/core/template-resources/src/main/resources). This will help ensure that data streams configured by EPM always get the same defaults as data streams the user creates using the default `logs-*-*` and `metrics-*-*` templates. For now, no default mappings are sourced from Elasticsearch. As part of this change the template format version was incremented to force EPM to reinstall all templates and rollover data streams on the Stack upgrade to the version including this change. ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <[email protected]>
Summary
Fixes #163141
Fixes #160288
Blocked by:
This switches where integrations installed by EPM get their default index settings from to use the source-of-truth component templates supplied by Elasticsearch. This will help ensure that data streams configured by EPM always get the same defaults as data streams the user creates using the default
logs-*-*
andmetrics-*-*
templates. For now, no default mappings are sourced from Elasticsearch.As part of this change the template format version was incremented to force EPM to reinstall all templates and rollover data streams on the Stack upgrade to the version including this change.
Checklist
Delete any items that are not applicable to this PR.