From 32ede85872b35837c4b7c318b493ca1e5681b3f4 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 2 Jun 2018 22:13:58 -0400 Subject: [PATCH 01/11] Add check for feature aware implementations This commit adds a check that any class in X-Pack that is a feature aware custom also implements the appropriate mix-in interface in X-Pack. These interfaces provide a default implementation of FeatureAware#getRequiredFeature that returns that x-pack is the required feature. By implementing this interface, this gives a consistent way for X-Pack feature aware customs to return the appopriate required feature and this check enforces that all such feature aware customs return the appropriate required feature. --- build.gradle | 11 +- .../gradle/precommit/FeatureAwareTask.groovy | 89 +++++ .../gradle/precommit/PrecommitTasks.groovy | 20 ++ x-pack/test/feature-aware/build.gradle | 20 ++ .../test/feature_aware/FeatureAwareCheck.java | 174 ++++++++++ .../feature_aware/FeatureAwareCheckTests.java | 323 ++++++++++++++++++ 6 files changed, 636 insertions(+), 1 deletion(-) create mode 100644 buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FeatureAwareTask.groovy create mode 100644 x-pack/test/feature-aware/build.gradle create mode 100644 x-pack/test/feature-aware/src/main/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheck.java create mode 100644 x-pack/test/feature-aware/src/test/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheckTests.java diff --git a/build.gradle b/build.gradle index 05ad5479e8dea..620e043d1c081 100644 --- a/build.gradle +++ b/build.gradle @@ -226,6 +226,7 @@ subprojects { "org.elasticsearch.distribution.deb:elasticsearch:${version}": ':distribution:packages:deb', "org.elasticsearch.distribution.deb:elasticsearch-oss:${version}": ':distribution:packages:oss-deb', "org.elasticsearch.test:logger-usage:${version}": ':test:logger-usage', + "org.elasticsearch.xpack.test:feature-aware:${version}": ':x-pack:test:feature-aware', // for transport client "org.elasticsearch.plugin:transport-netty4-client:${version}": ':modules:transport-netty4', "org.elasticsearch.plugin:reindex-client:${version}": ':modules:reindex', @@ -311,7 +312,15 @@ gradle.projectsEvaluated { // :test:framework:test cannot run before and after :server:test return } - configurations.all { + configurations.all { Configuration configuration -> + /* + * The featureAwarePlugin configuration has a dependency on x-pack:plugin:core and x-pack:plugin:core has a dependency on the + * featureAwarePlugin configuration. The below task ordering logic would force :x-pack:plugin:core:test + * :x-pack:test:feature-aware:test to depend on each other circularly. We break that cycle here. + */ + if (configuration.name == "featureAwarePlugin") { + return + } dependencies.all { Dependency dep -> Project upstreamProject = dependencyToProject(dep) if (upstreamProject != null) { diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FeatureAwareTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FeatureAwareTask.groovy new file mode 100644 index 0000000000000..c34af25f0f794 --- /dev/null +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FeatureAwareTask.groovy @@ -0,0 +1,89 @@ +/* + * 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. + */ + +package org.elasticsearch.gradle.precommit + +import org.elasticsearch.gradle.LoggedExec +import org.gradle.api.file.FileCollection +import org.gradle.api.tasks.InputFiles +import org.gradle.api.tasks.OutputFile + +class FeatureAwareTask extends LoggedExec { + + // we touch this when the task succeeds + private File successMarker = new File(project.buildDir, 'markers/featureAware') + + private FileCollection classpath + + private FileCollection classDirectories + + FeatureAwareTask() { + project.afterEvaluate { + dependsOn classpath + executable = new File(project.runtimeJavaHome, 'bin/java') + if (classDirectories == null) { + // default to main class files if such a source set exists + final List files = [] + if (project.sourceSets.findByName("main")) { + files.add(project.sourceSets.main.output.classesDir) + dependsOn project.tasks.classes + } + // filter out non-existent classes directories from empty source sets + classDirectories = project.files(files).filter { it.exists() } + } + description = "Runs FeatureAwareCheck on ${classDirectories.files}." + doFirst { + args('-cp', getClasspath().asPath, 'org.elasticsearch.xpack.test.feature_aware.FeatureAwareCheck') + getClassDirectories().each { args it.getAbsolutePath() } + } + doLast { + successMarker.parentFile.mkdirs() + successMarker.setText("", 'UTF-8') + } + } + } + + @InputFiles + FileCollection getClasspath() { + return classpath + } + + void setClasspath(final FileCollection classpath) { + this.classpath = classpath + } + + @InputFiles + FileCollection getClassDirectories() { + return classDirectories + } + + void setClassDirectories(final FileCollection classDirectories) { + this.classDirectories = classDirectories + } + + @OutputFile + File getSuccessMarker() { + return successMarker + } + + void setSuccessMarker(final File successMarker) { + this.successMarker = successMarker + } + +} diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy index 09f0ad01578c9..1fd349dc1c457 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy @@ -20,6 +20,7 @@ package org.elasticsearch.gradle.precommit import de.thetaphi.forbiddenapis.gradle.CheckForbiddenApis import de.thetaphi.forbiddenapis.gradle.ForbiddenApisPlugin +import org.elasticsearch.gradle.LoggedExec import org.gradle.api.Project import org.gradle.api.Task import org.gradle.api.plugins.JavaBasePlugin @@ -65,6 +66,9 @@ class PrecommitTasks { precommitTasks.add(configureLoggerUsage(project)) } + if (project.path.startsWith(":x-pack:plugin")) { + precommitTasks.add(configureFeatureAware(project)) + } Map precommitOptions = [ name: 'precommit', @@ -176,4 +180,20 @@ class PrecommitTasks { return loggerUsageTask } + + private static Task configureFeatureAware(final Project project) { + final Task featureAwareTask = project.tasks.create('featureAwareCheck', FeatureAwareTask.class) + + project.configurations.create('featureAwarePlugin') + project.dependencies.add( + 'featureAwarePlugin', + "org.elasticsearch.xpack.test:feature-aware:${org.elasticsearch.gradle.VersionProperties.elasticsearch}") + + featureAwareTask.configure { FeatureAwareTask fat -> + fat.classpath = project.configurations.featureAwarePlugin + } + + return featureAwareTask + } + } diff --git a/x-pack/test/feature-aware/build.gradle b/x-pack/test/feature-aware/build.gradle new file mode 100644 index 0000000000000..d4d2a33edb967 --- /dev/null +++ b/x-pack/test/feature-aware/build.gradle @@ -0,0 +1,20 @@ +import org.elasticsearch.gradle.precommit.PrecommitTasks + +apply plugin: 'elasticsearch.build' + +dependencies { + compile 'org.ow2.asm:asm:6.2' + compile "org.elasticsearch:elasticsearch:${version}" + compile "org.elasticsearch.plugin:x-pack-core:${version}" + testCompile "org.elasticsearch.test:framework:${version}" +} + +forbiddenApisMain.enabled = true +forbiddenApisMain { + signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt')] // does not depend on core, only jdk signatures +} + +dependencyLicenses.enabled = false + +jarHell.enabled = false +thirdPartyAudit.enabled = false diff --git a/x-pack/test/feature-aware/src/main/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheck.java b/x-pack/test/feature-aware/src/main/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheck.java new file mode 100644 index 0000000000000..03dc47fffa229 --- /dev/null +++ b/x-pack/test/feature-aware/src/main/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheck.java @@ -0,0 +1,174 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.test.feature_aware; + +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.common.SuppressForbidden; +import org.elasticsearch.persistent.PersistentTaskParams; +import org.elasticsearch.xpack.core.XPackPlugin; +import org.objectweb.asm.ClassReader; + +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.FileVisitResult; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.function.Consumer; + +/** + * Used in the featureAwareCheck to check for classes in X-Pack that implement customs but do not extend the appropriate marker interface. + */ +public final class FeatureAwareCheck { + + /** + * Check the class directories specified by the arguments for classes in X-Pack that implement customs but do not extend the appropriate + * marker interface that provides a mix-in implementation of {@link ClusterState.FeatureAware#getRequiredFeature()}. + * + * @param args the class directories to check + * @throws IOException if an I/O exception is walking the class directories + */ + public static void main(final String[] args) throws IOException { + systemOutPrintln("checking for custom violations"); + final List violations = new ArrayList<>(); + checkDirectories(violations::add, args); + if (violations.isEmpty()) { + systemOutPrintln("no custom violations found"); + } else { + violations.forEach(violation -> + systemOutPrintln( + "class [" + violation.name + "] implements" + + " [" + violation.interfaceName + " but does not implement" + + " [" + violation.expectedInterfaceName + "]") + ); + throw new IllegalStateException( + "found custom" + (violations.size() == 1 ? "" : "s") + " in X-Pack not extending appropriate X-Pack mix-in"); + } + } + + @SuppressForbidden(reason = "System.out#println") + private static void systemOutPrintln(final String s) { + System.out.println(s); + } + + private static void checkDirectories( + final Consumer callback, + final String... classDirectories) throws IOException { + for (final String classDirectory : classDirectories) { + final Path root = Paths.get(classDirectory); + if (Files.isDirectory(root)) { + Files.walkFileTree(root, new SimpleFileVisitor() { + @Override + public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) throws IOException { + if (Files.isRegularFile(file) && file.getFileName().toString().endsWith(".class")) { + try (InputStream in = Files.newInputStream(file)) { + checkClass(in, callback); + } + } + return super.visitFile(file, attrs); + } + }); + } else { + throw new FileNotFoundException("class directory [" + classDirectory + "] should exist"); + } + } + } + + /** + * Represents a feature-aware violation + */ + static class FeatureAwareViolation { + + final String name; + final String interfaceName; + final String expectedInterfaceName; + + /** + * Constructs a representation of a feature-aware violation. + * + * @param name the name of the custom class + * @param interfaceName the name of the feature-aware interface + * @param expectedInterfaceName the name of the expected mix-in class + */ + FeatureAwareViolation(final String name, final String interfaceName, final String expectedInterfaceName) { + this.name = name; + this.interfaceName = interfaceName; + this.expectedInterfaceName = expectedInterfaceName; + } + + } + + /** + * Loads a class from the specified input stream and checks that if it implements a feature-aware custom then it extends the appropriate + * mix-in interface from X-Pack. If the class does not, then the specified callback is invoked. + * + * @param in the input stream + * @param callback the callback to invoke + * @throws IOException if an I/O exception occurs loading the class hierarchy + */ + static void checkClass(final InputStream in, final Consumer callback) throws IOException { + // the class format only reports declared interfaces so we have to walk the hierarchy looking for all interfaces + final List interfaces = new ArrayList<>(); + ClassReader cr = new ClassReader(in); + final String name = cr.getClassName(); + do { + interfaces.addAll(Arrays.asList(cr.getInterfaces())); + final String superName = cr.getSuperName(); + if ("java/lang/Object".equals(superName)) { + break; + } + cr = new ClassReader(superName); + } while (true); + checkClass(name, interfaces, callback); + } + + private static void checkClass( + final String name, + final List interfaces, + final Consumer callback) { + checkCustomForClass(ClusterState.Custom.class, XPackPlugin.XPackClusterStateCustom.class, name, interfaces, callback); + checkCustomForClass(MetaData.Custom.class, XPackPlugin.XPackMetaDataCustom.class, name, interfaces, callback); + checkCustomForClass(PersistentTaskParams.class, XPackPlugin.XPackPersistentTaskParams.class, name, interfaces, callback); + } + + private static void checkCustomForClass( + final Class interfaceToCheck, + final Class expectedInterface, + final String name, + final List interfaces, + final Consumer callback) { + final Set interfaceSet = new TreeSet<>(interfaces); + final String interfaceToCheckName = formatClassName(interfaceToCheck); + final String expectedXPackInterfaceName = formatClassName(expectedInterface); + if (interfaceSet.contains(interfaceToCheckName) + && name.equals(expectedXPackInterfaceName) == false + && interfaceSet.contains(expectedXPackInterfaceName) == false) { + assert name.startsWith("org/elasticsearch/license") || name.startsWith("org/elasticsearch/xpack"); + callback.accept(new FeatureAwareViolation(name, interfaceToCheckName, expectedXPackInterfaceName)); + } + } + + /** + * Format the specified class to a name in the ASM format replacing all dots in the class name with forward-slashes. + * + * @param clazz the class whose name to format + * @return the formatted class name + */ + static String formatClassName(final Class clazz) { + return clazz.getName().replace(".", "/"); + } + +} diff --git a/x-pack/test/feature-aware/src/test/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheckTests.java b/x-pack/test/feature-aware/src/test/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheckTests.java new file mode 100644 index 0000000000000..2dde9efce42bb --- /dev/null +++ b/x-pack/test/feature-aware/src/test/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheckTests.java @@ -0,0 +1,323 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.test.feature_aware; + +import org.elasticsearch.Version; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.Diff; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.persistent.PersistentTaskParams; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.core.XPackPlugin; + +import java.io.IOException; +import java.util.EnumSet; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; + +import static org.hamcrest.Matchers.equalTo; + +public class FeatureAwareCheckTests extends ESTestCase { + + public void testClusterStateCustomViolation() throws IOException { + runCustomViolationTest( + ClusterStateCustomViolation.class, + getClass(), + ClusterState.Custom.class, + XPackPlugin.XPackClusterStateCustom.class); + } + + public void testClusterStateCustom() throws IOException { + runCustomTest(XPackClusterStateCustom.class, getClass(), ClusterState.Custom.class, XPackPlugin.XPackClusterStateCustom.class); + } + + public void testClusterStateCustomMarkerInterface() throws IOException { + // marker interfaces do not implement the marker interface but should not fail the feature aware check + runCustomTest( + XPackPlugin.XPackClusterStateCustom.class, + XPackPlugin.class, + ClusterState.Custom.class, + XPackPlugin.XPackClusterStateCustom.class); + } + + public void testMetaDataCustomViolation() throws IOException { + runCustomViolationTest(MetaDataCustomViolation.class, getClass(), MetaData.Custom.class, XPackPlugin.XPackMetaDataCustom.class); + } + + public void testMetaDataCustom() throws IOException { + runCustomTest(XPackMetaDataCustom.class, getClass(), MetaData.Custom.class, XPackPlugin.XPackMetaDataCustom.class); + } + + public void testMetaDataCustomMarkerInterface() throws IOException { + // marker interfaces do not implement the marker interface but should not fail the feature aware check + runCustomTest( + XPackPlugin.XPackMetaDataCustom.class, + XPackPlugin.class, + MetaData.Custom.class, + XPackPlugin.XPackMetaDataCustom.class); + } + + public void testPersistentTaskParamsViolation() throws IOException { + runCustomViolationTest( + PersistentTaskParamsViolation.class, + getClass(), + PersistentTaskParams.class, + XPackPlugin.XPackPersistentTaskParams.class); + } + + public void testPersistentTaskParams() throws IOException { + runCustomTest(XPackPersistentTaskParams.class, getClass(), PersistentTaskParams.class, XPackPlugin.XPackPersistentTaskParams.class); + } + + public void testPersistentTaskParamsMarkerInterface() throws IOException { + // marker interfaces do not implement the marker interface but should not fail the feature aware check + runCustomTest( + XPackPlugin.XPackPersistentTaskParams.class, + XPackPlugin.class, + PersistentTaskParams.class, + XPackPlugin.XPackPersistentTaskParams.class); + } + + abstract class ClusterStateCustomFeatureAware implements ClusterState.Custom { + + private final String writeableName; + + ClusterStateCustomFeatureAware(final String writeableName) { + this.writeableName = writeableName; + } + + @Override + public Diff diff(ClusterState.Custom previousState) { + return null; + } + + @Override + public String getWriteableName() { + return writeableName; + } + + @Override + public Version getMinimalSupportedVersion() { + return Version.CURRENT.minimumCompatibilityVersion(); + } + + @Override + public void writeTo(final StreamOutput out) throws IOException { + + } + + @Override + public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { + return builder; + } + + } + + class ClusterStateCustomViolation extends ClusterStateCustomFeatureAware { + + ClusterStateCustomViolation() { + super("cluster_state_custom_violation"); + } + } + + class XPackClusterStateCustom extends ClusterStateCustomFeatureAware implements XPackPlugin.XPackClusterStateCustom { + + XPackClusterStateCustom() { + super("x_pack_cluster_state_custom"); + } + + } + + abstract class MetaDataCustomFeatureAware implements MetaData.Custom { + + private final String writeableName; + + MetaDataCustomFeatureAware(final String writeableName) { + this.writeableName = writeableName; + } + + @Override + public EnumSet context() { + return MetaData.ALL_CONTEXTS; + } + + @Override + public Diff diff(MetaData.Custom previousState) { + return null; + } + + @Override + public String getWriteableName() { + return writeableName; + } + + @Override + public Version getMinimalSupportedVersion() { + return Version.CURRENT.minimumCompatibilityVersion(); + } + + @Override + public void writeTo(final StreamOutput out) throws IOException { + + } + + @Override + public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { + return builder; + } + + } + + class MetaDataCustomViolation extends MetaDataCustomFeatureAware { + + MetaDataCustomViolation() { + super("meta_data_custom_violation"); + } + + } + + class XPackMetaDataCustom extends MetaDataCustomFeatureAware implements XPackPlugin.XPackMetaDataCustom { + + XPackMetaDataCustom() { + super("x_pack_meta_data_custom"); + } + + } + + abstract class PersistentTaskParamsFeatureAware implements PersistentTaskParams { + + private final String writeableName; + + PersistentTaskParamsFeatureAware(final String writeableName) { + this.writeableName = writeableName; + } + + @Override + public String getWriteableName() { + return writeableName; + } + + @Override + public Version getMinimalSupportedVersion() { + return Version.CURRENT.minimumCompatibilityVersion(); + } + + @Override + public void writeTo(final StreamOutput out) throws IOException { + + } + + @Override + public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { + return builder; + } + + } + + class PersistentTaskParamsViolation extends PersistentTaskParamsFeatureAware { + + PersistentTaskParamsViolation() { + super("persistent_task_params_violation"); + } + + } + + class XPackPersistentTaskParams extends PersistentTaskParamsFeatureAware implements XPackPlugin.XPackPersistentTaskParams { + + XPackPersistentTaskParams() { + super("x_pack_persistent_task_params"); + } + + } + + private class FeatureAwareViolationConsumer implements Consumer { + + private final AtomicBoolean called = new AtomicBoolean(); + private final String name; + private final String interfaceName; + private final String expectedInterfaceName; + + FeatureAwareViolationConsumer(final String name, final String interfaceName, final String expectedInterfaceName) { + this.name = name; + this.interfaceName = interfaceName; + this.expectedInterfaceName = expectedInterfaceName; + } + + @Override + public void accept(final org.elasticsearch.xpack.test.feature_aware.FeatureAwareCheck.FeatureAwareViolation featureAwareViolation) { + called.set(true); + assertThat(featureAwareViolation.name, equalTo(name)); + assertThat(featureAwareViolation.interfaceName, equalTo(interfaceName)); + assertThat(featureAwareViolation.expectedInterfaceName, equalTo(expectedInterfaceName)); + } + + } + + /** + * Runs a test on an actual class implementing a custom interface and not the expected marker interface. + * + * @param clazz the custom implementation + * @param outerClazz the outer class to load the custom implementation relative to + * @param interfaceClazz the custom + * @param expectedInterfaceClazz the marker interface + * @throws IOException if an I/O error occurs reading the class + */ + private void runCustomViolationTest( + final Class clazz, + final Class outerClazz, + final Class interfaceClazz, + final Class expectedInterfaceClazz) throws IOException { + runTest(clazz, outerClazz, interfaceClazz, expectedInterfaceClazz, true); + } + + /** + * Runs a test on an actual class implementing a custom interface and the expected marker interface. + * + * @param clazz the custom implementation + * @param outerClazz the outer class to load the custom implementation relative to + * @param interfaceClazz the custom + * @param expectedInterfaceClazz the marker interface + * @throws IOException if an I/O error occurs reading the class + */ + private void runCustomTest( + final Class clazz, + final Class outerClazz, + final Class interfaceClazz, + final Class expectedInterfaceClazz) throws IOException { + runTest(clazz, outerClazz, interfaceClazz, expectedInterfaceClazz, false); + } + + /** + * Runs a test on an actual class implementing a custom interface and should implement the expected marker interface if and only if + * the specified violation parameter is false. + * + * @param clazz the custom implementation + * @param outerClazz the outer class to load the custom implementation relative to + * @param interfaceClazz the custom + * @param expectedInterfaceClazz the marker interface + * @param violation whether or not the actual class is expected to fail the feature aware check + * @throws IOException if an I/O error occurs reading the class + */ + private void runTest( + final Class clazz, + final Class outerClazz, + final Class interfaceClazz, + final Class expectedInterfaceClazz, + final boolean violation) throws IOException { + final String name = clazz.getName(); + final FeatureAwareViolationConsumer callback = + new FeatureAwareViolationConsumer( + FeatureAwareCheck.formatClassName(clazz), + FeatureAwareCheck.formatClassName(interfaceClazz), + FeatureAwareCheck.formatClassName(expectedInterfaceClazz)); + FeatureAwareCheck.checkClass(outerClazz.getResourceAsStream(name.substring(1 + name.lastIndexOf(".")) + ".class"), callback); + assertThat(callback.called.get(), equalTo(violation)); + } + +} From b24bee3b24986959f4acda6c845a94bde82a4803 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 4 Jun 2018 12:21:18 -0400 Subject: [PATCH 02/11] Add comment --- .../org/elasticsearch/gradle/precommit/PrecommitTasks.groovy | 1 + 1 file changed, 1 insertion(+) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy index 1fd349dc1c457..21acf4b21fe28 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy @@ -184,6 +184,7 @@ class PrecommitTasks { private static Task configureFeatureAware(final Project project) { final Task featureAwareTask = project.tasks.create('featureAwareCheck', FeatureAwareTask.class) + // see the root Gradle file for additional logic regarding this configuration project.configurations.create('featureAwarePlugin') project.dependencies.add( 'featureAwarePlugin', From 0c3c29e2e76bf591d826fc3fdd56a8b007398859 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 4 Jun 2018 13:46:12 -0400 Subject: [PATCH 03/11] Fix forbidden --- x-pack/test/feature-aware/build.gradle | 6 +----- .../xpack/test/feature_aware/FeatureAwareCheck.java | 8 +++++++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/x-pack/test/feature-aware/build.gradle b/x-pack/test/feature-aware/build.gradle index d4d2a33edb967..217ed25a2d4b1 100644 --- a/x-pack/test/feature-aware/build.gradle +++ b/x-pack/test/feature-aware/build.gradle @@ -1,5 +1,3 @@ -import org.elasticsearch.gradle.precommit.PrecommitTasks - apply plugin: 'elasticsearch.build' dependencies { @@ -10,11 +8,9 @@ dependencies { } forbiddenApisMain.enabled = true -forbiddenApisMain { - signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt')] // does not depend on core, only jdk signatures -} dependencyLicenses.enabled = false jarHell.enabled = false + thirdPartyAudit.enabled = false diff --git a/x-pack/test/feature-aware/src/main/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheck.java b/x-pack/test/feature-aware/src/main/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheck.java index 03dc47fffa229..5f1775fd2b279 100644 --- a/x-pack/test/feature-aware/src/main/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheck.java +++ b/x-pack/test/feature-aware/src/main/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheck.java @@ -9,6 +9,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.common.SuppressForbidden; +import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.persistent.PersistentTaskParams; import org.elasticsearch.xpack.core.XPackPlugin; import org.objectweb.asm.ClassReader; @@ -68,7 +69,7 @@ private static void checkDirectories( final Consumer callback, final String... classDirectories) throws IOException { for (final String classDirectory : classDirectories) { - final Path root = Paths.get(classDirectory); + final Path root = pathsGet(classDirectory); if (Files.isDirectory(root)) { Files.walkFileTree(root, new SimpleFileVisitor() { @Override @@ -87,6 +88,11 @@ public FileVisitResult visitFile(final Path file, final BasicFileAttributes attr } } + @SuppressForbidden(reason = "Paths#get") + private static Path pathsGet(final String pathString) { + return Paths.get(pathString); + } + /** * Represents a feature-aware violation */ From dc0adb73de53c6e16d3dd4a838c6254c014e4a68 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 4 Jun 2018 13:47:30 -0400 Subject: [PATCH 04/11] Move description --- .../org/elasticsearch/gradle/precommit/FeatureAwareTask.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FeatureAwareTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FeatureAwareTask.groovy index c34af25f0f794..2f03271812b25 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FeatureAwareTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FeatureAwareTask.groovy @@ -34,6 +34,7 @@ class FeatureAwareTask extends LoggedExec { private FileCollection classDirectories FeatureAwareTask() { + description = "Runs FeatureAwareCheck on main classes." project.afterEvaluate { dependsOn classpath executable = new File(project.runtimeJavaHome, 'bin/java') @@ -47,7 +48,6 @@ class FeatureAwareTask extends LoggedExec { // filter out non-existent classes directories from empty source sets classDirectories = project.files(files).filter { it.exists() } } - description = "Runs FeatureAwareCheck on ${classDirectories.files}." doFirst { args('-cp', getClasspath().asPath, 'org.elasticsearch.xpack.test.feature_aware.FeatureAwareCheck') getClassDirectories().each { args it.getAbsolutePath() } From e12e812868967d0fb4508947653b78cf530950b2 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 4 Jun 2018 15:43:51 -0400 Subject: [PATCH 05/11] We need the classes dir on the classpath too --- .../elasticsearch/gradle/precommit/PrecommitTasks.groovy | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy index 21acf4b21fe28..4ef3b480afac3 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy @@ -23,6 +23,8 @@ import de.thetaphi.forbiddenapis.gradle.ForbiddenApisPlugin import org.elasticsearch.gradle.LoggedExec import org.gradle.api.Project import org.gradle.api.Task +import org.gradle.api.internal.tasks.DefaultSourceSet +import org.gradle.api.internal.tasks.DefaultSourceSetContainer import org.gradle.api.plugins.JavaBasePlugin import org.gradle.api.plugins.quality.Checkstyle @@ -66,7 +68,7 @@ class PrecommitTasks { precommitTasks.add(configureLoggerUsage(project)) } - if (project.path.startsWith(":x-pack:plugin")) { + if (project.path.startsWith(":x-pack:plugin:")) { precommitTasks.add(configureFeatureAware(project)) } @@ -189,6 +191,9 @@ class PrecommitTasks { project.dependencies.add( 'featureAwarePlugin', "org.elasticsearch.xpack.test:feature-aware:${org.elasticsearch.gradle.VersionProperties.elasticsearch}") + project.dependencies.add( + 'featureAwarePlugin', + project.sourceSets.main.output.getClassesDirs()) featureAwareTask.configure { FeatureAwareTask fat -> fat.classpath = project.configurations.featureAwarePlugin From a0ecfd16020b9b5347b779416bc43887551d640a Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 4 Jun 2018 15:50:16 -0400 Subject: [PATCH 06/11] Remove imports --- .../org/elasticsearch/gradle/precommit/PrecommitTasks.groovy | 3 --- 1 file changed, 3 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy index 4ef3b480afac3..c0573b4a74358 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy @@ -20,11 +20,8 @@ package org.elasticsearch.gradle.precommit import de.thetaphi.forbiddenapis.gradle.CheckForbiddenApis import de.thetaphi.forbiddenapis.gradle.ForbiddenApisPlugin -import org.elasticsearch.gradle.LoggedExec import org.gradle.api.Project import org.gradle.api.Task -import org.gradle.api.internal.tasks.DefaultSourceSet -import org.gradle.api.internal.tasks.DefaultSourceSetContainer import org.gradle.api.plugins.JavaBasePlugin import org.gradle.api.plugins.quality.Checkstyle From 12acfb7fcbdc66b23e3a818d46d19c78cac9ff89 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 4 Jun 2018 17:34:53 -0400 Subject: [PATCH 07/11] Commit to trigger build --- .../xpack/test/feature_aware/FeatureAwareCheck.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/feature-aware/src/main/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheck.java b/x-pack/test/feature-aware/src/main/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheck.java index 5f1775fd2b279..7746692b408e9 100644 --- a/x-pack/test/feature-aware/src/main/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheck.java +++ b/x-pack/test/feature-aware/src/main/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheck.java @@ -94,7 +94,7 @@ private static Path pathsGet(final String pathString) { } /** - * Represents a feature-aware violation + * Represents a feature-aware violation. */ static class FeatureAwareViolation { From a1c4bab9c90fb4918a7c1ae0aeb1ebbe6658c3ae Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 5 Jun 2018 08:59:31 -0400 Subject: [PATCH 08/11] Move feature aware configuration --- .../gradle/precommit/PrecommitTasks.groovy | 23 ---------------- x-pack/plugin/build.gradle | 26 +++++++++++++++---- 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy index c0573b4a74358..7e0d466c859e7 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy @@ -65,10 +65,6 @@ class PrecommitTasks { precommitTasks.add(configureLoggerUsage(project)) } - if (project.path.startsWith(":x-pack:plugin:")) { - precommitTasks.add(configureFeatureAware(project)) - } - Map precommitOptions = [ name: 'precommit', group: JavaBasePlugin.VERIFICATION_GROUP, @@ -180,23 +176,4 @@ class PrecommitTasks { return loggerUsageTask } - private static Task configureFeatureAware(final Project project) { - final Task featureAwareTask = project.tasks.create('featureAwareCheck', FeatureAwareTask.class) - - // see the root Gradle file for additional logic regarding this configuration - project.configurations.create('featureAwarePlugin') - project.dependencies.add( - 'featureAwarePlugin', - "org.elasticsearch.xpack.test:feature-aware:${org.elasticsearch.gradle.VersionProperties.elasticsearch}") - project.dependencies.add( - 'featureAwarePlugin', - project.sourceSets.main.output.getClassesDirs()) - - featureAwareTask.configure { FeatureAwareTask fat -> - fat.classpath = project.configurations.featureAwarePlugin - } - - return featureAwareTask - } - } diff --git a/x-pack/plugin/build.gradle b/x-pack/plugin/build.gradle index 4a0b29c42582a..0eb7e3b1c0edc 100644 --- a/x-pack/plugin/build.gradle +++ b/x-pack/plugin/build.gradle @@ -1,12 +1,8 @@ import org.elasticsearch.gradle.LoggedExec -import org.elasticsearch.gradle.MavenFilteringHack +import org.elasticsearch.gradle.precommit.FeatureAwareTask import org.elasticsearch.gradle.test.NodeInfo import java.nio.charset.StandardCharsets -import java.nio.file.Files -import java.nio.file.Path -import java.nio.file.StandardCopyOption -import org.elasticsearch.gradle.test.RunTask; apply plugin: 'elasticsearch.standalone-rest-test' apply plugin: 'elasticsearch.rest-test' @@ -17,6 +13,26 @@ dependencies { testCompile project(path: xpackModule('core'), configuration: 'testArtifacts') } +subprojects { + final SourceSet main = project.sourceSets.findByName("main") + if (main != null) { + final FeatureAwareTask featureAwareTask = project.tasks.create('featureAwareCheck', FeatureAwareTask.class) + + // see the root Gradle file for additional logic regarding this configuration + project.configurations.create('featureAwarePlugin') + project.dependencies.add( + 'featureAwarePlugin', + "org.elasticsearch.xpack.test:feature-aware:${org.elasticsearch.gradle.VersionProperties.elasticsearch}") + project.dependencies.add('featureAwarePlugin', main.output.getClassesDirs()) + + featureAwareTask.configure { FeatureAwareTask fat -> + fat.classpath = project.configurations.featureAwarePlugin + } + + project.precommit.dependsOn featureAwareTask + } +} + // https://github.com/elastic/x-plugins/issues/724 configurations { testArtifacts.extendsFrom testRuntime From e6d9997c0ab3bde3d4cbc41a7b167188ebcce96e Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 5 Jun 2018 09:39:06 -0400 Subject: [PATCH 09/11] Move all of task configuration to x-pack:plugin --- .../gradle/precommit/FeatureAwareTask.groovy | 89 ------------------- x-pack/plugin/build.gradle | 56 ++++++++---- 2 files changed, 40 insertions(+), 105 deletions(-) delete mode 100644 buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FeatureAwareTask.groovy diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FeatureAwareTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FeatureAwareTask.groovy deleted file mode 100644 index 2f03271812b25..0000000000000 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/FeatureAwareTask.groovy +++ /dev/null @@ -1,89 +0,0 @@ -/* - * 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. - */ - -package org.elasticsearch.gradle.precommit - -import org.elasticsearch.gradle.LoggedExec -import org.gradle.api.file.FileCollection -import org.gradle.api.tasks.InputFiles -import org.gradle.api.tasks.OutputFile - -class FeatureAwareTask extends LoggedExec { - - // we touch this when the task succeeds - private File successMarker = new File(project.buildDir, 'markers/featureAware') - - private FileCollection classpath - - private FileCollection classDirectories - - FeatureAwareTask() { - description = "Runs FeatureAwareCheck on main classes." - project.afterEvaluate { - dependsOn classpath - executable = new File(project.runtimeJavaHome, 'bin/java') - if (classDirectories == null) { - // default to main class files if such a source set exists - final List files = [] - if (project.sourceSets.findByName("main")) { - files.add(project.sourceSets.main.output.classesDir) - dependsOn project.tasks.classes - } - // filter out non-existent classes directories from empty source sets - classDirectories = project.files(files).filter { it.exists() } - } - doFirst { - args('-cp', getClasspath().asPath, 'org.elasticsearch.xpack.test.feature_aware.FeatureAwareCheck') - getClassDirectories().each { args it.getAbsolutePath() } - } - doLast { - successMarker.parentFile.mkdirs() - successMarker.setText("", 'UTF-8') - } - } - } - - @InputFiles - FileCollection getClasspath() { - return classpath - } - - void setClasspath(final FileCollection classpath) { - this.classpath = classpath - } - - @InputFiles - FileCollection getClassDirectories() { - return classDirectories - } - - void setClassDirectories(final FileCollection classDirectories) { - this.classDirectories = classDirectories - } - - @OutputFile - File getSuccessMarker() { - return successMarker - } - - void setSuccessMarker(final File successMarker) { - this.successMarker = successMarker - } - -} diff --git a/x-pack/plugin/build.gradle b/x-pack/plugin/build.gradle index 0eb7e3b1c0edc..007bca7ff40d4 100644 --- a/x-pack/plugin/build.gradle +++ b/x-pack/plugin/build.gradle @@ -1,5 +1,5 @@ import org.elasticsearch.gradle.LoggedExec -import org.elasticsearch.gradle.precommit.FeatureAwareTask +import org.elasticsearch.gradle.plugin.PluginBuildPlugin import org.elasticsearch.gradle.test.NodeInfo import java.nio.charset.StandardCharsets @@ -14,22 +14,46 @@ dependencies { } subprojects { - final SourceSet main = project.sourceSets.findByName("main") - if (main != null) { - final FeatureAwareTask featureAwareTask = project.tasks.create('featureAwareCheck', FeatureAwareTask.class) - - // see the root Gradle file for additional logic regarding this configuration - project.configurations.create('featureAwarePlugin') - project.dependencies.add( - 'featureAwarePlugin', - "org.elasticsearch.xpack.test:feature-aware:${org.elasticsearch.gradle.VersionProperties.elasticsearch}") - project.dependencies.add('featureAwarePlugin', main.output.getClassesDirs()) - - featureAwareTask.configure { FeatureAwareTask fat -> - fat.classpath = project.configurations.featureAwarePlugin - } + afterEvaluate { + if (project.plugins.hasPlugin(PluginBuildPlugin)) { + // see the root Gradle file for additional logic regarding this configuration + project.configurations.create('featureAwarePlugin') + project.dependencies.add('featureAwarePlugin', project.configurations.runtime) + project.dependencies.add( + 'featureAwarePlugin', + "org.elasticsearch.xpack.test:feature-aware:${org.elasticsearch.gradle.VersionProperties.elasticsearch}") + project.dependencies.add('featureAwarePlugin', project.sourceSets.main.output.getClassesDirs()) + + final Task featureAwareTask = project.tasks.create("featureAwareCheck", LoggedExec) { + description = "Runs FeatureAwareCheck on main classes." + dependsOn project.configurations.featureAwarePlugin + + final File successMarker = new File(project.buildDir, 'markers/featureAware') + outputs.file(successMarker) + + executable = new File(project.runtimeJavaHome, 'bin/java') + + // default to main class files if such a source set exists + final List files = [] + if (project.sourceSets.findByName("main")) { + files.add(project.sourceSets.main.output.classesDir) + dependsOn project.tasks.classes + } + // filter out non-existent classes directories from empty source sets + final FileCollection classDirectories = project.files(files).filter { it.exists() } - project.precommit.dependsOn featureAwareTask + doFirst { + args('-cp', project.configurations.featureAwarePlugin.asPath, 'org.elasticsearch.xpack.test.feature_aware.FeatureAwareCheck') + classDirectories.each { args it.getAbsolutePath() } + } + doLast { + successMarker.parentFile.mkdirs() + successMarker.setText("", 'UTF-8') + } + } + + project.precommit.dependsOn featureAwareTask + } } } From 061c881debed1e0279b8922661ac21fbd68c61ce Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 5 Jun 2018 09:41:34 -0400 Subject: [PATCH 10/11] Revert touching PrecommitTasks --- .../org/elasticsearch/gradle/precommit/PrecommitTasks.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy index 7e0d466c859e7..09f0ad01578c9 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy @@ -65,6 +65,7 @@ class PrecommitTasks { precommitTasks.add(configureLoggerUsage(project)) } + Map precommitOptions = [ name: 'precommit', group: JavaBasePlugin.VERIFICATION_GROUP, @@ -175,5 +176,4 @@ class PrecommitTasks { return loggerUsageTask } - } From fa0651ba9e2d808c31d207acd6c624d3679817db Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Tue, 5 Jun 2018 15:50:15 -0400 Subject: [PATCH 11/11] Fix classpath --- x-pack/plugin/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/build.gradle b/x-pack/plugin/build.gradle index 007bca7ff40d4..ac423c4281138 100644 --- a/x-pack/plugin/build.gradle +++ b/x-pack/plugin/build.gradle @@ -18,7 +18,7 @@ subprojects { if (project.plugins.hasPlugin(PluginBuildPlugin)) { // see the root Gradle file for additional logic regarding this configuration project.configurations.create('featureAwarePlugin') - project.dependencies.add('featureAwarePlugin', project.configurations.runtime) + project.dependencies.add('featureAwarePlugin', project.configurations.compileClasspath) project.dependencies.add( 'featureAwarePlugin', "org.elasticsearch.xpack.test:feature-aware:${org.elasticsearch.gradle.VersionProperties.elasticsearch}")