Makes some fields in Index Lifecycle API optional (#3687)

Specifically this change makes it optional to:
* Specify `includes`, `excludes` and `requires`maps in the allocate action as long as at least one fo the options is specified and is not an empty map
* Specify an `after` parameter on a phase. If no `after` value is specified `TimeValue.ZERO` is used and the phase will be moved to as soon as the previous phase reports `ACTIONS COMPLETED`. `after` is always non-null when we are serialising the Phase.
* Specify a `type` for a LifecyclePolicy. If no `type` is specified `TimeSeriesLifecycleType.INSTANCE` is used since this is currently the only production `type`. `type` is always non-null when we are serialising the LifecyclePolicy.

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

@@ -52,9 +52,9 @@ public class AllocateAction implements LifecycleAction {
             a -> new AllocateAction((Map<String, String>) a[0], (Map<String, String>) a[1], (Map<String, String>) a[2]));
 
     static {
-        PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> p.mapStrings(), INCLUDE_FIELD);
+        PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> p.mapStrings(), INCLUDE_FIELD);
-        PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> p.mapStrings(), EXCLUDE_FIELD);
+        PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> p.mapStrings(), EXCLUDE_FIELD);
-        PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> p.mapStrings(), REQUIRE_FIELD);
+        PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> p.mapStrings(), REQUIRE_FIELD);
     }
 
     private final Map<String, String> include;
@@ -67,9 +67,26 @@ public class AllocateAction implements LifecycleAction {
     }
 
     public AllocateAction(Map<String, String> include, Map<String, String> exclude, Map<String, String> require) {
-        this.include = include;
+        if (include == null) {
-        this.exclude = exclude;
+            this.include = Collections.emptyMap();
-        this.require = require;
+        } else {
+            this.include = include;
+        }
+        if (exclude == null) {
+            this.exclude = Collections.emptyMap();
+        } else {
+            this.exclude = exclude;
+        }
+        if (require == null) {
+            this.require = Collections.emptyMap();
+        } else {
+            this.require = require;
+        }
+        if (this.include.isEmpty() && this.exclude.isEmpty() && this.require.isEmpty()) {
+            throw new IllegalArgumentException(
+                    "At least one of " + INCLUDE_FIELD.getPreferredName() + ", " + EXCLUDE_FIELD.getPreferredName() + " or "
+                            + REQUIRE_FIELD.getPreferredName() + "must contain attributes for action " + NAME);
+        }
         FilterAllocationDecider decider = new FilterAllocationDecider(Settings.EMPTY,
                 new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS));
         this.allocationDeciders = new AllocationDeciders(Settings.EMPTY, Collections.singletonList(decider));

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

@@ -28,8 +28,6 @@ import java.util.Objects;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
-import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
-
 /**
  * Represents the lifecycle of an index from creation to deletion. A
  * {@link LifecyclePolicy} is made up of a set of {@link Phase}s which it will
@@ -54,9 +52,9 @@ public class LifecyclePolicy extends AbstractDiffable<LifecyclePolicy>
                 return new LifecyclePolicy(type, name, phaseMap);
             });
     static {
-        PARSER.declareField(constructorArg(), (p, c) -> p.namedObject(LifecycleType.class, p.text(), null), TYPE_FIELD,
+        PARSER.declareField(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> p.namedObject(LifecycleType.class, p.text(), null),
-                ValueType.STRING);
+                TYPE_FIELD, ValueType.STRING);
-        PARSER.declareNamedObjects(constructorArg(), (p, c, n) -> Phase.parse(p, n), v -> {
+        PARSER.declareNamedObjects(ConstructingObjectParser.constructorArg(), (p, c, n) -> Phase.parse(p, n), v -> {
             throw new IllegalArgumentException("ordered " + PHASES_FIELD.getPreferredName() + " are not supported");
         }, PHASES_FIELD);
     }
@@ -73,10 +71,14 @@ public class LifecyclePolicy extends AbstractDiffable<LifecyclePolicy>
      *            {@link LifecyclePolicy}.
      */
     public LifecyclePolicy(LifecycleType type, String name, Map<String, Phase> phases) {
-        this.type = type;
+        if (type == null) {
+            this.type = TimeseriesLifecycleType.INSTANCE;
+        } else {
+            this.type = type;
+        }
         this.name = name;
         this.phases = phases;
-        type.validate(phases.values());
+        this.type.validate(phases.values());
     }
 
     /**
@@ -106,6 +108,13 @@ public class LifecyclePolicy extends AbstractDiffable<LifecyclePolicy>
         return name;
     }
 
+    /**
+     * @return the type of this {@link LifecyclePolicy}
+     */
+    public LifecycleType getType() {
+        return type;
+    }
+
     /**
      * @return the {@link Phase}s for this {@link LifecyclePolicy} in the order
      *         in which they will be executed.

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

@@ -45,7 +45,7 @@ public class Phase implements ToXContentObject, Writeable {
             (a, name) -> new Phase(name, (TimeValue) a[0],
         convertListToMapValues(LifecycleAction::getWriteableName, (List<LifecycleAction>) a[1])));
     static {
-        PARSER.declareField(ConstructingObjectParser.constructorArg(),
+        PARSER.declareField(ConstructingObjectParser.optionalConstructorArg(),
                 (p, c) -> TimeValue.parseTimeValue(p.text(), AFTER_FIELD.getPreferredName()), AFTER_FIELD, ValueType.VALUE);
         PARSER.declareNamedObjects(ConstructingObjectParser.constructorArg(),
                 (p, c, n) -> p.namedObject(LifecycleAction.class, n, null), v -> {
@@ -75,7 +75,11 @@ public class Phase implements ToXContentObject, Writeable {
      */
     public Phase(String name, TimeValue after, Map<String, LifecycleAction> actions) {
         this.name = name;
-        this.after = after;
+        if (after == null) {
+            this.after = TimeValue.ZERO;
+        } else {
+            this.after = after;
+        }
         this.actions = actions;
     }
 

