Refactor Index Sync Logic and Improve Component Test Coverage by CAWilson94 · Pull Request #225783 · elastic/kibana

CAWilson94 · 2025-06-30T09:33:53Z

This pull request refactors the PrivilegeMonitoringDataClient class to improve code clarity and modularity in line with this issue: #225551

Renamed syncUsernamesFromIndex → fetchUsernamesFromIndex — now focused on retrieving usernames from the source index.
Added bulkUpsertMonitoredUsers — responsible for create/update logic in the internal index.
Added ingestUsersFromIndexSource — wraps fetch + sync logic with error handling and logging.
Helper methods added to reduce complexity in plainIndexSync.
Better handling for null indexPattern and index_not_found - removed continue statements, now handling explicitly.
Component testing for above.
Pulled in API Key and Sync Task Updates from this PR: #225551

🚧 Add testing for new types and functionalities from API Key and Sync Task merging.

How To Test

For testing steps, please see the steps on the API and Sync Task PR: #225551

…ent calls

…rations; helper functions for testing; formatting

x-pack/test_serverless/api_integration/test_suites/security/platform_security/authorization.ts

elasticmachine · 2025-07-07T07:41:41Z

🤖 Jobs for this PR can be triggered through checkboxes. 🚧

ℹ️ To trigger the CI, please tick the checkbox below 👇

Click to trigger kibana-pull-request for this PR!
Click to trigger kibana-deploy-project-from-pr for this PR!
Click to trigger kibana-deploy-cloud-from-pr for this PR!

CAWilson94 · 2025-07-07T08:12:43Z

...gins/security_solution/server/lib/entity_analytics/privilege_monitoring/auth/api_key.test.ts

