Skip to content

Commit

Permalink
Merge branch 'main' into metafields-index-service
Browse files Browse the repository at this point in the history
Signed-off-by: Craig Perkins <[email protected]>
  • Loading branch information
cwperks authored Jun 27, 2024
2 parents 5cc3b1f + d9e9944 commit b71d1ed
Show file tree
Hide file tree
Showing 12 changed files with 240 additions and 20 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Apply the date histogram rewrite optimization to range aggregation ([#13865](https://github.com/opensearch-project/OpenSearch/pull/13865))
- [Writable Warm] Add composite directory implementation and integrate it with FileCache ([12782](https://github.com/opensearch-project/OpenSearch/pull/12782))
- Fix race condition while parsing derived fields from search definition ([14445](https://github.com/opensearch-project/OpenSearch/pull/14445))
- Add allowlist setting for ingest-common processors ([#14439](https://github.com/opensearch-project/OpenSearch/issues/14439))
- Add allowlist setting for ingest-common and search-pipeline-common processors ([#14439](https://github.com/opensearch-project/OpenSearch/issues/14439))

### Dependencies
- Bump `org.gradle.test-retry` from 1.5.8 to 1.5.9 ([#13442](https://github.com/opensearch-project/OpenSearch/pull/13442))
Expand All @@ -25,12 +25,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Bump `com.gradle.develocity` from 3.17.4 to 3.17.5 ([#14397](https://github.com/opensearch-project/OpenSearch/pull/14397))
- Bump `opentelemetry` from 1.36.0 to 1.39.0 ([#14457](https://github.com/opensearch-project/OpenSearch/pull/14457))
- Bump `azure-identity` from 1.11.4 to 1.13.0, Bump `msal4j` from 1.14.3 to 1.15.1, Bump `msal4j-persistence-extension` from 1.2.0 to 1.3.0 ([#14506](https://github.com/opensearch-project/OpenSearch/pull/14506))
- Bump `com.azure:azure-storage-common` from 12.21.2 to 12.25.1 ([#14517](https://github.com/opensearch-project/OpenSearch/pull/14517))

### Changed
- [Tiered Caching] Move query recomputation logic outside write lock ([#14187](https://github.com/opensearch-project/OpenSearch/pull/14187))
- unsignedLongRangeQuery now returns MatchNoDocsQuery if the lower bounds are greater than the upper bounds ([#14416](https://github.com/opensearch-project/OpenSearch/pull/14416))
- Updated the `indices.query.bool.max_clause_count` setting from being static to dynamically updateable ([#13568](https://github.com/opensearch-project/OpenSearch/pull/13568))
- Make the class CommunityIdProcessor final ([#14448](https://github.com/opensearch-project/OpenSearch/pull/14448))
- Allow @InternalApi annotation on classes not meant to be constructed outside of the OpenSearch core ([#14575](https://github.com/opensearch-project/OpenSearch/pull/14575))
- Move getMetadataFields to IndexService ([#14586](https://github.com/opensearch-project/OpenSearch/pull/14586))

### Deprecated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,4 +473,17 @@ public void testPublicApiWithProtectedInterface() {

assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR))));
}

/**
* The constructor arguments have relaxed semantics at the moment: those could be not annotated or be annotated as {@link InternalApi}
*/
public void testPublicApiConstructorAnnotatedInternalApi() {
final CompilerResult result = compile("PublicApiConstructorAnnotatedInternalApi.java", "NotAnnotated.java");
assertThat(result, instanceOf(Failure.class));

final Failure failure = (Failure) result;
assertThat(failure.diagnotics(), hasSize(2));

assertThat(failure.diagnotics(), not(hasItem(matching(Diagnostic.Kind.ERROR))));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@

package org.opensearch.common.annotation.processor;

import org.opensearch.common.annotation.PublicApi;
import org.opensearch.common.annotation.InternalApi;

@PublicApi(since = "1.0.0")
@InternalApi
public class InternalApiAnnotated {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* 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.common.annotation.processor;

import org.opensearch.common.annotation.InternalApi;
import org.opensearch.common.annotation.PublicApi;

@PublicApi(since = "1.0.0")
public class PublicApiConstructorAnnotatedInternalApi {
/**
* The constructors have relaxed semantics at the moment: those could be not annotated or be annotated as {@link InternalApi}
*/
@InternalApi
public PublicApiConstructorAnnotatedInternalApi(NotAnnotated arg) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ private Map<String, Processor.Factory> filterForAllowlistSetting(Settings settin
// Assert that no unknown processors are defined in the allowlist
final Set<String> unknownAllowlistProcessors = allowlist.stream()
.filter(p -> map.containsKey(p) == false)
.collect(Collectors.toSet());
.collect(Collectors.toUnmodifiableSet());
if (unknownAllowlistProcessors.isEmpty() == false) {
throw new IllegalArgumentException(
"Processor(s) "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,61 @@

package org.opensearch.search.pipeline.common;

import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Settings;
import org.opensearch.plugins.Plugin;
import org.opensearch.plugins.SearchPipelinePlugin;
import org.opensearch.search.pipeline.Processor;
import org.opensearch.search.pipeline.SearchPhaseResultsProcessor;
import org.opensearch.search.pipeline.SearchRequestProcessor;
import org.opensearch.search.pipeline.SearchResponseProcessor;

import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;

/**
* Plugin providing common search request/response processors for use in search pipelines.
*/
public class SearchPipelineCommonModulePlugin extends Plugin implements SearchPipelinePlugin {

static final Setting<List<String>> REQUEST_PROCESSORS_ALLOWLIST_SETTING = Setting.listSetting(
"search.pipeline.common.request.processors.allowed",
List.of(),
Function.identity(),
Setting.Property.NodeScope
);

static final Setting<List<String>> RESPONSE_PROCESSORS_ALLOWLIST_SETTING = Setting.listSetting(
"search.pipeline.common.response.processors.allowed",
List.of(),
Function.identity(),
Setting.Property.NodeScope
);

static final Setting<List<String>> SEARCH_PHASE_RESULTS_PROCESSORS_ALLOWLIST_SETTING = Setting.listSetting(
"search.pipeline.common.search.phase.results.processors.allowed",
List.of(),
Function.identity(),
Setting.Property.NodeScope
);

/**
* No constructor needed, but build complains if we don't have a constructor with JavaDoc.
*/
public SearchPipelineCommonModulePlugin() {}

@Override
public List<Setting<?>> getSettings() {
return List.of(
REQUEST_PROCESSORS_ALLOWLIST_SETTING,
RESPONSE_PROCESSORS_ALLOWLIST_SETTING,
SEARCH_PHASE_RESULTS_PROCESSORS_ALLOWLIST_SETTING
);
}

/**
* Returns a map of processor factories.
*
Expand All @@ -34,25 +71,62 @@ public SearchPipelineCommonModulePlugin() {}
*/
@Override
public Map<String, Processor.Factory<SearchRequestProcessor>> getRequestProcessors(Parameters parameters) {
return Map.of(
FilterQueryRequestProcessor.TYPE,
new FilterQueryRequestProcessor.Factory(parameters.namedXContentRegistry),
ScriptRequestProcessor.TYPE,
new ScriptRequestProcessor.Factory(parameters.scriptService),
OversampleRequestProcessor.TYPE,
new OversampleRequestProcessor.Factory()
return filterForAllowlistSetting(
REQUEST_PROCESSORS_ALLOWLIST_SETTING,
parameters.env.settings(),
Map.of(
FilterQueryRequestProcessor.TYPE,
new FilterQueryRequestProcessor.Factory(parameters.namedXContentRegistry),
ScriptRequestProcessor.TYPE,
new ScriptRequestProcessor.Factory(parameters.scriptService),
OversampleRequestProcessor.TYPE,
new OversampleRequestProcessor.Factory()
)
);
}

@Override
public Map<String, Processor.Factory<SearchResponseProcessor>> getResponseProcessors(Parameters parameters) {
return Map.of(
RenameFieldResponseProcessor.TYPE,
new RenameFieldResponseProcessor.Factory(),
TruncateHitsResponseProcessor.TYPE,
new TruncateHitsResponseProcessor.Factory(),
CollapseResponseProcessor.TYPE,
new CollapseResponseProcessor.Factory()
return filterForAllowlistSetting(
RESPONSE_PROCESSORS_ALLOWLIST_SETTING,
parameters.env.settings(),
Map.of(
RenameFieldResponseProcessor.TYPE,
new RenameFieldResponseProcessor.Factory(),
TruncateHitsResponseProcessor.TYPE,
new TruncateHitsResponseProcessor.Factory(),
CollapseResponseProcessor.TYPE,
new CollapseResponseProcessor.Factory()
)
);
}

@Override
public Map<String, Processor.Factory<SearchPhaseResultsProcessor>> getSearchPhaseResultsProcessors(Parameters parameters) {
return filterForAllowlistSetting(SEARCH_PHASE_RESULTS_PROCESSORS_ALLOWLIST_SETTING, parameters.env.settings(), Map.of());
}

private <T extends Processor> Map<String, Processor.Factory<T>> filterForAllowlistSetting(
Setting<List<String>> allowlistSetting,
Settings settings,
Map<String, Processor.Factory<T>> map
) {
if (allowlistSetting.exists(settings) == false) {
return Map.copyOf(map);
}
final Set<String> allowlist = Set.copyOf(allowlistSetting.get(settings));
// Assert that no unknown processors are defined in the allowlist
final Set<String> unknownAllowlistProcessors = allowlist.stream()
.filter(p -> map.containsKey(p) == false)
.collect(Collectors.toUnmodifiableSet());
if (unknownAllowlistProcessors.isEmpty() == false) {
throw new IllegalArgumentException(
"Processor(s) " + unknownAllowlistProcessors + " were defined in [" + allowlistSetting.getKey() + "] but do not exist"
);
}
return map.entrySet()
.stream()
.filter(e -> allowlist.contains(e.getKey()))
.collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*
* 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.search.pipeline.common;

import org.opensearch.common.settings.Settings;
import org.opensearch.env.TestEnvironment;
import org.opensearch.plugins.SearchPipelinePlugin;
import org.opensearch.test.OpenSearchTestCase;

import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.BiFunction;

public class SearchPipelineCommonModulePluginTests extends OpenSearchTestCase {

public void testRequestProcessorAllowlist() throws IOException {
final String key = SearchPipelineCommonModulePlugin.REQUEST_PROCESSORS_ALLOWLIST_SETTING.getKey();
runAllowlistTest(key, List.of(), SearchPipelineCommonModulePlugin::getRequestProcessors);
runAllowlistTest(key, List.of("filter_query"), SearchPipelineCommonModulePlugin::getRequestProcessors);
runAllowlistTest(key, List.of("script"), SearchPipelineCommonModulePlugin::getRequestProcessors);
runAllowlistTest(key, List.of("oversample", "script"), SearchPipelineCommonModulePlugin::getRequestProcessors);
runAllowlistTest(key, List.of("filter_query", "script", "oversample"), SearchPipelineCommonModulePlugin::getRequestProcessors);

final IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> runAllowlistTest(key, List.of("foo"), SearchPipelineCommonModulePlugin::getRequestProcessors)
);
assertTrue(e.getMessage(), e.getMessage().contains("foo"));
}

public void testResponseProcessorAllowlist() throws IOException {
final String key = SearchPipelineCommonModulePlugin.RESPONSE_PROCESSORS_ALLOWLIST_SETTING.getKey();
runAllowlistTest(key, List.of(), SearchPipelineCommonModulePlugin::getResponseProcessors);
runAllowlistTest(key, List.of("rename_field"), SearchPipelineCommonModulePlugin::getResponseProcessors);
runAllowlistTest(key, List.of("truncate_hits"), SearchPipelineCommonModulePlugin::getResponseProcessors);
runAllowlistTest(key, List.of("collapse", "truncate_hits"), SearchPipelineCommonModulePlugin::getResponseProcessors);
runAllowlistTest(
key,
List.of("rename_field", "truncate_hits", "collapse"),
SearchPipelineCommonModulePlugin::getResponseProcessors
);

final IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> runAllowlistTest(key, List.of("foo"), SearchPipelineCommonModulePlugin::getResponseProcessors)
);
assertTrue(e.getMessage(), e.getMessage().contains("foo"));
}

public void testSearchPhaseResultsProcessorAllowlist() throws IOException {
final String key = SearchPipelineCommonModulePlugin.SEARCH_PHASE_RESULTS_PROCESSORS_ALLOWLIST_SETTING.getKey();
runAllowlistTest(key, List.of(), SearchPipelineCommonModulePlugin::getSearchPhaseResultsProcessors);

final IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> runAllowlistTest(key, List.of("foo"), SearchPipelineCommonModulePlugin::getSearchPhaseResultsProcessors)
);
assertTrue(e.getMessage(), e.getMessage().contains("foo"));
}

private void runAllowlistTest(
String settingKey,
List<String> allowlist,
BiFunction<SearchPipelineCommonModulePlugin, SearchPipelinePlugin.Parameters, Map<String, ?>> function
) throws IOException {
final Settings settings = Settings.builder().putList(settingKey, allowlist).build();
try (SearchPipelineCommonModulePlugin plugin = new SearchPipelineCommonModulePlugin()) {
assertEquals(Set.copyOf(allowlist), function.apply(plugin, createParameters(settings)).keySet());
}
}

public void testAllowlistNotSpecified() throws IOException {
final Settings settings = Settings.EMPTY;
try (SearchPipelineCommonModulePlugin plugin = new SearchPipelineCommonModulePlugin()) {
assertEquals(Set.of("oversample", "filter_query", "script"), plugin.getRequestProcessors(createParameters(settings)).keySet());
assertEquals(
Set.of("rename_field", "truncate_hits", "collapse"),
plugin.getResponseProcessors(createParameters(settings)).keySet()
);
assertEquals(Set.of(), plugin.getSearchPhaseResultsProcessors(createParameters(settings)).keySet());
}
}

private static SearchPipelinePlugin.Parameters createParameters(Settings settings) {
return new SearchPipelinePlugin.Parameters(
TestEnvironment.newEnvironment(Settings.builder().put(settings).put("path.home", "").build()),
null,
null,
null,
() -> 0L,
(a, b) -> null,
null,
null,
$ -> {},
null
);
}
}
2 changes: 1 addition & 1 deletion plugins/repository-azure/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ dependencies {
api 'com.azure:azure-core:1.49.1'
api 'com.azure:azure-json:1.1.0'
api 'com.azure:azure-xml:1.0.0'
api 'com.azure:azure-storage-common:12.21.2'
api 'com.azure:azure-storage-common:12.25.1'
api 'com.azure:azure-core-http-netty:1.15.1'
api "io.netty:netty-codec-dns:${versions.netty}"
api "io.netty:netty-codec-socks:${versions.netty}"
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
96e2df76ce9a8fa084ae289bb59295d565f2b8d5
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ public Void run() {
// - https://github.com/Azure/azure-sdk-for-java/pull/25004
// - https://github.com/Azure/azure-sdk-for-java/pull/24374
Configuration.getGlobalConfiguration().put("AZURE_JACKSON_ADAPTER_USE_ACCESS_HELPER", "true");
// See please:
// - https://github.com/Azure/azure-sdk-for-java/issues/37464
Configuration.getGlobalConfiguration().put("AZURE_ENABLE_SHUTDOWN_HOOK_WITH_PRIVILEGE", "true");
}

public AzureStorageService(Settings settings) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ grant {
permission java.lang.RuntimePermission "accessDeclaredMembers";
permission java.lang.reflect.ReflectPermission "suppressAccessChecks";
permission java.lang.RuntimePermission "setContextClassLoader";
permission java.lang.RuntimePermission "shutdownHooks";

// azure client set Authenticator for proxy username/password
permission java.net.NetPermission "setDefaultAuthenticator";
Expand Down

0 comments on commit b71d1ed

Please sign in to comment.