Merge "Don't return tracebacks in API response in debug mode"

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.