Skip to content

Commit aed6d9b

Browse files
jfredenjfreden
authored
[8.19] Improve security migration resilience by handling version conflicts (#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
1 parent bc15a63 commit aed6d9b

File tree

16 files changed

+288
-23
lines changed

16 files changed

+288
-23
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
@@ -137,6 +137,7 @@ private static IndexVersion def(int id, Version luceneVersion) {
137137
public static final IndexVersion UPGRADE_TO_LUCENE_9_12_2 = def(8_534_0_00, Version.LUCENE_9_12_2);
138138
public static final IndexVersion SPARSE_VECTOR_PRUNING_INDEX_OPTIONS_SUPPORT_BACKPORT_8_X = def(8_535_0_00, Version.LUCENE_9_12_2);
139139
public static final IndexVersion MATCH_ONLY_TEXT_STORED_AS_BYTES_BACKPORT_8_X = def(8_536_0_00, Version.LUCENE_9_12_2);
140+
public static final IndexVersion SECURITY_MIGRATIONS_METADATA_FLATTENED_UPDATE = def(8_537_0_00, Version.LUCENE_9_12_2);
140141

141142
/*
142143
* 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
@@ -236,8 +236,6 @@ public void testMigrationFallbackNamePreCondition() throws Exception {
236236
waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION);
237237
// First migration is on a new index, so should skip all migrations. If we reset, it should re-trigger and run all migrations
238238
resetMigration();
239-
// Wait for the first migration to finish
240-
waitForMigrationCompletion(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION - 1);
241239

242240
// Make sure migration didn't run yet (blocked by the fallback name)
243241
assertMigrationLessThan(SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION);
@@ -307,10 +305,7 @@ public void testNewIndexSkipMigration() {
307305
ensureGreen();
308306
deleteSecurityIndex(); // hack to force a new security index to be created
309307
ensureGreen();
310-
CountDownLatch awaitMigrations = awaitMigrationVersionUpdates(
311-
masterNode,
312-
SecurityMigrations.CLEANUP_ROLE_MAPPING_DUPLICATES_MIGRATION_VERSION
313-
);
308+
CountDownLatch awaitMigrations = awaitMigrationVersionUpdates(masterNode, SecurityMigrations.MIGRATIONS_BY_VERSION.lastKey());
314309
// Create a native role mapping to create security index and trigger migration
315310
createNativeRoleMapping("everyone_kibana_alone");
316311
// Make sure no migration ran (set to current version without applying prior migrations)

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

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
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+
final ExecutorService executor = Executors.newSingleThreadExecutor();
70+
final AtomicBoolean runUpdateRolesBackground = new AtomicBoolean(true);
71+
try {
72+
executor.submit(() -> {
73+
while (runUpdateRolesBackground.get()) {
74+
// Only update half the list so the other half can be verified as migrated
75+
RoleDescriptor roleToUpdate = randomFrom(roles.subList(0, roles.size() / 2));
76+
77+
RoleDescriptor updatedRole = new RoleDescriptor(
78+
roleToUpdate.getName(),
79+
new String[] { "monitor" },
80+
null,
81+
null,
82+
null,
83+
null,
84+
Map.of("test", "value", "timestamp", System.currentTimeMillis(), "random", randomAlphaOfLength(10)),
85+
null
86+
);
87+
nativeRoleStore.putRole(
88+
WriteRequest.RefreshPolicy.IMMEDIATE,
89+
updatedRole,
90+
ActionListener.wrap(resp -> {}, ESTestCase::fail)
91+
);
92+
try {
93+
Thread.sleep(10);
94+
} catch (InterruptedException e) {
95+
throw new RuntimeException(e);
96+
}
97+
}
98+
});
99+
100+
resetMigration();
101+
waitForMigrationCompletion();
102+
} finally {
103+
runUpdateRolesBackground.set(false);
104+
executor.shutdown();
105+
}
106+
assertAllRolesHaveMetadataFlattened();
107+
}
108+
109+
private void resetMigration() {
110+
client().execute(
111+
UpdateIndexMigrationVersionAction.INSTANCE,
112+
new UpdateIndexMigrationVersionAction.Request(
113+
TimeValue.MAX_VALUE,
114+
ROLE_METADATA_FLATTENED_MIGRATION_VERSION - 1,
115+
INTERNAL_SECURITY_MAIN_INDEX_7
116+
)
117+
).actionGet();
118+
}
119+
120+
private List<RoleDescriptor> createRoles() throws IOException {
121+
var roles = randomList(
122+
25,
123+
50,
124+
() -> new RoleDescriptor(
125+
randomAlphaOfLength(20),
126+
null,
127+
null,
128+
null,
129+
null,
130+
null,
131+
Map.of("test", "value", "timestamp", System.currentTimeMillis(), "random", randomAlphaOfLength(10)),
132+
Map.of()
133+
)
134+
);
135+
for (RoleDescriptor role : roles) {
136+
indexRoleDirectly(role);
137+
}
138+
indicesAdmin().prepareRefresh(INTERNAL_SECURITY_MAIN_INDEX_7).get();
139+
return roles;
140+
}
141+
142+
private void indexRoleDirectly(RoleDescriptor role) throws IOException {
143+
XContentBuilder builder = buildRoleDocument(role);
144+
prepareIndex(INTERNAL_SECURITY_MAIN_INDEX_7).setId(ROLE_TYPE + "-" + role.getName())
145+
.setSource(builder)
146+
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
147+
.get();
148+
}
149+
150+
private XContentBuilder buildRoleDocument(RoleDescriptor role) throws IOException {
151+
XContentBuilder builder = jsonBuilder().startObject();
152+
// metadata_flattened is populated by the native role store, so write directly to index to simulate pre-migration state
153+
role.innerToXContent(builder, ToXContent.EMPTY_PARAMS, true);
154+
builder.endObject();
155+
return builder;
156+
}
157+
158+
private int getCurrentMigrationVersion() {
159+
ClusterService clusterService = internalCluster().getInstance(ClusterService.class);
160+
IndexMetadata indexMetadata = clusterService.state().metadata().index(INTERNAL_SECURITY_MAIN_INDEX_7);
161+
if (indexMetadata == null || indexMetadata.getCustomData(MIGRATION_VERSION_CUSTOM_KEY) == null) {
162+
return 0;
163+
}
164+
return Integer.parseInt(indexMetadata.getCustomData(MIGRATION_VERSION_CUSTOM_KEY).get(MIGRATION_VERSION_CUSTOM_DATA_KEY));
165+
}
166+
167+
private void waitForMigrationCompletion() throws Exception {
168+
assertBusy(() -> assertThat(getCurrentMigrationVersion(), greaterThanOrEqualTo(ROLE_METADATA_FLATTENED_MIGRATION_VERSION)));
169+
}
170+
171+
private void assertAllRolesHaveMetadataFlattened() {
172+
SearchRequest searchRequest = new SearchRequest(INTERNAL_SECURITY_MAIN_INDEX_7);
173+
searchRequest.source().query(QueryBuilders.termQuery("type", "role")).size(1000);
174+
SearchResponse response = client().search(searchRequest).actionGet();
175+
for (SearchHit hit : response.getHits().getHits()) {
176+
@SuppressWarnings("unchecked")
177+
Map<String, Object> metadata = (Map<String, Object>) hit.getSourceAsMap().get("metadata_flattened");
178+
// Only check non-reserved roles
179+
if (metadata.get("_reserved") == null) {
180+
assertEquals("value", metadata.get("test"));
181+
}
182+
}
183+
response.decRef();
184+
}
185+
186+
@Override
187+
protected Collection<Class<? extends Plugin>> nodePlugins() {
188+
return Stream.concat(super.nodePlugins().stream(), Stream.of(PainlessPlugin.class)).collect(Collectors.toList());
189+
}
190+
}

‎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
@@ -482,7 +482,7 @@ public class Security extends Plugin
482482

483483
public static final String SECURITY_CRYPTO_THREAD_POOL_NAME = XPackField.SECURITY + "-crypto";
484484

485-
private static final int MAX_SECURITY_MIGRATION_RETRY_COUNT = 10;
485+
private static final int MAX_SECURITY_MIGRATION_RETRY_COUNT = 1000;
486486

487487
// TODO: ip filtering does not actually track license usage yet
488488
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
@@ -44,10 +44,12 @@
4444
import org.elasticsearch.index.IndexVersion;
4545
import org.elasticsearch.indices.IndexClosedException;
4646
import org.elasticsearch.indices.SystemIndexDescriptor;
47+
import org.elasticsearch.persistent.PersistentTasksCustomMetadata;
4748
import org.elasticsearch.rest.RestStatus;
4849
import org.elasticsearch.threadpool.Scheduler;
4950
import org.elasticsearch.xcontent.XContentType;
5051
import org.elasticsearch.xpack.core.security.authz.RoleMappingMetadata;
52+
import org.elasticsearch.xpack.core.security.support.SecurityMigrationTaskParams;
5153
import org.elasticsearch.xpack.security.SecurityFeatures;
5254
import org.elasticsearch.xpack.security.action.rolemapping.ReservedRoleMappingAction;
5355

@@ -341,6 +343,9 @@ public void clusterChanged(ClusterChangedEvent event) {
341343
event.state(),
342344
migrationsVersion
343345
);
346+
var persistentTaskCustomMetadata = PersistentTasksCustomMetadata.getPersistentTasksCustomMetadata(event.state());
347+
final boolean securityMigrationRunning = persistentTaskCustomMetadata != null
348+
&& persistentTaskCustomMetadata.getTask(SecurityMigrationTaskParams.TASK_NAME) != null;
344349
final boolean mappingIsUpToDate = indexMetadata == null || checkIndexMappingUpToDate(event.state());
345350
final SystemIndexDescriptor.MappingsVersion minClusterMappingVersion = getMinSecurityIndexMappingVersion(event.state());
346351
final int indexMappingVersion = loadIndexMappingVersion(systemIndexDescriptor.getAliasName(), event.state());
@@ -370,6 +375,7 @@ public void clusterChanged(ClusterChangedEvent event) {
370375
indexAvailableForWrite,
371376
mappingIsUpToDate,
372377
createdOnLatestVersion,
378+
securityMigrationRunning,
373379
roleMappingsCleanupMigrationStatus,
374380
migrationsVersion,
375381
minClusterMappingVersion,
@@ -774,6 +780,7 @@ public static class State {
774780
false,
775781
false,
776782
false,
783+
false,
777784
null,
778785
null,
779786
null,
@@ -790,6 +797,7 @@ public static class State {
790797
public final boolean indexAvailableForWrite;
791798
public final boolean mappingUpToDate;
792799
public final boolean createdOnLatestVersion;
800+
public final boolean securityMigrationRunning;
793801
public final RoleMappingsCleanupMigrationStatus roleMappingsCleanupMigrationStatus;
794802
public final Integer migrationsVersion;
795803
// Min mapping version supported by the descriptors in the cluster
@@ -809,6 +817,7 @@ public State(
809817
boolean indexAvailableForWrite,
810818
boolean mappingUpToDate,
811819
boolean createdOnLatestVersion,
820+
boolean securityMigrationRunning,
812821
RoleMappingsCleanupMigrationStatus roleMappingsCleanupMigrationStatus,
813822
Integer migrationsVersion,
814823
SystemIndexDescriptor.MappingsVersion minClusterMappingVersion,
@@ -826,6 +835,7 @@ public State(
826835
this.mappingUpToDate = mappingUpToDate;
827836
this.migrationsVersion = migrationsVersion;
828837
this.createdOnLatestVersion = createdOnLatestVersion;
838+
this.securityMigrationRunning = securityMigrationRunning;
829839
this.roleMappingsCleanupMigrationStatus = roleMappingsCleanupMigrationStatus;
830840
this.minClusterMappingVersion = minClusterMappingVersion;
831841
this.indexMappingVersion = indexMappingVersion;
@@ -847,6 +857,7 @@ public boolean equals(Object o) {
847857
&& indexAvailableForWrite == state.indexAvailableForWrite
848858
&& mappingUpToDate == state.mappingUpToDate
849859
&& createdOnLatestVersion == state.createdOnLatestVersion
860+
&& securityMigrationRunning == state.securityMigrationRunning
850861
&& roleMappingsCleanupMigrationStatus == state.roleMappingsCleanupMigrationStatus
851862
&& Objects.equals(indexMappingVersion, state.indexMappingVersion)
852863
&& Objects.equals(migrationsVersion, state.migrationsVersion)
@@ -870,6 +881,7 @@ public int hashCode() {
870881
indexAvailableForWrite,
871882
mappingUpToDate,
872883
createdOnLatestVersion,
884+
securityMigrationRunning,
873885
roleMappingsCleanupMigrationStatus,
874886
migrationsVersion,
875887
minClusterMappingVersion,
@@ -895,6 +907,8 @@ public String toString() {
895907
+ mappingUpToDate
896908
+ ", createdOnLatestVersion="
897909
+ createdOnLatestVersion
910+
+ ", securityMigrationRunning="
911+
+ securityMigrationRunning
898912
+ ", roleMappingsCleanupMigrationStatus="
899913
+ roleMappingsCleanupMigrationStatus
900914
+ ", migrationsVersion="

‎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
@@ -47,7 +47,7 @@ public SecurityMigrationExecutor(
4747
@Override
4848
protected void nodeOperation(AllocatedPersistentTask task, SecurityMigrationTaskParams params, PersistentTaskState state) {
4949
ActionListener<Void> listener = ActionListener.wrap((res) -> task.markAsCompleted(), (exception) -> {
50-
logger.warn("Security migration failed: " + exception);
50+
logger.warn("Security migration failed", exception);
5151
task.markAsFailed(exception);
5252
});
5353

0 commit comments

Comments
 (0)