openstack/swift
Code Issues Proposed changes

Remove confusable query string on post as copy

Browse Source 
Current post as copy routine (i.e. POST object with post_as_copy option
turned on) on Object Controller uses "multipart-manifest" query string
which is feeded to env['copy_hook'] to decide which data (the manifest or
object pointed by the manifest) should be copied.

However, the way using the query string will confuse operators looking at
logging system (or analyzing the log) because whole POST object requests
have 'multipart-manifest=get' like as:

POST /v1/AUTH_test/d4c816b24d38489082f5118599a67920/manifest-abcde%3Fmultipart-manifest%3Dget

We cannot know whether the query string was added by hand
(from user) or not. In addition, the query isn't needed by the
backend conversation between proxy-server and object-server.
(Just needed by "copy_hook" on the proxy controller!)

To remove the confusable query string and to keep the log to be clean,
this patch introduces new environment variable "swift.post_as_copy"
and changes proxy controller and the copy_hook to use the new env.

This item was originally discussed at
https://review.openstack.org/#/c/177132/

Co-Authored-By: Alistair Coles <alistair.coles@hp.com>

Change-Id: I0cd37520eea1825a10ebd27ccdc7e9162647233e
This commit is contained in:
Kota Tsuyuzaki 2015-04-24 02:15:36 -07:00
parent 29f4393d88
commit 025c4c4339
5 changed files with 89 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
swift/common/middleware/slo.py
View File

@ -537,7 +537,8 @@ class StaticLargeObject(object):
def slo_hook(source_req, source_resp, sink_req):
x_slo = source_resp.headers.get('X-Static-Large-Object')
if (config_true_value(x_slo)
and source_req.params.get('multipart-manifest') != 'get'):
and source_req.params.get('multipart-manifest') != 'get'
and 'swift.post_as_copy' not in source_req.environ):
source_resp = SloGetContext(self).get_or_head_response(
source_req, source_resp.headers.items(),
source_resp.app_iter)

16
swift/proxy/controllers/obj.py
View File

@ -268,12 +268,8 @@ class BaseObjectController(Controller):
req.headers['Content-Length'] = 0
req.headers['X-Copy-From'] = quote('/%s/%s' % (self.container_name,
self.object_name))
req.headers['X-Fresh-Metadata'] = 'true'
req.environ['swift.post_as_copy'] = True
req.environ['swift_versioned_copy'] = True
if req.environ.get('QUERY_STRING'):
req.environ['QUERY_STRING'] += '&multipart-manifest=get'
else:
req.environ['QUERY_STRING'] = 'multipart-manifest=get'
resp = self.PUT(req)
# Older editions returned 202 Accepted on object POSTs, so we'll
# convert any 201 Created responses to that for compatibility with
@ -577,8 +573,11 @@ class BaseObjectController(Controller):
if not req.content_type_manually_set:
sink_req.headers['Content-Type'] = \
source_resp.headers['Content-Type']
if config_true_value(
sink_req.headers.get('x-fresh-metadata', 'false')):
fresh_meta_flag = config_true_value(
sink_req.headers.get('x-fresh-metadata', 'false'))
if fresh_meta_flag or 'swift.post_as_copy' in sink_req.environ:
# post-as-copy: ignore new sysmeta, copy existing sysmeta
condition = lambda k: is_sys_meta('object', k)
remove_items(sink_req.headers, condition)
@ -590,7 +589,8 @@ class BaseObjectController(Controller):
# copy over x-static-large-object for POSTs and manifest copies
if 'X-Static-Large-Object' in source_resp.headers and \
req.params.get('multipart-manifest') == 'get':
(req.params.get('multipart-manifest') == 'get' or
'swift.post_as_copy' in req.environ):
sink_req.headers['X-Static-Large-Object'] = \
source_resp.headers['X-Static-Large-Object']

5
test/functional/swift_test_client.py
View File

