Python 3.11 and return (un)suppressed findings (#20)

* chore: update Python version to 3.12 and adjust dependencies * feature: return list of (un)suppressed findings instead of just a success boolean * fix: add setuptools to dev requirements as pipenv doesn't seed by default anymore * downgrade to python 3.11 due to prospector incompatibility * update version * fix pipfile typo * revert version, managed by CI
schubergphilis · Oct 2, 2024 · 31fb56a · 31fb56a
1 parent 6e20bde
commit 31fb56a
Show file tree

Hide file tree

Showing 11 changed files with 484 additions and 548 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -19,7 +19,7 @@ jobs:
       - name: Set up Python
         uses: actions/setup-python@v4
         with:
-          python-version: 3.8.18
+          python-version: 3.11.10
 
       - name: Install pipenv
         run: pip install pipenv
@@ -46,7 +46,7 @@ jobs:
       - name: Set up Python
         uses: actions/setup-python@v4
         with:
-          python-version: 3.8.18
+          python-version: 3.11.10
 
       - name: Install pipenv
         run: pip install pipenv

diff --git a/.python-version b/.python-version
@@ -1 +1 @@
-3.8.18
+3.11.10
diff --git a/Pipfile b/Pipfile
@@ -8,16 +8,16 @@ sphinx = ">=7.0,<8.0"
 sphinx-rtd-theme = ">=1.0,<2.0"
 prospector = ">=1.8,<2.0"
 coverage = ">=7,<8.0"
-nose = ">=1.3,<2.0"
+pynose = ">=1.5.3,<2.0"
 nose-htmloutput = ">=0.1,<1.0"
-tox = ">=4.0<5.0"
+tox = ">=4.0,<5.0"
 betamax = ">=0.8,<1.0"
 betamax-serializers = "~=0.2,<1.0"
 semver = ">=3.0,<4.0"
 gitwrapperlib = ">=1.0,<2.0"
 twine = ">=4.0,<5.0"
 coloredlogs = ">=15.0,<16.0"
-emoji = ">=2.0,<3.0"
+emoji = ">=2.13.2,<3.0"
 toml = ">=0.1,<1.0"
 typing-extensions = ">=4.0,<5.0"
 

diff --git a/Pipfile.lock b/Pipfile.lock
diff --git a/_CI/files/environment_variables.json b/_CI/files/environment_variables.json
@@ -1,6 +1,6 @@
 {
     "PIPENV_VENV_IN_PROJECT": "true",
-    "PIPENV_DEFAULT_PYTHON_VERSION": "3.8",
+    "PIPENV_DEFAULT_PYTHON_VERSION": "3.11",
     "PYPI_URL": "https://upload.pypi.org/legacy/",
     "PROJECT_SLUG": "awsfindingsmanagerlib"
 }
diff --git a/awsfindingsmanagerlib/awsfindingsmanagerlib.py b/awsfindingsmanagerlib/awsfindingsmanagerlib.py
diff --git a/dev-requirements.txt b/dev-requirements.txt
@@ -7,19 +7,19 @@
 #
 # Please use Pipfile to update the requirements.
 #
-sphinx>=7.1.2 ; python_version >= '3.8'
+sphinx>=7.3.7 ; python_version >= '3.9'
 sphinx-rtd-theme>=1.3.0 ; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4, 3.5'
-prospector>=1.10.3 ; python_version < '4.0' and python_full_version >= '3.7.2'
-coverage>=7.5.3 ; python_version >= '3.8'
-nose>=1.3.7
+prospector>=1.10.3 ; python_full_version >= '3.7.2' and python_version < '4.0'
+coverage>=7.6.1 ; python_version >= '3.8'
+pynose>=1.5.3 ; python_version >= '3.7'
 nose-htmloutput>=0.6.0
-tox>=4.15.1 ; python_version >= '3.8'
+tox>=4.21.0 ; python_version >= '3.8'
 betamax>=0.9.0 ; python_full_version >= '3.8.1'
 betamax-serializers~=0.2.1
 semver>=3.0.2 ; python_version >= '3.7'
 gitwrapperlib>=1.0.4
 twine>=4.0.2 ; python_version >= '3.7'
 coloredlogs>=15.0.1 ; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4'
-emoji>=2.12.1 ; python_version >= '3.7'
-toml>=0.10.2 ; python_version >= '2.6' and python_version not in '3.0, 3.1, 3.2, 3.3'
-typing-extensions>=4.12.2 ; python_version < '3.10'
+emoji>=2.13.2 ; python_version >= '3.7'
+toml>=0.10.2 ; python_version >= '2.6' and python_version not in '3.0, 3.1, 3.2'
+typing-extensions>=4.12.2 ; python_version >= '3.8'
diff --git a/requirements.txt b/requirements.txt
@@ -7,9 +7,9 @@
 #
 # Please use Pipfile to update the requirements.
 #
-boto3>=1.34.129 ; python_version >= '3.8'
+boto3>=1.35.31 ; python_version >= '3.8'
 opnieuw>=1.2.1 ; python_version >= '3.7'
-python-dateutil>=2.9.0.post0 ; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'
+python-dateutil>=2.9.0.post0 ; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2'
 schema~=0.7.7
 requests~=2.32.3 ; python_version >= '3.8'
-pyyaml~=6.0.1 ; python_version >= '3.6'
+pyyaml~=6.0.2 ; python_version >= '3.8'
diff --git a/tests/test_suppressions.py b/tests/test_suppressions.py
@@ -69,64 +69,78 @@
     # hence no dev for S3.14
     for env in ['dev', 'acc', 'prd'] if security_control_id != 'S3.14' else ['acc', 'prd']:
         with open(f'tests/fixtures/findings/full/{security_control_id}/{env}.json', encoding='utf-8') as findings_file:
-            findings_by_security_control_id_fixture[security_control_id].append(json.load(findings_file))
+            findings_by_security_control_id_fixture[security_control_id].append(
+                json.load(findings_file))
 
 with open('tests/fixtures/matches.json', encoding='utf-8') as matches_file:
     full_matches_fixture = json.load(matches_file)
 
+
+def batch_update_findings_mock(_, payload):
+    return (True, payload)
+
+
 class TestValidation(FindingsManagerTestCase):
     backend_file = './tests/fixtures/suppressions/single.yaml'
 
     def test_basic_run(self):
         self.assertEqual(
             [],
-            self.findings_manager._construct_findings_on_matching_rules(api_consolidated_findings_fixture['Findings'])
+            self.findings_manager._construct_findings_on_matching_rules(
+                api_consolidated_findings_fixture['Findings'])
         )
 
+
 class TestLegacyValidation(FindingsManagerTestCase):
     backend_file = './tests/fixtures/suppressions/legacy.yaml'
 
     def test_basic_run(self):
         self.assertEqual(
             [],
-            self.findings_manager._construct_findings_on_matching_rules(gui_legacy_findings_fixture)
+            self.findings_manager._construct_findings_on_matching_rules(
+                gui_legacy_findings_fixture)
         )
 
+
 class TestBasicRun(FindingsManagerTestCase):
     @patch(
         'awsfindingsmanagerlib.FindingsManager._get_security_hub_paginator_iterator',
         lambda *_, **__: [api_consolidated_findings_fixture],
     )
-    @patch('awsfindingsmanagerlib.FindingsManager._batch_update_findings')
+    @patch('awsfindingsmanagerlib.FindingsManager._batch_update_findings', side_effect=batch_update_findings_mock)
     def test_basic_run(self, _batch_update_findings_mocked: MagicMock):
-        self.assertTrue(self.findings_manager.suppress_matching_findings())
-        self.assert_batch_update_findings_called_with(
-            [batch_update_findings_fixture], _batch_update_findings_mocked
-        )
+        success, payloads = self.findings_manager.suppress_matching_findings()
+        self.assertTrue(success)
+        self.assert_batch_update_findings(
+            [batch_update_findings_fixture], payloads)
+
 
 class TestFullSuppressions(FindingsManagerTestCase):
     backend_file = './tests/fixtures/suppressions/full.yaml'
 
     def test_validation(self):
         self.assertEqual(full_matches_fixture,
-            [dict(finding._data, matched_rule=finding._matched_rule._data)
-             for finding in self.findings_manager._construct_findings_on_matching_rules(full_findings_fixture)]
-        )
+                         [dict(finding._data, matched_rule=finding._matched_rule._data)
+                          for finding in self.findings_manager._construct_findings_on_matching_rules(full_findings_fixture)]
+                         )
 
