[Libguestfs] [PATCH v2 3/3] v2v: -o rhv-upload: Add a test.

[Richard W.M. Jones]

On Tue, Oct 09, 2018 at 02:28:10PM +0300, Nir Soffer wrote:
> > +# Create a background thread running a web server which is
> > +# simulating the imageio server.
> >
> 
> This functionality should be separated from the fake SDK module, since it is
> not part of the SDK, and may be replaced by real imageio server later.

Well possibly, but it was very convenient to put it in the class here,
and this test is meant for running completely standalone without any
other service available.

> > +class RequestHandler(BaseHTTPRequestHandler):
> >
> 
> This request handler is using HTTP/1.0, and will close the connection after
> every request. This is not a good implementation of the imageio server, and
> also
> hides bugs in this code.
> 
> Should be fixed by adding:
> 
>     protocol_version = "HTTP/1.1"

I tried the attached patch, but for some reason it just hangs.

> > +    def do_OPTIONS(self):
> 
> +        self.send_response(200)
> > +        self.send_header("Content-type", "application/json;
> > charset=UTF-8")
> > +        self.end_headers()
> > +        # Advertize only zero support.
> > +        self.wfile.write(b'''{ "features": [ "zero" ] }''')
> > +
> > +    # eg. zero request.  Just ignore it.
> > +    def do_PATCH(self):
> >
> 
> This must read content-length bytes from self.rfile.
> 
> 
> > +        self.send_response(200)
> > +        self.end_headers()
> > +
> > +    # Flush request.  Ignore it.
> > +    def do_PUT(self):
> >
> 
> This must read content-length bytes from self.rfile.
> 
> Both bugs are hidden by the fact that this server close the connection at
> the end
> of the request.

Also this is covered by the attached patch.

> > +        self.send_response(200)
> > +        self.end_headers()
> > +
> > +def server():
> > +    server_address = ("", imageio_port)
> > +    httpd = HTTPServer(server_address, RequestHandler)
> > +    httpd.serve_forever()
> >
>
> Using HTTP instead of HTTPS is not a good idea. We are not testing
> the same code on the client side.
>
> It is easy to run HTTPS server using pre-created certificate, if we
> disable certificate verification on the client side
> (e.g. --insecure).

This one doesn't seem to be as simple to fix.  However point taken, so
I added a note in the code.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
-------------- next part --------------
>From 3d9b5303275e3c695833753c67dbcde95817ba37 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Mon, 15 Oct 2018 16:07:52 +0100
Subject: [PATCH] v2v: -o rhv-upload: Test using HTTP/1.1 protocol.

Fixes commit b54b58c44e3f1f54a05117c758eaa21d9f1085f0.

Thanks: Nir Soffer.
---
 .../ovirtsdk4/__init__.py                         | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/v2v/test-v2v-o-rhv-upload-module/ovirtsdk4/__init__.py b/v2v/test-v2v-o-rhv-upload-module/ovirtsdk4/__init__.py
index b5f739471..50b9ef9ec 100644
--- a/v2v/test-v2v-o-rhv-upload-module/ovirtsdk4/__init__.py
+++ b/v2v/test-v2v-o-rhv-upload-module/ovirtsdk4/__init__.py
@@ -116,7 +116,10 @@ from http.server import HTTPServer, BaseHTTPRequestHandler
 import threading
 
 class RequestHandler(BaseHTTPRequestHandler):
+    protocol_version = 'HTTP/1.1'
+
     def do_OPTIONS(self):
+        self.discard_request()
         self.send_response(200)
         self.send_header("Content-type", "application/json; charset=UTF-8")
         self.end_headers()
@@ -125,15 +128,27 @@ class RequestHandler(BaseHTTPRequestHandler):
 
     # eg. zero request.  Just ignore it.
     def do_PATCH(self):
+        self.discard_request()
         self.send_response(200)
+        self.send_header("Content-Length", "0")
         self.end_headers()
 
     # Flush request.  Ignore it.
     def do_PUT(self):
+        self.discard_request()
         self.send_response(200)
+        self.send_header("Content-Length", "0")
         self.end_headers()
 
+    def discard_request(self):
+        length = self.headers['Content-Length']
+        if length:
+            length = int(length)
+            content = self.rfile.read(length)
+
 server_address = ("", 0)
+# XXX This should test HTTPS, not HTTP, because we are testing a
+# different path through the main code.
 httpd = HTTPServer(server_address, RequestHandler)
 imageio_port = httpd.server_address[1]
 
-- 
2.19.0.rc0