You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ji...@apache.org on 2018/11/21 16:19:24 UTC
[geode] branch develop updated: GEODE-6064: redact the password in
describeConfig command (#2863)
This is an automated email from the ASF dual-hosted git repository.
jinmeiliao pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new f2e43b0 GEODE-6064: redact the password in describeConfig command (#2863)
f2e43b0 is described below
commit f2e43b03c5ad6db2b59b405f6b6fd5cf6c850257
Author: jinmeiliao <ji...@pivotal.io>
AuthorDate: Wed Nov 21 08:19:15 2018 -0800
GEODE-6064: redact the password in describeConfig command (#2863)
---
.../cli/commands/DescribeConfigCommand.java | 92 ++++++++++------------
.../GetMemberConfigInformationFunction.java | 8 +-
.../cli/commands/DescribeConfigCommandTest.java | 40 ++++++++++
3 files changed, 86 insertions(+), 54 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommand.java
index 74ba46c..fe05625 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommand.java
@@ -14,7 +14,6 @@
*/
package org.apache.geode.management.internal.cli.commands;
-import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -23,13 +22,12 @@ import java.util.TreeSet;
import org.springframework.shell.core.annotation.CliCommand;
import org.springframework.shell.core.annotation.CliOption;
-import org.apache.geode.cache.execute.FunctionInvocationTargetException;
-import org.apache.geode.cache.execute.ResultCollector;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.internal.util.ArgumentRedactor;
import org.apache.geode.management.cli.CliMetaData;
import org.apache.geode.management.cli.ConverterHint;
import org.apache.geode.management.internal.cli.domain.MemberConfigurationInfo;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
import org.apache.geode.management.internal.cli.functions.GetMemberConfigInformationFunction;
import org.apache.geode.management.internal.cli.i18n.CliStrings;
import org.apache.geode.management.internal.cli.result.model.DataResultModel;
@@ -62,62 +60,55 @@ public class DescribeConfigCommand extends InternalGfshCommand {
specifiedDefaultValue = "true") boolean hideDefaults) {
ResultModel result = new ResultModel();
- try {
- DistributedMember targetMember = null;
- if (memberNameOrId != null && !memberNameOrId.isEmpty()) {
- targetMember = getMember(memberNameOrId);
- }
+ DistributedMember targetMember = null;
- ResultCollector<?, ?> rc =
- executeFunction(getMemberConfigFunction, hideDefaults, targetMember);
- ArrayList<?> output = (ArrayList<?>) rc.getResult();
- Object obj = output.get(0);
+ if (memberNameOrId != null && !memberNameOrId.isEmpty()) {
+ targetMember = getMember(memberNameOrId);
+ }
- if (obj != null && obj instanceof MemberConfigurationInfo) {
- MemberConfigurationInfo memberConfigInfo = (MemberConfigurationInfo) obj;
+ CliFunctionResult functionResult =
+ executeFunctionAndGetFunctionResult(getMemberConfigFunction, hideDefaults, targetMember);
+ Object obj = functionResult.getResultObject();
- result
- .setHeader(CliStrings.format(CliStrings.DESCRIBE_CONFIG__HEADER__TEXT, memberNameOrId));
+ if (obj != null && obj instanceof MemberConfigurationInfo) {
+ MemberConfigurationInfo memberConfigInfo = (MemberConfigurationInfo) obj;
- List<String> jvmArgsList = memberConfigInfo.getJvmInputArguments();
- TabularResultModel jvmInputArgs = result.addTable(JVM_ARGS_SECTION);
+ result
+ .setHeader(CliStrings.format(CliStrings.DESCRIBE_CONFIG__HEADER__TEXT, memberNameOrId));
- for (String jvmArg : jvmArgsList) {
- // This redaction should be redundant, since jvmArgs should have already been redacted in
- // MemberConfigurationInfo. Still, better redundant than missing.
- jvmInputArgs.accumulate("JVM command line arguments", ArgumentRedactor.redact(jvmArg));
- }
+ List<String> jvmArgsList = memberConfigInfo.getJvmInputArguments();
+ TabularResultModel jvmInputArgs = result.addTable(JVM_ARGS_SECTION);
- addSection(API_PROPERTIES_SECTION, result, memberConfigInfo.getGfePropsSetUsingApi(),
- "GemFire properties defined using the API");
- addSection(RUNTIME_PROPERTIES_SECTION, result, memberConfigInfo.getGfePropsRuntime(),
- "GemFire properties defined at the runtime");
- addSection(FILE_PROPERTIES_SECTION, result, memberConfigInfo.getGfePropsSetFromFile(),
- "GemFire properties defined with the property file");
- addSection(DEFAULT_PROPERTIES_SECTION, result,
- memberConfigInfo.getGfePropsSetWithDefaults(),
- "GemFire properties using default values");
- addSection(CACHE_ATTRIBUTES_SECTION, result, memberConfigInfo.getCacheAttributes(),
- "Cache attributes");
-
- List<Map<String, String>> cacheServerAttributesList =
- memberConfigInfo.getCacheServerAttributes();
-
- if (cacheServerAttributesList != null && !cacheServerAttributesList.isEmpty()) {
- for (Map<String, String> cacheServerAttributes : cacheServerAttributesList) {
- addSection(CACHESERVER_ATTRIBUTES_SECTION, result, cacheServerAttributes,
- "Cache-server attributes");
- }
- }
+ for (String jvmArg : jvmArgsList) {
+ // This redaction should be redundant, since jvmArgs should have already been redacted in
+ // MemberConfigurationInfo. Still, better redundant than missing.
+ jvmInputArgs.accumulate("JVM command line arguments", ArgumentRedactor.redact(jvmArg));
}
- } catch (FunctionInvocationTargetException e) {
- result = ResultModel.createCommandProcessingError(CliStrings
- .format(CliStrings.COULD_NOT_EXECUTE_COMMAND_TRY_AGAIN, CliStrings.DESCRIBE_CONFIG));
- } catch (Exception e) {
- result = ResultModel.createError(e.getMessage());
+ addSection(API_PROPERTIES_SECTION, result, memberConfigInfo.getGfePropsSetUsingApi(),
+ "GemFire properties defined using the API");
+ addSection(RUNTIME_PROPERTIES_SECTION, result, memberConfigInfo.getGfePropsRuntime(),
+ "GemFire properties defined at the runtime");
+ addSection(FILE_PROPERTIES_SECTION, result, memberConfigInfo.getGfePropsSetFromFile(),
+ "GemFire properties defined with the property file");
+ addSection(DEFAULT_PROPERTIES_SECTION, result,
+ memberConfigInfo.getGfePropsSetWithDefaults(),
+ "GemFire properties using default values");
+ addSection(CACHE_ATTRIBUTES_SECTION, result, memberConfigInfo.getCacheAttributes(),
+ "Cache attributes");
+
+ List<Map<String, String>> cacheServerAttributesList =
+ memberConfigInfo.getCacheServerAttributes();
+
+ if (cacheServerAttributesList != null && !cacheServerAttributesList.isEmpty()) {
+ for (Map<String, String> cacheServerAttributes : cacheServerAttributesList) {
+ addSection(CACHESERVER_ATTRIBUTES_SECTION, result, cacheServerAttributes,
+ "Cache-server attributes");
+ }
+ }
}
+
return result;
}
@@ -130,7 +121,8 @@ public class DescribeConfigCommand extends InternalGfshCommand {
for (String attribute : attributes) {
String attributeValue = attrMap.get(attribute);
- dataSection.addData(attribute, attributeValue);
+ dataSection.addData(attribute,
+ ArgumentRedactor.redactArgumentIfNecessary(attribute, attributeValue));
}
}
}
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetMemberConfigInformationFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetMemberConfigInformationFunction.java
index 58f9f65..64f743e 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetMemberConfigInformationFunction.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/GetMemberConfigInformationFunction.java
@@ -34,17 +34,17 @@ import org.apache.geode.distributed.internal.InternalDistributedSystem;
import org.apache.geode.internal.ConfigSource;
import org.apache.geode.internal.cache.CacheConfig;
import org.apache.geode.internal.cache.GemFireCacheImpl;
-import org.apache.geode.internal.cache.execute.InternalFunction;
import org.apache.geode.internal.cache.ha.HARegionQueue;
import org.apache.geode.internal.util.ArgumentRedactor;
+import org.apache.geode.management.cli.CliFunction;
import org.apache.geode.management.internal.cli.domain.MemberConfigurationInfo;
-public class GetMemberConfigInformationFunction implements InternalFunction {
+public class GetMemberConfigInformationFunction extends CliFunction {
private static final long serialVersionUID = 1L;
@Override
- public void execute(FunctionContext context) {
+ public CliFunctionResult executeFunction(FunctionContext context) throws Exception {
Object argsObject = context.getArguments();
boolean hideDefaults = ((Boolean) argsObject).booleanValue();
@@ -127,7 +127,7 @@ public class GetMemberConfigInformationFunction implements InternalFunction {
memberConfigInfo.setCacheServerAttributes(cacheServerAttributesList);
- context.getResultSender().lastResult(memberConfigInfo);
+ return new CliFunctionResult(context.getMemberName(), memberConfigInfo);
}
/****
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommandTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommandTest.java
index b9e7c34..b7002cd 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommandTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DescribeConfigCommandTest.java
@@ -16,10 +16,23 @@
package org.apache.geode.management.internal.cli.commands;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Test;
+import org.apache.geode.distributed.ConfigurationProperties;
+import org.apache.geode.distributed.DistributedMember;
+import org.apache.geode.management.internal.cli.domain.MemberConfigurationInfo;
+import org.apache.geode.management.internal.cli.functions.CliFunctionResult;
import org.apache.geode.test.junit.rules.GfshParserRule;
@@ -28,8 +41,35 @@ public class DescribeConfigCommandTest {
@ClassRule
public static GfshParserRule parser = new GfshParserRule();
+ private DescribeConfigCommand command;
+
+ @Before
+ public void setUp() throws Exception {
+ command = spy(DescribeConfigCommand.class);
+ }
+
@Test
public void describeConfigWithoutMemberName() throws Exception {
assertThat(parser.parse("describe config")).isNull();
}
+
+ @Test
+ public void passwordShouldBeRedacted() {
+ MemberConfigurationInfo info = new MemberConfigurationInfo();
+ Map<String, String> properties = new HashMap<>();
+ properties.put(ConfigurationProperties.SSL_KEYSTORE, "somewhere/something");
+ properties.put(ConfigurationProperties.SSL_KEYSTORE_PASSWORD, "mySecretPassword");
+
+ info.setGfePropsSetFromFile(properties);
+ CliFunctionResult functionResult = mock(CliFunctionResult.class);
+ when(functionResult.getResultObject()).thenReturn(info);
+ doReturn(mock(DistributedMember.class)).when(command).getMember(any());
+ doReturn(functionResult).when(command).executeFunctionAndGetFunctionResult(any(), any(), any());
+
+ parser.executeAndAssertThat(command, "describe config --member=test").statusIsSuccess()
+ .hasDataSection("file-properties")
+ .hasContent()
+ .doesNotContainValue("mySecretPassword")
+ .containsValue("********");
+ }
}