openstack
/
glance
Code Issues Proposed changes

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 2014-02-21 19:33:40 +05:30
parent 8762a7b072
commit c0e1d5bd29
3 changed files with 112 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
glance/db/sqlalchemy/migrate_repo/versions/015_quote_swift_credentials.py
View File

@ -51,14 +51,20 @@ def migrate_location_credentials(migrate_engine, to_quoted):
'swift')).execute()) 'swift')).execute())
for image in images: for image in images:
fixed_uri = legacy_parse_uri(image['location'], to_quoted, try:
image['id']) fixed_uri = legacy_parse_uri(image['location'], to_quoted)
images_table.update()\ images_table.update()\
.where(images_table.c.id == image['id'])\ .where(images_table.c.id == image['id'])\
.values(location=fixed_uri).execute() .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 Parse URLs. This method fixes an issue where credentials specified
in the URL are interpreted differently in Python 2.6.1+ than prior 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: " "like so: "
"swift+http://user:pass@authurl.com/v1/container/obj") "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) raise exception.BadStoreUri(message=reason)
pieces = urlparse.urlparse(uri) pieces = urlparse.urlparse(uri)
@ -120,8 +124,7 @@ def legacy_parse_uri(uri, to_quote, image_id):
if len(cred_parts) == 1: if len(cred_parts) == 1:
reason = (_("Badly formed credentials '%(creds)s' in Swift " reason = (_("Badly formed credentials '%(creds)s' in Swift "
"URI") % {'creds': creds}) "URI") % {'creds': creds})
LOG.error(reason) raise exception.BadStoreUri(message=reason)
raise exception.BadStoreUri()
elif len(cred_parts) == 3: elif len(cred_parts) == 3:
user = ':'.join(cred_parts[0:2]) user = ':'.join(cred_parts[0:2])
else: else:
@ -132,8 +135,7 @@ def legacy_parse_uri(uri, to_quote, image_id):
else: else:
if len(cred_parts) != 2: if len(cred_parts) != 2:
reason = (_("Badly formed credentials in Swift URI.")) reason = (_("Badly formed credentials in Swift URI."))
LOG.debug(reason) raise exception.BadStoreUri(message=reason)
raise exception.BadStoreUri()
user, key = cred_parts user, key = cred_parts
user = urllib.unquote(user) user = urllib.unquote(user)
key = urllib.unquote(key) key = urllib.unquote(key)
@ -150,8 +152,7 @@ def legacy_parse_uri(uri, to_quote, image_id):
auth_or_store_url = '/'.join(path_parts) auth_or_store_url = '/'.join(path_parts)
except IndexError: except IndexError:
reason = _("Badly formed S3 URI: %(uri)s") % {'uri': uri} reason = _("Badly formed S3 URI: %(uri)s") % {'uri': uri}
LOG.error(message=reason) raise exception.BadStoreUri(message=reason)
raise exception.BadStoreUri()
if auth_or_store_url.startswith('http://'): if auth_or_store_url.startswith('http://'):
auth_or_store_url = auth_or_store_url[len('http://'):] auth_or_store_url = auth_or_store_url[len('http://'):]

27
glance/db/sqlalchemy/migrate_repo/versions/017_quote_encrypted_swift_credentials.py
View File

