From 9a1cca3c66829e698774d712f461f83fb6b08960 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 5 Jun 2018 14:54:50 +0300 Subject: [PATCH 1/8] WIP --- .../plugins/PluginsReloadIT.java | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 server/src/test/java/org/elasticsearch/plugins/PluginsReloadIT.java diff --git a/server/src/test/java/org/elasticsearch/plugins/PluginsReloadIT.java b/server/src/test/java/org/elasticsearch/plugins/PluginsReloadIT.java new file mode 100644 index 0000000000000..9dcbf840c62d6 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/plugins/PluginsReloadIT.java @@ -0,0 +1,83 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.plugins; + +import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.rest.AbstractRestChannel; +import org.elasticsearch.rest.RestController; +import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.RestResponse; +import org.elasticsearch.rest.action.admin.cluster.RestReloadSecureSettingsAction; +import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.test.rest.FakeRestRequest; + +import java.util.Arrays; +import java.util.Collection; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicReference; + +import static org.mockito.Mockito.mock; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; + +public class PluginsReloadIT extends ESIntegTestCase { + + public void testPasswordBroadcast() throws Exception { +// final NodeClient nodeClient = internalCluster().getInstance(NodeClient.class); +// final RestReloadSecureSettingsAction reloadSettingsAction = new RestReloadSecureSettingsAction(Settings.EMPTY, +// mock(RestController.class)); +// final RestRequest reloadSettingsRequest = new FakeRestRequest(); +// reloadSettingsRequest.params().put("secure_settings_password", "password"); +// final CountDownLatch reloadSettingsLatch = new CountDownLatch(1); +// final AtomicReference reloadSettingsError = new AtomicReference<>(); +// reloadSettingsAction.handleRequest(reloadSettingsRequest, new AbstractRestChannel(reloadSettingsRequest, true) { +// @Override +// public void sendResponse(RestResponse response) { +// try { +// assertThat(response.content().utf8ToString(), not(containsString("not_used_but_this_is_a_secret"))); +// } catch (final AssertionError ex) { +// reloadSettingsError.set(ex); +// } +// reloadSettingsLatch.countDown(); +// } +// }, nodeClient); +// reloadSettingsLatch.await(); +// if (reloadSettingsError.get() != null) { +// throw reloadSettingsError.get(); +// } + } + + @Override + protected Collection> nodePlugins() { + return Arrays.asList(MockReloadablePlugin.class); + } + + private static class MockReloadablePlugin extends Plugin implements ReloadablePlugin { + + public MockReloadablePlugin() { + } + + @Override + public void reload(Settings settings) throws Exception { + settings.equals(null); + } + } +} From e8c4e1287272d314dece7f0740316b0eba130535 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Tue, 5 Jun 2018 20:37:35 +0300 Subject: [PATCH 2/8] Raw test, no keystore present --- .../RestReloadSecureSettingsAction.java | 22 +---- .../action/admin/ReloadSecureSettingsIT.java | 93 +++++++++++++++++++ .../plugins/PluginsReloadIT.java | 83 ----------------- 3 files changed, 95 insertions(+), 103 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java delete mode 100644 server/src/test/java/org/elasticsearch/plugins/PluginsReloadIT.java diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestReloadSecureSettingsAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestReloadSecureSettingsAction.java index 4533f36dd6cfc..8a73f0db339df 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestReloadSecureSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestReloadSecureSettingsAction.java @@ -19,19 +19,13 @@ package org.elasticsearch.rest.action.admin.cluster; -import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsResponse; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.rest.BaseRestHandler; -import org.elasticsearch.rest.BytesRestResponse; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.RestResponse; -import org.elasticsearch.rest.RestStatus; -import org.elasticsearch.rest.action.RestActions; -import org.elasticsearch.rest.action.RestBuilderListener; +import org.elasticsearch.rest.action.RestActions.NodesResponseRestListener; import java.io.IOException; @@ -59,19 +53,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client .setTimeout(request.param("timeout")) .setNodesIds(nodesIds) .setSecureStorePassword(request.param("secure_settings_password", "")) - .execute(new RestBuilderListener(channel) { - @Override - public RestResponse buildResponse(NodesReloadSecureSettingsResponse response, XContentBuilder builder) - throws Exception { - builder.startObject(); - RestActions.buildNodesHeader(builder, channel.request(), response); - builder.field("cluster_name", response.getClusterName().value()); - response.toXContent(builder, channel.request()); - builder.endObject(); - - return new BytesRestResponse(RestStatus.OK, builder); - } - }); + .execute(new NodesResponseRestListener<>(channel)); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java b/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java new file mode 100644 index 0000000000000..9a9227965c3b1 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java @@ -0,0 +1,93 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.action.admin; + +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsResponse; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.ReloadablePlugin; +import org.elasticsearch.test.ESIntegTestCase; +import java.util.Arrays; +import java.util.Collection; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicReference; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.containsString; + +public class ReloadSecureSettingsIT extends ESIntegTestCase { + + public void testPasswordBroadcast() throws Exception { + final AtomicReference reloadSettingsError = new AtomicReference<>(); + final CountDownLatch latch = new CountDownLatch(1); + client().admin().cluster().prepareReloadSecureSettings().setSecureStorePassword("").execute( + new ActionListener() { + @Override + public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { + try { + assertThat(nodesReloadResponse, notNullValue()); + final Map nodesMap = nodesReloadResponse.getNodesMap(); + assertThat(nodesMap.size(), equalTo(cluster().size())); + for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { + assertThat(nodeResponse.reloadException(), notNullValue()); + assertThat(nodeResponse.reloadException(), instanceOf(IllegalStateException.class)); + assertThat(nodeResponse.reloadException().getMessage(), containsString("Keystore is missing")); + } + } catch (final AssertionError e) { + reloadSettingsError.set(e); + } finally { + latch.countDown(); + } + } + + @Override + public void onFailure(Exception e) { + fail("Node request failed"); + latch.countDown(); + } + }); + latch.await(); + if (reloadSettingsError.get() != null) { + throw reloadSettingsError.get(); + } + + } + + @Override + protected Collection> nodePlugins() { + return Arrays.asList(MockReloadablePlugin.class); + } + + public static class MockReloadablePlugin extends Plugin implements ReloadablePlugin { + + public MockReloadablePlugin() { + } + + @Override + public void reload(Settings settings) throws Exception { + settings.equals(null); + } + } + +} diff --git a/server/src/test/java/org/elasticsearch/plugins/PluginsReloadIT.java b/server/src/test/java/org/elasticsearch/plugins/PluginsReloadIT.java deleted file mode 100644 index 9dcbf840c62d6..0000000000000 --- a/server/src/test/java/org/elasticsearch/plugins/PluginsReloadIT.java +++ /dev/null @@ -1,83 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.plugins; - -import org.elasticsearch.client.node.NodeClient; -import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.rest.AbstractRestChannel; -import org.elasticsearch.rest.RestController; -import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.RestResponse; -import org.elasticsearch.rest.action.admin.cluster.RestReloadSecureSettingsAction; -import org.elasticsearch.test.ESIntegTestCase; -import org.elasticsearch.test.rest.FakeRestRequest; - -import java.util.Arrays; -import java.util.Collection; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.atomic.AtomicReference; - -import static org.mockito.Mockito.mock; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.not; - -public class PluginsReloadIT extends ESIntegTestCase { - - public void testPasswordBroadcast() throws Exception { -// final NodeClient nodeClient = internalCluster().getInstance(NodeClient.class); -// final RestReloadSecureSettingsAction reloadSettingsAction = new RestReloadSecureSettingsAction(Settings.EMPTY, -// mock(RestController.class)); -// final RestRequest reloadSettingsRequest = new FakeRestRequest(); -// reloadSettingsRequest.params().put("secure_settings_password", "password"); -// final CountDownLatch reloadSettingsLatch = new CountDownLatch(1); -// final AtomicReference reloadSettingsError = new AtomicReference<>(); -// reloadSettingsAction.handleRequest(reloadSettingsRequest, new AbstractRestChannel(reloadSettingsRequest, true) { -// @Override -// public void sendResponse(RestResponse response) { -// try { -// assertThat(response.content().utf8ToString(), not(containsString("not_used_but_this_is_a_secret"))); -// } catch (final AssertionError ex) { -// reloadSettingsError.set(ex); -// } -// reloadSettingsLatch.countDown(); -// } -// }, nodeClient); -// reloadSettingsLatch.await(); -// if (reloadSettingsError.get() != null) { -// throw reloadSettingsError.get(); -// } - } - - @Override - protected Collection> nodePlugins() { - return Arrays.asList(MockReloadablePlugin.class); - } - - private static class MockReloadablePlugin extends Plugin implements ReloadablePlugin { - - public MockReloadablePlugin() { - } - - @Override - public void reload(Settings settings) throws Exception { - settings.equals(null); - } - } -} From 2ed5ae1498b9f85cff125dfa0fa7ef6094a731a5 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Thu, 7 Jun 2018 18:39:26 +0300 Subject: [PATCH 3/8] Proper integ test for reload secret settings --- ...nsportNodesReloadSecureSettingsAction.java | 4 + .../common/settings/KeyStoreWrapper.java | 4 +- .../action/admin/ReloadSecureSettingsIT.java | 343 +++++++++++++++++- .../action/admin/elasticsearch_1.keystore | Bin 0 -> 672 bytes .../action/admin/elasticsearch_2.keystore | Bin 0 -> 672 bytes .../action/admin/elasticsearch_3.keystore | Bin 0 -> 672 bytes .../action/admin/elasticsearch_4.keystore | Bin 0 -> 672 bytes .../action/admin/invalid.txt.keystore | 3 + 8 files changed, 348 insertions(+), 6 deletions(-) create mode 100644 server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_1.keystore create mode 100644 server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_2.keystore create mode 100644 server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_3.keystore create mode 100644 server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_4.keystore create mode 100644 server/src/test/resources/org/elasticsearch/action/admin/invalid.txt.keystore diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java index 5e8cb306d497d..7db0b3d122f81 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java @@ -19,6 +19,8 @@ package org.elasticsearch.action.admin.cluster.node.reload; +import org.apache.logging.log4j.message.ParameterizedMessage; +import org.apache.logging.log4j.util.Supplier; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.FailedNodeException; import org.elasticsearch.action.support.ActionFilters; @@ -102,6 +104,8 @@ protected NodesReloadSecureSettingsResponse.NodeResponse nodeOperation(NodeReque try { p.reload(settingsWithKeystore); } catch (final Exception e) { + logger.warn((Supplier) () -> new ParameterizedMessage("Plugin [{}] threw [{}] on node [{}]", + p.getClass().getSimpleName(), e.getClass().getSimpleName(), clusterService.localNode().getName()), e); exceptions.add(e); } }); diff --git a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java index f47760491f8d5..3a8a06949b29c 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java +++ b/server/src/main/java/org/elasticsearch/common/settings/KeyStoreWrapper.java @@ -308,7 +308,9 @@ public void decrypt(char[] password) throws GeneralSecurityException, IOExceptio } if (formatVersion <= 2) { decryptLegacyEntries(); - assert password.length == 0; + if (password.length != 0) { + throw new IllegalArgumentException("Keystore format does not accept non-empty passwords"); + } return; } diff --git a/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java b/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java index 9a9227965c3b1..8044d2d012702 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java @@ -20,26 +20,52 @@ package org.elasticsearch.action.admin; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsResponse; +import org.elasticsearch.common.settings.KeyStoreWrapper; +import org.elasticsearch.common.settings.SecureSetting; +import org.elasticsearch.common.settings.SecureSettings; +import org.elasticsearch.common.settings.SecureString; +import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.env.Environment; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.PluginsService; import org.elasticsearch.plugins.ReloadablePlugin; import org.elasticsearch.test.ESIntegTestCase; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; +import java.security.GeneralSecurityException; import java.util.Arrays; import java.util.Collection; +import java.util.List; import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicReference; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.containsString; public class ReloadSecureSettingsIT extends ESIntegTestCase { - public void testPasswordBroadcast() throws Exception { + private static final String[] RESOURCE_KEYSTORE_NAMES = new String[] { "elasticsearch_1.keystore", "elasticsearch_2.keystore", + "elasticsearch_3.keystore", "elasticsearch_4.keystore" }; + + public void testMissingKeystoreFile() throws Exception { + final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); + final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) + .stream().findFirst().get(); + final Environment environment = internalCluster().getInstance(Environment.class); final AtomicReference reloadSettingsError = new AtomicReference<>(); + // keystore file should be missing for this test case + Files.deleteIfExists(KeyStoreWrapper.keystorePath(environment.configFile())); + final int initialReloadCount = mockReloadablePlugin.getReloadCount(); final CountDownLatch latch = new CountDownLatch(1); client().admin().cluster().prepareReloadSecureSettings().setSecureStorePassword("").execute( new ActionListener() { @@ -63,7 +89,190 @@ public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { @Override public void onFailure(Exception e) { - fail("Node request failed"); + reloadSettingsError.set(new AssertionError("Nodes request failed", e)); + latch.countDown(); + } + }); + latch.await(); + if (reloadSettingsError.get() != null) { + throw reloadSettingsError.get(); + } + // in the missing keystore case no reload should be triggered + assertThat(mockReloadablePlugin.getReloadCount(), equalTo(initialReloadCount)); + } + + public void testNullKeystorePassword() throws Exception { + final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); + final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) + .stream().findFirst().get(); + final AtomicReference reloadSettingsError = new AtomicReference<>(); + final int initialReloadCount = mockReloadablePlugin.getReloadCount(); + final CountDownLatch latch = new CountDownLatch(1); + client().admin().cluster().prepareReloadSecureSettings().execute( + new ActionListener() { + @Override + public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { + try { + reloadSettingsError.set(new AssertionError("Null keystore password should fail")); + } finally { + latch.countDown(); + } + } + + @Override + public void onFailure(Exception e) { + try { + assertThat(e, instanceOf(ActionRequestValidationException.class)); + assertThat(e.getMessage(), containsString("secure settings password cannot be null")); + } catch (final AssertionError ae) { + reloadSettingsError.set(ae); + } finally { + latch.countDown(); + } + } + }); + latch.await(); + if (reloadSettingsError.get() != null) { + throw reloadSettingsError.get(); + } + // in the null password case no reload should be triggered + assertThat(mockReloadablePlugin.getReloadCount(), equalTo(initialReloadCount)); + } + + public void testInvalidKeystoreFile() throws Exception { + final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); + final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) + .stream().findFirst().get(); + final Environment environment = internalCluster().getInstance(Environment.class); + final AtomicReference reloadSettingsError = new AtomicReference<>(); + final int initialReloadCount = mockReloadablePlugin.getReloadCount(); + // invalid "keystore" file should be present in the config dir + try (InputStream keystore = ReloadSecureSettingsIT.class.getResourceAsStream("invalid.txt.keystore")) { + if (Files.exists(environment.configFile()) == false) { + Files.createDirectory(environment.configFile()); + } + Files.copy(keystore, KeyStoreWrapper.keystorePath(environment.configFile()), StandardCopyOption.REPLACE_EXISTING); + } + final CountDownLatch latch = new CountDownLatch(1); + client().admin().cluster().prepareReloadSecureSettings().setSecureStorePassword("").execute( + new ActionListener() { + @Override + public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { + try { + assertThat(nodesReloadResponse, notNullValue()); + final Map nodesMap = nodesReloadResponse.getNodesMap(); + assertThat(nodesMap.size(), equalTo(cluster().size())); + for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { + assertThat(nodeResponse.reloadException(), notNullValue()); + } + } catch (final AssertionError e) { + reloadSettingsError.set(e); + } finally { + latch.countDown(); + } + } + + @Override + public void onFailure(Exception e) { + reloadSettingsError.set(new AssertionError("Nodes request failed", e)); + latch.countDown(); + } + }); + latch.await(); + if (reloadSettingsError.get() != null) { + throw reloadSettingsError.get(); + } + // in the invalid keystore format case no reload should be triggered + assertThat(mockReloadablePlugin.getReloadCount(), equalTo(initialReloadCount)); + } + + public void testWrongKeystorePassword() throws Exception { + final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); + final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) + .stream().findFirst().get(); + final Environment environment = internalCluster().getInstance(Environment.class); + final AtomicReference reloadSettingsError = new AtomicReference<>(); + final int initialReloadCount = mockReloadablePlugin.getReloadCount(); + // "some" keystore should be present in this case + loadResourceAsKeystore(environment, randomFrom(RESOURCE_KEYSTORE_NAMES)); + final CountDownLatch latch = new CountDownLatch(1); + client().admin().cluster().prepareReloadSecureSettings().setSecureStorePassword("Wrong password here").execute( + new ActionListener() { + @Override + public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { + try { + assertThat(nodesReloadResponse, notNullValue()); + final Map nodesMap = nodesReloadResponse.getNodesMap(); + assertThat(nodesMap.size(), equalTo(cluster().size())); + for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { + assertThat(nodeResponse.reloadException(), notNullValue()); + assertThat(nodeResponse.reloadException(), instanceOf(IllegalArgumentException.class)); + assertThat(nodeResponse.reloadException().getMessage(), + containsString("Keystore format does not accept non-empty passwords")); + } + } catch (final AssertionError e) { + reloadSettingsError.set(e); + } finally { + latch.countDown(); + } + } + + @Override + public void onFailure(Exception e) { + reloadSettingsError.set(new AssertionError("Nodes request failed", e)); + latch.countDown(); + } + }); + latch.await(); + if (reloadSettingsError.get() != null) { + throw reloadSettingsError.get(); + } + // in the wrong password case no reload should be triggered + assertThat(mockReloadablePlugin.getReloadCount(), equalTo(initialReloadCount)); + } + + public void testMisbehavingPlugin() throws Exception { + final Environment environment = internalCluster().getInstance(Environment.class); + final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); + final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) + .stream().findFirst().get(); + // make plugins throw on reload + for (final String nodeName : internalCluster().getNodeNames()) { + internalCluster().getInstance(PluginsService.class, nodeName) + .filterPlugins(MisbehavingReloadablePlugin.class) + .stream().findFirst().get().turnSulky(); + } + final AtomicReference reloadSettingsError = new AtomicReference<>(); + final int initialReloadCount = mockReloadablePlugin.getReloadCount(); + // "some" keystore should be present + final SecureSettings secureSettings = loadResourceAsKeystore(environment, randomFrom(RESOURCE_KEYSTORE_NAMES)); + // read setting value from the test case (not from the node) + final String dummyValue = MockReloadablePlugin.DUMMY_SECRET_SETTING + .get(Settings.builder().put(environment.settings()).setSecureSettings(secureSettings).build()) + .toString(); + final CountDownLatch latch = new CountDownLatch(1); + client().admin().cluster().prepareReloadSecureSettings().setSecureStorePassword("").execute( + new ActionListener() { + @Override + public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { + try { + assertThat(nodesReloadResponse, notNullValue()); + final Map nodesMap = nodesReloadResponse.getNodesMap(); + assertThat(nodesMap.size(), equalTo(cluster().size())); + for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { + assertThat(nodeResponse.reloadException(), notNullValue()); + assertThat(nodeResponse.reloadException().getMessage(), containsString("When sulky I throw")); + } + } catch (final AssertionError e) { + reloadSettingsError.set(e); + } finally { + latch.countDown(); + } + } + + @Override + public void onFailure(Exception e) { + reloadSettingsError.set(new AssertionError("Nodes request failed", e)); latch.countDown(); } }); @@ -71,22 +280,146 @@ public void onFailure(Exception e) { if (reloadSettingsError.get() != null) { throw reloadSettingsError.get(); } + // even if one plugin fails to reload (throws Exception), others should be + // unperturbed + assertThat(mockReloadablePlugin.getReloadCount() - initialReloadCount, equalTo(1)); + // mock plugin should have been reloaded successfully + assertThat(mockReloadablePlugin.getDummySecretValue(), equalTo(dummyValue)); + } + public void testReloadWhileKeystoreChanged() throws Exception { + final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); + final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) + .stream().findFirst().get(); + final Environment environment = internalCluster().getInstance(Environment.class); + final int initialReloadCount = mockReloadablePlugin.getReloadCount(); + for (int i = 0; i < randomIntBetween(4, 8); i++) { + // write keystore + final SecureSettings secureSettings = loadResourceAsKeystore(environment, randomFrom(RESOURCE_KEYSTORE_NAMES)); + // read setting value from the test case (not from the node) + final String dummyValue = MockReloadablePlugin.DUMMY_SECRET_SETTING + .get(Settings.builder().put(environment.settings()).setSecureSettings(secureSettings).build()) + .toString(); + // reload call + successfulReloadCall(); + assertThat(mockReloadablePlugin.getDummySecretValue(), equalTo(dummyValue)); + assertThat(mockReloadablePlugin.getReloadCount() - initialReloadCount, equalTo(i + 1)); + } } @Override protected Collection> nodePlugins() { - return Arrays.asList(MockReloadablePlugin.class); + return Arrays.asList(MockReloadablePlugin.class, MisbehavingReloadablePlugin.class); } - public static class MockReloadablePlugin extends Plugin implements ReloadablePlugin { + private void successfulReloadCall() throws InterruptedException { + final AtomicReference reloadSettingsError = new AtomicReference<>(); + final CountDownLatch latch = new CountDownLatch(1); + client().admin().cluster().prepareReloadSecureSettings().setSecureStorePassword("").execute( + new ActionListener() { + @Override + public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { + try { + assertThat(nodesReloadResponse, notNullValue()); + final Map nodesMap = nodesReloadResponse.getNodesMap(); + assertThat(nodesMap.size(), equalTo(cluster().size())); + for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { + assertThat(nodeResponse.reloadException(), nullValue()); + } + } catch (final AssertionError e) { + reloadSettingsError.set(e); + } finally { + latch.countDown(); + } + } + + @Override + public void onFailure(Exception e) { + reloadSettingsError.set(new AssertionError("Nodes request failed", e)); + latch.countDown(); + } + }); + latch.await(); + if (reloadSettingsError.get() != null) { + throw reloadSettingsError.get(); + } + } + + private SecureSettings loadResourceAsKeystore(Environment environment, String resourceName) + throws IOException, GeneralSecurityException { + try (InputStream keystore = ReloadSecureSettingsIT.class.getResourceAsStream(resourceName)) { + if (Files.exists(environment.configFile()) == false) { + Files.createDirectory(environment.configFile()); + } + Files.copy(keystore, KeyStoreWrapper.keystorePath(environment.configFile()), StandardCopyOption.REPLACE_EXISTING); + } + // read the keystore from the testcase code (not node code) + final KeyStoreWrapper keyStoreWrapper = KeyStoreWrapper.load(environment.configFile()); + keyStoreWrapper.decrypt(new char[0]); + return keyStoreWrapper; + } + + public static class CountingReloadablePlugin extends Plugin implements ReloadablePlugin { + + private volatile int reloadCount; + + public CountingReloadablePlugin() { + } + + @Override + public void reload(Settings settings) throws Exception { + reloadCount++; + } + + public int getReloadCount() { + return reloadCount; + } + + } + + public static class MockReloadablePlugin extends CountingReloadablePlugin { + + static final Setting DUMMY_SECRET_SETTING = SecureSetting.secureString("dummy_secret_setting", null); + private volatile String dummySecretValue; public MockReloadablePlugin() { } @Override public void reload(Settings settings) throws Exception { - settings.equals(null); + super.reload(settings); + dummySecretValue = DUMMY_SECRET_SETTING.get(settings).toString(); + } + + @Override + public List> getSettings() { + return Arrays.asList(DUMMY_SECRET_SETTING); + } + + public String getDummySecretValue() { + return dummySecretValue; + } + + } + + public static class MisbehavingReloadablePlugin extends CountingReloadablePlugin { + + private volatile boolean sulky = false; + + public MisbehavingReloadablePlugin() { + } + + @Override + public void reload(Settings settings) throws Exception { + super.reload(settings); + if (sulky) { + sulky = false; + throw new Exception("When sulky I throw"); + } + } + + public void turnSulky() { + this.sulky = true; } } diff --git a/server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_1.keystore b/server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_1.keystore new file mode 100644 index 0000000000000000000000000000000000000000..4b0857397f3f546faddb4a47ad05baadd6f54ac8 GIT binary patch literal 672 zcmcD&o+B=nnv+;ul9^nbnpl*ap_iRnSzMA|l*+)sz{J27;O!i2Xv7@gvQ^ zdx3R9mO%!x0Mmj zN``o_FOtDNDuMD!fPTzm$YV&y=`#ysQ%g%z6Eky513Lpn15P$pZ9ZluDOLs+kt6I2 zgf8sL6V`YtQ8hg_=jGz$>I@cvQ^ zdx3R9mO%!x0Mmj zN``o_FOtDNDuMD!fPTzm$YV&y=`#ysGcyZgpa(4t> literal 0 HcmV?d00001 diff --git a/server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_3.keystore b/server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_3.keystore new file mode 100644 index 0000000000000000000000000000000000000000..55cde4fe150f6919cb23b823b105479de98223b0 GIT binary patch literal 672 zcmcD&o+B=nnv+;ul9^nbnpl*ap_iRnSzMA|l*+)sz{J27;O!i2Xv7@gvQ^ zdx3R9mO%!x0MmjtT#P)kmOA%&roAr}ZM z8REgdNCx|;1j;J``Z1Fsk0BkW&n%41O-#*<3=B;T>S6VF*CL;IG{241u)@( J!Nff#>;S~-&2s<% literal 0 HcmV?d00001 diff --git a/server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_4.keystore b/server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_4.keystore new file mode 100644 index 0000000000000000000000000000000000000000..731f9edda14b4dd355633b51e1772c2c1ce41d30 GIT binary patch literal 672 zcmcD&o+B=nnv+;ul9^nbnpl*ap_iRnSzMA|l*+)sz{J27;O!i2Xv7@gvQ^ zdx3R9mO%!x0Mmj8~{zYQ-ttqZc&U>EKDKiy<_({XYX-gr=fb&9Tomv0TKRIS;)XrzrW(c=m?Io0VyGpj!H~jG%8(0$ zl??G Date: Mon, 11 Jun 2018 20:02:20 +0300 Subject: [PATCH 4/8] Small adjustment to the log message --- .../node/reload/TransportNodesReloadSecureSettingsAction.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java index 7db0b3d122f81..82eea171f01c7 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java @@ -104,8 +104,8 @@ protected NodesReloadSecureSettingsResponse.NodeResponse nodeOperation(NodeReque try { p.reload(settingsWithKeystore); } catch (final Exception e) { - logger.warn((Supplier) () -> new ParameterizedMessage("Plugin [{}] threw [{}] on node [{}]", - p.getClass().getSimpleName(), e.getClass().getSimpleName(), clusterService.localNode().getName()), e); + logger.warn((Supplier) () -> new ParameterizedMessage("Plugin [{}] threw [{}] during reload", + p.getClass().getSimpleName(), e.getClass().getSimpleName()), e); exceptions.add(e); } }); From afb0e11c798ac6355317e8b8b82268d56c359dbc Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Wed, 13 Jun 2018 20:17:35 +0300 Subject: [PATCH 5/8] Address feedback --- ...nsportNodesReloadSecureSettingsAction.java | 4 +-- .../action/admin/ReloadSecureSettingsIT.java | 26 +++++++++++-------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java index 82eea171f01c7..08b98f9219f37 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java @@ -104,8 +104,8 @@ protected NodesReloadSecureSettingsResponse.NodeResponse nodeOperation(NodeReque try { p.reload(settingsWithKeystore); } catch (final Exception e) { - logger.warn((Supplier) () -> new ParameterizedMessage("Plugin [{}] threw [{}] during reload", - p.getClass().getSimpleName(), e.getClass().getSimpleName()), e); + logger.warn((Supplier) () -> new ParameterizedMessage("Reload failed for plugin [{}]", p.getClass().getSimpleName()), + e); exceptions.add(e); } }); diff --git a/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java b/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java index 8044d2d012702..1ea31e8dfc3f4 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java @@ -41,6 +41,7 @@ import java.security.GeneralSecurityException; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.CountDownLatch; @@ -240,7 +241,7 @@ public void testMisbehavingPlugin() throws Exception { for (final String nodeName : internalCluster().getNodeNames()) { internalCluster().getInstance(PluginsService.class, nodeName) .filterPlugins(MisbehavingReloadablePlugin.class) - .stream().findFirst().get().turnSulky(); + .stream().findFirst().get().setShouldThrow(true); } final AtomicReference reloadSettingsError = new AtomicReference<>(); final int initialReloadCount = mockReloadablePlugin.getReloadCount(); @@ -261,7 +262,7 @@ public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { assertThat(nodesMap.size(), equalTo(cluster().size())); for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { assertThat(nodeResponse.reloadException(), notNullValue()); - assertThat(nodeResponse.reloadException().getMessage(), containsString("When sulky I throw")); + assertThat(nodeResponse.reloadException().getMessage(), containsString("If shouldThrow I throw")); } } catch (final AssertionError e) { reloadSettingsError.set(e); @@ -309,7 +310,10 @@ public void testReloadWhileKeystoreChanged() throws Exception { @Override protected Collection> nodePlugins() { - return Arrays.asList(MockReloadablePlugin.class, MisbehavingReloadablePlugin.class); + final List> plugins = Arrays.asList(MockReloadablePlugin.class, MisbehavingReloadablePlugin.class); + // shuffle as reload is called in order + Collections.shuffle(plugins); + return plugins; } private void successfulReloadCall() throws InterruptedException { @@ -393,7 +397,7 @@ public void reload(Settings settings) throws Exception { @Override public List> getSettings() { - return Arrays.asList(DUMMY_SECRET_SETTING); + return Collections.singletonList(DUMMY_SECRET_SETTING); } public String getDummySecretValue() { @@ -404,22 +408,22 @@ public String getDummySecretValue() { public static class MisbehavingReloadablePlugin extends CountingReloadablePlugin { - private volatile boolean sulky = false; + private boolean shouldThrow = false; public MisbehavingReloadablePlugin() { } @Override - public void reload(Settings settings) throws Exception { + public synchronized void reload(Settings settings) throws Exception { super.reload(settings); - if (sulky) { - sulky = false; - throw new Exception("When sulky I throw"); + if (shouldThrow) { + shouldThrow = false; + throw new Exception("If shouldThrow I throw"); } } - public void turnSulky() { - this.sulky = true; + public synchronized void setShouldThrow(boolean shouldThrow) { + this.shouldThrow = shouldThrow; } } From c46d3111b7f43f11a1f1f8e6aa6e6ae3b00f55a6 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Fri, 15 Jun 2018 19:13:25 +0300 Subject: [PATCH 6/8] Write keystore by ignoring the last Access Exception --- .../action/admin/ReloadSecureSettingsIT.java | 64 ++++++++----------- 1 file changed, 26 insertions(+), 38 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java b/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java index 1ea31e8dfc3f4..f158408e545e8 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java @@ -23,10 +23,7 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsResponse; import org.elasticsearch.common.settings.KeyStoreWrapper; -import org.elasticsearch.common.settings.SecureSetting; import org.elasticsearch.common.settings.SecureSettings; -import org.elasticsearch.common.settings.SecureString; -import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.env.Environment; import org.elasticsearch.plugins.Plugin; @@ -38,7 +35,7 @@ import java.io.InputStream; import java.nio.file.Files; import java.nio.file.StandardCopyOption; -import java.security.GeneralSecurityException; +import java.security.AccessControlException; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -55,9 +52,6 @@ public class ReloadSecureSettingsIT extends ESIntegTestCase { - private static final String[] RESOURCE_KEYSTORE_NAMES = new String[] { "elasticsearch_1.keystore", "elasticsearch_2.keystore", - "elasticsearch_3.keystore", "elasticsearch_4.keystore" }; - public void testMissingKeystoreFile() throws Exception { final PluginsService pluginsService = internalCluster().getInstance(PluginsService.class); final MockReloadablePlugin mockReloadablePlugin = pluginsService.filterPlugins(MockReloadablePlugin.class) @@ -195,7 +189,7 @@ public void testWrongKeystorePassword() throws Exception { final AtomicReference reloadSettingsError = new AtomicReference<>(); final int initialReloadCount = mockReloadablePlugin.getReloadCount(); // "some" keystore should be present in this case - loadResourceAsKeystore(environment, randomFrom(RESOURCE_KEYSTORE_NAMES)); + writeEmptyKeystore(environment, new char[0]); final CountDownLatch latch = new CountDownLatch(1); client().admin().cluster().prepareReloadSecureSettings().setSecureStorePassword("Wrong password here").execute( new ActionListener() { @@ -207,9 +201,7 @@ public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) { assertThat(nodesMap.size(), equalTo(cluster().size())); for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) { assertThat(nodeResponse.reloadException(), notNullValue()); - assertThat(nodeResponse.reloadException(), instanceOf(IllegalArgumentException.class)); - assertThat(nodeResponse.reloadException().getMessage(), - containsString("Keystore format does not accept non-empty passwords")); + assertThat(nodeResponse.reloadException(), instanceOf(IOException.class)); } } catch (final AssertionError e) { reloadSettingsError.set(e); @@ -246,9 +238,9 @@ public void testMisbehavingPlugin() throws Exception { final AtomicReference reloadSettingsError = new AtomicReference<>(); final int initialReloadCount = mockReloadablePlugin.getReloadCount(); // "some" keystore should be present - final SecureSettings secureSettings = loadResourceAsKeystore(environment, randomFrom(RESOURCE_KEYSTORE_NAMES)); - // read setting value from the test case (not from the node) - final String dummyValue = MockReloadablePlugin.DUMMY_SECRET_SETTING + final SecureSettings secureSettings = writeEmptyKeystore(environment, new char[0]); + // read seed setting value from the test case (not from the node) + final String seedValue = KeyStoreWrapper.SEED_SETTING .get(Settings.builder().put(environment.settings()).setSecureSettings(secureSettings).build()) .toString(); final CountDownLatch latch = new CountDownLatch(1); @@ -285,7 +277,7 @@ public void onFailure(Exception e) { // unperturbed assertThat(mockReloadablePlugin.getReloadCount() - initialReloadCount, equalTo(1)); // mock plugin should have been reloaded successfully - assertThat(mockReloadablePlugin.getDummySecretValue(), equalTo(dummyValue)); + assertThat(mockReloadablePlugin.getSeedValue(), equalTo(seedValue)); } public void testReloadWhileKeystoreChanged() throws Exception { @@ -296,14 +288,14 @@ public void testReloadWhileKeystoreChanged() throws Exception { final int initialReloadCount = mockReloadablePlugin.getReloadCount(); for (int i = 0; i < randomIntBetween(4, 8); i++) { // write keystore - final SecureSettings secureSettings = loadResourceAsKeystore(environment, randomFrom(RESOURCE_KEYSTORE_NAMES)); - // read setting value from the test case (not from the node) - final String dummyValue = MockReloadablePlugin.DUMMY_SECRET_SETTING + final SecureSettings secureSettings = writeEmptyKeystore(environment, new char[0]); + // read seed setting value from the test case (not from the node) + final String seedValue = KeyStoreWrapper.SEED_SETTING .get(Settings.builder().put(environment.settings()).setSecureSettings(secureSettings).build()) .toString(); // reload call successfulReloadCall(); - assertThat(mockReloadablePlugin.getDummySecretValue(), equalTo(dummyValue)); + assertThat(mockReloadablePlugin.getSeedValue(), equalTo(seedValue)); assertThat(mockReloadablePlugin.getReloadCount() - initialReloadCount, equalTo(i + 1)); } } @@ -349,17 +341,19 @@ public void onFailure(Exception e) { } } - private SecureSettings loadResourceAsKeystore(Environment environment, String resourceName) - throws IOException, GeneralSecurityException { - try (InputStream keystore = ReloadSecureSettingsIT.class.getResourceAsStream(resourceName)) { - if (Files.exists(environment.configFile()) == false) { - Files.createDirectory(environment.configFile()); + private SecureSettings writeEmptyKeystore(Environment environment, char[] password) throws Exception { + final KeyStoreWrapper keyStoreWrapper = KeyStoreWrapper.create(); + try { + keyStoreWrapper.save(environment.configFile(), password); + } catch (final AccessControlException e) { + if (e.getPermission() instanceof RuntimePermission && e.getPermission().getName().equals("accessUserInformation")) { + // this is expected: the save method is extra diligent and wants to make sure + // the keystore is readable, not relying on umask and whatnot. It's ok, we don't + // care about this in tests. + } else { + throw e; } - Files.copy(keystore, KeyStoreWrapper.keystorePath(environment.configFile()), StandardCopyOption.REPLACE_EXISTING); } - // read the keystore from the testcase code (not node code) - final KeyStoreWrapper keyStoreWrapper = KeyStoreWrapper.load(environment.configFile()); - keyStoreWrapper.decrypt(new char[0]); return keyStoreWrapper; } @@ -383,8 +377,7 @@ public int getReloadCount() { public static class MockReloadablePlugin extends CountingReloadablePlugin { - static final Setting DUMMY_SECRET_SETTING = SecureSetting.secureString("dummy_secret_setting", null); - private volatile String dummySecretValue; + private volatile String seedValue; public MockReloadablePlugin() { } @@ -392,16 +385,11 @@ public MockReloadablePlugin() { @Override public void reload(Settings settings) throws Exception { super.reload(settings); - dummySecretValue = DUMMY_SECRET_SETTING.get(settings).toString(); - } - - @Override - public List> getSettings() { - return Collections.singletonList(DUMMY_SECRET_SETTING); + this.seedValue = KeyStoreWrapper.SEED_SETTING.get(settings).toString(); } - public String getDummySecretValue() { - return dummySecretValue; + public String getSeedValue() { + return seedValue; } } From 6f8f504484038315769d107eaf54eac573860052 Mon Sep 17 00:00:00 2001 From: Albert Zaharovits Date: Fri, 15 Jun 2018 19:15:48 +0300 Subject: [PATCH 7/8] Removed test keystore resources --- .../action/admin/elasticsearch_1.keystore | Bin 672 -> 0 bytes .../action/admin/elasticsearch_2.keystore | Bin 672 -> 0 bytes .../action/admin/elasticsearch_3.keystore | Bin 672 -> 0 bytes .../action/admin/elasticsearch_4.keystore | Bin 672 -> 0 bytes 4 files changed, 0 insertions(+), 0 deletions(-) delete mode 100644 server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_1.keystore delete mode 100644 server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_2.keystore delete mode 100644 server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_3.keystore delete mode 100644 server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_4.keystore diff --git a/server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_1.keystore b/server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_1.keystore deleted file mode 100644 index 4b0857397f3f546faddb4a47ad05baadd6f54ac8..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 672 zcmcD&o+B=nnv+;ul9^nbnpl*ap_iRnSzMA|l*+)sz{J27;O!i2Xv7@gvQ^ zdx3R9mO%!x0Mmj zN``o_FOtDNDuMD!fPTzm$YV&y=`#ysQ%g%z6Eky513Lpn15P$pZ9ZluDOLs+kt6I2 zgf8sL6V`YtQ8hg_=jGz$>I@cvQ^ zdx3R9mO%!x0Mmj zN``o_FOtDNDuMD!fPTzm$YV&y=`#ysGcyZgpa(4t> diff --git a/server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_3.keystore b/server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_3.keystore deleted file mode 100644 index 55cde4fe150f6919cb23b823b105479de98223b0..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 672 zcmcD&o+B=nnv+;ul9^nbnpl*ap_iRnSzMA|l*+)sz{J27;O!i2Xv7@gvQ^ zdx3R9mO%!x0MmjtT#P)kmOA%&roAr}ZM z8REgdNCx|;1j;J``Z1Fsk0BkW&n%41O-#*<3=B;T>S6VF*CL;IG{241u)@( J!Nff#>;S~-&2s<% diff --git a/server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_4.keystore b/server/src/test/resources/org/elasticsearch/action/admin/elasticsearch_4.keystore deleted file mode 100644 index 731f9edda14b4dd355633b51e1772c2c1ce41d30..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 672 zcmcD&o+B=nnv+;ul9^nbnpl*ap_iRnSzMA|l*+)sz{J27;O!i2Xv7@gvQ^ zdx3R9mO%!x0Mmj8~{zYQ-ttqZc&U>EKDKiy<_({XYX-gr=fb&9Tomv0TKRIS;)XrzrW(c=m?Io0VyGpj!H~jG%8(0$ zl??G Date: Sat, 16 Jun 2018 21:02:47 +0300 Subject: [PATCH 8/8] Shuffle with random() --- .../org/elasticsearch/action/admin/ReloadSecureSettingsIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java b/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java index f158408e545e8..db3bf14c66347 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/ReloadSecureSettingsIT.java @@ -304,7 +304,7 @@ public void testReloadWhileKeystoreChanged() throws Exception { protected Collection> nodePlugins() { final List> plugins = Arrays.asList(MockReloadablePlugin.class, MisbehavingReloadablePlugin.class); // shuffle as reload is called in order - Collections.shuffle(plugins); + Collections.shuffle(plugins, random()); return plugins; }