Browse Source

Get-templates APIs don't support lists (#78989)

We document that `GET /_index_template/...` accepts a comma-separated
list of template names but in fact today this API accepts only a single
name or pattern. Likewise `GET /_cat/templates/...` (at least it didn't
until #78829 but that's not released yet). This commit fixes the docs to
indicate these APIs accept only a single template name and also adds
some extra validation to reject requests containing a `,` since such a
request cannot match any actual templates.

It also adjusts `GET /_cat/templates` to use the filtering built into
`TransportGetComposableIndexTemplateAction` rather than retrieving all
templates and then filtering them on the coordinating node.
David Turner 4 years ago
parent
commit
d2bb6ebb69

+ 2 - 3
docs/reference/cat/templates.asciidoc

@@ -26,9 +26,8 @@ and <<mapping,field mappings>> to new indices at creation.
 ==== {api-path-parms-title}
 
 `<template_name>`::
-(Optional, string) Comma-separated list of index template names used to limit
-the request. Accepts wildcard expressions.
-
+(Optional, string) The name of the template to return. Accepts wildcard
+expressions. If omitted, all templates are returned.
 
 [[cat-templates-query-params]]
 ==== {api-query-parms-title}

+ 2 - 4
docs/reference/indices/get-index-template.asciidoc

@@ -51,10 +51,8 @@ privilege>> to use this API.
 [[get-template-api-path-params]]
 ==== {api-path-parms-title}
 
-include::{docdir}/rest-api/common-parms.asciidoc[tag=index-template]
-+
-To retrieve all index templates, omit this parameter or use a value of `*`.
-
+(Optional, string) The name of the template to return. Accepts wildcard
+expressions. If omitted, all templates are returned.
 
 [[get-template-api-query-params]]
 ==== {api-query-parms-title}

+ 2 - 2
rest-api-spec/src/main/resources/rest-api-spec/api/indices.get_index_template.json

@@ -24,8 +24,8 @@
           ],
           "parts":{
             "name":{
-              "type":"list",
-              "description":"The comma separated names of the index templates"
+              "type":"string",
+              "description":"A pattern that returned template names must match"
             }
           }
         }

+ 8 - 51
rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cat.templates/20_matching.yml

@@ -75,21 +75,6 @@ setup:
     - match:
         $body: /test-composable-1\ntest-composable-2\ntest-legacy-1\ntest-legacy-2\n/
 
----
-"Matching all templates with other patterns":
-    - skip:
-        version: " - 7.99.99"
-        reason: "support for multiple patterns added in 8.0.0"
-
-    - do:
-        cat.templates:
-            name: "nonexistent*,*,other-name"
-            h: [name]
-            s: [name]
-
-    - match:
-        $body: /test-composable-1\ntest-composable-2\ntest-legacy-1\ntest-legacy-2\n/
-
 ---
 "Matching no templates":
 
@@ -146,41 +131,13 @@ setup:
     - match:
         $body: /^test-composable-2\ntest-legacy-2\n$/
 
----
-"Matching lists of names":
-    - skip:
-        version: " - 7.99.99"
-        reason: "support for multiple patterns added in 8.0.0"
-
-    - do:
-        cat.templates:
-            name: "test-legacy-1,test-composable-2"
-            h: [name]
-            s: [name]
-
-    - match:
-        $body: /^test-composable-2\ntest-legacy-1\n$/
 
 ---
-"Matching names and wildcards":
-    - skip:
-        version: " - 7.99.99"
-        reason: "support for multiple patterns added in 8.0.0"
-
-    - do:
-        cat.templates:
-            name: "test-legacy-1,test-composable-*"
-            h: [name]
-            s: [name]
-
-    - match:
-        $body: /^test-composable-1\ntest-composable-2\ntest-legacy-1\n$/
-
-    - do:
-        cat.templates:
-            name: "test-legacy-*,test-composable-2"
-            h: [name]
-            s: [name]
-
-    - match:
-        $body: /^test-composable-2\ntest-legacy-1\ntest-legacy-2\n$/
+"Reject request containing comma":
+  - skip:
+      version: " - 7.99.99"
+      reason: "validation only added in 8.0.0"
+  - do:
+      catch:  bad_request
+      cat.templates:
+        name: test1,test2

