m1a2st commented on code in PR #19650: URL: https://github.com/apache/kafka/pull/19650#discussion_r2075617657
########## 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> whiteListSet) { Review Comment: Could you avoid using `whiteList`, and replace with more neutral terminology. -- 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