From 56a34093425fd0c9e60ba22c02a02d9e7cb679fa Mon Sep 17 00:00:00 2001 From: AleJo2995 Date: Tue, 12 Sep 2023 07:55:13 -0600 Subject: [PATCH] fix: parameter aggregation fix (#1443) * fix: parameter aggregation fix Signed-off-by: Alejandro Jose Leiva Palomo * fix: arranging tests Signed-off-by: Alejandro Jose Leiva Palomo * fix: triggering build Signed-off-by: Alejandro Jose Leiva Palomo * fix: increase time out for cache response Signed-off-by: Alejandro Jose Leiva Palomo * fix: profile-values are shown in markdown even when there are values already Signed-off-by: Alejandro Jose Leiva Palomo * fix: adding alt identifier validation Signed-off-by: Alejandro Jose Leiva Palomo * fix: correct profile values validation Signed-off-by: Alejandro Jose Leiva Palomo * fix: remove parameter aggregation from assembly and remove label being shown in assembled profile Signed-off-by: Alejandro Jose Leiva Palomo * fix: correcting test failures and various formatting issues Signed-off-by: Alejandro Jose Leiva Palomo * fix: change order for parameters in markdown Signed-off-by: Alejandro Jose Leiva Palomo * fix: not setting empty values Signed-off-by: Alejandro Jose Leiva Palomo --------- Signed-off-by: Alejandro Jose Leiva Palomo --- .../core/commands/author/profile_test.py | 23 ++++++++- .../trestle/core/commands/author/ssp_test.py | 1 - trestle/common/const.py | 4 ++ trestle/common/model_utils.py | 6 +++ trestle/core/catalog/catalog_reader.py | 9 +++- trestle/core/catalog/catalog_writer.py | 48 ++++++++++++------- trestle/core/commands/author/profile.py | 35 -------------- trestle/core/remote/cache.py | 2 +- 8 files changed, 72 insertions(+), 56 deletions(-) diff --git a/tests/trestle/core/commands/author/profile_test.py b/tests/trestle/core/commands/author/profile_test.py index da75698be..5c2b9047e 100644 --- a/tests/trestle/core/commands/author/profile_test.py +++ b/tests/trestle/core/commands/author/profile_test.py @@ -933,6 +933,7 @@ def test_profile_force_overwrite(tmp_trestle_dir: pathlib.Path, monkeypatch: Mon header, tree = md_api.processor.process_markdown(md_path) assert header + header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES] = [] old_value = header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES] header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES] = 'New value' @@ -948,6 +949,7 @@ def test_profile_force_overwrite(tmp_trestle_dir: pathlib.Path, monkeypatch: Mon test_utils.execute_command_and_assert(prof_generate, 0, monkeypatch) header, _ = md_api.processor.process_markdown(md_path) + header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES] = [] assert header[const.SET_PARAMS_TAG]['ac-5_prm_1'][const.VALUES] == old_value # test that file is unchanged @@ -1129,20 +1131,37 @@ def test_profile_generate_assemble_parameter_aggregation( nist_cat, _ = ModelUtils.load_model_for_class(tmp_trestle_dir, 'nist_cat', cat.Catalog, FileContentType.JSON) appended_prop = {'name': 'aggregates', 'value': 'at-02_odp.01'} + second_appended_prop = {'name': 'aggregates', 'value': 'at-02_odp.02'} + third_appended_prop = {'name': 'alt-identifier', 'value': 'this_is_an_identifier'} ac_1 = nist_cat.groups[0].controls[0] - ac_1.params[2].props = [] - ac_1.params[2].props.append(appended_prop) + ac_1.params[6].props = [] + ac_1.params[6].props.append(appended_prop) + ac_1.params[6].props.append(second_appended_prop) + ac_1.params[6].props.append(third_appended_prop) appended_extra_param = { 'id': 'at-02_odp.01', 'props': [{ 'name': 'label', 'value': 'AT-02_ODP[01]', 'class': 'sp800-53a' }], 'label': 'frequency', + 'values': ['value-1', 'value-2'], + 'guidelines': [{ + 'prose': 'blah' + }] + } + second_appended_extra_param = { + 'id': 'at-02_odp.02', + 'props': [{ + 'name': 'label', 'value': 'AT-02_ODP[02]', 'class': 'sp800-53a' + }], + 'label': 'frequency', + 'values': ['value-3', 'value-4'], 'guidelines': [{ 'prose': 'blah' }] } ac_1.params.append(appended_extra_param) + ac_1.params.append(second_appended_extra_param) ModelUtils.save_top_level_model(nist_cat, tmp_trestle_dir, 'nist_cat', FileContentType.JSON) diff --git a/tests/trestle/core/commands/author/ssp_test.py b/tests/trestle/core/commands/author/ssp_test.py index 2dd68da19..6f2646a14 100644 --- a/tests/trestle/core/commands/author/ssp_test.py +++ b/tests/trestle/core/commands/author/ssp_test.py @@ -215,7 +215,6 @@ def test_ssp_generate_header_edit(tmp_trestle_dir: pathlib.Path) -> None: # confirm new items were added from yaml but not when the same key was alread present (values not updated) header, tree = md_api.processor.process_markdown(ac_1) assert 'control-origination' in header - assert header['x-trestle-set-params']['ac-1_prm_5']['values'] is None assert header['x-trestle-set-params']['ac-1_prm_5']['label'] == 'meetings cancelled from cli yaml' # generate again with header and DO overwrite header values diff --git a/trestle/common/const.py b/trestle/common/const.py index 6f9a02912..57b296e58 100644 --- a/trestle/common/const.py +++ b/trestle/common/const.py @@ -583,6 +583,10 @@ CONTROL_IMPLEMENTATION = 'control-implementation' +AGGREGATES = 'aggregates' + +ALT_IDENTIFIER = 'alt-identifier' + IMPLEMENTED_REQUIREMENT = 'implemented-requirement' # Following 5 are allowed control origination values for diff --git a/trestle/common/model_utils.py b/trestle/common/model_utils.py index 09a07bc42..13e93ff5d 100644 --- a/trestle/common/model_utils.py +++ b/trestle/common/model_utils.py @@ -628,6 +628,12 @@ def dict_to_parameter(param_dict: Dict[str, Any]) -> common.Parameter: if const.DISPLAY_NAME in param_dict: display_name = param_dict.pop(const.DISPLAY_NAME) props.append(common.Property(name=const.DISPLAY_NAME, value=display_name, ns=const.TRESTLE_GENERIC_NS)) + if const.AGGREGATES in param_dict: + # removing aggregates as this is prop just informative in markdown + param_dict.pop(const.AGGREGATES) + if const.ALT_IDENTIFIER in param_dict: + # removing alt-identifier as this is prop just informative in markdown + param_dict.pop(const.ALT_IDENTIFIER) if 'ns' in param_dict: param_dict.pop('ns') diff --git a/trestle/core/catalog/catalog_reader.py b/trestle/core/catalog/catalog_reader.py index e4d612a1f..f4d648afc 100644 --- a/trestle/core/catalog/catalog_reader.py +++ b/trestle/core/catalog/catalog_reader.py @@ -71,9 +71,16 @@ def read_additional_content( ) alters_map[sort_id] = control_alters for param_id, param_dict in control_param_dict.items(): + param_dict[const.VALUES] = param_dict[const.VALUES] if const.VALUES in param_dict else [] # if profile_values are present, overwrite values with them if const.PROFILE_VALUES in param_dict: - param_dict[const.VALUES] = param_dict.pop(const.PROFILE_VALUES) + if param_dict[const.PROFILE_VALUES] != [] and param_dict[const.PROFILE_VALUES] is not None: + if not write_mode and '' in param_dict[const.PROFILE_VALUES]: + param_dict[const.PROFILE_VALUES].remove('') + if param_dict[const.PROFILE_VALUES] != [] and param_dict[const.PROFILE_VALUES] is not None: + param_dict[const.VALUES] = param_dict[const.PROFILE_VALUES] + if not write_mode: + param_dict.pop(const.PROFILE_VALUES) final_param_dict[param_id] = param_dict param_sort_map[param_id] = sort_id new_alters: List[prof.Alter] = [] diff --git a/trestle/core/catalog/catalog_writer.py b/trestle/core/catalog/catalog_writer.py index 07c274041..0a632d2ce 100644 --- a/trestle/core/catalog/catalog_writer.py +++ b/trestle/core/catalog/catalog_writer.py @@ -19,7 +19,7 @@ import trestle.common.const as const import trestle.oscal.catalog as cat -from trestle.common.list_utils import as_list, deep_get, delete_list_from_list, none_if_empty +from trestle.common.list_utils import as_list, deep_get, none_if_empty from trestle.common.model_utils import ModelUtils from trestle.core.catalog.catalog_interface import CatalogInterface from trestle.core.catalog.catalog_merger import CatalogMerger @@ -63,18 +63,6 @@ def write_catalog_as_profile_markdown( # get all params and vals for this control from the resolved profile catalog with block adds in effect control_param_dict = ControlInterface.get_control_param_dict(control, False) - to_delete = [] - # removes aggregate parameters to be non-editable in markdowns - props_by_param_ids = {} - for param_id, values_dict in control_param_dict.items(): - props_by_param_ids[param_id] = [ - prop for prop in as_list(values_dict.props) if prop.name == 'aggregates' - ] - to_delete = list({k: v for k, v in props_by_param_ids.items() if v != []}.keys()) - unique_params_to_del = list(set(to_delete)) - if unique_params_to_del: - delete_list_from_list(control_param_dict, unique_params_to_del) - set_param_dict = self._construct_set_parameters_dict(profile_set_param_dict, control_param_dict, context) if set_param_dict: @@ -174,15 +162,43 @@ def _construct_set_parameters_dict( # all the other elements are from the profile set_param new_dict[const.VALUES] = orig_dict.get(const.VALUES, None) new_dict[const.GUIDELINES] = orig_dict.get(const.GUIDELINES, None) + if new_dict[const.VALUES] is None: + new_dict.pop(const.VALUES) + if new_dict[const.GUIDELINES] is None: + new_dict.pop(const.GUIDELINES) else: # if the profile doesnt change this param at all, show it in the header with values tmp_dict = ModelUtils.parameter_to_dict(param_dict, True) values = tmp_dict.get('values', None) - new_dict = {'id': param_id, 'values': values} + # if values are None then don´t display them in the markdown + if values is not None: + new_dict = {'id': param_id, 'values': values, const.PROFILE_VALUES: ['']} + else: + new_dict = {'id': param_id, const.PROFILE_VALUES: ['']} new_dict.pop('id', None) - if display_name: + # validates if there are aggregated parameter values to the current parameter + aggregated_props = [prop for prop in as_list(param_dict.props) if prop.name == const.AGGREGATES] + if aggregated_props != []: + props_to_add = [] + for prop in aggregated_props: + props_to_add.append(prop.value) + new_dict[const.AGGREGATES] = props_to_add + new_dict.pop(const.PROFILE_VALUES, None) + alt_identifier = [prop for prop in as_list(param_dict.props) if prop.name == const.ALT_IDENTIFIER] + if alt_identifier != []: + new_dict[const.ALT_IDENTIFIER] = alt_identifier[0].value + # adds display name, if no display name then do not add to dict + if display_name != '' and display_name is not None: new_dict[const.DISPLAY_NAME] = display_name - key_order = (const.LABEL, const.GUIDELINES, const.PROFILE_VALUES, const.VALUES, const.DISPLAY_NAME) + key_order = ( + const.LABEL, + const.GUIDELINES, + const.VALUES, + const.AGGREGATES, + const.ALT_IDENTIFIER, + const.DISPLAY_NAME, + const.PROFILE_VALUES + ) ordered_dict = {k: new_dict[k] for k in key_order if k in new_dict.keys()} set_param_dict[param_id] = ordered_dict diff --git a/trestle/core/commands/author/profile.py b/trestle/core/commands/author/profile.py index 2f4725d4f..c2469cfe6 100644 --- a/trestle/core/commands/author/profile.py +++ b/trestle/core/commands/author/profile.py @@ -285,33 +285,6 @@ def _replace_modify_set_params( profile.modify.set_parameters = none_if_empty(profile.modify.set_parameters) return changed - @staticmethod - def _add_aggregated_parameter( - param: Any, param_dict: Dict[str, Any], control_id: str, controls: Any, param_map: Dict[str, str] - ) -> None: - """ - Add aggregated parameter value to original parameter. - - Notes: - None - """ - # verifies aggregated param is not on grabbed param dict - if param.id not in list(param_dict.keys()): - param.values = [] - agg_props = [prop for prop in param.props if prop.name == 'aggregates'] - for prop in as_list(agg_props): - if param_dict[prop.value].get('values'): - agg_param_values = param_dict[prop.value].get('values') - for value in as_list(agg_param_values): - param.values.append(value) - else: - agg_param_values = [p for p in controls[control_id] if p.id == prop.value][0] - param.values.append(agg_param_values.props[0].value) - dict_param = ModelUtils.parameter_to_dict(param, False) - param_dict[param.id] = dict_param - param_map[param.id] = control_id - return None - @staticmethod def assemble_profile( trestle_root: pathlib.Path, @@ -381,14 +354,6 @@ def assemble_profile( catalog_api = CatalogAPI(catalog=catalog, context=context) found_alters, param_dict, param_map = catalog_api.read_additional_content_from_md(label_as_key=True) - controls = {} - for group in as_list(catalog.groups): - for control in as_list(group.controls): - controls[control.id] = control.params - for control_id, params in controls.items(): - for param in as_list(params): - ProfileAssemble._add_aggregated_parameter(param, param_dict, control_id, controls, param_map) - # technically if allowed sections is [] it means no sections are allowed if allowed_sections is not None: for bad_part in [ part for alter in found_alters for add in as_list(alter.adds) diff --git a/trestle/core/remote/cache.py b/trestle/core/remote/cache.py index e6b747351..635000451 100644 --- a/trestle/core/remote/cache.py +++ b/trestle/core/remote/cache.py @@ -286,7 +286,7 @@ def _do_fetch(self) -> None: auth = HTTPBasicAuth(self._username, self._password) try: - response = requests.get(self._url, auth=auth, verify=verify, timeout=10) + response = requests.get(self._url, auth=auth, verify=verify, timeout=30) except Exception as e: raise TrestleError(f'Cache update failure to connect via HTTPS: {self._url} ({e})')