Enforce limitations on ILM policy names (elastic#35104)

Enforces restrictions on ILM policy names to ensure we don't accept
policy names the system can't handle, or may reserve for future use.

Author: gwbrown

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecyclePolicy.java

@@ -21,12 +21,14 @@
 import org.elasticsearch.xpack.core.indexlifecycle.Step.StepKey;
 
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.ListIterator;
 import java.util.Map;
 import java.util.Objects;
+import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
@@ -40,6 +42,7 @@
 public class LifecyclePolicy extends AbstractDiffable<LifecyclePolicy>
         implements ToXContentObject, Diffable<LifecyclePolicy> {
     private static final Logger logger = LogManager.getLogger(LifecyclePolicy.class);
+    private static final int MAX_INDEX_NAME_BYTES = 255;
 
     public static final ParseField PHASES_FIELD = new ParseField("phases");
 
@@ -241,6 +244,30 @@ public boolean isActionSafe(StepKey stepKey) {
         }
     }
 
+    /**
+     * Validate the name for an policy against some static rules. Intended to match
+     * {@link org.elasticsearch.cluster.metadata.MetaDataCreateIndexService#validateIndexOrAliasName(String, BiFunction)}
+     * @param policy the policy name to validate
+     * @throws IllegalArgumentException if the name is invalid
+     */
+    public static void validatePolicyName(String policy) {
+        if (policy.contains(",")) {
+            throw new IllegalArgumentException("invalid policy name [" + policy + "]: must not contain ','");
+        }
+        if (policy.contains(" ")) {
+            throw new IllegalArgumentException("invalid policy name [" + policy + "]: must not contain spaces");
+        }
+        if (policy.charAt(0) == '_') {
+            throw new IllegalArgumentException("invalid policy name [" + policy + "]: must not start with '_'");
+        }
+        int byteCount = 0;
+        byteCount = policy.getBytes(StandardCharsets.UTF_8).length;
+        if (byteCount > MAX_INDEX_NAME_BYTES) {
+            throw new IllegalArgumentException("invalid policy name [" + policy + "]: name is too long, (" + byteCount + " > " +
+                MAX_INDEX_NAME_BYTES + ")");
+        }
+    }
+
     @Override
     public int hashCode() {
         return Objects.hash(name, phases);

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/LifecyclePolicyTests.java

@@ -316,4 +316,20 @@ public void testIsActionSafe() {
 
         assertTrue(policy.isActionSafe(new StepKey("new", randomAlphaOfLength(10), randomAlphaOfLength(10))));
     }
+
+    public void testValidatePolicyName() {
+        expectThrows(IllegalArgumentException.class, () -> LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(0,10) +
+            "," + randomAlphaOfLengthBetween(0,10)));
+        expectThrows(IllegalArgumentException.class, () -> LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(0,10) +
+            " " + randomAlphaOfLengthBetween(0,10)));
+        expectThrows(IllegalArgumentException.class, () -> LifecyclePolicy.validatePolicyName("_" + randomAlphaOfLengthBetween(1, 20)));
+        expectThrows(IllegalArgumentException.class, () -> LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(256, 1000)));
+
+        LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(1,10) + "_" + randomAlphaOfLengthBetween(0,10));
+
+        LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(0,10) + "-" + randomAlphaOfLengthBetween(0,10));
+        LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(0,10) + "+" + randomAlphaOfLengthBetween(0,10));
+        
+        LifecyclePolicy.validatePolicyName(randomAlphaOfLengthBetween(1,255));
+    }
 }

x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeSeriesLifecycleActionsIT.java

@@ -37,6 +37,7 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.UnsupportedEncodingException;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -46,6 +47,7 @@
 
 import static java.util.Collections.singletonMap;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.not;
@@ -435,6 +437,26 @@ public void testNonexistentPolicy() throws Exception {
 
     }
 
+    public void testInvalidPolicyNames() throws UnsupportedEncodingException {
+        ResponseException ex;
+
+        policy = randomAlphaOfLengthBetween(0,10) + "," + randomAlphaOfLengthBetween(0,10);
+        ex = expectThrows(ResponseException.class, () -> createNewSingletonPolicy("delete", new DeleteAction()));
+        assertThat(ex.getCause().getMessage(), containsString("invalid policy name"));
+        
+        policy = randomAlphaOfLengthBetween(0,10) + "%20" + randomAlphaOfLengthBetween(0,10);
+        ex = expectThrows(ResponseException.class, () -> createNewSingletonPolicy("delete", new DeleteAction()));
+        assertThat(ex.getCause().getMessage(), containsString("invalid policy name"));
+
+        policy = "_" + randomAlphaOfLengthBetween(1, 20);
+        ex = expectThrows(ResponseException.class, () -> createNewSingletonPolicy("delete", new DeleteAction()));
+        assertThat(ex.getMessage(), containsString("invalid policy name"));
+
+        policy = randomAlphaOfLengthBetween(256, 1000);
+        ex = expectThrows(ResponseException.class, () -> createNewSingletonPolicy("delete", new DeleteAction()));
+        assertThat(ex.getMessage(), containsString("invalid policy name"));
+    }
+
     private void createFullPolicy(TimeValue hotTime) throws IOException {
         Map<String, LifecycleAction> warmActions = new HashMap<>();
         warmActions.put(ForceMergeAction.NAME, new ForceMergeAction(1));

x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/action/TransportPutLifecycleAction.java

@@ -23,6 +23,7 @@
 import org.elasticsearch.transport.TransportService;
 import org.elasticsearch.xpack.core.ClientHelper;
 import org.elasticsearch.xpack.core.indexlifecycle.IndexLifecycleMetadata;
+import org.elasticsearch.xpack.core.indexlifecycle.LifecyclePolicy;
 import org.elasticsearch.xpack.core.indexlifecycle.LifecyclePolicyMetadata;
 import org.elasticsearch.xpack.core.indexlifecycle.action.PutLifecycleAction;
 import org.elasticsearch.xpack.core.indexlifecycle.action.PutLifecycleAction.Request;
@@ -66,6 +67,7 @@ protected void masterOperation(Request request, ClusterState state, ActionListen
         Map<String, String> filteredHeaders = threadPool.getThreadContext().getHeaders().entrySet().stream()
             .filter(e -> ClientHelper.SECURITY_HEADER_FILTERS.contains(e.getKey()))
             .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+        LifecyclePolicy.validatePolicyName(request.getPolicy().getName());
         clusterService.submitStateUpdateTask("put-lifecycle-" + request.getPolicy().getName(),
                 new AckedClusterStateUpdateTask<Response>(request, listener) {
                     @Override