Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate snapshot lifecycle policies #40654

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -16,22 +18,25 @@
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;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
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
Expand Down Expand Up @@ -108,9 +113,64 @@ public Map<String, Object> getConfig() {
return this.configuration;
}

public ValidationException validate() {
// TODO: implement validation
return null;
public ActionRequestValidationException validate() {
ActionRequestValidationException err = new ActionRequestValidationException();

// ID validation
dakrone marked this conversation as resolved.
Show resolved Hide resolved
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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public void writeTo(StreamOutput out) throws IOException {

@Override
public ActionRequestValidationException validate() {
return null;
return lifecycle.validate();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
dakrone marked this conversation as resolved.
Show resolved Hide resolved
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -65,6 +66,8 @@ protected PutSnapshotLifecycleAction.Response read(StreamInput in) throws IOExce
protected void masterOperation(final PutSnapshotLifecycleAction.Request request,
final ClusterState state,
final ActionListener<PutSnapshotLifecycleAction.Response> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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", "<my, snapshot-{now/M}>",
"* * * * * L", " ", Collections.emptyMap());

ValidationException e = policy.validate();
assertThat(e.validationErrors(),
containsInAnyOrder("invalid policy id [a,b]: must not contain ','",
"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());

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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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.
Expand Down