feat: Roleid validation via ncname and parametrized tests (#499)

Signed-off-by: FrankSuits <frankst@au1.ibm.com>

Co-authored-by: Chris Butler <chris@thebutlers.me>

tests/trestle/core/commands/validate_test.py

@@ -23,108 +23,125 @@
 
 from tests import test_utils
 
+import trestle.oscal.assessment_plan as ap
 from trestle import cli
-from trestle.core import utils
-from trestle.core.models.file_content_type import FileContentType
-from trestle.oscal import target as ostarget
+from trestle.core.generators import generate_sample_model
 
 test_data_dir = pathlib.Path('tests/data').resolve()
 
 
-def test_target_dups(tmp_trestle_dir: pathlib.Path) -> None:
-    """Test model validation."""
-    content_type = FileContentType.YAML
-    models_dir_name = test_utils.TARGET_DEFS_DIR
-    model_ref = ostarget.TargetDefinition
+@pytest.mark.parametrize(
+    'name, mode, parent',
+    [
+        ('my_test_model', '-f', False), ('my_test_model', '-n', False), ('my_test_model', '-f', True),
+        ('my_test_model', '-t', False), ('my_test_model', '-a', False)
+    ]
+)
+def test_validation_happy(name, mode, parent, tmp_trestle_dir: pathlib.Path) -> None:
+    """Test successful validation runs."""
+    (tmp_trestle_dir / test_utils.TARGET_DEFS_DIR / 'my_test_model').mkdir(exist_ok=True, parents=True)
+    (tmp_trestle_dir / test_utils.TARGET_DEFS_DIR / 'my_test_model2').mkdir(exist_ok=True, parents=True)
+    shutil.copyfile(
+        test_data_dir / 'yaml/good_target.yaml',
+        tmp_trestle_dir / test_utils.TARGET_DEFS_DIR / 'my_test_model/target-definition.yaml'
+    )
+    shutil.copyfile(
+        test_data_dir / 'yaml/good_target.yaml',
+        tmp_trestle_dir / test_utils.TARGET_DEFS_DIR / 'my_test_model2/target-definition.yaml'
+    )
+
+    model_def_file = tmp_trestle_dir / test_utils.TARGET_DEFS_DIR / name / ('target-definition.yaml')
+
+    if mode == '-f':
+        if not parent:
+            testcmd = f'trestle validate {mode} {model_def_file} -m duplicates -i uuid'
+        else:
+            testcmd = f'trestle validate {mode} {model_def_file.parent} -m duplicates -i uuid'
+    elif mode == '-n':
+        testcmd = f'trestle validate -t target-definition -n {name} -m duplicates -i uuid'
+    else:
+        testcmd = 'trestle validate -a -m duplicates -i uuid'
 
-    test_utils.ensure_trestle_config_dir(tmp_trestle_dir)
-
-    file_ext = FileContentType.to_file_extension(content_type)
-    models_full_path = tmp_trestle_dir / models_dir_name / 'my_test_model'
-    models_full_path2 = tmp_trestle_dir / models_dir_name / 'my_test_model2'
-    models_full_path.mkdir(exist_ok=True, parents=True)
-    models_full_path2.mkdir(exist_ok=True, parents=True)
-
-    model_alias = utils.classname_to_alias(model_ref.__name__, 'json')
-
-    model_def_file = models_full_path / f'{model_alias}{file_ext}'
-    model_def_file2 = models_full_path2 / f'{model_alias}{file_ext}'
-
-    shutil.copyfile(test_data_dir / 'yaml/good_target.yaml', model_def_file)
-    shutil.copyfile(test_data_dir / 'yaml/good_target.yaml', model_def_file2)
-
-    # first validate the single file
-    testcmd = f'trestle validate -f {model_def_file} -m duplicates -i uuid'
     with patch.object(sys, 'argv', testcmd.split()):
         with pytest.raises(SystemExit) as pytest_wrapped_e:
             cli.run()
         assert pytest_wrapped_e.type == SystemExit
         assert pytest_wrapped_e.value.code == 0
 
-    # validate the single file by type and name
-    testcmd = f'trestle validate -t {model_alias} -n my_test_model -m duplicates -i uuid'
-    with patch.object(sys, 'argv', testcmd.split()):
-        with pytest.raises(SystemExit) as pytest_wrapped_e:
-            cli.run()
-        assert pytest_wrapped_e.type == SystemExit
-        assert pytest_wrapped_e.value.code == 0
 
-    testcmd = f'trestle validate -f {model_def_file.parent} -m duplicates -i uuid'
-    with patch.object(sys, 'argv', testcmd.split()):
-        with pytest.raises(SystemExit) as pytest_wrapped_e:
-            cli.run()
-        assert pytest_wrapped_e.type == SystemExit
-        assert pytest_wrapped_e.value.code == 0
+@pytest.mark.parametrize(
+    'name, mode, parent',
+    [
+        ('my_test_model', '-f', False), ('my_test_model', '-n', False), ('my_test_model', '-f', True),
+        ('my_test_model', '-t', False), ('my_test_model', '-a', False), ('foo', '-n', False)
+    ]
+)
+def test_validation_unhappy(name, mode, parent, tmp_trestle_dir: pathlib.Path) -> None:
+    """Test failure modes of validation."""
+    (tmp_trestle_dir / test_utils.TARGET_DEFS_DIR / 'my_test_model').mkdir(exist_ok=True, parents=True)
+    (tmp_trestle_dir / test_utils.TARGET_DEFS_DIR / 'my_test_model2').mkdir(exist_ok=True, parents=True)
+    shutil.copyfile(
+        test_data_dir / 'yaml/bad_target_dup_uuid.yaml',
+        tmp_trestle_dir / test_utils.TARGET_DEFS_DIR / 'my_test_model/target-definition.yaml'
+    )
+    shutil.copyfile(
+        test_data_dir / 'yaml/good_target.yaml',
+        tmp_trestle_dir / test_utils.TARGET_DEFS_DIR / 'my_test_model2/target-definition.yaml'
+    )
+
+    model_def_file = tmp_trestle_dir / test_utils.TARGET_DEFS_DIR / ('my_test_model/target-definition.yaml')
+
+    if mode == '-f':
+        if not parent:
+            testcmd = f'trestle validate {mode} {model_def_file} -m duplicates -i uuid'
+        else:
+            testcmd = f'trestle validate {mode} {model_def_file.parent} -m duplicates -i uuid'
+    elif mode == '-n':
+        testcmd = f'trestle validate -t target-definition -n {name} -m duplicates -i uuid'
+    else:
+        testcmd = 'trestle validate -a -m duplicates -i uuid'
 
-    # now validate both models by type
-    testcmd = 'trestle validate -t target-definition -m duplicates -i uuid'
-    with patch.object(sys, 'argv', testcmd.split()):
-        with pytest.raises(SystemExit) as pytest_wrapped_e:
-            cli.run()
-        assert pytest_wrapped_e.type == SystemExit
-        assert pytest_wrapped_e.value.code == 0
-
-    # now validate all models
-    testcmd = 'trestle validate -a -m duplicates -i uuid'
-    with patch.object(sys, 'argv', testcmd.split()):
-        with pytest.raises(SystemExit) as pytest_wrapped_e:
-            cli.run()
-        assert pytest_wrapped_e.type == SystemExit
-        assert pytest_wrapped_e.value.code == 0
-
-    shutil.copyfile(test_data_dir / 'yaml/bad_target_dup_uuid.yaml', model_def_file)
-
-    testcmd = f'trestle validate -f {model_def_file} -m duplicates -i uuid'
-    with patch.object(sys, 'argv', testcmd.split()):
-        with pytest.raises(SystemExit) as pytest_wrapped_e:
-            cli.run()
-        assert pytest_wrapped_e.type == SystemExit
-        assert pytest_wrapped_e.value.code == 1
-
-    testcmd = f'trestle validate -t {model_alias} -n my_test_model -m duplicates -i uuid'
     with patch.object(sys, 'argv', testcmd.split()):
         with pytest.raises(SystemExit) as pytest_wrapped_e:
             cli.run()
         assert pytest_wrapped_e.type == SystemExit
         assert pytest_wrapped_e.value.code == 1
 
-    testcmd = f'trestle validate -t {model_alias} -m duplicates -i uuid'
-    with patch.object(sys, 'argv', testcmd.split()):
-        with pytest.raises(SystemExit) as pytest_wrapped_e:
-            cli.run()
-        assert pytest_wrapped_e.type == SystemExit
-        assert pytest_wrapped_e.value.code == 1
 
-    testcmd = 'trestle validate -a -m duplicates -i uuid'
-    with patch.object(sys, 'argv', testcmd.split()):
-        with pytest.raises(SystemExit) as pytest_wrapped_e:
-            cli.run()
-        assert pytest_wrapped_e.type == SystemExit
-        assert pytest_wrapped_e.value.code == 1
+@pytest.mark.parametrize(
+    'name, mode, parent, new_role, code',
+    [
+        ('my_ap', '-f', False, 'role', 0), ('my_ap', '-n', False, 'role', 0), ('my_ap', '-f', True, 'role', 0),
+        ('my_ap', '-t', False, 'role', 0), ('my_ap', '-a', False, 'role', 0), ('my_ap', '-f', False, 'r:ole', 1),
+        ('my_ap', '-n', False, 'r:ole', 1), ('my_ap', '-f', True, 'r:ole', 1), ('my_ap', '-t', False, 'r:ole', 1),
+        ('my_ap', '-a', False, 'r:ole', 1), ('foo', '-n', False, 'role', 1)
+    ]
+)
+def test_roleid_cases(name, mode, parent, new_role, code, tmp_trestle_dir: pathlib.Path) -> None:
+    """Test good and bad roleid cases."""
+    (tmp_trestle_dir / 'assessment-plans/my_ap').mkdir(exist_ok=True, parents=True)
+    role_ids = [ap.RoleId(__root__='role1'), ap.RoleId(__root__=new_role), ap.RoleId(__root__='REPLACE_ME')]
+    system_user = ap.SystemUser(role_ids=role_ids)
+    local_definitions = ap.LocalDefinitions(users={'my_users': system_user})
+    ap_obj = generate_sample_model(ap.AssessmentPlan)
+    ap_obj.local_definitions = local_definitions
+    ap_path = tmp_trestle_dir / 'assessment-plans/my_ap/assessment-plan.json'
+    ap_obj.oscal_write(ap_path)
+
+    if mode == '-f':
+        if not parent:
+            testcmd = f'trestle validate {mode} {ap_path} -m ncname -i roleid'
+        else:
+            testcmd = f'trestle validate {mode} {ap_path.parent} -m ncname -i roleid'
+    elif mode == '-n':
+        testcmd = f'trestle validate -t assessment-plan -n {name} -m ncname -i roleid'
+    elif mode == '-t':
+        testcmd = 'trestle validate -t assessment-plan -m ncname -i roleid'
+    else:
+        testcmd = 'trestle validate -a -m ncname -i roleid'
 
-    testcmd = 'trestle validate -f foo -m duplicates -i uuid'
     with patch.object(sys, 'argv', testcmd.split()):
         with pytest.raises(SystemExit) as pytest_wrapped_e:
             cli.run()
         assert pytest_wrapped_e.type == SystemExit
-        assert pytest_wrapped_e.value.code == 1
+        assert pytest_wrapped_e.value.code == code

tests/trestle/core/validator_helper_test.py

@@ -16,6 +16,7 @@
 """Tests for models util module."""
 
 import pathlib
+from uuid import uuid4
 
 import trestle.core.validator_helper as validator_helper
 import trestle.oscal.catalog as catalog
@@ -90,3 +91,20 @@ def test_find_all_attribs_by_regex() -> None:
     cat = catalog.Catalog.oscal_read(catalog_path)
     attrs = validator_helper.find_all_attribs_by_regex(cat, r'party.uuid')
     assert len(attrs) == 2
+
+
+def test_validations_on_dict() -> None:
+    """Test regen of uuid in dict."""
+    my_uuid = str(uuid4())
+    my_dict = {'uuid': my_uuid, 'ref': my_uuid}
+    new_dict, lut = validator_helper.regenerate_uuids_in_place(my_dict, {})
+    assert my_dict['uuid'] != new_dict['uuid']
+    assert len(lut) == 1
+
+    fixed_dict, count = validator_helper.update_new_uuid_refs(new_dict, lut)
+    assert fixed_dict['uuid'] == fixed_dict['ref']
+    assert count == 1
+
+    attrs = validator_helper.find_all_attribs_by_regex(fixed_dict, 'uuid')
+    assert len(attrs) == 1
+    assert attrs[0] == ('uuid', fixed_dict['uuid'])

trestle/core/commands/validate.py

@@ -42,5 +42,5 @@ def _run(self, args: argparse.Namespace) -> int:
         try:
             return validator.validate(self, args)
         except Exception as e:
-            logger.error(f'Error in trestle validate: {e}')
+            logger.warning(f'Error in trestle validate: {e}')
         return 1

trestle/core/const.py

@@ -38,8 +38,8 @@
     'target-definition': 'target-definitions',
     'component-definition': 'component-definitions',
     'system-security-plan': 'system-security-plans',
-    'assessment-plan': 'assessment_plans',
-    'assessment-results': 'assessment_results',
+    'assessment-plan': 'assessment-plans',
+    'assessment-results': 'assessment-results',
     'plan-of-action-and-milestones': 'plan-of-action-and-milestones'
 }
 """Element path separator"""
