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.

Sign up for GitHub

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

Only allow retrieving a single index or component template #54694

Merged
merged 4 commits into from
Apr 7, 2020
Merged

Only allow retrieving a single index or component template #54694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
986def0
Only allow retrieving a single index or component template
dakrone Apr 2, 2020
f86999d
Throw exception when resource is not found
dakrone Apr 3, 2020
8f03b2e
Merge branch 'master' into itv2-only-get-single-item
elasticmachine Apr 3, 2020
0f717d9
Merge branch 'master' into itv2-only-get-single-item
elasticmachine Apr 7, 2020
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
39 changes: 14 additions & 25 deletions .../java/org/elasticsearch/action/admin/indices/template/get/GetComponentTemplateAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
import org.elasticsearch.action.ActionType;
import org.elasticsearch.action.support.master.MasterNodeReadRequest;
import org.elasticsearch.cluster.metadata.ComponentTemplate;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ToXContentObject;
Expand All @@ -36,8 +36,6 @@
import java.util.Map;
import java.util.Objects;

import static org.elasticsearch.action.ValidateActions.addValidationError;

/**
* Action to retrieve one or more component templates
*/
Expand All @@ -55,53 +53,44 @@ private GetComponentTemplateAction() {
*/
public static class Request extends MasterNodeReadRequest<Request> {

private String[] names;
@Nullable
private String name;

public Request() { }

public Request(String... names) {
this.names = names;
public Request(String name) {
this.name = name;
}

public Request(StreamInput in) throws IOException {
super(in);
names = in.readStringArray();
name = in.readOptionalString();
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeStringArray(names);
out.writeOptionalString(name);
}

@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
if (names == null) {
validationException = addValidationError("names is null or empty", validationException);
} else {
for (String name : names) {
if (name == null || Strings.hasText(name) == false) {
validationException = addValidationError("name is missing", validationException);
}
}
}
return validationException;
return null;
}

/**
* Sets the names of the component templates.
* Sets the name of the component templates.
*/
public Request names(String... names) {
this.names = names;
public Request name(String name) {
this.name = name;
return this;
}

/**
* The names of the component templates.
* The name of the component templates.
*/
public String[] names() {
return this.names;
public String name() {
return this.name;
}
}

Expand Down
44 changes: 16 additions & 28 deletions ...in/java/org/elasticsearch/action/admin/indices/template/get/GetIndexTemplateV2Action.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,18 @@
import org.elasticsearch.action.ActionType;
import org.elasticsearch.action.support.master.MasterNodeReadRequest;
import org.elasticsearch.cluster.metadata.IndexTemplateV2;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;

import java.io.IOException;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;

import static org.elasticsearch.action.ValidateActions.addValidationError;

public class GetIndexTemplateV2Action extends ActionType<GetIndexTemplateV2Action.Response> {

public static final GetIndexTemplateV2Action INSTANCE = new GetIndexTemplateV2Action();
Expand All @@ -53,58 +50,49 @@ private GetIndexTemplateV2Action() {
*/
public static class Request extends MasterNodeReadRequest<Request> {

private String[] names;
@Nullable
private String name;

public Request() { }

public Request(String... names) {
this.names = names;
public Request(@Nullable String name) {
this.name = name;
}

public Request(StreamInput in) throws IOException {
super(in);
names = in.readStringArray();
name = in.readOptionalString();
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeStringArray(names);
out.writeOptionalString(name);
}

@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
if (names == null) {
validationException = addValidationError("names is null or empty", validationException);
} else {
for (String name : names) {
if (name == null || Strings.hasText(name) == false) {
validationException = addValidationError("name is missing", validationException);
}
}
}
return validationException;
return null;
}

/**
* Sets the names of the index templates.
* Sets the name of the index template.
*/
public Request names(String... names) {
this.names = names;
public Request name(String name) {
this.name = name;
return this;
}

/**
* The names of the index templates.
* The name of the index templates.
*/
public String[] names() {
return this.names;
public String name() {
return this.name;
}

@Override
public int hashCode() {
return Arrays.hashCode(names);
return Objects.hash(name);
}

@Override
Expand All @@ -116,7 +104,7 @@ public boolean equals(Object obj) {
return false;
}
Request other = (Request) obj;
return Arrays.equals(other.names, this.names);
return Objects.equals(name, other.name);
}
}

Expand Down
20 changes: 11 additions & 9 deletions .../elasticsearch/action/admin/indices/template/get/TransportGetComponentTemplateAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.action.admin.indices.template.get;

import org.elasticsearch.ResourceNotFoundException;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.master.TransportMasterNodeReadAction;
Expand Down Expand Up @@ -71,22 +72,23 @@ protected void masterOperation(Task task, GetComponentTemplateAction.Request req
Map<String, ComponentTemplate> allTemplates = state.metadata().componentTemplates();

// If we did not ask for a specific name, then we return all templates
if (request.names().length == 0) {
if (request.name() == null) {
listener.onResponse(new GetComponentTemplateAction.Response(allTemplates));
return;
}

final Map<String, ComponentTemplate> results = new HashMap<>();
for (String name : request.names()) {
if (Regex.isSimpleMatchPattern(name)) {
for (Map.Entry<String, ComponentTemplate> entry : allTemplates.entrySet()) {
if (Regex.simpleMatch(name, entry.getKey())) {
results.put(entry.getKey(), entry.getValue());
}
String name = request.name();
if (Regex.isSimpleMatchPattern(name)) {
for (Map.Entry<String, ComponentTemplate> entry : allTemplates.entrySet()) {
if (Regex.simpleMatch(name, entry.getKey())) {
results.put(entry.getKey(), entry.getValue());
}
} else if (allTemplates.containsKey(name)) {
results.put(name, allTemplates.get(name));
}
} else if (allTemplates.containsKey(name)) {
results.put(name, allTemplates.get(name));
Copy link
Member

Choose a reason for hiding this comment

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

I think in case when the template is missing here (when asking for a specific template by name) we want to throw a resource not found exception. Like was done here: https://github.com/elastic/elasticsearch/pull/54530/files#diff-e753c14c84c5edb785bc197306e5a24fR180

} else {
throw new ResourceNotFoundException("component template matching [" + request.name() + "] not found");
}

listener.onResponse(new GetComponentTemplateAction.Response(results));
Expand Down
20 changes: 11 additions & 9 deletions ...rg/elasticsearch/action/admin/indices/template/get/TransportGetIndexTemplateV2Action.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.action.admin.indices.template.get;

import org.elasticsearch.ResourceNotFoundException;
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.master.TransportMasterNodeReadAction;
Expand Down Expand Up @@ -71,22 +72,23 @@ protected void masterOperation(Task task, GetIndexTemplateV2Action.Request reque
Map<String, IndexTemplateV2> allTemplates = state.metadata().templatesV2();

// If we did not ask for a specific name, then we return all templates
if (request.names().length == 0) {
if (request.name() == null) {
listener.onResponse(new GetIndexTemplateV2Action.Response(allTemplates));
return;
}

final Map<String, IndexTemplateV2> results = new HashMap<>();
for (String name : request.names()) {
if (Regex.isSimpleMatchPattern(name)) {
for (Map.Entry<String, IndexTemplateV2> entry : allTemplates.entrySet()) {
if (Regex.simpleMatch(name, entry.getKey())) {
results.put(entry.getKey(), entry.getValue());
}
String name = request.name();
if (Regex.isSimpleMatchPattern(name)) {
for (Map.Entry<String, IndexTemplateV2> entry : allTemplates.entrySet()) {
if (Regex.simpleMatch(name, entry.getKey())) {
results.put(entry.getKey(), entry.getValue());
}
} else if (allTemplates.containsKey(name)) {
results.put(name, allTemplates.get(name));
}
} else if (allTemplates.containsKey(name)) {
results.put(name, allTemplates.get(name));
Copy link
Member

Choose a reason for hiding this comment

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

Throw a resource not found exception if template does not exist?

} else {
throw new ResourceNotFoundException("index template matching [" + request.name() + "] not found");
}

listener.onResponse(new GetIndexTemplateV2Action.Response(results));
Expand Down
6 changes: 2 additions & 4 deletions ...main/java/org/elasticsearch/rest/action/admin/indices/RestGetComponentTemplateAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import org.elasticsearch.action.admin.indices.template.get.GetComponentTemplateAction;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestRequest;
Expand Down Expand Up @@ -54,14 +53,13 @@ public String getName() {

@Override
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
final String[] names = Strings.splitStringByCommaToArray(request.param("name"));

final GetComponentTemplateAction.Request getRequest = new GetComponentTemplateAction.Request(names);
final GetComponentTemplateAction.Request getRequest = new GetComponentTemplateAction.Request(request.param("name"));

getRequest.local(request.paramAsBoolean("local", getRequest.local()));
getRequest.masterNodeTimeout(request.paramAsTime("master_timeout", getRequest.masterNodeTimeout()));

final boolean implicitAll = getRequest.names().length == 0;
final boolean implicitAll = getRequest.name() == null;

return channel ->
client.execute(GetComponentTemplateAction.INSTANCE, getRequest, new RestToXContentListener<>(channel) {
Expand Down
7 changes: 2 additions & 5 deletions ...c/main/java/org/elasticsearch/rest/action/admin/indices/RestGetIndexTemplateV2Action.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import org.elasticsearch.action.admin.indices.template.get.GetIndexTemplateV2Action;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestRequest;
Expand Down Expand Up @@ -54,14 +53,12 @@ public String getName() {

@Override
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
final String[] names = Strings.splitStringByCommaToArray(request.param("name"));

final GetIndexTemplateV2Action.Request getRequest = new GetIndexTemplateV2Action.Request(names);
final GetIndexTemplateV2Action.Request getRequest = new GetIndexTemplateV2Action.Request(request.param("name"));

getRequest.local(request.paramAsBoolean("local", getRequest.local()));
getRequest.masterNodeTimeout(request.paramAsTime("master_timeout", getRequest.masterNodeTimeout()));

final boolean implicitAll = getRequest.names().length == 0;
final boolean implicitAll = getRequest.name() == null;

return channel ->
client.execute(GetIndexTemplateV2Action.INSTANCE, getRequest, new RestToXContentListener<>(channel) {
Expand Down
2 changes: 1 addition & 1 deletion ...a/org/elasticsearch/action/admin/indices/template/get/GetIndexTemplateV2RequestTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ protected Writeable.Reader<GetIndexTemplateV2Action.Request> instanceReader() {

@Override
protected GetIndexTemplateV2Action.Request createTestInstance() {
return new GetIndexTemplateV2Action.Request(generateRandomStringArray(5, 5, false, false));
return new GetIndexTemplateV2Action.Request(randomBoolean() ? null : randomAlphaOfLength(4));
}

@Override
Expand Down