+ * 2.0.
+ */
+
+describe('Privilege Monitoring API Key Manager', () => {


elasticmachine · 2025-07-07T09:15:37Z

💔 Build Failed

Buildkite Build
Commit: f005418

Failed CI Steps

Test Failures

[job] [logs] Jest Tests #4 / MonitoringEntitySourceDataClient init should initialize Monitoring Entity Source Sync Config Successfully
[job] [logs] Jest Tests #4 / MonitoringEntitySourceDataClient init should initialize Monitoring Entity Source Sync Config Successfully
[job] [logs] FTR Configs #24 / Serverless security API Platform security APIs security/authorization available features composite features
[job] [logs] FTR Configs #24 / Serverless security API Platform security APIs security/authorization available features composite features

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`securitySolution`	629	627	-2

Total ESLint disabled count

id	before	after	diff
`securitySolution`	727	725	-2

History

machadoum · 2025-07-07T12:57:16Z

...olution/server/lib/entity_analytics/privilege_monitoring/privilege_monitoring_data_client.ts

+    source: MonitoringEntitySourceDescriptor,
+    index: string


Why does it receive two params if index is already inside source?

May be a leftover from a previous iteration, good spot. I don't see a reason to have them separate. Will update this 🫡

Oh no, I see why now. I was trying to reduce the checks inside the ingestUsersFromIndexSource method so it can always assume it receives a valid index. If I used source.indexPattern directly in that method, I’d have to move the source.indexPattern === undefined check into it.

I felt it was a bit cleaner and easier to follow with that validation staying in plainIndexSync, since it keeps the top-level flow clear.

My usual preference is to push validation down to the lowest-level method so that ingestUsersFromIndexSource would be more self-contained and flexible for future reuse — but in this case, keeping it in the top-level method just felt simpler.

Let me know if you’d prefer the check moved down into ingestUsersFromIndexSource — happy to refactor if that feels more consistent!

CAWilson94 · 2025-07-14T13:08:49Z

...n/server/lib/entity_analytics/privilege_monitoring/saved_objects/monitoring_entity_source.ts

Merged in line with: #227420

CAWilson94 · 2025-07-21T16:19:16Z

...security/plugins/security_solution/server/lib/entity_analytics/privilege_monitoring/types.ts

 */

 export type PrivMonUserSource = 'csv' | 'api' | 'index_sync';
+export type SyncIntervalConfig = EntityAnalyticsConfig['monitoring']['privileges']['syncInterval'];


should be monitoring -> privileges -> developer -> syncInterval

CAWilson94 · 2025-07-23T14:39:42Z

...olution/server/lib/entity_analytics/privilege_monitoring/privilege_monitoring_data_client.ts


    const query = kuery ? toElasticsearchQuery(fromKueryExpression(kuery)) : { match_all: {} };
+
    while (true) {


instead of while true, needs to be more readable. Could rename this to something like 'hits' but look for where we do composite in risk scoring @hop-dev

CAWilson94 added 7 commits June 27, 2025 17:13

Split fetch user names into fetch and sync methods

1f48865

Splitting larger methods up into smaller dedicated functions

5bb0ecf

null handle for bulkUpsert - no usernames to sync case

b034c35

formatting

a3a8248

snapshot auth update

1e822b7

component testing updates; cohesive formatting; wait for added in cli…

b9b2177

…ent calls

unit tests for basic coverage of monitoring sync; typing for bulk ope…

c6215da

…rations; helper functions for testing; formatting

CAWilson94 commented Jul 6, 2025

View reviewed changes

x-pack/test_serverless/api_integration/test_suites/security/platform_security/authorization.ts Show resolved Hide resolved

CAWilson94 mentioned this pull request Jul 6, 2025

Add API Key Management and Enable Index Sync via Scheduled Task #225551

Merged

Merge branch 'main' into ea-225551-refactor-priv-sync

738a38a

CAWilson94 marked this pull request as ready for review July 7, 2025 07:36

CAWilson94 requested review from a team as code owners July 7, 2025 07:36

CAWilson94 requested a review from machadoum July 7, 2025 07:36

Merge branch 'main' into ea-225551-refactor-priv-sync

f005418

CAWilson94 marked this pull request as draft July 7, 2025 07:41

api key test skeleton WiP

ec0c975

CAWilson94 commented Jul 7, 2025

View reviewed changes

machadoum reviewed Jul 7, 2025

View reviewed changes

Merge branch 'main' into ea-225551-refactor-priv-sync

d6806b8

CAWilson94 commented Jul 14, 2025

View reviewed changes

CAWilson94 added 5 commits July 17, 2025 10:05

WiP register index with monitoring saved object debugging

0441aee

SO cleanup

03a5f92

Added delete route to API for monitoring source

b1f1c03

WiP

7e61f99

config wired in for interval sync

f6d66f1

CAWilson94 commented Jul 21, 2025

View reviewed changes

CAWilson94 commented Jul 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Index Sync Logic and Improve Component Test Coverage#225783

Refactor Index Sync Logic and Improve Component Test Coverage#225783
CAWilson94 wants to merge 16 commits intoelastic:mainfrom
CAWilson94:ea-225551-refactor-priv-sync

CAWilson94 commented Jun 30, 2025 •

edited

Loading

Uh oh!

Uh oh!

elasticmachine commented Jul 7, 2025

Uh oh!

CAWilson94 Jul 7, 2025

Uh oh!

elasticmachine commented Jul 7, 2025 •

edited

Loading

ESLint disabled line counts

Total ESLint disabled count

Uh oh!

machadoum Jul 7, 2025

Uh oh!

CAWilson94 Jul 14, 2025

Uh oh!

CAWilson94 Jul 14, 2025

Uh oh!

CAWilson94 Jul 14, 2025

Uh oh!

CAWilson94 Jul 21, 2025

Uh oh!

CAWilson94 Jul 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		const query = kuery ? toElasticsearchQuery(fromKueryExpression(kuery)) : { match_all: {} };

		while (true) {

Conversation

CAWilson94 commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How To Test

Uh oh!

Uh oh!

elasticmachine commented Jul 7, 2025

Uh oh!

CAWilson94 Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💔 Build Failed

Failed CI Steps

Test Failures

Metrics [docs]

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

machadoum Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

CAWilson94 Jul 14, 2025

Choose a reason for hiding this comment

Uh oh!

CAWilson94 Jul 14, 2025

Choose a reason for hiding this comment

Uh oh!

CAWilson94 Jul 14, 2025

Choose a reason for hiding this comment

Uh oh!

CAWilson94 Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

CAWilson94 Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

CAWilson94 commented Jun 30, 2025 •

edited

Loading

elasticmachine commented Jul 7, 2025 •

edited

Loading