+ 34 - 2
rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.get_index_template/10_basic.yml

@@ -59,11 +59,43 @@ setup:
   - is_true: index_templates.0.name
   - is_true: index_templates.1.name
 
+---
+"Pattern matching in index templates":
+  - skip:
+      version: " - 7.7.99"
+      reason: "index template v2 API unavailable before 7.8"
+      features: allowed_warnings
+
+  - do:
+      allowed_warnings:
+        - "index template [test2] has index patterns [test2-*] matching patterns from existing older templates [global] with patterns (global => [*]); this template [test2] will take precedence during new index creation"
+      indices.put_index_template:
+        name: test2
+        body:
+          index_patterns: test2-*
+          template:
+            settings:
+              number_of_shards:   1
+
+  - do:
+      indices.get_index_template:
+        name: "*"
+
+  - is_true: index_templates.0.name
+  - is_true: index_templates.1.name
+
+  - do:
+      indices.get_index_template:
+        name: "test*"
+
+  - match: { index_templates.0.name: "/test.*/" }
+  - match: { index_templates.1.name: "/test.*/" }
+
 ---
 "Get index template with local flag":
   - skip:
-      version: " - 7.99.99"
-      reason: "index template v2 API has not been backported"
+      version: " - 7.7.99"
+      reason: "index template v2 API unavailable before 7.8"
 
   - do:
       indices.get_index_template:

+ 31 - 5
rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.get_index_template/20_get_missing.yml

@@ -1,20 +1,46 @@
 setup:
   - skip:
-      version: " - 7.99.99"
-      reason: "index template v2 API has not been backported"
+      version: " - 7.7.99"
+      reason: "index template v2 API unavailable before 7.8"
 
   - do:
       indices.delete_index_template:
         name:   '*'
         ignore: 404
+
 ---
 "Get missing template":
   - skip:
-      version: " - 7.99.99"
-      reason: "index template v2 API has not been backported"
+      version: " - 7.7.99"
+      reason: "index template v2 API unavailable before 7.8"
 
   - do:
       catch:  missing
       indices.get_index_template:
-        name: test
+        name: nonexistent
+
+  - match: { error.reason: "index template matching [nonexistent] not found" }
+
+---
+"Get non-matching wildcard":
+  - skip:
+      version: " - 7.7.99"
+      reason: "index template v2 API unavailable before 7.8"
+
+  - do:
+      catch: missing
+      indices.get_index_template:
+        name: "non-matching-wildcard*"
 
+  - is_false: error
+  - is_true: index_templates
+
+---
+"Reject request containing comma":
+  - skip:
+      version: " - 7.99.99"
+      reason: "validation only added in 8.0.0"
+  - do:
+      catch:  bad_request
+      indices.get_index_template:
+        name: test1,test2

+ 9 - 13
server/src/main/java/org/elasticsearch/action/admin/indices/template/get/GetComposableIndexTemplateAction.java

@@ -13,10 +13,10 @@ import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.action.ActionType;
 import org.elasticsearch.action.support.master.MasterNodeReadRequest;
 import org.elasticsearch.cluster.metadata.ComposableIndexTemplate;
-import org.elasticsearch.core.Nullable;
-import org.elasticsearch.xcontent.ParseField;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.core.Nullable;
+import org.elasticsearch.xcontent.ParseField;
 import org.elasticsearch.xcontent.ToXContentObject;
 import org.elasticsearch.xcontent.XContentBuilder;
 
@@ -39,11 +39,15 @@ public class GetComposableIndexTemplateAction extends ActionType<GetComposableIn
     public static class Request extends MasterNodeReadRequest<Request> {
 
         @Nullable
-        private String name;
-
-        public Request() { }
+        private final String name;
 
+        /**
+         * @param name A template name or pattern, or {@code null} to retrieve all templates.
+         */
         public Request(@Nullable String name) {
+            if (name != null && name.contains(",")) {
+                throw new IllegalArgumentException("template name may not contain ','");
+            }
             this.name = name;
         }
 
@@ -63,14 +67,6 @@ public class GetComposableIndexTemplateAction extends ActionType<GetComposableIn
             return null;
         }
 