@@ -87,6 +87,7 @@
 ARG_DESC_ITEM = 'Item used'
 
 VAL_MODE_DUPLICATES = 'duplicates'
+VAL_MODE_NCNAME = 'ncname'
 
 FILE_ENCODING = 'utf8'
 
@@ -99,3 +100,5 @@
 WEBSITE_ROOT = 'https://ibm.github.io/compliance-trestle'
 
 BUG_REPORT = 'https://github.com/IBM/compliance-trestle/issues/new/choose'
+
+NCNAME_REGEX = r'^[a-zA-Z_][\w.-]*$'

trestle/core/duplicates_validator.py

@@ -16,20 +16,14 @@
 """Validate by confirming no duplicate items."""
 
 import argparse
+import logging
 import pathlib
-from abc import ABC, abstractmethod
 
-from trestle.core.validator_helper import has_no_duplicate_values_by_name
+from trestle.core.validator_helper import Validator, has_no_duplicate_values_by_name
 from trestle.utils import fs
 from trestle.utils.load_distributed import load_distributed
 
-
-class Validator(ABC):
-    """Abstract Validator interface."""
-
-    @abstractmethod
-    def validate(self, args: argparse.Namespace) -> int:
-        """Validate the model."""
+logger = logging.getLogger(__name__)
 
 
 class DuplicatesValidator(Validator):
