fix x-open-expired 404 on HEAD?part-number reqs

Fixes a bug with the x-open-expired feature where our magic header
does not get copied when refetching all manifests that causes
404 on HEAD requests with part-number=N query parameter since
the object-server returns an empty response body and the
proxy needs to refetch. The fix also applies to segment GET
requests  if the segments have expired.

Change-Id: If0382d433f73cc0333bb4d0319fe1487b7783e4c

swift/common/middleware/slo.py

@@ -980,9 +980,11 @@ class SloGetContext(WSGIContext):
         friendly_close(resp_iter)
         del req.environ['swift.non_client_disconnect']
 
+        headers_subset = ['x-auth-token', 'x-open-expired']
         get_req = make_subrequest(
             req.environ, method='GET',
-            headers={'x-auth-token': req.headers.get('x-auth-token')},
+            headers={k: req.headers.get(k)
+                     for k in headers_subset if k in req.headers},
             agent='%(orig)s SLO MultipartGET', swift_source='SLO')
         resp_iter = self._app_call(get_req.environ)
         new_resp_attrs = RespAttrs.from_headers(self._response_headers)

swift/common/request_helpers.py

@@ -563,8 +563,8 @@ class SegmentedIterable(object):
                 path = quote(seg_path) + '?multipart-manifest=get'
                 seg_req = make_subrequest(
                     self.req.environ, path=path, method='GET',
-                    headers={'x-auth-token': self.req.headers.get(
+                    headers={h: self.req.headers.get(h)
-                        'x-auth-token')},
+                             for h in ('x-auth-token', 'x-open-expired')},
                     agent=('%(orig)s ' + self.ua_suffix),
                     swift_source=self.swift_source)
 

test/functional/__init__.py

@@ -579,6 +579,7 @@ def in_process_setup(the_object_server=object_server):
         'container_update_timeout': '3',
         'allow_account_management': 'true',
         'account_autocreate': 'true',
+        'allow_open_expired': 'true',
         'allow_versions': 'True',
         'allow_versioned_writes': 'True',
         # Below are values used by the functional test framework, as well as

test/functional/test_slo.py

@@ -18,6 +18,7 @@ import base64
 import email.parser
 import itertools
 import json
+import time
 from copy import deepcopy
 from unittest import SkipTest
 
@@ -25,8 +26,9 @@ import six
 from six.moves import urllib
 
 from swift.common.swob import normalize_etag
-from swift.common.utils import md5
+from swift.common.utils import md5, config_true_value
 
+from test.functional import check_response, retry
 import test.functional as tf
 from test.functional import cluster_info
 from test.functional.tests import Utils, Base, Base2, BaseEnv
@@ -51,6 +53,12 @@ def group_file_contents(file_contents):
         for char, grp in itertools.groupby(byte_iter)]
 
 
+def md5hex(s):
+    if not isinstance(s, bytes):
+        s = s.encode('ascii')
+    return md5(s, usedforsecurity=False).hexdigest()
+
+
 class TestSloEnv(BaseEnv):
     slo_enabled = None  # tri-state: None initially, then True/False
 
@@ -450,6 +458,163 @@ class TestSlo(Base):
             self.assertEqual(headers['x-parts-count'], '5')
             start = end + 1
 
