Add check for feature aware implementations

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) {

x-pack/plugin/build.gradle

@@ -1,12 +1,8 @@
 import org.elasticsearch.gradle.LoggedExec
-import org.elasticsearch.gradle.MavenFilteringHack
+import org.elasticsearch.gradle.plugin.PluginBuildPlugin
 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,50 @@ dependencies {
   testCompile project(path: xpackModule('core'), configuration: 'testArtifacts')
 }
 
+subprojects {
+  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.compileClasspath)
+      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() }
+
+        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
+    }
+  }
+}
+
 // https://github.com/elastic/x-plugins/issues/724
 configurations {
   testArtifacts.extendsFrom testRuntime

x-pack/test/feature-aware/build.gradle

@@ -0,0 +1,16 @@
+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
+
+dependencyLicenses.enabled = false
+
+jarHell.enabled = false
+
+thirdPartyAudit.enabled = false

x-pack/test/feature-aware/src/main/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheck.java

@@ -0,0 +1,180 @@
+/*
+ * 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.common.io.PathUtils;
+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<FeatureAwareViolation> 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<FeatureAwareViolation> callback,
+            final String... classDirectories) throws IOException {
+        for (final String classDirectory : classDirectories) {
+            final Path root = pathsGet(classDirectory);
+            if (Files.isDirectory(root)) {
+                Files.walkFileTree(root, new SimpleFileVisitor<Path>() {
+                    @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");
+            }
+        }
+    }
+
+    @SuppressForbidden(reason = "Paths#get")
+    private static Path pathsGet(final String pathString) {
+        return Paths.get(pathString);
+    }
+
+    /**
+     * 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<FeatureAwareViolation> callback) throws IOException {
+        // the class format only reports declared interfaces so we have to walk the hierarchy looking for all interfaces
+        final List<String> 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<String> interfaces,
+            final Consumer<FeatureAwareViolation> 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<? extends ClusterState.FeatureAware> interfaceToCheck,
+            final Class<? extends ClusterState.FeatureAware> expectedInterface,
+            final String name,
+            final List<String> interfaces,
+            final Consumer<FeatureAwareViolation> callback) {
+        final Set<String> 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(".", "/");
+    }
+
+}