[pull] master from bugy:master

src/config/config_service.py

@@ -3,6 +3,7 @@
 import os
 import re
 import shutil
+from datetime import datetime
 from typing import NamedTuple, Optional
 
 from auth.authorization import Authorizer
@@ -14,8 +15,6 @@
 from utils.file_utils import to_filename
 from utils.process_utils import ProcessInvoker
 from utils.string_utils import is_blank, strip
-from datetime import datetime
-
 
 SCRIPT_EDIT_CODE_MODE = 'new_code'
 SCRIPT_EDIT_UPLOAD_MODE = 'upload_script'
@@ -56,12 +55,19 @@ def _create_archive_filename(filename):
 
 
 class ConfigService:
-    def __init__(self, authorizer, conf_folder, process_invoker: ProcessInvoker) -> None:
+    def __init__(
+            self,
+            authorizer,
+            conf_folder,
+            group_scripts_by_folder: bool,
+            process_invoker: ProcessInvoker) -> None:
+
         self._authorizer = authorizer  # type: Authorizer
         self._script_configs_folder = os.path.join(conf_folder, 'runners')
         self._scripts_folder = os.path.join(conf_folder, 'scripts')
         self._scripts_deleted_folder = os.path.join(conf_folder, 'deleted')
         self._process_invoker = process_invoker
+        self._group_scripts_by_folder = group_scripts_by_folder
 
         file_utils.prepare_folder(self._script_configs_folder)
         file_utils.prepare_folder(self._scripts_deleted_folder)
@@ -117,7 +123,7 @@ def update_config(self, user, config, filename, uploaded_script):
 
         with open(original_file_path, 'r') as f:
             original_config_json = json.load(f)
-            short_original_config = script_config.read_short(original_file_path, original_config_json)
+            short_original_config = self.read_short_config(original_config_json, original_file_path)
 
         name = config['name']
 
@@ -133,6 +139,12 @@ def update_config(self, user, config, filename, uploaded_script):
         LOGGER.info('Updating script config "' + name + '" in ' + original_file_path)
         self._save_config(config, original_file_path)
 
+    def read_short_config(self, config_json, file_path):
+        return script_config.read_short(
+            file_path,
+            config_json,
+            self._group_scripts_by_folder,
+            self._script_configs_folder)
 
     def delete_config(self, user, name):
         self._check_admin_access(user)
@@ -220,7 +232,7 @@ def list_configs(self, user, mode=None):
         def load_script(path, content) -> Optional[ShortConfig]:
             try:
                 config_object = self.load_config_file(path, content)
-                short_config = script_config.read_short(path, config_object)
+                short_config = self.read_short_config(config_object, path)
 
                 if short_config is None:
                     return None
@@ -257,7 +269,9 @@ def load_config_model(self, name, user, parameter_values=None, skip_invalid_para
             user,
             parameter_values,
             skip_invalid_parameters,
-            self._process_invoker)
+            self._process_invoker,
+            self._group_scripts_by_folder,
+            self._script_configs_folder)
 
     def _visit_script_configs(self, visitor):
         configs_dir = self._script_configs_folder
@@ -296,7 +310,7 @@ def _find_config(self, name, user) -> Optional[ConfigSearchResult]:
         def find_and_load(path: str, content):
             try:
                 config_object = self.load_config_file(path, content)
-                short_config = script_config.read_short(path, config_object)
+                short_config = self.read_short_config(config_object, path)
 
                 if short_config is None:
                     return None
@@ -331,7 +345,9 @@ def _load_script_config(
             user,
             parameter_values,
             skip_invalid_parameters,
-            process_invoker):
+            process_invoker,
+            group_scripts_by_folder,
+            script_configs_folder):
 
         if isinstance(content_or_json_dict, str):
             json_object = custom_json.loads(content_or_json_dict)
@@ -342,6 +358,8 @@ def _load_script_config(
             path,
             user.get_username(),
             user.get_audit_name(),
+            group_scripts_by_folder,
+            script_configs_folder,
             process_invoker,
             pty_enabled_default=os_utils.is_pty_supported())
 

src/main.py

@@ -103,7 +103,8 @@ def main():
 
     process_invoker = ProcessInvoker(server_config.env_vars)
 
