Fix bug expirer unexpectedly deletes object created after x-delete-at

As reported at bug/1546067, expirer might accidentally deletes an object
which is created after x-delete-at timestamp. This is because expirer
sends a request with "X-Timestamp: <current_timestamp>" and tombstone
is named as <requested_x_timestamp>.ts so if object creation time is
between x-delete-at and expirer's DELETE request x-timestamp, the object
might be hidden by tombstone.

This possibility can be simply removed if the value of x-timestamp which
an expirer sends is the same timestamp as x-delete-at of an actual object.
Namely, expirer pretends to delete an object at the time an user really
wants to delete it.

Change-Id: I53e343f4e73b0b1c4ced9a3bc054541473d26cf8
Closes-Bug: #1546067

swift/obj/expirer.py

@@ -293,5 +293,6 @@ class ObjectExpirer(Daemon):
         """
         path = '/v1/' + urllib.parse.quote(actual_obj.lstrip('/'))
         self.swift.make_request('DELETE', path,
-                                {'X-If-Delete-At': str(timestamp)},
+                                {'X-If-Delete-At': str(timestamp),
+                                 'X-Timestamp': str(timestamp)},
                                 (2, HTTP_PRECONDITION_FAILED))

test/probe/test_object_expirer.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import random
+import time
 import uuid
 import unittest
 
@@ -126,5 +127,50 @@ class TestObjectExpirer(ReplProbeTest):
                 self.assertTrue(Timestamp(metadata['x-backend-timestamp']) >
                                 create_timestamp)
 
+    def test_expirer_object_should_not_be_expired(self):
+        obj_brain = BrainSplitter(self.url, self.token, self.container_name,
+                                  self.object_name, 'object', self.policy)
+
+        # T(obj_created) < T(obj_deleted with x-delete-at) < T(obj_recreated)
+        #   < T(expirer_executed)
+        # Recreated obj should be appeared in any split brain case
+
+        # T(obj_created)
+        first_created_at = time.time()
+        # T(obj_deleted with x-delete-at)
+        # object-server accepts req only if X-Delete-At is later than 'now'
+        delete_at = int(time.time() + 1.5)
+        # T(obj_recreated)
+        recreated_at = time.time() + 2.0
+        # T(expirer_executed) - 'now'
+        sleep_for_expirer = 2.01
+
+        obj_brain.put_container(int(self.policy))
+        obj_brain.put_object(
+            headers={'X-Delete-At': delete_at,
+                     'X-Timestamp': Timestamp(first_created_at).internal})
+
+        # some object servers stopped
+        obj_brain.stop_primary_half()
+        obj_brain.put_object(
+            headers={'X-Timestamp': Timestamp(recreated_at).internal,
+                     'X-Object-Meta-Expired': 'False'})
+
+        # make sure auto-created containers get in the account listing
+        Manager(['container-updater']).once()
+        # some object servers recovered
+        obj_brain.start_primary_half()
+        # sleep to make sure expirer runs at the time after obj is recreated
+        time.sleep(sleep_for_expirer)
+        self.expirer.once()
+        # inconsistent state of objects is recovered
+        Manager(['object-replicator']).once()
+
+        # check if you can get recreated object
+        metadata = self.client.get_object_metadata(
+            self.account, self.container_name, self.object_name)
+        self.assertIn('x-object-meta-expired', metadata)
+
+
 if __name__ == "__main__":
     unittest.main()

test/unit/obj/test_expirer.py

@@ -675,6 +675,8 @@ class TestObjectExpirer(TestCase):
         ts = '1234'
         x.delete_actual_object('/path/to/object', ts)
         self.assertEqual(got_env[0]['HTTP_X_IF_DELETE_AT'], ts)
+        self.assertEqual(got_env[0]['HTTP_X_TIMESTAMP'],
+                         got_env[0]['HTTP_X_IF_DELETE_AT'])
 
     def test_delete_actual_object_nourlquoting(self):
         # delete_actual_object should not do its own url quoting because
@@ -692,6 +694,8 @@ class TestObjectExpirer(TestCase):
         ts = '1234'
         x.delete_actual_object('/path/to/object name', ts)
         self.assertEqual(got_env[0]['HTTP_X_IF_DELETE_AT'], ts)
+        self.assertEqual(got_env[0]['HTTP_X_TIMESTAMP'],
+                         got_env[0]['HTTP_X_IF_DELETE_AT'])
         self.assertEqual(got_env[0]['PATH_INFO'], '/v1/path/to/object name')
 
     def test_delete_actual_object_raises_404(self):