diff --git a/CHANGELOG.md b/CHANGELOG.md index 6406993a86a58..6662eac9a885e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Changed http code on create index API with bad input raising NotXContentException from 500 to 400 ([#4773](https://github.com/opensearch-project/OpenSearch/pull/4773)) - Change http code for DecommissioningFailedException from 500 to 400 ([#5283](https://github.com/opensearch-project/OpenSearch/pull/5283)) - Require MediaType in Strings.toString API ([#6009](https://github.com/opensearch-project/OpenSearch/pull/6009)) +- Improve summary error message for invalid setting updates ([#4792](https://github.com/opensearch-project/OpenSearch/pull/4792)) ### Deprecated diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/ClusterClientIT.java b/client/rest-high-level/src/test/java/org/opensearch/client/ClusterClientIT.java index 82d2cbe9149ca..a955b97b15e34 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/ClusterClientIT.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/ClusterClientIT.java @@ -162,7 +162,7 @@ public void testClusterUpdateSettingNonExistent() { assertThat(exception.status(), equalTo(RestStatus.BAD_REQUEST)); assertThat( exception.getMessage(), - equalTo("OpenSearch exception [type=illegal_argument_exception, reason=transient setting [" + setting + "], not recognized]") + equalTo("OpenSearch exception [type=settings_exception, reason=transient setting [" + setting + "], not recognized]") ); } diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java index 750b0c15e9c14..f92623e989a2e 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/IndicesClientIT.java @@ -1437,8 +1437,7 @@ public void testIndexPutSettings() throws IOException { assertThat( exception.getMessage(), startsWith( - "OpenSearch exception [type=illegal_argument_exception, " - + "reason=final index setting [index.number_of_shards], not updateable" + "OpenSearch exception [type=settings_exception, " + "reason=final index setting [index.number_of_shards], not updateable" ) ); } @@ -1475,7 +1474,7 @@ public void testIndexPutSettingNonExistent() throws IOException { assertThat( exception.getMessage(), equalTo( - "OpenSearch exception [type=illegal_argument_exception, " + "OpenSearch exception [type=settings_exception, " + "reason=unknown setting [index.no_idea_what_you_are_talking_about] please check that any required plugins are installed, " + "or check the breaking changes documentation for removed settings]" ) diff --git a/qa/evil-tests/src/test/java/org/opensearch/cluster/metadata/EvilSystemPropertyTests.java b/qa/evil-tests/src/test/java/org/opensearch/cluster/metadata/EvilSystemPropertyTests.java index ddb7913b0a1ed..869a84c1bbc02 100644 --- a/qa/evil-tests/src/test/java/org/opensearch/cluster/metadata/EvilSystemPropertyTests.java +++ b/qa/evil-tests/src/test/java/org/opensearch/cluster/metadata/EvilSystemPropertyTests.java @@ -31,8 +31,10 @@ package org.opensearch.cluster.metadata; +import org.hamcrest.Matchers; import org.opensearch.common.SuppressForbidden; import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsException; import org.opensearch.test.OpenSearchTestCase; import static org.opensearch.cluster.metadata.IndexMetadata.DEFAULT_NUMBER_OF_SHARDS; diff --git a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java index 51ef63ae9e9c1..39c2b5b113a43 100644 --- a/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/action/admin/indices/create/CreateIndexIT.java @@ -47,6 +47,7 @@ import org.opensearch.cluster.metadata.Metadata; import org.opensearch.common.collect.ImmutableOpenMap; import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsException; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.index.IndexNotFoundException; @@ -82,7 +83,7 @@ public void testCreationDateGivenFails() { try { prepareCreate("test").setSettings(Settings.builder().put(IndexMetadata.SETTING_CREATION_DATE, 4L)).get(); fail(); - } catch (IllegalArgumentException ex) { + } catch (SettingsException ex) { assertEquals( "unknown setting [index.creation_date] please check that any required plugins are installed, or check the " + "breaking changes documentation for removed settings", @@ -203,7 +204,7 @@ public void testUnknownSettingFails() { try { prepareCreate("test").setSettings(Settings.builder().put("index.unknown.value", "this must fail").build()).get(); fail("should have thrown an exception about the shard count"); - } catch (IllegalArgumentException e) { + } catch (SettingsException e) { assertEquals( "unknown setting [index.unknown.value] please check that any required plugins are installed, or check the" + " breaking changes documentation for removed settings", diff --git a/server/src/internalClusterTest/java/org/opensearch/cluster/settings/ClusterSettingsIT.java b/server/src/internalClusterTest/java/org/opensearch/cluster/settings/ClusterSettingsIT.java index 14365bba1e016..79b674b23fd48 100644 --- a/server/src/internalClusterTest/java/org/opensearch/cluster/settings/ClusterSettingsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/cluster/settings/ClusterSettingsIT.java @@ -41,6 +41,7 @@ import org.opensearch.cluster.routing.allocation.decider.EnableAllocationDecider; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsException; import org.opensearch.common.unit.ByteSizeUnit; import org.opensearch.common.unit.TimeValue; import org.opensearch.indices.recovery.RecoverySettings; @@ -78,7 +79,7 @@ public void testClusterNonExistingSettingsUpdate() { try { client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder().put(key1, value1).build()).get(); fail("bogus value"); - } catch (IllegalArgumentException ex) { + } catch (SettingsException ex) { assertEquals("transient setting [no_idea_what_you_are_talking_about], not recognized", ex.getMessage()); } } diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/settings/InternalSettingsIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/settings/InternalSettingsIT.java index c8e2a45891cb3..6a05e893d4fc9 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/settings/InternalSettingsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/settings/InternalSettingsIT.java @@ -34,6 +34,7 @@ import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse; import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsException; import org.opensearch.plugins.Plugin; import org.opensearch.test.OpenSearchIntegTestCase; @@ -64,8 +65,8 @@ public void testUpdateInternalIndexSettingViaSettingsAPI() { final GetSettingsResponse response = client().admin().indices().prepareGetSettings("test").get(); assertThat(response.getSetting("test", "index.internal"), equalTo("internal")); // we can not update the setting via the update settings API - final IllegalArgumentException e = expectThrows( - IllegalArgumentException.class, + final SettingsException e = expectThrows( + SettingsException.class, () -> client().admin() .indices() .prepareUpdateSettings("test") diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/settings/PrivateSettingsIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/settings/PrivateSettingsIT.java index c089381478dd3..0ff41f37d4000 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/settings/PrivateSettingsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/settings/PrivateSettingsIT.java @@ -35,6 +35,7 @@ import org.opensearch.action.admin.indices.settings.get.GetSettingsResponse; import org.opensearch.common.ValidationException; import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsException; import org.opensearch.plugins.Plugin; import org.opensearch.test.OpenSearchIntegTestCase; @@ -64,8 +65,8 @@ public void testSetPrivateIndexSettingOnCreate() { public void testUpdatePrivateIndexSettingViaSettingsAPI() { createIndex("test"); // we can not update the setting via the update settings API - final IllegalArgumentException e = expectThrows( - IllegalArgumentException.class, + final SettingsException e = expectThrows( + SettingsException.class, () -> client().admin() .indices() .prepareUpdateSettings("test") diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/settings/UpdateSettingsIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/settings/UpdateSettingsIT.java index 9970ff99a806c..af1fdfed6512a 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/settings/UpdateSettingsIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/settings/UpdateSettingsIT.java @@ -40,6 +40,7 @@ import org.opensearch.common.Priority; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsException; import org.opensearch.common.unit.TimeValue; import org.opensearch.index.IndexModule; import org.opensearch.index.IndexService; @@ -174,28 +175,28 @@ protected Settings nodeSettings(int nodeOrdinal) { } public void testUpdateDependentClusterSettings() { - IllegalArgumentException iae = expectThrows( - IllegalArgumentException.class, + SettingsException e = expectThrows( + SettingsException.class, () -> client().admin() .cluster() .prepareUpdateSettings() .setPersistentSettings(Settings.builder().put("cluster.acc.test.pw", "asdf")) .get() ); - assertEquals("missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", iae.getMessage()); + assertEquals("missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", e.getMessage()); - iae = expectThrows( - IllegalArgumentException.class, + e = expectThrows( + SettingsException.class, () -> client().admin() .cluster() .prepareUpdateSettings() .setTransientSettings(Settings.builder().put("cluster.acc.test.pw", "asdf")) .get() ); - assertEquals("missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", iae.getMessage()); + assertEquals("missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", e.getMessage()); - iae = expectThrows( - IllegalArgumentException.class, + e = expectThrows( + SettingsException.class, () -> client().admin() .cluster() .prepareUpdateSettings() @@ -203,7 +204,7 @@ public void testUpdateDependentClusterSettings() { .setPersistentSettings(Settings.builder().put("cluster.acc.test.user", "asdf")) .get() ); - assertEquals("missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", iae.getMessage()); + assertEquals("missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", e.getMessage()); if (randomBoolean()) { client().admin() @@ -211,15 +212,15 @@ public void testUpdateDependentClusterSettings() { .prepareUpdateSettings() .setTransientSettings(Settings.builder().put("cluster.acc.test.pw", "asdf").put("cluster.acc.test.user", "asdf")) .get(); - iae = expectThrows( - IllegalArgumentException.class, + e = expectThrows( + SettingsException.class, () -> client().admin() .cluster() .prepareUpdateSettings() .setTransientSettings(Settings.builder().putNull("cluster.acc.test.user")) .get() ); - assertEquals("missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", iae.getMessage()); + assertEquals("missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", e.getMessage()); client().admin() .cluster() .prepareUpdateSettings() @@ -232,15 +233,15 @@ public void testUpdateDependentClusterSettings() { .setPersistentSettings(Settings.builder().put("cluster.acc.test.pw", "asdf").put("cluster.acc.test.user", "asdf")) .get(); - iae = expectThrows( - IllegalArgumentException.class, + e = expectThrows( + SettingsException.class, () -> client().admin() .cluster() .prepareUpdateSettings() .setPersistentSettings(Settings.builder().putNull("cluster.acc.test.user")) .get() ); - assertEquals("missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", iae.getMessage()); + assertEquals("missing required setting [cluster.acc.test.user] for setting [cluster.acc.test.pw]", e.getMessage()); client().admin() .cluster() @@ -252,11 +253,11 @@ public void testUpdateDependentClusterSettings() { } public void testUpdateDependentIndexSettings() { - IllegalArgumentException iae = expectThrows( - IllegalArgumentException.class, + SettingsException e = expectThrows( + SettingsException.class, () -> prepareCreate("test", Settings.builder().put("index.acc.test.pw", "asdf")).get() ); - assertEquals("missing required setting [index.acc.test.user] for setting [index.acc.test.pw]", iae.getMessage()); + assertEquals("missing required setting [index.acc.test.user] for setting [index.acc.test.pw]", e.getMessage()); createIndex("test"); for (int i = 0; i < 2; i++) { @@ -265,8 +266,8 @@ public void testUpdateDependentIndexSettings() { client().admin().indices().prepareClose("test").get(); } - iae = expectThrows( - IllegalArgumentException.class, + e = expectThrows( + SettingsException.class, () -> client().admin() .indices() .prepareUpdateSettings("test") @@ -274,7 +275,7 @@ public void testUpdateDependentIndexSettings() { .execute() .actionGet() ); - assertEquals("missing required setting [index.acc.test.user] for setting [index.acc.test.pw]", iae.getMessage()); + assertEquals("missing required setting [index.acc.test.user] for setting [index.acc.test.pw]", e.getMessage()); // user has no dependency client().admin() @@ -293,8 +294,8 @@ public void testUpdateDependentIndexSettings() { .actionGet(); // now try to remove it and make sure it fails - iae = expectThrows( - IllegalArgumentException.class, + e = expectThrows( + SettingsException.class, () -> client().admin() .indices() .prepareUpdateSettings("test") @@ -302,7 +303,7 @@ public void testUpdateDependentIndexSettings() { .execute() .actionGet() ); - assertEquals("missing required setting [index.acc.test.user] for setting [index.acc.test.pw]", iae.getMessage()); + assertEquals("missing required setting [index.acc.test.user] for setting [index.acc.test.pw]", e.getMessage()); // now we are consistent client().admin() @@ -480,8 +481,8 @@ public void testOpenCloseUpdateSettings() throws Exception { assertThat(indexMetadata.getSettings().get("index.refresh_interval"), equalTo("1s")); assertThat(indexMetadata.getSettings().get("index.fielddata.cache"), equalTo("none")); - IllegalArgumentException ex = expectThrows( - IllegalArgumentException.class, + SettingsException ex = expectThrows( + SettingsException.class, () -> client().admin() .indices() .prepareUpdateSettings("test") diff --git a/server/src/internalClusterTest/java/org/opensearch/indices/template/SimpleIndexTemplateIT.java b/server/src/internalClusterTest/java/org/opensearch/indices/template/SimpleIndexTemplateIT.java index c2c0452e04ca8..b35a5a0c277f7 100644 --- a/server/src/internalClusterTest/java/org/opensearch/indices/template/SimpleIndexTemplateIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/indices/template/SimpleIndexTemplateIT.java @@ -48,6 +48,7 @@ import org.opensearch.common.ParsingException; import org.opensearch.common.bytes.BytesArray; import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsException; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.common.xcontent.XContentType; import org.opensearch.index.mapper.MapperParsingException; @@ -494,8 +495,8 @@ public void testInvalidSettings() throws Exception { GetIndexTemplatesResponse response = client().admin().indices().prepareGetTemplates().get(); assertThat(response.getIndexTemplates(), empty()); - IllegalArgumentException e = expectThrows( - IllegalArgumentException.class, + SettingsException e = expectThrows( + SettingsException.class, () -> client().admin() .indices() .preparePutTemplate("template_1") diff --git a/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java index 50f6714ec41b9..abc4ee70cb3e5 100644 --- a/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/AbstractScopedSettings.java @@ -100,16 +100,14 @@ protected AbstractScopedSettings( Map> keySettings = new HashMap<>(); for (Setting setting : settingsSet) { if (setting.getProperties().contains(scope) == false) { - throw new IllegalArgumentException( - "Setting " + setting + " must be a " + scope + " setting but has: " + setting.getProperties() - ); + throw new SettingsException("Setting " + setting + " must be a " + scope + " setting but has: " + setting.getProperties()); } validateSettingKey(setting); if (setting.hasComplexMatcher()) { Setting overlappingSetting = findOverlappingSetting(setting, complexMatchers); if (overlappingSetting != null) { - throw new IllegalArgumentException( + throw new SettingsException( "complex setting key: [" + setting.getKey() + "] overlaps existing setting key: [" @@ -130,7 +128,7 @@ protected void validateSettingKey(Setting setting) { if (isValidKey(setting.getKey()) == false && (setting.isGroupSetting() && isValidGroupKey(setting.getKey()) || isValidAffixKey(setting.getKey())) == false || setting.getKey().endsWith(".0")) { - throw new IllegalArgumentException("illegal settings key: [" + setting.getKey() + "]"); + throw new SettingsException("illegal settings key: [" + setting.getKey() + "]"); } } @@ -249,7 +247,7 @@ public synchronized Settings applySettings(Settings newSettings) { */ public synchronized void addSettingsUpdateConsumer(Setting setting, Consumer consumer, Consumer validator) { if (setting != get(setting.getKey())) { - throw new IllegalArgumentException("Setting is not registered for key [" + setting.getKey() + "]"); + throw new SettingsException("Setting is not registered for key [" + setting.getKey() + "]"); } addSettingsUpdater(setting.newUpdater(consumer, logger, validator)); } @@ -412,7 +410,7 @@ public void apply(Map values, Settings current, Settings previ private void ensureSettingIsRegistered(Setting.AffixSetting setting) { final Setting registeredSetting = this.complexMatchers.get(setting.getKey()); if (setting != registeredSetting) { - throw new IllegalArgumentException("Setting is not registered for key [" + setting.getKey() + "]"); + throw new SettingsException("Setting is not registered for key [" + setting.getKey() + "]"); } } @@ -428,7 +426,7 @@ public synchronized void addAffixMapUpdateConsumer( ) { final Setting registeredSetting = this.complexMatchers.get(setting.getKey()); if (setting != registeredSetting) { - throw new IllegalArgumentException("Setting is not registered for key [" + setting.getKey() + "]"); + throw new SettingsException("Setting is not registered for key [" + setting.getKey() + "]"); } addSettingsUpdater(setting.newAffixMapUpdater(consumer, logger, validator)); } @@ -483,10 +481,10 @@ public synchronized void addSettingsUpdateConsumer( BiConsumer validator ) { if (a != get(a.getKey())) { - throw new IllegalArgumentException("Setting is not registered for key [" + a.getKey() + "]"); + throw new SettingsException("Setting is not registered for key [" + a.getKey() + "]"); } if (b != get(b.getKey())) { - throw new IllegalArgumentException("Setting is not registered for key [" + b.getKey() + "]"); + throw new SettingsException("Setting is not registered for key [" + b.getKey() + "]"); } addSettingsUpdater(Setting.compoundUpdater(consumer, validator, a, b, logger)); } @@ -583,7 +581,7 @@ public final void validate( * @param key the key of the setting to validate * @param settings the settings * @param validateDependencies true if dependent settings should be validated - * @throws IllegalArgumentException if the setting is invalid + * @throws SettingsException if the setting is invalid */ void validate(final String key, final Settings settings, final boolean validateDependencies) { validate(key, settings, validateDependencies, false); @@ -596,7 +594,7 @@ void validate(final String key, final Settings settings, final boolean validateD * @param settings the settings * @param validateDependencies true if dependent settings should be validated * @param validateInternalOrPrivateIndex true if internal index settings should be validated - * @throws IllegalArgumentException if the setting is invalid + * @throws SettingsException if the setting is invalid */ void validate( final String key, @@ -628,7 +626,7 @@ void validate( msg += " please check that any required plugins are installed, or check the breaking changes documentation for removed " + "settings"; } - throw new IllegalArgumentException(msg); + throw new SettingsException(msg); } else { Set settingsDependencies = setting.getSettingsDependencies(key); if (setting.hasComplexMatcher()) { @@ -645,7 +643,7 @@ void validate( dependency.getKey(), setting.getKey() ); - throw new IllegalArgumentException(message); + throw new SettingsException(message); } // validate the dependent setting value settingDependency.validate(setting.getKey(), setting.get(settings), dependency.get(settings)); @@ -654,11 +652,11 @@ void validate( // the only time that validateInternalOrPrivateIndex should be true is if this call is coming via the update settings API if (validateInternalOrPrivateIndex) { if (setting.isInternalIndex()) { - throw new IllegalArgumentException( + throw new SettingsException( "can not update internal setting [" + setting.getKey() + "]; this setting is managed via a dedicated API" ); } else if (setting.isPrivateIndex()) { - throw new IllegalArgumentException( + throw new SettingsException( "can not update private setting [" + setting.getKey() + "]; this setting is managed by OpenSearch" ); } @@ -805,12 +803,12 @@ public Settings diff(Settings source, Settings defaultSettings) { */ public T get(Setting setting) { if (setting.getProperties().contains(scope) == false) { - throw new IllegalArgumentException( + throw new SettingsException( "settings scope doesn't match the setting scope [" + this.scope + "] not in [" + setting.getProperties() + "]" ); } if (get(setting.getKey()) == null) { - throw new IllegalArgumentException("setting " + setting.getKey() + " has not been registered"); + throw new SettingsException("setting " + setting.getKey() + " has not been registered"); } return setting.get(this.lastSettingsApplied, settings); } @@ -819,7 +817,7 @@ public T get(Setting setting) { * Updates a target settings builder with new, updated or deleted settings from a given settings builder. *

* Note: This method will only allow updates to dynamic settings. if a non-dynamic setting is updated an - * {@link IllegalArgumentException} is thrown instead. + * {@link SettingsException} is thrown instead. *

* * @param toApply the new settings to apply @@ -883,7 +881,7 @@ private boolean updateSettings(Settings toApply, Settings.Builder target, Settin toRemove.add(key); // we don't set changed here it's set after we apply deletes below if something actually changed } else if (get(key) == null) { - throw new IllegalArgumentException(type + " setting [" + key + "], not recognized"); + throw new SettingsException(type + " setting [" + key + "], not recognized"); } else if (isDelete == false && canUpdate.test(key)) { get(key).validateWithoutDependencies(toApply); // we might not have a full picture here do to a dependency validation settingsBuilder.copy(key, toApply); @@ -891,9 +889,9 @@ private boolean updateSettings(Settings toApply, Settings.Builder target, Settin changed |= toApply.get(key).equals(target.get(key)) == false; } else { if (isFinalSetting(key)) { - throw new IllegalArgumentException("final " + type + " setting [" + key + "], not updateable"); + throw new SettingsException("final " + type + " setting [" + key + "], not updateable"); } else { - throw new IllegalArgumentException(type + " setting [" + key + "], not dynamically updateable"); + throw new SettingsException(type + " setting [" + key + "], not dynamically updateable"); } } } diff --git a/server/src/main/java/org/opensearch/common/settings/SettingsException.java b/server/src/main/java/org/opensearch/common/settings/SettingsException.java index 965255903d23f..cdf9ea11a6932 100644 --- a/server/src/main/java/org/opensearch/common/settings/SettingsException.java +++ b/server/src/main/java/org/opensearch/common/settings/SettingsException.java @@ -34,6 +34,7 @@ import org.opensearch.OpenSearchException; import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.rest.RestStatus; import java.io.IOException; @@ -59,4 +60,9 @@ public SettingsException(StreamInput in) throws IOException { public SettingsException(String msg, Object... args) { super(msg, args); } + + @Override + public RestStatus status() { + return RestStatus.BAD_REQUEST; + } } diff --git a/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java b/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java index cb05a4210b483..b1c8b655c4c6f 100644 --- a/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java +++ b/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java @@ -14,6 +14,7 @@ import org.apache.logging.log4j.Logger; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.SettingsException; import org.opensearch.common.settings.WriteableSetting; import org.opensearch.transport.TransportResponse; import org.opensearch.transport.TransportService; @@ -81,7 +82,7 @@ TransportResponse handleAddSettingsUpdateConsumerRequest(AddSettingsUpdateConsum ); }); } - } catch (IllegalArgumentException e) { + } catch (SettingsException e) { logger.error(e.toString()); status = false; } diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java index d54b7e9389bf7..a361e67644e4d 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataIndexTemplateServiceTests.java @@ -45,6 +45,7 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.IndexScopedSettings; import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsException; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.NamedXContentRegistry; @@ -415,7 +416,7 @@ public void testAddComponentTemplate() throws Exception { ); ComponentTemplate componentTemplate4 = new ComponentTemplate(template, 1L, new HashMap<>()); expectThrows( - IllegalArgumentException.class, + SettingsException.class, () -> metadataIndexTemplateService.addComponentTemplate(throwState, true, "foo2", componentTemplate4) ); } diff --git a/server/src/test/java/org/opensearch/cluster/routing/allocation/DiskThresholdSettingsTests.java b/server/src/test/java/org/opensearch/cluster/routing/allocation/DiskThresholdSettingsTests.java index 5184ca7fe887d..d23b079e35ef9 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/allocation/DiskThresholdSettingsTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/allocation/DiskThresholdSettingsTests.java @@ -221,7 +221,6 @@ public void testInvalidHighDiskThreshold() { final String expected = "illegal value can't update [cluster.routing.allocation.disk.watermark.high] from [90%] to [75%]"; assertThat(e, hasToString(containsString(expected))); assertNotNull(e.getCause()); - assertNotNull(e.getCause()); assertThat(e.getCause(), instanceOf(IllegalArgumentException.class)); final IllegalArgumentException cause = (IllegalArgumentException) e.getCause(); assertThat(cause, hasToString(containsString("low disk watermark [85%] more than high disk watermark [75%]"))); diff --git a/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java b/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java index c6eb1843d05e1..1dcb8ee00ebb2 100644 --- a/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java +++ b/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java @@ -80,7 +80,7 @@ public void testResetSetting() { ClusterSettings service = new ClusterSettings(currentSettings, new HashSet<>(Arrays.asList(dynamicSetting, staticSetting))); expectThrows( - IllegalArgumentException.class, + SettingsException.class, () -> service.updateDynamicSettings( Settings.builder().put("some.dyn.setting", 8).putNull("some.static.setting").build(), Settings.builder().put(currentSettings), @@ -173,7 +173,7 @@ public void testAddConsumer() { try { service.addSettingsUpdateConsumer(testSetting2, consumer2::set); fail("setting not registered"); - } catch (IllegalArgumentException ex) { + } catch (SettingsException ex) { assertEquals("Setting is not registered for key [foo.bar.baz]", ex.getMessage()); } @@ -183,7 +183,7 @@ public void testAddConsumer() { consumer2.set(b); }); fail("setting not registered"); - } catch (IllegalArgumentException ex) { + } catch (SettingsException ex) { assertEquals("Setting is not registered for key [foo.bar.baz]", ex.getMessage()); } assertEquals(0, consumer.get()); @@ -208,11 +208,11 @@ public void testDependentSettings() { AbstractScopedSettings service = new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(intSetting, stringSetting))); - IllegalArgumentException iae = expectThrows( - IllegalArgumentException.class, + SettingsException e = expectThrows( + SettingsException.class, () -> service.validate(Settings.builder().put("foo.test.bar", 7).build(), true) ); - assertEquals("missing required setting [foo.test.name] for setting [foo.test.bar]", iae.getMessage()); + assertEquals("missing required setting [foo.test.name] for setting [foo.test.bar]", e.getMessage()); service.validate(Settings.builder().put("foo.test.name", "test").put("foo.test.bar", 7).build(), true); @@ -247,11 +247,11 @@ public void validate(final String key, final Object value, final Object dependen AbstractScopedSettings service = new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(intSetting, stringSetting))); - SettingsException iae = expectThrows( + SettingsException e = expectThrows( SettingsException.class, () -> service.validate(Settings.builder().put("foo.test.bar", 7).put("foo.test.name", "invalid").build(), true) ); - assertEquals("[foo.test.bar] is set but [name] is [invalid]", iae.getMessage()); + assertEquals("[foo.test.bar] is set but [name] is [invalid]", e.getMessage()); service.validate(Settings.builder().put("foo.test.bar", 7).put("foo.test.name", "valid").build(), true); @@ -288,8 +288,8 @@ public void testDependentSettingsWithFallback() { new HashSet<>(Arrays.asList(nameFallbackSetting, nameSetting, barSetting)) ); - final IllegalArgumentException e = expectThrows( - IllegalArgumentException.class, + final SettingsException e = expectThrows( + SettingsException.class, () -> service.validate(Settings.builder().put("foo.test.bar", 7).build(), true) ); assertThat(e, hasToString(containsString("missing required setting [foo.test.name] for setting [foo.test.bar]"))); @@ -963,11 +963,11 @@ public void testGetSetting() { public void testValidateWithSuggestion() { IndexScopedSettings settings = new IndexScopedSettings(Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS); - IllegalArgumentException iae = expectThrows( - IllegalArgumentException.class, + SettingsException e = expectThrows( + SettingsException.class, () -> settings.validate(Settings.builder().put("index.numbe_of_replica", "1").build(), false) ); - assertEquals(iae.getMessage(), "unknown setting [index.numbe_of_replica] did you mean [index.number_of_replicas]?"); + assertEquals(e.getMessage(), "unknown setting [index.numbe_of_replica] did you mean [index.number_of_replicas]?"); } public void testValidate() { @@ -976,13 +976,13 @@ public void testValidate() { + " removed settings"; settings.validate(Settings.builder().put("index.store.type", "boom").build(), false); - IllegalArgumentException e = expectThrows( - IllegalArgumentException.class, + SettingsException settingsException = expectThrows( + SettingsException.class, () -> settings.validate(Settings.builder().put("index.store.type", "boom").put("i.am.not.a.setting", true).build(), false) ); - assertEquals("unknown setting [i.am.not.a.setting]" + unknownMsgSuffix, e.getMessage()); + assertEquals("unknown setting [i.am.not.a.setting]" + unknownMsgSuffix, settingsException.getMessage()); - e = expectThrows( + IllegalArgumentException e = expectThrows( IllegalArgumentException.class, () -> settings.validate(Settings.builder().put("index.store.type", "boom").put("index.number_of_replicas", true).build(), false) ); @@ -1011,7 +1011,7 @@ public void testValidateSecureSettings() { Settings settings = Settings.builder().setSecureSettings(secureSettings).build(); final ClusterSettings clusterSettings = new ClusterSettings(settings, Collections.emptySet()); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> clusterSettings.validate(settings, false)); + SettingsException e = expectThrows(SettingsException.class, () -> clusterSettings.validate(settings, false)); assertThat(e.getMessage(), startsWith("unknown secure setting [some.secure.setting]")); ClusterSettings clusterSettings2 = new ClusterSettings( @@ -1071,14 +1071,14 @@ public void testKeyPattern() { try { new IndexScopedSettings(Settings.EMPTY, Collections.singleton(Setting.groupSetting("index. foo.", Property.IndexScope))); fail(); - } catch (IllegalArgumentException e) { + } catch (SettingsException e) { assertEquals("illegal settings key: [index. foo.]", e.getMessage()); } new IndexScopedSettings(Settings.EMPTY, Collections.singleton(Setting.groupSetting("index.", Property.IndexScope))); try { new IndexScopedSettings(Settings.EMPTY, Collections.singleton(Setting.boolSetting("index.", true, Property.IndexScope))); fail(); - } catch (IllegalArgumentException e) { + } catch (SettingsException e) { assertEquals("illegal settings key: [index.]", e.getMessage()); } new IndexScopedSettings(Settings.EMPTY, Collections.singleton(Setting.boolSetting("index.boo", true, Property.IndexScope))); @@ -1152,7 +1152,7 @@ public void testOverlappingComplexMatchSettings() { try { new ClusterSettings(Settings.EMPTY, settings); fail("an exception should have been thrown because settings overlap"); - } catch (IllegalArgumentException e) { + } catch (SettingsException e) { if (groupFirst) { assertEquals("complex setting key: [foo.bar] overlaps existing setting key: [foo.]", e.getMessage()); } else { @@ -1163,8 +1163,8 @@ public void testOverlappingComplexMatchSettings() { public void testUpdateNumberOfShardsFail() { IndexScopedSettings settings = new IndexScopedSettings(Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS); - IllegalArgumentException ex = expectThrows( - IllegalArgumentException.class, + SettingsException ex = expectThrows( + SettingsException.class, () -> settings.updateSettings( Settings.builder().put("index.number_of_shards", 8).build(), Settings.builder(), @@ -1181,8 +1181,8 @@ public void testFinalSettingUpdateFail() { Settings currentSettings = Settings.builder().put("some.final.setting", 9).put("some.final.group.foo", 7).build(); ClusterSettings service = new ClusterSettings(currentSettings, new HashSet<>(Arrays.asList(finalSetting, finalGroupSetting))); - IllegalArgumentException exc = expectThrows( - IllegalArgumentException.class, + SettingsException exc = expectThrows( + SettingsException.class, () -> service.updateDynamicSettings( Settings.builder().put("some.final.setting", 8).build(), Settings.builder().put(currentSettings), @@ -1193,7 +1193,7 @@ public void testFinalSettingUpdateFail() { assertThat(exc.getMessage(), containsString("final node setting [some.final.setting]")); exc = expectThrows( - IllegalArgumentException.class, + SettingsException.class, () -> service.updateDynamicSettings( Settings.builder().putNull("some.final.setting").build(), Settings.builder().put(currentSettings), @@ -1204,7 +1204,7 @@ public void testFinalSettingUpdateFail() { assertThat(exc.getMessage(), containsString("final node setting [some.final.setting]")); exc = expectThrows( - IllegalArgumentException.class, + SettingsException.class, () -> service.updateSettings( Settings.builder().put("some.final.group.new", 8).build(), Settings.builder().put(currentSettings), @@ -1215,7 +1215,7 @@ public void testFinalSettingUpdateFail() { assertThat(exc.getMessage(), containsString("final node setting [some.final.group.new]")); exc = expectThrows( - IllegalArgumentException.class, + SettingsException.class, () -> service.updateSettings( Settings.builder().put("some.final.group.foo", 5).build(), Settings.builder().put(currentSettings), @@ -1232,7 +1232,7 @@ public void testInternalIndexSettingsFailsValidation() { Settings.EMPTY, Collections.singleton(indexInternalSetting) ); - final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { + final SettingsException e = expectThrows(SettingsException.class, () -> { final Settings settings = Settings.builder().put("index.internal", "internal").build(); indexScopedSettings.validate(settings, false, /* validateInternalOrPrivateIndex */ true); }); @@ -1246,7 +1246,7 @@ public void testPrivateIndexSettingsFailsValidation() { Settings.EMPTY, Collections.singleton(indexInternalSetting) ); - final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { + final SettingsException e = expectThrows(SettingsException.class, () -> { final Settings settings = Settings.builder().put("index.private", "private").build(); indexScopedSettings.validate(settings, false, /* validateInternalOrPrivateIndex */ true); }); diff --git a/server/src/test/java/org/opensearch/common/settings/SettingsModuleTests.java b/server/src/test/java/org/opensearch/common/settings/SettingsModuleTests.java index b145ec0d79311..cfc123e210a16 100644 --- a/server/src/test/java/org/opensearch/common/settings/SettingsModuleTests.java +++ b/server/src/test/java/org/opensearch/common/settings/SettingsModuleTests.java @@ -233,7 +233,7 @@ public void testMutuallyExclusiveScopes() { public void testOldMaxClauseCountSetting() { Settings settings = Settings.builder().put("index.query.bool.max_clause_count", 1024).build(); - IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> new SettingsModule(settings)); + SettingsException ex = expectThrows(SettingsException.class, () -> new SettingsModule(settings)); assertEquals( "unknown setting [index.query.bool.max_clause_count] did you mean [indices.query.bool.max_clause_count]?", ex.getMessage() diff --git a/server/src/test/java/org/opensearch/index/IndexModuleTests.java b/server/src/test/java/org/opensearch/index/IndexModuleTests.java index 565ae5a16a384..ad6344882b70d 100644 --- a/server/src/test/java/org/opensearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/opensearch/index/IndexModuleTests.java @@ -60,6 +60,7 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsException; import org.opensearch.common.util.BigArrays; import org.opensearch.common.util.PageCacheRecycler; import org.opensearch.common.util.concurrent.OpenSearchRejectedExecutionException; @@ -332,7 +333,7 @@ public void testListener() throws IOException { try { module.addSettingsUpdateConsumer(booleanSetting2, atomicBoolean::set); fail("not registered"); - } catch (IllegalArgumentException ex) { + } catch (SettingsException ex) { } diff --git a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java index 28044410a21f4..c6bccbe26a846 100644 --- a/server/src/test/java/org/opensearch/index/IndexSettingsTests.java +++ b/server/src/test/java/org/opensearch/index/IndexSettingsTests.java @@ -40,6 +40,7 @@ import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Setting.Property; import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsException; import org.opensearch.common.unit.ByteSizeValue; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.FeatureFlags; @@ -605,8 +606,8 @@ public void testPrivateSettingsValidation() { { // validation should fail since we are not ignoring private settings - final IllegalArgumentException e = expectThrows( - IllegalArgumentException.class, + final SettingsException e = expectThrows( + SettingsException.class, () -> indexScopedSettings.validate(settings, randomBoolean()) ); assertThat(e, hasToString(containsString("unknown setting [index.creation_date]"))); @@ -614,8 +615,8 @@ public void testPrivateSettingsValidation() { { // validation should fail since we are not ignoring private settings - final IllegalArgumentException e = expectThrows( - IllegalArgumentException.class, + final SettingsException e = expectThrows( + SettingsException.class, () -> indexScopedSettings.validate(settings, randomBoolean(), false, randomBoolean()) ); assertThat(e, hasToString(containsString("unknown setting [index.creation_date]"))); @@ -633,8 +634,8 @@ public void testArchivedSettingsValidation() { { // validation should fail since we are not ignoring archived settings - final IllegalArgumentException e = expectThrows( - IllegalArgumentException.class, + final SettingsException e = expectThrows( + SettingsException.class, () -> indexScopedSettings.validate(settings, randomBoolean()) ); assertThat(e, hasToString(containsString("unknown setting [archived.foo]"))); @@ -642,8 +643,8 @@ public void testArchivedSettingsValidation() { { // validation should fail since we are not ignoring archived settings - final IllegalArgumentException e = expectThrows( - IllegalArgumentException.class, + final SettingsException e = expectThrows( + SettingsException.class, () -> indexScopedSettings.validate(settings, randomBoolean(), randomBoolean(), false) ); assertThat(e, hasToString(containsString("unknown setting [archived.foo]"))); @@ -712,8 +713,8 @@ public void testQueryDefaultField() { public void testUpdateSoftDeletesFails() { IndexScopedSettings settings = new IndexScopedSettings(Settings.EMPTY, IndexScopedSettings.BUILT_IN_INDEX_SETTINGS); - IllegalArgumentException error = expectThrows( - IllegalArgumentException.class, + SettingsException error = expectThrows( + SettingsException.class, () -> settings.updateSettings( Settings.builder().put("index.soft_deletes.enabled", randomBoolean()).build(), Settings.builder(), @@ -817,8 +818,8 @@ public void testUpdateRemoteStoreFails() { Set> remoteStoreSettingSet = new HashSet<>(); remoteStoreSettingSet.add(IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING); IndexScopedSettings settings = new IndexScopedSettings(Settings.EMPTY, remoteStoreSettingSet); - IllegalArgumentException error = expectThrows( - IllegalArgumentException.class, + SettingsException error = expectThrows( + SettingsException.class, () -> settings.updateSettings( Settings.builder().put("index.remote_store.enabled", randomBoolean()).build(), Settings.builder(), @@ -833,8 +834,8 @@ public void testUpdateRemoteTranslogStoreFails() { Set> remoteStoreSettingSet = new HashSet<>(); remoteStoreSettingSet.add(IndexMetadata.INDEX_REMOTE_TRANSLOG_STORE_ENABLED_SETTING); IndexScopedSettings settings = new IndexScopedSettings(Settings.EMPTY, remoteStoreSettingSet); - IllegalArgumentException error = expectThrows( - IllegalArgumentException.class, + SettingsException error = expectThrows( + SettingsException.class, () -> settings.updateSettings( Settings.builder().put("index.remote_store.translog.enabled", randomBoolean()).build(), Settings.builder(), @@ -907,8 +908,8 @@ public void testUpdateRemoteRepositoryFails() { Set> remoteStoreSettingSet = new HashSet<>(); remoteStoreSettingSet.add(IndexMetadata.INDEX_REMOTE_STORE_REPOSITORY_SETTING); IndexScopedSettings settings = new IndexScopedSettings(Settings.EMPTY, remoteStoreSettingSet); - IllegalArgumentException error = expectThrows( - IllegalArgumentException.class, + SettingsException error = expectThrows( + SettingsException.class, () -> settings.updateSettings( Settings.builder().put("index.remote_store.repository", randomUnicodeOfLength(10)).build(), Settings.builder(), diff --git a/server/src/test/java/org/opensearch/search/SearchServiceTests.java b/server/src/test/java/org/opensearch/search/SearchServiceTests.java index 1f824d40eb638..535eb3a118189 100644 --- a/server/src/test/java/org/opensearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/opensearch/search/SearchServiceTests.java @@ -59,6 +59,7 @@ import org.opensearch.common.io.stream.StreamInput; import org.opensearch.common.io.stream.StreamOutput; import org.opensearch.common.settings.Settings; +import org.opensearch.common.settings.SettingsException; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.concurrent.OpenSearchRejectedExecutionException; import org.opensearch.common.xcontent.XContentBuilder; @@ -1071,15 +1072,15 @@ public void testSetSearchThrottled() { ) ).actionGet(); - IllegalArgumentException iae = expectThrows( - IllegalArgumentException.class, + SettingsException se = expectThrows( + SettingsException.class, () -> client().admin() .indices() .prepareUpdateSettings("throttled_threadpool_index") .setSettings(Settings.builder().put(IndexSettings.INDEX_SEARCH_THROTTLED.getKey(), false)) .get() ); - assertEquals("can not update private setting [index.search.throttled]; this setting is managed by OpenSearch", iae.getMessage()); + assertEquals("can not update private setting [index.search.throttled]; this setting is managed by OpenSearch", se.getMessage()); assertFalse(service.getIndicesService().indexServiceSafe(index).getIndexSettings().isSearchThrottled()); SearchRequest searchRequest = new SearchRequest().allowPartialSearchResults(false); ShardSearchRequest req = new ShardSearchRequest(