-        /**
-         * Sets the name of the index template.
-         */
-        public Request name(String name) {
-            this.name = name;
-            return this;
-        }
-
         /**
          * The name of the index templates.
          */

+ 1 - 12
server/src/main/java/org/elasticsearch/action/admin/indices/template/get/GetIndexTemplatesRequest.java

@@ -22,10 +22,7 @@ import static org.elasticsearch.action.ValidateActions.addValidationError;
  */
 public class GetIndexTemplatesRequest extends MasterNodeReadRequest<GetIndexTemplatesRequest> {
 
-    private String[] names;
-
-    public GetIndexTemplatesRequest() {
-    }
+    private final String[] names;
 
     public GetIndexTemplatesRequest(String... names) {
         this.names = names;
@@ -57,14 +54,6 @@ public class GetIndexTemplatesRequest extends MasterNodeReadRequest<GetIndexTemp
         return validationException;
     }
 
-    /**
-     * Sets the names of the index templates.
-     */
-    public GetIndexTemplatesRequest names(String... names) {
-        this.names = names;
-        return this;
-    }
-
     /**
      * The names of the index templates.
      */

+ 29 - 61
server/src/main/java/org/elasticsearch/rest/action/cat/RestTemplatesAction.java

@@ -8,6 +8,8 @@
 
 package org.elasticsearch.rest.action.cat;
 
+import org.elasticsearch.ExceptionsHelper;
+import org.elasticsearch.ResourceNotFoundException;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.StepListener;
 import org.elasticsearch.action.admin.indices.template.get.GetComposableIndexTemplateAction;
@@ -16,19 +18,14 @@ import org.elasticsearch.action.admin.indices.template.get.GetIndexTemplatesResp
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.cluster.metadata.ComposableIndexTemplate;
 import org.elasticsearch.cluster.metadata.IndexTemplateMetadata;
-import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.Table;
-import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.rest.RestResponse;
 import org.elasticsearch.rest.action.RestResponseListener;
 
-import java.util.ArrayList;
-import java.util.HashSet;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
-import java.util.function.Predicate;
 
 import static org.elasticsearch.rest.RestRequest.Method.GET;
 
@@ -53,14 +50,17 @@ public class RestTemplatesAction extends AbstractCatAction {
 
     @Override
     protected RestChannelConsumer doCatRequest(final RestRequest request, NodeClient client) {
-        final String[] templateNames = Strings.splitStringByCommaToArray(request.param("name", ""));
+        final String matchPattern = request.hasParam("name") ? request.param("name") : null;
 
-        final GetIndexTemplatesRequest getIndexTemplatesRequest = new GetIndexTemplatesRequest(templateNames);
+        final GetIndexTemplatesRequest getIndexTemplatesRequest
+            = matchPattern == null
+            ? new GetIndexTemplatesRequest()
+            : new GetIndexTemplatesRequest(matchPattern);
         getIndexTemplatesRequest.local(request.paramAsBoolean("local", getIndexTemplatesRequest.local()));
         getIndexTemplatesRequest.masterNodeTimeout(request.paramAsTime("master_timeout", getIndexTemplatesRequest.masterNodeTimeout()));
 
         final GetComposableIndexTemplateAction.Request getComposableTemplatesRequest
-            = new GetComposableIndexTemplateAction.Request();
+            = new GetComposableIndexTemplateAction.Request(matchPattern);
         getComposableTemplatesRequest.local(request.paramAsBoolean("local", getComposableTemplatesRequest.local()));
         getComposableTemplatesRequest.masterNodeTimeout(
             request.paramAsTime("master_timeout", getComposableTemplatesRequest.masterNodeTimeout()));
@@ -71,7 +71,16 @@ public class RestTemplatesAction extends AbstractCatAction {
             client.admin().indices().getTemplates(getIndexTemplatesRequest, getIndexTemplatesStep);
 
             final StepListener<GetComposableIndexTemplateAction.Response> getComposableTemplatesStep = new StepListener<>();
-            client.execute(GetComposableIndexTemplateAction.INSTANCE, getComposableTemplatesRequest, getComposableTemplatesStep);
+            client.execute(
+                GetComposableIndexTemplateAction.INSTANCE,
+                getComposableTemplatesRequest,
+                getComposableTemplatesStep.delegateResponse((l, e) -> {
+                    if (ExceptionsHelper.unwrapCause(e) instanceof ResourceNotFoundException) {
+                        l.onResponse(new GetComposableIndexTemplateAction.Response(Collections.emptyMap()));
+                    } else {
+                        l.onFailure(e);
+                    }
+                }));
 
             final ActionListener<Table> tableListener = new RestResponseListener<>(channel) {
                 @Override
@@ -85,8 +94,7 @@ public class RestTemplatesAction extends AbstractCatAction {
                     ActionListener.completeWith(tableListener, () -> buildTable(
                         request,
                         getIndexTemplatesResponse,
-                        getComposableIndexTemplatesResponse,
-                        templateNames)
+                        getComposableIndexTemplatesResponse)
                     ), tableListener::onFailure
                 ), tableListener::onFailure);
         };
@@ -108,14 +116,10 @@ public class RestTemplatesAction extends AbstractCatAction {
     private Table buildTable(
         RestRequest request,
         GetIndexTemplatesResponse getIndexTemplatesResponse,
-        GetComposableIndexTemplateAction.Response getComposableIndexTemplatesResponse,
-        String[] requestedNames
+        GetComposableIndexTemplateAction.Response getComposableIndexTemplatesResponse
     ) {
-        final Predicate<String> namePredicate = getNamePredicate(requestedNames);
-
         final Table table = getTableWithHeader(request);
         for (IndexTemplateMetadata indexData : getIndexTemplatesResponse.getIndexTemplates()) {
-            assert namePredicate.test(indexData.getName());
             table.startRow();
             table.addCell(indexData.name());
             table.addCell("[" + String.join(", ", indexData.patterns()) + "]");
@@ -127,52 +131,16 @@ public class RestTemplatesAction extends AbstractCatAction {
 
         for (Map.Entry<String, ComposableIndexTemplate> entry : getComposableIndexTemplatesResponse.indexTemplates().entrySet()) {
             final String name = entry.getKey();
-            if (namePredicate.test(name)) {
-                final ComposableIndexTemplate template = entry.getValue();
-                table.startRow();
-                table.addCell(name);
-                table.addCell("[" + String.join(", ", template.indexPatterns()) + "]");
-                table.addCell(template.priorityOrZero());
-                table.addCell(template.version());
-                table.addCell("[" + String.join(", ", template.composedOf()) + "]");
-                table.endRow();
-            }
+            final ComposableIndexTemplate template = entry.getValue();
+            table.startRow();
+            table.addCell(name);
+            table.addCell("[" + String.join(", ", template.indexPatterns()) + "]");
+            table.addCell(template.priorityOrZero());
+            table.addCell(template.version());
+            table.addCell("[" + String.join(", ", template.composedOf()) + "]");
+            table.endRow();
         }
 
         return table;
     }
-
-    private Predicate<String> getNamePredicate(String[] requestedNames) {
-        if (requestedNames.length == 0) {
-            return name -> true;
-        }
-
-        final Set<String> exactMatches = new HashSet<>();
-        final List<String> patterns = new ArrayList<>();
-        for (String requestedName : requestedNames) {
-            if (Regex.isMatchAllPattern(requestedName)) {
-                return name -> true;
-            } else if (Regex.isSimpleMatchPattern(requestedName)) {
-                patterns.add(requestedName);
-            } else {
-                exactMatches.add(requestedName);
-            }
-        }
-
-        if (patterns.isEmpty()) {
-            return exactMatches::contains;
-        }
-
-        return name -> {
-            if (exactMatches.contains(name)) {
-                return true;
-            }
-            for (String pattern : patterns) {
-                if (Regex.simpleMatch(pattern, name)) {
-                    return true;
-                }
-            }
-            return false;
-        };
-    }
 }