This is an automated email from the ASF dual-hosted git repository.
jinrongtong pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/rocketmq.git
The following commit(s) were added to refs/heads/develop by this push:
new 9cfe724e6a Add validation in broker/namesrv configure updating command
(#7584)
9cfe724e6a is described below
commit 9cfe724e6a188ea444c90ee00f2453da1b807bfa
Author: dinglei <libya_...@163.com>
AuthorDate: Tue Nov 28 10:04:17 2023 +0800
Add validation in broker/namesrv configure updating command (#7584)
* Add validation for keys in black list in mqadmin command.
* Cancel validation for keys in black list in putKV command.
---
.../broker/processor/AdminBrokerProcessor.java | 25 +++++++++++++++---
.../broker/processor/AdminBrokerProcessorTest.java | 12 ++++++++-
.../org/apache/rocketmq/common/BrokerConfig.java | 11 ++++++++
.../apache/rocketmq/common/ControllerConfig.java | 11 ++++++++
.../rocketmq/common/namesrv/NamesrvConfig.java | 10 ++++++++
.../processor/ControllerRequestProcessor.java | 27 +++++++++++++++----
.../controller/ControllerRequestProcessorTest.java | 23 ++++++++++++++++-
.../namesrv/processor/DefaultRequestProcessor.java | 30 +++++++++++++++++++---
.../namesrv/processor/RequestProcessorTest.java | 15 +++++++++--
9 files changed, 149 insertions(+), 15 deletions(-)
diff --git
a/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
b/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
index 863b275d1f..978c2e81d0 100644
---
a/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
+++
b/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
@@ -25,6 +25,7 @@ import java.io.UnsupportedEncodingException;
import java.net.UnknownHostException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
@@ -193,9 +194,19 @@ import static
org.apache.rocketmq.remoting.protocol.RemotingCommand.buildErrorRe
public class AdminBrokerProcessor implements NettyRequestProcessor {
private static final Logger LOGGER =
LoggerFactory.getLogger(LoggerName.BROKER_LOGGER_NAME);
protected final BrokerController brokerController;
+ protected Set<String> configBlackList = new HashSet<>();
public AdminBrokerProcessor(final BrokerController brokerController) {
this.brokerController = brokerController;
+ initConfigBlackList();
+ }
+
+ private void initConfigBlackList() {
+ configBlackList.add("brokerConfigPath");
+ configBlackList.add("rocketmqHome");
+ configBlackList.add("configBlackList");
+ String[] configArray =
brokerController.getBrokerConfig().getConfigBlackList().split(";");
+ configBlackList.addAll(Arrays.asList(configArray));
}
@Override
@@ -919,10 +930,9 @@ public class AdminBrokerProcessor implements
NettyRequestProcessor {
Properties properties = MixAll.string2Properties(bodyStr);
if (properties != null) {
LOGGER.info("updateBrokerConfig, new config: [{}] client:
{} ", properties, callerAddress);
-
- if (properties.containsKey("brokerConfigPath")) {
+ if (validateBlackListConfigExist(properties)) {
response.setCode(ResponseCode.NO_PERMISSION);
- response.setRemark("Can not update config path");
+ response.setRemark("Can not update config in black
list.");
return response;
}
@@ -2796,4 +2806,13 @@ public class AdminBrokerProcessor implements
NettyRequestProcessor {
}
return false;
}
+
+ private boolean validateBlackListConfigExist(Properties properties) {
+ for (String blackConfig:configBlackList) {
+ if (properties.containsKey(blackConfig)) {
+ return true;
+ }
+ }
+ return false;
+ }
}
diff --git
a/broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java
b/broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java
index ec252cecea..c6b889baee 100644
---
a/broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java
+++
b/broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java
@@ -370,8 +370,18 @@ public class AdminBrokerProcessorTest {
assertThat(response).isNotNull();
assertThat(response.getCode()).isEqualTo(ResponseCode.NO_PERMISSION);
- assertThat(response.getRemark()).contains("Can not update config
path");
+ assertThat(response.getRemark()).contains("Can not update config in
black list.");
+ //update disallowed value
+ properties.clear();
+ properties.setProperty("configBlackList", "test;path");
+
updateConfigRequest.setBody(MixAll.properties2String(properties).getBytes(StandardCharsets.UTF_8));
+
+ response = adminBrokerProcessor.processRequest(ctx,
updateConfigRequest);
+
+ assertThat(response).isNotNull();
+ assertThat(response.getCode()).isEqualTo(ResponseCode.NO_PERMISSION);
+ assertThat(response.getRemark()).contains("Can not update config in
black list.");
}
@Test
diff --git a/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java
b/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java
index c186352d14..96e0f8e918 100644
--- a/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java
+++ b/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java
@@ -406,6 +406,17 @@ public class BrokerConfig extends BrokerIdentity {
private int splitRegistrationSize = 800;
+ /**
+ * Config in this black list will be not allowed to update by command.
+ * Try to update this config black list by restart process.
+ * Try to update configures in black list by restart process.
+ */
+ private String configBlackList = "configBlackList;brokerConfigPath";
+
+ public String getConfigBlackList() {
+ return configBlackList;
+ }
+
public long getMaxPopPollingSize() {
return maxPopPollingSize;
}
diff --git
a/common/src/main/java/org/apache/rocketmq/common/ControllerConfig.java
b/common/src/main/java/org/apache/rocketmq/common/ControllerConfig.java
index 1e9c80b222..55854cfd2c 100644
--- a/common/src/main/java/org/apache/rocketmq/common/ControllerConfig.java
+++ b/common/src/main/java/org/apache/rocketmq/common/ControllerConfig.java
@@ -83,6 +83,17 @@ public class ControllerConfig {
private boolean metricsInDelta = false;
+ /**
+ * Config in this black list will be not allowed to update by command.
+ * Try to update this config black list by restart process.
+ * Try to update configures in black list by restart process.
+ */
+ private String configBlackList = "configBlackList;configStorePath";
+
+ public String getConfigBlackList() {
+ return configBlackList;
+ }
+
public String getRocketmqHome() {
return rocketmqHome;
}
diff --git
a/common/src/main/java/org/apache/rocketmq/common/namesrv/NamesrvConfig.java
b/common/src/main/java/org/apache/rocketmq/common/namesrv/NamesrvConfig.java
index 5b8a6dedb7..b82d1b8f83 100644
--- a/common/src/main/java/org/apache/rocketmq/common/namesrv/NamesrvConfig.java
+++ b/common/src/main/java/org/apache/rocketmq/common/namesrv/NamesrvConfig.java
@@ -90,6 +90,16 @@ public class NamesrvConfig {
* 2. This flag does not support static topic currently.
*/
private boolean deleteTopicWithBrokerRegistration = false;
+ /**
+ * Config in this black list will be not allowed to update by command.
+ * Try to update this config black list by restart process.
+ * Try to update configures in black list by restart process.
+ */
+ private String configBlackList =
"configBlackList;configStorePath;kvConfigPath";
+
+ public String getConfigBlackList() {
+ return configBlackList;
+ }
public boolean isOrderMessageEnable() {
return orderMessageEnable;
diff --git
a/controller/src/main/java/org/apache/rocketmq/controller/processor/ControllerRequestProcessor.java
b/controller/src/main/java/org/apache/rocketmq/controller/processor/ControllerRequestProcessor.java
index 93ecbbd9dd..a8a3d25875 100644
---
a/controller/src/main/java/org/apache/rocketmq/controller/processor/ControllerRequestProcessor.java
+++
b/controller/src/main/java/org/apache/rocketmq/controller/processor/ControllerRequestProcessor.java
@@ -20,8 +20,11 @@ import com.google.common.base.Stopwatch;
import io.netty.channel.ChannelHandlerContext;
import io.opentelemetry.api.common.Attributes;
import java.io.UnsupportedEncodingException;
+import java.util.Arrays;
+import java.util.HashSet;
import java.util.List;
import java.util.Properties;
+import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;
@@ -73,12 +76,20 @@ public class ControllerRequestProcessor implements
NettyRequestProcessor {
private static final int WAIT_TIMEOUT_OUT = 5;
private final ControllerManager controllerManager;
private final BrokerHeartbeatManager heartbeatManager;
+ protected Set<String> configBlackList = new HashSet<>();
public ControllerRequestProcessor(final ControllerManager
controllerManager) {
this.controllerManager = controllerManager;
this.heartbeatManager = controllerManager.getHeartbeatManager();
+ initConfigBlackList();
+ }
+ private void initConfigBlackList() {
+ configBlackList.add("configBlackList");
+ configBlackList.add("configStorePath");
+ configBlackList.add("rocketmqHome");
+ String[] configArray =
controllerManager.getControllerConfig().getConfigBlackList().split(";");
+ configBlackList.addAll(Arrays.asList(configArray));
}
-
@Override
public RemotingCommand processRequest(ChannelHandlerContext ctx,
RemotingCommand request) throws Exception {
if (ctx != null) {
@@ -280,10 +291,9 @@ public class ControllerRequestProcessor implements
NettyRequestProcessor {
response.setRemark("string2Properties error");
return response;
}
-
- if (properties.containsKey("configStorePath")) {
+ if (validateBlackListConfigExist(properties)) {
response.setCode(ResponseCode.NO_PERMISSION);
- response.setRemark("Can not update config path");
+ response.setRemark("Can not update config in black list.");
return response;
}
@@ -319,5 +329,12 @@ public class ControllerRequestProcessor implements
NettyRequestProcessor {
public boolean rejectRequest() {
return false;
}
-
+ private boolean validateBlackListConfigExist(Properties properties) {
+ for (String blackConfig : configBlackList) {
+ if (properties.containsKey(blackConfig)) {
+ return true;
+ }
+ }
+ return false;
+ }
}
diff --git
a/controller/src/test/java/org/apache/rocketmq/controller/ControllerRequestProcessorTest.java
b/controller/src/test/java/org/apache/rocketmq/controller/ControllerRequestProcessorTest.java
index ede6ca36a4..46f86ad324 100644
---
a/controller/src/test/java/org/apache/rocketmq/controller/ControllerRequestProcessorTest.java
+++
b/controller/src/test/java/org/apache/rocketmq/controller/ControllerRequestProcessorTest.java
@@ -64,7 +64,28 @@ public class ControllerRequestProcessorTest {
assertThat(response).isNotNull();
assertThat(response.getCode()).isEqualTo(ResponseCode.NO_PERMISSION);
- assertThat(response.getRemark()).contains("Can not update config
path");
+ assertThat(response.getRemark()).contains("Can not update config in
black list.");
+ // Update disallowed value
+ properties.clear();
+ properties.setProperty("rocketmqHome", "test/path");
+
updateConfigRequest.setBody(MixAll.properties2String(properties).getBytes(StandardCharsets.UTF_8));
+
+ response = controllerRequestProcessor.processRequest(null,
updateConfigRequest);
+
+ assertThat(response).isNotNull();
+ assertThat(response.getCode()).isEqualTo(ResponseCode.NO_PERMISSION);
+ assertThat(response.getRemark()).contains("Can not update config in
black list.");
+
+ // Update disallowed value
+ properties.clear();
+ properties.setProperty("configBlackList", "test;path");
+
updateConfigRequest.setBody(MixAll.properties2String(properties).getBytes(StandardCharsets.UTF_8));
+
+ response = controllerRequestProcessor.processRequest(null,
updateConfigRequest);
+
+ assertThat(response).isNotNull();
+ assertThat(response.getCode()).isEqualTo(ResponseCode.NO_PERMISSION);
+ assertThat(response.getRemark()).contains("Can not update config in
black list.");
}
}
diff --git
a/namesrv/src/main/java/org/apache/rocketmq/namesrv/processor/DefaultRequestProcessor.java
b/namesrv/src/main/java/org/apache/rocketmq/namesrv/processor/DefaultRequestProcessor.java
index 485b95c42d..2daa95b9bc 100644
---
a/namesrv/src/main/java/org/apache/rocketmq/namesrv/processor/DefaultRequestProcessor.java
+++
b/namesrv/src/main/java/org/apache/rocketmq/namesrv/processor/DefaultRequestProcessor.java
@@ -18,8 +18,11 @@ package org.apache.rocketmq.namesrv.processor;
import io.netty.channel.ChannelHandlerContext;
import java.io.UnsupportedEncodingException;
+import java.util.Arrays;
+import java.util.HashSet;
import java.util.List;
import java.util.Properties;
+import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;
import org.apache.commons.lang3.StringUtils;
import org.apache.rocketmq.common.MQVersion;
@@ -71,8 +74,20 @@ public class DefaultRequestProcessor implements
NettyRequestProcessor {
protected final NamesrvController namesrvController;
+ protected Set<String> configBlackList = new HashSet<>();
+
public DefaultRequestProcessor(NamesrvController namesrvController) {
this.namesrvController = namesrvController;
+ initConfigBlackList();
+ }
+
+ private void initConfigBlackList() {
+ configBlackList.add("configBlackList");
+ configBlackList.add("configStorePath");
+ configBlackList.add("kvConfigPath");
+ configBlackList.add("rocketmqHome");
+ String[] configArray =
namesrvController.getNamesrvConfig().getConfigBlackList().split(";");
+ configBlackList.addAll(Arrays.asList(configArray));
}
@Override
@@ -153,6 +168,7 @@ public class DefaultRequestProcessor implements
NettyRequestProcessor {
response.setRemark("namespace or key is null");
return response;
}
+
this.namesrvController.getKvConfigManager().putKVConfig(
requestHeader.getNamespace(),
requestHeader.getKey(),
@@ -623,10 +639,9 @@ public class DefaultRequestProcessor implements
NettyRequestProcessor {
response.setRemark("string2Properties error");
return response;
}
-
- if (properties.containsKey("kvConfigPath") ||
properties.containsKey("configStorePath")) {
+ if (validateBlackListConfigExist(properties)) {
response.setCode(ResponseCode.NO_PERMISSION);
- response.setRemark("Can not update config path");
+ response.setRemark("Can not update config in black list.");
return response;
}
@@ -658,4 +673,13 @@ public class DefaultRequestProcessor implements
NettyRequestProcessor {
return response;
}
+ private boolean validateBlackListConfigExist(Properties properties) {
+ for (String blackConfig : configBlackList) {
+ if (properties.containsKey(blackConfig)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
}
diff --git
a/namesrv/src/test/java/org/apache/rocketmq/namesrv/processor/RequestProcessorTest.java
b/namesrv/src/test/java/org/apache/rocketmq/namesrv/processor/RequestProcessorTest.java
index 5bdf96d9de..2b2cf62949 100644
---
a/namesrv/src/test/java/org/apache/rocketmq/namesrv/processor/RequestProcessorTest.java
+++
b/namesrv/src/test/java/org/apache/rocketmq/namesrv/processor/RequestProcessorTest.java
@@ -203,7 +203,7 @@ public class RequestProcessorTest {
assertThat(response).isNotNull();
assertThat(response.getCode()).isEqualTo(ResponseCode.NO_PERMISSION);
- assertThat(response.getRemark()).contains("Can not update config
path");
+ assertThat(response.getRemark()).contains("Can not update config in
black list.");
//update disallowed values
properties.clear();
@@ -214,7 +214,18 @@ public class RequestProcessorTest {
assertThat(response).isNotNull();
assertThat(response.getCode()).isEqualTo(ResponseCode.NO_PERMISSION);
- assertThat(response.getRemark()).contains("Can not update config
path");
+ assertThat(response.getRemark()).contains("Can not update config in
black list");
+
+ //update disallowed values
+ properties.clear();
+ properties.setProperty("configBlackList", "test;path");
+
updateConfigRequest.setBody(MixAll.properties2String(properties).getBytes(StandardCharsets.UTF_8));
+
+ response = defaultRequestProcessor.processRequest(null,
updateConfigRequest);
+
+ assertThat(response).isNotNull();
+ assertThat(response.getCode()).isEqualTo(ResponseCode.NO_PERMISSION);
+ assertThat(response.getRemark()).contains("Can not update config in
black list");
}
@Test
