diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index b9c6c269cb..83777b45f4 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -1026,9 +1026,28 @@ # The length of time in seconds that the temporary URL will be # valid for. Defaults to 20 minutes. If some deploys get a 401 # response code when trying to download from the temporary -# URL, try raising this duration. (integer value) +# URL, try raising this duration. This value must be greater +# than or equal to the value for +# swift_temp_url_expected_download_start_delay (integer value) #swift_temp_url_duration=1200 +# Whether to cache generated Swift temporary URLs. Setting it +# to true is only useful when an image caching proxy is used. +# Defaults to False. (boolean value) +#swift_temp_url_cache_enabled=false + +# This is the delay (in seconds) from the time of the deploy +# request (when the Swift temporary URL is generated) to when +# the IPA ramdisk starts up and URL is used for the image +# download. This value is used to check if the Swift temporary +# URL duration is large enough to let the image download +# begin. Also if temporary URL caching is enabled this will +# determine if a cached entry will still be valid when the +# download starts. swift_temp_url_duration value must be +# greater than or equal to this option's value. Defaults to 0. +# (integer value) +#swift_temp_url_expected_download_start_delay=0 + # The "endpoint" (scheme, hostname, optional port) for the # Swift URL of the form # "endpoint_url/api_version/[account/]container/object_id". Do diff --git a/ironic/common/glance_service/v2/image_service.py b/ironic/common/glance_service/v2/image_service.py index a7ec857a3d..1b2ddcc430 100644 --- a/ironic/common/glance_service/v2/image_service.py +++ b/ironic/common/glance_service/v2/image_service.py @@ -13,8 +13,12 @@ # License for the specific language governing permissions and limitations # under the License. +import collections +import time + from oslo_config import cfg from oslo_utils import uuidutils +import six from six.moves.urllib import parse as urlparse from swiftclient import utils as swift_utils @@ -46,7 +50,27 @@ glance_opts = [ 'will be valid for. Defaults to 20 minutes. If some ' 'deploys get a 401 response code when trying to ' 'download from the temporary URL, try raising this ' - 'duration.')), + 'duration. This value must be greater than or equal to ' + 'the value for ' + 'swift_temp_url_expected_download_start_delay')), + cfg.BoolOpt('swift_temp_url_cache_enabled', + default=False, + help=_('Whether to cache generated Swift temporary URLs. ' + 'Setting it to true is only useful when an image ' + 'caching proxy is used. Defaults to False.')), + cfg.IntOpt('swift_temp_url_expected_download_start_delay', + default=0, min=0, + help=_('This is the delay (in seconds) from the time of the ' + 'deploy request (when the Swift temporary URL is ' + 'generated) to when the IPA ramdisk starts up and URL ' + 'is used for the image download. This value is used to ' + 'check if the Swift temporary URL duration is large ' + 'enough to let the image download begin. Also if ' + 'temporary URL caching is enabled this will determine ' + 'if a cached entry will still be valid when the ' + 'download starts. swift_temp_url_duration value must be ' + 'greater than or equal to this option\'s value. ' + 'Defaults to 0.')), cfg.StrOpt( 'swift_endpoint_url', help=_('The "endpoint" (scheme, hostname, optional port) for ' @@ -93,16 +117,29 @@ glance_opts = [ choices=['swift', 'radosgw'], help=_('Type of endpoint to use for temporary URLs. If the ' 'Glance backend is Swift, use "swift"; if it is CEPH ' - 'with RADOS gateway, use "radosgw".')) + 'with RADOS gateway, use "radosgw".')), ] CONF = cfg.CONF CONF.register_opts(glance_opts, group='glance') +TempUrlCacheElement = collections.namedtuple('TempUrlCacheElement', + ['url', 'url_expires_at']) + class GlanceImageService(base_image_service.BaseImageService, service.ImageService): + # A dictionary containing cached temp URLs in namedtuples + # in format: + # { + # <image_id> : ( + # url=<temp_url>, + # url_expires_at=<expiration_time> + # ) + # } + _cache = {} + def detail(self, **kwargs): return self._detail(method='list', **kwargs) @@ -124,10 +161,50 @@ class GlanceImageService(base_image_service.BaseImageService, def delete(self, image_id): return self._delete(image_id, method='delete') + def _generate_temp_url(self, path, seconds, key, method, endpoint, + image_id): + """Get Swift temporary URL. + + Generates (or returns the cached one if caching is enabled) a + temporary URL that gives unauthenticated access to the Swift object. + + :param path: The full path to the Swift object. Example: + /v1/AUTH_account/c/o. + :param seconds: The amount of time in seconds the temporary URL will + be valid for. + :param key: The secret temporary URL key set on the Swift cluster. + :param method: A HTTP method, typically either GET or PUT, to allow for + this temporary URL. + :param endpoint: Endpoint URL of Swift service. + :param image_id: UUID of a Glance image. + :returns: temporary URL + """ + + if CONF.glance.swift_temp_url_cache_enabled: + self._remove_expired_items_from_cache() + if image_id in self._cache: + return self._cache[image_id].url + + path = swift_utils.generate_temp_url( + path=path, seconds=seconds, key=key, method=method) + + temp_url = '{endpoint_url}{url_path}'.format( + endpoint_url=endpoint, url_path=path) + + if CONF.glance.swift_temp_url_cache_enabled: + query = urlparse.urlparse(temp_url).query + exp_time_str = dict(urlparse.parse_qsl(query))['temp_url_expires'] + self._cache[image_id] = TempUrlCacheElement( + url=temp_url, url_expires_at=int(exp_time_str) + ) + + return temp_url + def swift_temp_url(self, image_info): """Generate a no-auth Swift temporary URL. - This function will generate the temporary Swift URL using the image + This function will generate (or return the cached one if temp URL + cache is enabled) the temporary Swift URL using the image id from Glance and the config options: 'swift_endpoint_url', 'swift_api_version', 'swift_account' and 'swift_container'. The temporary URL will be valid for 'swift_temp_url_duration' seconds. @@ -156,11 +233,13 @@ class GlanceImageService(base_image_service.BaseImageService, 'The given image info does not have a valid image id: %s') % image_info) + image_id = image_info['id'] + url_fragments = { 'api_version': CONF.glance.swift_api_version, 'account': CONF.glance.swift_account, - 'container': self._get_swift_container(image_info['id']), - 'object_id': image_info['id'] + 'container': self._get_swift_container(image_id), + 'object_id': image_id } endpoint_url = CONF.glance.swift_endpoint_url @@ -180,14 +259,15 @@ class GlanceImageService(base_image_service.BaseImageService, template = '/{api_version}/{account}/{container}/{object_id}' url_path = template.format(**url_fragments) - path = swift_utils.generate_temp_url( + + return self._generate_temp_url( path=url_path, seconds=CONF.glance.swift_temp_url_duration, key=CONF.glance.swift_temp_url_key, - method='GET') - - return '{endpoint_url}{url_path}'.format( - endpoint_url=endpoint_url, url_path=path) + method='GET', + endpoint=endpoint_url, + image_id=image_id + ) def _validate_temp_url_config(self): """Validate the required settings for a temporary URL.""" @@ -204,9 +284,13 @@ class GlanceImageService(base_image_service.BaseImageService, raise exc.MissingParameterValue(_( 'Swift temporary URLs require a Swift account string. ' 'You must provide "swift_account" as a config option.')) - if CONF.glance.swift_temp_url_duration < 0: + if (CONF.glance.swift_temp_url_duration < + CONF.glance.swift_temp_url_expected_download_start_delay): raise exc.InvalidParameterValue(_( - '"swift_temp_url_duration" must be a positive integer.')) + '"swift_temp_url_duration" must be greater than or equal to ' + '"[glance]swift_temp_url_expected_download_start_delay" ' + 'option, otherwise the Swift temporary URL may expire before ' + 'the download starts.')) seed_num_chars = CONF.glance.swift_store_multiple_containers_seed if (seed_num_chars is None or seed_num_chars < 0 or seed_num_chars > 32): @@ -260,3 +344,18 @@ class GlanceImageService(base_image_service.BaseImageService, raise exc.ImageNotFound(image_id=image_id) return getattr(image_meta, 'direct_url', None) + + def _remove_expired_items_from_cache(self): + """Remove expired items from temporary URL cache + + This function removes entries that will expire before the expected + usage time. + """ + max_valid_time = ( + int(time.time()) + + CONF.glance.swift_temp_url_expected_download_start_delay) + keys_to_remove = [ + k for k, v in six.iteritems(self._cache) + if (v.url_expires_at < max_valid_time)] + for k in keys_to_remove: + del self._cache[k] diff --git a/ironic/tests/unit/common/test_glance_service.py b/ironic/tests/unit/common/test_glance_service.py index a95b467e32..749730139e 100644 --- a/ironic/tests/unit/common/test_glance_service.py +++ b/ironic/tests/unit/common/test_glance_service.py @@ -22,12 +22,14 @@ import mock from oslo_config import cfg from oslo_context import context from oslo_serialization import jsonutils +from oslo_utils import uuidutils from six.moves.urllib import parse as urlparse import testtools from ironic.common import exception from ironic.common.glance_service import base_image_service from ironic.common.glance_service import service_utils +from ironic.common.glance_service.v2 import image_service as glance_v2 from ironic.common import image_service as service from ironic.tests import base from ironic.tests.unit import stubs @@ -688,6 +690,17 @@ class TestGlanceSwiftTempURL(base.TestCase): key=CONF.glance.swift_temp_url_key, method='GET') + @mock.patch('swiftclient.utils.generate_temp_url', autospec=True) + def test_swift_temp_url_invalid_image_info(self, tempurl_mock): + self.service._validate_temp_url_config = mock.Mock() + image_info = {} + self.assertRaises(exception.ImageUnacceptable, + self.service.swift_temp_url, image_info) + image_info = {'id': 'not an id'} + self.assertRaises(exception.ImageUnacceptable, + self.service.swift_temp_url, image_info) + self.assertFalse(tempurl_mock.called) + @mock.patch('swiftclient.utils.generate_temp_url', autospec=True) def test_swift_temp_url_radosgw(self, tempurl_mock): self.config(temp_url_endpoint_type='radosgw', group='glance') @@ -800,8 +813,10 @@ class TestGlanceSwiftTempURL(base.TestCase): self.config(temp_url_endpoint_type='radosgw', group='glance') self.service._validate_temp_url_config() - def test__validate_temp_url_endpoint_negative_duration(self): - self.config(swift_temp_url_duration=-1, + def test__validate_temp_url_endpoint_less_than_download_delay(self): + self.config(swift_temp_url_expected_download_start_delay=1000, + group='glance') + self.config(swift_temp_url_duration=15, group='glance') self.assertRaises(exception.InvalidParameterValue, self.service._validate_temp_url_config) @@ -821,6 +836,215 @@ class TestGlanceSwiftTempURL(base.TestCase): self.service._validate_temp_url_config) +class TestSwiftTempUrlCache(base.TestCase): + + def setUp(self): + super(TestSwiftTempUrlCache, self).setUp() + client = stubs.StubGlanceClient() + self.context = context.RequestContext() + self.context.auth_token = 'fake' + self.config(swift_temp_url_expected_download_start_delay=100, + group='glance') + self.config(swift_temp_url_key='correcthorsebatterystaple', + group='glance') + self.config(swift_endpoint_url='https://swift.example.com', + group='glance') + self.config(swift_account='AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30', + group='glance') + self.config(swift_api_version='v1', + group='glance') + self.config(swift_container='glance', + group='glance') + self.config(swift_temp_url_duration=1200, + group='glance') + self.config(swift_temp_url_cache_enabled=True, + group='glance') + self.config(swift_store_multiple_containers_seed=0, + group='glance') + self.glance_service = service.GlanceImageService(client, version=2, + context=self.context) + + @mock.patch('swiftclient.utils.generate_temp_url', autospec=True) + def test_add_items_to_cache(self, tempurl_mock): + fake_image = { + 'id': uuidutils.generate_uuid() + } + + path = ('/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30' + '/glance' + '/%s' % fake_image['id']) + exp_time = int(time.time()) + 1200 + tempurl_mock.return_value = ( + path + '?temp_url_sig=hmacsig&temp_url_expires=%s' % exp_time) + + cleanup_mock = mock.Mock() + self.glance_service._remove_expired_items_from_cache = cleanup_mock + self.glance_service._validate_temp_url_config = mock.Mock() + + temp_url = self.glance_service.swift_temp_url( + image_info=fake_image) + + self.assertEqual(CONF.glance.swift_endpoint_url + + tempurl_mock.return_value, + temp_url) + cleanup_mock.assert_called_once_with() + tempurl_mock.assert_called_with( + path=path, + seconds=CONF.glance.swift_temp_url_duration, + key=CONF.glance.swift_temp_url_key, + method='GET') + self.assertEqual((temp_url, exp_time), + self.glance_service._cache[fake_image['id']]) + + @mock.patch('swiftclient.utils.generate_temp_url', autospec=True) + def test_return_cached_tempurl(self, tempurl_mock): + fake_image = { + 'id': uuidutils.generate_uuid() + } + + exp_time = int(time.time()) + 1200 + temp_url = CONF.glance.swift_endpoint_url + ( + '/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30' + '/glance' + '/%(uuid)s' + '?temp_url_sig=hmacsig&temp_url_expires=%(exp_time)s' % + {'uuid': fake_image['id'], 'exp_time': exp_time} + ) + self.glance_service._cache[fake_image['id']] = ( + glance_v2.TempUrlCacheElement(url=temp_url, + url_expires_at=exp_time) + ) + + cleanup_mock = mock.Mock() + self.glance_service._remove_expired_items_from_cache = cleanup_mock + self.glance_service._validate_temp_url_config = mock.Mock() + + self.assertEqual( + temp_url, self.glance_service.swift_temp_url(image_info=fake_image) + ) + + cleanup_mock.assert_called_once_with() + self.assertFalse(tempurl_mock.called) + + @mock.patch('swiftclient.utils.generate_temp_url', autospec=True) + def test_do_not_return_expired_tempurls(self, tempurl_mock): + fake_image = { + 'id': uuidutils.generate_uuid() + } + old_exp_time = int(time.time()) + 99 + path = ( + '/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30' + '/glance' + '/%s' % fake_image['id'] + ) + query = '?temp_url_sig=hmacsig&temp_url_expires=%s' + self.glance_service._cache[fake_image['id']] = ( + glance_v2.TempUrlCacheElement( + url=(CONF.glance.swift_endpoint_url + path + + query % old_exp_time), + url_expires_at=old_exp_time) + ) + + new_exp_time = int(time.time()) + 1200 + tempurl_mock.return_value = ( + path + query % new_exp_time) + + self.glance_service._validate_temp_url_config = mock.Mock() + + fresh_temp_url = self.glance_service.swift_temp_url( + image_info=fake_image) + + self.assertEqual(CONF.glance.swift_endpoint_url + + tempurl_mock.return_value, + fresh_temp_url) + tempurl_mock.assert_called_with( + path=path, + seconds=CONF.glance.swift_temp_url_duration, + key=CONF.glance.swift_temp_url_key, + method='GET') + self.assertEqual( + (fresh_temp_url, new_exp_time), + self.glance_service._cache[fake_image['id']]) + + def test_remove_expired_items_from_cache(self): + expired_items = { + uuidutils.generate_uuid(): glance_v2.TempUrlCacheElement( + 'fake-url-1', + int(time.time()) - 10 + ), + uuidutils.generate_uuid(): glance_v2.TempUrlCacheElement( + 'fake-url-2', + int(time.time()) + 90 # Agent won't be able to start in time + ) + } + valid_items = { + uuidutils.generate_uuid(): glance_v2.TempUrlCacheElement( + 'fake-url-3', + int(time.time()) + 1000 + ), + uuidutils.generate_uuid(): glance_v2.TempUrlCacheElement( + 'fake-url-4', + int(time.time()) + 2000 + ) + } + self.glance_service._cache.update(expired_items) + self.glance_service._cache.update(valid_items) + self.glance_service._remove_expired_items_from_cache() + for uuid in valid_items: + self.assertEqual(valid_items[uuid], + self.glance_service._cache[uuid]) + for uuid in expired_items: + self.assertNotIn(uuid, self.glance_service._cache) + + @mock.patch('swiftclient.utils.generate_temp_url', autospec=True) + def _test__generate_temp_url(self, fake_image, tempurl_mock): + path = ('/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30' + '/glance' + '/%s' % fake_image['id']) + tempurl_mock.return_value = ( + path + '?temp_url_sig=hmacsig&temp_url_expires=1400001200') + + self.glance_service._validate_temp_url_config = mock.Mock() + + temp_url = self.glance_service._generate_temp_url( + path, seconds=CONF.glance.swift_temp_url_duration, + key=CONF.glance.swift_temp_url_key, method='GET', + endpoint=CONF.glance.swift_endpoint_url, + image_id=fake_image['id'] + ) + + self.assertEqual(CONF.glance.swift_endpoint_url + + tempurl_mock.return_value, + temp_url) + tempurl_mock.assert_called_with( + path=path, + seconds=CONF.glance.swift_temp_url_duration, + key=CONF.glance.swift_temp_url_key, + method='GET') + + def test_swift_temp_url_cache_enabled(self): + fake_image = { + 'id': uuidutils.generate_uuid() + } + rm_expired = mock.Mock() + self.glance_service._remove_expired_items_from_cache = rm_expired + self._test__generate_temp_url(fake_image) + rm_expired.assert_called_once_with() + self.assertIn(fake_image['id'], self.glance_service._cache) + + def test_swift_temp_url_cache_disabled(self): + self.config(swift_temp_url_cache_enabled=False, + group='glance') + fake_image = { + 'id': uuidutils.generate_uuid() + } + rm_expired = mock.Mock() + self.glance_service._remove_expired_items_from_cache = rm_expired + self._test__generate_temp_url(fake_image) + self.assertFalse(rm_expired.called) + self.assertNotIn(fake_image['id'], self.glance_service._cache) + + class TestGlanceUrl(base.TestCase): def test_generate_glance_http_url(self):