Skip to content

Commit 4cf122c

Browse files
ldematteldematte
committed
PR comments
1 parent c77ee2b commit 4cf122c

File tree

6 files changed

+38
-22
lines changed

6 files changed

+38
-22
lines changed

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@
4949
import java.nio.file.attribute.FileAttribute;
5050
import java.nio.file.spi.FileSystemProvider;
5151
import java.util.ArrayList;
52-
import java.util.Arrays;
5352
import java.util.HashMap;
5453
import java.util.List;
5554
import java.util.Map;
@@ -162,7 +161,7 @@ private static PolicyManager createPolicyManager() {
162161
"org.elasticsearch.nativeaccess",
163162
List.of(
164163
new LoadNativeLibrariesEntitlement(),
165-
new FilesEntitlement(Arrays.stream(dataDirs).map(d -> FileData.ofPath(d, READ_WRITE)).toList())
164+
new FilesEntitlement(List.of(FileData.ofRelativePath(Path.of(""), FilesEntitlement.BaseDir.DATA, READ_WRITE)))
166165
)
167166
)
168167
)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ public PolicyManager(
153153
this.pluginResolver = pluginResolver;
154154
this.apmAgentPackageName = apmAgentPackageName;
155155
this.entitlementsModule = entitlementsModule;
156-
this.pathLookup = pathLookup;
156+
this.pathLookup = requireNonNull(pathLookup);
157157

158158
for (var e : serverEntitlements.entrySet()) {
159159
validateEntitlementsPerModule(SERVER_COMPONENT_NAME, e.getKey(), e.getValue());

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,13 +185,13 @@ public static FilesEntitlement build(List<Object> paths) {
185185

186186
Path relativePath = Path.of(relativePathAsString);
187187
if (relativePath.isAbsolute()) {
188-
throw new PolicyValidationException("'relative_path' must be relative");
188+
throw new PolicyValidationException("'relative_path' [" + relativePathAsString + "] must be relative");
189189
}
190190
filesData.add(FileData.ofRelativePath(relativePath, baseDir, parseMode(mode)));
191191
} else if (pathAsString != null) {
192192
Path path = Path.of(pathAsString);
193193
if (path.isAbsolute() == false) {
194-
throw new PolicyValidationException("'path' must be absolute");
194+
throw new PolicyValidationException("'path' [" + pathAsString + "] must be absolute");
195195
}
196196
filesData.add(FileData.ofPath(path, parseMode(mode)));
197197
} else {

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

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ public class PolicyManagerTests extends ESTestCase {
5353
*/
5454
private static Module NO_ENTITLEMENTS_MODULE;
5555

56+
private static final PathLookup TEST_PATH_LOOKUP = new PathLookup(
57+
Path.of("/config"),
58+
new Path[] { Path.of("/data1/"), Path.of("/data2") },
59+
Path.of("/temp")
60+
);
61+
5662
@BeforeClass
5763
public static void beforeClass() {
5864
try {
@@ -61,7 +67,6 @@ public static void beforeClass() {
6167
} catch (Exception e) {
6268
throw new IllegalStateException(e);
6369
}
64-
6570
}
6671

6772
public void testGetEntitlementsThrowsOnMissingPluginUnnamedModule() {
@@ -72,7 +77,7 @@ public void testGetEntitlementsThrowsOnMissingPluginUnnamedModule() {
7277
c -> "plugin1",
7378
TEST_AGENTS_PACKAGE_NAME,
7479
NO_ENTITLEMENTS_MODULE,
75-
null
80+
TEST_PATH_LOOKUP
7681
);
7782

7883
// Any class from the current module (unnamed) will do
@@ -92,7 +97,7 @@ public void testGetEntitlementsThrowsOnMissingPolicyForPlugin() {
9297
c -> "plugin1",
9398
TEST_AGENTS_PACKAGE_NAME,
9499
NO_ENTITLEMENTS_MODULE,
95-
null
100+
TEST_PATH_LOOKUP
96101
);
97102

98103
// Any class from the current module (unnamed) will do
@@ -112,7 +117,7 @@ public void testGetEntitlementsFailureIsCached() {
112117
c -> "plugin1",
113118
TEST_AGENTS_PACKAGE_NAME,
114119
NO_ENTITLEMENTS_MODULE,
115-
null
120+
TEST_PATH_LOOKUP
116121
);
117122

118123
// Any class from the current module (unnamed) will do
@@ -137,7 +142,7 @@ public void testGetEntitlementsReturnsEntitlementsForPluginUnnamedModule() {
137142
c -> "plugin2",
138143
TEST_AGENTS_PACKAGE_NAME,
139144
NO_ENTITLEMENTS_MODULE,
140-
null
145+
TEST_PATH_LOOKUP
141146
);
142147

143148
// Any class from the current module (unnamed) will do
@@ -155,7 +160,7 @@ public void testGetEntitlementsThrowsOnMissingPolicyForServer() throws ClassNotF
155160
c -> null,
156161
TEST_AGENTS_PACKAGE_NAME,
157162
NO_ENTITLEMENTS_MODULE,
158-
null
163+
TEST_PATH_LOOKUP
159164
);
160165

161166
// Tests do not run modular, so we cannot use a server class.
@@ -182,7 +187,7 @@ public void testGetEntitlementsReturnsEntitlementsForServerModule() throws Class
182187
c -> null,
183188
TEST_AGENTS_PACKAGE_NAME,
184189
NO_ENTITLEMENTS_MODULE,
185-
null
190+
TEST_PATH_LOOKUP
186191
);
187192

188193
// Tests do not run modular, so we cannot use a server class.
@@ -208,7 +213,7 @@ public void testGetEntitlementsReturnsEntitlementsForPluginModule() throws IOExc
208213
c -> "mock-plugin",
209214
TEST_AGENTS_PACKAGE_NAME,
210215
NO_ENTITLEMENTS_MODULE,
211-
null
216+
TEST_PATH_LOOKUP
212217
);
213218

214219
var layer = createLayerForJar(jar, "org.example.plugin");
@@ -228,7 +233,7 @@ public void testGetEntitlementsResultIsCached() {
228233
c -> "plugin2",
229234
TEST_AGENTS_PACKAGE_NAME,
230235
NO_ENTITLEMENTS_MODULE,
231-
null
236+
TEST_PATH_LOOKUP
232237
);
233238

234239
// Any class from the current module (unnamed) will do
@@ -287,7 +292,7 @@ public void testAgentsEntitlements() throws IOException, ClassNotFoundException
287292
c -> c.getPackageName().startsWith(TEST_AGENTS_PACKAGE_NAME) ? null : "test",
288293
TEST_AGENTS_PACKAGE_NAME,
289294
NO_ENTITLEMENTS_MODULE,
290-
null
295+
TEST_PATH_LOOKUP
291296
);
292297
ModuleEntitlements agentsEntitlements = policyManager.getEntitlements(TestAgent.class);
293298
assertThat(agentsEntitlements.hasEntitlement(CreateClassLoaderEntitlement.class), is(true));
@@ -315,7 +320,7 @@ public void testDuplicateEntitlements() {
315320
c -> "test",
316321
TEST_AGENTS_PACKAGE_NAME,
317322
NO_ENTITLEMENTS_MODULE,
318-
null
323+
TEST_PATH_LOOKUP
319324
)
320325
);
321326
assertEquals(
@@ -332,7 +337,7 @@ public void testDuplicateEntitlements() {
332337
c -> "test",
333338
TEST_AGENTS_PACKAGE_NAME,
334339
NO_ENTITLEMENTS_MODULE,
335-
null
340+
TEST_PATH_LOOKUP
336341
)
337342
);
338343
assertEquals(
@@ -366,7 +371,7 @@ public void testDuplicateEntitlements() {
366371
c -> "plugin1",
367372
TEST_AGENTS_PACKAGE_NAME,
368373
NO_ENTITLEMENTS_MODULE,
369-
null
374+
TEST_PATH_LOOKUP
370375
)
371376
);
372377
assertEquals(
@@ -386,7 +391,7 @@ public void testPluginResolverOverridesAgents() {
386391
c -> "test", // Insist that the class is in a plugin
387392
TEST_AGENTS_PACKAGE_NAME,
388393
NO_ENTITLEMENTS_MODULE,
389-
null
394+
TEST_PATH_LOOKUP
390395
);
391396
ModuleEntitlements notAgentsEntitlements = policyManager.getEntitlements(TestAgent.class);
392397
assertThat(notAgentsEntitlements.hasEntitlement(CreateClassLoaderEntitlement.class), is(false));
@@ -407,7 +412,7 @@ private static PolicyManager policyManager(String agentsPackageName, Module enti
407412
c -> "test",
408413
agentsPackageName,
409414
entitlementsModule,
410-
null
415+
TEST_PATH_LOOKUP
411416
);
412417
}
413418

‎libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserFailureTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public void testEntitlementRelativePathWhenAbsolute() {
7373
""".getBytes(StandardCharsets.UTF_8)), "test-failure-policy.yaml", false).parsePolicy());
7474
assertEquals(
7575
"[2:5] policy parsing error for [test-failure-policy.yaml] in scope [entitlement-module-name] "
76-
+ "for entitlement type [files]: 'path' must be absolute",
76+
+ "for entitlement type [files]: 'path' [test-path] must be absolute",
7777
ppe.getMessage()
7878
);
7979
}
@@ -88,7 +88,7 @@ public void testEntitlementAbsolutePathWhenRelative() {
8888
""".getBytes(StandardCharsets.UTF_8)), "test-failure-policy.yaml", false).parsePolicy());
8989
assertEquals(
9090
"[2:5] policy parsing error for [test-failure-policy.yaml] in scope [entitlement-module-name] "
91-
+ "for entitlement type [files]: 'relative_path' must be relative",
91+
+ "for entitlement type [files]: 'relative_path' [/test-path] must be relative",
9292
ppe.getMessage()
9393
);
9494
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,16 @@
99

