Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow bootstrap plugins to appear in _cat/plugins #66260

Merged
merged 7 commits into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,16 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.Locale;
import java.util.stream.Collectors;

import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.emptyString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.not;
import static org.junit.Assume.assumeTrue;

/**
Expand Down Expand Up @@ -146,6 +152,54 @@ public void test30ElasticsearchStartsWhenSystemPropertySet() throws Exception {
}
}

/**
* Check that the _cat API handles bootstrap plugins correctly. They should be filtered unless
* explicitly asked for. Plugin types can be included in the response if requested.
*/
public void test40CatApiFiltersPlugin() throws Exception {
install();

int total = 20 * 1024 * 1024;
int available = 10 * 1024 * 1024;

installation.executables().pluginTool.run("install --batch \"" + QUOTA_AWARE_FS_PLUGIN.toUri() + "\"");

final Path quotaPath = getRootTempDir().resolve("quota.properties");
Files.writeString(quotaPath, String.format(Locale.ROOT, "total=%d\nremaining=%d\n", total, available));

sh.getEnv().put("ES_JAVA_OPTS", "-Des.fs.quota.file=" + quotaPath.toUri());

try {
startElasticsearch();

// Check that bootstrap plugins are filtered out by default
String response = ServerUtils.makeRequest(Request.Get("http://localhost:9200/_cat/plugins")).trim();
assertThat(response, emptyString());

// Check that bootstrap plugins can be included
response = ServerUtils.makeRequest(Request.Get("http://localhost:9200/_cat/plugins?include_bootstrap=true")).trim();
assertThat(response, not(emptyString()));

List<String> lines = response.lines().collect(Collectors.toList());
assertThat(lines, hasSize(1));
assertThat(lines.get(0).split(" ")[1], equalTo("quota-aware-fs"));

// Check that the plugin type can be included in the response
response = ServerUtils.makeRequest(Request.Get("http://localhost:9200/_cat/plugins?include_bootstrap=true&h=component,type"))
.trim();
assertThat(response, not(emptyString()));

lines = response.lines().collect(Collectors.toList());
assertThat(lines, hasSize(1));

final String[] fields = lines.get(0).split(" ");
assertThat(fields, arrayContaining("quota-aware-fs", "bootstrap"));
} finally {
stopElasticsearch();
Files.deleteIfExists(quotaPath);
}
}

