chia7712 commented on code in PR #19650:
URL: https://github.com/apache/kafka/pull/19650#discussion_r2080071682
##########
clients/clients-integration-tests/src/test/java/org/apache/kafka/clients/admin/StaticBrokerConfigTest.java:
##########
@@ -108,4 +119,68 @@ 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()) {
+ 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();
+ 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
+ Config brokerConfig = configResourceMap.get(brokerResource);
Review Comment:
Could you please verify the internal config for controller node too?
##########
clients/clients-integration-tests/src/test/java/org/apache/kafka/clients/admin/StaticBrokerConfigTest.java:
##########
@@ -108,4 +119,68 @@ 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()) {
+ 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();
+ 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
+ Config brokerConfig = configResourceMap.get(brokerResource);
+ assertNotContainsInternalConfig(brokerConfig,
KafkaConfig.configDef().configKeys(), Set.of(
+ ServerConfigs.UNSTABLE_FEATURE_VERSIONS_ENABLE_CONFIG,
+ ServerConfigs.UNSTABLE_API_VERSIONS_ENABLE_CONFIG,
+ KRaftConfigs.SERVER_MAX_STARTUP_TIME_MS_CONFIG));
+
+ // 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());
+ }
+ }
+
+ @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) {
+ assertNotContainsInternalConfig(config, configKeyMap, Set.of());
+ }
+
+ private void assertNotContainsInternalConfig(Config config, Map<String,
ConfigDef.ConfigKey> configKeyMap,
+ Set<String>
ignoreConfigNames) {
+ assertFalse(config.entries().isEmpty());
+ for (ConfigEntry topicConfigEntry : config.entries()) {
+ String configName = topicConfigEntry.name();
+ ConfigDef.ConfigKey configKey = configKeyMap.get(configName);
+
+ assertNotNull(configKey);
+ if (!ignoreConfigNames.contains(configName)) {
+ assertFalse(configKey.internalConfig);
Review Comment:
Could you please add error message for this assert?
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
