-
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
[EPM][Security Solution] Implementing dataset component templates #70517
[EPM][Security Solution] Implementing dataset component templates #70517
Conversation
Pinging @elastic/endpoint-data-visibility-team (Team:Endpoint Data Visibility) |
Pinging @elastic/endpoint-app-team (Feature:Endpoint) |
Pinging @elastic/ingest-management (Team:Ingest Management) |
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.
Thanks for the link to the Registry type and updating Ingest's TS types. LGTM
componentPromises.push(settings.clusterPromise); | ||
} | ||
|
||
// TODO: Check return values for errors |
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.
Add/link a ticket?
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.
Good call. I created a new ticket here: #70586
I'll update the second link once this PR is merged.
}): IndexTemplate { | ||
const template = getBaseTemplate(type, templateName, mappings, packageName); | ||
const template = getBaseTemplate(type, templateName, mappings, packageName, composedOfTemplates); |
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.
5 params is hefty, but seemingly not introduced here and can be reduced later
I wonder how #64760 plays into this? I assume these are additional components? Mentioning here so we can plan on the naming side for it. |
Yeah I guess I kind of stole the names that we'd probably use 😆 . We can always refactor it so that the Elasticsearch overrides from the manifest.yml would have names like |
@elasticmachine merge upstream |
...registryElasticsearch['index_template.mappings'], | ||
// temporary change until https://github.com/elastic/elasticsearch/issues/58956 is resolved | ||
properties: { | ||
'@timestamp': { |
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 think I'll be able to remove this once this: elastic/elasticsearch#58642 is merged.
++ on refactoring and making sure the "override" are in its own place and cannot conflict. Perhaps we should use |
…t-mappings-settings
…gest-support-mappings-settings
x-pack/test/ingest_manager_api_integration/apis/fixtures/package_registry_config.yml
Outdated
Show resolved
Hide resolved
@@ -29,7 +29,7 @@ export default function ({ getService }: FtrProviderContext) { | |||
return response.body; | |||
}; | |||
const listResponse = await fetchPackageList(); | |||
expect(listResponse.response.length).to.be(11); | |||
expect(listResponse.response.length).to.be(12); |
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.
Incrementing because I add the overrides
package.
💚 Build SucceededBuild metrics
History
To update your PR or re-run it, just comment with: |
…astic#70517) * Implementing dataset component templates * Fixing test * Temporary fix to include timestamp with any component template created * Update package registry docker image for CI. * Adapt to new registry filesystem layout. * Adjust tests to changed registry behavior. * Adding a test for mappings and settings overrides * Wrap all the tests in the docker check Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Sonja Krause-Harder <[email protected]>
…0517) (#70862) * Implementing dataset component templates * Fixing test * Temporary fix to include timestamp with any component template created * Update package registry docker image for CI. * Adapt to new registry filesystem layout. * Adjust tests to changed registry behavior. * Adding a test for mappings and settings overrides * Wrap all the tests in the docker check Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Sonja Krause-Harder <[email protected]> Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Sonja Krause-Harder <[email protected]>
This PR adds support for installing two component templates for each dataset that specifies the
elasticsearch
field.A settings and mappings component template will be created depending on whether
mappings
and/orsettings
is included in the dataset of the package in the registry.The fields being added to the
Dataset
interface correspond to here: https://github.com/elastic/package-registry/blob/master/util/dataset.go#L42 and this PR: https://github.com/elastic/package-registry/pull/552/filesEndpoint package installed component templates
Endpoint package mapping now including dynamic: false