[
https://issues.apache.org/jira/browse/GEODE-3946?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16321639#comment-16321639
]
ASF GitHub Bot commented on GEODE-3946:
---------------------------------------
jinmeiliao closed pull request #1259: GEODE-3946: add version check when
connect to cluster using gfsh
URL: https://github.com/apache/geode/pull/1259
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/geode-core/src/main/java/org/apache/geode/management/MemberMXBean.java
b/geode-core/src/main/java/org/apache/geode/management/MemberMXBean.java
index 15a309c3ab..a240106803 100644
--- a/geode-core/src/main/java/org/apache/geode/management/MemberMXBean.java
+++ b/geode-core/src/main/java/org/apache/geode/management/MemberMXBean.java
@@ -286,10 +286,18 @@ String processCommand(String commandString, Map<String,
String> env,
String status();
/**
- * Returns the GemFire version.
+ * Returns the GemFire version, including build id, jdk version, product
name and release version
+ * etc.
*/
+ @ResourceOperation()
String getVersion();
+ /**
+ * returns only the version string
+ */
+ @ResourceOperation()
+ String getReleaseVersion();
+
/**
* Returns whether this member is attached to at least one Locator.
*
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBean.java
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBean.java
index b05258f03a..f068efe866 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBean.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBean.java
@@ -355,6 +355,11 @@ public String getVersion() {
return bridge.getVersion();
}
+ @Override
+ public String getReleaseVersion() {
+ return bridge.getReleaseVersion();
+ }
+
@Override
public boolean hasGatewayReceiver() {
return bridge.hasGatewayReceiver();
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java
index 0b340fd516..485e194a59 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/beans/MemberMBeanBridge.java
@@ -1786,4 +1786,8 @@ public long getFreeMemory() {
public long getUsedMemory() {
return getVMStatistic(StatsKey.VM_USED_MEMORY).longValue() / MBFactor;
}
+
+ public String getReleaseVersion() {
+ return GemFireVersion.getGemFireVersion();
+ }
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
index 789acf3213..47b6f7303c 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConnectCommand.java
@@ -64,6 +64,7 @@
import org.apache.geode.management.internal.cli.result.ResultBuilder;
import org.apache.geode.management.internal.cli.shell.Gfsh;
import org.apache.geode.management.internal.cli.shell.JmxOperationInvoker;
+import org.apache.geode.management.internal.cli.shell.OperationInvoker;
import org.apache.geode.management.internal.cli.util.ConnectionEndpoint;
import org.apache.geode.management.internal.security.ResourceConstants;
import org.apache.geode.management.internal.web.shell.HttpOperationInvoker;
@@ -157,7 +158,31 @@ public Result connect(
result = jmxConnect(gfProperties, useSsl, jmxManagerEndPoint,
locatorEndPoint, false);
}
- return result;
+ OperationInvoker invoker = gfsh.getOperationInvoker();
+ if (invoker == null || !invoker.isConnected()) {
+ return result;
+ }
+
+ String gfshVersion = gfsh.getVersion();
+ String remoteVersion = null;
+ try {
+ remoteVersion = invoker.getRemoteVersion();
+ if (remoteVersion.equalsIgnoreCase(gfshVersion)) {
+ return result;
+ }
+ } catch (Exception e) {
+ gfsh.logInfo("failed to get the the remote version.", e);
+ }
+
+ // will reach here only when remoteVersion is not available or does not
match
+ invoker.stop();
+ if (remoteVersion == null) {
+ return ResultBuilder.createUserErrorResult(
+ String.format("Cannot use a %s gfsh client to connect to this
cluster.", gfshVersion));
+ } else {
+ return ResultBuilder.createUserErrorResult(String.format(
+ "Cannot use a %s gfsh client to connect to a %s cluster.",
gfshVersion, remoteVersion));
+ }
}
/**
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/ConnectionEndpointConverter.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/ConnectionEndpointConverter.java
index ce2965cc91..208a24adc7 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/ConnectionEndpointConverter.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/ConnectionEndpointConverter.java
@@ -102,47 +102,4 @@ public boolean getAllPossibleValues(List<Completion>
completions, Class<?> targe
return completions.size() > 0;
}
-
- public static void main(String[] args) {
- ConnectionEndpointConverter cec = new ConnectionEndpointConverter();
-
- try {
- System.out.println(cec.convertFromText("halibut[2555]", null, null));
- } catch (Exception e) {
- e.printStackTrace();
- }
-
- try {
-
System.out.println(cec.convertFromText("halibut.pune.gemstone.com[2555]", null,
null));
- } catch (Exception e) {
- e.printStackTrace();
- }
-
- try {
- System.out.println(cec.convertFromText("halibut[]", null, null));
- } catch (Exception e) {
- e.printStackTrace();
- }
-
- try {
- System.out.println(cec.convertFromText("halibut2555]", null, null));
- } catch (Exception e) {
- e.printStackTrace();
- }
-
- try {
- System.out.println(cec.convertFromText("halibut[", null, null));
- } catch (Exception e) {
- e.printStackTrace();
- }
-
- try {
- System.out.println(cec.convertFromText("halibut", null, null));
- } catch (Exception e) {
- e.printStackTrace();
- }
-
-
- }
-
}
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
index 963ce75862..538019bca2 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/JmxOperationInvoker.java
@@ -212,6 +212,10 @@ public Object getAttribute(String resourceName, String
attributeName)
}
}
+ public String getRemoteVersion() {
+ return memberMXBeanProxy.getReleaseVersion();
+ }
+
@Override
public Object invoke(String resourceName, String operationName, Object[]
params,
String[] signature) throws JMXInvocationException {
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/OperationInvoker.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/OperationInvoker.java
index cf235e9049..44ab84a83d 100755
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/OperationInvoker.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/OperationInvoker.java
@@ -129,6 +129,8 @@ public Object invoke(String resourceName, String
operationName, Object[] params,
*/
public Set<ObjectName> queryNames(ObjectName objectName, QueryExp
queryExpression);
+ public String getRemoteVersion();
+
/**
* Processes the requested command. Sends the command to the GemFire Manager
for remote processing
* (execution). NOTE refactoring return type in favor of covariant return
types.
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java
b/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java
index 39b075368a..6b2d5f931e 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ShellCommandsController.java
@@ -133,6 +133,12 @@ public String fullVersion() {
return GemFireVersion.asString();
}
+ @RequestMapping(method = RequestMethod.GET, value = "/version/release")
+ @ResponseBody
+ public String releaseVersion() {
+ return GemFireVersion.getGemFireVersion();
+ }
+
private ResponseEntity<InputStreamResource> getResponse(String result) {
CommandResult commandResult = ResultBuilder.fromJson(result);
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/HttpOperationInvoker.java
b/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/HttpOperationInvoker.java
index 056cf55142..88f7452132 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/HttpOperationInvoker.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/web/shell/HttpOperationInvoker.java
@@ -405,6 +405,11 @@ public String toString() {
return String.format("GemFire Manager HTTP service @ %1$s", httpRequester);
}
+ public String getRemoteVersion() {
+ final URI link = HttpRequester.createURI(baseUrl, "/version/release");
+ return httpRequester.get(link, String.class);
+ }
+
/**
* Processes the requested command. Sends the command to the GemFire Manager
for remote processing
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandDUnitTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandDUnitTest.java
new file mode 100644
index 0000000000..689ac59e29
--- /dev/null
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandDUnitTest.java
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.cli.commands;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Properties;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.test.dunit.rules.ClusterStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.dunit.standalone.VersionManager;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+
+
+@Category(DistributedTest.class)
+public class ConnectCommandDUnitTest {
+
+ @Rule
+ public ClusterStartupRule cluster = new ClusterStartupRule();
+
+ @Rule
+ public GfshCommandRule gfsh = new GfshCommandRule();
+
+ @Test
+ public void useCurrentGfshToConnectToOlderLocator() throws Exception {
+ MemberVM locator = cluster.startLocatorVM(0, new Properties(),
VersionManager.GEODE_130);
+
+ gfsh.executeAndAssertThat("connect --jmx-manager=localhost[" +
locator.getJmxPort() + "]")
+ .statusIsError().containsOutput("Cannot use a")
+ .containsOutput("gfsh client to connect to this cluster");
+ assertThat(gfsh.getGfsh().getOperationInvoker().isConnected()).isFalse();
+ }
+}
diff --git
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandTest.java
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandTest.java
index 084d5ffafc..7d6eecf085 100644
---
a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandTest.java
+++
b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ConnectCommandTest.java
@@ -40,6 +40,7 @@
import org.junit.experimental.categories.Category;
import org.mockito.ArgumentCaptor;
+import org.apache.geode.management.cli.Result;
import org.apache.geode.management.internal.cli.i18n.CliStrings;
import org.apache.geode.management.internal.cli.result.CommandResult;
import org.apache.geode.management.internal.cli.shell.Gfsh;
@@ -56,6 +57,7 @@
private ConnectCommand connectCommand;
private Gfsh gfsh;
private CommandResult result;
+ private OperationInvoker operationInvoker;
private Properties properties;
private ArgumentCaptor<File> fileCaptor;
@@ -63,12 +65,13 @@
public void before() throws Exception {
properties = new Properties();
gfsh = mock(Gfsh.class);
- when(gfsh.getOperationInvoker()).thenReturn(mock(OperationInvoker.class));
+ operationInvoker = mock(OperationInvoker.class);
+ when(gfsh.getOperationInvoker()).thenReturn(operationInvoker);
// using spy instead of mock because we want to call the real method when
we do connect
connectCommand = spy(ConnectCommand.class);
when(connectCommand.getGfsh()).thenReturn(gfsh);
doReturn(properties).when(connectCommand).loadProperties(any());
- CommandResult result = mock(CommandResult.class);
+ result = mock(CommandResult.class);
when(connectCommand.httpConnect(any(), any(),
anyBoolean())).thenReturn(result);
when(connectCommand.jmxConnect(any(), anyBoolean(), any(), any(),
anyBoolean()))
.thenReturn(result);
@@ -297,4 +300,36 @@ public void resolveSslProperties() throws Exception {
assertThat(properties.getProperty(SSL_KEYSTORE)).isEqualTo("keystore2");
assertThat(properties.getProperty(SSL_KEYSTORE_PASSWORD)).isEqualTo("password");
}
+
+ @Test
+ public void connectToManagerWithDifferentVersion() {
+ when(gfsh.getVersion()).thenReturn("1.2");
+ when(operationInvoker.getRemoteVersion()).thenReturn("1.3");
+ when(operationInvoker.isConnected()).thenReturn(true);
+ gfshParserRule.executeAndAssertThat(connectCommand, "connect
--locator=localhost:4040")
+ .statusIsError()
+ .containsOutput("Cannot use a 1.2 gfsh client to connect to a 1.3
cluster.");
+ }
+
+ @Test
+ public void connectToOlderManagerWithNewerGfsh() {
+ when(gfsh.getVersion()).thenReturn("1.5");
+ when(operationInvoker.getRemoteVersion())
+ .thenThrow(new RuntimeException("release version not available"));
+ when(operationInvoker.isConnected()).thenReturn(true);
+
+ gfshParserRule.executeAndAssertThat(connectCommand, "connect
--locator=localhost:4040")
+ .statusIsError().containsOutput("Cannot use a 1.5 gfsh client to
connect to this cluster.");
+ }
+
+ @Test
+ public void connectToAValidManager() {
+ when(gfsh.getVersion()).thenReturn("1.5");
+ when(operationInvoker.getRemoteVersion()).thenReturn("1.5");
+ when(operationInvoker.isConnected()).thenReturn(true);
+
+ when(result.getStatus()).thenReturn(Result.Status.OK);
+ gfshParserRule.executeAndAssertThat(connectCommand, "connect
--locator=localhost:4040")
+ .statusIsSuccess();
+ }
}
diff --git
a/geode-core/src/test/java/org/apache/geode/test/dunit/standalone/VersionManager.java
b/geode-core/src/test/java/org/apache/geode/test/dunit/standalone/VersionManager.java
index 6cf7bd0243..f13718b748 100755
---
a/geode-core/src/test/java/org/apache/geode/test/dunit/standalone/VersionManager.java
+++
b/geode-core/src/test/java/org/apache/geode/test/dunit/standalone/VersionManager.java
@@ -38,6 +38,9 @@
*/
public class VersionManager {
public static final String CURRENT_VERSION = "000";
+ public static final String GEODE_110 = "110";
+ public static final String GEODE_120 = "120";
+ public static final String GEODE_130 = "130";
private static VersionManager instance;
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Attempting to connect an older version gfsh to a newer version JMX manager
> should fail
> --------------------------------------------------------------------------------------
>
> Key: GEODE-3946
> URL: https://issues.apache.org/jira/browse/GEODE-3946
> Project: Geode
> Issue Type: Bug
> Components: gfsh
> Reporter: Barry Oglesby
> Assignee: Kenneth Howe
> Priority: Trivial
>
> Currently, an older version of gfsh can connect to a newer version JMX
> manager, but when a command is invoked, it'll fail with a cryptic message.
> An example is:
> 9.1.1 JMX manager
> 9.0.3 gfsh
> Attempting to execute a query with this scenario logs a 'Could not parse
> command string' message:
> {noformat}
> gfsh>query --query='SELECT sauce FROM /data' --interactive=true
> Could not parse command string. query --query='SELECT sauce FROM /data'
> --interactive=true --step-name=SELECT_EXEC
> {noformat}
> Instead, gfsh should fail at connect time with an
> {{IncompatibleVersionException}} or something similar.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