@@ -49,6 +43,9 @@ def validate(self, args: argparse.Namespace) -> int:
             models_path = trestle_root / fs.model_type_to_model_dir(args.type)
             for m in models:
                 model_path = models_path / m
+                if not model_path.exists():
+                    logger.warning(f'No model found to validate at {model_path}')
+                    return 1
                 _, _, model = load_distributed(model_path)
                 if not has_no_duplicate_values_by_name(model, args.item):
                     return 1

trestle/core/object_factory.py

@@ -32,4 +32,4 @@ def register_object(self, mode: str, obj: Any) -> None:
 
     def get(self, args: argparse.Namespace) -> Any:
         """Create the object from the args."""
-        return self._objects.get(args.mode)
+        return self._objects.get(args.mode + '_' + args.item)

trestle/core/roleid_validator.py

@@ -0,0 +1,87 @@
+# -*- mode:python; coding:utf-8 -*-
+
+# Copyright (c) 2020 IBM Corp. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Validate by confirming no duplicate items."""
+
+import argparse
+import logging
+import pathlib
+import re
+
+from trestle.core.base_model import OscalBaseModel
+from trestle.core.const import NCNAME_REGEX
+from trestle.core.err import TrestleError
+from trestle.core.validator_helper import Validator, find_values_by_name
+from trestle.utils import fs
+from trestle.utils.load_distributed import load_distributed
+
+logger = logging.getLogger(__name__)
+
+
+def _roleids_are_valid(model: OscalBaseModel) -> bool:
+    role_ids_list = find_values_by_name(model, 'role_ids')
+    p = re.compile(NCNAME_REGEX)
+    for role_id_list in role_ids_list:
+        for role_id in role_id_list:
+            s = str(role_id.__root__)
+            matched = p.match(s)
+            if matched is None:
+                return False
+    return True
+
+
+class RoleIdValidator(Validator):
+    """Check that all RoleId values conform to NCName regex."""
+
+    def validate(self, args: argparse.Namespace) -> int:
+        """Perform the validation."""
+        trestle_root = fs.get_trestle_project_root(pathlib.Path.cwd())
+
+        # validate by type - all of type or just specified by name
+        if 'type' in args and args.type is not None:
+            models = []
+            if 'name' in args and args.name is not None:
+                models = [args.name]
+            else:
+                models = fs.get_models_of_type(args.type)
+            models_path = trestle_root / fs.model_type_to_model_dir(args.type)
+            for m in models:
+                model_path = models_path / m
+                try:
+                    _, _, model = load_distributed(model_path)
+                except TrestleError as e:
+                    logger.warning(f'File load error {e}')
+                    return 1
+                if not _roleids_are_valid(model):
+                    return 1
+            return 0
+
+        # validate all
+        if 'all' in args and args.all:
+            model_tups = fs.get_all_models()
+            for mt in model_tups:
+                model_path = trestle_root / fs.model_type_to_model_dir(mt[0]) / mt[1]
+                _, _, model = load_distributed(model_path)
+                if not _roleids_are_valid(model):
+                    return 1
+            return 0
+
+        # validate file
+        if 'file' in args and args.file:
+            file_path = trestle_root / args.file
+            _, _, model = load_distributed(file_path)
+            if not _roleids_are_valid(model):
+                return 1
+        return 0