Merge pull request #13043 from jasontedor/fix/no-multiple-settings

Do not permit multiple settings files
elastic · Aug 21, 2015 · 907f648 · 907f648
2 parents 5002f3b + fe8eb80
commit 907f648
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 24 deletions.
diff --git a/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java b/core/src/main/java/org/elasticsearch/node/internal/InternalSettingsPreparer.java
@@ -21,6 +21,7 @@
 
 import com.google.common.base.Charsets;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Sets;
 import com.google.common.collect.UnmodifiableIterator;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.common.Booleans;
@@ -40,6 +41,7 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ThreadLocalRandom;
 
 import static org.elasticsearch.common.Strings.cleanPath;
@@ -113,12 +115,21 @@ public static Tuple<Settings, Environment> prepareSettings(Settings pSettings, b
                 }
             }
             if (loadFromEnv) {
+                boolean settingsFileFound = false;
+                Set<String> foundSuffixes = Sets.newHashSet();
                 for (String allowedSuffix : ALLOWED_SUFFIXES) {
                     Path path = environment.configFile().resolve("elasticsearch" + allowedSuffix);
                     if (Files.exists(path)) {
-                        settingsBuilder.loadFromPath(path);
+                        if (!settingsFileFound) {
+                            settingsBuilder.loadFromPath(path);
+                        }
+                        settingsFileFound = true;
+                        foundSuffixes.add(allowedSuffix);
                     }
                 }
+                if (foundSuffixes.size() > 1) {
+                    throw new SettingsException("multiple settings files found with suffixes: " + Strings.collectionToDelimitedString(foundSuffixes, ","));
+                }
             }
         }
 

diff --git a/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java b/core/src/test/java/org/elasticsearch/node/internal/InternalSettingsPreparerTests.java
@@ -28,7 +28,9 @@
 import org.elasticsearch.test.ESTestCase;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -42,6 +44,8 @@
 import static org.hamcrest.Matchers.*;
 
 public class InternalSettingsPreparerTests extends ESTestCase {
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
 
     @Before
     public void setupSystemProperties() {
@@ -75,29 +79,6 @@ public void testIgnoreSystemProperties() {
         assertThat(tuple.v1().get("node.zone"), equalTo("bar"));
     }
 
-    @Test
-    public void testAlternateConfigFileSuffixes() throws Exception {
-        InputStream yaml = getClass().getResourceAsStream("/config/elasticsearch.yaml");
-        InputStream json = getClass().getResourceAsStream("/config/elasticsearch.json");
-        InputStream properties = getClass().getResourceAsStream("/config/elasticsearch.properties");
-        Path home = createTempDir();
-        Path config = home.resolve("config");
-        Files.createDirectory(config);
-        Files.copy(yaml, config.resolve("elasticsearch.yaml"));
-        Files.copy(json, config.resolve("elasticsearch.json"));
-        Files.copy(properties, config.resolve("elasticsearch.properties"));
-
-        // test that we can read config files with .yaml, .json, and .properties suffixes
-        Tuple<Settings, Environment> tuple = InternalSettingsPreparer.prepareSettings(settingsBuilder()
-                .put("config.ignore_system_properties", true)
-                .put("path.home", home)
-                .build(), true);
-
-        assertThat(tuple.v1().get("yaml.config.exists"), equalTo("true"));
-        assertThat(tuple.v1().get("json.config.exists"), equalTo("true"));
-        assertThat(tuple.v1().get("properties.config.exists"), equalTo("true"));
-    }
-
     @Test
     public void testReplacePromptPlaceholders() {
         final List<String> replacedSecretProperties = new ArrayList<>();
@@ -248,4 +229,24 @@ public void testGarbageIsNotSwallowed() throws IOException {
                 .put("path.home", home)
                 .build(), true);
     }
+
+    public void testMultipleSettingsFileNotAllowed() throws IOException {
+        InputStream yaml = getClass().getResourceAsStream("/config/elasticsearch.yaml");
+        InputStream properties = getClass().getResourceAsStream("/config/elasticsearch.properties");
+        Path home = createTempDir();
+        Path config = home.resolve("config");
+        Files.createDirectory(config);
+        Files.copy(yaml, config.resolve("elasticsearch.yaml"));
+        Files.copy(properties, config.resolve("elasticsearch.properties"));
+
+        expectedException.expect(SettingsException.class);
+        expectedException.expectMessage("multiple settings files found with suffixes: ");
+        expectedException.expectMessage("yaml");
+        expectedException.expectMessage("properties");
+
+        InternalSettingsPreparer.prepareSettings(settingsBuilder()
+                .put("config.ignore_system_properties", true)
+                .put("path.home", home)
+                .build(), true);
+    }
 }