Add checksum to object extended attributes

Currently, our integrity checking for objects is pretty weak when it
comes to object metadata. If the extended attributes on a .data or
.meta file get corrupted in such a way that we can still unpickle it,
we don't have anything that detects that.

This could be especially bad with encrypted etags; if the encrypted
etag (X-Object-Sysmeta-Crypto-Etag or whatever it is) gets some bits
flipped, then we'll cheerfully decrypt the cipherjunk into plainjunk,
then send it to the client. Net effect is that the client sees a GET
response with an ETag that doesn't match the MD5 of the object *and*
Swift has no way of detecting and quarantining this object.

Note that, with an unencrypted object, if the ETag metadatum gets
mangled, then the object will be quarantined by the object server or
auditor, whichever notices first.

As part of this commit, I also ripped out some mocking of
getxattr/setxattr in tests. It appears to be there to allow unit tests
to run on systems where /tmp doesn't support xattrs. However, since
the mock is keyed off of inode number and inode numbers get re-used,
there's lots of leakage between different test runs. On a real FS,
unlinking a file and then creating a new one of the same name will
also reset the xattrs; this isn't the case with the mock.

The mock was pretty old; Ubuntu 12.04 and up all support xattrs in
/tmp, and recent Red Hat / CentOS releases do too. The xattr mock was
added in 2011; maybe it was to support Ubuntu Lucid Lynx?

Bonus: now you can pause a test with the debugger, inspect its files
in /tmp, and actually see the xattrs along with the data.

Since this patch now uses a real filesystem for testing filesystem
operations, tests are skipped if the underlying filesystem does not
support setting xattrs (eg tmpfs or more than 4k of xattrs on ext4).

References to "/tmp" have been replaced with calls to
tempfile.gettempdir(). This will allow setting the TMPDIR envvar in
test setup and getting an XFS filesystem instead of ext4 or tmpfs.

THIS PATCH SIGNIFICANTLY CHANGES TESTING ENVIRONMENTS

With this patch, every test environment will require TMPDIR to be
using a filesystem that supports at least 4k of extended attributes.
Neither ext4 nor tempfs support this. XFS is recommended.

So why all the SkipTests? Why not simply raise an error? We still need
the tests to run on the base image for OpenStack's CI system. Since
we were previously mocking out xattr, there wasn't a problem, but we
also weren't actually testing anything. This patch adds functionality
to validate xattr data, so we need to drop the mock.

`test.unit.skip_if_no_xattrs()` is also imported into `test.functional`
so that functional tests can import it from the functional test
namespace.

The related OpenStack CI infrastructure changes are made in
https://review.openstack.org/#/c/394600/.

Co-Authored-By: John Dickinson <me@not.mn>

Change-Id: I98a37c0d451f4960b7a12f648e4405c6c6716808

README.rst

@@ -82,6 +82,12 @@ You can run unit tests with ``.unittests``, functional tests with
 ``.functests``, and probe tests with ``.probetests``. There is an
 additional ``.alltests`` script that wraps the other three.
 
+To fully run the tests, the target environment must use a filesystem that
+supports large xattrs. XFS is strongly recommended. For unit tests and in-
+process functional tests, either mount ``/tmp`` with XFS or provide another
+XFS filesystem via the ``TMPDIR`` environment variable. Without this setting,
+tests should still pass, but a very large number will be skipped.
+
 Code Organization
 ~~~~~~~~~~~~~~~~~
 

doc/source/development_guidelines.rst

@@ -77,6 +77,9 @@ To execute the tests:
   --recreate`` or remove the ``.tox`` directory to force ``tox`` to recreate the
   dependency list.
 
+  Swift's tests require having an XFS directory available in ``/tmp`` or
+  in the ``TMPDIR`` environment variable.
+
 Swift's functional tests may be executed against a :doc:`development_saio` or
 other running Swift cluster using the command::
 

doc/source/development_saio.rst

@@ -201,6 +201,23 @@ On Fedora 19 or later, you need to place these in ``/etc/rc.d/rc.local``.
 
 On OpenSuse you need to place these in ``/etc/init.d/boot.local``.
 
+Creating an XFS tmp dir
+-----------------------
+
+Tests require having an XFS directory available in ``/tmp`` or in the
+``TMPDIR`` environment variable. To set up ``/tmp`` with an XFS filesystem,
+do the following::
+
+        cd ~
+        truncate -s 1GB xfs_file  # create 1GB fil for XFS in your home directory
+        mkfs.xfs xfs_file
+        sudo mount -o loop,noatime,nodiratime xfs_file /tmp
+        sudo chmod -R 1777 /tmp
+
+To persist this, edit and add the following to ``/etc/fstab``::
+
+        /home/swift/xfs_file /tmp xfs rw,noatime,nodiratime,attr2,inode64,noquota 0 0
+
 ----------------
 Getting the code
 ----------------

swift/common/exceptions.py

@@ -105,6 +105,10 @@ class DiskFileXattrNotSupported(DiskFileError):
     pass
 
 
+class DiskFileBadMetadataChecksum(DiskFileError):
+    pass
+
+
 class DeviceUnavailable(SwiftException):
     pass
 

swift/common/manager.py

@@ -23,6 +23,7 @@ import time
 import subprocess
 import re
 from swift import gettext_ as _
+import tempfile
 
 from swift.common.utils import search_tree, remove_file, write_file
 from swift.common.exceptions import InvalidPidFileException
@@ -82,7 +83,7 @@ def setup_env():
                 "Running as non-root?"))
 
     # Set PYTHON_EGG_CACHE if it isn't already set
-    os.environ.setdefault('PYTHON_EGG_CACHE', '/tmp')
+    os.environ.setdefault('PYTHON_EGG_CACHE', tempfile.gettempdir())
 
 
 def command(func):

swift/obj/diskfile.py

@@ -70,7 +70,8 @@ from swift.common.splice import splice, tee
 from swift.common.exceptions import DiskFileQuarantined, DiskFileNotExist, \
     DiskFileCollision, DiskFileNoSpace, DiskFileDeviceUnavailable, \
     DiskFileDeleted, DiskFileError, DiskFileNotOpen, PathNotDir, \