+    def test_x_delete_at_with_part_number_and_open_expired(self):
+        cont_name = self.env.account2.container(self.env.container.name)
+        allow_open_expired = config_true_value(tf.cluster_info['swift'].get(
+            'allow_open_expired', 'false'))
+
+        if not allow_open_expired:
+            raise SkipTest('allow_open_expired is disabled')
+
+        # data for segments
+        segments = [b'one', b'two', b'three', b'four']
+        etags = []
+        for segment in segments:
+            etags.append(md5hex(segment))
+
+        def put_manifest(url, token, parsed, conn, object_segments):
+            now = int(time.time())
+            delete_time = now + 2
+            manifest_data = []
+            start = 0
+
+            for segment_object in range(len(object_segments)):
+                size = len(object_segments[segment_object])
+                end = start + size - 1
+                manifest_data.append({
+                    'path': '/%s/segments/%s' % (cont_name,
+                                                 str(segment_object)),
+                    'etag': etags[segment_object],
+                    'size_bytes': size,
+                })
+                start = end + 1
+
+            conn.request(
+                'PUT',
+                '%s/%s/manifest?multipart-manifest=put' % (parsed.path,
+                                                           cont_name),
+                body=json.dumps(manifest_data),
+                headers={
+                    'X-Auth-Token': token,
+                    'X-Delete-At': delete_time,
+                    'X-Static-Large-Object': 'true',
+                    'Content-Type': 'application/json'
+                }
+            )
+            resp = check_response(conn)
+            body = resp.read()
+            self.assertEqual(resp.status, 201,
+                             "Response status is not 201: %s" % body)
+
+        def put_segments(url, token, parsed, conn, object_segments):
+            now = int(time.time())
+            delete_time = now + 2
+
+            for objnum in range(len(object_segments)):
+                conn.request('PUT', '%s/%s/segments/%s' % (
+                    parsed.path,
+                    cont_name,
+                    str(objnum)),
+                    body=object_segments[objnum],
+                    headers={
+                        'X-Auth-Token': token,
+                        'X-Delete-At': delete_time})
+                resp = check_response(conn)
+                body = resp.read()
+                self.assertEqual(resp.status, 201,
+                                 "Response status is not 201: %s" % body)
+
+        retry(put_segments, segments)
+        retry(put_manifest, segments)
+
+        # get the manifest
+        def get_manifest(url, token, parsed, conn, extra_headers=None):
+            headers = {'X-Auth-Token': token}
+            if extra_headers:
+                headers.update(extra_headers)
+            conn.request(
+                'GET',
+                '%s/%s/manifest?multipart-manifest=get' %
+                (parsed.path, cont_name),
+                '', headers)
+            return check_response(conn)
+
+        resp = retry(get_manifest)
+        self.assertEqual(resp.status, 200)
+        # wait for the manifest to expire
+        # the objects will also have expired at the same time
+        # since their x-delete-at times are the same
+        time.sleep(3)
+
+        resp = retry(get_manifest)
+        resp.read()
+        # check to see manifest has expired
+        self.assertEqual(resp.status, 404, resp.headers.get('x-trans-id'))
+
+        def get_or_head_part(url, token, parsed, conn,
+                             extra_headers=None, method='GET',
+                             part_number=None):
+            headers = {'X-Auth-Token': token}
+            if extra_headers:
+                headers.update(extra_headers)
+            conn.request(method, '%s/%s/manifest?part-number=%s' % (
+                parsed.path,
+                cont_name,
+                part_number
+            ), '', headers)
+            return check_response(conn)
+
+        resp = retry(get_manifest, extra_headers={'X-Open-Expired': True})
+        resp.read()
+        # read the expired object with magic x-open-expired header
+        self.assertEqual(resp.status, 200)
+
+        for objnum in range(len(segments)):
+            part_num = str(objnum + 1)
+            resp = retry(get_or_head_part,
+                         extra_headers={'X-Open-Expired': True},
+                         part_number=part_num,)
+            resp.read()
+            self.assertEqual(resp.status, 206)
+
+        for objnum in range(len(segments)):
+            part_num = str(objnum + 1)
+            resp = retry(get_or_head_part,
+                         extra_headers={'X-Open-Expired': True},
+                         method='HEAD', part_number=part_num)
+            resp.read()
+            self.assertEqual(resp.status, 206)
+
+        # no x-open-expired case and it should 404
+        for objnum in range(len(segments)):
+            part_num = str(objnum + 1)
+            resp = retry(get_or_head_part,
+                         method='HEAD', part_number=part_num)
+            resp.read()
+            self.assertEqual(resp.status, 404)
+
+        # same situation here
+        for objnum in range(len(segments)):
+            part_num = str(objnum + 1)
+            resp = retry(get_or_head_part,
+                         method='GET', part_number=part_num)
+            resp.read()
+            self.assertEqual(resp.status, 404)
+
+        def head_manifest(url, token, parsed, conn, extra_headers=None):
+            headers = {'X-Auth-Token': token}
+            if extra_headers:
+                headers.update(extra_headers)
+            conn.request('HEAD', '%s/%s/manifest' % (parsed.path,
+                                                     cont_name),
+                         '', headers)
+            return check_response(conn)
+
+        resp = retry(head_manifest, extra_headers={'X-Open-Expired': True})
+        resp.read()
+        # head expired object with magic x-open-expired header
+        self.assertEqual(resp.status, 200)
+
     def test_slo_container_listing(self):
         # the listing object size should equal the sum of the size of the
         # segments, not the size of the manifest body