-    @patch('awsfindingsmanagerlib.FindingsManager._batch_update_findings')
+    @patch('awsfindingsmanagerlib.FindingsManager._batch_update_findings', side_effect=batch_update_findings_mock)
     def test_payload_construction(self, _batch_update_findings_mocked: MagicMock):
-        self.assertTrue(self.findings_manager.suppress_findings_on_matching_rules(full_findings_fixture))
-        self.assert_batch_update_findings_called_with(batch_update_findings_full_fixture, _batch_update_findings_mocked)
+        success, payloads = self.findings_manager.suppress_findings_on_matching_rules(
+            full_findings_fixture)
+        self.assertTrue(success)
+        self.assert_batch_update_findings(
+            batch_update_findings_full_fixture, payloads)
 
     @patch(
         'awsfindingsmanagerlib.FindingsManager._get_security_hub_paginator_iterator',
         lambda *_, **kwargs: [{
             'Findings': findings_by_security_control_id_fixture[kwargs['query_filter']['ComplianceSecurityControlId'][0]['Value']]
         }],
     )
-    @patch('awsfindingsmanagerlib.FindingsManager._batch_update_findings')
+    @patch('awsfindingsmanagerlib.FindingsManager._batch_update_findings', side_effect=batch_update_findings_mock)
     def test_from_query(self, _batch_update_findings_mocked: MagicMock):