-    ReplicationLockTimeout, DiskFileExpired, DiskFileXattrNotSupported
+    ReplicationLockTimeout, DiskFileExpired, DiskFileXattrNotSupported, \
+    DiskFileBadMetadataChecksum
 from swift.common.swob import multi_range_iterator
 from swift.common.storage_policy import (
     get_policy_string, split_policy_string, PolicyError, POLICIES,
@@ -83,6 +84,7 @@ DEFAULT_RECLAIM_AGE = timedelta(weeks=1).total_seconds()
 HASH_FILE = 'hashes.pkl'
 HASH_INVALIDATIONS_FILE = 'hashes.invalid'
 METADATA_KEY = 'user.swift.metadata'
+METADATA_CHECKSUM_KEY = 'user.swift.metadata_checksum'
 DROP_CACHE_WINDOW = 1024 * 1024
 # These are system-set metadata keys that cannot be changed with a POST.
 # They should be lowercase.
@@ -145,16 +147,33 @@ def read_metadata(fd):
                                                      (key or '')))
             key += 1
     except (IOError, OSError) as e:
-        for err in 'ENOTSUP', 'EOPNOTSUPP':
+        if errno.errorcode.get(e.errno) in ('ENOTSUP', 'EOPNOTSUPP'):
-            if hasattr(errno, err) and e.errno == getattr(errno, err):
+            msg = "Filesystem at %s does not support xattr"
-                msg = "Filesystem at %s does not support xattr" % \
+            logging.exception(msg, _get_filename(fd))
-                      _get_filename(fd)
-                logging.exception(msg)
             raise DiskFileXattrNotSupported(e)
         if e.errno == errno.ENOENT:
             raise DiskFileNotExist()
         # TODO: we might want to re-raise errors that don't denote a missing
         # xattr here.  Seems to be ENODATA on linux and ENOATTR on BSD/OSX.
+
+    metadata_checksum = None
+    try:
+        metadata_checksum = xattr.getxattr(fd, METADATA_CHECKSUM_KEY)
+    except (IOError, OSError) as e:
+        # All the interesting errors were handled above; the only thing left
+        # here is ENODATA / ENOATTR to indicate that this attribute doesn't
+        # exist. This is fine; it just means that this object predates the
+        # introduction of metadata checksums.
+        pass
+
+    if metadata_checksum:
+        computed_checksum = hashlib.md5(metadata).hexdigest()
+        if metadata_checksum != computed_checksum:
+            raise DiskFileBadMetadataChecksum(
+                "Metadata checksum mismatch for %s: "
+                "stored checksum='%s', computed='%s'" % (
+                    fd, metadata_checksum, computed_checksum))
+
     # strings are utf-8 encoded when written, but have not always been
     # (see https://bugs.launchpad.net/swift/+bug/1678018) so encode them again
     # when read
