Unifies how BadStoreUri gets raised and logged

Refactoring store code to raise BadStoreUri in unified way. Changes BadStoreUri messages being logged as info from debug. Fixes translations accordingly. Ensures no URI's nor credentials logged with these messages. Closes bug #1287506 Change-Id: I246885d68b0af9e91ede48aaa2b1e6849e5b3b3d
openstack · Jun 27, 2014 · 97882f7 · 97882f7
1 parent 8c2d811
commit 97882f7
Show file tree

Hide file tree

Showing 9 changed files with 88 additions and 89 deletions.
diff --git a/glance/location.py b/glance/location.py
@@ -76,7 +76,8 @@ def _check_location_uri(context, store_api, uri):
     except (exception.UnknownScheme, exception.NotFound):
         is_ok = False
     if not is_ok:
-        raise exception.BadStoreUri(_('Invalid location: %s') % uri)
+        reason = _('Invalid location')
+        raise exception.BadStoreUri(message=reason)
 
 
 def _check_image_location(context, store_api, location):
@@ -269,7 +270,8 @@ def get_attr(self):
 
     def set_attr(self, value):
         if not isinstance(value, (list, StoreLocations)):
-            raise exception.BadStoreUri(_('Invalid locations: %s') % value)
+            reason = _('Invalid locations')
+            raise exception.BadStoreUri(message=reason)
         ori_value = getattr(getattr(self, target), attr)
         if ori_value != value:
             # NOTE(zhiyan): Enforced locations list was previously empty list.

diff --git a/glance/store/cinder.py b/glance/store/cinder.py
@@ -112,17 +112,17 @@ def get_uri(self):
 
     def parse_uri(self, uri):
         if not uri.startswith('cinder://'):
-            reason = _("URI must start with cinder://")
-            LOG.error(reason)
-            raise exception.BadStoreUri(uri, reason)
+            reason = _("URI must start with 'cinder://'")
+            LOG.info(reason)
+            raise exception.BadStoreUri(message=reason)
 
         self.scheme = 'cinder'
         self.volume_id = uri[9:]
 
         if not utils.is_uuid_like(self.volume_id):
-            reason = _("URI contains invalid volume ID: %s") % self.volume_id
-            LOG.error(reason)
-            raise exception.BadStoreUri(uri, reason)
+            reason = _("URI contains invalid volume ID")
+            LOG.info(reason)
+            raise exception.BadStoreUri(message=reason)
 
 
 class Store(glance.store.base.Store):

diff --git a/glance/store/filesystem.py b/glance/store/filesystem.py
@@ -76,9 +76,9 @@ def parse_uri(self, uri):
         self.scheme = pieces.scheme
         path = (pieces.netloc + pieces.path).strip()
         if path == '':
-            reason = "No path specified in URI: %s" % uri
-            LOG.debug(reason)
-            raise exception.BadStoreUri('No path specified')
+            reason = _("No path specified in URI")
+            LOG.info(reason)
+            raise exception.BadStoreUri(message=reason)
         self.path = path
 
 

diff --git a/glance/store/http.py b/glance/store/http.py
@@ -79,15 +79,14 @@ def parse_uri(self, uri):
             try:
                 self.user, self.password = creds.split(':')
             except ValueError:
-                reason = (_("Credentials '%s' not well-formatted.")
-                          % "".join(creds))
-                LOG.debug(reason)
-                raise exception.BadStoreUri()
+                reason = _("Credentials are not well-formatted.")
+                LOG.info(reason)
+                raise exception.BadStoreUri(message=reason)
         else:
             self.user = None
         if netloc == '':
             reason = _("No address specified in HTTP URL")
-            LOG.debug(reason)
+            LOG.info(reason)
             raise exception.BadStoreUri(message=reason)
         self.netloc = netloc
         self.path = path
@@ -149,8 +148,8 @@ def get_size(self, location):
             size = self._query(location, 'HEAD')[2]
         except socket.error:
             reason = _("The HTTP URL is invalid.")
-            LOG.debug(reason)
-            raise exception.BadStoreUri(reason)
+            LOG.info(reason)
+            raise exception.BadStoreUri(message=reason)
         except Exception:
             # NOTE(flaper87): Catch more granular exceptions,
             # keeping this branch for backwards compatibility.