@ -851,7 +851,7 @@ class File(Base):
finally:
fobj.close()
def sync_metadata(self, metadata=None, cfg=None):
def sync_metadata(self, metadata=None, cfg=None, parms=None):
if metadata is None:
metadata = {}
if cfg is None:
@ -868,7 +868,8 @@ class File(Base):
else:
headers['Content-Length'] = 0
self.conn.make_request('POST', self.path, hdrs=headers, cfg=cfg)
self.conn.make_request('POST', self.path, hdrs=headers,
parms=parms, cfg=cfg)
if self.conn.response.status not in (201, 202):
raise ResponseError(self.conn.response, 'POST',

53
test/functional/tests.py
View File

@ -2151,6 +2151,7 @@ class TestSloEnv(object):
'manifest-bcd-submanifest')},
seg_info['seg_e']]),
parms={'multipart-manifest': 'put'})
cls.seg_info = seg_info
class TestSlo(Base):
@ -2356,6 +2357,58 @@ class TestSlo(Base):
except ValueError:
self.fail("COPY didn't copy the manifest (invalid json on GET)")
def _make_manifest(self):
# To avoid the bug 1453807 on fast-post, make a new manifest
# for post test.
file_item = self.env.container.file("manifest-post")
seg_info = self.env.seg_info
file_item.write(
json.dumps([seg_info['seg_a'], seg_info['seg_b'],
seg_info['seg_c'], seg_info['seg_d'],
seg_info['seg_e']]),
parms={'multipart-manifest': 'put'})
return file_item
def test_slo_post_the_manifest_metadata_update(self):
file_item = self._make_manifest()
# sanity check, check the object is an SLO manifest
file_item.info()
file_item.header_fields([('slo', 'x-static-large-object')])
# POST a user metadata (i.e. x-object-meta-post)
file_item.sync_metadata({'post': 'update'})
updated = self.env.container.file("manifest-post")
updated.info()
updated.header_fields([('user-meta', 'x-object-meta-post')]) # sanity
updated_contents = updated.read(parms={'multipart-manifest': 'get'})
try:
json.loads(updated_contents)
except ValueError:
self.fail("Unexpected content on GET, expected a json body")
def test_slo_post_the_manifest_metadata_update_with_qs(self):
# multipart-manifest query should be ignored on post
for verb in ('put', 'get', 'delete'):
file_item = self._make_manifest()
# sanity check, check the object is an SLO manifest
file_item.info()
file_item.header_fields([('slo', 'x-static-large-object')])
# POST a user metadata (i.e. x-object-meta-post)
file_item.sync_metadata(metadata={'post': 'update'},
parms={'multipart-manifest': verb})
updated = self.env.container.file("manifest-post")
updated.info()
updated.header_fields(
[('user-meta', 'x-object-meta-post')]) # sanity
updated_contents = updated.read(
parms={'multipart-manifest': 'get'})
try:
json.loads(updated_contents)
except ValueError:
self.fail(
"Unexpected content on GET, expected a json body")
def test_slo_get_the_manifest(self):
manifest = self.env.container.file("manifest-abcde")
got_body = manifest.read(parms={'multipart-manifest': 'get'})

25
test/unit/proxy/controllers/test_obj.py
View File

@ -598,13 +598,31 @@ class TestReplicatedObjController(BaseObjectControllerMixin,
def test_POST_as_COPY_simple(self):
req = swift.common.swob.Request.blank('/v1/a/c/o', method='POST')
head_resp = [200] * self.obj_ring.replicas + \
get_resp = [200] * self.obj_ring.replicas + \
[404] * self.obj_ring.max_more_nodes
put_resp = [201] * self.obj_ring.replicas
codes = head_resp + put_resp
codes = get_resp + put_resp
with set_http_connect(*codes):
resp = req.get_response(self.app)
self.assertEquals(resp.status_int, 202)
self.assertEquals(req.environ['QUERY_STRING'], '')
self.assertTrue('swift.post_as_copy' in req.environ)
def test_POST_as_COPY_static_large_object(self):
req = swift.common.swob.Request.blank('/v1/a/c/o', method='POST')
get_resp = [200] * self.obj_ring.replicas + \
[404] * self.obj_ring.max_more_nodes
put_resp = [201] * self.obj_ring.replicas
codes = get_resp + put_resp
slo_headers = \
[{'X-Static-Large-Object': True}] * self.obj_ring.replicas
get_headers = slo_headers + [{}] * (len(codes) - len(slo_headers))
headers = {'headers': get_headers}
with set_http_connect(*codes, **headers):
resp = req.get_response(self.app)
self.assertEquals(resp.status_int, 202)
self.assertEquals(req.environ['QUERY_STRING'], '')
self.assertTrue('swift.post_as_copy' in req.environ)
def test_POST_delete_at(self):
t = str(int(time.time() + 100))
@ -624,6 +642,9 @@ class TestReplicatedObjController(BaseObjectControllerMixin,
with set_http_connect(*codes, give_connect=capture_headers):
resp = req.get_response(self.app)
self.assertEquals(resp.status_int, 200)
self.assertEquals(req.environ['QUERY_STRING'], '') # sanity
self.assertTrue('swift.post_as_copy' in req.environ)
for given_headers in post_headers:
self.assertEquals(given_headers.get('X-Delete-At'), t)
self.assertTrue('X-Delete-At-Host' in given_headers)