-    config_service = ConfigService(authorizer, CONFIG_FOLDER, process_invoker)
+    config_service = ConfigService(
+        authorizer, CONFIG_FOLDER, server_config.groups_config.group_by_folders, process_invoker)
 
     alerts_service = AlertsService(server_config.alerts_config)
     alerts_service = alerts_service

src/model/script_config.py

@@ -62,14 +62,16 @@ def __init__(self,
                  path,
                  username,
                  audit_name,
+                 group_by_folders: bool,
+                 script_configs_folder: str,
                  process_invoker: ProcessInvoker,
                  pty_enabled_default=True):
         super().__init__()
 
-        short_config = read_short(path, config_object)
+        short_config = read_short(path, config_object, group_by_folders, script_configs_folder)
         self.name = short_config.name
         self._pty_enabled_default = pty_enabled_default
-        self._config_folder = os.path.dirname(path)
+        self._config_folder = script_configs_folder
         self._process_invoker = process_invoker
 
         self._username = username
@@ -363,11 +365,16 @@ def _build_name_from_path(file_path):
     return name.strip()
 
 
-def read_short(file_path, json_object):
+def read_short(file_path, json_object, group_by_folders: bool, script_configs_folder: str):
     name = _read_name(file_path, json_object)
     allowed_users = json_object.get('allowed_users')
     admin_users = json_object.get('admin_users')
     group = read_str_from_config(json_object, 'group', blank_to_none=True)
+    if ('group' not in json_object) and group_by_folders:
+        relative_path = file_utils.relative_path(file_path, script_configs_folder)
+        while os.path.dirname(relative_path):
+            relative_path = os.path.dirname(relative_path)
+            group = relative_path
 
     hidden = read_bool_from_config('hidden', json_object, default=False)
     if hidden:

src/model/server_conf.py

@@ -29,6 +29,7 @@ def __init__(self) -> None:
         self.allowed_users = None
         self.alerts_config = None
         self.logging_config = None
+        self.groups_config = ScriptGroupsConfig()  # type: ScriptGroupsConfig
         self.admin_config = None
         self.title = None
         self.enable_script_titles = None
@@ -75,6 +76,24 @@ def from_json(cls, json_config):
         return config
 
 
