HADOOP-17367. Add InetAddress api to ProxyUsers.authorize (#2449). Contributed by Daryn Sharp and Ahmed Hussein

(cherry picked from commit 6b3d65b46e)

hadoop-common-project/hadoop-common/pom.xml

@@ -38,7 +38,6 @@
     <wsce.config.file>wsce-site.xml</wsce.config.file>
   </properties>
 
-
   <dependencies>
     <dependency>
       <groupId>org.apache.hadoop</groupId>

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/DefaultImpersonationProvider.java

@@ -18,6 +18,7 @@
 
 package org.apache.hadoop.security.authorize;
 
+import java.net.InetAddress;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
@@ -105,8 +106,8 @@ public class DefaultImpersonationProvider implements ImpersonationProvider {
   }
 
   @Override
-  public void authorize(UserGroupInformation user, 
+  public void authorize(UserGroupInformation user,
-      String remoteAddress) throws AuthorizationException {
+      InetAddress remoteAddress) throws AuthorizationException {
     
     if (user == null) {
       throw new IllegalArgumentException("user is null.");

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/ImpersonationProvider.java

@@ -18,6 +18,9 @@
 
 package org.apache.hadoop.security.authorize;
 
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
 import org.apache.hadoop.conf.Configurable;
@@ -38,12 +41,29 @@ public interface ImpersonationProvider  extends Configurable {
   public void init(String configurationPrefix);
 
   /**
-   * Authorize the superuser which is doing doAs
+   * Authorize the superuser which is doing doAs.
-   * 
+   * {@link #authorize(UserGroupInformation, InetAddress)} should
+   *             be preferred to avoid possibly re-resolving the ip address.
+   * @param user ugi of the effective or proxy user which contains a real user.
+   * @param remoteAddress the ip address of client.
+   * @throws AuthorizationException
+   */
+  default void authorize(UserGroupInformation user, String remoteAddress)
+      throws AuthorizationException {
+    try {
+      authorize(user, InetAddress.getByName(remoteAddress));
+    } catch (UnknownHostException e) {
+      throw new AuthorizationException(e);
+    }
+  }
+
+  /**
+   * Authorize the superuser which is doing doAs.
+   *
    * @param user ugi of the effective or proxy user which contains a real user
    * @param remoteAddress the ip address of client
    * @throws AuthorizationException
    */
-  public void authorize(UserGroupInformation user, String remoteAddress)
+  void authorize(UserGroupInformation user, InetAddress remoteAddress)
       throws AuthorizationException;
 }

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/authorize/ProxyUsers.java

@@ -18,6 +18,8 @@
 
 package org.apache.hadoop.security.authorize;
 
+import java.net.InetAddress;
+
 import com.google.common.base.Preconditions;
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
@@ -86,22 +88,41 @@ public class ProxyUsers {
   }
   
   /**
-   * Authorize the superuser which is doing doAs
+   * Authorize the superuser which is doing doAs.
-   * 
+   * {@link #authorize(UserGroupInformation, InetAddress)} should be preferred
+   * to avoid possibly re-resolving the ip address.
+   *
    * @param user ugi of the effective or proxy user which contains a real user
    * @param remoteAddress the ip address of client
    * @throws AuthorizationException
    */
   public static void authorize(UserGroupInformation user, 
       String remoteAddress) throws AuthorizationException {
-    if (sip==null) {
+    getSip().authorize(user, remoteAddress);
-      // In a race situation, It is possible for multiple threads to satisfy this condition.
-      // The last assignment will prevail.
-      refreshSuperUserGroupsConfiguration(); 
-    }
-    sip.authorize(user, remoteAddress);
   }
-  
+
+  /**
+   * Authorize the superuser which is doing doAs.
+   *
+   * @param user ugi of the effective or proxy user which contains a real user
+   * @param remoteAddress the inet address of client
+   * @throws AuthorizationException
+   */
+  public static void authorize(UserGroupInformation user,
+      InetAddress remoteAddress) throws AuthorizationException {
+    getSip().authorize(user, remoteAddress);
+  }
+
+  private static ImpersonationProvider getSip() {
+    if (sip == null) {
+      // In a race situation, It is possible for multiple threads to satisfy
+      // this condition.
+      // The last assignment will prevail.
+      refreshSuperUserGroupsConfiguration();
+    }
+    return sip;
+  }
+
   /**
    * This function is kept to provide backward compatibility.
    * @param user
@@ -118,7 +139,7 @@ public class ProxyUsers {
   
   @VisibleForTesting 
   public static DefaultImpersonationProvider getDefaultImpersonationProvider() {
-    return ((DefaultImpersonationProvider)sip);
+    return ((DefaultImpersonationProvider) getSip());
   }
       
 }

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/MachineList.java

@@ -21,6 +21,7 @@ import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
@@ -29,7 +30,6 @@ import java.util.Set;
 import org.apache.commons.net.util.SubnetUtils;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.net.InetAddresses;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -61,9 +61,9 @@ public class MachineList {
   }
 
   private final boolean all;
-  private final Set<String> ipAddresses;
+  private final Set<InetAddress> inetAddresses;
+  private final Collection<String> entries;
   private final List<SubnetUtils.SubnetInfo> cidrAddresses;
-  private final Set<String> hostNames;
   private final InetAddressFactory addressFactory;
 
   /**
@@ -71,7 +71,11 @@ public class MachineList {
    * @param hostEntries comma separated ip/cidr/host addresses
    */
   public MachineList(String hostEntries) {
-    this(StringUtils.getTrimmedStringCollection(hostEntries));
+    this(hostEntries, InetAddressFactory.S_INSTANCE);
+  }
+
+  public MachineList(String hostEntries, InetAddressFactory addressFactory) {
+    this(StringUtils.getTrimmedStringCollection(hostEntries), addressFactory);
   }
 
   /**
@@ -88,19 +92,19 @@ public class MachineList {
    * @param hostEntries
    * @param addressFactory addressFactory to convert host to InetAddress
    */
-  public MachineList(Collection<String> hostEntries, InetAddressFactory addressFactory) {
+  public MachineList(Collection<String> hostEntries,
+      InetAddressFactory addressFactory) {
     this.addressFactory = addressFactory;
     if (hostEntries != null) {
+      entries = new ArrayList<>(hostEntries);
       if ((hostEntries.size() == 1) && (hostEntries.contains(WILDCARD_VALUE))) {
-        all = true; 
+        all = true;
-        ipAddresses = null; 
+        inetAddresses = null;
-        hostNames = null; 
         cidrAddresses = null; 
       } else {
         all = false;
-        Set<String> ips = new HashSet<String>();
+        Set<InetAddress> addrs = new HashSet<>();
         List<SubnetUtils.SubnetInfo> cidrs = new LinkedList<SubnetUtils.SubnetInfo>();
-        Set<String> hosts = new HashSet<String>();
         for (String hostEntry : hostEntries) {
           //ip address range
           if (hostEntry.indexOf("/") > -1) {
@@ -112,25 +116,29 @@ public class MachineList {
               LOG.warn("Invalid CIDR syntax : " + hostEntry);
               throw e;
             }
-          } else if (InetAddresses.isInetAddress(hostEntry)) { //ip address
+          } else {
-            ips.add(hostEntry);
+            try {
-          } else { //hostname
+              addrs.add(addressFactory.getByName(hostEntry));
-            hosts.add(hostEntry);
+            } catch (UnknownHostException e) {
+              LOG.warn(e.toString());
+            }
           }
         }
-        ipAddresses = (ips.size() > 0) ? ips : null;
+        inetAddresses = (addrs.size() > 0) ? addrs : null;
         cidrAddresses = (cidrs.size() > 0) ? cidrs : null;
-        hostNames = (hosts.size() > 0) ? hosts : null;
       }
     } else {
-      all = false; 
+      all = false;
-      ipAddresses = null;
+      inetAddresses = null;
-      hostNames = null; 
+      cidrAddresses = null;
-      cidrAddresses = null; 
+      entries = Collections.emptyList();
     }
   }
   /**
-   * Accepts an ip address and return true if ipAddress is in the list
+   * Accepts an ip address and return true if ipAddress is in the list.
+   * {@link #includes(InetAddress)} should be preferred
+   * to avoid possibly re-resolving the ip address.
+   *
    * @param ipAddress
    * @return true if ipAddress is part of the list
    */
@@ -144,71 +152,47 @@ public class MachineList {
       throw new IllegalArgumentException("ipAddress is null.");
     }
 
-    //check in the set of ipAddresses
+    try {
-    if ((ipAddresses != null) && ipAddresses.contains(ipAddress)) {
+      return includes(addressFactory.getByName(ipAddress));
+    } catch (UnknownHostException e) {
+      return false;
+    }
+  }
+
+  /**
+   * Accepts an inet address and return true if address is in the list.
+   * @param address
+   * @return true if address is part of the list
+   */
+  public boolean includes(InetAddress address) {
+    if (all) {
       return true;
     }
-    
+    if (address == null) {
-    //iterate through the ip ranges for inclusion
+      throw new IllegalArgumentException("address is null.");
+    }
+    if (inetAddresses != null && inetAddresses.contains(address)) {
+      return true;
+    }
+    // iterate through the ip ranges for inclusion
     if (cidrAddresses != null) {
+      String ipAddress = address.getHostAddress();
       for(SubnetUtils.SubnetInfo cidrAddress : cidrAddresses) {
         if(cidrAddress.isInRange(ipAddress)) {
           return true;
         }
       }
     }
-    
-    //check if the ipAddress matches one of hostnames
-    if (hostNames != null) {
-      //convert given ipAddress to hostname and look for a match
-      InetAddress hostAddr;
-      try {
-        hostAddr = addressFactory.getByName(ipAddress);
-        if ((hostAddr != null) && hostNames.contains(hostAddr.getCanonicalHostName())) {
-          return true;
-        }
-      } catch (UnknownHostException e) {
-        //ignore the exception and proceed to resolve the list of hosts
-      }
-
-      //loop through host addresses and convert them to ip and look for a match
-      for (String host : hostNames) {
-        try {
-          hostAddr = addressFactory.getByName(host);
-        } catch (UnknownHostException e) {
-          continue;
-        }
-        if (hostAddr.getHostAddress().equals(ipAddress)) {
-          return true;
-        }
-      }
-    }
     return false;
   }
-
   /**
-   * returns the contents of the MachineList as a Collection<String>
+   * returns the contents of the MachineList as a Collection&lt;String&gt; .
-   * This can be used for testing 
+   * This can be used for testing .
-   * @return contents of the MachineList
+   *
+   * @return contents of the MachineList.
    */
   @VisibleForTesting
   public Collection<String> getCollection() {
-    Collection<String> list = new ArrayList<String>();
+    return entries;
-    if (all) {
-      list.add("*"); 
-    } else {
-      if (ipAddresses != null) {
-        list.addAll(ipAddresses);
-      }
-      if (hostNames != null) {
-        list.addAll(hostNames);
-      }
-      if (cidrAddresses != null) {
-        for(SubnetUtils.SubnetInfo cidrAddress : cidrAddresses) {
-          list.add(cidrAddress.getCidrSignature());
-        }
-      }
-    }
-    return list;
   }
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/security/authorize/TestProxyUsers.java

@@ -21,6 +21,8 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
 import java.security.SecureRandom;
 import java.util.Arrays;
 import java.util.Collection;
@@ -370,7 +372,7 @@ public class TestProxyUsers {
         PROXY_USER_NAME, realUserUgi, GROUP_NAMES);
 
     // remote address is null
-    ProxyUsers.authorize(proxyUserUgi, null);
+    ProxyUsers.authorize(proxyUserUgi, (InetAddress) null);
   }
 
   @Test
@@ -533,9 +535,21 @@ public class TestProxyUsers {
     assertNotAuthorized(proxyUserUgi, "1.2.3.4");
   }
 
+  private static InetAddress toFakeAddress(String ip) {
+    try {
+      InetAddress addr = InetAddress.getByName(ip);
+      return InetAddress.getByAddress(ip.replace('.', '-'),
+          addr.getAddress());
+    } catch (UnknownHostException e) {
+      throw new IllegalArgumentException(e);
+    }
+  }
+
   private void assertNotAuthorized(UserGroupInformation proxyUgi, String host) {
     try {
+      // test both APIs.
       ProxyUsers.authorize(proxyUgi, host);
+      ProxyUsers.authorize(proxyUgi, toFakeAddress(host));
       fail("Allowed authorization of " + proxyUgi + " from " + host);
     } catch (AuthorizationException e) {
       // Expected
@@ -544,7 +558,9 @@ public class TestProxyUsers {
   
   private void assertAuthorized(UserGroupInformation proxyUgi, String host) {
     try {
+      // test both APIs.
       ProxyUsers.authorize(proxyUgi, host);
+      ProxyUsers.authorize(proxyUgi, toFakeAddress(host));
     } catch (AuthorizationException e) {
       fail("Did not allow authorization of " + proxyUgi + " from " + host);
     }
@@ -560,9 +576,9 @@ public class TestProxyUsers {
      * Authorize a user (superuser) to impersonate another user (user1) if the 
      * superuser belongs to the group "sudo_user1" .
      */
-
+    @Override
-    public void authorize(UserGroupInformation user, 
+    public void authorize(UserGroupInformation user,
-        String remoteAddress) throws AuthorizationException{
+        InetAddress remoteAddress) throws AuthorizationException{
       UserGroupInformation superUser = user.getRealUser();
 
       String sudoGroupName = "sudo_" + user.getShortUserName();
@@ -572,6 +588,7 @@ public class TestProxyUsers {
       }
     }
 
+
     @Override
     public void setConf(Configuration conf) {
 
@@ -597,7 +614,6 @@ public class TestProxyUsers {
         );
     ProxyUsers.refreshSuperUserGroupsConfiguration(conf);
 
-
     // First try proxying a group that's allowed
     UserGroupInformation realUserUgi = UserGroupInformation
         .createRemoteUser(REAL_USER_NAME);
@@ -608,7 +624,8 @@ public class TestProxyUsers {
     SecureRandom sr = new SecureRandom();
     for (int i=1; i < 1000000; i++){
       try {
-        ProxyUsers.authorize(proxyUserUgi,  "1.2.3."+ sr.nextInt(testRange));
+        ProxyUsers.authorize(proxyUserUgi,
+            toFakeAddress("1.2.3."+ sr.nextInt(testRange)));
        } catch (AuthorizationException e) {
       }
     }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestMachineList.java

@@ -25,9 +25,11 @@ import static org.junit.Assert.fail;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
 
+import com.google.common.net.InetAddresses;;
 import org.junit.Test;
-import org.mockito.Mockito;
 
 public class TestMachineList {
   private static String IP_LIST = "10.119.103.110,10.119.103.112,10.119.103.114";
@@ -43,10 +45,40 @@ public class TestMachineList {
   private static String HOSTNAME_IP_CIDR_LIST =
     "host1,10.222.0.0/16,10.119.103.110,10.119.103.112,10.119.103.114,10.241.23.0/24,host4,";
 
+  class TestAddressFactory extends MachineList.InetAddressFactory {
+    private Map<String, InetAddress> cache = new HashMap<>();
+    InetAddress put(String ip) throws UnknownHostException {
+      return put(ip, ip);
+    }
+    InetAddress put(String ip, String... hosts) throws UnknownHostException {
+      InetAddress addr = InetAddress.getByName(ip);
+      for (String host : hosts) {
+        addr = InetAddress.getByAddress(host, addr.getAddress());
+        cache.put(host, addr);
+        // last host wins the PTR lookup.
+        cache.put(ip, addr);
+      }
+      return addr;
+    }
+    @Override
+    public InetAddress getByName(String host) throws UnknownHostException {
+      InetAddress addr = cache.get(host);
+      if (addr == null) {
+        if (!InetAddresses.isInetAddress(host)) {
+          throw new UnknownHostException(host);
+        }
+        // ip resolves to itself to fake being unresolvable.
+        addr = InetAddress.getByName(host);
+        addr = InetAddress.getByAddress(host, addr.getAddress());
+      }
+      return addr;
+    }
+  }
+
   @Test
   public void testWildCard() {
     //create MachineList with a list of of IPs
-    MachineList ml = new MachineList("*");
+    MachineList ml = new MachineList("*", new TestAddressFactory());
 
     //test for inclusion with any IP
     assertTrue(ml.includes("10.119.103.112"));
@@ -56,7 +88,7 @@ public class TestMachineList {
   @Test
   public void testIPList() {
     //create MachineList with a list of of IPs
-    MachineList ml = new MachineList(IP_LIST);
+    MachineList ml = new MachineList(IP_LIST, new TestAddressFactory());
 
     //test for inclusion with an known IP
     assertTrue(ml.includes("10.119.103.112"));
@@ -68,7 +100,7 @@ public class TestMachineList {
   @Test
   public void testIPListSpaces() {
     //create MachineList with a ip string which has duplicate ip and spaces
-    MachineList ml = new MachineList(IP_LIST_SPACES);
+    MachineList ml = new MachineList(IP_LIST_SPACES, new TestAddressFactory());
 
     //test for inclusion with an known IP
     assertTrue(ml.includes("10.119.103.112"));
@@ -79,42 +111,28 @@ public class TestMachineList {
 
   @Test
   public void testStaticIPHostNameList()throws UnknownHostException {
-    //create MachineList with a list of of Hostnames
+    // create MachineList with a list of of Hostnames
-    InetAddress addressHost1 = InetAddress.getByName("1.2.3.1");
+    TestAddressFactory addressFactory = new TestAddressFactory();
-    InetAddress addressHost4 = InetAddress.getByName("1.2.3.4");
+    addressFactory.put("1.2.3.1", "host1");
-
+    addressFactory.put("1.2.3.4", "host4");
-    MachineList.InetAddressFactory addressFactory = 
-      Mockito.mock(MachineList.InetAddressFactory.class);
-    Mockito.when(addressFactory.getByName("host1")).thenReturn(addressHost1);
-    Mockito.when(addressFactory.getByName("host4")).thenReturn(addressHost4);
 
     MachineList ml = new MachineList(
         StringUtils.getTrimmedStringCollection(HOST_LIST), addressFactory);
 
-    //test for inclusion with an known IP
+    // test for inclusion with an known IP
     assertTrue(ml.includes("1.2.3.4"));
 
-    //test for exclusion with an unknown IP
+    // test for exclusion with an unknown IP
     assertFalse(ml.includes("1.2.3.5"));
   }
 
   @Test
   public void testHostNames() throws UnknownHostException {
-    //create MachineList with a list of of Hostnames
+    // create MachineList with a list of of Hostnames
-    InetAddress addressHost1 = InetAddress.getByName("1.2.3.1");
+    TestAddressFactory addressFactory = new TestAddressFactory();
-    InetAddress addressHost4 = InetAddress.getByName("1.2.3.4");
+    addressFactory.put("1.2.3.1", "host1");
-    InetAddress addressMockHost4 = Mockito.mock(InetAddress.class);
+    addressFactory.put("1.2.3.4", "host4", "differentname");
-    Mockito.when(addressMockHost4.getCanonicalHostName()).thenReturn("differentName");
+    addressFactory.put("1.2.3.5", "host5");
-
-    InetAddress addressMockHost5 = Mockito.mock(InetAddress.class);
-    Mockito.when(addressMockHost5.getCanonicalHostName()).thenReturn("host5");
-
-    MachineList.InetAddressFactory addressFactory = 
-      Mockito.mock(MachineList.InetAddressFactory.class);
-    Mockito.when(addressFactory.getByName("1.2.3.4")).thenReturn(addressMockHost4);
-    Mockito.when(addressFactory.getByName("1.2.3.5")).thenReturn(addressMockHost5);
-    Mockito.when(addressFactory.getByName("host1")).thenReturn(addressHost1);
-    Mockito.when(addressFactory.getByName("host4")).thenReturn(addressHost4);
 
     MachineList ml = new MachineList(
         StringUtils.getTrimmedStringCollection(HOST_LIST), addressFactory );
@@ -128,21 +146,11 @@ public class TestMachineList {
 
   @Test
   public void testHostNamesReverserIpMatch() throws UnknownHostException {
-    //create MachineList with a list of of Hostnames
+    // create MachineList with a list of of Hostnames
-    InetAddress addressHost1 = InetAddress.getByName("1.2.3.1");
+    TestAddressFactory addressFactory = new TestAddressFactory();
-    InetAddress addressHost4 = InetAddress.getByName("1.2.3.4");
+    addressFactory.put("1.2.3.1", "host1");
-    InetAddress addressMockHost4 =  Mockito.mock(InetAddress.class);
+    addressFactory.put("1.2.3.4", "host4");
-    Mockito.when(addressMockHost4.getCanonicalHostName()).thenReturn("host4");
+    addressFactory.put("1.2.3.5", "host5");
-
-    InetAddress addressMockHost5 =  Mockito.mock(InetAddress.class);
-    Mockito.when(addressMockHost5.getCanonicalHostName()).thenReturn("host5");
-
-    MachineList.InetAddressFactory addressFactory = 
-      Mockito.mock(MachineList.InetAddressFactory.class);
-    Mockito.when(addressFactory.getByName("1.2.3.4")).thenReturn(addressMockHost4);
-    Mockito.when(addressFactory.getByName("1.2.3.5")).thenReturn(addressMockHost5);
-    Mockito.when(addressFactory.getByName("host1")).thenReturn(addressHost1);
-    Mockito.when(addressFactory.getByName("host4")).thenReturn(addressHost4);
 
     MachineList ml = new MachineList(
         StringUtils.getTrimmedStringCollection(HOST_LIST), addressFactory );
@@ -157,7 +165,7 @@ public class TestMachineList {
   @Test
   public void testCIDRs() {
     //create MachineList with a list of of ip ranges specified in CIDR format
-    MachineList ml = new MachineList(CIDR_LIST);
+    MachineList ml = new MachineList(CIDR_LIST, new TestAddressFactory());
 
     //test for inclusion/exclusion 
     assertFalse(ml.includes("10.221.255.255"));
@@ -181,16 +189,17 @@ public class TestMachineList {
   @Test(expected = IllegalArgumentException.class)
   public void testNullIpAddress() {
     //create MachineList with a list of of ip ranges specified in CIDR format
-    MachineList ml = new MachineList(CIDR_LIST);
+    MachineList ml = new MachineList(CIDR_LIST, new TestAddressFactory());
 
     //test for exclusion with a null IP
-    assertFalse(ml.includes(null));
+    assertFalse(ml.includes((String) null));
+    assertFalse(ml.includes((InetAddress) null));
   }
 
   @Test
   public void testCIDRWith16bitmask() {
     //create MachineList with a list of of ip ranges specified in CIDR format
-    MachineList ml = new MachineList(CIDR_LIST1);
+    MachineList ml = new MachineList(CIDR_LIST1, new TestAddressFactory());
 
     //test for inclusion/exclusion 
     assertFalse(ml.includes("10.221.255.255"));
@@ -209,7 +218,7 @@ public class TestMachineList {
   @Test
   public void testCIDRWith8BitMask() {
     //create MachineList with a list of of ip ranges specified in CIDR format
-    MachineList ml = new MachineList(CIDR_LIST2);
+    MachineList ml = new MachineList(CIDR_LIST2, new TestAddressFactory());
 
     //test for inclusion/exclusion  
     assertFalse(ml.includes("10.241.22.255"));
@@ -228,7 +237,7 @@ public class TestMachineList {
   public void testInvalidCIDR() {
     //create MachineList with an Invalid CIDR
     try {
-    new MachineList(INVALID_CIDR);
+      MachineList ml = new MachineList(INVALID_CIDR, new TestAddressFactory());
     fail("Expected IllegalArgumentException");
     } catch (IllegalArgumentException e) {
       //expected Exception
@@ -240,7 +249,7 @@ public class TestMachineList {
   @Test
   public void testIPandCIDRs() {
     //create MachineList with a list of of ip ranges and ip addresses
-    MachineList ml = new MachineList(IP_CIDR_LIST);
+    MachineList ml = new MachineList(IP_CIDR_LIST, new TestAddressFactory());
 
     //test for inclusion with an known IP
     assertTrue(ml.includes("10.119.103.112"));
@@ -263,7 +272,8 @@ public class TestMachineList {
   @Test
   public void testHostNameIPandCIDRs() {
     //create MachineList with a mix of ip addresses , hostnames and ip ranges
-    MachineList ml = new MachineList(HOSTNAME_IP_CIDR_LIST);
+    MachineList ml = new MachineList(HOSTNAME_IP_CIDR_LIST,
+        new TestAddressFactory());
 
     //test for inclusion with an known IP
     assertTrue(ml.includes("10.119.103.112"));
@@ -286,7 +296,8 @@ public class TestMachineList {
   @Test
   public void testGetCollection() {
     //create MachineList with a mix of ip addresses , hostnames and ip ranges
-    MachineList ml = new MachineList(HOSTNAME_IP_CIDR_LIST);
+    MachineList ml =
+        new MachineList(HOSTNAME_IP_CIDR_LIST, new TestAddressFactory());
 
     Collection<String> col = ml.getCollection();
     //test getCollectionton to return the full collection