1010
package org.elasticsearch.entitlement.runtime.policy.entitlements;
1111

12+
import org.elasticsearch.entitlement.runtime.policy.PathLookup;
1213
import org.elasticsearch.entitlement.runtime.policy.PolicyValidationException;
1314
import org.elasticsearch.test.ESTestCase;
1415

16+
import java.nio.file.Path;
1517
import java.util.List;
1618
import java.util.Map;
1719

20+
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ_WRITE;
21+
import static org.hamcrest.Matchers.contains;
1822
import static org.hamcrest.Matchers.is;
1923

2024
public class FilesEntitlementTests extends ESTestCase {
@@ -33,4 +37,12 @@ public void testInvalidRelativeDirectory() {
3337
);
3438
assertThat(ex.getMessage(), is("invalid relative directory: bar, valid values: [config, data]"));
3539
}
40+
41+
public void testFileDataRelativeWithEmptyDirectory() {
42+
var fileData = FilesEntitlement.FileData.ofRelativePath(Path.of(""), FilesEntitlement.BaseDir.DATA, READ_WRITE);
43+
var dataDirs = fileData.resolvePaths(
44+
new PathLookup(Path.of("/config"), new Path[] { Path.of("/data1/"), Path.of("/data2") }, Path.of("/temp"))
45+
);
46+
assertThat(dataDirs.toList(), contains(Path.of("/data1/"), Path.of("/data2")));
47+
}
3648
}

0 commit comments

Comments
 (0)