diff --git a/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java b/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java index b1c8b655c4c6f..67c56b7f458ff 100644 --- a/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java +++ b/server/src/main/java/org/opensearch/extensions/AddSettingsUpdateConsumerRequestHandler.java @@ -15,6 +15,7 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.SettingsException; +import org.opensearch.common.settings.SettingsModule; import org.opensearch.common.settings.WriteableSetting; import org.opensearch.transport.TransportResponse; import org.opensearch.transport.TransportService; @@ -31,6 +32,7 @@ public class AddSettingsUpdateConsumerRequestHandler { private final ClusterService clusterService; private final TransportService transportService; private final String updateSettingsRequestType; + private final SettingsModule settingsModule; /** * Instantiates a new Add Settings Update Consumer Request Handler with the {@link ClusterService} and {@link TransportService} @@ -42,11 +44,13 @@ public class AddSettingsUpdateConsumerRequestHandler { public AddSettingsUpdateConsumerRequestHandler( ClusterService clusterService, TransportService transportService, - String updateSettingsRequestType + String updateSettingsRequestType, + SettingsModule settingsModule ) { this.clusterService = clusterService; this.transportService = transportService; this.updateSettingsRequestType = updateSettingsRequestType; + this.settingsModule = settingsModule; } /** @@ -68,25 +72,53 @@ TransportResponse handleAddSettingsUpdateConsumerRequest(AddSettingsUpdateConsum // Extract setting and type from writeable setting Setting setting = extensionComponentSetting.getSetting(); + + // we need to get the actual setting from nodeSetting or indexsetting maps in SettingsModule + // use conditional based on setting properties + Setting settingForUpdateConsumer = null; + if (setting.hasIndexScope()) { + settingForUpdateConsumer = settingsModule.getIndexScopedSettings().get(setting.getKey()); + } else if (setting.hasNodeScope()) { + settingForUpdateConsumer = settingsModule.getClusterSettings().get(setting.getKey()); + } + // do a null check and throw IllegalArgument exception here if neither index or node scope + if (settingForUpdateConsumer == null) { + throw new IllegalArgumentException("Not index or node scope"); + } + WriteableSetting.SettingType settingType = extensionComponentSetting.getType(); - // Register setting update consumer with callback method to extension - clusterService.getClusterSettings().addSettingsUpdateConsumer(setting, (data) -> { - logger.debug("Sending extension request type: " + updateSettingsRequestType); - UpdateSettingsResponseHandler updateSettingsResponseHandler = new UpdateSettingsResponseHandler(); - transportService.sendRequest( - extensionNode, - updateSettingsRequestType, - new UpdateSettingsRequest(settingType, setting, data), - updateSettingsResponseHandler - ); - }); + if (setting.hasIndexScope()) { + // Register setting update consumer with callback method to extension + clusterService.getClusterSettings().addSettingsUpdateConsumer(settingForUpdateConsumer, (data) -> { + logger.debug("Sending extension request type: " + updateSettingsRequestType); + UpdateSettingsResponseHandler updateSettingsResponseHandler = new UpdateSettingsResponseHandler(); + transportService.sendRequest( + extensionNode, + updateSettingsRequestType, + new UpdateSettingsRequest(settingType, setting, data), + updateSettingsResponseHandler + ); + }); + } + if (setting.hasNodeScope()) { + // Register setting update consumer with callback method to extension + clusterService.getClusterSettings().addSettingsUpdateConsumer(settingForUpdateConsumer, (data) -> { + logger.debug("Sending extension request type: " + updateSettingsRequestType); + UpdateSettingsResponseHandler updateSettingsResponseHandler = new UpdateSettingsResponseHandler(); + transportService.sendRequest( + extensionNode, + updateSettingsRequestType, + new UpdateSettingsRequest(settingType, setting, data), + updateSettingsResponseHandler + ); + }); + } } - } catch (SettingsException e) { + } catch (SettingsException | IllegalArgumentException e) { logger.error(e.toString()); status = false; } - return new AcknowledgedResponse(status); } } diff --git a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java index 9d74e8f22d2b1..5c6b9f3141aa0 100644 --- a/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java +++ b/server/src/main/java/org/opensearch/extensions/ExtensionsManager.java @@ -172,7 +172,8 @@ public void initializeServicesAndRestHandler( this.addSettingsUpdateConsumerRequestHandler = new AddSettingsUpdateConsumerRequestHandler( clusterService, transportService, - REQUEST_EXTENSION_UPDATE_SETTINGS + REQUEST_EXTENSION_UPDATE_SETTINGS, + settingsModule ); this.client = client; this.extensionTransportActionsHandler = new ExtensionTransportActionsHandler( 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 1dcb8ee00ebb2..0c5cece4249ef 100644 --- a/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java +++ b/server/src/test/java/org/opensearch/common/settings/ScopedSettingsTests.java @@ -1462,4 +1462,20 @@ public List getListValue(final List value) { ); } + public void testAddSettingsUpdateConsumer() { + Setting testSetting = Setting.intSetting("foo.bar", 1, Property.Dynamic, Property.NodeScope); + Setting testSetting2 = Setting.intSetting("foo.bar.baz", 1, Property.Dynamic, Property.NodeScope); + AbstractScopedSettings service = new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(testSetting, testSetting2))); + AtomicInteger consumer2 = new AtomicInteger(); + service.addSettingsUpdateConsumer(testSetting2, consumer2::set, (s) -> assertTrue(s > 0)); + Setting wrongKeySetting = Setting.intSetting("foo.bar.wrong", 1, Property.Dynamic, Property.NodeScope); + + expectThrows(SettingsException.class, () -> service.addSettingsUpdateConsumer(wrongKeySetting, consumer2::set, (i) -> { + if (i == 42) throw new AssertionError("wrong key"); + })); + + expectThrows(NullPointerException.class, () -> service.addSettingsUpdateConsumer(null, consumer2::set, (i) -> { + if (i == 42) throw new AssertionError("empty key"); + })); + } }