From 9c2e1defa2fa4f858513712cc1d87b223e322cf6 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Fri, 1 Mar 2019 20:24:33 +0200 Subject: [PATCH 01/11] Main --- .../support/SecurityIndexManager.java | 57 +++++++++++++++++-- 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java index 9e94dbd188f1c..3503396387f51 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java @@ -45,11 +45,13 @@ import java.nio.charset.StandardCharsets; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Predicate; @@ -167,8 +169,10 @@ public void clusterChanged(ClusterChangedEvent event) { this.indexState = newState; if (newState.equals(previousState) == false) { - for (BiConsumer listener : stateChangeListeners) { - listener.accept(previousState, newState); + // point in time iterator + final Iterator> stateListenerIterator = stateChangeListeners.iterator(); + while (stateListenerIterator.hasNext()) { + stateListenerIterator.next().accept(previousState, newState); } } } @@ -281,7 +285,10 @@ private static Version readMappingVersion(String indexName, MappingMetaData mapp */ public void checkIndexVersionThenExecute(final Consumer consumer, final Runnable andThen) { final State indexState = this.indexState; // use a local copy so all checks execute against the same state! - if (indexState.indexExists && indexState.isIndexUpToDate == false) { + if (indexState.concreteIndexName == null) { + // index not recovered from gateway + delayUntilStateRecovered(consumer, () -> checkIndexVersionThenExecute(consumer, andThen)); + } else if (indexState.indexExists && indexState.isIndexUpToDate == false) { consumer.accept(new IllegalStateException( "Security index is not on the current version. Security features relying on the index will not be available until " + "the upgrade API is run on the security index")); @@ -297,14 +304,17 @@ public void checkIndexVersionThenExecute(final Consumer consumer, fin public void prepareIndexIfNeededThenExecute(final Consumer consumer, final Runnable andThen) { final State indexState = this.indexState; // use a local copy so all checks execute against the same state! // TODO we should improve this so we don't fire off a bunch of requests to do the same thing (create or update mappings) - if (indexState.indexExists && indexState.isIndexUpToDate == false) { + if (indexState.concreteIndexName == null) { + // index not recovered from gateway + delayUntilStateRecovered(consumer, () -> prepareIndexIfNeededThenExecute(consumer, andThen)); + } else if (indexState.indexExists && indexState.isIndexUpToDate == false) { consumer.accept(new IllegalStateException( "Security index is not on the current version. Security features relying on the index will not be available until " + "the upgrade API is run on the security index")); } else if (indexState.indexExists == false) { - LOGGER.info("security index does not exist. Creating [{}] with alias [{}]", INTERNAL_SECURITY_INDEX, SECURITY_INDEX_NAME); + LOGGER.info("security index does not exist. Creating [{}] with alias [{}]", indexState.concreteIndexName, SECURITY_INDEX_NAME); Tuple mappingAndSettings = loadMappingAndSettingsSourceFromTemplate(); - CreateIndexRequest request = new CreateIndexRequest(INTERNAL_SECURITY_INDEX) + CreateIndexRequest request = new CreateIndexRequest(indexState.concreteIndexName) .alias(new Alias(SECURITY_INDEX_NAME)) .mapping("doc", mappingAndSettings.v1(), XContentType.JSON) .waitForActiveShards(ActiveShardCount.ALL) @@ -373,6 +383,41 @@ public static boolean isIndexDeleted(State previousState, State currentState) { return previousState.indexStatus != null && currentState.indexStatus == null; } + /** + * Delay the {@code runnable} invocation until cluster state recovered. + */ + private void delayUntilStateRecovered(final Consumer consumer, final Runnable runnable) { + // context preserving one shoot runnable + final AtomicBoolean done = new AtomicBoolean(false); + final Runnable delayedRunnable = client.threadPool().getThreadContext().preserveContext(() -> { + if (false == done.get()) { + done.set(true); + runnable.run(); + } + }); + final BiConsumer gatewayRecoveryListener = new BiConsumer() { + @Override + public void accept(State prevState, State newState) { + // any cluster state update is a sign that the state recovered + if (newState.concreteIndexName != null) { + stateChangeListeners.remove(this); + client.threadPool().generic().execute(delayedRunnable); + } else { + consumer.accept(new IllegalStateException("State has been recovered, but the security index name is unknown.")); + } + } + }; + // enqueue and wait for the first cluster state update + stateChangeListeners.add(gatewayRecoveryListener); + // maybe state recovered in the meantime since we last checked + final State indexState = this.indexState; + if (indexState.concreteIndexName != null) { + // state indeed recovered and we _might_ have lost the notification + stateChangeListeners.remove(gatewayRecoveryListener); + delayedRunnable.run(); + } + } + /** * State of the security index. */ From 4db7e680aa1f7429bbdaca8e397c37fc5ef2543b Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Fri, 1 Mar 2019 20:47:53 +0200 Subject: [PATCH 02/11] test 1 --- .../security/support/SecurityIndexManager.java | 5 +++++ .../support/SecurityIndexManagerTests.java | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java index 3503396387f51..c261649fbea15 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java @@ -107,6 +107,11 @@ public boolean indexExists() { return this.indexState.indexExists; } + public boolean stateRecovered() { + // concrete security index name is a proxy for the cluster state recovery + return this.indexState.concreteIndexName != null; + } + /** * Returns whether the index is on the current format if it exists. If the index does not exist * we treat the index as up to date as we expect it to be created with the current format. diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java index 0741d1c04e995..59682a7564267 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.block.ClusterBlocks; import org.elasticsearch.cluster.health.ClusterHealthStatus; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.IndexTemplateMetaData; @@ -42,6 +43,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.gateway.GatewayService; import org.elasticsearch.index.Index; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.ESTestCase; @@ -55,6 +57,7 @@ import static org.elasticsearch.xpack.security.support.SecurityIndexManager.SECURITY_TEMPLATE_NAME; import static org.elasticsearch.xpack.security.support.SecurityIndexManager.TEMPLATE_VERSION_PATTERN; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -192,6 +195,18 @@ public void testIndexHealthChangeListeners() throws Exception { assertEquals(ClusterHealthStatus.GREEN, currentState.get().indexStatus); } + public void testListeneredNotCalledWhenStateNotRecovered() throws Exception { + final AtomicBoolean listenerCalled = new AtomicBoolean(false); + manager.addIndexStateListener((prev, current) -> { + listenerCalled.set(true); + }); + final ClusterBlocks.Builder blocks = ClusterBlocks.builder().addGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK); + manager.clusterChanged(event(new ClusterState.Builder(CLUSTER_NAME).blocks(blocks))); + assertThat(listenerCalled.get(), is(false)); + manager.clusterChanged(event(new ClusterState.Builder(CLUSTER_NAME))); + assertThat(listenerCalled.get(), is(true)); + } + public void testIndexOutOfDateListeners() throws Exception { final AtomicBoolean listenerCalled = new AtomicBoolean(false); manager.clusterChanged(event(new ClusterState.Builder(CLUSTER_NAME))); @@ -236,12 +251,14 @@ private void assertInitialState() { assertThat(manager.indexExists(), Matchers.equalTo(false)); assertThat(manager.isAvailable(), Matchers.equalTo(false)); assertThat(manager.isMappingUpToDate(), Matchers.equalTo(false)); + assertThat(manager.stateRecovered(), Matchers.equalTo(false)); } private void assertIndexUpToDateButNotAvailable() { assertThat(manager.indexExists(), Matchers.equalTo(true)); assertThat(manager.isAvailable(), Matchers.equalTo(false)); assertThat(manager.isMappingUpToDate(), Matchers.equalTo(true)); + assertThat(manager.stateRecovered(), Matchers.equalTo(true)); } public static ClusterState.Builder createClusterState(String indexName, String templateName) throws IOException { From 99a1a05dba2a93864268d6dadc0cd26e5041646f Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Fri, 1 Mar 2019 21:19:01 +0200 Subject: [PATCH 03/11] Tests --- .../support/SecurityIndexManagerTests.java | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java index 59682a7564267..c9034ffe15a09 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java @@ -41,6 +41,7 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.gateway.GatewayService; @@ -75,6 +76,7 @@ public void setUpManager() { final Client mockClient = mock(Client.class); final ThreadPool threadPool = mock(ThreadPool.class); when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY)); + when(threadPool.generic()).thenReturn(EsExecutors.newDirectExecutorService()); when(mockClient.threadPool()).thenReturn(threadPool); when(mockClient.settings()).thenReturn(Settings.EMPTY); final ClusterService clusterService = mock(ClusterService.class); @@ -200,11 +202,25 @@ public void testListeneredNotCalledWhenStateNotRecovered() throws Exception { manager.addIndexStateListener((prev, current) -> { listenerCalled.set(true); }); + final AtomicBoolean prepareCalled = new AtomicBoolean(false); + manager.prepareIndexIfNeededThenExecute(c -> {}, () -> { + prepareCalled.set(true); + }); final ClusterBlocks.Builder blocks = ClusterBlocks.builder().addGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK); + // state not recovered manager.clusterChanged(event(new ClusterState.Builder(CLUSTER_NAME).blocks(blocks))); assertThat(listenerCalled.get(), is(false)); - manager.clusterChanged(event(new ClusterState.Builder(CLUSTER_NAME))); + assertThat(prepareCalled.get(), is(false)); + // state still not recovered + manager.clusterChanged(event(new ClusterState.Builder(CLUSTER_NAME).blocks(blocks))); + assertThat(listenerCalled.get(), is(false)); + assertThat(prepareCalled.get(), is(false)); + // state recovered with index + ClusterState.Builder clusterStateBuilder = createClusterState(INDEX_NAME, TEMPLATE_NAME, SecurityIndexManager.INTERNAL_INDEX_FORMAT); + markShardsAvailable(clusterStateBuilder); + manager.clusterChanged(event(clusterStateBuilder)); assertThat(listenerCalled.get(), is(true)); + assertThat(prepareCalled.get(), is(true)); } public void testIndexOutOfDateListeners() throws Exception { From f734038c0034a2babe7b0065f505702e12870da9 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Sun, 3 Mar 2019 22:49:09 +0200 Subject: [PATCH 04/11] Iter --- .../support/SecurityIndexManager.java | 22 +++++++++---------- .../support/SecurityIndexManagerTests.java | 9 ++++---- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java index c261649fbea15..30e17e1db3243 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java @@ -107,8 +107,8 @@ public boolean indexExists() { return this.indexState.indexExists; } - public boolean stateRecovered() { - // concrete security index name is a proxy for the cluster state recovery + public boolean isStateRecovered() { + // as soon as state recovers we know the name of the .security index return this.indexState.concreteIndexName != null; } @@ -168,6 +168,7 @@ public void clusterChanged(ClusterChangedEvent event) { final Version mappingVersion = oldestIndexMappingVersion(event.state()); final ClusterHealthStatus indexStatus = indexMetaData == null ? null : new ClusterIndexHealth(indexMetaData, event.state().getRoutingTable().index(indexMetaData.getIndex())).getStatus(); + // index name non-null iff state recovered final String concreteIndexName = indexMetaData == null ? INTERNAL_SECURITY_INDEX : indexMetaData.getIndex().getName(); final State newState = new State(indexExists, isIndexUpToDate, indexAvailable, mappingIsUpToDate, mappingVersion, concreteIndexName, indexStatus); @@ -290,8 +291,7 @@ private static Version readMappingVersion(String indexName, MappingMetaData mapp */ public void checkIndexVersionThenExecute(final Consumer consumer, final Runnable andThen) { final State indexState = this.indexState; // use a local copy so all checks execute against the same state! - if (indexState.concreteIndexName == null) { - // index not recovered from gateway + if (false == isStateRecovered()) { delayUntilStateRecovered(consumer, () -> checkIndexVersionThenExecute(consumer, andThen)); } else if (indexState.indexExists && indexState.isIndexUpToDate == false) { consumer.accept(new IllegalStateException( @@ -309,8 +309,7 @@ public void checkIndexVersionThenExecute(final Consumer consumer, fin public void prepareIndexIfNeededThenExecute(final Consumer consumer, final Runnable andThen) { final State indexState = this.indexState; // use a local copy so all checks execute against the same state! // TODO we should improve this so we don't fire off a bunch of requests to do the same thing (create or update mappings) - if (indexState.concreteIndexName == null) { - // index not recovered from gateway + if (false == isStateRecovered()) { delayUntilStateRecovered(consumer, () -> prepareIndexIfNeededThenExecute(consumer, andThen)); } else if (indexState.indexExists && indexState.isIndexUpToDate == false) { consumer.accept(new IllegalStateException( @@ -392,10 +391,10 @@ public static boolean isIndexDeleted(State previousState, State currentState) { * Delay the {@code runnable} invocation until cluster state recovered. */ private void delayUntilStateRecovered(final Consumer consumer, final Runnable runnable) { - // context preserving one shoot runnable final AtomicBoolean done = new AtomicBoolean(false); + // context preserving one-shot runnable final Runnable delayedRunnable = client.threadPool().getThreadContext().preserveContext(() -> { - if (false == done.get()) { + if (done.compareAndSet(false, true)) { done.set(true); runnable.run(); } @@ -403,11 +402,13 @@ private void delayUntilStateRecovered(final Consumer consumer, final final BiConsumer gatewayRecoveryListener = new BiConsumer() { @Override public void accept(State prevState, State newState) { - // any cluster state update is a sign that the state recovered + assert isStateRecovered() : "State listener is notified for updates only after state recovered."; + assert newState.concreteIndexName != null : "The newly applied state following a recovery should name the .security index"; if (newState.concreteIndexName != null) { stateChangeListeners.remove(this); client.threadPool().generic().execute(delayedRunnable); } else { + // any cluster state update is an indication that the state recovered consumer.accept(new IllegalStateException("State has been recovered, but the security index name is unknown.")); } } @@ -415,8 +416,7 @@ public void accept(State prevState, State newState) { // enqueue and wait for the first cluster state update stateChangeListeners.add(gatewayRecoveryListener); // maybe state recovered in the meantime since we last checked - final State indexState = this.indexState; - if (indexState.concreteIndexName != null) { + if (isStateRecovered()) { // state indeed recovered and we _might_ have lost the notification stateChangeListeners.remove(gatewayRecoveryListener); delayedRunnable.run(); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java index c9034ffe15a09..67779d3d21b7b 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java @@ -197,7 +197,7 @@ public void testIndexHealthChangeListeners() throws Exception { assertEquals(ClusterHealthStatus.GREEN, currentState.get().indexStatus); } - public void testListeneredNotCalledWhenStateNotRecovered() throws Exception { + public void testListeneredNotCalledBeforeStateNotRecovered() throws Exception { final AtomicBoolean listenerCalled = new AtomicBoolean(false); manager.addIndexStateListener((prev, current) -> { listenerCalled.set(true); @@ -216,7 +216,8 @@ public void testListeneredNotCalledWhenStateNotRecovered() throws Exception { assertThat(listenerCalled.get(), is(false)); assertThat(prepareCalled.get(), is(false)); // state recovered with index - ClusterState.Builder clusterStateBuilder = createClusterState(INDEX_NAME, TEMPLATE_NAME, SecurityIndexManager.INTERNAL_INDEX_FORMAT); + ClusterState.Builder clusterStateBuilder = createClusterState(INDEX_NAME, TEMPLATE_NAME, + SecurityIndexManager.INTERNAL_INDEX_FORMAT); markShardsAvailable(clusterStateBuilder); manager.clusterChanged(event(clusterStateBuilder)); assertThat(listenerCalled.get(), is(true)); @@ -267,14 +268,14 @@ private void assertInitialState() { assertThat(manager.indexExists(), Matchers.equalTo(false)); assertThat(manager.isAvailable(), Matchers.equalTo(false)); assertThat(manager.isMappingUpToDate(), Matchers.equalTo(false)); - assertThat(manager.stateRecovered(), Matchers.equalTo(false)); + assertThat(manager.isStateRecovered(), Matchers.equalTo(false)); } private void assertIndexUpToDateButNotAvailable() { assertThat(manager.indexExists(), Matchers.equalTo(true)); assertThat(manager.isAvailable(), Matchers.equalTo(false)); assertThat(manager.isMappingUpToDate(), Matchers.equalTo(true)); - assertThat(manager.stateRecovered(), Matchers.equalTo(true)); + assertThat(manager.isStateRecovered(), Matchers.equalTo(true)); } public static ClusterState.Builder createClusterState(String indexName, String templateName) throws IOException { From 1cd74ec7fa97d00c0ef711900150db8ad183ded0 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Sun, 3 Mar 2019 22:54:36 +0200 Subject: [PATCH 05/11] Iter2 --- .../xpack/security/support/SecurityIndexManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java index 30e17e1db3243..592eb229ca81a 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java @@ -316,6 +316,7 @@ public void prepareIndexIfNeededThenExecute(final Consumer consumer, "Security index is not on the current version. Security features relying on the index will not be available until " + "the upgrade API is run on the security index")); } else if (indexState.indexExists == false) { + assert INTERNAL_SECURITY_INDEX.equals(indexState.concreteIndexName); LOGGER.info("security index does not exist. Creating [{}] with alias [{}]", indexState.concreteIndexName, SECURITY_INDEX_NAME); Tuple mappingAndSettings = loadMappingAndSettingsSourceFromTemplate(); CreateIndexRequest request = new CreateIndexRequest(indexState.concreteIndexName) @@ -395,7 +396,6 @@ private void delayUntilStateRecovered(final Consumer consumer, final // context preserving one-shot runnable final Runnable delayedRunnable = client.threadPool().getThreadContext().preserveContext(() -> { if (done.compareAndSet(false, true)) { - done.set(true); runnable.run(); } }); From 63633651325207434ae97aca3e4a215810bee9b7 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Sun, 3 Mar 2019 23:06:39 +0200 Subject: [PATCH 06/11] back to the frozen state --- .../xpack/security/support/SecurityIndexManager.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java index 592eb229ca81a..481e5d0208031 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java @@ -291,7 +291,8 @@ private static Version readMappingVersion(String indexName, MappingMetaData mapp */ public void checkIndexVersionThenExecute(final Consumer consumer, final Runnable andThen) { final State indexState = this.indexState; // use a local copy so all checks execute against the same state! - if (false == isStateRecovered()) { + if (indexState.concreteIndexName == null) { + // state not yet recovered delayUntilStateRecovered(consumer, () -> checkIndexVersionThenExecute(consumer, andThen)); } else if (indexState.indexExists && indexState.isIndexUpToDate == false) { consumer.accept(new IllegalStateException( @@ -309,7 +310,8 @@ public void checkIndexVersionThenExecute(final Consumer consumer, fin public void prepareIndexIfNeededThenExecute(final Consumer consumer, final Runnable andThen) { final State indexState = this.indexState; // use a local copy so all checks execute against the same state! // TODO we should improve this so we don't fire off a bunch of requests to do the same thing (create or update mappings) - if (false == isStateRecovered()) { + if (indexState.concreteIndexName == null) { + // state not yet recovered delayUntilStateRecovered(consumer, () -> prepareIndexIfNeededThenExecute(consumer, andThen)); } else if (indexState.indexExists && indexState.isIndexUpToDate == false) { consumer.accept(new IllegalStateException( From b358996d2f4a6d928d6845570d64b058b176b576 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 4 Mar 2019 17:42:28 +0200 Subject: [PATCH 07/11] Revert --- .../support/SecurityIndexManager.java | 64 ++----------------- .../support/SecurityIndexManagerTests.java | 34 ---------- 2 files changed, 6 insertions(+), 92 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java index 481e5d0208031..9e94dbd188f1c 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java @@ -45,13 +45,11 @@ import java.nio.charset.StandardCharsets; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Predicate; @@ -107,11 +105,6 @@ public boolean indexExists() { return this.indexState.indexExists; } - public boolean isStateRecovered() { - // as soon as state recovers we know the name of the .security index - return this.indexState.concreteIndexName != null; - } - /** * Returns whether the index is on the current format if it exists. If the index does not exist * we treat the index as up to date as we expect it to be created with the current format. @@ -168,17 +161,14 @@ public void clusterChanged(ClusterChangedEvent event) { final Version mappingVersion = oldestIndexMappingVersion(event.state()); final ClusterHealthStatus indexStatus = indexMetaData == null ? null : new ClusterIndexHealth(indexMetaData, event.state().getRoutingTable().index(indexMetaData.getIndex())).getStatus(); - // index name non-null iff state recovered final String concreteIndexName = indexMetaData == null ? INTERNAL_SECURITY_INDEX : indexMetaData.getIndex().getName(); final State newState = new State(indexExists, isIndexUpToDate, indexAvailable, mappingIsUpToDate, mappingVersion, concreteIndexName, indexStatus); this.indexState = newState; if (newState.equals(previousState) == false) { - // point in time iterator - final Iterator> stateListenerIterator = stateChangeListeners.iterator(); - while (stateListenerIterator.hasNext()) { - stateListenerIterator.next().accept(previousState, newState); + for (BiConsumer listener : stateChangeListeners) { + listener.accept(previousState, newState); } } } @@ -291,10 +281,7 @@ private static Version readMappingVersion(String indexName, MappingMetaData mapp */ public void checkIndexVersionThenExecute(final Consumer consumer, final Runnable andThen) { final State indexState = this.indexState; // use a local copy so all checks execute against the same state! - if (indexState.concreteIndexName == null) { - // state not yet recovered - delayUntilStateRecovered(consumer, () -> checkIndexVersionThenExecute(consumer, andThen)); - } else if (indexState.indexExists && indexState.isIndexUpToDate == false) { + if (indexState.indexExists && indexState.isIndexUpToDate == false) { consumer.accept(new IllegalStateException( "Security index is not on the current version. Security features relying on the index will not be available until " + "the upgrade API is run on the security index")); @@ -310,18 +297,14 @@ public void checkIndexVersionThenExecute(final Consumer consumer, fin public void prepareIndexIfNeededThenExecute(final Consumer consumer, final Runnable andThen) { final State indexState = this.indexState; // use a local copy so all checks execute against the same state! // TODO we should improve this so we don't fire off a bunch of requests to do the same thing (create or update mappings) - if (indexState.concreteIndexName == null) { - // state not yet recovered - delayUntilStateRecovered(consumer, () -> prepareIndexIfNeededThenExecute(consumer, andThen)); - } else if (indexState.indexExists && indexState.isIndexUpToDate == false) { + if (indexState.indexExists && indexState.isIndexUpToDate == false) { consumer.accept(new IllegalStateException( "Security index is not on the current version. Security features relying on the index will not be available until " + "the upgrade API is run on the security index")); } else if (indexState.indexExists == false) { - assert INTERNAL_SECURITY_INDEX.equals(indexState.concreteIndexName); - LOGGER.info("security index does not exist. Creating [{}] with alias [{}]", indexState.concreteIndexName, SECURITY_INDEX_NAME); + LOGGER.info("security index does not exist. Creating [{}] with alias [{}]", INTERNAL_SECURITY_INDEX, SECURITY_INDEX_NAME); Tuple mappingAndSettings = loadMappingAndSettingsSourceFromTemplate(); - CreateIndexRequest request = new CreateIndexRequest(indexState.concreteIndexName) + CreateIndexRequest request = new CreateIndexRequest(INTERNAL_SECURITY_INDEX) .alias(new Alias(SECURITY_INDEX_NAME)) .mapping("doc", mappingAndSettings.v1(), XContentType.JSON) .waitForActiveShards(ActiveShardCount.ALL) @@ -390,41 +373,6 @@ public static boolean isIndexDeleted(State previousState, State currentState) { return previousState.indexStatus != null && currentState.indexStatus == null; } - /** - * Delay the {@code runnable} invocation until cluster state recovered. - */ - private void delayUntilStateRecovered(final Consumer consumer, final Runnable runnable) { - final AtomicBoolean done = new AtomicBoolean(false); - // context preserving one-shot runnable - final Runnable delayedRunnable = client.threadPool().getThreadContext().preserveContext(() -> { - if (done.compareAndSet(false, true)) { - runnable.run(); - } - }); - final BiConsumer gatewayRecoveryListener = new BiConsumer() { - @Override - public void accept(State prevState, State newState) { - assert isStateRecovered() : "State listener is notified for updates only after state recovered."; - assert newState.concreteIndexName != null : "The newly applied state following a recovery should name the .security index"; - if (newState.concreteIndexName != null) { - stateChangeListeners.remove(this); - client.threadPool().generic().execute(delayedRunnable); - } else { - // any cluster state update is an indication that the state recovered - consumer.accept(new IllegalStateException("State has been recovered, but the security index name is unknown.")); - } - } - }; - // enqueue and wait for the first cluster state update - stateChangeListeners.add(gatewayRecoveryListener); - // maybe state recovered in the meantime since we last checked - if (isStateRecovered()) { - // state indeed recovered and we _might_ have lost the notification - stateChangeListeners.remove(gatewayRecoveryListener); - delayedRunnable.run(); - } - } - /** * State of the security index. */ diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java index 67779d3d21b7b..0741d1c04e995 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java @@ -26,7 +26,6 @@ import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.block.ClusterBlocks; import org.elasticsearch.cluster.health.ClusterHealthStatus; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.IndexTemplateMetaData; @@ -41,10 +40,8 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.gateway.GatewayService; import org.elasticsearch.index.Index; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.ESTestCase; @@ -58,7 +55,6 @@ import static org.elasticsearch.xpack.security.support.SecurityIndexManager.SECURITY_TEMPLATE_NAME; import static org.elasticsearch.xpack.security.support.SecurityIndexManager.TEMPLATE_VERSION_PATTERN; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.is; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -76,7 +72,6 @@ public void setUpManager() { final Client mockClient = mock(Client.class); final ThreadPool threadPool = mock(ThreadPool.class); when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY)); - when(threadPool.generic()).thenReturn(EsExecutors.newDirectExecutorService()); when(mockClient.threadPool()).thenReturn(threadPool); when(mockClient.settings()).thenReturn(Settings.EMPTY); final ClusterService clusterService = mock(ClusterService.class); @@ -197,33 +192,6 @@ public void testIndexHealthChangeListeners() throws Exception { assertEquals(ClusterHealthStatus.GREEN, currentState.get().indexStatus); } - public void testListeneredNotCalledBeforeStateNotRecovered() throws Exception { - final AtomicBoolean listenerCalled = new AtomicBoolean(false); - manager.addIndexStateListener((prev, current) -> { - listenerCalled.set(true); - }); - final AtomicBoolean prepareCalled = new AtomicBoolean(false); - manager.prepareIndexIfNeededThenExecute(c -> {}, () -> { - prepareCalled.set(true); - }); - final ClusterBlocks.Builder blocks = ClusterBlocks.builder().addGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK); - // state not recovered - manager.clusterChanged(event(new ClusterState.Builder(CLUSTER_NAME).blocks(blocks))); - assertThat(listenerCalled.get(), is(false)); - assertThat(prepareCalled.get(), is(false)); - // state still not recovered - manager.clusterChanged(event(new ClusterState.Builder(CLUSTER_NAME).blocks(blocks))); - assertThat(listenerCalled.get(), is(false)); - assertThat(prepareCalled.get(), is(false)); - // state recovered with index - ClusterState.Builder clusterStateBuilder = createClusterState(INDEX_NAME, TEMPLATE_NAME, - SecurityIndexManager.INTERNAL_INDEX_FORMAT); - markShardsAvailable(clusterStateBuilder); - manager.clusterChanged(event(clusterStateBuilder)); - assertThat(listenerCalled.get(), is(true)); - assertThat(prepareCalled.get(), is(true)); - } - public void testIndexOutOfDateListeners() throws Exception { final AtomicBoolean listenerCalled = new AtomicBoolean(false); manager.clusterChanged(event(new ClusterState.Builder(CLUSTER_NAME))); @@ -268,14 +236,12 @@ private void assertInitialState() { assertThat(manager.indexExists(), Matchers.equalTo(false)); assertThat(manager.isAvailable(), Matchers.equalTo(false)); assertThat(manager.isMappingUpToDate(), Matchers.equalTo(false)); - assertThat(manager.isStateRecovered(), Matchers.equalTo(false)); } private void assertIndexUpToDateButNotAvailable() { assertThat(manager.indexExists(), Matchers.equalTo(true)); assertThat(manager.isAvailable(), Matchers.equalTo(false)); assertThat(manager.isMappingUpToDate(), Matchers.equalTo(true)); - assertThat(manager.isStateRecovered(), Matchers.equalTo(true)); } public static ClusterState.Builder createClusterState(String indexName, String templateName) throws IOException { From ea90cc131eeb7cff03abd598eb4ae9ca284d78d1 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 4 Mar 2019 19:18:47 +0200 Subject: [PATCH 08/11] Done --- .../support/SecurityIndexManager.java | 13 +++- .../support/SecurityIndexManagerTests.java | 73 +++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java index 9e94dbd188f1c..4f4852b3ad709 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.ResourceAlreadyExistsException; import org.elasticsearch.Version; @@ -40,6 +41,7 @@ import org.elasticsearch.gateway.GatewayService; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.index.mapper.MapperService; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames; import org.elasticsearch.xpack.core.template.TemplateUtils; @@ -81,7 +83,7 @@ public class SecurityIndexManager implements ClusterStateListener { private volatile State indexState; public SecurityIndexManager(Client client, String indexName, ClusterService clusterService) { - this(client, indexName, new State(false, false, false, false, null, null, null)); + this(client, indexName, State.UNRECOVERED_STATE); clusterService.addListener(this); } @@ -121,6 +123,10 @@ public boolean isMappingUpToDate() { return this.indexState.mappingUpToDate; } + public boolean isStateRecovered() { + return this.indexState != State.UNRECOVERED_STATE; + } + public ElasticsearchException getUnavailableReason() { final State localState = this.indexState; if (localState.indexAvailable) { @@ -297,7 +303,9 @@ public void checkIndexVersionThenExecute(final Consumer consumer, fin public void prepareIndexIfNeededThenExecute(final Consumer consumer, final Runnable andThen) { final State indexState = this.indexState; // use a local copy so all checks execute against the same state! // TODO we should improve this so we don't fire off a bunch of requests to do the same thing (create or update mappings) - if (indexState.indexExists && indexState.isIndexUpToDate == false) { + if (indexState == State.UNRECOVERED_STATE) { + consumer.accept(new ElasticsearchStatusException("State not yet recovered", RestStatus.SERVICE_UNAVAILABLE)); + } else if (indexState.indexExists && indexState.isIndexUpToDate == false) { consumer.accept(new IllegalStateException( "Security index is not on the current version. Security features relying on the index will not be available until " + "the upgrade API is run on the security index")); @@ -377,6 +385,7 @@ public static boolean isIndexDeleted(State previousState, State currentState) { * State of the security index. */ public static class State { + public final static State UNRECOVERED_STATE = new State(false, false, false, false, null, null, null); public final boolean indexExists; public final boolean isIndexUpToDate; public final boolean indexAvailable; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java index 0741d1c04e995..7045d70c38142 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/support/SecurityIndexManagerTests.java @@ -15,6 +15,7 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiConsumer; +import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.Version; import org.elasticsearch.action.Action; import org.elasticsearch.action.ActionListener; @@ -26,6 +27,7 @@ import org.elasticsearch.cluster.ClusterChangedEvent; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.block.ClusterBlocks; import org.elasticsearch.cluster.health.ClusterHealthStatus; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.IndexTemplateMetaData; @@ -40,10 +42,13 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.EsExecutors; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.gateway.GatewayService; import org.elasticsearch.index.Index; import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xpack.security.test.SecurityTestUtils; @@ -55,6 +60,10 @@ import static org.elasticsearch.xpack.security.support.SecurityIndexManager.SECURITY_TEMPLATE_NAME; import static org.elasticsearch.xpack.security.support.SecurityIndexManager.TEMPLATE_VERSION_PATTERN; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -72,6 +81,7 @@ public void setUpManager() { final Client mockClient = mock(Client.class); final ThreadPool threadPool = mock(ThreadPool.class); when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY)); + when(threadPool.generic()).thenReturn(EsExecutors.newDirectExecutorService()); when(mockClient.threadPool()).thenReturn(threadPool); when(mockClient.settings()).thenReturn(Settings.EMPTY); final ClusterService clusterService = mock(ClusterService.class); @@ -192,6 +202,67 @@ public void testIndexHealthChangeListeners() throws Exception { assertEquals(ClusterHealthStatus.GREEN, currentState.get().indexStatus); } + public void testWriteBeforeStateNotRecovered() throws Exception { + final AtomicBoolean prepareRunnableCalled = new AtomicBoolean(false); + final AtomicReference prepareException = new AtomicReference<>(null); + manager.prepareIndexIfNeededThenExecute(ex -> { + prepareException.set(ex); + }, () -> { + prepareRunnableCalled.set(true); + }); + assertThat(prepareException.get(), is(notNullValue())); + assertThat(prepareException.get(), instanceOf(ElasticsearchStatusException.class)); + assertThat(((ElasticsearchStatusException)prepareException.get()).status(), is(RestStatus.SERVICE_UNAVAILABLE)); + assertThat(prepareRunnableCalled.get(), is(false)); + prepareException.set(null); + prepareRunnableCalled.set(false); + // state not recovered + final ClusterBlocks.Builder blocks = ClusterBlocks.builder().addGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK); + manager.clusterChanged(event(new ClusterState.Builder(CLUSTER_NAME).blocks(blocks))); + manager.prepareIndexIfNeededThenExecute(ex -> { + prepareException.set(ex); + }, () -> { + prepareRunnableCalled.set(true); + }); + assertThat(prepareException.get(), is(notNullValue())); + assertThat(prepareException.get(), instanceOf(ElasticsearchStatusException.class)); + assertThat(((ElasticsearchStatusException)prepareException.get()).status(), is(RestStatus.SERVICE_UNAVAILABLE)); + assertThat(prepareRunnableCalled.get(), is(false)); + prepareException.set(null); + prepareRunnableCalled.set(false); + // state recovered with index + ClusterState.Builder clusterStateBuilder = createClusterState(INDEX_NAME, TEMPLATE_NAME, + SecurityIndexManager.INTERNAL_INDEX_FORMAT); + markShardsAvailable(clusterStateBuilder); + manager.clusterChanged(event(clusterStateBuilder)); + manager.prepareIndexIfNeededThenExecute(ex -> { + prepareException.set(ex); + }, () -> { + prepareRunnableCalled.set(true); + }); + assertThat(prepareException.get(), is(nullValue())); + assertThat(prepareRunnableCalled.get(), is(true)); + } + + public void testListeneredNotCalledBeforeStateNotRecovered() throws Exception { + final AtomicBoolean listenerCalled = new AtomicBoolean(false); + manager.addIndexStateListener((prev, current) -> { + listenerCalled.set(true); + }); + final ClusterBlocks.Builder blocks = ClusterBlocks.builder().addGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK); + // state not recovered + manager.clusterChanged(event(new ClusterState.Builder(CLUSTER_NAME).blocks(blocks))); + assertThat(manager.isStateRecovered(), is(false)); + assertThat(listenerCalled.get(), is(false)); + // state recovered with index + ClusterState.Builder clusterStateBuilder = createClusterState(INDEX_NAME, TEMPLATE_NAME, + SecurityIndexManager.INTERNAL_INDEX_FORMAT); + markShardsAvailable(clusterStateBuilder); + manager.clusterChanged(event(clusterStateBuilder)); + assertThat(manager.isStateRecovered(), is(true)); + assertThat(listenerCalled.get(), is(true)); + } + public void testIndexOutOfDateListeners() throws Exception { final AtomicBoolean listenerCalled = new AtomicBoolean(false); manager.clusterChanged(event(new ClusterState.Builder(CLUSTER_NAME))); @@ -236,12 +307,14 @@ private void assertInitialState() { assertThat(manager.indexExists(), Matchers.equalTo(false)); assertThat(manager.isAvailable(), Matchers.equalTo(false)); assertThat(manager.isMappingUpToDate(), Matchers.equalTo(false)); + assertThat(manager.isStateRecovered(), Matchers.equalTo(false)); } private void assertIndexUpToDateButNotAvailable() { assertThat(manager.indexExists(), Matchers.equalTo(true)); assertThat(manager.isAvailable(), Matchers.equalTo(false)); assertThat(manager.isMappingUpToDate(), Matchers.equalTo(true)); + assertThat(manager.isStateRecovered(), Matchers.equalTo(true)); } public static ClusterState.Builder createClusterState(String indexName, String templateName) throws IOException { From 0851725eed8fb246e64c9fd330e5a1eaf33fe886 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Mon, 4 Mar 2019 23:32:51 +0200 Subject: [PATCH 09/11] Checkstyle --- .../xpack/security/support/SecurityIndexManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java index 4f4852b3ad709..8f6c5ba3e5b1b 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java @@ -385,7 +385,7 @@ public static boolean isIndexDeleted(State previousState, State currentState) { * State of the security index. */ public static class State { - public final static State UNRECOVERED_STATE = new State(false, false, false, false, null, null, null); + public static final State UNRECOVERED_STATE = new State(false, false, false, false, null, null, null); public final boolean indexExists; public final boolean isIndexUpToDate; public final boolean indexAvailable; From 9146fdbfb6532941f9e1ab4c57fe2e1a4e48a103 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Tue, 5 Mar 2019 09:26:03 +0200 Subject: [PATCH 10/11] Update x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java Co-Authored-By: albertzaharovits --- .../xpack/security/support/SecurityIndexManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java index 8f6c5ba3e5b1b..260cfd671685c 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java @@ -304,7 +304,7 @@ public void prepareIndexIfNeededThenExecute(final Consumer consumer, final State indexState = this.indexState; // use a local copy so all checks execute against the same state! // TODO we should improve this so we don't fire off a bunch of requests to do the same thing (create or update mappings) if (indexState == State.UNRECOVERED_STATE) { - consumer.accept(new ElasticsearchStatusException("State not yet recovered", RestStatus.SERVICE_UNAVAILABLE)); + consumer.accept(new ElasticsearchStatusException("Cluster state has not been recovered yet, cannot write to the security index", RestStatus.SERVICE_UNAVAILABLE)); } else if (indexState.indexExists && indexState.isIndexUpToDate == false) { consumer.accept(new IllegalStateException( "Security index is not on the current version. Security features relying on the index will not be available until " + From 974fc47680599999e40453da036f00b7dd01e9eb Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 5 Mar 2019 09:40:38 +0200 Subject: [PATCH 11/11] Checkstyle --- .../xpack/security/support/SecurityIndexManager.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java index 260cfd671685c..e20a35d870542 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java @@ -304,7 +304,8 @@ public void prepareIndexIfNeededThenExecute(final Consumer consumer, final State indexState = this.indexState; // use a local copy so all checks execute against the same state! // TODO we should improve this so we don't fire off a bunch of requests to do the same thing (create or update mappings) if (indexState == State.UNRECOVERED_STATE) { - consumer.accept(new ElasticsearchStatusException("Cluster state has not been recovered yet, cannot write to the security index", RestStatus.SERVICE_UNAVAILABLE)); + consumer.accept(new ElasticsearchStatusException("Cluster state has not been recovered yet, cannot write to the security index", + RestStatus.SERVICE_UNAVAILABLE)); } else if (indexState.indexExists && indexState.isIndexUpToDate == false) { consumer.accept(new IllegalStateException( "Security index is not on the current version. Security features relying on the index will not be available until " +