Преглед на файлове

Support entitlements in internal cluster tests (#130710) (#131455)

* To prevent an implicit grant-all if storing node homes inside the Java temp dir, the temporary folder of ESTestCase is configured separately from the Java temp dir in internalClusterTests (by means of the system property tempDir, see TestRuleTemporaryFilesCleanup)

* Move ReloadingDatabasesWhilePerformingGeoLookupsIT from internalClusterTest to test, file permissions in internalClusterTest are stricter on the lucene tempDir
Moritz Mack преди 2 месеца
родител
ревизия
4e857c3fe2

+ 41 - 25
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchTestBasePlugin.java

@@ -34,6 +34,7 @@ import org.gradle.api.tasks.testing.Test;
 import java.io.File;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Stream;
 
 import javax.inject.Inject;
@@ -50,6 +51,8 @@ public abstract class ElasticsearchTestBasePlugin implements Plugin<Project> {
 
     public static final String DUMP_OUTPUT_ON_FAILURE_PROP_NAME = "dumpOutputOnFailure";
 
+    public static final Set<String> TEST_TASKS_WITH_ENTITLEMENTS = Set.of("test", "internalClusterTest");
+
     @Inject
     protected abstract ProviderFactory getProviderFactory();
 
@@ -174,14 +177,23 @@ public abstract class ElasticsearchTestBasePlugin implements Plugin<Project> {
             nonInputProperties.systemProperty("workspace.dir", Util.locateElasticsearchWorkspace(project.getGradle()));
             // we use 'temp' relative to CWD since this is per JVM and tests are forbidden from writing to CWD
             nonInputProperties.systemProperty("java.io.tmpdir", test.getWorkingDir().toPath().resolve("temp"));
+            if (test.getName().equals("internalClusterTest")) {
+                // configure a node home directory independent of the Java temp dir so that entitlements can be properly enforced
+                nonInputProperties.systemProperty("tempDir", test.getWorkingDir().toPath().resolve("nodesTemp"));
+            }
 
             SourceSetContainer sourceSets = project.getExtensions().getByType(SourceSetContainer.class);
             SourceSet mainSourceSet = sourceSets.findByName(SourceSet.MAIN_SOURCE_SET_NAME);
             SourceSet testSourceSet = sourceSets.findByName(SourceSet.TEST_SOURCE_SET_NAME);
-            if ("test".equals(test.getName()) && mainSourceSet != null && testSourceSet != null) {
+            SourceSet internalClusterTestSourceSet = sourceSets.findByName("internalClusterTest");
+
+            if (TEST_TASKS_WITH_ENTITLEMENTS.contains(test.getName()) && mainSourceSet != null && testSourceSet != null) {
                 FileCollection mainRuntime = mainSourceSet.getRuntimeClasspath();
                 FileCollection testRuntime = testSourceSet.getRuntimeClasspath();
-                FileCollection testOnlyFiles = testRuntime.minus(mainRuntime);
+                FileCollection internalClusterTestRuntime = ("internalClusterTest".equals(test.getName())
+                    && internalClusterTestSourceSet != null) ? internalClusterTestSourceSet.getRuntimeClasspath() : project.files();
+                FileCollection testOnlyFiles = testRuntime.plus(internalClusterTestRuntime).minus(mainRuntime);
+
                 test.doFirst(task -> test.environment("es.entitlement.testOnlyPath", testOnlyFiles.getAsPath()));
             }
 
@@ -241,14 +253,15 @@ public abstract class ElasticsearchTestBasePlugin implements Plugin<Project> {
      * Computes and sets the {@code --patch-module=java.base} and {@code --add-opens=java.base} JVM command line options.
      */
     private void configureJavaBaseModuleOptions(Project project) {
-        project.getTasks().withType(Test.class).matching(task -> task.getName().equals("test")).configureEach(test -> {
-            FileCollection patchedImmutableCollections = patchedImmutableCollections(project);
+        project.getTasks().withType(Test.class).configureEach(test -> {
+            // patch immutable collections only for "test" task
+            FileCollection patchedImmutableCollections = test.getName().equals("test") ? patchedImmutableCollections(project) : null;
             if (patchedImmutableCollections != null) {
                 test.getInputs().files(patchedImmutableCollections);
                 test.systemProperty("tests.hackImmutableCollections", "true");
             }
 
-            FileCollection entitlementBridge = entitlementBridge(project);
+            FileCollection entitlementBridge = TEST_TASKS_WITH_ENTITLEMENTS.contains(test.getName()) ? entitlementBridge(project) : null;
             if (entitlementBridge != null) {
                 test.getInputs().files(entitlementBridge);
             }
@@ -312,27 +325,30 @@ public abstract class ElasticsearchTestBasePlugin implements Plugin<Project> {
         }
         FileCollection bridgeFiles = bridgeConfig;
 
-        project.getTasks().withType(Test.class).configureEach(test -> {
-            // See also SystemJvmOptions.maybeAttachEntitlementAgent.
-
-            // Agent
-            if (agentFiles.isEmpty() == false) {
-                test.getInputs().files(agentFiles);
-                test.systemProperty("es.entitlement.agentJar", agentFiles.getAsPath());
-                test.systemProperty("jdk.attach.allowAttachSelf", true);
-            }
+        project.getTasks()
+            .withType(Test.class)
+            .matching(test -> TEST_TASKS_WITH_ENTITLEMENTS.contains(test.getName()))
+            .configureEach(test -> {
+                // See also SystemJvmOptions.maybeAttachEntitlementAgent.
+
+                // Agent
+                if (agentFiles.isEmpty() == false) {
+                    test.getInputs().files(agentFiles);
+                    test.systemProperty("es.entitlement.agentJar", agentFiles.getAsPath());
+                    test.systemProperty("jdk.attach.allowAttachSelf", true);
+                }
 
-            // Bridge
-            if (bridgeFiles.isEmpty() == false) {
-                String modulesContainingEntitlementInstrumentation = "java.logging,java.net.http,java.naming,jdk.net";
-                test.getInputs().files(bridgeFiles);
-                // Tests may not be modular, but the JDK still is
-                test.jvmArgs(
-                    "--add-exports=java.base/org.elasticsearch.entitlement.bridge=ALL-UNNAMED,"
-                        + modulesContainingEntitlementInstrumentation
-                );
-            }
-        });
+                // Bridge
+                if (bridgeFiles.isEmpty() == false) {
+                    String modulesContainingEntitlementInstrumentation = "java.logging,java.net.http,java.naming,jdk.net";
+                    test.getInputs().files(bridgeFiles);
+                    // Tests may not be modular, but the JDK still is
+                    test.jvmArgs(
+                        "--add-exports=java.base/org.elasticsearch.entitlement.bridge=ALL-UNNAMED,"
+                            + modulesContainingEntitlementInstrumentation
+                    );
+                }
+            });
     }
 
 }

+ 6 - 3
build-tools/src/main/java/org/elasticsearch/gradle/test/TestBuildInfoPlugin.java

@@ -58,9 +58,12 @@ public class TestBuildInfoPlugin implements Plugin<Project> {
         });
 
         if (project.getRootProject().getName().equals("elasticsearch")) {
-            project.getTasks().withType(Test.class).matching(test -> List.of("test").contains(test.getName())).configureEach(test -> {
-                test.systemProperty("es.entitlement.enableForTests", "true");
-            });
+            project.getTasks()
+                .withType(Test.class)
+                .matching(test -> List.of("test", "internalClusterTest").contains(test.getName()))
+                .configureEach(test -> {
+                    test.systemProperty("es.entitlement.enableForTests", "true");
+                });
         }
     }
 }

+ 1 - 1
libs/build.gradle

@@ -50,7 +50,7 @@ configure(childProjects.values()) {
   // Omit oddball libraries that aren't in server.
   def nonServerLibs = ['plugin-scanner']
   if (false == nonServerLibs.contains(project.name)) {
-    project.getTasks().withType(Test.class).matching(test -> ['test'].contains(test.name)).configureEach(test -> {
+    project.getTasks().withType(Test.class).matching(test -> ['test', 'internalClusterTest'].contains(test.name)).configureEach(test -> {
       test.systemProperty('es.entitlement.enableForTests', 'true')
     })
   }

+ 1 - 1
modules/ingest-geoip/src/internalClusterTest/java/org/elasticsearch/ingest/geoip/ReloadingDatabasesWhilePerformingGeoLookupsIT.java → modules/ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/ReloadingDatabasesWhilePerformingGeoLookupsTests.java

@@ -57,7 +57,7 @@ import static org.mockito.Mockito.when;
                     // 'WindowsFS.checkDeleteAccess(...)').
     }
 )
-public class ReloadingDatabasesWhilePerformingGeoLookupsIT extends ESTestCase {
+public class ReloadingDatabasesWhilePerformingGeoLookupsTests extends ESTestCase {
 
     /**
      * This tests essentially verifies that a Maxmind database reader doesn't fail with:

+ 2 - 0
server/src/internalClusterTest/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapAndDetachCommandIT.java

@@ -24,6 +24,7 @@ import org.elasticsearch.gateway.GatewayMetaState;
 import org.elasticsearch.gateway.PersistedClusterStateService;
 import org.elasticsearch.node.Node;
 import org.elasticsearch.test.ESIntegTestCase;
+import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.InternalTestCluster;
 
 import java.io.IOException;
@@ -39,6 +40,7 @@ import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.notNullValue;
 
 @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, autoManageMasterNodes = false)
+@ESTestCase.WithoutEntitlements // CLI tools don't run with entitlements enforced
 public class UnsafeBootstrapAndDetachCommandIT extends ESIntegTestCase {
 
     private MockTerminal executeCommand(ElasticsearchNodeCommand command, Environment environment, boolean abort) throws Exception {

+ 1 - 10
test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java

@@ -14,7 +14,6 @@ import org.apache.logging.log4j.Logger;
 import org.elasticsearch.common.network.IfConfig;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.core.Booleans;
-import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.PathUtils;
 import org.elasticsearch.entitlement.bootstrap.TestEntitlementBootstrap;
 import org.elasticsearch.jdk.JarHell;
@@ -76,20 +75,12 @@ public class BootstrapForTesting {
 
         // Fire up entitlements
         try {
-            TestEntitlementBootstrap.bootstrap(javaTmpDir, maybePath(System.getProperty("tests.config")));
+            TestEntitlementBootstrap.bootstrap(javaTmpDir);
         } catch (IOException e) {
             throw new IllegalStateException(e.getClass().getSimpleName() + " while initializing entitlements for tests", e);
         }
     }
 
-    private static @Nullable Path maybePath(String str) {
-        if (str == null) {
-            return null;
-        } else {
-            return PathUtils.get(str);
-        }
-    }
-
     // does nothing, just easy way to make sure the class is loaded.
     public static void ensureInitialized() {}
 }

+ 72 - 3
test/framework/src/main/java/org/elasticsearch/entitlement/bootstrap/TestEntitlementBootstrap.java

@@ -12,6 +12,7 @@ package org.elasticsearch.entitlement.bootstrap;
 import org.elasticsearch.bootstrap.TestBuildInfo;
 import org.elasticsearch.bootstrap.TestBuildInfoParser;
 import org.elasticsearch.bootstrap.TestScopeResolver;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.core.Booleans;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.PathUtils;
@@ -19,6 +20,7 @@ import org.elasticsearch.core.Strings;
 import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.entitlement.initialization.EntitlementInitialization;
 import org.elasticsearch.entitlement.runtime.policy.PathLookup;
+import org.elasticsearch.entitlement.runtime.policy.PathLookup.BaseDir;
 import org.elasticsearch.entitlement.runtime.policy.Policy;
 import org.elasticsearch.entitlement.runtime.policy.PolicyParser;
 import org.elasticsearch.entitlement.runtime.policy.TestPathLookup;
@@ -32,39 +34,106 @@ import java.io.InputStream;
 import java.net.URI;
 import java.net.URL;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.BiFunction;
+import java.util.function.Consumer;
 
 import static java.util.stream.Collectors.toCollection;
 import static java.util.stream.Collectors.toSet;
-import static org.elasticsearch.entitlement.runtime.policy.PathLookup.BaseDir.CONFIG;
 import static org.elasticsearch.entitlement.runtime.policy.PathLookup.BaseDir.TEMP;
+import static org.elasticsearch.env.Environment.PATH_DATA_SETTING;
+import static org.elasticsearch.env.Environment.PATH_HOME_SETTING;
+import static org.elasticsearch.env.Environment.PATH_REPO_SETTING;
 
 public class TestEntitlementBootstrap {
 
     private static final Logger logger = LogManager.getLogger(TestEntitlementBootstrap.class);
 
+    private static Map<BaseDir, Collection<Path>> baseDirPaths = new ConcurrentHashMap<>();
     private static TestPolicyManager policyManager;
 
     /**
      * Activates entitlement checking in tests.
      */
-    public static void bootstrap(@Nullable Path tempDir, @Nullable Path configDir) throws IOException {
+    public static void bootstrap(@Nullable Path tempDir) throws IOException {
         if (isEnabledForTest() == false) {
             return;
         }
-        TestPathLookup pathLookup = new TestPathLookup(Map.of(TEMP, zeroOrOne(tempDir), CONFIG, zeroOrOne(configDir)));
+        var previousTempDir = baseDirPaths.put(TEMP, zeroOrOne(tempDir));
+        assert previousTempDir == null : "Test entitlement bootstrap called multiple times";
+        TestPathLookup pathLookup = new TestPathLookup(baseDirPaths);
         policyManager = createPolicyManager(pathLookup);
         EntitlementInitialization.initializeArgs = new EntitlementInitialization.InitializeArgs(pathLookup, Set.of(), policyManager);
         logger.debug("Loading entitlement agent");
         EntitlementBootstrap.loadAgent(EntitlementBootstrap.findAgentJar(), EntitlementInitialization.class.getName());
     }
 
+    public static void registerNodeBaseDirs(Settings settings, Path configPath) {
+        if (policyManager == null) {
+            return;
+        }
+        Path homeDir = absolutePath(PATH_HOME_SETTING.get(settings));
+        Path configDir = configPath != null ? configPath : homeDir.resolve("config");
+        Collection<Path> dataDirs = dataDirs(settings, homeDir);
+        Collection<Path> repoDirs = repoDirs(settings);
+        logger.debug("Registering node dirs: config [{}], dataDirs [{}], repoDirs [{}]", configDir, dataDirs, repoDirs);
+        baseDirPaths.compute(BaseDir.CONFIG, baseDirModifier(paths -> paths.add(configDir)));
+        baseDirPaths.compute(BaseDir.DATA, baseDirModifier(paths -> paths.addAll(dataDirs)));
+        baseDirPaths.compute(BaseDir.SHARED_REPO, baseDirModifier(paths -> paths.addAll(repoDirs)));
+        policyManager.reset();
+    }
+
+    public static void unregisterNodeBaseDirs(Settings settings, Path configPath) {
+        if (policyManager == null) {
+            return;
+        }
+        Path homeDir = absolutePath(PATH_HOME_SETTING.get(settings));
+        Path configDir = configPath != null ? configPath : homeDir.resolve("config");
+        Collection<Path> dataDirs = dataDirs(settings, homeDir);
+        Collection<Path> repoDirs = repoDirs(settings);
+        logger.debug("Unregistering node dirs: config [{}], dataDirs [{}], repoDirs [{}]", configDir, dataDirs, repoDirs);
+        baseDirPaths.compute(BaseDir.CONFIG, baseDirModifier(paths -> paths.remove(configDir)));
+        baseDirPaths.compute(BaseDir.DATA, baseDirModifier(paths -> paths.removeAll(dataDirs)));
+        baseDirPaths.compute(BaseDir.SHARED_REPO, baseDirModifier(paths -> paths.removeAll(repoDirs)));
+        policyManager.reset();
+    }
+
+    private static Collection<Path> dataDirs(Settings settings, Path homeDir) {
+        List<String> dataDirs = PATH_DATA_SETTING.get(settings);
+        return dataDirs.isEmpty()
+            ? List.of(homeDir.resolve("data"))
+            : dataDirs.stream().map(TestEntitlementBootstrap::absolutePath).toList();
+    }
+
+    private static Collection<Path> repoDirs(Settings settings) {
+        return PATH_REPO_SETTING.get(settings).stream().map(TestEntitlementBootstrap::absolutePath).toList();
+    }
+
+    private static BiFunction<BaseDir, Collection<Path>, Collection<Path>> baseDirModifier(Consumer<Collection<Path>> consumer) {
+        return (BaseDir baseDir, Collection<Path> paths) -> {
+            if (paths == null) {
+                paths = new HashSet<>();
+            }
+            consumer.accept(paths);
+            return paths;
+        };
+    }
+
+    @SuppressForbidden(reason = "must be resolved using the default file system, rather then the mocked test file system")
+    private static Path absolutePath(String path) {
+        return Paths.get(path).toAbsolutePath().normalize();
+    }
+
     private static <T> List<T> zeroOrOne(T item) {
         if (item == null) {
             return List.of();

+ 22 - 10
test/framework/src/main/java/org/elasticsearch/node/MockNode.java

@@ -24,6 +24,7 @@ import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.common.util.MockBigArrays;
 import org.elasticsearch.common.util.MockPageCacheRecycler;
 import org.elasticsearch.common.util.PageCacheRecycler;
+import org.elasticsearch.entitlement.bootstrap.TestEntitlementBootstrap;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.http.HttpServerTransport;
 import org.elasticsearch.indices.ExecutorSelector;
@@ -53,6 +54,7 @@ import org.elasticsearch.transport.TransportInterceptor;
 import org.elasticsearch.transport.TransportService;
 import org.elasticsearch.transport.TransportSettings;
 
+import java.io.IOException;
 import java.nio.file.Path;
 import java.util.Collection;
 import java.util.Collections;
@@ -250,16 +252,7 @@ public class MockNode extends Node {
         final Path configPath,
         final boolean forbidPrivateIndexSettings
     ) {
-        this(
-            InternalSettingsPreparer.prepareEnvironment(
-                Settings.builder().put(TransportSettings.PORT.getKey(), ESTestCase.getPortRange()).put(settings).build(),
-                Collections.emptyMap(),
-                configPath,
-                () -> "mock_ node"
-            ),
-            classpathPlugins,
-            forbidPrivateIndexSettings
-        );
+        this(prepareEnvironment(settings, configPath), classpathPlugins, forbidPrivateIndexSettings);
     }
 
     private MockNode(
@@ -278,6 +271,25 @@ public class MockNode extends Node {
         this.classpathPlugins = classpathPlugins;
     }
 
+    private static Environment prepareEnvironment(final Settings settings, final Path configPath) {
+        TestEntitlementBootstrap.registerNodeBaseDirs(settings, configPath);
+        return InternalSettingsPreparer.prepareEnvironment(
+            Settings.builder().put(TransportSettings.PORT.getKey(), ESTestCase.getPortRange()).put(settings).build(),
+            Collections.emptyMap(),
+            configPath,
+            () -> "mock_ node"
+        );
+    }
+
+    @Override
+    public synchronized void close() throws IOException {
+        try {
+            super.close();
+        } finally {
+            TestEntitlementBootstrap.unregisterNodeBaseDirs(getEnvironment().settings(), getEnvironment().configDir());
+        }
+    }
+
     /**
      * The classpath plugins this node was constructed with.
      */

+ 0 - 1
test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java

@@ -285,7 +285,6 @@ import static org.hamcrest.Matchers.startsWith;
  * </ul>
  */
 @LuceneTestCase.SuppressFileSystems("ExtrasFS") // doesn't work with potential multi data path from test cluster yet
-@ESTestCase.WithoutEntitlements // ES-12042
 public abstract class ESIntegTestCase extends ESTestCase {
 
     /** node names of the corresponding clusters will start with these prefixes */

+ 0 - 1
test/framework/src/main/java/org/elasticsearch/test/ESSingleNodeTestCase.java

@@ -90,7 +90,6 @@ import static org.hamcrest.Matchers.lessThanOrEqualTo;
  * A test that keep a singleton node started for all tests that can be used to get
  * references to Guice injectors in unit tests.
  */
-@ESTestCase.WithoutEntitlements // ES-12042
 public abstract class ESSingleNodeTestCase extends ESTestCase {
 
     private static Node NODE = null;

+ 1 - 0
x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/test/SecuritySingleNodeTestCase.java

@@ -69,6 +69,7 @@ import static org.hamcrest.Matchers.is;
  * {@link SecurityIntegTestCase} due to simplicity and improved speed from not needing to start
  * multiple nodes and wait for the cluster to form.
  */
+@ESTestCase.WithoutEntitlements // requires entitlement delegation ES-12382
 public abstract class SecuritySingleNodeTestCase extends ESSingleNodeTestCase {
 
     private static SecuritySettingsSource SECURITY_DEFAULT_SETTINGS = null;

+ 1 - 0
x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityIntegTestCase.java

@@ -60,6 +60,7 @@ import static org.hamcrest.Matchers.hasItem;
  *
  * @see SecuritySettingsSource
  */
+@ESTestCase.WithoutEntitlements // requires entitlement delegation ES-12382
 public abstract class SecurityIntegTestCase extends ESIntegTestCase {
 
     private static SecuritySettingsSource SECURITY_DEFAULT_SETTINGS;