Refactor server side copy as middleware

Rewrite server side copy and 'object post as copy' feature as middleware to
simplify the PUT method in the object controller code. COPY is no longer
a verb implemented as public method in Proxy application.

The server side copy middleware is inserted to the left of dlo, slo and
versioned_writes middlewares in the proxy server pipeline. As a result,
dlo and slo copy_hooks are no longer required. SLO manifests are now
validated when copied so when copying a manifest to another account the
referenced segments must be readable in that account for the manifest
copy to succeed (previously this validation was not made, meaning the
manifest was copied but could be unusable if the segments were not
readable).

With this change, there should be no change in functionality or existing
behavior. This is asserted with (almost) no changes required to existing
functional tests.

Some notes (for operators):
* Middleware required to be auto-inserted before slo and dlo and
  versioned_writes
* Turning off server side copy is not configurable.
* object_post_as_copy is no longer a configurable option of proxy server
  but of this middleware. However, for smooth upgrade, config option set
  in proxy server app is also read.

DocImpact: Introducing server side copy as middleware

Co-Authored-By: Alistair Coles <alistair.coles@hpe.com>
Co-Authored-By: Thiago da Silva <thiago@redhat.com>

Change-Id: Ic96a92e938589a2f6add35a40741fd062f1c29eb
Signed-off-by: Prashanth Pai <ppai@redhat.com>
Signed-off-by: Thiago da Silva <thiago@redhat.com>

doc/saio/swift/proxy-server.conf

@@ -9,7 +9,7 @@ eventlet_debug = true
 [pipeline:main]
 # Yes, proxy-logging appears twice. This is so that
 # middleware-originated requests get logged too.
-pipeline = catch_errors gatekeeper healthcheck proxy-logging cache bulk tempurl ratelimit crossdomain container_sync tempauth staticweb container-quotas account-quotas slo dlo versioned_writes proxy-logging proxy-server
+pipeline = catch_errors gatekeeper healthcheck proxy-logging cache bulk tempurl ratelimit crossdomain container_sync tempauth staticweb copy container-quotas account-quotas slo dlo versioned_writes proxy-logging proxy-server
 
 [filter:catch_errors]
 use = egg:swift#catch_errors
@@ -68,6 +68,9 @@ use = egg:swift#gatekeeper
 use = egg:swift#versioned_writes
 allow_versioned_writes = true
 
+[filter:copy]
+use = egg:swift#copy
+
 [app:proxy-server]
 use = egg:swift#proxy
 allow_account_management = true

doc/source/logs.rst

@@ -103,6 +103,7 @@ LE                      :ref:`list_endpoints`
 KS                      :ref:`keystoneauth`
 RL                      :ref:`ratelimit`
 VW                      :ref:`versioned_writes`
+SSC                     :ref:`copy`
 ======================= =============================
 
 

doc/source/middleware.rst

@@ -187,6 +187,15 @@ Recon
     :members:
     :show-inheritance:
 
+.. _copy:
+
+Server Side Copy
+================
+
+.. automodule:: swift.common.middleware.copy
+    :members:
+    :show-inheritance:
+
 Static Large Objects
 ====================
 

etc/proxy-server.conf-sample

@@ -79,12 +79,12 @@ bind_port = 8080
 [pipeline:main]
 # This sample pipeline uses tempauth and is used for SAIO dev work and
 # testing. See below for a pipeline using keystone.
-pipeline = catch_errors gatekeeper healthcheck proxy-logging cache container_sync bulk tempurl ratelimit tempauth container-quotas account-quotas slo dlo versioned_writes proxy-logging proxy-server
+pipeline = catch_errors gatekeeper healthcheck proxy-logging cache container_sync bulk tempurl ratelimit tempauth copy container-quotas account-quotas slo dlo versioned_writes proxy-logging proxy-server
 
 # The following pipeline shows keystone integration. Comment out the one
 # above and uncomment this one. Additional steps for integrating keystone are
 # covered further below in the filter sections for authtoken and keystoneauth.
-#pipeline = catch_errors gatekeeper healthcheck proxy-logging cache container_sync bulk tempurl ratelimit authtoken keystoneauth container-quotas account-quotas slo dlo versioned_writes proxy-logging proxy-server
+#pipeline = catch_errors gatekeeper healthcheck proxy-logging cache container_sync bulk tempurl ratelimit authtoken keystoneauth copy container-quotas account-quotas slo dlo versioned_writes proxy-logging proxy-server
 
 [app:proxy-server]
 use = egg:swift#proxy