@@ -176,16 +175,16 @@ def _query(self, location, verb, depth=0):
                 LOG.debug(reason)
                 raise exception.NotFound(reason)
             reason = _("HTTP URL returned a %s status code.") % resp.status
-            LOG.debug(reason)
-            raise exception.BadStoreUri(loc.path, reason)
+            LOG.info(reason)
+            raise exception.BadStoreUri(message=reason)
 
         location_header = resp.getheader("location")
         if location_header:
             if resp.status not in (301, 302):
                 reason = (_("The HTTP URL attempted to redirect with an "
                             "invalid %s status code.") % resp.status)
-                LOG.debug(reason)
-                raise exception.BadStoreUri(loc.path, reason)
+                LOG.info(reason)
+                raise exception.BadStoreUri(message=reason)
             location_class = glance.store.location.Location
             new_loc = location_class(location.store_name,
                                      location.store_location.__class__,

diff --git a/glance/store/rbd.py b/glance/store/rbd.py
@@ -28,6 +28,7 @@
 
 from glance.common import exception
 from glance.common import utils
+from glance.openstack.common import gettextutils
 import glance.openstack.common.log as logging
 from glance.openstack.common import units
 import glance.store.base
@@ -47,6 +48,7 @@
 DEFAULT_SNAPNAME = 'snap'
 
 LOG = logging.getLogger(__name__)
+_LI = gettextutils._LI
 
 rbd_opts = [
     cfg.IntOpt('rbd_store_chunk_size', default=DEFAULT_CHUNKSIZE,
@@ -107,18 +109,16 @@ def parse_uri(self, uri):
         prefix = 'rbd://'
         if not uri.startswith(prefix):
             reason = _('URI must start with rbd://')
-            msg = "Invalid URI: %(uri)s: %(reason)s" % {'uri': uri,
-                                                        'reason': reason}
-            LOG.debug(msg)
+            msg = _LI("Invalid URI: %s") % reason
+            LOG.info(msg)
             raise exception.BadStoreUri(message=reason)
         # convert to ascii since librbd doesn't handle unicode
         try:
             ascii_uri = str(uri)
         except UnicodeError:
-            reason = 'URI contains non-ascii characters'
-            msg = "Invalid URI: %(uri)s: %(reason)s" % {'uri': uri,
-                                                        'reason': reason}
-            LOG.debug(msg)
+            reason = _('URI contains non-ascii characters')
+            msg = _LI("Invalid URI: %s") % reason
+            LOG.info(msg)
             raise exception.BadStoreUri(message=reason)
         pieces = ascii_uri[len(prefix):].split('/')
         if len(pieces) == 1:
@@ -128,16 +128,14 @@ def parse_uri(self, uri):
             self.fsid, self.pool, self.image, self.snapshot = \
                 map(urlparse.unquote, pieces)
         else:
-            reason = 'URI must have exactly 1 or 4 components'
-            msg = "Invalid URI: %(uri)s: %(reason)s" % {'uri': uri,
-                                                        'reason': reason}
-            LOG.debug(msg)
+            reason = _('URI must have exactly 1 or 4 components')
+            msg = _LI("Invalid URI: %s") % reason
+            LOG.info(msg)
             raise exception.BadStoreUri(message=reason)
         if any(map(lambda p: p == '', pieces)):
-            reason = 'URI cannot contain empty components'
-            msg = "Invalid URI: %(uri)s: %(reason)s" % {'uri': uri,
-                                                        'reason': reason}
-            LOG.debug(msg)
+            reason = _('URI cannot contain empty components')
+            msg = _LI("Invalid URI: %s") % reason
+            LOG.info(msg)
             raise exception.BadStoreUri(message=reason)
 
 

diff --git a/glance/store/s3.py b/glance/store/s3.py
@@ -195,15 +195,15 @@ def parse_uri(self, uri):
         # s3://accesskey:secretkey@https://s3.amazonaws.com/bucket/key-id
         # are immediately rejected.
         if uri.count('://') != 1:
-            reason = ("URI cannot contain more than one occurrence "
-                      "of a scheme. If you have specified a URI like "
-                      "s3://accesskey:secretkey@"
-                      "https://s3.amazonaws.com/bucket/key-id"
-                      ", you need to change it to use the "
-                      "s3+https:// scheme, like so: "
-                      "s3+https://accesskey:secretkey@"
-                      "s3.amazonaws.com/bucket/key-id")
-            LOG.debug("Invalid store uri: %s" % reason)
+            reason = _("URI cannot contain more than one occurrence "
+                       "of a scheme. If you have specified a URI like "
+                       "s3://accesskey:secretkey@"
+                       "https://s3.amazonaws.com/bucket/key-id"
+                       ", you need to change it to use the "
+                       "s3+https:// scheme, like so: "
+                       "s3+https://accesskey:secretkey@"
+                       "s3.amazonaws.com/bucket/key-id")
+            LOG.info(_LI("Invalid store uri: %s") % reason)
             raise exception.BadStoreUri(message=reason)
 
         pieces = urlparse.urlparse(uri)
@@ -229,9 +229,9 @@ def parse_uri(self, uri):
                 self.accesskey = access_key
                 self.secretkey = secret_key
             except IndexError:
-                reason = "Badly formed S3 credentials %s" % creds
-                LOG.debug(reason)
-                raise exception.BadStoreUri()
+                reason = _("Badly formed S3 credentials")
+                LOG.info(reason)
+                raise exception.BadStoreUri(message=reason)
         else:
             self.accesskey = None
             path = entire_path
@@ -243,11 +243,11 @@ def parse_uri(self, uri):
                 self.s3serviceurl = '/'.join(path_parts).strip('/')
             else:
                 reason = _("Badly formed S3 URI. Missing s3 service URL.")
-                raise exception.BadStoreUri()
+                raise exception.BadStoreUri(message=reason)
         except IndexError:
-            reason = "Badly formed S3 URI: %s" % uri
-            LOG.debug(reason)
-            raise exception.BadStoreUri()
+            reason = _("Badly formed S3 URI")
+            LOG.info(reason)
+            raise exception.BadStoreUri(message=reason)
 
 
 class ChunkedFile(object):

diff --git a/glance/store/sheepdog.py b/glance/store/sheepdog.py
@@ -147,12 +147,12 @@ def get_uri(self):
     def parse_uri(self, uri):
         valid_schema = 'sheepdog://'
         if not uri.startswith(valid_schema):
-            raise exception.BadStoreUri(_("URI must start with %s://") %
-                                        valid_schema)
+            reason = _("URI must start with '%s://'") % valid_schema
+            raise exception.BadStoreUri(message=reason)
         self.image = uri[len(valid_schema):]
         if not utils.is_uuid_like(self.image):
-            raise exception.BadStoreUri(_("URI must contains well-formated "
-                                          "image id"))
+            reason = _("URI must contains well-formated image id")
+            raise exception.BadStoreUri(message=reason)
 
 
 class ImageIterator(object):

diff --git a/glance/store/swift.py b/glance/store/swift.py
@@ -29,7 +29,7 @@
 from glance.common import exception
 from glance.common import swift_store_utils
 from glance.openstack.common import excutils
-from glance.openstack.common.gettextutils import _LI
+from glance.openstack.common import gettextutils
 import glance.openstack.common.log as logging
 import glance.store
 import glance.store.base
@@ -41,6 +41,7 @@
     pass
 
 LOG = logging.getLogger(__name__)
+_LI = gettextutils._LI
 
 DEFAULT_CONTAINER = 'glance'
 DEFAULT_LARGE_OBJECT_SIZE = 5 * 1024  # 5GB
@@ -221,8 +222,8 @@ def _get_conf_value_from_account_ref(self, netloc):
         except KeyError:
             reason = _("Badly formed Swift URI. Credentials not found for"
                        "account reference")
-            LOG.debug(reason)
-            raise exception.BadStoreUri()
+            LOG.info(reason)
+            raise exception.BadStoreUri(message=reason)
         return netloc
 
     def _form_uri_parts(self, netloc, path):
@@ -244,9 +245,9 @@ def _form_uri_parts(self, netloc, path):
         if creds:
             cred_parts = creds.split(':')
             if len(cred_parts) < 2:
-                reason = "Badly formed credentials in Swift URI."
-                LOG.debug(reason)
-                raise exception.BadStoreUri()
+                reason = _("Badly formed credentials in Swift URI.")
+                LOG.info(reason)
+                raise exception.BadStoreUri(message=reason)
             key = cred_parts.pop()
             user = ':'.join(cred_parts)
             creds = urllib.unquote(creds)
@@ -270,9 +271,9 @@ def _form_auth_or_store_url(self, netloc, path):
                 path_parts.insert(0, netloc)
                 self.auth_or_store_url = '/'.join(path_parts)
         except IndexError:
-            reason = "Badly formed Swift URI."
-            LOG.debug(reason)
-            raise exception.BadStoreUri()
+            reason = _("Badly formed Swift URI.")
+            LOG.info(reason)
+            raise exception.BadStoreUri(message=reason)
 
     def parse_uri(self, uri):
         """
@@ -295,7 +296,7 @@ def parse_uri(self, uri):
                        ", you need to change it to use the "
                        "swift+http:// scheme, like so: "
                        "swift+http://user:pass@authurl.com/v1/container/obj")
-            LOG.debug("Invalid store URI: %(reason)s", {'reason': reason})
+            LOG.info(_LI("Invalid store URI: %(reason)s"), {'reason': reason})
             raise exception.BadStoreUri(message=reason)
 
         pieces = urlparse.urlparse(uri)
@@ -691,8 +692,8 @@ def validate_location(self, uri):
 
     def get_connection(self, location):
         if not location.user:
-            reason = "Location is missing user:password information."
-            LOG.debug(reason)
+            reason = _("Location is missing user:password information.")
+            LOG.info(reason)
             raise exception.BadStoreUri(message=reason)
 
         auth_url = location.swift_url
@@ -703,10 +704,10 @@ def get_connection(self, location):
             try:
                 tenant_name, user = location.user.split(':')
             except ValueError:
-                reason = ("Badly formed tenant:user '%(user)s' in "
-                          "Swift URI" % {'user': location.user})
-                LOG.debug(reason)
-                raise exception.BadStoreUri()
+                reason = (_("Badly formed tenant:user '%(user)s' in "
+                            "Swift URI") % {'user': location.user})
+                LOG.info(reason)
+                raise exception.BadStoreUri(message=reason)
         else:
             tenant_name = None
             user = location.user

diff --git a/glance/store/vmware_datastore.py b/glance/store/vmware_datastore.py
@@ -186,10 +186,9 @@ def _is_valid_path(self, path):
 
     def parse_uri(self, uri):
         if not uri.startswith('%s://' % STORE_SCHEME):
-            reason = (_("URI %(uri)s must start with %(scheme)s://") %
-                      {'uri': uri, 'scheme': STORE_SCHEME})
-            LOG.error(reason)
-            raise exception.BadStoreUri(reason)
+            reason = (_("URI must start with %s://") % STORE_SCHEME)
+            LOG.info(reason)
+            raise exception.BadStoreUri(message=reason)
         (self.scheme, self.server_host,
          path, params, query, fragment) = urlparse.urlparse(uri)
         if not query:
@@ -202,9 +201,9 @@ def parse_uri(self, uri):
             self.path = path
             self.query = query
             return
-        reason = 'Badly formed VMware datastore URI %(uri)s.' % {'uri': uri}
-        LOG.debug(reason)
-        raise exception.BadStoreUri(reason)
+        reason = _('Badly formed VMware datastore URI')
+        LOG.info(reason)
+        raise exception.BadStoreUri(message=reason)
 
 
 class Store(glance.store.base.Store):
@@ -403,18 +402,18 @@ def _query(self, location, method, headers, depth=0):
                 msg = 'VMware datastore could not find image at URI.'
                 LOG.debug(msg)
                 raise exception.NotFound(msg)
-            msg = ('HTTP request returned a %(status)s status code.'
-                   % {'status': resp.status})
-            LOG.debug(msg)
-            raise exception.BadStoreUri(msg)
+            reason = (_('HTTP request returned a %(status)s status code.')
+                      % {'status': resp.status})
+            LOG.info(reason)
+            raise exception.BadStoreUri(message=reason)
         location_header = resp.getheader('location')
         if location_header:
             if resp.status not in (301, 302):
-                msg = ("The HTTP URL %(path)s attempted to redirect "
-                       "with an invalid %(status)s status code."
-                       % {'path': loc.path, 'status': resp.status})
-                LOG.debug(msg)
-                raise exception.BadStoreUri(msg)
+                reason = (_("The HTTP URL %(path)s attempted to redirect "
+                          "with an invalid %(status)s status code.")
+                          % {'path': loc.path, 'status': resp.status})
+                LOG.info(reason)
+                raise exception.BadStoreUri(message=reason)
             location_class = glance.store.location.Location
             new_loc = location_class(location.store_name,
                                      location.store_location.__class__,