Replace swiftclient usage with openstacksdk

Object create/delete operations translate clearly from swiftclient to
the SDK. Switching the temp URL handling is a little more disruptive but
the result is slightly more centralized and enables key rotation.

Change-Id: I8df2f032224bd5e540139a798a7ab76a1aeebb06
Closes-Bug: #2042493
This commit is contained in:
Steve Baker 2023-11-06 10:01:24 +13:00
parent 16a806f941
commit 2db444bce1
11 changed files with 288 additions and 325 deletions

View File

@ -25,7 +25,6 @@ from glanceclient import client
from glanceclient import exc as glance_exc
from oslo_log import log
from oslo_utils import uuidutils
from swiftclient import utils as swift_utils
import tenacity
from ironic.common import exception
@ -236,8 +235,9 @@ class GlanceImageService(object):
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)
swiftapi = swift.SwiftAPI()
path = swiftapi.generate_temp_url(
path=path, timeout=seconds, temp_url_key=key, method=method)
temp_url = '{endpoint_url}{url_path}'.format(
endpoint_url=endpoint, url_path=path)
@ -321,8 +321,7 @@ class GlanceImageService(object):
if not key:
swift_api = swift.SwiftAPI()
key_header = 'x-account-meta-temp-url-key'
key = swift_api.connection.head_account().get(key_header)
key = swift_api.get_temp_url_key()
if not key:
raise exception.MissingParameterValue(_(

View File

@ -14,18 +14,19 @@
# License for the specific language governing permissions and limitations
# under the License.
from http import client as http_client
from urllib import parse as urlparse
from swiftclient import client as swift_client
from swiftclient import exceptions as swift_exceptions
from swiftclient import utils as swift_utils
import openstack
from openstack.connection import exceptions as openstack_exc
from oslo_log import log
from ironic.common import exception
from ironic.common.i18n import _
from ironic.common import keystone
from ironic.conf import CONF
LOG = log.getLogger(__name__)
_SWIFT_SESSION = None
@ -40,45 +41,11 @@ 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
:raises: ConfigInvalid if required keystone authorization credentials
with swift are missing.
"""
params = {'retries': CONF.swift.swift_max_retries}
# NOTE(pas-ha) swiftclient still (as of 3.3.0) does not use
# (adapter-based) SessionClient, and uses the passed in session
# only to resolve endpoint and get a token,
# but not to make further requests to Swift itself (LP 1736135).
# Thus we need to deconstruct back all the adapter- and
# session-related args as loaded by keystoneauth from config
# to pass them to the client explicitly.
# TODO(pas-ha) re-write this when swiftclient is brought on par
# with other OS clients re auth plugins, sessions and adapters
# support.
# TODO(pas-ha) pass the context here and use token from context
# with service auth
params['session'] = session = get_swift_session()
endpoint = keystone.get_endpoint('swift', session=session)
params['os_options'] = {'object_storage_url': endpoint}
# deconstruct back session-related options
params['timeout'] = session.timeout
if session.verify is False:
params['insecure'] = True
elif isinstance(session.verify, str):
params['cacert'] = session.verify
if session.cert:
# NOTE(pas-ha) although setting cert as path to single file
# with both client cert and key is supported by Session,
# keystoneauth loading always sets the session.cert
# as tuple of cert and key.
params['cert'], params['cert_key'] = session.cert
self.connection = swift_client.Connection(**params)
"""Initialize the connection with swift"""
self.connection = openstack.connection.Connection(
session=get_swift_session(),
oslo_conf=CONF)
def create_object(self, container, obj, filename,
object_headers=None):
@ -92,29 +59,28 @@ class SwiftAPI(object):
:raises: SwiftOperationError, if any operation with Swift fails.
"""
try:
self.connection.put_container(container)
except swift_exceptions.ClientException as e:
self.connection.create_container(container)
except openstack_exc.OpenStackCloudException as e:
operation = _("put container")
raise exception.SwiftOperationError(operation=operation, error=e)
with open(filename, "rb") as fileobj:
if object_headers is None:
object_headers = {}
try:
obj_uuid = self.connection.put_object(container,
obj,
fileobj,
headers=object_headers)
except swift_exceptions.ClientException as e:
obj_uuid = self.connection.create_object(
container, obj, filename=filename, **object_headers)
except openstack_exc.OpenStackCloudException as e:
operation = _("put object")
raise exception.SwiftOperationError(operation=operation,
error=e)
return obj_uuid
def create_object_from_data(self, object, data, container):
def create_object_from_data(self, obj, data, container):
"""Uploads a given string to Swift.
:param object: The name of the object in Swift
:param obj: The name of the object in Swift
:param data: string data to put in the object
:param container: The name of the container for the object.
Defaults to the value set in the configuration options.
@ -122,20 +88,24 @@ class SwiftAPI(object):
:raises: utils.Error, if any operation with Swift fails.
"""
try:
self.connection.put_container(container)
except swift_exceptions.ClientException as e:
self.connection.create_container(container)
except openstack_exc.OpenStackCloudException as e:
operation = _("put container")
raise exception.SwiftOperationError(operation=operation, error=e)
try:
obj_uuid = self.connection.create_object(
container, object, data=data)
except swift_exceptions.ClientException as e:
container, obj, data=data)
except openstack_exc.OpenStackCloudException as e:
operation = _("put object")
raise exception.SwiftOperationError(operation=operation, error=e)
return obj_uuid
def get_temp_url_key(self):
"""Get the best temporary url key from the account headers."""
return self.connection.object_store.get_temp_url_key()
def get_temp_url(self, container, obj, timeout):
"""Returns the temp url for the given Swift object.
@ -147,27 +117,26 @@ class SwiftAPI(object):
:returns: The temp url for the object.
:raises: SwiftOperationError, if any operation with Swift fails.
"""
try:
account_info = self.connection.head_account()
except swift_exceptions.ClientException as e:
operation = _("head account")
raise exception.SwiftOperationError(operation=operation,
error=e)
parse_result = urlparse.urlparse(self.connection.url)
endpoint = keystone.get_endpoint('swift', session=get_swift_session())
parse_result = urlparse.urlparse(endpoint)
swift_object_path = '/'.join((parse_result.path, container, obj))
temp_url_key = account_info.get('x-account-meta-temp-url-key')
temp_url_key = self.get_temp_url_key()
if not temp_url_key:
raise exception.MissingParameterValue(_(
'Swift temporary URLs require a shared secret to be '
'created. You must provide pre-generate the key on '
'the project used to access Swift.'))
url_path = swift_utils.generate_temp_url(swift_object_path, timeout,
temp_url_key, 'GET')
url_path = self.generate_temp_url(
swift_object_path, timeout, 'GET', temp_url_key=temp_url_key)
return urlparse.urlunparse(
(parse_result.scheme, parse_result.netloc, url_path,
None, None, None))
def generate_temp_url(self, path, timeout, method, temp_url_key):
"""Returns the temp url for a given path"""
return self.connection.object_store.generate_temp_url(
path, timeout, method, temp_url_key=temp_url_key)
def get_object(self, object, container):
"""Downloads a given object from Swift.
@ -178,8 +147,9 @@ class SwiftAPI(object):
:raises: utils.Error, if the Swift operation fails.
"""
try:
obj = self.connection.download_object(object, container=container)
except swift_exceptions.ClientException as e:
headers, obj = self.connection.get_object(
object, container=container)
except openstack_exc.OpenStackCloudException as e:
operation = _("get object")
raise exception.SwiftOperationError(operation=operation, error=e)
@ -196,42 +166,11 @@ class SwiftAPI(object):
"""
try:
self.connection.delete_object(container, obj)
except swift_exceptions.ClientException as e:
except openstack_exc.OpenStackCloudException as e:
operation = _("delete object")
if e.http_status == http_client.NOT_FOUND:
if isinstance(e, openstack_exc.ResourceNotFound):
raise exception.SwiftObjectNotFoundError(obj=obj,
container=container,
operation=operation)
raise exception.SwiftOperationError(operation=operation, error=e)
def head_object(self, container, obj):
"""Retrieves the information about the given Swift object.
:param container: The name of the container in which Swift object
is placed.
:param obj: The name of the object in Swift
:returns: The information about the object as returned by
Swift client's head_object call.
:raises: SwiftOperationError, if operation with Swift fails.
"""
try:
return self.connection.head_object(container, obj)
except swift_exceptions.ClientException as e:
operation = _("head object")
raise exception.SwiftOperationError(operation=operation, error=e)
def update_object_meta(self, container, obj, object_headers):
"""Update the metadata of a given Swift object.
:param container: The name of the container in which Swift object
is placed.
:param obj: The name of the object in Swift
:param object_headers: the headers for the object to pass to Swift
:raises: SwiftOperationError, if operation with Swift fails.
"""
try:
self.connection.post_object(container, obj, object_headers)
except swift_exceptions.ClientException as e:
operation = _("post object")
raise exception.SwiftOperationError(operation=operation, error=e)

View File

@ -14,23 +14,12 @@
# License for the specific language governing permissions and limitations
# under the License.
from oslo_config import cfg
from ironic.common.i18n import _
from ironic.conf import auth
opts = [
cfg.IntOpt('swift_max_retries',
default=2,
help=_('Maximum number of times to retry a Swift request, '
'before failing.'))
]
def register_opts(conf):
conf.register_opts(opts, group='swift')
auth.register_auth_opts(conf, 'swift', service_type='object-store')
def list_opts():
return auth.add_auth_opts(opts, service_type='object-store')
return auth.add_auth_opts([], service_type='object-store')

View File

@ -18,7 +18,6 @@ import urllib
from oslo_log import log as logging
from oslo_utils import netutils
import swiftclient.exceptions
from ironic.common import exception
from ironic.common import states
@ -91,8 +90,8 @@ def clean_up_swift_entries(task):
plugin_obj_name = f'{_OBJECT_NAME_PREFIX}-{task.node.uuid}-plugin'
try:
swift_api.delete_object(inventory_obj_name, container)
except swiftclient.exceptions.ClientException as e:
if e.http_status != 404:
except exception.SwiftOperationError as e:
if not isinstance(e, exception.SwiftObjectNotFoundError):
LOG.error("Object %(obj)s in container %(cont)s with inventory "
"for node %(node)s failed to be deleted: %(e)s",
{'obj': inventory_obj_name, 'node': task.node.uuid,
@ -101,8 +100,8 @@ def clean_up_swift_entries(task):
node=task.node.uuid)
try:
swift_api.delete_object(plugin_obj_name, container)
except swiftclient.exceptions.ClientException as e:
if e.http_status != 404:
except exception.SwiftOperationError as e:
if not isinstance(e, exception.SwiftObjectNotFoundError):
LOG.error("Object %(obj)s in container %(cont)s with plugin data "
"for node %(node)s failed to be deleted: %(e)s",
{'obj': plugin_obj_name, 'node': task.node.uuid,

View File

@ -302,14 +302,13 @@ def cleanup(node):
LOG.debug('For node %(node)s cleaning up files from Swift container '
'%(container)s.',
{'node': node.uuid, 'container': container})
_, objects = swift_api.connection.get_container(container)
objects = swift_api.connection.list_objects(container)
for o in objects:
name = o.get('name')
if name and name.startswith(node.uuid):
if o.name and o.name.startswith(node.uuid):
try:
swift_api.delete_object(container, name)
swift_api.delete_object(container, o.name)
except exception.SwiftOperationError as error:
LOG.warning('For node %(node)s failed to clean up '
'%(object)s. Error: %(error)s',
{'node': node.uuid, 'object': name,
{'node': node.uuid, 'object': o.name,
'error': error})

View File

@ -30,6 +30,7 @@ from ironic.common import context
from ironic.common import exception
from ironic.common.glance_service import image_service
from ironic.common.glance_service import service_utils
from ironic.common import swift
from ironic.tests import base
from ironic.tests.unit import stubs
@ -499,12 +500,13 @@ class TestGlanceSwiftTempURL(base.TestCase):
'id': '757274c4-2856-4bd2-bb20-9a4a231e187b'
}
@mock.patch('swiftclient.utils.generate_temp_url', autospec=True)
def test_swift_temp_url(self, tempurl_mock):
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
def test_swift_temp_url(self, swift_mock):
path = ('/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30'
'/glance'
'/757274c4-2856-4bd2-bb20-9a4a231e187b')
tempurl_mock = swift_mock.return_value.generate_temp_url
tempurl_mock.return_value = (
path + '?temp_url_sig=hmacsig&temp_url_expires=1400001200')
@ -517,19 +519,21 @@ class TestGlanceSwiftTempURL(base.TestCase):
temp_url)
tempurl_mock.assert_called_with(
path=path,
seconds=CONF.glance.swift_temp_url_duration,
key=CONF.glance.swift_temp_url_key,
timeout=CONF.glance.swift_temp_url_duration,
temp_url_key=CONF.glance.swift_temp_url_key,
method='GET')
@mock.patch('ironic.common.swift.get_swift_session', autospec=True)
@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):
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
def test_swift_temp_url_endpoint_detected(self, swift_mock,
adapter_mock, session_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 = swift_mock.return_value.generate_temp_url
tempurl_mock.return_value = (
path + '?temp_url_sig=hmacsig&temp_url_expires=1400001200')
endpoint = 'http://another.example.com:8080'
@ -543,19 +547,21 @@ class TestGlanceSwiftTempURL(base.TestCase):
temp_url)
tempurl_mock.assert_called_with(
path=path,
seconds=CONF.glance.swift_temp_url_duration,
key=CONF.glance.swift_temp_url_key,
timeout=CONF.glance.swift_temp_url_duration,
temp_url_key=CONF.glance.swift_temp_url_key,
method='GET')
@mock.patch('ironic.common.swift.get_swift_session', autospec=True)
@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):
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
def test_swift_temp_url_endpoint_with_suffix(self, swift_mock,
adapter_mock, session_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 = swift_mock.return_value.generate_temp_url
tempurl_mock.return_value = (
path + '?temp_url_sig=hmacsig&temp_url_expires=1400001200')
endpoint = 'http://another.example.com:8080'
@ -570,20 +576,21 @@ class TestGlanceSwiftTempURL(base.TestCase):
temp_url)
tempurl_mock.assert_called_with(
path=path,
seconds=CONF.glance.swift_temp_url_duration,
key=CONF.glance.swift_temp_url_key,
timeout=CONF.glance.swift_temp_url_duration,
temp_url_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):
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
def test_swift_temp_url_account_detected(self, swift_mock, session_mock):
self.config(swift_account=None, group='glance')
path = ('/v1/AUTH_42/glance'
'/757274c4-2856-4bd2-bb20-9a4a231e187b')
tempurl_mock = swift_mock.return_value.generate_temp_url
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 = session_mock.return_value.auth.get_auth_ref.return_value
auth_ref.project_id = '42'
self.service._validate_temp_url_config = mock.Mock()
@ -595,23 +602,24 @@ class TestGlanceSwiftTempURL(base.TestCase):
temp_url)
tempurl_mock.assert_called_with(
path=path,
seconds=CONF.glance.swift_temp_url_duration,
key=CONF.glance.swift_temp_url_key,
timeout=CONF.glance.swift_temp_url_duration,
temp_url_key=CONF.glance.swift_temp_url_key,
method='GET')
swift_mock.assert_called_once_with()
session_mock.assert_called_once_with()
@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_with_prefix(self, tempurl_mock,
swift_mock):
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
def test_swift_temp_url_account_detected_with_prefix(self, swift_mock,
session_mock):
self.config(swift_account=None, group='glance')
self.config(swift_account_prefix='SWIFTPREFIX', group='glance')
path = ('/v1/SWIFTPREFIX_42/glance'
'/757274c4-2856-4bd2-bb20-9a4a231e187b')
tempurl_mock = swift_mock.return_value.generate_temp_url
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 = session_mock.return_value.auth.get_auth_ref.return_value
auth_ref.project_id = '42'
self.service._validate_temp_url_config = mock.Mock()
@ -623,23 +631,24 @@ class TestGlanceSwiftTempURL(base.TestCase):
temp_url)
tempurl_mock.assert_called_with(
path=path,
seconds=CONF.glance.swift_temp_url_duration,
key=CONF.glance.swift_temp_url_key,
timeout=CONF.glance.swift_temp_url_duration,
temp_url_key=CONF.glance.swift_temp_url_key,
method='GET')
swift_mock.assert_called_once_with()
session_mock.assert_called_once_with()
@mock.patch('ironic.common.swift.get_swift_session', autospec=True)
@mock.patch('swiftclient.utils.generate_temp_url', autospec=True)
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
def test_swift_temp_url_account_detected_with_prefix_underscore(
self, tempurl_mock, swift_mock):
self, swift_mock, session_mock):
self.config(swift_account=None, group='glance')
self.config(swift_account_prefix='SWIFTPREFIX_', group='glance')
path = ('/v1/SWIFTPREFIX_42/glance'
'/757274c4-2856-4bd2-bb20-9a4a231e187b')
tempurl_mock = swift_mock.return_value.generate_temp_url
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 = session_mock.return_value.auth.get_auth_ref.return_value
auth_ref.project_id = '42'
self.service._validate_temp_url_config = mock.Mock()
@ -651,25 +660,22 @@ class TestGlanceSwiftTempURL(base.TestCase):
temp_url)
tempurl_mock.assert_called_with(
path=path,
seconds=CONF.glance.swift_temp_url_duration,
key=CONF.glance.swift_temp_url_key,
timeout=CONF.glance.swift_temp_url_duration,
temp_url_key=CONF.glance.swift_temp_url_key,
method='GET')
swift_mock.assert_called_once_with()
session_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):
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
def test_swift_temp_url_key_detected(self, 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 = swift_mock.return_value.generate_temp_url
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'
}
swift_mock.return_value.get_temp_url_key.return_value = 'secret'
self.service._validate_temp_url_config = mock.Mock()
@ -680,35 +686,33 @@ class TestGlanceSwiftTempURL(base.TestCase):
temp_url)
tempurl_mock.assert_called_with(
path=path,
seconds=CONF.glance.swift_temp_url_duration,
key='secret',
timeout=CONF.glance.swift_temp_url_duration,
temp_url_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):
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
def test_swift_temp_url_no_key_detected(self, 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 = swift_mock.return_value.generate_temp_url
tempurl_mock.return_value = (
path + '?temp_url_sig=hmacsig&temp_url_expires=1400001200')
conn = swift_mock.return_value.connection
conn.head_account.return_value = {}
swift_mock.return_value.get_temp_url_key.return_value = None
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):
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
def test_swift_temp_url_invalid_image_info(self, swift_mock):
self.service._validate_temp_url_config = mock.Mock()
image_info = {}
tempurl_mock = swift_mock.return_value.generate_temp_url
self.assertRaises(exception.ImageUnacceptable,
self.service.swift_temp_url, image_info)
image_info = {'id': 'not an id'}
@ -716,8 +720,8 @@ class TestGlanceSwiftTempURL(base.TestCase):
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_multiple_containers(self, tempurl_mock):
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
def test_swift_temp_url_multiple_containers(self, swift_mock):
self.config(swift_store_multiple_containers_seed=8,
group='glance')
@ -725,6 +729,7 @@ class TestGlanceSwiftTempURL(base.TestCase):
path = ('/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30'
'/glance_757274c4'
'/757274c4-2856-4bd2-bb20-9a4a231e187b')
tempurl_mock = swift_mock.return_value.generate_temp_url
tempurl_mock.return_value = (
path + '?temp_url_sig=hmacsig&temp_url_expires=1400001200')
@ -737,8 +742,8 @@ class TestGlanceSwiftTempURL(base.TestCase):
temp_url)
tempurl_mock.assert_called_with(
path=path,
seconds=CONF.glance.swift_temp_url_duration,
key=CONF.glance.swift_temp_url_key,
timeout=CONF.glance.swift_temp_url_duration,
temp_url_key=CONF.glance.swift_temp_url_key,
method='GET')
def test_swift_temp_url_url_bad_no_info(self):
@ -804,8 +809,8 @@ class TestSwiftTempUrlCache(base.TestCase):
self.glance_service = image_service.GlanceImageService(
client, context=self.context)
@mock.patch('swiftclient.utils.generate_temp_url', autospec=True)
def test_add_items_to_cache(self, tempurl_mock):
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
def test_add_items_to_cache(self, swift_mock):
fake_image = {
'id': uuidutils.generate_uuid()
}
@ -814,6 +819,7 @@ class TestSwiftTempUrlCache(base.TestCase):
'/glance'
'/%s' % fake_image['id'])
exp_time = int(time.time()) + 1200
tempurl_mock = swift_mock.return_value.generate_temp_url
tempurl_mock.return_value = (
path + '?temp_url_sig=hmacsig&temp_url_expires=%s' % exp_time)
@ -830,14 +836,14 @@ class TestSwiftTempUrlCache(base.TestCase):
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,
timeout=CONF.glance.swift_temp_url_duration,
temp_url_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):
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
def test_return_cached_tempurl(self, swift_mock):
fake_image = {
'id': uuidutils.generate_uuid()
}
@ -850,6 +856,7 @@ class TestSwiftTempUrlCache(base.TestCase):
'?temp_url_sig=hmacsig&temp_url_expires=%(exp_time)s' %
{'uuid': fake_image['id'], 'exp_time': exp_time}
)
tempurl_mock = swift_mock.return_value.generate_temp_url
self.glance_service._cache[fake_image['id']] = (
image_service.TempUrlCacheElement(url=temp_url,
url_expires_at=exp_time)
@ -866,8 +873,8 @@ class TestSwiftTempUrlCache(base.TestCase):
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):
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
def test_do_not_return_expired_tempurls(self, swift_mock):
fake_image = {
'id': uuidutils.generate_uuid()
}
@ -886,6 +893,7 @@ class TestSwiftTempUrlCache(base.TestCase):
)
new_exp_time = int(time.time()) + 1200
tempurl_mock = swift_mock.return_value.generate_temp_url
tempurl_mock.return_value = (
path + query % new_exp_time)
@ -899,8 +907,8 @@ class TestSwiftTempUrlCache(base.TestCase):
fresh_temp_url)
tempurl_mock.assert_called_with(
path=path,
seconds=CONF.glance.swift_temp_url_duration,
key=CONF.glance.swift_temp_url_key,
timeout=CONF.glance.swift_temp_url_duration,
temp_url_key=CONF.glance.swift_temp_url_key,
method='GET')
self.assertEqual(
(fresh_temp_url, new_exp_time),
@ -936,11 +944,12 @@ class TestSwiftTempUrlCache(base.TestCase):
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):
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
def _test__generate_temp_url(self, fake_image, swift_mock):
path = ('/v1/AUTH_a422b2-91f3-2f46-74b7-d7c9e8958f5d30'
'/glance'
'/%s' % fake_image['id'])
tempurl_mock = swift_mock.return_value.generate_temp_url
tempurl_mock.return_value = (
path + '?temp_url_sig=hmacsig&temp_url_expires=1400001200')
@ -958,8 +967,8 @@ class TestSwiftTempUrlCache(base.TestCase):
temp_url)
tempurl_mock.assert_called_with(
path=path,
seconds=CONF.glance.swift_temp_url_duration,
key=CONF.glance.swift_temp_url_key,
timeout=CONF.glance.swift_temp_url_duration,
temp_url_key=CONF.glance.swift_temp_url_key,
method='GET')
def test_swift_temp_url_cache_enabled(self):

View File

@ -12,155 +12,159 @@
# License for the specific language governing permissions and limitations
# under the License.
import builtins
from http import client as http_client
import io
from unittest import mock
from oslo_config import cfg
from swiftclient import client as swift_client
from swiftclient import exceptions as swift_exception
from swiftclient import utils as swift_utils
import openstack
from openstack.connection import exceptions as openstack_exc
from ironic.common import exception
from ironic.common import keystone
from ironic.common import swift
from ironic.conf import CONF
from ironic.tests import base
CONF = cfg.CONF
@mock.patch.object(swift, 'get_swift_session', autospec=True,
return_value=mock.Mock(verify=False, cert=('spam', 'ham'),
return_value=mock.MagicMock(verify=False,
cert=('spam', 'ham'),
timeout=42))
@mock.patch.object(swift_client, 'Connection', autospec=True)
@mock.patch.object(openstack.connection, 'Connection', autospec=True)
class SwiftTestCase(base.TestCase):
def setUp(self):
super(SwiftTestCase, self).setUp()
self.swift_exception = swift_exception.ClientException('', '')
self.swift_exception = openstack_exc.OpenStackCloudException()
def test___init__(self, connection_mock, keystone_mock):
@mock.patch.object(keystone, 'get_auth', autospec=True,
return_value=mock.Mock())
@mock.patch.object(keystone, 'get_endpoint', autospec=True,
return_value='http://example.com/v1')
def test___init__(self, get_endpoint_mock, get_auth_mock, connection_mock,
session_mock):
"""Check if client is properly initialized with swift"""
self.config(group='swift',
endpoint_override='http://example.com/objects')
swift.SwiftAPI()
connection_mock.assert_called_once_with(
retries=2,
session=keystone_mock.return_value,
timeout=42,
insecure=True,
cert='spam',
cert_key='ham',
os_options={'object_storage_url': 'http://example.com/objects'}
)
@mock.patch.object(builtins, 'open', autospec=True)
def test_create_object(self, open_mock, connection_mock, keystone_mock):
swiftapi = swift.SwiftAPI()
connection_obj_mock = connection_mock.return_value
mock_file_handle = mock.MagicMock(spec=io.BytesIO)
mock_file_handle.__enter__.return_value = 'file-object'
open_mock.return_value = mock_file_handle
connection_mock.assert_called_once_with(
session=session_mock.return_value,
oslo_conf=CONF
)
self.assertEqual(connection_mock.return_value, swiftapi.connection)
connection_obj_mock.put_object.return_value = 'object-uuid'
def test_create_object(self, connection_mock, session_mock):
swiftapi = swift.SwiftAPI()
connection = connection_mock.return_value
connection.create_object.return_value = 'object-uuid'
object_uuid = swiftapi.create_object('container', 'object',
'some-file-location')
'some-file-location',
object_headers={'foo': 'bar'})
connection_obj_mock.put_container.assert_called_once_with('container')
connection_obj_mock.put_object.assert_called_once_with(
'container', 'object', 'file-object', headers=None)
connection.create_container.assert_called_once_with('container')
connection.create_object.assert_called_once_with(
'container', 'object', filename='some-file-location', foo='bar')
self.assertEqual('object-uuid', object_uuid)
@mock.patch.object(builtins, 'open', autospec=True)
def test_create_object_create_container_fails(self, open_mock,
connection_mock,
keystone_mock):
def test_create_object_create_container_fails(self, connection_mock,
session_mock):
swiftapi = swift.SwiftAPI()
connection_obj_mock = connection_mock.return_value
connection_obj_mock.put_container.side_effect = self.swift_exception
connection = connection_mock.return_value
connection.create_container.side_effect = self.swift_exception
self.assertRaises(exception.SwiftOperationError,
swiftapi.create_object, 'container',
'object', 'some-file-location')
connection_obj_mock.put_container.assert_called_once_with('container')
self.assertFalse(connection_obj_mock.put_object.called)
connection.create_container.assert_called_once_with('container')
self.assertFalse(connection.create_object.called)
@mock.patch.object(builtins, 'open', autospec=True)
def test_create_object_put_object_fails(self, open_mock, connection_mock,
keystone_mock):
def test_create_object_create_object_fails(self, connection_mock,
session_mock):
swiftapi = swift.SwiftAPI()
mock_file_handle = mock.MagicMock(spec=io.BytesIO)
mock_file_handle.__enter__.return_value = 'file-object'
open_mock.return_value = mock_file_handle
connection_obj_mock = connection_mock.return_value
connection_obj_mock.head_account.side_effect = None
connection_obj_mock.put_object.side_effect = self.swift_exception
connection = connection_mock.return_value
connection.create_object.side_effect = self.swift_exception
self.assertRaises(exception.SwiftOperationError,
swiftapi.create_object, 'container',
'object', 'some-file-location')
connection_obj_mock.put_container.assert_called_once_with('container')
connection_obj_mock.put_object.assert_called_once_with(
'container', 'object', 'file-object', headers=None)
connection.create_container.assert_called_once_with('container')
connection.create_object.assert_called_once_with(
'container', 'object', filename='some-file-location')
@mock.patch.object(swift_utils, 'generate_temp_url', autospec=True)
def test_get_temp_url(self, gen_temp_url_mock, connection_mock,
keystone_mock):
def test_create_object_from_data(self, connection_mock, session_mock):
swiftapi = swift.SwiftAPI()
connection_obj_mock = connection_mock.return_value
connection_obj_mock.url = 'http://host/v1/AUTH_tenant_id'
head_ret_val = {'x-account-meta-temp-url-key': 'secretkey'}
connection_obj_mock.head_account.return_value = head_ret_val
gen_temp_url_mock.return_value = 'temp-url-path'
connection = connection_mock.return_value
connection.create_object.return_value = 'object-uuid'
object_uuid = swiftapi.create_object_from_data(
'object', 'some-data', 'container')
connection.create_container.assert_called_once_with('container')
connection.create_object.assert_called_once_with(
'container', 'object', data='some-data')
self.assertEqual('object-uuid', object_uuid)
def test_create_object_from_data_create_container_fails(
self, connection_mock, session_mock):
swiftapi = swift.SwiftAPI()
connection = connection_mock.return_value
connection.create_container.side_effect = self.swift_exception
self.assertRaises(exception.SwiftOperationError,
swiftapi.create_object_from_data,
'object', 'some-data', 'container')
connection.create_container.assert_called_once_with('container')
self.assertFalse(connection.create_object.called)
def test_create_object_from_data_create_object_fails(self, connection_mock,
session_mock):
swiftapi = swift.SwiftAPI()
connection = connection_mock.return_value
connection.create_object.side_effect = self.swift_exception
self.assertRaises(exception.SwiftOperationError,
swiftapi.create_object_from_data,
'object', 'some-data', 'container')
connection.create_container.assert_called_once_with('container')
connection.create_object.assert_called_once_with(
'container', 'object', data='some-data')
@mock.patch.object(keystone, 'get_auth', autospec=True)
@mock.patch.object(keystone, 'get_endpoint', autospec=True)
def test_get_temp_url(self, get_endpoint_mock, get_auth_mock,
connection_mock, session_mock):
swiftapi = swift.SwiftAPI()
connection = connection_mock.return_value
get_endpoint_mock.return_value = 'http://example.com/v1/AUTH_tenant_id'
object_store = connection.object_store
object_store.get_temp_url_key.return_value = 'secretkey'
object_store.generate_temp_url.return_value = \
'/v1/AUTH_tenant_id/temp-url-path'
temp_url_returned = swiftapi.get_temp_url('container', 'object', 10)
connection_obj_mock.head_account.assert_called_once_with()
object_path_expected = '/v1/AUTH_tenant_id/container/object'
gen_temp_url_mock.assert_called_once_with(object_path_expected, 10,
'secretkey', 'GET')
self.assertEqual('http://host/temp-url-path', temp_url_returned)
object_store.generate_temp_url.assert_called_once_with(
object_path_expected, 10, 'GET', temp_url_key='secretkey')
self.assertEqual('http://example.com/v1/AUTH_tenant_id/temp-url-path',
temp_url_returned)
def test_delete_object(self, connection_mock, keystone_mock):
def test_delete_object(self, connection_mock, session_mock):
swiftapi = swift.SwiftAPI()
connection_obj_mock = connection_mock.return_value
connection = connection_mock.return_value
swiftapi.delete_object('container', 'object')
connection_obj_mock.delete_object.assert_called_once_with('container',
connection.delete_object.assert_called_once_with('container',
'object')
def test_delete_object_exc_resource_not_found(self, connection_mock,
keystone_mock):
session_mock):
swiftapi = swift.SwiftAPI()
exc = swift_exception.ClientException(
"Resource not found", http_status=http_client.NOT_FOUND)
connection_obj_mock = connection_mock.return_value
connection_obj_mock.delete_object.side_effect = exc
exc = openstack_exc.ResourceNotFound(message="Resource not found")
connection = connection_mock.return_value
connection.delete_object.side_effect = exc
self.assertRaises(exception.SwiftObjectNotFoundError,
swiftapi.delete_object, 'container', 'object')
connection_obj_mock.delete_object.assert_called_once_with('container',
connection.delete_object.assert_called_once_with('container',
'object')
def test_delete_object_exc(self, connection_mock, keystone_mock):
def test_delete_object_exc(self, connection_mock, session_mock):
swiftapi = swift.SwiftAPI()
exc = swift_exception.ClientException("Operation error")
connection_obj_mock = connection_mock.return_value
connection_obj_mock.delete_object.side_effect = exc
exc = self.swift_exception
connection = connection_mock.return_value
connection.delete_object.side_effect = exc
self.assertRaises(exception.SwiftOperationError,
swiftapi.delete_object, 'container', 'object')
connection_obj_mock.delete_object.assert_called_once_with('container',
connection.delete_object.assert_called_once_with('container',
'object')
def test_head_object(self, connection_mock, keystone_mock):
swiftapi = swift.SwiftAPI()
connection_obj_mock = connection_mock.return_value
expected_head_result = {'a': 'b'}
connection_obj_mock.head_object.return_value = expected_head_result
actual_head_result = swiftapi.head_object('container', 'object')
connection_obj_mock.head_object.assert_called_once_with('container',
'object')
self.assertEqual(expected_head_result, actual_head_result)
def test_update_object_meta(self, connection_mock, keystone_mock):
swiftapi = swift.SwiftAPI()
connection_obj_mock = connection_mock.return_value
headers = {'a': 'b'}
swiftapi.update_object_meta('container', 'object', headers)
connection_obj_mock.post_object.assert_called_once_with(
'container', 'object', headers)

View File

@ -431,8 +431,12 @@ class FirmwareUtilsTestCase(base.TestCase):
uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3',
driver_internal_info={'firmware_cleanup': ['http', 'swift']})
object_name = '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe'
get_container = mock_swift_api.return_value.connection.get_container
get_container.return_value = (mock.Mock(), [{'name': object_name}])
obj = mock.Mock()
obj.name = object_name
connection = mock.MagicMock()
mock_swift_api.return_value.connection = connection
connection.list_objects.return_value = [obj]
firmware_utils.cleanup(node)
@ -470,8 +474,12 @@ class FirmwareUtilsTestCase(base.TestCase):
uuid='55cdaba0-1123-4622-8b37-bb52dd6285d3',
driver_internal_info={'firmware_cleanup': ['swift']})
object_name = '55cdaba0-1123-4622-8b37-bb52dd6285d3/file.exe'
get_container = mock_swift_api.return_value.connection.get_container
get_container.return_value = (mock.Mock(), [{'name': object_name}])
obj = mock.Mock()
obj.name = object_name
connection = mock.MagicMock()
mock_swift_api.return_value.connection = connection
connection.list_objects.return_value = [obj]
mock_swift_api.return_value.delete_object.side_effect =\
exception.SwiftOperationError

View File

@ -18,7 +18,6 @@ from unittest import mock
from oslo_utils import importutils
from oslo_utils import uuidutils
import swiftclient.exceptions
from ironic.common import context as ironic_context
from ironic.common import exception
@ -125,10 +124,8 @@ class SwiftCleanUp(db_base.DbTestCase):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
swift_obj_mock.delete_object.side_effect = [
swiftclient.exceptions.ClientException("not found",
http_status=404),
swiftclient.exceptions.ClientException("not found",
http_status=404)]
exception.SwiftObjectNotFoundError("not found"),
exception.SwiftObjectNotFoundError("not found")]
utils.clean_up_swift_entries(task)
@mock.patch.object(swift, 'SwiftAPI', autospec=True)
@ -140,10 +137,8 @@ class SwiftCleanUp(db_base.DbTestCase):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
swift_obj_mock.delete_object.side_effect = [
swiftclient.exceptions.ClientException("failed",
http_status=417),
swiftclient.exceptions.ClientException("not found",
http_status=404)]
exception.SwiftOperationError("failed"),
exception.SwiftObjectNotFoundError("not found")]
self.assertRaises(exception.SwiftObjectStillExists,
utils.clean_up_swift_entries, task)
@ -156,10 +151,8 @@ class SwiftCleanUp(db_base.DbTestCase):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
swift_obj_mock.delete_object.side_effect = [
swiftclient.exceptions.ClientException("failed",
http_status=417),
swiftclient.exceptions.ClientException("failed",
http_status=417)]
exception.SwiftOperationError("failed"),
exception.SwiftOperationError("failed")]
self.assertRaises((exception.SwiftObjectStillExists,
exception.SwiftObjectStillExists),
utils.clean_up_swift_entries, task)

View File

@ -0,0 +1,25 @@
---
features:
- |
Previously the key for building temporary URLs from Swift was taken from the
`x-account-meta-temp-url-key` header in the object store account. Now the
header `x-account-meta-temp-url-key-2` is also checked, which allows
password rotation to occur without breaking old URLs.
This applies to the following temporary URL scenarios:
* Temp URL image transfer from Glance
(when `[glance]swift_temp_url_key` is not set)
* Publishing an image with the Swift publisher
(`[redfish]use_swift=True` or `[ilo]use_web_server_for_images=False`)
* Storing the config drive in Swift
(`[deploy]configdrive_use_object_store=True`)
* Fetching Swift stored firmware update payloads.
upgrade:
- |
`python-swiftclient` is no longer a dependency, all OpenStack Swift
operations are now down using `openstacksdk`.
Configuration option `[swift]swift_max_retries` has been removed and any
custom value will no longer have any effect on failed object-store
operations.

View File

@ -15,7 +15,6 @@ python-cinderclient!=4.0.0,>=3.3.0 # Apache-2.0
python-glanceclient>=2.8.0 # Apache-2.0
keystoneauth1>=4.2.0 # Apache-2.0
ironic-lib>=5.5.0 # Apache-2.0
python-swiftclient>=3.2.0 # Apache-2.0
pytz>=2013.6 # MIT
stevedore>=1.29.0 # Apache-2.0
oslo.concurrency>=4.2.0 # Apache-2.0