honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-02-24 22:09:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

Deprecate setting 'reindex.remote.whitelist' and introduce the alternative setting 'reindex.remote.allowlist' (#2221)

Browse Source 
* Add setting reindex.remote.allowlist, and deprecate setting reindex.remote.whitelist

Signed-off-by: Tianli Feng <ftianli@amazon.com>

* Add unit test for renaming the setting reindex.remote.allowlist

Signed-off-by: Tianli Feng <ftianli@amazon.com>

* Remove system.out.println()

Signed-off-by: Tianli Feng <ftianli@amazon.com>

* Adjust format by spotlessApply task

Signed-off-by: Tianli Feng <ftianli@amazon.com>

* Replace REMOTE_CLUSTER_WHITELIST with REMOTE_CLUSTER_ALLOWLIST

Signed-off-by: Tianli Feng <ftianli@amazon.com>

* Add a unit test to test final setting value when both settings have got a value

Signed-off-by: Tianli Feng <ftianli@amazon.com>

* Rename the unit test class name

Signed-off-by: Tianli Feng <ftianli@amazon.com>

* Remove the Access modifiers public from the constant REMOTE_CLUSTER_WHITELIST

Signed-off-by: Tianli Feng <ftianli@amazon.com>

* Initialize ReindexPlugin without using the @Before method

Signed-off-by: Tianli Feng <ftianli@amazon.com>

* Rename 'unwhitelisted' to 'unallowlisted' in a yml file used for REST api testing.

Signed-off-by: Tianli Feng <ftianli@amazon.com>
This commit is contained in:
Tianli Feng 2022-03-08 08:35:36 -08:00 committed by GitHub
parent 65debde436
commit 63c75d1b1d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 103 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
client/rest-high-level/build.gradle
View File

@ -92,7 +92,7 @@ check.dependsOn(asyncIntegTest)
testClusters.all {
testDistribution = 'ARCHIVE'
systemProperty 'opensearch.scripting.update.ctx_in_params', 'false'
setting 'reindex.remote.whitelist', '[ "[::1]:*", "127.0.0.1:*" ]'
setting 'reindex.remote.allowlist', '[ "[::1]:*", "127.0.0.1:*" ]'
extraConfigFile 'roles.yml', file('roles.yml')
user username: System.getProperty('tests.rest.cluster.username', 'test_user'),

2
modules/reindex/build.gradle
View File

@ -50,7 +50,7 @@ testClusters.all {
module ':modules:parent-join'
module ':modules:lang-painless'
// Allowlist reindexing from the local node so we can test reindex-from-remote.
setting 'reindex.remote.whitelist', '127.0.0.1:*'
setting 'reindex.remote.allowlist', '127.0.0.1:*'
}
test {

1
modules/reindex/src/main/java/org/opensearch/index/reindex/ReindexPlugin.java
View File

@ -133,6 +133,7 @@ public class ReindexPlugin extends Plugin implements ActionPlugin, ExtensiblePlu
public List<Setting<?>> getSettings() {
final List<Setting<?>> settings = new ArrayList<>();
settings.add(TransportReindexAction.REMOTE_CLUSTER_WHITELIST);
settings.add(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST);
settings.addAll(ReindexSslConfig.getSettings());
return settings;
}

4
modules/reindex/src/main/java/org/opensearch/index/reindex/ReindexValidator.java
View File

@ -70,7 +70,7 @@ class ReindexValidator {
IndexNameExpressionResolver resolver,
AutoCreateIndex autoCreateIndex
) {
this.remoteAllowlist = buildRemoteAllowlist(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(settings));
this.remoteAllowlist = buildRemoteAllowlist(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings));
this.clusterService = clusterService;
this.resolver = resolver;
this.autoCreateIndex = autoCreateIndex;
@ -101,7 +101,7 @@ class ReindexValidator {
if (allowlist.run(check)) {
return;
}
String allowListKey = TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getKey();
String allowListKey = TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.getKey();
throw new IllegalArgumentException('[' + check + "] not allowlisted in " + allowListKey);
}

11
modules/reindex/src/main/java/org/opensearch/index/reindex/TransportReindexAction.java
View File

@ -56,10 +56,19 @@ import java.util.function.Function;
import static java.util.Collections.emptyList;
public class TransportReindexAction extends HandledTransportAction<ReindexRequest, BulkByScrollResponse> {
public static final Setting<List<String>> REMOTE_CLUSTER_WHITELIST = Setting.listSetting(
static final Setting<List<String>> REMOTE_CLUSTER_WHITELIST = Setting.listSetting(
"reindex.remote.whitelist",
emptyList(),
Function.identity(),
Property.NodeScope,
Property.Deprecated
);
// The setting below is going to replace the above.
// To keep backwards compatibility, the old usage is remained, and it's also used as the fallback for the new usage.
public static final Setting<List<String>> REMOTE_CLUSTER_ALLOWLIST = Setting.listSetting(
"reindex.remote.allowlist",
REMOTE_CLUSTER_WHITELIST,
Function.identity(),
Property.NodeScope
);
public static Optional<RemoteReindexExtension> remoteExtension = Optional.empty();

2
modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexFromRemoteWhitelistTests.java
View File

@ -131,7 +131,7 @@ public class ReindexFromRemoteWhitelistTests extends OpenSearchTestCase {
IllegalArgumentException.class,
() -> checkRemoteAllowlist(buildRemoteAllowlist(allowlist), newRemoteInfo("not in list", port))
);
assertEquals("[not in list:" + port + "] not allowlisted in reindex.remote.whitelist", e.getMessage());
assertEquals("[not in list:" + port + "] not allowlisted in reindex.remote.allowlist", e.getMessage());
}
public void testRejectMatchAll() {

2
modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexFromRemoteWithAuthTests.java
View File

@ -99,7 +99,7 @@ public class ReindexFromRemoteWithAuthTests extends OpenSearchSingleNodeTestCase
protected Settings nodeSettings() {
Settings.Builder settings = Settings.builder().put(super.nodeSettings());
// Allowlist reindexing from the http host we're going to use
settings.put(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getKey(), "127.0.0.1:*");
settings.put(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.getKey(), "127.0.0.1:*");
settings.put(NetworkModule.HTTP_TYPE_KEY, Netty4Plugin.NETTY_HTTP_TRANSPORT_NAME);
return settings.build();
}

83
modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexRenamedSettingTests.java Normal file
View File

@ -0,0 +1,83 @@
/*
* 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.index.reindex;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Settings;
import org.opensearch.test.OpenSearchTestCase;
import java.util.Arrays;
import java.util.List;
/**
* A unit test to validate the former name of the setting 'reindex.remote.allowlist' still take effect,
* after it is deprecated, so that the backwards compatibility is maintained.
* The test can be removed along with removing support of the deprecated setting.
*/
public class ReindexRenamedSettingTests extends OpenSearchTestCase {
private final ReindexPlugin plugin = new ReindexPlugin();
/**
* Validate the both settings are known and supported.
*/
public void testReindexSettingsExist() {
List<Setting<?>> settings = plugin.getSettings();
assertTrue(
"Both 'reindex.remote.allowlist' and its predecessor should be supported settings of Reindex plugin",
settings.containsAll(
Arrays.asList(TransportReindexAction.REMOTE_CLUSTER_WHITELIST, TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST)
)
);
}
/**
* Validate the default value of the both settings is the same.
*/
public void testSettingFallback() {
assertEquals(
TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(Settings.EMPTY),
TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(Settings.EMPTY)
);
}
/**
* Validate the new setting can be configured correctly, and it doesn't impact the old setting.
*/
public void testSettingGetValue() {
Settings settings = Settings.builder().put("reindex.remote.allowlist", "127.0.0.1:*").build();
assertEquals(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), Arrays.asList("127.0.0.1:*"));
assertEquals(
TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(settings),
TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getDefault(Settings.EMPTY)
);
}
/**
* Validate the value of the old setting will be applied to the new setting, if the new setting is not configured.
*/
public void testSettingGetValueWithFallback() {
Settings settings = Settings.builder().put("reindex.remote.whitelist", "127.0.0.1:*").build();
assertEquals(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), Arrays.asList("127.0.0.1:*"));
assertSettingDeprecationsAndWarnings(new Setting<?>[] { TransportReindexAction.REMOTE_CLUSTER_WHITELIST });
}
/**
* Validate the value of the old setting will be ignored, if the new setting is configured.
*/
public void testSettingGetValueWhenBothAreConfigured() {
Settings settings = Settings.builder()
.put("reindex.remote.allowlist", "127.0.0.1:*")
.put("reindex.remote.whitelist", "[::1]:*, 127.0.0.1:*")
.build();
assertEquals(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.get(settings), Arrays.asList("127.0.0.1:*"));
assertEquals(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.get(settings), Arrays.asList("[::1]:*", "127.0.0.1:*"));
assertSettingDeprecationsAndWarnings(new Setting<?>[] { TransportReindexAction.REMOTE_CLUSTER_WHITELIST });
}
}

2
modules/reindex/src/test/java/org/opensearch/index/reindex/RetryTests.java
View File

@ -103,7 +103,7 @@ public class RetryTests extends OpenSearchIntegTestCase {
final Settings nodeSettings() {
return Settings.builder()
// allowlist reindexing from the HTTP host we're going to use
.put(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getKey(), "127.0.0.1:*")
.put(TransportReindexAction.REMOTE_CLUSTER_ALLOWLIST.getKey(), "127.0.0.1:*")
.build();
}

4
modules/reindex/src/yamlRestTest/resources/rest-api-spec/test/reindex/20_validation.yml
View File

@ -306,9 +306,9 @@
index: dest
---
"unwhitelisted remote host fails":
"unallowlisted remote host fails":
- do:
catch: /\[badremote:9200\] not allowlisted in reindex.remote.whitelist/
catch: /\[badremote:9200\] not allowlisted in reindex.remote.allowlist/
reindex:
body:
source: