Skip to content

Commit 0dfe5d1

Browse files
slobodanadamovicslobodanadamovic
authored
Fix internal cluster and single node security tests (elastic#121466) (elastic#122733)
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
1 parent 59d8912 commit 0dfe5d1

File tree

4 files changed

+74
-98
lines changed

4 files changed

+74
-98
lines changed

‎muted-tests.yml‎

Lines changed: 1 addition & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -227,44 +227,17 @@ tests:
227227
- class: org.elasticsearch.action.search.SearchProgressActionListenerIT
228228
method: testSearchProgressWithQuery
229229
issue: https://github.com/elastic/elasticsearch/issues/120994
230-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
231-
method: testSuggestProfilesWithName
232-
issue: https://github.com/elastic/elasticsearch/issues/121022
233-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
234-
method: testProfileAPIsWhenIndexNotCreated
235-
issue: https://github.com/elastic/elasticsearch/issues/121096
236-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
237-
method: testGetProfiles
238-
issue: https://github.com/elastic/elasticsearch/issues/121101
239-
- class: org.elasticsearch.xpack.security.authc.service.ServiceAccountSingleNodeTests
240-
method: testAuthenticateWithServiceFileToken
241-
issue: https://github.com/elastic/elasticsearch/issues/120988
242-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
243-
method: testUpdateProfileData
244-
issue: https://github.com/elastic/elasticsearch/issues/121108
245230
- class: org.elasticsearch.backwards.MixedClusterClientYamlTestSuiteIT
246231
method: test {p0=nodes.stats/11_indices_metrics/indices mappings exact count test for indices level}
247232
issue: https://github.com/elastic/elasticsearch/issues/120950
248233
- class: org.elasticsearch.xpack.shutdown.AllocationFailuresResetOnShutdownIT
249234
method: testResetAllocationFailuresOnNodeShutdown
250235
issue: https://github.com/elastic/elasticsearch/issues/121129
251-
- class: org.elasticsearch.xpack.security.authc.jwt.JwtRealmSingleNodeTests
252-
method: testActivateProfileForJWT
253-
issue: https://github.com/elastic/elasticsearch/issues/120983
254-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
255-
method: testProfileIndexAutoCreation
256-
issue: https://github.com/elastic/elasticsearch/issues/120987
257236
- class: org.elasticsearch.xpack.security.FileSettingsRoleMappingsRestartIT
258237
method: testFileSettingsReprocessedOnRestartWithoutVersionChange
259238
issue: https://github.com/elastic/elasticsearch/issues/120964
260-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
261-
method: testGetUsersWithProfileUidWhenProfileIndexDoesNotExists
262-
issue: https://github.com/elastic/elasticsearch/issues/121179
263239
- class: org.elasticsearch.xpack.ml.integration.PyTorchModelIT
264240
issue: https://github.com/elastic/elasticsearch/issues/121165
265-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
266-
method: testSetEnabled
267-
issue: https://github.com/elastic/elasticsearch/issues/121183
268241
- class: org.elasticsearch.xpack.test.rest.XPackRestIT
269242
method: test {p0=transform/*}
270243
issue: https://github.com/elastic/elasticsearch/issues/120816
@@ -293,30 +266,12 @@ tests:
293266
- class: org.elasticsearch.smoketest.DocsClientYamlTestSuiteIT
294267
method: test {yaml=reference/index-modules/slowlog/line_102}
295268
issue: https://github.com/elastic/elasticsearch/issues/121288
296-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
297-
method: testSuggestProfilesWithHint
298-
issue: https://github.com/elastic/elasticsearch/issues/121116
299269
- class: org.elasticsearch.env.NodeEnvironmentTests
300270
method: testGetBestDowngradeVersion
301271
issue: https://github.com/elastic/elasticsearch/issues/121316
302272
- class: org.elasticsearch.index.engine.ShuffleForcedMergePolicyTests
303273
method: testDiagnostics
304274
issue: https://github.com/elastic/elasticsearch/issues/121336
305-
- class: org.elasticsearch.xpack.security.authc.jwt.JwtRealmSingleNodeTests
306-
method: testGrantApiKeyForJWT
307-
issue: https://github.com/elastic/elasticsearch/issues/121039
308-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
309-
method: testGetUsersWithProfileUid
310-
issue: https://github.com/elastic/elasticsearch/issues/121483
311-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
312-
method: testActivateProfile
313-
issue: https://github.com/elastic/elasticsearch/issues/121151
314-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
315-
method: testHasPrivileges
316-
issue: https://github.com/elastic/elasticsearch/issues/121346
317-
- class: org.elasticsearch.xpack.security.profile.ProfileIntegTests
318-
method: testSuggestProfileWithData
319-
issue: https://github.com/elastic/elasticsearch/issues/121258
320275
- class: org.elasticsearch.search.CrossClusterSearchUnavailableClusterIT
321276
method: testSearchSkipUnavailable
322277
issue: https://github.com/elastic/elasticsearch/issues/121497
@@ -367,4 +322,4 @@ tests:
367322
# issue: "https://github.com/elastic/elasticsearch/..."
368323
# - class: "org.elasticsearch.xpack.esql.**"
369324
# method: "test {union_types.MultiIndexIpStringStatsInline *}"
370-
# issue: "https://github.com/elastic/elasticsearch/..."
325+
# issue: "https://github.com/elastic/elasticsearch/..."

‎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(TEST_REQUEST_TIMEOUT);
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)