Third party audit improovements (#36167)

- fix up to date checks to ignore elasticsearch jars. We were not scanning them but these still triggered a rebuild.
- add tests to assert correct behavior and up to date checks.
- make the task less verbose with `-i` and include the output only on errors.

buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ThirdPartyAuditTask.java

@@ -24,13 +24,16 @@ import org.gradle.api.DefaultTask;
 import org.gradle.api.GradleException;
 import org.gradle.api.JavaVersion;
 import org.gradle.api.artifacts.Configuration;
-import org.gradle.api.file.FileCollection;
+import org.gradle.api.artifacts.Dependency;
 import org.gradle.api.file.FileTree;
+import org.gradle.api.specs.Spec;
+import org.gradle.api.tasks.CacheableTask;
 import org.gradle.api.tasks.Input;
 import org.gradle.api.tasks.InputFile;
 import org.gradle.api.tasks.InputFiles;
+import org.gradle.api.tasks.Optional;
 import org.gradle.api.tasks.OutputDirectory;
-import org.gradle.api.tasks.StopExecutionException;
+import org.gradle.api.tasks.SkipWhenEmpty;
 import org.gradle.api.tasks.TaskAction;
 import org.gradle.process.ExecResult;
 
@@ -49,6 +52,7 @@ import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
+@CacheableTask
 public class ThirdPartyAuditTask extends DefaultTask {
 
     private static final Pattern MISSING_CLASS_PATTERN = Pattern.compile(
@@ -93,16 +97,8 @@ public class ThirdPartyAuditTask extends DefaultTask {
         this.signatureFile = signatureFile;
     }
 
-    @InputFiles
-    public Configuration getRuntimeConfiguration() {
-        Configuration runtime = getProject().getConfigurations().findByName("runtime");
-        if (runtime == null) {
-            return getProject().getConfigurations().getByName("testCompile");
-        }
-        return runtime;
-    }
-
     @Input
+    @Optional
     public String getJavaHome() {
         return javaHome;
     }
@@ -111,11 +107,6 @@ public class ThirdPartyAuditTask extends DefaultTask {
         this.javaHome = javaHome;
     }
 
-    @InputFiles
-    public Configuration getCompileOnlyConfiguration() {
-        return getProject().getConfigurations().getByName("compileOnly");
-    }
-
     @OutputDirectory
     public File getJarExpandDir() {
         return new File(
@@ -139,9 +130,29 @@ public class ThirdPartyAuditTask extends DefaultTask {
         return Collections.unmodifiableSet(excludes);
     }
 
+    @InputFiles
+    @SkipWhenEmpty
+    public Set<File> getJarsToScan() {
+        // These are SelfResolvingDependency, and some of them backed by file collections, like  the Gradle API files,
+        // or dependencies added as `files(...)`, we can't be sure if those are third party or not.
+        // err on the side of scanning these to make sure we don't miss anything
+        Spec<Dependency> reallyThirdParty = dep -> dep.getGroup() != null &&
+            dep.getGroup().startsWith("org.elasticsearch") == false;
+        Set<File> jars = getRuntimeConfiguration()
+            .getResolvedConfiguration()
+            .getFiles(reallyThirdParty);
+        Set<File> compileOnlyConfiguration = getProject().getConfigurations().getByName("compileOnly").getResolvedConfiguration()
+            .getFiles(reallyThirdParty);
+        // don't scan provided dependencies that we already scanned, e.x. don't scan cores dependencies for every plugin
+        if (compileOnlyConfiguration != null) {
+            jars.removeAll(compileOnlyConfiguration);
+        }
+        return jars;
+    }
+
     @TaskAction
     public void runThirdPartyAudit() throws IOException {
-        FileCollection jars = getJarsToScan();
+        Set<File> jars = getJarsToScan();
 
         extractJars(jars);
 
@@ -161,14 +172,17 @@ public class ThirdPartyAuditTask extends DefaultTask {
 
         Set<String> jdkJarHellClasses = runJdkJarHellCheck();
 
-        assertNoPointlessExclusions(missingClasses, violationsClasses, jdkJarHellClasses);
-
-        assertNoMissingAndViolations(missingClasses, violationsClasses);
-
-        assertNoJarHell(jdkJarHellClasses);
+        try {
+            assertNoPointlessExclusions(missingClasses, violationsClasses, jdkJarHellClasses);
+            assertNoMissingAndViolations(missingClasses, violationsClasses);
+            assertNoJarHell(jdkJarHellClasses);
+        } catch (IllegalStateException e) {
+            getLogger().error(forbiddenApisOutput);
+            throw e;
+        }
     }
 
-    private void extractJars(FileCollection jars) {
+    private void extractJars(Set<File> jars) {
         File jarExpandDir = getJarExpandDir();
         // We need to clean up to make sure old dependencies don't linger
         getProject().delete(jarExpandDir);
@@ -209,7 +223,10 @@ public class ThirdPartyAuditTask extends DefaultTask {
     private void assertNoJarHell(Set<String> jdkJarHellClasses) {
         jdkJarHellClasses.removeAll(excludes);
         if (jdkJarHellClasses.isEmpty() == false) {
-            throw new IllegalStateException("Jar Hell with the JDK:" + formatClassList(jdkJarHellClasses));
+            throw new IllegalStateException(
+                "Audit of third party dependencies failed:\n" +
+                    "  Jar Hell with the JDK:\n" + formatClassList(jdkJarHellClasses)
+            );
         }
     }
 
@@ -245,11 +262,13 @@ public class ThirdPartyAuditTask extends DefaultTask {
     private String runForbiddenAPIsCli() throws IOException {
         ByteArrayOutputStream errorOut = new ByteArrayOutputStream();
         getProject().javaexec(spec -> {
-            spec.setExecutable(javaHome + "/bin/java");
+            if (javaHome != null) {
+                spec.setExecutable(javaHome + "/bin/java");
+            }
             spec.classpath(
                 getForbiddenAPIsConfiguration(),
                 getRuntimeConfiguration(),
-                getCompileOnlyConfiguration()
+                getProject().getConfigurations().getByName("compileOnly")
             );
             spec.setMain("de.thetaphi.forbiddenapis.cli.CliMain");
             spec.args(
@@ -267,26 +286,9 @@ public class ThirdPartyAuditTask extends DefaultTask {
         try (ByteArrayOutputStream outputStream = errorOut) {
             forbiddenApisOutput = outputStream.toString(StandardCharsets.UTF_8.name());
         }
-        if (getLogger().isInfoEnabled()) {
-            getLogger().info(forbiddenApisOutput);
-        }
         return forbiddenApisOutput;
     }
 
-    private FileCollection getJarsToScan() {
-        FileCollection jars = getRuntimeConfiguration()
-            .fileCollection(dep -> dep.getGroup().startsWith("org.elasticsearch") == false);
-        Configuration compileOnlyConfiguration = getCompileOnlyConfiguration();
-        // don't scan provided dependencies that we already scanned, e.x. don't scan cores dependencies for every plugin
-        if (compileOnlyConfiguration != null) {
-            jars.minus(compileOnlyConfiguration);
-        }
-        if (jars.isEmpty()) {
-            throw new StopExecutionException("No jars to scan");
-        }
-        return jars;
-    }
-
     private String formatClassList(Set<String> classList) {
         return classList.stream()
             .map(name -> "  * " + name)
@@ -304,7 +306,7 @@ public class ThirdPartyAuditTask extends DefaultTask {
                 spec.classpath(
                     location.toURI().getPath(),
                     getRuntimeConfiguration(),
-                    getCompileOnlyConfiguration()
+                    getProject().getConfigurations().getByName("compileOnly")
                 );
             } catch (URISyntaxException e) {
                 throw new AssertionError(e);
@@ -312,7 +314,9 @@ public class ThirdPartyAuditTask extends DefaultTask {
             spec.setMain(JdkJarHellCheck.class.getName());
             spec.args(getJarExpandDir());
             spec.setIgnoreExitValue(true);
-            spec.setExecutable(javaHome + "/bin/java");
+            if (javaHome != null) {
+                spec.setExecutable(javaHome + "/bin/java");
+            }
             spec.setStandardOutput(standardOut);
         });
         if (execResult.getExitValue() == 0) {
@@ -325,5 +329,11 @@ public class ThirdPartyAuditTask extends DefaultTask {
         return new TreeSet<>(Arrays.asList(jdkJarHellCheckList.split("\\r?\\n")));
     }
 
-
+    private Configuration getRuntimeConfiguration() {
+        Configuration runtime = getProject().getConfigurations().findByName("runtime");
+        if (runtime == null) {
+            return getProject().getConfigurations().getByName("testCompile");
+        }
+        return runtime;
+    }
 }

buildSrc/src/test/java/org/elasticsearch/gradle/precommit/ThirdPartyAuditTaskIT.java

@@ -0,0 +1,135 @@
+package org.elasticsearch.gradle.precommit;
+
+import org.elasticsearch.gradle.test.GradleIntegrationTestCase;
+import org.gradle.testkit.runner.BuildResult;
+import org.junit.Before;
+
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+public class ThirdPartyAuditTaskIT extends GradleIntegrationTestCase {
+
+    @Before
+    public void setUp() throws Exception {
+        // Build the sample jars
+        getGradleRunner("thirdPartyAudit")
+            .withArguments("build", "-s")
+            .build();
+    }
+
+    public void testElasticsearchIgnored() {
+        BuildResult result = getGradleRunner("thirdPartyAudit")
+            .withArguments("clean", "empty", "-s",
+                "-PcompileOnlyGroup=elasticsearch.gradle:broken-log4j", "-PcompileOnlyVersion=0.0.1",
+                "-PcompileGroup=elasticsearch.gradle:dummy-io", "-PcompileVersion=0.0.1"
+            )
+            .build();
+        assertTaskNoSource(result, ":empty");
+    }
+
+    public void testWithEmptyRules() {
+        BuildResult result = getGradleRunner("thirdPartyAudit")
+            .withArguments("clean", "empty", "-s",
+                "-PcompileOnlyGroup=other.gradle:broken-log4j", "-PcompileOnlyVersion=0.0.1",
+                "-PcompileGroup=other.gradle:dummy-io", "-PcompileVersion=0.0.1"
+            )
+            .build();
+
+        assertTaskSuccessful(result, ":empty");
+
+        result = getGradleRunner("thirdPartyAudit")
+            .withArguments("empty", "-s",
+                "-PcompileOnlyGroup=other.gradle:broken-log4j", "-PcompileOnlyVersion=0.0.1",
+                "-PcompileGroup=other.gradle:dummy-io", "-PcompileVersion=0.0.1"
+            )
+            .build();
+
+        assertTaskUpToDate(result, ":empty");
+
+        result = getGradleRunner("thirdPartyAudit")
+            .withArguments("empty", "-s",
+                "-PcompileOnlyGroup=other.gradle:broken-log4j", "-PcompileOnlyVersion=0.0.1",
+                "-PcompileGroup=other.gradle:dummy-io", "-PcompileVersion=0.0.2"
+            )
+            .build();
+
+        assertTaskSuccessful(result, ":empty");
+    }
+
+    public void testViolationFoundAndCompileOnlyIgnored() {
+        BuildResult result = getGradleRunner("thirdPartyAudit")
+            .withArguments("clean", "absurd", "-s",
+                "-PcompileOnlyGroup=other.gradle:broken-log4j", "-PcompileOnlyVersion=0.0.1",
+                "-PcompileGroup=other.gradle:dummy-io", "-PcompileVersion=0.0.1"
+            )
+            .buildAndFail();
+
+        assertTaskFailed(result, ":absurd");
+        assertOutputContains(result.getOutput(),
+            "> Audit of third party dependencies failed:",
+            "  Classes with violations:",
+            "    * TestingIO"
+        );
+        assertOutputDoesNotContain(result.getOutput(),"Missing classes:");
+    }
+
+    public void testClassNotFoundAndCompileOnlyIgnored() {
+        BuildResult result = getGradleRunner("thirdPartyAudit")
+            .withArguments("clean", "absurd", "-s",
+                "-PcompileGroup=other.gradle:broken-log4j", "-PcompileVersion=0.0.1",
+                "-PcompileOnlyGroup=other.gradle:dummy-io", "-PcompileOnlyVersion=0.0.1"
+            )
+            .buildAndFail();
+        assertTaskFailed(result, ":absurd");
+
+        assertOutputContains(result.getOutput(),
+            "> Audit of third party dependencies failed:",
+            "  Missing classes:",
+            "    * org.apache.logging.log4j.LogManager"
+        );
+        assertOutputDoesNotContain(result.getOutput(), "Classes with violations:");
+    }
+
+    public void testJarHellWithJDK() {
+        BuildResult result = getGradleRunner("thirdPartyAudit")
+            .withArguments("clean", "absurd", "-s",
+                "-PcompileGroup=other.gradle:jarhellJdk", "-PcompileVersion=0.0.1",
+                "-PcompileOnlyGroup=other.gradle:dummy-io", "-PcompileOnlyVersion=0.0.1"
+            )
+            .buildAndFail();
+        assertTaskFailed(result, ":absurd");
+
+        assertOutputContains(result.getOutput(),
+            "> Audit of third party dependencies failed:",
+            "   Jar Hell with the JDK:",
+            "    * java.lang.String"
+        );
+        assertOutputDoesNotContain(result.getOutput(), "Classes with violations:");
+    }
+
+    public void testElasticsearchIgnoredWithViolations() {
+        BuildResult result = getGradleRunner("thirdPartyAudit")
+            .withArguments("clean", "absurd", "-s",
+                "-PcompileOnlyGroup=elasticsearch.gradle:broken-log4j", "-PcompileOnlyVersion=0.0.1",
+                "-PcompileGroup=elasticsearch.gradle:dummy-io", "-PcompileVersion=0.0.1"
+            )
+            .build();
+        assertTaskNoSource(result, ":absurd");
+    }
+
+}

buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java

@@ -90,6 +90,12 @@ public abstract class GradleIntegrationTestCase extends GradleUnitTestCase {
         }
     }
 
+    protected void assertTaskNoSource(BuildResult result, String... taskNames) {
+        for (String taskName : taskNames) {
+            assertTaskOutcome(result, taskName, TaskOutcome.NO_SOURCE);
+        }
+    }
+
     private void assertTaskOutcome(BuildResult result, String taskName, TaskOutcome taskOutcome) {
         BuildTask task = result.task(taskName);
         if (task == null) {
@@ -97,8 +103,8 @@ public abstract class GradleIntegrationTestCase extends GradleUnitTestCase {
                 "\n\nOutput is:\n" + result.getOutput());
         }
         assertEquals(
-            "Expected task `" + taskName +"` to be successful but it was: " + task.getOutcome() +
-                taskOutcome + "\n\nOutput is:\n" + result.getOutput() ,
+            "Expected task `" + taskName +"` to be " + taskOutcome + " but it was: " + task.getOutcome() +
+                "\n\nOutput is:\n" + result.getOutput() ,
             taskOutcome,
             task.getOutcome()
         );

buildSrc/src/testKit/thirdPartyAudit/build.gradle

@@ -0,0 +1,38 @@
+import org.elasticsearch.gradle.precommit.ThirdPartyAuditTask
+
+plugins {
+    id 'java'
+    //just to get build-tools
+    id 'elasticsearch.testclusters'
+}
+
+repositories {
+    /**
+     * Local test repo contains dummy jars with different group names and versions.
+     *   - broken-log4j creates a log4j logger but has no pom, so the class will be missing
+     *   - dummy-io has a class that creates a new java.io.File ( something which third-party-audit-absurd.txt forbids )
+     *   - version 0.0.2 has the same class and one extra file just to make the jar different
+     */
+    maven {
+        url = file("sample_jars/build/testrepo")
+    }
+    jcenter()
+}
+
+configurations.create("forbiddenApisCliJar")
+
+dependencies {
+    forbiddenApisCliJar 'de.thetaphi:forbiddenapis:2.6'
+    compileOnly "org.${project.properties.compileOnlyGroup}:${project.properties.compileOnlyVersion}"
+    compile "org.${project.properties.compileGroup}:${project.properties.compileVersion}"
+}
+
+task empty(type: ThirdPartyAuditTask) {
+    targetCompatibility = JavaVersion.VERSION_11
+    signatureFile = file('third-party-audit-empty.txt')
+}
+
+task absurd(type: ThirdPartyAuditTask) {
+    targetCompatibility = JavaVersion.VERSION_11
+    signatureFile = file('third-party-audit-absurd.txt')
+}

buildSrc/src/testKit/thirdPartyAudit/sample_jars/build.gradle

@@ -0,0 +1,52 @@
+plugins {
+    id 'java'
+}
+repositories {
+    mavenCentral()
+}
+dependencies {
+    compile 'org.apache.logging.log4j:log4j-core:2.11.1'
+}
+
+// Tests have to clean mid-test but we don't want the sample jars to go away
+clean.enabled = false
+
+["0.0.1", "0.0.2"].forEach { v ->
+    ["elasticsearch", "other"].forEach { p ->
+        task "dummy-${p}-${v}"(type: Jar) {
+            destinationDir = file("${buildDir}/testrepo/org/${p}/gradle/dummy-io/${v}/")
+            archiveName = "dummy-io-${v}.jar"
+            from sourceSets.main.output
+            include "**/TestingIO.class"
+            if (v == "0.0.2") {
+                manifest {
+                    attributes(
+                            "X-Different": "Different manifest, different jar"
+                    )
+                }
+            }
+        }
+        build.dependsOn("dummy-${p}-${v}")
+    }
+}
+
+["0.0.1"].forEach { v ->
+    ["elasticsearch", "other"].forEach { p ->
+        task "broken-log4j-${p}-${v}"(type: Jar) {
+            destinationDir = file("${buildDir}/testrepo/org/${p}/gradle/broken-log4j/${v}/")
+            archiveName = "broken-log4j-${v}.jar"
+            from sourceSets.main.output
+            include "**/TestingLog4j.class"
+        }
+        build.dependsOn("broken-log4j-${p}-${v}")
+    }
+}
+
+task jarhellJdk(type: Jar) {
+    destinationDir = file("${buildDir}/testrepo/org/other/gradle/jarhellJdk/0.0.1/")
+    archiveName = "jarhellJdk-0.0.1.jar"
+    from sourceSets.main.output
+    include "**/String.class"
+    into "java/lang"
+    build.dependsOn("jarhellJdk")
+}

buildSrc/src/testKit/thirdPartyAudit/sample_jars/src/main/java/String.java

@@ -0,0 +1,3 @@
+class String {
+
+}

buildSrc/src/testKit/thirdPartyAudit/sample_jars/src/main/java/TestingIO.java

@@ -0,0 +1,8 @@
+import java.io.File;
+
+public class TestingIO {
+    public TestingIO() {
+        new File("foo");
+    }
+}
+

buildSrc/src/testKit/thirdPartyAudit/sample_jars/src/main/java/TestingLog4j.java

@@ -0,0 +1,7 @@
+import org.apache.logging.log4j.LogManager;
+
+public class TestingLog4j {
+    public TestingLog4j() {
+        LogManager.getLogger();
+    }
+}

buildSrc/src/testKit/thirdPartyAudit/settings.gradle

@@ -0,0 +1 @@
+include 'sample_jars'

buildSrc/src/testKit/thirdPartyAudit/third-party-audit-absurd.txt

@@ -0,0 +1,2 @@
+@defaultMessage non-public internal runtime class
+java.io.**

buildSrc/src/testKit/thirdPartyAudit/third-party-audit-empty.txt

@@ -0,0 +1 @@
+@defaultMessage non-public internal runtime class