test/probe/test_object_expirer.py

@@ -410,6 +410,129 @@ class TestObjectExpirer(ReplProbeTest):
                                headers={'X-Open-Expired': True})
         self.assertEqual(e.exception.http_status, 404)
 
+    def _setup_test_slo_object(self):
+        segment_container = self.container_name + '_segments'
+        client.put_container(self.url, self.token, self.container_name, {})
+        client.put_container(self.url, self.token, segment_container, {})
+        client.put_object(self.url, self.token,
+                          segment_container, 'segment_1', b'12')
+        client.put_object(self.url, self.token,
+                          segment_container, 'segment_2', b'5678')
+        client.put_object(
+            self.url, self.token, self.container_name, 'slo', json.dumps([
+                {'path': segment_container + '/segment_1'},
+                {'data': 'Cg=='},
+                {'path': segment_container + '/segment_2'},
+            ]), query_string='multipart-manifest=put')
+        _, body = client.get_object(self.url, self.token,
+                                    self.container_name, 'slo')
+        self.assertEqual(body, b'12\n5678')
+
+        return segment_container, self.container_name
+
+    def test_open_expired_enabled_with_part_num(self):
+        allow_open_expired = config_true_value(
+            self.cluster_info['swift'].get('allow_open_expired')
+        )
+
+        if not allow_open_expired:
+            raise unittest.SkipTest(
+                "allow_open_expired is disabled in this swift cluster"
+            )
+
+        seg_container, container_name = self._setup_test_slo_object()
+        now = time.time()
+        delete_at = int(now + 1)
+
+        client.post_object(
+            self.url, self.token, container_name, 'slo',
+            headers={
+                'X-Delete-At': str(delete_at),
+                'X-Object-Meta-Test': 'foo'
+            }
+        )
+
+        # make sure auto-created containers get in the account listing
+        Manager(['container-updater']).once()
+
+        # sleep until after expired but not reaped
+        while time.time() <= delete_at:
+            time.sleep(0.1)
+
+        # should get a 404, object is expired
+        while True:
+            try:
+                client.head_object(self.url, self.token, container_name, 'slo')
+                time.sleep(1)  # Wait for a short period before trying again
+            except ClientException as e:
+                # check if the object is expired
+                if e.http_status == 404:
+                    break  # The object is expired, so we can exit the loop
+
+        resp_headers = client.head_object(
+            self.url, self.token, container_name, 'slo',
+            headers={'X-Open-Expired': True},
+            query_string='part-number=1'
+        )
+
+        self.assertEqual(resp_headers.get('x-object-meta-test'), 'foo')
+        self.assertEqual(resp_headers.get('content-range'), 'bytes 0-1/7')
+        self.assertEqual(resp_headers.get('content-length'), '2')
+        self.assertEqual(resp_headers.get('x-parts-count'), '3')
+        self.assertEqual(resp_headers.get('x-static-large-object'), 'True')
+        self.assertEqual(resp_headers.get('accept-ranges'), 'bytes')
+
+        with self.assertRaises(ClientException) as e:
+            client.head_object(self.url, self.token, container_name, 'slo')
+        self.assertEqual(e.exception.http_status, 404)
+
+        now = time.time()
+        delete_at = int(now + 2)
+        for seg_obj_name in ['segment_1', 'segment_2']:
+            client.post_object(
+                self.url, self.token, seg_container, seg_obj_name,
+                headers={
+                    'X-Open-Expired': True,
+                    'X-Segment-Meta-Test': 'segment-foo',
+                    'X-Delete-At': str(delete_at)
+                }
+            )
+
+        # make sure auto-created containers get in the account listing
+        Manager(['container-updater']).once()
+        while time.time() <= delete_at:
+            time.sleep(0.1)
+
+        # should get a 404, segment object is expired
+        with self.assertRaises(ClientException) as e:
+            client.head_object(self.url, self.token, seg_container,
+                               'segment_2')
+        self.assertEqual(e.exception.http_status, 404)
+
+        # magic of x-open-expired
+        resp_headers = client.head_object(
+            self.url, self.token, seg_container, 'segment_2',
+            headers={'X-Open-Expired': True},
+            query_string='part-number=1'
+        )
+
+        # keep in mind that the segment object is expired
+        self.assertEqual(resp_headers.get('content-length'), '4')
+        self.assertTrue(time.time() > delete_at)
+
+        # expirer runs to reap the whichever object was set for expiry
+        self.expirer.once()
+
+        for seg_obj_name in ['segment_1', 'segment_2']:
+            # should get a 404 even with x-open-expired since object is reaped
+            with self.assertRaises(ClientException) as e:
+                client.head_object(
+                    self.url, self.token, seg_container, seg_obj_name,
+                    headers={'X-Open-Expired': True},
+                    query_string='part-number=1'
+                )
+            self.assertEqual(e.exception.http_status, 404)
+
     def test_open_expired_disabled(self):
 
         # When the global configuration option allow_open_expired is set to
