Merge ~blake-rouse/maas:http-boot-endpoint into maas:master

[Blake Rouse]

Add a HTTP /boot endpoint inside of the rackd process that takes a request and processes it through the same backend as the TFTP server. Add / proxy_pass to the nginx configuration to send requests on 5248 to the /boot endpoint.

This only adds the endpoint it does *not* update any booting machine to actually use the endpoint. That will occur in a following branch.

To validate that it works on a machine with the rackd install the following command should generate the same file that is located in /var/lib/maas/boot-resources/current/grub/grub.cfg.

curl http://localhost:5248/grub/grub.cfg

To test with PXE and a dynamically generated file use:

curl http://localhost:5248/pxelinux.cfg-01-{$mac-with-dashes}

[Alberto Donato (ack)]

Nice! +1

It would be nice to also add a metric for transfer times for http boot resources (similarly to the one we have for tftp).
It's not a blocker for this branch, though

review: Approve

[Andres Rodriguez (andreserl)]

lgtm! Just quick questions inline.

review: Approve

diff --git a/src/provisioningserver/rackdservices/http.py b/src/provisioningserver/rackdservices/http.py
index a4b5976..b0f9332 100644
--- a/src/provisioningserver/rackdservices/http.py
+++ b/src/provisioningserver/rackdservices/http.py
@@ -15,6 +15,7 @@ import sys
 
 import attr
 from netaddr import IPAddress
+from provisioningserver import services
 from provisioningserver.events import (
     EVENT_TYPES,
     send_node_event_ip_address,
@@ -30,12 +31,19 @@ from provisioningserver.utils import (
 )
 from provisioningserver.utils.fs import atomic_write
 from provisioningserver.utils.twisted import callOut
+from tftp.errors import (
+    AccessViolation,
+    FileNotFound,
+)
 from twisted.application.internet import TimerService
 from twisted.internet import reactor
 from twisted.internet.defer import maybeDeferred
 from twisted.internet.task import deferLater
 from twisted.internet.threads import deferToThread
+from twisted.python import context
 from twisted.web import resource
+from twisted.web.server import NOT_DONE_YET
+from twisted.web.static import NoRangeStaticProducer
 
 
 log = LegacyLogger()
@@ -226,11 +234,100 @@ class HTTPLogResource(resource.Resource):
         return b''
 
 
+class HTTPBootResource(resource.Resource):
+    isLeaf = True
+
+    def render_GET(self, request):
+        # Be sure that the TFTP endpoint is running.
+        try:
+            tftp = services.getServiceNamed('tftp')
+        except KeyError:
+            # TFTP service is not installed cannot handle a boot request.
+            request.setResponseCode(503)
+            return b'HTTP boot service not ready.'
+
+        # Extract the local servers IP/port of the request.
+        localHost = request.getHeader('X-Server-Addr')
+        try:
+            localPort = int(request.getHeader('X-Server-Port'))
+        except (TypeError, ValueError):
+            localPort = 0
+
+        # Extract the original clients IP/port of the request.
+        remoteHost = request.getHeader('X-Forwarded-For')
+        try:
+            remotePort = int(request.getHeader('X-Forwarded-Port'))
+        except (TypeError, ValueError):
+            remotePort = 0
+
+        # localHost and remoteHost are required headers.
+        if not localHost or not remoteHost:
+            request.setResponseCode(400)
+            return b'Missing X-Server-Addr and X-Forwarded-For HTTP headers.'
+
+        def handleFailure(failure):
+            if failure.check(AccessViolation):
+                request.setResponseCode(403)
+                request.write(b'')
+            elif failure.check(FileNotFound):
+                request.setResponseCode(404)
+                request.write(b'')
+            else:
+                log.err(failure, "Failed to handle boot HTTP request.")
+                request.setResponseCode(500)
+                request.write(str(failure.value).encode('utf-8'))
+            request.finish()
+
+        def writeResponse(reader):
+            # Some readers from `tftp` do not provide a way to get the size
+            # of the generated content. Only set `Content-Length` when size
+            # can be determined for the response.
+            if hasattr(reader, 'size'):
+                request.setHeader(b'Content-Length', reader.size)
+
+            # The readers from `tftp` use `finish` instead of `close`, but
+            # `NoRangeStaticProducer` expects `close` instead of `finish`. Map
+            # `finish` to `close` so the file handlers are cleaned up.
+            reader.close = reader.finish
+
+            # Produce the result without allowing range. This producer will
+            # call `close` on the reader and `finish` on the request when done.
+            producer = NoRangeStaticProducer(request, reader)
+            producer.start()
+
+        path = b'/'.join(request.postpath)
+        d = context.call(
+            {
+                "local": (localHost, localPort),
+                "remote": (remoteHost, remotePort),
+            },
+            tftp.backend.get_reader, path, skip_logging=True)
+        d.addCallback(writeResponse)
+        d.addErrback(handleFailure)
+        d.addErrback(log.err, "Failed to handle boot HTTP request.")
+
+        # Log the HTTP request to rackd.log and push that event to the
+        # region controller.
+        log_path = path.decode('utf-8')
+        log.info(
+            "{path} requested by {remoteHost}",
+            path=log_path, remoteHost=remoteHost)
+        d = deferLater(
+            reactor, 0, send_node_event_ip_address,
+            event_type=EVENT_TYPES.NODE_HTTP_REQUEST,
+            ip_address=remoteHost, description=log_path)
+        d.addErrback(log.err, "Logging HTTP request failed.")
+
+        # Response is handled in the defer.
+        return NOT_DONE_YET
+
+
 class HTTPResource(resource.Resource):
     """The root resource for HTTP."""
 
     def __init__(self):
         super().__init__()
+        self.putChild(b'boot', HTTPBootResource())
         self.putChild(b'log', HTTPLogResource())
         self.putChild(
             b'metrics', PrometheusMetricsResource(PROMETHEUS_METRICS))
diff --git a/src/provisioningserver/rackdservices/tests/test_http.py b/src/provisioningserver/rackdservices/tests/test_http.py
index 42227bc..9f10fa2 100644
--- a/src/provisioningserver/rackdservices/tests/test_http.py
+++ b/src/provisioningserver/rackdservices/tests/test_http.py
@@ -27,6 +27,7 @@ from maastesting.twisted import (
     TwistedLoggerFixture,
 )
 from provisioningserver import services
+from provisioningserver.boot import BytesReader
 from provisioningserver.events import EVENT_TYPES
 from provisioningserver.rackdservices import http
 from provisioningserver.rpc import (
@@ -41,11 +42,26 @@ from testtools.matchers import (
     IsInstance,
     MatchesStructure,
 )
+from tftp.errors import (
+    AccessViolation,
+    FileNotFound,
+)
+from twisted.application.service import Service
 from twisted.internet import reactor
-from twisted.internet.defer import inlineCallbacks
+from twisted.internet.defer import (
+    fail,
+    inlineCallbacks,
+    succeed,
+)
 from twisted.web.http_headers import Headers
-from twisted.web.server import Request
-from twisted.web.test.test_web import DummyChannel
+from twisted.web.server import (
+    NOT_DONE_YET,
+    Request,
+)
+from twisted.web.test.test_web import (
+    DummyChannel,
+    DummyRequest,
+)
 
 
 def prepareRegion(test):
@@ -301,3 +317,244 @@ class TestHTTPLogResource(MAASTestCase):
                 ANY, 0, http.send_node_event_ip_address,
                 event_type=EVENT_TYPES.NODE_HTTP_REQUEST,
                 ip_address=ip, description=path))
