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 19 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 @@ -94,6 +94,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Provide mechanism to configure XContent parsing constraints (after update to Jackson 2.15.0 and above) ([#7550](https://github.com/opensearch-project/OpenSearch/pull/7550))
- 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))
- [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 ExtensionAdditionalSettings extends AbstractScopedSettings {
cwperks marked this conversation as resolved.
Show resolved Hide resolved

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

public ExtensionAdditionalSettings(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 @@ -17,9 +17,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 @@ -39,6 +41,7 @@
import org.opensearch.cluster.ClusterSettingsResponse;
import org.opensearch.cluster.node.DiscoveryNode;
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 @@ -116,6 +119,7 @@
private CustomSettingsRequestHandler customSettingsRequestHandler;
private TransportService transportService;
private ClusterService clusterService;
private Set<Setting<?>> additionalSettings;
cwperks marked this conversation as resolved.
Show resolved Hide resolved
private Settings environmentSettings;
private AddSettingsUpdateConsumerRequestHandler addSettingsUpdateConsumerRequestHandler;
private NodeClient client;
Expand All @@ -124,9 +128,10 @@
* 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 @@ -135,6 +140,11 @@
// will be initialized in initializeServicesAndRestHandler which is called after the Node is initialized
this.transportService = null;
this.clusterService = null;
if (additionalSettings == null) {
cwperks marked this conversation as resolved.
Show resolved Hide resolved
this.additionalSettings = new HashSet<>();

Check warning on line 144 in server/src/main/java/org/opensearch/extensions/ExtensionsManager.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/extensions/ExtensionsManager.java#L144

Added line #L144 was not covered by tests
} else {
this.additionalSettings = additionalSettings;
}
this.client = null;
this.extensionTransportActionsHandler = null;

Expand Down Expand Up @@ -629,6 +639,17 @@
}
}

ExtensionAdditionalSettings extAdditionalSettings = new ExtensionAdditionalSettings(additionalSettings);
Set<String> additionalSettingsKeys = additionalSettings.stream().map(s -> s.getKey()).collect(Collectors.toSet());
reta marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -639,7 +660,8 @@
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 ExtensionAdditionalSettings 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,
ExtensionAdditionalSettings 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 ExtensionAdditionalSettings getAdditionalSettings() {
cwperks marked this conversation as resolved.
Show resolved Hide resolved
return additionalSettings;
}

public String getMinimumCompatibleVersion() {
return minimumCompatibleVersion;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.net.UnknownHostException;
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 @@ -33,7 +34,7 @@
public class NoopExtensionsManager extends ExtensionsManager {

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

@Override
Expand Down
10 changes: 9 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 @@ -465,8 +467,14 @@

final IdentityService identityService = new IdentityService(settings, identityPlugins);

final List<ExtensionAwarePlugin> extensionAwarePlugins = new ArrayList<>();
cwperks marked this conversation as resolved.
Show resolved Hide resolved
if (FeatureFlags.isEnabled(FeatureFlags.EXTENSIONS)) {
this.extensionsManager = new ExtensionsManager(initialEnvironment.extensionDir());
extensionAwarePlugins.addAll(pluginsService.filterPlugins(ExtensionAwarePlugin.class));
Set<Setting<?>> additionalSettings = new HashSet<>();

Check warning on line 473 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#L472-L473

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

Check warning on line 477 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#L475-L477

Added lines #L475 - L477 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,28 @@
/*
* 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 26 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#L26

Added line #L26 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();
}