@@ -129,11 +129,6 @@ use = egg:swift#proxy
 # 'false' no one, even authorized, can.
 # allow_account_management = false
 #
-# Set object_post_as_copy = false to turn on fast posts where only the metadata
-# changes are stored anew and the original data file is kept in place. This
-# makes for quicker posts.
-# object_post_as_copy = true
-#
 # If set to 'true' authorized accounts that do not yet exist within the Swift
 # cluster will be automatically created.
 # account_autocreate = false
@@ -749,3 +744,14 @@ use = egg:swift#versioned_writes
 # in the container configuration file, which will be eventually
 # deprecated. See documentation for more details.
 # allow_versioned_writes = false
+
+# Note: Put after auth and before dlo and slo middlewares.
+# If you don't put it in the pipeline, it will be inserted for you.
+[filter:copy]
+use = egg:swift#copy
+# Set object_post_as_copy = false to turn on fast posts where only the metadata
+# changes are stored anew and the original data file is kept in place. This
+# makes for quicker posts.
+# When object_post_as_copy is set to True, a POST request will be transformed
+# into a COPY request where source and destination objects are the same.
+# object_post_as_copy = true

setup.cfg

@@ -96,6 +96,7 @@ paste.filter_factory =
     container_sync = swift.common.middleware.container_sync:filter_factory
     xprofile = swift.common.middleware.xprofile:filter_factory
     versioned_writes = swift.common.middleware.versioned_writes:filter_factory
+    copy = swift.common.middleware.copy:filter_factory
 
 [build_sphinx]
 all_files = 1

swift/common/constraints.py

@@ -20,7 +20,6 @@
 import six
 from six.moves.configparser import ConfigParser, NoSectionError, NoOptionError
 from six.moves import urllib
-from six.moves.urllib.parse import unquote
 
 from swift.common import utils, exceptions
 from swift.common.swob import HTTPBadRequest, HTTPLengthRequired, \
@@ -205,10 +204,6 @@ def check_object_creation(req, object_name):
                                   request=req,
                                   content_type='text/plain')
 
