From a794743d439e504007cc6eba8049fd1d61c87c10 Mon Sep 17 00:00:00 2001 From: Henning Andersen <33268011+henningandersen@users.noreply.github.com> Date: Fri, 15 Jan 2021 18:06:30 +0100 Subject: [PATCH] Reject autoscaling policies with no deciders (#67284) A policy that resolves to no deciders after defaulting can never deliver any autoscaling capacities. To help operators not instantiate such policies, this commit adds validation that the resulting policy must resolve to at least one decider. --- .../autoscaling/delete_autoscaling_policy.yml | 8 ++++---- .../autoscaling/put_autoscaling_policy.yml | 13 ++++++++++-- ...TransportPutAutoscalingPolicyActionIT.java | 20 +++++++++++++++++-- .../AutoscalingCalculateCapacityService.java | 6 ++++++ ...oscalingCalculateCapacityServiceTests.java | 15 ++++++++++++++ 5 files changed, 54 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/autoscaling/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/autoscaling/delete_autoscaling_policy.yml b/x-pack/plugin/autoscaling/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/autoscaling/delete_autoscaling_policy.yml index 624bbcb99236..e6fdef26bf0f 100644 --- a/x-pack/plugin/autoscaling/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/autoscaling/delete_autoscaling_policy.yml +++ b/x-pack/plugin/autoscaling/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/autoscaling/delete_autoscaling_policy.yml @@ -44,13 +44,13 @@ autoscaling.put_autoscaling_policy: name: my_autoscaling_policy_1 body: - roles: [] + roles: [ "data" ] - do: autoscaling.put_autoscaling_policy: name: my_autoscaling_policy_2 body: - roles: [] + roles: [ "data" ] - do: autoscaling.delete_autoscaling_policy: @@ -72,13 +72,13 @@ autoscaling.put_autoscaling_policy: name: my_autoscaling_policy_delete body: - roles: [] + roles: [ "data" ] - do: autoscaling.put_autoscaling_policy: name: my_autoscaling_policy_keep body: - roles: [] + roles: [ "data" ] - do: autoscaling.delete_autoscaling_policy: diff --git a/x-pack/plugin/autoscaling/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/autoscaling/put_autoscaling_policy.yml b/x-pack/plugin/autoscaling/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/autoscaling/put_autoscaling_policy.yml index 7551b7bba43a..4f603ca377b9 100644 --- a/x-pack/plugin/autoscaling/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/autoscaling/put_autoscaling_policy.yml +++ b/x-pack/plugin/autoscaling/qa/rest/src/yamlRestTest/resources/rest-api-spec/test/autoscaling/put_autoscaling_policy.yml @@ -4,7 +4,7 @@ autoscaling.put_autoscaling_policy: name: my_autoscaling_policy body: - roles: [ master ] + roles: [ data_content ] - match: { "acknowledged": true } @@ -19,7 +19,7 @@ - do: autoscaling.get_autoscaling_policy: name: my_autoscaling_policy - - match: { roles: [ master ] } + - match: { roles: [ data_content ] } - match: { deciders.fixed: {} } # update roles @@ -112,3 +112,12 @@ policy: fixed: storage: bad + +--- +"Test put autoscaling policy with no default deciders": + - do: + catch: bad_request + autoscaling.put_autoscaling_policy: + name: my_autoscaling_policy + body: + roles: [ ] diff --git a/x-pack/plugin/autoscaling/src/internalClusterTest/java/org/elasticsearch/xpack/autoscaling/action/TransportPutAutoscalingPolicyActionIT.java b/x-pack/plugin/autoscaling/src/internalClusterTest/java/org/elasticsearch/xpack/autoscaling/action/TransportPutAutoscalingPolicyActionIT.java index 8b4bfa876082..6fd7f7fa1eb6 100644 --- a/x-pack/plugin/autoscaling/src/internalClusterTest/java/org/elasticsearch/xpack/autoscaling/action/TransportPutAutoscalingPolicyActionIT.java +++ b/x-pack/plugin/autoscaling/src/internalClusterTest/java/org/elasticsearch/xpack/autoscaling/action/TransportPutAutoscalingPolicyActionIT.java @@ -11,9 +11,12 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Strings; import org.elasticsearch.xpack.autoscaling.AutoscalingIntegTestCase; import org.elasticsearch.xpack.autoscaling.AutoscalingMetadata; -import org.elasticsearch.xpack.autoscaling.AutoscalingTestCase; import org.elasticsearch.xpack.autoscaling.policy.AutoscalingPolicy; +import java.util.List; +import java.util.TreeMap; +import java.util.TreeSet; + import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; import static org.elasticsearch.xpack.autoscaling.AutoscalingTestCase.mutateAutoscalingDeciders; import static org.elasticsearch.xpack.autoscaling.AutoscalingTestCase.randomAutoscalingDeciders; @@ -39,7 +42,7 @@ public class TransportPutAutoscalingPolicyActionIT extends AutoscalingIntegTestC final AutoscalingPolicy policy = putRandomAutoscalingPolicy(); final AutoscalingPolicy updatedPolicy = new AutoscalingPolicy( policy.name(), - AutoscalingTestCase.randomRoles(), + new TreeSet<>(randomSubsetOf(List.of("data", "data_content", "data_hot", "data_warm", "data_cold"))), mutateAutoscalingDeciders(policy.deciders()) ); putAutoscalingPolicy(updatedPolicy); @@ -73,6 +76,19 @@ public class TransportPutAutoscalingPolicyActionIT extends AutoscalingIntegTestC ); } + public void testPutNoDeciderPolicy() { + String policyName = randomAlphaOfLength(8); + IllegalArgumentException exception = expectThrows( + IllegalArgumentException.class, + () -> putAutoscalingPolicy(new AutoscalingPolicy(policyName, new TreeSet<>(), new TreeMap<>())) + ); + + assertThat( + exception.getMessage(), + containsString("no default nor user configured deciders for policy [" + policyName + "] with roles [[]]") + ); + } + private AutoscalingPolicy putRandomAutoscalingPolicy() { final AutoscalingPolicy policy = randomAutoscalingPolicy(); putAutoscalingPolicy(policy); diff --git a/x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/capacity/AutoscalingCalculateCapacityService.java b/x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/capacity/AutoscalingCalculateCapacityService.java index 2243a6e96729..1d1131e26e8e 100644 --- a/x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/capacity/AutoscalingCalculateCapacityService.java +++ b/x-pack/plugin/autoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/capacity/AutoscalingCalculateCapacityService.java @@ -50,6 +50,12 @@ public class AutoscalingCalculateCapacityService implements PolicyValidator { public void validate(AutoscalingPolicy policy) { policy.deciders().forEach((name, configuration) -> validate(name, configuration, policy.roles())); + SortedMap deciders = addDefaultDeciders(policy); + if (deciders.isEmpty()) { + throw new IllegalArgumentException( + "no default nor user configured deciders for policy [" + policy.name() + "] with roles [" + policy.roles() + "]" + ); + } } private void validate(final String deciderName, final Settings configuration, SortedSet roles) { diff --git a/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/AutoscalingCalculateCapacityServiceTests.java b/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/AutoscalingCalculateCapacityServiceTests.java index 4f5e3db5d395..f36347ce30d2 100644 --- a/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/AutoscalingCalculateCapacityServiceTests.java +++ b/x-pack/plugin/autoscaling/src/test/java/org/elasticsearch/xpack/autoscaling/capacity/AutoscalingCalculateCapacityServiceTests.java @@ -335,6 +335,21 @@ public class AutoscalingCalculateCapacityServiceTests extends AutoscalingTestCas ); } + public void testValidateNotEmptyDeciders() { + AutoscalingCalculateCapacityService service = new AutoscalingCalculateCapacityService(Set.of(new FixedAutoscalingDeciderService())); + String policyName = randomAlphaOfLength(8); + AutoscalingPolicy policy = new AutoscalingPolicy( + policyName, + new TreeSet<>(randomBoolean() ? Set.of() : Set.of("master")), + new TreeMap<>() + ); + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> service.validate(policy)); + assertThat( + exception.getMessage(), + equalTo("no default nor user configured deciders for policy [" + policyName + "] with roles [" + policy.roles() + "]") + ); + } + public void testValidateSettingName() { AutoscalingCalculateCapacityService service = new AutoscalingCalculateCapacityService(Set.of(new FixedAutoscalingDeciderService())); Set legalNames = Set.of(