diff --git a/devstack/lib/ironic b/devstack/lib/ironic index 0600ab3dc3..b1a4dba0e8 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -1198,21 +1198,13 @@ function configure_ironic_conductor { # directly from glance, and don't set them where the image is pushed # over iSCSI. if is_glance_configuration_required; then - if [[ "$SWIFT_ENABLE_TEMPURLS" == "True" ]] ; then - iniset $IRONIC_CONF_FILE glance swift_temp_url_key $SWIFT_TEMPURL_KEY - else + if [[ "$SWIFT_ENABLE_TEMPURLS" == "False" ]] ; then die $LINENO "SWIFT_ENABLE_TEMPURLS must be True. This is " \ "required either because IRONIC_DEPLOY_DRIVER was " \ "set to some agent_* driver OR configuration of " \ "Glance with Swift was explicitly requested with " \ "IRONIC_CONFIGURE_GLANCE_WITH_SWIFT=True" fi - iniset $IRONIC_CONF_FILE glance swift_endpoint_url ${SWIFT_SERVICE_PROTOCOL}://${SERVICE_HOST}:${SWIFT_DEFAULT_BIND_PORT:-8080} - iniset $IRONIC_CONF_FILE glance swift_api_version v1 - local tenant_id - tenant_id=$(get_or_create_project $SERVICE_PROJECT_NAME default) - iniset $IRONIC_CONF_FILE glance swift_account AUTH_${tenant_id} - iniset $IRONIC_CONF_FILE glance swift_container glance iniset $IRONIC_CONF_FILE glance swift_temp_url_duration 3600 fi diff --git a/doc/source/install/configure-glance-swift.rst b/doc/source/install/configure-glance-swift.rst index 13e67f435d..d707475343 100644 --- a/doc/source/install/configure-glance-swift.rst +++ b/doc/source/install/configure-glance-swift.rst @@ -50,18 +50,18 @@ and Object Storage service as described below. $ openstack object store account set --property Temp-Url-Key=secret +#. Optionally, configure the ironic-conductor service. The default + configuration assumes that: -#. Configure the ironic-conductor service. - The configuration file is typically located at - ``/etc/ironic/ironic.conf``. - Some of the required values are available in the response of an - ``openstack object store account show`` command; - others have to match those configured in Image and Object Store services - configuration files. Below is a example of a minimal set of configuration - options to specify when Object Storage service is provided by swift - (check configuration file sample included within ironic - code ``etc/ironic/ironic.conf.sample`` for full list of available options - and their detailed descriptions): + #. the Object Storage service is implemented by swift_, + #. the Object Storage service URL is available from the service catalog, + #. the project, used by the Image service to access the Object Storage, is + the same as the project, used by the Bare Metal service to access it, + #. the container, used by the Image service, is called ``glance``. + + If any of these assumptions do not hold, you may want to change your + configuration file (typically located at ``/etc/ironic/ironic.conf``), + for example: .. code-block:: ini @@ -74,3 +74,5 @@ and Object Storage service as described below. swift_temp_url_key = secret #. (Re)start the ironic-conductor service. + +.. _swift: https://docs.openstack.org/swift/latest/ diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index e821dcbf6d..8490ff7d7d 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -1756,10 +1756,11 @@ # The account that Glance uses to communicate with Swift. The # format is "AUTH_uuid". "uuid" is the UUID for the account -# configured in the glance-api.conf. Required for temporary -# URLs when Glance backend is Swift. For example: -# "AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30". Swift temporary -# URL format: +# configured in the glance-api.conf. For example: +# "AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30". If not set, the +# default value is calculated based on the ID of the project +# used to access Swift (as set in the [swift] section). Swift +# temporary URL format: # "endpoint_url/api_version/[account/]container/object_id" # (string value) #swift_account = @@ -1783,8 +1784,8 @@ # not include trailing "/". For example, use # "https://swift.example.com". If using RADOS Gateway, # endpoint may also contain /swift path; if it does not, it -# will be appended. Required for temporary URLs. (string -# value) +# will be appended. Used for temporary URLs, will be fetched +# from the service catalog, if not provided. (string value) #swift_endpoint_url = # This should match a config by the same name in the Glance @@ -1823,7 +1824,9 @@ #swift_temp_url_expected_download_start_delay = 0 # The secret token given to Swift to allow temporary URL -# downloads. Required for temporary URLs. (string value) +# downloads. Required for temporary URLs. For the Swift +# backend, the key on the service project (as set in the +# [swift] section) is used by default. (string value) #swift_temp_url_key = # Tenant ID (string value) diff --git a/ironic/common/glance_service/v2/image_service.py b/ironic/common/glance_service/v2/image_service.py index 0e0d9a8b0d..78c9165a60 100644 --- a/ironic/common/glance_service/v2/image_service.py +++ b/ironic/common/glance_service/v2/image_service.py @@ -14,6 +14,7 @@ # under the License. import collections +import re import time from oslo_utils import uuidutils @@ -25,6 +26,8 @@ from ironic.common.glance_service import base_image_service from ironic.common.glance_service import service from ironic.common.glance_service import service_utils from ironic.common.i18n import _ +from ironic.common import keystone +from ironic.common import swift from ironic.conf import CONF TempUrlCacheElement = collections.namedtuple('TempUrlCacheElement', @@ -126,12 +129,26 @@ class GlanceImageService(base_image_service.BaseImageService, url_fragments = { 'api_version': CONF.glance.swift_api_version, - 'account': CONF.glance.swift_account, 'container': self._get_swift_container(image_id), 'object_id': image_id } endpoint_url = CONF.glance.swift_endpoint_url + if not endpoint_url: + swift_session = swift.get_swift_session() + adapter = keystone.get_adapter('swift', session=swift_session) + endpoint_url = adapter.get_endpoint() + + if not endpoint_url: + raise exc.MissingParameterValue(_( + 'Swift temporary URLs require a Swift endpoint URL, but it ' + 'was not found in the service catalog. ' + 'You must provide "swift_endpoint_url" as a config option.')) + + # Strip /v1/AUTH_%(tenant_id)s, if present + endpoint_url = re.sub('/v1/AUTH_[^/]+/?$', '', endpoint_url) + + key = CONF.glance.swift_temp_url_key if CONF.deploy.object_store_endpoint_type == 'radosgw': chunks = urlparse.urlsplit(CONF.glance.swift_endpoint_url) if not chunks.path: @@ -143,8 +160,28 @@ class GlanceImageService(base_image_service.BaseImageService, 'hostname, optional port and optional /swift path ' 'without trailing slash; provided value is: %s') % endpoint_url) + template = '/{api_version}/{container}/{object_id}' else: + account = CONF.glance.swift_account + if not account: + swift_session = swift.get_swift_session() + auth_ref = swift_session.auth.get_auth_ref(swift_session) + account = 'AUTH_%s' % auth_ref.project_id + + if not key: + swift_api = swift.SwiftAPI() + key_header = 'x-account-meta-temp-url-key' + key = swift_api.connection.head_account().get(key_header) + + if not key: + raise exc.MissingParameterValue(_( + 'Swift temporary URLs require a shared secret to be ' + 'created. You must provide "swift_temp_url_key" as a ' + 'config option or pre-generate the key on the project ' + 'used to access Swift.')) + + url_fragments['account'] = account template = '/{api_version}/{account}/{container}/{object_id}' url_path = template.format(**url_fragments) @@ -152,7 +189,7 @@ class GlanceImageService(base_image_service.BaseImageService, return self._generate_temp_url( path=url_path, seconds=CONF.glance.swift_temp_url_duration, - key=CONF.glance.swift_temp_url_key, + key=key, method='GET', endpoint=endpoint_url, image_id=image_id @@ -160,19 +197,11 @@ class GlanceImageService(base_image_service.BaseImageService, def _validate_temp_url_config(self): """Validate the required settings for a temporary URL.""" - if not CONF.glance.swift_temp_url_key: + if (not CONF.glance.swift_temp_url_key and + CONF.deploy.object_store_endpoint_type != 'swift'): raise exc.MissingParameterValue(_( 'Swift temporary URLs require a shared secret to be created. ' 'You must provide "swift_temp_url_key" as a config option.')) - if not CONF.glance.swift_endpoint_url: - raise exc.MissingParameterValue(_( - 'Swift temporary URLs require a Swift endpoint URL. ' - 'You must provide "swift_endpoint_url" as a config option.')) - if (not CONF.glance.swift_account and - CONF.deploy.object_store_endpoint_type == 'swift'): - 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 < CONF.glance.swift_temp_url_expected_download_start_delay): raise exc.InvalidParameterValue(_( diff --git a/ironic/common/swift.py b/ironic/common/swift.py index 6d07ed5386..b67df2a40b 100644 --- a/ironic/common/swift.py +++ b/ironic/common/swift.py @@ -29,7 +29,7 @@ from ironic.conf import CONF _SWIFT_SESSION = None -def _get_swift_session(): +def get_swift_session(): global _SWIFT_SESSION if not _SWIFT_SESSION: auth = keystone.get_auth('swift') @@ -40,6 +40,9 @@ def _get_swift_session(): class SwiftAPI(object): """API for communicating with Swift.""" + connection = None + """Underlying Swift connection object.""" + def __init__(self): """Initialize the connection with swift or radosgw @@ -64,7 +67,7 @@ class SwiftAPI(object): # support. # TODO(pas-ha) pass the context here and use token from context # with service auth - params['session'] = session = _get_swift_session() + params['session'] = session = get_swift_session() adapter = keystone.get_adapter('swift', session=session) params['os_options'] = { 'object_storage_url': adapter.get_endpoint()} diff --git a/ironic/conf/glance.py b/ironic/conf/glance.py index 089bf839d2..8ba3878f2e 100644 --- a/ironic/conf/glance.py +++ b/ironic/conf/glance.py @@ -33,7 +33,9 @@ opts = [ # radosgw-admin user modify --uid=user --temp-url-key=secretkey cfg.StrOpt('swift_temp_url_key', help=_('The secret token given to Swift to allow temporary URL ' - 'downloads. Required for temporary URLs.'), + 'downloads. Required for temporary URLs. For the ' + 'Swift backend, the key on the service project (as set ' + 'in the [swift] section) is used by default.'), secret=True), cfg.IntOpt('swift_temp_url_duration', default=1200, @@ -70,7 +72,8 @@ opts = [ 'Do not include trailing "/". ' 'For example, use "https://swift.example.com". If using RADOS ' 'Gateway, endpoint may also contain /swift path; if it does ' - 'not, it will be appended. Required for temporary URLs.')), + 'not, it will be appended. Used for temporary URLs, will ' + 'be fetched from the service catalog, if not provided.')), cfg.StrOpt( 'swift_api_version', default='v1', @@ -82,9 +85,10 @@ opts = [ help=_('The account that Glance uses to communicate with ' 'Swift. The format is "AUTH_uuid". "uuid" is the ' 'UUID for the account configured in the glance-api.conf. ' - 'Required for temporary URLs when Glance backend is Swift. ' 'For example: "AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30". ' - 'Swift temporary URL format: ' + 'If not set, the default value is calculated based on the ID ' + 'of the project used to access Swift (as set in the [swift] ' + 'section). Swift temporary URL format: ' '"endpoint_url/api_version/[account/]container/object_id"')), cfg.StrOpt( 'swift_container', diff --git a/ironic/tests/unit/common/test_glance_service.py b/ironic/tests/unit/common/test_glance_service.py index b449ee7592..7e135fb403 100644 --- a/ironic/tests/unit/common/test_glance_service.py +++ b/ironic/tests/unit/common/test_glance_service.py @@ -527,6 +527,134 @@ class TestGlanceSwiftTempURL(base.TestCase): key=CONF.glance.swift_temp_url_key, method='GET') + @mock.patch('ironic.common.keystone.get_adapter', autospec=True) + @mock.patch('swiftclient.utils.generate_temp_url', autospec=True) + def test_swift_temp_url_endpoint_detected(self, tempurl_mock, + adapter_mock): + self.config(swift_endpoint_url=None, group='glance') + + path = ('/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30' + '/glance' + '/757274c4-2856-4bd2-bb20-9a4a231e187b') + tempurl_mock.return_value = ( + path + '?temp_url_sig=hmacsig&temp_url_expires=1400001200') + endpoint = 'http://another.example.com:8080' + adapter_mock.return_value.get_endpoint.return_value = endpoint + + self.service._validate_temp_url_config = mock.Mock() + + temp_url = self.service.swift_temp_url(image_info=self.fake_image) + + self.assertEqual(endpoint + 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') + + @mock.patch('ironic.common.keystone.get_adapter', autospec=True) + @mock.patch('swiftclient.utils.generate_temp_url', autospec=True) + def test_swift_temp_url_endpoint_with_suffix(self, tempurl_mock, + adapter_mock): + self.config(swift_endpoint_url=None, group='glance') + + path = ('/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30' + '/glance' + '/757274c4-2856-4bd2-bb20-9a4a231e187b') + tempurl_mock.return_value = ( + path + '?temp_url_sig=hmacsig&temp_url_expires=1400001200') + endpoint = 'http://another.example.com:8080' + adapter_mock.return_value.get_endpoint.return_value = ( + endpoint + '/v1/AUTH_foobar') + + self.service._validate_temp_url_config = mock.Mock() + + temp_url = self.service.swift_temp_url(image_info=self.fake_image) + + self.assertEqual(endpoint + 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') + + @mock.patch('ironic.common.swift.get_swift_session', autospec=True) + @mock.patch('swiftclient.utils.generate_temp_url', autospec=True) + def test_swift_temp_url_account_detected(self, tempurl_mock, swift_mock): + self.config(swift_account=None, group='glance') + + path = ('/v1/AUTH_42/glance' + '/757274c4-2856-4bd2-bb20-9a4a231e187b') + tempurl_mock.return_value = ( + path + '?temp_url_sig=hmacsig&temp_url_expires=1400001200') + auth_ref = swift_mock.return_value.auth.get_auth_ref.return_value + auth_ref.project_id = '42' + + self.service._validate_temp_url_config = mock.Mock() + + temp_url = self.service.swift_temp_url(image_info=self.fake_image) + + 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') + swift_mock.assert_called_once_with() + + @mock.patch('ironic.common.swift.SwiftAPI', autospec=True) + @mock.patch('swiftclient.utils.generate_temp_url', autospec=True) + def test_swift_temp_url_key_detected(self, tempurl_mock, swift_mock): + self.config(swift_temp_url_key=None, group='glance') + + path = ('/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30' + '/glance' + '/757274c4-2856-4bd2-bb20-9a4a231e187b') + tempurl_mock.return_value = ( + path + '?temp_url_sig=hmacsig&temp_url_expires=1400001200') + conn = swift_mock.return_value.connection + conn.head_account.return_value = { + 'x-account-meta-temp-url-key': 'secret' + } + + self.service._validate_temp_url_config = mock.Mock() + + temp_url = self.service.swift_temp_url(image_info=self.fake_image) + + 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='secret', + method='GET') + conn.head_account.assert_called_once_with() + + @mock.patch('ironic.common.swift.SwiftAPI', autospec=True) + @mock.patch('swiftclient.utils.generate_temp_url', autospec=True) + def test_swift_temp_url_no_key_detected(self, tempurl_mock, swift_mock): + self.config(swift_temp_url_key=None, group='glance') + + path = ('/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30' + '/glance' + '/757274c4-2856-4bd2-bb20-9a4a231e187b') + tempurl_mock.return_value = ( + path + '?temp_url_sig=hmacsig&temp_url_expires=1400001200') + conn = swift_mock.return_value.connection + conn.head_account.return_value = {} + + self.service._validate_temp_url_config = mock.Mock() + + self.assertRaises(exception.InvalidParameterValue, + self.service.swift_temp_url, + image_info=self.fake_image) + conn.head_account.assert_called_once_with() + @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() @@ -630,18 +758,14 @@ class TestGlanceSwiftTempURL(base.TestCase): def test__validate_temp_url_config(self): self.service._validate_temp_url_config() + def test__validate_temp_url_key_no_exception(self): + self.config(swift_temp_url_key=None, group='glance') + self.config(object_store_endpoint_type='swift', group='deploy') + self.service._validate_temp_url_config() + def test__validate_temp_url_key_exception(self): self.config(swift_temp_url_key=None, group='glance') - self.assertRaises(exception.MissingParameterValue, - self.service._validate_temp_url_config) - - def test__validate_temp_url_endpoint_config_exception(self): - self.config(swift_endpoint_url=None, group='glance') - self.assertRaises(exception.MissingParameterValue, - self.service._validate_temp_url_config) - - def test__validate_temp_url_account_exception(self): - self.config(swift_account=None, group='glance') + self.config(object_store_endpoint_type='radosgw', group='deploy') self.assertRaises(exception.MissingParameterValue, self.service._validate_temp_url_config) diff --git a/ironic/tests/unit/common/test_swift.py b/ironic/tests/unit/common/test_swift.py index a9dd404743..af340aab1d 100644 --- a/ironic/tests/unit/common/test_swift.py +++ b/ironic/tests/unit/common/test_swift.py @@ -32,7 +32,7 @@ if six.PY3: file = io.BytesIO -@mock.patch.object(swift, '_get_swift_session', autospec=True, +@mock.patch.object(swift, 'get_swift_session', autospec=True, return_value=mock.Mock(verify=False, cert=('spam', 'ham'), timeout=42)) @mock.patch.object(swift_client, 'Connection', autospec=True) diff --git a/releasenotes/notes/default-swift_account-b008d08e85bdf154.yaml b/releasenotes/notes/default-swift_account-b008d08e85bdf154.yaml new file mode 100644 index 0000000000..c09ec78a0e --- /dev/null +++ b/releasenotes/notes/default-swift_account-b008d08e85bdf154.yaml @@ -0,0 +1,15 @@ +features: + - | + If the ``[glance]swift_account`` option is not set, the default value is + now calculated based on the ID of the project used to access the object + store. Previously this option was required. This change does not affect + using RadosGW as an object store backend. + - | + If the ``[glance]swift_temp_url_key`` option is not set, ironic now tries + to fetch the key from the project used to access swift (often + called ``service``). This change does not affect using RadosGW as an + object store backend. + - | + If the ``[glance]swift_endpoint_url`` option is not set, ironic now tries + to fetch the Object Store service URL from the service catalog. The + ``/v1/AUTH_*`` suffix is stripped, if present.