Auto-detect the defaults for [glance]swift_{account,temp_url_key,endpoint_url}

In many cases, including devstack and TripleO, the swift account to use
is based on the project_id of the 'service' project, so it makes sense
to use it as a default.

Similarly, the temporary URL key can be fetches from Swift, using HEAD
on the account used to access it.

In both cases it is assumed that the same project is used by Ironic
and Glance to access Swift. Explicit values have to be provided
otherwise.

Finally, the endpoint URL can be fetched from the service catalog. Care
is taken to strip the /v1/AUTH_<tenant ID> suffix.

Closes-Bug: #1737714
Change-Id: I701899928f0f780939f17aabc59eb90593ba95f0

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
+        if [[ "$SWIFT_ENABLE_TEMPURLS" == "False" ]] ; then
-            iniset $IRONIC_CONF_FILE glance swift_temp_url_key $SWIFT_TEMPURL_KEY
-        else
             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
 

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 Object Storage service is implemented by swift_,
-   The configuration file is typically located at
+   #. the Object Storage service URL is available from the service catalog,
-   ``/etc/ironic/ironic.conf``.
+   #. the project, used by the Image service to access the Object Storage, is
-   Some of the required values are available in the response of an
+      the same as the project, used by the Bare Metal service to access it,
-   ``openstack object store account show`` command;
+   #. the container, used by the Image service, is called ``glance``.
-   others have to match those configured in Image and Object Store services
+
-   configuration files. Below is a example of a minimal set of configuration
+   If any of these assumptions do not hold, you may want to change your
-   options to specify when Object Storage service is provided by swift
+   configuration file (typically located at ``/etc/ironic/ironic.conf``),
-   (check configuration file sample included within ironic
+   for example:
-   code ``etc/ironic/ironic.conf.sample`` for full list of available options
-   and their detailed descriptions):
 
    .. 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/

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
+# configured in the glance-api.conf. For example:
-# URLs when Glance backend is Swift. For example:
+# "AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30". If not set, the
-# "AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30". Swift temporary
+# default value is calculated based on the ID of the project
-# URL format:
+# 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 = <None>
@@ -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
+# will be appended. Used for temporary URLs, will be fetched
-# value)
+# from the service catalog, if not provided. (string value)
 #swift_endpoint_url = <None>
 
 # 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 = <None>
 
 # Tenant ID (string value)

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(_(

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()}

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',

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.config(object_store_endpoint_type='radosgw', group='deploy')
-                          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.assertRaises(exception.MissingParameterValue,
                           self.service._validate_temp_url_config)
 

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)

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.