TaiJuWu commented on code in PR #19650:
URL: https://github.com/apache/kafka/pull/19650#discussion_r2080977162
##########
clients/clients-integration-tests/src/test/java/org/apache/kafka/clients/admin/StaticBrokerConfigTest.java:
##########
@@ -108,4 +119,80 @@ public void
testTopicConfigsGetImpactedIfStaticConfigsAddToBroker(ClusterInstanc
"Config value should not be custom value since controller
doesn't have related static config");
}
}
+
+ @ClusterTest(types = {Type.KRAFT})
+ public void
testInternalConfigsDoNotReturnForDescribeConfigs(ClusterInstance cluster)
throws Exception {
+ try (
+ Admin admin = cluster.admin();
+ Admin controllerAdmin = cluster.admin(Map.of(), true)
+ ) {
+ ConfigResource brokerResource = new
ConfigResource(ConfigResource.Type.BROKER, "0");
+ ConfigResource topicResource = new
ConfigResource(ConfigResource.Type.TOPIC, TOPIC);
+ ConfigResource groupResource = new
ConfigResource(ConfigResource.Type.GROUP, "testGroup");
+ ConfigResource clientMetricsResource = new
ConfigResource(ConfigResource.Type.CLIENT_METRICS, "testClient");
+
+ admin.createTopics(List.of(new NewTopic(TOPIC, 1, (short)
1))).config(TOPIC).get();
+ // make sure the topic metadata exist
+ cluster.waitForTopic(TOPIC, 1);
+ Map<ConfigResource, Config> configResourceMap =
admin.describeConfigs(
+ List.of(brokerResource, topicResource, groupResource,
clientMetricsResource)).all().get();
+
+ // test for case ConfigResource.Type == BROKER
+ // Notice: since the testing framework actively sets three
internal configurations when starting the
+ // broker (see
org.apache.kafka.common.test.KafkaClusterTestKit.Builder.createNodeConfig()),
+ // so the API `describeConfigs` will also return these three
configurations. However, other internal
+ // configurations will not be returned
+ Set<String> ignoreConfigNames = Set.of(
+ ServerConfigs.UNSTABLE_FEATURE_VERSIONS_ENABLE_CONFIG,
+ ServerConfigs.UNSTABLE_API_VERSIONS_ENABLE_CONFIG,
+ KRaftConfigs.SERVER_MAX_STARTUP_TIME_MS_CONFIG);
+ Config brokerConfig = configResourceMap.get(brokerResource);
+ assertNotContainsInternalConfig(brokerConfig,
KafkaConfig.configDef().configKeys(), ignoreConfigNames);
+
+ // test for case ConfigResource.Type == TOPIC
+ Config topicConfig = configResourceMap.get(topicResource);
+ assertNotContainsInternalConfig(topicConfig,
LogConfig.configKeys());
+
+ // test for case ConfigResource.Type == GROUP
+ Config groupConfig = configResourceMap.get(groupResource);
+ assertNotContainsInternalConfig(groupConfig,
GroupConfig.configDef().configKeys());
+
+ // test for case ConfigResource.Type == CLIENT_METRICS
+ Config clientMetricsConfig =
configResourceMap.get(clientMetricsResource);
+ assertNotContainsInternalConfig(clientMetricsConfig,
ClientMetricsConfigs.configDef().configKeys());
+
+ // test for controller node, and ConfigResource.Type == BROKER
+ ConfigResource controllerResource = new
ConfigResource(ConfigResource.Type.BROKER, "3000");
+ Map<ConfigResource, Config> controllerConfigMap =
controllerAdmin.describeConfigs(List.of(controllerResource)).all().get();
+ Config controllerConfig =
controllerConfigMap.get(controllerResource);
+ assertNotContainsInternalConfig(controllerConfig,
KafkaConfig.configDef().configKeys(), ignoreConfigNames);
+ }
+ }
+
+ @ClusterTest(types = {Type.KRAFT})
+ public void testInternalConfigsDoNotReturnForCreateTopics(ClusterInstance
cluster) throws Exception {
+ try (Admin admin = cluster.admin()) {
+ // test for createTopics API
+ Config config = admin.createTopics(List.of(new NewTopic(TOPIC, 1,
(short) 1))).config(TOPIC).get();
+ assertNotContainsInternalConfig(config, LogConfig.configKeys());
+ }
+ }
+
+ private void assertNotContainsInternalConfig(Config config, Map<String,
ConfigDef.ConfigKey> configKeyMap) {
Review Comment:
How about `assertNotContainsAnyInternalConfig`?
I think it is more clear.
--
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]
