Skip to content

Commit 1d275ab

Browse files
authored
Update policy parser to allow static methods for entitlement creation (elastic#121706) (elastic#121796)
This updates the PolicyParser to allow static methods to have an ExternalEntitlement annotation. This removes a limitation where constructors cannot properly support type-erasure with different types of data structures for internal entitlement generation and external entitlement generation (for example List<Object> from the parser and List<SomeData> from an internal builder). We continue to enforce that only one constructor/method may be annotated with ExternalEntitlement per Entitlement class.
1 parent 21db843 commit 1d275ab

File tree

5 files changed

+122
-9
lines changed

5 files changed

+122
-9
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
* using this annotation is considered parseable as part of a policy file
2323
* for entitlements.
2424
*/
25-
@Target(ElementType.CONSTRUCTOR)
25+
@Target({ ElementType.CONSTRUCTOR, ElementType.METHOD })
2626
@Retention(RetentionPolicy.RUNTIME)
2727
public @interface ExternalEntitlement {
2828

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

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import java.io.UncheckedIOException;
2828
import java.lang.reflect.Constructor;
2929
import java.lang.reflect.InvocationTargetException;
30+
import java.lang.reflect.Method;
31+
import java.lang.reflect.Modifier;
3032
import java.util.ArrayList;
3133
import java.util.Arrays;
3234
import java.util.List;
@@ -147,6 +149,7 @@ protected Entitlement parseEntitlement(String scopeName, String entitlementType)
147149
}
148150

149151
Constructor<?> entitlementConstructor = null;
152+
Method entitlementMethod = null;
150153
ExternalEntitlement entitlementMetadata = null;
151154
for (var ctor : entitlementClass.getConstructors()) {
152155
var metadata = ctor.getAnnotation(ExternalEntitlement.class);
@@ -161,8 +164,27 @@ protected Entitlement parseEntitlement(String scopeName, String entitlementType)
161164
entitlementConstructor = ctor;
162165
entitlementMetadata = metadata;
163166
}
164-
165167
}
168+
for (var method : entitlementClass.getMethods()) {
169+
var metadata = method.getAnnotation(ExternalEntitlement.class);
170+
if (metadata != null) {
171+
if (Modifier.isStatic(method.getModifiers()) == false) {
172+
throw new IllegalStateException(
173+
"entitlement class [" + entitlementClass.getName() + "] has non-static method annotated with ExternalEntitlement"
174+
);
175+
}
176+
if (entitlementMetadata != null) {
177+
throw new IllegalStateException(
178+
"entitlement class ["
179+
+ entitlementClass.getName()
180+
+ "] has more than one constructor and/or method annotated with ExternalEntitlement"
181+
);
182+
}
183+
entitlementMethod = method;
184+
entitlementMetadata = metadata;
185+
}
186+
}
187+
166188
if (entitlementMetadata == null) {
167189
throw newPolicyParserException(scopeName, "unknown entitlement type [" + entitlementType + "]");
168190
}
@@ -171,7 +193,9 @@ protected Entitlement parseEntitlement(String scopeName, String entitlementType)
171193
throw newPolicyParserException("entitlement type [" + entitlementType + "] is allowed only on modules");
172194
}
173195

174-
Class<?>[] parameterTypes = entitlementConstructor.getParameterTypes();
196+
Class<?>[] parameterTypes = entitlementConstructor != null
197+
? entitlementConstructor.getParameterTypes()
198+
: entitlementMethod.getParameterTypes();
175199
String[] parametersNames = entitlementMetadata.parameterNames();
176200

177201
if (parameterTypes.length != 0 || parametersNames.length != 0) {
@@ -204,7 +228,11 @@ protected Entitlement parseEntitlement(String scopeName, String entitlementType)
204228
}
205229

