From bd60603e443ec79b0783bebe2cda48a6b9c89ced Mon Sep 17 00:00:00 2001 From: Jim Rollenhagen Date: Fri, 11 Dec 2015 10:03:09 -0800 Subject: [PATCH] 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 | 6 + etc/ironic/ironic.conf.sample | 126 ++++++++++++++---- ironic/api/app.py | 5 + ironic/api/hooks.py | 5 +- ironic/tests/unit/api/test_hooks.py | 47 ++++++- ...ug-no-api-tracebacks-a8a0caddc9676b06.yaml | 7 + 6 files changed, 164 insertions(+), 32 deletions(-) create mode 100644 releasenotes/notes/debug-no-api-tracebacks-a8a0caddc9676b06.yaml diff --git a/doc/source/deploy/troubleshooting.rst b/doc/source/deploy/troubleshooting.rst index f548920c1e..d2fc5ce226 100644 --- a/doc/source/deploy/troubleshooting.rst +++ b/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. diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 23d05484ec..c45f04c5ed 100644 --- a/etc/ironic/ironic.conf.sample +++ b/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= +#notification_transport_url= # 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= # 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 diff --git a/ironic/api/app.py b/ironic/api/app.py index 1854221de7..d3fdd531a8 100644 --- a/ironic/api/app.py +++ b/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 ' diff --git a/ironic/api/hooks.py b/ironic/api/hooks.py index 73d50466ba..3c406e5548 100644 --- a/ironic/api/hooks.py +++ b/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') diff --git a/ironic/tests/unit/api/test_hooks.py b/ironic/tests/unit/api/test_hooks.py index 0a77236753..c4372a3c40 100644 --- a/ironic/tests/unit/api/test_hooks.py +++ b/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): diff --git a/releasenotes/notes/debug-no-api-tracebacks-a8a0caddc9676b06.yaml b/releasenotes/notes/debug-no-api-tracebacks-a8a0caddc9676b06.yaml new file mode 100644 index 0000000000..70241cbb27 --- /dev/null +++ b/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.