From c98d3a2ca1be94fa19800cce6bc2e116858af710 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Tue, 6 Dec 2022 15:31:40 -0500 Subject: [PATCH 01/10] Wait until security index is ready. --- .../org/elasticsearch/xpack/security/Security.java | 1 + .../rolemapping/ReservedRoleMappingAction.java | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index d8d3cf61c0419..f9e89b6762f08 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -937,6 +937,7 @@ Collection createComponents( components.add(new SecurityUsageServices(realms, allRolesStore, nativeRoleMappingStore, ipFilter.get(), profileService)); reservedRoleMappingAction.set(new ReservedRoleMappingAction(nativeRoleMappingStore)); + systemIndices.getMainIndexManager().onStateRecovered(state -> reservedRoleMappingAction.get().securityIndexRecovered()); cacheInvalidatorRegistry.validate(); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/ReservedRoleMappingAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/ReservedRoleMappingAction.java index 74e0cc081ace5..69d42606f1c44 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/ReservedRoleMappingAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/ReservedRoleMappingAction.java @@ -9,6 +9,7 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.GroupedActionListener; +import org.elasticsearch.common.util.concurrent.ListenableFuture; import org.elasticsearch.reservedstate.NonStateTransformResult; import org.elasticsearch.reservedstate.ReservedClusterStateHandler; import org.elasticsearch.reservedstate.TransformState; @@ -41,6 +42,7 @@ public class ReservedRoleMappingAction implements ReservedClusterStateHandler
  • securityIndexRecoveryListener = new ListenableFuture<>(); /** * Creates a ReservedRoleMappingAction @@ -84,7 +86,13 @@ public TransformState transform(Object source, TransformState prevState) throws // non cluster state transform call. @SuppressWarnings("unchecked") var requests = prepare((List) source); - return new TransformState(prevState.state(), prevState.keys(), l -> nonStateTransform(requests, prevState, l)); + return new TransformState( + prevState.state(), + prevState.keys(), + l -> securityIndexRecoveryListener.addListener( + ActionListener.wrap(ignored -> nonStateTransform(requests, prevState, l), l::onFailure) + ) + ); } private void nonStateTransform( @@ -144,4 +152,8 @@ public List fromXContent(XContentParser parser) throws IO return result; } + + public void securityIndexRecovered() { + securityIndexRecoveryListener.onResponse(null); + } } From 936835111c4f4a07135a160282583b026dee4794 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski <6207777+grcevski@users.noreply.github.com> Date: Tue, 6 Dec 2022 16:55:46 -0500 Subject: [PATCH 02/10] Update docs/changelog/92173.yaml --- docs/changelog/92173.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/92173.yaml diff --git a/docs/changelog/92173.yaml b/docs/changelog/92173.yaml new file mode 100644 index 0000000000000..9b263d979e3be --- /dev/null +++ b/docs/changelog/92173.yaml @@ -0,0 +1,6 @@ +pr: 92173 +summary: Wait until security index is ready for role mappings +area: Infra/Core +type: bug +issues: + - 91939 From 46d6325b1e13e2dd9e15549f671999a280ed83d0 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Wed, 7 Dec 2022 11:06:34 -0500 Subject: [PATCH 03/10] Fix test. --- .../action/reservedstate/ReservedRoleMappingActionTests.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/reservedstate/ReservedRoleMappingActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/reservedstate/ReservedRoleMappingActionTests.java index b6d1a0d126796..6cdca0cb3b24d 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/reservedstate/ReservedRoleMappingActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/reservedstate/ReservedRoleMappingActionTests.java @@ -76,6 +76,7 @@ public void testValidation() { ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")).build(); TransformState prevState = new TransformState(state, Collections.emptySet()); ReservedRoleMappingAction action = new ReservedRoleMappingAction(nativeRoleMappingStore); + action.securityIndexRecovered(); String badPolicyJSON = """ { @@ -109,6 +110,7 @@ public void testAddRemoveRoleMapping() throws Exception { ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")).build(); TransformState prevState = new TransformState(state, Collections.emptySet()); ReservedRoleMappingAction action = new ReservedRoleMappingAction(nativeRoleMappingStore); + action.securityIndexRecovered(); String emptyJSON = ""; @@ -181,6 +183,7 @@ public void testNonStateTransformWaitsOnAsyncActions() throws Exception { ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")).build(); TransformState updatedState = new TransformState(state, Collections.emptySet()); ReservedRoleMappingAction action = new ReservedRoleMappingAction(nativeRoleMappingStore); + action.securityIndexRecovered(); String json = """ { From 9cfae25da50a7da63935893c8a231091d5b4c751 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Wed, 7 Dec 2022 11:07:54 -0500 Subject: [PATCH 04/10] Update docs. --- docs/changelog/92173.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/92173.yaml b/docs/changelog/92173.yaml index 9b263d979e3be..c8c787d78a446 100644 --- a/docs/changelog/92173.yaml +++ b/docs/changelog/92173.yaml @@ -1,5 +1,5 @@ pr: 92173 -summary: Wait until security index is ready for role mappings +summary: In file based settings, wait until security index is ready for role mappings area: Infra/Core type: bug issues: From f76771bf632757707bcc470bccc80f54bf482c07 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Thu, 8 Dec 2022 12:38:53 -0500 Subject: [PATCH 05/10] Add restart test. --- .../RoleMappingFileSettingsIT.java | 66 ++++++++++++++++++- 1 file changed, 63 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java index 5e6f63f04ff58..fcc8ce98d7fb1 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java @@ -110,6 +110,36 @@ public class RoleMappingFileSettingsIT extends NativeRealmIntegTestCase { } }"""; + private static String testJSONOnlyRoleMappings = """ + { + "metadata": { + "version": "%s", + "compatibility": "8.4.0" + }, + "state": { + "role_mappings": { + "everyone_kibana_alone": { + "enabled": true, + "roles": [ "kibana_user" ], + "rules": { "field": { "username": "*" } }, + "metadata": { + "uuid" : "b9a59ba9-6b92-4be2-bb8d-02bb270cb3a7", + "_foo": "something" + } + }, + "everyone_fleet_alone": { + "enabled": true, + "roles": [ "fleet_user" ], + "rules": { "field": { "username": "*" } }, + "metadata": { + "uuid" : "b9a59ba9-6b92-4be3-bb8d-02bb270cb3a7", + "_foo": "something_else" + } + } + } + } + }"""; + private static String testErrorJSON = """ { "metadata": { @@ -164,7 +194,7 @@ private void writeJSONFile(String node, String json) throws Exception { Files.move(tempFilePath, fileSettingsService.operatorSettingsFile(), StandardCopyOption.ATOMIC_MOVE); } - private Tuple setupClusterStateListener(String node) { + private Tuple setupClusterStateListener(String node, String expectedKey) { ClusterService clusterService = internalCluster().clusterService(node); CountDownLatch savedClusterState = new CountDownLatch(1); AtomicLong metadataVersion = new AtomicLong(-1); @@ -174,7 +204,7 @@ public void clusterChanged(ClusterChangedEvent event) { ReservedStateMetadata reservedState = event.state().metadata().reservedStateMetadata().get(FileSettingsService.NAMESPACE); if (reservedState != null) { ReservedStateHandlerMetadata handlerMetadata = reservedState.handlers().get(ReservedRoleMappingAction.NAME); - if (handlerMetadata != null && handlerMetadata.keys().contains("everyone_kibana")) { + if (handlerMetadata != null && handlerMetadata.keys().contains(expectedKey)) { clusterService.removeListener(this); metadataVersion.set(event.state().metadata().version()); savedClusterState.countDown(); @@ -280,7 +310,7 @@ private void assertRoleMappingsSaveOK(CountDownLatch savedClusterState, AtomicLo public void testRoleMappingsApplied() throws Exception { ensureGreen(); - var savedClusterState = setupClusterStateListener(internalCluster().getMasterName()); + var savedClusterState = setupClusterStateListener(internalCluster().getMasterName(), "everyone_kibana"); writeJSONFile(internalCluster().getMasterName(), testJSON); assertRoleMappingsSaveOK(savedClusterState.v1(), savedClusterState.v2()); @@ -432,6 +462,36 @@ public void testRoleMappingFailsToWriteToStore() throws Exception { assertTrue(handlerMetadata == null || handlerMetadata.keys().isEmpty()); } + public void testReservedStatePersistsOnRestart() throws Exception { + ensureGreen(); + final String masterNode = internalCluster().getMasterName(); + var savedClusterState = setupClusterStateListener(masterNode, "everyone_kibana_alone"); + + FileSettingsService masterFileSettingsService = internalCluster().getInstance(FileSettingsService.class, masterNode); + + assertTrue(masterFileSettingsService.watching()); + + logger.info("--> write some role mappings, no other file settings"); + writeJSONFile(masterNode, testJSONOnlyRoleMappings); + boolean awaitSuccessful = savedClusterState.v1().await(20, TimeUnit.SECONDS); + assertTrue(awaitSuccessful); + + logger.info("--> restart master"); + internalCluster().restartNode(masterNode); + + var clusterStateResponse = client().admin().cluster().state(new ClusterStateRequest()).actionGet(); + assertThat( + clusterStateResponse.getState() + .metadata() + .reservedStateMetadata() + .get(FileSettingsService.NAMESPACE) + .handlers() + .get(ReservedRoleMappingAction.NAME) + .keys(), + containsInAnyOrder("everyone_fleet_alone", "everyone_kibana_alone") + ); + } + private PutRoleMappingRequest sampleRestRequest(String name) throws Exception { var json = """ { From 23d515d709607c99c272436048e237791877107f Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Thu, 8 Dec 2022 12:47:13 -0500 Subject: [PATCH 06/10] Add more verification. --- .../integration/RoleMappingFileSettingsIT.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java index fcc8ce98d7fb1..dc8084a7d28f8 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java @@ -490,6 +490,15 @@ public void testReservedStatePersistsOnRestart() throws Exception { .keys(), containsInAnyOrder("everyone_fleet_alone", "everyone_kibana_alone") ); + + var request = new GetRoleMappingsRequest(); + request.setNames("everyone_kibana_alone", "everyone_fleet_alone"); + var response = client().execute(GetRoleMappingsAction.INSTANCE, request).get(); + assertTrue(response.hasMappings()); + assertThat( + Arrays.stream(response.mappings()).map(r -> r.getName()).collect(Collectors.toSet()), + allOf(notNullValue(), containsInAnyOrder("everyone_kibana_alone", "everyone_fleet_alone")) + ); } private PutRoleMappingRequest sampleRestRequest(String name) throws Exception { From bfea174c19915c54ffa32ac7e67cbd169ca108e3 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Mon, 12 Dec 2022 12:42:15 -0500 Subject: [PATCH 07/10] Split test to ensure only 1 master. --- .../FileSettingsRoleMappingsRestartIT.java | 96 +++++++++++++++++++ .../RoleMappingFileSettingsIT.java | 76 +-------------- 2 files changed, 99 insertions(+), 73 deletions(-) create mode 100644 x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FileSettingsRoleMappingsRestartIT.java diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FileSettingsRoleMappingsRestartIT.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FileSettingsRoleMappingsRestartIT.java new file mode 100644 index 0000000000000..b6e2ffa31eb3a --- /dev/null +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FileSettingsRoleMappingsRestartIT.java @@ -0,0 +1,96 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.integration; + +import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest; +import org.elasticsearch.reservedstate.service.FileSettingsService; +import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsAction; +import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsRequest; +import org.elasticsearch.xpack.security.action.rolemapping.ReservedRoleMappingAction; + +import java.util.Arrays; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; + +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.notNullValue; + +@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, autoManageMasterNodes = false) +public class FileSettingsRoleMappingsRestartIT extends RoleMappingFileSettingsIT { + private static String testJSONOnlyRoleMappings = """ + { + "metadata": { + "version": "%s", + "compatibility": "8.4.0" + }, + "state": { + "role_mappings": { + "everyone_kibana_alone": { + "enabled": true, + "roles": [ "kibana_user" ], + "rules": { "field": { "username": "*" } }, + "metadata": { + "uuid" : "b9a59ba9-6b92-4be2-bb8d-02bb270cb3a7", + "_foo": "something" + } + }, + "everyone_fleet_alone": { + "enabled": true, + "roles": [ "fleet_user" ], + "rules": { "field": { "username": "*" } }, + "metadata": { + "uuid" : "b9a59ba9-6b92-4be3-bb8d-02bb270cb3a7", + "_foo": "something_else" + } + } + } + } + }"""; + + public void testReservedStatePersistsOnRestart() throws Exception { + internalCluster().setBootstrapMasterNodeIndex(0); + + final String masterNode = internalCluster().getMasterName(); + var savedClusterState = setupClusterStateListener(masterNode, "everyone_kibana_alone"); + + FileSettingsService masterFileSettingsService = internalCluster().getInstance(FileSettingsService.class, masterNode); + + assertTrue(masterFileSettingsService.watching()); + + logger.info("--> write some role mappings, no other file settings"); + writeJSONFile(masterNode, testJSONOnlyRoleMappings); + boolean awaitSuccessful = savedClusterState.v1().await(20, TimeUnit.SECONDS); + assertTrue(awaitSuccessful); + + logger.info("--> restart master"); + internalCluster().restartNode(masterNode); + + var clusterStateResponse = client().admin().cluster().state(new ClusterStateRequest()).actionGet(); + assertThat( + clusterStateResponse.getState() + .metadata() + .reservedStateMetadata() + .get(FileSettingsService.NAMESPACE) + .handlers() + .get(ReservedRoleMappingAction.NAME) + .keys(), + containsInAnyOrder("everyone_fleet_alone", "everyone_kibana_alone") + ); + + var request = new GetRoleMappingsRequest(); + request.setNames("everyone_kibana_alone", "everyone_fleet_alone"); + var response = client().execute(GetRoleMappingsAction.INSTANCE, request).get(); + assertTrue(response.hasMappings()); + assertThat( + Arrays.stream(response.mappings()).map(r -> r.getName()).collect(Collectors.toSet()), + allOf(notNullValue(), containsInAnyOrder("everyone_kibana_alone", "everyone_fleet_alone")) + ); + } +} diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java index dc8084a7d28f8..8b23725f9fa02 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java @@ -35,7 +35,6 @@ import org.junit.After; import java.io.ByteArrayInputStream; -import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -110,36 +109,6 @@ public class RoleMappingFileSettingsIT extends NativeRealmIntegTestCase { } }"""; - private static String testJSONOnlyRoleMappings = """ - { - "metadata": { - "version": "%s", - "compatibility": "8.4.0" - }, - "state": { - "role_mappings": { - "everyone_kibana_alone": { - "enabled": true, - "roles": [ "kibana_user" ], - "rules": { "field": { "username": "*" } }, - "metadata": { - "uuid" : "b9a59ba9-6b92-4be2-bb8d-02bb270cb3a7", - "_foo": "something" - } - }, - "everyone_fleet_alone": { - "enabled": true, - "roles": [ "fleet_user" ], - "rules": { "field": { "username": "*" } }, - "metadata": { - "uuid" : "b9a59ba9-6b92-4be3-bb8d-02bb270cb3a7", - "_foo": "something_else" - } - } - } - } - }"""; - private static String testErrorJSON = """ { "metadata": { @@ -168,7 +137,7 @@ public class RoleMappingFileSettingsIT extends NativeRealmIntegTestCase { }"""; @After - public void cleanUp() throws IOException { + public void cleanUp() { ClusterUpdateSettingsResponse settingsResponse = client().admin() .cluster() .prepareUpdateSettings() @@ -177,7 +146,7 @@ public void cleanUp() throws IOException { assertTrue(settingsResponse.isAcknowledged()); } - private void writeJSONFile(String node, String json) throws Exception { + protected void writeJSONFile(String node, String json) throws Exception { long version = versionCounter.incrementAndGet(); FileSettingsService fileSettingsService = internalCluster().getInstance(FileSettingsService.class, node); @@ -194,7 +163,7 @@ private void writeJSONFile(String node, String json) throws Exception { Files.move(tempFilePath, fileSettingsService.operatorSettingsFile(), StandardCopyOption.ATOMIC_MOVE); } - private Tuple setupClusterStateListener(String node, String expectedKey) { + protected Tuple setupClusterStateListener(String node, String expectedKey) { ClusterService clusterService = internalCluster().clusterService(node); CountDownLatch savedClusterState = new CountDownLatch(1); AtomicLong metadataVersion = new AtomicLong(-1); @@ -462,45 +431,6 @@ public void testRoleMappingFailsToWriteToStore() throws Exception { assertTrue(handlerMetadata == null || handlerMetadata.keys().isEmpty()); } - public void testReservedStatePersistsOnRestart() throws Exception { - ensureGreen(); - final String masterNode = internalCluster().getMasterName(); - var savedClusterState = setupClusterStateListener(masterNode, "everyone_kibana_alone"); - - FileSettingsService masterFileSettingsService = internalCluster().getInstance(FileSettingsService.class, masterNode); - - assertTrue(masterFileSettingsService.watching()); - - logger.info("--> write some role mappings, no other file settings"); - writeJSONFile(masterNode, testJSONOnlyRoleMappings); - boolean awaitSuccessful = savedClusterState.v1().await(20, TimeUnit.SECONDS); - assertTrue(awaitSuccessful); - - logger.info("--> restart master"); - internalCluster().restartNode(masterNode); - - var clusterStateResponse = client().admin().cluster().state(new ClusterStateRequest()).actionGet(); - assertThat( - clusterStateResponse.getState() - .metadata() - .reservedStateMetadata() - .get(FileSettingsService.NAMESPACE) - .handlers() - .get(ReservedRoleMappingAction.NAME) - .keys(), - containsInAnyOrder("everyone_fleet_alone", "everyone_kibana_alone") - ); - - var request = new GetRoleMappingsRequest(); - request.setNames("everyone_kibana_alone", "everyone_fleet_alone"); - var response = client().execute(GetRoleMappingsAction.INSTANCE, request).get(); - assertTrue(response.hasMappings()); - assertThat( - Arrays.stream(response.mappings()).map(r -> r.getName()).collect(Collectors.toSet()), - allOf(notNullValue(), containsInAnyOrder("everyone_kibana_alone", "everyone_fleet_alone")) - ); - } - private PutRoleMappingRequest sampleRestRequest(String name) throws Exception { var json = """ { From 769b42d652f1684e2848f7a9b214aae2d4aafc34 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Mon, 12 Dec 2022 18:27:53 -0500 Subject: [PATCH 08/10] Don't run tests twice --- .../FileSettingsRoleMappingsRestartIT.java | 57 ++++++++++++++++++- .../RoleMappingFileSettingsIT.java | 4 +- 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FileSettingsRoleMappingsRestartIT.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FileSettingsRoleMappingsRestartIT.java index b6e2ffa31eb3a..fbde69214414e 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FileSettingsRoleMappingsRestartIT.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FileSettingsRoleMappingsRestartIT.java @@ -8,14 +8,28 @@ package org.elasticsearch.integration; import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest; +import org.elasticsearch.cluster.ClusterChangedEvent; +import org.elasticsearch.cluster.ClusterStateListener; +import org.elasticsearch.cluster.metadata.ReservedStateHandlerMetadata; +import org.elasticsearch.cluster.metadata.ReservedStateMetadata; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.core.Strings; +import org.elasticsearch.core.Tuple; import org.elasticsearch.reservedstate.service.FileSettingsService; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsAction; import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsRequest; import org.elasticsearch.xpack.security.action.rolemapping.ReservedRoleMappingAction; +import org.elasticsearch.xpack.security.authc.esnative.NativeRealmIntegTests; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.Arrays; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; import static org.hamcrest.Matchers.allOf; @@ -23,7 +37,9 @@ import static org.hamcrest.Matchers.notNullValue; @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, autoManageMasterNodes = false) -public class FileSettingsRoleMappingsRestartIT extends RoleMappingFileSettingsIT { +public class FileSettingsRoleMappingsRestartIT extends NativeRealmIntegTests { + private static AtomicLong versionCounter = new AtomicLong(1); + private static String testJSONOnlyRoleMappings = """ { "metadata": { @@ -54,6 +70,45 @@ public class FileSettingsRoleMappingsRestartIT extends RoleMappingFileSettingsIT } }"""; + private void writeJSONFile(String node, String json) throws Exception { + long version = versionCounter.incrementAndGet(); + + FileSettingsService fileSettingsService = internalCluster().getInstance(FileSettingsService.class, node); + assertTrue(fileSettingsService.watching()); + + Files.deleteIfExists(fileSettingsService.operatorSettingsFile()); + + Files.createDirectories(fileSettingsService.operatorSettingsDir()); + Path tempFilePath = createTempFile(); + + logger.info("--> writing JSON config to node {} with path {}", node, tempFilePath); + logger.info(Strings.format(json, version)); + Files.write(tempFilePath, Strings.format(json, version).getBytes(StandardCharsets.UTF_8)); + Files.move(tempFilePath, fileSettingsService.operatorSettingsFile(), StandardCopyOption.ATOMIC_MOVE); + } + + private Tuple setupClusterStateListener(String node, String expectedKey) { + ClusterService clusterService = internalCluster().clusterService(node); + CountDownLatch savedClusterState = new CountDownLatch(1); + AtomicLong metadataVersion = new AtomicLong(-1); + clusterService.addListener(new ClusterStateListener() { + @Override + public void clusterChanged(ClusterChangedEvent event) { + ReservedStateMetadata reservedState = event.state().metadata().reservedStateMetadata().get(FileSettingsService.NAMESPACE); + if (reservedState != null) { + ReservedStateHandlerMetadata handlerMetadata = reservedState.handlers().get(ReservedRoleMappingAction.NAME); + if (handlerMetadata != null && handlerMetadata.keys().contains(expectedKey)) { + clusterService.removeListener(this); + metadataVersion.set(event.state().metadata().version()); + savedClusterState.countDown(); + } + } + } + }); + + return new Tuple<>(savedClusterState, metadataVersion); + } + public void testReservedStatePersistsOnRestart() throws Exception { internalCluster().setBootstrapMasterNodeIndex(0); diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java index 8b23725f9fa02..47603fb93662e 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/RoleMappingFileSettingsIT.java @@ -146,7 +146,7 @@ public void cleanUp() { assertTrue(settingsResponse.isAcknowledged()); } - protected void writeJSONFile(String node, String json) throws Exception { + private void writeJSONFile(String node, String json) throws Exception { long version = versionCounter.incrementAndGet(); FileSettingsService fileSettingsService = internalCluster().getInstance(FileSettingsService.class, node); @@ -163,7 +163,7 @@ protected void writeJSONFile(String node, String json) throws Exception { Files.move(tempFilePath, fileSettingsService.operatorSettingsFile(), StandardCopyOption.ATOMIC_MOVE); } - protected Tuple setupClusterStateListener(String node, String expectedKey) { + private Tuple setupClusterStateListener(String node, String expectedKey) { ClusterService clusterService = internalCluster().clusterService(node); CountDownLatch savedClusterState = new CountDownLatch(1); AtomicLong metadataVersion = new AtomicLong(-1); From a3e15c3831ff9f2741bf488849a0b7dea686deac Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Tue, 13 Dec 2022 13:41:40 -0500 Subject: [PATCH 09/10] Add more tests --- .../FileSettingsRoleMappingsRestartIT.java | 7 +- .../FileSettingsRoleMappingsStartupIT.java | 102 ++++++++++++++++++ .../ReservedRoleMappingAction.java | 3 +- ...lReservedSecurityStateHandlerProvider.java | 2 +- ...dUnstableSecurityStateHandlerProvider.java | 28 +++++ .../security/UnstableLocalStateSecurity.java | 97 +++++++++++++++++ ...dstate.ReservedClusterStateHandlerProvider | 1 + 7 files changed, 234 insertions(+), 6 deletions(-) rename x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/{integration => xpack/security}/FileSettingsRoleMappingsRestartIT.java (96%) create mode 100644 x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsStartupIT.java create mode 100644 x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/LocalReservedUnstableSecurityStateHandlerProvider.java create mode 100644 x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/UnstableLocalStateSecurity.java diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FileSettingsRoleMappingsRestartIT.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsRestartIT.java similarity index 96% rename from x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FileSettingsRoleMappingsRestartIT.java rename to x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsRestartIT.java index fbde69214414e..05c117d08711a 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/FileSettingsRoleMappingsRestartIT.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsRestartIT.java @@ -5,7 +5,7 @@ * 2.0. */ -package org.elasticsearch.integration; +package org.elasticsearch.xpack.security; import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest; import org.elasticsearch.cluster.ClusterChangedEvent; @@ -17,10 +17,10 @@ import org.elasticsearch.core.Tuple; import org.elasticsearch.reservedstate.service.FileSettingsService; import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.test.SecurityIntegTestCase; import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsAction; import org.elasticsearch.xpack.core.security.action.rolemapping.GetRoleMappingsRequest; import org.elasticsearch.xpack.security.action.rolemapping.ReservedRoleMappingAction; -import org.elasticsearch.xpack.security.authc.esnative.NativeRealmIntegTests; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -37,7 +37,7 @@ import static org.hamcrest.Matchers.notNullValue; @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, autoManageMasterNodes = false) -public class FileSettingsRoleMappingsRestartIT extends NativeRealmIntegTests { +public class FileSettingsRoleMappingsRestartIT extends SecurityIntegTestCase { private static AtomicLong versionCounter = new AtomicLong(1); private static String testJSONOnlyRoleMappings = """ @@ -74,7 +74,6 @@ private void writeJSONFile(String node, String json) throws Exception { long version = versionCounter.incrementAndGet(); FileSettingsService fileSettingsService = internalCluster().getInstance(FileSettingsService.class, node); - assertTrue(fileSettingsService.watching()); Files.deleteIfExists(fileSettingsService.operatorSettingsFile()); diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsStartupIT.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsStartupIT.java new file mode 100644 index 0000000000000..ecfc89afd6b37 --- /dev/null +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsStartupIT.java @@ -0,0 +1,102 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.security; + +import org.elasticsearch.analysis.common.CommonAnalysisPlugin; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.Strings; +import org.elasticsearch.index.mapper.extras.MapperExtrasPlugin; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.reindex.ReindexPlugin; +import org.elasticsearch.reservedstate.service.FileSettingsService; +import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.test.InternalSettingsPlugin; +import org.elasticsearch.transport.netty4.Netty4Plugin; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.util.Arrays; +import java.util.Collection; +import java.util.concurrent.atomic.AtomicLong; + +import static org.elasticsearch.test.NodeRoles.dataOnlyNode; + +@ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, autoManageMasterNodes = false) +public class FileSettingsRoleMappingsStartupIT extends ESIntegTestCase { + private static AtomicLong versionCounter = new AtomicLong(1); + private static String testJSONForFailedCase = """ + { + "metadata": { + "version": "%s", + "compatibility": "8.4.0" + }, + "state": { + "role_mappings": { + "everyone_kibana_2": { + "enabled": true, + "roles": [ "kibana_user" ], + "rules": { "field": { "username": "*" } }, + "metadata": { + "uuid" : "b9a59ba9-6b92-4be2-bb8d-02bb270cb3a7", + "_foo": "something" + } + } + } + } + }"""; + + private void writeJSONFile(String node, String json) throws Exception { + long version = versionCounter.incrementAndGet(); + + FileSettingsService fileSettingsService = internalCluster().getInstance(FileSettingsService.class, node); + + Files.deleteIfExists(fileSettingsService.operatorSettingsFile()); + + Files.createDirectories(fileSettingsService.operatorSettingsDir()); + Path tempFilePath = createTempFile(); + + logger.info("--> writing JSON config to node {} with path {}", node, tempFilePath); + logger.info(Strings.format(json, version)); + Files.write(tempFilePath, Strings.format(json, version).getBytes(StandardCharsets.UTF_8)); + Files.move(tempFilePath, fileSettingsService.operatorSettingsFile(), StandardCopyOption.ATOMIC_MOVE); + } + + public void testFailsOnStartWithError() throws Exception { + internalCluster().setBootstrapMasterNodeIndex(0); + + String dataNode = internalCluster().startNode(Settings.builder().put(dataOnlyNode()).put("discovery.initial_state_timeout", "1s")); + logger.info("--> write some role mappings, no other file settings"); + writeJSONFile(dataNode, testJSONForFailedCase); + + logger.info("--> stop data node"); + internalCluster().stopNode(dataNode); + logger.info("--> start master node"); + assertEquals( + "unable to launch a new watch service", + expectThrows(IllegalStateException.class, () -> internalCluster().startMasterOnlyNode()).getMessage() + ); + } + + public Collection> nodePlugins() { + return Arrays.asList( + UnstableLocalStateSecurity.class, + Netty4Plugin.class, + ReindexPlugin.class, + CommonAnalysisPlugin.class, + InternalSettingsPlugin.class, + MapperExtrasPlugin.class + ); + } + + @Override + protected boolean addMockTransportService() { + return false; // security has its own transport service + } +} diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/ReservedRoleMappingAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/ReservedRoleMappingAction.java index 22143539fe047..2328c8478debc 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/ReservedRoleMappingAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/rolemapping/ReservedRoleMappingAction.java @@ -95,7 +95,8 @@ public TransformState transform(Object source, TransformState prevState) throws ); } - private void nonStateTransform( + // Exposed for testing purposes + protected void nonStateTransform( Collection requests, TransformState prevState, ActionListener listener diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/LocalReservedSecurityStateHandlerProvider.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/LocalReservedSecurityStateHandlerProvider.java index e63f5993a041f..619a3b73017b9 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/LocalReservedSecurityStateHandlerProvider.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/LocalReservedSecurityStateHandlerProvider.java @@ -20,7 +20,7 @@ * for {@link org.elasticsearch.test.ESIntegTestCase} because the Security Plugin is really LocalStateSecurity in those tests. */ public class LocalReservedSecurityStateHandlerProvider implements ReservedClusterStateHandlerProvider { - private final LocalStateSecurity plugin; + protected final LocalStateSecurity plugin; public LocalReservedSecurityStateHandlerProvider() { throw new IllegalStateException("Provider must be constructed using PluginsService"); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/LocalReservedUnstableSecurityStateHandlerProvider.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/LocalReservedUnstableSecurityStateHandlerProvider.java new file mode 100644 index 0000000000000..b4a07093e49c3 --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/LocalReservedUnstableSecurityStateHandlerProvider.java @@ -0,0 +1,28 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.security; + +import org.elasticsearch.reservedstate.ReservedClusterStateHandlerProvider; + +/** + * Mock Security Provider implementation for the {@link ReservedClusterStateHandlerProvider} service interface. This is used + * for {@link org.elasticsearch.test.ESIntegTestCase} because the Security Plugin is really LocalStateSecurity in those tests. + *

    + * Unlike {@link LocalReservedSecurityStateHandlerProvider} this implementation is mocked to implement the + * {@link UnstableLocalStateSecurity}. Separate implementation is needed, because the SPI creation code matches the constructor + * signature when instantiating. E.g. we need to match {@link UnstableLocalStateSecurity} instead of {@link LocalStateSecurity} + */ +public class LocalReservedUnstableSecurityStateHandlerProvider extends LocalReservedSecurityStateHandlerProvider { + public LocalReservedUnstableSecurityStateHandlerProvider() { + throw new IllegalStateException("Provider must be constructed using PluginsService"); + } + + public LocalReservedUnstableSecurityStateHandlerProvider(UnstableLocalStateSecurity plugin) { + super(plugin); + } +} diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/UnstableLocalStateSecurity.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/UnstableLocalStateSecurity.java new file mode 100644 index 0000000000000..e6321af58c56d --- /dev/null +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/UnstableLocalStateSecurity.java @@ -0,0 +1,97 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.security; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.license.XPackLicenseState; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.reservedstate.NonStateTransformResult; +import org.elasticsearch.reservedstate.ReservedClusterStateHandler; +import org.elasticsearch.reservedstate.TransformState; +import org.elasticsearch.xpack.core.security.action.rolemapping.PutRoleMappingRequest; +import org.elasticsearch.xpack.core.ssl.SSLService; +import org.elasticsearch.xpack.security.action.rolemapping.ReservedRoleMappingAction; + +import java.nio.file.Path; +import java.util.Collection; +import java.util.List; +import java.util.Optional; + +/** + * A test class that allows us to Inject new type of Reserved Handler that can + * simulate errors in saving role mappings. + *

    + * We can't use our regular path to simply make an extension of LocalStateSecurity + * in an integration test class, because the reserved handlers are injected through + * SPI. (see {@link LocalReservedUnstableSecurityStateHandlerProvider}) + */ +public class UnstableLocalStateSecurity extends LocalStateSecurity { + + public UnstableLocalStateSecurity(Settings settings, Path configPath) throws Exception { + super(settings, configPath); + // We reuse most of the initialization of LocalStateSecurity, we then just overwrite + // the security plugin with an extra method to give us a fake RoleMappingAction. + Optional security = plugins.stream().filter(p -> p instanceof Security).findFirst(); + if (security.isPresent()) { + plugins.remove(security.get()); + } + + UnstableLocalStateSecurity thisVar = this; + var action = new ReservedUnstableRoleMappingAction(); + + plugins.add(new Security(settings, super.securityExtensions()) { + @Override + protected SSLService getSslService() { + return thisVar.getSslService(); + } + + @Override + protected XPackLicenseState getLicenseState() { + return thisVar.getLicenseState(); + } + + @Override + List> reservedClusterStateHandlers() { + // pretend the security index is initialized after 2 seconds + var timer = new java.util.Timer(); + timer.schedule(new java.util.TimerTask() { + @Override + public void run() { + action.securityIndexRecovered(); + timer.cancel(); + } + }, 2_000); + return List.of(action); + } + }); + } + + public static class ReservedUnstableRoleMappingAction extends ReservedRoleMappingAction { + /** + * Creates a fake ReservedRoleMappingAction that doesn't actually use the role mapping store + */ + public ReservedUnstableRoleMappingAction() { + // we don't actually need a NativeRoleMappingStore + super(null); + } + + /** + * The nonStateTransform method is the only one that uses the native store, we simply pretend + * something has called the onFailure method of the listener. + */ + @Override + protected void nonStateTransform( + Collection requests, + TransformState prevState, + ActionListener listener + ) { + listener.onFailure(new IllegalStateException("Fake exception")); + } + } +} diff --git a/x-pack/plugin/security/src/test/resources/META-INF/services/org.elasticsearch.reservedstate.ReservedClusterStateHandlerProvider b/x-pack/plugin/security/src/test/resources/META-INF/services/org.elasticsearch.reservedstate.ReservedClusterStateHandlerProvider index 3d17572429bac..77c38d302d9c9 100644 --- a/x-pack/plugin/security/src/test/resources/META-INF/services/org.elasticsearch.reservedstate.ReservedClusterStateHandlerProvider +++ b/x-pack/plugin/security/src/test/resources/META-INF/services/org.elasticsearch.reservedstate.ReservedClusterStateHandlerProvider @@ -6,3 +6,4 @@ # org.elasticsearch.xpack.security.LocalReservedSecurityStateHandlerProvider +org.elasticsearch.xpack.security.LocalReservedUnstableSecurityStateHandlerProvider From 4cc41f60482241d5ed7507a04876b3c49ca1698e Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Sat, 17 Dec 2022 11:04:05 -0500 Subject: [PATCH 10/10] Rename test --- .../xpack/security/FileSettingsRoleMappingsStartupIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsStartupIT.java b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsStartupIT.java index ecfc89afd6b37..dffc8d26d46bc 100644 --- a/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsStartupIT.java +++ b/x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/FileSettingsRoleMappingsStartupIT.java @@ -68,7 +68,7 @@ private void writeJSONFile(String node, String json) throws Exception { Files.move(tempFilePath, fileSettingsService.operatorSettingsFile(), StandardCopyOption.ATOMIC_MOVE); } - public void testFailsOnStartWithError() throws Exception { + public void testFailsOnStartMasterNodeWithError() throws Exception { internalCluster().setBootstrapMasterNodeIndex(0); String dataNode = internalCluster().startNode(Settings.builder().put(dataOnlyNode()).put("discovery.initial_state_timeout", "1s"));