Skip to content

Commit 6b81b37

Browse files
SrdjanLLsorenlouvkibanamachine
authored
[Lock Manager] Handle alias resolution when checking lock index mappings (#244559)
Relates to: elastic/sdh-kibana#5923 ## Summary - Handle Lock Manager's alias resolution during migration: - when checking lock index mappings - in cases when concrete index creation is attempted, but the name is already used as alias ## Testing New unit tests: ```bash yarn test:jest src/platform/packages/shared/kbn-lock-manager/src/setup_lock_manager_index.test.ts ``` Updated integration tests: ```bash node scripts/functional_tests_server --config x-pack/solutions/observability/test/api_integration_deployment_agnostic/configs/stateful/oblt.ai_assistant.stateful.config.ts # Then run in a separate terminal: node scripts/functional_test_runner --config x-pack/solutions/observability/test/api_integration_deployment_agnostic/configs/stateful/oblt.ai_assistant.stateful.config.ts --grep="LockManager" ``` ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [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 ### Identify risks - This PR aims to mitigate risks that occur during migration. The implementation makes certain assumptions on the way migration tooling functions. In the unlikely events of that changing, the logic may need to be revisited ### Reviewer notes - AI assisted coding with Cursor and Claude 4.5 Opus: - Plan generation (to help cover all relevant places - Unit test generation --------- Co-authored-by: Søren Louv-Jansen <[email protected]> Co-authored-by: kibanamachine <[email protected]>
1 parent ace94fb commit 6b81b37

File tree

5 files changed

+307
-2
lines changed

5 files changed

+307
-2
lines changed

src/platform/packages/shared/kbn-lock-manager/moon.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ project:
2020
dependsOn:
2121
- '@kbn/logging'
2222
- '@kbn/core'
23+
- '@kbn/core-elasticsearch-client-server-mocks'
24+
- '@kbn/core-logging-server-mocks'
2325
tags:
2426
- shared-server
2527
- package
Lines changed: 221 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,221 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
import { errors } from '@elastic/elasticsearch';
11+
import type { MappingTypeMapping } from '@elastic/elasticsearch/lib/api/types';
12+
import { elasticsearchClientMock } from '@kbn/core-elasticsearch-client-server-mocks';
13+
import { loggingSystemMock } from '@kbn/core-logging-server-mocks';
14+
import {
15+
removeLockIndexWithIncorrectMappings,
16+
ensureTemplatesAndIndexCreated,
17+
LOCKS_CONCRETE_INDEX_NAME,
18+
} from './setup_lock_manager_index';
19+
20+
describe('removeLockIndexWithIncorrectMappings', () => {
21+
let esClient: ReturnType<typeof elasticsearchClientMock.createInternalClient>;
22+
const logger = loggingSystemMock.createLogger();
23+
24+
const correctMappings: MappingTypeMapping = {
25+
dynamic: 'false',
26+
properties: {
27+
token: { type: 'keyword' },
28+
expiresAt: { type: 'date' },
29+
createdAt: { type: 'date' },
30+
metadata: { enabled: false, type: 'object' },
31+
},
32+
};
33+
34+
const incorrectMappings: MappingTypeMapping = {
35+
dynamic: 'false',
36+
properties: {
37+
token: { type: 'text' }, // incorrect - should be keyword
38+
expiresAt: { type: 'text' }, // incorrect - should be date
39+
createdAt: { type: 'date' },
40+
metadata: { enabled: false, type: 'object' },
41+
},
42+
};
43+
44+
beforeEach(() => {
45+
esClient = elasticsearchClientMock.createInternalClient();
46+
});
47+
48+
describe('when index name matches LOCKS_CONCRETE_INDEX_NAME', () => {
49+
it('does not delete index when mappings are correct', async () => {
50+
esClient.indices.getMapping.mockResolvedValueOnce({
51+
[LOCKS_CONCRETE_INDEX_NAME]: { mappings: correctMappings },
52+
});
53+
54+
await removeLockIndexWithIncorrectMappings(esClient, logger);
55+
56+
expect(esClient.indices.delete).not.toHaveBeenCalled();
57+
});
58+
59+
it('deletes index when mappings are incorrect', async () => {
60+
esClient.indices.getMapping.mockResolvedValueOnce({
61+
[LOCKS_CONCRETE_INDEX_NAME]: { mappings: incorrectMappings },
62+
});
63+
64+
await removeLockIndexWithIncorrectMappings(esClient, logger);
65+
66+
expect(esClient.indices.delete).toHaveBeenCalledWith({ index: LOCKS_CONCRETE_INDEX_NAME });
67+
});
68+
});
69+
70+
describe('when index is accessed via alias (response key differs from requested index)', () => {
71+
const reindexedIndexName = `${LOCKS_CONCRETE_INDEX_NAME}-reindexed-for-10`;
72+
73+
it('does not delete index when mappings are correct', async () => {
74+
esClient.indices.getMapping.mockResolvedValueOnce({
75+
[reindexedIndexName]: { mappings: correctMappings },
76+
});
77+
78+
await removeLockIndexWithIncorrectMappings(esClient, logger);
79+
80+
expect(esClient.indices.delete).not.toHaveBeenCalled();
81+
});
82+
83+
it('deletes index when mappings are incorrect', async () => {
84+
esClient.indices.getMapping.mockResolvedValueOnce({
85+
[reindexedIndexName]: { mappings: incorrectMappings },
86+
});
87+
88+
await removeLockIndexWithIncorrectMappings(esClient, logger);
89+
90+
expect(esClient.indices.delete).toHaveBeenCalledWith({ index: LOCKS_CONCRETE_INDEX_NAME });
91+
});
92+
});
93+
94+
describe('when index does not exist (404 error)', () => {
95+
it('returns early without error', async () => {
96+
esClient.indices.getMapping.mockRejectedValueOnce(
97+
new errors.ResponseError({
98+
statusCode: 404,
99+
body: { error: { type: 'index_not_found_exception' } },
100+
headers: {},
101+
meta: {} as any,
102+
warnings: [],
103+
})
104+
);
105+
106+
await removeLockIndexWithIncorrectMappings(esClient, logger);
107+
108+
expect(esClient.indices.delete).not.toHaveBeenCalled();
109+
});
110+
});
111+
112+
describe('when getMapping returns empty response', () => {
113+
it('returns early without error', async () => {
114+
esClient.indices.getMapping.mockResolvedValueOnce({});
115+
116+
await removeLockIndexWithIncorrectMappings(esClient, logger);
117+
118+
expect(esClient.indices.delete).not.toHaveBeenCalled();
119+
});
120+
});
121+
122+
describe('when mappings object is empty', () => {
123+
it('returns early without error', async () => {
124+
esClient.indices.getMapping.mockResolvedValueOnce({
125+
[LOCKS_CONCRETE_INDEX_NAME]: { mappings: undefined as any },
126+
});
127+
128+
await removeLockIndexWithIncorrectMappings(esClient, logger);
129+
130+
expect(esClient.indices.delete).not.toHaveBeenCalled();
131+
});
132+
});
133+
134+
describe('when getMapping fails with non-404 error', () => {
135+
it('returns without throwing', async () => {
136+
esClient.indices.getMapping.mockRejectedValueOnce(
137+
new errors.ResponseError({
138+
statusCode: 500,
139+
body: { error: { type: 'internal_server_error' } },
140+
headers: {},
141+
meta: {} as any,
142+
warnings: [],
143+
})
144+
);
145+
146+
await removeLockIndexWithIncorrectMappings(esClient, logger);
147+
148+
expect(esClient.indices.delete).not.toHaveBeenCalled();
149+
});
150+
});
151+
152+
describe('when delete fails', () => {
153+
it('does not throw', async () => {
154+
esClient.indices.getMapping.mockResolvedValueOnce({
155+
[LOCKS_CONCRETE_INDEX_NAME]: { mappings: incorrectMappings },
156+
});
157+
esClient.indices.delete.mockRejectedValueOnce(new Error('Delete failed'));
158+
159+
await expect(removeLockIndexWithIncorrectMappings(esClient, logger)).resolves.not.toThrow();
160+
});
161+
});
162+
});
163+
164+
describe('ensureTemplatesAndIndexCreated', () => {
165+
let esClient: ReturnType<typeof elasticsearchClientMock.createInternalClient>;
166+
const logger = loggingSystemMock.createLogger();
167+
168+
beforeEach(() => {
169+
esClient = elasticsearchClientMock.createInternalClient();
170+
});
171+
172+
it('creates index successfully', async () => {
173+
await ensureTemplatesAndIndexCreated(esClient, logger);
174+
175+
expect(esClient.cluster.putComponentTemplate).toHaveBeenCalled();
176+
expect(esClient.indices.putIndexTemplate).toHaveBeenCalled();
177+
expect(esClient.indices.create).toHaveBeenCalledWith({ index: LOCKS_CONCRETE_INDEX_NAME });
178+
});
179+
180+
it('handles resource_already_exists_exception without throwing', async () => {
181+
esClient.indices.create.mockRejectedValueOnce(
182+
new errors.ResponseError({
183+
statusCode: 400,
184+
body: { error: { type: 'resource_already_exists_exception' } },
185+
headers: {},
186+
meta: {} as any,
187+
warnings: [],
188+
})
189+
);
190+
191+
await expect(ensureTemplatesAndIndexCreated(esClient, logger)).resolves.not.toThrow();
192+
});
193+
194+
it('handles invalid_index_name_exception (index name exists as alias) without throwing', async () => {
195+
esClient.indices.create.mockRejectedValueOnce(
196+
new errors.ResponseError({
197+
statusCode: 400,
198+
body: { error: { type: 'invalid_index_name_exception' } },
199+
headers: {},
200+
meta: {} as any,
201+
warnings: [],
202+
})
203+
);
204+
205+
await expect(ensureTemplatesAndIndexCreated(esClient, logger)).resolves.not.toThrow();
206+
});
207+
208+
it('throws on other errors', async () => {
209+
esClient.indices.create.mockRejectedValueOnce(
210+
new errors.ResponseError({
211+
statusCode: 500,
212+
body: { error: { type: 'internal_server_error' } },
213+
headers: {},
214+
meta: {} as any,
215+
warnings: [],
216+
})
217+
);
218+
219+
await expect(ensureTemplatesAndIndexCreated(esClient, logger)).rejects.toThrow();
220+
});
221+
});

src/platform/packages/shared/kbn-lock-manager/src/setup_lock_manager_index.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,16 @@ export async function removeLockIndexWithIncorrectMappings(
3535
return;
3636
}
3737

38-
const { mappings } = res[LOCKS_CONCRETE_INDEX_NAME];
38+
const indexMappings = Object.values(res);
39+
if (indexMappings.length === 0) {
40+
logger.warn(`No mappings found for "${LOCKS_CONCRETE_INDEX_NAME}"`);
41+
return;
42+
}
43+
const { mappings } = indexMappings[0];
44+
if (!mappings) {
45+
logger.debug(`Mappings object is empty for "${LOCKS_CONCRETE_INDEX_NAME}"`);
46+
return;
47+
}
3948
const hasIncorrectMappings =
4049
mappings.properties?.token?.type !== 'keyword' ||
4150
mappings.properties?.expiresAt?.type !== 'date';
@@ -96,7 +105,12 @@ export async function ensureTemplatesAndIndexCreated(
96105
error instanceof errors.ResponseError &&
97106
error.body.error.type === 'resource_already_exists_exception';
98107

99-
if (isIndexAlreadyExistsError) {
108+
// Handle the case where the index name already exists as an alias (e.g., after a reindex operation during cluster upgrade)
109+
const isIndexNameExistsAsAliasError =
110+
error instanceof errors.ResponseError &&
111+
error.body.error.type === 'invalid_index_name_exception';
112+
113+
if (isIndexAlreadyExistsError || isIndexNameExistsAsAliasError) {
100114
logger.debug(`Index ${LOCKS_CONCRETE_INDEX_NAME} already exists. Skipping creation.`);
101115
return;
102116
}

src/platform/packages/shared/kbn-lock-manager/tsconfig.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,7 @@
1616
"kbn_references": [
1717
"@kbn/logging",
1818
"@kbn/core",
19+
"@kbn/core-elasticsearch-client-server-mocks",
20+
"@kbn/core-logging-server-mocks",
1921
]
2022
}

x-pack/solutions/observability/test/api_integration_deployment_agnostic/apis/ai_assistant/distributed_lock_manager/distributed_lock_manager.spec.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -804,6 +804,72 @@ export default function ApiTest({ getService }: DeploymentAgnosticFtrProviderCon
804804
expect(indexExists).to.be(true);
805805
});
806806
});
807+
808+
describe('when lock index is accessed via alias (simulating reindexed scenario)', () => {
809+
const reindexedIndexName = `${LOCKS_CONCRETE_INDEX_NAME}-reindexed-for-10`;
810+
811+
before(async () => {
812+
await deleteLockIndexAssets(es, log);
813+
814+
// Create the actual index with correct mappings but different name
815+
await es.indices.create({
816+
index: reindexedIndexName,
817+
settings: {
818+
number_of_shards: 1,
819+
auto_expand_replicas: '0-1',
820+
hidden: true,
821+
},
822+
mappings: {
823+
dynamic: false,
824+
properties: {
825+
token: { type: 'keyword' },
826+
metadata: { enabled: false },
827+
createdAt: { type: 'date' },
828+
expiresAt: { type: 'date' },
829+
},
830+
},
831+
});
832+
833+
// Add alias pointing LOCKS_CONCRETE_INDEX_NAME to the reindexed index
834+
await es.indices.putAlias({
835+
index: reindexedIndexName,
836+
name: LOCKS_CONCRETE_INDEX_NAME,
837+
});
838+
});
839+
840+
after(async () => {
841+
// Clean up alias and reindexed index
842+
await es.indices.deleteAlias(
843+
{ index: reindexedIndexName, name: LOCKS_CONCRETE_INDEX_NAME },
844+
{ ignore: [404] }
845+
);
846+
await es.indices.delete({ index: reindexedIndexName }, { ignore: [404] });
847+
});
848+
849+
it('should not throw when getMapping returns a different index name than requested', async () => {
850+
// Verify the alias setup
851+
const aliasExists = await es.indices.existsAlias({ name: LOCKS_CONCRETE_INDEX_NAME });
852+
expect(aliasExists).to.be(true);
853+
854+
// This should NOT throw "Cannot destructure property 'mappings' of 'res[LOCKS_CONCRETE_INDEX_NAME]' as it is undefined"
855+
try {
856+
await setupLockManagerIndex(es, logger);
857+
} catch (error) {
858+
expect().fail(
859+
`setupLockManagerIndex should not throw when index is accessed via alias, but got: ${error.message}`
860+
);
861+
}
862+
});
863+
864+
it('should successfully acquire a lock when index is accessed via alias', async () => {
865+
const lockManager = new LockManager('alias_test_lock', es, logger);
866+
const acquired = await lockManager.acquire();
867+
expect(acquired).to.be(true);
868+
869+
// Cleanup
870+
await lockManager.release();
871+
});
872+
});
807873
});
808874

809875
describe('setup robustness', () => {

0 commit comments

Comments
 (0)