@@ -169,21 +188,23 @@ def write_metadata(fd, metadata, xattr_size=65536):
     :param metadata: metadata to write
     """
     metastr = pickle.dumps(_encode_metadata(metadata), PICKLE_PROTOCOL)
+    metastr_md5 = hashlib.md5(metastr).hexdigest()
     key = 0
-    while metastr:
     try:
+        while metastr:
             xattr.setxattr(fd, '%s%s' % (METADATA_KEY, key or ''),
                            metastr[:xattr_size])
             metastr = metastr[xattr_size:]
             key += 1
+        xattr.setxattr(fd, METADATA_CHECKSUM_KEY, metastr_md5)
     except IOError as e:
-            for err in 'ENOTSUP', 'EOPNOTSUPP':
+        # errno module doesn't always have both of these, hence the ugly
-                if hasattr(errno, err) and e.errno == getattr(errno, err):
+        # check
-                    msg = "Filesystem at %s does not support xattr" % \
+        if errno.errorcode.get(e.errno) in ('ENOTSUP', 'EOPNOTSUPP'):
-                          _get_filename(fd)
+            msg = "Filesystem at %s does not support xattr"
-                    logging.exception(msg)
+            logging.exception(msg, _get_filename(fd))
             raise DiskFileXattrNotSupported(e)
-            if e.errno in (errno.ENOSPC, errno.EDQUOT):
+        elif e.errno in (errno.ENOSPC, errno.EDQUOT):
             msg = "No space left on device for %s" % _get_filename(fd)
             logging.exception(msg)
             raise DiskFileNoSpace()
@@ -2389,6 +2410,8 @@ class BaseDiskFile(object):
             return read_metadata(source)
         except (DiskFileXattrNotSupported, DiskFileNotExist):
             raise
+        except DiskFileBadMetadataChecksum as err:
+            raise self._quarantine(quarantine_filename, str(err))
         except Exception as err:
             raise self._quarantine(
                 quarantine_filename,

test/functional/__init__.py

@@ -31,7 +31,6 @@ from contextlib import closing
 from gzip import GzipFile
 from shutil import rmtree
 from tempfile import mkdtemp
-from unittest2 import SkipTest
 
 from six.moves.configparser import ConfigParser, NoSectionError
 from six.moves import http_client
@@ -44,10 +43,13 @@ from swift.common.utils import set_swift_dir
 from test import get_config, listen_zero
 from test.functional.swift_test_client import Account, Connection, Container, \
     ResponseError
-# This has the side effect of mocking out the xattr module so that unit tests
+
-# (and in this case, when in-process functional tests are called for) can run
-# on file systems that don't support extended attributes.
 from test.unit import debug_logger, FakeMemcache
+# importing skip_if_no_xattrs so that functional tests can grab it from the
+# test.functional namespace. Importing SkipTest so this works under both
+# nose and testr test runners.
+from test.unit import skip_if_no_xattrs as real_skip_if_no_xattrs
+from test.unit import SkipTest
 
 from swift.common import constraints, utils, ring, storage_policy
 from swift.common.ring import Ring
@@ -110,6 +112,7 @@ insecure = False
 in_process = False
 _testdir = _test_servers = _test_coros = _test_socks = None
 policy_specified = None
+skip_if_no_xattrs = None
 
 
 class FakeMemcacheMiddleware(MemcacheMiddleware):
@@ -660,6 +663,7 @@ def get_cluster_info():
 def setup_package():
 
     global policy_specified
+    global skip_if_no_xattrs
     policy_specified = os.environ.get('SWIFT_TEST_POLICY')
     in_process_env = os.environ.get('SWIFT_TEST_IN_PROCESS')
     if in_process_env is not None:
@@ -698,6 +702,7 @@ def setup_package():
     if in_process:
         in_mem_obj_env = os.environ.get('SWIFT_TEST_IN_MEMORY_OBJ')
         in_mem_obj = utils.config_true_value(in_mem_obj_env)
+        skip_if_no_xattrs = real_skip_if_no_xattrs
         try:
             in_process_setup(the_object_server=(
                 mem_object_server if in_mem_obj else object_server))
@@ -705,6 +710,8 @@ def setup_package():
             print(('Exception during in-process setup: %s'
                    % str(exc)), file=sys.stderr)
             raise
+    else:
+        skip_if_no_xattrs = lambda: None
 
     global web_front_end
     web_front_end = config.get('web_front_end', 'integral')

test/functional/test_account.py

@@ -834,6 +834,9 @@ class TestAccount(unittest2.TestCase):
         if tf.skip:
             raise SkipTest
 
+        if tf.in_process:
+            tf.skip_if_no_xattrs()
+
         def post(url, token, parsed, conn, extra_headers):
             headers = {'X-Auth-Token': token}
             headers.update(extra_headers)

test/functional/test_container.py

@@ -438,6 +438,9 @@ class TestContainer(unittest2.TestCase):
         if tf.skip:
             raise SkipTest
 
+        if tf.in_process:
+            tf.skip_if_no_xattrs()
+
         def post(url, token, parsed, conn, extra_headers):
             headers = {'X-Auth-Token': token}
             headers.update(extra_headers)
@@ -580,6 +583,9 @@ class TestContainer(unittest2.TestCase):
     def test_cross_account_public_container(self):
         if tf.skip or tf.skip2:
             raise SkipTest
+
+        if tf.in_process:
+            tf.skip_if_no_xattrs()
         # Obtain the first account's string
         first_account = ['unknown']
 
@@ -649,6 +655,9 @@ class TestContainer(unittest2.TestCase):
     def test_nonadmin_user(self):
         if tf.skip or tf.skip3:
             raise SkipTest
+
+        if tf.in_process:
+            tf.skip_if_no_xattrs()
         # Obtain the first account's string
         first_account = ['unknown']
 
@@ -1562,6 +1571,9 @@ class TestContainer(unittest2.TestCase):
         if 'container_quotas' not in cluster_info:
             raise SkipTest('Container quotas not enabled')
 
+        if tf.in_process:
+            tf.skip_if_no_xattrs()
+
         def post(url, token, parsed, conn, name, value):
             conn.request('POST', parsed.path + '/' + self.name, '',
                          {'X-Auth-Token': token, name: value})

test/functional/test_object.py

@@ -42,6 +42,9 @@ class TestObject(unittest2.TestCase):
     def setUp(self):
         if tf.skip or tf.skip2:
             raise SkipTest
+
+        if tf.in_process:
+            tf.skip_if_no_xattrs()
         self.container = uuid4().hex
 
         self.containers = []

test/functional/test_versioned_writes.py

@@ -379,6 +379,9 @@ class TestObjectVersioning(Base):
             self.assertNotIn('x-object-manifest', resp_headers)
 
     def _test_versioning_dlo_setup(self):
+        if tf.in_process:
+            tf.skip_if_no_xattrs()
+
         container = self.env.container
         versions_container = self.env.versions_container
         obj_name = Utils.create_name()
@@ -695,6 +698,8 @@ class TestSloWithVersioning(unittest2.TestCase):
     def setUp(self):
         if 'slo' not in cluster_info:
             raise SkipTest("SLO not enabled")
+        if tf.in_process:
+            tf.skip_if_no_xattrs()
 
         self.conn = Connection(tf.config)
         self.conn.authenticate()

test/functional/tests.py

@@ -87,16 +87,19 @@ class BaseEnv(object):
 
 
 class Base(unittest2.TestCase):
-    # subclasses may override env class
     env = BaseEnv
 
+    @classmethod
+    def tearDownClass(cls):
+        cls.env.tearDown()
+
     @classmethod
     def setUpClass(cls):
         cls.env.setUp()
 
-    @classmethod
+    def setUp(self):
-    def tearDownClass(cls):
+        if tf.in_process:
-        cls.env.tearDown()
+            tf.skip_if_no_xattrs()
 
     def assert_body(self, body):
         response_body = self.env.conn.response.read()
@@ -2721,6 +2724,9 @@ class TestServiceToken(unittest2.TestCase):
         if tf.skip_service_tokens:
             raise SkipTest
 
+        if tf.in_process:
+            tf.skip_if_no_xattrs()
+
         self.SET_TO_USERS_TOKEN = 1
         self.SET_TO_SERVICE_TOKEN = 2
 

test/unit/__init__.py

@@ -19,7 +19,6 @@ from __future__ import print_function
 import os
 import copy
 import logging
-import errno
 from six.moves import range
 from six import BytesIO
 import sys
@@ -32,11 +31,14 @@ import time
 import eventlet
 from eventlet import greenpool, debug as eventlet_debug
 from eventlet.green import socket
-from tempfile import mkdtemp
+from tempfile import mkdtemp, mkstemp, gettempdir
 from shutil import rmtree
 import signal
 import json
 import random
+import errno
+import xattr
+
 
 from swift.common.utils import Timestamp, NOTICE
 from test import get_config
@@ -57,7 +59,12 @@ import six.moves.cPickle as pickle
 from gzip import GzipFile
 import mock as mocklib
 import inspect
-from nose import SkipTest
+import unittest
+import unittest2
+
+
+class SkipTest(unittest2.SkipTest, unittest.SkipTest):
+    pass
 
 EMPTY_ETAG = md5().hexdigest()
 
@@ -402,36 +409,6 @@ def tmpfile(content):
     finally:
         os.unlink(file_name)
 
-xattr_data = {}
-
-
-def _get_inode(fd):
-    if not isinstance(fd, int):
-        try:
-            fd = fd.fileno()
-        except AttributeError:
-            return os.stat(fd).st_ino
-    return os.fstat(fd).st_ino
-
-
-def _setxattr(fd, k, v):
-    inode = _get_inode(fd)
-    data = xattr_data.get(inode, {})
-    data[k] = v
-    xattr_data[inode] = data
-
-
-def _getxattr(fd, k):
-    inode = _get_inode(fd)
-    data = xattr_data.get(inode, {}).get(k)
-    if not data:
-        raise IOError(errno.ENODATA, "Fake IOError")
-    return data
-
-import xattr
-xattr.setxattr = _setxattr
-xattr.getxattr = _getxattr
-
 
 @contextmanager
 def temptree(files, contents=''):
@@ -1289,3 +1266,51 @@ def fake_ec_node_response(node_frags, policy):
         return StubResponse(200, body, headers)
 
     return get_response
+
+
+supports_xattr_cached_val = None
+
+
+def xattr_supported_check():
+    """
+    This check simply sets more than 4k of metadata on a tempfile and
+    returns True if it worked and False if not.
+
+    We want to use *more* than 4k of metadata in this check because
+    some filesystems (eg ext4) only allow one blocksize worth of
+    metadata. The XFS filesystem doesn't have this limit, and so this
+    check returns True when TMPDIR is XFS. This check will return
+    False under ext4 (which supports xattrs <= 4k) and tmpfs (which
+    doesn't support xattrs at all).
+
+    """
+    global supports_xattr_cached_val
+
+    if supports_xattr_cached_val is not None:
+        return supports_xattr_cached_val
+
+    # assume the worst -- xattrs aren't supported
+    supports_xattr_cached_val = False
+
+    big_val = 'x' * (4096 + 1)  # more than 4k of metadata
+    try:
+        fd, tmppath = mkstemp()
+        xattr.setxattr(fd, 'user.swift.testing_key', big_val)
+    except IOError as e:
+        if errno.errorcode.get(e.errno) in ('ENOSPC', 'ENOTSUP', 'EOPNOTSUPP'):
+            # filesystem does not support xattr of this size
+            return False
+        raise
+    else:
+        supports_xattr_cached_val = True
+        return True
+    finally:
+        # clean up the tmpfile
+        os.close(fd)
+        os.unlink(tmppath)
+
+
+def skip_if_no_xattrs():
+    if not xattr_supported_check():
+        raise SkipTest('Large xattrs not supported in `%s`. Skipping test' %
+                       gettempdir())

test/unit/cli/test_info.py

@@ -20,7 +20,7 @@ from shutil import rmtree
 from tempfile import mkdtemp
 
 from six.moves import cStringIO as StringIO
-from test.unit import patch_policies, write_fake_ring
+from test.unit import patch_policies, write_fake_ring, skip_if_no_xattrs
 
 from swift.common import ring, utils
 from swift.common.swob import Request
@@ -40,6 +40,7 @@ from swift.obj.diskfile import write_metadata
                  StoragePolicy(3, 'three', False)])
 class TestCliInfoBase(unittest.TestCase):
     def setUp(self):
+        skip_if_no_xattrs()
         self.orig_hp = utils.HASH_PATH_PREFIX, utils.HASH_PATH_SUFFIX
         utils.HASH_PATH_PREFIX = 'info'
         utils.HASH_PATH_SUFFIX = 'info'

test/unit/cli/test_relinker.py

@@ -26,11 +26,12 @@ from swift.common.storage_policy import (
 
 from swift.obj.diskfile import write_metadata
 
-from test.unit import FakeLogger
+from test.unit import FakeLogger, skip_if_no_xattrs
 
 
 class TestRelinker(unittest.TestCase):
     def setUp(self):
+        skip_if_no_xattrs()
         self.logger = FakeLogger()
         self.testdir = tempfile.mkdtemp()
         self.devices = os.path.join(self.testdir, 'node')

test/unit/cli/test_ringbuilder.py

@@ -1425,7 +1425,7 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin):
         self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv)
 
     def test_validate_non_existent_file(self):
-        rand_file = '%s/%s' % ('/tmp', str(uuid.uuid4()))
+        rand_file = '%s/%s' % (tempfile.gettempdir(), str(uuid.uuid4()))
         argv = ["", rand_file, "validate"]
         self.assertSystemExit(EXIT_ERROR, ringbuilder.main, argv)
 

test/unit/common/middleware/crypto/test_encryption.py

@@ -29,7 +29,7 @@ from swift.common.ring import Ring
 from swift.common.swob import Request
 from swift.obj import diskfile
 
-from test.unit import FakeLogger
+from test.unit import FakeLogger, skip_if_no_xattrs
 from test.unit.common.middleware.crypto.crypto_helpers import (
     md5hex, encrypt, TEST_KEYMASTER_CONF)
 from test.unit.helpers import setup_servers, teardown_servers
@@ -54,6 +54,7 @@ class TestCryptoPipelineChanges(unittest.TestCase):
             cls._test_context = None
 
     def setUp(self):
+        skip_if_no_xattrs()
         self.plaintext = 'unencrypted body content'
         self.plaintext_etag = md5hex(self.plaintext)
         self._setup_crypto_app()

test/unit/common/middleware/test_recon.py

@@ -268,7 +268,8 @@ class TestReconSuccess(TestCase):
         return app
 
     def _create_ring(self, ringpath, replica_map, devs, part_shift):
-        ring.RingData(replica_map, devs, part_shift).save(ringpath)
+        ring.RingData(replica_map, devs, part_shift).save(ringpath,
+                                                          mtime=None)
 
     def _create_rings(self):
         # make the rings unique so they have different md5 sums

test/unit/common/test_linkat.py

@@ -20,6 +20,7 @@ import unittest
 import os
 import mock
 from uuid import uuid4
+from tempfile import gettempdir
 
 from swift.common.linkat import linkat
 from swift.common.utils import O_TMPFILE
@@ -42,7 +43,7 @@ class TestLinkat(unittest.TestCase):
         with open('/dev/null', 'r') as fd:
             self.assertRaises(IOError, linkat,
                               linkat.AT_FDCWD, "/proc/self/fd/%s" % (fd),
-                              linkat.AT_FDCWD, "/tmp/testlinkat",
+                              linkat.AT_FDCWD, "%s/testlinkat" % gettempdir(),
                               linkat.AT_SYMLINK_FOLLOW)
         self.assertEqual(ctypes.get_errno(), 0)
 
@@ -83,8 +84,8 @@ class TestLinkat(unittest.TestCase):
         path = None
         ret = -1
         try:
-            fd = os.open('/tmp', O_TMPFILE | os.O_WRONLY)
+            fd = os.open(gettempdir(), O_TMPFILE | os.O_WRONLY)
-            path = os.path.join('/tmp', uuid4().hex)
+            path = os.path.join(gettempdir(), uuid4().hex)
             ret = linkat(linkat.AT_FDCWD, "/proc/self/fd/%d" % (fd),
                          linkat.AT_FDCWD, path, linkat.AT_SYMLINK_FOLLOW)
             self.assertEqual(ret, 0)

test/unit/common/test_manager.py

@@ -24,6 +24,7 @@ import signal
 import errno
 from collections import defaultdict
 from time import sleep, time
+import tempfile
 
 from six.moves import reload_module
 
@@ -115,7 +116,8 @@ class TestManagerModule(unittest.TestCase):
             ]
             self.assertEqual(manager.resource.called_with_args, expected)
             self.assertTrue(
-                manager.os.environ['PYTHON_EGG_CACHE'].startswith('/tmp'))
+                manager.os.environ['PYTHON_EGG_CACHE'].startswith(
+                    tempfile.gettempdir()))
 
             # test error condition
             manager.resource = MockResource(error=ValueError())
@@ -123,7 +125,8 @@ class TestManagerModule(unittest.TestCase):
             manager.setup_env()
             self.assertEqual(manager.resource.called_with_args, [])
             self.assertTrue(
-                manager.os.environ['PYTHON_EGG_CACHE'].startswith('/tmp'))
+                manager.os.environ['PYTHON_EGG_CACHE'].startswith(
+                    tempfile.gettempdir()))
 
             manager.resource = MockResource(error=OSError())
             manager.os.environ = {}

test/unit/common/test_utils.py

@@ -2018,7 +2018,7 @@ foo = bar
 [section2]
 log_name = yarr'''
         # setup a real file
-        fd, temppath = tempfile.mkstemp(dir='/tmp')
+        fd, temppath = tempfile.mkstemp()
         with os.fdopen(fd, 'wb') as f:
             f.write(conf)
         make_filename = lambda: temppath
@@ -2067,7 +2067,7 @@ foo = bar
 [section2]
 log_name = %(yarr)s'''
         # setup a real file
-        fd, temppath = tempfile.mkstemp(dir='/tmp')
+        fd, temppath = tempfile.mkstemp()
         with os.fdopen(fd, 'wb') as f:
             f.write(conf)
         make_filename = lambda: temppath
@@ -3275,7 +3275,7 @@ cluster_dfw1 = http://dfw1.host/v1/
         tmpdir = mkdtemp()
         try:
             link = os.path.join(tmpdir, "tmp")
-            os.symlink("/tmp", link)
+            os.symlink(tempfile.gettempdir(), link)
             self.assertFalse(utils.ismount(link))
         finally:
             shutil.rmtree(tmpdir)
@@ -3580,7 +3580,7 @@ cluster_dfw1 = http://dfw1.host/v1/
         tempdir = None
         fd = None
         try:
-            tempdir = mkdtemp(dir='/tmp')
+            tempdir = mkdtemp()
             fd, temppath = tempfile.mkstemp(dir=tempdir)
 
             _mock_fsync = mock.Mock()
@@ -3618,7 +3618,7 @@ cluster_dfw1 = http://dfw1.host/v1/
     def test_renamer_with_fsync_dir(self):
         tempdir = None
         try:
-            tempdir = mkdtemp(dir='/tmp')
+            tempdir = mkdtemp()
             # Simulate part of object path already existing
             part_dir = os.path.join(tempdir, 'objects/1234/')
             os.makedirs(part_dir)
@@ -3665,7 +3665,7 @@ cluster_dfw1 = http://dfw1.host/v1/
         tempdir = None
         fd = None
         try:
-            tempdir = mkdtemp(dir='/tmp')
+            tempdir = mkdtemp()
             os.makedirs(os.path.join(tempdir, 'a/b'))
             # 4 new dirs created
             dirpath = os.path.join(tempdir, 'a/b/1/2/3/4')
@@ -3788,7 +3788,7 @@ cluster_dfw1 = http://dfw1.host/v1/
 
     @requires_o_tmpfile_support
     def test_link_fd_to_path_linkat_success(self):
-        tempdir = mkdtemp(dir='/tmp')
+        tempdir = mkdtemp()
         fd = os.open(tempdir, utils.O_TMPFILE | os.O_WRONLY)
         data = "I'm whatever Gotham needs me to be"
         _m_fsync_dir = mock.Mock()
@@ -3808,7 +3808,7 @@ cluster_dfw1 = http://dfw1.host/v1/
 
     @requires_o_tmpfile_support
     def test_link_fd_to_path_target_exists(self):
-        tempdir = mkdtemp(dir='/tmp')
+        tempdir = mkdtemp()
         # Create and write to a file
         fd, path = tempfile.mkstemp(dir=tempdir)
         os.write(fd, "hello world")
@@ -3843,7 +3843,7 @@ cluster_dfw1 = http://dfw1.host/v1/
 
     @requires_o_tmpfile_support
     def test_linkat_race_dir_not_exists(self):
-        tempdir = mkdtemp(dir='/tmp')
+        tempdir = mkdtemp()
         target_dir = os.path.join(tempdir, uuid4().hex)
         target_path = os.path.join(target_dir, uuid4().hex)
         os.mkdir(target_dir)

test/unit/obj/test_auditor.py

@@ -14,7 +14,6 @@
 # limitations under the License.
 import json
 
-from test import unit
 import unittest
 import mock
 import os
@@ -26,7 +25,7 @@ from tempfile import mkdtemp
 import textwrap
 from os.path import dirname, basename
 from test.unit import (debug_logger, patch_policies, make_timestamp_iter,
-                       DEFAULT_TEST_EC_TYPE)
+                       DEFAULT_TEST_EC_TYPE, skip_if_no_xattrs)
 from swift.obj import auditor, replicator
 from swift.obj.diskfile import (
     DiskFile, write_metadata, invalidate_hash, get_data_dir,
@@ -63,6 +62,7 @@ def works_only_once(callable_thing, exception):
 class TestAuditor(unittest.TestCase):
 
     def setUp(self):
+        skip_if_no_xattrs()
         self.testdir = os.path.join(mkdtemp(), 'tmp_test_object_auditor')
         self.devices = os.path.join(self.testdir, 'node')
         self.rcache = os.path.join(self.testdir, 'object.recon')
@@ -118,7 +118,6 @@ class TestAuditor(unittest.TestCase):
 
     def tearDown(self):
         rmtree(os.path.dirname(self.testdir), ignore_errors=1)
-        unit.xattr_data = {}
 
     def test_worker_conf_parms(self):
         def check_common_defaults():

test/unit/obj/test_diskfile.py

@@ -44,7 +44,8 @@ from swift.obj.diskfile import MD5_OF_EMPTY_STRING, update_auditor_status
 from test.unit import (mock as unit_mock, temptree, mock_check_drive,
                        patch_policies, debug_logger, EMPTY_ETAG,
                        make_timestamp_iter, DEFAULT_TEST_EC_TYPE,
-                       requires_o_tmpfile_support, encode_frag_archive_bodies)
+                       requires_o_tmpfile_support, encode_frag_archive_bodies,
+                       skip_if_no_xattrs)
 from nose import SkipTest
 from swift.obj import diskfile
 from swift.common import utils
@@ -61,6 +62,7 @@ from swift.common.storage_policy import (
     BaseStoragePolicy, REPL_POLICY, EC_POLICY)
 from test.unit.obj.common import write_diskfile
 
+
 test_policies = [
     StoragePolicy(0, name='zero', is_default=True),
     ECStoragePolicy(1, name='one', is_default=False,
@@ -145,6 +147,7 @@ def _make_metafilename(meta_timestamp, ctype_timestamp=None):
 class TestDiskFileModuleMethods(unittest.TestCase):
 
     def setUp(self):
+        skip_if_no_xattrs()
         utils.HASH_PATH_SUFFIX = 'endcap'
         utils.HASH_PATH_PREFIX = ''
         # Setup a test ring per policy (stolen from common/test_ring.py)
@@ -682,6 +685,7 @@ class BaseDiskFileTestMixin(object):
     mgr_cls = None
 
     def setUp(self):
+        skip_if_no_xattrs()
         self.tmpdir = mkdtemp()
         self.testdir = os.path.join(
             self.tmpdir, 'tmp_test_obj_server_DiskFile')
@@ -3526,6 +3530,13 @@ class DiskFileMixin(BaseDiskFileTestMixin):
             wrong_byte = 'X' if meta_xattr[0] != 'X' else 'Y'
             xattr.setxattr(data_files[0], "user.swift.metadata",
                            wrong_byte + meta_xattr[1:])
+        elif invalid_type == 'Subtly-Corrupt-Xattrs':
+            # We have to go below read_metadata/write_metadata to get proper
+            # corruption.
+            meta_xattr = xattr.getxattr(data_files[0], "user.swift.metadata")
+            wrong_checksum = md5(meta_xattr + "some extra stuff").hexdigest()
+            xattr.setxattr(data_files[0], "user.swift.metadata_checksum",
+                           wrong_checksum)
         elif invalid_type == 'Truncated-Xattrs':
             meta_xattr = xattr.getxattr(data_files[0], "user.swift.metadata")
             xattr.setxattr(data_files[0], "user.swift.metadata",
@@ -3684,6 +3695,11 @@ class DiskFileMixin(BaseDiskFileTestMixin):
     def test_quarantine_corrupt_xattrs(self):
         self.run_quarantine_invalids('Corrupt-Xattrs')
 
+    def test_quarantine_subtly_corrupt_xattrs(self):
+        # xattrs that unpickle without error, but whose checksum does not
+        # match
+        self.run_quarantine_invalids('Subtly-Corrupt-Xattrs')
+
     def test_quarantine_truncated_xattrs(self):
         self.run_quarantine_invalids('Truncated-Xattrs')
 
@@ -3746,18 +3762,7 @@ class DiskFileMixin(BaseDiskFileTestMixin):
             invalid_type='Bad-Content-Length')
 
     def test_quarantine_fstat_oserror(self):
-        invocations = [0]
+        with mock.patch('os.fstat', side_effect=OSError()):
-        orig_os_fstat = os.fstat
-
-        def bad_fstat(fd):
-            invocations[0] += 1
-            if invocations[0] == 4:
-                # FIXME - yes, this an icky way to get code coverage ... worth
-                # it?
-                raise OSError()
-            return orig_os_fstat(fd)
-
-        with mock.patch('os.fstat', bad_fstat):
             self.assertRaises(
                 DiskFileQuarantined,
                 self._get_open_disk_file)
@@ -5957,6 +5962,7 @@ class TestSuffixHashes(unittest.TestCase):
     """
 
     def setUp(self):
+        skip_if_no_xattrs()
         self.testdir = tempfile.mkdtemp()
         self.logger = debug_logger('suffix-hash-test')
         self.devices = os.path.join(self.testdir, 'node')

test/unit/obj/test_reconstructor.py

@@ -45,7 +45,7 @@ from swift.obj.reconstructor import REVERT
 from test.unit import (patch_policies, debug_logger, mocked_http_conn,
                        FabricatedRing, make_timestamp_iter,
                        DEFAULT_TEST_EC_TYPE, encode_frag_archive_bodies,
-                       quiet_eventlet_exceptions)
+                       quiet_eventlet_exceptions, skip_if_no_xattrs)
 from test.unit.obj.common import write_diskfile
 
 
