> On May 14, 2015, 10:21 a.m., Rajini Sivaram wrote: > > clients/src/test/java/org/apache/kafka/common/network/SSLSelectorTest.java, > > line 36 > > <https://reviews.apache.org/r/33620/diff/5/?file=957076#file957076line36> > > > > All these tests were hanging when I ran them from a Windows machine > > with IBM JRE. The tests did pass for me on Ubuntu with OpenJDK. Will try to > > figure out why the tests weren't working with IBM JRE.
IBM JSSE doesn't enable TLSv1.2 when SSL context is created with protocol TLS. Will drop this issue and create another one to fix this. > On May 14, 2015, 10:21 a.m., Rajini Sivaram wrote: > > clients/src/main/java/org/apache/kafka/common/network/Authenticator.java, > > line 47 > > <https://reviews.apache.org/r/33620/diff/5/?file=957060#file957060line47> > > > > Don't think authenticator interface should handle selection > > interestOps. Needs better encapsulation. > > Sriharsha Chintalapani wrote: > SaslAuth works by reading socketChannel and doing accept token on > incoming data on server side and similarly saslclient sends a new token . Why > do you think authenticate shouldn't handle interestOps since its reading and > writing to socketChannel. Whats your proposal At the moment management of interestOps is spread across Selector, TransportLayer and Authenticator. I would have hoped that they could be contained within the transport layer. My concern is that with transport layer setting interestOps from multiple threads (for delegated tasks in SSL), it may be too messy to handle interestOps within a pluggable authenticator as well. Maybe it is better to address this when SaslAuth implementation is ready. > On May 14, 2015, 10:21 a.m., Rajini Sivaram wrote: > > clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java, > > line 45 > > <https://reviews.apache.org/r/33620/diff/5/?file=957077#file957077line45> > > > > SelectorTest should be run with SSL as well as PlainText since all > > selector functions should be tested with SSL as the transport layer. > > Simplest way is to parameterize the test. > > https://reviews.apache.org/r/33133/diff/ has the code for parameterizing > > the test, which you may be able to reuse with some tweaks. > > > > > > Tried this and testMute() fails with SSL > > Sriharsha Chintalapani wrote: > There is SSLSelectorTest which configures the Selector with SSL and runs > tests with EchoServer with sslserversocket. Not sure why Selector which is > PlainText should run SSL? As far as I can tell "Selector" is not PlainText, it is used by all transports. Hence I think all the functionality in Selector currently tested in SelectorTest should also be run with other transports as these are implemented. SSLSelectorTest tests only a small subset. testMute() is an example which currently fails. - Rajini ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33620/#review83741 ----------------------------------------------------------- On May 12, 2015, 11:20 p.m., Sriharsha Chintalapani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33620/ > ----------------------------------------------------------- > > (Updated May 12, 2015, 11:20 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1690 > https://issues.apache.org/jira/browse/KAFKA-1690 > > > Repository: kafka > > > Description > ------- > > KAFKA-1690. new java producer needs ssl support as a client. > > > KAFKA-1690. new java producer needs ssl support as a client. > > > KAFKA-1690. new java producer needs ssl support as a client. > > > KAFKA-1690. new java producer needs ssl support as a client. SSLFactory tests. > > > KAFKA-1690. new java producer needs ssl support as a client. Added > PrincipalBuilder. > > > Diffs > ----- > > build.gradle fef515b3b2276b1f861e7cc2e33e74c3ce5e405b > checkstyle/checkstyle.xml a215ff36e9252879f1e0be5a86fef9a875bb8f38 > checkstyle/import-control.xml f2e6cec267e67ce8e261341e373718e14a8e8e03 > clients/src/main/java/org/apache/kafka/clients/ClientUtils.java > 0d68bf1e1e90fe9d5d4397ddf817b9a9af8d9f7a > clients/src/main/java/org/apache/kafka/clients/CommonClientConfigs.java > cf32e4e7c40738fe6d8adc36ae0cfad459ac5b0b > clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java > bdff518b732105823058e6182f445248b45dc388 > clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java > d301be4709f7b112e1f3a39f3c04cfa65f00fa60 > clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java > 8e336a3aa96c73f52beaeb56b931baf4b026cf21 > clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java > 187d0004c8c46b6664ddaffecc6166d4b47351e5 > clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java > c4fa058692f50abb4f47bd344119d805c60123f5 > clients/src/main/java/org/apache/kafka/common/network/Authenticator.java > PRE-CREATION > clients/src/main/java/org/apache/kafka/common/network/Channel.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/network/DefaultAuthenticator.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/network/PlainTextTransportLayer.java > PRE-CREATION > clients/src/main/java/org/apache/kafka/common/network/SSLFactory.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java > PRE-CREATION > clients/src/main/java/org/apache/kafka/common/network/Selectable.java > b5f8d83e89f9026dc0853e5f92c00b2d7f043e22 > clients/src/main/java/org/apache/kafka/common/network/Selector.java > 57de0585e5e9a53eb9dcd99cac1ab3eb2086a302 > clients/src/main/java/org/apache/kafka/common/network/TransportLayer.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/protocol/SecurityProtocol.java > dab1a94dd29563688b6ecf4eeb0e180b06049d3f > > clients/src/main/java/org/apache/kafka/common/security/auth/DefaultPrincipalBuilder.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java > PRE-CREATION > > clients/src/main/java/org/apache/kafka/common/security/auth/PrincipalBuilder.java > PRE-CREATION > clients/src/main/java/org/apache/kafka/common/utils/Utils.java > f73eedb030987f018d8446bb1dcd98d19fa97331 > clients/src/test/java/org/apache/kafka/common/network/EchoServer.java > PRE-CREATION > clients/src/test/java/org/apache/kafka/common/network/SSLFactoryTest.java > PRE-CREATION > clients/src/test/java/org/apache/kafka/common/network/SSLSelectorTest.java > PRE-CREATION > clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java > d5b306b026e788b4e5479f3419805aa49ae889f3 > clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java > 2ebe3c21f611dc133a2dbb8c7dfb0845f8c21498 > clients/src/test/java/org/apache/kafka/test/TestSSLUtils.java PRE-CREATION > > Diff: https://reviews.apache.org/r/33620/diff/ > > > Testing > ------- > > > Thanks, > > Sriharsha Chintalapani > >
