Skip to content

Commit

Permalink
[Extensions] Add ExtensionAwarePlugin extension point to add custom s…
Browse files Browse the repository at this point in the history
…ettings for extensions (#7526)

* WIP on extension settings

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Use getExtensionSettings from the identity service

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Add extension scoped settings and add area for additional settings

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Run spotlessApply

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Re-run CI

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* spotlessApply

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Change contructor to take list of additionalSettings

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* One constructor

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Remove isAuthenticated

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Re-run CI

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Re-run CI

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Create ExtensionAwarePlugin extension point

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Update CHANGELOG

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Address code review feedback

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Compute additionalSettingsKeys outside of loop

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Address code review feedback

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Add comment

Signed-off-by: Craig Perkins <cwperx@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
  • Loading branch information
cwperks committed May 25, 2023
1 parent c41c5c2 commit 277eb3d
Show file tree
Hide file tree
Showing 10 changed files with 197 additions and 25 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Support to clear filecache using clear indices cache API ([#7498](https://github.com/opensearch-project/OpenSearch/pull/7498))
- Create NamedRoute to map extension routes to a shortened name ([#6870](https://github.com/opensearch-project/OpenSearch/pull/6870))
- Added @dbwiddis as on OpenSearch maintainer ([#7665](https://github.com/opensearch-project/OpenSearch/pull/7665))
- [Extensions] Add ExtensionAwarePlugin extension point to add custom settings for extensions ([#7526](https://github.com/opensearch-project/OpenSearch/pull/7526))

### Dependencies
- Bump `com.netflix.nebula:gradle-info-plugin` from 12.0.0 to 12.1.3 (#7564)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ public enum Property {
*/
Deprecated,

/**
* Extension scope
*/
ExtensionScope,

/**
* Node scope
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.extensions;

import org.opensearch.common.settings.AbstractScopedSettings;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Setting.Property;
import org.opensearch.common.settings.SettingUpgrader;
import org.opensearch.common.settings.Settings;

import java.util.Collections;
import java.util.Set;

/**
* Encapsulates all valid extension level settings.
*
* @opensearch.internal
*/
public final class ExtensionScopedSettings extends AbstractScopedSettings {

public ExtensionScopedSettings(final Set<Setting<?>> settingsSet) {
this(settingsSet, Collections.emptySet());
}

public ExtensionScopedSettings(final Set<Setting<?>> settingsSet, final Set<SettingUpgrader<?>> settingUpgraders) {
super(Settings.EMPTY, settingsSet, settingUpgraders, Property.ExtensionScope);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
import java.util.concurrent.TimeUnit;
Expand All @@ -37,6 +39,7 @@
import org.opensearch.client.node.NodeClient;
import org.opensearch.cluster.ClusterSettingsResponse;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.settings.Setting;
import org.opensearch.core.util.FileSystemUtils;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.settings.Settings;
Expand Down Expand Up @@ -106,6 +109,7 @@ public static enum OpenSearchRequestType {
private CustomSettingsRequestHandler customSettingsRequestHandler;
private TransportService transportService;
private ClusterService clusterService;
private final Set<Setting<?>> additionalSettings;
private Settings environmentSettings;
private AddSettingsUpdateConsumerRequestHandler addSettingsUpdateConsumerRequestHandler;
private NodeClient client;
Expand All @@ -114,9 +118,10 @@ public static enum OpenSearchRequestType {
* Instantiate a new ExtensionsManager object to handle requests and responses from extensions. This is called during Node bootstrap.
*
* @param extensionsPath Path to a directory containing extensions.
* @param additionalSettings Additional settings to read in from extensions.yml
* @throws IOException If the extensions discovery file is not properly retrieved.
*/
public ExtensionsManager(Path extensionsPath) throws IOException {
public ExtensionsManager(Path extensionsPath, Set<Setting<?>> additionalSettings) throws IOException {
logger.info("ExtensionsManager initialized");
this.extensionsPath = extensionsPath;
this.initializedExtensions = new HashMap<String, DiscoveryExtensionNode>();
Expand All @@ -125,6 +130,11 @@ public ExtensionsManager(Path extensionsPath) throws IOException {
// will be initialized in initializeServicesAndRestHandler which is called after the Node is initialized
this.transportService = null;
this.clusterService = null;
// Settings added to extensions.yml by ExtensionAwarePlugins, such as security settings
this.additionalSettings = new HashSet<>();
if (additionalSettings != null) {
this.additionalSettings.addAll(additionalSettings);
}
this.client = null;
this.extensionTransportActionsHandler = null;

Expand Down Expand Up @@ -466,6 +476,7 @@ private ExtensionsSettings readFromExtensionsYml(Path filePath) throws IOExcepti
}
List<HashMap<String, ?>> unreadExtensions = new ArrayList<>((Collection<HashMap<String, ?>>) obj.get("extensions"));
List<Extension> readExtensions = new ArrayList<Extension>();
Set<String> additionalSettingsKeys = additionalSettings.stream().map(s -> s.getKey()).collect(Collectors.toSet());
for (HashMap<String, ?> extensionMap : unreadExtensions) {
try {
// checking to see whether any required fields are missing from extension.yml file or not
Expand Down Expand Up @@ -500,6 +511,16 @@ private ExtensionsSettings readFromExtensionsYml(Path filePath) throws IOExcepti
}
}

ExtensionScopedSettings extAdditionalSettings = new ExtensionScopedSettings(additionalSettings);
Map<String, ?> additionalSettingsMap = extensionMap.entrySet()
.stream()
.filter(kv -> additionalSettingsKeys.contains(kv.getKey()))
.collect(Collectors.toMap(map -> map.getKey(), map -> map.getValue()));

Settings.Builder output = Settings.builder();
output.loadFromMap(additionalSettingsMap);
extAdditionalSettings.applySettings(output.build());

// Create extension read from yml config
readExtensions.add(
new Extension(
Expand All @@ -510,7 +531,8 @@ private ExtensionsSettings readFromExtensionsYml(Path filePath) throws IOExcepti
extensionMap.get("version").toString(),
extensionMap.get("opensearchVersion").toString(),
extensionMap.get("minimumCompatibleVersion").toString(),
extensionDependencyList
extensionDependencyList,
extAdditionalSettings
)
);
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public static class Extension {
private String opensearchVersion;
private String minimumCompatibleVersion;
private List<ExtensionDependency> dependencies = Collections.emptyList();
private ExtensionScopedSettings additionalSettings;

public Extension(
String name,
Expand All @@ -53,7 +54,8 @@ public Extension(
String version,
String opensearchVersion,
String minimumCompatibleVersion,
List<ExtensionDependency> dependencies
List<ExtensionDependency> dependencies,
ExtensionScopedSettings additionalSettings
) {
this.name = name;
this.uniqueId = uniqueId;
Expand All @@ -63,6 +65,7 @@ public Extension(
this.opensearchVersion = opensearchVersion;
this.minimumCompatibleVersion = minimumCompatibleVersion;
this.dependencies = dependencies;
this.additionalSettings = additionalSettings;
}

public Extension() {
Expand Down Expand Up @@ -127,6 +130,10 @@ public List<ExtensionDependency> getDependencies() {
return dependencies;
}

public ExtensionScopedSettings getAdditionalSettings() {
return additionalSettings;
}

public String getMinimumCompatibleVersion() {
return minimumCompatibleVersion;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.io.IOException;
import java.nio.file.Path;
import java.util.Optional;
import java.util.Set;

import org.opensearch.action.ActionModule;
import org.opensearch.client.node.NodeClient;
Expand All @@ -31,7 +32,7 @@
public class NoopExtensionsManager extends ExtensionsManager {

public NoopExtensionsManager() throws IOException {
super(Path.of(""));
super(Path.of(""), Set.of());
}

@Override
Expand Down
9 changes: 8 additions & 1 deletion server/src/main/java/org/opensearch/node/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import org.opensearch.extensions.NoopExtensionsManager;
import org.opensearch.monitor.fs.FsInfo;
import org.opensearch.monitor.fs.FsProbe;
import org.opensearch.plugins.ExtensionAwarePlugin;
import org.opensearch.plugins.SearchPipelinePlugin;
import org.opensearch.search.backpressure.SearchBackpressureService;
import org.opensearch.search.backpressure.settings.SearchBackpressureSettings;
Expand Down Expand Up @@ -234,6 +235,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -466,7 +468,12 @@ protected Node(
final IdentityService identityService = new IdentityService(settings, identityPlugins);

if (FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)) {
this.extensionsManager = new ExtensionsManager(initialEnvironment.extensionDir());
final List<ExtensionAwarePlugin> extensionAwarePlugins = pluginsService.filterPlugins(ExtensionAwarePlugin.class);
Set<Setting<?>> additionalSettings = new HashSet<>();
for (ExtensionAwarePlugin extAwarePlugin : extensionAwarePlugins) {
additionalSettings.addAll(extAwarePlugin.getExtensionSettings());
}
this.extensionsManager = new ExtensionsManager(initialEnvironment.extensionDir(), additionalSettings);
} else {
this.extensionsManager = new NoopExtensionsManager();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.plugins;

import org.opensearch.common.settings.Setting;

import java.util.Collections;
import java.util.List;

/**
* Plugin that provides extra settings for extensions
*
* @opensearch.experimental
*/
public interface ExtensionAwarePlugin {

/**
* Returns a list of additional {@link Setting} definitions that this plugin adds for extensions
*/
default List<Setting<?>> getExtensionSettings() {
return Collections.emptyList();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@ public interface IdentityPlugin {
*
* Should never return null
* */
public Subject getSubject();
Subject getSubject();
}

0 comments on commit 277eb3d

Please sign in to comment.