Skip to content

Commit 72d548b

Browse files
authored
[9.1] Improve security migration resilience by handling version conflicts (#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
1 parent 50f58de commit 72d548b

File tree

16 files changed

+294
-31
lines changed

16 files changed

+294
-31
lines changed

docs/changelog/137558.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 137558
2+
summary: Improve security migration resilience by handling version conflicts
3+
area: Security
4+
type: enhancement
5+
issues: []

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ private static Version parseUnchecked(String version) {
180180
public static final IndexVersion SPARSE_VECTOR_PRUNING_INDEX_OPTIONS_SUPPORT = def(9_031_0_00, Version.LUCENE_10_2_2);
181181
public static final IndexVersion DEFAULT_DENSE_VECTOR_TO_BBQ_HNSW = def(9_032_0_00, Version.LUCENE_10_2_2);
182182
public static final IndexVersion MATCH_ONLY_TEXT_STORED_AS_BYTES = def(9_033_0_00, Version.LUCENE_10_2_2);
183+
public static final IndexVersion SECURITY_MIGRATIONS_METADATA_FLATTENED_UPDATE = def(9_034_0_00, Version.LUCENE_10_2_2);
183184

184185
/*
185186
* STOP! READ THIS FIRST! No, really,

x-pack/plugin/security/build.gradle

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ dependencies {
4242
internalClusterTestImplementation(testArtifact(project(xpackModule('core'))))
4343
api "com.unboundid:unboundid-ldapsdk:${versions.ldapsdk}"
4444

45+
internalClusterTestImplementation project(path: ':modules:lang-painless')
46+
internalClusterTestImplementation project(path: ':modules:lang-painless:spi')
47+
4548
// the following are all SAML dependencies - might as well download the whole internet
4649
api "org.opensaml:opensaml-core:${versions.opensaml}"
4750
api "org.opensaml:opensaml-saml-api:${versions.opensaml}"

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/support/CleanupRoleMappingDuplicatesMigrationIT.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,6 @@ public void testMigrationFallbackNamePreCondition() throws Exception {
241241
waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION);
242242
// First migration is on a new index, so should skip all migrations. If we reset, it should re-trigger and run all migrations
243243
resetMigration();
244-
// Wait for the first migration to finish
245-
waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION - 1);
246244

247245
// Make sure migration didn't run yet (blocked by the fallback name)
248246
assertMigrationLessThan(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION);
@@ -315,10 +313,7 @@ public void testNewIndexSkipMigration() {
315313
ensureGreen();
316314
deleteSecurityIndex(); // hack to force a new security index to be created
317315
ensureGreen();
318-
CountDownLatch awaitMigrations = awaitMigrationVersionUpdates(
319-
masterNode,
320-
SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION
321-
);
316+
CountDownLatch awaitMigrations = awaitMigrationVersionUpdates(masterNode, SecurityMigrations.MIGRATIONS_BY_VERSION.lastKey());
322317
// Create a native role mapping to create security index and trigger migration
323318
createNativeRoleMapping("everyone_kibana_alone");
324319
// Make sure no migration ran (set to current version without applying prior migrations)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
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; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
package org.elasticsearch.xpack.security.support;
9+
10+
import org.elasticsearch.action.ActionListener;
11+
import org.elasticsearch.action.search.SearchRequest;
12+
import org.elasticsearch.action.search.SearchResponse;
13+
import org.elasticsearch.action.support.WriteRequest;
14+
import org.elasticsearch.cluster.metadata.IndexMetadata;
15+
import org.elasticsearch.cluster.service.ClusterService;
16+
import org.elasticsearch.core.TimeValue;
17+
import org.elasticsearch.index.query.QueryBuilders;
18+
import org.elasticsearch.painless.PainlessPlugin;
19+
import org.elasticsearch.plugins.Plugin;
20+
import org.elasticsearch.search.SearchHit;
21+
import org.elasticsearch.test.ESIntegTestCase;
22+
import org.elasticsearch.test.ESTestCase;
23+
import org.elasticsearch.test.SecurityIntegTestCase;
24+
import org.elasticsearch.xcontent.ToXContent;
25+
import org.elasticsearch.xcontent.XContentBuilder;
26+
import org.elasticsearch.xpack.core.security.action.UpdateIndexMigrationVersionAction;
27+
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
28+
import org.elasticsearch.xpack.security.authz.store.NativeRolesStore;
29+
import org.junit.Before;
30+
31+
import java.io.IOException;
32+
import java.util.Collection;
33+
import java.util.List;
34+
import java.util.Map;
35+
import java.util.concurrent.ExecutorService;
36+
import java.util.concurrent.Executors;
37+
import java.util.concurrent.atomic.AtomicBoolean;
38+
import java.util.concurrent.atomic.AtomicLong;
39+
import java.util.stream.Collectors;
40+
import java.util.stream.Stream;
41+
42+
import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
43+
import static org.elasticsearch.xpack.core.security.action.UpdateIndexMigrationVersionAction.MIGRATION_VERSION_CUSTOM_DATA_KEY;
44+
import static org.elasticsearch.xpack.core.security.action.UpdateIndexMigrationVersionAction.MIGRATION_VERSION_CUSTOM_KEY;
45+
import static org.elasticsearch.xpack.core.security.authz.RoleDescriptor.ROLE_TYPE;
46+
import static org.elasticsearch.xpack.core.security.test.TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7;
47+
import static org.elasticsearch.xpack.security.support.SecurityMigrations.ROLE_METADATA_FLATTENED_MIGRATION_VERSION;
48+
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
49+
50+
@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, autoManageMasterNodes = false)
51+
public class MetadataFlattenedMigrationIntegTests extends SecurityIntegTestCase {
52+
53+
private final AtomicLong versionCounter = new AtomicLong(1);
54+
55+
@Before
56+
public void resetVersion() {
57+
versionCounter.set(1);
58+
}
59+
60+
public void testMigrationWithConcurrentUpdates() throws Exception {
61+
internalCluster().setBootstrapMasterNodeIndex(0);
62+
internalCluster().startNode();
63+
ensureGreen();
64+
65+
waitForMigrationCompletion();
66+
var roles = createRoles();
67+
final var nativeRoleStore = internalCluster().getInstance(NativeRolesStore.class);
68+
69+
try (ExecutorService executor = Executors.newSingleThreadExecutor()) {
70+
final AtomicBoolean runUpdateRolesBackground = new AtomicBoolean(true);
71+
executor.submit(() -> {
72+
while (runUpdateRolesBackground.get()) {
73+
// Only update half the list so the other half can be verified as migrated
74+
RoleDescriptor roleToUpdate = randomFrom(roles.subList(0, roles.size() / 2));
75+
76+
RoleDescriptor updatedRole = new RoleDescriptor(
77+
roleToUpdate.getName(),
78+
new String[] { "monitor" },
79+
null,
80+
null,
81+
null,
82+
null,
83+
Map.of("test", "value", "timestamp", System.currentTimeMillis(), "random", randomAlphaOfLength(10)),
84+
null
85+
);
86+
nativeRoleStore.putRole(
87+
WriteRequest.RefreshPolicy.IMMEDIATE,
88+
updatedRole,
89+
ActionListener.wrap(resp -> {}, ESTestCase::fail)
90+
);
91+
try {
92+
Thread.sleep(10);
93+
} catch (InterruptedException e) {
94+
throw new RuntimeException(e);
95+
}
96+
}
97+
});
98+
99+
resetMigration();
100+
try {
101+
waitForMigrationCompletion();
102+
} finally {
103+
runUpdateRolesBackground.set(false);
104+
executor.shutdown();
105+
}
106+
}
107+
assertAllRolesHaveMetadataFlattened();
108+
}
109+
110+
private void resetMigration() {
111+
client().execute(
112+
UpdateIndexMigrationVersionAction.INSTANCE,
113+
new UpdateIndexMigrationVersionAction.Request(
114+
TimeValue.MAX_VALUE,
115+
ROLE_METADATA_FLATTENED_MIGRATION_VERSION - 1,
116+
INTERNAL_SECURITY_MAIN_INDEX_7
117+
)
118+
).actionGet();
119+
}
120+
121+
private List<RoleDescriptor> createRoles() throws IOException {
122+
var roles = randomList(
123+
25,
124+
50,
125+
() -> new RoleDescriptor(
126+
randomAlphaOfLength(20),
127+
null,
128+
null,
129+
null,
130+
null,
131+
null,
132+
Map.of("test", "value", "timestamp", System.currentTimeMillis(), "random", randomAlphaOfLength(10)),
133+
Map.of()
134+
)
135+
);
136+
for (RoleDescriptor role : roles) {
137+
indexRoleDirectly(role);
138+
}
139+
indicesAdmin().prepareRefresh(INTERNAL_SECURITY_MAIN_INDEX_7).get();
140+
return roles;
141+
}
142+
143+
private void indexRoleDirectly(RoleDescriptor role) throws IOException {
144+
XContentBuilder builder = buildRoleDocument(role);
145+
prepareIndex(INTERNAL_SECURITY_MAIN_INDEX_7).setId(ROLE_TYPE + "-" + role.getName())
146+
.setSource(builder)
147+
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
148+
.get();
149+
}
150+
151+
private XContentBuilder buildRoleDocument(RoleDescriptor role) throws IOException {
152+
XContentBuilder builder = jsonBuilder().startObject();
153+
// metadata_flattened is populated by the native role store, so write directly to index to simulate pre-migration state
154+
role.innerToXContent(builder, ToXContent.EMPTY_PARAMS, true);
155+
builder.endObject();
156+
return builder;
157+
}
158+
159+
private int getCurrentMigrationVersion() {
160+
ClusterService clusterService = internalCluster().getInstance(ClusterService.class);
161+
IndexMetadata indexMetadata = clusterService.state().metadata().getProject().index(INTERNAL_SECURITY_MAIN_INDEX_7);
162+
if (indexMetadata == null || indexMetadata.getCustomData(MIGRATION_VERSION_CUSTOM_KEY) == null) {
163+
return 0;
164+
}
165+
return Integer.parseInt(indexMetadata.getCustomData(MIGRATION_VERSION_CUSTOM_KEY).get(MIGRATION_VERSION_CUSTOM_DATA_KEY));
166+
}
167+
168+
private void waitForMigrationCompletion() throws Exception {
169+
assertBusy(() -> assertThat(getCurrentMigrationVersion(), greaterThanOrEqualTo(ROLE_METADATA_FLATTENED_MIGRATION_VERSION)));
170+
}
171+
172+
private void assertAllRolesHaveMetadataFlattened() {
173+
SearchRequest searchRequest = new SearchRequest(INTERNAL_SECURITY_MAIN_INDEX_7);
174+
searchRequest.source().query(QueryBuilders.termQuery("type", "role")).size(1000);
175+
SearchResponse response = client().search(searchRequest).actionGet();
176+
for (SearchHit hit : response.getHits().getHits()) {
177+
@SuppressWarnings("unchecked")
178+
Map<String, Object> metadata = (Map<String, Object>) hit.getSourceAsMap().get("metadata_flattened");
179+
// Only check non-reserved roles
180+
if (metadata.get("_reserved") == null) {
181+
assertEquals("value", metadata.get("test"));
182+
}
183+
}
184+
response.decRef();
185+
}
186+
187+
@Override
188+
protected Collection<Class<? extends Plugin>> nodePlugins() {
189+
return Stream.concat(super.nodePlugins().stream(), Stream.of(PainlessPlugin.class)).collect(Collectors.toList());
190+
}
191+
}

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ public class Security extends Plugin
488488

489489
public static final String SECURITY_CRYPTO_THREAD_POOL_NAME = XPackField.SECURITY + "-crypto";
490490

491-
private static final int MAX_SECURITY_MIGRATION_RETRY_COUNT = 10;
491+
private static final int MAX_SECURITY_MIGRATION_RETRY_COUNT = 1000;
492492

493493
// TODO: ip filtering does not actually track license usage yet
494494
public static final LicensedFeature.Momentary IP_FILTERING_FEATURE = LicensedFeature.momentaryLenient(

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,12 @@
5050
import org.elasticsearch.index.IndexVersion;
5151
import org.elasticsearch.indices.IndexClosedException;
5252
import org.elasticsearch.indices.SystemIndexDescriptor;
53+
import org.elasticsearch.persistent.PersistentTasksCustomMetadata;
5354
import org.elasticsearch.rest.RestStatus;
5455
import org.elasticsearch.threadpool.Scheduler;
5556
import org.elasticsearch.xcontent.XContentType;
5657
import org.elasticsearch.xpack.core.security.authz.RoleMappingMetadata;
58+
import org.elasticsearch.xpack.core.security.support.SecurityMigrationTaskParams;
5759
import org.elasticsearch.xpack.security.SecurityFeatures;
5860
import org.elasticsearch.xpack.security.action.rolemapping.ReservedRoleMappingAction;
5961

@@ -158,6 +160,7 @@ private IndexState unavailableState(ProjectId projectId, ProjectStatus status) {
158160
false,
159161
false,
160162
null,
163+
false,
161164
null,
162165
null,
163166
null,
@@ -180,6 +183,7 @@ public class IndexState {
180183
public final boolean mappingUpToDate;
181184
public final boolean createdOnLatestVersion;
182185
public final RoleMappingsCleanupMigrationStatus roleMappingsCleanupMigrationStatus;
186+
public final boolean securityMigrationRunning;
183187
public final Integer migrationsVersion;
184188
// Min mapping version supported by the descriptors in the cluster
185189
public final SystemIndexDescriptor.MappingsVersion minClusterMappingVersion;
@@ -201,6 +205,7 @@ public IndexState(
201205
boolean mappingUpToDate,
202206
boolean createdOnLatestVersion,
203207
RoleMappingsCleanupMigrationStatus roleMappingsCleanupMigrationStatus,
208+
boolean securityMigrationRunning,
204209
Integer migrationsVersion,
205210
SystemIndexDescriptor.MappingsVersion minClusterMappingVersion,
206211
Integer indexMappingVersion,
@@ -220,6 +225,7 @@ public IndexState(
220225
this.migrationsVersion = migrationsVersion;
221226
this.createdOnLatestVersion = createdOnLatestVersion;
222227
this.roleMappingsCleanupMigrationStatus = roleMappingsCleanupMigrationStatus;
228+
this.securityMigrationRunning = securityMigrationRunning;
223229
this.minClusterMappingVersion = minClusterMappingVersion;
224230
this.indexMappingVersion = indexMappingVersion;
225231
this.concreteIndexName = concreteIndexName;
@@ -247,6 +253,7 @@ public boolean equals(Object o) {
247253
&& mappingUpToDate == other.mappingUpToDate
248254
&& createdOnLatestVersion == other.createdOnLatestVersion
249255
&& roleMappingsCleanupMigrationStatus == other.roleMappingsCleanupMigrationStatus
256+
&& securityMigrationRunning == other.securityMigrationRunning
250257
&& Objects.equals(indexMappingVersion, other.indexMappingVersion)
251258
&& Objects.equals(migrationsVersion, other.migrationsVersion)
252259
&& Objects.equals(minClusterMappingVersion, other.minClusterMappingVersion)
@@ -268,6 +275,7 @@ public int hashCode() {
268275
mappingUpToDate,
269276
createdOnLatestVersion,
270277
roleMappingsCleanupMigrationStatus,
278+
securityMigrationRunning,
271279
migrationsVersion,
272280
minClusterMappingVersion,
273281
indexMappingVersion,
@@ -370,6 +378,8 @@ public String toString() {
370378
+ createdOnLatestVersion
371379
+ ", roleMappingsCleanupMigrationStatus="
372380
+ roleMappingsCleanupMigrationStatus
381+
+ ", securityMigrationRunning="
382+
+ securityMigrationRunning
373383
+ ", migrationsVersion="
374384
+ migrationsVersion
375385
+ ", minClusterMappingVersion="
@@ -820,6 +830,9 @@ private IndexState updateProjectState(ProjectState project) {
820830
project,
821831
migrationsVersion
822832
);
833+
var persistentTaskCustomMetadata = PersistentTasksCustomMetadata.get(project.metadata());
834+
final boolean securityMigrationRunning = persistentTaskCustomMetadata != null
835+
&& persistentTaskCustomMetadata.getTask(SecurityMigrationTaskParams.TASK_NAME) != null;
823836
final boolean mappingIsUpToDate = indexMetadata == null || checkIndexMappingUpToDate(project);
824837
final SystemIndexDescriptor.MappingsVersion minClusterMappingVersion = getMinSecurityIndexMappingVersion(project);
825838
final int indexMappingVersion = loadIndexMappingVersion(systemIndexDescriptor.getAliasName(), project.metadata());
@@ -852,6 +865,7 @@ private IndexState updateProjectState(ProjectState project) {
852865
mappingIsUpToDate,
853866
createdOnLatestVersion,
854867
roleMappingsCleanupMigrationStatus,
868+
securityMigrationRunning,
855869
migrationsVersion,
856870
minClusterMappingVersion,
857871
indexMappingVersion,

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityMigrationExecutor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public SecurityMigrationExecutor(
5454
@Override
5555
protected void nodeOperation(AllocatedPersistentTask task, SecurityMigrationTaskParams params, PersistentTaskState state) {
5656
ActionListener<Void> listener = ActionListener.wrap((res) -> task.markAsCompleted(), (exception) -> {
57-
logger.warn("Security migration failed: " + exception);
57+
logger.warn("Security migration failed", exception);
5858
task.markAsFailed(exception);
5959
});
6060

0 commit comments

Comments
 (0)