@@ -681,22 +804,7 @@ class TestObjectExpirer(ReplProbeTest):
         if not self.cluster_info.get('slo', {}).get('allow_async_delete'):
             raise unittest.SkipTest('allow_async_delete not enabled')
 
-        segment_container = self.container_name + '_segments'
+        segment_container, _ = self._setup_test_slo_object()
-        client.put_container(self.url, self.token, self.container_name, {})
-        client.put_container(self.url, self.token, segment_container, {})
-        client.put_object(self.url, self.token,
-                          segment_container, 'segment_1', b'1234')
-        client.put_object(self.url, self.token,
-                          segment_container, 'segment_2', b'5678')
-        client.put_object(
-            self.url, self.token, self.container_name, 'slo', json.dumps([
-                {'path': segment_container + '/segment_1'},
-                {'data': 'Cg=='},
-                {'path': segment_container + '/segment_2'},
-            ]), query_string='multipart-manifest=put')
-        _, body = client.get_object(self.url, self.token,
-                                    self.container_name, 'slo')
-        self.assertEqual(body, b'1234\n5678')
 
         client.delete_object(
             self.url, self.token, self.container_name, 'slo',
@@ -717,7 +825,7 @@ class TestObjectExpirer(ReplProbeTest):
                          ['segment_1', 'segment_2'])
         _, body = client.get_object(self.url, self.token,
                                     segment_container, 'segment_1')
-        self.assertEqual(body, b'1234')
+        self.assertEqual(body, b'12')
         _, body = client.get_object(self.url, self.token,
                                     segment_container, 'segment_2')
         self.assertEqual(body, b'5678')

test/unit/common/middleware/test_slo.py

@@ -1981,6 +1981,18 @@ class SloGETorHEADTestCase(SloTestCase):
             extra_headers={'X-Object-Meta-Nature': 'Regular'},
             container='gettest')
 
+    def _setup_manifest_zero_byte(self):
+        """
+        This is a zero-byte manifest.
+        """
+        _single_segment_manifest = [
+            {'name': '/gettest/zero', 'hash': md5hex(''), 'bytes': '0',
+             'content_type': 'text/plain'},
+        ]
+        self._setup_manifest(
+            'zero-byte', _single_segment_manifest,
+            container='gettest')
+
     def _setup_manifest_data(self):
         _data_manifest = [
             {
@@ -2011,6 +2023,21 @@ class SloGETorHEADTestCase(SloTestCase):
             'X-Object-Meta-Plant': 'Ficus',
         }, container='gettest')
 
