Skip to content

Commit

Permalink
Make configPath unable to update at runtime (#6733)
Browse files Browse the repository at this point in the history
* Make configPath unable to update at runtime

* Make configPath unable to update at runtime

* Add ASF header

* Add ASF header

* Pass the check style

* Polish the response code

* Polish the response code
  • Loading branch information
RongtongJin committed May 11, 2023
1 parent 5dc2e20 commit 9d411cf
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -775,13 +775,21 @@ private synchronized RemotingCommand updateBrokerConfig(ChannelHandlerContext ct
String bodyStr = new String(body, MixAll.DEFAULT_CHARSET);
Properties properties = MixAll.string2Properties(bodyStr);
if (properties != null) {
LOGGER.info("updateBrokerConfig, new config: [{}] client: {} ", properties, ctx.channel().remoteAddress());
LOGGER.info("updateBrokerConfig, new config: [{}] client: {} ", properties, callerAddress);

if (properties.containsKey("brokerConfigPath")) {
response.setCode(ResponseCode.NO_PERMISSION);
response.setRemark("Can not update config path");
return response;
}

this.brokerController.getConfiguration().update(properties);
if (properties.containsKey("brokerPermission")) {
long stateMachineVersion = brokerController.getMessageStore() != null ? brokerController.getMessageStore().getStateMachineVersion() : 0;
this.brokerController.getTopicConfigManager().getDataVersion().nextVersion(stateMachineVersion);
this.brokerController.registerBrokerAll(false, false, true);
}

} else {
LOGGER.error("string2Properties error");
response.setCode(ResponseCode.SYSTEM_ERROR);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@
import java.net.SocketAddress;
import java.net.UnknownHostException;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.LongAdder;
Expand Down Expand Up @@ -276,6 +278,36 @@ public void testGetBrokerConfig() throws Exception {
assertThat(response.getCode()).isEqualTo(ResponseCode.SUCCESS);
}

@Test
public void testProcessRequest_UpdateConfigPath() throws RemotingCommandException {
final RemotingCommand updateConfigRequest = RemotingCommand.createRequestCommand(RequestCode.UPDATE_BROKER_CONFIG, null);
Properties properties = new Properties();

ChannelHandlerContext ctx = mock(ChannelHandlerContext.class);
when(ctx.channel()).thenReturn(null);

// Update allowed value
properties.setProperty("allAckInSyncStateSet", "true");
updateConfigRequest.setBody(MixAll.properties2String(properties).getBytes(StandardCharsets.UTF_8));

RemotingCommand response = adminBrokerProcessor.processRequest(ctx, updateConfigRequest);

assertThat(response).isNotNull();
assertThat(response.getCode()).isEqualTo(ResponseCode.SUCCESS);

//update disallowed value
properties.clear();
properties.setProperty("brokerConfigPath", "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 path");

}

@Test
public void testSearchOffsetByTimestamp() throws Exception {
messageStore = mock(MessageStore.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,12 @@ private RemotingCommand handleUpdateControllerConfig(ChannelHandlerContext ctx,
return response;
}

if (properties.containsKey("configStorePath")) {
response.setCode(ResponseCode.NO_PERMISSION);
response.setRemark("Can not update config path");
return response;
}

this.controllerManager.getConfiguration().update(properties);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* 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:https://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.rocketmq.controller;

import java.nio.charset.StandardCharsets;
import java.util.Properties;
import org.apache.rocketmq.common.ControllerConfig;
import org.apache.rocketmq.common.MixAll;
import org.apache.rocketmq.controller.processor.ControllerRequestProcessor;
import org.apache.rocketmq.remoting.netty.NettyClientConfig;
import org.apache.rocketmq.remoting.netty.NettyServerConfig;
import org.apache.rocketmq.remoting.protocol.RemotingCommand;
import org.apache.rocketmq.remoting.protocol.RequestCode;
import org.apache.rocketmq.remoting.protocol.ResponseCode;
import org.junit.Before;
import org.junit.Test;

import static org.assertj.core.api.Assertions.assertThat;

public class ControllerRequestProcessorTest {

private ControllerRequestProcessor controllerRequestProcessor;

@Before
public void init() throws Exception {
controllerRequestProcessor = new ControllerRequestProcessor(new ControllerManager(new ControllerConfig(), new NettyServerConfig(), new NettyClientConfig()));
}

@Test
public void testProcessRequest_UpdateConfigPath() throws Exception {
final RemotingCommand updateConfigRequest = RemotingCommand.createRequestCommand(RequestCode.UPDATE_CONTROLLER_CONFIG, null);
Properties properties = new Properties();

// Update allowed value
properties.setProperty("notifyBrokerRoleChanged", "true");
updateConfigRequest.setBody(MixAll.properties2String(properties).getBytes(StandardCharsets.UTF_8));

RemotingCommand response = controllerRequestProcessor.processRequest(null, updateConfigRequest);

assertThat(response).isNotNull();
assertThat(response.getCode()).isEqualTo(ResponseCode.SUCCESS);

// Update disallowed value
properties.clear();
properties.setProperty("configStorePath", "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 path");

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,12 @@ private RemotingCommand updateConfig(ChannelHandlerContext ctx, RemotingCommand
return response;
}

if (properties.containsKey("kvConfigPath") || properties.containsKey("configStorePathName")) {
response.setCode(ResponseCode.NO_PERMISSION);
response.setRemark("Can not update config path");
return response;
}

this.namesrvController.getConfiguration().update(properties);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.net.InetSocketAddress;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Map;
import java.util.Properties;
import java.util.concurrent.ConcurrentHashMap;
import org.apache.rocketmq.common.MixAll;
import org.apache.rocketmq.common.TopicConfig;
import org.apache.rocketmq.common.UtilAll;
import org.apache.rocketmq.common.namesrv.NamesrvConfig;
Expand Down Expand Up @@ -85,7 +88,6 @@ public void init() throws Exception {

clientRequestProcessor = new ClientRequestProcessor(namesrvController);


registerRouteInfoManager();

logger = mock(Logger.class);
Expand Down Expand Up @@ -178,6 +180,43 @@ public void testProcessRequest_UnSupportedRequest() throws RemotingCommandExcept
assertThat(response.getCode()).isEqualTo(ResponseCode.REQUEST_CODE_NOT_SUPPORTED);
}

@Test
public void testProcessRequest_UpdateConfigPath() throws RemotingCommandException {
final RemotingCommand updateConfigRequest = RemotingCommand.createRequestCommand(RequestCode.UPDATE_NAMESRV_CONFIG, null);
Properties properties = new Properties();

// Update allowed value
properties.setProperty("enableTopicList", "true");
updateConfigRequest.setBody(MixAll.properties2String(properties).getBytes(StandardCharsets.UTF_8));

RemotingCommand response = defaultRequestProcessor.processRequest(null, updateConfigRequest);

assertThat(response).isNotNull();
assertThat(response.getCode()).isEqualTo(ResponseCode.SUCCESS);

//update disallowed value
properties.clear();
properties.setProperty("configStorePathName", "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 path");

//update disallowed values
properties.clear();
properties.setProperty("kvConfigPath", "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 path");
}

@Test
public void testProcessRequest_RegisterBroker() throws RemotingCommandException,
NoSuchFieldException, IllegalAccessException {
Expand Down

0 comments on commit 9d411cf

Please sign in to comment.