Repository: google/turbinia
Pull request #447 (2019-09-27 22:01:46)

Minor config handling changes for #191
Rework the handling of source-based config file a little, and return friendlier error messages instead of tracebacks on failures to load the config file.

This change:
* adds a warning log when the source-based config file is detected
* adds a usage message that is displayed in a couple of spots when the config file is not found/fails to load, instead of a raw traceback
* renames config/turbinia_config.py to config/turbinia_config_tmpl.py to emphasize it should be treated as a template
* updates documentation references to turbinia_config.py accordingly
* removes the TurbiniaConfigException in favor of TurbiniaException

Tests all seem to pass (but note #446), but happy to make more changes as needed if I've misunderstood or gone in a weird direction with it. [Sample output of some shell commands](https://gist.github.com/johngalvin/d3aba7ab2779bd1bf89ce83a79b115ad) shows what the friendlier message looks like on output (including with a "bad config" failure to load traceback, too).


Patch:
diff --git a/docs/faq.md b/docs/faq.md
index c8228ac..dce004f 100644
--- a/docs/faq.md
+++ b/docs/faq.md
@@ -7,8 +7,8 @@ Configuration can either go into `~/.turbiniarc` or `/etc/turbinia/turbinia.conf
 
 ## Where are the configuration options documented?
 
-The configuration options are documented in [turbinia_config.py](
-https://github.com/google/turbinia/blob/master/turbinia/config/turbinia_config.py)
+The configuration options are documented in [turbinia_config_tmpl.py](
+https://github.com/google/turbinia/blob/master/turbinia/config/turbinia_config_tmpl.py)
 
 ## How can I write new Tasks?
 
@@ -27,4 +27,3 @@ to the Workers that ran that Task.  Files with paths starting with gs:// are
 in the Google Cloud Storage bucket (as specified by `GCS_OUTPUT_PATH` in the
 config).  Only Evidence types with the `copyable` property will actually be
 copied into Cloud Storage.
-
diff --git a/docs/install.md b/docs/install.md
index 3227f3f..8e444c6 100644
--- a/docs/install.md
+++ b/docs/install.md
@@ -36,7 +36,7 @@ the GoogleCloudDiskRawEmbedded evidence type.  This can be used
 when the images do not originate from VMs/containers that are running in the
 cloud.
 
-Another option is to 
+Another option is to
 [convert the raw image to a cloud image](https://cloud.google.com/compute/docs/images/import-existing-image),
 and then create a Persistent Disk from that and process it as the GoogleCloudDisk
 Evidence type.
@@ -92,7 +92,7 @@ single instance.
 *   Select a GCP region that you wish to deploy Turbinia into.  Note
     that one of the GCP dependencies is Cloud Functions, and that
     only works in certain regions, so you will need to deploy in one
-    of [those supported regions](https://cloud.google.com/functions/docs/locations).     
+    of [those supported regions](https://cloud.google.com/functions/docs/locations).
 *   Enable
     [Cloud Functions](https://console.cloud.google.com/apis/library/cloudfunctions.googleapis.com)
 *   Follow the
@@ -150,7 +150,7 @@ single instance.
 *   Configure the init scripts to run Turbinia on start by copying the
     file [turbinia/tools/turbinia@.service](https://github.com/google/turbinia/blob/master/tools/turbinia%40.service)
     to `/etc/systemd/system/turbinia@server` for the server or
-    `/etc/systemd/system/turbinia@psqworker` for a GCP worker or 
+    `/etc/systemd/system/turbinia@psqworker` for a GCP worker or
     `/etc/systemd/system/turbinia@celeryworker` for a local (non-cloud)
     installation.
 *   If you are running Turbinia locally, skip to
@@ -202,7 +202,7 @@ single instance.
         for other packaged installation.
 *   Create and configure the Turbinia configuration file in `/etc/turbinia/turbinia.conf`.
     *   `sudo mkdir /etc/turbinia`
-    *   `wget -nd https://raw.githubusercontent.com/google/turbinia/master/turbinia/config/turbinia_config.py -O /etc/turbinia/turbinia.conf`
+    *   `wget -nd https://raw.githubusercontent.com/google/turbinia/master/turbinia/config/turbinia_config_tmpl.py -O /etc/turbinia/turbinia.conf`
         * ***Note*** This is the configuration file from HEAD, but
           make sure to copy the correct version for the release you
           are installing.
@@ -212,7 +212,7 @@ single instance.
     *   ***NOTE***: Match the `PUBSUB_TOPIC` variable in the configuration to
         the name of the topic you created in GCP.
     *   ***NOTE***: If you are running Turbinia locally, make sure to set `GCS_OUTPUT_PATH` to `None`.
-*   If you are running Turbinia locally, return to 
+*   If you are running Turbinia locally, return to
     [Install additional dependencies](#2-install-additional-dependencies),
     otherwise continue to [Deploy the Cloud Functions](#deploy-the-cloud-functions).
 
diff --git a/turbinia/config/__init__.py b/turbinia/config/__init__.py
index 23106d3..3f0457e 100644
--- a/turbinia/config/__init__.py
+++ b/turbinia/config/__init__.py
@@ -22,16 +22,22 @@ import logging
 import os
 import sys
 
+from turbinia import TurbiniaException
+
 DATETIME_FORMAT = '%Y-%m-%dT%H:%M:%S.%fZ'
 
 # Look for config files with these names
-CONFIGFILES = ['.turbiniarc', 'turbinia.conf', 'turbinia_config.py']
-# Look in homedir first, then /etc/turbinia, and finally in the source
-# config dir for config files
+CONFIGFILES = ['.turbiniarc', 'turbinia.conf', 'turbinia_config_tmpl.py']
+# Look in homedir first, then /etc/turbinia
 CONFIGPATH = [
-    os.path.expanduser('~'), '/etc/turbinia',
-    os.path.dirname(os.path.abspath(__file__))
+    os.path.expanduser('~'),
+    '/etc/turbinia',
+    os.path.dirname(os.path.abspath(__file__)),
 ]
+# Config setup reminder for cleaner error handling on empty configs.
+CONFIG_MSG = (
+    'Copy turbinia/config/turbinia_config_tmpl.py to ~/.turbiniarc '
+    'or /etc/turbinia/turbinia.conf, edit, and re-run.')
 
 # Required config vars
 REQUIRED_VARS = [
@@ -83,11 +89,6 @@ CONFIG = None
 log = logging.getLogger('turbinia')
 
 
-class TurbiniaConfigException(Exception):
-  """Exception for Turbinia configuration."""
-  pass
-
-
 def LoadConfig(config_file=None):
   """Finds Turbinia config file and loads it.
 
@@ -119,9 +120,13 @@ def LoadConfig(config_file=None):
         break
 
   if config_file is None:
-    raise TurbiniaConfigException('No config files found')
+    raise TurbiniaException('No config files found')
 
   log.debug('Loading config from {0:s}'.format(config_file))
+  # Warn about using fallback source config, but it's currently necessary for
+  # tests. See issue #446.
+  if 'turbinia_config_tmpl' in config_file:
+    log.warning('Using fallback source config. {0:s}'.format(CONFIG_MSG))
   try:
     _config = imp.load_source('config', config_file)
   except IOError as exception:
@@ -129,7 +134,7 @@ def LoadConfig(config_file=None):
         'Could not load config file {0:s}: {1!s}'.format(
             config_file, exception))
     log.error(message)
-    raise TurbiniaConfigException(message)
+    raise TurbiniaException(message)
 
   _config.configSource = config_file
   ValidateAndSetConfig(_config)
@@ -153,10 +158,10 @@ def ValidateAndSetConfig(_config):
   CONFIGVARS = REQUIRED_VARS + OPTIONAL_VARS
   for var in CONFIGVARS:
     if not hasattr(_config, var):
-      raise TurbiniaConfigException(
+      raise TurbiniaException(
           'No config attribute {0:s}:{1:s}'.format(_config.configSource, var))
     if var in REQUIRED_VARS and getattr(_config, var) is None:
-      raise TurbiniaConfigException(
+      raise TurbiniaException(
           'Config attribute {0:s}:{1:s} is not set'.format(
               _config.configSource, var))
 
diff --git a/turbinia/config/logger.py b/turbinia/config/logger.py
index b2822e1..4879d6e 100644
--- a/turbinia/config/logger.py
+++ b/turbinia/config/logger.py
@@ -19,6 +19,7 @@ import logging
 import warnings
 
 from turbinia import config
+from turbinia import TurbiniaException
 
 
 def setup(need_file_handler=True, need_stream_handler=True):
@@ -52,7 +53,14 @@ def setup(need_file_handler=True, need_stream_handler=True):
         need_stream_handler = False
 
   if need_file_handler:
-    config.LoadConfig()
+    try:
+      config.LoadConfig()
+    except TurbiniaException as exception:
+      print(
+          'Could not load config file ({0:s}).\n{1:s}'.format(
+              exception, config.CONFIG_MSG))
+      sys.exit(1)
+
     file_handler = logging.FileHandler(config.LOG_FILE)
     formatter = logging.Formatter('%(asctime)s:%(levelname)s:%(message)s')
     file_handler.setFormatter(formatter)
diff --git a/turbinia/config/turbinia_config.py b/turbinia/config/turbinia_config_tmpl.py
similarity index 100%
rename from turbinia/config/turbinia_config.py
rename to turbinia/config/turbinia_config_tmpl.py
diff --git a/turbinia/turbiniactl.py b/turbinia/turbiniactl.py
index 9022f22..19cb58c 100644
--- a/turbinia/turbiniactl.py
+++ b/turbinia/turbiniactl.py
@@ -26,6 +26,7 @@ import os
 import sys
 
 from turbinia import config
+from turbinia import TurbiniaException
 from turbinia.config import logger
 from turbinia.lib import libcloudforensics
 from turbinia import __version__
@@ -350,10 +351,17 @@ def main():
 
   # Load the config before final logger setup so we can the find the path to the
   # log file.
-  if args.config_file:
-    config.LoadConfig(config_file=args.config_file)
-  else:
-    config.LoadConfig()
+  try:
+    if args.config_file:
+      config.LoadConfig(config_file=args.config_file)
+    else:
+      config.LoadConfig()
+  except TurbiniaException as exception:
+    print(
+        'Could not load config file ({0:s}).\n{1:s}'.format(
+            exception, config.CONFIG_MSG))
+    sys.exit(1)
+
   if args.log_file:
     config.LOG_FILE = args.log_file
   if args.output_dir:


Test patch:
diff --git a/turbinia/config/turbinia_config_test.py b/turbinia/config/turbinia_config_test.py
index 5c06e8b..2a63456 100644
--- a/turbinia/config/turbinia_config_test.py
+++ b/turbinia/config/turbinia_config_test.py
@@ -21,6 +21,7 @@ import tempfile
 import unittest
 
 from turbinia import config
+from turbinia import TurbiniaException
 
 
 class TestTurbiniaConfig(unittest.TestCase):
@@ -92,13 +93,13 @@ class TestTurbiniaConfig(unittest.TestCase):
     """Test that config errors out when not all required variables exist."""
     self.WriteConfig('EXISTS = "bar"\n')
     config.REQUIRED_VARS = ['DOESNOTEXIST', 'EXISTS']
-    self.assertRaises(config.TurbiniaConfigException, config.LoadConfig)
+    self.assertRaises(TurbiniaException, config.LoadConfig)
 
   def testUnsetRequiredKeyConfig(self):
     """Test that config errors out when not all required variables are set."""
     self.WriteConfig('UNSETKEY = None\nSETKEY = "bar"\n')
     config.REQUIRED_VARS = ['UNSETKEY', 'SETKEY']
-    self.assertRaises(config.TurbiniaConfigException, config.LoadConfig)
+    self.assertRaises(TurbiniaException, config.LoadConfig)
 
   def testUnsetOptionalKeyConfig(self):
     """Test that optional vars don't need to be set."""
@@ -112,12 +113,11 @@ class TestTurbiniaConfig(unittest.TestCase):
   def testMissingConfigPath(self):
     """Test non-existent config path."""
     config.CONFIGPATH = ['DOESNOTEXIST']
-    self.assertRaises(config.TurbiniaConfigException, config.LoadConfig)
+    self.assertRaises(TurbiniaException, config.LoadConfig)
 
   def testMissingConfigFile(self):
     """Test non-existent config file."""
-    self.assertRaises(
-        config.TurbiniaConfigException, config.LoadConfig, '/does/not/exist')
+    self.assertRaises(TurbiniaException, config.LoadConfig, '/does/not/exist')
 
   def testExplicitConfigPath(self):
     """Test setting direct config file path."""