From 77c886ab18ba241eaa7418f1e0d095fe6639ae19 Mon Sep 17 00:00:00 2001 From: Tobias Urdin Date: Tue, 3 May 2022 13:27:15 +0000 Subject: [PATCH] backup/swift: Add support sending service user token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds support to the Swift backup driver to send a service user token in the X-Service-Token header when talking to Swift which will support long running processes to continue functioning when the user token is expired if the target supports it. [1] [2] In the patch I'm favoring passing the X-Service-Token from Cinder as a header instead of passing the service user credentials down to the python-swiftclient, it makes more sense to not hand it off. We already have a auth plugin for the service user which ensures that the token is always valid, an invalid token would disrupt the process and cause the long running process to fail. The new config option to enable the service auth in the Swift driver serves the purpose of not enabling the feature by default for deployments already enabling service user for Nova and Glance. I'm working on implementing the X-Service-Token support in Ceph RadosGW's Swift API implementation [3], OpenStack Swift already supports service token. [1] https://specs.openstack.org/openstack/keystone-specs/specs/keystonemiddleware/juno/service-tokens.html [2] https://docs.openstack.org/cinder/latest/configuration/block-storage/service-token.html [3] https://github.com/ceph/ceph/pull/45395 Related-Bug: #1298135 Change-Id: I69a478dc18c18e6d67be83d61c9643afab72c118 --- cinder/backup/drivers/swift.py | 54 +++++++++++++++---- cinder/service_auth.py | 21 +++++--- .../unit/backup/drivers/test_backup_swift.py | 30 +++++++++++ cinder/tests/unit/backup/fake_swift_client.py | 10 ++-- .../tests/unit/backup/fake_swift_client2.py | 10 ++-- ...-swift-service-token-9b86e8e73ebd2a22.yaml | 9 ++++ 6 files changed, 107 insertions(+), 27 deletions(-) create mode 100644 releasenotes/notes/cinder-backup-swift-service-token-9b86e8e73ebd2a22.yaml diff --git a/cinder/backup/drivers/swift.py b/cinder/backup/drivers/swift.py index cf93b461290..e1afe705376 100644 --- a/cinder/backup/drivers/swift.py +++ b/cinder/backup/drivers/swift.py @@ -57,6 +57,8 @@ from cinder.backup import chunkeddriver from cinder import exception from cinder.i18n import _ from cinder import interface +from cinder import service_auth +from cinder.utils import retry LOG = logging.getLogger(__name__) @@ -141,6 +143,11 @@ swiftbackup_service_opts = [ default=False, help='Bypass verification of server certificate when ' 'making SSL connection to Swift.'), + cfg.BoolOpt('backup_swift_service_auth', + default=False, + help='Send a X-Service-Token header with service auth ' + 'credentials. If enabled you also must set the ' + 'service_user group and enable send_service_user_token.'), ] CONF = cfg.CONF @@ -174,6 +181,21 @@ class SwiftBackupDriver(chunkeddriver.ChunkedBackupDriver): def get_driver_options(): return swiftbackup_service_opts + @retry(Exception, retries=CONF.backup_swift_retry_attempts, + backoff_rate=CONF.backup_swift_retry_backoff) + def _headers(self, headers=None): + """Add service token to headers if its enabled""" + if not CONF.backup_swift_service_auth: + return headers + + result = headers or {} + + sa_plugin = service_auth.get_service_auth_plugin() + if sa_plugin is not None: + result['X-Service-Token'] = sa_plugin.get_token() + + return result + def initialize(self): self.swift_attempts = CONF.backup_swift_retry_attempts self.swift_backoff = CONF.backup_swift_retry_backoff @@ -274,11 +296,12 @@ class SwiftBackupDriver(chunkeddriver.ChunkedBackupDriver): cacert=CONF.backup_swift_ca_cert_file) class SwiftObjectWriter(object): - def __init__(self, container, object_name, conn): + def __init__(self, container, object_name, conn, headers_func=None): self.container = container self.object_name = object_name self.conn = conn self.data = bytearray() + self.headers_func = headers_func def __enter__(self): return self @@ -292,9 +315,11 @@ class SwiftBackupDriver(chunkeddriver.ChunkedBackupDriver): def close(self): reader = io.BytesIO(self.data) try: + headers = self.headers_func() if self.headers_func else None etag = self.conn.put_object(self.container, self.object_name, reader, - content_length=len(self.data)) + content_length=len(self.data), + headers=headers) except socket.error as err: raise exception.SwiftConnectionFailed(reason=err) md5 = secretutils.md5(self.data, usedforsecurity=False).hexdigest() @@ -306,10 +331,11 @@ class SwiftBackupDriver(chunkeddriver.ChunkedBackupDriver): return md5 class SwiftObjectReader(object): - def __init__(self, container, object_name, conn): + def __init__(self, container, object_name, conn, headers_func=None): self.container = container self.object_name = object_name self.conn = conn + self.headers_func = headers_func def __enter__(self): return self @@ -319,8 +345,10 @@ class SwiftBackupDriver(chunkeddriver.ChunkedBackupDriver): def read(self): try: + headers = self.headers_func() if self.headers_func else None (_resp, body) = self.conn.get_object(self.container, - self.object_name) + self.object_name, + headers=headers) except socket.error as err: raise exception.SwiftConnectionFailed(reason=err) return body @@ -335,14 +363,15 @@ class SwiftBackupDriver(chunkeddriver.ChunkedBackupDriver): existing container. """ try: - self.conn.head_container(container) + self.conn.head_container(container, headers=self._headers()) except swift_exc.ClientException as e: if e.http_status == 404: try: storage_policy = CONF.backup_swift_create_storage_policy headers = ({'X-Storage-Policy': storage_policy} if storage_policy else None) - self.conn.put_container(container, headers=headers) + self.conn.put_container(container, + headers=self._headers(headers)) except socket.error as err: raise exception.SwiftConnectionFailed(reason=err) return @@ -355,9 +384,11 @@ class SwiftBackupDriver(chunkeddriver.ChunkedBackupDriver): def get_container_entries(self, container, prefix): """Get container entry names""" try: + headers = self._headers() swift_objects = self.conn.get_container(container, prefix=prefix, - full_listing=True)[1] + full_listing=True, + headers=headers)[1] except socket.error as err: raise exception.SwiftConnectionFailed(reason=err) swift_object_names = [swift_obj['name'] for swift_obj in swift_objects] @@ -369,7 +400,8 @@ class SwiftBackupDriver(chunkeddriver.ChunkedBackupDriver): Returns a writer object that stores a chunk of volume data in a Swift object store. """ - return self.SwiftObjectWriter(container, object_name, self.conn) + return self.SwiftObjectWriter(container, object_name, self.conn, + self._headers) def get_object_reader(self, container, object_name, extra_metadata=None): """Return reader object. @@ -377,12 +409,14 @@ class SwiftBackupDriver(chunkeddriver.ChunkedBackupDriver): Returns a reader object that retrieves a chunk of backed-up volume data from a Swift object store. """ - return self.SwiftObjectReader(container, object_name, self.conn) + return self.SwiftObjectReader(container, object_name, self.conn, + self._headers) def delete_object(self, container, object_name): """Deletes a backup object from a Swift object store.""" try: - self.conn.delete_object(container, object_name) + self.conn.delete_object(container, object_name, + headers=self._headers()) except socket.error as err: raise exception.SwiftConnectionFailed(reason=err) diff --git a/cinder/service_auth.py b/cinder/service_auth.py index 6c4aaf809db..1e092454e06 100644 --- a/cinder/service_auth.py +++ b/cinder/service_auth.py @@ -52,12 +52,7 @@ def reset_globals(): _SERVICE_AUTH = None -def get_auth_plugin(context, auth=None): - if auth: - user_auth = auth - else: - user_auth = context.get_auth_plugin() - +def get_service_auth_plugin(): if CONF.service_user.send_service_user_token: global _SERVICE_AUTH if not _SERVICE_AUTH: @@ -67,7 +62,19 @@ def get_auth_plugin(context, auth=None): # This can happen if no auth_type is specified, which probably # means there's no auth information in the [service_user] group raise exception.ServiceUserTokenNoAuth() + return _SERVICE_AUTH + return None + + +def get_auth_plugin(context, auth=None): + if auth: + user_auth = auth + else: + user_auth = context.get_auth_plugin() + + service_auth = get_service_auth_plugin() + if service_auth is not None: return service_token.ServiceTokenAuthWrapper( - user_auth=user_auth, service_auth=_SERVICE_AUTH) + user_auth=user_auth, service_auth=service_auth) return user_auth diff --git a/cinder/tests/unit/backup/drivers/test_backup_swift.py b/cinder/tests/unit/backup/drivers/test_backup_swift.py index 8c73d6c74c5..2eadfae2388 100644 --- a/cinder/tests/unit/backup/drivers/test_backup_swift.py +++ b/cinder/tests/unit/backup/drivers/test_backup_swift.py @@ -37,6 +37,7 @@ from cinder import db from cinder import exception from cinder.i18n import _ from cinder import objects +from cinder import service_auth from cinder.tests.unit.backup import fake_swift_client from cinder.tests.unit.backup import fake_swift_client2 from cinder.tests.unit import fake_constants as fake @@ -299,6 +300,35 @@ class BackupSwiftTestCase(test.TestCase): starting_backoff=ANY, cacert=ANY) + def _test_backup_swift_service_auth_headers_no_impact(self): + service = swift_dr.SwiftBackupDriver(self.ctxt) + self.assertIsNone(service._headers()) + current = {'some': 'header'} + self.assertEqual(service._headers(current), current) + + def test_backup_swift_service_auth_headers_disabled(self): + self._test_backup_swift_service_auth_headers_no_impact() + + def test_backup_swift_service_auth_headers_partial_enabled(self): + self.override_config('send_service_user_token', True, + group='service_user') + self._test_backup_swift_service_auth_headers_no_impact() + + @mock.patch.object(service_auth, 'get_service_auth_plugin') + def test_backup_swift_service_auth_headers_enabled(self, mock_plugin): + class FakeServiceAuthPlugin: + def get_token(self): + return "fake" + self.override_config('send_service_user_token', True, + group='service_user') + self.override_config('backup_swift_service_auth', True) + mock_plugin.return_value = FakeServiceAuthPlugin() + service = swift_dr.SwiftBackupDriver(self.ctxt) + expected = {'X-Service-Token': 'fake'} + self.assertEqual(service._headers(), expected) + expected = {'X-Service-Token': 'fake', 'some': 'header'} + self.assertEqual(service._headers({'some': 'header'}), expected) + @mock.patch.object(fake_swift_client.FakeSwiftConnection, 'put_container') def test_default_backup_swift_create_storage_policy(self, mock_put): service = swift_dr.SwiftBackupDriver(self.ctxt) diff --git a/cinder/tests/unit/backup/fake_swift_client.py b/cinder/tests/unit/backup/fake_swift_client.py index 0a9e885a59b..18e2ba6ee8e 100644 --- a/cinder/tests/unit/backup/fake_swift_client.py +++ b/cinder/tests/unit/backup/fake_swift_client.py @@ -37,7 +37,7 @@ class FakeSwiftConnection(object): def __init__(self, *args, **kwargs): pass - def head_container(self, container): + def head_container(self, container, headers=None): if container in ['missing_container', 'missing_container_socket_error_on_put']: raise swift.ClientException('fake exception', @@ -53,17 +53,17 @@ class FakeSwiftConnection(object): if container == 'missing_container_socket_error_on_put': raise socket.error(111, 'ECONNREFUSED') - def get_container(self, container, **kwargs): + def get_container(self, container, headers=None, **kwargs): fake_header = None fake_body = [{'name': 'backup_001'}, {'name': 'backup_002'}, {'name': 'backup_003'}] return fake_header, fake_body - def head_object(self, container, name): + def head_object(self, container, name, headers=None): return {'etag': 'fake-md5-sum'} - def get_object(self, container, name): + def get_object(self, container, name, headers=None): if container == 'socket_error_on_get': raise socket.error(111, 'ECONNREFUSED') if 'metadata' in name: @@ -102,7 +102,7 @@ class FakeSwiftConnection(object): raise socket.error(111, 'ECONNREFUSED') return 'fake-md5-sum' - def delete_object(self, container, name): + def delete_object(self, container, name, headers=None): if container == 'socket_error_on_delete': raise socket.error(111, 'ECONNREFUSED') pass diff --git a/cinder/tests/unit/backup/fake_swift_client2.py b/cinder/tests/unit/backup/fake_swift_client2.py index 512fb392ac4..b15c3f6202d 100644 --- a/cinder/tests/unit/backup/fake_swift_client2.py +++ b/cinder/tests/unit/backup/fake_swift_client2.py @@ -36,7 +36,7 @@ class FakeSwiftConnection2(object): def __init__(self, *args, **kwargs): self.tempdir = tempfile.mkdtemp() - def head_container(self, container): + def head_container(self, container, headers=None): if container == 'missing_container': raise swift.ClientException('fake exception', http_status=http_client.NOT_FOUND) @@ -49,7 +49,7 @@ class FakeSwiftConnection2(object): def put_container(self, container, headers=None): pass - def get_container(self, container, **kwargs): + def get_container(self, container, headers=None, **kwargs): fake_header = None container_dir = tempfile.gettempdir() + '/' + container fake_body = [] @@ -62,10 +62,10 @@ class FakeSwiftConnection2(object): return fake_header, fake_body - def head_object(self, container, name): + def head_object(self, container, name, headers=None): return {'etag': 'fake-md5-sum'} - def get_object(self, container, name): + def get_object(self, container, name, headers=None): if container == 'socket_error_on_get': raise socket.error(111, 'ECONNREFUSED') object_path = tempfile.gettempdir() + '/' + container + '/' + name @@ -80,5 +80,5 @@ class FakeSwiftConnection2(object): object_file.write(reader.read()) return md5(reader.read(), usedforsecurity=False).hexdigest() - def delete_object(self, container, name): + def delete_object(self, container, name, headers=None): pass diff --git a/releasenotes/notes/cinder-backup-swift-service-token-9b86e8e73ebd2a22.yaml b/releasenotes/notes/cinder-backup-swift-service-token-9b86e8e73ebd2a22.yaml new file mode 100644 index 00000000000..e1addcc77c0 --- /dev/null +++ b/releasenotes/notes/cinder-backup-swift-service-token-9b86e8e73ebd2a22.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + The Swift backup driver now supports sending a X-Service-Token header with + a service token when the new ``backup_swift_service_auth`` config option is + enabled. Please note that you still need to configure the ``[service_user]`` + group and also set ``send_service_user_token`` to enable the behavior and not + only the Swift backup driver option. Note ``send_service_user_token`` enables + it globally and will also affect communication with Nova and Glance.