diff --git a/cinder/backup/drivers/google.py b/cinder/backup/drivers/gcs.py similarity index 89% rename from cinder/backup/drivers/google.py rename to cinder/backup/drivers/gcs.py index c8b190aae1b..65aff80bb93 100644 --- a/cinder/backup/drivers/google.py +++ b/cinder/backup/drivers/gcs.py @@ -27,13 +27,26 @@ Server-centric flow is used for authentication. """ import base64 +from distutils import version import hashlib -import httplib2 +import os +try: + from google.auth import exceptions as gexceptions + from google.oauth2 import service_account + import google_auth_httplib2 +except ImportError: + service_account = google_auth_httplib2 = gexceptions = None + +try: + from oauth2client import client +except ImportError: + client = None + +import googleapiclient from googleapiclient import discovery from googleapiclient import errors from googleapiclient import http -from oauth2client import client from oslo_config import cfg from oslo_log import log as logging from oslo_utils import timeutils @@ -99,6 +112,7 @@ gcsbackup_service_opts = [ CONF = cfg.CONF CONF.register_opts(gcsbackup_service_opts) +OAUTH_EXCEPTIONS = None def gcs_logger(func): @@ -107,7 +121,7 @@ def gcs_logger(func): return func(self, *args, **kwargs) except errors.Error as err: raise exception.GCSApiFailure(reason=err) - except client.Error as err: + except OAUTH_EXCEPTIONS as err: raise exception.GCSOAuth2Failure(reason=err) except Exception as err: raise exception.GCSConnectionFailure(reason=err) @@ -120,8 +134,8 @@ class GoogleBackupDriver(chunkeddriver.ChunkedBackupDriver): """Provides backup, restore and delete of backup objects within GCS.""" def __init__(self, context, db=None): + global OAUTH_EXCEPTIONS backup_bucket = CONF.backup_gcs_bucket - backup_credential = CONF.backup_gcs_credential_file self.gcs_project_id = CONF.backup_gcs_project_id chunk_size_bytes = CONF.backup_gcs_object_size sha_block_size_bytes = CONF.backup_gcs_block_size @@ -131,27 +145,41 @@ class GoogleBackupDriver(chunkeddriver.ChunkedBackupDriver): backup_bucket, enable_progress_timer, db) - credentials = client.GoogleCredentials.from_stream(backup_credential) self.reader_chunk_size = CONF.backup_gcs_reader_chunk_size self.writer_chunk_size = CONF.backup_gcs_writer_chunk_size self.bucket_location = CONF.backup_gcs_bucket_location self.storage_class = CONF.backup_gcs_storage_class self.num_retries = CONF.backup_gcs_num_retries - http_user_agent = http.set_user_agent( - httplib2.Http(proxy_info=self.get_gcs_proxy_info()), - CONF.backup_gcs_user_agent) + + # Set or overwrite environmental proxy variables for httplib2 since + # it's the only mechanism supported when using googleapiclient with + # google-auth + if CONF.backup_gcs_proxy_url: + os.environ['http_proxy'] = CONF.backup_gcs_proxy_url + + backup_credential = CONF.backup_gcs_credential_file + # If we have google client that support google-auth library + # (v1.6.0 or higher) and all required libraries are installed use + # google-auth for the credentials + if (version.LooseVersion(googleapiclient.__version__) >= + version.LooseVersion('1.6.0') and service_account): + creds = service_account.Credentials.from_service_account_file( + backup_credential) + OAUTH_EXCEPTIONS = (gexceptions.RefreshError, + gexceptions.DefaultCredentialsError) + + # Can't use google-auth, use deprecated oauth2client + else: + creds = client.GoogleCredentials.from_stream(backup_credential) + OAUTH_EXCEPTIONS = client.Error + self.conn = discovery.build('storage', 'v1', - http=http_user_agent, - credentials=credentials) + # Avoid log error on oauth2client >= 4.0.0 + cache_discovery=False, + credentials=creds) self.resumable = self.writer_chunk_size != -1 - def get_gcs_proxy_info(self): - if CONF.backup_gcs_proxy_url: - return httplib2.proxy_info_from_url(CONF.backup_gcs_proxy_url) - else: - return httplib2.proxy_info_from_environment() - def check_for_setup_error(self): required_options = ('backup_gcs_bucket', 'backup_gcs_credential_file', 'backup_gcs_project_id') diff --git a/cinder/backup/manager.py b/cinder/backup/manager.py index 29f80adce50..64b34e1e7ab 100644 --- a/cinder/backup/manager.py +++ b/cinder/backup/manager.py @@ -90,6 +90,11 @@ CONF.register_opts(backup_manager_opts) CONF.import_opt('use_multipath_for_image_xfer', 'cinder.volume.driver') CONF.import_opt('num_volume_device_scan_tries', 'cinder.volume.driver') QUOTAS = quota.QUOTAS +MAPPING = { + # Module name "google" conflicts with google library namespace inside the + # driver when it imports google.auth + 'cinder.backup.drivers.google': 'cinder.backup.drivers.gcs', +} # TODO(geguileo): Once Eventlet issue #432 gets fixed we can just tpool.execute @@ -113,12 +118,13 @@ class BackupManager(manager.ThreadPoolManager): self.is_initialized = False self._set_tpool_size(CONF.backup_native_threads_pool_size) self._process_number = kwargs.get('process_number', 1) - - @property - def driver_name(self): - """This function maps old backup services to backup drivers.""" - - return CONF.backup_driver + self.driver_name = CONF.backup_driver + if self.driver_name in MAPPING: + new_name = MAPPING[self.driver_name] + LOG.warning('Backup driver path %s is deprecated, update your ' + 'configuration to the new path %s', + self.driver_name, new_name) + self.driver_name = new_name def get_backup_driver(self, context): driver = None @@ -503,6 +509,28 @@ class BackupManager(manager.ThreadPoolManager): context, backup) return updates + def _is_our_backup(self, backup): + # Accept strings and Service OVO + if not isinstance(backup, six.string_types): + backup = backup.service + + if not backup: + return True + + # TODO(tommylikehu): We upgraded the 'driver_name' from module + # to class name, so we use 'in' here to match two namings, + # this can be replaced with equal sign during next + # release (Rocky). + if self.driver_name.startswith(backup): + return True + + # We support renaming of drivers, so check old names as well + for key, value in MAPPING.items(): + if key.startswith(backup) and self.driver_name.startswith(value): + return True + + return False + def restore_backup(self, context, backup, volume_id): """Restore volume backups from configured backup service.""" LOG.info('Restore backup started, backup: %(backup_id)s ' @@ -548,19 +576,13 @@ class BackupManager(manager.ThreadPoolManager): 'backup_id': backup['id'], 'backup_size': backup['size']}) - backup_service = backup['service'] - configured_service = self.driver_name - # TODO(tommylikehu): We upgraded the 'driver_name' from module - # to class name, so we use 'in' here to match two namings, - # this can be replaced with equal sign during next - # release (Rocky). - if backup_service not in configured_service: + if not self._is_our_backup(backup): err = _('Restore backup aborted, the backup service currently' ' configured [%(configured_service)s] is not the' ' backup service that was used to create this' ' backup [%(backup_service)s].') % { - 'configured_service': configured_service, - 'backup_service': backup_service, + 'configured_service': self.driver_name, + 'backup_service': backup.service, } backup.status = fields.BackupStatus.AVAILABLE backup.save() @@ -691,23 +713,17 @@ class BackupManager(manager.ThreadPoolManager): self._update_backup_error(backup, err, status) raise exception.InvalidBackup(reason=err) - backup_service = backup['service'] - if backup_service is not None: - configured_service = self.driver_name - # TODO(tommylikehu): We upgraded the 'driver_name' from module - # to class name, so we use 'in' here to match two namings, - # this can be replaced with equal sign during next - # release (Rocky). - if backup_service not in configured_service: - err = _('Delete backup aborted, the backup service currently' - ' configured [%(configured_service)s] is not the' - ' backup service that was used to create this' - ' backup [%(backup_service)s].')\ - % {'configured_service': configured_service, - 'backup_service': backup_service} - self._update_backup_error(backup, err) - raise exception.InvalidBackup(reason=err) + if not self._is_our_backup(backup): + err = _('Delete backup aborted, the backup service currently' + ' configured [%(configured_service)s] is not the' + ' backup service that was used to create this' + ' backup [%(backup_service)s].')\ + % {'configured_service': self.driver_name, + 'backup_service': backup.service} + self._update_backup_error(backup, err) + raise exception.InvalidBackup(reason=err) + if backup.service: try: backup_service = self.get_backup_driver(context) backup_service.delete_backup(backup) @@ -787,19 +803,13 @@ class BackupManager(manager.ThreadPoolManager): raise exception.InvalidBackup(reason=err) backup_record = {'backup_service': backup.service} - backup_service = backup.service - configured_service = self.driver_name - # TODO(tommylikehu): We upgraded the 'driver_name' from module - # to class name, so we use 'in' here to match two namings, - # this can be replaced with equal sign during next - # release (Rocky). - if backup_service not in configured_service: + if not self._is_our_backup(backup): err = (_('Export record aborted, the backup service currently ' 'configured [%(configured_service)s] is not the ' 'backup service that was used to create this ' 'backup [%(backup_service)s].') % - {'configured_service': configured_service, - 'backup_service': backup_service}) + {'configured_service': self.driver_name, + 'backup_service': backup.service}) raise exception.InvalidBackup(reason=err) # Call driver to create backup description string @@ -834,7 +844,7 @@ class BackupManager(manager.ThreadPoolManager): LOG.info('Import record started, backup_url: %s.', backup_url) # Can we import this backup? - if backup_service != self.driver_name: + if not self._is_our_backup(backup_service): # No, are there additional potential backup hosts in the list? if len(backup_hosts) > 0: # try the next host on the list, maybe he can import @@ -946,27 +956,22 @@ class BackupManager(manager.ThreadPoolManager): {'backup_id': backup.id, 'status': status}) - backup_service_name = backup.service - LOG.info('Backup service: %s.', backup_service_name) - if backup_service_name is not None: - configured_service = self.driver_name - # TODO(tommylikehu): We upgraded the 'driver_name' from module - # to class name, so we use 'in' here to match two namings, - # this can be replaced with equal sign during next - # release (Rocky). - if backup_service_name not in configured_service: - err = _('Reset backup status aborted, the backup service' - ' currently configured [%(configured_service)s] ' - 'is not the backup service that was used to create' - ' this backup [%(backup_service)s].') % \ - {'configured_service': configured_service, - 'backup_service': backup_service_name} - raise exception.InvalidBackup(reason=err) + LOG.info('Backup service: %s.', backup.service) + if not self._is_our_backup(backup): + err = _('Reset backup status aborted, the backup service' + ' currently configured [%(configured_service)s] ' + 'is not the backup service that was used to create' + ' this backup [%(backup_service)s].') % \ + {'configured_service': self.driver_name, + 'backup_service': backup.service} + raise exception.InvalidBackup(reason=err) + + if backup.service is not None: # Verify backup try: # check whether the backup is ok or not - if (status == fields.BackupStatus.AVAILABLE - and backup['status'] != fields.BackupStatus.RESTORING): + if (status == fields.BackupStatus.AVAILABLE and + backup['status'] != fields.BackupStatus.RESTORING): # check whether we could verify the backup is ok or not backup_service = self.get_backup_driver(context) if isinstance(backup_service, diff --git a/cinder/opts.py b/cinder/opts.py index 9ec766c9e44..59eae69d648 100644 --- a/cinder/opts.py +++ b/cinder/opts.py @@ -35,8 +35,8 @@ from cinder.backup import api as cinder_backup_api from cinder.backup import chunkeddriver as cinder_backup_chunkeddriver from cinder.backup import driver as cinder_backup_driver from cinder.backup.drivers import ceph as cinder_backup_drivers_ceph +from cinder.backup.drivers import gcs as cinder_backup_drivers_gcs from cinder.backup.drivers import glusterfs as cinder_backup_drivers_glusterfs -from cinder.backup.drivers import google as cinder_backup_drivers_google from cinder.backup.drivers import nfs as cinder_backup_drivers_nfs from cinder.backup.drivers import posix as cinder_backup_drivers_posix from cinder.backup.drivers import swift as cinder_backup_drivers_swift @@ -217,8 +217,8 @@ def list_opts(): cinder_backup_chunkeddriver.chunkedbackup_service_opts, cinder_backup_driver.service_opts, cinder_backup_drivers_ceph.service_opts, + cinder_backup_drivers_gcs.gcsbackup_service_opts, cinder_backup_drivers_glusterfs.glusterfsbackup_service_opts, - cinder_backup_drivers_google.gcsbackup_service_opts, cinder_backup_drivers_nfs.nfsbackup_service_opts, cinder_backup_drivers_posix.posixbackup_service_opts, cinder_backup_drivers_swift.swiftbackup_service_opts, diff --git a/cinder/tests/unit/backup/drivers/test_backup_google.py b/cinder/tests/unit/backup/drivers/test_backup_google.py index f266112ab49..500a079ec57 100644 --- a/cinder/tests/unit/backup/drivers/test_backup_google.py +++ b/cinder/tests/unit/backup/drivers/test_backup_google.py @@ -32,7 +32,7 @@ from eventlet import tpool import mock from oslo_utils import units -from cinder.backup.drivers import google as google_dr +from cinder.backup.drivers import gcs as google_dr from cinder import context from cinder import db from cinder import exception @@ -219,21 +219,17 @@ class GoogleBackupDriverTestCase(test.TestCase): @gcs_client @mock.patch('httplib2.proxy_info_from_url') def test_backup_proxy_configured(self, mock_proxy_info): - google_dr.CONF.set_override("backup_gcs_proxy_url", - "http://myproxy.example.com") + # Configuration overwrites enviromental variable + proxy_cfg = "http://myproxy.example.com" + os.environ['http_proxy'] = proxy_cfg + '_fake' + google_dr.CONF.set_override("backup_gcs_proxy_url", proxy_cfg) google_dr.GoogleBackupDriver(self.ctxt) - mock_proxy_info.assert_called_with("http://myproxy.example.com") + self.assertEqual(proxy_cfg, os.environ.get('http_proxy')) @gcs_client - @mock.patch('httplib2.proxy_info_from_environment') - def test_backup_proxy_environment(self, mock_proxy_env): - google_dr.GoogleBackupDriver(self.ctxt) - mock_proxy_env.assert_called_once_with() - - @gcs_client - @mock.patch('cinder.backup.drivers.google.GoogleBackupDriver.' + @mock.patch('cinder.backup.drivers.gcs.GoogleBackupDriver.' '_send_progress_end') - @mock.patch('cinder.backup.drivers.google.GoogleBackupDriver.' + @mock.patch('cinder.backup.drivers.gcs.GoogleBackupDriver.' '_send_progress_notification') def test_backup_default_container_notify(self, _send_progress, _send_progress_end): @@ -612,3 +608,54 @@ class GoogleBackupDriverTestCase(test.TestCase): self.assertEqual('none', result[0]) self.assertEqual(already_compressed_data, result[1]) + + @mock.patch('googleapiclient.__version__', '1.5.5') + @mock.patch.object(google_dr.client.GoogleCredentials, 'from_stream') + @mock.patch.object(google_dr.discovery, 'build') + @mock.patch.object(google_dr, 'service_account') + def test_non_google_auth_version(self, account, build, from_stream): + # Prior to v1.6.0 Google api client doesn't support google-auth library + google_dr.CONF.set_override('backup_gcs_credential_file', + 'credentials_file') + + google_dr.GoogleBackupDriver(self.ctxt) + + from_stream.assert_called_once_with('credentials_file') + account.Credentials.from_service_account_file.assert_not_called() + build.assert_called_once_with('storage', 'v1', cache_discovery=False, + credentials=from_stream.return_value) + + @mock.patch('googleapiclient.__version__', '1.6.6') + @mock.patch.object(google_dr.client.GoogleCredentials, 'from_stream') + @mock.patch.object(google_dr.discovery, 'build') + @mock.patch.object(google_dr, 'service_account', None) + def test_no_httplib2_auth(self, build, from_stream): + # Google api client requires google-auth-httplib2 if not present we + # use legacy credentials + google_dr.CONF.set_override('backup_gcs_credential_file', + 'credentials_file') + + google_dr.GoogleBackupDriver(self.ctxt) + + from_stream.assert_called_once_with('credentials_file') + build.assert_called_once_with('storage', 'v1', cache_discovery=False, + credentials=from_stream.return_value) + + @mock.patch('googleapiclient.__version__', '1.6.6') + @mock.patch.object(google_dr, 'gexceptions', mock.Mock()) + @mock.patch.object(google_dr.client.GoogleCredentials, 'from_stream') + @mock.patch.object(google_dr.discovery, 'build') + @mock.patch.object(google_dr, 'service_account') + def test_google_auth_used(self, account, build, from_stream): + # Google api client requires google-auth-httplib2 if not present we + # use legacy credentials + google_dr.CONF.set_override('backup_gcs_credential_file', + 'credentials_file') + + google_dr.GoogleBackupDriver(self.ctxt) + + from_stream.assert_not_called() + create_creds = account.Credentials.from_service_account_file + create_creds.assert_called_once_with('credentials_file') + build.assert_called_once_with('storage', 'v1', cache_discovery=False, + credentials=create_creds.return_value) diff --git a/cinder/tests/unit/backup/test_backup.py b/cinder/tests/unit/backup/test_backup.py index 8ca0f1ac862..f290b670f98 100644 --- a/cinder/tests/unit/backup/test_backup.py +++ b/cinder/tests/unit/backup/test_backup.py @@ -1823,6 +1823,7 @@ class BackupTestCaseWithVerify(BaseBackupTest): service_name = ('cinder.tests.unit.backup.' 'fake_service_with_verify.FakeBackupServiceWithVerify') self.override_config('backup_driver', service_name) + self.backup_mgr.driver_name = service_name vol_id = self._create_volume_db_entry(status='available', size=1) backup = self._create_backup_db_entry(status=fields.BackupStatus.ERROR, diff --git a/releasenotes/notes/google-auth-for-gcs-backup-1642cd0e741fbdf9.yaml b/releasenotes/notes/google-auth-for-gcs-backup-1642cd0e741fbdf9.yaml new file mode 100644 index 00000000000..57f87965186 --- /dev/null +++ b/releasenotes/notes/google-auth-for-gcs-backup-1642cd0e741fbdf9.yaml @@ -0,0 +1,13 @@ +--- +features: + - Google backup driver now supports ``google-auth`` library, and is the + preferred library if both ``google-auth`` (together with + ``google-auth-httplib2``) and ``oauth2client`` libraries are present in the + system. +deprecations: + - Cinder's Google backup driver is now called gcs, so ``backup_driver`` + configuration for Google Cloud Storage should be updated from + ``cinder.backup.drivers.google`` to ``cinder.backup.driver.gcs``. +fixes: + - Google backup driver now works when using ``google-api-python-client`` + version 1.6.0 or higher.