From ab75553df0a95dd201af38abf27ba6121420e136 Mon Sep 17 00:00:00 2001 From: Rene Groeschke Date: Mon, 11 Oct 2021 10:16:03 +0200 Subject: [PATCH] Simplify TestCluster extraJar configuration (#78837) Allow passing FileCollection instead of single Jar files. This makes using the API way easier as gradle configurations for resolving jars do not need to be resolved eagerly --- .../src/main/groovy/elasticsearch.fips.gradle | 16 +++++--------- .../gradle/TestClustersPluginFuncTest.groovy | 2 +- .../testclusters/ElasticsearchCluster.java | 5 +++-- .../testclusters/ElasticsearchNode.java | 21 ++++++++++++------- .../TestClusterConfiguration.java | 3 ++- 5 files changed, 24 insertions(+), 23 deletions(-) diff --git a/build-tools-internal/src/main/groovy/elasticsearch.fips.gradle b/build-tools-internal/src/main/groovy/elasticsearch.fips.gradle index 1ba28c49a9456..0a6175ff430a0 100644 --- a/build-tools-internal/src/main/groovy/elasticsearch.fips.gradle +++ b/build-tools-internal/src/main/groovy/elasticsearch.fips.gradle @@ -39,26 +39,20 @@ if (BuildParams.inFipsJvm) { copy fipsPolicy.name copy 'cacerts.bcfks' } + def extraFipsJarsConfiguration = configurations.detachedConfiguration(bcFips, bcTlsFips) project.afterEvaluate { - def extraFipsJars = configurations.detachedConfiguration(bcFips, bcTlsFips) // ensure that bouncycastle is on classpath for the all of test types, must happen in evaluateAfter since the rest tests explicitly // set the class path to help maintain pure black box testing, and here we are adding to that classpath tasks.withType(Test).configureEach { Test test -> - test.setClasspath(test.getClasspath().plus(extraFipsJars)) + test.setClasspath(test.getClasspath().plus(extraFipsJarsConfiguration)) } } pluginManager.withPlugin("elasticsearch.testclusters") { - afterEvaluate { - // This afterEvaluate hooks is required to avoid deprecated configuration resolution - // This configuration can be removed once system modules are available - def extraFipsJars = configurations.detachedConfiguration(bcFips, bcTlsFips) - testClusters.configureEach { - extraFipsJars.files.each { - extraJarFile it - } - } + // This configuration can be removed once system modules are available + testClusters.configureEach { + extraJarFiles extraFipsJarsConfiguration } tasks.withType(TestClustersAware).configureEach { dependsOn 'fipsResources' diff --git a/build-tools/src/integTest/groovy/org/elasticsearch/gradle/TestClustersPluginFuncTest.groovy b/build-tools/src/integTest/groovy/org/elasticsearch/gradle/TestClustersPluginFuncTest.groovy index 686bfaa32ba13..4de1d133900be 100644 --- a/build-tools/src/integTest/groovy/org/elasticsearch/gradle/TestClustersPluginFuncTest.groovy +++ b/build-tools/src/integTest/groovy/org/elasticsearch/gradle/TestClustersPluginFuncTest.groovy @@ -206,7 +206,7 @@ class TestClustersPluginFuncTest extends AbstractGradleFuncTest { testClusters { myCluster { testDistribution = 'default' - extraJarFile(file('${someJar().absolutePath}')) + extraJarFiles(files('${someJar().absolutePath}')) } } diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java index 51658eea326b1..947d1c6dc22f8 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java @@ -15,6 +15,7 @@ import org.gradle.api.NamedDomainObjectContainer; import org.gradle.api.Project; import org.gradle.api.file.ArchiveOperations; +import org.gradle.api.file.FileCollection; import org.gradle.api.file.FileSystemOperations; import org.gradle.api.file.RegularFile; import org.gradle.api.logging.Logger; @@ -433,8 +434,8 @@ public void extraConfigFile(String destination, File from, PropertyNormalization } @Override - public void extraJarFile(File from) { - nodes.all(node -> node.extraJarFile(from)); + public void extraJarFiles(FileCollection from) { + nodes.all(node -> node.extraJarFiles(from)); } @Override diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java index 9f83f43aa7cc7..8ba787a773f95 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java @@ -142,7 +142,7 @@ public class ElasticsearchNode implements TestClusterConfiguration { private final LazyPropertyMap environment = new LazyPropertyMap<>("Environment", this); private final LazyPropertyList jvmArgs = new LazyPropertyList<>("JVM arguments", this); private final LazyPropertyMap extraConfigFiles = new LazyPropertyMap<>("Extra config files", this, FileEntry::new); - private final LazyPropertyList extraJarFiles = new LazyPropertyList<>("Extra jar files", this); + private final LazyPropertyList extraJarConfigurations = new LazyPropertyList<>("Extra jar files", this); private final List> credentials = new ArrayList<>(); final LinkedHashMap defaultConfig = new LinkedHashMap<>(); @@ -602,7 +602,7 @@ private boolean canUseSharedDistribution() { // using original location can be too long due to MAX_PATH restrictions on windows CI // TODO revisit when moving to shorter paths on CI by using Teamcity return OS.current() != OS.WINDOWS - && extraJarFiles.size() == 0 + && extraJarConfigurations.size() == 0 && modules.size() == 0 && plugins.size() == 0 && requiresAddingXPack() == false; @@ -668,10 +668,18 @@ private void copyExtraConfigFiles() { * //TODO: Remove this when system modules are available */ private void copyExtraJars() { + List extraJarFiles = this.extraJarConfigurations.stream() + .flatMap(fileCollection -> fileCollection.getFiles().stream()) + .collect(Collectors.toList()); + if (extraJarFiles.isEmpty() == false) { - logToProcessStdout("Setting up " + extraJarFiles.size() + " additional jar dependencies"); + logToProcessStdout("Setting up " + this.extraJarConfigurations.size() + " additional jar dependencies"); } extraJarFiles.forEach(from -> { + if (from.getName().endsWith(".jar") == false) { + throw new IllegalArgumentException("extra jar file " + from.toString() + " doesn't appear to be a JAR"); + } + Path destination = getDistroDir().resolve("lib").resolve(from.getName()); try { Files.copy(from.toPath(), destination, StandardCopyOption.REPLACE_EXISTING); @@ -720,11 +728,8 @@ public void extraConfigFile(String destination, File from, PropertyNormalization } @Override - public void extraJarFile(File from) { - if (from.toString().endsWith(".jar") == false) { - throw new IllegalArgumentException("extra jar file " + from.toString() + " doesn't appear to be a JAR"); - } - extraJarFiles.add(from); + public void extraJarFiles(FileCollection from) { + extraJarConfigurations.add(from); } @Override diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/TestClusterConfiguration.java b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/TestClusterConfiguration.java index 990dba16d553b..9e8688e47d842 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/TestClusterConfiguration.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/testclusters/TestClusterConfiguration.java @@ -9,6 +9,7 @@ import org.elasticsearch.gradle.FileSupplier; import org.elasticsearch.gradle.PropertyNormalization; +import org.gradle.api.file.FileCollection; import org.gradle.api.file.RegularFile; import org.gradle.api.logging.Logging; import org.gradle.api.provider.Provider; @@ -89,7 +90,7 @@ public interface TestClusterConfiguration { void extraConfigFile(String destination, File from, PropertyNormalization normalization); - void extraJarFile(File from); + void extraJarFiles(FileCollection from); void user(Map userSpec);