diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java index e1837978541a2..7b5cd1b245d51 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java @@ -6,6 +6,8 @@ package org.elasticsearch.xpack.core.snapshotlifecycle; +import org.elasticsearch.ExceptionsHelper; +import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.cluster.AbstractDiffable; @@ -16,7 +18,6 @@ import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.UUIDs; -import org.elasticsearch.common.ValidationException; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -24,14 +25,18 @@ import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.xpack.core.scheduler.Cron; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Objects; +import static org.elasticsearch.cluster.metadata.MetaDataCreateIndexService.MAX_INDEX_NAME_BYTES; + /** * A {@code SnapshotLifecyclePolicy} is a policy for the cluster including a schedule of when a * snapshot should be triggered, what the snapshot should be named, what repository it should go @@ -108,9 +113,64 @@ public Map getConfig() { return this.configuration; } - public ValidationException validate() { - // TODO: implement validation - return null; + public ActionRequestValidationException validate() { + ActionRequestValidationException err = new ActionRequestValidationException(); + + // ID validation + if (id.contains(",")) { + err.addValidationError("invalid policy id [" + id + "]: must not contain ','"); + } + if (id.contains(" ")) { + err.addValidationError("invalid policy id [" + id + "]: must not contain spaces"); + } + if (id.charAt(0) == '_') { + err.addValidationError("invalid policy id [" + id + "]: must not start with '_'"); + } + int byteCount = id.getBytes(StandardCharsets.UTF_8).length; + if (byteCount > MAX_INDEX_NAME_BYTES) { + err.addValidationError("invalid policy id [" + id + "]: name is too long, (" + byteCount + " > " + + MAX_INDEX_NAME_BYTES + " bytes)"); + } + + // Snapshot name validation + // We generate a snapshot name here to make sure it validates after applying date math + final String snapshotName = generateSnapshotName(new ResolverContext()); + if (Strings.hasText(name) == false) { + err.addValidationError("invalid snapshot name [" + name + "]: cannot be empty"); + } + if (snapshotName.contains("#")) { + err.addValidationError("invalid snapshot name [" + name + "]: must not contain '#'"); + } + if (snapshotName.charAt(0) == '_') { + err.addValidationError("invalid snapshot name [" + name + "]: must not start with '_'"); + } + if (snapshotName.toLowerCase(Locale.ROOT).equals(snapshotName) == false) { + err.addValidationError("invalid snapshot name [" + name + "]: must be lowercase"); + } + if (Strings.validFileName(snapshotName) == false) { + err.addValidationError("invalid snapshot name [" + name + "]: must not contain contain the following characters " + + Strings.INVALID_FILENAME_CHARS); + } + + // Schedule validation + if (Strings.hasText(schedule) == false) { + err.addValidationError("invalid schedule [" + schedule + "]: must not be empty"); + } else { + try { + new Cron(schedule); + } catch (IllegalArgumentException e) { + err.addValidationError("invalid schedule: " + + ExceptionsHelper.unwrapCause(e).getMessage()); + } + } + + // Repository validation, validation of whether the repository actually exists happens + // elsewhere as it requires cluster state + if (Strings.hasText(repository) == false) { + err.addValidationError("invalid repository name [" + repository + "]: cannot be empty"); + } + + return err.validationErrors().size() == 0 ? null : err; } /** diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/action/PutSnapshotLifecycleAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/action/PutSnapshotLifecycleAction.java index 5b38a5666efb4..d5387ad1ba7e0 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/action/PutSnapshotLifecycleAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/action/PutSnapshotLifecycleAction.java @@ -80,7 +80,7 @@ public void writeTo(StreamOutput out) throws IOException { @Override public ActionRequestValidationException validate() { - return null; + return lifecycle.validate(); } @Override diff --git a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java index e08b4cfdc41b5..b91ec81b277c5 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java @@ -6,9 +6,11 @@ package org.elasticsearch.xpack.snapshotlifecycle; +import org.apache.http.util.EntityUtils; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; +import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.RestClient; import org.elasticsearch.common.Strings; import org.elasticsearch.common.xcontent.ToXContent; @@ -28,12 +30,29 @@ import java.util.Map; 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.startsWith; public class SnapshotLifecycleIT extends ESRestTestCase { + public void testMissingRepo() throws Exception { + SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy("test-policy", "snap", + "*/1 * * * * ?", "missing-repo", Collections.emptyMap()); + + Request putLifecycle = new Request("PUT", "/_ilm/snapshot/test-policy"); + XContentBuilder lifecycleBuilder = JsonXContent.contentBuilder(); + policy.toXContent(lifecycleBuilder, ToXContent.EMPTY_PARAMS); + putLifecycle.setJsonEntity(Strings.toString(lifecycleBuilder)); + ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(putLifecycle)); + Response resp = e.getResponse(); + assertThat(resp.getStatusLine().getStatusCode(), equalTo(400)); + String jsonError = EntityUtils.toString(resp.getEntity()); + assertThat(jsonError, containsString("\"type\":\"illegal_argument_exception\"")); + assertThat(jsonError, containsString("\"reason\":\"no such repository [missing-repo]\"")); + } + @SuppressWarnings("unchecked") public void testFullPolicySnapshot() throws Exception { final String IDX = "test"; diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleService.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleService.java index 40d84d55450dc..798d6e2b14337 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleService.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleService.java @@ -12,6 +12,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateListener; import org.elasticsearch.cluster.LocalNodeMasterListener; +import org.elasticsearch.cluster.metadata.RepositoriesMetaData; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ConcurrentCollections; @@ -25,6 +26,7 @@ import java.io.Closeable; import java.time.Clock; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.function.Supplier; import java.util.regex.Pattern; @@ -176,6 +178,16 @@ public void cancelScheduledSnapshot(final String lifecycleJobId) { scheduler.remove(lifecycleJobId); } + /** + * Validates that the {@code repository} exists as a registered snapshot repository + * @throws IllegalArgumentException if the repository does not exist + */ + public static void validateRepositoryExists(final String repository, final ClusterState state) { + Optional.ofNullable((RepositoriesMetaData) state.metaData().custom(RepositoriesMetaData.TYPE)) + .map(repoMeta -> repoMeta.repository(repository)) + .orElseThrow(() -> new IllegalArgumentException("no such repository [" + repository + "]")); + } + @Override public String executorName() { return ThreadPool.Names.SNAPSHOT; diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportPutSnapshotLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportPutSnapshotLifecycleAction.java index 01fd69ae9abbd..f87a287e0aea2 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportPutSnapshotLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/snapshotlifecycle/action/TransportPutSnapshotLifecycleAction.java @@ -27,6 +27,7 @@ import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecycleMetadata; import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicyMetadata; import org.elasticsearch.xpack.core.snapshotlifecycle.action.PutSnapshotLifecycleAction; +import org.elasticsearch.xpack.snapshotlifecycle.SnapshotLifecycleService; import java.io.IOException; import java.time.Instant; @@ -65,6 +66,8 @@ protected PutSnapshotLifecycleAction.Response read(StreamInput in) throws IOExce protected void masterOperation(final PutSnapshotLifecycleAction.Request request, final ClusterState state, final ActionListener listener) { + SnapshotLifecycleService.validateRepositoryExists(request.getLifecycle().getRepository(), 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 diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecyclePolicyTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecyclePolicyTests.java index c5fedb7403ebd..3d1b6e924481c 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecyclePolicyTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecyclePolicyTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.snapshotlifecycle; +import org.elasticsearch.common.ValidationException; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.AbstractSerializingTestCase; @@ -16,6 +17,7 @@ import java.util.HashMap; import java.util.Map; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.startsWith; @@ -41,6 +43,28 @@ public void testNameGeneration() { assertThat(p.generateSnapshotName(context), startsWith("name-2019-03-15.21:09:00-")); } + public void testValidation() { + SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy("a,b", "", + "* * * * * L", " ", Collections.emptyMap()); + + ValidationException e = policy.validate(); + assertThat(e.validationErrors(), + containsInAnyOrder("invalid policy id [a,b]: must not contain ','", + "invalid snapshot name []: 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()); + + 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")); + } + @Override protected SnapshotLifecyclePolicy doParseInstance(XContentParser parser) throws IOException { return SnapshotLifecyclePolicy.parse(parser, id); diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleServiceTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleServiceTests.java index 8fc6ecdc29742..78436c6413722 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleServiceTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleServiceTests.java @@ -10,6 +10,8 @@ import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.metadata.RepositoriesMetaData; +import org.elasticsearch.cluster.metadata.RepositoryMetaData; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.ClusterServiceUtils; @@ -32,6 +34,7 @@ import java.util.function.Consumer; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; @@ -45,6 +48,33 @@ public void testGetJobId() { assertThat(SnapshotLifecycleService.getJobId(meta), equalTo(id + "-" + version)); } + public void testRepositoryExistenceForExistingRepo() { + ClusterState state = ClusterState.builder(new ClusterName("cluster")).build(); + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> SnapshotLifecycleService.validateRepositoryExists("repo", state)); + + assertThat(e.getMessage(), containsString("no such repository [repo]")); + + RepositoryMetaData repo = new RepositoryMetaData("repo", "fs", Settings.EMPTY); + RepositoriesMetaData repoMeta = new RepositoriesMetaData(Collections.singletonList(repo)); + ClusterState stateWithRepo = ClusterState.builder(state) + .metaData(MetaData.builder() + .putCustom(RepositoriesMetaData.TYPE, repoMeta)) + .build(); + + SnapshotLifecycleService.validateRepositoryExists("repo", stateWithRepo); + } + + public void testRepositoryExistenceForMissingRepo() { + ClusterState state = ClusterState.builder(new ClusterName("cluster")).build(); + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> SnapshotLifecycleService.validateRepositoryExists("repo", state)); + + assertThat(e.getMessage(), containsString("no such repository [repo]")); + } + /** * Test new policies getting scheduled correctly, updated policies also being scheduled, * and deleted policies having their schedules cancelled.