Log 'image_id' with all BadStoreURI error messages
- handle all BadStoreUri exceptions raised by 'legacy_parse_uri' function in one location so that it becomes easier to log the error message with 'image_id' - add tests for 'legacy_parse_uri' Closes-Bug: #1243704 Change-Id: Ifc5de11832860ed51c1eb359d6f5cf78de8c0ba4
This commit is contained in:
Venkatesh Sampath
parent
8762a7b072
commit
c0e1d5bd29
|
|
@ -51,14 +51,20 @@ def migrate_location_credentials(migrate_engine, to_quoted):
|
|||
'swift')).execute())
|
||||
|
||||
for image in images:
|
||||
fixed_uri = legacy_parse_uri(image['location'], to_quoted,
|
||||
image['id'])
|
||||
images_table.update()\
|
||||
.where(images_table.c.id == image['id'])\
|
||||
.values(location=fixed_uri).execute()
|
||||
try:
|
||||
fixed_uri = legacy_parse_uri(image['location'], to_quoted)
|
||||
images_table.update()\
|
||||
.where(images_table.c.id == image['id'])\
|
||||
.values(location=fixed_uri).execute()
|
||||
except exception.BadStoreUri as e:
|
||||
err_msg = _("Invalid store uri for image: %(image_id)s. "
|
||||
"Details: %(reason)s") % {'image_id': image.id,
|
||||
'reason': unicode(e)}
|
||||
LOG.exception(err_msg)
|
||||
raise
|
||||
|
||||
|
||||
def legacy_parse_uri(uri, to_quote, image_id):
|
||||
def legacy_parse_uri(uri, to_quote):
|
||||
"""
|
||||
Parse URLs. This method fixes an issue where credentials specified
|
||||
in the URL are interpreted differently in Python 2.6.1+ than prior
|
||||
|
|
@ -86,8 +92,6 @@ def legacy_parse_uri(uri, to_quote, image_id):
|
|||
"like so: "
|
||||
"swift+http://user:pass@authurl.com/v1/container/obj")
|
||||
|
||||
LOG.error(_("Invalid store uri for image %(image_id)s: %(reason)s") %
|
||||
{'image_id': image_id, 'reason': reason})
|
||||
raise exception.BadStoreUri(message=reason)
|
||||
|
||||
pieces = urlparse.urlparse(uri)
|
||||
|
|
@ -120,8 +124,7 @@ def legacy_parse_uri(uri, to_quote, image_id):
|
|||
if len(cred_parts) == 1:
|
||||
reason = (_("Badly formed credentials '%(creds)s' in Swift "
|
||||
"URI") % {'creds': creds})
|
||||
LOG.error(reason)
|
||||
raise exception.BadStoreUri()
|
||||
raise exception.BadStoreUri(message=reason)
|
||||
elif len(cred_parts) == 3:
|
||||
user = ':'.join(cred_parts[0:2])
|
||||
else:
|
||||
|
|
@ -132,8 +135,7 @@ def legacy_parse_uri(uri, to_quote, image_id):
|
|||
else:
|
||||
if len(cred_parts) != 2:
|
||||
reason = (_("Badly formed credentials in Swift URI."))
|
||||
LOG.debug(reason)
|
||||
raise exception.BadStoreUri()
|
||||
raise exception.BadStoreUri(message=reason)
|
||||
user, key = cred_parts
|
||||
user = urllib.unquote(user)
|
||||
key = urllib.unquote(key)
|
||||
|
|
@ -150,8 +152,7 @@ def legacy_parse_uri(uri, to_quote, image_id):
|
|||
auth_or_store_url = '/'.join(path_parts)
|
||||
except IndexError:
|
||||
reason = _("Badly formed S3 URI: %(uri)s") % {'uri': uri}
|
||||
LOG.error(message=reason)
|
||||
raise exception.BadStoreUri()
|
||||
raise exception.BadStoreUri(message=reason)
|
||||
|
||||
if auth_or_store_url.startswith('http://'):
|
||||
auth_or_store_url = auth_or_store_url[len('http://'):]
|
||||
|
|
|
|||
|
|
@ -78,14 +78,19 @@ def migrate_location_credentials(migrate_engine, to_quoted):
|
|||
|
||||
for image in images:
|
||||
try:
|
||||
fixed_uri = fix_uri_credentials(image['location'], to_quoted,
|
||||
image['id'])
|
||||
fixed_uri = fix_uri_credentials(image['location'], to_quoted)
|
||||
images_table.update()\
|
||||
.where(images_table.c.id == image['id'])\
|
||||
.values(location=fixed_uri).execute()
|
||||
except exception.Invalid:
|
||||
msg = _("Failed to decrypt location value for image %(image_id)s")
|
||||
LOG.warn(msg % {'image_id': image['id']})
|
||||
except exception.BadStoreUri as e:
|
||||
err_msg = _("Invalid store uri for image: %(image_id)s. "
|
||||
"Details: %(reason)s") % {'image_id': image.id,
|
||||
'reason': unicode(e)}
|
||||
LOG.exception(err_msg)
|
||||
raise
|
||||
|
||||
|
||||
def decrypt_location(uri):
|
||||
|
|
@ -96,7 +101,7 @@ def encrypt_location(uri):
|
|||
return crypt.urlsafe_encrypt(CONF.metadata_encryption_key, uri, 64)
|
||||
|
||||
|
||||
def fix_uri_credentials(uri, to_quoted, image_id):
|
||||
def fix_uri_credentials(uri, to_quoted):
|
||||
"""
|
||||
Fix the given uri's embedded credentials by round-tripping with
|
||||
StoreLocation.
|
||||
|
|
@ -118,10 +123,10 @@ def fix_uri_credentials(uri, to_quoted, image_id):
|
|||
except (TypeError, ValueError) as e:
|
||||
raise exception.Invalid(str(e))
|
||||
|
||||
return legacy_parse_uri(decrypted_uri, to_quoted, image_id)
|
||||
return legacy_parse_uri(decrypted_uri, to_quoted)
|
||||
|
||||
|
||||
def legacy_parse_uri(uri, to_quote, image_id):
|
||||
def legacy_parse_uri(uri, to_quote):
|
||||
"""
|
||||
Parse URLs. This method fixes an issue where credentials specified
|
||||
in the URL are interpreted differently in Python 2.6.1+ than prior
|
||||
|
|
@ -148,9 +153,6 @@ def legacy_parse_uri(uri, to_quote, image_id):
|
|||
", you need to change it to use the swift+http:// scheme, "
|
||||
"like so: "
|
||||
"swift+http://user:pass@authurl.com/v1/container/obj")
|
||||
|
||||
LOG.error(_("Invalid store uri for image %(image_id)s: %(reason)s") %
|
||||
{'image_id': image_id, 'reason': reason})
|
||||
raise exception.BadStoreUri(message=reason)
|
||||
|
||||
pieces = urlparse.urlparse(uri)
|
||||
|
|
@ -183,8 +185,7 @@ def legacy_parse_uri(uri, to_quote, image_id):
|
|||
if len(cred_parts) == 1:
|
||||
reason = (_("Badly formed credentials '%(creds)s' in Swift "
|
||||
"URI") % {'creds': creds})
|
||||
LOG.error(reason)
|
||||
raise exception.BadStoreUri()
|
||||
raise exception.BadStoreUri(message=reason)
|
||||
elif len(cred_parts) == 3:
|
||||
user = ':'.join(cred_parts[0:2])
|
||||
else:
|
||||
|
|
@ -195,8 +196,7 @@ def legacy_parse_uri(uri, to_quote, image_id):
|
|||
else:
|
||||
if len(cred_parts) != 2:
|
||||
reason = (_("Badly formed credentials in Swift URI."))
|
||||
LOG.debug(reason)
|
||||
raise exception.BadStoreUri()
|
||||
raise exception.BadStoreUri(message=reason)
|
||||
user, key = cred_parts
|
||||
user = urllib.unquote(user)
|
||||
key = urllib.unquote(key)
|
||||
|
|
@ -213,8 +213,7 @@ def legacy_parse_uri(uri, to_quote, image_id):
|
|||
auth_or_store_url = '/'.join(path_parts)
|
||||
except IndexError:
|
||||
reason = _("Badly formed S3 URI: %(uri)s") % {'uri': uri}
|
||||
LOG.error(message=reason)
|
||||
raise exception.BadStoreUri()
|
||||
raise exception.BadStoreUri(message=reason)
|
||||
|
||||
if auth_or_store_url.startswith('http://'):
|
||||
auth_or_store_url = auth_or_store_url[len('http://'):]
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ from __future__ import print_function
|
|||
|
||||
import ConfigParser
|
||||
import datetime
|
||||
import exceptions
|
||||
import os
|
||||
import pickle
|
||||
import subprocess
|
||||
|
|
@ -40,9 +41,11 @@ from six.moves import xrange
|
|||
import sqlalchemy
|
||||
|
||||
from glance.common import crypt
|
||||
from glance.common import exception
|
||||
from glance.common import utils
|
||||
import glance.db.migration as migration
|
||||
import glance.db.sqlalchemy.migrate_repo
|
||||
from glance.db.sqlalchemy.migrate_repo.schema import from_migration_import
|
||||
from glance.db.sqlalchemy import models
|
||||
from glance.openstack.common import jsonutils
|
||||
from glance.openstack.common import log as logging
|
||||
|
|
@ -712,6 +715,72 @@ class TestMigrations(test_utils.BaseTestCase):
|
|||
if row['name'] == 'ramdisk_id':
|
||||
self.assertEqual(row['value'], str(ids['ramdisk']))
|
||||
|
||||
def _assert_invalid_swift_uri_raises_bad_store_uri(self,
|
||||
legacy_parse_uri_fn):
|
||||
invalid_uri = ('swift://http://acct:usr:pass@example.com'
|
||||
'/container/obj-id')
|
||||
# URI cannot contain more than one occurrence of a scheme.
|
||||
self.assertRaises(exception.BadStoreUri,
|
||||
legacy_parse_uri_fn,
|
||||
invalid_uri,
|
||||
True)
|
||||
|
||||
invalid_scheme_uri = ('http://acct:usr:pass@example.com'
|
||||
'/container/obj-id')
|
||||
self.assertRaises(exceptions.AssertionError,
|
||||
legacy_parse_uri_fn,
|
||||
invalid_scheme_uri,
|
||||
True)
|
||||
|
||||
invalid_account_missing_uri = 'swift+http://container/obj-id'
|
||||
# Badly formed S3 URI: swift+http://container/obj-id
|
||||
self.assertRaises(exception.BadStoreUri,
|
||||
legacy_parse_uri_fn,
|
||||
invalid_account_missing_uri,
|
||||
True)
|
||||
|
||||
invalid_container_missing_uri = ('swift+http://'
|
||||
'acct:usr:pass@example.com/obj-id')
|
||||
# Badly formed S3 URI: swift+http://acct:usr:pass@example.com/obj-id
|
||||
self.assertRaises(exception.BadStoreUri,
|
||||
legacy_parse_uri_fn,
|
||||
invalid_container_missing_uri,
|
||||
True)
|
||||
|
||||
invalid_object_missing_uri = ('swift+http://'
|
||||
'acct:usr:pass@example.com/container')
|
||||
# Badly formed S3 URI: swift+http://acct:usr:pass@example.com/container
|
||||
self.assertRaises(exception.BadStoreUri,
|
||||
legacy_parse_uri_fn,
|
||||
invalid_object_missing_uri,
|
||||
True)
|
||||
|
||||
invalid_user_without_pass_uri = ('swift://acctusr@example.com'
|
||||
'/container/obj-id')
|
||||
# Badly formed credentials '%(creds)s' in Swift URI
|
||||
self.assertRaises(exception.BadStoreUri,
|
||||
legacy_parse_uri_fn,
|
||||
invalid_user_without_pass_uri,
|
||||
True)
|
||||
|
||||
# Badly formed credentials in Swift URI.
|
||||
self.assertRaises(exception.BadStoreUri,
|
||||
legacy_parse_uri_fn,
|
||||
invalid_user_without_pass_uri,
|
||||
False)
|
||||
|
||||
def test_legacy_parse_swift_uri_015(self):
|
||||
(legacy_parse_uri,) = from_migration_import(
|
||||
'015_quote_swift_credentials', ['legacy_parse_uri'])
|
||||
|
||||
uri = legacy_parse_uri(
|
||||
'swift://acct:usr:pass@example.com/container/obj-id',
|
||||
True)
|
||||
self.assertTrue(uri, 'swift://acct%3Ausr:pass@example.com'
|
||||
'/container/obj-id')
|
||||
|
||||
self._assert_invalid_swift_uri_raises_bad_store_uri(legacy_parse_uri)
|
||||
|
||||
def _pre_upgrade_015(self, engine):
|
||||
images = get_table(engine, 'images')
|
||||
unquoted_locations = [
|
||||
|
|
@ -774,6 +843,21 @@ class TestMigrations(test_utils.BaseTestCase):
|
|||
"columns! image_members table columns: %s"
|
||||
% image_members.c.keys())
|
||||
|
||||
def test_legacy_parse_swift_uri_017(self):
|
||||
metadata_encryption_key = 'a' * 16
|
||||
self.config(metadata_encryption_key=metadata_encryption_key)
|
||||
|
||||
(legacy_parse_uri, encrypt_location) = from_migration_import(
|
||||
'017_quote_encrypted_swift_credentials', ['legacy_parse_uri',
|
||||
'encrypt_location'])
|
||||
|
||||
uri = legacy_parse_uri('swift://acct:usr:pass@example.com'
|
||||
'/container/obj-id', True)
|
||||
self.assertTrue(uri, encrypt_location(
|
||||
'swift://acct%3Ausr:pass@example.com/container/obj-id'))
|
||||
|
||||
self._assert_invalid_swift_uri_raises_bad_store_uri(legacy_parse_uri)
|
||||
|
||||
def _pre_upgrade_017(self, engine):
|
||||
metadata_encryption_key = 'a' * 16
|
||||
self.config(metadata_encryption_key=metadata_encryption_key)
|
||||
|
|
|
|||
Loading…
Reference in New Issue