You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/03/30 07:59:46 UTC
[GitHub] [kafka] ijuma opened a new pull request #10435: KAFKA-12578: Remove deprecated security classes/methods
ijuma opened a new pull request #10435:
URL: https://github.com/apache/kafka/pull/10435
More specifically:
- Constants in SslConfigs
- Constants in SaslConfigs
- AclBinding constructor
- AclBindingFilter constructor
- PrincipalBuilder and DefaultPrincipalBuilder classes
These removals seem non controversial. There is a straightforward alternative.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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
[GitHub] [kafka] chia7712 commented on a change in pull request #10435: KAFKA-12578: Remove deprecated security classes/methods for 3.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10435:
URL: https://github.com/apache/kafka/pull/10435#discussion_r604187554
##########
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:
+1 to remove it :)
--
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
[GitHub] [kafka] ijuma commented on pull request #10435: KAFKA-12578: Remove deprecated security classes/methods for 3.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10435:
URL: https://github.com/apache/kafka/pull/10435#issuecomment-810345988
Updated @chia7712
--
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
[GitHub] [kafka] ijuma commented on a change in pull request #10435: KAFKA-12578: Remove deprecated security classes/methods for 3.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10435:
URL: https://github.com/apache/kafka/pull/10435#discussion_r604182448
##########
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:
Good point, this is not used anymore. It's not deprecated, but given that it's not used anywhere, it seems fair to remove it at the same time. Thoughts?
--
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
[GitHub] [kafka] chia7712 commented on pull request #10435: KAFKA-12578: Remove deprecated security classes/methods for 3.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #10435:
URL: https://github.com/apache/kafka/pull/10435#issuecomment-810724864
> Do you mind if we tackle this one separately?
+1
--
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
[GitHub] [kafka] ijuma merged pull request #10435: KAFKA-12578: Remove deprecated security classes/methods for 3.0
Posted by GitBox <gi...@apache.org>.
ijuma merged pull request #10435:
URL: https://github.com/apache/kafka/pull/10435
--
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
[GitHub] [kafka] ijuma commented on pull request #10435: KAFKA-12578: Remove deprecated security classes/methods for 3.0
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10435:
URL: https://github.com/apache/kafka/pull/10435#issuecomment-810715188
@chia7712 `Resource` is public API, yes. But you're right that it may not be used via client facing APIs. Do you mind if we tackle this one separately?
--
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
[GitHub] [kafka] ijuma commented on pull request #10435: KAFKA-12578: Remove deprecated security classes/methods
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10435:
URL: https://github.com/apache/kafka/pull/10435#issuecomment-810023871
I still need to update the upgrade notes.
--
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
[GitHub] [kafka] chia7712 commented on pull request #10435: KAFKA-12578: Remove deprecated security classes/methods for 3.0
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #10435:
URL: https://github.com/apache/kafka/pull/10435#issuecomment-810385150
Is `Resource` exposed to users? It appears that we don't create `Resource` except for `Resource CLUSTER`. If `Resource` is not a kind of public APIs, I feel it can be removed also. (`Resource.CLUSTER` can be replaced by `ResourceType` and `String`)
--
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
[GitHub] [kafka] chia7712 commented on a change in pull request #10435: KAFKA-12578: Remove deprecated security classes/methods for 3.0
Posted by 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