From 8d279c8cc461e0f12d18231e5dfc9dd8027caa3d Mon Sep 17 00:00:00 2001
From: Jake Moshenko <jake.moshenko@coreos.com>
Date: Fri, 14 Apr 2017 11:18:01 -0400
Subject: [PATCH] Unify app and api exception handling Move some confi to an
 immutable section Make ApiExceptions real werkzeug exceptions

---
 config.py                 | 75 +++++++++++++++++++++------------------
 endpoints/api/__init__.py | 21 -----------
 endpoints/exception.py    | 10 ++++--
 3 files changed, 47 insertions(+), 59 deletions(-)

diff --git a/config.py b/config.py
index 89e1c46ee..d5febdee3 100644
--- a/config.py
+++ b/config.py
@@ -35,7 +35,45 @@ def frontend_visible_config(config_dict):
   return visible_dict
 
 
-class DefaultConfig(object):
+# Configuration that should not be changed by end users
+class ImmutableConfig(object):
+
+  # Requests based HTTP client with a large request pool
+  HTTPCLIENT = build_requests_session()
+
+  # Status tag config
+  STATUS_TAGS = {}
+  for tag_name in ['building', 'failed', 'none', 'ready', 'cancelled']:
+    tag_path = os.path.join('buildstatus', tag_name + '.svg')
+    with open(tag_path) as tag_svg:
+      STATUS_TAGS[tag_name] = tag_svg.read()
+
+  # Reverse DNS prefixes that are reserved for internal use on labels and should not be allowable
+  # to be set via the API.
+  DEFAULT_LABEL_KEY_RESERVED_PREFIXES = ['com.docker.', 'io.docker.', 'org.dockerproject.',
+                                         'org.opencontainers.', 'io.cncf.',
+                                         'io.kubernetes.', 'io.k8s.',
+                                         'io.quay', 'com.coreos', 'com.tectonic',
+                                         'internal', 'quay']
+
+  # Colors for local avatars.
+  AVATAR_COLORS = ['#969696', '#aec7e8', '#ff7f0e', '#ffbb78', '#2ca02c', '#98df8a', '#d62728',
+                   '#ff9896', '#9467bd', '#c5b0d5', '#8c564b', '#c49c94', '#e377c2', '#f7b6d2',
+                   '#7f7f7f', '#c7c7c7', '#bcbd22', '#1f77b4', '#17becf', '#9edae5', '#393b79',
+                   '#5254a3', '#6b6ecf', '#9c9ede', '#9ecae1', '#31a354', '#b5cf6b', '#a1d99b',
+                   '#8c6d31', '#ad494a', '#e7ba52', '#a55194']
+
+  # Colors for channels.
+  CHANNEL_COLORS = ['#969696', '#aec7e8', '#ff7f0e', '#ffbb78', '#2ca02c', '#98df8a', '#d62728',
+                    '#ff9896', '#9467bd', '#c5b0d5', '#8c564b', '#c49c94', '#e377c2', '#f7b6d2',
+                    '#7f7f7f', '#c7c7c7', '#bcbd22', '#1f77b4', '#17becf', '#9edae5', '#393b79',
+                    '#5254a3', '#6b6ecf', '#9c9ede', '#9ecae1', '#31a354', '#b5cf6b', '#a1d99b',
+                    '#8c6d31', '#ad494a', '#e7ba52', '#a55194']
+
+  PROPAGATE_EXCEPTIONS = True
+
+
+class DefaultConfig(ImmutableConfig):
   # Flask config
   JSONIFY_PRETTYPRINT_REGULAR = False
   SESSION_COOKIE_SECURE = False
@@ -125,16 +163,6 @@ class DefaultConfig(object):
   # Gitlab Config.
   GITLAB_TRIGGER_CONFIG = None
 
-  # Requests based HTTP client with a large request pool
-  HTTPCLIENT = build_requests_session()
-
-  # Status tag config
-  STATUS_TAGS = {}
-  for tag_name in ['building', 'failed', 'none', 'ready', 'cancelled']:
-    tag_path = os.path.join('buildstatus', tag_name + '.svg')
-    with open(tag_path) as tag_svg:
-      STATUS_TAGS[tag_name] = tag_svg.read()
-
   NOTIFICATION_QUEUE_NAME = 'notification'
   DOCKERFILE_BUILD_QUEUE_NAME = 'dockerfilebuild'
   REPLICATION_QUEUE_NAME = 'imagestoragereplication'
@@ -239,7 +267,6 @@ class DefaultConfig(object):
   # See: https://github.com/docker/docker/blob/master/registry/session.go#L320
   LIBRARY_NAMESPACE = 'library'
 
