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

Issue:
trestle validate component-definitions incorrectly failing on port-ranges
## Describe the bug

After upgrading to trestle 3.0.1, a ComponentDefinition file that was valid under trestle 2.6.1 started to fail with messages about the `protocols -> port-ranges` `start` and `end` values not being valid dicts. They should be non-negative integers.

## To Reproduce

Steps to reproduce the behavior:

Attempt to validate this minimum reproduction case:

```json
{
  "component-definition": {
    "uuid": "e7ba3d37-2ba0-4db9-9cf8-f5ec1214ffd5",
    "metadata": {
      "title": "Generic component-definition created by trestle named replication.",
      "last-modified": "2024-06-11T21:16:49.175968+00:00",
      "version": "0.0.0",
      "oscal-version": "1.1.2"
    },
    "components": [
      {
        "uuid": "4cef9684-578e-460e-a2d1-6eb78b8a76ca",
        "type": "REPLACE_ME",
        "title": "REPLACE_ME",
        "description": "REPLACE_ME",
        "protocols": [
          {
            "uuid": "0130b4ef-4561-4363-bab5-1e20b9888e49",
            "name": "https",
            "title": "Transport Layer Security",
            "port-ranges": [
              {
                "start": 443,
                "end": 443,
                "transport": "TCP"
              }
            ]
          }
        ],
        "control-implementations": []
      }
    ]
  }
}
```

## Expected behavior

`trestle validate` should return `VALID`.

## Screenshots / Logs.

<img width="1242" alt="Screenshot 2024-06-11 at 5 40 29 PM" src="https://github.com/oscal-compass/compliance-trestle/assets/285842/41618055-6437-4179-92d1-0ff322c82ddb">

## Environment

- OS: docker - python:3.11.9-slim base image.
- Python version: 3.11.9
- Installed packages: just trestle 3.0.1


Pull request:
fix: handle NonNegativeIntegerDatatype and PositiveIntegerDatatype in gen_oscal
## 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.
- [ ] 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

fixes #1580 

## 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/scripts/oscal_normalize.py b/scripts/oscal_normalize.py
index cd5fe724..73a37b29 100644
--- a/scripts/oscal_normalize.py
+++ b/scripts/oscal_normalize.py
@@ -1033,7 +1033,13 @@ def kill_roots(file_classes):
                 for ii in range(1, len(c.lines)):
                     line = c.lines[ii]
                     for name, body in new_root_classes.items():
-                        # handle special case
+                        # handle special cases
+                        if 'NonNegativeIntegerDatatype' in line:
+                            line = line.replace('NonNegativeIntegerDatatype', 'conint(ge=0, multiple_of=1)', 1)
+                        if 'PositiveIntegerDatatype' in line:
+                            line = line.replace('PositiveIntegerDatatype', 'conint(ge=1, multiple_of=1)', 1)
+                        if 'IntegerDatatype' in line:
+                            line = line.replace('IntegerDatatype', 'conint(multiple_of=1)', 1)
                         if c.name == 'OscalVersion':
                             if any(token in line for token in [' __root__: StringDatatype']):
                                 line = line.replace(name, body, 1)
diff --git a/trestle/oscal/common.py b/trestle/oscal/common.py
index 2908b765..71a463c8 100644
--- a/trestle/oscal/common.py
+++ b/trestle/oscal/common.py
@@ -408,6 +408,25 @@ class PortRangeValidValues(Enum):
     UDP = 'UDP'
 
 
+class PortRange(OscalBaseModel):
+    """
+    Where applicable this is the IPv4 port range on which the service operates.
+    """
+
+    class Config:
+        extra = Extra.forbid
+
+    start: Optional[conint(ge=0, multiple_of=1)] = Field(
+        None, description='Indicates the starting port number in a port range', title='Start'
+    )
+    end: Optional[conint(ge=0, multiple_of=1)] = Field(
+        None, description='Indicates the ending port number in a port range', title='End'
+    )
+    transport: Optional[PortRangeValidValues] = Field(
+        None, description='Indicates the transport type.', title='Transport'
+    )
+
+
 class PartyUuid(OscalBaseModel):
     __root__: UUIDDatatype = Field(
         ..., description='Reference to a party by UUID.', title='Party Universally Unique Identifier Reference'
@@ -982,9 +1001,9 @@ class AtFrequency(OscalBaseModel):
     class Config:
         extra = Extra.forbid
 
-    period: PositiveIntegerDatatype = Field(
-        ..., description='The task must occur after the specified period has elapsed.', title='Period'
-    )
+    period: conint(
+        ge=1, multiple_of=1
+    ) = Field(..., description='The task must occur after the specified period has elapsed.', title='Period')
     unit: TimeUnitValidValues = Field(..., description='The unit of time for the period.', title='Time Unit')
 
 
@@ -1557,25 +1576,6 @@ class Published(OscalBaseModel):
     )
 
 
