Tweak log configuration logic to silence messages

st2client/st2client/base.py

@@ -153,24 +153,28 @@ def get_client(self, args, debug=False):
 
         return client
 
-    def _get_config_file_options(self, args):
+    def _get_config_file_options(self, args, validate_config_permissions=True):
         """
         Parse the config and return kwargs which can be passed to the Client
         constructor.
 
         :rtype: ``dict``
         """
-        rc_options = self._parse_config_file(args=args)
+        rc_options = self._parse_config_file(
+            args=args, validate_config_permissions=validate_config_permissions)
         result = {}
         for kwarg_name, (section, option) in six.iteritems(CONFIG_OPTION_TO_CLIENT_KWARGS_MAP):
             result[kwarg_name] = rc_options.get(section, {}).get(option, None)
 
         return result
 
-    def _parse_config_file(self, args):
+    def _parse_config_file(self, args, validate_config_permissions=True):
         config_file_path = self._get_config_file_path(args=args)
 
-        parser = CLIConfigParser(config_file_path=config_file_path, validate_config_exists=False)
+        parser = CLIConfigParser(config_file_path=config_file_path,
+                                 validate_config_exists=False,
+                                 validate_config_permissions=validate_config_permissions,
+                                 log=self.LOG)
         result = parser.parse()
         return result
 
@@ -258,10 +262,14 @@ def _get_cached_auth_token(self, client, username, password):
 
         if others_st_mode >= 2:
             # Every user has access to this file which is dangerous
-            message = ('Permissions (%s) for cached token file "%s" are to permissive. Please '
+            message = ('Permissions (%s) for cached token file "%s" are too permissive. Please '
                        'restrict the permissions and make sure only your own user can read '
-                       'from the file' % (file_st_mode, cached_token_path))
-            self.LOG.warn(message)
+                       'from or write to the file.'
+                       '\n\n'
+                       'You can fix this by running:'
+                       '\n\n'
+                       '    chmod o-rw %s')
+            self.LOG.warn(message % (file_st_mode, cached_token_path, cached_token_path))
 
         with open(cached_token_path) as fp:
             data = fp.read()
@@ -356,7 +364,7 @@ def _get_cached_token_path_for_user(self, username):
         return result
 
     def _print_config(self, args):
-        config = self._parse_config_file(args=args)
+        config = self._parse_config_file(args=args, validate_config_permissions=False)
 
         for section, options in six.iteritems(config):
             print('[%s]' % (section))

st2client/st2client/config_parser.py

@@ -121,13 +121,18 @@
 
 
 class CLIConfigParser(object):
-    LOG = logging.getLogger(__name__)  # logger instance to use
-
-    def __init__(self, config_file_path, validate_config_exists=True):
+    def __init__(self, config_file_path, validate_config_exists=True,
+                 validate_config_permissions=True, log=None):
         if validate_config_exists and not os.path.isfile(config_file_path):
             raise ValueError('Config file "%s" doesn\'t exist')
 
+        if log is None:
+            log = logging.getLogger(__name__)
+            logging.basicConfig()
+
         self.config_file_path = config_file_path
