From e99bb254734c4e1e0fa08fd98941dcf440ebdc51 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Tue, 16 Jul 2019 16:24:12 -0600 Subject: [PATCH 1/2] Allow empty configuration for SLM policies When putting or updating a snapshot lifecycle policy it was not possible to elide the `config` map. This commit makes the configuration optional, the same way that it is when taking a snapshot. Relates to #38461 --- .../snapshotlifecycle/SnapshotLifecyclePolicy.java | 13 +++++++------ .../snapshotlifecycle/SnapshotLifecyclePolicy.java | 12 ++++++++---- .../history/SnapshotHistoryItem.java | 2 +- .../history/SnapshotHistoryStoreTests.java | 9 ++++++--- .../SnapshotLifecyclePolicyTests.java | 9 ++++++--- 5 files changed, 28 insertions(+), 17 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/snapshotlifecycle/SnapshotLifecyclePolicy.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/snapshotlifecycle/SnapshotLifecyclePolicy.java index 8d8e78184ff59..3fd357e2090a2 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/snapshotlifecycle/SnapshotLifecyclePolicy.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/snapshotlifecycle/SnapshotLifecyclePolicy.java @@ -19,7 +19,7 @@ package org.elasticsearch.client.snapshotlifecycle; -import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.xcontent.ConstructingObjectParser; @@ -43,8 +43,6 @@ public class SnapshotLifecyclePolicy implements ToXContentObject { private static final ParseField SCHEDULE = new ParseField("schedule"); private static final ParseField REPOSITORY = new ParseField("repository"); private static final ParseField CONFIG = new ParseField("config"); - private static final IndexNameExpressionResolver.DateMathExpressionResolver DATE_MATH_RESOLVER = - new IndexNameExpressionResolver.DateMathExpressionResolver(); @SuppressWarnings("unchecked") private static final ConstructingObjectParser PARSER = @@ -61,11 +59,11 @@ public class SnapshotLifecyclePolicy implements ToXContentObject { PARSER.declareString(ConstructingObjectParser.constructorArg(), NAME); PARSER.declareString(ConstructingObjectParser.constructorArg(), SCHEDULE); PARSER.declareString(ConstructingObjectParser.constructorArg(), REPOSITORY); - PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> p.map(), CONFIG); + PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> p.map(), CONFIG); } public SnapshotLifecyclePolicy(final String id, final String name, final String schedule, - final String repository, Map configuration) { + final String repository, @Nullable Map configuration) { this.id = Objects.requireNonNull(id); this.name = name; this.schedule = schedule; @@ -89,6 +87,7 @@ public String getRepository() { return this.repository; } + @Nullable public Map getConfig() { return this.configuration; } @@ -103,7 +102,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field(NAME.getPreferredName(), this.name); builder.field(SCHEDULE.getPreferredName(), this.schedule); builder.field(REPOSITORY.getPreferredName(), this.repository); - builder.field(CONFIG.getPreferredName(), this.configuration); + if (this.configuration != null) { + builder.field(CONFIG.getPreferredName(), this.configuration); + } builder.endObject(); return builder; } 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 5db1996a45982..960e0f355d545 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 @@ -15,6 +15,7 @@ import org.elasticsearch.cluster.Diffable; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.Context; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.UUIDs; @@ -76,11 +77,11 @@ public class SnapshotLifecyclePolicy extends AbstractDiffable p.map(), CONFIG); + PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> p.map(), CONFIG); } public SnapshotLifecyclePolicy(final String id, final String name, final String schedule, - final String repository, Map configuration) { + final String repository, @Nullable Map configuration) { this.id = Objects.requireNonNull(id); this.name = name; this.schedule = schedule; @@ -112,6 +113,7 @@ public String getRepository() { return this.repository; } + @Nullable public Map getConfig() { return this.configuration; } @@ -172,7 +174,7 @@ public ActionRequestValidationException validate() { } } - if (configuration.containsKey(METADATA_FIELD_NAME)) { + if (configuration != null && configuration.containsKey(METADATA_FIELD_NAME)) { if (configuration.get(METADATA_FIELD_NAME) instanceof Map == false) { err.addValidationError("invalid configuration." + METADATA_FIELD_NAME + " [" + configuration.get(METADATA_FIELD_NAME) + "]: must be an object if present"); @@ -265,7 +267,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field(NAME.getPreferredName(), this.name); builder.field(SCHEDULE.getPreferredName(), this.schedule); builder.field(REPOSITORY.getPreferredName(), this.repository); - builder.field(CONFIG.getPreferredName(), this.configuration); + if (this.configuration != null) { + builder.field(CONFIG.getPreferredName(), this.configuration); + } builder.endObject(); return builder; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java index 1c5a9ef751e9d..8120be9683fb7 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java @@ -94,7 +94,7 @@ public static SnapshotHistoryItem parse(XContentParser parser, String name) { this.snapshotName = Objects.requireNonNull(snapshotName); this.operation = Objects.requireNonNull(operation); this.success = success; - this.snapshotConfiguration = Objects.requireNonNull(snapshotConfiguration); + this.snapshotConfiguration = snapshotConfiguration; this.errorDetails = errorDetails; } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java index 146af810fdd79..43d069cfa0d9e 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java @@ -176,9 +176,12 @@ public void testIndexNameGeneration() { } public static SnapshotLifecyclePolicy randomSnapshotLifecyclePolicy(String id) { - Map config = new HashMap<>(); - for (int i = 0; i < randomIntBetween(2, 5); i++) { - config.put(randomAlphaOfLength(4), randomAlphaOfLength(4)); + Map config = null; + if (randomBoolean()) { + config = new HashMap<>(); + for (int i = 0; i < randomIntBetween(2, 5); i++) { + config.put(randomAlphaOfLength(4), randomAlphaOfLength(4)); + } } return new SnapshotLifecyclePolicy(id, randomAlphaOfLength(4), 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 c2aac7120ab8a..1cc88bc4d75b1 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 @@ -131,9 +131,12 @@ protected SnapshotLifecyclePolicy createTestInstance() { } public static SnapshotLifecyclePolicy randomSnapshotLifecyclePolicy(String id) { - Map config = new HashMap<>(); - for (int i = 0; i < randomIntBetween(2, 5); i++) { - config.put(randomAlphaOfLength(4), randomAlphaOfLength(4)); + Map config = null; + if (randomBoolean()) { + config = new HashMap<>(); + for (int i = 0; i < randomIntBetween(2, 5); i++) { + config.put(randomAlphaOfLength(4), randomAlphaOfLength(4)); + } } return new SnapshotLifecyclePolicy(id, randomAlphaOfLength(4), From b1ddf2539acbe00a71dafb6cd42124e266766760 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 18 Jul 2019 12:24:50 -0600 Subject: [PATCH 2/2] Add Objects.requireNonNull for required parts of the policy --- .../core/snapshotlifecycle/SnapshotLifecyclePolicy.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 960e0f355d545..e1d760702bde3 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 @@ -82,10 +82,10 @@ public class SnapshotLifecyclePolicy extends AbstractDiffable configuration) { - this.id = Objects.requireNonNull(id); - this.name = name; - this.schedule = schedule; - this.repository = repository; + this.id = Objects.requireNonNull(id, "policy id is required"); + this.name = Objects.requireNonNull(name, "policy snapshot name is required"); + this.schedule = Objects.requireNonNull(schedule, "policy schedule is required"); + this.repository = Objects.requireNonNull(repository, "policy snapshot repository is required"); this.configuration = configuration; }