Libguestfs - Jun 2018 - Re: [PATCH] v2v: rhv-upload-plugin: Remove unneeded auth

Old imageio proxy was using Authorization header for GET and PUT
requests. Remove unneeded authorization when sending OPTIONS request.

Remove unneeded duplicated comments about authorization for old
imageio, and replace them with a comment when we set needs_auth.
---
 v2v/rhv-upload-plugin.py | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
index 8805e3552..f404bd758 100644
--- a/v2v/rhv-upload-plugin.py
+++ b/v2v/rhv-upload-plugin.py
@@ -191,16 +191,17 @@ def open(readonly):
     )
 
     # The first request is to fetch the features of the server.
+
+    # Authentication was needed only for GET and PUT requests when
+    # communicating with old imageio-proxy.
     needs_auth = not params['rhv_direct']
+
     can_flush = False
     can_trim = False
     can_zero = False
     unix_socket = None
 
-    http.putrequest("OPTIONS", destination_url.path)
-    http.putheader("Authorization", transfer.signed_ticket)
-    http.endheaders()
-
+    http.request("OPTIONS", destination_url.path)
     r = http.getresponse()
     data = r.read()
 
@@ -298,7 +299,6 @@ def pread(h, count, offset):
     transfer = h['transfer']
 
     headers = {"Range", "bytes=%d-%d" % (offset,
offset+count-1)}
-    # Authorization is only needed for old imageio.
     if h['needs_auth']:
         headers["Authorization"] = transfer.signed_ticket
 
@@ -321,7 +321,6 @@ def pwrite(h, buf, offset):
     h['highestwrite'] = max(h['highestwrite'], offset+count)
 
     http.putrequest("PUT", h['path'] + "?flush=n")
-    # Authorization is only needed for old imageio.
     if h['needs_auth']:
         http.putheader("Authorization", transfer.signed_ticket)
     # The oVirt server only uses the first part of the range, and the
@@ -378,7 +377,6 @@ def emulate_zero(h, count, offset):
     # After that we must emulate them with writes.
     if offset+count < h['highestwrite']:
         http.putrequest("PUT", h['path'])
