-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29714/
-----------------------------------------------------------
(Updated Jan. 16, 2015, 12:48 a.m.)
Review request for kafka.
Bugs: KAFKA-1810
https://issues.apache.org/jira/browse/KAFKA-1810
Repository: kafka
Description (updated)
-------
Put back in the 1512 changes and moved the initial BigInt out of the check
CIDRRange contains method
Diffs
-----
core/src/main/scala/kafka/network/IPFilter.scala PRE-CREATION
core/src/main/scala/kafka/network/SocketServer.scala
39b1651b680b2995cedfde95d74c086d9c6219ef
core/src/main/scala/kafka/server/KafkaConfig.scala
6e26c5436feb4629d17f199011f3ebb674aa767f
core/src/main/scala/kafka/server/KafkaServer.scala
1691ad7fc80ca0b112f68e3ea0cbab265c75b26b
core/src/main/scala/kafka/utils/VerifiableProperties.scala
2ffc7f452dc7a1b6a06ca7a509ed49e1ab3d1e68
core/src/test/scala/unit/kafka/network/IPFilterTest.scala PRE-CREATION
core/src/test/scala/unit/kafka/network/SocketServerTest.scala
78b431f9c88cca1bc5e430ffd41083d0e92b7e75
core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala
2377abe4933e065d037828a214c3a87e1773a8ef
Diff: https://reviews.apache.org/r/29714/diff/
Testing
-------
This code centers around a new class, CIDRRange in IPFilter.scala. The IPFilter
class is created and holds two fields, the ruleType (allow|deny|none) and a
list of CIDRRange objects. This is used in the Socket Server acceptor thread.
The check does an exists on the value in the list if the rule type is allow or
deny. On object creation, we pre-calculate the lower and upper range values and
store those as a BigInt. The overhead of the check should be fairly minimal as
it involves converting the incoming IP Address to a BigInt and then just doing
a compare to the low/high values. In writing this review up I realized that I
can optimize this further to convert to bigint first and move that conversion
out of the range check, which I can address.
Testing covers the CIDRRange and IPFilter classes and validation of IPV6, IPV4,
and configurations. Additionally the functionality is tested in
SocketServerTest. Other changes are just to assist in configuration.
I modified the SocketServerTest to use a method for grabbing the Socket server
to make the code a bit more concise.
One key point is that, if there is an error in configuration, we halt the
startup of the broker. The thinking there is that if you screw up
security-related configs, you want to know about it right away rather than
silently accept connections. (thanks Joe Stein for the input).
There are two new exceptions realted to this functionality, one to handle
configuration errors, and one to handle blocking the request. Currently the
level is set to INFO. Does it make sense to move this to WARN ?
Thanks,
Jeff Holoman
