Skip to content

Commit 718dcce

Browse files
maxcoldkibanamachine
authored andcommitted
[Entity Analytics] Parameterise refresh on resolution bulk writes (elastic#268467)
## Summary Fixes the 403 returned by `POST /api/security/entity_store/resolution/link` and `/unlink` on serverless with `platform_engineer` role (caught in e2e tests). Root cause: `bulkUpdateEntityDocs` called `esClient.bulk({ refresh: true })`, which requires the `indices:admin/refresh/unpromotable` action — not granted to `platform_engineer` on `.entities.v2.latest.*`. This PR replaces the hardcoded `refresh: true` with a configurable `refresh` option on `bulkUpdateEntityDocs`, defaulting to `'wait_for'`. Both `'wait_for'` and `false` only require `write` privilege. Caller settings: | Caller | Setting | Why | | --- | --- | --- | | UI flyout routes (`/resolution/link`, `/resolution/unlink`) | `'wait_for'` (default) | UI immediately refetches the resolution group; without read-your-writes guarantee the refetch might get stale state | | CSV upload (`processRow`) | `false` | `'wait_for'` adds ~1s per row in sequential CSV processing; 200-row uploads were exceeding the HTTP socket timeout. With `false`, 200 rows complete in ~2s. Trade-off: within a single upload, two rows resolving the same alias to different targets silently take the last writer (though this matches existing "latest wins" semantics) | | Automated resolution maintainer | `false` | Buckets are pre-collected in memory; nothing within the same task run reads back the write, so the refresh wait is dead time | Fixes elastic#266752 Related: elastic#266589 (Cypress e2e that surfaced the bug) ### Checklist - [x] 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) - [x] [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 - [x] 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) - [x] 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. - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] 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) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. ### Identify risks - **Stale read for ~1s after CSV upload / maintainer write.** Severity: low. Mitigation: the CSV per-row response is built from in-process state, not a re-read; the maintainer doesn't read back its own writes within a run. Subsequent reads from a different request will see the new state after the next natural index refresh (<1s). - **"Last write wins" within a single CSV upload.** Severity: low. Mitigation: matches the broader "latest wins" approach already accepted. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
1 parent dafdc05 commit 718dcce

11 files changed

Lines changed: 96 additions & 22 deletions

File tree

oas_docs/output/kibana.serverless.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76103,7 +76103,7 @@ paths:
7610376103

7610476104
Refer to [Spaces](https://www.elastic.co/docs/deploy-manage/manage-spaces) for more information.
7610576105

76106-
Link one or more entities to a target entity, creating a resolution group. Requires an enterprise license.<br/><br/>[Required authorization] Route required privileges: securitySolution AND securitySolution-entity-analytics.
76106+
Link one or more entities to a target entity, creating a resolution group. Changes become visible on subsequent reads after the next index refresh (typically <1s). Requires an enterprise license.<br/><br/>[Required authorization] Route required privileges: securitySolution AND securitySolution-entity-analytics.
7610776107
operationId: post-security-entity-store-resolution-link
7610876108
parameters:
7610976109
- description: A required header to protect against CSRF attacks
@@ -76217,7 +76217,7 @@ paths:
7621776217

7621876218
Refer to [Spaces](https://www.elastic.co/docs/deploy-manage/manage-spaces) for more information.
7621976219

76220-
Remove one or more entities from their resolution group. Requires an enterprise license.<br/><br/>[Required authorization] Route required privileges: securitySolution AND securitySolution-entity-analytics.
76220+
Remove one or more entities from their resolution group. Changes become visible on subsequent reads after the next index refresh (typically <1s). Requires an enterprise license.<br/><br/>[Required authorization] Route required privileges: securitySolution AND securitySolution-entity-analytics.
7622176221
operationId: post-security-entity-store-resolution-unlink
7622276222
parameters:
7622376223
- description: A required header to protect against CSRF attacks

oas_docs/output/kibana.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81312,7 +81312,7 @@ paths:
8131281312

8131381313
Refer to [Spaces](https://www.elastic.co/docs/deploy-manage/manage-spaces) for more information.
8131481314

81315-
Link one or more entities to a target entity, creating a resolution group. Requires an enterprise license.<br/><br/>[Required authorization] Route required privileges: securitySolution AND securitySolution-entity-analytics.
81315+
Link one or more entities to a target entity, creating a resolution group. Changes become visible on subsequent reads after the next index refresh (typically <1s). Requires an enterprise license.<br/><br/>[Required authorization] Route required privileges: securitySolution AND securitySolution-entity-analytics.
8131681316
operationId: post-security-entity-store-resolution-link
8131781317
parameters:
8131881318
- description: A required header to protect against CSRF attacks
@@ -81426,7 +81426,7 @@ paths:
8142681426

8142781427
Refer to [Spaces](https://www.elastic.co/docs/deploy-manage/manage-spaces) for more information.
8142881428

81429-
Remove one or more entities from their resolution group. Requires an enterprise license.<br/><br/>[Required authorization] Route required privileges: securitySolution AND securitySolution-entity-analytics.
81429+
Remove one or more entities from their resolution group. Changes become visible on subsequent reads after the next index refresh (typically <1s). Requires an enterprise license.<br/><br/>[Required authorization] Route required privileges: securitySolution AND securitySolution-entity-analytics.
8143081430
operationId: post-security-entity-store-resolution-unlink
8143181431
parameters:
8143281432
- description: A required header to protect against CSRF attacks

x-pack/solutions/security/plugins/entity_store/server/domain/resolution/resolution_client.test.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ describe('ResolutionClient', () => {
9999
});
100100
expect(mockEsClient.bulk).toHaveBeenCalledWith(
101101
expect.objectContaining({
102-
refresh: true,
102+
refresh: 'wait_for',
103103
operations: expect.arrayContaining([
104104
expect.objectContaining({
105105
update: expect.objectContaining({
@@ -111,6 +111,21 @@ describe('ResolutionClient', () => {
111111
);
112112
});
113113

114+
it('should forward refresh: false to the bulk call when caller passes it', async () => {
115+
const targetDoc = createEntityDoc('target-1');
116+
const entity1Doc = createEntityDoc('entity-1');
117+
118+
mockEsClient.search.mockResolvedValueOnce(
119+
createSearchResponse([targetDoc, entity1Doc]) as never
120+
);
121+
mockEsClient.search.mockResolvedValueOnce(createSearchResponse([]) as never);
122+
mockEsClient.bulk.mockResolvedValueOnce({ errors: false, items: [] } as never);
123+
124+
await client.linkEntities('target-1', ['entity-1'], { refresh: false });
125+
126+
expect(mockEsClient.bulk).toHaveBeenCalledWith(expect.objectContaining({ refresh: false }));
127+
});
128+
114129
it('should pass nested doc payloads to ES bulk (not flat dotted keys)', async () => {
115130
const targetDoc = createEntityDoc('target-1');
116131
const entity1Doc = createEntityDoc('entity-1');
@@ -348,7 +363,7 @@ describe('ResolutionClient', () => {
348363
expect(result).toEqual({ unlinked: ['alias-1'], skipped: [] });
349364
expect(mockEsClient.bulk).toHaveBeenCalledWith(
350365
expect.objectContaining({
351-
refresh: true,
366+
refresh: 'wait_for',
352367
operations: expect.arrayContaining([
353368
expect.objectContaining({
354369
doc: {
@@ -366,6 +381,17 @@ describe('ResolutionClient', () => {
366381
);
367382
});
368383

384+
it('should forward refresh: false to the bulk call when caller passes it', async () => {
385+
const aliasDoc = createEntityDoc('alias-1', 'user', 'target-1');
386+
387+
mockEsClient.search.mockResolvedValueOnce(createSearchResponse([aliasDoc]) as never);
388+
mockEsClient.bulk.mockResolvedValueOnce({ errors: false, items: [] } as never);
389+
390+
await client.unlinkEntities(['alias-1'], { refresh: false });
391+
392+
expect(mockEsClient.bulk).toHaveBeenCalledWith(expect.objectContaining({ refresh: false }));
393+
});
394+
369395
it('should throw EntitiesNotFoundError when entities are missing', async () => {
370396
mockEsClient.search.mockResolvedValueOnce(createSearchResponse([]) as never);
371397

x-pack/solutions/security/plugins/entity_store/server/domain/resolution/resolution_client.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
ResolutionUpdateError,
2121
SelfLinkError,
2222
} from '../errors';
23+
import type { RefreshOption } from '../../infra/elasticsearch/resolution';
2324
import {
2425
searchEntitiesByIds,
2526
searchByResolvedToField,
@@ -54,6 +55,11 @@ export interface ResolutionGroup {
5455
group_size: number;
5556
}
5657

58+
/** Options forwarded to the underlying bulk write. */
59+
export interface ResolutionWriteOptions {
60+
refresh?: RefreshOption;
61+
}
62+
5763
interface FetchedEntities {
5864
sources: Map<string, Record<string, unknown>>;
5965
docIds: Map<string, string>;
@@ -75,7 +81,12 @@ export class ResolutionClient {
7581
* Validates chain prevention (can't link an alias) and has-aliases prevention
7682
* (can't link an entity that has aliases pointing to it).
7783
*/
78-
public async linkEntities(targetId: string, rawEntityIds: string[]): Promise<LinkResult> {
84+
public async linkEntities(
85+
targetId: string,
86+
rawEntityIds: string[],
87+
options: ResolutionWriteOptions = {}
88+
): Promise<LinkResult> {
89+
const { refresh = 'wait_for' } = options;
7990
const index = getLatestEntitiesIndexName(this.namespace);
8091

8192
// 1. Deduplicate entity_ids
@@ -133,7 +144,7 @@ export class ResolutionClient {
133144
docId: docIds.get(entityId)!,
134145
doc: { [RESOLVED_TO_FIELD]: targetId },
135146
}));
136-
const linkResult = await bulkUpdateEntityDocs(this.esClient, { index, updates });
147+
const linkResult = await bulkUpdateEntityDocs(this.esClient, { index, updates, refresh });
137148

138149
this.throwOnBulkErrors(linkResult, `linking entities to '${targetId}'`);
139150

@@ -144,7 +155,11 @@ export class ResolutionClient {
144155
* Unlinks alias entities by removing their resolved_to field.
145156
* Unlinked entities become standalone.
146157
*/
147-
public async unlinkEntities(rawEntityIds: string[]): Promise<UnlinkResult> {
158+
public async unlinkEntities(
159+
rawEntityIds: string[],
160+
options: ResolutionWriteOptions = {}
161+
): Promise<UnlinkResult> {
162+
const { refresh = 'wait_for' } = options;
148163
const index = getLatestEntitiesIndexName(this.namespace);
149164

150165
// 1. Deduplicate and fetch all entities
@@ -176,7 +191,7 @@ export class ResolutionClient {
176191
docId: docIds.get(entityId)!,
177192
doc: { [RESOLVED_TO_FIELD]: null },
178193
}));
179-
const unlinkResult = await bulkUpdateEntityDocs(this.esClient, { index, updates });
194+
const unlinkResult = await bulkUpdateEntityDocs(this.esClient, { index, updates, refresh });
180195

181196
this.throwOnBulkErrors(unlinkResult, 'unlinking entities');
182197

x-pack/solutions/security/plugins/entity_store/server/infra/elasticsearch/resolution.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,24 @@ interface BulkFieldUpdate {
9191
doc: Record<string, unknown>;
9292
}
9393

94+
/**
95+
* Refresh option for bulk writes against the latest entities index.
96+
*
97+
* - `'wait_for'` (default): block until the next scheduled refresh fires
98+
* (typically <1s). Read-your-writes guaranteed on subsequent searches.
99+
* Only requires `write` privilege.
100+
* - `false`: return immediately; the doc becomes searchable on the next
101+
* natural refresh. Stale reads possible for ~1s. Only requires `write`.
102+
*
103+
* `true` is intentionally not supported: forcing a refresh requires the
104+
* `indices:admin/refresh/unpromotable` action.
105+
*
106+
* Caller guidance: UI routes rely on the default so the immediate refetch
107+
* sees the new state. Bulk callers (CSV upload, automated maintainer) pass
108+
* `false` to avoid the per-write refresh wait.
109+
*/
110+
export type RefreshOption = boolean | 'wait_for';
111+
94112
/**
95113
* Bulk updates entity documents by pre-computed _id.
96114
* Uses retry_on_conflict to handle concurrent modifications.
@@ -100,18 +118,20 @@ export const bulkUpdateEntityDocs = (
100118
params: {
101119
index: string;
102120
updates: BulkFieldUpdate[];
121+
refresh?: RefreshOption;
103122
}
104123
): Promise<BulkResponse> => {
105-
const operations = params.updates.flatMap(({ docId, doc }) => [
124+
const { index, updates, refresh = 'wait_for' } = params;
125+
const operations = updates.flatMap(({ docId, doc }) => [
106126
{
107127
update: {
108-
_index: params.index,
128+
_index: index,
109129
_id: docId,
110130
retry_on_conflict: RETRY_ON_CONFLICT,
111131
},
112132
},
113133
{ doc: unflattenObject(doc) },
114134
]);
115135

116-
return esClient.bulk({ operations, refresh: true });
136+
return esClient.bulk({ operations, refresh });
117137
};

x-pack/solutions/security/plugins/entity_store/server/maintainers/automated_resolution/__tests__/run.test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,9 @@ describe('Automated Resolution', () => {
206206
createDeps(state, mockEsClient, mockResolutionClient)
207207
);
208208

209-
expect(mockLinkEntities).toHaveBeenCalledWith('user-okta', ['user-entra']);
209+
expect(mockLinkEntities).toHaveBeenCalledWith('user-okta', ['user-entra'], {
210+
refresh: false,
211+
});
210212
expect(result.lastRun?.resolutionsCreated).toBe(1);
211213
});
212214

@@ -236,7 +238,9 @@ describe('Automated Resolution', () => {
236238
createDeps(state, mockEsClient, mockResolutionClient)
237239
);
238240

239-
expect(mockLinkEntities).toHaveBeenCalledWith('user-existing-target', ['user-new']);
241+
expect(mockLinkEntities).toHaveBeenCalledWith('user-existing-target', ['user-new'], {
242+
refresh: false,
243+
});
240244
expect(result.lastRun?.resolutionsCreated).toBe(1);
241245
});
242246

x-pack/solutions/security/plugins/entity_store/server/maintainers/automated_resolution/run.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ async function resolveMatchBuckets(
302302
.map((e) => e.entityId);
303303
if (aliasIds.length === 0) continue;
304304

305-
const result = await resolutionClient.linkEntities(targetId, aliasIds);
305+
const result = await resolutionClient.linkEntities(targetId, aliasIds, { refresh: false });
306306
resolutionsCreated += result.linked.length;
307307
} else if (unresolvedEntities.length >= 2) {
308308
// New group: pick target via namespace priority, link the rest
@@ -311,7 +311,9 @@ async function resolveMatchBuckets(
311311
.filter((e) => e.entityId !== targetEntity.entityId)
312312
.map((e) => e.entityId);
313313

314-
const result = await resolutionClient.linkEntities(targetEntity.entityId, aliasIds);
314+
const result = await resolutionClient.linkEntities(targetEntity.entityId, aliasIds, {
315+
refresh: false,
316+
});
315317
resolutionsCreated += result.linked.length;
316318
}
317319
// else: only 1 unresolved, no existing targets → no match, skip

x-pack/solutions/security/plugins/entity_store/server/routes/apis/resolution/link.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ export function registerResolutionLink(router: EntityStorePluginRouter) {
3939
access: 'public',
4040
summary: 'Link entities',
4141
description:
42-
'Link one or more entities to a target entity, creating a resolution group. Requires an enterprise license.',
42+
'Link one or more entities to a target entity, creating a resolution group. ' +
43+
'Changes become visible on subsequent reads after the next index refresh ' +
44+
'(typically <1s). Requires an enterprise license.',
4345
options: {
4446
tags: ['oas-tag:Security entity store'],
4547
},

x-pack/solutions/security/plugins/entity_store/server/routes/apis/resolution/unlink.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ export function registerResolutionUnlink(router: EntityStorePluginRouter) {
3131
access: 'public',
3232
summary: 'Unlink entities',
3333
description:
34-
'Remove one or more entities from their resolution group. Requires an enterprise license.',
34+
'Remove one or more entities from their resolution group. Changes become ' +
35+
'visible on subsequent reads after the next index refresh (typically <1s). ' +
36+
'Requires an enterprise license.',
3537
options: {
3638
tags: ['oas-tag:Security entity store'],
3739
},

x-pack/solutions/security/plugins/security_solution/server/lib/entity_analytics/entity_resolution/csv_upload.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ describe('processResolutionCsvUpload', () => {
340340
});
341341
});
342342

343-
it('should call linkEntities with correct args', async () => {
343+
it('should call linkEntities with correct args and refresh: false', async () => {
344344
mockResolutionClient.linkEntities.mockResolvedValue({
345345
linked: ['alias:1'],
346346
skipped: [],
@@ -350,7 +350,9 @@ describe('processResolutionCsvUpload', () => {
350350
const csv = 'type,user.email,resolved_to\nuser,alias@test.com,target:golden';
351351
await processResolutionCsvUpload(createMockStream(csv), deps());
352352

353-
expect(mockResolutionClient.linkEntities).toHaveBeenCalledWith('target:golden', ['alias:1']);
353+
expect(mockResolutionClient.linkEntities).toHaveBeenCalledWith('target:golden', ['alias:1'], {
354+
refresh: false,
355+
});
354356
});
355357

356358
it('should report success with linked and skipped counts', async () => {

0 commit comments

Comments
 (0)