-    if 'X-Copy-From' in req.headers and req.content_length:
-        return HTTPBadRequest(body='Copy requests require a zero byte body',
-                              request=req, content_type='text/plain')
-
     if len(object_name) > MAX_OBJECT_NAME_LENGTH:
         return HTTPBadRequest(body='Object name length of %d longer than %d' %
                               (len(object_name), MAX_OBJECT_NAME_LENGTH),
@@ -359,63 +354,6 @@ def check_utf8(string):
         return False
 
 
-def check_path_header(req, name, length, error_msg):
-    """
-    Validate that the value of path-like header is
-    well formatted. We assume the caller ensures that
-    specific header is present in req.headers.
-
-    :param req: HTTP request object
-    :param name: header name
-    :param length: length of path segment check
-    :param error_msg: error message for client
-    :returns: A tuple with path parts according to length
-    :raise: HTTPPreconditionFailed if header value
-            is not well formatted.
-    """
-    src_header = unquote(req.headers.get(name))
-    if not src_header.startswith('/'):
-        src_header = '/' + src_header
-    try:
-        return utils.split_path(src_header, length, length, True)
-    except ValueError:
-        raise HTTPPreconditionFailed(
-            request=req,
-            body=error_msg)
-
-
-def check_copy_from_header(req):
-    """
-    Validate that the value from x-copy-from header is
-    well formatted. We assume the caller ensures that
-    x-copy-from header is present in req.headers.
-
-    :param req: HTTP request object
-    :returns: A tuple with container name and object name
-    :raise: HTTPPreconditionFailed if x-copy-from value
-            is not well formatted.
-    """
-    return check_path_header(req, 'X-Copy-From', 2,
-                             'X-Copy-From header must be of the form '
-                             '<container name>/<object name>')
-
-
-def check_destination_header(req):
-    """
-    Validate that the value from destination header is
-    well formatted. We assume the caller ensures that
-    destination header is present in req.headers.
-
-    :param req: HTTP request object
-    :returns: A tuple with container name and object name
-    :raise: HTTPPreconditionFailed if destination value
-            is not well formatted.
-    """
-    return check_path_header(req, 'Destination', 2,
-                             'Destination header must be of the form '
-                             '<container name>/<object name>')
-
-
 def check_name_format(req, name, target_type):
     """
     Validate that the header contains valid account or container name.

swift/common/middleware/account_quotas.py

@@ -52,11 +52,10 @@
 account size has been updated.
 """
 
-from swift.common.constraints import check_copy_from_header
 from swift.common.swob import HTTPForbidden, HTTPBadRequest, \
     HTTPRequestEntityTooLarge, wsgify
 from swift.common.utils import register_swift_info
-from swift.proxy.controllers.base import get_account_info, get_object_info
+from swift.proxy.controllers.base import get_account_info
 
 
 class AccountQuotaMiddleware(object):
@@ -71,7 +70,7 @@ def __init__(self, app, *args, **kwargs):
     @wsgify
     def __call__(self, request):
 
-        if request.method not in ("POST", "PUT", "COPY"):
+        if request.method not in ("POST", "PUT"):
             return self.app
 
         try:
@@ -106,15 +105,6 @@ def __call__(self, request):
         if request.method == "POST" or not obj:
             return self.app
 
-        if request.method == 'COPY':
-            copy_from = container + '/' + obj
-        else:
-            if 'x-copy-from' in request.headers:
-                src_cont, src_obj = check_copy_from_header(request)
-                copy_from = "%s/%s" % (src_cont, src_obj)
-            else:
-                copy_from = None
-
         content_length = (request.content_length or 0)
 
         account_info = get_account_info(request.environ, self.app)
@@ -127,14 +117,6 @@ def __call__(self, request):
         if quota < 0:
             return self.app
 
-        if copy_from:
-            path = '/' + ver + '/' + account + '/' + copy_from
-            object_info = get_object_info(request.environ, self.app, path)
-            if not object_info or not object_info['length']:
-                content_length = 0
-            else:
-                content_length = int(object_info['length'])
-
         new_size = int(account_info['bytes']) + content_length
         if quota < new_size:
             resp = HTTPRequestEntityTooLarge(body='Upload exceeds quota.')

swift/common/middleware/container_quotas.py

@@ -51,13 +51,11 @@
     [filter:container_quotas]
     use = egg:swift#container_quotas
 """
-from swift.common.constraints import check_copy_from_header, \
-    check_account_format, check_destination_header
 from swift.common.http import is_success
 from swift.common.swob import HTTPRequestEntityTooLarge, HTTPBadRequest, \
     wsgify
 from swift.common.utils import register_swift_info
-from swift.proxy.controllers.base import get_container_info, get_object_info
+from swift.proxy.controllers.base import get_container_info
 
 
 class ContainerQuotaMiddleware(object):
@@ -91,25 +89,9 @@ def __call__(self, req):
                 return HTTPBadRequest(body='Invalid count quota.')
 
         # check user uploads against quotas
-        elif obj and req.method in ('PUT', 'COPY'):
-            container_info = None
-            if req.method == 'PUT':
-                container_info = get_container_info(
-                    req.environ, self.app, swift_source='CQ')
-            if req.method == 'COPY' and 'Destination' in req.headers:
-                dest_account = account
-                if 'Destination-Account' in req.headers:
-                    dest_account = req.headers.get('Destination-Account')
-                    dest_account = check_account_format(req, dest_account)
-                dest_container, dest_object = check_destination_header(req)
-                path_info = req.environ['PATH_INFO']
-                req.environ['PATH_INFO'] = "/%s/%s/%s/%s" % (
-                    version, dest_account, dest_container, dest_object)
-                try:
-                    container_info = get_container_info(
-                        req.environ, self.app, swift_source='CQ')
-                finally:
-                    req.environ['PATH_INFO'] = path_info
+        elif obj and req.method in ('PUT'):
+            container_info = get_container_info(
+                req.environ, self.app, swift_source='CQ')
             if not container_info or not is_success(container_info['status']):
                 # this will hopefully 404 later
                 return self.app
@@ -118,16 +100,6 @@ def __call__(self, req):
                     'bytes' in container_info and \
                     container_info['meta']['quota-bytes'].isdigit():
                 content_length = (req.content_length or 0)
-                if 'x-copy-from' in req.headers or req.method == 'COPY':
-                    if 'x-copy-from' in req.headers:
-                        container, obj = check_copy_from_header(req)
-                    path = '/%s/%s/%s/%s' % (version, account,
-                                             container, obj)
-                    object_info = get_object_info(req.environ, self.app, path)
-                    if not object_info or not object_info['length']:
-                        content_length = 0
-                    else:
-                        content_length = int(object_info['length'])
                 new_size = int(container_info['bytes']) + content_length
                 if int(container_info['meta']['quota-bytes']) < new_size:
                     return self.bad_response(req, container_info)