-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29714/
-----------------------------------------------------------
(Updated March 15, 2015, 5:13 a.m.)
Review request for kafka.
Bugs: KAFKA-1810
https://issues.apache.org/jira/browse/KAFKA-1810
Repository: kafka
Description (updated)
-------
KAFKA-1810 ConfigDef Refactor
Diffs (updated)
-----
core/src/main/scala/kafka/network/IPFilter.scala PRE-CREATION
core/src/main/scala/kafka/network/SocketServer.scala
76ce41aed6e04ac5ba88395c4d5008aca17f9a73
core/src/main/scala/kafka/server/KafkaConfig.scala
46d21c73f1feb3410751899380b35da0c37c975c
core/src/main/scala/kafka/server/KafkaServer.scala
dddef938fabae157ed8644536eb1a2f329fb42b7
core/src/test/scala/unit/kafka/network/IpFilterTest.scala PRE-CREATION
core/src/test/scala/unit/kafka/network/SocketServerTest.scala
0af23abf146d99e3d6cf31e5d6b95a9e63318ddb
core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala
191251d1340b5e5b2d649b37af3c6c1896d07e6e
core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala
7f47e6f9a74314ed9e9f19d0c97931f3f2e49259
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
