xijiu commented on code in PR #19650: URL: https://github.com/apache/kafka/pull/19650#discussion_r2081038035
########## 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: Good idea, I have changed it, PTAL -- 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