Don't return tracebacks in API response in debug mode

The API should not return tracebacks in a production environment. As
deployers often run services in debug mode, because OpenStack is hard to
debug, we should not return tracebacks in debug mode. To allow
developers etc to continue to use this feature, add a new config option
'debug_tracebacks_in_api' that maintains this behavior.

Also add to troubleshooting docs.

Change-Id: Idbbf7efc45140e9e3d8b9491edd58905cbba0363
Closes-Bug: #1525002

doc/source/deploy/troubleshooting.rst

@@ -70,3 +70,9 @@ A few things should be checked in this case:
    log, it means the conductor run into a special error during deployment.
    So you can check the log carefully to fix or work around and then try
    again.
+
+API Errors
+==========
+
+The `debug_tracebacks_in_api` config option may be set to return tracebacks
+in the API response for all 4xx and 5xx errors.

etc/ironic/ironic.conf.sample

@@ -10,6 +10,11 @@
 # disabled. (string value)
 #auth_strategy=keystone
 
+# Return server tracebacks in the API response for any error
+# responses. WARNING: this is insecure and should not be used
+# in a production environment. (boolean value)
+#debug_tracebacks_in_api=false
+
 # Enable pecan debug mode. WARNING: this is insecure and
 # should not be used in a production environment. (boolean
 # value)
@@ -321,11 +326,11 @@
 # value)
 #rpc_poll_timeout=1
 
-# Configures zmq-messaging to use broker or not. (boolean
+# Shows whether zmq-messaging uses broker or not. (boolean
 # value)
-#zmq_use_broker=false
+#zmq_use_broker=true
 
-# Minimal port number for random ports range. (port value)
+# Minimal port number for random ports range. (integer value)
 #rpc_zmq_min_port=49152
 
 # Maximal port number for random ports range. (integer value)
@@ -338,7 +343,7 @@
 # Host to locate redis. (string value)
 #host=127.0.0.1
 
-# Use this port to connect to redis host. (port value)
+# Use this port to connect to redis host. (integer value)
 #port=6379
 
 # Password for Redis server (optional). (string value)
@@ -351,19 +356,16 @@
 # The Drivers(s) to handle sending notifications. Possible
 # values are messaging, messagingv2, routing, log, test, noop
 # (multi valued)
-# Deprecated group/name - [DEFAULT]/notification_driver
-#driver=
+#notification_driver=
 
 # A URL representing the messaging driver to use for
 # notifications. If not set, we fall back to the same
 # configuration used for RPC. (string value)
-# Deprecated group/name - [DEFAULT]/notification_transport_url
-#transport_url=<None>
+#notification_transport_url=<None>
 
 # AMQP topic used for OpenStack notifications. (list value)
 # Deprecated group/name - [rpc_notifier2]/topics
-# Deprecated group/name - [DEFAULT]/notification_topics
-#topics=notifications
+#notification_topics=notifications
 
 # Seconds to wait for a response from a call. (integer value)
 #rpc_response_timeout=60
@@ -374,7 +376,7 @@
 #transport_url=<None>
 
 # The messaging driver to use, defaults to rabbit. Other
-# drivers include amqp and zmq. (string value)
+# drivers include qpid and zmq. (string value)
 #rpc_backend=rabbit
 
 # The default exchange under which topics are scoped. May be
@@ -971,14 +973,17 @@
 
 # Size of EFI system partition in MiB when configuring UEFI
 # systems for local boot. (integer value)
+# Deprecated group/name - [deploy]/efi_system_partition_size
 #efi_system_partition_size=200
 
 # Block size to use when writing to the nodes disk. (string
 # value)
+# Deprecated group/name - [deploy]/dd_block_size
 #dd_block_size=1M
 
 # Maximum attempts to verify an iSCSI connection is active,
 # sleeping 1 second between attempts. (integer value)
+# Deprecated group/name - [deploy]/iscsi_verify_attempts
 #iscsi_verify_attempts=3
 
 
@@ -1515,7 +1520,7 @@
 # Host to locate redis. (string value)
 #host=127.0.0.1
 
-# Use this port to connect to redis host. (port value)
+# Use this port to connect to redis host. (integer value)
 #port=6379
 
 # Password for Redis server (optional). (string value)
@@ -1680,6 +1685,86 @@
 #password=
 
 
