Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Extensions] Add ExtensionAwarePlugin extension point to add custom settings for extensions #7526

Merged
merged 25 commits into from
May 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
f40c25c
WIP on extension settings
cwperks May 11, 2023
cf826b3
Merge branch 'main' into get-extension-settings
cwperks May 11, 2023
0acc450
Use getExtensionSettings from the identity service
cwperks May 11, 2023
9717fd6
Add extension scoped settings and add area for additional settings
cwperks May 11, 2023
72bb099
Run spotlessApply
cwperks May 11, 2023
531447f
Re-run CI
cwperks May 11, 2023
23d19e8
spotlessApply
cwperks May 11, 2023
941ef40
Merge branch 'main' into get-extension-settings
cwperks May 11, 2023
b75b3e3
Merge branch 'main' into get-extension-settings
cwperks May 12, 2023
6960029
Merge branch 'main' into get-extension-settings
cwperks May 18, 2023
b74d523
Change contructor to take list of additionalSettings
cwperks May 18, 2023
5b72fc6
One constructor
cwperks May 18, 2023
8229628
Remove isAuthenticated
cwperks May 18, 2023
2203259
Merge branch 'main' into get-extension-settings
cwperks May 19, 2023
11fa440
Re-run CI
cwperks May 19, 2023
4ba38fa
Re-run CI
cwperks May 19, 2023
1fe5ea6
Create ExtensionAwarePlugin extension point
cwperks May 23, 2023
6dfa284
Update CHANGELOG
cwperks May 23, 2023
ba92b06
Merge branch 'main' into get-extension-settings
cwperks May 23, 2023
689ef48
Address code review feedback
cwperks May 23, 2023
e588b22
Compute additionalSettingsKeys outside of loop
cwperks May 23, 2023
c53852c
Merge branch 'main' into get-extension-settings
cwperks May 23, 2023
1d1273f
Address code review feedback
cwperks May 23, 2023
e7ed02b
Merge branch 'main' into get-extension-settings
cwperks May 23, 2023
9aa929c
Add comment
cwperks May 23, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,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 {
cwperks marked this conversation as resolved.
Show resolved Hide resolved
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<>();
owaiskazi19 marked this conversation as resolved.
Show resolved Hide resolved
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 @@
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<>();

Check warning on line 472 in server/src/main/java/org/opensearch/node/Node.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/node/Node.java#L471-L472

Added lines #L471 - L472 were not covered by tests
for (ExtensionAwarePlugin extAwarePlugin : extensionAwarePlugins) {
additionalSettings.addAll(extAwarePlugin.getExtensionSettings());
}
this.extensionsManager = new ExtensionsManager(initialEnvironment.extensionDir(), additionalSettings);

Check warning on line 476 in server/src/main/java/org/opensearch/node/Node.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/node/Node.java#L474-L476

Added lines #L474 - L476 were not covered by tests
} 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
owaiskazi19 marked this conversation as resolved.
Show resolved Hide resolved
*/
public interface ExtensionAwarePlugin {
owaiskazi19 marked this conversation as resolved.
Show resolved Hide resolved

/**
* Returns a list of additional {@link Setting} definitions that this plugin adds for extensions
*/
default List<Setting<?>> getExtensionSettings() {
return Collections.emptyList();

Check warning on line 27 in server/src/main/java/org/opensearch/plugins/ExtensionAwarePlugin.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/plugins/ExtensionAwarePlugin.java#L27

Added line #L27 was not covered by tests
}
}
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();
}