From 8477c1bb88387a29716668ed9464a93c8208318c Mon Sep 17 00:00:00 2001 From: Nick Knize Date: Wed, 8 Jun 2022 10:02:12 -0500 Subject: [PATCH] Revert "[Remove] MainResponse version override cluster setting (#3031) (#3032)" (#3530) This reverts commit 199fbb817c45346b40c98c23a7558aff05236920. Signed-off-by: Nicholas Walter Knize (cherry picked from commit 069948a761b7410cf0eb72356ae3301a7c30445e) --- .../org/opensearch/client/PingAndInfoIT.java | 22 ++++++++++ .../opensearch/action/main/MainResponse.java | 37 ++++++++++++++-- .../action/main/TransportMainAction.java | 39 ++++++++++++++++- .../common/settings/ClusterSettings.java | 2 + .../action/main/MainActionTests.java | 43 +++++++++++++++++++ .../action/main/MainResponseTests.java | 16 +++++++ 6 files changed, 154 insertions(+), 5 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/PingAndInfoIT.java b/client/rest-high-level/src/test/java/org/opensearch/client/PingAndInfoIT.java index 09ef90cef144d..72201084570bb 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/PingAndInfoIT.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/PingAndInfoIT.java @@ -33,6 +33,7 @@ package org.opensearch.client; import org.apache.http.client.methods.HttpGet; +import org.opensearch.action.main.TransportMainAction; import org.opensearch.client.core.MainResponse; import java.io.IOException; @@ -62,4 +63,25 @@ public void testInfo() throws IOException { assertTrue(versionMap.get("number").toString().startsWith(info.getVersion().getNumber())); assertEquals(versionMap.get("lucene_version"), info.getVersion().getLuceneVersion()); } + + public void testInfo_overrideResponseVersion() throws IOException { + Request overrideResponseVersionRequest = new Request("PUT", "/_cluster/settings"); + overrideResponseVersionRequest.setOptions(expectWarnings(TransportMainAction.OVERRIDE_MAIN_RESPONSE_VERSION_DEPRECATION_MESSAGE)); + overrideResponseVersionRequest.setJsonEntity("{\"persistent\":{\"compatibility\": {\"override_main_response_version\":true}}}"); + client().performRequest(overrideResponseVersionRequest); + + MainResponse info = highLevelClient().info(RequestOptions.DEFAULT); + assertEquals("7.10.2", info.getVersion().getNumber()); + + // Set back to default version. + Request resetResponseVersionRequest = new Request("PUT", "/_cluster/settings"); + resetResponseVersionRequest.setJsonEntity("{\"persistent\":{\"compatibility\": {\"override_main_response_version\":null}}}"); + client().performRequest(resetResponseVersionRequest); + + Map infoAsMap = entityAsMap(adminClient().performRequest(new Request(HttpGet.METHOD_NAME, "/"))); + @SuppressWarnings("unchecked") + Map versionMap = (Map) infoAsMap.get("version"); + info = highLevelClient().info(RequestOptions.DEFAULT); + assertTrue(versionMap.get("number").toString().startsWith(info.getVersion().getNumber())); + } } diff --git a/server/src/main/java/org/opensearch/action/main/MainResponse.java b/server/src/main/java/org/opensearch/action/main/MainResponse.java index 691bbda512275..b567d75860019 100644 --- a/server/src/main/java/org/opensearch/action/main/MainResponse.java +++ b/server/src/main/java/org/opensearch/action/main/MainResponse.java @@ -60,6 +60,7 @@ public class MainResponse extends ActionResponse implements ToXContentObject { private ClusterName clusterName; private String clusterUuid; private Build build; + private String versionNumber; public static final String TAGLINE = "The OpenSearch Project: https://opensearch.org/"; MainResponse() {} @@ -74,6 +75,7 @@ public class MainResponse extends ActionResponse implements ToXContentObject { if (in.getVersion().before(LegacyESVersion.V_7_0_0)) { in.readBoolean(); } + versionNumber = build.getQualifiedVersion(); } public MainResponse(String nodeName, Version version, ClusterName clusterName, String clusterUuid, Build build) { @@ -82,6 +84,16 @@ public MainResponse(String nodeName, Version version, ClusterName clusterName, S this.clusterName = clusterName; this.clusterUuid = clusterUuid; this.build = build; + this.versionNumber = build.getQualifiedVersion(); + } + + public MainResponse(String nodeName, Version version, ClusterName clusterName, String clusterUuid, Build build, String versionNumber) { + this.nodeName = nodeName; + this.version = version; + this.clusterName = clusterName; + this.clusterUuid = clusterUuid; + this.build = build; + this.versionNumber = versionNumber; } public String getNodeName() { @@ -104,10 +116,18 @@ public Build getBuild() { return build; } + public String getVersionNumber() { + return versionNumber; + } + @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(nodeName); - Version.writeVersion(version, out); + if (out.getVersion().before(Version.V_1_0_0)) { + Version.writeVersion(LegacyESVersion.V_7_10_2, out); + } else { + Version.writeVersion(version, out); + } clusterName.writeTo(out); out.writeString(clusterUuid); Build.writeBuild(build, out); @@ -122,9 +142,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field("name", nodeName); builder.field("cluster_name", clusterName.value()); builder.field("cluster_uuid", clusterUuid); - builder.startObject("version") - .field("distribution", build.getDistribution()) - .field("number", build.getQualifiedVersion()) + builder.startObject("version"); + if (isCompatibilityModeDisabled()) { + builder.field("distribution", build.getDistribution()); + } + builder.field("number", versionNumber) .field("build_type", build.type().displayName()) .field("build_hash", build.hash()) .field("build_date", build.date()) @@ -138,6 +160,12 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } + private boolean isCompatibilityModeDisabled() { + // if we are not in compatibility mode (spoofing versionNumber), then + // build.getQualifiedVersion is always used. + return build.getQualifiedVersion().equals(versionNumber); + } + private static final ObjectParser PARSER = new ObjectParser<>( MainResponse.class.getName(), true, @@ -166,6 +194,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws response.version = Version.fromString( ((String) value.get("number")).replace("-SNAPSHOT", "").replaceFirst("-(alpha\\d+|beta\\d+|rc\\d+)", "") ); + response.versionNumber = response.version.toString(); }, (parser, context) -> parser.map(), new ParseField("version")); } diff --git a/server/src/main/java/org/opensearch/action/main/TransportMainAction.java b/server/src/main/java/org/opensearch/action/main/TransportMainAction.java index 2916c3dd88d49..82964d0bc55c4 100644 --- a/server/src/main/java/org/opensearch/action/main/TransportMainAction.java +++ b/server/src/main/java/org/opensearch/action/main/TransportMainAction.java @@ -33,6 +33,7 @@ package org.opensearch.action.main; import org.opensearch.Build; +import org.opensearch.LegacyESVersion; import org.opensearch.Version; import org.opensearch.action.ActionListener; import org.opensearch.action.support.ActionFilters; @@ -40,6 +41,8 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Inject; +import org.opensearch.common.logging.DeprecationLogger; +import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.node.Node; import org.opensearch.tasks.Task; @@ -52,8 +55,23 @@ */ public class TransportMainAction extends HandledTransportAction { + private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(TransportMainAction.class); + + public static final String OVERRIDE_MAIN_RESPONSE_VERSION_KEY = "compatibility.override_main_response_version"; + + public static final Setting OVERRIDE_MAIN_RESPONSE_VERSION = Setting.boolSetting( + OVERRIDE_MAIN_RESPONSE_VERSION_KEY, + false, + Setting.Property.NodeScope, + Setting.Property.Dynamic + ); + + public static final String OVERRIDE_MAIN_RESPONSE_VERSION_DEPRECATION_MESSAGE = "overriding main response version" + + " number will be removed in a future version"; + private final String nodeName; private final ClusterService clusterService; + private volatile String responseVersion; @Inject public TransportMainAction( @@ -65,13 +83,32 @@ public TransportMainAction( super(MainAction.NAME, transportService, actionFilters, MainRequest::new); this.nodeName = Node.NODE_NAME_SETTING.get(settings); this.clusterService = clusterService; + setResponseVersion(OVERRIDE_MAIN_RESPONSE_VERSION.get(settings)); + + clusterService.getClusterSettings().addSettingsUpdateConsumer(OVERRIDE_MAIN_RESPONSE_VERSION, this::setResponseVersion); + } + + private void setResponseVersion(boolean isResponseVersionOverrideEnabled) { + if (isResponseVersionOverrideEnabled) { + DEPRECATION_LOGGER.deprecate(OVERRIDE_MAIN_RESPONSE_VERSION.getKey(), OVERRIDE_MAIN_RESPONSE_VERSION_DEPRECATION_MESSAGE); + this.responseVersion = LegacyESVersion.V_7_10_2.toString(); + } else { + this.responseVersion = Build.CURRENT.getQualifiedVersion(); + } } @Override protected void doExecute(Task task, MainRequest request, ActionListener listener) { ClusterState clusterState = clusterService.state(); listener.onResponse( - new MainResponse(nodeName, Version.CURRENT, clusterState.getClusterName(), clusterState.metadata().clusterUUID(), Build.CURRENT) + new MainResponse( + nodeName, + Version.CURRENT, + clusterState.getClusterName(), + clusterState.metadata().clusterUUID(), + Build.CURRENT, + responseVersion + ) ); } } diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index be92bf1643aee..9da423cd01fe3 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -32,6 +32,7 @@ package org.opensearch.common.settings; import org.apache.logging.log4j.LogManager; +import org.opensearch.action.main.TransportMainAction; import org.opensearch.cluster.routing.allocation.decider.NodeLoadAwareAllocationDecider; import org.opensearch.index.IndexModule; import org.opensearch.index.IndexSettings; @@ -553,6 +554,7 @@ public void apply(Settings value, Settings current, Settings previous) { FsHealthService.REFRESH_INTERVAL_SETTING, FsHealthService.SLOW_PATH_LOGGING_THRESHOLD_SETTING, FsHealthService.HEALTHY_TIMEOUT_SETTING, + TransportMainAction.OVERRIDE_MAIN_RESPONSE_VERSION, NodeLoadAwareAllocationDecider.CLUSTER_ROUTING_ALLOCATION_LOAD_AWARENESS_PROVISIONED_CAPACITY_SETTING, NodeLoadAwareAllocationDecider.CLUSTER_ROUTING_ALLOCATION_LOAD_AWARENESS_SKEW_FACTOR_SETTING, NodeLoadAwareAllocationDecider.CLUSTER_ROUTING_ALLOCATION_LOAD_AWARENESS_ALLOW_UNASSIGNED_PRIMARIES_SETTING, diff --git a/server/src/test/java/org/opensearch/action/main/MainActionTests.java b/server/src/test/java/org/opensearch/action/main/MainActionTests.java index 479e36c2e13ce..3cbb6b3eb29bd 100644 --- a/server/src/test/java/org/opensearch/action/main/MainActionTests.java +++ b/server/src/test/java/org/opensearch/action/main/MainActionTests.java @@ -32,6 +32,7 @@ package org.opensearch.action.main; +import org.opensearch.LegacyESVersion; import org.opensearch.action.ActionListener; import org.opensearch.action.support.ActionFilters; import org.opensearch.cluster.ClusterName; @@ -55,6 +56,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.opensearch.action.main.TransportMainAction.OVERRIDE_MAIN_RESPONSE_VERSION_KEY; public class MainActionTests extends OpenSearchTestCase { @@ -128,4 +130,45 @@ public void onFailure(Exception e) { assertNotNull(responseRef.get()); verify(clusterService, times(1)).state(); } + + public void testMainResponseVersionOverrideEnabledByConfigSetting() { + final ClusterName clusterName = new ClusterName("opensearch"); + ClusterState state = ClusterState.builder(clusterName).blocks(mock(ClusterBlocks.class)).build(); + + final ClusterService clusterService = mock(ClusterService.class); + when(clusterService.state()).thenReturn(state); + when(clusterService.getClusterSettings()).thenReturn( + new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS) + ); + + TransportService transportService = new TransportService( + Settings.EMPTY, + mock(Transport.class), + null, + TransportService.NOOP_TRANSPORT_INTERCEPTOR, + x -> null, + null, + Collections.emptySet() + ); + + final Settings settings = Settings.builder().put("node.name", "my-node").put(OVERRIDE_MAIN_RESPONSE_VERSION_KEY, true).build(); + + TransportMainAction action = new TransportMainAction(settings, transportService, mock(ActionFilters.class), clusterService); + AtomicReference responseRef = new AtomicReference<>(); + action.doExecute(mock(Task.class), new MainRequest(), new ActionListener() { + @Override + public void onResponse(MainResponse mainResponse) { + responseRef.set(mainResponse); + } + + @Override + public void onFailure(Exception e) { + logger.error("unexpected error", e); + } + }); + + final MainResponse mainResponse = responseRef.get(); + assertEquals(LegacyESVersion.V_7_10_2.toString(), mainResponse.getVersionNumber()); + assertWarnings(TransportMainAction.OVERRIDE_MAIN_RESPONSE_VERSION_DEPRECATION_MESSAGE); + } } diff --git a/server/src/test/java/org/opensearch/action/main/MainResponseTests.java b/server/src/test/java/org/opensearch/action/main/MainResponseTests.java index d2b6ae8bcaece..b333118c4e070 100644 --- a/server/src/test/java/org/opensearch/action/main/MainResponseTests.java +++ b/server/src/test/java/org/opensearch/action/main/MainResponseTests.java @@ -138,6 +138,22 @@ public void testToXContent() throws IOException { ); } + public void toXContent_overrideMainResponseVersion() throws IOException { + String responseVersion = LegacyESVersion.V_7_10_2.toString(); + MainResponse response = new MainResponse( + "nodeName", + Version.CURRENT, + new ClusterName("clusterName"), + randomAlphaOfLengthBetween(10, 20), + Build.CURRENT, + responseVersion + ); + XContentBuilder builder = XContentFactory.jsonBuilder(); + response.toXContent(builder, ToXContent.EMPTY_PARAMS); + assertTrue(Strings.toString(builder).contains("\"number\":\"" + responseVersion + "\",")); + assertFalse(Strings.toString(builder).contains("\"distribution\":\"" + Build.CURRENT.getDistribution() + "\",")); + } + @Override protected MainResponse mutateInstance(MainResponse mutateInstance) { String clusterUuid = mutateInstance.getClusterUuid();