@@ -149,6 +149,7 @@ class TestGlobalSetupObjectReconstructor(unittest.TestCase):
     legacy_durable = False
 
     def setUp(self):
+        skip_if_no_xattrs()
         self.testdir = tempfile.mkdtemp()
         _create_test_rings(self.testdir)
         POLICIES[0].object_ring = ring.Ring(self.testdir, ring_name='object')
@@ -2387,6 +2388,7 @@ class TestWorkerReconstructor(unittest.TestCase):
 @patch_policies(with_ec_default=True)
 class BaseTestObjectReconstructor(unittest.TestCase):
     def setUp(self):
+        skip_if_no_xattrs()
         self.policy = POLICIES.default
         self.policy.object_ring._rtime = time.time() + 3600
         self.testdir = tempfile.mkdtemp()

test/unit/obj/test_replicator.py

@@ -30,7 +30,8 @@ from eventlet.green import subprocess
 from eventlet import Timeout
 
 from test.unit import (debug_logger, patch_policies, make_timestamp_iter,
-                       mocked_http_conn, FakeLogger, mock_check_drive)
+                       mocked_http_conn, FakeLogger, mock_check_drive,
+                       skip_if_no_xattrs)
 from swift.common import utils
 from swift.common.utils import (hash_path, mkdirs, normalize_timestamp,
                                 storage_directory)
