openstack/swift
Code Issues Proposed changes

Merge "Fix handling of DELETE obj reqs with old timestamp"

Browse Source
This commit is contained in:
Jenkins 2013-08-07 16:01:43 +00:00 committed by Gerrit Code Review
commit 95bcd7180c
3 changed files with 222 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
swift/obj/diskfile.py
View File

@ -366,7 +366,6 @@ class DiskFile(object):
self.logger = logger
self.disallowed_metadata_keys = disallowed_metadata_keys or []
self.metadata = {}
self.meta_file = None
self.data_file = None
self.fp = None
self.iter_etag = None
@ -379,13 +378,16 @@ class DiskFile(object):
if not exists(self.datadir):
return
files = sorted(os.listdir(self.datadir), reverse=True)
meta_file = None
for afile in files:
if afile.endswith('.ts'):
self.data_file = self.meta_file = None
self.metadata = {'deleted': True}
return
if afile.endswith('.meta') and not self.meta_file:
self.meta_file = join(self.datadir, afile)
self.data_file = None
with open(join(self.datadir, afile)) as mfp:
self.metadata = read_metadata(mfp)
self.metadata['deleted'] = True
break
if afile.endswith('.meta') and not meta_file:
meta_file = join(self.datadir, afile)
if afile.endswith('.data') and not self.data_file:
self.data_file = join(self.datadir, afile)
break
@ -395,8 +397,8 @@ class DiskFile(object):
self.metadata = read_metadata(self.fp)
if not keep_data_fp:
self.close(verify_file=False)
if self.meta_file:
with open(self.meta_file) as mfp:
if meta_file:
with open(meta_file) as mfp:
for key in self.metadata.keys():
if key.lower() not in self.disallowed_metadata_keys:
del self.metadata[key]

35
swift/obj/server.py
View File

@ -43,7 +43,8 @@ from swift.common.swob import HTTPAccepted, HTTPBadRequest, HTTPCreated, \
HTTPInternalServerError, HTTPNoContent, HTTPNotFound, HTTPNotModified, \
HTTPPreconditionFailed, HTTPRequestTimeout, HTTPUnprocessableEntity, \
HTTPClientDisconnect, HTTPMethodNotAllowed, Request, Response, UTC, \
HTTPInsufficientStorage, HTTPForbidden, HTTPException, HeaderKeyDict
HTTPInsufficientStorage, HTTPForbidden, HTTPException, HeaderKeyDict, \
HTTPConflict
from swift.obj.diskfile import DiskFile, get_hashes
@ -317,6 +318,9 @@ class ObjectController(object):
except (DiskFileError, DiskFileNotExist):
disk_file.quarantine()
return HTTPNotFound(request=request)
orig_timestamp = disk_file.metadata.get('X-Timestamp', '0')
if orig_timestamp >= request.headers['x-timestamp']:
return HTTPConflict(request=request)
metadata = {'X-Timestamp': request.headers['x-timestamp']}
metadata.update(val for val in request.headers.iteritems()
if val[0].startswith('X-Object-Meta-'))
@ -364,6 +368,8 @@ class ObjectController(object):
return HTTPInsufficientStorage(drive=device, request=request)
old_delete_at = int(disk_file.metadata.get('X-Delete-At') or 0)
orig_timestamp = disk_file.metadata.get('X-Timestamp')
if orig_timestamp and orig_timestamp >= request.headers['x-timestamp']:
return HTTPConflict(request=request)
upload_expiration = time.time() + self.max_upload_time
etag = md5()
elapsed_time = 0
@ -563,25 +569,26 @@ class ObjectController(object):
return HTTPPreconditionFailed(
request=request,
body='X-If-Delete-At and X-Delete-At do not match')
orig_timestamp = disk_file.metadata.get('X-Timestamp')
if disk_file.is_deleted() or disk_file.is_expired():
response_class = HTTPNotFound
else:
response_class = HTTPNoContent
metadata = {
'X-Timestamp': request.headers['X-Timestamp'], 'deleted': True,
}
old_delete_at = int(disk_file.metadata.get('X-Delete-At') or 0)
if old_delete_at:
self.delete_at_update('DELETE', old_delete_at, account,
container, obj, request, device)
disk_file.put_metadata(metadata, tombstone=True)
disk_file.unlinkold(metadata['X-Timestamp'])
if not orig_timestamp or \
orig_timestamp < request.headers['x-timestamp']:
orig_timestamp = disk_file.metadata.get('X-Timestamp', 0)
req_timestamp = request.headers['X-Timestamp']
if disk_file.is_deleted() or disk_file.is_expired():
response_class = HTTPNotFound
else:
if orig_timestamp < req_timestamp:
response_class = HTTPNoContent
else:
response_class = HTTPConflict
if orig_timestamp < req_timestamp:
disk_file.put_metadata({'X-Timestamp': req_timestamp},
tombstone=True)
disk_file.unlinkold(req_timestamp)
self.container_update(
'DELETE', account, container, obj, request,
HeaderKeyDict({'x-timestamp': metadata['X-Timestamp']}),
HeaderKeyDict({'x-timestamp': req_timestamp}),
device)
resp = response_class(request=request)
return resp

