Skip to content

Commit 24441ac

Browse files
ldematteldematte
committed
Addressing PR comments
1 parent 447e9f2 commit 24441ac

File tree

9 files changed

+71
-165
lines changed

9 files changed

+71
-165
lines changed

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

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
import org.elasticsearch.entitlement.instrumentation.MethodKey;
1919
import org.elasticsearch.entitlement.instrumentation.Transformer;
2020
import org.elasticsearch.entitlement.runtime.api.ElasticsearchEntitlementChecker;
21-
import org.elasticsearch.entitlement.runtime.policy.DirectoryResolver;
21+
import org.elasticsearch.entitlement.runtime.policy.PathLookup;
2222
import org.elasticsearch.entitlement.runtime.policy.Policy;
2323
import org.elasticsearch.entitlement.runtime.policy.PolicyManager;
2424
import org.elasticsearch.entitlement.runtime.policy.Scope;
@@ -130,7 +130,7 @@ private static PolicyManager createPolicyManager() {
130130
Map<String, Policy> pluginPolicies = bootstrapArgs.pluginPolicies();
131131
Path[] dataDirs = EntitlementBootstrap.bootstrapArgs().dataDirs();
132132

133-
var directoryResolver = new EnvironmentDirectoryResolver(bootstrapArgs);
133+
var pathLookup = new PathLookup(bootstrapArgs.configDir(), bootstrapArgs.dataDirs(), bootstrapArgs.configDir());
134134

135135
// TODO(ES-10031): Decide what goes in the elasticsearch default policy and extend it
136136
var serverPolicy = new Policy(
@@ -175,7 +175,7 @@ private static PolicyManager createPolicyManager() {
175175
resolver,
176176
AGENTS_PACKAGE_NAME,
177177
ENTITLEMENTS_MODULE,
178-
directoryResolver
178+
pathLookup
179179
);
180180
}
181181

@@ -320,27 +320,4 @@ private static ElasticsearchEntitlementChecker initChecker() {
320320
"org.elasticsearch.entitlement.instrumentation",
321321
Set.of()
322322
).get();
323-
324-
static class EnvironmentDirectoryResolver implements DirectoryResolver {
325-
private final EntitlementBootstrap.BootstrapArgs bootstrapArgs;
326-
327-
EnvironmentDirectoryResolver(EntitlementBootstrap.BootstrapArgs bootstrapArgs) {
328-
this.bootstrapArgs = bootstrapArgs;
329-
}
330-
331-
@Override
332-
public Path resolveConfig(Path path) {
333-
return bootstrapArgs.configDir().resolve(path);
334-
}
335-
336-
@Override
337-
public Stream<Path> resolveData(Path path) {
338-
return Arrays.stream(bootstrapArgs.dataDirs()).map(dir -> dir.resolve(path));
339-
}
340-
341-
@Override
342-
public Path resolveTemp(Path path) {
343-
return bootstrapArgs.tempDir().resolve(path);
344-
}
345-
}
346323
}

‎libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java‎

Lines changed: 13 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -17,49 +17,27 @@
1717
import java.util.List;
1818
import java.util.Objects;
1919
import java.util.function.Consumer;
20-
import java.util.stream.Stream;
2120

2221
import static org.elasticsearch.core.PathUtils.getDefaultFileSystem;
2322

