Merge lp:~blake-rouse/maas/fix-1441841 into lp:~maas-committers/maas/trunk
- fix-1441841
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Blake Rouse |
Approved revision: | no longer in the source branch. |
Merged at revision: | 3838 |
Proposed branch: | lp:~blake-rouse/maas/fix-1441841 |
Merge into: | lp:~maas-committers/maas/trunk |
Diff against target: |
913 lines (+436/-103) 13 files modified
src/maasserver/api/ip_addresses.py (+5/-3) src/maasserver/api/tests/test_ipaddresses.py (+22/-5) src/maasserver/api/tests/test_node.py (+5/-3) src/maasserver/models/macaddress.py (+3/-0) src/maasserver/models/staticipaddress.py (+35/-16) src/maasserver/models/tests/test_staticipaddress.py (+109/-45) src/maasserver/static/js/angular/controllers/add_device.js (+9/-5) src/maasserver/static/js/angular/controllers/tests/test_add_device.js (+13/-0) src/maasserver/static/js/angular/services/tests/test_validation.js (+164/-15) src/maasserver/static/js/angular/services/validation.js (+48/-1) src/maasserver/tests/test_forms_nodegroupinterface.py (+3/-1) src/maasserver/tests/test_forms_validate_new_static_ip_range.py (+18/-8) src/maasserver/tests/test_node_action.py (+2/-1) |
To merge this branch: | bzr merge lp:~blake-rouse/maas/fix-1441841 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Raphaël Badin (community) | Approve | ||
Andres Rodriguez (community) | Needs Information | ||
Review via email: mp+256988@code.launchpad.net |
Commit message
Fix the validation in AddDeviceContro
This allows users to add a static ip address anywhere in the selected network except for the dynamic range.
Description of the change
Blake Rouse (blake-rouse) wrote : | # |
Sorry meant dynamic range. Fixed commit message.
Blake Rouse (blake-rouse) wrote : | # |
This required a fix in the backend not just the frontend. This now allows a requested_address to be anywhere in the network, not just within the static range.
Raphaël Badin (rvb) wrote : | # |
Looks good. Can you please add an API test where the requested IP is *not* in the static range (nor in the dynamic range)?
Blake Rouse (blake-rouse) wrote : | # |
I added that test. While doing so I noticed that I was not actually validating that the requested_address was not inside the dynamic range. I added that logic and tests as well.
Preview Diff
1 | === modified file 'src/maasserver/api/ip_addresses.py' |
2 | --- src/maasserver/api/ip_addresses.py 2015-04-17 08:32:15 +0000 |
3 | +++ src/maasserver/api/ip_addresses.py 2015-04-23 19:17:12 +0000 |
4 | @@ -81,12 +81,14 @@ |
5 | """ |
6 | if mac is None: |
7 | sip = StaticIPAddress.objects.allocate_new( |
8 | - range_low=interface.static_ip_range_low, |
9 | - range_high=interface.static_ip_range_high, |
10 | + network=interface.network, |
11 | + static_range_low=interface.static_ip_range_low, |
12 | + static_range_high=interface.static_ip_range_high, |
13 | + dynamic_range_low=interface.ip_range_low, |
14 | + dynamic_range_high=interface.ip_range_high, |
15 | alloc_type=IPADDRESS_TYPE.USER_RESERVED, |
16 | requested_address=requested_address, |
17 | user=user, hostname=hostname) |
18 | - commit_within_atomic_block() |
19 | maaslog.info("User %s was allocated IP %s", user.username, sip.ip) |
20 | else: |
21 | # The user has requested a static IP linked to a MAC |
22 | |
23 | === modified file 'src/maasserver/api/tests/test_ipaddresses.py' |
24 | --- src/maasserver/api/tests/test_ipaddresses.py 2015-04-17 07:26:25 +0000 |
25 | +++ src/maasserver/api/tests/test_ipaddresses.py 2015-04-23 19:17:12 +0000 |
26 | @@ -213,6 +213,25 @@ |
27 | self.expectThat(returned_address["ip"], Equals(ip_in_network)) |
28 | self.expectThat(staticipaddress.ip, Equals(ip_in_network)) |
29 | |
30 | + def test_POST_reserve_creates_address_outside_of_static_range(self): |
31 | + interface = self.make_interface() |
32 | + interface.ip_range_high = unicode( |
33 | + IPAddress(interface.ip_range_high) - 1) |
34 | + interface.save() |
35 | + net = interface.network |
36 | + ip_in_network = unicode( |
37 | + IPAddress(interface.ip_range_high) + 1) |
38 | + response = self.post_reservation_request( |
39 | + net=net, requested_address=ip_in_network) |
40 | + self.assertEqual(httplib.OK, response.status_code, response.content) |
41 | + returned_address = json.loads(response.content) |
42 | + [staticipaddress] = StaticIPAddress.objects.all() |
43 | + self.expectThat( |
44 | + returned_address["alloc_type"], |
45 | + Equals(IPADDRESS_TYPE.USER_RESERVED)) |
46 | + self.expectThat(returned_address["ip"], Equals(ip_in_network)) |
47 | + self.expectThat(staticipaddress.ip, Equals(ip_in_network)) |
48 | + |
49 | def test_POST_reserve_requested_address_detects_in_use_address(self): |
50 | interface = self.make_interface() |
51 | net = interface.network |
52 | @@ -227,13 +246,11 @@ |
53 | response.content, Equals( |
54 | "The IP address %s is already in use." % ip_in_network)) |
55 | |
56 | - def test_POST_reserve_requested_address_detects_out_of_range_addr(self): |
57 | + def test_POST_reserve_requested_address_rejects_ip_in_dynamic_range(self): |
58 | interface = self.make_interface() |
59 | net = interface.network |
60 | - ip_not_in_network = unicode( |
61 | - IPAddress(interface.static_ip_range_low) - 1) |
62 | - response = self.post_reservation_request( |
63 | - net=net, requested_address=ip_not_in_network) |
64 | + ip_in_network = interface.ip_range_low |
65 | + response = self.post_reservation_request(net, ip_in_network) |
66 | self.assertEqual( |
67 | httplib.FORBIDDEN, response.status_code, response.content) |
68 | |
69 | |
70 | === modified file 'src/maasserver/api/tests/test_node.py' |
71 | --- src/maasserver/api/tests/test_node.py 2015-04-17 08:36:07 +0000 |
72 | +++ src/maasserver/api/tests/test_node.py 2015-04-23 19:17:12 +0000 |
73 | @@ -1208,11 +1208,12 @@ |
74 | [observed_static_ip] = StaticIPAddress.objects.all() |
75 | self.assertEqual(observed_static_ip.ip, requested_address) |
76 | |
77 | - def test_claim_sticky_ip_address_detects_out_of_range_requested_ip(self): |
78 | + def test_claim_sticky_ip_address_detects_out_of_network_requested_ip(self): |
79 | self.become_admin() |
80 | node = factory.make_Node_with_MACAddress_and_NodeGroupInterface() |
81 | ngi = node.get_primary_mac().cluster_interface |
82 | - requested_address = IPAddress(ngi.static_ip_range_low) - 1 |
83 | + other_network = factory.make_ipv4_network(but_not=ngi.network) |
84 | + requested_address = factory.pick_ip_in_network(other_network) |
85 | |
86 | response = self.client.post( |
87 | self.get_node_uri(node), |
88 | @@ -1265,7 +1266,8 @@ |
89 | ngi.save() |
90 | with transaction.atomic(): |
91 | StaticIPAddress.objects.allocate_new( |
92 | - ngi.static_ip_range_high, ngi.static_ip_range_low) |
93 | + ngi.network, ngi.static_ip_range_low, ngi.static_ip_range_high, |
94 | + ngi.ip_range_low, ngi.ip_range_high) |
95 | |
96 | response = self.client.post( |
97 | TestNodeAPI.get_node_uri(node), {'op': 'start'}) |
98 | |
99 | === modified file 'src/maasserver/models/macaddress.py' |
100 | --- src/maasserver/models/macaddress.py 2015-04-08 20:14:03 +0000 |
101 | +++ src/maasserver/models/macaddress.py 2015-04-23 19:17:12 +0000 |
102 | @@ -238,8 +238,11 @@ |
103 | ) |
104 | |
105 | new_sip = StaticIPAddress.objects.allocate_new( |
106 | + cluster_interface.network, |
107 | cluster_interface.static_ip_range_low, |
108 | cluster_interface.static_ip_range_high, |
109 | + cluster_interface.ip_range_low, |
110 | + cluster_interface.ip_range_high, |
111 | alloc_type, requested_address=requested_address, |
112 | user=user) |
113 | MACStaticIPAddressLink(mac_address=self, ip_address=new_sip).save() |
114 | |
115 | === modified file 'src/maasserver/models/staticipaddress.py' |
116 | --- src/maasserver/models/staticipaddress.py 2015-04-16 12:15:52 +0000 |
117 | +++ src/maasserver/models/staticipaddress.py 2015-04-23 19:17:12 +0000 |
118 | @@ -131,13 +131,24 @@ |
119 | ipaddress.save() |
120 | return ipaddress |
121 | |
122 | - def allocate_new(self, range_low, range_high, |
123 | - alloc_type=IPADDRESS_TYPE.AUTO, user=None, |
124 | - requested_address=None, hostname=None): |
125 | + def allocate_new( |
126 | + self, network, static_range_low, static_range_high, |
127 | + dynamic_range_low, dynamic_range_high, |
128 | + alloc_type=IPADDRESS_TYPE.AUTO, user=None, |
129 | + requested_address=None, hostname=None): |
130 | """Return a new StaticIPAddress. |
131 | |
132 | - :param range_low: The lowest address to allocate in a range |
133 | - :param range_high: The highest address to allocate in a range |
134 | + :param network: The network the address should be allocated in. |
135 | + :param static_range_low: The lowest static address to allocate in a |
136 | + range. Used if `requested_address` is not passed. |
137 | + :param static_range_high: The highest static address to allocate in a |
138 | + range. Used if `requested_address` is not passed. |
139 | + :param dynamic_range_low: The lowest dynamic address. Used if |
140 | + `requested_address` is passed, check that its not inside the |
141 | + dynamic range. |
142 | + :param dynamic_range_high: The highest dynamic address. Used if |
143 | + `requested_address` is passed, check that its not inside the |
144 | + dynamic range. |
145 | :param alloc_type: What sort of IP address to allocate in the |
146 | range of choice in IPADDRESS_TYPE. |
147 | :param user: If providing a user, the alloc_type must be |
148 | @@ -157,15 +168,15 @@ |
149 | # taken, and so we must first eliminate all other possible causes. |
150 | self._verify_alloc_type(alloc_type, user) |
151 | |
152 | - range_low = IPAddress(range_low) |
153 | - range_high = IPAddress(range_high) |
154 | - static_range = IPRange(range_low, range_high) |
155 | - |
156 | if requested_address is None: |
157 | + static_range_low = IPAddress(static_range_low) |
158 | + static_range_high = IPAddress(static_range_high) |
159 | + static_range = IPRange(static_range_low, static_range_high) |
160 | + |
161 | with locks.staticip_acquire: |
162 | requested_address = self._async_find_free_ip( |
163 | - range_low, range_high, static_range, alloc_type, user, |
164 | - hostname=hostname).wait(30) |
165 | + static_range_low, static_range_high, static_range, |
166 | + alloc_type, user, hostname=hostname).wait(30) |
167 | try: |
168 | return self._attempt_allocation( |
169 | requested_address, alloc_type, user, |
170 | @@ -176,12 +187,20 @@ |
171 | # let the retry mechanism do its thing. |
172 | raise make_serialization_failure() |
173 | else: |
174 | + dynamic_range_low = IPAddress(dynamic_range_low) |
175 | + dynamic_range_high = IPAddress(dynamic_range_high) |
176 | + dynamic_range = IPRange(dynamic_range_low, dynamic_range_high) |
177 | + |
178 | requested_address = IPAddress(requested_address) |
179 | - if requested_address not in static_range: |
180 | - raise StaticIPAddressOutOfRange( |
181 | - "%s is not inside the range %s to %s" % ( |
182 | - requested_address.format(), range_low.format(), |
183 | - range_high.format())) |
184 | + if requested_address not in network: |
185 | + raise StaticIPAddressOutOfRange( |
186 | + "%s is not inside the network %s" % ( |
187 | + requested_address.format(), network)) |
188 | + if requested_address in dynamic_range: |
189 | + raise StaticIPAddressOutOfRange( |
190 | + "%s is inside the dynamic range %s to %s" % ( |
191 | + requested_address.format(), dynamic_range_low.format(), |
192 | + dynamic_range_high.format())) |
193 | return self._attempt_allocation( |
194 | requested_address, alloc_type, user=user, hostname=hostname) |
195 | |
196 | |
197 | === modified file 'src/maasserver/models/tests/test_staticipaddress.py' |
198 | --- src/maasserver/models/tests/test_staticipaddress.py 2015-04-16 12:30:02 +0000 |
199 | +++ src/maasserver/models/tests/test_staticipaddress.py 2015-04-23 19:17:12 +0000 |
200 | @@ -48,108 +48,164 @@ |
201 | |
202 | class TestStaticIPAddressManager(MAASServerTestCase): |
203 | |
204 | + def make_ip_ranges(self, network=None): |
205 | + interface = factory.make_NodeGroupInterface( |
206 | + factory.make_NodeGroup(), network=network) |
207 | + return ( |
208 | + interface.network, |
209 | + interface.static_ip_range_low, |
210 | + interface.static_ip_range_high, |
211 | + interface.ip_range_low, |
212 | + interface.ip_range_low, |
213 | + ) |
214 | + |
215 | def test_allocate_new_returns_ip_in_correct_range(self): |
216 | - low, high = factory.make_ip_range() |
217 | - ipaddress = StaticIPAddress.objects.allocate_new(low, high) |
218 | + network, static_low, static_high, dynamic_low, dynamic_high = ( |
219 | + self.make_ip_ranges()) |
220 | + ipaddress = StaticIPAddress.objects.allocate_new( |
221 | + network, static_low, static_high, dynamic_low, dynamic_high) |
222 | self.assertIsInstance(ipaddress, StaticIPAddress) |
223 | - iprange = IPRange(low, high) |
224 | + iprange = IPRange(static_low, static_high) |
225 | self.assertIn(IPAddress(ipaddress.ip), iprange) |
226 | |
227 | def test_allocate_new_allocates_IPv6_address(self): |
228 | - low, high = factory.make_ipv6_range() |
229 | - ipaddress = StaticIPAddress.objects.allocate_new(low, high) |
230 | + network = factory.make_ipv6_network() |
231 | + network, static_low, static_high, dynamic_low, dynamic_high = ( |
232 | + self.make_ip_ranges(network)) |
233 | + ipaddress = StaticIPAddress.objects.allocate_new( |
234 | + network, static_low, static_high, dynamic_low, dynamic_high) |
235 | self.assertIsInstance(ipaddress, StaticIPAddress) |
236 | - self.assertIn(IPAddress(ipaddress.ip), IPRange(low, high)) |
237 | + self.assertIn( |
238 | + IPAddress(ipaddress.ip), IPRange(static_low, static_high)) |
239 | |
240 | def test_allocate_new_sets_user(self): |
241 | - low, high = factory.make_ip_range() |
242 | + network, static_low, static_high, dynamic_low, dynamic_high = ( |
243 | + self.make_ip_ranges()) |
244 | user = factory.make_User() |
245 | ipaddress = StaticIPAddress.objects.allocate_new( |
246 | - low, high, alloc_type=IPADDRESS_TYPE.USER_RESERVED, user=user) |
247 | + network, static_low, static_high, dynamic_low, dynamic_high, |
248 | + alloc_type=IPADDRESS_TYPE.USER_RESERVED, user=user) |
249 | self.assertEqual(user, ipaddress.user) |
250 | |
251 | def test_allocate_new_with_user_disallows_wrong_alloc_types(self): |
252 | - low, high = factory.make_ip_range() |
253 | + network, static_low, static_high, dynamic_low, dynamic_high = ( |
254 | + self.make_ip_ranges()) |
255 | user = factory.make_User() |
256 | alloc_type = factory.pick_enum( |
257 | IPADDRESS_TYPE, but_not=[IPADDRESS_TYPE.USER_RESERVED]) |
258 | self.assertRaises( |
259 | - AssertionError, StaticIPAddress.objects.allocate_new, low, high, |
260 | + AssertionError, StaticIPAddress.objects.allocate_new, network, |
261 | + static_low, static_high, dynamic_low, dynamic_high, |
262 | user=user, alloc_type=alloc_type) |
263 | |
264 | def test_allocate_new_with_reserved_type_requires_a_user(self): |
265 | - low, high = factory.make_ip_range() |
266 | + network, static_low, static_high, dynamic_low, dynamic_high = ( |
267 | + self.make_ip_ranges()) |
268 | self.assertRaises( |
269 | - AssertionError, StaticIPAddress.objects.allocate_new, low, high, |
270 | + AssertionError, StaticIPAddress.objects.allocate_new, network, |
271 | + static_low, static_high, dynamic_low, dynamic_high, |
272 | alloc_type=IPADDRESS_TYPE.USER_RESERVED) |
273 | |
274 | def test_allocate_new_compares_by_IP_not_alphabetically(self): |
275 | # Django has a bug that casts IP addresses with HOST(), which |
276 | # results in alphabetical comparisons of strings instead of IP |
277 | # addresses. See https://bugs.launchpad.net/maas/+bug/1338452 |
278 | - low = "10.0.0.98" |
279 | - high = "10.0.0.100" |
280 | + network = "10.0.0.0/8" |
281 | + static_low = "10.0.0.98" |
282 | + static_high = "10.0.0.100" |
283 | + dynamic_low = "10.0.0.101" |
284 | + dynamic_high = "10.0.0.105" |
285 | factory.make_StaticIPAddress("10.0.0.99") |
286 | - ipaddress = StaticIPAddress.objects.allocate_new(low, high) |
287 | + ipaddress = StaticIPAddress.objects.allocate_new( |
288 | + network, static_low, static_high, dynamic_low, dynamic_high) |
289 | self.assertEqual(ipaddress.ip, "10.0.0.98") |
290 | |
291 | def test_allocate_new_returns_requested_IP_if_available(self): |
292 | - low, high = factory.make_ip_range() |
293 | - requested_address = low + 1 |
294 | + network, static_low, static_high, dynamic_low, dynamic_high = ( |
295 | + self.make_ip_ranges()) |
296 | + requested_address = unicode(IPAddress(static_low) + 1) |
297 | ipaddress = StaticIPAddress.objects.allocate_new( |
298 | - low, high, factory.pick_enum( |
299 | + network, static_low, static_high, dynamic_low, dynamic_high, |
300 | + factory.pick_enum( |
301 | IPADDRESS_TYPE, but_not=[IPADDRESS_TYPE.USER_RESERVED]), |
302 | requested_address=requested_address) |
303 | self.assertEqual(requested_address.format(), ipaddress.ip) |
304 | |
305 | def test_allocate_new_raises_when_requested_IP_unavailable(self): |
306 | - low, high = factory.make_ip_range() |
307 | + network, static_low, static_high, dynamic_low, dynamic_high = ( |
308 | + self.make_ip_ranges()) |
309 | requested_address = StaticIPAddress.objects.allocate_new( |
310 | - low, high, factory.pick_enum( |
311 | + network, static_low, static_high, dynamic_low, dynamic_high, |
312 | + factory.pick_enum( |
313 | IPADDRESS_TYPE, but_not=[IPADDRESS_TYPE.USER_RESERVED])).ip |
314 | self.assertRaises( |
315 | StaticIPAddressUnavailable, StaticIPAddress.objects.allocate_new, |
316 | - low, high, requested_address=requested_address) |
317 | + network, static_low, static_high, dynamic_low, dynamic_high, |
318 | + requested_address=requested_address) |
319 | |
320 | def test_allocate_new_raises_serialization_error_if_ip_taken(self): |
321 | - low, high = factory.make_ip_range() |
322 | + network, static_low, static_high, dynamic_low, dynamic_high = ( |
323 | + self.make_ip_ranges()) |
324 | # Simulate a "IP already taken" error. |
325 | mock_attempt_allocation = self.patch( |
326 | StaticIPAddress.objects, '_attempt_allocation') |
327 | mock_attempt_allocation.side_effect = StaticIPAddressUnavailable() |
328 | |
329 | error = self.assertRaises( |
330 | - Exception, StaticIPAddress.objects.allocate_new, low, high) |
331 | + Exception, StaticIPAddress.objects.allocate_new, |
332 | + network, static_low, static_high, dynamic_low, dynamic_high) |
333 | self.assertTrue(is_serialization_failure(error)) |
334 | |
335 | def test_allocate_new_does_not_use_lock_for_requested_ip(self): |
336 | # When requesting a specific IP address, there's no need to |
337 | # acquire the lock. |
338 | lock = self.patch(locks, 'staticip_acquire') |
339 | - low, high = factory.make_ip_range() |
340 | - requested_address = low + 1 |
341 | + network, static_low, static_high, dynamic_low, dynamic_high = ( |
342 | + self.make_ip_ranges()) |
343 | + requested_address = unicode(IPAddress(static_low) + 1) |
344 | ipaddress = StaticIPAddress.objects.allocate_new( |
345 | - low, high, requested_address=requested_address) |
346 | + network, static_low, static_high, dynamic_low, dynamic_high, |
347 | + requested_address=requested_address) |
348 | self.assertIsInstance(ipaddress, StaticIPAddress) |
349 | self.assertThat(lock.__enter__, MockNotCalled()) |
350 | |
351 | - def test_allocate_new_raises_when_requested_IP_out_of_range(self): |
352 | - low, high = factory.make_ip_range() |
353 | - requested_address = low - 1 |
354 | - e = self.assertRaises( |
355 | - StaticIPAddressOutOfRange, StaticIPAddress.objects.allocate_new, |
356 | - low, high, factory.pick_enum( |
357 | - IPADDRESS_TYPE, but_not=[IPADDRESS_TYPE.USER_RESERVED]), |
358 | - requested_address=requested_address) |
359 | - self.assertEqual( |
360 | - "%s is not inside the range %s to %s" % ( |
361 | - requested_address, low, high), |
362 | + def test_allocate_new_raises_when_requested_IP_out_of_network(self): |
363 | + network, static_low, static_high, dynamic_low, dynamic_high = ( |
364 | + self.make_ip_ranges()) |
365 | + other_network = factory.make_ipv4_network(but_not=network) |
366 | + requested_address = factory.pick_ip_in_network(other_network) |
367 | + e = self.assertRaises( |
368 | + StaticIPAddressOutOfRange, StaticIPAddress.objects.allocate_new, |
369 | + network, static_low, static_high, dynamic_low, dynamic_high, |
370 | + factory.pick_enum( |
371 | + IPADDRESS_TYPE, but_not=[IPADDRESS_TYPE.USER_RESERVED]), |
372 | + requested_address=requested_address) |
373 | + self.assertEqual( |
374 | + "%s is not inside the network %s" % ( |
375 | + requested_address, network), |
376 | + e.message) |
377 | + |
378 | + def test_allocate_new_raises_when_requested_IP_in_dynamic_range(self): |
379 | + network, static_low, static_high, dynamic_low, dynamic_high = ( |
380 | + self.make_ip_ranges()) |
381 | + requested_address = dynamic_low |
382 | + e = self.assertRaises( |
383 | + StaticIPAddressOutOfRange, StaticIPAddress.objects.allocate_new, |
384 | + network, static_low, static_high, dynamic_low, dynamic_high, |
385 | + factory.pick_enum( |
386 | + IPADDRESS_TYPE, but_not=[IPADDRESS_TYPE.USER_RESERVED]), |
387 | + requested_address=requested_address) |
388 | + self.assertEqual( |
389 | + "%s is inside the dynamic range %s to %s" % ( |
390 | + requested_address, dynamic_low, dynamic_high), |
391 | e.message) |
392 | |
393 | def test_allocate_new_raises_when_alloc_type_is_None(self): |
394 | error = self.assertRaises( |
395 | ValueError, StaticIPAddress.objects.allocate_new, |
396 | - sentinel.range_low, sentinel.range_high, alloc_type=None) |
397 | + sentinel.network, sentinel.static_range_low, |
398 | + sentinel.static_range_low, sentinel.dynamic_range_low, |
399 | + sentinel.dynamic_range_high, alloc_type=None) |
400 | self.assertEqual( |
401 | "IP address type None is not a member of IPADDRESS_TYPE.", |
402 | unicode(error)) |
403 | @@ -157,15 +213,19 @@ |
404 | def test_allocate_new_raises_when_alloc_type_is_invalid(self): |
405 | error = self.assertRaises( |
406 | ValueError, StaticIPAddress.objects.allocate_new, |
407 | - sentinel.range_low, sentinel.range_high, alloc_type=12345) |
408 | + sentinel.network, sentinel.static_range_low, |
409 | + sentinel.static_range_low, sentinel.dynamic_range_low, |
410 | + sentinel.dynamic_range_high, alloc_type=12345) |
411 | self.assertEqual( |
412 | "IP address type 12345 is not a member of IPADDRESS_TYPE.", |
413 | unicode(error)) |
414 | |
415 | def test_allocate_new_uses_staticip_acquire_lock(self): |
416 | lock = self.patch(locks, 'staticip_acquire') |
417 | - low, high = factory.make_ip_range() |
418 | - ipaddress = StaticIPAddress.objects.allocate_new(low, high) |
419 | + network, static_low, static_high, dynamic_low, dynamic_high = ( |
420 | + self.make_ip_ranges()) |
421 | + ipaddress = StaticIPAddress.objects.allocate_new( |
422 | + network, static_low, static_high, dynamic_low, dynamic_high) |
423 | self.assertIsInstance(ipaddress, StaticIPAddress) |
424 | self.assertThat(lock.__enter__, MockCalledOnceWith()) |
425 | self.assertThat( |
426 | @@ -273,15 +333,19 @@ |
427 | ''' |
428 | |
429 | def test_allocate_new_raises_when_addresses_exhausted(self): |
430 | - low = high = "192.168.230.1" |
431 | + network = "192.168.230.0/24" |
432 | + static_low = static_high = "192.168.230.1" |
433 | + dynamic_low = dynamic_high = "192.168.230.2" |
434 | with transaction.atomic(): |
435 | - StaticIPAddress.objects.allocate_new(low, high) |
436 | + StaticIPAddress.objects.allocate_new( |
437 | + network, static_low, static_high, dynamic_low, dynamic_high) |
438 | with transaction.atomic(): |
439 | e = self.assertRaises( |
440 | StaticIPAddressExhaustion, |
441 | - StaticIPAddress.objects.allocate_new, low, high) |
442 | + StaticIPAddress.objects.allocate_new, |
443 | + network, static_low, static_high, dynamic_low, dynamic_high) |
444 | self.assertEqual( |
445 | - "No more IPs available in range %s-%s" % (low, high), |
446 | + "No more IPs available in range %s-%s" % (static_low, static_high), |
447 | unicode(e)) |
448 | |
449 | |
450 | |
451 | === modified file 'src/maasserver/static/js/angular/controllers/add_device.js' |
452 | --- src/maasserver/static/js/angular/controllers/add_device.js 2015-04-03 18:35:20 +0000 |
453 | +++ src/maasserver/static/js/angular/controllers/add_device.js 2015-04-23 19:17:12 +0000 |
454 | @@ -139,16 +139,16 @@ |
455 | if(!ValidationService.validateIP($scope.device.ipAddress)) { |
456 | return true; |
457 | } |
458 | - var i, inRange, managedInterfaces = $scope.getManagedInterfaces(); |
459 | + var i, inNetwork, managedInterfaces = $scope.getManagedInterfaces(); |
460 | if(angular.isObject($scope.device.ipAssignment)){ |
461 | if($scope.device.ipAssignment.name === "external") { |
462 | // External IP address cannot be within a managed interface |
463 | // on one of the clusters. |
464 | for(i = 0; i < managedInterfaces.length; i++) { |
465 | - inRange = ValidationService.validateIPInRange( |
466 | + inNetwork = ValidationService.validateIPInNetwork( |
467 | $scope.device.ipAddress, |
468 | managedInterfaces[i].network); |
469 | - if(inRange) { |
470 | + if(inNetwork) { |
471 | return true; |
472 | } |
473 | } |
474 | @@ -158,9 +158,13 @@ |
475 | // of the selected clusterInterface. |
476 | var clusterInterface = getInterfaceById( |
477 | $scope.device.clusterInterfaceId); |
478 | - inRange = ValidationService.validateIPInRange( |
479 | + inNetwork = ValidationService.validateIPInNetwork( |
480 | $scope.device.ipAddress, clusterInterface.network); |
481 | - if(!inRange) { |
482 | + var inDynamicRange = ValidationService.validateIPInRange( |
483 | + $scope.device.ipAddress, clusterInterface.network, |
484 | + clusterInterface.dynamic_range.low, |
485 | + clusterInterface.dynamic_range.high); |
486 | + if(!inNetwork || inDynamicRange) { |
487 | return true; |
488 | } |
489 | } |
490 | |
491 | === modified file 'src/maasserver/static/js/angular/controllers/tests/test_add_device.js' |
492 | --- src/maasserver/static/js/angular/controllers/tests/test_add_device.js 2015-04-03 18:35:20 +0000 |
493 | +++ src/maasserver/static/js/angular/controllers/tests/test_add_device.js 2015-04-23 19:17:12 +0000 |
494 | @@ -403,6 +403,19 @@ |
495 | }; |
496 | expect($scope.ipHasError()).toBe(true); |
497 | }); |
498 | + |
499 | + it("returns true if static ip in dynamic range of network", function() { |
500 | + var controller = makeController(); |
501 | + var nic = makeManagedClusterInterface(); |
502 | + var cluster = makeCluster([nic]); |
503 | + $scope.clusters = [cluster]; |
504 | + $scope.device.ipAddress = nic.dynamic_range.low; |
505 | + $scope.device.clusterInterfaceId = nic.id; |
506 | + $scope.device.ipAssignment = { |
507 | + name: "static" |
508 | + }; |
509 | + expect($scope.ipHasError()).toBe(true); |
510 | + }); |
511 | }); |
512 | |
513 | describe("deviceHasError", function() { |
514 | |
515 | === modified file 'src/maasserver/static/js/angular/services/tests/test_validation.js' |
516 | --- src/maasserver/static/js/angular/services/tests/test_validation.js 2015-04-03 18:26:06 +0000 |
517 | +++ src/maasserver/static/js/angular/services/tests/test_validation.js 2015-04-23 19:17:12 +0000 |
518 | @@ -267,51 +267,200 @@ |
519 | }); |
520 | }); |
521 | |
522 | - describe("validateIPInRange", function() { |
523 | + describe("validateIPInNetwork", function() { |
524 | |
525 | var scenarios = [ |
526 | { |
527 | ip: "192.168.2.1", |
528 | - range: "192.168.1.0/24", |
529 | + network: "192.168.1.0/24", |
530 | valid: false |
531 | }, |
532 | { |
533 | ip: "192.168.1.1", |
534 | - range: "192.168.1.0/24", |
535 | + network: "192.168.1.0/24", |
536 | valid: true |
537 | }, |
538 | { |
539 | ip: "192.168.1.1", |
540 | - range: "172.16.0.0/16", |
541 | + network: "172.16.0.0/16", |
542 | valid: false |
543 | }, |
544 | { |
545 | ip: "172.17.1.1", |
546 | - range: "172.16.0.0/16", |
547 | + network: "172.16.0.0/16", |
548 | valid: false |
549 | }, |
550 | { |
551 | ip: "172.16.1.1", |
552 | - range: "172.16.0.0/16", |
553 | + network: "172.16.0.0/16", |
554 | valid: true |
555 | }, |
556 | { |
557 | ip: "11.1.1.1", |
558 | - range: "10.0.0.0/8", |
559 | + network: "10.0.0.0/8", |
560 | valid: false |
561 | }, |
562 | { |
563 | ip: "10.1.1.1", |
564 | - range: "10.0.0.0/8", |
565 | - valid: true |
566 | - } |
567 | - ]; |
568 | - |
569 | - angular.forEach(scenarios, function(scenario) { |
570 | - it("validates: " + scenario.ip + " in range: " + scenario.range, |
571 | + network: "10.0.0.0/8", |
572 | + valid: true |
573 | + }, |
574 | + { |
575 | + ip: "2001:67C:1562::16", |
576 | + network: "2001:67C:1562::0/32", |
577 | + valid: true |
578 | + }, |
579 | + { |
580 | + ip: "2002:67C:1562::16", |
581 | + network: "2001:67C:1562::0/32", |
582 | + valid: false |
583 | + }, |
584 | + { |
585 | + ip: "2001:67C:1561::16", |
586 | + network: "2001:67C:1562::0/64", |
587 | + valid: false |
588 | + } |
589 | + ]; |
590 | + |
591 | + angular.forEach(scenarios, function(scenario) { |
592 | + it("validates: " + scenario.ip + " in network: " + scenario.network, |
593 | + function() { |
594 | + var result = ValidationService.validateIPInNetwork( |
595 | + scenario.ip, scenario.network); |
596 | + expect(result).toBe(scenario.valid); |
597 | + }); |
598 | + }); |
599 | + }); |
600 | + |
601 | + describe("validateIPInRange", function() { |
602 | + |
603 | + var scenarios = [ |
604 | + { |
605 | + ip: "192.168.1.1", |
606 | + network: "192.168.1.0/24", |
607 | + range: { |
608 | + low: "192.168.1.2", |
609 | + high: "192.168.1.100" |
610 | + }, |
611 | + valid: false |
612 | + }, |
613 | + { |
614 | + ip: "192.168.1.2", |
615 | + network: "192.168.1.0/24", |
616 | + range: { |
617 | + low: "192.168.1.2", |
618 | + high: "192.168.1.100" |
619 | + }, |
620 | + valid: true |
621 | + }, |
622 | + { |
623 | + ip: "192.168.1.3", |
624 | + network: "192.168.1.0/24", |
625 | + range: { |
626 | + low: "192.168.1.2", |
627 | + high: "192.168.1.100" |
628 | + }, |
629 | + valid: true |
630 | + }, |
631 | + { |
632 | + ip: "192.168.1.100", |
633 | + network: "192.168.1.0/24", |
634 | + range: { |
635 | + low: "192.168.1.2", |
636 | + high: "192.168.1.100" |
637 | + }, |
638 | + valid: true |
639 | + }, |
640 | + { |
641 | + ip: "192.168.1.101", |
642 | + network: "192.168.1.0/24", |
643 | + range: { |
644 | + low: "192.168.1.2", |
645 | + high: "192.168.1.100" |
646 | + }, |
647 | + valid: false |
648 | + }, |
649 | + { |
650 | + ip: "192.168.1.2", |
651 | + network: "192.168.2.0/24", |
652 | + range: { |
653 | + low: "192.168.2.2", |
654 | + high: "192.168.2.100" |
655 | + }, |
656 | + valid: false |
657 | + }, |
658 | + { |
659 | + ip: "2001:67C:1562::1", |
660 | + network: "2001:67C:1562::0/32", |
661 | + range: { |
662 | + low: "2001:67C:1562::2", |
663 | + high: "2001:67C:1562::FFFF:FFFF" |
664 | + }, |
665 | + valid: false |
666 | + }, |
667 | + { |
668 | + ip: "2001:67C:1562::2", |
669 | + network: "2001:67C:1562::0/32", |
670 | + range: { |
671 | + low: "2001:67C:1562::2", |
672 | + high: "2001:67C:1562::FFFF:FFFF" |
673 | + }, |
674 | + valid: true |
675 | + }, |
676 | + { |
677 | + ip: "2001:67C:1562::3", |
678 | + network: "2001:67C:1562::0/32", |
679 | + range: { |
680 | + low: "2001:67C:1562::2", |
681 | + high: "2001:67C:1562::FFFF:FFFF" |
682 | + }, |
683 | + valid: true |
684 | + }, |
685 | + { |
686 | + ip: "2001:67C:1562::FFFF:FFFE", |
687 | + network: "2001:67C:1562::0/32", |
688 | + range: { |
689 | + low: "2001:67C:1562::2", |
690 | + high: "2001:67C:1562::FFFF:FFFF" |
691 | + }, |
692 | + valid: true |
693 | + }, |
694 | + { |
695 | + ip: "2001:67C:1562::FFFF:FFFF", |
696 | + network: "2001:67C:1562::0/32", |
697 | + range: { |
698 | + low: "2001:67C:1562::2", |
699 | + high: "2001:67C:1562::FFFF:FFFF" |
700 | + }, |
701 | + valid: true |
702 | + }, |
703 | + { |
704 | + ip: "2001:67C:1562::1:0:0", |
705 | + network: "2001:67C:1562::0/32", |
706 | + range: { |
707 | + low: "2001:67C:1562::2", |
708 | + high: "2001:67C:1562::FFFF:FFFF" |
709 | + }, |
710 | + valid: false |
711 | + }, |
712 | + { |
713 | + ip: "2001:67C:1562::2", |
714 | + network: "2001:67C:1563::0/64", |
715 | + range: { |
716 | + low: "2001:67C:1563::2", |
717 | + high: "2001:67C:1563::FFFF:FFFF" |
718 | + }, |
719 | + valid: false |
720 | + } |
721 | + ]; |
722 | + |
723 | + angular.forEach(scenarios, function(scenario) { |
724 | + it("validates: " + scenario.ip + " in range: " + |
725 | + scenario.range.low + " - " + scenario.range.high, |
726 | function() { |
727 | var result = ValidationService.validateIPInRange( |
728 | - scenario.ip, scenario.range); |
729 | + scenario.ip, scenario.network, |
730 | + scenario.range.low, scenario.range.high); |
731 | expect(result).toBe(scenario.valid); |
732 | }); |
733 | }); |
734 | |
735 | === modified file 'src/maasserver/static/js/angular/services/validation.js' |
736 | --- src/maasserver/static/js/angular/services/validation.js 2015-04-03 18:26:06 +0000 |
737 | +++ src/maasserver/static/js/angular/services/validation.js 2015-04-23 19:17:12 +0000 |
738 | @@ -155,7 +155,7 @@ |
739 | }; |
740 | |
741 | // Return true if the ipAddress is in the network. |
742 | - this.validateIPInRange = function(ipAddress, network) { |
743 | + this.validateIPInNetwork = function(ipAddress, network) { |
744 | var networkSplit = network.split('/'); |
745 | var networkAddress = networkSplit[0]; |
746 | var cidrBits = parseInt(networkSplit[1], 10); |
747 | @@ -175,4 +175,51 @@ |
748 | } |
749 | return false; |
750 | }; |
751 | + |
752 | + // Return true if the ipAddress is in the network and between the |
753 | + // lowAddress and highAddress inclusive. |
754 | + this.validateIPInRange = function( |
755 | + ipAddress, network, lowAddress, highAddress) { |
756 | + // If the ip address is not even in the network then its |
757 | + // not in the range. |
758 | + if(!this.validateIPInNetwork(ipAddress, network)) { |
759 | + return false; |
760 | + } |
761 | + |
762 | + var i, ipOctets, lowOctets, highOctets; |
763 | + if(this.validateIPv4(ipAddress) && |
764 | + this.validateIPv4(lowAddress) && |
765 | + this.validateIPv4(highAddress)) { |
766 | + |
767 | + // Check that each octet is of the ip address is more or equal |
768 | + // to the low address and less or equal to the high address. |
769 | + ipOctets = ipv4ToOctets(ipAddress); |
770 | + lowOctets = ipv4ToOctets(lowAddress); |
771 | + highOctets = ipv4ToOctets(highAddress); |
772 | + for(i = 0; i < 4; i++) { |
773 | + if(ipOctets[i] > highOctets[i] || |
774 | + ipOctets[i] < lowOctets[i]) { |
775 | + return false; |
776 | + } |
777 | + } |
778 | + return true; |
779 | + } else if(this.validateIPv6(ipAddress) && |
780 | + this.validateIPv6(lowAddress) && |
781 | + this.validateIPv6(highAddress)) { |
782 | + |
783 | + // Check that each octet is of the ip address is more or equal |
784 | + // to the low address and less or equal to the high address. |
785 | + ipOctets = ipv6ToOctets(ipAddress); |
786 | + lowOctets = ipv6ToOctets(lowAddress); |
787 | + highOctets = ipv6ToOctets(highAddress); |
788 | + for(i = 0; i < 8; i++) { |
789 | + if(ipOctets[i] > highOctets[i] || |
790 | + ipOctets[i] < lowOctets[i]) { |
791 | + return false; |
792 | + } |
793 | + } |
794 | + return true; |
795 | + } |
796 | + return false; |
797 | + }; |
798 | }); |
799 | |
800 | === modified file 'src/maasserver/tests/test_forms_nodegroupinterface.py' |
801 | --- src/maasserver/tests/test_forms_nodegroupinterface.py 2015-03-25 15:33:23 +0000 |
802 | +++ src/maasserver/tests/test_forms_nodegroupinterface.py 2015-04-23 19:17:12 +0000 |
803 | @@ -212,7 +212,9 @@ |
804 | network=network) |
805 | [interface] = nodegroup.get_managed_interfaces() |
806 | StaticIPAddress.objects.allocate_new( |
807 | - interface.static_ip_range_low, interface.static_ip_range_high) |
808 | + interface.network, interface.static_ip_range_low, |
809 | + interface.static_ip_range_high, interface.ip_range_low, |
810 | + interface.ip_range_high) |
811 | form = NodeGroupInterfaceForm( |
812 | data={'static_ip_range_low': '', 'static_ip_range_high': ''}, |
813 | instance=interface) |
814 | |
815 | === modified file 'src/maasserver/tests/test_forms_validate_new_static_ip_range.py' |
816 | --- src/maasserver/tests/test_forms_validate_new_static_ip_range.py 2015-03-25 15:33:23 +0000 |
817 | +++ src/maasserver/tests/test_forms_validate_new_static_ip_range.py 2015-04-23 19:17:12 +0000 |
818 | @@ -49,7 +49,8 @@ |
819 | |
820 | def test_raises_error_when_allocated_ips_fall_outside_new_range(self): |
821 | interface = self.make_interface() |
822 | - StaticIPAddress.objects.allocate_new('10.1.0.56', '10.1.0.60') |
823 | + StaticIPAddress.objects.allocate_new( |
824 | + '10.1.0.0/16', '10.1.0.56', '10.1.0.60', '10.1.0.1', '10.1.0.10') |
825 | error = self.assertRaises( |
826 | ValidationError, |
827 | validate_new_static_ip_ranges, |
828 | @@ -62,7 +63,8 @@ |
829 | |
830 | def test_removing_static_range_raises_error_if_ips_allocated(self): |
831 | interface = self.make_interface() |
832 | - StaticIPAddress.objects.allocate_new('10.1.0.56', '10.1.0.60') |
833 | + StaticIPAddress.objects.allocate_new( |
834 | + '10.1.0.0/16', '10.1.0.56', '10.1.0.60', '10.1.0.1', '10.1.0.10') |
835 | error = self.assertRaises( |
836 | ValidationError, |
837 | validate_new_static_ip_ranges, |
838 | @@ -75,7 +77,8 @@ |
839 | |
840 | def test_allows_range_expansion(self): |
841 | interface = self.make_interface() |
842 | - StaticIPAddress.objects.allocate_new('10.1.0.56', '10.1.0.60') |
843 | + StaticIPAddress.objects.allocate_new( |
844 | + '10.1.0.0/16', '10.1.0.56', '10.1.0.60', '10.1.0.1', '10.1.0.10') |
845 | is_valid = validate_new_static_ip_ranges( |
846 | interface, static_ip_range_low='10.1.0.40', |
847 | static_ip_range_high='10.1.0.100') |
848 | @@ -83,7 +86,8 @@ |
849 | |
850 | def test_allows_allocated_ip_as_upper_bound(self): |
851 | interface = self.make_interface() |
852 | - StaticIPAddress.objects.allocate_new('10.1.0.55', '10.1.0.55') |
853 | + StaticIPAddress.objects.allocate_new( |
854 | + '10.1.0.0/16', '10.1.0.55', '10.1.0.55', '10.1.0.1', '10.1.0.10') |
855 | is_valid = validate_new_static_ip_ranges( |
856 | interface, |
857 | static_ip_range_low=interface.static_ip_range_low, |
858 | @@ -92,7 +96,8 @@ |
859 | |
860 | def test_allows_allocated_ip_as_lower_bound(self): |
861 | interface = self.make_interface() |
862 | - StaticIPAddress.objects.allocate_new('10.1.0.55', '10.1.0.55') |
863 | + StaticIPAddress.objects.allocate_new( |
864 | + '10.1.0.0/16', '10.1.0.55', '10.1.0.55', '10.1.0.1', '10.1.0.10') |
865 | is_valid = validate_new_static_ip_ranges( |
866 | interface, static_ip_range_low='10.1.0.55', |
867 | static_ip_range_high=interface.static_ip_range_high) |
868 | @@ -103,7 +108,9 @@ |
869 | interface.management = NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED |
870 | interface.save() |
871 | StaticIPAddress.objects.allocate_new( |
872 | - interface.static_ip_range_low, interface.static_ip_range_high) |
873 | + interface.network, interface.static_ip_range_low, |
874 | + interface.static_ip_range_high, interface.ip_range_low, |
875 | + interface.ip_range_high) |
876 | is_valid = validate_new_static_ip_ranges( |
877 | interface, static_ip_range_low='10.1.0.57', |
878 | static_ip_range_high='10.1.0.58') |
879 | @@ -114,7 +121,8 @@ |
880 | interface.static_ip_range_low = None |
881 | interface.static_ip_range_high = None |
882 | interface.save() |
883 | - StaticIPAddress.objects.allocate_new('10.1.0.56', '10.1.0.60') |
884 | + StaticIPAddress.objects.allocate_new( |
885 | + '10.1.0.0/16', '10.1.0.56', '10.1.0.60', '10.1.0.1', '10.1.0.10') |
886 | is_valid = validate_new_static_ip_ranges( |
887 | interface, static_ip_range_low='10.1.0.57', |
888 | static_ip_range_high='10.1.0.58') |
889 | @@ -123,7 +131,9 @@ |
890 | def test_ignores_unchanged_static_range(self): |
891 | interface = self.make_interface() |
892 | StaticIPAddress.objects.allocate_new( |
893 | - interface.static_ip_range_low, interface.static_ip_range_high) |
894 | + interface.network, interface.static_ip_range_low, |
895 | + interface.static_ip_range_high, interface.ip_range_low, |
896 | + interface.ip_range_high) |
897 | is_valid = validate_new_static_ip_ranges( |
898 | interface, |
899 | static_ip_range_low=interface.static_ip_range_low, |
900 | |
901 | === modified file 'src/maasserver/tests/test_node_action.py' |
902 | --- src/maasserver/tests/test_node_action.py 2015-04-16 13:49:51 +0000 |
903 | +++ src/maasserver/tests/test_node_action.py 2015-04-23 19:17:12 +0000 |
904 | @@ -485,7 +485,8 @@ |
905 | ngi.save() |
906 | with transaction.atomic(): |
907 | StaticIPAddress.objects.allocate_new( |
908 | - ngi.static_ip_range_high, ngi.static_ip_range_low) |
909 | + ngi.network, ngi.static_ip_range_low, ngi.static_ip_range_high, |
910 | + ngi.ip_range_low, ngi.ip_range_high) |
911 | |
912 | e = self.assertRaises(NodeActionError, Deploy(node, user).execute) |
913 | self.expectThat( |
Except for the static range? or except for the dynamic range?