-
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
[Draft][Rule Registry] Create index template per namespace #107700
Changes from all commits
ba09fc5
4f16ccc
98fe156
bcbd952
016958e
02bb41c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,8 +5,10 @@ | |
* 2.0. | ||
*/ | ||
|
||
import { IndicesPutIndexTemplateRequest } from '@elastic/elasticsearch/api/types'; | ||
import { ResponseError } from '@elastic/elasticsearch/lib/errors'; | ||
import { IndexPatternsFetcher } from '../../../../../src/plugins/data/server'; | ||
import { DEFAULT_ILM_POLICY_ID } from '../../common/assets'; | ||
import { RuleDataWriteDisabledError } from '../rule_data_plugin_service/errors'; | ||
import { | ||
IRuleDataClient, | ||
|
@@ -111,24 +113,86 @@ export class RuleDataClient implements IRuleDataClient { | |
}; | ||
} | ||
|
||
createNamespacedIndexTemplate({ | ||
primaryNamespacedAlias, | ||
secondaryNamespacedAlias, | ||
namespace, | ||
}: { | ||
primaryNamespacedAlias: string; | ||
secondaryNamespacedAlias?: string; | ||
namespace?: string; | ||
}): IndicesPutIndexTemplateRequest { | ||
return { | ||
name: primaryNamespacedAlias, | ||
body: { | ||
index_patterns: [`${primaryNamespacedAlias}-*`], | ||
composed_of: [...this.options.componentTemplateNames], | ||
template: { | ||
aliases: | ||
secondaryNamespacedAlias != null | ||
? { | ||
[secondaryNamespacedAlias]: { | ||
is_write_index: false, | ||
}, | ||
} | ||
: undefined, | ||
settings: { | ||
'index.lifecycle': { | ||
name: DEFAULT_ILM_POLICY_ID, | ||
// TODO: fix the types in the ES package, they don't include rollover_alias??? | ||
// @ts-expect-error | ||
rollover_alias: primaryNamespacedAlias, | ||
}, | ||
}, | ||
}, | ||
_meta: { | ||
namespace, | ||
}, | ||
// 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, | ||
}, | ||
}; | ||
} | ||
|
||
async createWriteTargetIfNeeded({ namespace }: { namespace?: string }) { | ||
const alias = getNamespacedAlias({ alias: this.options.alias, namespace }); | ||
const primaryNamespacedAlias = getNamespacedAlias({ alias: this.options.alias, namespace }); | ||
|
||
const clusterClient = await this.getClusterClient(); | ||
|
||
const { body: aliasExists } = await clusterClient.indices.existsAlias({ | ||
name: alias, | ||
name: primaryNamespacedAlias, | ||
}); | ||
|
||
const concreteIndexName = `${alias}-000001`; | ||
const concreteIndexName = `${primaryNamespacedAlias}-000001`; | ||
|
||
if (!aliasExists) { | ||
const secondaryNamespacedAlias = | ||
this.options.secondaryAlias != null | ||
? getNamespacedAlias({ alias: this.options.secondaryAlias, namespace }) | ||
: undefined; | ||
const template = this.createNamespacedIndexTemplate({ | ||
primaryNamespacedAlias, | ||
secondaryNamespacedAlias, | ||
namespace, | ||
}); | ||
// 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? | ||
Comment on lines
+179
to
+180
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely true. It may not be critical for 7.15 though since it's the first release. |
||
// (Before upgrading any indices, find and upgrade all namespaced index templates - component templates | ||
// will already have been upgraded by solutions or rule registry, in the case of technical/ECS templates) | ||
// With the current structure, it's tricky because the index template creation | ||
// depends on both the namespace and secondary alias, both of which are not currently available | ||
// 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we will need to somehow retrieve the namespace from existing index templates, I think it's a good idea to store it in |
||
await clusterClient.indices.putIndexTemplate(template); | ||
try { | ||
await clusterClient.indices.create({ | ||
index: concreteIndexName, | ||
body: { | ||
aliases: { | ||
[alias]: { | ||
[primaryNamespacedAlias]: { | ||
is_write_index: true, | ||
}, | ||
}, | ||
|
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.
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.
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.
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.
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.
Oh... Holy moly, yes you're right!
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.
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 😄