Skip to content
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] Support user overrides in composable templates #101769

Merged
merged 26 commits into from
Jun 23, 2021

Conversation

jfsiii
Copy link
Contributor

@jfsiii jfsiii commented Jun 9, 2021

Summary

Closes #90454
Closes #72959

  • Rename the component templates which are installed for some packages from ${templateName}-mappings and ${templateName}-settings to ${templateName}@mappings and ${templateName}@settings
  • When any package is installed, add a component template named ${templateName}@custom
  • Any of above templates also include a _meta property with { package: { name: packageName } }
  • On package installation, add any installed component templates to the installed_es property of the epm-packages saved object
  • On package removal, remove any installed component templates from the installed_es property of the epm-packages saved object
Kibana logs showing component templates added for package
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.file@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.registry@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [.logs-endpoint.diagnostic.collection@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.library@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.security@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.network@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.alerts@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.metrics@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.process@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.policy@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.metadata@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.registry@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [.logs-endpoint.diagnostic.collection@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.security@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.file@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.library@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.network@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.alerts@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.metrics@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.policy@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.process@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.metadata@custom]
screenshot - component templates are editable in the Stack Management UI Screen Shot 2021-06-17 at 4 06 24 PM

Checklist

Delete any items that are not applicable to this PR.

@jfsiii
Copy link
Contributor Author

jfsiii commented Jun 15, 2021

@elasticmachine merge upstream

@jen-huang jen-huang self-requested a review June 16, 2021 20:23
@spalger
Copy link
Contributor

spalger commented Jun 16, 2021

jenkins, test this

(restarting due to jenkins upgrade)

@jfsiii jfsiii self-assigned this Jun 16, 2021
@jfsiii
Copy link
Contributor Author

jfsiii commented Jun 16, 2021

@elasticmachine merge upstream

@ruflin
Copy link
Member

ruflin commented Jun 18, 2021

@jpountz Pinging you on this PR as we introduce here a new naming convention for component templates for the data stream naming scheme with is {type}-{dataset}@{component-name}. Any concerns around this?

@ruflin
Copy link
Member

ruflin commented Jun 18, 2021

I was playing around with this. I installed apache but there was only the @custom component template, everything else seems to be in the main template. Not really an issue but is this expected?

I remove the apache package again and the custom template also disappears. I'm torn if this is expected or might be a suprise to the user? Should we make it an option to the user if these should be removed to? @jen-huang @jfsiii What do you see as the expected behavior?

@jpountz
Copy link

jpountz commented Jun 18, 2021

a new naming convention for component templates for the data stream naming scheme with is {type}-{dataset}@{component-name}. Any concerns around this?

No, but I'd like @dakrone to check too.

@jfsiii
Copy link
Contributor Author

jfsiii commented Jun 18, 2021

@ruflin

It would be nice to test somehow if the package upgrade works properly from "old asset name" to "new asset name". Not sure what the best way to test this is.

Thanks for mentioning that. The current code merely creates the new component templates. It doesn't move or rename the -settings or -mappings to new @settings or @mappings. I think adding a test for that scenario shouldn't be difficult.

I installed apache but there was only the @Custom component template, everything else seems to be in the main template. Not really an issue but is this expected?

It is the current behavior and expected but I see why it may not be "correct" or "desirable". cc: @simitt & @mostlyjason The current code in main only creates the mappings or settings component templates if the package includes them (e.g. endpoint setting mappings of dynamic: false here). We create the @custom component template regardless of package settings based on this feedback

I remove the apache package again and the custom template also disappears. I'm torn if this is expected or might be a surprise to the user? Should we make it an option to the user if these should be removed too?

Good question, cc: @mostlyjason & @simitt again. It's not a technical challenge to leave it installed, but giving the user a choice would seem to add some UI and we're ~1 week from FF. We could have it default one way (possibly with a param in the API) and add the UI to override in a later iteration.

@simitt
Copy link
Contributor

simitt commented Jun 21, 2021

I installed apache but there was only the @Custom component template, everything else seems to be in the main template. Not really an issue but is this expected?

It is the current behavior and expected but I see why it may not be "correct" or "desirable". cc: @simitt & @mostlyjason The current code in main only creates the mappings or settings component templates if the package includes them (e.g. endpoint setting mappings of dynamic: false here). We create the @Custom component template regardless of package settings based on this feedback

Apologies for the last minute change proposal - but could we install the custom component template only if the package sets a flag? (for example: custom: true). This way the change would not impact all existing packages and package devs could decide if settings/mappings should be configurable or not.

I remove the apache package again and the custom template also disappears. I'm torn if this is expected or might be a surprise to the user? Should we make it an option to the user if these should be removed too?

Good question, cc: @mostlyjason & @simitt again. It's not a technical challenge to leave it installed, but giving the user a choice would seem to add some UI and we're ~1 week from FF. We could have it default one way (possibly with a param in the API) and add the UI to override in a later iteration.

Not certain what the best behavior is for deleting. What happens if the package is reinstalled - would it get overwritten on install?

@ruflin
Copy link
Member

ruflin commented Jun 21, 2021

@jfsiii For the @mappings and @settings templates I think it is fine to keep it the way it is right now. As "we" control these templates we can change the naming at any time and split them out. Nit: I stumbled over this because of the PR description: "When a package installed, 3 component templates are created."

@jfsiii
Copy link
Contributor Author

jfsiii commented Jun 21, 2021

Not certain what the best behavior is for deleting. What happens if the package is reinstalled - would it get overwritten on install?

@simitt No. We only install the @custom template if one isn't already installed

if (isUserSettingsTemplate(name)) {
// look for existing user_settings template
const result = await esClient.cluster.getComponentTemplate({ name }, { ignore: [404] });
const hasUserSettingsTemplate = result.body.component_templates?.length === 1;
if (!hasUserSettingsTemplate) {
// only add if one isn't already present
const { clusterPromise } = putComponentTemplate(esClient, { body, name, create: true });
return clusterPromise;
}
} else {

I think I can add/tweak a test to confirm that reinstalling doesn't override an existing template

@jfsiii
Copy link
Contributor Author

jfsiii commented Jun 21, 2021

@elasticmachine merge upstream

@jfsiii
Copy link
Contributor Author

jfsiii commented Jun 21, 2021

Apologies for the last minute change proposal - but could we install the custom component template only if the package sets a flag? (for example: custom: true). This way the change would not impact all existing packages and package devs could decide if settings/mappings should be configurable or not.

@simitt I'll leave the decision to @ruflin @mostlyjason @jen-huang , but adding the property in a package means changes & approval (however minor) from the registry & package-spec.

It's not a significant amount of engineering effort (it's essentially the approach I took initially) but it does require coordination with other parts of the stack.

@jfsiii
Copy link
Contributor Author

jfsiii commented Jun 21, 2021

Nit: I stumbled over this because of the PR description: "When a package installed, 3 component templates are created."

@ruflin Thanks! I updated the description and hope it's more accurate & helpful now.

@dakrone
Copy link
Member

dakrone commented Jun 21, 2021

a new naming convention for component templates for the data stream naming scheme with is {type}-{dataset}@{component-name}. Any concerns around this?

No, but I'd like @dakrone to check too.

That naming scheme seems fine to me also.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @jfsiii

@ruflin
Copy link
Member

ruflin commented Jun 22, 2021

To keep this moving, should we get this PR in as and follow up on the part around custom: true and the potential optional removal? Like this we have a working version for the next minor and can see if the other changes make it in time? I'm aware the follow ups could be a breaking change so it is not ideal at the same time I do not want to block progress on this change.

@simitt
Copy link
Contributor

simitt commented Jun 22, 2021

While I have been pushing for this, the APM integration itself is only beta in 7.14 - IMO we should not merge changes in that might require breaking changes in following versions.

@jfsiii
Copy link
Contributor Author

jfsiii commented Jun 22, 2021

@nchaulet mentioned that after #102225 lands

we will have to ensure the global fleet component template is not removed when uninstalling a package.

So whichever lands first the other PR will need to update code & tests to make sure the global fleet component template is not removed

Copy link
Contributor

@jen-huang jen-huang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work, thanks for all the tests as well. I tested installing, upgrading, and uninstalling packages locally and things worked as expected. In the case of upgrade, my user settings were kept around in the custom template. 🎉

For the items discussed, I will create a follow up issue (#103034) to discuss them further, but I would like to get this PR in as it doesn't sound like any follow up enhancements would be breaking to what this PR introduces:

  • Whether or not packages should specify if a custom template is installed or not: from discussion with @ruflin and @mostlyjason, it would be rare for a package to not want this (Endpoint seems to be the only case), so we can add an opt-out option later on.
  • Whether or not the custom template should be deleted when a package is uninstalled: let's wait for user feedback on whether deleting it automatically is an annoying UX. In the future we could potentially expose a checkbox/flag to delete custom settings on uninstall.

@jfsiii jfsiii merged commit 868ae59 into elastic:master Jun 23, 2021
@jen-huang jen-huang added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 23, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 23, 2021
## Summary
Closes elastic#90454
Closes elastic#72959

 * Rename the component templates which are [installed for some packages](https://github.com/elastic/kibana/blob/master/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/install.ts#L197-L213) from `${templateName}-mappings` and `${templateName}-settings` to `${templateName}@mappings` and `${templateName}@settings`
 * When any package is installed, add a component template named `${templateName}@custom`
 * Any of above templates also include a `_meta` property with `{ package: { name: packageName } }`
 * On package installation, add any installed component templates to the `installed_es` property of the `epm-packages` saved object
 * On package removal, remove any installed component templates from the `installed_es` property of the `epm-packages` saved object

<details><summary>Kibana logs showing component templates added for package</summary>

```
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.file@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.registry@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [.logs-endpoint.diagnostic.collection@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.library@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.security@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.network@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.alerts@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.metrics@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.process@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.policy@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.metadata@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.registry@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [.logs-endpoint.diagnostic.collection@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.security@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.file@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.library@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.network@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.alerts@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.metrics@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.policy@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.process@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.metadata@custom]
```

</details>

<details><summary>screenshot - component templates are editable in the Stack Management UI</summary>
<img width="1342" alt="Screen Shot 2021-06-17 at 4 06 24 PM" src="https://user-images.githubusercontent.com/57655/122465421-1502bb80-cf86-11eb-94f4-9880cb3ea844.png">
</details>


### Checklist

Delete any items that are not applicable to this PR.

- [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials
- [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
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 23, 2021
…3126)

## Summary
Closes #90454
Closes #72959

 * Rename the component templates which are [installed for some packages](https://github.com/elastic/kibana/blob/master/x-pack/plugins/fleet/server/services/epm/elasticsearch/template/install.ts#L197-L213) from `${templateName}-mappings` and `${templateName}-settings` to `${templateName}@mappings` and `${templateName}@settings`
 * When any package is installed, add a component template named `${templateName}@custom`
 * Any of above templates also include a `_meta` property with `{ package: { name: packageName } }`
 * On package installation, add any installed component templates to the `installed_es` property of the `epm-packages` saved object
 * On package removal, remove any installed component templates from the `installed_es` property of the `epm-packages` saved object

<details><summary>Kibana logs showing component templates added for package</summary>

```
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.file@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.registry@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [.logs-endpoint.diagnostic.collection@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.library@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.security@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.network@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.alerts@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.metrics@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.process@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.policy@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.metadata@mappings]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.registry@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [.logs-endpoint.diagnostic.collection@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.security@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.file@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.library@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.network@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.alerts@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.metrics@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.policy@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [logs-endpoint.events.process@custom]
   │ info [o.e.c.m.MetadataIndexTemplateService] [JFSIII.local] adding component template [metrics-endpoint.metadata@custom]
```

</details>

<details><summary>screenshot - component templates are editable in the Stack Management UI</summary>
<img width="1342" alt="Screen Shot 2021-06-17 at 4 06 24 PM" src="https://user-images.githubusercontent.com/57655/122465421-1502bb80-cf86-11eb-94f4-9880cb3ea844.png">
</details>


### Checklist

Delete any items that are not applicable to this PR.

- [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials
- [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: John Schulz <john.schulz@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:enhancement Team:Fleet Team label for Observability Data Collection Fleet team v7.14.0 v8.0.0
Projects
None yet
9 participants