@@ -179,6 +180,7 @@ def _create_test_rings(path, devs=None, next_part_power=None):
 class TestObjectReplicator(unittest.TestCase):
 
     def setUp(self):
+        skip_if_no_xattrs()
         utils.HASH_PATH_SUFFIX = 'endcap'
         utils.HASH_PATH_PREFIX = ''
         # recon cache path

test/unit/obj/test_server.py

@@ -45,9 +45,9 @@ from swift import __version__ as swift_version
 from swift.common.http import is_success
 from test import listen_zero
 from test.unit import FakeLogger, debug_logger, mocked_http_conn, \
-    make_timestamp_iter, DEFAULT_TEST_EC_TYPE, mock_check_drive
+    make_timestamp_iter, DEFAULT_TEST_EC_TYPE, skip_if_no_xattrs, \
-from test.unit import connect_tcp, readuntil2crlfs, patch_policies, \
+    connect_tcp, readuntil2crlfs, patch_policies, encode_frag_archive_bodies, \
-    encode_frag_archive_bodies
+    mock_check_drive
 from swift.obj import server as object_server
 from swift.obj import updater
 from swift.obj import diskfile
@@ -140,6 +140,7 @@ class TestObjectController(unittest.TestCase):
 
     def setUp(self):
         """Set up for testing swift.object.server.ObjectController"""
