Skip to content

Consolidate path setting files entitlements to config #123649

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 28, 2025
Merged

Consolidate path setting files entitlements to config #123649

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a45fd61
Consolidate path setting files entitlements to config
rjernst Feb 27, 2025
1a70ebd
iter
rjernst Feb 28, 2025
20c7192
[CI] Auto commit changes from spotless
elasticsearchmachine Feb 28, 2025
6bfcd7d
test
rjernst Feb 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 33 additions & 68 deletions ...main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,8 @@ static FileData ofRelativePath(Path relativePath, BaseDir baseDir, Mode mode) {
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);
}

/**
Expand Down Expand Up @@ -225,71 +221,39 @@ public FileData withPlatform(Platform platform) {
}
}

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("*")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering why we need this asterisk check. Does the settingGlobResolver path do the wrong thing if the setting contains no asterisk?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the setting glob resolver only knows how to deal with glob.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But a glob with no asterisks is still a glob right? Even if it's trivial.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just a way to understand if we want to call the "regular" Settings resolved (which maps to Settings#get) or the glob resolver (which maps to Settings#getGlobValues). The latter throws if you don't pass down something that contains a ".*."
Btw I think we should match that here, and look for ".*.", not just *.

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
public FileData withPlatform(Platform platform) {
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;
Expand Down Expand Up @@ -371,7 +335,7 @@ public static FilesEntitlement build(List<Object> paths) {
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");
Expand All @@ -382,11 +346,10 @@ public static FilesEntitlement build(List<Object> paths) {
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]"
);
}

Expand All @@ -399,20 +362,23 @@ public static FilesEntitlement build(List<Object> paths) {
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");
Expand All @@ -425,12 +391,11 @@ public static FilesEntitlement build(List<Object> paths) {
}
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");
}
Expand Down
4 changes: 2 additions & 2 deletions .../src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserFailureTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public void testEntitlementMutuallyExclusiveParameters() {
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()
);
}
Expand All @@ -89,7 +89,7 @@ public void testEntitlementAtLeastOneParameter() {
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()
);
}
Expand Down
7 changes: 2 additions & 5 deletions ...tlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,7 @@ public void testParseFiles() throws IOException {
- 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",
Expand All @@ -202,8 +200,7 @@ public void testParseFiles() throws IOException {
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")
)
)
)
Expand Down
78 changes: 47 additions & 31 deletions ...java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlementTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,72 +98,88 @@ public void testFileDataRelativeWithEmptyDirectory() {
}

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,
() -> FilesEntitlement.build(
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);
}
}