iloveeyjafjalla opened a new pull request, #11409: URL: https://github.com/apache/rocketmq/pull/11409
### Which Issue(s) This PR Fixes - Fixes #11408 ### Brief Description A standalone broker derives the three `AuthConfig` identity fields from the configuration it just loaded (`broker/src/main/java/org/apache/rocketmq/broker/BrokerStartup.java:241-243`), and the proxy does the same for the first two (`proxy/.../config/Configuration.java:50-51`). The two places that start a broker **inside a broker container** only built the `AuthConfig` from the per-broker properties, so `configName`, `clusterName` and `authConfigPath` stayed `null` (none of them has a default): - `container/.../BrokerContainerStartup.java:134-140` (`createAndInitializeBroker`) - `container/.../BrokerContainerProcessor.java:120-123` (the `mqadmin addBroker` RPC path) This change mirrors `BrokerStartup` in both entry points, so an inner broker behaves like the same broker started by `mqbroker`: ```java authConfig.setConfigName(brokerConfig.getBrokerName()); authConfig.setClusterName(brokerConfig.getBrokerClusterName()); authConfig.setAuthConfigPath(messageStoreConfig.getStorePathRootDir() + File.separator + "config"); ``` The three consequences that were observed before the change: 1. **ACL 2.0 user/ACL data was created at a relative path.** `LocalAuthenticationMetadataProvider.initialize` / `LocalAuthorizationMetadataProvider.initialize` open RocksDB at `authConfig.getAuthConfigPath() + File.separator + "users"|"acls"`; with a null path that is the literal relative directory `null/users` / `null/acls` under the container process's working directory, instead of `<storePathRootDir>/config/...` next to the broker store. Restarting the container from another working directory silently came up with an empty user/ACL store. 2. **Cluster-scoped admin requests were authorized as `Cluster:null`.** `DefaultAuthorizationContextBuilder` builds those resources from `authConfig.getClusterName()` (`:465`, `:553`, `:579`, `:672`, for `UPDATE_AND_CREATE_TOPIC_LIST`, `GET_ALL_TOPIC_CONFIG`, `DELETE_TOPIC_IN_BROKER_LIST`, `DELETE_SUBSCRIPTION_GROUP_LIST`), so a policy granting a concrete `Cluster:<clusterName>` could never match on brokers started by a container. 3. **All inner brokers shared one set of auth singletons.** `AuthenticationFactory`/`AuthorizationFactory` key their provider/evaluator cache by `config.getConfigName()` (`AuthenticationFactory.java:51`, `:78`, `:95`), so every broker of the container collapsed onto the same `..._null` entry and reused the instances created for the first one. Scope note: a master and a slave that share one `brokerName` inside a single container still share the per-name factory entries — that is the same key the standalone startup uses (there each of them is its own JVM), so this change deliberately keeps that parity instead of inventing a per-brokerId key. I am happy to switch to a unique per inner broker key if that is preferred. ### How Did You Test This Change? New test `BrokerContainerStartupTest#testBrokerStartedByContainerInitializesAuthConfigLikeStandaloneBroker`: it starts a broker through the container (`startBrokerContainer` + `createAndStartBrokers`, the same harness the existing test uses) and asserts, on the controller the container returns, that - `getAuthConfig().getConfigName()` equals `getBrokerConfig().getBrokerName()`, - `getAuthConfig().getClusterName()` equals `getBrokerConfig().getBrokerClusterName()`, - `getAuthConfig().getAuthConfigPath()` equals `getMessageStoreConfig().getStorePathRootDir() + File.separator + "config"`. Results on Temurin JDK 8 (the JDK CI uses), base `a7edeecfb` plus this patch: - before the fix: `org.opentest4j.AssertionFailedError: expected: "TestBroker_1791880213725" but was: null` (`Tests run: 1, Failures: 1`) - after the fix: `BrokerContainerStartupTest` — `Tests run: 2, Failures: 0, Errors: 0` - `mvn -B -ntp -pl container package` (module tests + checkstyle + spotbugs enabled): `BUILD SUCCESS`, 24 tests / 0 failures, `You have 0 Checkstyle violations.`, `BugInstance size is 0` Local environment note, for transparency: this container test needs every dependency rebuilt with a JDK 8 bootclasspath. With the previously installed jars (built by a newer JDK with `source/target=8`) the pre-existing `testStartBrokerContainer` fails on JDK 8 with `NoSuchMethodError: java.nio.ByteBuffer.position(I)Ljava/nio/ByteBuffer;` from `org.apache.rocketmq.store.timer.TimerWheel.checkPhyPos`, i.e. before any of my assertions run. I reinstalled the modules with JDK 8 (`mvn -DskipTests install`), after which both tests run; the failure is unrelated to this change and CI (a full JDK 8 build) is not affected. This change was prepared with AI assistance (analysis, patch and test drafting); the diff and every result above were reproduced locally by running the module's tests against the base commit and the patched tree. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
