honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-03-09 14:34:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add check for feature aware implementations (#31081)

Browse Source 
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.
This commit is contained in:
Jason Tedor 2018-06-05 19:56:22 -04:00 committed by GitHub
parent 7c05f69c39
commit 805648848d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 574 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
build.gradle
View File

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

50
x-pack/plugin/build.gradle
View File

@ -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

16
x-pack/test/feature-aware/build.gradle Normal file
View File

@ -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

180
x-pack/test/feature-aware/src/main/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheck.java Normal file
View File

@ -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(".", "/");
}
}

323
x-pack/test/feature-aware/src/test/java/org/elasticsearch/xpack/test/feature_aware/FeatureAwareCheckTests.java Normal file
View File

@ -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<ClusterState.Custom> 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<MetaData.XContentContext> context() {
return MetaData.ALL_CONTEXTS;
}
@Override
public Diff<MetaData.Custom> 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<FeatureAwareCheck.FeatureAwareViolation> {
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<? extends ClusterState.FeatureAware> clazz,
final Class<?> outerClazz,
final Class<? extends ClusterState.FeatureAware> interfaceClazz,
final Class<? extends ClusterState.FeatureAware> 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<? extends ClusterState.FeatureAware> clazz,
final Class<?> outerClazz,
final Class<? extends ClusterState.FeatureAware> interfaceClazz,
final Class<? extends ClusterState.FeatureAware> 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<? extends ClusterState.FeatureAware> clazz,
final Class<?> outerClazz,
final Class<? extends ClusterState.FeatureAware> interfaceClazz,
final Class<? extends ClusterState.FeatureAware> 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));
}
}