+        self.validate_config_permissions = validate_config_permissions
+        self.LOG = log
 
     def parse(self):
         """
@@ -143,37 +148,38 @@ def parse(self):
 
         config_dir_path = os.path.dirname(self.config_file_path)
 
-        # Make sure the directory permissions == 0o770
-        if bool(os.stat(config_dir_path).st_mode & 0o777 ^ 0o770):
-            self.LOG.warn(
-                # TODO: Perfect place for an f-string
-                "The StackStorm configuration directory permissions are "
-                "insecure (too permissive)."
-                "\n\n"
-                "You can fix this by running:"
-                "\n\n"
-                "chmod 770 {config_dir}".format(config_dir=config_dir_path))
-
-        # Make sure the setgid bit is set on the directory
-        if not bool(os.stat(config_dir_path).st_mode & 0o2000):
-            self.LOG.info(
-                # TODO: Perfect place for an f-string
-                "The SGID bit is not set on the StackStorm configuration "
-                "directory."
-                "\n\n"
-                "You can fix this by running:"
-                "\n\n"
-                "chmod g+s {config_dir}".format(config_dir=config_dir_path))
-
-        # Make sure the file permissions == 0o660
-        if bool(os.stat(self.config_file_path).st_mode & 0o777 ^ 0o660):
-            self.LOG.warn(
-                # TODO: Another perfect place for an f-string
-                "The StackStorm configuration file permissions are insecure."
-                "\n\n"
-                "You can fix this by running:"
-                "\n\n"
-                "chmod 660 {config_file}".format(config_file=self.config_file_path))
+        if self.validate_config_permissions:
+            # Make sure the directory permissions == 0o770
+            if bool(os.stat(config_dir_path).st_mode & 0o777 ^ 0o770):
+                self.LOG.warn(
+                    # TODO: Perfect place for an f-string
+                    "The StackStorm configuration directory permissions are "
+                    "insecure (too permissive)."
+                    "\n\n"
+                    "You can fix this by running:"
+                    "\n\n"
+                    "    chmod 770 {config_dir}\n".format(config_dir=config_dir_path))
+
+            # Make sure the setgid bit is set on the directory
+            if not bool(os.stat(config_dir_path).st_mode & 0o2000):
+                self.LOG.info(
+                    # TODO: Perfect place for an f-string
+                    "The SGID bit is not set on the StackStorm configuration "
+                    "directory."
+                    "\n\n"
+                    "You can fix this by running:"
+                    "\n\n"
+                    "    chmod g+s {config_dir}\n".format(config_dir=config_dir_path))
+
+            # Make sure the file permissions == 0o660
+            if bool(os.stat(self.config_file_path).st_mode & 0o777 ^ 0o660):
+                self.LOG.warn(
+                    # TODO: Another perfect place for an f-string
+                    "The StackStorm configuration file permissions are insecure."
+                    "\n\n"
+                    "You can fix this by running:"
+                    "\n\n"
+                    "    chmod 660 {config_file}\n".format(config_file=self.config_file_path))
 
         config = ConfigParser()
         with io.open(self.config_file_path, 'r', encoding='utf8') as fp:

st2client/st2client/shell.py

@@ -365,7 +365,7 @@ def run(self, argv):
             return 3
 
         # Parse config and store it in the config module
-        config = self._parse_config_file(args=args)
+        config = self._parse_config_file(args=args, validate_config_permissions=False)
         set_config(config=config)
 
         self._check_locale_and_print_warning()

st2client/tests/unit/test_config_parser.py

@@ -123,7 +123,7 @@ def test_warn_on_bad_config_permissions(self):
                 "\n\n"
                 "You can fix this by running:"
                 "\n\n"
-                "chmod g+s {config_dir}".format(config_dir=TEMP_CONFIG_DIR),
+                "    chmod g+s {config_dir}\n".format(config_dir=TEMP_CONFIG_DIR),
                 parser.LOG.info.call_args_list[0][0][0])
 
             self.assertEqual(parser.LOG.warn.call_count, 2)
@@ -133,15 +133,15 @@ def test_warn_on_bad_config_permissions(self):
                 "\n\n"
                 "You can fix this by running:"
                 "\n\n"
-                "chmod 770 {config_dir}".format(config_dir=TEMP_CONFIG_DIR),
+                "    chmod 770 {config_dir}\n".format(config_dir=TEMP_CONFIG_DIR),
                 parser.LOG.warn.call_args_list[0][0][0])
 
             self.assertEqual(
                 "The StackStorm configuration file permissions are insecure."
                 "\n\n"
                 "You can fix this by running:"
                 "\n\n"
-                "chmod 660 {config_file}".format(config_file=TEMP_FILE_PATH),
+                "    chmod 660 {config_file}\n".format(config_file=TEMP_FILE_PATH),
                 parser.LOG.warn.call_args_list[1][0][0])
 
             # Make sure we left the file alone
@@ -159,6 +159,56 @@ def test_warn_on_bad_config_permissions(self):
                 os.removedirs(TEMP_CONFIG_DIR)
                 self.assertFalse(os.path.exists(TEMP_FILE_PATH))
 
+    def test_disable_permissions_warnings(self):
+        TEMP_FILE_PATH = os.path.join('st2config', '.st2', 'config')
+        TEMP_CONFIG_DIR = os.path.dirname(TEMP_FILE_PATH)
+
+        if os.path.exists(TEMP_FILE_PATH):
+            os.remove(TEMP_FILE_PATH)
+        self.assertFalse(os.path.exists(TEMP_FILE_PATH))
+
+        if os.path.exists(TEMP_CONFIG_DIR):
+            os.removedirs(TEMP_CONFIG_DIR)
+        self.assertFalse(os.path.exists(TEMP_CONFIG_DIR))
+
+        try:
+            # Setup the config directory
+            os.makedirs(TEMP_CONFIG_DIR)
+            os.chmod(TEMP_CONFIG_DIR, 0o0755)
+
+            self.assertNotEqual(os.stat(TEMP_CONFIG_DIR).st_mode & 0o7777, 0o0770)
+
+            # Setup the config file
+            shutil.copyfile(CONFIG_FILE_PATH_FULL, TEMP_FILE_PATH)
+            os.chmod(TEMP_FILE_PATH, 0o664)
+
+            self.assertNotEqual(os.stat(TEMP_FILE_PATH).st_mode & 0o777, 0o770)
+
+            parser = CLIConfigParser(config_file_path=TEMP_FILE_PATH,
+                                     validate_config_exists=True,
+                                     validate_config_permissions=False)
+            parser.LOG = mock.Mock()
+
+            result = parser.parse()  # noqa F841
+
+            self.assertEqual(parser.LOG.info.call_count, 0)
+            self.assertEqual(parser.LOG.warn.call_count, 0)
+
+            # Make sure we left the file alone
+            self.assertTrue(os.path.exists(TEMP_FILE_PATH))
+            self.assertEqual(os.stat(TEMP_FILE_PATH).st_mode & 0o777, 0o664)
+
+            self.assertTrue(os.path.exists(TEMP_CONFIG_DIR))
+            self.assertEqual(os.stat(TEMP_CONFIG_DIR).st_mode & 0o7777, 0o0755)
+        finally:
+            if os.path.exists(TEMP_FILE_PATH):
+                os.remove(TEMP_FILE_PATH)
+                self.assertFalse(os.path.exists(TEMP_FILE_PATH))
+
+            if os.path.exists(TEMP_CONFIG_DIR):
+                os.removedirs(TEMP_CONFIG_DIR)
+                self.assertFalse(os.path.exists(TEMP_FILE_PATH))
+
     def test_parse(self):
         # File doesn't exist
         parser = CLIConfigParser(config_file_path='doesnotexist', validate_config_exists=False)

st2client/tests/unit/test_shell.py

@@ -22,6 +22,7 @@
 import logging
 import tempfile
 
+import requests
 import six
 import mock
 import unittest2
@@ -461,6 +462,29 @@ def _write_mock_package_metadata_file(self):
 
         return package_metadata_path
 
+    @mock.patch.object(
+        requests, 'get',
+        mock.MagicMock(return_value=base.FakeResponse("{}", 200, 'OK')))
+    def test_dont_warn_multiple_times(self):
+        mock_temp_dir_path = tempfile.mkdtemp()
+        mock_config_dir_path = os.path.join(mock_temp_dir_path, 'testconfig')
+        mock_config_path = os.path.join(mock_config_dir_path, 'config')
+
+        os.makedirs(mock_config_dir_path)
+
+        old_perms = os.stat(mock_config_dir_path).st_mode
+        new_perms = old_perms | 0o7
+        os.chmod(mock_config_dir_path, new_perms)
+
+        shell = Shell()
+        shell.LOG = mock.Mock()
+
+        # Test without token.
+        shell.run(['--config-file', mock_config_path, 'action', 'list'])
+
+        self.assertEqual(shell.LOG.info.call_count, 1)
+        self.assertEqual(shell.LOG.warn.call_count, 2)
+
 
 class CLITokenCachingTestCase(unittest2.TestCase):
     def setUp(self):
@@ -553,7 +577,7 @@ def test_get_cached_auth_token_invalid_permissions(self):
         self.assertEqual(shell.LOG.warn.call_count, 1)
         log_message = shell.LOG.warn.call_args[0][0]
 
-        expected_msg = ('Permissions .*? for cached token file .*? are to permissive')
+        expected_msg = ('Permissions .*? for cached token file .*? are too permissive.*')
         self.assertRegexpMatches(log_message, expected_msg)
 
     def test_cache_auth_token_invalid_permissions(self):