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

[REST Compatible API] 'template' parameter and field on PUT index template #71238

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
@@ -0,0 +1,54 @@
{
"indices.put_template_with_param":{
"documentation":{
"url":"https://www.elastic.co/guide/en/elasticsearch/reference/master/indices-templates.html",
"description":"Creates or updates an index template."
},
"stability":"stable",
"visibility":"public",
"headers":{
"accept": [ "application/vnd.elasticsearch+json;compatible-with=7"],
"content_type": ["application/vnd.elasticsearch+json;compatible-with=7"]
},
"url":{
"paths":[
{
"path":"/_template/{name}",
"methods":[
"PUT",
"POST"
],
"parts":{
"name":{
"type":"string",
"description":"The name of the template"
}
}
}
]
},
"params":{
"template":{
"type":"string",
"description":"The indices that this template should apply to, replaced by index_patterns within the template definition."
},
"order":{
"type":"number",
"description":"The order for this template when merging multiple matching ones (higher numbers are merged later, overriding the lower numbers)"
},
"create":{
"type":"boolean",
"description":"Whether the index template should only be added if new or can also replace an existing one",
"default":false
},
"master_timeout":{
"type":"time",
"description":"Specify timeout for connection to master"
}
},
"body":{
"description":"The template definition",
"required":true
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
---
setup:
- skip:
version: "9.0.0 - "
reason: "compatible from 8.x to 7.x"
features:
- "headers"
- "warnings"

---
"Put template":

- do:
headers:
Content-Type: "application/vnd.elasticsearch+json;compatible-with=7"
Accept: "application/vnd.elasticsearch+json;compatible-with=7"
warnings:
- "Deprecated field [template] used, replaced by [index_patterns]"
indices.put_template:
name: test
body:
template: test-*
settings:
number_of_shards: 1
number_of_replicas: 0
mappings:
properties:
field:
type: keyword

- do:
indices.get_template:
name: test
flat_settings: true

- match: {test.index_patterns: ["test-*"]}
- match: {test.settings: {index.number_of_shards: '1', index.number_of_replicas: '0'}}
- match: {test.mappings: {properties: {field: {type: keyword}}}}

---
"Put template (with template parameter)":

- do:
headers:
Content-Type: "application/vnd.elasticsearch+json;compatible-with=7"
Accept: "application/vnd.elasticsearch+json;compatible-with=7"
warnings:
- "Deprecated parameter [template] used, replaced by [index_patterns]"
indices.put_template_with_param:
name: test
template: "test-*"
body:
settings:
number_of_shards: 1
number_of_replicas: 0
mappings:
properties:
field:
type: keyword

- do:
indices.get_template:
name: test
flat_settings: true

- match: {test.index_patterns: ["test-*"]}
- match: {test.settings: {index.number_of_shards: '1', index.number_of_replicas: '0'}}
- match: {test.mappings: {properties: {field: {type: keyword}}}}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@

import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateRequest;
import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.RestApiVersion;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestRequest;
Expand All @@ -26,6 +28,8 @@

public class RestPutIndexTemplateAction extends BaseRestHandler {

private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestPutIndexTemplateAction.class);

@Override
public List<Route> routes() {
return List.of(
Expand All @@ -40,9 +44,14 @@ public String getName() {

@Override
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {

PutIndexTemplateRequest putRequest = new PutIndexTemplateRequest(request.param("name"));
putRequest.patterns(asList(request.paramAsStringArray("index_patterns", Strings.EMPTY_ARRAY)));
if (request.getRestApiVersion() == RestApiVersion.V_7 && request.hasParam("template")) {
deprecationLogger.compatibleApiWarning("template_parameter_deprecation",
"Deprecated parameter [template] used, replaced by [index_patterns]");
putRequest.patterns(List.of(request.param("template")));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the message be a bit more descriptive ?

The deprecated field [template] was included in the request, API compatibility was used to map this field to [index_patterns]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could change it, sure, but I was aiming to have it be very much in line with the automatic logging we'll see from ParseField when api compatibility is invoked.

@Override
public void logReplacedField(String parserName, Supplier<XContentLocation> location, String oldName, String replacedName,
boolean isCompatibleDeprecation) {
String prefix = parserLocation(parserName, location);
TriConsumer<String, Object[], String> loggingFunction = getLoggingFunction(isCompatibleDeprecation);
loggingFunction.apply("{}Deprecated field [{}] used, replaced by [{}]",
new Object[]{prefix, oldName, replacedName}, oldName);
}

So on the one hand we could be consistent with that (my original aim) or on the other we could be more descriptive. I don't have a strong preference -- which way would you prefer it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's leave this as-is and discuss in our next sync. No need to block this PR.

} else {
putRequest.patterns(asList(request.paramAsStringArray("index_patterns", Strings.EMPTY_ARRAY)));
}
putRequest.order(request.paramAsInt("order", putRequest.order()));
putRequest.masterNodeTimeout(request.paramAsTime("master_timeout", putRequest.masterNodeTimeout()));
putRequest.create(request.paramAsBoolean("create", false));
Expand All @@ -51,6 +60,11 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
Map<String, Object> sourceAsMap = XContentHelper.convertToMap(request.requiredContent(), false,
request.getXContentType()).v2();
sourceAsMap = RestCreateIndexAction.prepareMappings(sourceAsMap);
if (request.getRestApiVersion() == RestApiVersion.V_7 && sourceAsMap.containsKey("template")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need a log message here since we already have one above ?

Copy link
Contributor Author

@joegallo joegallo Apr 2, 2021

Choose a reason for hiding this comment

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

Unfortunately, yes. They're different scenarios -- ?template=blah in the url versus "template":"blah" in the json body.

deprecationLogger.compatibleApiWarning("template_field_deprecation",
"Deprecated field [template] used, replaced by [index_patterns]");
putRequest.patterns(List.of((String) sourceAsMap.remove("template")));
}
putRequest.source(sourceAsMap);

return channel -> client.admin().indices().putTemplate(putRequest, new RestToXContentListener<>(channel));
Expand Down