+
+
+class TestHTTPBootResource(MAASTestCase):
+
+    run_tests_with = MAASTwistedRunTest.make_factory(timeout=5)
+
+    def setUp(self):
+        super().setUp()
+        self.tftp = Service()
+        self.tftp.setName('tftp')
+        self.tftp.backend = Mock()
+        self.tftp.backend.get_reader = Mock()
+        self.tftp.setServiceParent(services)
+
+        def teardown():
+            if self.tftp:
+                self.tftp.disownServiceParent()
+                self.tftp = None
+
+        self.addCleanup(teardown)
+
+    def render_GET(self, resource, request):
+        result = resource.render_GET(request)
+        if isinstance(result, bytes):
+            request.write(result)
+            request.finish()
+            return succeed(None)
+        elif result is NOT_DONE_YET:
+            if request.finished:
+                return succeed(None)
+            else:
+                return request.notifyFinish()
+        else:
+            raise ValueError("Unexpected return value: %r" % (result,))
+
+    @inlineCallbacks
+    def test_render_GET_503_when_no_tftp_service(self):
+        # Remove the fake 'tftp' service.
+        self.tftp.disownServiceParent()
+        self.tftp = None
+
+        path = factory.make_name('path')
+        ip = factory.make_ip_address()
+        request = DummyRequest([path.encode('utf-8')])
+        request.requestHeaders = Headers({
+            'X-Server-Addr': ['192.168.1.1'],
+            'X-Server-Port': ['5248'],
+            'X-Forwarded-For': [ip],
+            'X-Forwarded-Port': ['%s' % factory.pick_port()],
+        })
+
+        self.patch(http.log, 'info')
+        mock_deferLater = self.patch(http, 'deferLater')
+        mock_deferLater.side_effect = always_succeed_with(None)
+
+        resource = http.HTTPBootResource()
+        yield self.render_GET(resource, request)
+
+        self.assertEquals(503, request.responseCode)
+        self.assertEquals(
+            b'HTTP boot service not ready.', b''.join(request.written))
+
+    @inlineCallbacks
+    def test_render_GET_400_when_no_local_addr(self):
+        path = factory.make_name('path')
+        ip = factory.make_ip_address()
+        request = DummyRequest([path.encode('utf-8')])
+        request.requestHeaders = Headers({
+            'X-Forwarded-For': [ip],
+            'X-Forwarded-Port': ['%s' % factory.pick_port()],
+        })
+
+        self.patch(http.log, 'info')
+        mock_deferLater = self.patch(http, 'deferLater')
+        mock_deferLater.side_effect = always_succeed_with(None)
+
+        resource = http.HTTPBootResource()
+        yield self.render_GET(resource, request)
+
+        self.assertEquals(400, request.responseCode)
+        self.assertEquals(
+            b'Missing X-Server-Addr and X-Forwarded-For HTTP headers.',
+            b''.join(request.written))
+
+    @inlineCallbacks
+    def test_render_GET_400_when_no_remote_addr(self):
+        path = factory.make_name('path')
+        request = DummyRequest([path.encode('utf-8')])
+        request.requestHeaders = Headers({
+            'X-Server-Addr': ['192.168.1.1'],
+            'X-Server-Port': ['5248'],
+        })
+
+        self.patch(http.log, 'info')
+        mock_deferLater = self.patch(http, 'deferLater')
+        mock_deferLater.side_effect = always_succeed_with(None)
+
+        resource = http.HTTPBootResource()
+        yield self.render_GET(resource, request)
+
+        self.assertEquals(400, request.responseCode)
+        self.assertEquals(
+            b'Missing X-Server-Addr and X-Forwarded-For HTTP headers.',
+            b''.join(request.written))
+
+    @inlineCallbacks
+    def test_render_GET_403_access_violation(self):
+        path = factory.make_name('path')
+        ip = factory.make_ip_address()
+        request = DummyRequest([path.encode('utf-8')])
+        request.requestHeaders = Headers({
+            'X-Server-Addr': ['192.168.1.1'],
+            'X-Server-Port': ['5248'],
+            'X-Forwarded-For': [ip],
+            'X-Forwarded-Port': ['%s' % factory.pick_port()],
+        })
+
+        self.patch(http.log, 'info')
+        mock_deferLater = self.patch(http, 'deferLater')
+        mock_deferLater.side_effect = always_succeed_with(None)
+
+        self.tftp.backend.get_reader.return_value = fail(AccessViolation())
+
+        resource = http.HTTPBootResource()
+        yield self.render_GET(resource, request)
+
+        self.assertEquals(403, request.responseCode)
+        self.assertEquals(
+            b'', b''.join(request.written))
+
+    @inlineCallbacks
+    def test_render_GET_404_file_not_found(self):
+        path = factory.make_name('path')
+        ip = factory.make_ip_address()
+        request = DummyRequest([path.encode('utf-8')])
+        request.requestHeaders = Headers({
+            'X-Server-Addr': ['192.168.1.1'],
+            'X-Server-Port': ['5248'],
+            'X-Forwarded-For': [ip],
+            'X-Forwarded-Port': ['%s' % factory.pick_port()],
+        })
+
+        self.patch(http.log, 'info')
+        mock_deferLater = self.patch(http, 'deferLater')
+        mock_deferLater.side_effect = always_succeed_with(None)
+
+        self.tftp.backend.get_reader.return_value = fail(FileNotFound(path))
+
+        resource = http.HTTPBootResource()
+        yield self.render_GET(resource, request)
+
+        self.assertEquals(404, request.responseCode)
+        self.assertEquals(
+            b'', b''.join(request.written))
+
+    @inlineCallbacks
+    def test_render_GET_500_server_error(self):
+        path = factory.make_name('path')
+        ip = factory.make_ip_address()
+        request = DummyRequest([path.encode('utf-8')])
+        request.requestHeaders = Headers({
+            'X-Server-Addr': ['192.168.1.1'],
+            'X-Server-Port': ['5248'],
+            'X-Forwarded-For': [ip],
+            'X-Forwarded-Port': ['%s' % factory.pick_port()],
+        })
+
+        self.patch(http.log, 'info')
+        mock_deferLater = self.patch(http, 'deferLater')
+        mock_deferLater.side_effect = always_succeed_with(None)
+
+        exc = factory.make_exception("internal error")
+        self.tftp.backend.get_reader.return_value = fail(exc)
+
+        resource = http.HTTPBootResource()
+        yield self.render_GET(resource, request)
+
+        self.assertEquals(500, request.responseCode)
+        self.assertEquals(
+            str(exc).encode('utf-8'), b''.join(request.written))
+
+    @inlineCallbacks
+    def test_render_GET_produces_from_reader(self):
+        path = factory.make_name('path')
+        ip = factory.make_ip_address()
+        request = DummyRequest([path.encode('utf-8')])
+        request.requestHeaders = Headers({
+            'X-Server-Addr': ['192.168.1.1'],
+            'X-Server-Port': ['5248'],
+            'X-Forwarded-For': [ip],
+            'X-Forwarded-Port': ['%s' % factory.pick_port()],
+        })
+
+        self.patch(http.log, 'info')
+        mock_deferLater = self.patch(http, 'deferLater')
+        mock_deferLater.side_effect = always_succeed_with(None)
+
+        content = factory.make_string(size=100).encode('utf-8')
+        reader = BytesReader(content)
+        self.tftp.backend.get_reader.return_value = succeed(reader)
+
+        resource = http.HTTPBootResource()
+        yield self.render_GET(resource, request)
+
+        self.assertEquals(
+            [100], request.responseHeaders.getRawHeaders(b'Content-Length'))
+        self.assertEquals(
+            content, b''.join(request.written))
+
+    @inlineCallbacks
+    def test_render_GET_logs_node_event_with_original_path_ip(self):
+        path = factory.make_name('path')
+        ip = factory.make_ip_address()
+        request = DummyRequest([path.encode('utf-8')])
+        request.requestHeaders = Headers({
+            'X-Server-Addr': ['192.168.1.1'],
+            'X-Server-Port': ['5248'],
+            'X-Forwarded-For': [ip],
+            'X-Forwarded-Port': ['%s' % factory.pick_port()],
+        })
+
+        log_info = self.patch(http.log, 'info')
+        mock_deferLater = self.patch(http, 'deferLater')
+        mock_deferLater.side_effect = always_succeed_with(None)
+
+        self.tftp.backend.get_reader.return_value = fail(AccessViolation())
+
+        resource = http.HTTPBootResource()
+        yield self.render_GET(resource, request)
+
+        self.assertThat(
+            log_info,
+            MockCalledOnceWith(
+                "{path} requested by {remoteHost}",
+                path=path, remoteHost=ip))
+        self.assertThat(
+            mock_deferLater,
+            MockCalledOnceWith(
+                ANY, 0, http.send_node_event_ip_address,
+                event_type=EVENT_TYPES.NODE_HTTP_REQUEST,
+                ip_address=ip, description=path))
diff --git a/src/provisioningserver/rackdservices/tftp.py b/src/provisioningserver/rackdservices/tftp.py
index 6254283..268189e 100644
--- a/src/provisioningserver/rackdservices/tftp.py
+++ b/src/provisioningserver/rackdservices/tftp.py
@@ -360,7 +360,7 @@ class TFTPBackend(FilesystemSynchronousBackend):
 
     @deferred
     @typed
