Settings: Fix secure settings by prefix

[rjernst]

This commit fixes a bug in retrieving a sub Settings object for a given
prefix with secure settings. Before this commit the returned Settings
would be filtered by the prefix, but the found setting names would not
have the prefix removed.

Note that nothing was yet using the prefix behavior for secure settings, so I marked this as a non-issue.

[dadoonet]

Ha thanks! I was planning to ping about this as I found the same issue while working on azure repo plugin and found that we have the issue with s3.
That's going to fix the problem I saw. (I've been hitting my head to the wall yesterday :))

[dadoonet]

I actually read your PR description and I think that we had this issue in s3 with multiple clients. I'll paste in few hours a test case for this. Can you hold merging until then?

[dadoonet]

@rjernst So I tried to see if your branch actually fixes the issue I have seen and apparently it does not.
So may be I'm doing something bad. Here is the test I wrote:

Index: plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/AwsS3ServiceImplTests.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/AwsS3ServiceImplTests.java	(date 1496730707000)
+++ plugins/repository-s3/src/test/java/org/elasticsearch/repositories/s3/AwsS3ServiceImplTests.java	(revision )
@@ -29,6 +29,9 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.test.ESTestCase;
 
+import java.util.Map;
+
+import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
@@ -114,6 +117,18 @@
             "aws_proxy_password", 3, false, 10000);
     }
 
+    public void testAWSConfigurationWith2ClientSettings() {
+        MockSecureSettings secureSettings = new MockSecureSettings();
+        secureSettings.setString("s3.client.myconfig.access_key", "myconfig_key");
+        secureSettings.setString("s3.client.myconfig.secret_key", "myconfig_secret");
+        secureSettings.setString("s3.client.default.access_key", "default_key");
+        secureSettings.setString("s3.client.default.secret_key", "default_secret");
+        Settings settings = Settings.builder().setSecureSettings(secureSettings).build();
+        Map<String, S3ClientSettings> loadedSettings = S3ClientSettings.load(settings);
+
+        assertThat(loadedSettings.keySet(), containsInAnyOrder("myconfig", "default"));
+    }
+
     public void testRepositoryMaxRetries() {
         Settings settings = Settings.builder()
             .put("s3.client.default.max_retries", 5)

Indeed. You did not fix the getGroups method. Basically this is not passing:

    public void testGroupsWithSecuredSettings() {
        MockSecureSettings secureSettings = new MockSecureSettings();
        secureSettings.setString("s3.client.myconfig.access_key", "myconfig_key");
        secureSettings.setString("s3.client.myconfig.secret_key", "myconfig_secret");
        secureSettings.setString("s3.client.default.access_key", "default_key");
        secureSettings.setString("s3.client.default.secret_key", "default_secret");
        Settings settings = Settings.builder().setSecureSettings(secureSettings).build();

        assertThat(settings.getGroups("s3.client.").keySet(), containsInAnyOrder("myconfig", "default"));
    }

Do you want me to open a new issue about this or should be use another method than getGroups?
I think that this is happening only when we have secure settings only.

LMK where I can help.

[rjernst]

Hrm, it looks like groups are very messed up. Let's address that in a separate issue.