launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #09027
[Merge] lp:~rvb/maas/pxe-off-preseed into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/pxe-off-preseed into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/maas/pxe-off-preseed/+merge/111225
This branch adds an anonymous method to turn off PXE booting (to the metadata service) and uses that method to generate the url put in the preseed file. That url will be called by the node once at the end of the installation process.
= Pre-imp =
This is part of the plan which was devised last week with Gavin (see http://bit.ly/KXW5ZU for details) and I've had a pre-imp call with Julian about this.
= Notes =
- I've filed bug 1015559 about the fact that the new method does not require authentication. The bug is referenced in the code.
- The call to "wget" is now a POST request. Cobbler uses a GET request to modify data :(.
- Drive-by fix: I've fixed the value of 'server_name' in the context used when rendering the preseed file. It now uses the method get_maas_server_host.
--
https://code.launchpad.net/~rvb/maas/pxe-off-preseed/+merge/111225
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/pxe-off-preseed into lp:maas.
=== modified file 'contrib/preseeds_v2/generic'
--- contrib/preseeds_v2/generic 2012-06-19 16:17:12 +0000
+++ contrib/preseeds_v2/generic 2012-06-20 13:41:18 +0000
@@ -19,6 +19,6 @@
# Executes late command and disables PXE.
d-i preseed/late_command string true && \
in-target sh -c 'f=$1; shift; echo $0 > $f && chmod 0440 $f $*' 'ubuntu ALL=(ALL) NOPASSWD: ALL' /etc/sudoers.d/maas && \
- wget "{{node_disable_pxe_url}}" -O /dev/null && \
+ wget "{{node_disable_pxe_url}}" --post-data "{{node_disable_pxe_data}}" -O /dev/null && \
true
{{enddef}}
=== modified file 'src/maasserver/preseed.py'
--- src/maasserver/preseed.py 2012-06-19 16:01:07 +0000
+++ src/maasserver/preseed.py 2012-06-20 13:41:18 +0000
@@ -13,11 +13,13 @@
__all__ = []
from os.path import join
+from urllib import urlencode
from urlparse import urlparse
from django.conf import settings
from maasserver.models import Config
from maasserver.provisioning import compose_preseed
+from maasserver.utils import absolute_reverse
import tempita
@@ -145,13 +147,19 @@
:return: The context dictionary.
:rtype: dict.
"""
- server_host = ''
+ server_host = get_maas_server_host()
+ # 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-anon-node-edit', args=['latest', node.system_id])
+ node_disable_pxe_data = urlencode({'op': 'netboot_off'})
return {
'node': node,
'release': release,
'server_host': server_host,
'preseed_data': compose_preseed(node),
- 'node_disable_pxe_url': '', # TODO.
+ 'node_disable_pxe_url': node_disable_pxe_url,
+ 'node_disable_pxe_data': node_disable_pxe_data,
}
=== modified file 'src/maasserver/tests/test_preseed.py'
--- src/maasserver/tests/test_preseed.py 2012-06-19 16:01:07 +0000
+++ src/maasserver/tests/test_preseed.py 2012-06-20 13:41:18 +0000
@@ -304,7 +304,7 @@
context = get_preseed_context(node, release)
self.assertItemsEqual(
['node', 'release', 'server_host', 'preseed_data',
- 'node_disable_pxe_url'],
+ 'node_disable_pxe_url', 'node_disable_pxe_data'],
context)
=== modified file 'src/maasserver/tests/test_utils.py'
--- src/maasserver/tests/test_utils.py 2012-04-30 16:26:38 +0000
+++ src/maasserver/tests/test_utils.py 2012-06-20 13:41:18 +0000
@@ -12,7 +12,14 @@
__metaclass__ = type
__all__ = []
-from maasserver.utils import map_enum
+from django.conf import settings
+from django.core.urlresolvers import reverse
+from maasserver.testing.factory import factory
+from maasserver.testing.testcase import TestCase as DjangoTestCase
+from maasserver.utils import (
+ absolute_reverse,
+ map_enum,
+ )
from maastesting.testcase import TestCase
@@ -49,3 +56,28 @@
THREE = 3
self.assertEqual({'ONE': 1, 'THREE': 3}, map_enum(Enum))
+
+
+class TestAbsoluteReverse(DjangoTestCase):
+
+ def test_absolute_reverse_uses_DEFAULT_MAAS_URL(self):
+ maas_url = 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_kwargs(self):
+ node = factory.make_node()
+ self.patch(settings, 'DEFAULT_MAAS_URL', '')
+ absolute_url = absolute_reverse(
+ 'node-view', kwargs={'system_id': node.system_id})
+ expected_url = reverse('node-view', args=[node.system_id])
+ self.assertEqual(expected_url, absolute_url)
+
+ def test_absolute_reverse_uses_args(self):
+ node = factory.make_node()
+ self.patch(settings, 'DEFAULT_MAAS_URL', '')
+ absolute_url = absolute_reverse('node-view', args=[node.system_id])
+ expected_url = reverse('node-view', args=[node.system_id])
+ self.assertEqual(expected_url, absolute_url)
=== modified file 'src/maasserver/utils.py'
--- src/maasserver/utils.py 2012-06-15 05:07:40 +0000
+++ src/maasserver/utils.py 2012-06-20 13:41:18 +0000
@@ -11,11 +11,15 @@
__metaclass__ = type
__all__ = [
+ 'absolute_reverse',
'get_db_state',
'ignore_unused',
'map_enum',
]
+from django.conf import settings
+from django.core.urlresolvers import reverse
+
def get_db_state(instance, field_name):
"""Get the persisted state of a given field for a given model instance.
@@ -53,3 +57,17 @@
for key, value in vars(enum_class).items()
if not key.startswith('_')
}
+
+
+def absolute_reverse(view_name, *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
+ the configured DEFAULT_MAAS_URL.
+
+ :param view_name: Django's view function name or URL pattern name for
+ which to compute the absolute URL.
+ :param args: Positional arguments for Django's 'reverse' method.
+ :param kwargs: Named arguments for Django's 'reverse' method.
+ """
+ return settings.DEFAULT_MAAS_URL + reverse(view_name, *args, **kwargs)
=== modified file 'src/metadataserver/api.py'
--- src/metadataserver/api.py 2012-06-18 04:46:17 +0000
+++ src/metadataserver/api.py 2012-06-20 13:41:18 +0000
@@ -11,6 +11,7 @@
__metaclass__ = type
__all__ = [
+ 'AnonMetaDataHandler',
'IndexHandler',
'MetaDataHandler',
'UserDataHandler',
@@ -20,6 +21,7 @@
from django.conf import settings
from django.core.exceptions import PermissionDenied
from django.http import HttpResponse
+from django.shortcuts import get_object_or_404
from maasserver.api import (
api_exported,
api_operations,
@@ -37,6 +39,7 @@
)
from maasserver.models import (
MACAddress,
+ Node,
SSHKey,
)
from metadataserver.models import (
@@ -321,3 +324,20 @@
mimetype='application/octet-stream')
except NodeUserData.DoesNotExist:
raise MAASAPINotFound("No user data available for this node.")
+
+
+@api_operations
+class AnonMetaDataHandler(VersionIndexHandler):
+ """Anonymous metadata."""
+
+ @api_exported('POST')
+ def netboot_off(self, request, version=None, system_id=None):
+ """Turn off netboot on the node.
+
+ A commissioning node can call this to turn off netbooting when
+ it finishes installing itself.
+ """
+ node = get_object_or_404(Node, system_id=system_id)
+ node.netboot = False
+ node.save()
+ return rc.ALL_OK
=== modified file 'src/metadataserver/tests/test_api.py'
--- src/metadataserver/tests/test_api.py 2012-06-18 05:09:25 +0000
+++ src/metadataserver/tests/test_api.py 2012-06-20 13:41:18 +0000
@@ -541,3 +541,15 @@
response = client.post(self.make_url('/latest/'), {'op': 'netboot_on'})
node = reload_object(node)
self.assertTrue(node.netboot, response)
+
+ def test_anonymous_netboot_off(self):
+ node = factory.make_node(netboot=True)
+ anon_netboot_off_url = self.make_url(
+ '/latest/%s/edit/' % node.system_id)
+ response = self.client.post(
+ anon_netboot_off_url, {'op': 'netboot_off'})
+ node = reload_object(node)
+ self.assertEqual(
+ (httplib.OK, False),
+ (response.status_code, node.netboot),
+ response)
=== modified file 'src/metadataserver/urls.py'
--- src/metadataserver/urls.py 2012-06-06 03:11:48 +0000
+++ src/metadataserver/urls.py 2012-06-20 13:41:18 +0000
@@ -20,6 +20,7 @@
)
from maasserver.api_auth import api_auth
from metadataserver.api import (
+ AnonMetaDataHandler,
IndexHandler,
MetaDataHandler,
UserDataHandler,
@@ -34,6 +35,10 @@
index_handler = Resource(IndexHandler, authentication=api_auth)
+# Handlers for anonymous node operations.
+meta_data_node_anon_handler = Resource(AnonMetaDataHandler)
+
+
# Handlers for anonymous random metadata access.
meta_data_by_mac_handler = Resource(MetaDataHandler)
user_data_by_mac_handler = Resource(UserDataHandler)
@@ -56,6 +61,18 @@
url(r'', index_handler, name='metadata'),
)
+# Anonymous random metadata access. These serve requests from the nodes
+# which happen when the environment is so minimal that proper
+# authenticated calls are not possible.
+anon_patterns = patterns(
+ '',
+ # XXX: rvb 2012-06-20 bug=1015559: This method is accessible
+ # without authentication. This is a security threat.
+ url(
+ r'(?P<version>[^/]+)/(?P<system_id>[\w\-]+)/edit/$',
+ meta_data_node_anon_handler,
+ name='metadata-anon-node-edit'),
+ )
# Anonymous random metadata access keyed by MAC address. These won't
# work unless ALLOW_ANONYMOUS_METADATA_ACCESS is enabled, which you
@@ -80,4 +97,4 @@
# URL patterns. The anonymous patterns are listed first because they're
# so recognizable: there's no chance of a regular metadata access being
# mistaken for one of these based on URL pattern match.
-urlpatterns = by_mac_patterns + node_patterns
+urlpatterns = anon_patterns + by_mac_patterns + node_patterns