-    def get_reader(self, file_name: TFTPPath):
+    def get_reader(self, file_name: TFTPPath, skip_logging: bool=False):
         """See `IBackend.get_reader()`.
 
         If `file_name` matches a boot method then the response is obtained
@@ -371,7 +371,10 @@ class TFTPBackend(FilesystemSynchronousBackend):
         # of '/', example being 'bootx64.efi'. Convert all '\' to '/' to be
         # unix compatiable.
         file_name = file_name.replace(b'\\', b'/')
-        log_request(file_name)
+        if not skip_logging:
+            # HTTP handler will call with `skip_logging` set to True so that
+            # 2 log messages are not created.
+            log_request(file_name)
         d = self.get_boot_method(file_name)
         d.addCallback(partial(self.handle_boot_method, file_name))
         d.addErrback(self.no_response_errback, file_name)
diff --git a/src/provisioningserver/templates/http/rackd.nginx.conf.template b/src/provisioningserver/templates/http/rackd.nginx.conf.template
index 8b67e90..0634a2d 100644
--- a/src/provisioningserver/templates/http/rackd.nginx.conf.template
+++ b/src/provisioningserver/templates/http/rackd.nginx.conf.template
@@ -37,4 +37,12 @@ server {
         proxy_set_header X-Original-URI $request_uri;
         proxy_set_header X-Original-Remote-IP $remote_addr;
     }
+
+    location / {
+        proxy_pass http://localhost:5249/boot/;
+        proxy_set_header X-Server-Addr $server_addr;
+        proxy_set_header X-Server-Port $server_port;
+        proxy_set_header X-Forwarded-For $remote_addr;
+        proxy_set_header X-Forwarded-Port $remote_port;
+    }
 }