-        self.assertTrue(self.findings_manager.suppress_matching_findings())
-        self.assert_batch_update_findings_called_with(
-            batch_update_findings_full_fixture, _batch_update_findings_mocked
-        )
+        success, payloads = self.findings_manager.suppress_matching_findings()
+        self.assertTrue(success)
+        self.assert_batch_update_findings(
+            batch_update_findings_full_fixture, payloads)
diff --git a/tests/utils.py b/tests/utils.py
@@ -69,28 +69,25 @@ def setUp(self) -> None:
         self.findings_manager = FindingsManager()
         self.findings_manager.register_rules(rules)
 
-    def assert_batch_update_findings_called_with(self, batch_update_findings_expected: List[dict], _batch_update_findings_mocked: MagicMock):
+    def assert_batch_update_findings(self, batch_update_findings_expected: List[dict], batch_update_findings: List[dict]):
         """
-        Compare expected to actual (=mocked) api call payload.
-
-        Sadly, something like this does not work: _batch_update_findings_mocked.assert_called_once_with(ANY, batch_update_findings),
-        because FindingIdentifiers is a randomly ordered collection.
+        Compare expected to actual api call payload.
         """
-        self.assertEqual(
-            len(batch_update_findings_expected),
-            _batch_update_findings_mocked.call_count
-        )
+        self.assertEqual(len(batch_update_findings_expected),
+                         len(batch_update_findings))
 
         for expected in batch_update_findings_expected:
-            for call in _batch_update_findings_mocked.call_args_list:
+            for finding in batch_update_findings:
                 try:
-                    received_args = call.args[1]
-                    self.assertEqual(expected.keys(), received_args.keys())
-                    self.assertEqual(expected['Note'], received_args['Note'])
-                    self.assertEqual(expected['Workflow'], received_args['Workflow'])
-                    self.assertEqual(len(expected['FindingIdentifiers']), len(received_args['FindingIdentifiers']))
+                    self.assertTrue(
+                        set(expected.keys()).issubset(set(finding.keys())))
+                    self.assertEqual(expected['Note'], finding['Note'])
+                    self.assertEqual(
+                        expected['Workflow'], finding['Workflow'])
+                    self.assertEqual(len(expected['FindingIdentifiers']), len(
+                        finding['FindingIdentifiers']))
                     for item in expected['FindingIdentifiers']:
-                        self.assertIn(item, received_args['FindingIdentifiers'])
+                        self.assertIn(item, finding['FindingIdentifiers'])
                     break
                 except:
                     continue

diff --git a/tox.ini b/tox.ini
@@ -5,11 +5,11 @@
 # and then run "tox" from this directory.
 
 [tox]
-envlist =  py38,
+envlist =  py311,
 
 [testenv]
 allowlist_externals = *
-commands = ./setup.py nosetests --with-coverage --cover-tests --cover-html --cover-html-dir=test-output/coverage --with-html --html-file test-output/nosetests.html
+commands = nosetests --with-coverage --cover-tests --cover-html --cover-html-dir=test-output/coverage --with-html --html-file test-output/nosetests.html
 deps =
     -rrequirements.txt
     -rdev-requirements.txt