Skip to content

Commit ecb1f47

Browse files
slobodanadamovicslobodanadamovic
committed
Fix internal cluster and single node security tests (elastic#121466)
This PR fixes SecuritySingleNodeTestCase and ProfileIntegTests tests. - The security single node test failures are solved by ensuring every test starts with security index created and available. This is in order to have consistent state for every test. With the changes introduce in the elastic#120323 PR, only the first test would execute with .security index being created async. Subsequent tests would execute without security index creation due to the fact that whole cluster is wiped after each test. This caused a flakiness only for the first test, because there was no mechanism in place to ensure that the .security index is active before test execution. - The profile integration tests are solved by introducing an anonymous role which don't have application privileges. The application privileges are resolved from the .security index and assigned to all users, including the es_test_root user which is used during cluster wiping. Due to asynchronous nature of cluster setup and .security index creation, this now causes flakiness. The main problem is that wiping is done asynchronously and uses es_test_root which had assigned anonymous rac_role which depends on .security index being available for search in order to resolve application privileges. The application privilege resolution is done in buildRoleFromDescriptors which currently does not wait for security index availability(can be improved - but still wouldn't fix internal cluster tests). This wasn't a problem before just because we simply return empty results when .security index does not exist. There is some complexity in making internal clusters wait for availability of security shards before the test, so I think this solution is acceptable given that it's not required for this tests to have anonymous role with application privileges. Resolves elastic#121022 Resolves elastic#121096 Resolves elastic#121101 Resolves elastic#120988 Resolves elastic#121108 Resolves elastic#120983 Resolves elastic#120987 Resolves elastic#121179 Resolves elastic#121183 Resolves elastic#121346 Resolves elastic#121151 Resolves elastic#120985 Resolves elastic#121039 Resolves elastic#121483 Resolves elastic#121116 Resolves elastic#121258 Resolves elastic#121486 (cherry picked from commit 369c641) # Conflicts: # muted-tests.yml # x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmElasticAutoconfigIntegTests.java
1 parent eed3950 commit ecb1f47

File tree

4 files changed

+74
-101
lines changed

4 files changed

+74
-101
lines changed

‎muted-tests.yml‎

Lines changed: 1 addition & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -395,44 +395,20 @@ tests:
395395
issue: https://github.com/elastic/elasticsearch/issues/120973
396396
- class: org.elasticsearch.packaging.test.DockerTests
397397
issue: https://github.com/elastic/elasticsearch/issues/120978
398-
- class: org.elasticsearch.xpack.security.authc.jwt.JwtRealmSingleNodeTests
399-
method: testActivateProfileForJWT
400-
issue: https://github.com/elastic/elasticsearch/issues/120983
401-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
402-
method: testProfileIndexAutoCreation
403-
issue: https://github.com/elastic/elasticsearch/issues/120987
404398
- class: org.elasticsearch.xpack.security.authc.service.ServiceAccountIT
405399
method: testAuthenticateShouldNotFallThroughInCaseOfFailure
406400
issue: https://github.com/elastic/elasticsearch/issues/120902
407401
- class: org.elasticsearch.xpack.security.FileSettingsRoleMappingsRestartIT
408402
method: testReservedStatePersistsOnRestart
409403
issue: https://github.com/elastic/elasticsearch/issues/120923
410-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
411-
method: testSuggestProfilesWithHint
412-
issue: https://github.com/elastic/elasticsearch/issues/121116
413404
- class: org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT
414405
method: test {p0=synonyms/110_synonyms_invalid/Reload index with an invalid synonym rule with lenient set to false}
415406
issue: https://github.com/elastic/elasticsearch/issues/121117
416-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
417-
method: testActivateProfile
418-
issue: https://github.com/elastic/elasticsearch/issues/121151
419407
- class: org.elasticsearch.xpack.ml.integration.PyTorchModelIT
420408
issue: https://github.com/elastic/elasticsearch/issues/121165
421-
- class: org.elasticsearch.xpack.security.authc.jwt.JwtRealmSingleNodeTests
422-
method: testClientSecretRotation
423-
issue: https://github.com/elastic/elasticsearch/issues/120985
424409
- class: org.elasticsearch.xpack.transform.integration.TransformAuditorIT
425410
method: testAuditorWritesAudits
426411
issue: https://github.com/elastic/elasticsearch/issues/121241
427-
- class: org.elasticsearch.xpack.security.authc.jwt.JwtRealmSingleNodeTests
428-
method: testGrantApiKeyForJWT
429-
issue: https://github.com/elastic/elasticsearch/issues/121039
430-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
431-
method: testSuggestProfileWithData
432-
issue: https://github.com/elastic/elasticsearch/issues/121258
433-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
434-
method: testSuggestProfilesWithName
435-
issue: https://github.com/elastic/elasticsearch/issues/121022
436412
- class: org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT
437413
method: test {p0=nodes.stats/11_indices_metrics/indices mappings exact count test for indices level}
438414
issue: https://github.com/elastic/elasticsearch/issues/120950
@@ -445,9 +421,6 @@ tests:
445421
- class: org.elasticsearch.upgrades.VectorSearchIT
446422
method: testBBQVectorSearch {upgradedNodes=0}
447423
issue: https://github.com/elastic/elasticsearch/issues/121253
448-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
449-
method: testSetEnabled
450-
issue: https://github.com/elastic/elasticsearch/issues/121183
451424
- class: org.elasticsearch.upgrades.VectorSearchIT
452425
method: testBBQVectorSearch {upgradedNodes=1}
453426
issue: https://github.com/elastic/elasticsearch/issues/121271
@@ -457,20 +430,11 @@ tests:
457430
- class: org.elasticsearch.xpack.sql.qa.single_node.JdbcDocCsvSpecIT
458431
method: test {docs.testFilterToday}
459432
issue: https://github.com/elastic/elasticsearch/issues/121474
460-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
461-
method: testGetProfiles
462-
issue: https://github.com/elastic/elasticsearch/issues/121101
463-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
464-
method: testGetUsersWithProfileUidWhenProfileIndexDoesNotExists
465-
issue: https://github.com/elastic/elasticsearch/issues/121179
466433
- class: org.elasticsearch.xpack.application.CohereServiceUpgradeIT
467434
issue: https://github.com/elastic/elasticsearch/issues/121537
468435
- class: org.elasticsearch.smoketest.DocsClientYamlTestSuiteIT
469436
method: test {yaml=reference/snapshot-restore/apis/get-snapshot-api/line_488}
470437
issue: https://github.com/elastic/elasticsearch/issues/121611
471-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
472-
method: testUpdateProfileData
473-
issue: https://github.com/elastic/elasticsearch/issues/121108
474438
- class: org.elasticsearch.smoketest.DocsClientYamlTestSuiteIT
475439
method: test {yaml=reference/rest-api/common-options/line_102}
476440
issue: https://github.com/elastic/elasticsearch/issues/121748
@@ -480,12 +444,6 @@ tests:
480444
- class: org.elasticsearch.xpack.test.rest.XPackRestIT
481445
method: test {p0=ml/*}
482446
issue: https://github.com/elastic/elasticsearch/issues/120816
483-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
484-
method: testProfileAPIsWhenIndexNotCreated
485-
issue: https://github.com/elastic/elasticsearch/issues/121096
486-
- class: org.elasticsearch.xpack.security.authc.service.ServiceAccountSingleNodeTests
487-
method: testAuthenticateWithServiceFileToken
488-
issue: https://github.com/elastic/elasticsearch/issues/120988
489447
- class: org.elasticsearch.smoketest.DocsClientYamlTestSuiteIT
490448
method: test {yaml=reference/cat/allocation/cat-allocation-example}
491449
issue: https://github.com/elastic/elasticsearch/issues/121976
@@ -496,12 +454,6 @@ tests:
496454
issue: https://github.com/elastic/elasticsearch/issues/121291
497455
- class: org.elasticsearch.xpack.esql.qa.multi_node.EsqlSpecIT
498456
issue: https://github.com/elastic/elasticsearch/issues/121411
499-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
500-
method: testHasPrivileges
501-
issue: https://github.com/elastic/elasticsearch/issues/121346
502-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
503-
method: testGetUsersWithProfileUid
504-
issue: https://github.com/elastic/elasticsearch/issues/121483
505457
- class: org.elasticsearch.xpack.application.FullClusterRestartIT
506458
issue: https://github.com/elastic/elasticsearch/issues/121935
507459
- class: org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT
@@ -518,4 +470,4 @@ tests:
518470
issue: https://github.com/elastic/elasticsearch/issues/122683
519471
- class: org.elasticsearch.xpack.esql.action.CrossClusterAsyncQueryStopIT
520472
method: testStopQueryLocal
521-
issue: https://github.com/elastic/elasticsearch/issues/121672
473+
issue: https://github.com/elastic/elasticsearch/issues/121672

‎x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/test/SecuritySingleNodeTestCase.java‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,16 @@
77
package org.elasticsearch.test;
88

99
import org.apache.http.HttpHost;
10+
import org.elasticsearch.ResourceAlreadyExistsException;
1011
import org.elasticsearch.action.admin.cluster.node.info.NodeInfo;
1112
import org.elasticsearch.action.admin.cluster.node.info.NodesInfoResponse;
1213
import org.elasticsearch.action.admin.cluster.node.info.PluginsAndModules;
14+
import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
15+
import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest;
16+
import org.elasticsearch.action.admin.indices.get.GetIndexRequest;
17+
import org.elasticsearch.action.admin.indices.get.GetIndexResponse;
18+
import org.elasticsearch.action.support.ActiveShardCount;
19+
import org.elasticsearch.action.support.IndicesOptions;
1320
import org.elasticsearch.client.RequestOptions;
1421
import org.elasticsearch.client.RestClient;
1522
import org.elasticsearch.client.RestClientBuilder;
@@ -27,6 +34,7 @@
2734
import org.elasticsearch.license.LicenseSettings;
2835
import org.elasticsearch.plugins.Plugin;
2936
import org.elasticsearch.xpack.core.security.authc.support.Hasher;
37+
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
3038
import org.elasticsearch.xpack.core.security.test.TestRestrictedIndices;
3139
import org.elasticsearch.xpack.security.LocalStateSecurity;
3240
import org.elasticsearch.xpack.security.support.SecurityMigrations;
@@ -45,9 +53,12 @@
4553
import java.util.stream.Collectors;
4654

4755
import static org.elasticsearch.test.SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING;
56+
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
4857
import static org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken.basicAuthHeaderValue;
4958
import static org.elasticsearch.xpack.security.support.SecurityIndexManager.getMigrationVersionFromIndexMetadata;
59+
import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_MAIN_ALIAS;
5060
import static org.hamcrest.Matchers.hasItem;
61+
import static org.hamcrest.Matchers.is;
5162

5263
/**
5364
* A test that starts a single node with security enabled. This test case allows for customization
@@ -82,6 +93,13 @@ public static void destroyDefaultSettings() {
8293
tearDownRestClient();
8394
}
8495

96+
@Override
97+
public void setUp() throws Exception {
98+
super.setUp();
99+
deleteSecurityIndexIfExists();
100+
createSecurityIndexWithWaitForActiveShards();
101+
}
102+
85103
@Override
86104
public void tearDown() throws Exception {
87105
awaitSecurityMigration();
@@ -100,7 +118,7 @@ private boolean isMigrationComplete(ClusterState state) {
100118
return getMigrationVersionFromIndexMetadata(indexMetadata) == SecurityMigrations.MIGRATIONS_BY_VERSION.lastKey();
101119
}
102120

103-
private void awaitSecurityMigration() {
121+
protected void awaitSecurityMigration() {
104122
final var latch = new CountDownLatch(1);
105123
ClusterService clusterService = getInstanceFromNode(ClusterService.class);
106124
clusterService.addListener((event) -> {
@@ -362,4 +380,40 @@ private static RestClient createRestClient(
362380
}
363381
return builder.build();
364382
}
383+
384+
protected void deleteSecurityIndexIfExists() {
385+
// delete the security index, if it exist
386+
GetIndexRequest getIndexRequest = new GetIndexRequest(TEST_REQUEST_TIMEOUT);
387+
getIndexRequest.indices(SECURITY_MAIN_ALIAS);
388+
getIndexRequest.indicesOptions(IndicesOptions.lenientExpandOpen());
389+
GetIndexResponse getIndexResponse = client().admin().indices().getIndex(getIndexRequest).actionGet();
390+
if (getIndexResponse.getIndices().length > 0) {
391+
assertThat(getIndexResponse.getIndices().length, is(1));
392+
assertThat(getIndexResponse.getIndices()[0], is(TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7));
393+
394+
// Security migration needs to finish before deleting the index
395+
awaitSecurityMigration();
396+
DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest(getIndexResponse.getIndices());
397+
assertAcked(client().admin().indices().delete(deleteIndexRequest).actionGet());
398+
}
399+
}
400+
401+
protected void createSecurityIndexWithWaitForActiveShards() {
402+
final Client client = client().filterWithHeader(
403+
Collections.singletonMap(
404+
"Authorization",
405+
UsernamePasswordToken.basicAuthHeaderValue(
406+
SecuritySettingsSource.ES_TEST_ROOT_USER,
407+
SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING
408+
)
409+
)
410+
);
411+
CreateIndexRequest createIndexRequest = new CreateIndexRequest(SECURITY_MAIN_ALIAS).waitForActiveShards(ActiveShardCount.ALL)
412+
.masterNodeTimeout(TEST_REQUEST_TIMEOUT);
413+
try {
414+
client.admin().indices().create(createIndexRequest).actionGet();
415+
} catch (ResourceAlreadyExistsException e) {
416+
logger.info("Security index already exists, ignoring.", e);
417+
}
418+
}
365419
}

‎x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/esnative/ReservedRealmElasticAutoconfigIntegTests.java‎

Lines changed: 2 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,10 @@
88
package org.elasticsearch.xpack.security.authc.esnative;
99

1010
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
11-
import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest;
12-
import org.elasticsearch.action.admin.indices.get.GetIndexRequest;
13-
import org.elasticsearch.action.admin.indices.get.GetIndexResponse;
14-
import org.elasticsearch.action.support.IndicesOptions;
1511
import org.elasticsearch.client.Request;
1612
import org.elasticsearch.client.RequestOptions;
1713
import org.elasticsearch.client.ResponseException;
18-
import org.elasticsearch.cluster.ClusterState;
19-
import org.elasticsearch.cluster.metadata.IndexMetadata;
2014
import org.elasticsearch.cluster.metadata.Metadata;
21-
import org.elasticsearch.cluster.service.ClusterService;
2215
import org.elasticsearch.common.Strings;
2316
import org.elasticsearch.common.settings.MockSecureSettings;
2417
import org.elasticsearch.common.settings.SecureString;
@@ -29,14 +22,9 @@
2922
import org.elasticsearch.xpack.core.security.action.user.PutUserRequest;
3023
import org.elasticsearch.xpack.core.security.authc.support.Hasher;
3124
import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
32-
import org.elasticsearch.xpack.core.security.test.TestRestrictedIndices;
3325
import org.junit.BeforeClass;
3426

35-
import java.util.concurrent.CountDownLatch;
36-
3727
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
38-
import static org.elasticsearch.xpack.core.security.action.UpdateIndexMigrationVersionAction.MIGRATION_VERSION_CUSTOM_KEY;
39-
import static org.elasticsearch.xpack.security.support.SecuritySystemIndices.SECURITY_MAIN_ALIAS;
4028
import static org.hamcrest.Matchers.is;
4129

4230
public class ReservedRealmElasticAutoconfigIntegTests extends SecuritySingleNodeTestCase {
@@ -70,46 +58,10 @@ protected SecureString getBootstrapPassword() {
7058
return null; // no bootstrap password for this test
7159
}
7260

73-
private boolean isMigrationComplete(ClusterState state) {
74-
IndexMetadata indexMetadata = state.metadata().getIndices().get(TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7);
75-
return indexMetadata != null && indexMetadata.getCustomData(MIGRATION_VERSION_CUSTOM_KEY) != null;
76-
}
77-
78-
private void awaitSecurityMigrationRanOnce() {
79-
final var latch = new CountDownLatch(1);
80-
ClusterService clusterService = getInstanceFromNode(ClusterService.class);
81-
clusterService.addListener((event) -> {
82-
if (isMigrationComplete(event.state())) {
83-
latch.countDown();
84-
}
85-
});
86-
if (isMigrationComplete(clusterService.state())) {
87-
latch.countDown();
88-
}
89-
safeAwait(latch);
90-
}
91-
92-
private void deleteSecurityIndex() {
93-
// delete the security index, if it exist
94-
GetIndexRequest getIndexRequest = new GetIndexRequest();
95-
getIndexRequest.indices(SECURITY_MAIN_ALIAS);
96-
getIndexRequest.indicesOptions(IndicesOptions.lenientExpandOpen());
97-
GetIndexResponse getIndexResponse = client().admin().indices().getIndex(getIndexRequest).actionGet();
98-
if (getIndexResponse.getIndices().length > 0) {
99-
assertThat(getIndexResponse.getIndices().length, is(1));
100-
assertThat(getIndexResponse.getIndices()[0], is(TestRestrictedIndices.INTERNAL_SECURITY_MAIN_INDEX_7));
101-
102-
// Security migration needs to finish before deleting the index
103-
awaitSecurityMigrationRanOnce();
104-
DeleteIndexRequest deleteIndexRequest = new DeleteIndexRequest(getIndexResponse.getIndices());
105-
assertAcked(client().admin().indices().delete(deleteIndexRequest).actionGet());
106-
}
107-
}
108-
10961
public void testAutoconfigFailedPasswordPromotion() throws Exception {
11062
try {
11163
// .security index is created automatically on node startup so delete the security index first
112-
deleteSecurityIndex();
64+
deleteSecurityIndexIfExists();
11365
// prevents the .security index from being created automatically (after elastic user authentication)
11466
ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest(
11567
TEST_REQUEST_TIMEOUT,
@@ -176,7 +128,7 @@ public void testAutoconfigSucceedsAfterPromotionFailure() throws Exception {
176128
putUserRequest.roles(Strings.EMPTY_ARRAY);
177129
client().execute(PutUserAction.INSTANCE, putUserRequest).get();
178130
// Security migration needs to finish before making the cluster read only
179-
awaitSecurityMigrationRanOnce();
131+
awaitSecurityMigration();
180132

181133
// but then make the cluster read-only
182134
ClusterUpdateSettingsRequest updateSettingsRequest = new ClusterUpdateSettingsRequest(

‎x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/profile/ProfileIntegTests.java‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,28 @@
9999

100100
public class ProfileIntegTests extends AbstractProfileIntegTestCase {
101101

102+
protected static final String ANONYMOUS_ROLE = "anonymous_role";
103+
104+
@Override
105+
protected String configRoles() {
106+
return super.configRoles()
107+
+ "\n"
108+
+ ANONYMOUS_ROLE
109+
+ ":\n"
110+
+ " cluster:\n"
111+
+ " - 'manage_own_api_key'\n"
112+
+ " - 'manage_token'\n"
113+
+ " - 'manage_service_account'\n"
114+
+ " - 'monitor'\n";
115+
}
116+
102117
@Override
103118
protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
104119
final Settings.Builder builder = Settings.builder().put(super.nodeSettings(nodeOrdinal, otherSettings));
105120
// This setting tests that the setting is registered
106121
builder.put("xpack.security.authc.domains.my_domain.realms", "file");
107122
// enable anonymous
108-
builder.putList(AnonymousUser.ROLES_SETTING.getKey(), RAC_ROLE);
123+
builder.putList(AnonymousUser.ROLES_SETTING.getKey(), ANONYMOUS_ROLE);
109124
return builder.build();
110125
}
111126

0 commit comments

Comments
 (0)