Improve security migration resilience by handling version conflicts #137558

jfreden · 2025-11-04T09:58:31Z

This PR adds resilience to the metadata_flattened security migration that was reported to have failed on clusters where concurrent role modifications happened while the migration was running. In the normal case this is not expected to happen, but for a very large number of roles or very frequent role updates a version conflict could occur.

The change adds logic to:

Handle version conflicts
Handle shard failures
Handle timeouts
Trigger immediate retries in the framework if a failure occurs
Bump the number of retries

Resolves: #110532

jfreden · 2025-11-04T12:39:04Z

...t/java/org/elasticsearch/xpack/security/support/CleanupRoleMappingDuplicatesMigrationIT.java

        waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION);
        // First migration is on a new index, so should skip all migrations. If we reset, it should re-trigger and run all migrations
        resetMigration();
-        // Wait for the first migration to finish


This is now the first migration so we don't need this line anymore.

jfreden · 2025-11-04T12:39:38Z

...t/java/org/elasticsearch/xpack/security/support/CleanupRoleMappingDuplicatesMigrationIT.java

-            masterNode,
-            SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION
-        );
+        CountDownLatch awaitMigrations = awaitMigrationVersionUpdates(masterNode, SecurityMigrations.MIGRATIONS_BY_VERSION.lastKey());


Order of migrations changed, this should have been lastKey from the start.

jfreden · 2025-11-04T12:41:43Z

...in/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java

            project,
            migrationsVersion
        );
+        var persistentTaskCustomMetadata = PersistentTasksCustomMetadata.get(project.metadata());


When a migration is running, its persistent task will be present in cluster state, when it's not it will not be present in cluster state. When a persistent task completes (failure or success) it's removed from cluster state. We want to make sure that an index state change is triggered when a persistent task fails to make sure it's retried immediately, that's why we need this state here.

jfreden · 2025-11-04T12:43:04Z

...ugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityMigrations.java


-    public static final Integer ROLE_METADATA_FLATTENED_MIGRATION_VERSION = 1;
    public static final Integer CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION = 2;
+    public static final Integer ROLE_METADATA_FLATTENED_MIGRATION_VERSION = 3;


I'm bumping the version here to make sure this migration runs again with proper error handling, I'm also "removing" the old migration since we don't need it anymore.

jfreden · 2025-11-04T12:44:01Z

...ugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityMigrations.java

