Fix ElasticsearchNode input calculation to be Gradle 7.0 compliant (#70943) (#71458)

This makes test cluster input calculation Gradle 7.0 compliant.

It reworks input tracking for test cluster by using artifact transforms to inspect plugin zips 
in ElasticseachNode effectivly. 
By using artifact transforms we also ensure we do not unpack unchanged modules or plugins 
again and again and reuse former unpacked archives instead across multiple builds.

We also add full  integ test coverage for ElasticsearchNode inputs calculation logic here.

Author: breskeby

buildSrc/src/integTest/groovy/org/elasticsearch/gradle/TestClustersPluginFuncTest.groovy

@@ -9,8 +9,12 @@
 package org.elasticsearch.gradle
 
 import org.elasticsearch.gradle.fixtures.AbstractGradleFuncTest
+import org.gradle.testkit.runner.GradleRunner
 import spock.lang.IgnoreIf
+import spock.lang.Unroll
 
+import static org.elasticsearch.gradle.fixtures.DistributionDownloadFixture.withChangedClasspathMockedDistributionDownload
+import static org.elasticsearch.gradle.fixtures.DistributionDownloadFixture.withChangedConfigMockedDistributionDownload
 import static org.elasticsearch.gradle.fixtures.DistributionDownloadFixture.withMockedDistributionDownload
 
 /**
@@ -22,14 +26,34 @@ class TestClustersPluginFuncTest extends AbstractGradleFuncTest {
 
     def setup() {
         buildFile << """
-            import org.elasticsearch.gradle.testclusters.DefaultTestClustersTask
+            import org.elasticsearch.gradle.testclusters.TestClustersAware
+            import org.elasticsearch.gradle.testclusters.ElasticsearchCluster
             plugins {
                 id 'elasticsearch.testclusters'
             }
 
-            class SomeClusterAwareTask extends DefaultTestClustersTask {
+            class SomeClusterAwareTask extends DefaultTask implements TestClustersAware {
+
+                private Collection<ElasticsearchCluster> clusters = new HashSet<>();
+            
+                @Override
+                @Nested
+                public Collection<ElasticsearchCluster> getClusters() {
+                    return clusters;
+                }
+                
+                @OutputFile
+                Provider<RegularFile> outputFile
+             
+                @Inject
+                SomeClusterAwareTask(ProjectLayout projectLayout) {
+                    outputFile = projectLayout.getBuildDirectory().file("someclusteraware.txt")
+                }
+   
                 @TaskAction void doSomething() {
+                    outputFile.get().getAsFile().text = "done"
                     println 'SomeClusterAwareTask executed'
+                    
                 }
             }
         """
@@ -61,6 +85,98 @@ class TestClustersPluginFuncTest extends AbstractGradleFuncTest {
         assertNoCustomDistro('myCluster')
     }
 
+    @Unroll
+    def "test cluster #inputProperty change is detected"() {
+        given:
+        buildFile << """
+            testClusters {
+              myCluster {
+                testDistribution = 'default'
+              }
+            }
+
+            tasks.register('myTask', SomeClusterAwareTask) {
+                useCluster testClusters.myCluster
+            }
+        """
+
+        when:
+        def runner = gradleRunner("myTask", '-i', '-g', 'guh')
+        def runningClosure = { GradleRunner r -> r.build() }
+        withMockedDistributionDownload(runner, runningClosure)
+        def result = inputProperty == "distributionClasspath" ?
+                withChangedClasspathMockedDistributionDownload(runner, runningClosure) :
+                withChangedConfigMockedDistributionDownload(runner, runningClosure)
+
+        then:
+        normalized(result.output).contains("Task ':myTask' is not up-to-date because:\n  Input property 'clusters.myCluster\$0.nodes.\$0.$inputProperty'")
+        result.output.contains("elasticsearch-keystore script executed!")
+        assertEsLogContains("myCluster", "Starting Elasticsearch process")
+        assertEsLogContains("myCluster", "Stopping node")
+        assertNoCustomDistro('myCluster')
+
+        where:
+        inputProperty << ["distributionClasspath", "distributionFiles"]
+    }
+
+    @Unroll
+    def "test cluster modules #propertyName change is detected"() {
+        given:
+        addSubProject("test-module") << """
+            plugins {
+                id 'elasticsearch.esplugin'
+            }
+            // do not hassle with resolving predefined dependencies
+            configurations.compileOnly.dependencies.clear()
+            configurations.testImplementation.dependencies.clear()
+            
+            esplugin {
+                name = 'test-module'
+                classname 'org.acme.TestModule'
+                description = "test module description"
+            }
+            
+            licenseFile = file('license.txt')
+            noticeFile = file('notice.txt')
+            version = "1.0"
+            group = 'org.acme'
+        """
+        buildFile << """
+            testClusters {
+              myCluster {
+                testDistribution = 'default'
+                module ':test-module'
+              }
+            }
+
+            tasks.register('myTask', SomeClusterAwareTask) {
+                useCluster testClusters.myCluster
+            }
+        """
+
+        when:
+        withMockedDistributionDownload(gradleRunner("myTask", '-g', 'guh')) {
+            build()
+        }
+        fileChange.delegate = this
+        fileChange.call(this)
+        def result = withMockedDistributionDownload(gradleRunner("myTask", '-i', '-g', 'guh')) {
+            build()
+        }
+
+        then:
+        normalized(result.output).contains("Task ':myTask' is not up-to-date because:\n" +
+                "  Input property 'clusters.myCluster\$0.nodes.\$0.$propertyName'")
+        result.output.contains("elasticsearch-keystore script executed!")
+        assertEsLogContains("myCluster", "Starting Elasticsearch process")
+        assertEsLogContains("myCluster", "Stopping node")
+
+        where:
+        propertyName         | fileChange
+        "installedFiles"     | { def testClazz -> testClazz.file("test-module/src/main/plugin-metadata/someAddedConfig.txt") << "new resource file" }
+        "installedClasspath" | { def testClazz -> testClazz.file("test-module/src/main/java/SomeClass.java") << "class SomeClass {}" }
+    }
+
     def "can declare test cluster in lazy evaluated task configuration block"() {
         given:
         buildFile << """

buildSrc/src/integTest/groovy/org/elasticsearch/gradle/fixtures/DistributionDownloadFixture.groovy

@@ -23,10 +23,30 @@ class DistributionDownloadFixture {
                 gradleRunner, buildRunClosure)
     }
 
+    static BuildResult withChangedClasspathMockedDistributionDownload(GradleRunner gradleRunner, Closure<BuildResult> buildRunClosure) {
+        return doRunWithMockedDistributionDownload(VersionProperties.getElasticsearch(), ElasticsearchDistribution.CURRENT_PLATFORM,
+                gradleRunner, buildRunClosure, true, false)
+    }
+    static BuildResult withChangedConfigMockedDistributionDownload(GradleRunner gradleRunner, Closure<BuildResult> buildRunClosure) {
+        return doRunWithMockedDistributionDownload(VersionProperties.getElasticsearch(), ElasticsearchDistribution.CURRENT_PLATFORM,
+                gradleRunner, buildRunClosure, false, true)
+    }
+
     static BuildResult withMockedDistributionDownload(String version, ElasticsearchDistribution.Platform platform,
                                                       GradleRunner gradleRunner, Closure<BuildResult> buildRunClosure) {
+        return doRunWithMockedDistributionDownload(version, platform, gradleRunner, buildRunClosure, false, false)
+    }
+
+    static BuildResult withChangedClasspathMockedDistributionDownload(String version, ElasticsearchDistribution.Platform platform,
+                                                      GradleRunner gradleRunner, Closure<BuildResult> buildRunClosure) {
+        return doRunWithMockedDistributionDownload(version, platform, gradleRunner, buildRunClosure, true, false)
+    }
+
+    private static BuildResult doRunWithMockedDistributionDownload(String version, ElasticsearchDistribution.Platform platform,
+                                                                   GradleRunner gradleRunner, Closure<BuildResult> buildRunClosure,
+                                                                   boolean withAddedJar, boolean withAddedConfig) {
         String urlPath = urlPath(version, platform);
-        return WiremockFixture.withWireMock(urlPath, filebytes(urlPath)) { server ->
+        return WiremockFixture.withWireMock(urlPath, filebytes(urlPath, withAddedJar, withAddedConfig)) { server ->
             File initFile = new File(gradleRunner.getProjectDir(), INIT_SCRIPT)
             initFile.text = """allprojects { p ->
                 p.repositories.all { repo ->
@@ -47,8 +67,9 @@ class DistributionDownloadFixture {
         "/downloads/elasticsearch/elasticsearch-${version}-${platform}-${Architecture.current().classifier}.$fileType"
     }
 
-    private static byte[] filebytes(String urlPath) throws IOException {
-        String suffix = urlPath.endsWith("zip") ? "zip" : "tar.gz";
-        return DistributionDownloadFixture.getResourceAsStream("/org/elasticsearch/gradle/fake_elasticsearch." + suffix).getBytes()
+    private static byte[] filebytes(String urlPath, boolean withAddedJar, boolean withAddedConfig) throws IOException {
+        String distro = (withAddedJar ? "-with-added-jar" : "") + (withAddedConfig ? "-with-added-config" : "")
+        String suffix = urlPath.endsWith("zip") ? ".zip" : ".tar.gz";
+        return DistributionDownloadFixture.getResourceAsStream("/org/elasticsearch/gradle/fake_elasticsearch${distro}" + suffix).getBytes()
     }
 }

buildSrc/src/integTest/resources/org/elasticsearch/gradle/fake_elasticsearch-with-added-config.tar.gz

@@ -82,7 +82,7 @@ public static void configureRepositories(Project project) {
     }
 
     private static void assertRepositoryURIIsSecure(final String repositoryName, final String projectPath, final URI uri) {
-        if (uri != null && SECURE_URL_SCHEMES.contains(uri.getScheme()) == false) {
+        if (uri != null && SECURE_URL_SCHEMES.contains(uri.getScheme()) == false && uri.getHost().equals("localhost") == false) {
             String url;
             try {
                 url = uri.toURL().toString();

buildSrc/src/integTest/resources/org/elasticsearch/gradle/fake_elasticsearch-with-added-jar.tar.gz

@@ -23,16 +23,23 @@
 import org.elasticsearch.gradle.VersionProperties;
 import org.elasticsearch.gradle.http.WaitForHttpResource;
 import org.elasticsearch.gradle.info.BuildParams;
+import org.elasticsearch.gradle.transform.UnzipTransform;
 import org.elasticsearch.gradle.util.Pair;
 import org.gradle.api.Action;
 import org.gradle.api.Named;
 import org.gradle.api.NamedDomainObjectContainer;
 import org.gradle.api.Project;
 import org.gradle.api.artifacts.Configuration;
+import org.gradle.api.artifacts.Dependency;
+import org.gradle.api.artifacts.type.ArtifactTypeDefinition;
+import org.gradle.api.attributes.Attribute;
 import org.gradle.api.file.ArchiveOperations;
+import org.gradle.api.file.ConfigurableFileCollection;
+import org.gradle.api.file.FileCollection;
 import org.gradle.api.file.FileSystemOperations;
 import org.gradle.api.file.FileTree;
 import org.gradle.api.file.RegularFile;
+import org.gradle.api.internal.artifacts.ArtifactAttributes;
 import org.gradle.api.logging.Logger;
 import org.gradle.api.logging.Logging;
 import org.gradle.api.provider.Provider;
@@ -64,7 +71,6 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
@@ -126,6 +132,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
 
     private final LinkedHashMap<String, Predicate<TestClusterConfiguration>> waitConditions = new LinkedHashMap<>();
     private final Map<String, Configuration> pluginAndModuleConfigurations = new HashMap<>();
+    private final ConfigurableFileCollection pluginAndModuleConfiguration;
     private final List<Provider<File>> plugins = new ArrayList<>();
     private final List<Provider<File>> modules = new ArrayList<>();
     final LazyPropertyMap<String, CharSequence> settings = new LazyPropertyMap<>("Settings", this);
@@ -161,6 +168,8 @@ public class ElasticsearchNode implements TestClusterConfiguration {
     private String keystorePassword = "";
     private boolean preserveDataDir = false;
 
+    private Attribute<Boolean> bundleAttribute = Attribute.of("bundle", Boolean.class);
+
     ElasticsearchNode(
         String clusterName,
         String path,
@@ -195,8 +204,10 @@ public class ElasticsearchNode implements TestClusterConfiguration {
         waitConditions.put("ports files", this::checkPortsFilesExistWithDelay);
         defaultConfig.put("cluster.name", clusterName);
 
+        pluginAndModuleConfiguration = project.getObjects().fileCollection();
         setTestDistribution(TestDistribution.INTEG_TEST);
         setVersion(VersionProperties.getElasticsearch());
+        configureArtifactTransforms();
     }
 
     @Input
@@ -281,11 +292,15 @@ Collection<Configuration> getPluginAndModuleConfigurations() {
     // creates a configuration to depend on the given plugin project, then wraps that configuration
     // to grab the zip as a file provider
     private Provider<RegularFile> maybeCreatePluginOrModuleDependency(String path) {
-        Configuration configuration = pluginAndModuleConfigurations.computeIfAbsent(
-            path,
-            key -> project.getConfigurations()
-                .detachedConfiguration(project.getDependencies().project(Map.of("path", path, "configuration", "zip")))
-        );
+        Configuration configuration = pluginAndModuleConfigurations.computeIfAbsent(path, key -> {
+            Dependency bundleDependency = this.project.getDependencies().project(Map.of("path", path, "configuration", "zip"));
+            return project.getConfigurations().detachedConfiguration(bundleDependency);
+        });
+
+        /**
+         * dependencies.create(files(provider { println("provider"); File(".") }))
+         *
+         * */
         Provider<File> fileProvider = configuration.getElements()
             .map(
                 s -> s.stream()
@@ -299,6 +314,7 @@ private Provider<RegularFile> maybeCreatePluginOrModuleDependency(String path) {
     @Override
     public void plugin(Provider<RegularFile> plugin) {
         checkFrozen();
+        registerExtractedConfig(plugin);
         this.plugins.add(plugin.map(RegularFile::getAsFile));
     }
 
@@ -310,9 +326,32 @@ public void plugin(String pluginProjectPath) {
     @Override
     public void module(Provider<RegularFile> module) {
         checkFrozen();
+        registerExtractedConfig(module);
         this.modules.add(module.map(RegularFile::getAsFile));
     }
 
+    private void registerExtractedConfig(Provider<RegularFile> pluginProvider) {
+        Dependency pluginDependency = this.project.getDependencies().create(project.files(pluginProvider));
+        Configuration extractedConfig = project.getConfigurations().detachedConfiguration(pluginDependency);
+        extractedConfig.getAttributes().attribute(ArtifactAttributes.ARTIFACT_FORMAT, ArtifactTypeDefinition.DIRECTORY_TYPE);
+        extractedConfig.getAttributes().attribute(bundleAttribute, true);
+        pluginAndModuleConfiguration.from(extractedConfig);
+    }
+
+    private void configureArtifactTransforms() {
+        project.getDependencies().getAttributesSchema().attribute(bundleAttribute);
+        project.getDependencies().getArtifactTypes().maybeCreate(ArtifactTypeDefinition.ZIP_TYPE);
+        project.getDependencies().registerTransform(UnzipTransform.class, transformSpec -> {
+            transformSpec.getFrom()
+                .attribute(ArtifactAttributes.ARTIFACT_FORMAT, ArtifactTypeDefinition.ZIP_TYPE)
+                .attribute(bundleAttribute, true);
+            transformSpec.getTo()
+                .attribute(ArtifactAttributes.ARTIFACT_FORMAT, ArtifactTypeDefinition.DIRECTORY_TYPE)
+                .attribute(bundleAttribute, true);
+            transformSpec.getParameters().setAsFiletreeOutput(true);
+        });
+    }
+
     @Override
     public void module(String moduleProjectPath) {
         module(maybeCreatePluginOrModuleDependency(moduleProjectPath));
@@ -1325,26 +1364,15 @@ private Path getExtractedDistributionDir() {
         return distributions.get(currentDistro).getExtracted().getSingleFile().toPath();
     }
 
-    private List<File> getInstalledFileSet(Action<? super PatternFilterable> filter) {
-        return Stream.concat(plugins.stream().map(Provider::get), modules.stream().map(Provider::get))
-            .filter(File::exists)
-            // TODO: We may be able to simplify this with Gradle 5.6
-            // https://docs.gradle.org/nightly/release-notes.html#improved-handling-of-zip-archives-on-classpaths
-            .map(zipFile -> archiveOperations.zipTree(zipFile).matching(filter))
-            .flatMap(tree -> tree.getFiles().stream())
-            .sorted(Comparator.comparing(File::getName))
-            .collect(Collectors.toList());
-    }
-
     @Classpath
-    public List<File> getInstalledClasspath() {
-        return getInstalledFileSet(filter -> filter.include("**/*.jar"));
+    public FileCollection getInstalledClasspath() {
+        return pluginAndModuleConfiguration.filter(f -> f.isDirectory() == false && f.getName().endsWith(".jar"));
     }
 
     @InputFiles
     @PathSensitive(PathSensitivity.RELATIVE)
-    public List<File> getInstalledFiles() {
-        return getInstalledFileSet(filter -> filter.exclude("**/*.jar"));
+    public FileCollection getInstalledFiles() {
+        return pluginAndModuleConfiguration.filter(f -> f.isDirectory() == false && f.getName().endsWith(".jar") == false);
     }
 
     @Classpath

buildSrc/src/main/java/org/elasticsearch/gradle/RepositoriesSetupPlugin.java

@@ -11,6 +11,7 @@
 import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
 import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
 import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream;
+import org.gradle.api.artifacts.transform.TransformOutputs;
 
 import java.io.File;
 import java.io.FileInputStream;
@@ -27,7 +28,7 @@ public abstract class SymbolicLinkPreservingUntarTransform implements UnpackTran
 
     private static final Path CURRENT_DIR_PATH = Paths.get(".");
 
-    public void unpack(File tarFile, File targetDir) throws IOException {
+    public void unpack(File tarFile, File targetDir, TransformOutputs outputs, boolean asFiletreeOutput) throws IOException {
         Function<String, Path> pathModifier = pathResolver();
         TarArchiveInputStream tar = new TarArchiveInputStream(new GzipCompressorInputStream(new FileInputStream(tarFile)));
         final Path destinationPath = targetDir.toPath();
@@ -54,6 +55,9 @@ public void unpack(File tarFile, File targetDir) throws IOException {
                 try (FileOutputStream fos = new FileOutputStream(destination.toFile())) {
                     tar.transferTo(fos);
                 }
+                if (asFiletreeOutput) {
+                    outputs.file(destination.toFile());
+                }
             }
             if (entry.isSymbolicLink() == false) {
                 // check if the underlying file system supports POSIX permissions