Fix google backup driver
Current backup driver is broken due to the upper constraints of google-api-python-client which is set to 1.6.6, but after 1.6.0 parameter http and credentials are mutually exclusive, the driver is also using the no longer preferred Google authentication library. This patch fixes this by only using the http_proxy environmental variable to set the proxying and adding support for google-auth and using it whenever possible. Closes-Bug: #1736569 Closes-Bug: #1769660 Change-Id: If1e34ea112b0cff328a58ec2fc9c3f5a5005f1c6
This commit is contained in:
Gorka Eguileor
parent
34845b054e
commit
79d7a4e8da
@ -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')
|
||||
@ -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,
|
||||
|
||||
@ -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,
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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,
|
||||
|
||||
@ -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.
|
||||
Loading…
Reference in New Issue
Block a user