[Draft][Rule Registry] Create index template per namespace #107700

marshallmain · 2021-08-04T21:18:09Z

Summary

Summarize your PR. If it involves visual changes include a screenshot or gif.

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
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

…d ILM policies

kibanamachine · 2021-08-05T16:56:32Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 02bb41c
Storybooks Preview
Pipeline Steps (look for red circles / failed steps)
Interpreting CI Failures

Failed CI Steps

Metrics [docs]

✅ unchanged

History

💔 Build #143127 failed bcbd952
💔 Build #143105 failed 4f16ccc

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

banderror · 2021-08-05T19:22:58Z

x-pack/plugins/rule_registry/server/rule_data_client/index.ts

+      // TODO: need a way to update this template if/when we decide to make changes to the
+      // built in index template. Probably do it as part of updateIndexMappingsForAsset?


Yeah, I think we will need it... One simple case that comes to my mind is when we add or remove a component template, we will need to update the index template as well.

Definitely true. It may not be critical for 7.15 though since it's the first release.

banderror · 2021-08-05T19:25:29Z

x-pack/plugins/rule_registry/server/rule_data_client/index.ts

+      // to updateIndexMappingsForAsset. We can make the secondary alias available since
+      // it's known at plugin startup time, but
+      // the namespace values can really only come from the existing templates that we're trying to update
+      // - maybe we want to store the namespace as a _meta field on the index template for easy retrieval


Yeah, we will need to somehow retrieve the namespace from existing index templates, I think it's a good idea to store it in _meta 👍 Thanks for this idea!

banderror · 2021-08-05T19:30:32Z

x-pack/plugins/rule_registry/server/rule_data_client/index.ts

+        // By setting the priority to namespace.length, we ensure that if one namespace is a prefix of another namespace
+        // then newly created indices will use the matching template with the *longest* namespace
+        priority: namespace?.length,


Not sure I got this part. Why do we need to set the priority if namespace cannot contain dashes?

I mean, let's say we have indices like

.alerts-security.alerts-foo-000001
.alerts-security.alerts-foobar-000001

Index template of the first one will have index_patterns: ['.alerts-security.alerts-foo-*'] which will exclude the second index.

Sorry, maybe I'm just completely not seeing the idea.

The namespace can contain dashes. Even if we prevent new namespaces from having dashes, we plan to use existing space IDs as namespaces for backwards compatibility, and space IDs can have dashes in them already.

Oh... Holy moly, yes you're right!

Here's an issue I wrote up on the problem with a little more detail: #107704

Proposal 4 on there is what I implemented here and I think it's definitely preferable to the other 3, but I had that idea after I already wrote the first 3 ideas so I just left them there 😄

