Repository: oscal-compass/compliance-trestle (python)

Issue:
Component definitions without rules have unexpected markdown editing behavior
## Describe the bug

Trestle does not support OSCAL-compliant Component Definitions and require Rule properties for correct processing. When using OSCAL Component Definitions without rule properties, the markdown files are generated without the source implementation information. If the markdown is subsequently edited, the information is not added to the JSON.

Additional discussion and reproduction information is located on #1631 

## To Reproduce

Steps to reproduce the behavior:

```bash
trestle init
trestle import -f https://raw.githubusercontent.com/CivicActions/oscal-component-definitions/main/aws/oscal/aws.json -o aws
trestle author component-generate -o md_comp/aws --name aws
trestle author component-assemble --markdown md_comp/aws/ -o aws
```

## Expected behavior

At minimum, an error should be thrown for unsupported OSCAL Component Definitions or control implementations.

From the discussion: `trestle author component-generate` is run for a component definition with no rule information, the resulting markdown files should contain the original information and allow editing of the OSCAL schema compliant implementation details (e.g. implementation description)

## Screenshots / Logs.

Additional information - Adding rules manually to the JSON fixed the behavior

```bash
            "implemented-requirements": [
              {
                "uuid": "c9cbefe4-5c62-4a14-bbb6-91cb5658dfc8",
                "control-id": "ac-2",
                "description": "The system partially inherits this control from the FedRAMP Provisional ATO granted to the AWS Cloud dated 1 May 2013 for the following: AWS account management and other things.",
                "props": [
                  {
                    "name": "Rule_Id",
                    "ns": "http://ibm.github.io/compliance-trestle/schemas/oscal/cd",
                    "value": "Test-rule_001"
                  },
                  {
                    "name": "implementation-status",
                    "value": "implemented"
                  }
                ]
              }
```
## Environment

- OS: \[e.g. iOS\] Fedora
- Python version: 3.11
- Installed packages: `trestle` 3.3.0




Pull request description:
fix(markdown): writes component data for markdown without rules
## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->

- [ ] Hot fix (emergency fix and release)
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Documentation (change which affects the documentation site)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Release (`develop` -> `main`)

## Quality assurance (all should be covered).

- [x] My code follows the code style of this project.
- [x] Documentation for my change is up to date?
- [x] My PR meets testing requirements.
- [x] All new and existing tests passed.
- [x] All commits are signed-off.

## Summary

Allows component definitions without rules to be edited by trestle.

- During assembly components will be written out if they are in the Markdown without rule constraints (_This was causing the written Markdown without Rules to be skipped_)
- In the catalog writer, the CompInfo is added to `catalog interface` without rule constraints (_This was causing the Markdown to be empty when rules were not present._)

Parts handling:

- Top level controls will be written out in the Markdown for component without rule constraints
- Parts are still conditionally written out, but the component is checked for rules or non-empty prose.

Closes #1667 

## Key links:

- [Sonar coverage](https://sonarcloud.io/dashboard?id=compliance-trestle)

# Before you merge

- Ensure it is a 'squash commit' if not a release.
- Ensure CI is currently passing
- Check sonar. If you are working for a fork a maintainer will reach out, if required.


Patch:
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 9a1d2acb..0286b5d5 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/trestle/core/catalog/catalog_writer.py b/trestle/core/catalog/catalog_writer.py
index a255dcd1..13cb1e58 100644
--- a/trestle/core/catalog/catalog_writer.py
+++ b/trestle/core/catalog/catalog_writer.py
@@ -355,31 +355,24 @@ class CatalogWriter():
             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 c1881c48..884910ac 100644
--- a/trestle/core/control_reader.py
+++ b/trestle/core/control_reader.py
@@ -283,9 +283,6 @@ class ControlReader:
         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 481e2393..62703bd5 100644
--- a/trestle/core/control_writer.py
+++ b/trestle/core/control_writer.py
@@ -129,7 +129,7 @@ class ControlWriter():
         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 @@ class ControlWriter():
                     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']


Test patch:
diff --git a/tests/data/json/comp_def_c.json b/tests/data/json/comp_def_c.json
new file mode 100644
index 00000000..f56d434d
--- /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 508b14f7..0160371c 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?
+
+<!-- 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