From 683ba1120171926c04854f88eeb0865891e17b4a Mon Sep 17 00:00:00 2001 From: Rakib Hasan Date: Fri, 3 Feb 2017 08:31:08 -0500 Subject: [PATCH 1/5] Fix for #223 User can send a post request to next_network and next_address actions. If it is a post request, then the action will get the networks or addresses and then save them in as network objects in the db --- nsot/api/views.py | 57 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/nsot/api/views.py b/nsot/api/views.py index 8344bf51..1aae708a 100644 --- a/nsot/api/views.py +++ b/nsot/api/views.py @@ -497,7 +497,7 @@ def supernets(self, request, pk=None, site_pk=None, *args, **kwargs): return self.list(request, queryset=networks, *args, **kwargs) - @detail_route(methods=['get']) + @detail_route(methods=['get', 'post']) def next_network(self, request, pk=None, site_pk=None, *args, **kwargs): """Return next available networks from this Network.""" network = self.get_resource_object(pk, site_pk) @@ -506,13 +506,34 @@ def next_network(self, request, pk=None, site_pk=None, *args, **kwargs): prefix_length = params.get('prefix_length') num = params.get('num') strict = qpbool(params.get('strict_allocation', False)) - networks = network.get_next_network( - prefix_length, num, strict, as_objects=False - ) - - return self.success(networks) + if request.method == 'POST': + log.debug('Recieved a post request in next_network') + networks = network.get_next_network( + prefix_length, num, strict, as_objects=True + ) + for n in networks: + is_ip = True if prefix_length == 32 or prefix_length == 128 else False + obj = models.Network(network_address=n.network_address, + broadcast_address=n.broadcast_address, + prefix_length=prefix_length, + ip_version=n.version, + site=models.Site.objects.get(pk=site_pk), + parent=network, + state='allocated', + is_ip=is_ip) + obj.save() + models.Change.objects.create( + obj=obj, user=self.request.user, event='Create' + ) + else: + log.debug('Recieved a get method in next_network') + networks = network.get_next_network( + prefix_length, num, strict, as_objects=False + ) + return self.success(networks) + return self.success([unicode(x) for x in networks]) - @detail_route(methods=['get']) + @detail_route(methods=['get', 'post']) def next_address(self, request, pk=None, site_pk=None, *args, **kwargs): """Return next available IPs from this Network.""" network = self.get_resource_object(pk, site_pk) @@ -520,9 +541,25 @@ def next_address(self, request, pk=None, site_pk=None, *args, **kwargs): params = request.query_params num = params.get('num') strict = qpbool(params.get('strict_allocation', False)) - addresses = network.get_next_address(num, strict, as_objects=False) - - return self.success(addresses) + if request.method == 'POST': + addresses = network.get_next_address(num, strict, as_objects=True) + for n in addresses: + obj = models.Network(network_address=n.network_address, + broadcast_address=n.broadcast_address, + prefix_length=32, + ip_version=n.version, + site=models.Site.objects.get(pk=site_pk), + parent=network, + state='allocated', + is_ip=True) + obj.save() + models.Change.objects.create( + obj=obj, user=self.request.user, event='Create' + ) + else: + addresses = network.get_next_address(num, strict, as_objects=False) + return self.success(addresses) + return self.success([unicode(x) for x in addresses]) @detail_route(methods=['get']) def ancestors(self, request, pk=None, site_pk=None, *args, **kwargs): From 6df9e56b6e1bff9f324a570311fd26784ecfdf09 Mon Sep 17 00:00:00 2001 From: Rakib Hasan Date: Sat, 4 Feb 2017 01:43:34 -0500 Subject: [PATCH 2/5] Added more tests If we requests that networks be allocated into the reserved states through the next_network and next_address actions, then they should be part of the results when the command nsot networks list -c 10.2.1.0/24 reserved --- nsot/api/views.py | 13 +++++++--- nsot/models.py | 2 ++ tests/api_tests/test_networks.py | 40 ++++++++++++++++++++++++++++++ tests/model_tests/test_networks.py | 4 ++- 4 files changed, 54 insertions(+), 5 deletions(-) diff --git a/nsot/api/views.py b/nsot/api/views.py index 1aae708a..06eaf0b4 100644 --- a/nsot/api/views.py +++ b/nsot/api/views.py @@ -508,18 +508,20 @@ def next_network(self, request, pk=None, site_pk=None, *args, **kwargs): strict = qpbool(params.get('strict_allocation', False)) if request.method == 'POST': log.debug('Recieved a post request in next_network') + state = 'reserved' if qpbool(params.get('reserve', False)) else 'allocated' + is_ip = True if prefix_length == 32 or prefix_length == 128 else False networks = network.get_next_network( prefix_length, num, strict, as_objects=True ) + log.debug('The state requested is %s'%state) for n in networks: - is_ip = True if prefix_length == 32 or prefix_length == 128 else False obj = models.Network(network_address=n.network_address, broadcast_address=n.broadcast_address, prefix_length=prefix_length, ip_version=n.version, site=models.Site.objects.get(pk=site_pk), parent=network, - state='allocated', + state=state, is_ip=is_ip) obj.save() models.Change.objects.create( @@ -543,14 +545,17 @@ def next_address(self, request, pk=None, site_pk=None, *args, **kwargs): strict = qpbool(params.get('strict_allocation', False)) if request.method == 'POST': addresses = network.get_next_address(num, strict, as_objects=True) + state = 'reserved' if qpbool(params.get('reserve', False)) else 'allocated' + log.debug('The state requested is %s'%state) for n in addresses: + prefix_length = 32 if n.version == 4 else 128 obj = models.Network(network_address=n.network_address, broadcast_address=n.broadcast_address, - prefix_length=32, + prefix_length=prefix_length, ip_version=n.version, site=models.Site.objects.get(pk=site_pk), parent=network, - state='allocated', + state=state, is_ip=True) obj.save() models.Change.objects.create( diff --git a/nsot/models.py b/nsot/models.py index 1e453bc0..d084fe13 100644 --- a/nsot/models.py +++ b/nsot/models.py @@ -831,6 +831,8 @@ def get_next_network(self, prefix_length, num=None, strict=False, """ start_time = time.time() # For debugging + log.debug('Allocation type is %s'% ('strict' if strict else 'loose')) + # If we're reserved, automatically ZILCH!! # TODO(jathan): Should we raise an error instead? if self.state == Network.RESERVED: diff --git a/tests/api_tests/test_networks.py b/tests/api_tests/test_networks.py index 061eef22..5557fa27 100644 --- a/tests/api_tests/test_networks.py +++ b/tests/api_tests/test_networks.py @@ -668,6 +668,46 @@ def test_get_next_detail_routes(site, client): ) +def test_next_network_allocation(site, client): + net_uri = site.list_uri('network') + + client.create(net_uri, cidr='10.1.2.0/24') + + net_24_resp = client.retrieve(net_uri, cidr='10.1.2.0/24') + net_24 = get_result(net_24_resp)[0] + net_24_obj_uri = site.detail_uri('network', id=net_24['id']) + + uri = reverse('network-next-network', args=(site.id, net_24['id'])) + + client.post(uri, params={u'prefix_length': u'32'}) + assert_success(client.retrieve(uri, prefix_length=32), [u'10.1.2.2/32']) + + client.post(uri, params={u'prefix_length': u'32', u'reserve': u'True'}) + + uri = reverse('network-reserved', args=(site.id,)) + assert get_result(client.retrieve(uri))[0]['network_address'] == u'10.1.2.2' + + +def test_next_address_allocation(site, client): + net_uri = site.list_uri('network') + + client.create(net_uri, cidr='10.1.2.0/24') + + net_24_resp = client.retrieve(net_uri, cidr='10.1.2.0/24') + net_24 = get_result(net_24_resp)[0] + net_24_obj_uri = site.detail_uri('network', id=net_24['id']) + + uri = reverse('network-next-address', args=(site.id, net_24['id'])) + + client.post(uri) + assert_success(client.retrieve(uri, prefix_length=32), [u'10.1.2.2/32']) + + client.post(uri, params={u'reserve': u'True'}) + + uri = reverse('network-reserved', args=(site.id,)) + assert get_result(client.retrieve(uri))[0]['network_address'] == u'10.1.2.2' + + def test_reservation_list_route(site, client): """Test the list route for getting reserved networks/addresses.""" net_uri = site.list_uri('network') diff --git a/tests/model_tests/test_networks.py b/tests/model_tests/test_networks.py index 84155ef1..a4c151c2 100644 --- a/tests/model_tests/test_networks.py +++ b/tests/model_tests/test_networks.py @@ -376,4 +376,6 @@ def test_strict_allocation_3(site): parent = models.Network.objects.create(site = site, cidr = u'2001:db8:abcd:0012::0/96') child = models.Network.objects.create(site = site, cidr = u'2001:db8:abcd:0012::0/97') expected = [ipaddress.ip_network(u'2001:db8:abcd:12::8000:0/128')] - assert parent.get_next_network(128, strict = True) == expected \ No newline at end of file + assert parent.get_next_network(128, strict = True) == expected + + From eaf082060df1c8ebdb843b7c0b0f4e6de411af5e Mon Sep 17 00:00:00 2001 From: Rakib Hasan Date: Mon, 6 Feb 2017 01:10:45 -0500 Subject: [PATCH 3/5] Added following changes - removed code to set is_ip and also set editable to False for is_ip in Network model - using RESERVED and ALLOCATED constants from model.Network class as state strings instead of the actual strings, 'reserved' and 'allocated' - in next_network and next_address views, moved network.get_next_network function call out of the if clause. Also set as_objects paramter for network.get_next_network to True --- nsot/api/views.py | 84 +++++++++++++----------------- nsot/models.py | 4 +- tests/model_tests/test_networks.py | 2 - 3 files changed, 37 insertions(+), 53 deletions(-) diff --git a/nsot/api/views.py b/nsot/api/views.py index 06eaf0b4..1e10b954 100644 --- a/nsot/api/views.py +++ b/nsot/api/views.py @@ -426,6 +426,23 @@ class NetworkViewSet(ResourceViewSet): lookup_value_regex = '[a-fA-F0-9:.]+(?:\/\d+)?' natural_key = 'cidr' + def allocate_networks(self, networks, prefix_length, site_pk, parent, state='allocated'): + site = models.Site.objects.get(pk=site_pk) + for n in networks: + obj = models.Network( + network_address=n.network_address, + broadcast_address=n.broadcast_address, + prefix_length=prefix_length, + ip_version=n.version, + site=site, + parent=parent, + state=state + ) + obj.save() + models.Change.objects.create( + obj=obj, user=self.request.user, event='Create' + ) + def get_serializer_class(self): if self.request.method == 'POST': return serializers.NetworkCreateSerializer @@ -501,69 +518,40 @@ def supernets(self, request, pk=None, site_pk=None, *args, **kwargs): def next_network(self, request, pk=None, site_pk=None, *args, **kwargs): """Return next available networks from this Network.""" network = self.get_resource_object(pk, site_pk) - params = request.query_params prefix_length = params.get('prefix_length') num = params.get('num') strict = qpbool(params.get('strict_allocation', False)) + networks = network.get_next_network( + prefix_length, num, strict, as_objects=True + ) if request.method == 'POST': - log.debug('Recieved a post request in next_network') - state = 'reserved' if qpbool(params.get('reserve', False)) else 'allocated' - is_ip = True if prefix_length == 32 or prefix_length == 128 else False - networks = network.get_next_network( - prefix_length, num, strict, as_objects=True - ) - log.debug('The state requested is %s'%state) - for n in networks: - obj = models.Network(network_address=n.network_address, - broadcast_address=n.broadcast_address, - prefix_length=prefix_length, - ip_version=n.version, - site=models.Site.objects.get(pk=site_pk), - parent=network, - state=state, - is_ip=is_ip) - obj.save() - models.Change.objects.create( - obj=obj, user=self.request.user, event='Create' - ) - else: - log.debug('Recieved a get method in next_network') - networks = network.get_next_network( - prefix_length, num, strict, as_objects=False - ) - return self.success(networks) + if qpbool(params.get('reserve', False)): + state = models.Network.RESERVED + else: + state = models.Network.ALLOCATED + self.allocate_networks(networks, prefix_length, site_pk, network, state=state) return self.success([unicode(x) for x in networks]) @detail_route(methods=['get', 'post']) def next_address(self, request, pk=None, site_pk=None, *args, **kwargs): """Return next available IPs from this Network.""" network = self.get_resource_object(pk, site_pk) - params = request.query_params num = params.get('num') strict = qpbool(params.get('strict_allocation', False)) + addresses = network.get_next_address(num, strict, as_objects=True) if request.method == 'POST': - addresses = network.get_next_address(num, strict, as_objects=True) - state = 'reserved' if qpbool(params.get('reserve', False)) else 'allocated' - log.debug('The state requested is %s'%state) - for n in addresses: - prefix_length = 32 if n.version == 4 else 128 - obj = models.Network(network_address=n.network_address, - broadcast_address=n.broadcast_address, - prefix_length=prefix_length, - ip_version=n.version, - site=models.Site.objects.get(pk=site_pk), - parent=network, - state=state, - is_ip=True) - obj.save() - models.Change.objects.create( - obj=obj, user=self.request.user, event='Create' - ) - else: - addresses = network.get_next_address(num, strict, as_objects=False) - return self.success(addresses) + if qpbool(params.get('reserve', False)): + state = models.Network.RESERVED + else: + state = models.Network.ALLOCATED + if addresses != []: + if addresses[0].version == 4: + prefix_length = 32 + else: + prefix_length = 128 + self.allocate_networks(addresses, prefix_length, site_pk, network, state=state) return self.success([unicode(x) for x in addresses]) @detail_route(methods=['get']) diff --git a/nsot/models.py b/nsot/models.py index d084fe13..001ed0d9 100644 --- a/nsot/models.py +++ b/nsot/models.py @@ -719,7 +719,7 @@ class Network(Resource): choices=IP_VERSION_CHOICES ) is_ip = models.BooleanField( - null=False, default=False, db_index=True, + null=False, default=False, db_index=True, editable=False, help_text='Whether the Network is a host address or not.' ) site = models.ForeignKey( @@ -831,8 +831,6 @@ def get_next_network(self, prefix_length, num=None, strict=False, """ start_time = time.time() # For debugging - log.debug('Allocation type is %s'% ('strict' if strict else 'loose')) - # If we're reserved, automatically ZILCH!! # TODO(jathan): Should we raise an error instead? if self.state == Network.RESERVED: diff --git a/tests/model_tests/test_networks.py b/tests/model_tests/test_networks.py index a4c151c2..fd669871 100644 --- a/tests/model_tests/test_networks.py +++ b/tests/model_tests/test_networks.py @@ -377,5 +377,3 @@ def test_strict_allocation_3(site): child = models.Network.objects.create(site = site, cidr = u'2001:db8:abcd:0012::0/97') expected = [ipaddress.ip_network(u'2001:db8:abcd:12::8000:0/128')] assert parent.get_next_network(128, strict = True) == expected - - From 8f8c824d25ff16d3556748e252c9a26a89499ccc Mon Sep 17 00:00:00 2001 From: Rakib Hasan Date: Wed, 31 May 2017 03:25:32 -0400 Subject: [PATCH 4/5] For Interface model functions get_ancestors and get_descendants, I am removing the separate id lists. There is no need to make a list for the Interface id's. --- nsot/models.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/nsot/models.py b/nsot/models.py index 001ed0d9..ec4ba492 100644 --- a/nsot/models.py +++ b/nsot/models.py @@ -1410,10 +1410,9 @@ def get_ancestors(self): p = self.parent ancestors = [] while p is not None: - ancestors.append(p) + ancestors.append(p.id) p = p.parent - ancestor_ids = [a.id for a in ancestors] - return Interface.objects.filter(id__in=ancestor_ids) + return Interface.objects.filter(id__in=ancestors) def get_children(self): """Return the immediate children of an Interface.""" @@ -1425,11 +1424,10 @@ def get_descendants(self): descendants = [] while len(s) > 0: top = s.pop() - descendants.append(top) + descendants.append(top.id) for c in top.get_children(): s.append(c) - descendant_ids = [c.id for c in descendants] - return Interface.objects.filter(id__in=descendant_ids) + return Interface.objects.filter(id__in=descendants) def get_root(self): """Return the parent of all ancestors of an Interface.""" From b3d67613615db3b7d339029b1dd60cc581fd9fb6 Mon Sep 17 00:00:00 2001 From: Rakib Hasan Date: Thu, 14 Sep 2017 01:55:44 -0400 Subject: [PATCH 5/5] Modified next_network and next_address to store and return cidr's - will pass as_objects=False to models.Network.get_next_network to get CIDR's - removed prefix_length & parent argument to NetworkViewSet.allocate_networks - broadcast_address, ip_version, parent are all automatically populated and overridden internally - returns return value from model.Network.get_next_network directly --- nsot/api/views.py | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/nsot/api/views.py b/nsot/api/views.py index 1e10b954..a5bef72c 100644 --- a/nsot/api/views.py +++ b/nsot/api/views.py @@ -426,18 +426,10 @@ class NetworkViewSet(ResourceViewSet): lookup_value_regex = '[a-fA-F0-9:.]+(?:\/\d+)?' natural_key = 'cidr' - def allocate_networks(self, networks, prefix_length, site_pk, parent, state='allocated'): + def allocate_networks(self, networks, site_pk, state='allocated'): site = models.Site.objects.get(pk=site_pk) for n in networks: - obj = models.Network( - network_address=n.network_address, - broadcast_address=n.broadcast_address, - prefix_length=prefix_length, - ip_version=n.version, - site=site, - parent=parent, - state=state - ) + obj = models.Network(cidr=n, site=site, state=state) obj.save() models.Change.objects.create( obj=obj, user=self.request.user, event='Create' @@ -523,15 +515,15 @@ def next_network(self, request, pk=None, site_pk=None, *args, **kwargs): num = params.get('num') strict = qpbool(params.get('strict_allocation', False)) networks = network.get_next_network( - prefix_length, num, strict, as_objects=True + prefix_length, num, strict, as_objects=False ) if request.method == 'POST': if qpbool(params.get('reserve', False)): state = models.Network.RESERVED else: state = models.Network.ALLOCATED - self.allocate_networks(networks, prefix_length, site_pk, network, state=state) - return self.success([unicode(x) for x in networks]) + self.allocate_networks(networks, site_pk, state) + return self.success(networks) @detail_route(methods=['get', 'post']) def next_address(self, request, pk=None, site_pk=None, *args, **kwargs): @@ -540,19 +532,14 @@ def next_address(self, request, pk=None, site_pk=None, *args, **kwargs): params = request.query_params num = params.get('num') strict = qpbool(params.get('strict_allocation', False)) - addresses = network.get_next_address(num, strict, as_objects=True) + addresses = network.get_next_address(num, strict, as_objects=False) if request.method == 'POST': if qpbool(params.get('reserve', False)): state = models.Network.RESERVED else: state = models.Network.ALLOCATED - if addresses != []: - if addresses[0].version == 4: - prefix_length = 32 - else: - prefix_length = 128 - self.allocate_networks(addresses, prefix_length, site_pk, network, state=state) - return self.success([unicode(x) for x in addresses]) + self.allocate_networks(addresses, site_pk, state) + return self.success(addresses) @detail_route(methods=['get']) def ancestors(self, request, pk=None, site_pk=None, *args, **kwargs):