-                if (response.getHits().getTotalHits().value() > 0) {
-                    logger.info("Preparing to migrate [" + response.getHits().getTotalHits().value() + "] roles");
-                    updateRolesByQuery(indexManager, client, filterQuery, listener);
+                if (response.isTimedOut() == false && response.getFailedShards() == 0) {


Added error handling here to make sure we don't mark as migrated if this initial search fails silently for some reason.

jfreden · 2025-11-04T15:13:20Z

server/src/main/java/org/elasticsearch/index/IndexVersions.java


    public static final IndexVersion REENABLED_TIMESTAMP_DOC_VALUES_SPARSE_INDEX = def(9_042_0_00, Version.LUCENE_10_3_1);
    public static final IndexVersion SKIPPERS_ENABLED_BY_DEFAULT = def(9_043_0_00, Version.LUCENE_10_3_1);
+    public static final IndexVersion SECURITY_MIGRATIONS_METADATA = def(9_044_0_00, Version.LUCENE_10_3_1);


New index version is needed to make sure we skip migration for brand new index.

jfreden · 2025-11-04T15:23:25Z

...ugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityMigrations.java

    public static class Manager {

-        private static final int MAX_SECURITY_MIGRATION_ATTEMPT_COUNT = 10;
+        private static final int MAX_SECURITY_MIGRATION_ATTEMPT_COUNT = 1000;


This is a pretty significant bump because we have no idea how many times the migration would need to be retried before it's successful. In the extreme case where we have 2M roles and frequent updates 1000 doesn't feel like a crazy number, but it's also very difficult to verify this.

There is no good reason to not allow this to be very large. The point of this is to make sure that security migrations are not retried forever.

elasticsearchmachine · 2025-11-04T15:28:09Z

Pinging @elastic/es-security (Team:Security)

jfreden · 2025-11-14T12:36:01Z

Good thing the tests caught this. From the mapping of the metadata setting the dynamic=false attribute on the mapping without specifying any properties of the object will result in that field not being indexed for search, but still present in the source. IIRC this was one of the issues we ran into when trying to fix this with other approaches.

That means we can't use must(QueryBuilders.existsQuery("metadata")) because it will never exist since it's not indexed for search.

Unfortunately I don't think there is a good way around this, but it's probably acceptable to just reprocess all docs with empty metadata for every retry. WDYT @richard-dennehy ?

…a_flattened

richard-dennehy · 2025-11-14T12:58:47Z

That's unfortunate, but maybe if we keep an eye on the security QA project as this rolls out, we won't be caught totally unaware of any performance impact?

jfreden · 2025-11-14T13:12:59Z

Yes, it would be part of upgrading the security cluster, which would happen in qa before prod so if there are any problems it would be caught there. Since that cluster has metadata for the roles, this logic should work for that cluster.

We can't really keep an eye on it though since we don't control that process (it's not a serverless project).

richard-dennehy · 2025-11-14T13:20:08Z

Would it be more accurate to say that because we have the security QA project with 3 million roles, someone inside Elastic will shout at us if the new migration accidentally unleashes demons, before this gets out to customers?

jfreden · 2025-11-14T13:31:09Z

Yes, but not sure if we should rely on that. I'll think about that a little. Might be good to check with the control-plane team.

…a_flattened

elasticsearchmachine · 2025-11-24T08:54:37Z

💔 Backport failed

Status	Branch	Result
❌	9.2	Commit could not be cherrypicked due to conflicts
❌	8.19	Commit could not be cherrypicked due to conflicts
❌	9.1	Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 137558

…lastic#137558) * Improve security migration resilience by handling version conflicts (cherry picked from commit c779717) # Conflicts: # server/src/main/java/org/elasticsearch/index/IndexVersions.java

…lastic#137558) * Improve security migration resilience by handling version conflicts (cherry picked from commit c779717) # Conflicts: # server/src/main/java/org/elasticsearch/index/IndexVersions.java # x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityMigrations.java

jfreden · 2025-11-24T09:25:15Z

💚 All backports created successfully

Status	Branch	Result
✅	9.2
✅	9.1
✅	8.19

Questions ?

Please refer to the Backport tool documentation

…lastic#137558) * Improve security migration resilience by handling version conflicts (cherry picked from commit c779717) # Conflicts: # server/src/main/java/org/elasticsearch/index/IndexVersions.java # x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java # x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityMigrations.java # x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java # x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmTests.java # x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStoreTests.java # x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java # x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/NativePrivilegeStoreTests.java # x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/CacheInvalidatorRegistryTests.java

…lastic#137558) * Improve security migration resilience by handling version conflicts

…icts (#137558) (#138476) * Improve security migration resilience by handling version conflicts (#137558) * Improve security migration resilience by handling version conflicts (cherry picked from commit c779717) # Conflicts: # server/src/main/java/org/elasticsearch/index/IndexVersions.java # x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityMigrations.java * fixup! Merge issue * fixup! Typo * fixup! BWC test

…icts (#137558) (#138475) * Improve security migration resilience by handling version conflicts (#137558) * Improve security migration resilience by handling version conflicts (cherry picked from commit c779717) # Conflicts: # server/src/main/java/org/elasticsearch/index/IndexVersions.java * fixup! Index version * fixup! Typo * fixup! BWC test

…licts (#137558) (#138477) * Improve security migration resilience by handling version conflicts (#137558) * Improve security migration resilience by handling version conflicts (cherry picked from commit c779717) # Conflicts: # server/src/main/java/org/elasticsearch/index/IndexVersions.java # x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java # x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityMigrations.java # x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java # x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/NativeRealmTests.java # x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/support/mapper/NativeRoleMappingStoreTests.java # x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java # x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/NativePrivilegeStoreTests.java # x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/CacheInvalidatorRegistryTests.java * fixup! Merge issue * fixup! Backported interface different * fixup! Backport interface * fixup! Backport... * fixup! BWC test

jfreden added the test-full-bwc Trigger full BWC version matrix tests label Nov 4, 2025

elasticsearchmachine added the v9.3.0 label Nov 4, 2025

jfreden commented Nov 4, 2025

View reviewed changes

jfreden force-pushed the add_cleanup_metadata_flattened branch from 436f987 to 37737b7 Compare November 4, 2025 15:12

jfreden commented Nov 4, 2025

View reviewed changes

jfreden force-pushed the add_cleanup_metadata_flattened branch from 37737b7 to 682da2d Compare November 4, 2025 15:14

jfreden commented Nov 4, 2025

View reviewed changes

jfreden force-pushed the add_cleanup_metadata_flattened branch from 682da2d to e5f599a Compare November 4, 2025 15:25

Improve security migration resilience by handling version conflicts

77ac083

jfreden force-pushed the add_cleanup_metadata_flattened branch from e5f599a to 77ac083 Compare November 4, 2025 15:25

jfreden added the :Security/Security Security issues without another label label Nov 4, 2025

jfreden marked this pull request as ready for review November 4, 2025 15:27

elasticsearchmachine added the Team:Security Meta label for security team label Nov 4, 2025

jfreden added branch:9.2 branch:9.1 branch:8.19 and removed Team:Security Meta label for security team labels Nov 4, 2025

jfreden requested a review from a team November 4, 2025 15:28

elasticsearchmachine added Team:Security Meta label for security team v9.2.1 v8.19.7 v9.1.7 and removed branch:9.2 branch:9.1 branch:8.19 labels Nov 4, 2025

Merge remote-tracking branch 'upstream/main' into add_cleanup_metadat…

4256889

…a_flattened

jfreden and others added 4 commits November 17, 2025 16:02

Only write if update needed to reduce risk of conflict

cf0b6c3

Merge remote-tracking branch 'upstream/main' into add_cleanup_metadat…

673fb23

…a_flattened

[CI] Auto commit changes from spotless

eb52cb8

Merge branch 'main' into add_cleanup_metadata_flattened

ab19799

ankit--sethi approved these changes Nov 17, 2025

View reviewed changes

jfreden and others added 4 commits November 18, 2025 08:07

Merge branch 'main' into add_cleanup_metadata_flattened

6ab32e9

Merge branch 'main' into add_cleanup_metadata_flattened

51c1ca4

Merge remote-tracking branch 'upstream/main' into add_cleanup_metadat…

c8a8e43

…a_flattened

fixup! Lucene version

2dc70af

jfreden added the auto-backport Automatically create backport pull requests when merged label Nov 24, 2025

jfreden merged commit c779717 into elastic:main Nov 24, 2025
35 checks passed

elasticsearchmachine added the backport pending label Nov 24, 2025

jfreden mentioned this pull request Nov 24, 2025

[9.2] Improve security migration resilience by handling version conflicts (#137558) #138475

Merged

jfreden mentioned this pull request Nov 24, 2025

[9.1] Improve security migration resilience by handling version conflicts (#137558) #138476

Merged

jfreden mentioned this pull request Nov 24, 2025

[8.19] Improve security migration resilience by handling version conflicts (#137558) #138477

Merged

ncordon pushed a commit to ncordon/elasticsearch that referenced this pull request Nov 26, 2025

Improve security migration resilience by handling version conflicts (e…

468ff1a

…lastic#137558) * Improve security migration resilience by handling version conflicts

Improve security migration resilience by handling version conflicts #137558

Improve security migration resilience by handling version conflicts #137558

Uh oh!

Conversation

jfreden commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jfreden Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

jfreden Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

jfreden Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

jfreden Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

jfreden Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

jfreden Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

jfreden Nov 4, 2025

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Nov 4, 2025

Uh oh!

jfreden commented Nov 14, 2025

Uh oh!

richard-dennehy commented Nov 14, 2025

Uh oh!

jfreden commented Nov 14, 2025

Uh oh!

richard-dennehy commented Nov 14, 2025

Uh oh!

jfreden commented Nov 14, 2025

Uh oh!

Uh oh!

elasticsearchmachine commented Nov 24, 2025

💔 Backport failed

Uh oh!

jfreden commented Nov 24, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jfreden commented Nov 4, 2025 •

edited

Loading