From 028a547569c52156fd3eac39c046eb05579807c8 Mon Sep 17 00:00:00 2001
From: Ryan Ernst <ryan@iernst.net>
Date: Tue, 6 Jun 2017 09:21:47 -0700
Subject: [PATCH] Settings: Fix setting groups to include secure settings

This commit fixes the group methdos of Settings to properly include
grouped secure settings. Previously the secure settings were included
but without the group prefix being removed.

closes #25069
---
 .../common/settings/Settings.java             | 38 ++++++-------------
 .../common/settings/SettingsTests.java        | 24 ++++++++++++
 2 files changed, 36 insertions(+), 26 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 fc7912c88ac8c..f71ddccd9d345 100644
--- a/core/src/main/java/org/elasticsearch/common/settings/Settings.java
+++ b/core/src/main/java/org/elasticsearch/common/settings/Settings.java
@@ -507,35 +507,21 @@ public Map<String, Settings> getGroups(String settingPrefix, boolean ignoreNonGr
     }
 
     private Map<String, Settings> getGroupsInternal(String settingPrefix, boolean ignoreNonGrouped) throws SettingsException {
-        // we don't really care that it might happen twice
-        Map<String, Map<String, String>> map = new LinkedHashMap<>();
-        for (Object o : settings.keySet()) {
-            String setting = (String) o;
-            if (setting.startsWith(settingPrefix)) {
-                String nameValue = setting.substring(settingPrefix.length());
-                int dotIndex = nameValue.indexOf('.');
-                if (dotIndex == -1) {
-                    if (ignoreNonGrouped) {
-                        continue;
-                    }
-                    throw new SettingsException("Failed to get setting group for [" + settingPrefix + "] setting prefix and setting ["
-                            + setting + "] because of a missing '.'");
-                }
-                String name = nameValue.substring(0, dotIndex);
-                String value = nameValue.substring(dotIndex + 1);
-                Map<String, String> groupSettings = map.get(name);
-                if (groupSettings == null) {
-                    groupSettings = new LinkedHashMap<>();
-                    map.put(name, groupSettings);
+        Settings prefixSettings = getByPrefix(settingPrefix);
+        Map<String, Settings> groups = new HashMap<>();
+        for (String groupName : prefixSettings.names()) {
+            Settings groupSettings = prefixSettings.getByPrefix(groupName + ".");
+            if (groupSettings.isEmpty()) {
+                if (ignoreNonGrouped) {
+                    continue;
                 }
-                groupSettings.put(value, get(setting));
+                throw new SettingsException("Failed to get setting group for [" + settingPrefix + "] setting prefix and setting ["
+                    + settingPrefix + groupName + "] because of a missing '.'");
             }
+            groups.put(groupName, groupSettings);
         }
-        Map<String, Settings> retVal = new LinkedHashMap<>();
-        for (Map.Entry<String, Map<String, String>> entry : map.entrySet()) {
-            retVal.put(entry.getKey(), new Settings(Collections.unmodifiableMap(entry.getValue()), secureSettings));
-        }
-        return Collections.unmodifiableMap(retVal);
+
+        return Collections.unmodifiableMap(groups);
     }
     /**
      * Returns group settings for the given setting prefix.
diff --git a/core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java b/core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java
index 96422d8a063f3..9fbad982bdb15 100644
--- a/core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java
+++ b/core/src/test/java/org/elasticsearch/common/settings/SettingsTests.java
@@ -38,6 +38,7 @@
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.arrayContaining;
 import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasToString;
@@ -525,6 +526,29 @@ public void testSecureSettingsPrefix() {
         assertTrue(prefixSettings.names().contains("foo"));
     }
 
+    public void testGroupPrefix() {
+        MockSecureSettings secureSettings = new MockSecureSettings();
+        secureSettings.setString("test.key1.foo", "somethingsecure");
+        secureSettings.setString("test.key1.bar", "somethingsecure");
+        secureSettings.setString("test.key2.foo", "somethingsecure");
+        secureSettings.setString("test.key2.bog", "somethingsecure");
+        Settings.Builder builder = Settings.builder();
+        builder.put("test.key1.baz", "blah1");
+        builder.put("test.key1.other", "blah2");
+        builder.put("test.key2.baz", "blah3");
+        builder.put("test.key2.else", "blah4");
+        builder.setSecureSettings(secureSettings);
+        Settings settings = builder.build();
+        Map<String, Settings> groups = settings.getGroups("test");
+        assertEquals(2, groups.size());
+        Settings key1 = groups.get("key1");
+        assertNotNull(key1);
+        assertThat(key1.names(), containsInAnyOrder("foo", "bar", "baz", "other"));
+        Settings key2 = groups.get("key2");
+        assertNotNull(key2);
+        assertThat(key2.names(), containsInAnyOrder("foo", "bog", "baz", "else"));
+    }
+
     public void testEmptyFilterMap() {
         Settings.Builder builder = Settings.builder();
         builder.put("a", "a1");