@ -78,14 +78,19 @@ def migrate_location_credentials(migrate_engine, to_quoted):
for image in images: for image in images:
try: try:
fixed_uri = fix_uri_credentials(image['location'], to_quoted, fixed_uri = fix_uri_credentials(image['location'], to_quoted)
image['id'])
images_table.update()\ images_table.update()\
.where(images_table.c.id == image['id'])\ .where(images_table.c.id == image['id'])\
.values(location=fixed_uri).execute() .values(location=fixed_uri).execute()
except exception.Invalid: except exception.Invalid:
msg = _("Failed to decrypt location value for image %(image_id)s") msg = _("Failed to decrypt location value for image %(image_id)s")
LOG.warn(msg % {'image_id': image['id']}) 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): def decrypt_location(uri):
@ -96,7 +101,7 @@ def encrypt_location(uri):
return crypt.urlsafe_encrypt(CONF.metadata_encryption_key, uri, 64) 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 Fix the given uri's embedded credentials by round-tripping with
StoreLocation. StoreLocation.
@ -118,10 +123,10 @@ def fix_uri_credentials(uri, to_quoted, image_id):
except (TypeError, ValueError) as e: except (TypeError, ValueError) as e:
raise exception.Invalid(str(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 Parse URLs. This method fixes an issue where credentials specified
in the URL are interpreted differently in Python 2.6.1+ than prior 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, " ", you need to change it to use the swift+http:// scheme, "
"like so: " "like so: "
"swift+http://user:pass@authurl.com/v1/container/obj") "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) raise exception.BadStoreUri(message=reason)
pieces = urlparse.urlparse(uri) pieces = urlparse.urlparse(uri)
@ -183,8 +185,7 @@ def legacy_parse_uri(uri, to_quote, image_id):
if len(cred_parts) == 1: if len(cred_parts) == 1:
reason = (_("Badly formed credentials '%(creds)s' in Swift " reason = (_("Badly formed credentials '%(creds)s' in Swift "
"URI") % {'creds': creds}) "URI") % {'creds': creds})
LOG.error(reason) raise exception.BadStoreUri(message=reason)
raise exception.BadStoreUri()
elif len(cred_parts) == 3: elif len(cred_parts) == 3:
user = ':'.join(cred_parts[0:2]) user = ':'.join(cred_parts[0:2])
else: else:
@ -195,8 +196,7 @@ def legacy_parse_uri(uri, to_quote, image_id):
else: else:
if len(cred_parts) != 2: if len(cred_parts) != 2:
reason = (_("Badly formed credentials in Swift URI.")) reason = (_("Badly formed credentials in Swift URI."))
LOG.debug(reason) raise exception.BadStoreUri(message=reason)
raise exception.BadStoreUri()
user, key = cred_parts user, key = cred_parts
user = urllib.unquote(user) user = urllib.unquote(user)
key = urllib.unquote(key) key = urllib.unquote(key)
@ -213,8 +213,7 @@ def legacy_parse_uri(uri, to_quote, image_id):
auth_or_store_url = '/'.join(path_parts) auth_or_store_url = '/'.join(path_parts)
except IndexError: except IndexError:
reason = _("Badly formed S3 URI: %(uri)s") % {'uri': uri} reason = _("Badly formed S3 URI: %(uri)s") % {'uri': uri}
LOG.error(message=reason) raise exception.BadStoreUri(message=reason)
raise exception.BadStoreUri()
if auth_or_store_url.startswith('http://'): if auth_or_store_url.startswith('http://'):
auth_or_store_url = auth_or_store_url[len('http://'):] auth_or_store_url = auth_or_store_url[len('http://'):]

84
glance/tests/unit/test_migrations.py
View File

@ -27,6 +27,7 @@ from __future__ import print_function
import ConfigParser import ConfigParser
import datetime import datetime
import exceptions
import os import os
import pickle import pickle
import subprocess import subprocess
@ -40,9 +41,11 @@ from six.moves import xrange
import sqlalchemy import sqlalchemy
from glance.common import crypt from glance.common import crypt
from glance.common import exception
from glance.common import utils from glance.common import utils
import glance.db.migration as migration import glance.db.migration as migration
import glance.db.sqlalchemy.migrate_repo 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.db.sqlalchemy import models
from glance.openstack.common import jsonutils from glance.openstack.common import jsonutils
from glance.openstack.common import log as logging from glance.openstack.common import log as logging
@ -712,6 +715,72 @@ class TestMigrations(test_utils.BaseTestCase):
if row['name'] == 'ramdisk_id': if row['name'] == 'ramdisk_id':
self.assertEqual(row['value'], str(ids['ramdisk'])) 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): def _pre_upgrade_015(self, engine):
images = get_table(engine, 'images') images = get_table(engine, 'images')
unquoted_locations = [ unquoted_locations = [
@ -774,6 +843,21 @@ class TestMigrations(test_utils.BaseTestCase):
"columns! image_members table columns: %s" "columns! image_members table columns: %s"
% image_members.c.keys()) % 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): def _pre_upgrade_017(self, engine):
metadata_encryption_key = 'a' * 16 metadata_encryption_key = 'a' * 16
self.config(metadata_encryption_key=metadata_encryption_key) self.config(metadata_encryption_key=metadata_encryption_key)