From 9d7062419a93d8a2dd739ddd92b13f5513ebaffc Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Wed, 14 May 2025 09:33:52 -0400 Subject: [PATCH 1/3] Fail fast on invalid entitlement patches --- .../entitlement/runtime/policy/PolicyUtils.java | 14 +++++++++----- .../runtime/policy/PolicyUtilsTests.java | 16 ++++++++++------ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtils.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtils.java index f7bcf63035bc8..2d76bc87bb8ff 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtils.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtils.java @@ -81,6 +81,10 @@ public static Map createPluginPolicies( return pluginPolicies; } + /** + * @throws PolicyParserException if the supplied policy is formatted incorrectly + * @throws IllegalStateException for any other error parsing the patch, such as nonexistent module names + */ public static Policy parseEncodedPolicyIfExists( String encodedPolicy, String version, @@ -106,11 +110,11 @@ public static Policy parseEncodedPolicyIfExists( version ); } - } catch (Exception ex) { - logger.warn( - Strings.format("Found a policy patch with invalid content. The patch will not be applied. Layer [%s]", layerName), - ex - ); + } catch (PolicyParserException e) { + // This is more specific and informative than a generalized IllegalStateException + throw e; + } catch (IOException | RuntimeException e) { + throw new IllegalStateException("Unable to parse policy patch for layer [" + layerName + "]", e); } } return null; diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtilsTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtilsTests.java index bb77f9c6a83ba..3ffb5866b177d 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtilsTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtilsTests.java @@ -134,6 +134,7 @@ public void testNoPatchWithVersionMismatch() { public void testNoPatchWithValidationError() { + // Nonexistent module names var policyPatch = """ versions: - 9.0.0 @@ -149,13 +150,15 @@ public void testNoPatchWithValidationError() { StandardCharsets.UTF_8 ); - var policy = PolicyUtils.parseEncodedPolicyIfExists(base64EncodedPolicy, "9.0.0", true, "test-plugin", Set.of()); - - assertThat(policy, nullValue()); + assertThrows( + IllegalStateException.class, + () -> PolicyUtils.parseEncodedPolicyIfExists(base64EncodedPolicy, "9.0.0", true, "test-plugin", Set.of()) + ); } public void testNoPatchWithParsingError() { + // no or field var policyPatch = """ entitlement-module-name: - load_native_libraries @@ -167,9 +170,10 @@ public void testNoPatchWithParsingError() { StandardCharsets.UTF_8 ); - var policy = PolicyUtils.parseEncodedPolicyIfExists(base64EncodedPolicy, "9.0.0", true, "test-plugin", Set.of()); - - assertThat(policy, nullValue()); + assertThrows( + PolicyParserException.class, + () -> PolicyUtils.parseEncodedPolicyIfExists(base64EncodedPolicy, "9.0.0", true, "test-plugin", Set.of()) + ); } public void testMergeScopes() { From 46bafb7c453ddbe1a3c56293d5bca77099ef09d9 Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Wed, 14 May 2025 10:40:24 -0400 Subject: [PATCH 2/3] Don't peel off `PolicyParserException` --- .../elasticsearch/entitlement/runtime/policy/PolicyUtils.java | 3 --- .../entitlement/runtime/policy/PolicyUtilsTests.java | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtils.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtils.java index 2d76bc87bb8ff..8184dd09790e0 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtils.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtils.java @@ -110,9 +110,6 @@ public static Policy parseEncodedPolicyIfExists( version ); } - } catch (PolicyParserException e) { - // This is more specific and informative than a generalized IllegalStateException - throw e; } catch (IOException | RuntimeException e) { throw new IllegalStateException("Unable to parse policy patch for layer [" + layerName + "]", e); } diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtilsTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtilsTests.java index 3ffb5866b177d..ed61087f7163c 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtilsTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtilsTests.java @@ -171,7 +171,7 @@ public void testNoPatchWithParsingError() { ); assertThrows( - PolicyParserException.class, + IllegalStateException.class, () -> PolicyUtils.parseEncodedPolicyIfExists(base64EncodedPolicy, "9.0.0", true, "test-plugin", Set.of()) ); } From 948f15bf80478887578473764255e9e2e9912d95 Mon Sep 17 00:00:00 2001 From: Patrick Doyle Date: Thu, 15 May 2025 10:09:14 -0400 Subject: [PATCH 3/3] Just catch Exception --- .../elasticsearch/entitlement/runtime/policy/PolicyUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtils.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtils.java index 8184dd09790e0..df51d0f4049e6 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtils.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyUtils.java @@ -110,7 +110,7 @@ public static Policy parseEncodedPolicyIfExists( version ); } - } catch (IOException | RuntimeException e) { + } catch (Exception e) { throw new IllegalStateException("Unable to parse policy patch for layer [" + layerName + "]", e); } }