diff --git a/glance_store/_drivers/filesystem.py b/glance_store/_drivers/filesystem.py index b020280b..a2f86361 100644 --- a/glance_store/_drivers/filesystem.py +++ b/glance_store/_drivers/filesystem.py @@ -22,6 +22,7 @@ import errno import hashlib import logging import os +import stat import urlparse from oslo.config import cfg @@ -30,7 +31,7 @@ import glance_store from glance_store.common import utils import glance_store.driver from glance_store import exceptions -from glance_store.i18n import _ +from glance_store import i18n import glance_store.location from glance_store.openstack.common import excutils from glance_store.openstack.common import jsonutils @@ -39,6 +40,9 @@ from glance_store.openstack.common import units LOG = logging.getLogger(__name__) +_ = i18n._ +_LE = i18n._LE +_LW = i18n._LW _FILESYSTEM_CONFIGS = [ cfg.StrOpt('filesystem_store_datadir', @@ -51,7 +55,16 @@ _FILESYSTEM_CONFIGS = [ help=_("The path to a file which contains the " "metadata to be returned with any location " "associated with this store. The file must " - "contain a valid JSON dict."))] + "contain a valid JSON dict.")), + cfg.IntOpt('filesystem_store_file_perm', + default=0, + help=_("The required permission for created image file. " + "In this way the user other service used, e.g. Nova, " + "who consumes the image could be the exclusive member " + "of the group that owns the files created. Assigning " + "it less then or equal to zero means don't change the " + "default permission of the file. This value will be " + "decoded as an octal digit."))] class StoreLocation(glance_store.location.StoreLocation): @@ -142,6 +155,35 @@ class Store(glance_store.driver.Store): raise exceptions.BadStoreConfiguration( store_name="filesystem", reason=msg) + def _set_exec_permission(self, datadir): + """ + Set the execution permission of owner-group and/or other-users to + image directory if the image file which contained needs relevant + access permissions. + + :datadir is a directory path in which glance writes image files. + """ + + if self.conf.glance_store.filesystem_store_file_perm <= 0: + return + + try: + mode = os.stat(datadir)[stat.ST_MODE] + perm = int(str(self.conf.glance_store.filesystem_store_file_perm), + 8) + if perm & stat.S_IRWXO > 0: + if not mode & stat.S_IXOTH: + # chmod o+x + mode |= stat.S_IXOTH + os.chmod(datadir, mode) + if perm & stat.S_IRWXG > 0: + if not mode & stat.S_IXGRP: + # chmod g+x + os.chmod(datadir, mode | stat.S_IXGRP) + except (IOError, OSError): + LOG.warn(_LW("Unable to set execution permission of owner-group " + "and/or other-users to datadir: %s") % datadir) + def _create_image_directories(self, directory_paths): """ Create directories to write image files if @@ -153,6 +195,7 @@ class Store(glance_store.driver.Store): for datadir in directory_paths: if os.path.exists(datadir): self._check_write_permission(datadir) + self._set_exec_permission(datadir) else: msg = _("Directory to write image files does not exist " "(%s). Creating.") % datadir @@ -160,6 +203,7 @@ class Store(glance_store.driver.Store): try: os.makedirs(datadir) self._check_write_permission(datadir) + self._set_exec_permission(datadir) except (IOError, OSError): if os.path.exists(datadir): # NOTE(markwash): If the path now exists, some other @@ -167,6 +211,7 @@ class Store(glance_store.driver.Store): # But it doesn't hurt, so we can safely ignore # the error. self._check_write_permission(datadir) + self._set_exec_permission(datadir) continue reason = _("Unable to create datadir: %s") % datadir LOG.error(reason) @@ -197,6 +242,19 @@ class Store(glance_store.driver.Store): raise exceptions.BadStoreConfiguration(store_name="filesystem", reason=reason) + if self.conf.glance_store.filesystem_store_file_perm > 0: + perm = int(str(self.conf.glance_store.filesystem_store_file_perm), + 8) + if not perm & stat.S_IRUSR: + reason = _LE("Specified an invalid " + "'filesystem_store_file_perm' option which " + "could make image file to be unaccessible by " + "glance service.") + LOG.error(reason) + reason = _("Invalid 'filesystem_store_file_perm' option.") + raise exceptions.BadStoreConfiguration(store_name="filesystem", + reason=reason) + self.multiple_datadirs = False directory_paths = set() if self.conf.glance_store.filesystem_store_datadir: @@ -468,6 +526,16 @@ class Store(glance_store.driver.Store): {'bytes_written': bytes_written, 'filepath': filepath, 'checksum_hex': checksum_hex}) + + if self.conf.glance_store.filesystem_store_file_perm > 0: + perm = int(str(self.conf.glance_store.filesystem_store_file_perm), + 8) + try: + os.chmod(filepath, perm) + except (IOError, OSError): + LOG.warn(_LW("Unable to set permission to image: %s") % + filepath) + return ('file://%s' % filepath, bytes_written, checksum_hex, metadata) @staticmethod diff --git a/glance_store/backend.py b/glance_store/backend.py index 5f28aa30..50ac83eb 100644 --- a/glance_store/backend.py +++ b/glance_store/backend.py @@ -140,7 +140,6 @@ class Indexable(object): def _load_store(conf, store_entry, invoke_load=True): - store_cls = None try: LOG.debug("Attempting to import store %s", store_entry) mgr = driver.DriverManager('glance_store.drivers', @@ -148,7 +147,7 @@ def _load_store(conf, store_entry, invoke_load=True): invoke_args=[conf], invoke_on_load=invoke_load) return mgr.driver - except RuntimeError as ex: + except RuntimeError: LOG.warn("Failed to load driver %(driver)s." "The driver will be disabled" % dict(driver=driver)) @@ -176,7 +175,6 @@ def create_stores(conf=CONF): from the given config. Duplicates are not re-registered. """ store_count = 0 - store_classes = set() for (store_entry, store_instance) in _load_stores(conf): schemes = store_instance.get_schemes() @@ -184,7 +182,7 @@ def create_stores(conf=CONF): if not schemes: raise exceptions.BackendException('Unable to register store %s. ' 'No schemes associated with it.' - % store_cls) + % store_entry) else: LOG.debug("Registering store %s with schemes %s", store_entry, schemes) diff --git a/tests/unit/test_filesystem_store.py b/tests/unit/test_filesystem_store.py index e14938c4..334081e4 100644 --- a/tests/unit/test_filesystem_store.py +++ b/tests/unit/test_filesystem_store.py @@ -21,6 +21,7 @@ import hashlib import json import mock import os +import stat import StringIO import uuid @@ -446,3 +447,111 @@ class TestStore(base.StoreBaseTest): self.assertRaises(exceptions.StorageFull, self.store.add, expected_image_id, image_file, expected_file_size) + + def test_configure_add_with_file_perm(self): + """ + Tests filesystem specified by filesystem_store_file_perm + are parsed correctly. + """ + store = self.useFixture(fixtures.TempDir()).path + self.conf.set_override('filesystem_store_datadir', store, + group='glance_store') + self.conf.set_override('filesystem_store_file_perm', 700, # -rwx------ + group='glance_store') + self.store.configure_add() + self.assertEqual(self.store.datadir, store) + + def test_configure_add_with_unaccessible_file_perm(self): + """ + Tests BadStoreConfiguration exception is raised if an invalid + file permission specified in filesystem_store_file_perm. + """ + store = self.useFixture(fixtures.TempDir()).path + self.conf.set_override('filesystem_store_datadir', store, + group='glance_store') + self.conf.set_override('filesystem_store_file_perm', 7, # -------rwx + group='glance_store') + self.assertRaises(exceptions.BadStoreConfiguration, + self.store.configure_add) + + def test_add_with_file_perm_for_group_other_users_access(self): + """ + Test that we can add an image via the filesystem backend with a + required image file permission. + """ + store = self.useFixture(fixtures.TempDir()).path + self.conf.set_override('filesystem_store_datadir', store, + group='glance_store') + self.conf.set_override('filesystem_store_file_perm', 744, # -rwxr--r-- + group='glance_store') + + # -rwx------ + os.chmod(store, 0o700) + self.assertEqual(0o700, stat.S_IMODE(os.stat(store)[stat.ST_MODE])) + + self.store.configure_add() + + Store.WRITE_CHUNKSIZE = 1024 + expected_image_id = str(uuid.uuid4()) + expected_file_size = 5 * units.Ki # 5K + expected_file_contents = "*" * expected_file_size + expected_checksum = hashlib.md5(expected_file_contents).hexdigest() + expected_location = "file://%s/%s" % (store, + expected_image_id) + image_file = six.StringIO(expected_file_contents) + + location, size, checksum, _ = self.store.add(expected_image_id, + image_file, + expected_file_size) + + self.assertEqual(expected_location, location) + self.assertEqual(expected_file_size, size) + self.assertEqual(expected_checksum, checksum) + + # -rwx--x--x for store directory + self.assertEqual(0o711, stat.S_IMODE(os.stat(store)[stat.ST_MODE])) + # -rwxr--r-- for image file + mode = os.stat(expected_location[len('file:/'):])[stat.ST_MODE] + perm = int(str(self.conf.glance_store.filesystem_store_file_perm), 8) + self.assertEqual(perm, stat.S_IMODE(mode)) + + def test_add_with_file_perm_for_owner_users_access(self): + """ + Test that we can add an image via the filesystem backend with a + required image file permission. + """ + store = self.useFixture(fixtures.TempDir()).path + self.conf.set_override('filesystem_store_datadir', store, + group='glance_store') + self.conf.set_override('filesystem_store_file_perm', 600, # -rw------- + group='glance_store') + + # -rwx------ + os.chmod(store, 0o700) + self.assertEqual(0o700, stat.S_IMODE(os.stat(store)[stat.ST_MODE])) + + self.store.configure_add() + + Store.WRITE_CHUNKSIZE = 1024 + expected_image_id = str(uuid.uuid4()) + expected_file_size = 5 * units.Ki # 5K + expected_file_contents = "*" * expected_file_size + expected_checksum = hashlib.md5(expected_file_contents).hexdigest() + expected_location = "file://%s/%s" % (store, + expected_image_id) + image_file = six.StringIO(expected_file_contents) + + location, size, checksum, _ = self.store.add(expected_image_id, + image_file, + expected_file_size) + + self.assertEqual(expected_location, location) + self.assertEqual(expected_file_size, size) + self.assertEqual(expected_checksum, checksum) + + # -rwx------ for store directory + self.assertEqual(0o700, stat.S_IMODE(os.stat(store)[stat.ST_MODE])) + # -rw------- for image file + mode = os.stat(expected_location[len('file:/'):])[stat.ST_MODE] + perm = int(str(self.conf.glance_store.filesystem_store_file_perm), 8) + self.assertEqual(perm, stat.S_IMODE(mode))