+    def _setup_manifest_bc_expires(self):
+        """
+        This manifest's segments are all regular objects due to expire.
+        """
+        _bc_expires_manifest = [
+            {'name': '/gettest/b_5', 'hash': md5hex('b' * 5), 'bytes': '5',
+             'content_type': 'text/plain'},
+            {'name': '/gettest/c_10', 'hash': md5hex('c' * 10), 'bytes': '10',
+             'content_type': 'text/plain'}
+        ]
+        self._setup_manifest('bc-expires', _bc_expires_manifest,
+                             extra_headers={'X-Object-Meta-Plant':
+                                            'Ficus-Expires'},
+                             container='gettest')
+
     def _setup_manifest_abcd(self):
         """
         This manifest uses manifest-bc as a sub-manifest!
@@ -5703,6 +5730,8 @@ class TestPartNumber(SloGETorHEADTestCase):
         self._setup_manifest_abcd_subranges()
         self._setup_manifest_aabbccdd()
         self._setup_manifest_single_segment()
+        self._setup_manifest_zero_byte()
+        self._setup_manifest_bc_expires()
 
         # this b_50 object doesn't follow the alphabet convention
         self.app.register(
@@ -5711,6 +5740,11 @@ class TestPartNumber(SloGETorHEADTestCase):
                                       'Etag': md5hex('b' * 50)},
             'b' * 50)
 
+        # Setup POST req separately for expiring manifest
+        self.app.register('POST',
+                          '/v1/AUTH_test/gettest/manifest-bc-expires',
+                          swob.HTTPAccepted, {})
+
         self._setup_manifest_data()
 
     def test_head_part_number(self):
@@ -5768,6 +5802,58 @@ class TestPartNumber(SloGETorHEADTestCase):
         ]
         self.assertEqual(self.app.calls, expected_calls)
 
+    def test_get_manifest_with_x_open_expired_part_num(self):
+        req = Request.blank(
+            '/v1/AUTH_test/gettest/manifest-bc-expires'
+            '?multipart-manifest=get',
+            environ={'REQUEST_METHOD': 'GET'})
+
+        captured_calls = []
+        orig_call = FakeSwift.__call__
+
+        def pseudo_middleware(app, env, start_response):
+            captured_calls.append((env['REQUEST_METHOD'], env['PATH_INFO']))
+            # pretend another middleware modified the path
+            # note: for convenience, the path "modification" actually results
+            # in one of the pre-registered paths
+            env['PATH_INFO'] += ''
+            return orig_call(app, env, start_response)
+
+        with patch.object(FakeSwift, '__call__', pseudo_middleware):
+            status, headers, body = self.call_slo(req)
+
+        self.assertEqual(status, '200 OK')
+        self.assertEqual([('GET',
+                           '/v1/AUTH_test/gettest/manifest-bc-expires')],
+                         captured_calls)
+
+        t = str(int(time.time()))
+        req = Request.blank(
+            '/v1/AUTH_test/gettest/manifest-bc-expires',
+            environ={'REQUEST_METHOD': 'POST'},
+            headers={'X-Delete-At': t}
+        )
+
+        with patch.object(FakeSwift, '__call__', pseudo_middleware):
+            status, headers, body = self.call_slo(req)
+
+        self.assertEqual(status, '202 Accepted')
+
+        req = Request.blank(
+            '/v1/AUTH_test/gettest/manifest-bc-expires?part-number=1',
+            environ={'REQUEST_METHOD': 'HEAD'},
+            headers={'x-open-expired': 'true'})
+
+        with patch.object(FakeSwift, '__call__', pseudo_middleware):
+            status, headers, body = self.call_slo(req)
+
+        self.assertEqual(status, '206 Partial Content')
+        self.assertEqual(headers['X-Static-Large-Object'], 'true')
+        self.assertEqual(headers['Etag'], '"%s"' %
+                         self.manifest_bc_expires_slo_etag)
+        self.assertEqual(self.app.call_count, 4)
+        self.assertTrue(self.app.calls_with_headers[2][2]['X-Open-Expired'])
+
     def test_get_part_number(self):
         # part number 1 is b_10
         req = Request.blank(
@@ -6028,6 +6114,32 @@ class TestPartNumber(SloGETorHEADTestCase):
         ]
         self.assertEqual(expected_calls, self.app.calls)
 
+    def test_part_number_zero_byte_manifest(self):
+        part_num = 1
+        req = Request.blank(
+            '/v1/AUTH_test/gettest/manifest-zero-byte?'
+            'partNumber=%s' % part_num,
+            method='HEAD')
+        status, headers, body = self.call_slo(req)
+        self.assertEqual(status, '200 OK')
+        self.assertEqual(headers['Etag'],
+                         '"%s"' % self.manifest_zero_byte_slo_etag)
+        self.assertEqual(headers['Content-Length'], '0')
+        self.assertEqual(headers['X-Static-Large-Object'], 'true')
+        self.assertEqual(headers['X-Manifest-Etag'],
+                         self.manifest_zero_byte_json_md5)
+        self.assertEqual(body, b'')  # it's a HEAD request, after all
+
+        expected_app_calls = [('HEAD',
+                               '/v1/AUTH_test/gettest/manifest-zero-byte?'
+                               'partNumber=%s' % part_num)]
+        if not self.modern_manifest_headers:
+            expected_app_calls.append((
+                'GET',
+                '/v1/AUTH_test/gettest/manifest-zero-byte?'
+                'partNumber=%s' % part_num))
+        self.assertEqual(self.app.calls, expected_app_calls)
+
     def test_part_number_zero_invalid_on_subrange(self):
         # either manifest, doesn't matter, part-number=0 is always invalid
         req = Request.blank(