From 25dbc7a4ae823c8645e8861f3763883e855b44af Mon Sep 17 00:00:00 2001 From: Jennifer Power Date: Mon, 16 Sep 2024 15:04:57 -0400 Subject: [PATCH] fix(markdown): writes component data for markdown without rules (#1695) * test: adds failing test to confirm component definition bug Signed-off-by: Jennifer Power * feat: adds implemented requirement and statement description information The comp_dict is populated with the information from the OSCAL JSON and logic on when to write parts left to the ControlWriter. Signed-off-by: Jennifer Power * fix: assemble component responses with and without rules Signed-off-by: Jennifer Power * fix: updates control_rules logic to fix test failure Signed-off-by: Jennifer Power * feat: centralizes logic for component inclusion in control writer To ensure parts are written out for component definitions without rules in a way that is not too verbose, parts will only be included if they have rules attached or non-empty prose. Signed-off-by: Jennifer Power * fix: updates formatting to make tests pass Signed-off-by: Jennifer Power * docs: updates docs to reflect component authoring behavior Signed-off-by: Jennifer Power --------- Signed-off-by: Jennifer Power --- .../ssp_profile_catalog_authoring.md | 3 +- tests/data/json/comp_def_c.json | 121 ++++++++++++++++++ .../core/commands/author/component_test.py | 61 +++++++++ trestle/core/catalog/catalog_writer.py | 43 +++---- trestle/core/control_reader.py | 3 - trestle/core/control_writer.py | 26 +++- 6 files changed, 220 insertions(+), 37 deletions(-) create mode 100644 tests/data/json/comp_def_c.json diff --git a/docs/tutorials/ssp_profile_catalog_authoring/ssp_profile_catalog_authoring.md b/docs/tutorials/ssp_profile_catalog_authoring/ssp_profile_catalog_authoring.md index 9a1d2acbc..0286b5d58 100644 --- a/docs/tutorials/ssp_profile_catalog_authoring/ssp_profile_catalog_authoring.md +++ b/docs/tutorials/ssp_profile_catalog_authoring/ssp_profile_catalog_authoring.md @@ -793,8 +793,7 @@ The markdown header lists all the rules that apply to this control, along with t The values for rule parameters are specified using the normal `SetParameter` mechanism in the ControlImplementation, but it's important to note that there are two different types of `SetParameter`: Those that apply to the normal parameters of the control, and those that apply strictly to the rules. -Note that markdown for a control is only created if there are rules associated with the control, and within the markdown the only parts written out that -prompt for responses are parts that have rules assigned. Thus the output markdown directory may be highly pruned of both controls and groups of controls if only some controls have rules associated. +Note that markdown is created for all of the controls in the `Component` control implementation. However, when processing control parts, the part is only written out if there are associated rules or pre-filled implementation descriptions. In addition, the rules should be regarded as read-only from the editing perspective, and you cannot change the rules associated with a control or its parts. But you may edit the rule parameter values as described in the comments of the markdown file under `x-trestle-comp-def-rules-param-vals`. You may also edit the prose and implementation status associated with a statement part at the bottom of the markdown file. diff --git a/tests/data/json/comp_def_c.json b/tests/data/json/comp_def_c.json new file mode 100644 index 000000000..f56d434d1 --- /dev/null +++ b/tests/data/json/comp_def_c.json @@ -0,0 +1,121 @@ +{ + "component-definition": { + "uuid": "2652b814-2a6b-4b6d-a0ae-8bc7a007209f", + "metadata": { + "title": "comp def c", + "last-modified": "2021-07-19T14:03:03+00:00", + "version": "0.21.0", + "oscal-version": "1.0.2", + "roles": [ + { + "id": "prepared-by", + "title": "Indicates the organization that created this content." + }, + { + "id": "prepared-for", + "title": "Indicates the organization for which this content was created.." + }, + { + "id": "content-approver", + "title": "Indicates the organization responsible for all content represented in the \"document\"." + } + ], + "parties": [ + { + "uuid": "ce1f379a-fcdd-485a-a7b7-6f02c0763dd2", + "type": "organization", + "name": "ACME", + "remarks": "ACME company" + }, + { + "uuid": "481856b6-16e4-4993-a3ed-2fb242ce235b", + "type": "organization", + "name": "Customer", + "remarks": "Customer for the Component Definition" + }, + { + "uuid": "2dc8b17f-daca-44a1-8a1d-c290120ea5e2", + "type": "organization", + "name": "ISV", + "remarks": "ISV for the Component Definition" + } + ], + "responsible-parties": [ + { + "role-id": "prepared-by", + "party-uuids": [ + "ce1f379a-fcdd-485a-a7b7-6f02c0763dd2" + ] + }, + { + "role-id": "prepared-for", + "party-uuids": [ + "481856b6-16e4-4993-a3ed-2fb242ce235b", + "2dc8b17f-daca-44a1-8a1d-c290120ea5e2" + ] + }, + { + "role-id": "content-approver", + "party-uuids": [ + "ce1f379a-fcdd-485a-a7b7-6f02c0763dd2" + ] + } + ] + }, + "components": [ + { + "uuid": "8220b305-0271-45f9-8a21-40ab6f197f70", + "type": "Service", + "title": "comp_cc", + "description": "comp cc", + "control-implementations": [ + { + "uuid": "76e89b67-3d6b-463d-90df-ec56a46c6069", + "source": "trestle://profiles/comp_prof_aa/profile.json", + "description": "trestle comp prof cc", + "implemented-requirements": [ + { + "uuid": "ca5ea4c5-ba51-4b1d-932a-5606891b7500", + "control-id": "ac-1", + "description": "imp req prose for ac-1 from comp cc", + "responsible-roles": [ + { + "role-id": "prepared-by", + "party-uuids": [ + "ce1f379a-fcdd-485a-a7b7-6f02c0763dd2" + ] + }, + { + "role-id": "prepared-for", + "party-uuids": [ + "481856b6-16e4-4993-a3ed-2fb242ce235b", + "2dc8b17f-daca-44a1-8a1d-c290120ea5e2" + ] + }, + { + "role-id": "content-approver", + "party-uuids": [ + "ce1f379a-fcdd-485a-a7b7-6f02c0763dd2" + ] + } + ], + "statements": [ + { + "statement-id": "ac-1_smt.a", + "uuid": "2652b814-2a6b-4b6d-a0ae-8bc7a0072200", + "description": "statement prose for part a. from comp cc" + } + ] + }, + { + "uuid": "ca5ea4c5-ba51-4b1d-932a-5606891b7599", + "control-id": "ac-3", + "description": "imp req prose for ac-3 from comp cc" + } + ] + } + ] + } + ] + } +} \ No newline at end of file diff --git a/tests/trestle/core/commands/author/component_test.py b/tests/trestle/core/commands/author/component_test.py index 508b14f71..0160371c4 100644 --- a/tests/trestle/core/commands/author/component_test.py +++ b/tests/trestle/core/commands/author/component_test.py @@ -202,3 +202,64 @@ def test_component_generate_more_than_one_param(tmp_trestle_dir: pathlib.Path, m # now assemble component generated assemble_cmd = f'trestle author component-assemble -m {md_path} -n {comp_name} -o {assem_name}' test_utils.execute_command_and_assert(assemble_cmd, CmdReturnCodes.SUCCESS.value, monkeypatch) + + +def test_component_workflow_no_rules(tmp_trestle_dir: pathlib.Path, monkeypatch: MonkeyPatch) -> None: + """Test component generate and assemble with no rules set.""" + comp_name = test_utils.setup_component_generate(tmp_trestle_dir, 'comp_def_c') + ac1_path = tmp_trestle_dir / 'md_comp/comp_cc/comp_prof_aa/ac/ac-1.md' + + orig_component, _ = model_utils.ModelUtils.load_model_for_class( + tmp_trestle_dir, comp_name, comp.ComponentDefinition + ) + + generate_cmd = f'trestle author component-generate -n {comp_name} -o {md_path}' + test_utils.execute_command_and_assert(generate_cmd, CmdReturnCodes.SUCCESS.value, monkeypatch) + + # Check that the example md file looks correct + _, tree = MarkdownProcessor().process_markdown(ac1_path) + + imp_req_md = """## What is the solution and how is it implemented? + + + + + +imp req prose for ac-1 from comp cc + +### Implementation Status: planned + +______________________________________________________________________ +""" # noqa E501 + + node = tree.get_node_for_key('## What is the solution and how is it implemented?') + assert node.content.raw_text == imp_req_md + + part_a_md = """## Implementation for part a. + +statement prose for part a. from comp cc + +### Implementation Status: planned + +______________________________________________________________________""" + + node = tree.get_node_for_key('## Implementation for part a.') + assert node.content.raw_text == part_a_md + + test_utils.substitute_text_in_file( + ac1_path, '### Implementation Status: planned', f'### Implementation Status: {const.STATUS_IMPLEMENTED}' + ) + # Check that the changes make it into the JSON + assem_name = 'assem_comp' + assemble_cmd = f'trestle author component-assemble -m {md_path} -n {comp_name} -o {assem_name}' + test_utils.execute_command_and_assert(assemble_cmd, CmdReturnCodes.SUCCESS.value, monkeypatch) + assem_component, _ = model_utils.ModelUtils.load_model_for_class( + tmp_trestle_dir, assem_name, comp.ComponentDefinition + ) + + # Check the ac-1 implementation status and that the model changed + assert not model_utils.ModelUtils.models_are_equivalent(orig_component, assem_component) # type: ignore + imp_reqs = assem_component.components[0].control_implementations[0].implemented_requirements # type: ignore + imp_req = next((i_req for i_req in imp_reqs if i_req.control_id == 'ac-1'), None) + assert imp_req.description == 'imp req prose for ac-1 from comp cc' + assert ControlInterface.get_status_from_props(imp_req).state == const.STATUS_IMPLEMENTED # type: ignore diff --git a/trestle/core/catalog/catalog_writer.py b/trestle/core/catalog/catalog_writer.py index a255dcd12..13cb1e584 100644 --- a/trestle/core/catalog/catalog_writer.py +++ b/trestle/core/catalog/catalog_writer.py @@ -355,31 +355,24 @@ def _update_values(set_param: comp.SetParameter, control_param_dict) -> None: ci_set_params = ControlInterface.get_set_params_from_item(control_imp) for imp_req in as_list(control_imp.implemented_requirements): control_part_id_map = part_id_map.get(imp_req.control_id, {}) - control_rules, statement_rules, _ = ControlInterface.get_rule_list_for_imp_req(imp_req) - if control_rules or statement_rules: - if control_rules: - status = ControlInterface.get_status_from_props(imp_req) - comp_info = ComponentImpInfo(imp_req.description, control_rules, [], status) - self._catalog_interface.add_comp_info(imp_req.control_id, context.comp_name, '', comp_info) - set_params = copy.deepcopy(ci_set_params) - set_params.update(ControlInterface.get_set_params_from_item(imp_req)) - for set_param in set_params.values(): - self._catalog_interface.add_comp_set_param(imp_req.control_id, context.comp_name, set_param) - for statement in as_list(imp_req.statements): - rule_list, _ = ControlInterface.get_rule_list_for_item(statement) - if rule_list: - status = ControlInterface.get_status_from_props(statement) - if statement.statement_id not in control_part_id_map: - label = statement.statement_id - logger.warning( - f'No statement label found for statement id {label}. Defaulting to {label}.' - ) - else: - label = control_part_id_map[statement.statement_id] - comp_info = ComponentImpInfo(statement.description, rule_list, [], status) - self._catalog_interface.add_comp_info( - imp_req.control_id, context.comp_name, label, comp_info - ) + status = ControlInterface.get_status_from_props(imp_req) + control_rules, _ = ControlInterface.get_rule_list_for_item(imp_req) + comp_info = ComponentImpInfo(imp_req.description, control_rules, [], status) + self._catalog_interface.add_comp_info(imp_req.control_id, context.comp_name, '', comp_info) + set_params = copy.deepcopy(ci_set_params) + set_params.update(ControlInterface.get_set_params_from_item(imp_req)) + for set_param in set_params.values(): + self._catalog_interface.add_comp_set_param(imp_req.control_id, context.comp_name, set_param) + for statement in as_list(imp_req.statements): + status = ControlInterface.get_status_from_props(statement) + if statement.statement_id not in control_part_id_map: + label = statement.statement_id + logger.warning(f'No statement label found for statement id {label}. Defaulting to {label}.') + else: + label = control_part_id_map[statement.statement_id] + rule_list, _ = ControlInterface.get_rule_list_for_item(statement) + comp_info = ComponentImpInfo(statement.description, rule_list, [], status) + self._catalog_interface.add_comp_info(imp_req.control_id, context.comp_name, label, comp_info) catalog_merger = CatalogMerger(self._catalog_interface) diff --git a/trestle/core/control_reader.py b/trestle/core/control_reader.py index c1881c488..884910ac5 100644 --- a/trestle/core/control_reader.py +++ b/trestle/core/control_reader.py @@ -283,9 +283,6 @@ def read_implemented_requirement(control_file: pathlib.Path, imp_req.statements = [] comp_dict = md_comp_dict[comp_name] for label, comp_info in comp_dict.items(): - # only assemble responses with associated rules - if not comp_info.rules: - continue # if no label it applies to the imp_req itself rather than a statement if not label: imp_req.description = ControlReader._handle_empty_prose(comp_info.prose, control_id) diff --git a/trestle/core/control_writer.py b/trestle/core/control_writer.py index 481e23931..62703bd5f 100644 --- a/trestle/core/control_writer.py +++ b/trestle/core/control_writer.py @@ -129,7 +129,7 @@ def _insert_comp_info( level = 3 if context.purpose == ContextPurpose.COMPONENT else 4 if part_label in comp_info: info = comp_info[part_label] - if context.purpose in [ContextPurpose.COMPONENT, ContextPurpose.SSP] and not info.rules: + if context.purpose in [ContextPurpose.COMPONENT, ContextPurpose.SSP] and not self._include_component(info): return self._md_file.new_paragraph() if info.prose: @@ -266,23 +266,35 @@ def _add_implementation_response_prompts( self._insert_comp_info(part_label, dic, context) self._md_file.new_hr() + def _include_component(self, comp_info: ComponentImpInfo) -> bool: + """ + Check if a component has the required Markdown fields. + + Notes: This is a simple function to centralize logic to check + when a component meets the requirement to get written to Markdown. + """ + if comp_info.rules or comp_info.prose: + return True + return False + def _skip_part(self, context: ControlContext, part_label: str, comp_dict: CompDict) -> bool: """ Check if a part should be skipped based on rules and context. Notes: The default logic is to conditionally add control parts based - on whether the component has rules associated with that part. This can be + on whether the component has rules or existing prose associated with that part. This can be changed using the control context for SSP markdown. """ if context.purpose == ContextPurpose.SSP and context.include_all_parts: return False else: - no_applied_rules = True + skip_item = True for _, dic in comp_dict.items(): - if part_label in dic and dic[part_label].rules: - no_applied_rules = False - break - return no_applied_rules + if part_label in dic: + if self._include_component(dic[part_label]): + skip_item = False + break + return skip_item def _dump_subpart_infos(self, level: int, part: Dict[str, Any]) -> None: name = part['name']