More flexible settings pattern (#123746) (#123759)

This commit reworks the settings globs to be more useable. Primarily it
expands the values so that the settings may be lists, iterating over
each value. Additionally it simplifies the function to also allow
non-glob settings so that this single method may be used to lookup all
values for a given setting pattern, whether it contains a glob or not.

Author: rjernst

libs/entitlement/src/main/java/org/elasticsearch/entitlement/bootstrap/EntitlementBootstrap.java

@@ -35,8 +35,7 @@ public class EntitlementBootstrap {
     public record BootstrapArgs(
         Map<String, Policy> pluginPolicies,
         Function<Class<?>, String> pluginResolver,
-        Function<String, String> settingResolver,
-        Function<String, Stream<String>> settingGlobResolver,
+        Function<String, Stream<String>> settingResolver,
         Path[] dataDirs,
         Path[] sharedRepoDirs,
         Path configDir,
@@ -51,7 +50,6 @@ public record BootstrapArgs(
             requireNonNull(pluginPolicies);
             requireNonNull(pluginResolver);
             requireNonNull(settingResolver);
-            requireNonNull(settingGlobResolver);
             requireNonNull(dataDirs);
             if (dataDirs.length == 0) {
                 throw new IllegalArgumentException("must provide at least one data directory");
@@ -78,8 +76,7 @@ public static BootstrapArgs bootstrapArgs() {
      *
      * @param pluginPolicies a map holding policies for plugins (and modules), by plugin (or module) name.
      * @param pluginResolver a functor to map a Java Class to the plugin it belongs to (the plugin name).
-     * @param settingResolver a functor to resolve the value of an Elasticsearch setting.
-     * @param settingGlobResolver a functor to resolve a glob expression for one or more Elasticsearch settings.
+     * @param settingResolver a functor to resolve a setting name pattern for one or more Elasticsearch settings.
      * @param dataDirs       data directories for Elasticsearch
      * @param sharedRepoDirs       shared repository directories for Elasticsearch
      * @param configDir      the config directory for Elasticsearch
@@ -93,8 +90,7 @@ public static BootstrapArgs bootstrapArgs() {
     public static void bootstrap(
         Map<String, Policy> pluginPolicies,
         Function<Class<?>, String> pluginResolver,
-        Function<String, String> settingResolver,
-        Function<String, Stream<String>> settingGlobResolver,
+        Function<String, Stream<String>> settingResolver,
         Path[] dataDirs,
         Path[] sharedRepoDirs,
         Path configDir,
@@ -113,7 +109,6 @@ public static void bootstrap(
             pluginPolicies,
             pluginResolver,
             settingResolver,
-            settingGlobResolver,
             dataDirs,
             sharedRepoDirs,
             configDir,

libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java

@@ -144,8 +144,7 @@ private static PolicyManager createPolicyManager() {
             bootstrapArgs.dataDirs(),
             bootstrapArgs.sharedRepoDirs(),
             bootstrapArgs.tempDir(),
-            bootstrapArgs.settingResolver(),
-            bootstrapArgs.settingGlobResolver()
+            bootstrapArgs.settingResolver()
         );
 
         List<Scope> serverScopes = new ArrayList<>();

libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PathLookup.java

@@ -19,6 +19,5 @@ public record PathLookup(
     Path[] dataDirs,
     Path[] sharedRepoDirs,
     Path tempDir,
-    Function<String, String> settingResolver,
-    Function<String, Stream<String>> settingGlobResolver
+    Function<String, Stream<String>> settingResolver
 ) {}

libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java

@@ -232,13 +232,7 @@ public PathSettingFileData withExclusive(boolean exclusive) {
 
         @Override
         public Stream<Path> resolveRelativePaths(PathLookup pathLookup) {
-            Stream<String> result;
-            if (setting.contains("*")) {
-                result = pathLookup.settingGlobResolver().apply(setting);
-            } else {
-                String path = pathLookup.settingResolver().apply(setting);
-                result = path == null ? Stream.of() : Stream.of(path);
-            }
+            Stream<String> result = pathLookup.settingResolver().apply(setting);
             if (ignoreUrl) {
                 result = result.filter(s -> s.toLowerCase(Locale.ROOT).startsWith("https://") == false);
             }

libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java

@@ -50,8 +50,7 @@ private static Path path(String s) {
         new Path[] { Path.of("/data1"), Path.of("/data2") },
         new Path[] { Path.of("/shared1"), Path.of("/shared2") },
         Path.of("/tmp"),
-        setting -> settings.get(setting),
-        glob -> settings.getGlobValues(glob)
+        pattern -> settings.getValues(pattern)
     );
 
     public void testEmpty() {

libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java

@@ -71,8 +71,7 @@ public static void beforeClass() {
                 new Path[] { TEST_BASE_DIR.resolve("/data1/"), TEST_BASE_DIR.resolve("/data2") },
                 new Path[] { TEST_BASE_DIR.resolve("/shared1"), TEST_BASE_DIR.resolve("/shared2") },
                 TEST_BASE_DIR.resolve("/temp"),
-                Settings.EMPTY::get,
-                Settings.EMPTY::getGlobValues
+                Settings.EMPTY::getValues
             );
         } catch (Exception e) {
             throw new IllegalStateException(e);

libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlementTests.java

@@ -48,8 +48,7 @@ public static void setupRoot() {
         new Path[] { Path.of("/data1"), Path.of("/data2") },
         new Path[] { Path.of("/shared1"), Path.of("/shared2") },
         Path.of("/tmp"),
-        setting -> settings.get(setting),
-        glob -> settings.getGlobValues(glob)
+        pattern -> settings.getValues(pattern)
     );
 
     public void testEmptyBuild() {

server/src/main/java/org/elasticsearch/bootstrap/Elasticsearch.java

@@ -242,8 +242,7 @@ private static void initPhase2(Bootstrap bootstrap) throws IOException {
             EntitlementBootstrap.bootstrap(
                 pluginPolicies,
                 pluginsResolver::resolveClassToPluginName,
-                nodeEnv.settings()::get,
-                nodeEnv.settings()::getGlobValues,
+                nodeEnv.settings()::getValues,
                 nodeEnv.dataDirs(),
                 nodeEnv.repoDirs(),
                 nodeEnv.configDir(),

server/src/main/java/org/elasticsearch/common/settings/Settings.java

@@ -291,21 +291,25 @@ public String get(String setting, String defaultValue) {
     }
 
     /**
-     * Returns the values for settings that match the given glob pattern.
-     * A single glob is supported.
+     * Returns the values for the given settings pattern.
      *
-     * @param settingGlob setting name containing a glob
-     * @return zero or more values for any settings in this settings object that match the glob pattern
+     * Either a concrete setting name, or a pattern containing a single glob is supported.
+     *
+     * @param settingPattern name of a setting or a setting name pattern containing a glob
+     * @return zero or more values for any settings in this settings object that match the given pattern
      */
-    public Stream<String> getGlobValues(String settingGlob) {
-        int globIndex = settingGlob.indexOf(".*.");
+    public Stream<String> getValues(String settingPattern) {
+        int globIndex = settingPattern.indexOf(".*.");
+        Stream<String> settingNames;
         if (globIndex == -1) {
-            throw new IllegalArgumentException("Pattern [" + settingGlob + "] does not contain a glob [*]");
+            settingNames = Stream.of(settingPattern);
+        } else {
+            String prefix = settingPattern.substring(0, globIndex + 1);
+            String suffix = settingPattern.substring(globIndex + 2);
+            Settings subSettings = getByPrefix(prefix);
+            settingNames = subSettings.names().stream().map(k -> prefix + k + suffix);
         }
-        String prefix = settingGlob.substring(0, globIndex + 1);
-        String suffix = settingGlob.substring(globIndex + 2);
-        Settings subSettings = getByPrefix(prefix);
-        return subSettings.names().stream().map(k -> k + suffix).map(subSettings::get).filter(Objects::nonNull);
+        return settingNames.map(this::getAsList).flatMap(List::stream).filter(Objects::nonNull);
     }
 
     /**

server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java

@@ -44,6 +44,7 @@
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasToString;
 import static org.hamcrest.Matchers.is;
@@ -711,9 +712,21 @@ public void testProcessSetting() throws IOException {
     }
 
     public void testGlobValues() throws IOException {
-        Settings test = Settings.builder().put("foo.x.bar", "1").put("foo.y.bar", "2").build();
-        var values = test.getGlobValues("foo.*.bar").toList();
+        Settings test = Settings.builder().put("foo.x.bar", "1").build();
+
+        // no values
+        assertThat(test.getValues("foo.*.baz").toList(), empty());
+        assertThat(test.getValues("fuz.*.bar").toList(), empty());
+
+        var values = test.getValues("foo.*.bar").toList();
+        assertThat(values, containsInAnyOrder("1"));
+
+        test = Settings.builder().put("foo.x.bar", "1").put("foo.y.bar", "2").build();
+        values = test.getValues("foo.*.bar").toList();
         assertThat(values, containsInAnyOrder("1", "2"));
+
+        values = test.getValues("foo.x.bar").toList();
+        assertThat(values, contains("1"));
     }
 
 }