+        skip_if_no_xattrs()
         utils.HASH_PATH_SUFFIX = 'endcap'
         utils.HASH_PATH_PREFIX = 'startcap'
         self.tmpdir = mkdtemp()
@@ -6942,6 +6943,7 @@ class TestObjectController(unittest.TestCase):
 class TestObjectServer(unittest.TestCase):
 
     def setUp(self):
+        skip_if_no_xattrs()
         # dirs
         self.tmpdir = mkdtemp()
         self.tempdir = os.path.join(self.tmpdir, 'tmp_test_obj_server')
@@ -7632,6 +7634,7 @@ class TestZeroCopy(unittest.TestCase):
         return True
 
     def setUp(self):
+        skip_if_no_xattrs()
         if not self._system_can_zero_copy():
             raise SkipTest("zero-copy support is missing")
 

test/unit/obj/test_ssync.py

@@ -34,8 +34,9 @@ from swift.obj.reconstructor import RebuildingECDiskFileStream, \
 from swift.obj.replicator import ObjectReplicator
 
 from test import listen_zero
-from test.unit import patch_policies, debug_logger, encode_frag_archive_bodies
 from test.unit.obj.common import BaseTest
+from test.unit import patch_policies, debug_logger, \
+    encode_frag_archive_bodies, skip_if_no_xattrs
 
 
 class TestBaseSsync(BaseTest):