200
test/unit/obj/test_server.py
View File

@ -223,6 +223,41 @@ class TestObjectController(unittest.TestCase):
"X-Object-Meta-3" in resp.headers)
self.assertEquals(resp.headers['Content-Type'], 'application/x-test')
def test_POST_old_timestamp(self):
ts = time()
timestamp = normalize_timestamp(ts)
req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
headers={'X-Timestamp': timestamp,
'Content-Type': 'application/x-test',
'X-Object-Meta-1': 'One',
'X-Object-Meta-Two': 'Two'})
req.body = 'VERIFY'
resp = self.object_controller.PUT(req)
self.assertEquals(resp.status_int, 201)
# Same timestamp should result in 409
req = Request.blank('/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'POST'},
headers={'X-Timestamp': timestamp,
'X-Object-Meta-3': 'Three',
'X-Object-Meta-4': 'Four',
'Content-Encoding': 'gzip',
'Content-Type': 'application/x-test'})
resp = self.object_controller.POST(req)
self.assertEquals(resp.status_int, 409)
# Earlier timestamp should result in 409
timestamp = normalize_timestamp(ts - 1)
req = Request.blank('/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'POST'},
headers={'X-Timestamp': timestamp,
'X-Object-Meta-5': 'Five',
'X-Object-Meta-6': 'Six',
'Content-Encoding': 'gzip',
'Content-Type': 'application/x-test'})
resp = self.object_controller.POST(req)
self.assertEquals(resp.status_int, 409)
def test_POST_not_exist(self):
timestamp = normalize_timestamp(time())
req = Request.blank('/sda1/p/a/c/fail',
@ -269,14 +304,16 @@ class TestObjectController(unittest.TestCase):
old_http_connect = object_server.http_connect
try:
timestamp = normalize_timestamp(time())
ts = time()
timestamp = normalize_timestamp(ts)
req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD':
'POST'}, headers={'X-Timestamp': timestamp, 'Content-Type':
'text/plain', 'Content-Length': '0'})
resp = self.object_controller.PUT(req)
self.assertEquals(resp.status_int, 201)
req = Request.blank('/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'POST'},
headers={'X-Timestamp': timestamp,
headers={'X-Timestamp': normalize_timestamp(ts + 1),
'X-Container-Host': '1.2.3.4:0',
'X-Container-Partition': '3',
'X-Container-Device': 'sda1',
@ -287,7 +324,7 @@ class TestObjectController(unittest.TestCase):
self.assertEquals(resp.status_int, 202)
req = Request.blank('/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'POST'},
headers={'X-Timestamp': timestamp,
headers={'X-Timestamp': normalize_timestamp(ts + 2),
'X-Container-Host': '1.2.3.4:0',
'X-Container-Partition': '3',
'X-Container-Device': 'sda1',
@ -298,7 +335,7 @@ class TestObjectController(unittest.TestCase):
self.assertEquals(resp.status_int, 202)
req = Request.blank('/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'POST'},
headers={'X-Timestamp': timestamp,
headers={'X-Timestamp': normalize_timestamp(ts + 3),
'X-Container-Host': '1.2.3.4:0',
'X-Container-Partition': '3',
'X-Container-Device': 'sda1',
@ -439,6 +476,32 @@ class TestObjectController(unittest.TestCase):
'name': '/a/c/o',
'Content-Encoding': 'gzip'})
def test_PUT_old_timestamp(self):
ts = time()
req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
headers={'X-Timestamp': normalize_timestamp(ts),
'Content-Length': '6',
'Content-Type': 'application/octet-stream'})
req.body = 'VERIFY'
resp = self.object_controller.PUT(req)
self.assertEquals(resp.status_int, 201)
req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
headers={'X-Timestamp': normalize_timestamp(ts),
'Content-Type': 'text/plain',
'Content-Encoding': 'gzip'})
req.body = 'VERIFY TWO'
resp = self.object_controller.PUT(req)
self.assertEquals(resp.status_int, 409)
req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
headers={'X-Timestamp': normalize_timestamp(ts - 1),
'Content-Type': 'text/plain',
'Content-Encoding': 'gzip'})
req.body = 'VERIFY THREE'
resp = self.object_controller.PUT(req)
self.assertEquals(resp.status_int, 409)
def test_PUT_no_etag(self):
req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
headers={'X-Timestamp': normalize_timestamp(time()),
@ -1014,7 +1077,7 @@ class TestObjectController(unittest.TestCase):
self.assertEquals(resp.status_int, 404)
def test_DELETE(self):
""" Test swift.object_server.ObjectController.DELETE """
# Test swift.object_server.ObjectController.DELETE
req = Request.blank('/sda1/p/a/c',
environ={'REQUEST_METHOD': 'DELETE'})
resp = self.object_controller.DELETE(req)
@ -1026,12 +1089,32 @@ class TestObjectController(unittest.TestCase):
self.assertEquals(resp.status_int, 400)
# self.assertRaises(KeyError, self.object_controller.DELETE, req)
# The following should have created a tombstone file
timestamp = normalize_timestamp(time())
req = Request.blank('/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'DELETE'},
headers={'X-Timestamp': timestamp})
resp = self.object_controller.DELETE(req)
self.assertEquals(resp.status_int, 404)
objfile = os.path.join(self.testdir, 'sda1',
storage_directory(object_server.DATADIR, 'p',
hash_path('a', 'c', 'o')),
timestamp + '.ts')
self.assert_(os.path.isfile(objfile))
# The following should *not* have created a tombstone file.
timestamp = normalize_timestamp(float(timestamp) - 1)
req = Request.blank('/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'DELETE'},
headers={'X-Timestamp': timestamp})
resp = self.object_controller.DELETE(req)
self.assertEquals(resp.status_int, 404)
objfile = os.path.join(self.testdir, 'sda1',
storage_directory(object_server.DATADIR, 'p',
hash_path('a', 'c', 'o')),
timestamp + '.ts')
self.assertFalse(os.path.isfile(objfile))
self.assertEquals(len(os.listdir(os.path.dirname(objfile))), 1)
sleep(.00001)
timestamp = normalize_timestamp(time())
@ -1045,17 +1128,19 @@ class TestObjectController(unittest.TestCase):
resp = self.object_controller.PUT(req)
self.assertEquals(resp.status_int, 201)
# The following should *not* have created a tombstone file.
timestamp = normalize_timestamp(float(timestamp) - 1)
req = Request.blank('/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'DELETE'},
headers={'X-Timestamp': timestamp})
resp = self.object_controller.DELETE(req)
self.assertEquals(resp.status_int, 204)
self.assertEquals(resp.status_int, 409)
objfile = os.path.join(self.testdir, 'sda1',
storage_directory(object_server.DATADIR, 'p',
hash_path('a', 'c', 'o')),
timestamp + '.ts')
self.assert_(os.path.isfile(objfile))
self.assertFalse(os.path.isfile(objfile))
self.assertEquals(len(os.listdir(os.path.dirname(objfile))), 1)
sleep(.00001)
timestamp = normalize_timestamp(time())
@ -1070,6 +1155,103 @@ class TestObjectController(unittest.TestCase):
timestamp + '.ts')
self.assert_(os.path.isfile(objfile))
def test_DELETE_container_updates(self):
# Test swift.object_server.ObjectController.DELETE and container
# updates, making sure container update is called in the correct
# state.
timestamp = normalize_timestamp(time())
req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
headers={
'X-Timestamp': timestamp,
'Content-Type': 'application/octet-stream',
'Content-Length': '4',
})
req.body = 'test'
resp = self.object_controller.PUT(req)
self.assertEquals(resp.status_int, 201)
calls_made = [0]
def our_container_update(*args, **kwargs):
calls_made[0] += 1
orig_cu = self.object_controller.container_update
self.object_controller.container_update = our_container_update
try:
# The following request should return 409 (HTTP Conflict). A
# tombstone file should not have been created with this timestamp.
timestamp = normalize_timestamp(float(timestamp) - 1)
req = Request.blank('/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'DELETE'},
headers={'X-Timestamp': timestamp})
resp = self.object_controller.DELETE(req)
self.assertEquals(resp.status_int, 409)
objfile = os.path.join(self.testdir, 'sda1',
storage_directory(object_server.DATADIR, 'p',
hash_path('a', 'c', 'o')),
timestamp + '.ts')
self.assertFalse(os.path.isfile(objfile))
self.assertEquals(len(os.listdir(os.path.dirname(objfile))), 1)
self.assertEquals(0, calls_made[0])
# The following request should return 204, and the object should
# be truly deleted (container update is performed) because this
# timestamp is newer. A tombstone file should have been created
# with this timestamp.
sleep(.00001)
timestamp = normalize_timestamp(time())
req = Request.blank('/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'DELETE'},
headers={'X-Timestamp': timestamp})
resp = self.object_controller.DELETE(req)
self.assertEquals(resp.status_int, 204)
objfile = os.path.join(self.testdir, 'sda1',
storage_directory(object_server.DATADIR, 'p',
hash_path('a', 'c', 'o')),
timestamp + '.ts')
self.assert_(os.path.isfile(objfile))
self.assertEquals(1, calls_made[0])
self.assertEquals(len(os.listdir(os.path.dirname(objfile))), 1)
# The following request should return a 404, as the object should
# already have been deleted, but it should have also performed a
# container update because the timestamp is newer, and a tombstone
# file should also exist with this timestamp.
sleep(.00001)
timestamp = normalize_timestamp(time())
req = Request.blank('/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'DELETE'},
headers={'X-Timestamp': timestamp})
resp = self.object_controller.DELETE(req)
self.assertEquals(resp.status_int, 404)
objfile = os.path.join(self.testdir, 'sda1',
storage_directory(object_server.DATADIR, 'p',
hash_path('a', 'c', 'o')),
timestamp + '.ts')
self.assert_(os.path.isfile(objfile))
self.assertEquals(2, calls_made[0])
self.assertEquals(len(os.listdir(os.path.dirname(objfile))), 1)
# The following request should return a 404, as the object should
# already have been deleted, and it should not have performed a
# container update because the timestamp is older, or created a
# tombstone file with this timestamp.
timestamp = normalize_timestamp(float(timestamp) - 1)
req = Request.blank('/sda1/p/a/c/o',
environ={'REQUEST_METHOD': 'DELETE'},
headers={'X-Timestamp': timestamp})
resp = self.object_controller.DELETE(req)
self.assertEquals(resp.status_int, 404)
objfile = os.path.join(self.testdir, 'sda1',
storage_directory(object_server.DATADIR, 'p',
hash_path('a', 'c', 'o')),
timestamp + '.ts')
self.assertFalse(os.path.isfile(objfile))
self.assertEquals(2, calls_made[0])
self.assertEquals(len(os.listdir(os.path.dirname(objfile))), 1)
finally:
self.object_controller.container_update = orig_cu
def test_call(self):
""" Test swift.object_server.ObjectController.__call__ """
inbuf = StringIO()
@ -1161,7 +1343,7 @@ class TestObjectController(unittest.TestCase):
'SERVER_PROTOCOL': 'HTTP/1.0',
'CONTENT_LENGTH': '0',
'CONTENT_TYPE': 'text/html',
'HTTP_X_TIMESTAMP': 1.2,
'HTTP_X_TIMESTAMP': '1.2',
'wsgi.version': (1, 0),
'wsgi.url_scheme': 'http',
'wsgi.input': inbuf,
@ -1184,7 +1366,7 @@ class TestObjectController(unittest.TestCase):
'SERVER_PROTOCOL': 'HTTP/1.0',
'CONTENT_LENGTH': '0',
'CONTENT_TYPE': 'text/html',
'HTTP_X_TIMESTAMP': 1.3,
'HTTP_X_TIMESTAMP': '1.3',
'wsgi.version': (1, 0),
'wsgi.url_scheme': 'http',
'wsgi.input': inbuf,