From 3ad877323978dcf4130e9543a2678bb928c1ecb8 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Wed, 15 Mar 2017 19:30:38 +0000 Subject: [PATCH] Version DLOs, just like every other type of object Previously, requests involving DLOs would bypass versioned_writes: * Any existing DLOs wouldn't get copied to the archive container during overwrites (or deletes, with history-mode), so there would be no evidence they had ever existed. * Any new DLOs wouldn't copy overwritten objects to the archive container, potentially leading to data loss. Now, DLOs will behave like every other type of object under versioned_writes. Change-Id: I488e13eead2f33dd272d03f6f898adc52fc7fdad Related-Change: Ie899290b3312e201979eafefb253d1a60b65b837 Related-Change: Ib5b29a19e1d577026deb50fc9d26064a8da81cd7 Closes-Bug: #1626989 --- swift/common/middleware/versioned_writes.py | 8 --- test/functional/test_versioned_writes.py | 69 +++++++++++++++++-- .../middleware/test_versioned_writes.py | 40 ++++++++--- test/unit/proxy/test_server.py | 9 ++- 4 files changed, 101 insertions(+), 25 deletions(-) diff --git a/swift/common/middleware/versioned_writes.py b/swift/common/middleware/versioned_writes.py index 3ece3524ee..343527efb5 100644 --- a/swift/common/middleware/versioned_writes.py +++ b/swift/common/middleware/versioned_writes.py @@ -429,10 +429,6 @@ class VersionedWritesContext(WSGIContext): get_resp = self._get_source_object(req, req.path_info) - if 'X-Object-Manifest' in get_resp.headers: - # do not version DLO manifest, proceed with original request - close_if_possible(get_resp.app_iter) - return if get_resp.status_int == HTTP_NOT_FOUND: # nothing to version, proceed with original request close_if_possible(get_resp.app_iter) @@ -470,10 +466,6 @@ class VersionedWritesContext(WSGIContext): :param account_name: account name. :param object_name: name of object of original request """ - if 'X-Object-Manifest' in req.headers: - # do not version DLO manifest, proceed with original request - return self.app - self._copy_current(req, versions_cont, api_version, account_name, object_name) return self.app diff --git a/test/functional/test_versioned_writes.py b/test/functional/test_versioned_writes.py index 49ce7a6c1c..e176cc5c77 100644 --- a/test/functional/test_versioned_writes.py +++ b/test/functional/test_versioned_writes.py @@ -365,7 +365,20 @@ class TestObjectVersioning(Base): versioned_obj.delete() self.assertRaises(ResponseError, versioned_obj.read) - def test_versioning_dlo(self): + def assert_most_recent_version(self, obj_name, content, + should_be_dlo=False): + archive_versions = self.env.versions_container.files(parms={ + 'prefix': '%03x%s/' % (len(obj_name), obj_name), + 'reverse': 'yes'}) + archive_file = self.env.versions_container.file(archive_versions[0]) + self.assertEqual(content, archive_file.read()) + resp_headers = dict(archive_file.conn.response.getheaders()) + if should_be_dlo: + self.assertIn('x-object-manifest', resp_headers) + else: + self.assertNotIn('x-object-manifest', resp_headers) + + def _test_versioning_dlo_setup(self): container = self.env.container versions_container = self.env.versions_container obj_name = Utils.create_name() @@ -375,23 +388,51 @@ class TestObjectVersioning(Base): obj_name_seg = obj_name + '/' + i versioned_obj = container.file(obj_name_seg) versioned_obj.write(i) + # immediately overwrite versioned_obj.write(i + i) self.assertEqual(3, versions_container.info()['object_count']) man_file = container.file(obj_name) - man_file.write('', hdrs={"X-Object-Manifest": "%s/%s/" % - (self.env.container.name, obj_name)}) + + # write a normal file first + man_file.write('old content') # guarantee that the timestamp changes time.sleep(.01) - # write manifest file again + # overwrite with a dlo manifest man_file.write('', hdrs={"X-Object-Manifest": "%s/%s/" % (self.env.container.name, obj_name)}) - self.assertEqual(3, versions_container.info()['object_count']) + self.assertEqual(4, versions_container.info()['object_count']) self.assertEqual("112233", man_file.read()) + self.assert_most_recent_version(obj_name, 'old content') + + # overwrite the manifest with a normal file + man_file.write('new content') + self.assertEqual(5, versions_container.info()['object_count']) + + # new most-recent archive is the dlo + self.assert_most_recent_version(obj_name, '112233', should_be_dlo=True) + + return obj_name, man_file + + def test_versioning_dlo(self): + obj_name, man_file = self._test_versioning_dlo_setup() + + # verify that restore works properly + man_file.delete() + self.assertEqual(4, self.env.versions_container.info()['object_count']) + self.assertEqual("112233", man_file.read()) + resp_headers = dict(man_file.conn.response.getheaders()) + self.assertIn('x-object-manifest', resp_headers) + + self.assert_most_recent_version(obj_name, 'old content') + + man_file.delete() + self.assertEqual(3, self.env.versions_container.info()['object_count']) + self.assertEqual("old content", man_file.read()) def test_versioning_container_acl(self): # create versions container and DO NOT give write access to account2 @@ -592,6 +633,24 @@ class TestObjectVersioningHistoryMode(TestObjectVersioning): self.assertEqual(404, cm.exception.status) self.assertEqual(11, versions_container.info()['object_count']) + def test_versioning_dlo(self): + obj_name, man_file = \ + self._test_versioning_dlo_setup() + + man_file.delete() + with self.assertRaises(ResponseError) as cm: + man_file.read() + self.assertEqual(404, cm.exception.status) + self.assertEqual(7, self.env.versions_container.info()['object_count']) + + expected = ['old content', '112233', 'new content', ''] + + bodies = [ + self.env.versions_container.file(f).read() + for f in self.env.versions_container.files(parms={ + 'prefix': '%03x%s/' % (len(obj_name), obj_name)})] + self.assertEqual(expected, bodies) + def test_versioning_check_acl(self): versioned_obj = self._test_versioning_check_acl_setup() versioned_obj.delete() diff --git a/test/unit/common/middleware/test_versioned_writes.py b/test/unit/common/middleware/test_versioned_writes.py index e23def4b80..16b2240f37 100644 --- a/test/unit/common/middleware/test_versioned_writes.py +++ b/test/unit/common/middleware/test_versioned_writes.py @@ -397,27 +397,42 @@ class VersionedWritesTestCase(VersionedWritesBaseTestCase): self.assertNotIn('GET', called_method) def test_put_request_is_dlo_manifest_with_container_config_true(self): - # set x-object-manifest on request and expect no versioning occurred - # only the PUT for the original client request self.app.register( 'PUT', '/v1/a/c/o', swob.HTTPCreated, {}, 'passed') + self.app.register( + 'GET', '/v1/a/c/o', swob.HTTPOk, + {'last-modified': 'Thu, 1 Jan 1970 00:01:00 GMT'}, 'old version') + self.app.register( + 'PUT', '/v1/a/ver_cont/001o/0000000060.00000', swob.HTTPCreated, + {}, '') cache = FakeCache({'versions': 'ver_cont'}) req = Request.blank( '/v1/a/c/o', + headers={'X-Object-Manifest': 'req/manifest'}, environ={'REQUEST_METHOD': 'PUT', 'swift.cache': cache, 'CONTENT_LENGTH': '100'}) - req.headers['X-Object-Manifest'] = 'req/manifest' status, headers, body = self.call_vw(req) self.assertEqual(status, '201 Created') - self.assertEqual(len(self.authorized), 1) + self.assertEqual(len(self.authorized), 2) self.assertRequestEqual(req, self.authorized[0]) - self.assertEqual(1, self.app.call_count) + self.assertRequestEqual(req, self.authorized[1]) + self.assertEqual(3, self.app.call_count) + self.assertEqual([ + ('GET', '/v1/a/c/o'), + ('PUT', '/v1/a/ver_cont/001o/0000000060.00000'), + ('PUT', '/v1/a/c/o'), + ], self.app.calls) + self.assertIn('x-object-manifest', + self.app.calls_with_headers[2].headers) def test_put_version_is_dlo_manifest_with_container_config_true(self): - # set x-object-manifest on response and expect no versioning occurred - # only initial GET on source object ok followed by PUT self.app.register('GET', '/v1/a/c/o', swob.HTTPOk, - {'X-Object-Manifest': 'resp/manifest'}, 'passed') + {'X-Object-Manifest': 'resp/manifest', + 'last-modified': 'Thu, 1 Jan 1970 01:00:00 GMT'}, + 'passed') + self.app.register( + 'PUT', '/v1/a/ver_cont/001o/0000003600.00000', swob.HTTPCreated, + {}, '') self.app.register( 'PUT', '/v1/a/c/o', swob.HTTPCreated, {}, 'passed') cache = FakeCache({'versions': 'ver_cont'}) @@ -433,7 +448,14 @@ class VersionedWritesTestCase(VersionedWritesBaseTestCase): self.assertEqual(len(self.authorized), 2) self.assertRequestEqual(req, self.authorized[0]) self.assertRequestEqual(req, self.authorized[1]) - self.assertEqual(2, self.app.call_count) + self.assertEqual(3, self.app.call_count) + self.assertEqual([ + ('GET', '/v1/a/c/o'), + ('PUT', '/v1/a/ver_cont/001o/0000003600.00000'), + ('PUT', '/v1/a/c/o'), + ], self.app.calls) + self.assertIn('x-object-manifest', + self.app.calls_with_headers[1].headers) def test_delete_object_no_versioning_with_container_config_true(self): # set False to versions_write obviously and expect no GET versioning diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 6136ee96f7..36ac8ea073 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -9088,8 +9088,8 @@ class TestSocketObjectVersions(unittest.TestCase): exp = 'HTTP/1.1 404' self.assertEqual(headers[:len(exp)], exp) - # make sure manifest files will be ignored - for _junk in range(1, versions_to_create): + # make sure manifest files are also versioned + for _junk in range(0, versions_to_create): sleep(.01) # guarantee that the timestamp changes sock = connect_tcp(('localhost', prolis.getsockname()[1])) fd = sock.makefile() @@ -9111,8 +9111,11 @@ class TestSocketObjectVersions(unittest.TestCase): % (vc, pre, o)) fd.flush() headers = readuntil2crlfs(fd) - exp = 'HTTP/1.1 204 No Content' + exp = 'HTTP/1.1 200 OK' self.assertEqual(headers[:len(exp)], exp) + body = fd.read() + versions = [x for x in body.split('\n') if x] + self.assertEqual(versions_to_create - 1, len(versions)) # DELETE v1/a/c/obj shouldn't delete v1/a/c/obj/sub versions sock = connect_tcp(('localhost', prolis.getsockname()[1]))