Add validation of the SLM schedule frequency (#64452)

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecycleSettings.java

@@ -25,6 +25,7 @@ public class LifecycleSettings {
     public static final String SLM_HISTORY_INDEX_ENABLED = "slm.history_index_enabled";
     public static final String SLM_RETENTION_SCHEDULE = "slm.retention_schedule";
     public static final String SLM_RETENTION_DURATION = "slm.retention_duration";
+    public static final String SLM_MINIMUM_INTERVAL = "slm.minimum_interval";
 
     public static final Setting<TimeValue> LIFECYCLE_POLL_INTERVAL_SETTING = Setting.timeSetting(LIFECYCLE_POLL_INTERVAL,
         TimeValue.timeValueMinutes(10), TimeValue.timeValueSeconds(1), Setting.Property.Dynamic, Setting.Property.NodeScope);
@@ -60,4 +61,7 @@ public class LifecycleSettings {
     }, Setting.Property.Dynamic, Setting.Property.NodeScope);
     public static final Setting<TimeValue> SLM_RETENTION_DURATION_SETTING = Setting.timeSetting(SLM_RETENTION_DURATION,
         TimeValue.timeValueHours(1), TimeValue.timeValueMillis(500), Setting.Property.Dynamic, Setting.Property.NodeScope);
+    public static final Setting<TimeValue> SLM_MINIMUM_INTERVAL_SETTING =
+        Setting.positiveTimeSetting(SLM_MINIMUM_INTERVAL, TimeValue.timeValueMinutes(15), Setting.Property.Dynamic,
+            Setting.Property.NodeScope);
 }

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicy.java

@@ -18,6 +18,7 @@ import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.xcontent.ConstructingObjectParser;
 import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -133,6 +134,26 @@ public class SnapshotLifecyclePolicy extends AbstractDiffable<SnapshotLifecycleP
         return schedule.getNextValidTimeAfter(System.currentTimeMillis());
     }
 
