[Remove] MainResponse version override cluster setting (#3031) (#3033)

With types removal OpenSearch 2.0.0 no longer supports HLRC compatibility with legacy clients. This commit removes all 1.x logic to spoof the version as a legacy cluster. Signed-off-by: Nicholas Walter Knize <[email protected]> (cherry picked from commit cf78065)
opensearch-project · Apr 21, 2022 · 9714f67 · 9714f67
1 parent e5e8369
commit 9714f67
Show file tree

Hide file tree

Showing 6 changed files with 5 additions and 154 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
@@ -33,7 +33,6 @@
 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;
@@ -63,25 +62,4 @@ 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<String, Object> infoAsMap = entityAsMap(adminClient().performRequest(new Request(HttpGet.METHOD_NAME, "/")));
-        @SuppressWarnings("unchecked")
-        Map<String, Object> versionMap = (Map<String, Object>) 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
@@ -55,7 +55,6 @@ 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() {}
@@ -70,7 +69,6 @@ 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) {
@@ -79,16 +77,6 @@ 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() {
@@ -111,18 +99,10 @@ public Build getBuild() {
         return build;
     }
 
-    public String getVersionNumber() {
-        return versionNumber;
-    }
-
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeString(nodeName);
-        if (out.getVersion().before(Version.V_1_0_0)) {
-            Version.writeVersion(LegacyESVersion.V_7_10_2, out);
-        } else {
-            Version.writeVersion(version, out);
-        }
+        Version.writeVersion(version, out);
         clusterName.writeTo(out);
         out.writeString(clusterUuid);
         Build.writeBuild(build, out);
@@ -137,11 +117,9 @@ 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");
-        if (isCompatibilityModeDisabled()) {
-            builder.field("distribution", build.getDistribution());
-        }
-        builder.field("number", versionNumber)
+        builder.startObject("version")
+            .field("distribution", build.getDistribution())
+            .field("number", build.getQualifiedVersion())
             .field("build_type", build.type().displayName())
             .field("build_hash", build.hash())
             .field("build_date", build.date())
@@ -155,12 +133,6 @@ 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<MainResponse, Void> PARSER = new ObjectParser<>(
         MainResponse.class.getName(),
         true,
@@ -189,7 +161,6 @@ private boolean isCompatibilityModeDisabled() {
             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
@@ -33,40 +33,22 @@
 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;
 import org.opensearch.action.support.HandledTransportAction;
 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;
 import org.opensearch.transport.TransportService;
 
 public class TransportMainAction extends HandledTransportAction<MainRequest, MainResponse> {
 
-    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<Boolean> 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(
@@ -78,32 +60,13 @@ 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<MainResponse> listener) {
         ClusterState clusterState = clusterService.state();
         listener.onResponse(
-            new MainResponse(
-                nodeName,
-                Version.CURRENT,
-                clusterState.getClusterName(),
-                clusterState.metadata().clusterUUID(),
-                Build.CURRENT,
-                responseVersion
-            )
+            new MainResponse(nodeName, Version.CURRENT, clusterState.getClusterName(), clusterState.metadata().clusterUUID(), Build.CURRENT)
         );
     }
 }
diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java
@@ -32,7 +32,6 @@
 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;
@@ -552,7 +551,6 @@ 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
@@ -32,7 +32,6 @@
 
 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;
@@ -56,7 +55,6 @@
 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 {
 
@@ -130,45 +128,4 @@ 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<MainResponse> responseRef = new AtomicReference<>();
-        action.doExecute(mock(Task.class), new MainRequest(), new ActionListener<MainResponse>() {
-            @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
@@ -138,22 +138,6 @@ 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();