+[oslo_messaging_qpid]
+
+#
+# Options defined in oslo.messaging
+#
+
+# Use durable queues in AMQP. (boolean value)
+# Deprecated group/name - [DEFAULT]/amqp_durable_queues
+# Deprecated group/name - [DEFAULT]/rabbit_durable_queues
+#amqp_durable_queues=false
+
+# Auto-delete queues in AMQP. (boolean value)
+# Deprecated group/name - [DEFAULT]/amqp_auto_delete
+#amqp_auto_delete=false
+
+# Send a single AMQP reply to call message. The current
+# behaviour since oslo-incubator is to send two AMQP replies -
+# first one with the payload, a second one to ensure the other
+# have finish to send the payload. We are going to remove it
+# in the N release, but we must keep backward compatible at
+# the same time. This option provides such compatibility - it
+# defaults to False in Liberty and can be turned on for early
+# adopters with a new installations or for testing. Please
+# note, that this option will be removed in the Mitaka
+# release. (boolean value)
+#send_single_reply=false
+
+# Qpid broker hostname. (string value)
+# Deprecated group/name - [DEFAULT]/qpid_hostname
+#qpid_hostname=localhost
+
+# Qpid broker port. (integer value)
+# Deprecated group/name - [DEFAULT]/qpid_port
+#qpid_port=5672
+
+# Qpid HA cluster host:port pairs. (list value)
+# Deprecated group/name - [DEFAULT]/qpid_hosts
+#qpid_hosts=$qpid_hostname:$qpid_port
+
+# Username for Qpid connection. (string value)
+# Deprecated group/name - [DEFAULT]/qpid_username
+#qpid_username=
+
+# Password for Qpid connection. (string value)
+# Deprecated group/name - [DEFAULT]/qpid_password
+#qpid_password=
+
+# Space separated list of SASL mechanisms to use for auth.
+# (string value)
+# Deprecated group/name - [DEFAULT]/qpid_sasl_mechanisms
+#qpid_sasl_mechanisms=
+
+# Seconds between connection keepalive heartbeats. (integer
+# value)
+# Deprecated group/name - [DEFAULT]/qpid_heartbeat
+#qpid_heartbeat=60
+
+# Transport to use, either 'tcp' or 'ssl'. (string value)
+# Deprecated group/name - [DEFAULT]/qpid_protocol
+#qpid_protocol=tcp
+
+# Whether to disable the Nagle algorithm. (boolean value)
+# Deprecated group/name - [DEFAULT]/qpid_tcp_nodelay
+#qpid_tcp_nodelay=true
+
+# The number of prefetched messages held by receiver. (integer
+# value)
+# Deprecated group/name - [DEFAULT]/qpid_receiver_capacity
+#qpid_receiver_capacity=1
+
+# The qpid topology version to use.  Version 1 is what was
+# originally used by impl_qpid.  Version 2 includes some
+# backwards-incompatible changes that allow broker federation
+# to work.  Users should update to version 2 when they are
+# able to take everything down, as it requires a clean break.
+# (integer value)
+# Deprecated group/name - [DEFAULT]/qpid_topology_version
+#qpid_topology_version=1
+
+
 [oslo_messaging_rabbit]
 
 #
@@ -1731,25 +1816,18 @@
 # Deprecated group/name - [DEFAULT]/kombu_reconnect_delay
 #kombu_reconnect_delay=1.0
 
-# How long to wait a missing client beforce abandoning to send
-# it its replies. This value should not be longer than
+# How long to wait before considering a reconnect attempt to
+# have failed. This value should not be longer than
 # rpc_response_timeout. (integer value)
-# Deprecated group/name - [oslo_messaging_rabbit]/kombu_reconnect_timeout
-#kombu_missing_consumer_retry_timeout=5
-
-# Determines how the next RabbitMQ node is chosen in case the
-# one we are currently connected to becomes unavailable. Takes
-# effect only if more than one RabbitMQ node is provided in
-# config. (string value)
-#kombu_failover_strategy=round-robin
+#kombu_reconnect_timeout=60
 
 # The RabbitMQ broker address where a single node is used.
 # (string value)
 # Deprecated group/name - [DEFAULT]/rabbit_host
 #rabbit_host=localhost
 
-# The RabbitMQ broker port where a single node is used. (port
-# value)
+# The RabbitMQ broker port where a single node is used.
+# (integer value)
 # Deprecated group/name - [DEFAULT]/rabbit_port
 #rabbit_port=5672
 

ironic/api/app.py

@@ -33,6 +33,11 @@ api_opts = [
         help=_('Authentication strategy used by ironic-api: one of "keystone" '
                'or "noauth". "noauth" should not be used in a production '
                'environment because all authentication will be disabled.')),
