[I] [Bug] mqadmin updateBrokerConfig -a false updates the slaves anyway [rocketmq]

["iloveeyjafjalla (via GitHub)" <gi...@apache.org>]

iloveeyjafjalla opened a new issue, #11385:
URL: https://github.com/apache/rocketmq/issues/11385

   ### Before Creating the Bug Report
   
   - [x] I found a bug, not just asking a question, which should be created in [GitHub Discussions](https://github.com/apache/rocketmq/discussions).
   - [x] I have searched the [GitHub Issues](https://github.com/apache/rocketmq/issues) and [GitHub Discussions](https://github.com/apache/rocketmq/discussions) of this repository and believe that this is not a duplicate.
   - [x] I have confirmed that this bug belongs to the current repository, not other repositories of RocketMQ.
   
   ### Runtime platform environment
   
   OS: any (mqadmin, `tools` module)
   
   ### RocketMQ version
   
   branch: develop
   Git commit id: 9ea2ccdc2b41445aee5e177e14e501d606f5a1ed (the command is unchanged on develop@b76420599)
   
   ### JDK Version
   
   Compiler: Temurin 1.8.0_504 (target 1.8, same as CI)
   
   ### Describe the Bug
   
   `mqadmin updateBrokerConfig -a false` updates the slaves anyway.
   
   The option is declared with the boolean as its argument, and the help says so:
   
   ```java
   opt = new Option("a", "updateAllBroker", true, "update all brokers include slave");
   ...
   if (commandLine.hasOption('a')) {
       brokerAddrSet = CommandUtil.fetchMasterAndSlaveAddrByClusterName(defaultMQAdminExt, clusterName);
   } else {
       brokerAddrSet = CommandUtil.fetchMasterAddrByClusterName(defaultMQAdminExt, clusterName);
   }
   ```
   
   `getOptionValue('a')` is never called, so any presence of the option — including the explicit `-a false` that the help's
   `<arg>` invites — selects the master+slave fan-out. A user who wants to touch only the masters but spells the flag out gets
   every broker in the cluster updated, which for role-sensitive keys (e.g. anything read at startup) means the slaves are
   configured differently from what the operator asked for. Because the option requires an argument, there is also no way to ask
   for "masters only" by writing the flag alone.
   
   ### Steps to Reproduce
   
   1. `mqadmin updateBrokerConfig -c <cluster> -a false -k <key> -v <value> -n <namesrv>`.
   2. The UPDATE_BROKER_CONFIG request is sent to the slaves of the cluster as well (see the test below, which counts the
      requests each mock broker received: the slave receives one although the flag says `false`).
   
   ### Expected behavior
   
   `-a false` (and a missing `-a`) update the masters only; `-a true` updates the masters and the slaves, as the help text
   describes.
   
   ### Actual behavior
   
   `-a false` behaves like `-a`/`-a true`: every broker of the cluster is updated.
   
   ### Additional context
   
   Fix: read the option's argument (`Boolean.parseBoolean(commandLine.getOptionValue('a'))`) instead of treating the option as a
   switch. No documentation or script in the repository passes a bare `-a`, so the strict `true|false` argument stays as
   documented.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Bug] mqadmin updateBrokerConfig -a false updates the slaves anyway [rocketmq]

["RockteMQ-AI (via GitHub)" <gi...@apache.org>]

RockteMQ-AI commented on issue #11385:
URL: https://github.com/apache/rocketmq/issues/11385#issuecomment-6135889898

   **Issue Evaluation**
   
   Category: `type/bug` | Status: **Confirmed**
   
   The reported issue has been verified against the current codebase (develop@b764205).
   
   **Root Cause:** In `UpdateBrokerConfigSubCommand.java`, the `-a` option is declared with `hasArg=true` (line 53), but the code only checks `commandLine.hasOption('a')` (line 96) which tests presence, not value. `getOptionValue('a')` is never called, so `-a false` still triggers the master+slave fan-out.
   
   **Impact:** When using `-c <cluster>` with `-a false`, the config update is silently sent to slave brokers as well, contrary to the user's explicit intent.
   
   **Severity:** Medium — affects role-sensitive configuration updates.
   
   **Note:** PR #11386 already proposes a fix for this issue.
   
   ---
   *Automated evaluation by *


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org