@@ -47,6 +48,7 @@ class TestBaseSsync(BaseTest):
     about the final state of the sender and receiver diskfiles.
     """
     def setUp(self):
+        skip_if_no_xattrs()
         super(TestBaseSsync, self).setUp()
         # rx side setup
         self.rx_testdir = os.path.join(self.tmpdir, 'tmp_test_ssync_receiver')

test/unit/obj/test_ssync_receiver.py

@@ -35,7 +35,7 @@ from swift.obj.reconstructor import ObjectReconstructor
 
 from test import listen_zero, unit
 from test.unit import (debug_logger, patch_policies, make_timestamp_iter,
-                       mock_check_drive)
+                       mock_check_drive, skip_if_no_xattrs)
 from test.unit.obj.common import write_diskfile
 
 
@@ -43,12 +43,11 @@ from test.unit.obj.common import write_diskfile
 class TestReceiver(unittest.TestCase):
 
     def setUp(self):
+        skip_if_no_xattrs()
         utils.HASH_PATH_SUFFIX = 'endcap'
         utils.HASH_PATH_PREFIX = 'startcap'
         # Not sure why the test.unit stuff isn't taking effect here; so I'm
         # reinforcing it.
-        diskfile.getxattr = unit._getxattr
-        diskfile.setxattr = unit._setxattr
         self.testdir = os.path.join(
             tempfile.mkdtemp(), 'tmp_test_ssync_receiver')
         utils.mkdirs(os.path.join(self.testdir, 'sda1', 'tmp'))
@@ -1963,6 +1962,7 @@ class TestSsyncRxServer(unittest.TestCase):
     # server socket.
 
     def setUp(self):
+        skip_if_no_xattrs()
         # dirs
         self.tmpdir = tempfile.mkdtemp()
         self.tempdir = os.path.join(self.tmpdir, 'tmp_test_obj_server')

test/unit/obj/test_ssync_sender.py

@@ -26,8 +26,9 @@ from swift.common.utils import Timestamp
 from swift.obj import ssync_sender, diskfile, ssync_receiver
 from swift.obj.replicator import ObjectReplicator
 
-from test.unit import patch_policies, make_timestamp_iter, debug_logger
 from test.unit.obj.common import BaseTest
+from test.unit import patch_policies, make_timestamp_iter, skip_if_no_xattrs, \
+    debug_logger
 
 
 class NullBufferedHTTPConnection(object):
@@ -84,6 +85,7 @@ class FakeConnection(object):
 class TestSender(BaseTest):
 
     def setUp(self):
+        skip_if_no_xattrs()
         super(TestSender, self).setUp()
         self.daemon = ObjectReplicator(self.daemon_conf,
                                        debug_logger('test-ssync-sender'))

test/unit/proxy/test_server.py

@@ -53,7 +53,8 @@ from test import listen_zero
 from test.unit import (
     connect_tcp, readuntil2crlfs, FakeLogger, fake_http_connect, FakeRing,
     FakeMemcache, debug_logger, patch_policies, write_fake_ring,
-    mocked_http_conn, DEFAULT_TEST_EC_TYPE, make_timestamp_iter)
+    mocked_http_conn, DEFAULT_TEST_EC_TYPE, make_timestamp_iter,
+    skip_if_no_xattrs)
 from test.unit.helpers import setup_servers, teardown_servers
 from swift.proxy import server as proxy_server
 from swift.proxy.controllers.obj import ReplicatedObjectController
@@ -237,6 +238,7 @@ def _limit_max_file_size(f):
 class TestController(unittest.TestCase):
 
     def setUp(self):
+        skip_if_no_xattrs()
         self.account_ring = FakeRing()
         self.container_ring = FakeRing()
         self.memcache = FakeMemcache()
@@ -1288,6 +1290,7 @@ class TestProxyServerLoading(unittest.TestCase):
 class TestProxyServerConfigLoading(unittest.TestCase):
 
     def setUp(self):
+        skip_if_no_xattrs()
         self.tempdir = mkdtemp()
         account_ring_path = os.path.join(self.tempdir, 'account.ring.gz')
         write_fake_ring(account_ring_path)
@@ -1987,6 +1990,7 @@ class TestReplicatedObjectController(
     Test suite for replication policy
     """
     def setUp(self):
