From 1473b19cde7560a1d3fd540aec13f067ad818c3e Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Fri, 14 Feb 2025 16:15:10 +0000 Subject: [PATCH 1/5] Bump versions after 8.16.4 release --- server/src/main/java/org/elasticsearch/TransportVersions.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index adfa8dd4b2527..2322d7b3acc05 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -102,6 +102,7 @@ static TransportVersion def(int id) { public static final TransportVersion ADD_COMPATIBILITY_VERSIONS_TO_NODE_INFO_BACKPORT_8_16 = def(8_772_0_02); public static final TransportVersion SKIP_INNER_HITS_SEARCH_SOURCE_BACKPORT_8_16 = def(8_772_0_03); public static final TransportVersion QUERY_RULES_LIST_INCLUDES_TYPES_BACKPORT_8_16 = def(8_772_0_04); + public static final TransportVersion INITIAL_ELASTICSEARCH_8_16_5 = def(8_772_0_05); public static final TransportVersion REMOVE_MIN_COMPATIBLE_SHARD_NODE = def(8_773_0_00); public static final TransportVersion REVERT_REMOVE_MIN_COMPATIBLE_SHARD_NODE = def(8_774_0_00); public static final TransportVersion ESQL_FIELD_ATTRIBUTE_PARENT_SIMPLIFIED = def(8_775_0_00); From c8836a85725b94225544b0c1d09b723031b9d4c6 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Fri, 14 Feb 2025 16:19:56 +0000 Subject: [PATCH 2/5] [TEST] ensure cluster is stable before running testReindex (#122589) Unable to reproduce however the coordinator node that is meant to route the write requests might've not been ready to do so in due time. This PR adds an ensureStableCluster in the test setup method. Fixes #120605 --- ...ByScrollUsesAllScrollDocumentsAfterConflictsIntegTests.java | 1 + muted-tests.yml | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/modules/reindex/src/internalClusterTest/java/org/elasticsearch/index/reindex/BulkByScrollUsesAllScrollDocumentsAfterConflictsIntegTests.java b/modules/reindex/src/internalClusterTest/java/org/elasticsearch/index/reindex/BulkByScrollUsesAllScrollDocumentsAfterConflictsIntegTests.java index 04d8bae9fda2f..fc60b6373d285 100644 --- a/modules/reindex/src/internalClusterTest/java/org/elasticsearch/index/reindex/BulkByScrollUsesAllScrollDocumentsAfterConflictsIntegTests.java +++ b/modules/reindex/src/internalClusterTest/java/org/elasticsearch/index/reindex/BulkByScrollUsesAllScrollDocumentsAfterConflictsIntegTests.java @@ -97,6 +97,7 @@ public void setUpCluster() { // Use a single thread pool for writes so we can enforce a consistent ordering internalCluster().startDataOnlyNode(Settings.builder().put("thread_pool.write.size", 1).build()); internalCluster().startCoordinatingOnlyNode(Settings.EMPTY); + ensureStableCluster(3); } public void testUpdateByQuery() throws Exception { diff --git a/muted-tests.yml b/muted-tests.yml index 3a9ce427d8f54..fca3aaea58b06 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -187,9 +187,6 @@ tests: issue: https://github.com/elastic/elasticsearch/issues/120482 - class: org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeToCharProcessorTests issue: https://github.com/elastic/elasticsearch/issues/120575 -- class: org.elasticsearch.index.reindex.BulkByScrollUsesAllScrollDocumentsAfterConflictsIntegTests - method: testReindex - issue: https://github.com/elastic/elasticsearch/issues/120605 - class: org.elasticsearch.xpack.inference.DefaultEndPointsIT method: testMultipleInferencesTriggeringDownloadAndDeploy issue: https://github.com/elastic/elasticsearch/issues/120668 From eff30608ce63be058c5718d1abeee8dc45ec4187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenzo=20Dematt=C3=A9?= Date: Fri, 14 Feb 2025 17:23:33 +0100 Subject: [PATCH 3/5] Add file read entitlement check to library load functions (#122494) --- .../qa/test/LoadNativeLibrariesCheckActions.java | 4 ++-- .../elasticsearch/entitlement/qa/test/NativeActions.java | 2 +- .../runtime/api/ElasticsearchEntitlementChecker.java | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/LoadNativeLibrariesCheckActions.java b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/LoadNativeLibrariesCheckActions.java index 50980bc230f55..5b3265c5496ba 100644 --- a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/LoadNativeLibrariesCheckActions.java +++ b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/LoadNativeLibrariesCheckActions.java @@ -12,7 +12,7 @@ class LoadNativeLibrariesCheckActions { static void runtimeLoad() { try { - Runtime.getRuntime().load("libSomeLibFile.so"); + Runtime.getRuntime().load(FileCheckActions.readDir().resolve("libSomeLibFile.so").toString()); } catch (UnsatisfiedLinkError ignored) { // The library does not exist, so we expect to fail loading it } @@ -20,7 +20,7 @@ static void runtimeLoad() { static void systemLoad() { try { - System.load("libSomeLibFile.so"); + System.load(FileCheckActions.readDir().resolve("libSomeLibFile.so").toString()); } catch (UnsatisfiedLinkError ignored) { // The library does not exist, so we expect to fail loading it } diff --git a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/NativeActions.java b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/NativeActions.java index 5079e0d38a001..d731f850e0f4d 100644 --- a/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/NativeActions.java +++ b/libs/entitlement/qa/entitlement-test-plugin/src/main/java/org/elasticsearch/entitlement/qa/test/NativeActions.java @@ -113,7 +113,7 @@ static void memorySegmentReinterpretWithSizeAndCleanup() { @EntitlementTest(expectedAccess = PLUGINS) static void symbolLookupWithPath() { try { - SymbolLookup.libraryLookup(Path.of("/foo/bar/libFoo.so"), Arena.ofAuto()); + SymbolLookup.libraryLookup(FileCheckActions.readDir().resolve("libFoo.so"), Arena.ofAuto()); } catch (IllegalArgumentException e) { // IllegalArgumentException is thrown if path does not point to a valid library (and it does not) } diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/api/ElasticsearchEntitlementChecker.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/api/ElasticsearchEntitlementChecker.java index 986d8bee5bf27..2265ee7f62123 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/api/ElasticsearchEntitlementChecker.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/api/ElasticsearchEntitlementChecker.java @@ -836,7 +836,7 @@ public void checkSelectorProviderInheritedChannel(Class callerClass, Selector @Override public void check$java_lang_Runtime$load(Class callerClass, Runtime that, String filename) { - // TODO: check filesystem entitlement READ + policyManager.checkFileRead(callerClass, Path.of(filename)); policyManager.checkLoadingNativeLibraries(callerClass); } @@ -847,7 +847,7 @@ public void checkSelectorProviderInheritedChannel(Class callerClass, Selector @Override public void check$java_lang_System$$load(Class callerClass, String filename) { - // TODO: check filesystem entitlement READ + policyManager.checkFileRead(callerClass, Path.of(filename)); policyManager.checkLoadingNativeLibraries(callerClass); } @@ -931,7 +931,7 @@ public void checkSelectorProviderInheritedChannel(Class callerClass, Selector @Override public void check$java_lang_foreign_SymbolLookup$$libraryLookup(Class callerClass, Path path, Arena arena) { - // TODO: check filesystem entitlement READ + policyManager.checkFileRead(callerClass, path); policyManager.checkLoadingNativeLibraries(callerClass); } From 91413351ce0775be0b33bd52cce4284739221110 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenzo=20Dematt=C3=A9?= Date: Fri, 14 Feb 2025 18:08:08 +0100 Subject: [PATCH 4/5] [Entitlements] Add ability to set path relative to a special directory for Files policies (#122370) --- .../EntitlementInitialization.java | 14 +- .../runtime/policy/FileAccessTree.java | 24 +-- .../runtime/policy/PathLookup.java | 14 ++ .../runtime/policy/PolicyManager.java | 11 +- .../policy/entitlements/FilesEntitlement.java | 151 ++++++++++++++++-- .../runtime/policy/FileAccessTreeTests.java | 67 +++++++- .../runtime/policy/PolicyManagerTests.java | 43 +++-- .../policy/PolicyParserFailureTests.java | 75 ++++++++- .../runtime/policy/PolicyParserTests.java | 80 +++++++++- .../entitlements/FilesEntitlementTests.java | 27 +++- .../runtime/policy/test-policy.yaml | 2 +- 11 files changed, 448 insertions(+), 60 deletions(-) create mode 100644 libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PathLookup.java diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java index 6fed3c2e4b98c..9c8e5c33632d7 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/initialization/EntitlementInitialization.java @@ -18,6 +18,7 @@ import org.elasticsearch.entitlement.instrumentation.MethodKey; import org.elasticsearch.entitlement.instrumentation.Transformer; import org.elasticsearch.entitlement.runtime.api.ElasticsearchEntitlementChecker; +import org.elasticsearch.entitlement.runtime.policy.PathLookup; import org.elasticsearch.entitlement.runtime.policy.Policy; import org.elasticsearch.entitlement.runtime.policy.PolicyManager; import org.elasticsearch.entitlement.runtime.policy.Scope; @@ -48,7 +49,6 @@ import java.nio.file.attribute.FileAttribute; import java.nio.file.spi.FileSystemProvider; import java.util.ArrayList; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -126,9 +126,9 @@ private static Class[] findClassesToRetransform(Class[] loadedClasses, Set } private static PolicyManager createPolicyManager() { - Map pluginPolicies = EntitlementBootstrap.bootstrapArgs().pluginPolicies(); - Path[] dataDirs = EntitlementBootstrap.bootstrapArgs().dataDirs(); - Path tempDir = EntitlementBootstrap.bootstrapArgs().tempDir(); + EntitlementBootstrap.BootstrapArgs bootstrapArgs = EntitlementBootstrap.bootstrapArgs(); + Map pluginPolicies = bootstrapArgs.pluginPolicies(); + var pathLookup = new PathLookup(bootstrapArgs.configDir(), bootstrapArgs.dataDirs(), bootstrapArgs.tempDir()); // TODO(ES-10031): Decide what goes in the elasticsearch default policy and extend it var serverPolicy = new Policy( @@ -147,7 +147,7 @@ private static PolicyManager createPolicyManager() { new LoadNativeLibrariesEntitlement(), new ManageThreadsEntitlement(), new FilesEntitlement( - List.of(new FilesEntitlement.FileData(EntitlementBootstrap.bootstrapArgs().tempDir().toString(), READ_WRITE)) + List.of(FilesEntitlement.FileData.ofPath(EntitlementBootstrap.bootstrapArgs().tempDir(), READ_WRITE)) ) ) ), @@ -159,7 +159,7 @@ private static PolicyManager createPolicyManager() { "org.elasticsearch.nativeaccess", List.of( new LoadNativeLibrariesEntitlement(), - new FilesEntitlement(Arrays.stream(dataDirs).map(d -> new FileData(d.toString(), READ_WRITE)).toList()) + new FilesEntitlement(List.of(FileData.ofRelativePath(Path.of(""), FilesEntitlement.BaseDir.DATA, READ_WRITE))) ) ) ) @@ -175,7 +175,7 @@ private static PolicyManager createPolicyManager() { resolver, AGENTS_PACKAGE_NAME, ENTITLEMENTS_MODULE, - tempDir + pathLookup ); } diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java index 700302a42070f..46ee46c7b30c5 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTree.java @@ -20,26 +20,30 @@ import static org.elasticsearch.core.PathUtils.getDefaultFileSystem; public final class FileAccessTree { + private static final String FILE_SEPARATOR = getDefaultFileSystem().getSeparator(); private final String[] readPaths; private final String[] writePaths; - private FileAccessTree(FilesEntitlement filesEntitlement, Path tempDir) { + private FileAccessTree(FilesEntitlement filesEntitlement, PathLookup pathLookup) { List readPaths = new ArrayList<>(); List writePaths = new ArrayList<>(); for (FilesEntitlement.FileData fileData : filesEntitlement.filesData()) { - var path = normalizePath(Path.of(fileData.path())); var mode = fileData.mode(); - if (mode == FilesEntitlement.Mode.READ_WRITE) { - writePaths.add(path); - } - readPaths.add(path); + var paths = fileData.resolvePaths(pathLookup); + paths.forEach(path -> { + var normalized = normalizePath(path); + if (mode == FilesEntitlement.Mode.READ_WRITE) { + writePaths.add(normalized); + } + readPaths.add(normalized); + }); } // everything has access to the temp dir - readPaths.add(tempDir.toString()); - writePaths.add(tempDir.toString()); + readPaths.add(pathLookup.tempDir().toString()); + writePaths.add(pathLookup.tempDir().toString()); readPaths.sort(String::compareTo); writePaths.sort(String::compareTo); @@ -48,8 +52,8 @@ private FileAccessTree(FilesEntitlement filesEntitlement, Path tempDir) { this.writePaths = writePaths.toArray(new String[0]); } - public static FileAccessTree of(FilesEntitlement filesEntitlement, Path tempDir) { - return new FileAccessTree(filesEntitlement, tempDir); + public static FileAccessTree of(FilesEntitlement filesEntitlement, PathLookup pathLookup) { + return new FileAccessTree(filesEntitlement, pathLookup); } boolean canRead(Path path) { diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PathLookup.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PathLookup.java new file mode 100644 index 0000000000000..8e8b7dbb02b79 --- /dev/null +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PathLookup.java @@ -0,0 +1,14 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.entitlement.runtime.policy; + +import java.nio.file.Path; + +public record PathLookup(Path configDir, Path[] dataDirs, Path tempDir) {} diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java index ec1ae642329fa..33ccf6fb05c9c 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyManager.java @@ -99,7 +99,7 @@ ModuleEntitlements policyEntitlements(String componentName, List en return new ModuleEntitlements( componentName, entitlements.stream().collect(groupingBy(Entitlement::getClass)), - FileAccessTree.of(filesEntitlement, tempDir) + FileAccessTree.of(filesEntitlement, pathLookup) ); } @@ -109,7 +109,7 @@ ModuleEntitlements policyEntitlements(String componentName, List en private final List apmAgentEntitlements; private final Map>> pluginsEntitlements; private final Function, String> pluginResolver; - private final Path tempDir; + private final PathLookup pathLookup; private final FileAccessTree defaultFileAccess; public static final String ALL_UNNAMED = "ALL-UNNAMED"; @@ -146,7 +146,7 @@ public PolicyManager( Function, String> pluginResolver, String apmAgentPackageName, Module entitlementsModule, - Path tempDir + PathLookup pathLookup ) { this.serverEntitlements = buildScopeEntitlementsMap(requireNonNull(serverPolicy)); this.apmAgentEntitlements = apmAgentEntitlements; @@ -156,9 +156,8 @@ public PolicyManager( this.pluginResolver = pluginResolver; this.apmAgentPackageName = apmAgentPackageName; this.entitlementsModule = entitlementsModule; - this.defaultFileAccess = FileAccessTree.of(FilesEntitlement.EMPTY, tempDir); - - this.tempDir = tempDir; + this.pathLookup = requireNonNull(pathLookup); + this.defaultFileAccess = FileAccessTree.of(FilesEntitlement.EMPTY, pathLookup); for (var e : serverEntitlements.entrySet()) { validateEntitlementsPerModule(SERVER_COMPONENT_NAME, e.getKey(), e.getValue()); diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java index 953954ec3769c..e9079948879eb 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlement.java @@ -10,12 +10,17 @@ package org.elasticsearch.entitlement.runtime.policy.entitlements; import org.elasticsearch.entitlement.runtime.policy.ExternalEntitlement; +import org.elasticsearch.entitlement.runtime.policy.PathLookup; import org.elasticsearch.entitlement.runtime.policy.PolicyValidationException; +import java.nio.file.Path; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.stream.Stream; /** * Describes a file entitlement with a path and mode. @@ -29,8 +34,104 @@ public enum Mode { READ_WRITE } - public record FileData(String path, Mode mode) { + public enum BaseDir { + CONFIG, + DATA + } + + public sealed interface FileData { + + final class AbsolutePathFileData implements FileData { + private final Path path; + private final Mode mode; + + private AbsolutePathFileData(Path path, Mode mode) { + this.path = path; + this.mode = mode; + } + + @Override + public Stream resolvePaths(PathLookup pathLookup) { + return Stream.of(path); + } + + @Override + public Mode mode() { + return mode; + } + + @Override + public boolean equals(Object obj) { + if (obj == this) return true; + if (obj == null || obj.getClass() != this.getClass()) return false; + var that = (AbsolutePathFileData) obj; + return Objects.equals(this.path, that.path) && Objects.equals(this.mode, that.mode); + } + + @Override + public int hashCode() { + return Objects.hash(path, mode); + } + } + + final class RelativePathFileData implements FileData { + private final Path relativePath; + private final BaseDir baseDir; + private final Mode mode; + + private RelativePathFileData(Path relativePath, BaseDir baseDir, Mode mode) { + this.relativePath = relativePath; + this.baseDir = baseDir; + this.mode = mode; + } + + @Override + public Stream resolvePaths(PathLookup pathLookup) { + Objects.requireNonNull(pathLookup); + switch (baseDir) { + case CONFIG: + return Stream.of(pathLookup.configDir().resolve(relativePath)); + case DATA: + return Arrays.stream(pathLookup.dataDirs()).map(d -> d.resolve(relativePath)); + default: + throw new IllegalArgumentException(); + } + } + + @Override + public Mode mode() { + return mode; + } + @Override + public boolean equals(Object obj) { + if (obj == this) return true; + if (obj == null || obj.getClass() != this.getClass()) return false; + var that = (RelativePathFileData) obj; + return Objects.equals(this.mode, that.mode) + && Objects.equals(this.relativePath, that.relativePath) + && Objects.equals(this.baseDir, that.baseDir); + } + + @Override + public int hashCode() { + return Objects.hash(relativePath, baseDir, mode); + } + } + + static FileData ofPath(Path path, Mode mode) { + assert path.isAbsolute(); + return new AbsolutePathFileData(path, mode); + } + + static FileData ofRelativePath(Path relativePath, BaseDir baseDir, Mode mode) { + assert relativePath.isAbsolute() == false; + return new RelativePathFileData(relativePath, baseDir, mode); + } + + Stream resolvePaths(PathLookup pathLookup); + + Mode mode(); } private static Mode parseMode(String mode) { @@ -43,6 +144,15 @@ private static Mode parseMode(String mode) { } } + private static BaseDir parseBaseDir(String baseDir) { + if (baseDir.equals("config")) { + return BaseDir.CONFIG; + } else if (baseDir.equals("data")) { + return BaseDir.DATA; + } + throw new PolicyValidationException("invalid relative directory: " + baseDir + ", valid values: [config, data]"); + } + @ExternalEntitlement(parameterNames = { "paths" }, esModulesOnly = false) @SuppressWarnings("unchecked") public static FilesEntitlement build(List paths) { @@ -52,18 +162,41 @@ public static FilesEntitlement build(List paths) { List filesData = new ArrayList<>(); for (Object object : paths) { Map file = new HashMap<>((Map) object); - String path = file.remove("path"); - if (path == null) { - throw new PolicyValidationException("files entitlement must contain path for every listed file"); - } + String pathAsString = file.remove("path"); + String relativePathAsString = file.remove("relative_path"); + String relativeTo = file.remove("relative_to"); String mode = file.remove("mode"); + + if (file.isEmpty() == false) { + throw new PolicyValidationException("unknown key(s) [" + file + "] in a listed file for files entitlement"); + } if (mode == null) { - throw new PolicyValidationException("files entitlement must contain mode for every listed file"); + throw new PolicyValidationException("files entitlement must contain 'mode' for every listed file"); } - if (file.isEmpty() == false) { - throw new PolicyValidationException("unknown key(s) " + file + " in a listed file for files entitlement"); + if (pathAsString != null && relativePathAsString != null) { + throw new PolicyValidationException("a files entitlement entry cannot contain both 'path' and 'relative_path'"); + } + + if (relativePathAsString != null) { + if (relativeTo == null) { + throw new PolicyValidationException("files entitlement with a 'relative_path' must specify 'relative_to'"); + } + final BaseDir baseDir = parseBaseDir(relativeTo); + + Path relativePath = Path.of(relativePathAsString); + if (relativePath.isAbsolute()) { + throw new PolicyValidationException("'relative_path' [" + relativePathAsString + "] must be relative"); + } + filesData.add(FileData.ofRelativePath(relativePath, baseDir, parseMode(mode))); + } else if (pathAsString != null) { + Path path = Path.of(pathAsString); + if (path.isAbsolute() == false) { + throw new PolicyValidationException("'path' [" + pathAsString + "] must be absolute"); + } + filesData.add(FileData.ofPath(path, parseMode(mode))); + } else { + throw new PolicyValidationException("files entitlement must contain either 'path' or 'relative_path' for every entry"); } - filesData.add(new FileData(path, parseMode(mode))); } return new FilesEntitlement(filesData); } diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java index 6f3e4795fc298..27e80e989dcdc 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java @@ -35,6 +35,12 @@ private static Path path(String s) { return root.resolve(s); } + private static final PathLookup TEST_PATH_LOOKUP = new PathLookup( + Path.of("/config"), + new Path[] { Path.of("/data1"), Path.of("/data2") }, + Path.of("/tmp") + ); + public void testEmpty() { var tree = accessTree(FilesEntitlement.EMPTY); assertThat(tree.canRead(path("path")), is(false)); @@ -84,6 +90,53 @@ public void testReadWriteUnderRead() { assertThat(tree.canWrite(path("foo/bar")), is(true)); } + public void testReadWithRelativePath() { + var tree = accessTree(entitlement(Map.of("relative_path", "foo", "mode", "read", "relative_to", "config"))); + assertThat(tree.canRead(path("foo")), is(false)); + + assertThat(tree.canRead(path("/config/foo")), is(true)); + + assertThat(tree.canRead(path("/config/foo/subdir")), is(true)); + assertThat(tree.canRead(path("/config/food")), is(false)); + assertThat(tree.canWrite(path("/config/foo")), is(false)); + + assertThat(tree.canRead(path("/config")), is(false)); + assertThat(tree.canRead(path("/config/before")), is(false)); + assertThat(tree.canRead(path("/config/later")), is(false)); + } + + public void testWriteWithRelativePath() { + var tree = accessTree(entitlement(Map.of("relative_path", "foo", "mode", "read_write", "relative_to", "config"))); + assertThat(tree.canWrite(path("/config/foo")), is(true)); + assertThat(tree.canWrite(path("/config/foo/subdir")), is(true)); + assertThat(tree.canWrite(path("foo")), is(false)); + assertThat(tree.canWrite(path("/config/food")), is(false)); + assertThat(tree.canRead(path("/config/foo")), is(true)); + assertThat(tree.canRead(path("foo")), is(false)); + + assertThat(tree.canWrite(path("/config")), is(false)); + assertThat(tree.canWrite(path("/config/before")), is(false)); + assertThat(tree.canWrite(path("/config/later")), is(false)); + } + + public void testMultipleDataDirs() { + var tree = accessTree(entitlement(Map.of("relative_path", "foo", "mode", "read_write", "relative_to", "data"))); + assertThat(tree.canWrite(path("/data1/foo")), is(true)); + assertThat(tree.canWrite(path("/data2/foo")), is(true)); + assertThat(tree.canWrite(path("/data3/foo")), is(false)); + assertThat(tree.canWrite(path("/data1/foo/subdir")), is(true)); + assertThat(tree.canWrite(path("foo")), is(false)); + assertThat(tree.canWrite(path("/data1/food")), is(false)); + assertThat(tree.canRead(path("/data1/foo")), is(true)); + assertThat(tree.canRead(path("/data2/foo")), is(true)); + assertThat(tree.canRead(path("foo")), is(false)); + + assertThat(tree.canWrite(path("/data1")), is(false)); + assertThat(tree.canWrite(path("/data2")), is(false)); + assertThat(tree.canWrite(path("/config/before")), is(false)); + assertThat(tree.canWrite(path("/config/later")), is(false)); + } + public void testNormalizePath() { var tree = accessTree(entitlement("foo/../bar", "read")); assertThat(tree.canRead(path("foo/../bar")), is(true)); @@ -106,17 +159,19 @@ public void testForwardSlashes() { public void testTempDirAccess() { Path tempDir = createTempDir(); - var tree = FileAccessTree.of(FilesEntitlement.EMPTY, tempDir); - + var tree = FileAccessTree.of( + FilesEntitlement.EMPTY, + new PathLookup(Path.of("/config"), new Path[] { Path.of("/data1"), Path.of("/data2") }, tempDir) + ); assertThat(tree.canRead(tempDir), is(true)); assertThat(tree.canWrite(tempDir), is(true)); } FileAccessTree accessTree(FilesEntitlement entitlement) { - return FileAccessTree.of(entitlement, createTempDir()); + return FileAccessTree.of(entitlement, TEST_PATH_LOOKUP); } - FilesEntitlement entitlement(String... values) { + static FilesEntitlement entitlement(String... values) { List filesData = new ArrayList<>(); for (int i = 0; i < values.length; i += 2) { Map fileData = new HashMap<>(); @@ -126,4 +181,8 @@ FilesEntitlement entitlement(String... values) { } return FilesEntitlement.build(filesData); } + + static FilesEntitlement entitlement(Map value) { + return FilesEntitlement.build(List.of(value)); + } } diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java index d5f2794b292f8..90279230dbe17 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyManagerTests.java @@ -53,6 +53,12 @@ public class PolicyManagerTests extends ESTestCase { */ private static Module NO_ENTITLEMENTS_MODULE; + private static final PathLookup TEST_PATH_LOOKUP = new PathLookup( + Path.of("/config"), + new Path[] { Path.of("/data1/"), Path.of("/data2") }, + Path.of("/temp") + ); + @BeforeClass public static void beforeClass() { try { @@ -61,7 +67,6 @@ public static void beforeClass() { } catch (Exception e) { throw new IllegalStateException(e); } - } public void testGetEntitlementsThrowsOnMissingPluginUnnamedModule() { @@ -72,7 +77,7 @@ public void testGetEntitlementsThrowsOnMissingPluginUnnamedModule() { c -> "plugin1", TEST_AGENTS_PACKAGE_NAME, NO_ENTITLEMENTS_MODULE, - createTempDir() + TEST_PATH_LOOKUP ); // Any class from the current module (unnamed) will do @@ -96,7 +101,7 @@ public void testGetEntitlementsThrowsOnMissingPolicyForPlugin() { c -> "plugin1", TEST_AGENTS_PACKAGE_NAME, NO_ENTITLEMENTS_MODULE, - createTempDir() + TEST_PATH_LOOKUP ); // Any class from the current module (unnamed) will do @@ -116,7 +121,7 @@ public void testGetEntitlementsFailureIsCached() { c -> "plugin1", TEST_AGENTS_PACKAGE_NAME, NO_ENTITLEMENTS_MODULE, - createTempDir() + TEST_PATH_LOOKUP ); // Any class from the current module (unnamed) will do @@ -141,7 +146,7 @@ public void testGetEntitlementsReturnsEntitlementsForPluginUnnamedModule() { c -> "plugin2", TEST_AGENTS_PACKAGE_NAME, NO_ENTITLEMENTS_MODULE, - createTempDir() + TEST_PATH_LOOKUP ); // Any class from the current module (unnamed) will do @@ -159,7 +164,7 @@ public void testGetEntitlementsThrowsOnMissingPolicyForServer() throws ClassNotF c -> null, TEST_AGENTS_PACKAGE_NAME, NO_ENTITLEMENTS_MODULE, - createTempDir() + TEST_PATH_LOOKUP ); // Tests do not run modular, so we cannot use a server class. @@ -189,7 +194,7 @@ public void testGetEntitlementsReturnsEntitlementsForServerModule() throws Class c -> null, TEST_AGENTS_PACKAGE_NAME, NO_ENTITLEMENTS_MODULE, - createTempDir() + TEST_PATH_LOOKUP ); // Tests do not run modular, so we cannot use a server class. @@ -215,7 +220,7 @@ public void testGetEntitlementsReturnsEntitlementsForPluginModule() throws IOExc c -> "mock-plugin", TEST_AGENTS_PACKAGE_NAME, NO_ENTITLEMENTS_MODULE, - createTempDir() + TEST_PATH_LOOKUP ); var layer = createLayerForJar(jar, "org.example.plugin"); @@ -235,7 +240,7 @@ public void testGetEntitlementsResultIsCached() { c -> "plugin2", TEST_AGENTS_PACKAGE_NAME, NO_ENTITLEMENTS_MODULE, - createTempDir() + TEST_PATH_LOOKUP ); // Any class from the current module (unnamed) will do @@ -294,7 +299,7 @@ public void testAgentsEntitlements() throws IOException, ClassNotFoundException c -> c.getPackageName().startsWith(TEST_AGENTS_PACKAGE_NAME) ? null : "test", TEST_AGENTS_PACKAGE_NAME, NO_ENTITLEMENTS_MODULE, - createTempDir() + TEST_PATH_LOOKUP ); ModuleEntitlements agentsEntitlements = policyManager.getEntitlements(TestAgent.class); assertThat(agentsEntitlements.hasEntitlement(CreateClassLoaderEntitlement.class), is(true)); @@ -322,7 +327,7 @@ public void testDuplicateEntitlements() { c -> "test", TEST_AGENTS_PACKAGE_NAME, NO_ENTITLEMENTS_MODULE, - createTempDir() + TEST_PATH_LOOKUP ) ); assertEquals( @@ -339,7 +344,7 @@ public void testDuplicateEntitlements() { c -> "test", TEST_AGENTS_PACKAGE_NAME, NO_ENTITLEMENTS_MODULE, - createTempDir() + TEST_PATH_LOOKUP ) ); assertEquals( @@ -362,7 +367,9 @@ public void testDuplicateEntitlements() { List.of( FilesEntitlement.EMPTY, new CreateClassLoaderEntitlement(), - new FilesEntitlement(List.of(new FilesEntitlement.FileData("test", FilesEntitlement.Mode.READ))) + new FilesEntitlement( + List.of(FilesEntitlement.FileData.ofPath(Path.of("/tmp/test"), FilesEntitlement.Mode.READ)) + ) ) ) ) @@ -371,7 +378,7 @@ public void testDuplicateEntitlements() { c -> "plugin1", TEST_AGENTS_PACKAGE_NAME, NO_ENTITLEMENTS_MODULE, - createTempDir() + TEST_PATH_LOOKUP ) ); assertEquals( @@ -391,7 +398,7 @@ public void testPluginResolverOverridesAgents() { c -> "test", // Insist that the class is in a plugin TEST_AGENTS_PACKAGE_NAME, NO_ENTITLEMENTS_MODULE, - createTempDir() + TEST_PATH_LOOKUP ); ModuleEntitlements notAgentsEntitlements = policyManager.getEntitlements(TestAgent.class); assertThat(notAgentsEntitlements.hasEntitlement(CreateClassLoaderEntitlement.class), is(false)); @@ -412,7 +419,7 @@ private static PolicyManager policyManager(String agentsPackageName, Module enti c -> "test", agentsPackageName, entitlementsModule, - createTempDir() + TEST_PATH_LOOKUP ); } @@ -432,7 +439,9 @@ private static Policy createPluginPolicy(String... pluginModules) { name -> new Scope( name, List.of( - new FilesEntitlement(List.of(new FilesEntitlement.FileData("/test/path", FilesEntitlement.Mode.READ))), + new FilesEntitlement( + List.of(FilesEntitlement.FileData.ofPath(Path.of("/test/path"), FilesEntitlement.Mode.READ)) + ), new CreateClassLoaderEntitlement() ) ) diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserFailureTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserFailureTests.java index 4f479a9bf59ac..924864d57b1cf 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserFailureTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserFailureTests.java @@ -45,7 +45,78 @@ public void testEntitlementMissingParameter() { """.getBytes(StandardCharsets.UTF_8)), "test-failure-policy.yaml", false).parsePolicy()); assertEquals( "[2:5] policy parsing error for [test-failure-policy.yaml] in scope [entitlement-module-name] " - + "for entitlement type [files]: files entitlement must contain mode for every listed file", + + "for entitlement type [files]: files entitlement must contain 'mode' for every listed file", + ppe.getMessage() + ); + } + + public void testEntitlementMissingDependentParameter() { + PolicyParserException ppe = expectThrows(PolicyParserException.class, () -> new PolicyParser(new ByteArrayInputStream(""" + entitlement-module-name: + - files: + - relative_path: test-path + mode: read + """.getBytes(StandardCharsets.UTF_8)), "test-failure-policy.yaml", false).parsePolicy()); + assertEquals( + "[2:5] policy parsing error for [test-failure-policy.yaml] in scope [entitlement-module-name] " + + "for entitlement type [files]: files entitlement with a 'relative_path' must specify 'relative_to'", + ppe.getMessage() + ); + } + + public void testEntitlementRelativePathWhenAbsolute() { + PolicyParserException ppe = expectThrows(PolicyParserException.class, () -> new PolicyParser(new ByteArrayInputStream(""" + entitlement-module-name: + - files: + - path: test-path + mode: read + """.getBytes(StandardCharsets.UTF_8)), "test-failure-policy.yaml", false).parsePolicy()); + assertEquals( + "[2:5] policy parsing error for [test-failure-policy.yaml] in scope [entitlement-module-name] " + + "for entitlement type [files]: 'path' [test-path] must be absolute", + ppe.getMessage() + ); + } + + public void testEntitlementAbsolutePathWhenRelative() { + PolicyParserException ppe = expectThrows(PolicyParserException.class, () -> new PolicyParser(new ByteArrayInputStream(""" + entitlement-module-name: + - files: + - relative_path: /test-path + relative_to: data + mode: read + """.getBytes(StandardCharsets.UTF_8)), "test-failure-policy.yaml", false).parsePolicy()); + assertEquals( + "[2:5] policy parsing error for [test-failure-policy.yaml] in scope [entitlement-module-name] " + + "for entitlement type [files]: 'relative_path' [/test-path] must be relative", + ppe.getMessage() + ); + } + + public void testEntitlementMutuallyExclusiveParameters() { + PolicyParserException ppe = expectThrows(PolicyParserException.class, () -> new PolicyParser(new ByteArrayInputStream(""" + entitlement-module-name: + - files: + - relative_path: test-path + path: test-path + mode: read + """.getBytes(StandardCharsets.UTF_8)), "test-failure-policy.yaml", false).parsePolicy()); + assertEquals( + "[2:5] policy parsing error for [test-failure-policy.yaml] in scope [entitlement-module-name] " + + "for entitlement type [files]: a files entitlement entry cannot contain both 'path' and 'relative_path'", + ppe.getMessage() + ); + } + + public void testEntitlementAtLeastOneParameter() { + PolicyParserException ppe = expectThrows(PolicyParserException.class, () -> new PolicyParser(new ByteArrayInputStream(""" + entitlement-module-name: + - files: + - mode: read + """.getBytes(StandardCharsets.UTF_8)), "test-failure-policy.yaml", false).parsePolicy()); + assertEquals( + "[2:5] policy parsing error for [test-failure-policy.yaml] in scope [entitlement-module-name] " + + "for entitlement type [files]: files entitlement must contain either 'path' or 'relative_path' for every entry", ppe.getMessage() ); } @@ -60,7 +131,7 @@ public void testEntitlementExtraneousParameter() { """.getBytes(StandardCharsets.UTF_8)), "test-failure-policy.yaml", false).parsePolicy()); assertEquals( "[2:5] policy parsing error for [test-failure-policy.yaml] in scope [entitlement-module-name] " - + "for entitlement type [files]: unknown key(s) {extra=test} in a listed file for files entitlement", + + "for entitlement type [files]: unknown key(s) [{extra=test}] in a listed file for files entitlement", ppe.getMessage() ); } diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java index e84c8ad2a83c7..b27a29978eec7 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java @@ -87,7 +87,7 @@ public void testPolicyBuilder() throws IOException { List.of( new Scope( "entitlement-module-name", - List.of(FilesEntitlement.build(List.of(Map.of("path", "test/path/to/file", "mode", "read_write")))) + List.of(FilesEntitlement.build(List.of(Map.of("path", "/test/path/to/file", "mode", "read_write")))) ) ) ); @@ -102,13 +102,89 @@ public void testPolicyBuilderOnExternalPlugin() throws IOException { List.of( new Scope( "entitlement-module-name", - List.of(FilesEntitlement.build(List.of(Map.of("path", "test/path/to/file", "mode", "read_write")))) + List.of(FilesEntitlement.build(List.of(Map.of("path", "/test/path/to/file", "mode", "read_write")))) ) ) ); assertEquals(expected, parsedPolicy); } + public void testParseFiles() throws IOException { + Policy policyWithOnePath = new PolicyParser(new ByteArrayInputStream(""" + entitlement-module-name: + - files: + - path: "/test/path/to/file" + mode: "read_write" + """.getBytes(StandardCharsets.UTF_8)), "test-policy.yaml", false).parsePolicy(); + Policy expected = new Policy( + "test-policy.yaml", + List.of( + new Scope( + "entitlement-module-name", + List.of(FilesEntitlement.build(List.of(Map.of("path", "/test/path/to/file", "mode", "read_write")))) + ) + ) + ); + assertEquals(expected, policyWithOnePath); + + Policy policyWithTwoPaths = new PolicyParser(new ByteArrayInputStream(""" + entitlement-module-name: + - files: + - path: "/test/path/to/file" + mode: "read_write" + - path: "/test/path/to/read-dir/" + mode: "read" + """.getBytes(StandardCharsets.UTF_8)), "test-policy.yaml", false).parsePolicy(); + expected = new Policy( + "test-policy.yaml", + List.of( + new Scope( + "entitlement-module-name", + List.of( + FilesEntitlement.build( + List.of( + Map.of("path", "/test/path/to/file", "mode", "read_write"), + Map.of("path", "/test/path/to/read-dir/", "mode", "read") + ) + ) + ) + ) + ) + ); + assertEquals(expected, policyWithTwoPaths); + + Policy policyWithMultiplePathsAndBaseDir = new PolicyParser(new ByteArrayInputStream(""" + entitlement-module-name: + - files: + - relative_path: "test/path/to/file" + relative_to: "data" + mode: "read_write" + - relative_path: "test/path/to/read-dir/" + relative_to: "config" + mode: "read" + - path: "/path/to/file" + mode: "read_write" + """.getBytes(StandardCharsets.UTF_8)), "test-policy.yaml", false).parsePolicy(); + expected = new Policy( + "test-policy.yaml", + List.of( + new Scope( + "entitlement-module-name", + List.of( + FilesEntitlement.build( + List.of( + Map.of("relative_path", "test/path/to/file", "mode", "read_write", "relative_to", "data"), + Map.of("relative_path", "test/path/to/read-dir/", "mode", "read", "relative_to", "config"), + Map.of("path", "/path/to/file", "mode", "read_write") + ) + ) + ) + ) + ) + ); + assertEquals(expected, policyWithMultiplePathsAndBaseDir); + } + public void testParseNetwork() throws IOException { Policy parsedPolicy = new PolicyParser(new ByteArrayInputStream(""" entitlement-module-name: diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlementTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlementTests.java index 5011fe2be462b..542b75e33a018 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlementTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FilesEntitlementTests.java @@ -9,17 +9,40 @@ package org.elasticsearch.entitlement.runtime.policy.entitlements; +import org.elasticsearch.entitlement.runtime.policy.PathLookup; import org.elasticsearch.entitlement.runtime.policy.PolicyValidationException; import org.elasticsearch.test.ESTestCase; +import java.nio.file.Path; import java.util.List; +import java.util.Map; + +import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ_WRITE; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.is; public class FilesEntitlementTests extends ESTestCase { public void testEmptyBuild() { PolicyValidationException pve = expectThrows(PolicyValidationException.class, () -> FilesEntitlement.build(List.of())); - assertEquals(pve.getMessage(), "must specify at least one path"); + assertEquals("must specify at least one path", pve.getMessage()); pve = expectThrows(PolicyValidationException.class, () -> FilesEntitlement.build(null)); - assertEquals(pve.getMessage(), "must specify at least one path"); + assertEquals("must specify at least one path", pve.getMessage()); + } + + public void testInvalidRelativeDirectory() { + var ex = expectThrows( + PolicyValidationException.class, + () -> FilesEntitlement.build(List.of((Map.of("relative_path", "foo", "mode", "read", "relative_to", "bar")))) + ); + assertThat(ex.getMessage(), is("invalid relative directory: bar, valid values: [config, data]")); + } + + public void testFileDataRelativeWithEmptyDirectory() { + var fileData = FilesEntitlement.FileData.ofRelativePath(Path.of(""), FilesEntitlement.BaseDir.DATA, READ_WRITE); + var dataDirs = fileData.resolvePaths( + new PathLookup(Path.of("/config"), new Path[] { Path.of("/data1/"), Path.of("/data2") }, Path.of("/temp")) + ); + assertThat(dataDirs.toList(), contains(Path.of("/data1/"), Path.of("/data2"))); } } diff --git a/libs/entitlement/src/test/resources/org/elasticsearch/entitlement/runtime/policy/test-policy.yaml b/libs/entitlement/src/test/resources/org/elasticsearch/entitlement/runtime/policy/test-policy.yaml index 6b1a5c22993fa..2b5a4cfa783fe 100644 --- a/libs/entitlement/src/test/resources/org/elasticsearch/entitlement/runtime/policy/test-policy.yaml +++ b/libs/entitlement/src/test/resources/org/elasticsearch/entitlement/runtime/policy/test-policy.yaml @@ -1,4 +1,4 @@ entitlement-module-name: - files: - - path: "test/path/to/file" + - path: "/test/path/to/file" mode: "read_write" From d59a0d9d44fb0cf553cb33a7a5e0da1f0b541c6b Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Fri, 14 Feb 2025 12:38:00 -0500 Subject: [PATCH 5/5] Canonicalize processor names and types in IngestStats (#122610) --- docs/changelog/122610.yaml | 5 +++ .../elasticsearch/ingest/IngestService.java | 35 +++++++++++++------ .../org/elasticsearch/ingest/IngestStats.java | 9 +++++ .../ingest/IngestServiceTests.java | 2 +- .../ingest/IngestStatsTests.java | 28 +++++++++++++++ 5 files changed, 68 insertions(+), 11 deletions(-) create mode 100644 docs/changelog/122610.yaml diff --git a/docs/changelog/122610.yaml b/docs/changelog/122610.yaml new file mode 100644 index 0000000000000..57977e703c06b --- /dev/null +++ b/docs/changelog/122610.yaml @@ -0,0 +1,5 @@ +pr: 122610 +summary: Canonicalize processor names and types in `IngestStats` +area: Ingest Node +type: bug +issues: [] diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestService.java b/server/src/main/java/org/elasticsearch/ingest/IngestService.java index 4c61a41f7cf8d..86e29fd5cb28f 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestService.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestService.java @@ -1195,20 +1195,35 @@ static String getProcessorName(Processor processor) { if (processor instanceof ConditionalProcessor conditionalProcessor) { processor = conditionalProcessor.getInnerProcessor(); } - StringBuilder sb = new StringBuilder(5); - sb.append(processor.getType()); + String tag = processor.getTag(); + if (tag != null && tag.isEmpty()) { + tag = null; // it simplifies the rest of the logic slightly to coalesce to null + } + + String pipelineName = null; if (processor instanceof PipelineProcessor pipelineProcessor) { - String pipelineName = pipelineProcessor.getPipelineTemplate().newInstance(Map.of()).execute(); - sb.append(":"); - sb.append(pipelineName); + pipelineName = pipelineProcessor.getPipelineTemplate().newInstance(Map.of()).execute(); } - String tag = processor.getTag(); - if (tag != null && tag.isEmpty() == false) { - sb.append(":"); - sb.append(tag); + + // if there's a tag, OR if it's a pipeline processor, then the processor name is a compound thing, + // BUT if neither of those apply, then it's just the type -- so we can return the type itself without + // allocating a new String object + if (tag == null && pipelineName == null) { + return processor.getType(); + } else { + StringBuilder sb = new StringBuilder(5); + sb.append(processor.getType()); + if (pipelineName != null) { + sb.append(":"); + sb.append(pipelineName); + } + if (tag != null) { + sb.append(":"); + sb.append(tag); + } + return sb.toString(); } - return sb.toString(); } /** diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestStats.java b/server/src/main/java/org/elasticsearch/ingest/IngestStats.java index da1b99f4f0759..9f403ca9300dd 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestStats.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestStats.java @@ -31,6 +31,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; +import java.util.function.Function; public record IngestStats(Stats totalStats, List pipelineStats, Map> processorStats) implements @@ -57,6 +58,11 @@ public record IngestStats(Stats totalStats, List pipelineStats, Ma * Read from a stream. */ public static IngestStats read(StreamInput in) throws IOException { + // while reading the processors, we're going to encounter identical name and type strings *repeatedly* + // it's advantageous to discard the endless copies of the same strings and canonical-ize them to keep our + // heap usage under control. note: this map is key to key, because of the limitations of the set interface. + final Map namesAndTypesCache = new HashMap<>(); + var stats = readStats(in); var size = in.readVInt(); if (stats == Stats.IDENTITY && size == 0) { @@ -76,6 +82,9 @@ public static IngestStats read(StreamInput in) throws IOException { var processorName = in.readString(); var processorType = in.readString(); var processorStat = readStats(in); + // pass these name and type through the local names and types cache to canonical-ize them + processorName = namesAndTypesCache.computeIfAbsent(processorName, Function.identity()); + processorType = namesAndTypesCache.computeIfAbsent(processorType, Function.identity()); processorStatsPerPipeline.add(new ProcessorStat(processorName, processorType, processorStat)); } processorStats.put(pipelineId, Collections.unmodifiableList(processorStatsPerPipeline)); diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java index bd4c5232a8ee4..eb56145359560 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestServiceTests.java @@ -2181,7 +2181,7 @@ public void testStatName() { Processor processor = mock(Processor.class); String name = randomAlphaOfLength(10); when(processor.getType()).thenReturn(name); - assertThat(IngestService.getProcessorName(processor), equalTo(name)); + assertThat(IngestService.getProcessorName(processor), sameInstance(name)); String tag = randomAlphaOfLength(10); when(processor.getTag()).thenReturn(tag); assertThat(IngestService.getProcessorName(processor), equalTo(name + ":" + tag)); diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestStatsTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestStatsTests.java index d9189c56e6689..8babb8bb9d395 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestStatsTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestStatsTests.java @@ -19,6 +19,7 @@ import java.util.Map; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.sameInstance; public class IngestStatsTests extends ESTestCase { @@ -37,6 +38,33 @@ public void testIdentitySerialization() throws IOException { assertThat(serializedStats, sameInstance(IngestStats.IDENTITY)); } + public void testProcessorNameAndTypeIdentitySerialization() throws IOException { + IngestStats.Builder builder = new IngestStats.Builder(); + builder.addPipelineMetrics("pipeline_id", new IngestPipelineMetric()); + builder.addProcessorMetrics("pipeline_id", "set", "set", new IngestMetric()); + builder.addProcessorMetrics("pipeline_id", "set:foo", "set", new IngestMetric()); + builder.addProcessorMetrics("pipeline_id", "set:bar", "set", new IngestMetric()); + builder.addTotalMetrics(new IngestMetric()); + + IngestStats serializedStats = serialize(builder.build()); + List processorStats = serializedStats.processorStats().get("pipeline_id"); + + // these are just table stakes + assertThat(processorStats.get(0).name(), is("set")); + assertThat(processorStats.get(0).type(), is("set")); + assertThat(processorStats.get(1).name(), is("set:foo")); + assertThat(processorStats.get(1).type(), is("set")); + assertThat(processorStats.get(2).name(), is("set:bar")); + assertThat(processorStats.get(2).type(), is("set")); + + // this is actually interesting, though -- we're canonical-izing these strings to keep our heap usage under control + final String set = processorStats.get(0).name(); + assertThat(processorStats.get(0).name(), sameInstance(set)); + assertThat(processorStats.get(0).type(), sameInstance(set)); + assertThat(processorStats.get(1).type(), sameInstance(set)); + assertThat(processorStats.get(2).type(), sameInstance(set)); + } + public void testStatsMerge() { var first = randomStats(); var second = randomStats();