…ing implementation (#108115) **Addresses:** #106421, #106428, #102089, #106433 ## Summary This PR focuses on consolidation of indexing implementations in `rule_registry` (#101016). It addresses some of the sub-tasks of the parent ticket. - [x] Encapsulate index bootstrapping logic in a new improved API exposed by `RuleDataService`. - [x] Enforce allowed values for the `datasetSuffix` on the API level. - [x] Migrate plugins using the existing `RuleDataService` API to the improved one. - [x] Make sure index names comply with design architecture. - #102089 - [x] Improve the API of `RuleDataClient`. - [x] Enhance index bootstrapping: support custom ILM policy per index (`{registrationContext}.{datasetSuffix}`). - [x] Enhance index bootstrapping: create index template per namespace and support rollovers properly - based on #107700 - [x] Enhance index bootstrapping: support secondary aliases - based on #107700 - [x] Remove `EventLogService` implementation - #106433 This will be addressed in follow-up PRs: - [ ] Enhance index bootstrapping: implement suggestions for backwards compatibility (naming scheme for alias and backing indices; versioning). - [ ] Enhance index bootstrapping: implement upgrades of existing index templates. - [ ] Make index bootstrapping logic more robust. This _is partially addressed_ in this PR, but more improvements are needed. - [ ] Change the way index prefix works. - [ ] Add support for optional TS schema (static typing). - [ ] Update `README` in `rule_registry`. ### Checklist - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [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

…ing implementation (elastic#108115) **Addresses:** elastic#106421, elastic#106428, elastic#102089, elastic#106433 ## Summary This PR focuses on consolidation of indexing implementations in `rule_registry` (elastic#101016). It addresses some of the sub-tasks of the parent ticket. - [x] Encapsulate index bootstrapping logic in a new improved API exposed by `RuleDataService`. - [x] Enforce allowed values for the `datasetSuffix` on the API level. - [x] Migrate plugins using the existing `RuleDataService` API to the improved one. - [x] Make sure index names comply with design architecture. - elastic#102089 - [x] Improve the API of `RuleDataClient`. - [x] Enhance index bootstrapping: support custom ILM policy per index (`{registrationContext}.{datasetSuffix}`). - [x] Enhance index bootstrapping: create index template per namespace and support rollovers properly - based on elastic#107700 - [x] Enhance index bootstrapping: support secondary aliases - based on elastic#107700 - [x] Remove `EventLogService` implementation - elastic#106433 This will be addressed in follow-up PRs: - [ ] Enhance index bootstrapping: implement suggestions for backwards compatibility (naming scheme for alias and backing indices; versioning). - [ ] Enhance index bootstrapping: implement upgrades of existing index templates. - [ ] Make index bootstrapping logic more robust. This _is partially addressed_ in this PR, but more improvements are needed. - [ ] Change the way index prefix works. - [ ] Add support for optional TS schema (static typing). - [ ] Update `README` in `rule_registry`. ### Checklist - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [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

…ing implementation (#108115) (#108638) **Addresses:** #106421, #106428, #102089, #106433 ## Summary This PR focuses on consolidation of indexing implementations in `rule_registry` (#101016). It addresses some of the sub-tasks of the parent ticket. - [x] Encapsulate index bootstrapping logic in a new improved API exposed by `RuleDataService`. - [x] Enforce allowed values for the `datasetSuffix` on the API level. - [x] Migrate plugins using the existing `RuleDataService` API to the improved one. - [x] Make sure index names comply with design architecture. - #102089 - [x] Improve the API of `RuleDataClient`. - [x] Enhance index bootstrapping: support custom ILM policy per index (`{registrationContext}.{datasetSuffix}`). - [x] Enhance index bootstrapping: create index template per namespace and support rollovers properly - based on #107700 - [x] Enhance index bootstrapping: support secondary aliases - based on #107700 - [x] Remove `EventLogService` implementation - #106433 This will be addressed in follow-up PRs: - [ ] Enhance index bootstrapping: implement suggestions for backwards compatibility (naming scheme for alias and backing indices; versioning). - [ ] Enhance index bootstrapping: implement upgrades of existing index templates. - [ ] Make index bootstrapping logic more robust. This _is partially addressed_ in this PR, but more improvements are needed. - [ ] Change the way index prefix works. - [ ] Add support for optional TS schema (static typing). - [ ] Update `README` in `rule_registry`. ### Checklist - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [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: Georgii Gorbachev <georgii.gorbachev@elastic.co>

marshallmain added 4 commits July 20, 2021 21:40

Add namespacing to index templates in support of secondary aliases an…

ba09fc5

…d ILM policies

Compute namespaced alias in one place to ensure consistency

4f16ccc

Merge branch 'master' into namespace-index-templates

98fe156

Formatting files

bcbd952

marshallmain mentioned this pull request Aug 4, 2021

[RAC][Rule Registry] Namespaces that are prefixes of other namespaces can cause unexpected behaviors #107704

Closed

marshallmain added 2 commits August 4, 2021 17:17

Merge branch 'master' into namespace-index-templates

016958e

Add template priority

02bb41c

banderror reviewed Aug 5, 2021

View reviewed changes

banderror mentioned this pull request Aug 10, 2021

[RAC][Rule Registry] Improve RuleDataService API and index bootstrapping implementation #108115

Merged

17 tasks

marshallmain closed this Aug 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Draft][Rule Registry] Create index template per namespace #107700

[Draft][Rule Registry] Create index template per namespace #107700

marshallmain commented Aug 4, 2021

kibanamachine commented Aug 5, 2021 •

edited

Loading

banderror Aug 5, 2021

marshallmain Aug 5, 2021

banderror Aug 5, 2021

banderror Aug 5, 2021

marshallmain Aug 5, 2021

banderror Aug 5, 2021

marshallmain Aug 5, 2021 •

edited

Loading

		// TODO: need a way to update this template if/when we decide to make changes to the
		// built in index template. Probably do it as part of updateIndexMappingsForAsset?

[Draft][Rule Registry] Create index template per namespace #107700

[Draft][Rule Registry] Create index template per namespace #107700

Conversation

marshallmain commented Aug 4, 2021

Summary

Checklist

Risk Matrix

For maintainers

kibanamachine commented Aug 5, 2021 • edited Loading

💔 Build Failed

Failed CI Steps

Metrics [docs]

History

banderror Aug 5, 2021

Choose a reason for hiding this comment

marshallmain Aug 5, 2021

Choose a reason for hiding this comment

banderror Aug 5, 2021

Choose a reason for hiding this comment

banderror Aug 5, 2021

Choose a reason for hiding this comment

marshallmain Aug 5, 2021

Choose a reason for hiding this comment

banderror Aug 5, 2021

Choose a reason for hiding this comment

marshallmain Aug 5, 2021 • edited Loading

Choose a reason for hiding this comment

kibanamachine commented Aug 5, 2021 •

edited

Loading

marshallmain Aug 5, 2021 •

edited

Loading