+        skip_if_no_xattrs()
         self.app = proxy_server.Application(
             None, FakeMemcache(),
             logger=debug_logger('proxy-ut'),
@@ -6383,6 +6387,7 @@ class BaseTestECObjectController(BaseTestObjectController):
 
 class TestECObjectController(BaseTestECObjectController, unittest.TestCase):
     def setUp(self):
+        skip_if_no_xattrs()
         self.ec_policy = POLICIES[3]
         super(TestECObjectController, self).setUp()
 
@@ -6390,11 +6395,15 @@ class TestECObjectController(BaseTestECObjectController, unittest.TestCase):
 class TestECDuplicationObjectController(
         BaseTestECObjectController, unittest.TestCase):
     def setUp(self):
+        skip_if_no_xattrs()
         self.ec_policy = POLICIES[4]
         super(TestECDuplicationObjectController, self).setUp()
 
 
 class TestECMismatchedFA(unittest.TestCase):
+    def setUp(self):
+        skip_if_no_xattrs()
+
     def tearDown(self):
         prosrv = _test_servers[0]
         # don't leak error limits and poison other tests
@@ -6581,6 +6590,7 @@ class TestECMismatchedFA(unittest.TestCase):
 class TestECGets(unittest.TestCase):
     def setUp(self):
         super(TestECGets, self).setUp()
+        skip_if_no_xattrs()
         self.tempdir = mkdtemp()
 
     def tearDown(self):
@@ -6852,6 +6862,7 @@ class TestObjectDisconnectCleanup(unittest.TestCase):
                 mkdirs(data_path)
 
     def setUp(self):
+        skip_if_no_xattrs()
         debug.hub_exceptions(False)
         self._cleanup_devices()
 
@@ -6960,6 +6971,7 @@ class TestObjectECRangedGET(unittest.TestCase):
 
     @classmethod
     def setUpClass(cls):
+        skip_if_no_xattrs()
         cls.obj_name = 'range-get-test'
         cls.tiny_obj_name = 'range-get-test-tiny'
         cls.aligned_obj_name = 'range-get-test-aligned'
@@ -9488,6 +9500,7 @@ class TestProxyObjectPerformance(unittest.TestCase):
         # This is just a simple test that can be used to verify and debug the
         # various data paths between the proxy server and the object
         # server. Used as a play ground to debug buffer sizes for sockets.
+        skip_if_no_xattrs()
         prolis = _test_sockets[0]
         sock = connect_tcp(('localhost', prolis.getsockname()[1]))
         # Client is transmitting in 2 MB chunks
@@ -9601,6 +9614,7 @@ class TestSocketObjectVersions(unittest.TestCase):
 
     def setUp(self):
         global _test_sockets
+        skip_if_no_xattrs()
         self.prolis = prolis = listen_zero()
         self._orig_prolis = _test_sockets[0]
         allowed_headers = ', '.join([

test/unit/proxy/test_sysmeta.py

@@ -30,7 +30,7 @@ from swift.proxy import server as proxy
 import swift.proxy.controllers
 from swift.proxy.controllers.base import get_object_info
 from test.unit import FakeMemcache, debug_logger, FakeRing, \
-    fake_http_connect, patch_policies
+    fake_http_connect, patch_policies, skip_if_no_xattrs
 
 
 class FakeServerConnection(WSGIContext):
@@ -132,6 +132,7 @@ class TestObjectSysmeta(unittest.TestCase):
                              % (key, resp.headers))
 
     def setUp(self):
+        skip_if_no_xattrs()
         self.app = proxy.Application(None, FakeMemcache(),
                                      logger=debug_logger('proxy-ut'),
                                      account_ring=FakeRing(replicas=1),