2423
public final class FileAccessTree {
2524

26-
private static final DirectoryResolver NULL_DIRECTORY_RESOLVER = new DirectoryResolver() {
27-
@Override
28-
public Path resolveConfig(Path path) {
29-
throw new UnsupportedOperationException();
30-
}
31-
32-
@Override
33-
public Stream<Path> resolveData(Path path) {
34-
throw new UnsupportedOperationException();
35-
}
36-
37-
@Override
38-
public Path resolveTemp(Path path) {
39-
throw new UnsupportedOperationException();
40-
}
41-
};
42-
43-
public static final FileAccessTree EMPTY = new FileAccessTree(FilesEntitlement.EMPTY, NULL_DIRECTORY_RESOLVER);
25+
public static final FileAccessTree EMPTY = new FileAccessTree(FilesEntitlement.EMPTY, null);
4426
private static final String FILE_SEPARATOR = getDefaultFileSystem().getSeparator();
4527

4628
private final String[] readPaths;
4729
private final String[] writePaths;
4830

49-
private static void resolvePath(
50-
FilesEntitlement.FileData fileData,
51-
DirectoryResolver directoryResolver,
52-
Consumer<String> resolvedPathReceiver
53-
) {
31+
private static void resolvePath(FilesEntitlement.FileData fileData, PathLookup pathLookup, Consumer<Path> resolvedPathReceiver) {
5432
if (fileData.path() != null) {
55-
resolvedPathReceiver.accept(normalizePath(fileData.path()));
56-
} else if (fileData.relativePath() != null && fileData.baseDir() != null) {
33+
resolvedPathReceiver.accept(fileData.path());
34+
} else if (fileData.relativePath() != null && fileData.baseDir() != null && pathLookup != null) {
5735
switch (fileData.baseDir()) {
5836
case CONFIG:
59-
resolvedPathReceiver.accept(normalizePath(directoryResolver.resolveConfig(fileData.relativePath())));
37+
resolvedPathReceiver.accept(pathLookup.configDir().resolve(fileData.relativePath()));
6038
break;
6139
case DATA:
62-
directoryResolver.resolveData(fileData.relativePath()).forEach(p -> resolvedPathReceiver.accept(normalizePath(p)));
40+
Arrays.stream(pathLookup.dataDirs()).map(d -> d.resolve(fileData.relativePath())).forEach(resolvedPathReceiver::accept);
6341
break;
6442
default:
6543
throw new IllegalArgumentException();
@@ -69,18 +47,17 @@ private static void resolvePath(
6947
}
7048
}
7149

72-
private FileAccessTree(FilesEntitlement filesEntitlement, DirectoryResolver directoryResolver) {
73-
Objects.requireNonNull(directoryResolver);
74-
50+
private FileAccessTree(FilesEntitlement filesEntitlement, PathLookup pathLookup) {
7551
List<String> readPaths = new ArrayList<>();
7652
List<String> writePaths = new ArrayList<>();
7753
for (FilesEntitlement.FileData fileData : filesEntitlement.filesData()) {
7854
var mode = fileData.mode();
79-
resolvePath(fileData, directoryResolver, pathStr -> {
55+
resolvePath(fileData, pathLookup, path -> {
56+
var normalized = normalizePath(path);
8057
if (mode == FilesEntitlement.Mode.READ_WRITE) {
81-
writePaths.add(pathStr);
58+
writePaths.add(normalized);
8259
}
83-
readPaths.add(pathStr);
60+
readPaths.add(normalized);
8461
});
8562
}
8663

@@ -91,8 +68,8 @@ private FileAccessTree(FilesEntitlement filesEntitlement, DirectoryResolver dire
9168
this.writePaths = writePaths.toArray(new String[0]);
9269
}
9370

94-
public static FileAccessTree of(FilesEntitlement filesEntitlement, DirectoryResolver directoryResolver) {
95-
return new FileAccessTree(filesEntitlement, directoryResolver);
71+
public static FileAccessTree of(FilesEntitlement filesEntitlement, PathLookup pathLookup) {
72+
return new FileAccessTree(filesEntitlement, pathLookup);
9673
}
9774

9875
boolean canRead(Path path) {

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,5 @@
1010
package org.elasticsearch.entitlement.runtime.policy;
1111

1212
import java.nio.file.Path;
13-
import java.util.stream.Stream;
1413

15-
public interface DirectoryResolver {
16-
Path resolveConfig(Path path);
17-
18-
Stream<Path> resolveData(Path path);
19-
20-
Path resolveTemp(Path path);
21-
}
14+
public record PathLookup(Path configDir, Path[] dataDirs, Path tempDir) {}

‎libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public static ModuleEntitlements none(String componentName) {
7373
return new ModuleEntitlements(componentName, Map.of(), FileAccessTree.EMPTY);
7474
}
7575

76-
public static ModuleEntitlements from(String componentName, List<Entitlement> entitlements, DirectoryResolver directoryResolver) {
76+
public static ModuleEntitlements from(String componentName, List<Entitlement> entitlements, PathLookup pathLookup) {
7777
FilesEntitlement filesEntitlement = FilesEntitlement.EMPTY;
7878
for (Entitlement entitlement : entitlements) {
7979
if (entitlement instanceof FilesEntitlement) {
@@ -83,7 +83,7 @@ public static ModuleEntitlements from(String componentName, List<Entitlement> en
8383
return new ModuleEntitlements(
8484
componentName,
8585
entitlements.stream().collect(groupingBy(Entitlement::getClass)),
86-
FileAccessTree.of(filesEntitlement, directoryResolver)
86+
FileAccessTree.of(filesEntitlement, pathLookup)
8787
);
8888
}
8989

@@ -106,7 +106,7 @@ public <E extends Entitlement> Stream<E> getEntitlements(Class<E> entitlementCla
106106
protected final List<Entitlement> apmAgentEntitlements;
107107
protected final Map<String, Map<String, List<Entitlement>>> pluginsEntitlements;
108108
private final Function<Class<?>, String> pluginResolver;
109-
private final DirectoryResolver directoryResolver;
109+
private final PathLookup pathLookup;
110110

111111
public static final String ALL_UNNAMED = "ALL-UNNAMED";
112112

@@ -142,7 +142,7 @@ public PolicyManager(
142142
Function<Class<?>, String> pluginResolver,
143143
String apmAgentPackageName,
144144
Module entitlementsModule,
145-
DirectoryResolver directoryResolver
145+
PathLookup pathLookup
146146
) {
147147
this.serverEntitlements = buildScopeEntitlementsMap(requireNonNull(serverPolicy));
148148
this.apmAgentEntitlements = apmAgentEntitlements;
@@ -152,7 +152,7 @@ public PolicyManager(
152152
this.pluginResolver = pluginResolver;
153153
this.apmAgentPackageName = apmAgentPackageName;
154154
this.entitlementsModule = entitlementsModule;
155-
this.directoryResolver = directoryResolver;
155+
this.pathLookup = pathLookup;
156156

157157
for (var e : serverEntitlements.entrySet()) {
158158
validateEntitlementsPerModule(SERVER_COMPONENT_NAME, e.getKey(), e.getValue());
@@ -431,7 +431,7 @@ private ModuleEntitlements computeEntitlements(Class<?> requestingClass) {
431431

432432
if (requestingModule.isNamed() == false && requestingClass.getPackageName().startsWith(apmAgentPackageName)) {
433433
// The APM agent is the only thing running non-modular in the system classloader
434-
return ModuleEntitlements.from(APM_AGENT_COMPONENT_NAME, apmAgentEntitlements, directoryResolver);
434+
return ModuleEntitlements.from(APM_AGENT_COMPONENT_NAME, apmAgentEntitlements, pathLookup);
435435
}
436436

437437
return ModuleEntitlements.none(UNKNOWN_COMPONENT_NAME);
@@ -446,7 +446,7 @@ private ModuleEntitlements getModuleScopeEntitlements(
446446
if (entitlements == null) {
447447
return ModuleEntitlements.none(componentName);
448448
}
449-
return ModuleEntitlements.from(componentName, entitlements, directoryResolver);
449+
return ModuleEntitlements.from(componentName, entitlements, pathLookup);
450450
}
451451

452452
private static boolean isServerModule(Module requestingModule) {

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,26 +131,29 @@ public static FilesEntitlement build(List<Object> paths) {
131131
if (mode == null) {
132132
throw new PolicyValidationException("files entitlement must contain 'mode' for every listed file");
133133
}
134-
if ((pathAsString == null && relativePathAsString == null) || (pathAsString != null && relativePathAsString != null)) {
135-
throw new PolicyValidationException("files entitlement must contain either 'path' or 'relative_path' for every entry");
134+
if (pathAsString != null && relativePathAsString != null) {
135+
throw new PolicyValidationException("a files entitlement entry cannot contain both 'path' and 'relative_path'");
136136
}
137+
137138
if (relativePathAsString != null) {
138139
if (relativeTo == null) {
139140
throw new PolicyValidationException("files entitlement with a 'relative_path' must specify 'relative_to'");
140141
}
141-
final var baseDir = parseBaseDir(relativeTo);
142+
final BaseDir baseDir = parseBaseDir(relativeTo);
142143

143144
Path relativePath = Path.of(relativePathAsString);
144145
if (relativePath.isAbsolute()) {
145146
throw new PolicyValidationException("'relative_path' must be relative");
146147
}
147148
filesData.add(FileData.ofRelativePath(relativePath, baseDir, parseMode(mode)));
148-
} else {
149+
} else if (pathAsString != null) {
149150
Path path = Path.of(pathAsString);
150151
if (path.isAbsolute() == false) {
151152
throw new PolicyValidationException("'path' must be absolute");
152153
}
153154
filesData.add(FileData.ofPath(path, parseMode(mode)));
155+
} else {
156+
throw new PolicyValidationException("files entitlement must contain either 'path' or 'relative_path' for every entry");
154157
}
155158
}
156159
return new FilesEntitlement(filesData);

‎libs/entitlement/src/test/java/org/elasticsearch/entitlement/initialization/EnvironmentDirectoryResolverTests.java‎

Lines changed: 0 additions & 53 deletions
This file was deleted.

0 commit comments

Comments
 (0)