From a3dc1ef2c6dde0b6c422ef20448736310b0de598 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 21 Mar 2025 15:26:23 -0700 Subject: [PATCH 1/3] Simplify entitlement rest test discovery This commit cleans up how entitlement test methods are discovered. It also adds another robustness check to ensure an annotation doesn't exist on a private method. --- .../qa/test/RestEntitlementsCheckAction.java | 72 +++++++++++-------- 1 file changed, 41 insertions(+), 31 deletions(-) diff --git a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java index 56b2850d35ec4..394784548b4fc 100644 --- a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java +++ b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java @@ -29,9 +29,8 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; -import java.util.function.Function; +import java.util.function.Predicate; import java.util.stream.Collectors; -import java.util.stream.Stream; import static java.util.Map.entry; import static org.elasticsearch.entitlement.qa.test.EntitlementTest.ExpectedAccess.ALWAYS_ALLOWED; @@ -49,27 +48,34 @@ record CheckAction( Integer fromJavaVersion ) {} - private static final Map checkActions = Stream.of( - getTestEntries(FileCheckActions.class), - getTestEntries(FileStoreActions.class), - getTestEntries(JvmActions.class), - getTestEntries(LoadNativeLibrariesCheckActions.class), - getTestEntries(ManageThreadsActions.class), - getTestEntries(NativeActions.class), - getTestEntries(NetworkAccessCheckActions.class), - getTestEntries(NioChannelsActions.class), - getTestEntries(NioFilesActions.class), - getTestEntries(NioFileSystemActions.class), - getTestEntries(OperatingSystemActions.class), - getTestEntries(PathActions.class), - getTestEntries(SpiActions.class), - getTestEntries(SystemActions.class), - getTestEntries(URLConnectionFileActions.class), - getTestEntries(URLConnectionNetworkActions.class) - ) - .flatMap(Function.identity()) - .filter(entry -> entry.getValue().fromJavaVersion() == null || Runtime.version().feature() >= entry.getValue().fromJavaVersion()) - .collect(Collectors.toUnmodifiableMap(Entry::getKey, Entry::getValue)); + private static final Map checkActions = collectTests( + FileCheckActions.class, + FileStoreActions.class, + JvmActions.class, + LoadNativeLibrariesCheckActions.class, + ManageThreadsActions.class, + NativeActions.class, + NetworkAccessCheckActions.class, + NioChannelsActions.class, + NioFilesActions.class, + NioFileSystemActions.class, + OperatingSystemActions.class, + PathActions.class, + SpiActions.class, + SystemActions.class, + URLConnectionFileActions.class, + URLConnectionNetworkActions.class + ); + + private static Map collectTests(Class... testClasses) { + List> entries = new ArrayList<>(); + for (Class testClass : testClasses) { + getTestEntries(entries, testClass, a -> a.fromJavaVersion() == null || Runtime.version().feature() >= a.fromJavaVersion()); + } + @SuppressWarnings("unchecked") + Entry[] entriesArray = entries.toArray(new Entry[0]); + return Map.ofEntries(entriesArray); + } private final Environment environment; @@ -82,8 +88,7 @@ private static Method[] getDeclaredMethods(Class clazz) { return clazz.getDeclaredMethods(); } - private static Stream> getTestEntries(Class actionsClass) { - List> entries = new ArrayList<>(); + private static void getTestEntries(List> entries, Class actionsClass, Predicate filter) { for (var method : getDeclaredMethods(actionsClass)) { var testAnnotation = method.getAnnotation(EntitlementTest.class); if (testAnnotation == null) { @@ -92,6 +97,9 @@ private static Stream> getTestEntries(Class action if (Modifier.isStatic(method.getModifiers()) == false) { throw new AssertionError("Entitlement test method [" + method + "] must be static"); } + if (Modifier.isPrivate(method.getModifiers())) { + throw new AssertionError("Entitlement test method [" + method + "] must not be private"); + } final CheckedConsumer call = createConsumerForMethod(method); CheckedConsumer runnable = env -> { try { @@ -107,14 +115,16 @@ private static Stream> getTestEntries(Class action } }; Integer fromJavaVersion = testAnnotation.fromJavaVersion() == -1 ? null : testAnnotation.fromJavaVersion(); - entries.add( - entry( - method.getName(), - new CheckAction(runnable, testAnnotation.expectedAccess(), testAnnotation.expectedExceptionIfDenied(), fromJavaVersion) - ) + var checkAction = new CheckAction( + runnable, + testAnnotation.expectedAccess(), + testAnnotation.expectedExceptionIfDenied(), + fromJavaVersion ); + if (filter.test(checkAction)) { + entries.add(entry(method.getName(), checkAction)); + } } - return entries.stream(); } private static CheckedConsumer createConsumerForMethod(Method method) { From 647f97dbd2f8ab422497cb3966191de530406a0a Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 9 Apr 2025 06:14:44 -0700 Subject: [PATCH 2/3] fix warning --- .../entitlement/qa/test/RestEntitlementsCheckAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java index 394784548b4fc..6a6f7111700ae 100644 --- a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java +++ b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java @@ -72,7 +72,7 @@ private static Map collectTests(Class... testClasses) { for (Class testClass : testClasses) { getTestEntries(entries, testClass, a -> a.fromJavaVersion() == null || Runtime.version().feature() >= a.fromJavaVersion()); } - @SuppressWarnings("unchecked") + @SuppressWarnings({"unchecked", "rawtypes"}) Entry[] entriesArray = entries.toArray(new Entry[0]); return Map.ofEntries(entriesArray); } From 5e07d63bbbda54eb6e519ecfe9d1db677dbea555 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 9 Apr 2025 13:22:53 +0000 Subject: [PATCH 3/3] [CI] Auto commit changes from spotless --- .../entitlement/qa/test/RestEntitlementsCheckAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java index 6a6f7111700ae..aeeaf7658fd7f 100644 --- a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java +++ b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/RestEntitlementsCheckAction.java @@ -72,7 +72,7 @@ private static Map collectTests(Class... testClasses) { for (Class testClass : testClasses) { getTestEntries(entries, testClass, a -> a.fromJavaVersion() == null || Runtime.version().feature() >= a.fromJavaVersion()); } - @SuppressWarnings({"unchecked", "rawtypes"}) + @SuppressWarnings({ "unchecked", "rawtypes" }) Entry[] entriesArray = entries.toArray(new Entry[0]); return Map.ofEntries(entriesArray); }