x-pack/plugin/index-lifecycle/src/test/java/org/elasticsearch/xpack/indexlifecycle/AllocateActionTests.java

@@ -34,6 +34,7 @@ import org.elasticsearch.test.AbstractSerializingTestCase;
 import org.elasticsearch.xpack.indexlifecycle.LifecycleAction.Listener;
 
 import java.io.IOException;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.function.BiConsumer;
@@ -49,9 +50,27 @@ public class AllocateActionTests extends AbstractSerializingTestCase<AllocateAct
 
     @Override
     protected AllocateAction createTestInstance() {
-        Map<String, String> includes = randomMap(0, 100);
+        boolean hasAtLeastOneMap = false;
-        Map<String, String> excludes = randomMap(0, 100);
+        Map<String, String> includes;
-        Map<String, String> requires = randomMap(0, 100);
+        if (randomBoolean()) {
+            includes = randomMap(0, 100);
+            hasAtLeastOneMap = true;
+        } else {
+            includes = randomBoolean() ? null : Collections.emptyMap();
+        }
+        Map<String, String> excludes;
+        if (randomBoolean()) {
+            hasAtLeastOneMap = true;
+            excludes = randomMap(0, 100);
+        } else {
+            excludes = randomBoolean() ? null : Collections.emptyMap();
+        }
+        Map<String, String> requires;
+        if (hasAtLeastOneMap == false || randomBoolean()) {
+            requires = randomMap(0, 100);
+        } else {
+            requires = randomBoolean() ? null : Collections.emptyMap();
+        }
         return new AllocateAction(includes, excludes, requires);
     }
 
@@ -84,6 +103,17 @@ public class AllocateActionTests extends AbstractSerializingTestCase<AllocateAct
         return new AllocateAction(include, exclude, require);
     }
 
+    public void testAllMapsNullOrEmpty() {
+        Map<String, String> include = randomBoolean() ? null : Collections.emptyMap();
+        Map<String, String> exclude = randomBoolean() ? null : Collections.emptyMap();
+        Map<String, String> require = randomBoolean() ? null : Collections.emptyMap();
+        IllegalArgumentException exception = expectThrows(IllegalArgumentException.class,
+                () -> new AllocateAction(include, exclude, require));
+        assertEquals("At least one of " + AllocateAction.INCLUDE_FIELD.getPreferredName() + ", "
+                + AllocateAction.EXCLUDE_FIELD.getPreferredName() + " or " + AllocateAction.REQUIRE_FIELD.getPreferredName()
+                + "must contain attributes for action " + AllocateAction.NAME, exception.getMessage());
+    }
+
     public void testExecuteNoExistingSettings() throws Exception {
         Map<String, String> includes = randomMap(1, 5);
         Map<String, String> excludes = randomMap(1, 5);

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

@@ -98,6 +98,11 @@ public class LifecyclePolicyTests extends AbstractSerializingTestCase<LifecycleP
         return LifecyclePolicy::new;
     }
 
+    public void testDefaultLifecycleType() {
+        LifecyclePolicy policy = new LifecyclePolicy(null, randomAlphaOfLength(10), Collections.emptyMap());
+        assertSame(TimeseriesLifecycleType.INSTANCE, policy.getType());
+    }
+
     @Before
     public void setupPolicy() {
         indexName = randomAlphaOfLengthBetween(1, 20);

x-pack/plugin/index-lifecycle/src/test/java/org/elasticsearch/xpack/indexlifecycle/PhaseTests.java

@@ -34,7 +34,10 @@ public class PhaseTests extends AbstractSerializingTestCase<Phase> {
 
     @Override
     protected Phase createTestInstance() {
-        TimeValue after = TimeValue.parseTimeValue(randomTimeValue(0, 1000000000, "s", "m", "h", "d"), "test_after");
+        TimeValue after = null;
+        if (randomBoolean()) {
+            after = TimeValue.parseTimeValue(randomTimeValue(0, 1000000000, "s", "m", "h", "d"), "test_after");
+        }
         Map<String, LifecycleAction> actions = Collections.emptyMap();
         if (randomBoolean()) {
             actions = Collections.singletonMap(DeleteAction.NAME, new DeleteAction());
@@ -86,6 +89,11 @@ public class PhaseTests extends AbstractSerializingTestCase<Phase> {
         return new Phase(name, after, actions);
     }
 
+    public void testDefaultAfter() {
+        Phase phase = new Phase(randomAlphaOfLength(20), null, Collections.emptyMap());
+        assertEquals(TimeValue.ZERO, phase.getAfter());
+    }
+
     public void testExecuteNewIndexCompleteActions() throws Exception {
         String indexName = randomAlphaOfLengthBetween(1, 20);
         String phaseName = randomAlphaOfLengthBetween(1, 20);

x-pack/plugin/index-lifecycle/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeseriesLifecycleTypeTests.java

@@ -5,7 +5,6 @@
  */
 package org.elasticsearch.xpack.indexlifecycle;
 
-import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.test.ESTestCase;