Add a validation rule to _rollover requests

joegallo · joegallo · commit 6658822fdfee · 2022-07-22T15:17:59.000-04:00
Unlike with an ILM rollover action, a max_* condition is not strictly
required -- an unconditional rollover is valid.

However, if any conditions are present, then at least one of them must
be a max_* condition.
diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.rollover/10_basic.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.rollover/10_basic.yml
@@ -174,14 +174,12 @@
 
   # perform alias rollover with no result
   - do:
+      catch: bad_request
       indices.rollover:
         alias: "logs_search"
         wait_for_active_shards: 1
         body:
           conditions:
             min_age: "0s"
             min_docs: 1
-
-  - match: { rolled_over: false }
-  - match: { dry_run: false }
-  - match: { conditions: { "[min_age: 0s]": true, "[min_docs: 1]": true } }
+  - match: { error.reason: "Validation Failed: 1: at least one max_* rollover condition must be set when using min_* conditions;" }
diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverRequest.java
@@ -193,6 +193,16 @@ public ActionRequestValidationException validate() {
         if (rolloverTarget == null) {
             validationException = addValidationError("rollover target is missing", validationException);
         }
+
+        // max_* conditions are the isRequired = false ones -- the request must have at least one max_* condition
+        boolean noMaxConditions = conditions.values().stream().noneMatch(Predicate.not(Condition::isRequired));
+        if (conditions.size() > 0 && noMaxConditions) {
+            validationException = addValidationError(
+                "at least one max_* rollover condition must be set when using min_* conditions",
+                validationException
+            );
+        }
+
         return validationException;
     }
 
diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/rollover/RolloverRequestTests.java
@@ -209,12 +209,36 @@ public void testSameConditionCanOnlyBeAddedOnce() {
     }
 
     public void testValidation() {
-        RolloverRequest rolloverRequest = new RolloverRequest();
-        assertNotNull(rolloverRequest.getCreateIndexRequest());
-        ActionRequestValidationException validationException = rolloverRequest.validate();
-        assertNotNull(validationException);
-        assertEquals(1, validationException.validationErrors().size());
-        assertEquals("rollover target is missing", validationException.validationErrors().get(0));
+        {
+            RolloverRequest rolloverRequest = new RolloverRequest();
+            assertNotNull(rolloverRequest.getCreateIndexRequest());
+            ActionRequestValidationException validationException = rolloverRequest.validate();
+            assertNotNull(validationException);
+            assertEquals(1, validationException.validationErrors().size());
+            assertEquals("rollover target is missing", validationException.validationErrors().get(0));
+        }
+
+        {
+            RolloverRequest rolloverRequest = new RolloverRequest("alias-index", "new-index-name");
+            rolloverRequest.addMinIndexDocsCondition(1L);
+            ActionRequestValidationException validationException = rolloverRequest.validate();
+            assertNotNull(validationException);
+            assertEquals(1, validationException.validationErrors().size());
+            assertEquals(
+                "at least one max_* rollover condition must be set when using min_* conditions",
+                validationException.validationErrors().get(0)
+            );
+        }
+
+        {
+            RolloverRequest rolloverRequest = new RolloverRequest("alias-index", "new-index-name");
+            if (randomBoolean()) {
+                rolloverRequest.addMaxIndexAgeCondition(TimeValue.timeValueHours(1));
+                rolloverRequest.addMinIndexDocsCondition(1L);
+            }
+            ActionRequestValidationException validationException = rolloverRequest.validate();
+            assertNull(validationException);
+        }
     }
 
     public void testParsingWithType() throws Exception {
