Merge lp:~mpontillo/maas/prevent-static-address-in-dynamic-range--bug-1604461 into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Ensure a specified IP address is not within a dyanamic range before allowing it to be reserved.

[Andres Rodriguez (andreserl)]

lgtm! Please back port this. Btw, this has no impact on BMC's no?

review: Approve

[Andres Rodriguez (andreserl)]

I Mean, does this have any impact on BMC's? AS per our discussion a BMC will reserve an IP address.. I guess it shouldn't if we don't know the subnet, but as per discussion, we could have a dynamic range defined, which is where BMC's are DHCP'ing from, and when a BMC is added, do we reserve this IP?

[Mike Pontillo (mpontillo)]

Correct, no impact on BMCs. This is validation at the API layer. That reminds me, I need to check the websocket path too.

[Mike Pontillo (mpontillo)]

The websocket path is okay. We actually don't support this operation on the websocket; we expect the user to create devices in the UI instead, and this was never an issue for IP addresses where we know the MAC binding.

=== modified file 'src/maasserver/api/ip_addresses.py'
--- src/maasserver/api/ip_addresses.py	2016-06-07 14:55:13 +0000
+++ src/maasserver/api/ip_addresses.py	2016-07-28 23:41:52 +0000
@@ -24,6 +24,7 @@
 )
 from maasserver.exceptions import (
     MAASAPIBadRequest,
+    MAASAPIForbidden,
     MAASAPIValidationError,
 )
 from maasserver.forms import (
@@ -72,6 +73,12 @@
         :type domain: Domain
         :raises StaticIPAddressExhaustion: If no IPs available.
         """
+        dynamic_range = subnet.get_dynamic_maasipset()
+        if ip_address is not None and ip_address in dynamic_range:
+            raise MAASAPIForbidden(
+                "IP address %s belongs to an existing dynamic range. To "
+                "reserve this IP address, a MAC address is required. (Create "
+                "a device instead.)" % ip_address)
         if hostname is not None and hostname.find('.') > 0:
             hostname, domain = hostname.split('.', 1)
             domain = Domain.objects.get_domain_or_404(
 
=== modified file 'src/maasserver/api/tests/test_ipaddresses.py'
--- src/maasserver/api/tests/test_ipaddresses.py	2016-06-07 14:55:13 +0000
+++ src/maasserver/api/tests/test_ipaddresses.py	2016-07-28 23:41:52 +0000
@@ -6,7 +6,6 @@
 __all__ = []
 
 import http.client
-from unittest import skip
 
 from django.conf import settings
 from django.core.urlresolvers import reverse
@@ -23,6 +22,7 @@
 from maasserver.testing.factory import factory
 from maasserver.utils.converters import json_load_bytes
 from maasserver.utils.orm import reload_object
+from netaddr import IPAddress
 from testtools.matchers import Equals
 
 
@@ -182,14 +182,10 @@
                 "The IP address %s is already in use." % ip_in_network).encode(
                 settings.DEFAULT_CHARSET)))
 
-    @skip(
-        "XXX bug=1539248 2015-01-28 blake_r: We need to take into account "
-        "all dynamic ranges inside the subnet.")
     def test_POST_reserve_ip_address_rejects_ip_in_dynamic_range(self):
-        interface = self.make_interface()
-        net = interface.network
-        ip_in_network = interface.ip_range_low
-        response = self.post_reservation_request(net, ip_in_network)
+        subnet = factory.make_ipv4_Subnet_with_IPRanges()
+        ip = str(IPAddress(subnet.get_dynamic_maasipset().first))
+        response = self.post_reservation_request(subnet, ip)
         self.assertEqual(
             http.client.FORBIDDEN, response.status_code, response.content)