-class PortRange(OscalBaseModel):
-    """
-    Where applicable this is the IPv4 port range on which the service operates.
-    """
-
-    class Config:
-        extra = Extra.forbid
-
-    start: Optional[NonNegativeIntegerDatatype] = Field(
-        None, description='Indicates the starting port number in a port range', title='Start'
-    )
-    end: Optional[NonNegativeIntegerDatatype] = Field(
-        None, description='Indicates the ending port number in a port range', title='End'
-    )
-    transport: Optional[PortRangeValidValues] = Field(
-        None, description='Indicates the transport type.', title='Transport'
-    )
-
-
 class Protocol(OscalBaseModel):
     """
     Information about the protocol used to provide a service.


Test patch:
diff --git a/tests/data/validate/component-definitions/x1/component-definition.json b/tests/data/validate/component-definitions/x1/component-definition.json
new file mode 100644
index 00000000..0dc7ec5f
--- /dev/null
+++ b/tests/data/validate/component-definitions/x1/component-definition.json
@@ -0,0 +1,20 @@
+{
+  "component-definition": {
+    "uuid": "e7ba3d37-2ba0-4db9-9cf8-f5ec1214ffd5",
+    "metadata": {
+      "title": "Generic component-definition created by trestle named replication.",
+      "last-modified": "2024-06-11T21:16:49.175968+00:00",
+      "version": "0.0.0",
+      "oscal-version": "1.0.4"
+    },
+    "components": [
+      {
+        "uuid": "4cef9684-578e-460e-a2d1-6eb78b8a76ca",
+        "type": "REPLACE_ME",
+        "title": "REPLACE_ME",
+        "description": "REPLACE_ME",
+        "control-implementations": []
+      }
+    ]
+  }
+}
\ No newline at end of file
diff --git a/tests/data/validate/component-definitions/x2/component-definition.json b/tests/data/validate/component-definitions/x2/component-definition.json
new file mode 100644
index 00000000..69c85ee2
--- /dev/null
+++ b/tests/data/validate/component-definitions/x2/component-definition.json
@@ -0,0 +1,34 @@
+{
+  "component-definition": {
+    "uuid": "e7ba3d37-2ba0-4db9-9cf8-f5ec1214ffd5",
+    "metadata": {
+      "title": "Generic component-definition created by trestle named replication.",
+      "last-modified": "2024-06-11T21:16:49.175968+00:00",
+      "version": "0.0.0",
+      "oscal-version": "1.0.4"
+    },
+    "components": [
+      {
+        "uuid": "4cef9684-578e-460e-a2d1-6eb78b8a76ca",
+        "type": "REPLACE_ME",
+        "title": "REPLACE_ME",
+        "description": "REPLACE_ME",
+        "protocols": [
+          {
+            "uuid": "0130b4ef-4561-4363-bab5-1e20b9888e49",
+            "name": "https",
+            "title": "Transport Layer Security",
+            "port-ranges": [
+              {
+                "start": 443,
+                "end": 443,
+                "transport": "TCP"
+              }
+            ]
+          }
+        ],
+        "control-implementations": []
+      }
+    ]
+  }
+}
\ No newline at end of file
diff --git a/tests/data/validate/component-definitions/x3/component-definition.json b/tests/data/validate/component-definitions/x3/component-definition.json
new file mode 100644
index 00000000..1fadb711
--- /dev/null
+++ b/tests/data/validate/component-definitions/x3/component-definition.json
@@ -0,0 +1,34 @@
+{
+  "component-definition": {
+    "uuid": "e7ba3d37-2ba0-4db9-9cf8-f5ec1214ffd5",
+    "metadata": {
+      "title": "Generic component-definition created by trestle named replication.",
+      "last-modified": "2024-06-11T21:16:49.175968+00:00",
+      "version": "0.0.0",
+      "oscal-version": "1.0.4"
+    },
+    "components": [
+      {
+        "uuid": "4cef9684-578e-460e-a2d1-6eb78b8a76ca",
+        "type": "REPLACE_ME",
+        "title": "REPLACE_ME",
+        "description": "REPLACE_ME",
+        "protocols": [
+          {
+            "uuid": "0130b4ef-4561-4363-bab5-1e20b9888e49",
+            "name": "https",
+            "title": "Transport Layer Security",
+            "port-ranges": [
+              {
+                "start": -443,
+                "end": 443,
+                "transport": "TCP"
+              }
+            ]
+          }
+        ],
+        "control-implementations": []
+      }
+    ]
+  }
+}
\ No newline at end of file
diff --git a/tests/trestle/core/commands/validate_test.py b/tests/trestle/core/commands/validate_test.py
index 4f14a089..8eb02f45 100644
--- a/tests/trestle/core/commands/validate_test.py
+++ b/tests/trestle/core/commands/validate_test.py
@@ -30,6 +30,7 @@ from tests.test_utils import setup_for_ssp
 import trestle.common.const as const
 import trestle.core.generators as gens
 import trestle.oscal.assessment_plan as ap
+import trestle.oscal.common as common
 import trestle.oscal.profile as prof
 import trestle.oscal.ssp as ossp
 from trestle import cli
@@ -407,3 +408,65 @@ def test_validate_ssp_with_no_profile(tmp_trestle_dir: pathlib.Path, monkeypatch
 
     new_ssp.import_profile.href = original_href
     ModelUtils.save_top_level_model(new_ssp, tmp_trestle_dir, ssp_name, FileContentType.JSON)
+
+
+def test_period(tmp_trestle_dir: pathlib.Path, monkeypatch: MonkeyPatch) -> None:
+    """Test period."""
+    unit = common.TimeUnitValidValues.seconds
+    _ = common.AtFrequency(period=1, unit=unit)
+    try:
+        _ = common.AtFrequency(period=0, unit=unit)
+        raise RuntimeError('must be positive integer')
+    except Exception:
+        pass
+
+
+def test_validate_component_definition(tmp_trestle_dir: pathlib.Path, monkeypatch: MonkeyPatch) -> None:
+    """Test validation of Component Definition."""
+    jfile = 'component-definition.json'
+
+    sdir = test_data_dir / 'validate' / 'component-definitions' / 'x1'
+    spth = sdir / f'{jfile}'
+
+    tdir = tmp_trestle_dir / test_utils.COMPONENT_DEF_DIR / 'my_test_model'
+    tpth = tdir / f'{jfile}'
+
+    (tdir).mkdir(exist_ok=True, parents=True)
+
+    shutil.copyfile(spth, tpth)
+    validate_command = f'trestle validate -f {tpth}'
+    test_utils.execute_command_and_assert(validate_command, 0, monkeypatch)
+
+
+def test_validate_component_definition_ports(tmp_trestle_dir: pathlib.Path, monkeypatch: MonkeyPatch) -> None:
+    """Test validation of ports in Component Definition."""
+    jfile = 'component-definition.json'
+
+    sdir = test_data_dir / 'validate' / 'component-definitions' / 'x2'
+    spth = sdir / f'{jfile}'
+
+    tdir = tmp_trestle_dir / test_utils.COMPONENT_DEF_DIR / 'my_test_model'
+    tpth = tdir / f'{jfile}'
+
+    (tdir).mkdir(exist_ok=True, parents=True)
+
+    shutil.copyfile(spth, tpth)
+    validate_command = f'trestle validate -f {tpth}'
+    test_utils.execute_command_and_assert(validate_command, 0, monkeypatch)
+
+
+def test_validate_component_definition_ports_invalid(tmp_trestle_dir: pathlib.Path, monkeypatch: MonkeyPatch) -> None:
+    """Test validation of ports in Component Definition."""
+    jfile = 'component-definition.json'
+
+    sdir = test_data_dir / 'validate' / 'component-definitions' / 'x3'
+    spth = sdir / f'{jfile}'
+
+    tdir = tmp_trestle_dir / test_utils.COMPONENT_DEF_DIR / 'my_test_model'
+    tpth = tdir / f'{jfile}'
+
+    (tdir).mkdir(exist_ok=True, parents=True)
+
+    shutil.copyfile(spth, tpth)
+    validate_command = f'trestle validate -f {tpth}'
+    test_utils.execute_command_and_assert(validate_command, 1, monkeypatch)