-
   BUILD_MANAGER = ('enterprise', {})
 
   DISTRIBUTED_STORAGE_CONFIG = {
@@ -265,9 +292,6 @@ class DefaultConfig(object):
   ACTION_LOG_ARCHIVE_LOCATION = 'local_us'
   ACTION_LOG_ARCHIVE_PATH = 'actionlogarchive/'
 
-  # For enterprise:
-  MAXIMUM_REPOSITORY_USAGE = 20
-
   # System logs.
   SYSTEM_LOGS_PATH = "/var/log/"
   SYSTEM_LOGS_FILE = "/var/log/syslog"
@@ -293,11 +317,6 @@ class DefaultConfig(object):
 
   # The various avatar background colors.
   AVATAR_KIND = 'local'
-  AVATAR_COLORS = ['#969696', '#aec7e8', '#ff7f0e', '#ffbb78', '#2ca02c', '#98df8a', '#d62728',
-                   '#ff9896', '#9467bd', '#c5b0d5', '#8c564b', '#c49c94', '#e377c2', '#f7b6d2',
-                   '#7f7f7f', '#c7c7c7', '#bcbd22', '#1f77b4', '#17becf', '#9edae5', '#393b79',
-                   '#5254a3', '#6b6ecf', '#9c9ede', '#9ecae1', '#31a354', '#b5cf6b', '#a1d99b',
-                   '#8c6d31', '#ad494a', '#e7ba52', '#a55194']
 
   # The location of the Quay documentation.
   DOCUMENTATION_LOCATION = 'http://docs.quay.io'
@@ -402,14 +421,6 @@ class DefaultConfig(object):
   # Namespace prefix for all prometheus metrics.
   PROMETHEUS_NAMESPACE = 'quay'
 
-  # Reverse DNS prefixes that are reserved for internal use on labels and should not be allowable
-  # to be set via the API.
-  DEFAULT_LABEL_KEY_RESERVED_PREFIXES = ['com.docker.', 'io.docker.', 'org.dockerproject.',
-                                         'org.opencontainers.', 'io.cncf.',
-                                         'io.kubernetes.', 'io.k8s.',
-                                         'io.quay', 'com.coreos', 'com.tectonic',
-                                         'internal', 'quay']
-
   # Overridable list of reverse DNS prefixes that are reserved for internal use on labels.
   LABEL_KEY_RESERVED_PREFIXES = []
 
@@ -432,7 +443,7 @@ class DefaultConfig(object):
 
   # Server where TUF metadata can be found
   TUF_SERVER = None
-  
+
   # Prefix to add to metadata e.g. <prefix>/<namespace>/<reponame>
   TUF_GUN_PREFIX = None
 
@@ -444,9 +455,3 @@ class DefaultConfig(object):
   TEAM_RESYNC_STALE_TIME = '30m'
   TEAM_SYNC_WORKER_FREQUENCY = 60 # seconds
 
-  # Colors for channels.
-  CHANNEL_COLORS = ['#969696', '#aec7e8', '#ff7f0e', '#ffbb78', '#2ca02c', '#98df8a', '#d62728',
-                    '#ff9896', '#9467bd', '#c5b0d5', '#8c564b', '#c49c94', '#e377c2', '#f7b6d2',
-                    '#7f7f7f', '#c7c7c7', '#bcbd22', '#1f77b4', '#17becf', '#9edae5', '#393b79',
-                    '#5254a3', '#6b6ecf', '#9c9ede', '#9ecae1', '#31a354', '#b5cf6b', '#a1d99b',
-                    '#8c6d31', '#ad494a', '#e7ba52', '#a55194']
diff --git a/endpoints/api/__init__.py b/endpoints/api/__init__.py
index f434d1fd5..f47649ec1 100644
--- a/endpoints/api/__init__.py
+++ b/endpoints/api/__init__.py
@@ -39,27 +39,6 @@ api_bp = Blueprint('api', __name__)
 class ApiExceptionHandlingApi(Api):
   @crossdomain(origin='*', headers=['Authorization', 'Content-Type'])
   def handle_error(self, error):
-    # TODO: Fix this into a proper error registration model that works in *both* Flask and
-    # Flask-Restful!
-    if isinstance(error, model.DataModelException):
-      return handle_dme(error)
-
-    if isinstance(error, CannotSendEmailException):
-      return handle_emailexception(error)
-
-    if isinstance(error, CannotWriteConfigException):
-      return handle_configexception(error)
-
-    if isinstance(error, model.TooManyLoginAttemptsException):
-      return handle_too_many_login_attempts(error)
-
-    if isinstance(error, ApiException):
-      response = Response(json.dumps(error.to_dict()), error.status_code,
-                          mimetype='application/json')
-      if error.status_code == 401:
-        response.headers['WWW-Authenticate'] = ('Bearer error="%s" error_description="%s"' %
-                                                (error.error_type.value, error.error_description))
-      return response
     return super(ApiExceptionHandlingApi, self).handle_error(error)
 
 
diff --git a/endpoints/exception.py b/endpoints/exception.py
index 59fe45ea9..537f3f167 100644
--- a/endpoints/exception.py
+++ b/endpoints/exception.py
@@ -1,6 +1,7 @@
 from enum import Enum
 
 from flask import url_for
+from werkzeug.exceptions import HTTPException
 
 from auth.auth_context import get_authenticated_user
 
@@ -32,7 +33,7 @@ ERROR_DESCRIPTION = {
 }
 
 
-class ApiException(Exception):
+class ApiException(HTTPException):
   """
   Represents an error in the application/problem+json format.
 
@@ -58,9 +59,12 @@ class ApiException(Exception):
   def __init__(self, error_type, status_code, error_description, payload=None):
     Exception.__init__(self)
     self.error_description = error_description
-    self.status_code = status_code
+    self.code = status_code
     self.payload = payload
     self.error_type = error_type
+    self.data = self.to_dict()
+
+    super(ApiException, self).__init__(error_description, None)
 
   def to_dict(self):
     rv = dict(self.payload or ())
@@ -72,7 +76,7 @@ class ApiException(Exception):
     rv['error_type'] = self.error_type.value  # TODO: deprecate
     rv['title'] = self.error_type.value
     rv['type'] = url_for('api.error', error_type=self.error_type.value, _external=True)
-    rv['status'] = self.status_code
+    rv['status'] = self.code
 
     return rv