فهرست منبع

Consolidate path setting files entitlements to config (#123649)

The setting based paths could be either absolute or relative, and they
are always relative to the config dir. This commit renames the
path_setting to make it clear it is related to config, and removes the
relative variant.
Ryan Ernst 7 ماه پیش
والد
کامیت
71f72b9b91

+ 33 - 68
libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java

@@ -91,12 +91,8 @@ public record FilesEntitlement(List<FileData> filesData) implements Entitlement
             return new RelativePathFileData(relativePath, baseDir, mode, null, false);
         }
 
-        static FileData ofPathSetting(String setting, Mode mode, boolean ignoreUrl) {
-            return new PathSettingFileData(setting, mode, ignoreUrl, null, false);
-        }
-
-        static FileData ofRelativePathSetting(String setting, BaseDir baseDir, Mode mode, boolean ignoreUrl) {
-            return new RelativePathSettingFileData(setting, baseDir, mode, ignoreUrl, null, false);
+        static FileData ofPathSetting(String setting, BaseDir baseDir, Mode mode, boolean ignoreUrl) {
+            return new PathSettingFileData(setting, baseDir, mode, ignoreUrl, null, false);
         }
 
         /**
@@ -225,46 +221,28 @@ public record FilesEntitlement(List<FileData> filesData) implements Entitlement
         }
     }
 
-    private record PathSettingFileData(String setting, Mode mode, boolean ignoreUrl, Platform platform, boolean exclusive)
+    private record PathSettingFileData(String setting, BaseDir baseDir, Mode mode, boolean ignoreUrl, Platform platform, boolean exclusive)
         implements
-            FileData {
+            RelativeFileData {
 
         @Override
         public PathSettingFileData withExclusive(boolean exclusive) {
-            return new PathSettingFileData(setting, mode, ignoreUrl, platform, exclusive);
-        }
-
-        @Override
-        public Stream<Path> resolvePaths(PathLookup pathLookup) {
-            return resolvePathSettings(pathLookup, setting, ignoreUrl);
-        }
-
-        @Override
-        public FileData withPlatform(Platform platform) {
-            if (platform == platform()) {
-                return this;
-            }
-            return new PathSettingFileData(setting, mode, ignoreUrl, platform, exclusive);
-        }
-    }
-
-    private record RelativePathSettingFileData(
-        String setting,
-        BaseDir baseDir,
-        Mode mode,
-        boolean ignoreUrl,
-        Platform platform,
-        boolean exclusive
-    ) implements FileData, RelativeFileData {
-
-        @Override
-        public RelativePathSettingFileData withExclusive(boolean exclusive) {
-            return new RelativePathSettingFileData(setting, baseDir, mode, ignoreUrl, platform, exclusive);
+            return new PathSettingFileData(setting, baseDir, mode, ignoreUrl, platform, exclusive);
         }
 
         @Override
         public Stream<Path> resolveRelativePaths(PathLookup pathLookup) {
-            return resolvePathSettings(pathLookup, setting, ignoreUrl);
+            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);
+            }
+            if (ignoreUrl) {
+                result = result.filter(s -> s.toLowerCase(Locale.ROOT).startsWith("https://") == false);
+            }
+            return result.map(pathLookup.configDir()::resolve);
         }
 
         @Override
@@ -272,24 +250,10 @@ public record FilesEntitlement(List<FileData> filesData) implements Entitlement
             if (platform == platform()) {
                 return this;
             }
-            return new RelativePathSettingFileData(setting, baseDir, mode, ignoreUrl, platform, exclusive);
+            return new PathSettingFileData(setting, baseDir, mode, ignoreUrl, platform, exclusive);
         }
     }
 
-    private static Stream<Path> resolvePathSettings(PathLookup pathLookup, String setting, boolean ignoreUrl) {
-        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);
-        }
-        if (ignoreUrl) {
-            result = result.filter(s -> s.toLowerCase(Locale.ROOT).startsWith("https://") == false);
-        }
-        return result.map(Path::of);
-    }
-
     private static Mode parseMode(String mode) {
         if (mode.equals("read")) {
             return Mode.READ;
@@ -371,7 +335,7 @@ public record FilesEntitlement(List<FileData> filesData) implements Entitlement
             String relativePathAsString = checkString.apply(file, "relative_path");
             String relativeTo = checkString.apply(file, "relative_to");
             String pathSetting = checkString.apply(file, "path_setting");
-            String relativePathSetting = checkString.apply(file, "relative_path_setting");
+            String settingBaseDirAsString = checkString.apply(file, "basedir_if_relative");
             String modeAsString = checkString.apply(file, "mode");
             String platformAsString = checkString.apply(file, "platform");
             Boolean ignoreUrlAsStringBoolean = checkBoolean.apply(file, "ignore_url");
@@ -382,11 +346,10 @@ public record FilesEntitlement(List<FileData> filesData) implements Entitlement
             if (file.isEmpty() == false) {
                 throw new PolicyValidationException("unknown key(s) [" + file + "] in a listed file for files entitlement");
             }
-            int foundKeys = (pathAsString != null ? 1 : 0) + (relativePathAsString != null ? 1 : 0) + (pathSetting != null ? 1 : 0)
-                + (relativePathSetting != null ? 1 : 0);
+            int foundKeys = (pathAsString != null ? 1 : 0) + (relativePathAsString != null ? 1 : 0) + (pathSetting != null ? 1 : 0);
             if (foundKeys != 1) {
                 throw new PolicyValidationException(
-                    "a files entitlement entry must contain one of " + "[path, relative_path, path_setting, relative_path_setting]"
+                    "a files entitlement entry must contain one of " + "[path, relative_path, path_setting]"
                 );
             }
 
@@ -399,20 +362,23 @@ public record FilesEntitlement(List<FileData> filesData) implements Entitlement
                 platform = parsePlatform(platformAsString);
             }
 
-            BaseDir baseDir = null;
-            if (relativeTo != null) {
-                baseDir = parseBaseDir(relativeTo);
+            if (relativeTo != null && relativePathAsString == null) {
+                throw new PolicyValidationException("'relative_to' may only be used with 'relative_path'");
             }
 
-            if (ignoreUrlAsStringBoolean != null && (relativePathAsString != null || pathAsString != null)) {
-                throw new PolicyValidationException("'ignore_url' may only be used with `path_setting` or `relative_path_setting`");
+            if (ignoreUrlAsStringBoolean != null && pathSetting == null) {
+                throw new PolicyValidationException("'ignore_url' may only be used with 'path_setting'");
+            }
+            if (settingBaseDirAsString != null && pathSetting == null) {
+                throw new PolicyValidationException("'basedir_if_relative' may only be used with 'path_setting'");
             }
 
             final FileData fileData;
             if (relativePathAsString != null) {
-                if (baseDir == null) {
+                if (relativeTo == null) {
                     throw new PolicyValidationException("files entitlement with a 'relative_path' must specify 'relative_to'");
                 }
+                BaseDir baseDir = parseBaseDir(relativeTo);
                 Path relativePath = Path.of(relativePathAsString);
                 if (FileData.isAbsolutePath(relativePathAsString)) {
                     throw new PolicyValidationException("'relative_path' [" + relativePathAsString + "] must be relative");
@@ -425,12 +391,11 @@ public record FilesEntitlement(List<FileData> filesData) implements Entitlement
                 }
                 fileData = FileData.ofPath(path, mode);
             } else if (pathSetting != null) {
-                fileData = FileData.ofPathSetting(pathSetting, mode, ignoreUrlAsString);
-            } else if (relativePathSetting != null) {
-                if (baseDir == null) {
-                    throw new PolicyValidationException("files entitlement with a 'relative_path_setting' must specify 'relative_to'");
+                if (settingBaseDirAsString == null) {
+                    throw new PolicyValidationException("files entitlement with a 'path_setting' must specify 'basedir_if_relative'");
                 }
-                fileData = FileData.ofRelativePathSetting(relativePathSetting, baseDir, mode, ignoreUrlAsString);
+                BaseDir baseDir = parseBaseDir(settingBaseDirAsString);
+                fileData = FileData.ofPathSetting(pathSetting, baseDir, mode, ignoreUrlAsString);
             } else {
                 throw new AssertionError("File entry validation error");
             }

+ 2 - 2
libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserFailureTests.java

@@ -75,7 +75,7 @@ public class PolicyParserFailureTests extends ESTestCase {
         assertEquals(
             "[2:5] policy parsing error for [test-failure-policy.yaml] in scope [entitlement-module-name] "
                 + "for entitlement type [files]: a files entitlement entry must contain one of "
-                + "[path, relative_path, path_setting, relative_path_setting]",
+                + "[path, relative_path, path_setting]",
             ppe.getMessage()
         );
     }
@@ -89,7 +89,7 @@ public class PolicyParserFailureTests extends ESTestCase {
         assertEquals(
             "[2:5] policy parsing error for [test-failure-policy.yaml] in scope [entitlement-module-name] "
                 + "for entitlement type [files]: a files entitlement entry must contain one of "
-                + "[path, relative_path, path_setting, relative_path_setting]",
+                + "[path, relative_path, path_setting]",
             ppe.getMessage()
         );
     }

+ 2 - 5
libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java

@@ -183,9 +183,7 @@ public class PolicyParserTests extends ESTestCase {
                     - path: '%s'
                       mode: "read_write"
                     - path_setting: foo.bar
-                      mode: read
-                    - relative_path_setting: foo.bar
-                      relative_to: config
+                      basedir_if_relative: config
                       mode: read
                 """, relativePathToFile, relativePathToDir, TEST_ABSOLUTE_PATH_TO_FILE).getBytes(StandardCharsets.UTF_8)),
             "test-policy.yaml",
@@ -202,8 +200,7 @@ public class PolicyParserTests extends ESTestCase {
                                 Map.of("relative_path", relativePathToFile, "mode", "read_write", "relative_to", "data"),
                                 Map.of("relative_path", relativePathToDir, "mode", "read", "relative_to", "config"),
                                 Map.of("path", TEST_ABSOLUTE_PATH_TO_FILE, "mode", "read_write"),
-                                Map.of("path_setting", "foo.bar", "mode", "read"),
-                                Map.of("relative_path_setting", "foo.bar", "relative_to", "config", "mode", "read")
+                                Map.of("path_setting", "foo.bar", "basedir_if_relative", "config", "mode", "read")
                             )
                         )
                     )

+ 47 - 31
libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlementTests.java

@@ -98,65 +98,61 @@ public class FilesEntitlementTests extends ESTestCase {
     }
 
     public void testPathSettingResolve() {
-        var entitlement = FilesEntitlement.build(List.of(Map.of("path_setting", "foo.bar", "mode", "read")));
+        var entitlement = FilesEntitlement.build(
+            List.of(Map.of("path_setting", "foo.bar", "basedir_if_relative", "config", "mode", "read"))
+        );
         var filesData = entitlement.filesData();
-        assertThat(filesData, contains(FileData.ofPathSetting("foo.bar", READ, false)));
+        assertThat(filesData, contains(FileData.ofPathSetting("foo.bar", CONFIG, READ, false)));
 
-        var fileData = FileData.ofPathSetting("foo.bar", READ, false);
+        var fileData = FileData.ofPathSetting("foo.bar", CONFIG, READ, false);
         // empty settings
         assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), empty());
 
-        fileData = FileData.ofPathSetting("foo.bar", READ, false);
+        fileData = FileData.ofPathSetting("foo.bar", CONFIG, READ, false);
         settings = Settings.builder().put("foo.bar", "/setting/path").build();
         assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), contains(Path.of("/setting/path")));
 
-        fileData = FileData.ofPathSetting("foo.*.bar", READ, false);
+        fileData = FileData.ofPathSetting("foo.*.bar", CONFIG, READ, false);
         settings = Settings.builder().put("foo.baz.bar", "/setting/path").build();
         assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), contains(Path.of("/setting/path")));
 
-        fileData = FileData.ofPathSetting("foo.*.bar", READ, false);
+        fileData = FileData.ofPathSetting("foo.*.bar", CONFIG, READ, false);
         settings = Settings.builder().put("foo.baz.bar", "/setting/path").put("foo.baz2.bar", "/other/path").build();
         assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), containsInAnyOrder(Path.of("/setting/path"), Path.of("/other/path")));
+
+        fileData = FileData.ofPathSetting("foo.bar", CONFIG, READ, false);
+        settings = Settings.builder().put("foo.bar", "relative_path").build();
+        assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), contains(Path.of("/config/relative_path")));
     }
 
-    public void testExclusiveParsing() throws Exception {
-        Policy parsedPolicy = new PolicyParser(new ByteArrayInputStream("""
-                    entitlement-module-name:
-                      - files:
-                        - path: /test
-                          mode: read
-                          exclusive: true
-            """.getBytes(StandardCharsets.UTF_8)), "test-policy.yaml", true).parsePolicy();
-        Policy expected = new Policy(
-            "test-policy.yaml",
-            List.of(
-                new Scope(
-                    "entitlement-module-name",
-                    List.of(FilesEntitlement.build(List.of(Map.of("path", "/test", "mode", "read", "exclusive", true))))
-                )
+    public void testPathSettingBasedirValidation() {
+        var e = expectThrows(
+            PolicyValidationException.class,
+            () -> FilesEntitlement.build(List.of(Map.of("path", "/foo", "mode", "read", "basedir_if_relative", "config")))
+        );
+        assertThat(e.getMessage(), is("'basedir_if_relative' may only be used with 'path_setting'"));
+
+        e = expectThrows(
+            PolicyValidationException.class,
+            () -> FilesEntitlement.build(
+                List.of(Map.of("relative_path", "foo", "relative_to", "config", "mode", "read", "basedir_if_relative", "config"))
             )
         );
-        assertEquals(expected, parsedPolicy);
+        assertThat(e.getMessage(), is("'basedir_if_relative' may only be used with 'path_setting'"));
     }
 
     public void testPathSettingIgnoreUrl() {
-        var fileData = FileData.ofPathSetting("foo.*.bar", READ, true);
+        var fileData = FileData.ofPathSetting("foo.*.bar", CONFIG, READ, true);
         settings = Settings.builder().put("foo.nonurl.bar", "/setting/path").put("foo.url.bar", "https://mysite").build();
         assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), contains(Path.of("/setting/path")));
     }
 
-    public void testRelativePathSettingIgnoreUrl() {
-        var fileData = FileData.ofRelativePathSetting("foo.*.bar", CONFIG, READ, true);
-        settings = Settings.builder().put("foo.nonurl.bar", "path").put("foo.url.bar", "https://mysite").build();
-        assertThat(fileData.resolvePaths(TEST_PATH_LOOKUP).toList(), contains(Path.of("/config/path")));
-    }
-
     public void testIgnoreUrlValidation() {
         var e = expectThrows(
             PolicyValidationException.class,
             () -> FilesEntitlement.build(List.of(Map.of("path", "/foo", "mode", "read", "ignore_url", true)))
         );
-        assertThat(e.getMessage(), is("'ignore_url' may only be used with `path_setting` or `relative_path_setting`"));
+        assertThat(e.getMessage(), is("'ignore_url' may only be used with 'path_setting'"));
 
         e = expectThrows(
             PolicyValidationException.class,
@@ -164,6 +160,26 @@ public class FilesEntitlementTests extends ESTestCase {
                 List.of(Map.of("relative_path", "foo", "relative_to", "config", "mode", "read", "ignore_url", true))
             )
         );
-        assertThat(e.getMessage(), is("'ignore_url' may only be used with `path_setting` or `relative_path_setting`"));
+        assertThat(e.getMessage(), is("'ignore_url' may only be used with 'path_setting'"));
+    }
+
+    public void testExclusiveParsing() throws Exception {
+        Policy parsedPolicy = new PolicyParser(new ByteArrayInputStream("""
+                    entitlement-module-name:
+                      - files:
+                        - path: /test
+                          mode: read
+                          exclusive: true
+            """.getBytes(StandardCharsets.UTF_8)), "test-policy.yaml", true).parsePolicy();
+        Policy expected = new Policy(
+            "test-policy.yaml",
+            List.of(
+                new Scope(
+                    "entitlement-module-name",
+                    List.of(FilesEntitlement.build(List.of(Map.of("path", "/test", "mode", "read", "exclusive", true))))
+                )
+            )
+        );
+        assertEquals(expected, parsedPolicy);
     }
 }