+    cfg.BoolOpt('debug_tracebacks_in_api',
+                default=False,
+                help=_('Return server tracebacks in the API response for any '
+                       'error responses. WARNING: this is insecure '
+                       'and should not be used in a production environment.')),
     cfg.BoolOpt('pecan_debug',
                 default=False,
                 help=_('Enable pecan debug mode. WARNING: this is insecure '

ironic/api/hooks.py

@@ -138,9 +138,8 @@ class NoExceptionTracebackHook(hooks.PecanHook):
             return
 
         json_body = state.response.json
-        # Do not remove traceback when server in debug mode (except 'Server'
-        # errors when 'debuginfo' will be used for traces).
-        if cfg.CONF.debug and json_body.get('faultcode') != 'Server':
+        # Do not remove traceback when traceback config is set
+        if cfg.CONF.debug_tracebacks_in_api:
             return
 
         faultstring = json_body.get('faultstring')

ironic/tests/unit/api/test_hooks.py

@@ -145,7 +145,7 @@ class TestNoExceptionTracebackHook(base.BaseApiTest):
         actual_msg = json.loads(response.json['error_message'])['faultstring']
         self.assertEqual(expected_msg, actual_msg)
 
-    def test_hook_without_traceback(self):
+    def _test_hook_without_traceback(self):
         msg = "Error message without traceback \n but \n multiline"
         self.root_convert_mock.side_effect = Exception(msg)
 
@@ -154,18 +154,41 @@ class TestNoExceptionTracebackHook(base.BaseApiTest):
         actual_msg = json.loads(response.json['error_message'])['faultstring']
         self.assertEqual(msg, actual_msg)
 
-    def test_hook_server_debug_on_serverfault(self):
+    def test_hook_without_traceback(self):
+        self._test_hook_without_traceback()
+
+    def test_hook_without_traceback_debug(self):
         cfg.CONF.set_override('debug', True)
+        self._test_hook_without_traceback()
+
+    def test_hook_without_traceback_debug_tracebacks(self):
+        cfg.CONF.set_override('debug_tracebacks_in_api', True)
+        self._test_hook_without_traceback()
+
+    def _test_hook_on_serverfault(self):
         self.root_convert_mock.side_effect = Exception(self.MSG_WITH_TRACE)
 
         response = self.get_json('/', path_prefix='', expect_errors=True)
 
         actual_msg = json.loads(
             response.json['error_message'])['faultstring']
-        self.assertEqual(self.MSG_WITHOUT_TRACE, actual_msg)
+        return actual_msg
 
-    def test_hook_server_debug_on_clientfault(self):
+    def test_hook_on_serverfault(self):
+        msg = self._test_hook_on_serverfault()
+        self.assertEqual(self.MSG_WITHOUT_TRACE, msg)
+
+    def test_hook_on_serverfault_debug(self):
         cfg.CONF.set_override('debug', True)
+        msg = self._test_hook_on_serverfault()
+        self.assertEqual(self.MSG_WITHOUT_TRACE, msg)
+
+    def test_hook_on_serverfault_debug_tracebacks(self):
+        cfg.CONF.set_override('debug_tracebacks_in_api', True)
+        msg = self._test_hook_on_serverfault()
+        self.assertEqual(self.MSG_WITH_TRACE, msg)
+
+    def _test_hook_on_clientfault(self):
         client_error = Exception(self.MSG_WITH_TRACE)
         client_error.code = http_client.BAD_REQUEST
         self.root_convert_mock.side_effect = client_error
@@ -174,7 +197,21 @@ class TestNoExceptionTracebackHook(base.BaseApiTest):
 
         actual_msg = json.loads(
             response.json['error_message'])['faultstring']
-        self.assertEqual(self.MSG_WITH_TRACE, actual_msg)
+        return actual_msg
+
+    def test_hook_on_clientfault(self):
+        msg = self._test_hook_on_clientfault()
+        self.assertEqual(self.MSG_WITHOUT_TRACE, msg)
+
+    def test_hook_on_clientfault_debug(self):
+        cfg.CONF.set_override('debug', True)
+        msg = self._test_hook_on_clientfault()
+        self.assertEqual(self.MSG_WITHOUT_TRACE, msg)
+
+    def test_hook_on_clientfault_debug_tracebacks(self):
+        cfg.CONF.set_override('debug_tracebacks_in_api', True)
+        msg = self._test_hook_on_clientfault()
+        self.assertEqual(self.MSG_WITH_TRACE, msg)
 
 
 class TestContextHook(base.BaseApiTest):

releasenotes/notes/debug-no-api-tracebacks-a8a0caddc9676b06.yaml

@@ -0,0 +1,7 @@
+---
+upgrade:
+  - Adds a config option 'debug_tracebacks_in_api' to allow
+    the API service to return tracebacks in API responses
+    in an error condition.
+fixes:
+  - No longer returns tracebacks for API errors in debug mode.