launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14509
[Merge] lp:~rvb/maas/bug-1081701-c into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/bug-1081701-c into lp:maas.
Commit message:
When generating a preseed, use the value of the field 'maas_url' from the nodegroup (either the one related to the node for which the preseed is being generated or the one derived from the originating IP) to generate the urls present in the preseed.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1081701 in MAAS: "The metadata address mentioned in the preseed is wrong."
https://bugs.launchpad.net/maas/+bug/1081701
For more details, see:
https://code.launchpad.net/~rvb/maas/bug-1081701-c/+merge/135915
This is the part c) of the action plan described in https://bugs.launchpad.net/maas/+bug/1081701/comments/8 to fix bug 1081701.
= Notes =
First off, don't be scared by the size of the diff, there is lots of moving things around.
- I've refactored src/maasserver/fields.py to extract the 'find_nodegroup' utility into src/maasserver/utils/__init__.py. I don't think a model field should really do all that cleverness and also, extracting the bit that figures out to which nodegroup a request should be tied is better put in the calling code than in the 'clean' method of the for.
- I've changed absolute_reverse to be able to use an alternative base (i.e., not the default one: DEFAULT_MAAS_URL).
- I split some of the method in preseed.py because the enlistment related methods need the base_url to be passed around and the other preseed-generating methods (those that operate on nodes) can extract that information from node.nodegroup,maas_url. I thought it was much clearer to refactor that module a little thaan having a situation where many methods would take a (potentially None) 'node' argument *plus* a 'base_url' argument that would be used only where 'node' would be None.
- Note that I really want to do "if not base_url:" because base_url can be '' or None.
Drive-by fix:
- I've fixed the raw query inside 'find_nodegroup' to use proper escaping (instead of using string interpolation to build the sql query).
--
https://code.launchpad.net/~rvb/maas/bug-1081701-c/+merge/135915
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/bug-1081701-c into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-11-23 12:08:48 +0000
+++ src/maasserver/api.py 2012-11-23 14:38:19 +0000
@@ -172,7 +172,7 @@
from maasserver.utils import (
absolute_reverse,
build_absolute_uri,
- get_origin_ip,
+ find_nodegroup,
map_enum,
strip_domain,
)
@@ -398,9 +398,9 @@
# If 'nodegroup' is not explicitely specified, get the origin of the
# request to figure out which nodegroup the new node should be
# attached to.
- origin_ip = get_origin_ip(request)
- if origin_ip is not None:
- altered_query_data['nodegroup'] = origin_ip
+ nodegroup = find_nodegroup(request)
+ if nodegroup is not None:
+ altered_query_data['nodegroup'] = nodegroup
Form = get_node_create_form(request.user)
form = Form(altered_query_data)
@@ -1654,7 +1654,9 @@
# 1-1 mapping.
subarch = pxelinux_subarch
- preseed_url = compose_enlistment_preseed_url()
+ nodegroup = find_nodegroup(request)
+ base_url = nodegroup.maas_url if nodegroup is not None else None
+ preseed_url = compose_enlistment_preseed_url(base_url=base_url)
hostname = 'maas-enlist'
domain = Config.objects.get_config('enlistment_domain')
=== modified file 'src/maasserver/compose_preseed.py'
--- src/maasserver/compose_preseed.py 2012-10-04 14:00:06 +0000
+++ src/maasserver/compose_preseed.py 2012-11-23 14:38:19 +0000
@@ -21,7 +21,7 @@
import yaml
-def compose_cloud_init_preseed(token):
+def compose_cloud_init_preseed(token, base_url=''):
"""Compose the preseed value for a node in any state but Commissioning."""
credentials = urlencode({
'oauth_consumer_key': token.consumer.key,
@@ -40,7 +40,8 @@
# ks_meta, and it gets fed straight into debconf.
preseed_items = [
('datasources', 'multiselect', 'MAAS'),
- ('maas-metadata-url', 'string', absolute_reverse('metadata')),
+ ('maas-metadata-url', 'string', absolute_reverse(
+ 'metadata', base_url=base_url)),
('maas-metadata-credentials', 'string', credentials),
('local-cloud-config', 'string', local_config)
]
@@ -54,12 +55,13 @@
for item_name, item_type, item_value in preseed_items)
-def compose_commissioning_preseed(token):
+def compose_commissioning_preseed(token, base_url=''):
"""Compose the preseed value for a Commissioning node."""
return "#cloud-config\n%s" % yaml.safe_dump({
'datasource': {
'MAAS': {
- 'metadata_url': absolute_reverse('metadata'),
+ 'metadata_url': absolute_reverse(
+ 'metadata', base_url=base_url),
'consumer_key': token.consumer.key,
'token_key': token.key,
'token_secret': token.secret,
@@ -86,7 +88,8 @@
# Circular import.
from metadataserver.models import NodeKey
token = NodeKey.objects.get_token_for_node(node)
+ base_url = node.nodegroup.maas_url
if node.status == NODE_STATUS.COMMISSIONING:
- return compose_commissioning_preseed(token)
+ return compose_commissioning_preseed(token, base_url)
else:
- return compose_cloud_init_preseed(token)
+ return compose_cloud_init_preseed(token, base_url)
=== modified file 'src/maasserver/fields.py'
--- src/maasserver/fields.py 2012-09-21 09:06:21 +0000
+++ src/maasserver/fields.py 2012-11-23 14:38:19 +0000
@@ -32,7 +32,6 @@
ModelChoiceField,
RegexField,
)
-from maasserver.utils.orm import get_one
import psycopg2.extensions
from south.modelsinspector import add_introspection_rules
@@ -85,25 +84,6 @@
else:
return "%s: %s" % (nodegroup.name, interface.ip)
- def find_nodegroup(self, ip_address):
- """Find the nodegroup whose subnet contains `ip_address`.
-
- The matching nodegroup may have multiple interfaces on the subnet,
- but there can be only one matching nodegroup.
- """
- # Avoid circular imports.
- from maasserver.models import NodeGroup
-
- return get_one(NodeGroup.objects.raw("""
- SELECT *
- FROM maasserver_nodegroup
- WHERE id IN (
- SELECT nodegroup_id
- FROM maasserver_nodegroupinterface
- WHERE (inet '%s' & subnet_mask) = (ip & subnet_mask)
- )
- """ % ip_address))
-
def clean(self, value):
"""Django method: provide expected output for various inputs.
@@ -126,11 +106,7 @@
elif isinstance(value, bytes) and '.' not in value:
nodegroup_id = int(value)
else:
- nodegroup = self.find_nodegroup(value)
- if nodegroup is None:
- raise ValidationError(
- "No known subnet contains %s." % value)
- nodegroup_id = nodegroup.id
+ raise ValidationError("Invalid nodegroup: %s." % value)
return super(NodeGroupFormField, self).clean(nodegroup_id)
=== modified file 'src/maasserver/preseed.py'
--- src/maasserver/preseed.py 2012-11-14 10:32:25 +0000
+++ src/maasserver/preseed.py 2012-11-23 14:38:19 +0000
@@ -38,22 +38,24 @@
GENERIC_FILENAME = 'generic'
-def get_enlist_preseed():
+def get_enlist_preseed(base_url=''):
"""Return the enlistment preseed.
:return: The rendered preseed string.
:rtype: basestring.
"""
- return render_preseed(None, PRESEED_TYPE.ENLIST)
-
-
-def get_enlist_userdata():
+ return render_enlistment_preseed(
+ PRESEED_TYPE.ENLIST, base_url=base_url)
+
+
+def get_enlist_userdata(base_url=''):
"""Return the enlistment preseed.
:return: The rendered enlistment user-data string.
:rtype: basestring.
"""
- return render_preseed(None, PRESEED_TYPE.ENLIST_USERDATA)
+ return render_enlistment_preseed(
+ PRESEED_TYPE.ENLIST_USERDATA, base_url=base_url)
def get_preseed(node):
@@ -207,9 +209,9 @@
return parsed_url.hostname, parsed_url.path
-def get_preseed_context(node, release=''):
- """Return the context dictionary to be used to render preseed templates
- for this node.
+def get_preseed_context(release='', base_url=''):
+ """Return the node-independent context dictionary to be used to render
+ preseed templates.
:param node: See `get_preseed_filenames`.
:param prefix: See `get_preseed_filenames`.
@@ -222,36 +224,58 @@
Config.objects.get_config('main_archive'))
ports_archive_hostname, ports_archive_directory = get_hostname_and_path(
Config.objects.get_config('ports_archive'))
- context = {
+ return {
'main_archive_hostname': main_archive_hostname,
'main_archive_directory': main_archive_directory,
'ports_archive_hostname': ports_archive_hostname,
'ports_archive_directory': ports_archive_directory,
'release': release,
'server_host': server_host,
- 'server_url': absolute_reverse('nodes_handler'),
- 'metadata_enlist_url': absolute_reverse('enlist'),
+ 'server_url': absolute_reverse('nodes_handler', base_url=base_url),
+ 'metadata_enlist_url': absolute_reverse('enlist', base_url=base_url),
'http_proxy': Config.objects.get_config('http_proxy'),
}
- if node is not None:
- # Create the url and the url-data (POST parameters) used to turn off
- # PXE booting once the install of the node is finished.
- node_disable_pxe_url = absolute_reverse(
- 'metadata-node-by-id', args=['latest', node.system_id])
- node_disable_pxe_data = urlencode({'op': 'netboot_off'})
- node_context = {
- 'node': node,
- 'preseed_data': compose_preseed(node),
- 'node_disable_pxe_url': node_disable_pxe_url,
- 'node_disable_pxe_data': node_disable_pxe_data,
- }
- context.update(node_context)
-
- return context
+
+
+def get_node_preseed_context(node, release=''):
+ """Return the node-dependent context dictionary to be used to render
+ preseed templates.
+
+ :param node: See `get_preseed_filenames`.
+ :param prefix: See `get_preseed_filenames`.
+ :param release: See `get_preseed_filenames`.
+ :return: The context dictionary.
+ :rtype: dict.
+ """
+ # Create the url and the url-data (POST parameters) used to turn off
+ # PXE booting once the install of the node is finished.
+ node_disable_pxe_url = absolute_reverse(
+ 'metadata-node-by-id', args=['latest', node.system_id],
+ base_url=node.nodegroup.maas_url)
+ node_disable_pxe_data = urlencode({'op': 'netboot_off'})
+ return {
+ 'node': node,
+ 'preseed_data': compose_preseed(node),
+ 'node_disable_pxe_url': node_disable_pxe_url,
+ 'node_disable_pxe_data': node_disable_pxe_data,
+ }
+
+
+def render_enlistment_preseed(prefix, release='', base_url=''):
+ """Return the enlistment preseed.
+
+ :param prefix: See `get_preseed_filenames`.
+ :param release: See `get_preseed_filenames`.
+ :return: The rendered preseed string.
+ :rtype: basestring.
+ """
+ template = load_preseed_template(None, prefix, release)
+ context = get_preseed_context(release, base_url=base_url)
+ return template.substitute(**context)
def render_preseed(node, prefix, release=''):
- """Find and load a `PreseedTemplate` for the given node.
+ """Return the preseed for the given node.
:param node: See `get_preseed_filenames`.
:param prefix: See `get_preseed_filenames`.
@@ -260,23 +284,26 @@
:rtype: basestring.
"""
template = load_preseed_template(node, prefix, release)
- context = get_preseed_context(node, release)
+ base_url = node.nodegroup.maas_url
+ context = get_preseed_context(release, base_url=base_url)
+ context.update(get_node_preseed_context(node, release))
return template.substitute(**context)
-def compose_enlistment_preseed_url():
+def compose_enlistment_preseed_url(base_url=''):
"""Compose enlistment preseed URL."""
# Always uses the latest version of the metadata API.
version = 'latest'
return absolute_reverse(
'metadata-enlist-preseed', args=[version],
- query={'op': 'get_enlist_preseed'})
+ query={'op': 'get_enlist_preseed'}, base_url=base_url)
def compose_preseed_url(node):
"""Compose a metadata URL for `node`'s preseed data."""
# Always uses the latest version of the metadata API.
version = 'latest'
+ base_url = node.nodegroup.maas_url
return absolute_reverse(
'metadata-node-by-id', args=[version, node.system_id],
- query={'op': 'get_preseed'})
+ query={'op': 'get_preseed'}, base_url=base_url)
=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py 2012-11-23 13:29:10 +0000
+++ src/maasserver/testing/factory.py 2012-11-23 14:38:19 +0000
@@ -185,7 +185,7 @@
router_ip=None, network=None, subnet_mask=None,
broadcast_ip=None, ip_range_low=None,
ip_range_high=None, interface=None, management=None,
- status=None, **kwargs):
+ status=None, maas_url='', **kwargs):
"""Create a :class:`NodeGroup`.
If network (an instance of IPNetwork) is provided, use it to populate
@@ -210,6 +210,7 @@
ng = NodeGroup.objects.new(
name=name, uuid=uuid, **interface_settings)
ng.status = status
+ ng.maas_url = maas_url
ng.save()
return ng
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-11-23 12:08:48 +0000
+++ src/maasserver/tests/test_api.py 2012-11-23 14:38:19 +0000
@@ -3447,6 +3447,20 @@
compose_enlistment_preseed_url(),
json.loads(response.content)["preseed_url"])
+ def test_pxeconfig_enlistment_preseed_url_detects_request_origin(self):
+ self.silence_get_ephemeral_name()
+ ng_url = 'http://%s' % factory.make_name('host')
+ network = IPNetwork("10.1.1/24")
+ ip = factory.getRandomIPInNetwork(network)
+ factory.make_node_group(maas_url=ng_url, network=network)
+ params = self.get_default_params()
+
+ response = self.client.get(
+ reverse('pxeconfig'), params, SERVER_NAME=ip)
+ self.assertThat(
+ json.loads(response.content)["preseed_url"],
+ StartsWith(ng_url))
+
def test_pxeconfig_has_preseed_url_for_known_node(self):
params = self.get_mac_params()
node = MACAddress.objects.get(mac_address=params['mac']).node
@@ -3455,6 +3469,22 @@
compose_preseed_url(node),
json.loads(response.content)["preseed_url"])
+ def test_preseed_url_for_known_node_uses_nodegroup_maas_url(self):
+ ng_url = 'http://%s' % factory.make_name('host')
+ network = IPNetwork("10.1.1/24")
+ ip = factory.getRandomIPInNetwork(network)
+ nodegroup = factory.make_node_group(maas_url=ng_url, network=network)
+ params = self.get_mac_params()
+ node = MACAddress.objects.get(mac_address=params['mac']).node
+ node.nodegroup = nodegroup
+ node.save()
+
+ response = self.client.get(
+ reverse('pxeconfig'), params, SERVER_NAME=ip)
+ self.assertThat(
+ json.loads(response.content)["preseed_url"],
+ StartsWith(ng_url))
+
def test_get_boot_purpose_unknown_node(self):
# A node that's not yet known to MAAS is assumed to be enlisting,
# which uses a "commissioning" image.
=== modified file 'src/maasserver/tests/test_fields.py'
--- src/maasserver/tests/test_fields.py 2012-10-08 07:35:35 +0000
+++ src/maasserver/tests/test_fields.py 2012-11-23 14:38:19 +0000
@@ -21,7 +21,6 @@
from maasserver.models import (
MACAddress,
NodeGroup,
- nodegroupinterface,
NodeGroupInterface,
)
from maasserver.testing.factory import factory
@@ -33,7 +32,6 @@
JSONFieldModel,
XMLFieldModel,
)
-from netaddr import IPNetwork
class TestNodeGroupFormField(TestCase):
@@ -70,51 +68,6 @@
nodegroup,
NodeGroupFormField().clean("%s" % nodegroup.id))
- def test_clean_finds_nodegroup_by_network_address(self):
- nodegroup = factory.make_node_group(
- network=IPNetwork("192.168.28.1/24"))
- self.assertEqual(
- nodegroup,
- NodeGroupFormField().clean('192.168.28.0'))
-
- def test_find_nodegroup_looks_up_nodegroup_by_controller_ip(self):
- nodegroup = factory.make_node_group()
- self.assertEqual(
- nodegroup,
- NodeGroupFormField().clean(nodegroup.get_managed_interface().ip))
-
- def test_find_nodegroup_accepts_any_ip_in_nodegroup_subnet(self):
- nodegroup = factory.make_node_group(
- network=IPNetwork("192.168.41.0/24"))
- self.assertEqual(
- nodegroup,
- NodeGroupFormField().clean('192.168.41.199'))
-
- def test_find_nodegroup_reports_if_not_found(self):
- self.assertRaises(
- ValidationError,
- NodeGroupFormField().clean,
- factory.getRandomIPAddress())
-
- def test_find_nodegroup_reports_if_multiple_matches(self):
- self.patch(nodegroupinterface, "MINIMUM_NETMASK_BITS", 1)
- factory.make_node_group(network=IPNetwork("10/8"))
- factory.make_node_group(network=IPNetwork("10.1.1/24"))
- self.assertRaises(
- NodeGroup.MultipleObjectsReturned,
- NodeGroupFormField().clean, '10.1.1.2')
-
- def test_find_nodegroup_handles_multiple_matches_on_same_nodegroup(self):
- self.patch(nodegroupinterface, "MINIMUM_NETMASK_BITS", 1)
- nodegroup = factory.make_node_group(network=IPNetwork("10/8"))
- NodeGroupInterface.objects.create(
- nodegroup=nodegroup, ip='10.0.0.2', subnet_mask='255.0.0.0',
- broadcast_ip='10.0.0.1', interface='eth71')
- NodeGroupInterface.objects.create(
- nodegroup=nodegroup, ip='10.0.0.3', subnet_mask='255.0.0.0',
- broadcast_ip='10.0.0.2', interface='eth72')
- self.assertEqual(nodegroup, NodeGroupFormField().clean('10.0.0.9'))
-
class TestMACAddressField(TestCase):
=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py 2012-11-13 10:44:22 +0000
+++ src/maasserver/tests/test_forms.py 2012-11-23 14:38:19 +0000
@@ -198,14 +198,6 @@
NodeGroup.objects.ensure_master(),
NodeWithMACAddressesForm(self.make_params()).save().nodegroup)
- def test_sets_nodegroup_on_new_node_if_requested(self):
- nodegroup = factory.make_node_group(
- network=IPNetwork("192.168.14.0/24"), ip_range_low='192.168.14.2',
- ip_range_high='192.168.14.254', ip='192.168.14.1')
- form = NodeWithMACAddressesForm(
- self.make_params(nodegroup=nodegroup.get_managed_interface().ip))
- self.assertEqual(nodegroup, form.save().nodegroup)
-
def test_leaves_nodegroup_alone_if_unset_on_existing_node(self):
# Selecting a node group for a node is only supported on new
# nodes. You can't change it later.
=== modified file 'src/maasserver/tests/test_preseed.py'
--- src/maasserver/tests/test_preseed.py 2012-11-14 10:32:25 +0000
+++ src/maasserver/tests/test_preseed.py 2012-11-23 14:38:19 +0000
@@ -30,12 +30,14 @@
GENERIC_FILENAME,
get_enlist_preseed,
get_hostname_and_path,
+ get_node_preseed_context,
get_preseed,
get_preseed_context,
get_preseed_filenames,
get_preseed_template,
load_preseed_template,
PreseedTemplate,
+ render_enlistment_preseed,
render_preseed,
split_subarch,
TemplateNotFoundError,
@@ -46,7 +48,10 @@
from maastesting.matchers import ContainsAll
from testtools.matchers import (
AllMatch,
+ Contains,
IsInstance,
+ MatchesAll,
+ Not,
StartsWith,
)
@@ -317,23 +322,6 @@
"""Tests for `get_preseed_context`."""
def test_get_preseed_context_contains_keys(self):
- node = factory.make_node()
- release = factory.getRandomString()
- context = get_preseed_context(node, release)
- self.assertItemsEqual(
- ['node', 'release', 'metadata_enlist_url',
- 'server_host', 'server_url', 'preseed_data',
- 'node_disable_pxe_url', 'node_disable_pxe_data',
- 'main_archive_hostname', 'main_archive_directory',
- 'ports_archive_hostname', 'ports_archive_directory',
- 'http_proxy',
- ],
- context)
-
- def test_get_preseed_context_if_node_None(self):
- # If the provided Node is None (when're in the context of an
- # enlistment preseed) the returned context does not include the
- # node context.
release = factory.getRandomString()
context = get_preseed_context(None, release)
self.assertItemsEqual(
@@ -370,6 +358,20 @@
))
+class TestNodePreseedContext(TestCase):
+ """Tests for `get_node_preseed_context`."""
+
+ def test_get_node_preseed_context_contains_keys(self):
+ node = factory.make_node()
+ release = factory.getRandomString()
+ context = get_node_preseed_context(node, release)
+ self.assertItemsEqual(
+ ['node', 'preseed_data', 'node_disable_pxe_url',
+ 'node_disable_pxe_data',
+ ],
+ context)
+
+
class TestPreseedTemplate(TestCase):
"""Tests for class:`PreseedTemplate`."""
@@ -399,6 +401,36 @@
# error.
self.assertIsInstance(preseed, str)
+ def test_get_preseed_uses_nodegroup_maas_url(self):
+ ng_url = 'http://%s' % factory.make_name('host')
+ ng = factory.make_node_group(maas_url=ng_url)
+ maas_url = 'http://%s' % factory.make_name('host')
+ node = factory.make_node(
+ nodegroup=ng, status=NODE_STATUS.COMMISSIONING)
+ self.patch(settings, 'DEFAULT_MAAS_URL', maas_url)
+ preseed = render_preseed(node, self.preseed, "precise")
+ self.assertThat(
+ preseed, MatchesAll(*[Contains(ng_url), Not(Contains(maas_url))]))
+
+
+class TestRenderEnlistmentPreseed(TestCase):
+ """Tests for `render_enlistment_preseed`."""
+
+ def test_render_enlistment_preseed(self):
+ preseed = render_enlistment_preseed(PRESEED_TYPE.ENLIST, "precise")
+ # The test really is that the preseed is rendered without an
+ # error.
+ self.assertIsInstance(preseed, str)
+
+ def test_get_preseed_uses_nodegroup_maas_url(self):
+ ng_url = 'http://%s' % factory.make_name('host')
+ maas_url = 'http://%s' % factory.make_name('host')
+ self.patch(settings, 'DEFAULT_MAAS_URL', maas_url)
+ preseed = render_enlistment_preseed(
+ PRESEED_TYPE.ENLIST, "precise", base_url=ng_url)
+ self.assertThat(
+ preseed, MatchesAll(*[Contains(ng_url), Not(Contains(maas_url))]))
+
class TestRenderPreseedArchives(TestCase):
"""Test that the default preseed contains the default mirrors."""
=== modified file 'src/maasserver/utils/__init__.py'
--- src/maasserver/utils/__init__.py 2012-11-22 13:33:46 +0000
+++ src/maasserver/utils/__init__.py 2012-11-23 14:38:19 +0000
@@ -13,6 +13,7 @@
__all__ = [
'absolute_reverse',
'build_absolute_uri',
+ 'find_nodegroup',
'get_db_state',
'get_origin_ip',
'ignore_unused',
@@ -67,7 +68,7 @@
}
-def absolute_reverse(view_name, query=None, *args, **kwargs):
+def absolute_reverse(view_name, query=None, base_url=None, *args, **kwargs):
"""Return the absolute URL (i.e. including the URL scheme specifier and
the network location of the MAAS server). Internally this method simply
calls Django's 'reverse' method and prefixes the result of that call with
@@ -78,11 +79,14 @@
:param query: Optional query argument which will be passed down to
urllib.urlencode. The result of that call will be appended to the
resulting url.
+ :param base_url: Optional url used as base. If None is provided, then
+ settings.DEFAULT_MAAS_URL will be used.
:param args: Positional arguments for Django's 'reverse' method.
:param kwargs: Named arguments for Django's 'reverse' method.
"""
- url = urljoin(
- settings.DEFAULT_MAAS_URL, reverse(view_name, *args, **kwargs))
+ if not base_url:
+ base_url = settings.DEFAULT_MAAS_URL
+ url = urljoin(base_url, reverse(view_name, *args, **kwargs))
if query is not None:
url += '?%s' % urlencode(query, doseq=True)
return url
@@ -118,3 +122,26 @@
return socket.gethostbyname(host)
except socket.error:
return None
+
+
+def find_nodegroup(request):
+ """Find the nodegroup whose subnet contains the IP Address of the
+ originating host of the request..
+
+ The matching nodegroup may have multiple interfaces on the subnet,
+ but there can be only one matching nodegroup.
+ """
+ # Circular imports.
+ from maasserver.models import NodeGroup
+ ip_address = get_origin_ip(request)
+ if ip_address is not None:
+ return get_one(NodeGroup.objects.raw("""
+ SELECT *
+ FROM maasserver_nodegroup
+ WHERE id IN (
+ SELECT nodegroup_id
+ FROM maasserver_nodegroupinterface
+ WHERE (inet %s & subnet_mask) = (ip & subnet_mask)
+ )
+ """, [ip_address]))
+ return None
=== modified file 'src/maasserver/utils/tests/test_utils.py'
--- src/maasserver/utils/tests/test_utils.py 2012-11-22 13:33:46 +0000
+++ src/maasserver/utils/tests/test_utils.py 2012-11-23 14:38:19 +0000
@@ -20,11 +20,17 @@
from django.http import HttpRequest
from django.test.client import RequestFactory
from maasserver.enum import NODE_STATUS_CHOICES
+from maasserver.models import (
+ NodeGroup,
+ nodegroupinterface,
+ NodeGroupInterface,
+ )
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestCase as DjangoTestCase
from maasserver.utils import (
absolute_reverse,
build_absolute_uri,
+ find_nodegroup,
get_db_state,
get_origin_ip,
map_enum,
@@ -35,6 +41,7 @@
call,
Mock,
)
+from netaddr import IPNetwork
class TestEnum(TestCase):
@@ -74,13 +81,19 @@
class TestAbsoluteReverse(DjangoTestCase):
- def test_absolute_reverse_uses_DEFAULT_MAAS_URL(self):
+ def test_absolute_reverse_uses_DEFAULT_MAAS_URL_by_default(self):
maas_url = 'http://%s' % factory.getRandomString()
self.patch(settings, 'DEFAULT_MAAS_URL', maas_url)
absolute_url = absolute_reverse('settings')
expected_url = settings.DEFAULT_MAAS_URL + reverse('settings')
self.assertEqual(expected_url, absolute_url)
+ def test_absolute_reverse_uses_given_base_url(self):
+ maas_url = 'http://%s' % factory.getRandomString()
+ absolute_url = absolute_reverse('settings', base_url=maas_url)
+ expected_url = maas_url + reverse('settings')
+ self.assertEqual(expected_url, absolute_url)
+
def test_absolute_reverse_uses_query_string(self):
self.patch(settings, 'DEFAULT_MAAS_URL', '')
parameters = {factory.getRandomString(): factory.getRandomString()}
@@ -184,26 +197,27 @@
self.assertEqual(results, map(strip_domain, inputs))
+def get_request(server_name, server_port='80'):
+ return RequestFactory().post(
+ '/', SERVER_NAME=server_name, SERVER_PORT=server_port)
+
+
class TestGetOriginIP(TestCase):
- def get_request(self, server_name, server_port='80'):
- return RequestFactory().post(
- '/', SERVER_NAME=server_name, SERVER_PORT=server_port)
-
def test_get_origin_ip_returns_ip(self):
ip = factory.getRandomIPAddress()
- request = self.get_request(ip)
+ request = get_request(ip)
self.assertEqual(ip, get_origin_ip(request))
def test_get_origin_ip_strips_port(self):
ip = factory.getRandomIPAddress()
- request = self.get_request(ip, '8888')
+ request = get_request(ip, '8888')
self.assertEqual(ip, get_origin_ip(request))
def test_get_origin_ip_resolves_hostname(self):
ip = factory.getRandomIPAddress()
hostname = factory.make_name('hostname')
- request = self.get_request(hostname)
+ request = get_request(hostname)
resolver = self.patch(socket, 'gethostbyname', Mock(return_value=ip))
self.assertEqual(
(ip, call(hostname)),
@@ -211,9 +225,56 @@
def test_get_origin_ip_returns_None_if_hostname_cannot_get_resolved(self):
hostname = factory.make_name('hostname')
- request = self.get_request(hostname)
+ request = get_request(hostname)
resolver = self.patch(
socket, 'gethostbyname', Mock(side_effect=socket.error))
self.assertEqual(
(None, call(hostname)),
(get_origin_ip(request), resolver.call_args))
+
+
+class TestFindNodegroup(DjangoTestCase):
+
+ def test_finds_nodegroup_by_network_address(self):
+ nodegroup = factory.make_node_group(
+ network=IPNetwork("192.168.28.1/24"))
+ self.assertEqual(
+ nodegroup,
+ find_nodegroup(get_request('192.168.28.0')))
+
+ def test_find_nodegroup_looks_up_nodegroup_by_controller_ip(self):
+ nodegroup = factory.make_node_group()
+ ip = nodegroup.get_managed_interface().ip
+ self.assertEqual(
+ nodegroup,
+ find_nodegroup(get_request(ip)))
+
+ def test_find_nodegroup_accepts_any_ip_in_nodegroup_subnet(self):
+ nodegroup = factory.make_node_group(
+ network=IPNetwork("192.168.41.0/24"))
+ self.assertEqual(
+ nodegroup,
+ find_nodegroup(get_request('192.168.41.199')))
+
+ def test_find_nodegroup_returns_None_if_not_found(self):
+ self.assertIsNone(
+ find_nodegroup(get_request(factory.getRandomIPAddress())))
+
+ def test_find_nodegroup_errors_if_multiple_matches(self):
+ self.patch(nodegroupinterface, "MINIMUM_NETMASK_BITS", 1)
+ factory.make_node_group(network=IPNetwork("10/8"))
+ factory.make_node_group(network=IPNetwork("10.1.1/24"))
+ self.assertRaises(
+ NodeGroup.MultipleObjectsReturned,
+ find_nodegroup, get_request('10.1.1.2'))
+
+ def test_find_nodegroup_handles_multiple_matches_on_same_nodegroup(self):
+ self.patch(nodegroupinterface, "MINIMUM_NETMASK_BITS", 1)
+ nodegroup = factory.make_node_group(network=IPNetwork("10/8"))
+ NodeGroupInterface.objects.create(
+ nodegroup=nodegroup, ip='10.0.0.2', subnet_mask='255.0.0.0',
+ broadcast_ip='10.0.0.1', interface='eth71')
+ NodeGroupInterface.objects.create(
+ nodegroup=nodegroup, ip='10.0.0.3', subnet_mask='255.0.0.0',
+ broadcast_ip='10.0.0.2', interface='eth72')
+ self.assertEqual(nodegroup, find_nodegroup(get_request('10.0.0.9')))