-        # Authorization is only needed for old imageio.
         if h['needs_auth']:
             http.putheader("Authorization", transfer.signed_ticket)
         http.putheader("Content-Range",
-- 
2.17.1

On Fri, Jun 29, 2018 at 05:41:10PM +0300, Nir Soffer
wrote:
> Old imageio proxy was using Authorization header for GET and PUT
> requests. Remove unneeded authorization when sending OPTIONS request.
> 
> Remove unneeded duplicated comments about authorization for old
> imageio, and replace them with a comment when we set needs_auth.
I'll push it, thanks.

Can we get rid of the Authorization stuff completely yet?

Rich.
>  v2v/rhv-upload-plugin.py | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
> index 8805e3552..f404bd758 100644
> --- a/v2v/rhv-upload-plugin.py
> +++ b/v2v/rhv-upload-plugin.py
> @@ -191,16 +191,17 @@ def open(readonly):
>      )
>  
>      # The first request is to fetch the features of the server.
> +
> +    # Authentication was needed only for GET and PUT requests when
> +    # communicating with old imageio-proxy.
>      needs_auth = not params['rhv_direct']
> +
>      can_flush = False
>      can_trim = False
>      can_zero = False
>      unix_socket = None
>  
> -    http.putrequest("OPTIONS", destination_url.path)
> -    http.putheader("Authorization", transfer.signed_ticket)
> -    http.endheaders()
> -
> +    http.request("OPTIONS", destination_url.path)
>      r = http.getresponse()
>      data = r.read()
>  
> @@ -298,7 +299,6 @@ def pread(h, count, offset):
>      transfer = h['transfer']
>  
>      headers = {"Range", "bytes=%d-%d" % (offset,
offset+count-1)}
> -    # Authorization is only needed for old imageio.
>      if h['needs_auth']:
>          headers["Authorization"] = transfer.signed_ticket
>  
> @@ -321,7 +321,6 @@ def pwrite(h, buf, offset):
>      h['highestwrite'] = max(h['highestwrite'],
offset+count)
>  
>      http.putrequest("PUT", h['path'] +
"?flush=n")
> -    # Authorization is only needed for old imageio.
>      if h['needs_auth']:
>          http.putheader("Authorization", transfer.signed_ticket)
>      # The oVirt server only uses the first part of the range, and the
> @@ -378,7 +377,6 @@ def emulate_zero(h, count, offset):
>      # After that we must emulate them with writes.
>      if offset+count < h['highestwrite']:
>          http.putrequest("PUT", h['path'])
> -        # Authorization is only needed for old imageio.
>          if h['needs_auth']:
>              http.putheader("Authorization",
transfer.signed_ticket)
>          http.putheader("Content-Range",
> -- 
> 2.17.1
-- 
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

On Fri, Jun 29, 2018 at 6:08 PM Richard W.M. Jones <rjones@redhat.com>
wrote:
> On Fri, Jun 29, 2018 at 05:41:10PM +0300, Nir Soffer wrote:
> > Old imageio proxy was using Authorization header for GET and PUT
> > requests. Remove unneeded authorization when sending OPTIONS request.
> >
> > Remove unneeded duplicated comments about authorization for old
> > imageio, and replace them with a comment when we set needs_auth.
>
> I'll push it, thanks.
>
> Can we get rid of the Authorization stuff completely yet?
>
Yes, if you want to drop support for ovirt < 4.2.

Rich.
>
> >  v2v/rhv-upload-plugin.py | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> >
> > diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
> > index 8805e3552..f404bd758 100644
> > --- a/v2v/rhv-upload-plugin.py
> > +++ b/v2v/rhv-upload-plugin.py
> > @@ -191,16 +191,17 @@ def open(readonly):
> >      )
> >
> >      # The first request is to fetch the features of the server.
> > +
> > +    # Authentication was needed only for GET and PUT requests when
> > +    # communicating with old imageio-proxy.
> >      needs_auth = not params['rhv_direct']
> > +
> >      can_flush = False
> >      can_trim = False
> >      can_zero = False
> >      unix_socket = None
> >
> > -    http.putrequest("OPTIONS", destination_url.path)
> > -    http.putheader("Authorization", transfer.signed_ticket)
> > -    http.endheaders()
> > -
> > +    http.request("OPTIONS", destination_url.path)
> >      r = http.getresponse()
> >      data = r.read()
> >
> > @@ -298,7 +299,6 @@ def pread(h, count, offset):
> >      transfer = h['transfer']
> >
> >      headers = {"Range", "bytes=%d-%d" % (offset,
offset+count-1)}
> > -    # Authorization is only needed for old imageio.
> >      if h['needs_auth']:
> >          headers["Authorization"] = transfer.signed_ticket
> >
> > @@ -321,7 +321,6 @@ def pwrite(h, buf, offset):
> >      h['highestwrite'] = max(h['highestwrite'],
offset+count)
> >
> >      http.putrequest("PUT", h['path'] +
"?flush=n")
> > -    # Authorization is only needed for old imageio.
> >      if h['needs_auth']:
> >          http.putheader("Authorization",
transfer.signed_ticket)
> >      # The oVirt server only uses the first part of the range, and the
> > @@ -378,7 +377,6 @@ def emulate_zero(h, count, offset):
> >      # After that we must emulate them with writes.
> >      if offset+count < h['highestwrite']:
> >          http.putrequest("PUT", h['path'])
> > -        # Authorization is only needed for old imageio.
> >          if h['needs_auth']:
> >              http.putheader("Authorization",
transfer.signed_ticket)
> >          http.putheader("Content-Range",
> > --
> > 2.17.1
>
> --
> 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
>