206230
try {
207-
return (Entitlement) entitlementConstructor.newInstance(parameterValues);
231+
if (entitlementConstructor != null) {
232+
return (Entitlement) entitlementConstructor.newInstance(parameterValues);
233+
} else {
234+
return (Entitlement) entitlementMethod.invoke(null, parameterValues);
235+
}
208236
} catch (InvocationTargetException | InstantiationException | IllegalAccessException e) {
209237
if (e.getCause() instanceof PolicyValidationException piae) {
210238
throw newPolicyParserException(startLocation, scopeName, entitlementType, piae);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ private static Mode parseMode(String mode) {
4343
}
4444

4545
@ExternalEntitlement(parameterNames = { "path", "mode" }, esModulesOnly = false)
46-
public FileEntitlement(String path, String mode) {
47-
this(path, parseMode(mode));
46+
public static FileEntitlement create(String path, String mode) {
47+
return new FileEntitlement(path, parseMode(mode));
4848
}
4949
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,6 @@ public void testNormalizePath() {
8585

8686
FileEntitlement entitlement(String path, String mode) {
8787
Path p = path(path);
88-
return new FileEntitlement(p.toString(), mode);
88+
return FileEntitlement.create(p.toString(), mode);
8989
}
9090
}

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

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,35 @@ public ManyConstructorsEntitlement(String s) {}
4040
public ManyConstructorsEntitlement(int i) {}
4141
}
4242

43+
public static class ManyMethodsEntitlement implements Entitlement {
44+
@ExternalEntitlement
45+
public static ManyMethodsEntitlement create(String s) {
46+
return new ManyMethodsEntitlement();
47+
}
48+
49+
@ExternalEntitlement
50+
public static ManyMethodsEntitlement create(int i) {
51+
return new ManyMethodsEntitlement();
52+
}
53+
}
54+
55+
public static class ConstructorAndMethodEntitlement implements Entitlement {
56+
@ExternalEntitlement
57+
public static ConstructorAndMethodEntitlement create(String s) {
58+
return new ConstructorAndMethodEntitlement(s);
59+
}
60+
61+
@ExternalEntitlement
62+
public ConstructorAndMethodEntitlement(String s) {}
63+
}
64+
65+
public static class NonStaticMethodEntitlement implements Entitlement {
66+
@ExternalEntitlement
67+
public NonStaticMethodEntitlement create() {
68+
return new NonStaticMethodEntitlement();
69+
}
70+
}
71+
4372
public void testGetEntitlementTypeName() {
4473
assertEquals("create_class_loader", PolicyParser.getEntitlementTypeName(CreateClassLoaderEntitlement.class));
4574

@@ -55,7 +84,7 @@ public void testPolicyBuilder() throws IOException {
5584
.parsePolicy();
5685
Policy expected = new Policy(
5786
"test-policy.yaml",
58-
List.of(new Scope("entitlement-module-name", List.of(new FileEntitlement("test/path/to/file", "read_write"))))
87+
List.of(new Scope("entitlement-module-name", List.of(FileEntitlement.create("test/path/to/file", "read_write"))))
5988
);
6089
assertEquals(expected, parsedPolicy);
6190
}
@@ -65,7 +94,7 @@ public void testPolicyBuilderOnExternalPlugin() throws IOException {
6594
.parsePolicy();
6695
Policy expected = new Policy(
6796
"test-policy.yaml",
68-
List.of(new Scope("entitlement-module-name", List.of(new FileEntitlement("test/path/to/file", "read_write"))))
97+
List.of(new Scope("entitlement-module-name", List.of(FileEntitlement.create("test/path/to/file", "read_write"))))
6998
);
7099
assertEquals(expected, parsedPolicy);
71100
}
@@ -174,4 +203,60 @@ public void testMultipleConstructorsAnnotated() throws IOException {
174203
)
175204
);
176205
}
206+
207+
public void testMultipleMethodsAnnotated() throws IOException {
208+
var parser = new PolicyParser(new ByteArrayInputStream("""
209+
entitlement-module-name:
210+
- many_methods
211+
""".getBytes(StandardCharsets.UTF_8)), "test-policy.yaml", true, Map.of("many_methods", ManyMethodsEntitlement.class));
212+
213+
var e = expectThrows(IllegalStateException.class, parser::parsePolicy);
214+
assertThat(
215+
e.getMessage(),
216+
equalTo(
217+
"entitlement class "
218+
+ "[org.elasticsearch.entitlement.runtime.policy.PolicyParserTests$ManyMethodsEntitlement]"
219+
+ " has more than one constructor and/or method annotated with ExternalEntitlement"
220+
)
221+
);
222+
}
223+
224+
public void testConstructorAndMethodAnnotated() throws IOException {
225+
var parser = new PolicyParser(
226+
new ByteArrayInputStream("""
227+
entitlement-module-name:
228+
- constructor_and_method
229+
""".getBytes(StandardCharsets.UTF_8)),
230+
"test-policy.yaml",
231+
true,
232+
Map.of("constructor_and_method", ConstructorAndMethodEntitlement.class)
233+
);
234+
235+
var e = expectThrows(IllegalStateException.class, parser::parsePolicy);
236+
assertThat(
237+
e.getMessage(),
238+
equalTo(
239+
"entitlement class "
240+
+ "[org.elasticsearch.entitlement.runtime.policy.PolicyParserTests$ConstructorAndMethodEntitlement]"
241+
+ " has more than one constructor and/or method annotated with ExternalEntitlement"
242+
)
243+
);
244+
}
245+
246+
public void testNonStaticMethodAnnotated() throws IOException {
247+
var parser = new PolicyParser(new ByteArrayInputStream("""
248+
entitlement-module-name:
249+
- non_static
250+
""".getBytes(StandardCharsets.UTF_8)), "test-policy.yaml", true, Map.of("non_static", NonStaticMethodEntitlement.class));
251+
252+
var e = expectThrows(IllegalStateException.class, parser::parsePolicy);
253+
assertThat(
254+
e.getMessage(),
255+
equalTo(
256+
"entitlement class "
257+
+ "[org.elasticsearch.entitlement.runtime.policy.PolicyParserTests$NonStaticMethodEntitlement]"
258+
+ " has non-static method annotated with ExternalEntitlement"
259+
)
260+
);
261+
}
177262
}

0 commit comments

Comments
 (0)