private static class Totals {
int totalInBytes;
int availableInBytes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@
"description":"Return help information",
"default":false
},
"include_bootstrap":{
"type":"boolean",
"description":"Include bootstrap plugins in the response",
"default":false
},
"s":{
"type":"list",
"description":"Comma-separated list of column names or column aliases to sort by"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
---
"Help":
- skip:
version: " - 7.99.99"
reason: output format changed in 8.0.0

- do:
cat.plugins:
help: true
Expand All @@ -11,4 +15,5 @@
component .+ \n
version .+ \n
description .+ \n
type .+ \n
$/
22 changes: 10 additions & 12 deletions server/src/main/java/org/elasticsearch/plugins/PluginsService.java
Original file line number Diff line number Diff line change
Expand Up @@ -351,15 +351,11 @@ private static Set<Bundle> findBundles(final Path directory, String type) throws
final Set<Bundle> bundles = new HashSet<>();
for (final Path plugin : findPluginDirs(directory)) {
final Bundle bundle = readPluginBundle(plugin, type);
if (bundle.plugin.getType() == PluginType.BOOTSTRAP) {
logger.trace("--- skipping bootstrap plugin [{}] [{}]", type, plugin.toAbsolutePath());
} else {
if (bundles.add(bundle) == false) {
throw new IllegalStateException("duplicate " + type + ": " + bundle.plugin);
}
if (type.equals("module") && bundle.plugin.getName().startsWith("test-") && Build.CURRENT.isSnapshot() == false) {
throw new IllegalStateException("external test module [" + plugin.getFileName() + "] found in non-snapshot build");
}
if (bundles.add(bundle) == false) {
throw new IllegalStateException("duplicate " + type + ": " + bundle.plugin);
}
if (type.equals("module") && bundle.plugin.getName().startsWith("test-") && Build.CURRENT.isSnapshot() == false) {
throw new IllegalStateException("external test module [" + plugin.getFileName() + "] found in non-snapshot build");
}
}

Expand Down Expand Up @@ -443,10 +439,12 @@ private List<Tuple<PluginInfo,Plugin>> loadBundles(Set<Bundle> bundles) {
Map<String, Set<URL>> transitiveUrls = new HashMap<>();
List<Bundle> sortedBundles = sortBundles(bundles);
for (Bundle bundle : sortedBundles) {
checkBundleJarHell(JarHell.parseClassPath(), bundle, transitiveUrls);
if (bundle.plugin.getType() != PluginType.BOOTSTRAP) {
checkBundleJarHell(JarHell.parseClassPath(), bundle, transitiveUrls);

final Plugin plugin = loadBundle(bundle, loaded);
plugins.add(new Tuple<>(bundle.plugin, plugin));
final Plugin plugin = loadBundle(bundle, loaded);
plugins.add(new Tuple<>(bundle.plugin, plugin));
}
}

loadExtensions(plugins);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,14 @@
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.common.Table;
import org.elasticsearch.plugins.PluginInfo;
import org.elasticsearch.plugins.PluginType;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.RestResponse;
import org.elasticsearch.rest.action.RestActionListener;
import org.elasticsearch.rest.action.RestResponseListener;

import java.util.List;
import java.util.Locale;

import static org.elasticsearch.rest.RestRequest.Method.GET;

Expand All @@ -58,6 +60,7 @@ protected void documentation(StringBuilder sb) {

@Override
public RestChannelConsumer doCatRequest(final RestRequest request, final NodeClient client) {
final boolean includeBootstrapPlugins = request.paramAsBoolean("include_bootstrap", false);
final ClusterStateRequest clusterStateRequest = new ClusterStateRequest();
clusterStateRequest.clear().nodes(true);
clusterStateRequest.local(request.paramAsBoolean("local", clusterStateRequest.local()));
Expand All @@ -72,7 +75,10 @@ public void processResponse(final ClusterStateResponse clusterStateResponse) thr
client.admin().cluster().nodesInfo(nodesInfoRequest, new RestResponseListener<NodesInfoResponse>(channel) {
@Override
public RestResponse buildResponse(final NodesInfoResponse nodesInfoResponse) throws Exception {
return RestTable.buildResponse(buildTable(request, clusterStateResponse, nodesInfoResponse), channel);
return RestTable.buildResponse(
buildTable(request, clusterStateResponse, nodesInfoResponse, includeBootstrapPlugins),
channel
);
}
});
}
Expand All @@ -88,11 +94,12 @@ protected Table getTableWithHeader(final RestRequest request) {
table.addCell("component", "alias:c;desc:component");
table.addCell("version", "alias:v;desc:component version");
table.addCell("description", "alias:d;default:false;desc:plugin details");
table.addCell("type", "alias:t;default:false;desc:plugin type");
table.endHeaders();
return table;
}

private Table buildTable(RestRequest req, ClusterStateResponse state, NodesInfoResponse nodesInfo) {
private Table buildTable(RestRequest req, ClusterStateResponse state, NodesInfoResponse nodesInfo, boolean includeBootstrapPlugins) {
DiscoveryNodes nodes = state.getState().nodes();
Table table = getTableWithHeader(req);

Expand All @@ -106,12 +113,16 @@ private Table buildTable(RestRequest req, ClusterStateResponse state, NodesInfoR
continue;
}
for (PluginInfo pluginInfo : plugins.getPluginInfos()) {
if (pluginInfo.getType() == PluginType.BOOTSTRAP && includeBootstrapPlugins == false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't seem to have any unit tests for this class. Would it be possible to add a simple one? The packaging tests added seem to be testing this new flag, but that doesn't seem like the right place, since the flag doesn't directly have anything to do with the quota aware fs plugin, it's just the only example we have so far a bootstrap plugin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rjernst I've added unit tests, please take a look.

continue;
}
table.startRow();
table.addCell(node.getId());
table.addCell(node.getName());
table.addCell(pluginInfo.getName());
table.addCell(pluginInfo.getVersion());
table.addCell(pluginInfo.getDescription());
table.addCell(pluginInfo.getType().toString().toLowerCase(Locale.ROOT));
table.endRow();
}
}
Expand Down