Skip to content

Commit a53da7d

Browse files
dakronedakrone
authored
Validate snapshot lifecycle policies (#40654)
(This is a PR against the `snapshot-lifecycle-management` branch) With the commit, we now validate the content of snapshot lifecycle policies when the policy is being created or updated. This checks for the validity of the id, name, schedule, and repository. Additionally, cluster state is checked to ensure that the repository exists prior to the lifecycle being added to the cluster state. Part of #38461
1 parent 51d7f0e commit a53da7d

File tree

7 files changed

+152
-5
lines changed

7 files changed

+152
-5
lines changed

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

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
package org.elasticsearch.xpack.core.snapshotlifecycle;
88

9+
import org.elasticsearch.ExceptionsHelper;
10+
import org.elasticsearch.action.ActionRequestValidationException;
911
import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest;
1012
import org.elasticsearch.action.support.IndicesOptions;
1113
import org.elasticsearch.cluster.AbstractDiffable;
@@ -16,22 +18,25 @@
1618
import org.elasticsearch.common.ParseField;
1719
import org.elasticsearch.common.Strings;
1820
import org.elasticsearch.common.UUIDs;
19-
import org.elasticsearch.common.ValidationException;
2021
import org.elasticsearch.common.io.stream.StreamInput;
2122
import org.elasticsearch.common.io.stream.StreamOutput;
2223
import org.elasticsearch.common.io.stream.Writeable;
2324
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
2425
import org.elasticsearch.common.xcontent.ToXContentObject;
2526
import org.elasticsearch.common.xcontent.XContentBuilder;
2627
import org.elasticsearch.common.xcontent.XContentParser;
28+
import org.elasticsearch.xpack.core.scheduler.Cron;
2729

2830
import java.io.IOException;
31+
import java.nio.charset.StandardCharsets;
2932
import java.util.Collections;
3033
import java.util.List;
3134
import java.util.Locale;
3235
import java.util.Map;
3336
import java.util.Objects;
3437

38+
import static org.elasticsearch.cluster.metadata.MetaDataCreateIndexService.MAX_INDEX_NAME_BYTES;
39+
3540
/**
3641
* A {@code SnapshotLifecyclePolicy} is a policy for the cluster including a schedule of when a
3742
* snapshot should be triggered, what the snapshot should be named, what repository it should go
@@ -108,9 +113,64 @@ public Map<String, Object> getConfig() {
108113
return this.configuration;
109114
}
110115

111-
public ValidationException validate() {
112-
// TODO: implement validation
113-
return null;
116+
public ActionRequestValidationException validate() {
117+
ActionRequestValidationException err = new ActionRequestValidationException();
118+
119+
// ID validation
120+
if (id.contains(",")) {
121+
err.addValidationError("invalid policy id [" + id + "]: must not contain ','");
122+
}
123+
if (id.contains(" ")) {
124+
err.addValidationError("invalid policy id [" + id + "]: must not contain spaces");
125+
}
126+
if (id.charAt(0) == '_') {
127+
err.addValidationError("invalid policy id [" + id + "]: must not start with '_'");
128+
}
129+
int byteCount = id.getBytes(StandardCharsets.UTF_8).length;
130+
if (byteCount > MAX_INDEX_NAME_BYTES) {
131+
err.addValidationError("invalid policy id [" + id + "]: name is too long, (" + byteCount + " > " +
132+
MAX_INDEX_NAME_BYTES + " bytes)");
133+
}
134+
135+
// Snapshot name validation
136+
// We generate a snapshot name here to make sure it validates after applying date math
137+
final String snapshotName = generateSnapshotName(new ResolverContext());
138+
if (Strings.hasText(name) == false) {
139+
err.addValidationError("invalid snapshot name [" + name + "]: cannot be empty");
140+
}
141+
if (snapshotName.contains("#")) {
142+
err.addValidationError("invalid snapshot name [" + name + "]: must not contain '#'");
143+
}
144+
if (snapshotName.charAt(0) == '_') {
145+
err.addValidationError("invalid snapshot name [" + name + "]: must not start with '_'");
146+
}
147+
if (snapshotName.toLowerCase(Locale.ROOT).equals(snapshotName) == false) {
148+
err.addValidationError("invalid snapshot name [" + name + "]: must be lowercase");
149+
}
150+
if (Strings.validFileName(snapshotName) == false) {
151+
err.addValidationError("invalid snapshot name [" + name + "]: must not contain contain the following characters " +
152+
Strings.INVALID_FILENAME_CHARS);
153+
}
154+
155+
// Schedule validation
156+
if (Strings.hasText(schedule) == false) {
157+
err.addValidationError("invalid schedule [" + schedule + "]: must not be empty");
158+
} else {
159+
try {
160+
new Cron(schedule);
161+
} catch (IllegalArgumentException e) {
162+
err.addValidationError("invalid schedule: " +
163+
ExceptionsHelper.unwrapCause(e).getMessage());
164+
}
165+
}
166+
167+
// Repository validation, validation of whether the repository actually exists happens
168+
// elsewhere as it requires cluster state
169+
if (Strings.hasText(repository) == false) {
170+
err.addValidationError("invalid repository name [" + repository + "]: cannot be empty");
171+
}
172+
173+
return err.validationErrors().size() == 0 ? null : err;
114174
}
115175

116176
/**

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/action/PutSnapshotLifecycleAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public void writeTo(StreamOutput out) throws IOException {
8080

8181
@Override
8282
public ActionRequestValidationException validate() {
83-
return null;
83+
return lifecycle.validate();
8484
}
8585

8686
@Override

‎x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecycleIT.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66

77
package org.elasticsearch.xpack.snapshotlifecycle;
88

9+
import org.apache.http.util.EntityUtils;
910
import org.elasticsearch.action.index.IndexRequestBuilder;
1011
import org.elasticsearch.client.Request;
1112
import org.elasticsearch.client.Response;
13+
import org.elasticsearch.client.ResponseException;
1214
import org.elasticsearch.client.RestClient;
1315
import org.elasticsearch.common.Strings;
1416
import org.elasticsearch.common.xcontent.ToXContent;
@@ -35,6 +37,22 @@
3537

3638
public class SnapshotLifecycleIT extends ESRestTestCase {
3739

40+
public void testMissingRepo() throws Exception {
41+
SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy("test-policy", "snap",
42+
"*/1 * * * * ?", "missing-repo", Collections.emptyMap());
43+
44+
Request putLifecycle = new Request("PUT", "/_ilm/snapshot/test-policy");
45+
XContentBuilder lifecycleBuilder = JsonXContent.contentBuilder();
46+
policy.toXContent(lifecycleBuilder, ToXContent.EMPTY_PARAMS);
47+
putLifecycle.setJsonEntity(Strings.toString(lifecycleBuilder));
48+
ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest(putLifecycle));
49+
Response resp = e.getResponse();
50+
assertThat(resp.getStatusLine().getStatusCode(), equalTo(400));
51+
String jsonError = EntityUtils.toString(resp.getEntity());
52+
assertThat(jsonError, containsString("\"type\":\"illegal_argument_exception\""));
53+
assertThat(jsonError, containsString("\"reason\":\"no such repository [missing-repo]\""));
54+
}
55+
3856
@SuppressWarnings("unchecked")
3957
public void testFullPolicySnapshot() throws Exception {
4058
final String indexName = "test";

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.cluster.ClusterState;
1313
import org.elasticsearch.cluster.ClusterStateListener;
1414
import org.elasticsearch.cluster.LocalNodeMasterListener;
15+
import org.elasticsearch.cluster.metadata.RepositoriesMetaData;
1516
import org.elasticsearch.cluster.service.ClusterService;
1617
import org.elasticsearch.common.settings.Settings;
1718
import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
@@ -25,6 +26,7 @@
2526
import java.io.Closeable;
2627
import java.time.Clock;
2728
import java.util.Map;
29+
import java.util.Optional;
2830
import java.util.Set;
2931
import java.util.function.Supplier;
3032
import java.util.regex.Pattern;
@@ -176,6 +178,16 @@ public void cancelScheduledSnapshot(final String lifecycleJobId) {
176178
scheduler.remove(lifecycleJobId);
177179
}
178180

181+
/**
182+
* Validates that the {@code repository} exists as a registered snapshot repository
183+
* @throws IllegalArgumentException if the repository does not exist
184+
*/
185+
public static void validateRepositoryExists(final String repository, final ClusterState state) {
186+
Optional.ofNullable((RepositoriesMetaData) state.metaData().custom(RepositoriesMetaData.TYPE))
187+
.map(repoMeta -> repoMeta.repository(repository))
188+
.orElseThrow(() -> new IllegalArgumentException("no such repository [" + repository + "]"));
189+
}
190+
179191
@Override
180192
public String executorName() {
181193
return ThreadPool.Names.SNAPSHOT;

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecycleMetadata;
2828
import org.elasticsearch.xpack.core.snapshotlifecycle.SnapshotLifecyclePolicyMetadata;
2929
import org.elasticsearch.xpack.core.snapshotlifecycle.action.PutSnapshotLifecycleAction;
30+
import org.elasticsearch.xpack.snapshotlifecycle.SnapshotLifecycleService;
3031

3132
import java.io.IOException;
3233
import java.time.Instant;
@@ -65,6 +66,8 @@ protected PutSnapshotLifecycleAction.Response read(StreamInput in) throws IOExce
6566
protected void masterOperation(final PutSnapshotLifecycleAction.Request request,
6667
final ClusterState state,
6768
final ActionListener<PutSnapshotLifecycleAction.Response> listener) {
69+
SnapshotLifecycleService.validateRepositoryExists(request.getLifecycle().getRepository(), state);
70+
6871
// headers from the thread context stored by the AuthenticationService to be shared between the
6972
// REST layer and the Transport layer here must be accessed within this thread and not in the
7073
// cluster state thread in the ClusterStateUpdateTask below since that thread does not share the

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
package org.elasticsearch.xpack.snapshotlifecycle;
88

9+
import org.elasticsearch.common.ValidationException;
910
import org.elasticsearch.common.io.stream.Writeable;
1011
import org.elasticsearch.common.xcontent.XContentParser;
1112
import org.elasticsearch.test.AbstractSerializingTestCase;
@@ -16,6 +17,7 @@
1617
import java.util.HashMap;
1718
import java.util.Map;
1819

20+
import static org.hamcrest.Matchers.containsInAnyOrder;
1921
import static org.hamcrest.Matchers.greaterThan;
2022
import static org.hamcrest.Matchers.startsWith;
2123

@@ -41,6 +43,28 @@ public void testNameGeneration() {
4143
assertThat(p.generateSnapshotName(context), startsWith("name-2019-03-15.21:09:00-"));
4244
}
4345

46+
public void testValidation() {
47+
SnapshotLifecyclePolicy policy = new SnapshotLifecyclePolicy("a,b", "<my, snapshot-{now/M}>",
48+
"* * * * * L", " ", Collections.emptyMap());
49+
50+
ValidationException e = policy.validate();
51+
assertThat(e.validationErrors(),
52+
containsInAnyOrder("invalid policy id [a,b]: must not contain ','",
53+
"invalid snapshot name [<my, snapshot-{now/M}>]: must not contain contain" +
54+
" the following characters [ , \", *, \\, <, |, ,, >, /, ?]",
55+
"invalid repository name [ ]: cannot be empty",
56+
"invalid schedule: invalid cron expression [* * * * * L]"));
57+
58+
policy = new SnapshotLifecyclePolicy("_my_policy", "mySnap",
59+
" ", "repo", Collections.emptyMap());
60+
61+
e = policy.validate();
62+
assertThat(e.validationErrors(),
63+
containsInAnyOrder("invalid policy id [_my_policy]: must not start with '_'",
64+
"invalid snapshot name [mySnap]: must be lowercase",
65+
"invalid schedule [ ]: must not be empty"));
66+
}
67+
4468
@Override
4569
protected SnapshotLifecyclePolicy doParseInstance(XContentParser parser) throws IOException {
4670
return SnapshotLifecyclePolicy.parse(parser, id);

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
import org.elasticsearch.cluster.ClusterName;
1111
import org.elasticsearch.cluster.ClusterState;
1212
import org.elasticsearch.cluster.metadata.MetaData;
13+
import org.elasticsearch.cluster.metadata.RepositoriesMetaData;
14+
import org.elasticsearch.cluster.metadata.RepositoryMetaData;
1315
import org.elasticsearch.cluster.service.ClusterService;
1416
import org.elasticsearch.common.settings.Settings;
1517
import org.elasticsearch.test.ClusterServiceUtils;
@@ -32,6 +34,7 @@
3234
import java.util.function.Consumer;
3335

3436
import static org.hamcrest.Matchers.containsInAnyOrder;
37+
import static org.hamcrest.Matchers.containsString;
3538
import static org.hamcrest.Matchers.equalTo;
3639
import static org.hamcrest.Matchers.greaterThan;
3740

@@ -50,6 +53,33 @@ public void testGetJobId() {
5053
assertThat(SnapshotLifecycleService.getJobId(meta), equalTo(id + "-" + version));
5154
}
5255

56+
public void testRepositoryExistenceForExistingRepo() {
57+
ClusterState state = ClusterState.builder(new ClusterName("cluster")).build();
58+
59+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
60+
() -> SnapshotLifecycleService.validateRepositoryExists("repo", state));
61+
62+
assertThat(e.getMessage(), containsString("no such repository [repo]"));
63+
64+
RepositoryMetaData repo = new RepositoryMetaData("repo", "fs", Settings.EMPTY);
65+
RepositoriesMetaData repoMeta = new RepositoriesMetaData(Collections.singletonList(repo));
66+
ClusterState stateWithRepo = ClusterState.builder(state)
67+
.metaData(MetaData.builder()
68+
.putCustom(RepositoriesMetaData.TYPE, repoMeta))
69+
.build();
70+
71+
SnapshotLifecycleService.validateRepositoryExists("repo", stateWithRepo);
72+
}
73+
74+
public void testRepositoryExistenceForMissingRepo() {
75+
ClusterState state = ClusterState.builder(new ClusterName("cluster")).build();
76+
77+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
78+
() -> SnapshotLifecycleService.validateRepositoryExists("repo", state));
79+
80+
assertThat(e.getMessage(), containsString("no such repository [repo]"));
81+
}
82+
5383
/**
5484
* Test new policies getting scheduled correctly, updated policies also being scheduled,
5585
* and deleted policies having their schedules cancelled.

0 commit comments

Comments
 (0)