[
https://issues.apache.org/jira/browse/HADOOP-10565?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14037038#comment-14037038
]
Arpit Agarwal commented on HADOOP-10565:
----------------------------------------
Hi [~benoyantony], my apologies for not reviewing earlier.
Comments on MachineList.java:
# Lines 197-206: The DNS lookup in a loop could be an issue. {{cacheHostIp}} is
false by default, and if reverse DNS lookup is broken which it often is, we'll
wind up doing multiple IP address lookups per request. This could tie up server
threads for a long time. Even with the cache unknown IP addresses could result
in multiple lookups. We need to think of a better solution.
# Too many unused constructors? I think you can just keep the ones on Line 69
and Line 92 and drop the rest.
# Line 114: propagate the exception to the caller?
# Line 186: rename _hostAddr_ to _resolvedIpAddress_?
# {{hostAddresses}} should probably be called {{hostnames}}. [Host
Address|https://en.wikipedia.org/wiki/Host_address] usually means IP address.
# _host address_ should be _host name_ in comments.
Nitpicks, could you please fix the coding style to be consistent?
# Typo in sentence _InetAddressFactory is used obtain InetAddress from host._?
# Extra space - {{private final Set<String> ipAddresses ;}}, also other places.
# Extra space before {{(}} - {{public MachineList (String hostIPs)}}.
# Similar cleanup for this line {{all = true; ipAddresses = hostAddresses =
null; cidrAddresses =null;}}. Also one assignment per line.
# MachineList.java:134 - Closing brace of {{if}} block and {{else}} should be
on the same line.
# MachineList.java:217 - Extra space - {{String> getCollection}}
> Support IP ranges (CIDR) in proxyuser.hosts
> --------------------------------------------
>
> Key: HADOOP-10565
> URL: https://issues.apache.org/jira/browse/HADOOP-10565
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: security
> Reporter: Benoy Antony
> Assignee: Benoy Antony
> Attachments: HADOOP-10565.patch, HADOOP-10565.patch
>
>
> In some use cases, there will be many hosts from which the user can
> impersonate.
> This requires specifying many ips in the XML configuration.
> It is cumbersome to specify and maintain long list of ips in proxyuser.hosts
> The problem can be solved if we enable proxyuser.hosts to accept ip ranges in
> CIDR format.
> In addition, the current ip authorization involve a liner scan of the ips and
> an attempt to do InetAddress.getByName() for each ip/host.
> It may be beneficial to group this functionality of ip authorization by
> looking up "ip addresses/host names/ip-ranges" into a separate class. This
> could be reused in other usecases which require similar functionality
--
This message was sent by Atlassian JIRA
(v6.2#6252)
