You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "nileshkumar3 (via GitHub)" <gi...@apache.org> on 2026/04/07 03:28:03 UTC

[PR] KAFKA-20389: Add 'internal' flag to ConfigKeyInfo [kafka]

nileshkumar3 opened a new pull request, #21976:
URL: https://github.com/apache/kafka/pull/21976

   Issue
   The Connect REST endpoint GET /connector-plugins/{pluginName}/config returns a list of ConfigKeyInfo objects. Currently these objects do not expose whether a config key is marked as internal in the underlying ConfigDef. Internal configs are not intended for general use, but without this flag, API consumers have no way to programmatically distinguish them from user-facing configs. Since this endpoint is part of the public API, changing its filtering behavior would be a breaking change. Instead, this PR exposes the information and lets clients decide.
   
   Changes
   ConfigKeyInfo: Added a new boolean internal record component, serialized as "internal" in JSON.
   AbstractHerder.convertConfigKey: Populates the new field from ConfigKey.internalConfig.
   ConnectorPluginsResourceTest: Updated all ConfigKeyInfo constructor call sites. Added a defineInternal config to the test connector and assertions verifying internal is true for internal keys and false for regular keys.
   
   Compatibility
   The new field is additive. JSON clients that ignore unknown fields are unaffected. No existing fields or behaviors are modified.
   
   Testing
   Existing ConnectorPluginsResourceTest updated and passing.
   New test coverage explicitly asserts the internal flag for both internal and non-internal config keys.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Add 'internal' flag to ConfigKeyInfo [kafka]

Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3050064620


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/entities/ConfigKeyInfo.java:
##########
@@ -31,6 +31,7 @@ public record ConfigKeyInfo(
     @JsonProperty("order_in_group") int orderInGroup,
     @JsonProperty("width") String width,
     @JsonProperty("display_name") String displayName,
-    @JsonProperty("dependents") List<String> dependents
+    @JsonProperty("dependents") List<String> dependents,
+    @JsonProperty("internal") boolean internal
 ) {

Review Comment:
   `ConfigKeyInfo` is not part of the public API. You can see all the public API classes in https://kafka.apache.org/42/javadoc/index.html



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Add 'internal' flag to ConfigKeyInfo [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3048600498


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/entities/ConfigKeyInfo.java:
##########
@@ -31,6 +31,7 @@ public record ConfigKeyInfo(
     @JsonProperty("order_in_group") int orderInGroup,
     @JsonProperty("width") String width,
     @JsonProperty("display_name") String displayName,
-    @JsonProperty("dependents") List<String> dependents
+    @JsonProperty("dependents") List<String> dependents,
+    @JsonProperty("internal") boolean internal
 ) {

Review Comment:
   @mimaison  Since ConfigKeyInfo is a public API class and adding a new record component changes its constructor signature, does this change require a KIP? The REST API change is additive (new internal field in JSON), but Java clients constructing ConfigKeyInfo directly would break.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Add 'internal' flag to ConfigKeyInfo [kafka]

Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on PR #21976:
URL: https://github.com/apache/kafka/pull/21976#issuecomment-4208903655

   I'm not sure this change makes sense. See my comment in https://issues.apache.org/jira/browse/KAFKA-20389


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3247424592


##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java:
##########
@@ -1086,7 +1086,7 @@ public void testSourceConnectorPluginConfig() throws ClassNotFoundException {
         testConnectorPluginConfig(
                 "source",
                 SampleSourceConnector::new,
-                SampleSourceConnector::config,
+                SampleSourceConnector::configWithInternalKey,

Review Comment:
   You're right that tests pass either way today. Switched to configWithInternalKey
   so the test actually exercises the filter (with config() removing
   !configKey.internalConfig wouldn't fail anything). I kept it separate from
   config() because defineInternal uses an empty group, which breaks other
   AbstractHerderTest assertions. Also added an explicit assertFalse on
   INTERNAL_ONLY_CONFIG_KEY to make the intent self-documenting.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21976:
URL: https://github.com/apache/kafka/pull/21976#issuecomment-6393087958

   This PR is being marked as stale since it has not had any activity in 90 days. If you
   would like to keep this PR alive, please leave a comment asking for a review. If the PR has 
   merge conflicts, update it with the latest from the base branch.
   <p>
   If you are having difficulty finding a reviewer, please reach out on the 
   [mailing list](https://kafka.apache.org/contact).
   <p>
   If this PR is no longer valid or desired, please feel free to close it. If no activity
   occurs in the next 30 days, it will be automatically closed.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on PR #21976:
URL: https://github.com/apache/kafka/pull/21976#issuecomment-4214530461

   > I'm not sure this change makes sense. See my comment in https://issues.apache.org/jira/browse/KAFKA-20389
   
   
   
   > I'm not sure this change makes sense. See my comment in https://issues.apache.org/jira/browse/KAFKA-20389
   
   Changed the code as the comment. request another round of review.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on PR #21976:
URL: https://github.com/apache/kafka/pull/21976#issuecomment-4238862241

   @mimaison Thanks for the review, addressed the review comments. Can you review again.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3260082105


##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java:
##########
@@ -1086,7 +1086,7 @@ public void testSourceConnectorPluginConfig() throws ClassNotFoundException {
         testConnectorPluginConfig(
                 "source",
                 SampleSourceConnector::new,
-                SampleSourceConnector::config,
+                SampleSourceConnector::configWithInternalKey,

Review Comment:
   @mimaison  Happy to go the other way if you prefer: revert testSourceConnectorPluginConfig
   back to SampleSourceConnector::config and add a separate dedicated test that
   uses configWithInternalKey purely to verify internal keys are filtered out.
   That keeps the existing test untouched and isolates the new coverage. Let me
   know which you'd prefer.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3238558216


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java:
##########
@@ -317,9 +317,9 @@ default void validateConnectorConfig(Map<String, String> connectorConfig, Callba
 
 
     /**
-     * Returns the configuration of a plugin
+     * Returns the non-internal configuration of a plugin.

Review Comment:
   removed this part as it is implicit.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Add 'internal' flag to ConfigKeyInfo [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3052223670


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/entities/ConfigKeyInfo.java:
##########
@@ -31,6 +31,7 @@ public record ConfigKeyInfo(
     @JsonProperty("order_in_group") int orderInGroup,
     @JsonProperty("width") String width,
     @JsonProperty("display_name") String displayName,
-    @JsonProperty("dependents") List<String> dependents
+    @JsonProperty("dependents") List<String> dependents,
+    @JsonProperty("internal") boolean internal
 ) {

Review Comment:
   Thanks @mimaison, this is helpful. request your review on this PR.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3235647350


##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/SourceConnectorWithInternalConfig.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.connect.runtime;
+
+import org.apache.kafka.common.config.ConfigDef;
+import org.apache.kafka.connect.connector.Task;
+import org.apache.kafka.connect.source.SourceConnector;
+
+import java.util.List;
+import java.util.Map;
+
+/** Test-only source connector including one internal {@link org.apache.kafka.common.config.ConfigDef} key. */

Review Comment:
   Could we add an internal config to one of the existing SampleConnector classes?



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Herder.java:
##########
@@ -317,9 +317,9 @@ default void validateConnectorConfig(Map<String, String> connectorConfig, Callba
 
 
     /**
-     * Returns the configuration of a plugin
+     * Returns the non-internal configuration of a plugin.

Review Comment:
   I don't know if we need to update this javadoc. It's kind of implied that internal configurations are not exposed.



##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java:
##########
@@ -1141,6 +1141,23 @@ public void testTransformationPluginConfig() throws ClassNotFoundException {
         );
     }
 
+    @Test
+    public void testConnectorPluginConfigOmitsInternalKeys() throws Exception {
+        String pluginName = "source-with-internal";
+        Class<?> pluginClass = Class.forName("org.apache.kafka.connect.runtime.SourceConnectorWithInternalConfig");
+        AbstractHerder herder = testHerder();
+
+        when(plugins.pluginClass(pluginName, null)).then(invocation -> pluginClass);
+        when(plugins.newPlugin(anyString(), any())).then(invocation -> pluginClass.getDeclaredConstructor().newInstance());
+        when(herder.plugins()).thenReturn(plugins);
+
+        List<ConfigKeyInfo> configs = herder.connectorPluginConfig(pluginName);
+        assertTrue(configs.stream().anyMatch(c -> c.name().equals("required")));
+        // any internal config keys should not be exposed
+        assertFalse(configs.stream().anyMatch(c -> c.name().equals("internal.only.config")));

Review Comment:
   Can we use the `INTERNAL_ONLY_CONFIG_KEY` constant instead of hardcoding the config name?



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on PR #21976:
URL: https://github.com/apache/kafka/pull/21976#issuecomment-4364582114

   > Looks clean and sufficient.
   
   Thanks @Claudenw  for review.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3238554467


##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/SourceConnectorWithInternalConfig.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.connect.runtime;
+
+import org.apache.kafka.common.config.ConfigDef;
+import org.apache.kafka.connect.connector.Task;
+import org.apache.kafka.connect.source.SourceConnector;
+
+import java.util.List;
+import java.util.Map;
+
+/** Test-only source connector including one internal {@link org.apache.kafka.common.config.ConfigDef} key. */

Review Comment:
   yes, added and removed this class.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3242192236


##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java:
##########
@@ -1086,7 +1086,7 @@ public void testSourceConnectorPluginConfig() throws ClassNotFoundException {
         testConnectorPluginConfig(
                 "source",
                 SampleSourceConnector::new,
-                SampleSourceConnector::config,
+                SampleSourceConnector::configWithInternalKey,

Review Comment:
   right test still pass, this was added to test filtering logic. Other Test fails if we add internal key to the config. For now internal key is not added  in the existing definition. i separated config vs configWithInternalKey to keep existing tests unaffected and to test the filtering logic in this.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3241714457


##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java:
##########
@@ -1086,7 +1086,7 @@ public void testSourceConnectorPluginConfig() throws ClassNotFoundException {
         testConnectorPluginConfig(
                 "source",
                 SampleSourceConnector::new,
-                SampleSourceConnector::config,
+                SampleSourceConnector::configWithInternalKey,

Review Comment:
   Why are we making this change?



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3380413974


##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java:
##########
@@ -1086,7 +1086,7 @@ public void testSourceConnectorPluginConfig() throws ClassNotFoundException {
         testConnectorPluginConfig(
                 "source",
                 SampleSourceConnector::new,
-                SampleSourceConnector::config,
+                SampleSourceConnector::configWithInternalKey,

Review Comment:
   @mimaison gentle ping on this thread.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on PR #21976:
URL: https://github.com/apache/kafka/pull/21976#issuecomment-5085783708

   Thanks @mimaison — reverted testSourceConnectorPluginConfig to config and added a dedicated testSourceConnectorPluginConfigFiltersInternalKeys. It returns a connector whose config() includes an internal key to validate that internal keys are filtered out. 


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3247424592


##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java:
##########
@@ -1086,7 +1086,7 @@ public void testSourceConnectorPluginConfig() throws ClassNotFoundException {
         testConnectorPluginConfig(
                 "source",
                 SampleSourceConnector::new,
-                SampleSourceConnector::config,
+                SampleSourceConnector::configWithInternalKey,

Review Comment:
   Added an explicit assertFalse assertion that INTERNAL_ONLY_CONFIG_KEY does not appear in the connectorPluginConfig result.
   
   This directly proves the filtering logic works. The count-based assertEquals alone was not sufficient: if the !configKey.internalConfig filter were removed from connectorPluginConfig, the count would go from 2 to 3 and the count assertion would fail — but only when configWithInternalKey is used as input. The new assertFalse makes the intent explicit and self-documenting regardless of input size.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3241870608


##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java:
##########
@@ -1086,7 +1086,7 @@ public void testSourceConnectorPluginConfig() throws ClassNotFoundException {
         testConnectorPluginConfig(
                 "source",
                 SampleSourceConnector::new,
-                SampleSourceConnector::config,
+                SampleSourceConnector::configWithInternalKey,

Review Comment:
   As far as I can tell, all tests still pass without this change.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3238555900


##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java:
##########
@@ -1141,6 +1141,23 @@ public void testTransformationPluginConfig() throws ClassNotFoundException {
         );
     }
 
+    @Test
+    public void testConnectorPluginConfigOmitsInternalKeys() throws Exception {
+        String pluginName = "source-with-internal";
+        Class<?> pluginClass = Class.forName("org.apache.kafka.connect.runtime.SourceConnectorWithInternalConfig");
+        AbstractHerder herder = testHerder();
+
+        when(plugins.pluginClass(pluginName, null)).then(invocation -> pluginClass);
+        when(plugins.newPlugin(anyString(), any())).then(invocation -> pluginClass.getDeclaredConstructor().newInstance());
+        when(herder.plugins()).thenReturn(plugins);
+
+        List<ConfigKeyInfo> configs = herder.connectorPluginConfig(pluginName);
+        assertTrue(configs.stream().anyMatch(c -> c.name().equals("required")));
+        // any internal config keys should not be exposed
+        assertFalse(configs.stream().anyMatch(c -> c.name().equals("internal.only.config")));

Review Comment:
   added the const.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on PR #21976:
URL: https://github.com/apache/kafka/pull/21976#issuecomment-4446690806

   > Thanks for the PR. I left a few comments
   
   Thanks @mimaison  for reviewing. I have fixed all the review comments, please review when you get a chance.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] KAFKA-20389: Filter internal keys from connector plugin config REST response [kafka]

Posted by "nileshkumar3 (via GitHub)" <gi...@apache.org>.
nileshkumar3 commented on code in PR #21976:
URL: https://github.com/apache/kafka/pull/21976#discussion_r3241826656


##########
connect/runtime/src/test/java/org/apache/kafka/connect/runtime/AbstractHerderTest.java:
##########
@@ -1086,7 +1086,7 @@ public void testSourceConnectorPluginConfig() throws ClassNotFoundException {
         testConnectorPluginConfig(
                 "source",
                 SampleSourceConnector::new,
-                SampleSourceConnector::config,
+                SampleSourceConnector::configWithInternalKey,

Review Comment:
   We need a ConfigDef that includes an internal key so this test verifies those keys are omitted from connectorPluginConfig and the expected size stays correct. We cannot add that internal key to config() because it breaks other AbstractHerderTest validation assertions .



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org