From 0dc546a6634751d797cc6e93d7534defe2eddee5 Mon Sep 17 00:00:00 2001 From: JinwooHwang Date: Tue, 6 Oct 2026 08:00:32 -0400 Subject: [PATCH] GEODE-10668: Consolidate CliMetaData handling in OnlineCommandProcessor --- .../cli/remote/OnlineCommandProcessor.java | 8 +++++++- .../remote/OnlineCommandProcessorTest.java | 19 +++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java index 45f9a0d4310a..9dd2269c624f 100644 --- a/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java +++ b/geode-gfsh/src/main/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessor.java @@ -115,6 +115,13 @@ public ResultModel executeCommand(String command, Map env, } Method method = parseResult.getMethod(); + CliMetaData metaData = method.getAnnotation(CliMetaData.class); + + // shell-only commands run in the gfsh client, not on a member + if (metaData != null && metaData.shellOnly()) { + return ResultModel.createError(((GfshParseResult) parseResult).getCommandName() + + " can only be run from gfsh and is not available on a member."); + } // do general authorization check here ResourceOperation resourceOperation = method.getAnnotation(ResourceOperation.class); @@ -124,7 +131,6 @@ public ResultModel executeCommand(String command, Map env, } // this command processor does not execute commands that need fileData passed from client - CliMetaData metaData = method.getAnnotation(CliMetaData.class); if (metaData != null && metaData.isFileUploaded() && stagedFilePaths == null) { return ResultModel .createError(command + " can not be executed only from server side"); diff --git a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java index 5a1dfe708023..732b1338d650 100644 --- a/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java +++ b/geode-gfsh/src/test/java/org/apache/geode/management/internal/cli/remote/OnlineCommandProcessorTest.java @@ -19,6 +19,8 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.Properties; @@ -29,6 +31,7 @@ import org.junit.rules.TemporaryFolder; import org.apache.geode.internal.security.SecurityService; +import org.apache.geode.management.cli.Result; import org.apache.geode.management.internal.cli.CommandManager; import org.apache.geode.management.internal.cli.GfshParser; import org.apache.geode.management.internal.cli.result.model.ResultModel; @@ -74,14 +77,14 @@ public void executeStripsComments() { @Test public void executeReturnsExecutorResult() { - ResultModel commandResult = onlineCommandProcessor.executeCommand("start locator"); + ResultModel commandResult = onlineCommandProcessor.executeCommand("list members"); assertThat(commandResult).isSameAs(result); } @Test public void handlesNotAuthorizedException() { when(executor.execute(any())).thenThrow(new NotAuthorizedException("not authorized")); - assertThatThrownBy(() -> onlineCommandProcessor.executeCommand("start locator")) + assertThatThrownBy(() -> onlineCommandProcessor.executeCommand("list members")) .isInstanceOf(NotAuthorizedException.class); } @@ -94,4 +97,16 @@ public void handlesParsingError() { .contains( "The command or some options in this command may not be supported by this locator"); } + + @Test + public void shellOnlyCommandReturnsError() { + ResultModel commandResult = onlineCommandProcessor.executeCommand("echo --string=hello"); + assertThat(commandResult.getStatus()).isEqualTo(Result.Status.ERROR); + } + + @Test + public void shellOnlyCommandIsNotPassedToExecutor() { + onlineCommandProcessor.executeCommand("start locator"); + verify(executor, never()).execute(any()); + } }