From 6e9783259ad3694f229c78474f6e077cbe839b46 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 23 Jun 2017 14:43:04 -0700 Subject: [PATCH 1/3] Test: Allow merging mock secure settings While real secure settings (ie an ES keystore) cannot be merged together, mocked secure settings can and need to be sometimes merged. This commit adds a merge method to allow tests to merge together multiple instances of secure settings. --- .../org/elasticsearch/common/settings/Settings.java | 8 ++++++++ .../common/settings/MockSecureSettings.java | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/core/src/main/java/org/elasticsearch/common/settings/Settings.java b/core/src/main/java/org/elasticsearch/common/settings/Settings.java index e444dea6b7964..d5ff09984b8f2 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -718,6 +718,14 @@ public Builder setSecureSettings(SecureSettings secureSettings) { if (secureSettings.isLoaded() == false) { throw new IllegalStateException("Secure settings must already be loaded"); } + if (secureSettings.getSettingNames().isEmpty()) { + // TODO: fix this leniency!!! + return this; // no secure settings to add... + } + if (this.secureSettings.get() != null) { + throw new IllegalArgumentException("Secure settings already set. Existing settings: " + + this.secureSettings.get().getSettingNames() + ", new settings: " + secureSettings.getSettingNames()); + } this.secureSettings.set(secureSettings); return this; } diff --git a/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java b/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java index bb5720c99e254..22496642cb9b7 100644 --- a/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java +++ b/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java @@ -72,6 +72,18 @@ public void setFile(String setting, byte[] value) { settingNames.add(setting); } + /** Merge the given secure settings into this one. */ + public void merge(MockSecureSettings secureSettings) { + for (String setting : secureSettings.getSettingNames()) { + if (settingNames.contains(setting)) { + throw new IllegalArgumentException("Cannot overwrite existing secure setting " + setting); + } + } + settingNames.addAll(secureSettings.settingNames); + secureStrings.putAll(secureSettings.secureStrings); + files.putAll(secureSettings.files); + } + @Override public void close() throws IOException { closed.set(true); From dd23eca751f41aeb5c1d8974348fa57e99325872 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sat, 24 Jun 2017 11:32:28 -0700 Subject: [PATCH 2/3] Remove leniency --- .../main/java/org/elasticsearch/common/settings/Settings.java | 4 ++-- .../main/java/org/elasticsearch/transport/TcpTransport.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/settings/Settings.java b/core/src/main/java/org/elasticsearch/common/settings/Settings.java index d5ff09984b8f2..dec67bbff50bf 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -718,10 +718,10 @@ public Builder setSecureSettings(SecureSettings secureSettings) { if (secureSettings.isLoaded() == false) { throw new IllegalStateException("Secure settings must already be loaded"); } - if (secureSettings.getSettingNames().isEmpty()) { + /*if (secureSettings.getSettingNames().isEmpty()) { // TODO: fix this leniency!!! return this; // no secure settings to add... - } + }*/ if (this.secureSettings.get() != null) { throw new IllegalArgumentException("Secure settings already set. Existing settings: " + this.secureSettings.get().getSettingNames() + ", new settings: " + secureSettings.getSettingNames()); diff --git a/core/src/main/java/org/elasticsearch/transport/TcpTransport.java b/core/src/main/java/org/elasticsearch/transport/TcpTransport.java index 5db64c98b8586..9631fc977c935 100644 --- a/core/src/main/java/org/elasticsearch/transport/TcpTransport.java +++ b/core/src/main/java/org/elasticsearch/transport/TcpTransport.java @@ -677,8 +677,8 @@ protected Map buildProfileSettings() { continue; } Settings mergedSettings = Settings.builder() - .put(defaultSettings) - .put(profileSettings) + .put(defaultSettings.getAsMap()) + .put(profileSettings.getAsMap()) .build(); result.put(name, mergedSettings); } From da5ca2b3841c1feb4629e9f96fa93c57cd7042d3 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Sat, 24 Jun 2017 11:33:26 -0700 Subject: [PATCH 3/3] for real --- .../main/java/org/elasticsearch/common/settings/Settings.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/settings/Settings.java b/core/src/main/java/org/elasticsearch/common/settings/Settings.java index dec67bbff50bf..182ea6ccb1d7b 100644 --- a/core/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/core/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -718,10 +718,6 @@ public Builder setSecureSettings(SecureSettings secureSettings) { if (secureSettings.isLoaded() == false) { throw new IllegalStateException("Secure settings must already be loaded"); } - /*if (secureSettings.getSettingNames().isEmpty()) { - // TODO: fix this leniency!!! - return this; // no secure settings to add... - }*/ if (this.secureSettings.get() != null) { throw new IllegalArgumentException("Secure settings already set. Existing settings: " + this.secureSettings.get().getSettingNames() + ", new settings: " + secureSettings.getSettingNames());