[GitHub] [kafka] chia7712 commented on a change in pull request #10435: KAFKA-12578: Remove deprecated security classes/methods for 3.0

[GitBox <gi...@apache.org>]

chia7712 commented on a change in pull request #10435:
URL: https://github.com/apache/kafka/pull/10435#discussion_r604089974



##########
File path: clients/src/main/java/org/apache/kafka/common/security/authenticator/DefaultKafkaPrincipalBuilder.java
##########
@@ -41,89 +39,39 @@
 import javax.security.sasl.SaslServer;
 import org.apache.kafka.common.security.ssl.SslPrincipalMapper;
 
-import java.io.Closeable;
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.security.Principal;
 
-import static java.util.Objects.requireNonNull;
-
 /**
  * Default implementation of {@link KafkaPrincipalBuilder} which provides basic support for
  * SSL authentication and SASL authentication. In the latter case, when GSSAPI is used, this
  * class applies {@link org.apache.kafka.common.security.kerberos.KerberosShortNamer} to transform
  * the name.
  *
- * NOTE: This is an internal class and can change without notice. Unlike normal implementations
- * of {@link KafkaPrincipalBuilder}, there is no default no-arg constructor since this class
- * must adapt implementations of the older {@link org.apache.kafka.common.security.auth.PrincipalBuilder} interface.
+ * NOTE: This is an internal class and can change without notice.
  */
-public class DefaultKafkaPrincipalBuilder implements KafkaPrincipalBuilder, KafkaPrincipalSerde, Closeable {
-    // Use FQN to avoid import deprecation warnings
-    @SuppressWarnings("deprecation")
-    private final org.apache.kafka.common.security.auth.PrincipalBuilder oldPrincipalBuilder;
-    private final Authenticator authenticator;
-    private final TransportLayer transportLayer;
+public class DefaultKafkaPrincipalBuilder implements KafkaPrincipalBuilder, KafkaPrincipalSerde {
     private final KerberosShortNamer kerberosShortNamer;
     private final SslPrincipalMapper sslPrincipalMapper;
 
-    /**
-     * Construct a new instance which wraps an instance of the older {@link org.apache.kafka.common.security.auth.PrincipalBuilder}.
-     *
-     * @param authenticator The authenticator in use
-     * @param transportLayer The underlying transport layer
-     * @param oldPrincipalBuilder Instance of {@link org.apache.kafka.common.security.auth.PrincipalBuilder}
-     * @param kerberosShortNamer Kerberos name rewrite rules or null if none have been configured
-     */
-    @SuppressWarnings("deprecation")
-    public static DefaultKafkaPrincipalBuilder fromOldPrincipalBuilder(Authenticator authenticator,
-                                                                       TransportLayer transportLayer,
-                                                                       org.apache.kafka.common.security.auth.PrincipalBuilder oldPrincipalBuilder,
-                                                                       KerberosShortNamer kerberosShortNamer) {
-        return new DefaultKafkaPrincipalBuilder(
-                requireNonNull(authenticator),
-                requireNonNull(transportLayer),
-                requireNonNull(oldPrincipalBuilder),
-                kerberosShortNamer,
-                null);
-    }
-
-    @SuppressWarnings("deprecation")
-    private DefaultKafkaPrincipalBuilder(Authenticator authenticator,
-                                         TransportLayer transportLayer,
-                                         org.apache.kafka.common.security.auth.PrincipalBuilder oldPrincipalBuilder,
-                                         KerberosShortNamer kerberosShortNamer,
-                                         SslPrincipalMapper sslPrincipalMapper) {
-        this.authenticator = authenticator;
-        this.transportLayer = transportLayer;
-        this.oldPrincipalBuilder = oldPrincipalBuilder;
-        this.kerberosShortNamer = kerberosShortNamer;
-        this.sslPrincipalMapper =  sslPrincipalMapper;
-    }
-
     /**
      * Construct a new instance.
      *
      * @param kerberosShortNamer Kerberos name rewrite rules or null if none have been configured
      * @param sslPrincipalMapper SSL Principal mapper or null if none have been configured
      */
     public DefaultKafkaPrincipalBuilder(KerberosShortNamer kerberosShortNamer, SslPrincipalMapper sslPrincipalMapper) {
-        this(null, null, null, kerberosShortNamer, sslPrincipalMapper);
+        this.kerberosShortNamer = kerberosShortNamer;
+        this.sslPrincipalMapper =  sslPrincipalMapper;

Review comment:
       redundant "space"
   

##########
File path: clients/src/main/java/org/apache/kafka/common/acl/AclBindingFilter.java
##########
@@ -50,18 +48,6 @@ public AclBindingFilter(ResourcePatternFilter patternFilter, AccessControlEntryF
         this.entryFilter = Objects.requireNonNull(entryFilter, "entryFilter");
     }
 
-    /**
-     * Create an instance of this filter with the provided parameters.
-     *
-     * @param resourceFilter non-null resource filter
-     * @param entryFilter non-null access control entry filter
-     * @deprecated Since 2.0. Use {@link #AclBindingFilter(ResourcePatternFilter, AccessControlEntryFilter)}
-     */
-    @Deprecated
-    public AclBindingFilter(ResourceFilter resourceFilter, AccessControlEntryFilter entryFilter) {

Review comment:
       Is `ResourceFilter` still useful for production?




-- 
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.

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