+class ScriptGroupsConfig:
+
+    def __init__(self) -> None:
+        self.group_by_folders = True
+
+    @classmethod
+    def from_json(cls, json_config):
+        config = ScriptGroupsConfig()
+
+        if json_config:
+            config.group_by_folders = model_helper.read_bool_from_config(
+                'group_by_folders',
+                json_config,
+                default=config.group_by_folders)
+
+        return config
+
+
 def _build_env_vars(json_object):
     sensitive_config_paths = [
         ['auth', 'secret'],
@@ -184,6 +203,7 @@ def from_json(conf_path, temp_folder):
     config.alerts_config = json_object.get('alerts')
     config.callbacks_config = json_object.get('callbacks')
     config.logging_config = LoggingConfig.from_json(json_object.get('logging'))
+    config.groups_config = ScriptGroupsConfig.from_json(json_object.get('script_groups'))
     config.user_groups = user_groups
     config.admin_users = admin_users
     config.full_history_users = full_history_users

src/tests/config_service_test.py

@@ -1,6 +1,7 @@
 import json
 import os
 import sys
+import tempfile
 import unittest
 from collections import OrderedDict
 from shutil import copyfile
@@ -30,6 +31,14 @@ def test_list_configs_when_one(self):
         self.assertEqual(1, len(configs))
         self.assertEqual('conf_x', configs[0].name)
 
+    def test_list_configs_when_one_and_symlink(self):
+        conf_path = os.path.join(test_utils.temp_folder, 'runners', 'sub', 'x.json')
+        with self._temporary_file_symlink(conf_path, {'name': 'test X'}):
+            configs = self.config_service.list_configs(self.user)
+            self.assertEqual(1, len(configs))
+            self.assertEqual('test X', configs[0].name)
+            self.assertEqual('sub', configs[0].group)
+
     def test_list_configs_when_multiple(self):
         _create_script_config_file('conf_x')
         _create_script_config_file('conf_y')
@@ -40,9 +49,9 @@ def test_list_configs_when_multiple(self):
         self.assertCountEqual(['conf_x', 'conf_y', 'A B C'], conf_names)
 
     def test_list_configs_when_multiple_and_subfolders(self):
-        _create_script_config_file('conf_x', subfolder = 's1')
-        _create_script_config_file('conf_y', subfolder = 's2')
-        _create_script_config_file('ABC', subfolder = os.path.join('s1', 'inner'))
+        _create_script_config_file('conf_x', subfolder='s1')
+        _create_script_config_file('conf_y', subfolder='s2')
+        _create_script_config_file('ABC', subfolder=os.path.join('s1', 'inner'))
 
         configs = self.config_service.list_configs(self.user)
         conf_names = [config.name for config in configs]
@@ -114,6 +123,41 @@ def test_load_config_with_slash_in_name(self):
         config = self.config_service.load_config_model('Name with slash /', self.user)
         self.assertEqual('Name with slash /', config.name)
 
+    def test_list_configs_when_multiple_subfolders_and_symlink(self):
+        def create_config_file(name, relative_path, group=None):
+            filename = os.path.basename(relative_path)
+            config = {'name': name}
+            if group is not None:
+                config['group'] = group
+            test_utils.write_script_config(
+                config,
+                filename,
+                config_folder=os.path.join(test_utils.temp_folder, 'runners', os.path.dirname(relative_path)))
+
+        subfolder = os.path.join(test_utils.temp_folder, 'runners', 'sub')
+        symlink_path = os.path.join(subfolder, 'x.json')
+        with self._temporary_file_symlink(symlink_path, {'name': 'test X'}):
+            create_config_file('conf Y', os.path.join('sub', 'y', 'conf_y.json'))
+            create_config_file('conf Z', os.path.join('sub', 'z', 'conf_z.json'))
+            create_config_file('conf A', 'conf_a.json')
+            create_config_file('conf B', os.path.join('b', 'conf_b.json'))
+            create_config_file('conf C', os.path.join('c', 'conf_c.json'), group='test group')
+            create_config_file('conf D', os.path.join('d', 'conf_d.json'), group='')
+
+            configs = self.config_service.list_configs(self.user)
+            actual_name_group_map = {c.name: c.group for c in configs}
+
+            self.assertEqual(
+                actual_name_group_map,
+                {'test X': 'sub',
+                 'conf Y': 'sub',
+                 'conf Z': 'sub',
+                 'conf A': None,
+                 'conf B': 'b',
+                 'conf C': 'test group',
+                 'conf D': None},
+            )
+
     def tearDown(self):
         super().tearDown()
         test_utils.cleanup()
@@ -125,7 +169,19 @@ def setUp(self):
         self.user = User('ConfigServiceTest', {AUTH_USERNAME: 'ConfigServiceTest'})
         self.admin_user = User('admin_user', {AUTH_USERNAME: 'The Admin'})
         authorizer = Authorizer(ANY_USER, ['admin_user'], [], [], EmptyGroupProvider())
-        self.config_service = ConfigService(authorizer, test_utils.temp_folder, test_utils.process_invoker)
+        self.config_service = ConfigService(authorizer, test_utils.temp_folder, True, test_utils.process_invoker)
+
+    @staticmethod
+    def _temporary_file_symlink(symlink_path, file_content: dict):
+        f = tempfile.NamedTemporaryFile()
+
+        f.write(json.dumps(file_content).encode('utf-8'))
+        f.flush()
+        subdir = os.path.dirname(symlink_path)
+        os.makedirs(subdir)
+        os.symlink(f.name, symlink_path)
+
+        return f
 
 
 class ConfigServiceAuthTest(unittest.TestCase):
@@ -209,7 +265,11 @@ def setUp(self):
         authorizer = Authorizer([], ['adm_user'], [], [], EmptyGroupProvider())
         self.user1 = User('user1', {})
         self.admin_user = User('adm_user', {})
-        self.config_service = ConfigService(authorizer, test_utils.temp_folder, test_utils.process_invoker)
+        self.config_service = ConfigService(
+            authorizer,
+            test_utils.temp_folder,
+            True,
+            test_utils.process_invoker)
 
 
 def script_path(path):
@@ -242,7 +302,7 @@ def setUp(self):
 
         authorizer = Authorizer([], ['admin_user', 'admin_non_editor'], [], ['admin_user'], EmptyGroupProvider())
         self.admin_user = User('admin_user', {})
-        self.config_service = ConfigService(authorizer, test_utils.temp_folder, test_utils.process_invoker)
+        self.config_service = ConfigService(authorizer, test_utils.temp_folder, True, test_utils.process_invoker)
 
     def tearDown(self):
         super().tearDown()
@@ -416,7 +476,7 @@ def setUp(self):
 
         authorizer = Authorizer([], ['admin_user', 'admin_non_editor'], [], ['admin_user'], EmptyGroupProvider())
         self.admin_user = User('admin_user', {})
-        self.config_service = ConfigService(authorizer, test_utils.temp_folder, test_utils.process_invoker)
+        self.config_service = ConfigService(authorizer, test_utils.temp_folder, True, test_utils.process_invoker)
 
         for suffix in 'XYZ':
             name = 'Conf ' + suffix
@@ -669,7 +729,7 @@ def setUp(self):
 
         authorizer = Authorizer([], ['admin_user'], [], [], EmptyGroupProvider())
         self.admin_user = User('admin_user', {})
-        self.config_service = ConfigService(authorizer, test_utils.temp_folder, test_utils.process_invoker)
+        self.config_service = ConfigService(authorizer, test_utils.temp_folder, True, test_utils.process_invoker)
 
     def tearDown(self):
         super().tearDown()
@@ -717,7 +777,7 @@ def setUp(self) -> None:
 
         authorizer = Authorizer([], ['admin_user', 'admin_non_editor'], [], ['admin_user'], EmptyGroupProvider())
         self.admin_user = User('admin_user', {})
-        self.config_service = ConfigService(authorizer, test_utils.temp_folder, test_utils.process_invoker)
+        self.config_service = ConfigService(authorizer, test_utils.temp_folder, True, test_utils.process_invoker)
 
         for pair in [('script.py', b'123'),
                      ('another.py', b'xyz'),

src/tests/execution_logging_test.py

@@ -542,7 +542,8 @@ def test_logging_values(self):
             'my_script',
             script_command='echo',
             parameters=[param1, param2, param3, param4],
-            logging_config=LoggingConfig('test-${SCRIPT}-${p1}'))
+            logging_config=LoggingConfig('test-${SCRIPT}-${p1}'),
+            path=os.path.join('conf', 'my_script.json'))
         config_model.set_all_param_values({'p1': 'abc', 'p3': True, 'p4': 987})
 
         execution_id = self.executor_service.start_script(
@@ -568,7 +569,7 @@ def test_logging_values(self):
         self.assertEqual('some text\nanother text', log)
 
         log_files = os.listdir(test_utils.temp_folder)
-        self.assertEqual(['test-my_script-abc.log'], log_files)
+        self.assertEqual(['test-my_script-abc.log', 'conf'], log_files)
 
     def test_exit_code(self):
         config_model = create_config_model(

src/tests/execution_service_test.py

@@ -10,7 +10,6 @@
 from execution.execution_service import ExecutionService
 from execution.executor import create_process_wrapper
 from model.model_helper import AccessProhibitedException
-from model.script_config import ConfigModel
 from tests import test_utils
 from tests.test_utils import mock_object, create_audit_names, _MockProcessWrapper, _IdGeneratorMock
 from utils import audit_utils
@@ -441,10 +440,12 @@ def _start_with_config(execution_service, config, parameter_values=None, user_id
 
 
 def _create_script_config(parameter_configs):
-    config = ConfigModel(
-        {'name': 'script_x',
-         'script_path': 'ls',
-         'parameters': parameter_configs},
-        'script_x.json', 'user1', 'localhost',
-        test_utils.process_invoker)
-    return config
+    return test_utils.create_config_model(
+        'script_x',
+        config={'name': 'script_x',
+                'script_path': 'ls',
+                'parameters': parameter_configs},
+        username='user1',
+        audit_name='localhost',
+
+    )