+    /**
+     * Calculate the difference between the next two valid times after now for the schedule.
+     * <p>
+     * In ordinary cases, this can be treated as the interval between executions of the schedule (for schedules like 'twice an hour' or
+     * 'every five minutes').
+     *
+     * @return a {@link TimeValue} representing the difference between the next two valid times after now, or {@link TimeValue#MINUS_ONE}
+     *         if either of the next two times after now is unsupported according to @{@link Cron#getNextValidTimeAfter(long)}
+     */
+    public TimeValue calculateNextInterval() {
+        final Cron schedule = new Cron(this.schedule);
+        long next1 = schedule.getNextValidTimeAfter(System.currentTimeMillis());
+        long next2 = schedule.getNextValidTimeAfter(next1);
+        if (next1 > 0 && next2 > 0) {
+            return TimeValue.timeValueMillis(next2 - next1);
+        } else {
+            return TimeValue.MINUS_ONE;
+        }
+    }
+
     public ActionRequestValidationException validate() {
         ActionRequestValidationException err = new ActionRequestValidationException();
 

x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/slm/SnapshotLifecycleRestIT.java

@@ -81,7 +81,7 @@ public class SnapshotLifecycleRestIT extends ESRestTestCase {
 
     public void testMissingRepo() throws Exception {
         SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy("missing-repo-policy", "snap",
-            "*/1 * * * * ?", "missing-repo", Collections.emptyMap(), SnapshotRetentionConfiguration.EMPTY);
+            "0 0/15 * * * ?", "missing-repo", Collections.emptyMap(), SnapshotRetentionConfiguration.EMPTY);
 
         Request putLifecycle = new Request("PUT", "/_slm/policy/missing-repo-policy");
         XContentBuilder lifecycleBuilder = JsonXContent.contentBuilder();
@@ -184,7 +184,18 @@ public class SnapshotLifecycleRestIT extends ESRestTestCase {
         final String indexPattern = "index-doesnt-exist";
         initializeRepo(repoName);
 
-        // Create a policy with ignore_unvailable: false and an index that doesn't exist
+        // allow arbitrarily frequent slm snapshots
+        ClusterUpdateSettingsRequest req = new ClusterUpdateSettingsRequest();
+        req.transientSettings(Settings.builder().put(LifecycleSettings.SLM_MINIMUM_INTERVAL, "0s"));
+        try (XContentBuilder builder = jsonBuilder()) {
+            req.toXContent(builder, ToXContent.EMPTY_PARAMS);
+            Request r = new Request("PUT", "/_cluster/settings");
+            r.setJsonEntity(Strings.toString(builder));
+            Response updateSettingsResp = client().performRequest(r);
+            assertAcked(updateSettingsResp);
+        }
+
+        // Create a policy with ignore_unavailable: false and an index that doesn't exist
         createSnapshotPolicy(policyName, "snap", "*/1 * * * * ?", repoName, indexPattern, false);
 
         assertBusy(() -> {
@@ -302,7 +313,7 @@ public class SnapshotLifecycleRestIT extends ESRestTestCase {
         });
 
         try {
-            createSnapshotPolicy(policyName, "snap", "*/1 * * * * ?", repoId, indexName, true,
+            createSnapshotPolicy(policyName, "snap", "0 0/15 * * * ?", repoId, indexName, true,
                 new SnapshotRetentionConfiguration(TimeValue.ZERO, null, null));
             long start = System.currentTimeMillis();
             final String snapshotName = executePolicy(policyName);

x-pack/plugin/ilm/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/ilm/11_basic_slm.yml

@@ -24,6 +24,20 @@ setup:
           settings:
             location: "my-snaps"
 
+  - do:
+      catch: /invalid schedule \[0 \* \* \* \* \?\][:] schedule would be too frequent, executing more than every \[15m\]/
+      slm.put_lifecycle:
+        policy_id: "daily-snapshots"
+        body: |
+          {
+            "schedule": "0 * * * * ?",
+            "name": "<production-snap-{now/d}>",
+            "repository": "repo",
+            "config": {
+              "indices": ["*"]
+            }
+          }
+
   - do:
       slm.put_lifecycle:
         policy_id: "daily-snapshots"

x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/slm/SnapshotLifecycleInitialisationTests.java

@@ -65,7 +65,7 @@ public class SnapshotLifecycleInitialisationTests extends ESSingleNodeTestCase {
 
         client().execute(PutSnapshotLifecycleAction.INSTANCE,
             new Request("snapshot-policy", new SnapshotLifecyclePolicy("test-policy", "snap",
-                "*/1 * * * * ?", "repo", Collections.emptyMap(), SnapshotRetentionConfiguration.EMPTY))
+                "0 0/15 * * * ?", "repo", Collections.emptyMap(), SnapshotRetentionConfiguration.EMPTY))
         ).get(10, TimeUnit.SECONDS);
 
         ClusterState state = getInstanceFromNode(ClusterService.class).state();

x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycle.java

@@ -165,7 +165,8 @@ public class IndexLifecycle extends Plugin implements ActionPlugin {
             RolloverAction.LIFECYCLE_ROLLOVER_ALIAS_SETTING,
             LifecycleSettings.SLM_HISTORY_INDEX_ENABLED_SETTING,
             LifecycleSettings.SLM_RETENTION_SCHEDULE_SETTING,
-            LifecycleSettings.SLM_RETENTION_DURATION_SETTING);
+            LifecycleSettings.SLM_RETENTION_DURATION_SETTING,
+            LifecycleSettings.SLM_MINIMUM_INTERVAL_SETTING);
     }
 
     @Override

x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/SnapshotLifecycleService.java

@@ -14,7 +14,9 @@ import org.elasticsearch.cluster.ClusterStateListener;
 import org.elasticsearch.cluster.metadata.RepositoriesMetadata;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
+import org.elasticsearch.xpack.core.ilm.LifecycleSettings;
 import org.elasticsearch.xpack.core.ilm.OperationMode;
 import org.elasticsearch.xpack.core.scheduler.CronSchedule;
 import org.elasticsearch.xpack.core.scheduler.SchedulerEngine;
@@ -234,6 +236,20 @@ public class SnapshotLifecycleService implements Closeable, ClusterStateListener
             .orElseThrow(() -> new IllegalArgumentException("no such repository [" + repository + "]"));
     }
 
+    /**
+     * Validates that the interval between snapshots is not smaller than the minimum interval
+     * (see {@link LifecycleSettings#SLM_MINIMUM_INTERVAL_SETTING})
+     * @throws IllegalArgumentException if the interval is less than the minimum
+     */
+    public static void validateMinimumInterval(final SnapshotLifecyclePolicy lifecycle, final ClusterState state) {
+        TimeValue minimum = LifecycleSettings.SLM_MINIMUM_INTERVAL_SETTING.get(state.metadata().settings());
+        TimeValue next = lifecycle.calculateNextInterval();
+        if (next.duration() > 0 && minimum.duration() > 0 && next.millis() < minimum.millis()) {
+            throw new IllegalArgumentException("invalid schedule [" + lifecycle.getSchedule() + "]: " +
+                "schedule would be too frequent, executing more than every [" + minimum.getStringRep() + "]");
+        }
+    }
+
     @Override
     public void close() {
         if (this.running.compareAndSet(true, false)) {

x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/slm/action/TransportPutSnapshotLifecycleAction.java

@@ -56,6 +56,8 @@ public class TransportPutSnapshotLifecycleAction extends
                                    final ActionListener<PutSnapshotLifecycleAction.Response> listener) {
         SnapshotLifecycleService.validateRepositoryExists(request.getLifecycle().getRepository(), state);
 
+        SnapshotLifecycleService.validateMinimumInterval(request.getLifecycle(), state);
+
         // headers from the thread context stored by the AuthenticationService to be shared between the
         // REST layer and the Transport layer here must be accessed within this thread and not in the
         // cluster state thread in the ClusterStateUpdateTask below since that thread does not share the

x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecyclePolicyTests.java

@@ -9,6 +9,7 @@ package org.elasticsearch.xpack.slm;
 import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest;
 import org.elasticsearch.common.ValidationException;
 import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.test.AbstractSerializingTestCase;
 import org.elasticsearch.xpack.core.slm.SnapshotLifecyclePolicy;
@@ -24,6 +25,7 @@ import static org.elasticsearch.xpack.core.slm.SnapshotLifecyclePolicyMetadataTe
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.nullValue;
 
 public class SnapshotLifecyclePolicyTests extends AbstractSerializingTestCase<SnapshotLifecyclePolicy> {
 
@@ -46,27 +48,58 @@ public class SnapshotLifecyclePolicyTests extends AbstractSerializingTestCase<Sn
         assertThat(p.calculateNextExecution(), equalTo(4078864860000L));
     }
 
+    public void testCalculateNextInterval() {
+        {
+            SnapshotLifecyclePolicy p = new SnapshotLifecyclePolicy("id", "name", "0 0/5 * * * ?", "repo", Collections.emptyMap(),
+                SnapshotRetentionConfiguration.EMPTY);
+            assertThat(p.calculateNextInterval(), equalTo(TimeValue.timeValueMinutes(5)));
+        }
+
+        {
+            SnapshotLifecyclePolicy p = new SnapshotLifecyclePolicy("id", "name", "0 1 2 3 4 ? 2099", "repo", Collections.emptyMap(),
+                SnapshotRetentionConfiguration.EMPTY);
+            assertThat(p.calculateNextInterval(), equalTo(TimeValue.MINUS_ONE));
+        }
+
+        {
+            SnapshotLifecyclePolicy p = new SnapshotLifecyclePolicy("id", "name", "* * * 31 FEB ? *", "repo", Collections.emptyMap(),
+                SnapshotRetentionConfiguration.EMPTY);
+            assertThat(p.calculateNextInterval(), equalTo(TimeValue.MINUS_ONE));
+        }
+    }
+
     public void testValidation() {
-        SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy("a,b", "<my, snapshot-{now/M}>",
-            "* * * * * L", "  ", Collections.emptyMap(), SnapshotRetentionConfiguration.EMPTY);
-
-        ValidationException e = policy.validate();
-        assertThat(e.validationErrors(),
-            containsInAnyOrder(
-                "invalid policy id [a,b]: must not contain the following characters [ , \", *, \\, <, |, ,, >, /, ?]",
-                "invalid snapshot name [<my, snapshot-{now/M}>]: must not contain contain" +
-                    " the following characters [ , \", *, \\, <, |, ,, >, /, ?]",
-                "invalid repository name [  ]: cannot be empty",
-                "invalid schedule: invalid cron expression [* * * * * L]"));
-
-        policy = new SnapshotLifecyclePolicy("_my_policy", "mySnap",
-            " ", "repo", Collections.emptyMap(), SnapshotRetentionConfiguration.EMPTY);
-
-        e = policy.validate();
-        assertThat(e.validationErrors(),
-            containsInAnyOrder("invalid policy id [_my_policy]: must not start with '_'",
-                "invalid snapshot name [mySnap]: must be lowercase",
-                "invalid schedule [ ]: must not be empty"));
+        {
+            SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy("a,b", "<my, snapshot-{now/M}>",
+                "* * * * * L", "  ", Collections.emptyMap(), SnapshotRetentionConfiguration.EMPTY);
+
+            ValidationException e = policy.validate();
+            assertThat(e.validationErrors(),
+                containsInAnyOrder(
+                    "invalid policy id [a,b]: must not contain the following characters [ , \", *, \\, <, |, ,, >, /, ?]",
+                    "invalid snapshot name [<my, snapshot-{now/M}>]: must not contain contain" +
+                        " the following characters [ , \", *, \\, <, |, ,, >, /, ?]",
+                    "invalid repository name [  ]: cannot be empty",
+                    "invalid schedule: invalid cron expression [* * * * * L]"));
+        }
+
+        {
+            SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy("_my_policy", "mySnap",
+                " ", "repo", Collections.emptyMap(), SnapshotRetentionConfiguration.EMPTY);
+
+            ValidationException e = policy.validate();
+            assertThat(e.validationErrors(),
+                containsInAnyOrder("invalid policy id [_my_policy]: must not start with '_'",
+                    "invalid snapshot name [mySnap]: must be lowercase",
+                    "invalid schedule [ ]: must not be empty"));
+        }
+
+        {
+            SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy("my_policy", "my_snap",
+                "0 0/30 * * * ?", "repo", Collections.emptyMap(), SnapshotRetentionConfiguration.EMPTY);
+            ValidationException e = policy.validate();
+            assertThat(e, nullValue());
+        }
     }
 
     public void testMetadataValidation() {

x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotLifecycleServiceTests.java

@@ -18,10 +18,12 @@ import org.elasticsearch.cluster.node.DiscoveryNodes;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.transport.TransportAddress;
+import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.test.ClusterServiceUtils;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.threadpool.TestThreadPool;
 import org.elasticsearch.threadpool.ThreadPool;
+import org.elasticsearch.xpack.core.ilm.LifecycleSettings;
 import org.elasticsearch.xpack.core.ilm.OperationMode;
 import org.elasticsearch.xpack.core.scheduler.SchedulerEngine;
 import org.elasticsearch.xpack.core.slm.SnapshotLifecycleMetadata;
@@ -341,6 +343,50 @@ public class SnapshotLifecycleServiceTests extends ESTestCase {
         }
     }
 
+    public void testValidateMinimumInterval() {
+        ClusterState defaultState = ClusterState.builder(new ClusterName("cluster")).build();
+
+        ClusterState validationOneMinuteState = ClusterState.builder(new ClusterName("cluster"))
+            .metadata(Metadata.builder().persistentSettings(
+                Settings.builder()
+                    .put(defaultState.metadata().persistentSettings())
+                    .put(LifecycleSettings.SLM_MINIMUM_INTERVAL, TimeValue.timeValueMinutes(1))
+                    .build()))
+            .build();
+
+        ClusterState validationDisabledState = ClusterState.builder(new ClusterName("cluster"))
+            .metadata(Metadata.builder().persistentSettings(
+                Settings.builder()
+                    .put(defaultState.metadata().persistentSettings())
+                    .put(LifecycleSettings.SLM_MINIMUM_INTERVAL, TimeValue.ZERO)
+                    .build()))
+            .build();
+
+        for (String schedule : List.of(
+            "0 0/15 * * * ?",
+            "0 0 1 * * ?",
+            "0 0 0 1 1 ? 2099" /* once */,
+            "* * * 31 FEB ? *" /* never */)) {
+            SnapshotLifecycleService.validateMinimumInterval(createPolicy("foo-1", schedule), defaultState);
+            SnapshotLifecycleService.validateMinimumInterval(createPolicy("foo-1", schedule), validationOneMinuteState);
+            SnapshotLifecycleService.validateMinimumInterval(createPolicy("foo-1", schedule), validationDisabledState);
+        }
+
+        IllegalArgumentException e;
+
+        e = expectThrows(IllegalArgumentException.class,
+            () -> SnapshotLifecycleService.validateMinimumInterval(createPolicy("foo-1", "0 0/1 * * * ?"), defaultState));
+        assertThat(e.getMessage(), equalTo("invalid schedule [0 0/1 * * * ?]: " +
+            "schedule would be too frequent, executing more than every [15m]"));
+        SnapshotLifecycleService.validateMinimumInterval(createPolicy("foo-1", "0 0/1 * * * ?"), validationOneMinuteState);
+
+        e = expectThrows(IllegalArgumentException.class,
+            () -> SnapshotLifecycleService.validateMinimumInterval(createPolicy("foo-1", "0/30 0/1 * * * ?"), validationOneMinuteState));
+        assertThat(e.getMessage(), equalTo("invalid schedule [0/30 0/1 * * * ?]: " +
+            "schedule would be too frequent, executing more than every [1m]"));
+        SnapshotLifecycleService.validateMinimumInterval(createPolicy("foo-1", "0/30 0/1 * * * ?"), validationDisabledState);
+    }
+
     class FakeSnapshotTask extends SnapshotLifecycleTask {
         private final Consumer<SchedulerEngine.Event> onTriggered;