Skip to content

Commit

Permalink
fix(markdown): writes component data for markdown without rules (#1695)
Browse files Browse the repository at this point in the history
* test: adds failing test to confirm component definition bug

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

* 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 <barnabei.jennifer@gmail.com>

* fix: assemble component responses with and without rules

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

* fix: updates control_rules logic to fix test failure

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

* 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 <barnabei.jennifer@gmail.com>

* fix: updates formatting to make tests pass

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

* docs: updates docs to reflect component authoring behavior

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

---------

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
  • Loading branch information
jpower432 authored Sep 16, 2024
1 parent e67f73c commit 25dbc7a
Show file tree
Hide file tree
Showing 6 changed files with 220 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
121 changes: 121 additions & 0 deletions tests/data/json/comp_def_c.json
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
]
}
]
}
}
61 changes: 61 additions & 0 deletions tests/trestle/core/commands/author/component_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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?
<!-- For implementation status enter one of: implemented, partial, planned, alternative, not-applicable -->
<!-- Note that the list of rules under ### Rules: is read-only and changes will not be captured after assembly to JSON -->
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
43 changes: 18 additions & 25 deletions trestle/core/catalog/catalog_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
3 changes: 0 additions & 3 deletions trestle/core/control_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 19 additions & 7 deletions trestle/core/control_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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']
Expand Down

0 comments on commit 25dbc7a

Please sign in to comment.