From 29414ab1466610481dc9bf0b107e627141e4dbac Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Wed, 1 Mar 2023 19:25:03 +0000 Subject: [PATCH] Allow internal container POSTs to not update put_timestamp There may be circumstances when an internal client wishes to modify container sysmeta that is hidden from the user. It is desirable that this happens without modifying the put-timestamp and therefore the last-modified time that is reported in responses to client HEADs and GETs. This patch modifies the container server so that a POST will not update the container put_timestamp if an X-Backend-No-Timestamp-Update header is included with the request and has a truthy value. Note: there are already circumstances in which container sysmeta is modified without changing the put_timestamp: - PUT requests with shard range content do not update put_timestamp. - the sharder updates sysmeta directly via the ContainerBroker without modifying put_timestamp. Change-Id: I835b2dd58bc1d4fb911629e4da2ea4b9697dd21b --- swift/common/direct_client.py | 25 ++++++ swift/container/server.py | 13 ++- test/probe/test_container_failures.py | 73 +++++++++++++++- test/unit/common/test_direct_client.py | 16 ++++ test/unit/container/test_server.py | 115 +++++++++++++++++++++++++ 5 files changed, 239 insertions(+), 3 deletions(-) diff --git a/swift/common/direct_client.py b/swift/common/direct_client.py index f6234fa385..deb9fbe9ee 100644 --- a/swift/common/direct_client.py +++ b/swift/common/direct_client.py @@ -378,6 +378,31 @@ def direct_put_container(node, part, account, container, conn_timeout=5, content_length=content_length, chunk_size=chunk_size) +def direct_post_container(node, part, account, container, conn_timeout=5, + response_timeout=15, headers=None): + """ + Make a POST request to a container server. + + :param node: node dictionary from the ring + :param part: partition the container is on + :param account: account name + :param container: container name + :param conn_timeout: timeout in seconds for establishing the connection + :param response_timeout: timeout in seconds for getting the response + :param headers: additional headers to include in the request + :raises ClientException: HTTP PUT request failed + """ + if headers is None: + headers = {} + + lower_headers = set(k.lower() for k in headers) + headers_out = gen_headers(headers, + add_ts='x-timestamp' not in lower_headers) + path = _make_path(account, container) + return _make_req(node, part, 'POST', path, headers_out, 'Container', + conn_timeout, response_timeout) + + def direct_put_container_object(node, part, account, container, obj, conn_timeout=5, response_timeout=15, headers=None): diff --git a/swift/container/server.py b/swift/container/server.py index 8afc047501..080716d68f 100644 --- a/swift/container/server.py +++ b/swift/container/server.py @@ -860,7 +860,14 @@ class ContainerController(BaseStorageServer): @public @timing_stats() def POST(self, req): - """Handle HTTP POST request.""" + """ + Handle HTTP POST request. + + A POST request will update the container's ``put_timestamp``, unless + it has an ``X-Backend-No-Timestamp-Update`` header with a truthy value. + + :param req: an instance of :class:`~swift.common.swob.Request`. + """ drive, part, account, container = get_container_name_and_placement(req) req_timestamp = valid_timestamp(req) if 'x-container-sync-to' in req.headers: @@ -878,7 +885,9 @@ class ContainerController(BaseStorageServer): broker = self._get_container_broker(drive, part, account, container) if broker.is_deleted(): return HTTPNotFound(request=req) - broker.update_put_timestamp(req_timestamp.internal) + if not config_true_value( + req.headers.get('x-backend-no-timestamp-update', False)): + broker.update_put_timestamp(req_timestamp.internal) self._update_metadata(req, broker, req_timestamp, 'POST') return HTTPNoContent(request=req) diff --git a/test/probe/test_container_failures.py b/test/probe/test_container_failures.py index 6a3beb1042..b965a90d4a 100644 --- a/test/probe/test_container_failures.py +++ b/test/probe/test_container_failures.py @@ -13,13 +13,15 @@ # implied. # See the License for the specific language governing permissions and # limitations under the License. - +import time from unittest import main from uuid import uuid4 from eventlet import GreenPool, Timeout import eventlet from sqlite3 import connect + +from swift.common.manager import Manager from swiftclient import client from swift.common import direct_client @@ -71,6 +73,75 @@ class TestContainerFailures(ReplProbeTest): self.assertEqual(headers['x-account-object-count'], '1') self.assertEqual(headers['x-account-bytes-used'], '3') + def test_metadata_replicated_with_no_timestamp_update(self): + self.maxDiff = None + # Create container1 + container1 = 'container-%s' % uuid4() + cpart, cnodes = self.container_ring.get_nodes(self.account, container1) + client.put_container(self.url, self.token, container1) + Manager(['container-replicator']).once() + + exp_hdrs = None + for cnode in cnodes: + hdrs = direct_client.direct_head_container( + cnode, cpart, self.account, container1) + hdrs.pop('Date') + if exp_hdrs: + self.assertEqual(exp_hdrs, hdrs) + exp_hdrs = hdrs + self.assertIsNotNone(exp_hdrs) + self.assertIn('Last-Modified', exp_hdrs) + put_time = float(exp_hdrs['X-Backend-Put-Timestamp']) + + # Post to only one replica of container1 at least 1 second after the + # put (to reveal any unexpected change in Last-Modified which is + # rounded to seconds) + time.sleep(put_time + 1 - time.time()) + post_hdrs = {'x-container-meta-foo': 'bar', + 'x-backend-no-timestamp-update': 'true'} + direct_client.direct_post_container( + cnodes[1], cpart, self.account, container1, headers=post_hdrs) + + # verify that put_timestamp was not modified + exp_hdrs.update({'x-container-meta-foo': 'bar'}) + hdrs = direct_client.direct_head_container( + cnodes[1], cpart, self.account, container1) + hdrs.pop('Date') + self.assertDictEqual(exp_hdrs, hdrs) + + # Get to a final state + Manager(['container-replicator']).once() + + # Assert all container1 servers have consistent metadata + for cnode in cnodes: + hdrs = direct_client.direct_head_container( + cnode, cpart, self.account, container1) + hdrs.pop('Date') + self.assertDictEqual(exp_hdrs, hdrs) + + # sanity check: verify the put_timestamp is modified without + # x-backend-no-timestamp-update + post_hdrs = {'x-container-meta-foo': 'baz'} + exp_hdrs.update({'x-container-meta-foo': 'baz'}) + direct_client.direct_post_container( + cnodes[1], cpart, self.account, container1, headers=post_hdrs) + + # verify that put_timestamp was modified + hdrs = direct_client.direct_head_container( + cnodes[1], cpart, self.account, container1) + self.assertLess(exp_hdrs['x-backend-put-timestamp'], + hdrs['x-backend-put-timestamp']) + self.assertNotEqual(exp_hdrs['last-modified'], hdrs['last-modified']) + hdrs.pop('Date') + for key in ('x-backend-put-timestamp', + 'x-put-timestamp', + 'last-modified'): + self.assertNotEqual(exp_hdrs[key], hdrs[key]) + exp_hdrs.pop(key) + hdrs.pop(key) + + self.assertDictEqual(exp_hdrs, hdrs) + def test_two_nodes_fail(self): # Create container1 container1 = 'container-%s' % uuid4() diff --git a/test/unit/common/test_direct_client.py b/test/unit/common/test_direct_client.py index cfc186d8de..b1d02c6102 100644 --- a/test/unit/common/test_direct_client.py +++ b/test/unit/common/test_direct_client.py @@ -620,6 +620,22 @@ class TestDirectClient(unittest.TestCase): self.assertEqual(raised.exception.http_status, 500) self.assertTrue('PUT' in str(raised.exception)) + def test_direct_post_container(self): + headers = {'x-foo': 'bar', 'User-Agent': 'my UA'} + + with mocked_http_conn(204) as conn: + resp = direct_client.direct_post_container( + self.node, self.part, self.account, self.container, + headers=headers) + self.assertEqual(conn.host, self.node['ip']) + self.assertEqual(conn.port, self.node['port']) + self.assertEqual(conn.method, 'POST') + self.assertEqual(conn.path, self.container_path) + self.assertEqual(conn.req_headers['User-Agent'], 'my UA') + self.assertTrue('x-timestamp' in conn.req_headers) + self.assertEqual('bar', conn.req_headers.get('x-foo')) + self.assertEqual(204, resp.status) + def test_direct_delete_container_object(self): with mocked_http_conn(204) as conn: rv = direct_client.direct_delete_container_object( diff --git a/test/unit/container/test_server.py b/test/unit/container/test_server.py index aca6d38b69..c09a8d9970 100644 --- a/test/unit/container/test_server.py +++ b/test/unit/container/test_server.py @@ -435,6 +435,69 @@ class TestContainerController(unittest.TestCase): resp = req.get_response(self.controller) self.assertEqual(resp.status_int, 202) + def test_PUT_HEAD_put_timestamp_updates(self): + put_ts = Timestamp(1) + req = Request.blank('/sda1/p/a/c', environ={'REQUEST_METHOD': 'PUT'}, + headers={'X-Timestamp': put_ts.internal}) + resp = req.get_response(self.controller) + self.assertEqual(resp.status_int, 201) + + def do_put_head(put_ts, meta_value, extra_hdrs, body='', path='a/c'): + # Set metadata header + req = Request.blank('/sda1/p/' + path, + environ={'REQUEST_METHOD': 'PUT'}, + headers={'X-Timestamp': put_ts.internal, + 'X-Container-Meta-Test': meta_value}, + body=body) + req.headers.update(extra_hdrs) + resp = req.get_response(self.controller) + self.assertTrue(resp.is_success) + req = Request.blank('/sda1/p/a/c', + environ={'REQUEST_METHOD': 'HEAD'}) + resp = req.get_response(self.controller) + self.assertEqual(resp.status_int, 204) + return resp.headers + + # put timestamp is advanced on PUT with container path + put_ts = Timestamp(2) + resp_hdrs = do_put_head(put_ts, 'val1', + {'x-backend-no-timestamp-update': 'false'}) + self.assertEqual(resp_hdrs.get('x-container-meta-test'), 'val1') + self.assertEqual(resp_hdrs.get('x-backend-put-timestamp'), + put_ts.internal) + self.assertEqual(resp_hdrs.get('x-put-timestamp'), put_ts.internal) + + put_ts = Timestamp(3) + resp_hdrs = do_put_head(put_ts, 'val2', + {'x-backend-no-timestamp-update': 'true'}) + self.assertEqual(resp_hdrs.get('x-container-meta-test'), 'val2') + self.assertEqual(resp_hdrs.get('x-backend-put-timestamp'), + put_ts.internal) + self.assertEqual(resp_hdrs.get('x-put-timestamp'), put_ts.internal) + + # put timestamp is NOT updated if record type is shard + put_ts = Timestamp(4) + resp_hdrs = do_put_head( + put_ts, 'val3', {'x-backend-record-type': 'shard'}, + body=json.dumps([dict(ShardRange('x/y', 123.4))])) + self.assertEqual(resp_hdrs.get('x-container-meta-test'), 'val3') + self.assertEqual(resp_hdrs.get('x-backend-put-timestamp'), + Timestamp(3).internal) + self.assertEqual(resp_hdrs.get('x-put-timestamp'), + Timestamp(3).internal) + + # put timestamp and metadata are NOT updated for request with obj path + put_ts = Timestamp(5) + resp_hdrs = do_put_head( + put_ts, 'val4', + {'x-content-type': 'plain/text', 'x-size': 0, 'x-etag': 'an-etag'}, + path='a/c/o') + self.assertEqual(resp_hdrs.get('x-container-meta-test'), 'val3') + self.assertEqual(resp_hdrs.get('x-backend-put-timestamp'), + Timestamp(3).internal) + self.assertEqual(resp_hdrs.get('x-put-timestamp'), + Timestamp(3).internal) + def test_PUT_insufficient_space(self): conf = {'devices': self.testdir, 'mount_check': 'false', @@ -1058,6 +1121,58 @@ class TestContainerController(unittest.TestCase): self.assertEqual(resp.status_int, 204) self.assertNotIn(key.lower(), resp.headers) + def test_POST_HEAD_no_timestamp_update(self): + put_ts = Timestamp(1) + req = Request.blank('/sda1/p/a/c', environ={'REQUEST_METHOD': 'PUT'}, + headers={'X-Timestamp': put_ts.internal}) + resp = req.get_response(self.controller) + self.assertEqual(resp.status_int, 201) + + def do_post_head(post_ts, value, extra_hdrs): + # Set metadata header + req = Request.blank('/sda1/p/a/c', + environ={'REQUEST_METHOD': 'POST'}, + headers={'X-Timestamp': post_ts.internal, + 'X-Container-Meta-Test': value}) + req.headers.update(extra_hdrs) + resp = req.get_response(self.controller) + self.assertEqual(resp.status_int, 204) + req = Request.blank('/sda1/p/a/c', + environ={'REQUEST_METHOD': 'HEAD'}) + resp = req.get_response(self.controller) + self.assertEqual(resp.status_int, 204) + return resp.headers + + # verify timestamp IS advanced + post_ts = Timestamp(2) + resp_hdrs = do_post_head(post_ts, 'val1', {}) + self.assertEqual(resp_hdrs.get('x-container-meta-test'), 'val1') + self.assertEqual(resp_hdrs.get('x-backend-put-timestamp'), + post_ts.internal) + + post_ts = Timestamp(3) + resp_hdrs = do_post_head(post_ts, 'val2', + {'x-backend-no-timestamp-update': 'false'}) + self.assertEqual(resp_hdrs.get('x-container-meta-test'), 'val2') + self.assertEqual(resp_hdrs.get('x-backend-put-timestamp'), + post_ts.internal) + + # verify timestamp IS NOT advanced, but metadata still updated + post_ts = Timestamp(4) + resp_hdrs = do_post_head(post_ts, 'val3', + {'x-backend-No-timeStamp-update': 'true'}) + self.assertEqual(resp_hdrs.get('x-container-meta-test'), 'val3') + self.assertEqual(resp_hdrs.get('x-backend-put-timestamp'), + Timestamp(3).internal) + + # verify timestamp will not go backwards + post_ts = Timestamp(2) + resp_hdrs = do_post_head(post_ts, 'val4', + {'x-backend-no-timestamp-update': 'true'}) + self.assertEqual(resp_hdrs.get('x-container-meta-test'), 'val3') + self.assertEqual(resp_hdrs.get('x-backend-put-timestamp'), + Timestamp(3).internal) + def test_POST_invalid_partition(self): req = Request.blank('/sda1/./a/c', environ={'REQUEST_METHOD': 'POST', 'HTTP_X_TIMESTAMP': '1'})