From 60c04f116b41f97a5dcf9c07ef6e77ae9bdfe61e Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Tue, 10 Oct 2023 16:54:21 +0100 Subject: [PATCH] s3api: Stop propagating storage policy to sub-requests The proxy_logging middleware needs an X-Backend-Storage-Policy-Index header to populate the storage policy field in logs, and will look in both request and response headers to find it. Previously, the s3api middleware would indiscriminately copy the X-Backend-Storage-Policy-Index from swift backend requests into the S3Request headers [1]. This works for logging but causes the header to leak between backend requests [2] and break mixed policy multipart uploads. This patch sets the X-Backend-Storage-Policy-Index header on s3api responses rather than requests. Additionally, the middleware now looks for the X-Backend-Storage-Policy-Index header in the swift backend request *and* response headers, in the same way that proxy_logging would (preferring a response header over a request header). This means that a policy index is now logged for bucket requests, which only have X-Backend-Storage-Policy-Index header in their response headers. The s3api adds the value from the *final* backend request/response pair to its response headers. Returning the policy index from the final backend request/response is consistent with swift.backend_path being set to that backend request's path i.e. proxy_logging will log the correct policy index for the logged path. The FakeSwift helper no longer looks in registered object responses for an X-Backend-Storage-Policy-Index header to update an object request. Real Swift object responses do not have an X-Backend-Storage-Policy-Index header. By default, FakeSwift will now update *all* object requests with an X-Backend-Storage-Policy-Index as follows: - If a matching container HEAD response has been registered then any X-Backend-Storage-Policy-Index found with that is used. - Otherwise the default policy index is used. Furthermore, FakeSwift now adds the X-Backend-Storage-Policy-Index header to the request *after* the request has been captured. Tests using FakeSwift.calls_wth_headers() to make assertions about captured headers no longer need to make allowance for the header that FakeSwift added. Co-Authored-By: Clay Gerrard Closes-Bug: #2038459 [1] Related-Change: I5fe5ab31d6b2d9f7b6ecb3bfa246433a78e54808 [2] Related-Change: I40b252446b3a1294a5ca8b531f224ce9c16f9aba Change-Id: I2793e335a08ad373c49cbbe6759d4e97cc420867 --- .zuul.yaml | 5 +- .../s3api/controllers/multi_upload.py | 4 + swift/common/middleware/s3api/s3api.py | 4 + swift/common/middleware/s3api/s3request.py | 13 +- test/probe/test_mixed_policy_mpu.py | 157 ++++++++++++++++++ test/unit/common/middleware/helpers.py | 46 +++-- test/unit/common/middleware/s3api/__init__.py | 11 ++ .../common/middleware/s3api/test_bucket.py | 35 +++- .../middleware/s3api/test_multi_upload.py | 153 ++++++++++++++--- test/unit/common/middleware/s3api/test_obj.py | 43 +++-- test/unit/common/middleware/test_helpers.py | 42 +++++ test/unit/common/middleware/test_slo.py | 2 - test/unit/common/middleware/test_symlink.py | 2 - test/unit/common/test_internal_client.py | 3 +- .../common/install_dependencies.yaml | 1 + 15 files changed, 457 insertions(+), 64 deletions(-) create mode 100644 test/probe/test_mixed_policy_mpu.py diff --git a/.zuul.yaml b/.zuul.yaml index 61ec8ad7b9..278ffab8f6 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -397,11 +397,13 @@ under Python 3. timeout: 7200 vars: + s3_acl: no bindep_profile: test py36 pre-run: - tools/playbooks/common/install_dependencies.yaml - tools/playbooks/saio_single_node_setup/setup_saio.yaml - tools/playbooks/saio_single_node_setup/make_rings.yaml + - tools/playbooks/saio_single_node_setup/add_s3api.yaml run: tools/playbooks/probetests/run.yaml post-run: tools/playbooks/probetests/post.yaml @@ -423,10 +425,7 @@ description: | Setup a SAIO dev environment and run Swift's CORS functional tests timeout: 1200 - vars: - s3_acl: no pre-run: - - tools/playbooks/saio_single_node_setup/add_s3api.yaml - tools/playbooks/cors/install_selenium.yaml run: tools/playbooks/cors/run.yaml post-run: tools/playbooks/cors/post.yaml diff --git a/swift/common/middleware/s3api/controllers/multi_upload.py b/swift/common/middleware/s3api/controllers/multi_upload.py index eff0c50fe5..9e8c2d4127 100644 --- a/swift/common/middleware/s3api/controllers/multi_upload.py +++ b/swift/common/middleware/s3api/controllers/multi_upload.py @@ -108,7 +108,9 @@ def _get_upload_info(req, app, upload_id): try: return req.get_response(app, 'HEAD', container=container, obj=obj) except NoSuchKey: + # ensure consistent path and policy are logged despite manifest HEAD upload_marker_path = req.environ.get('s3api.backend_path') + policy_index = req.policy_index try: resp = req.get_response(app, 'HEAD') if resp.sysmeta_headers.get(sysmeta_header( @@ -121,6 +123,8 @@ def _get_upload_info(req, app, upload_id): # path, so put it back if upload_marker_path is not None: req.environ['s3api.backend_path'] = upload_marker_path + if policy_index is not None: + req.policy_index = policy_index raise NoSuchUpload(upload_id=upload_id) finally: # ...making sure to restore any copy-source before returning diff --git a/swift/common/middleware/s3api/s3api.py b/swift/common/middleware/s3api/s3api.py index 04b402787e..bd0f0c01e1 100644 --- a/swift/common/middleware/s3api/s3api.py +++ b/swift/common/middleware/s3api/s3api.py @@ -366,6 +366,7 @@ class S3ApiMiddleware(object): if 's3api.backend_path' in env and 'swift.backend_path' not in env: env['swift.backend_path'] = env['s3api.backend_path'] + return resp(env, start_response) def handle_request(self, req): @@ -390,6 +391,9 @@ class S3ApiMiddleware(object): raise MethodNotAllowed(req.method, req.controller.resource_type()) + if req.policy_index is not None: + res.headers.setdefault('X-Backend-Storage-Policy-Index', + req.policy_index) return res def check_pipeline(self, wsgi_conf): diff --git a/swift/common/middleware/s3api/s3request.py b/swift/common/middleware/s3api/s3request.py index 7a6c032d77..33f507628c 100644 --- a/swift/common/middleware/s3api/s3request.py +++ b/swift/common/middleware/s3api/s3request.py @@ -26,7 +26,7 @@ from six.moves.urllib.parse import quote, unquote, parse_qsl import string from swift.common.utils import split_path, json, close_if_possible, md5, \ - streq_const_time + streq_const_time, get_policy_index from swift.common.registry import get_swift_info from swift.common import swob from swift.common.http import HTTP_OK, HTTP_CREATED, HTTP_ACCEPTED, \ @@ -552,6 +552,7 @@ class S3Request(swob.Request): } self.account = None self.user_id = None + self.policy_index = None # Avoids that swift.swob.Response replaces Location header value # by full URL when absolute path given. See swift.swob for more detail. @@ -922,8 +923,6 @@ class S3Request(swob.Request): src_resp = self.get_response(app, 'HEAD', src_bucket, swob.str_to_wsgi(src_obj), headers=headers, query=query) - # we can't let this HEAD req spoil our COPY - self.headers.pop('x-backend-storage-policy-index') if src_resp.status_int == 304: # pylint: disable-msg=E1101 raise PreconditionFailed() @@ -1370,11 +1369,11 @@ class S3Request(swob.Request): 2, 3, True) # Update s3.backend_path from the response environ self.environ['s3api.backend_path'] = sw_resp.environ['PATH_INFO'] - # Propogate backend headers back into our req headers for logging - for k, v in sw_req.headers.items(): - if k.lower().startswith('x-backend-'): - self.headers.setdefault(k, v) + # keep a record of the backend policy index so that the s3api can add + # it to the headers of whatever response it returns, which may not + # necessarily be this resp. + self.policy_index = get_policy_index(sw_req.headers, sw_resp.headers) resp = S3Response.from_swift_resp(sw_resp) status = resp.status_int # pylint: disable-msg=E1101 diff --git a/test/probe/test_mixed_policy_mpu.py b/test/probe/test_mixed_policy_mpu.py new file mode 100644 index 0000000000..b7706c3fd2 --- /dev/null +++ b/test/probe/test_mixed_policy_mpu.py @@ -0,0 +1,157 @@ +# Copyright (c) 2023 Nvidia +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import unittest +import uuid +from tempfile import mkdtemp +import os.path +import shutil +import random +from hashlib import md5 + +from swiftclient import client as swiftclient + +from test.probe.brain import BrainSplitter +from test.probe.common import ReplProbeTest, ENABLED_POLICIES + +from boto3.s3.transfer import TransferConfig + +import mock + + +class TestMixedPolicyMPU(ReplProbeTest): + + @unittest.skipIf(len(ENABLED_POLICIES) < 2, "Need more than one policy") + def setUp(self): + self.tempdir = mkdtemp() + super(TestMixedPolicyMPU, self).setUp() + s3api_info = self.cluster_info.get('s3api', {}) + if not s3api_info: + raise unittest.SkipTest('s3api not enabled') + + # lazy import boto only required if cluster supports s3api + from test.s3api import get_s3_client + self.s3 = get_s3_client(1) + + self.bucket_name = 'bucket-%s' % uuid.uuid4() + self.mpu_name = 'mpu-%s' % uuid.uuid4() + self.segment_bucket_name = self.bucket_name + '+segments' + self.bucket_brain = BrainSplitter(self.url, self.token, + self.bucket_name) + self.segments_brain = BrainSplitter(self.url, self.token, + self.segment_bucket_name) + self.other_policy = random.choice([p for p in ENABLED_POLICIES + if p != self.policy]) + + def make_large_file(self, chunksize, num_chunks): + filename = os.path.join(self.tempdir, 'big.file') + md5_hasher = md5() + slo_etag_hasher = md5() + with open(filename, 'wb') as f: + c = 'a' + for i in range(num_chunks): + c = chr(ord(c) + i) + chunk = c.encode() * chunksize + f.write(chunk) + md5_hasher.update(chunk) + chunk_etag = md5(chunk).hexdigest() + slo_etag_hasher.update(chunk_etag.encode()) + return filename, md5_hasher.hexdigest(), slo_etag_hasher.hexdigest() + + def tearDown(self): + shutil.rmtree(self.tempdir) + super(TestMixedPolicyMPU, self).tearDown() + + def _assert_container_storage_policy(self, container_name, + expected_policy): + headers = swiftclient.head_container(self.url, self.token, + container_name) + self.assertEqual(headers['x-storage-policy'], expected_policy.name) + + def test_mixed_policy_upload(self): + # Old swift had a cross policy contamination bug + # (https://bugs.launchpad.net/swift/+bug/2038459) that created + # the SLO manifest with the wrong x-backend-storage-policy-index: + # during the CompleteMultipartUpload it read the upload-id-marker from + # +segments, and applied that policy index to the manifest PUT, so the + # manifest object was stored in the wrong policy and requests for it + # would 404. + self.s3.create_bucket(Bucket=self.bucket_name) + self._assert_container_storage_policy(self.bucket_name, self.policy) + # create segments container in another policy + self.segments_brain.put_container(policy_index=int(self.other_policy)) + self._assert_container_storage_policy(self.segment_bucket_name, + self.other_policy) + # I think boto has a minimum chunksize that matches AWS, when I do this + # too small I get less chunks in the SLO than I expect + chunksize = 5 * 2 ** 20 + config = TransferConfig(multipart_threshold=chunksize, + multipart_chunksize=chunksize) + num_chunks = 3 + data_filename, md5_hash, slo_etag = self.make_large_file(chunksize, + num_chunks) + expected_size = chunksize * num_chunks + + self.s3.upload_file(data_filename, self.bucket_name, self.mpu_name, + Config=config) + # s3 mpu request succeeds + s3_head_resp = self.s3.head_object(Bucket=self.bucket_name, + Key=self.mpu_name) + self.assertEqual(expected_size, int(s3_head_resp['ContentLength'])) + self.assertEqual(num_chunks, int( + s3_head_resp['ETag'].strip('"').rsplit('-')[-1])) + # swift response is the same + swift_obj_headers, body = swiftclient.get_object( + self.url, self.token, self.bucket_name, self.mpu_name, + resp_chunk_size=65536) + self.assertEqual(expected_size, + int(swift_obj_headers['content-length'])) + self.assertEqual(slo_etag, swift_obj_headers['etag'].strip('"')) + hasher = md5() + for chunk in body: + hasher.update(chunk) + self.assertEqual(md5_hash, hasher.hexdigest()) + + # s3 listing has correct bytes + resp = self.s3.list_objects(Bucket=self.bucket_name) + # note: with PY2 the args order (expected, actual) is significant for + # mock.ANY == datetime(...) to be true + self.assertEqual([{ + u'ETag': s3_head_resp['ETag'], + u'Key': self.mpu_name, + u'LastModified': mock.ANY, + u'Size': expected_size, + u'Owner': {u'DisplayName': 'test:tester', u'ID': 'test:tester'}, + u'StorageClass': 'STANDARD', + }], resp['Contents']) + + # swift listing is the same + stat, listing = swiftclient.get_container( + self.url, self.token, self.bucket_name) + self.assertEqual(stat['x-storage-policy'], self.policy.name) + self.assertEqual(listing, [{ + 'bytes': expected_size, + 'content_type': 'application/octet-stream', + 'hash': swift_obj_headers['x-manifest-etag'], + 'last_modified': mock.ANY, + 'name': self.mpu_name, + 's3_etag': s3_head_resp['ETag'], + 'slo_etag': swift_obj_headers['etag'], + }]) + # check segments + stat, listing = swiftclient.get_container( + self.url, self.token, self.segment_bucket_name) + self.assertEqual(stat['x-storage-policy'], self.other_policy.name) + self.assertEqual([item['name'].split('/')[0] for item in listing], + [self.mpu_name] * 3) diff --git a/test/unit/common/middleware/helpers.py b/test/unit/common/middleware/helpers.py index 012af868ed..5c1dd1698b 100644 --- a/test/unit/common/middleware/helpers.py +++ b/test/unit/common/middleware/helpers.py @@ -22,6 +22,7 @@ from swift.common.header_key_dict import HeaderKeyDict from swift.common.request_helpers import is_user_meta, \ is_object_transient_sysmeta, resolve_etag_is_at_header, \ resolve_ignore_range_header +from swift.common.storage_policy import POLICIES from swift.common.swob import HTTPNotImplemented from swift.common.utils import split_path, md5 @@ -162,9 +163,11 @@ class FakeSwift(object): resp = resp_or_resps return resp - def _select_response(self, env, method, path): + def _select_response(self, env): # in some cases we can borrow different registered response # ... the order is brittle and significant + method = env['REQUEST_METHOD'] + path = self._parse_path(env)[0] preferences = [(method, path)] if env.get('QUERY_STRING'): # we can always reuse response w/o query string @@ -196,17 +199,35 @@ class FakeSwift(object): return resp_class, HeaderKeyDict(headers), body - def __call__(self, env, start_response): - method = env['REQUEST_METHOD'] - if method not in self.ALLOWED_METHODS: - raise HTTPNotImplemented() + def _get_policy_index(self, acc, cont): + path = '/v1/%s/%s' % (acc, cont) + env = {'PATH_INFO': path, + 'REQUEST_METHOD': 'HEAD'} + try: + resp_class, headers, _ = self._select_response(env) + policy_index = headers.get('X-Backend-Storage-Policy-Index') + except KeyError: + policy_index = None + if policy_index is None: + policy_index = str(int(POLICIES.default)) + return policy_index + def _parse_path(self, env): path = env['PATH_INFO'] + _, acc, cont, obj = split_path(env['PATH_INFO'], 0, 4, rest_with_last=True) if env.get('QUERY_STRING'): path += '?' + env['QUERY_STRING'] path = normalize_path(path) + return path, acc, cont, obj + + def __call__(self, env, start_response): + method = env['REQUEST_METHOD'] + if method not in self.ALLOWED_METHODS: + raise HTTPNotImplemented() + + path, acc, cont, obj = self._parse_path(env) if 'swift.authorize' in env: resp = env['swift.authorize'](swob.Request(env)) @@ -217,12 +238,7 @@ class FakeSwift(object): self.swift_sources.append(env.get('swift.source')) self.txn_ids.append(env.get('swift.trans_id')) - resp_class, headers, body = self._select_response(env, method, path) - - # Update req.headers before capturing the request - if method in ('GET', 'HEAD') and obj: - req.headers['X-Backend-Storage-Policy-Index'] = headers.get( - 'x-backend-storage-policy-index', '2') + resp_class, headers, body = self._select_response(env) # Capture the request before reading the body, in case the iter raises # an exception. @@ -272,6 +288,14 @@ class FakeSwift(object): self.req_bodies.append(req_body) + # Some middlewares (e.g. proxy_logging) inspect the request headers + # after it has been handled, so simulate some request headers updates + # that the real proxy makes. Do this *after* the request has been + # captured in the state it was received. + if obj: + req.headers.setdefault('X-Backend-Storage-Policy-Index', + self._get_policy_index(acc, cont)) + # Apply conditional etag overrides conditional_etag = resolve_etag_is_at_header(req, headers) diff --git a/test/unit/common/middleware/s3api/__init__.py b/test/unit/common/middleware/s3api/__init__.py index c7a8b6e6be..1fc614fb45 100644 --- a/test/unit/common/middleware/s3api/__init__.py +++ b/test/unit/common/middleware/s3api/__init__.py @@ -131,6 +131,17 @@ class S3ApiTestCase(unittest.TestCase): patcher.start() self.addCleanup(patcher.stop) + def _register_bucket_policy_index_head(self, bucket, bucket_policy_index): + # register bucket HEAD response with given policy index header + headers = {'X-Backend-Storage-Policy-Index': str(bucket_policy_index)} + self.swift.register('HEAD', '/v1/AUTH_test/' + bucket, + swob.HTTPNoContent, headers, None) + + def _assert_policy_index(self, req_headers, resp_headers, policy_index): + self.assertNotIn('X-Backend-Storage-Policy-Index', req_headers) + self.assertEqual(resp_headers.get('X-Backend-Storage-Policy-Index'), + str(policy_index)) + def _get_error_code(self, body): elem = fromstring(body, 'Error') return elem.find('./Code').text diff --git a/test/unit/common/middleware/s3api/test_bucket.py b/test/unit/common/middleware/s3api/test_bucket.py index 8fdc808094..6672639158 100644 --- a/test/unit/common/middleware/s3api/test_bucket.py +++ b/test/unit/common/middleware/s3api/test_bucket.py @@ -20,6 +20,7 @@ import six from six.moves.urllib.parse import quote, parse_qsl from swift.common import swob +from swift.common.middleware.proxy_logging import ProxyLoggingMiddleware from swift.common.middleware.versioned_writes.object_versioning import \ DELETE_MARKER_CONTENT_TYPE from swift.common.swob import Request @@ -101,7 +102,8 @@ class TestS3ApiBucket(S3ApiTestCase): 'GET', '/v1/AUTH_test/bucket+segments?format=json&marker=', swob.HTTPOk, {'Content-Type': 'application/json'}, listing_body) self.swift.register( - 'HEAD', '/v1/AUTH_test/junk', swob.HTTPNoContent, {}, None) + 'HEAD', '/v1/AUTH_test/junk', swob.HTTPNoContent, + {'X-Backend-Storage-Policy-Index': '3'}, None) self.swift.register( 'HEAD', '/v1/AUTH_test/nojunk', swob.HTTPNotFound, {}, None) self.swift.register( @@ -109,7 +111,8 @@ class TestS3ApiBucket(S3ApiTestCase): {}, None) self.swift.register( 'GET', '/v1/AUTH_test/junk', swob.HTTPOk, - {'Content-Type': 'application/json'}, listing_body) + {'Content-Type': 'application/json', + 'X-Backend-Storage-Policy-Index': '3'}, listing_body) self.swift.register( 'GET', '/v1/AUTH_test/junk-subdir', swob.HTTPOk, {'Content-Type': 'application/json; charset=utf-8'}, @@ -135,6 +138,28 @@ class TestS3ApiBucket(S3ApiTestCase): status, headers, body = self.call_s3api(req) self.assertEqual(status.split()[0], '200') + def _do_test_bucket_HEAD_policy_index_logging(self, bucket_policy_index): + self.logger.clear() + self._register_bucket_policy_index_head('junk', bucket_policy_index) + req = Request.blank('/junk', + environ={'REQUEST_METHOD': 'HEAD'}, + headers={'Authorization': 'AWS test:tester:hmac', + 'Date': self.get_date_header()}) + self.s3api = ProxyLoggingMiddleware(self.s3api, {}, logger=self.logger) + status, headers, body = self.call_s3api(req) + self._assert_policy_index(req.headers, headers, bucket_policy_index) + self.assertEqual('/v1/AUTH_test/junk', + req.environ['swift.backend_path']) + access_lines = self.logger.get_lines_for_level('info') + self.assertEqual(1, len(access_lines)) + parts = access_lines[0].split() + self.assertEqual(' '.join(parts[3:7]), 'HEAD /junk HTTP/1.0 200') + self.assertEqual(parts[-1], str(bucket_policy_index)) + + def test_bucket_HEAD_policy_index_logging(self): + self._do_test_bucket_HEAD_policy_index_logging(0) + self._do_test_bucket_HEAD_policy_index_logging(1) + def test_bucket_HEAD_error(self): req = Request.blank('/nojunk', environ={'REQUEST_METHOD': 'HEAD'}, @@ -210,7 +235,9 @@ class TestS3ApiBucket(S3ApiTestCase): 'Date': self.get_date_header()}) status, headers, body = self.call_s3api(req) self.assertEqual(status.split()[0], '200') - + self._assert_policy_index(req.headers, headers, 3) + self.assertEqual('/v1/AUTH_test/junk', + req.environ.get('swift.backend_path')) elem = fromstring(body, 'ListBucketResult') name = elem.find('./Name').text self.assertEqual(name, bucket_name) @@ -692,8 +719,6 @@ class TestS3ApiBucket(S3ApiTestCase): self.swift.register( 'HEAD', '/v1/AUTH_test/junk/%s' % quote(obj[0].encode('utf8')), swob.HTTPOk, {}, None) - # self.swift.register('HEAD', '/v1/AUTH_test/junk/viola', - # swob.HTTPOk, {}, None) self._add_versions_request(versioned_objects=[]) req = Request.blank('/junk?versions', diff --git a/test/unit/common/middleware/s3api/test_multi_upload.py b/test/unit/common/middleware/s3api/test_multi_upload.py index d7cb26d592..8337d7cf46 100644 --- a/test/unit/common/middleware/s3api/test_multi_upload.py +++ b/test/unit/common/middleware/s3api/test_multi_upload.py @@ -36,7 +36,7 @@ from test.unit.common.middleware.s3api.test_s3_acl import s3acl from swift.common.middleware.s3api.utils import sysmeta_header, mktime, \ S3Timestamp from swift.common.middleware.s3api.s3request import MAX_32BIT_INT -from swift.common.storage_policy import StoragePolicy +from swift.common.storage_policy import StoragePolicy, POLICIES from swift.proxy.controllers.base import get_cache_key XML = '' \ @@ -150,7 +150,7 @@ class TestS3ApiMultiUpload(S3ApiTestCase): swob.HTTPNoContent, {}, None) @s3acl - def test_bucket_upload_part(self): + def test_bucket_upload_part_missing_key(self): req = Request.blank('/bucket?partNumber=1&uploadId=x', environ={'REQUEST_METHOD': 'PUT'}, headers={'Authorization': 'AWS test:tester:hmac', @@ -158,8 +158,13 @@ class TestS3ApiMultiUpload(S3ApiTestCase): status, headers, body = self.call_s3api(req) self.assertEqual(self._get_error_code(body), 'InvalidRequest') self.assertEqual([], self.swift.calls) + self.assertNotIn('X-Backend-Storage-Policy-Index', headers) - def test_bucket_upload_part_success(self): + def _do_test_bucket_upload_part_success(self, bucket_policy_index, + segment_bucket_policy_index): + self._register_bucket_policy_index_head('bucket', bucket_policy_index) + self._register_bucket_policy_index_head('bucket+segments', + segment_bucket_policy_index) req = Request.blank('/bucket/object?partNumber=1&uploadId=X', method='PUT', headers={'Authorization': 'AWS test:tester:hmac', @@ -173,6 +178,16 @@ class TestS3ApiMultiUpload(S3ApiTestCase): ('HEAD', '/v1/AUTH_test/bucket+segments/object/X'), ('PUT', '/v1/AUTH_test/bucket+segments/object/X/1'), ], self.swift.calls) + self.assertEqual(req.environ.get('swift.backend_path'), + '/v1/AUTH_test/bucket+segments/object/X/1') + self._assert_policy_index(req.headers, headers, + segment_bucket_policy_index) + + def test_bucket_upload_part_success(self): + self._do_test_bucket_upload_part_success(0, 0) + + def test_bucket_upload_part_success_mixed_policy(self): + self._do_test_bucket_upload_part_success(0, 1) def test_bucket_upload_part_v4_bad_hash(self): authz_header = 'AWS4-HMAC-SHA256 ' + ', '.join([ @@ -729,11 +744,17 @@ class TestS3ApiMultiUpload(S3ApiTestCase): @patch('swift.common.middleware.s3api.controllers.' 'multi_upload.unique_id', lambda: 'X') - def _test_object_multipart_upload_initiate(self, headers, cache=None, - bucket_exists=True, - expected_policy=None, - expected_read_acl=None, - expected_write_acl=None): + def _test_object_multipart_upload_initiate( + self, headers, cache=None, bucket_exists=True, + bucket_policy_index=int(POLICIES.default), + segment_bucket_policy_index=None, + expected_read_acl=None, + expected_write_acl=None): + if segment_bucket_policy_index is None: + segment_bucket_policy_index = bucket_policy_index + self._register_bucket_policy_index_head('bucket', bucket_policy_index) + self._register_bucket_policy_index_head('bucket+segments', + segment_bucket_policy_index) headers.update({ 'Authorization': 'AWS test:tester:hmac', 'Date': self.get_date_header(), @@ -748,6 +769,11 @@ class TestS3ApiMultiUpload(S3ApiTestCase): fromstring(body, 'InitiateMultipartUploadResult') self.assertEqual(status.split()[0], '200') + self.assertEqual(req.environ['swift.backend_path'], + '/v1/AUTH_test/bucket+segments/object/X') + self._assert_policy_index(req.headers, headers, + segment_bucket_policy_index) + _, _, req_headers = self.swift.calls_with_headers[-1] self.assertEqual(req_headers.get('X-Object-Meta-Foo'), 'bar') self.assertEqual(req_headers.get('Content-Encoding'), 'gzip') @@ -762,10 +788,10 @@ class TestS3ApiMultiUpload(S3ApiTestCase): ('PUT', '/v1/AUTH_test/bucket+segments'), ('PUT', '/v1/AUTH_test/bucket+segments/object/X'), ], self.swift.calls) - if expected_policy: - _, _, req_headers = self.swift.calls_with_headers[-2] - self.assertEqual(req_headers.get('X-Storage-Policy'), - expected_policy) + expected_policy = POLICIES[bucket_policy_index].name + _, _, req_headers = self.swift.calls_with_headers[-2] + self.assertEqual(req_headers.get('X-Storage-Policy'), + expected_policy) if expected_read_acl: _, _, req_headers = self.swift.calls_with_headers[-2] @@ -795,6 +821,16 @@ class TestS3ApiMultiUpload(S3ApiTestCase): 'Content-MD5': base64.b64encode(b'blahblahblahblah').strip()}, fake_memcache) + def test_object_mpu_initiate_with_segment_bucket_mixed_policy(self): + fake_memcache = FakeMemcache() + fake_memcache.store[get_cache_key( + 'AUTH_test', 'bucket+segments')] = {'status': 204} + fake_memcache.store[get_cache_key( + 'AUTH_test', 'bucket')] = {'status': 204} + self._test_object_multipart_upload_initiate( + {}, fake_memcache, bucket_policy_index=0, + segment_bucket_policy_index=1) + def test_object_multipart_upload_initiate_without_segment_bucket(self): self.swift.register('PUT', '/v1/AUTH_test/bucket+segments', swob.HTTPCreated, {}, None) @@ -829,16 +865,16 @@ class TestS3ApiMultiUpload(S3ApiTestCase): self.s3api.conf.derived_container_policy_use_default = False self._test_object_multipart_upload_initiate({}, fake_memcache, bucket_exists=False, - expected_policy='silver') + bucket_policy_index=1) self._test_object_multipart_upload_initiate({'Etag': 'blahblahblah'}, fake_memcache, bucket_exists=False, - expected_policy='silver') + bucket_policy_index=1) self._test_object_multipart_upload_initiate( {'Content-MD5': base64.b64encode(b'blahblahblahblah').strip()}, fake_memcache, bucket_exists=False, - expected_policy='silver') + bucket_policy_index=1) def test_object_mpu_initiate_without_segment_bucket_same_acls(self): self.swift.register('PUT', '/v1/AUTH_test/bucket+segments', @@ -892,15 +928,23 @@ class TestS3ApiMultiUpload(S3ApiTestCase): @patch('swift.common.middleware.s3api.controllers.multi_upload.' 'unique_id', lambda: 'X') def _test_object_multipart_upload_initiate_s3acl( - self, cache, existance_cached, should_head, should_put): + self, cache, existance_cached, should_head, should_put, + bucket_policy_index=int(POLICIES.default), + segment_bucket_policy_index=None): + if segment_bucket_policy_index is None: + segment_bucket_policy_index = bucket_policy_index # mostly inlining stuff from @s3acl(s3_acl_only=True) self.s3api.conf.s3_acl = True self.swift.s3_acl = True container_headers = encode_acl('container', ACL( Owner('test:tester', 'test:tester'), [Grant(User('test:tester'), 'FULL_CONTROL')])) + container_headers['X-Backend-Storage-Policy-Index'] = \ + bucket_policy_index self.swift.register('HEAD', '/v1/AUTH_test/bucket', swob.HTTPNoContent, container_headers, None) + self._register_bucket_policy_index_head('bucket+segments', + segment_bucket_policy_index) cache.store[get_cache_key('AUTH_test')] = {'status': 204} req = Request.blank('/bucket/object?uploads', @@ -915,6 +959,10 @@ class TestS3ApiMultiUpload(S3ApiTestCase): status, headers, body = self.call_s3api(req) fromstring(body, 'InitiateMultipartUploadResult') self.assertEqual(status.split()[0], '200') + self.assertEqual(req.environ['swift.backend_path'], + '/v1/AUTH_test/bucket+segments/object/X') + self._assert_policy_index(req.headers, headers, + segment_bucket_policy_index) # This is the get_container_info existance check :'( expected = [] if not existance_cached: @@ -942,9 +990,7 @@ class TestS3ApiMultiUpload(S3ApiTestCase): self.assertEqual(acl_header.get(sysmeta_header('object', 'acl')), tmpacl_header) - def test_object_multipart_upload_initiate_s3acl_with_segment_bucket(self): - self.swift.register('HEAD', '/v1/AUTH_test/bucket+segments', - swob.HTTPNoContent, {}, None) + def test_object_mpu_initiate_s3acl_with_segment_bucket(self): kwargs = { 'existance_cached': False, 'should_head': True, @@ -953,6 +999,16 @@ class TestS3ApiMultiUpload(S3ApiTestCase): self._test_object_multipart_upload_initiate_s3acl( FakeMemcache(), **kwargs) + def test_object_mpu_initiate_s3acl_with_segment_bucket_mixed_policy(self): + kwargs = { + 'existance_cached': False, + 'should_head': True, + 'should_put': False, + } + self._test_object_multipart_upload_initiate_s3acl( + FakeMemcache(), bucket_policy_index=0, + segment_bucket_policy_index=1, **kwargs) + def test_object_multipart_upload_initiate_s3acl_with_cached_seg_buck(self): fake_memcache = FakeMemcache() fake_memcache.store.update({ @@ -967,7 +1023,7 @@ class TestS3ApiMultiUpload(S3ApiTestCase): self._test_object_multipart_upload_initiate_s3acl( fake_memcache, **kwargs) - def test_object_multipart_upload_initiate_s3acl_without_segment_bucket( + def test_object_mpu_initiate_s3acl_without_segment_bucket( self): fake_memcache = FakeMemcache() fake_memcache.store.update({ @@ -984,6 +1040,24 @@ class TestS3ApiMultiUpload(S3ApiTestCase): self._test_object_multipart_upload_initiate_s3acl( fake_memcache, **kwargs) + def test_object_mpu_initiate_s3acl_without_segment_bucket_mixed_policy( + self): + fake_memcache = FakeMemcache() + fake_memcache.store.update({ + get_cache_key('AUTH_test', 'bucket'): {'status': 204}, + get_cache_key('AUTH_test', 'bucket+segments'): {'status': 404}, + }) + self.swift.register('PUT', '/v1/AUTH_test/bucket+segments', + swob.HTTPCreated, {}, None) + kwargs = { + 'existance_cached': True, + 'should_head': False, + 'should_put': True, + } + self._test_object_multipart_upload_initiate_s3acl( + fake_memcache, bucket_policy_index=0, + segment_bucket_policy_index=0, **kwargs) + @s3acl(s3acl_only=True) @patch('swift.common.middleware.s3api.controllers.' 'multi_upload.unique_id', lambda: 'X') @@ -1050,7 +1124,9 @@ class TestS3ApiMultiUpload(S3ApiTestCase): status, headers, body = self.call_s3api(req) self.assertEqual(self._get_error_code(body), 'NoSuchBucket') - def _do_test_object_multipart_upload_complete(self): + def _do_test_object_multipart_upload_complete( + self, bucket_policy_index=int(POLICIES.default), + segment_bucket_policy_index=None): content_md5 = base64.b64encode(md5( XML.encode('ascii'), usedforsecurity=False).digest()) req = Request.blank('/bucket/object?uploadId=X', @@ -1059,6 +1135,11 @@ class TestS3ApiMultiUpload(S3ApiTestCase): 'Date': self.get_date_header(), 'Content-MD5': content_md5, }, body=XML) + if segment_bucket_policy_index is None: + segment_bucket_policy_index = bucket_policy_index + self._register_bucket_policy_index_head('bucket', bucket_policy_index) + self._register_bucket_policy_index_head('bucket+segments', + segment_bucket_policy_index) status, headers, body = self.call_s3api(req) elem = fromstring(body, 'CompleteMultipartUploadResult') self.assertNotIn('Etag', headers) @@ -1079,6 +1160,8 @@ class TestS3ApiMultiUpload(S3ApiTestCase): ]) self.assertEqual(req.environ['swift.backend_path'], '/v1/AUTH_test/bucket+segments/object/X') + self._assert_policy_index(req.headers, headers, + segment_bucket_policy_index) _, _, headers = self.swift.calls_with_headers[-2] self.assertEqual(headers.get('X-Object-Meta-Foo'), 'bar') @@ -1089,9 +1172,19 @@ class TestS3ApiMultiUpload(S3ApiTestCase): self.assertEqual(headers.get(h), override_etag) self.assertEqual(headers.get('X-Object-Sysmeta-S3Api-Upload-Id'), 'X') + # s3api doesn't set storage policy index on backend requests + spi = [hdrs.get('X-Backend-Storage-Policy-Index') + for _, _, hdrs in self.swift.calls_with_headers] + self.assertEqual(spi, [None] * 5) + def test_object_multipart_upload_complete(self): self._do_test_object_multipart_upload_complete() + def test_object_multipart_upload_complete_mixed_policy(self): + self._do_test_object_multipart_upload_complete( + bucket_policy_index=0, segment_bucket_policy_index=1 + ) + def test_object_multipart_upload_complete_other_headers(self): headers = {'x-object-meta-foo': 'bar', 'content-type': 'application/directory', @@ -1160,7 +1253,14 @@ class TestS3ApiMultiUpload(S3ApiTestCase): "etag": "fedcba9876543210fedcba9876543210"}, ]) - def test_object_multipart_upload_retry_complete(self): + def _do_test_object_multipart_upload_retry_complete( + self, bucket_policy_index=int(POLICIES.default), + segment_bucket_policy_index=None): + if segment_bucket_policy_index is None: + segment_bucket_policy_index = bucket_policy_index + self._register_bucket_policy_index_head('bucket', bucket_policy_index) + self._register_bucket_policy_index_head('bucket+segments', + segment_bucket_policy_index) content_md5 = base64.b64encode(md5( XML.encode('ascii'), usedforsecurity=False).digest()) self.swift.register('HEAD', '/v1/AUTH_test/bucket+segments/object/X', @@ -1197,6 +1297,15 @@ class TestS3ApiMultiUpload(S3ApiTestCase): ]) self.assertEqual(req.environ['swift.backend_path'], '/v1/AUTH_test/bucket+segments/object/X') + self._assert_policy_index(req.headers, headers, + segment_bucket_policy_index) + + def test_object_multipart_upload_retry_complete(self): + self._do_test_object_multipart_upload_retry_complete() + + def test_object_multipart_upload_retry_complete_mixed_policy(self): + self._do_test_object_multipart_upload_retry_complete( + bucket_policy_index=0, segment_bucket_policy_index=1) def test_object_multipart_upload_retry_complete_etag_mismatch(self): content_md5 = base64.b64encode(md5( diff --git a/test/unit/common/middleware/s3api/test_obj.py b/test/unit/common/middleware/s3api/test_obj.py index 22c4bfb1c9..6f0c64a24a 100644 --- a/test/unit/common/middleware/s3api/test_obj.py +++ b/test/unit/common/middleware/s3api/test_obj.py @@ -26,9 +26,10 @@ import six import json from swift.common import swob +from swift.common.storage_policy import StoragePolicy from swift.common.swob import Request from swift.common.middleware.proxy_logging import ProxyLoggingMiddleware -from test.unit import mock_timestamp_now +from test.unit import mock_timestamp_now, patch_policies from test.unit.common.middleware.s3api import S3ApiTestCase from test.unit.common.middleware.s3api.test_s3_acl import s3acl @@ -77,6 +78,8 @@ class TestS3ApiObj(S3ApiTestCase): None) def _test_object_GETorHEAD(self, method): + bucket_policy_index = 1 + self._register_bucket_policy_index_head('bucket', bucket_policy_index) req = Request.blank('/bucket/object', environ={'REQUEST_METHOD': method}, headers={'Authorization': 'AWS test:tester:hmac', @@ -84,7 +87,7 @@ class TestS3ApiObj(S3ApiTestCase): status, headers, body = self.call_s3api(req) self.assertEqual(status.split()[0], '200') # we'll want this for logging - self.assertEqual(req.headers['X-Backend-Storage-Policy-Index'], '2') + self._assert_policy_index(req.headers, headers, bucket_policy_index) unexpected_headers = [] for key, val in self.response_headers.items(): @@ -182,18 +185,30 @@ class TestS3ApiObj(S3ApiTestCase): def test_object_HEAD(self): self._test_object_GETorHEAD('HEAD') - def test_object_policy_index_logging(self): + def _do_test_object_policy_index_logging(self, bucket_policy_index): + self.logger.clear() + self._register_bucket_policy_index_head('bucket', bucket_policy_index) req = Request.blank('/bucket/object', headers={'Authorization': 'AWS test:tester:hmac', 'Date': self.get_date_header()}) self.s3api = ProxyLoggingMiddleware(self.s3api, {}, logger=self.logger) status, headers, body = self.call_s3api(req) + self._assert_policy_index(req.headers, headers, bucket_policy_index) + self.assertEqual('/v1/AUTH_test/bucket/object', + req.environ['swift.backend_path']) access_lines = self.logger.get_lines_for_level('info') self.assertEqual(1, len(access_lines)) parts = access_lines[0].split() self.assertEqual(' '.join(parts[3:7]), 'GET /bucket/object HTTP/1.0 200') - self.assertEqual(parts[-1], '2') + self.assertEqual(parts[-1], str(bucket_policy_index)) + + @patch_policies([ + StoragePolicy(0, 'gold', is_default=True), + StoragePolicy(1, 'silver')]) + def test_object_policy_index_logging(self): + self._do_test_object_policy_index_logging(0) + self._do_test_object_policy_index_logging(1) def _test_object_HEAD_Range(self, range_value): req = Request.blank('/bucket/object', @@ -847,11 +862,16 @@ class TestS3ApiObj(S3ApiTestCase): return_value=timestamp): return self.call_s3api(req) + @patch_policies([ + StoragePolicy(0, 'gold', is_default=True), + StoragePolicy(1, 'silver')]) def test_simple_object_copy(self): + src_policy_index = 0 + self._register_bucket_policy_index_head('some', src_policy_index) + dst_policy_index = 1 + self._register_bucket_policy_index_head('bucket', dst_policy_index) self.swift.register('HEAD', '/v1/AUTH_test/some/source', - swob.HTTPOk, { - 'x-backend-storage-policy-index': '1', - }, None) + swob.HTTPOk, {}, None) req = Request.blank( '/bucket/object', method='PUT', headers={ @@ -865,11 +885,14 @@ class TestS3ApiObj(S3ApiTestCase): return_value=timestamp): status, headers, body = self.call_s3api(req) self.assertEqual(status.split()[0], '200') + self._assert_policy_index(req.headers, headers, dst_policy_index) + self.assertEqual('/v1/AUTH_test/bucket/object', + req.environ['swift.backend_path']) + head_call, put_call = self.swift.calls_with_headers - self.assertEqual( - head_call.headers['x-backend-storage-policy-index'], '1') - self.assertEqual(put_call.headers['x-copy-from'], '/some/source') + self.assertNotIn('x-backend-storage-policy-index', head_call.headers) self.assertNotIn('x-backend-storage-policy-index', put_call.headers) + self.assertEqual(put_call.headers['x-copy-from'], '/some/source') @s3acl def test_object_PUT_copy(self): diff --git a/test/unit/common/middleware/test_helpers.py b/test/unit/common/middleware/test_helpers.py index 6def1edcf2..64c115b577 100644 --- a/test/unit/common/middleware/test_helpers.py +++ b/test/unit/common/middleware/test_helpers.py @@ -15,6 +15,7 @@ # limitations under the License. import unittest +from swift.common.storage_policy import POLICIES from swift.common.swob import Request, HTTPOk, HTTPNotFound, HTTPCreated from swift.common import request_helpers as rh from test.unit.common.middleware.helpers import FakeSwift @@ -509,6 +510,47 @@ class TestFakeSwift(unittest.TestCase): self.assertEqual('bytes=0-2', swift.calls_with_headers[-1].headers.get('Range')) + def test_object_GET_updated_with_storage_policy(self): + swift = FakeSwift() + swift.register('GET', '/v1/a/c/o', HTTPOk, {}, body=b'stuff') + req = Request.blank('/v1/a/c/o') + req.method = 'GET' + self.assertNotIn('X-Backend-Storage-Policy-Index', req.headers) + resp = req.get_response(swift) + self.assertEqual(200, resp.status_int) + self.assertEqual({'Content-Length': '5', + 'Content-Type': 'text/html; charset=UTF-8'}, + resp.headers) + self.assertEqual(b'stuff', resp.body) + self.assertEqual(1, swift.call_count) + self.assertEqual(('GET', '/v1/a/c/o'), swift.calls[0]) + self.assertEqual(('GET', '/v1/a/c/o', + {'Host': 'localhost:80'}), # from swob + swift.calls_with_headers[0]) + # default storage policy is applied... + self.assertEqual(str(int(POLICIES.default)), + req.headers.get('X-Backend-Storage-Policy-Index')) + + # register a container with storage policy 99... + swift.register('HEAD', '/v1/a/c', HTTPOk, + {'X-Backend-Storage-Policy-Index': '99'}, None) + req = Request.blank('/v1/a/c/o') + req.method = 'GET' + self.assertNotIn('X-Backend-Storage-Policy-Index', req.headers) + resp = req.get_response(swift) + self.assertEqual(200, resp.status_int) + self.assertEqual({'Content-Length': '5', + 'Content-Type': 'text/html; charset=UTF-8'}, + resp.headers) + self.assertEqual(b'stuff', resp.body) + self.assertEqual(2, swift.call_count) + self.assertEqual(('GET', '/v1/a/c/o'), swift.calls[1]) + self.assertEqual(('GET', '/v1/a/c/o', + {'Host': 'localhost:80'}), # from swob + swift.calls_with_headers[1]) + self.assertEqual( + '99', req.headers.get('X-Backend-Storage-Policy-Index')) + class TestFakeSwiftMultipleResponses(unittest.TestCase): diff --git a/test/unit/common/middleware/test_slo.py b/test/unit/common/middleware/test_slo.py index 49c6f32e87..0c9b78cae9 100644 --- a/test/unit/common/middleware/test_slo.py +++ b/test/unit/common/middleware/test_slo.py @@ -1365,13 +1365,11 @@ class TestSloDeleteManifest(SloTestCase): 'Range': 'bytes=-1', 'X-Backend-Ignore-Range-If-Metadata-Present': 'X-Static-Large-Object', - 'X-Backend-Storage-Policy-Index': '2', 'Content-Length': '0'}), ('GET', '/v1/AUTH_test/deltest/man-all-there?multipart-manifest=get', {'Host': 'localhost:80', 'User-Agent': 'Mozzarella Foxfire MultipartDELETE', - 'X-Backend-Storage-Policy-Index': '2', 'Content-Length': '0'}), ]) self.assertEqual(set(self.app.calls), set([ diff --git a/test/unit/common/middleware/test_symlink.py b/test/unit/common/middleware/test_symlink.py index c8064d8357..35e967c377 100644 --- a/test/unit/common/middleware/test_symlink.py +++ b/test/unit/common/middleware/test_symlink.py @@ -455,7 +455,6 @@ class TestSymlinkMiddleware(TestSymlinkMiddlewareBase): 'Host': 'localhost:80', 'X-Backend-Ignore-Range-If-Metadata-Present': 'x-object-sysmeta-symlink-target', - 'X-Backend-Storage-Policy-Index': '2', }) self.assertEqual(req_headers, calls[0].headers) req_headers['User-Agent'] = 'Swift' @@ -631,7 +630,6 @@ class TestSymlinkMiddleware(TestSymlinkMiddlewareBase): 'Host': 'localhost:80', 'X-Backend-Ignore-Range-If-Metadata-Present': 'x-object-sysmeta-symlink-target', - 'X-Backend-Storage-Policy-Index': '2', }) self.assertEqual(req_headers, calls[0].headers) req_headers['User-Agent'] = 'Swift' diff --git a/test/unit/common/test_internal_client.py b/test/unit/common/test_internal_client.py index 65bbed9a9b..e5f123b00d 100644 --- a/test/unit/common/test_internal_client.py +++ b/test/unit/common/test_internal_client.py @@ -1602,9 +1602,8 @@ class TestInternalClient(unittest.TestCase): self.assertEqual(app.call_count, 1) req_headers.update({ 'host': 'localhost:80', # from swob.Request.blank + 'user-agent': 'test', # from IC 'x-backend-allow-reserved-names': 'true', # also from IC - 'x-backend-storage-policy-index': '2', # from proxy-server app - 'user-agent': 'test', }) self.assertEqual(app.calls_with_headers, [( 'GET', path_info + '?symlink=get', HeaderKeyDict(req_headers))]) diff --git a/tools/playbooks/common/install_dependencies.yaml b/tools/playbooks/common/install_dependencies.yaml index 808487fe78..625dce68c6 100644 --- a/tools/playbooks/common/install_dependencies.yaml +++ b/tools/playbooks/common/install_dependencies.yaml @@ -48,6 +48,7 @@ - pytest - pytest-cov - python-swiftclient + - 'boto3>=1.9' - name: install PasteDeploy - CentOS 7 pip: name={{ item }} state=present extra_args='--upgrade'