mirror of
https://github.com/elastic/kibana.git
synced 2025-04-23 17:28:26 -04:00
[Session Management] Fix session index creation and update (#208244)
Closes https://github.com/elastic/kibana/issues/208243 ## Summary [This PR](https://github.com/elastic/kibana/pull/204097) introduced an unintended issue for users running 8.x with a 7.x archive. This has been fixed by correctly searching for existing index by both name and alias. ### To test: You can follow the same steps as the above PR: https://github.com/elastic/kibana/pull/204097 ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [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 - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss. Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging. - [ ] [See some risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) - [ ] ...
This commit is contained in:
Sid
GitHub
parent
38fc6344c6
commit
449ac98572
2 changed files with 110 additions and 23 deletions
|
|
@ -96,10 +96,76 @@ describe('Session index', () => {
|
|||
|
||||
expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.putIndexTemplate).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.putAlias).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
|
||||
index: indexName,
|
||||
name: aliasName,
|
||||
});
|
||||
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not create index if alias exists', async () => {
|
||||
mockElasticsearchClient.indices.existsTemplate.mockResponse(false);
|
||||
mockElasticsearchClient.indices.existsIndexTemplate.mockResponse(true);
|
||||
mockElasticsearchClient.indices.exists.mockImplementation(
|
||||
async ({ index }) => index === aliasName
|
||||
);
|
||||
|
||||
await sessionIndex.initialize();
|
||||
|
||||
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledWith({ index: aliasName });
|
||||
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledTimes(1);
|
||||
|
||||
expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.putIndexTemplate).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
|
||||
index: indexName,
|
||||
name: aliasName,
|
||||
});
|
||||
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not create index if index exists', async () => {
|
||||
mockElasticsearchClient.indices.existsTemplate.mockResponse(false);
|
||||
mockElasticsearchClient.indices.existsIndexTemplate.mockResponse(true);
|
||||
mockElasticsearchClient.indices.exists.mockImplementation(
|
||||
async ({ index }) => index === indexName
|
||||
);
|
||||
|
||||
await sessionIndex.initialize();
|
||||
|
||||
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledWith({ index: aliasName });
|
||||
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledWith({ index: indexName });
|
||||
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledTimes(2);
|
||||
|
||||
expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.putIndexTemplate).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
|
||||
index: indexName,
|
||||
name: aliasName,
|
||||
});
|
||||
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('creates index if neither index or alias exists', async () => {
|
||||
mockElasticsearchClient.indices.existsTemplate.mockResponse(false);
|
||||
mockElasticsearchClient.indices.existsIndexTemplate.mockResponse(true);
|
||||
mockElasticsearchClient.indices.exists.mockResponse(false);
|
||||
|
||||
await sessionIndex.initialize();
|
||||
|
||||
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledWith({ index: aliasName });
|
||||
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledWith({ index: indexName });
|
||||
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledTimes(2);
|
||||
|
||||
expect(mockElasticsearchClient.indices.deleteTemplate).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.putIndexTemplate).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
|
||||
index: indexName,
|
||||
name: aliasName,
|
||||
});
|
||||
expect(mockElasticsearchClient.indices.create).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not delete legacy index template if the legacy template API is not available (410)', async () => {
|
||||
const goneError = new errors.ResponseError(
|
||||
securityMock.createApiResponse(
|
||||
|
|
@ -216,7 +282,10 @@ describe('Session index', () => {
|
|||
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
|
||||
|
||||
expect(mockElasticsearchClient.indices.putAlias).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
|
||||
index: indexName,
|
||||
name: aliasName,
|
||||
});
|
||||
});
|
||||
|
||||
it('updates mappings for existing index without version in the meta', async () => {
|
||||
|
|
@ -237,7 +306,10 @@ describe('Session index', () => {
|
|||
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
|
||||
|
||||
expect(mockElasticsearchClient.indices.putAlias).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
|
||||
index: indexName,
|
||||
name: aliasName,
|
||||
});
|
||||
|
||||
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledTimes(1);
|
||||
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledWith({ index: aliasName });
|
||||
|
|
@ -266,7 +338,10 @@ describe('Session index', () => {
|
|||
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
|
||||
|
||||
expect(mockElasticsearchClient.indices.putAlias).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
|
||||
index: indexName,
|
||||
name: aliasName,
|
||||
});
|
||||
|
||||
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledTimes(1);
|
||||
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledWith({ index: aliasName });
|
||||
|
|
@ -295,7 +370,10 @@ describe('Session index', () => {
|
|||
expect(mockElasticsearchClient.indices.deleteIndexTemplate).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.create).not.toHaveBeenCalled();
|
||||
|
||||
expect(mockElasticsearchClient.indices.putAlias).not.toHaveBeenCalled();
|
||||
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledWith({
|
||||
index: indexName,
|
||||
name: aliasName,
|
||||
});
|
||||
|
||||
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledTimes(1);
|
||||
expect(mockElasticsearchClient.indices.getMapping).toHaveBeenCalledWith({ index: aliasName });
|
||||
|
|
@ -447,7 +525,7 @@ describe('Session index', () => {
|
|||
{ meta: true }
|
||||
);
|
||||
|
||||
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledTimes(1);
|
||||
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledTimes(2);
|
||||
expect(mockElasticsearchClient.indices.create).toHaveBeenCalledTimes(1);
|
||||
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledTimes(1);
|
||||
expect(mockElasticsearchClient.search).toHaveBeenCalledTimes(1);
|
||||
|
|
@ -1430,7 +1508,7 @@ describe('Session index', () => {
|
|||
metadata: { primaryTerm: 321, sequenceNumber: 654 },
|
||||
});
|
||||
|
||||
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledTimes(1);
|
||||
expect(mockElasticsearchClient.indices.exists).toHaveBeenCalledTimes(2);
|
||||
expect(mockElasticsearchClient.indices.create).toHaveBeenCalledTimes(1);
|
||||
expect(mockElasticsearchClient.indices.putAlias).toHaveBeenCalledTimes(1);
|
||||
|
||||
|
|
|
|||
|
|
@ -625,16 +625,35 @@ export class SessionIndex {
|
|||
);
|
||||
}
|
||||
|
||||
private async attachAliasToIndex() {
|
||||
// Prior to https://github.com/elastic/kibana/pull/134900, sessions would be written directly against the session index.
|
||||
// Now, we write sessions against a new session index alias. This call ensures that the alias exists, and is attached to the index.
|
||||
// This operation is safe to repeat, even if the alias already exists. This seems safer than retrieving the index details, and inspecting
|
||||
// it to see if the alias already exists.
|
||||
try {
|
||||
await this.options.elasticsearchClient.indices.putAlias({
|
||||
index: this.indexName,
|
||||
name: this.aliasName,
|
||||
});
|
||||
} catch (err) {
|
||||
this.options.logger.error(`Failed to attach alias to session index: ${err.message}`);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates the session index if it doesn't already exist.
|
||||
*/
|
||||
private async ensureSessionIndexExists() {
|
||||
// Check if required index exists.
|
||||
// It is possible for users to migrate from older versions of Kibana where the session index was created without
|
||||
// an alias (pre-8.4). In this case, we need to check if the index exists under the alias name, or the index name.
|
||||
// If the index exists under the alias name, we can assume that the alias is already attached.
|
||||
let indexExists = false;
|
||||
try {
|
||||
indexExists = await this.options.elasticsearchClient.indices.exists({
|
||||
index: this.aliasName,
|
||||
});
|
||||
indexExists =
|
||||
(await this.options.elasticsearchClient.indices.exists({ index: this.aliasName })) ||
|
||||
(await this.options.elasticsearchClient.indices.exists({ index: this.indexName }));
|
||||
} catch (err) {
|
||||
this.options.logger.error(`Failed to check if session index exists: ${err.message}`);
|
||||
throw err;
|
||||
|
|
@ -662,19 +681,7 @@ export class SessionIndex {
|
|||
}
|
||||
}
|
||||
|
||||
// Prior to https://github.com/elastic/kibana/pull/134900, sessions would be written directly against the session index.
|
||||
// Now, we write sessions against a new session index alias. This call ensures that the alias exists, and is attached to the index.
|
||||
// This operation is safe to repeat, even if the alias already exists. This seems safer than retrieving the index details, and inspecting
|
||||
// it to see if the alias already exists.
|
||||
try {
|
||||
await this.options.elasticsearchClient.indices.putAlias({
|
||||
index: this.indexName,
|
||||
name: this.aliasName,
|
||||
});
|
||||
} catch (err) {
|
||||
this.options.logger.error(`Failed to attach alias to session index: ${err.message}`);
|
||||
throw err;
|
||||
}
|
||||
await this.attachAliasToIndex();
|
||||
|
||||
return;
|
||||
}
|
||||
|
|
@ -683,6 +690,8 @@ export class SessionIndex {
|
|||
'Session index already exists. Attaching alias to the index and ensuring up-to-date mappings...'
|
||||
);
|
||||
|
||||
await this.attachAliasToIndex();
|
||||
|
||||
let indexMappingsVersion: string | undefined;
|
||||
try {
|
||||
const indexMappings = await this.options.elasticsearchClient.indices.getMapping({
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue