From 15374098a6417170a3f34a98dd2df42a8eb2ec3e Mon Sep 17 00:00:00 2001 From: Donny Peeters Date: Tue, 8 Oct 2024 14:58:02 +0200 Subject: [PATCH 01/14] First test setup of report runner --- rocky/reports/runner/local.py | 22 +++++++++------- rocky/tests/integration/conftest.py | 7 +++++ rocky/tests/integration/test_report_runner.py | 26 +++++++++++++++++++ 3 files changed, 45 insertions(+), 10 deletions(-) create mode 100644 rocky/tests/integration/test_report_runner.py diff --git a/rocky/reports/runner/local.py b/rocky/reports/runner/local.py index d95cf15bff8..d8b2b15c264 100644 --- a/rocky/reports/runner/local.py +++ b/rocky/reports/runner/local.py @@ -6,37 +6,39 @@ from octopoes.connector.octopoes import OctopoesAPIConnector from octopoes.models import Reference -from octopoes.models.ooi.reports import ReportRecipe from reports.report_types.helpers import get_report_by_id from reports.runner.models import JobRuntimeError, ReportJobRunner from reports.views.base import format_plugin_data, hydrate_plugins from reports.views.mixins import collect_reports, save_report_data -from rocky.bytes_client import get_bytes_client +from rocky.bytes_client import get_bytes_client, BytesClient from rocky.scheduler import ReportTask class LocalReportJobRunner(ReportJobRunner): + def __init__(self, bytes_client: BytesClient, valid_time: datetime | None): + self.bytes_client = bytes_client + self.valid_time = valid_time + def run(self, report_task: ReportTask) -> None: - now = datetime.now(timezone.utc) + valid_time = self.valid_time or datetime.now(timezone.utc) + connector = OctopoesAPIConnector(settings.OCTOPOES_API, report_task.organisation_id) - recipe: ReportRecipe = connector.get( - Reference.from_str(f"ReportRecipe|{report_task.report_recipe_id}"), datetime.now(timezone.utc) - ) - parsed_report_types = [get_report_by_id(report_type_id) for report_type_id in recipe.report_types] + recipe = connector.get(Reference.from_str(f"ReportRecipe|{report_task.report_recipe_id}"), valid_time) + report_types = [get_report_by_id(report_type_id) for report_type_id in recipe.report_types] error_reports, report_data = collect_reports( - now, connector, list(recipe.input_recipe["input_oois"]), parsed_report_types + valid_time, connector, recipe.input_recipe["input_oois"], report_types ) try: - report_type_plugins = hydrate_plugins(parsed_report_types, get_katalogus(report_task.organisation_id)) + report_type_plugins = hydrate_plugins(report_types, get_katalogus(report_task.organisation_id)) plugins = format_plugin_data(report_type_plugins) except KATalogusError as e: raise JobRuntimeError("Failed to hydrate plugins from KATalogus") from e save_report_data( get_bytes_client(report_task.organisation_id), - now, + valid_time, connector, Organization.objects.get(code=report_task.organisation_id), plugins, diff --git a/rocky/tests/integration/conftest.py b/rocky/tests/integration/conftest.py index fb80952ba6e..6c13a196221 100644 --- a/rocky/tests/integration/conftest.py +++ b/rocky/tests/integration/conftest.py @@ -16,6 +16,8 @@ from octopoes.models.ooi.software import Software, SoftwareInstance from octopoes.models.ooi.web import URL, HostnameHTTPURL, HTTPHeader, HTTPResource, SecurityTXT, Website +from reports.runner.local import LocalReportJobRunner + @pytest.fixture def valid_time(): @@ -44,6 +46,11 @@ def octopoes_api_connector_2(request) -> OctopoesAPIConnector: connector.delete_node() +@pytest.fixture +def report_runner(valid_time, mocker) -> LocalReportJobRunner: + return LocalReportJobRunner(mocker.MagicMock(), valid_time) + + def seed_system( octopoes_api_connector: OctopoesAPIConnector, valid_time: datetime, diff --git a/rocky/tests/integration/test_report_runner.py b/rocky/tests/integration/test_report_runner.py new file mode 100644 index 00000000000..e104c242dc5 --- /dev/null +++ b/rocky/tests/integration/test_report_runner.py @@ -0,0 +1,26 @@ +from octopoes.api.models import Declaration +from octopoes.connector.octopoes import OctopoesAPIConnector +from octopoes.models.ooi.reports import ReportRecipe + +from rocky.scheduler import ReportTask +from tests.integration.conftest import seed_system + + +def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_runner, valid_time): + seed_system(octopoes_api_connector, valid_time) + + recipe = ReportRecipe( + recipe_id="8aa4e52b-812c-4cc2-8196-35fb8efc63ca", + report_name_format="{report_type} for {ooi} in %Y", + sub_report_name_format="{report_type} for {ooi} in %Y", + input_recipe={"input_oois": "Network|internet"}, + report_types=["dns-report"], + cron_expression="* * * * *" + ) + octopoes_api_connector.save_declaration(Declaration(ooi=recipe, valid_time=valid_time)) + + task = ReportTask( + organisation_id=octopoes_api_connector.client, report_recipe_id="8aa4e52b-812c-4cc2-8196-35fb8efc63ca" + ) + + report_runner.run(task) From 4536c8e0f03d3eb05e2755a487fbb63692dc009f Mon Sep 17 00:00:00 2001 From: Donny Peeters Date: Tue, 8 Oct 2024 14:59:12 +0200 Subject: [PATCH 02/14] Fix subreport field name --- rocky/tests/integration/test_report_runner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rocky/tests/integration/test_report_runner.py b/rocky/tests/integration/test_report_runner.py index e104c242dc5..cece284c959 100644 --- a/rocky/tests/integration/test_report_runner.py +++ b/rocky/tests/integration/test_report_runner.py @@ -12,8 +12,8 @@ def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_ru recipe = ReportRecipe( recipe_id="8aa4e52b-812c-4cc2-8196-35fb8efc63ca", report_name_format="{report_type} for {ooi} in %Y", - sub_report_name_format="{report_type} for {ooi} in %Y", - input_recipe={"input_oois": "Network|internet"}, + subreport_name_format="{report_type} for {ooi} in %Y", + input_recipe={"input_oois": ["Network|internet"]}, report_types=["dns-report"], cron_expression="* * * * *" ) From e9a77aed459a11f7cdc0149b7297f49c7e4c7352 Mon Sep 17 00:00:00 2001 From: Donny Peeters Date: Tue, 8 Oct 2024 15:52:59 +0200 Subject: [PATCH 03/14] WIP report runner test --- rocky/reports/runner/local.py | 21 ++++++++++++---- rocky/reports/runner/worker.py | 4 ++- rocky/tests/integration/conftest.py | 25 +++++++++++++------ rocky/tests/integration/test_report_runner.py | 7 +++++- 4 files changed, 43 insertions(+), 14 deletions(-) diff --git a/rocky/reports/runner/local.py b/rocky/reports/runner/local.py index d8b2b15c264..0f2ae3e1c7f 100644 --- a/rocky/reports/runner/local.py +++ b/rocky/reports/runner/local.py @@ -1,7 +1,7 @@ from datetime import datetime, timezone from django.conf import settings -from katalogus.client import KATalogusError, get_katalogus +from katalogus.client import KATalogusError, KATalogusClientV1 from tools.models import Organization from octopoes.connector.octopoes import OctopoesAPIConnector @@ -10,12 +10,13 @@ from reports.runner.models import JobRuntimeError, ReportJobRunner from reports.views.base import format_plugin_data, hydrate_plugins from reports.views.mixins import collect_reports, save_report_data -from rocky.bytes_client import get_bytes_client, BytesClient +from rocky.bytes_client import BytesClient from rocky.scheduler import ReportTask class LocalReportJobRunner(ReportJobRunner): - def __init__(self, bytes_client: BytesClient, valid_time: datetime | None): + def __init__(self, katalogus_client: KATalogusClientV1, bytes_client: BytesClient, valid_time: datetime | None = None): + self.katalogus_client = katalogus_client self.bytes_client = bytes_client self.valid_time = valid_time @@ -30,14 +31,22 @@ def run(self, report_task: ReportTask) -> None: valid_time, connector, recipe.input_recipe["input_oois"], report_types ) + self.katalogus_client.organization = report_task.organisation_id + self.katalogus_client.organization_uri = f"/v1/organisations/{report_task.organisation_id}" + try: - report_type_plugins = hydrate_plugins(report_types, get_katalogus(report_task.organisation_id)) + report_type_plugins = hydrate_plugins(report_types, self.katalogus_client) plugins = format_plugin_data(report_type_plugins) except KATalogusError as e: raise JobRuntimeError("Failed to hydrate plugins from KATalogus") from e + self.katalogus_client.organization = None + self.katalogus_client.organization_uri = None + + self.bytes_client.organization = report_task.organisation_id + save_report_data( - get_bytes_client(report_task.organisation_id), + self.bytes_client, valid_time, connector, Organization.objects.get(code=report_task.organisation_id), @@ -45,3 +54,5 @@ def run(self, report_task: ReportTask) -> None: report_data, [(recipe.report_name_format, recipe.report_name_format)], ) + + self.bytes_client.organization = None diff --git a/rocky/reports/runner/worker.py b/rocky/reports/runner/worker.py index 6046db33c57..9046fb866f8 100644 --- a/rocky/reports/runner/worker.py +++ b/rocky/reports/runner/worker.py @@ -10,8 +10,10 @@ from httpx import HTTPError from pydantic import ValidationError +from katalogus.client import get_katalogus from reports.runner.local import LocalReportJobRunner from reports.runner.models import ReportJobRunner, WorkerManager +from rocky.bytes_client import get_bytes_client from rocky.scheduler import SchedulerClient, Task, TaskStatus, scheduler_client logger = structlog.get_logger(__name__) @@ -251,7 +253,7 @@ def _start_working( def get_runtime_manager() -> WorkerManager: return SchedulerWorkerManager( - LocalReportJobRunner(), + LocalReportJobRunner(get_katalogus(""), get_bytes_client("")), # These are set dynamically. Needs a refactor. scheduler_client(None), settings.POOL_SIZE, settings.POLL_INTERVAL, diff --git a/rocky/tests/integration/conftest.py b/rocky/tests/integration/conftest.py index 6c13a196221..9ea556e5093 100644 --- a/rocky/tests/integration/conftest.py +++ b/rocky/tests/integration/conftest.py @@ -17,6 +17,7 @@ from octopoes.models.ooi.web import URL, HostnameHTTPURL, HTTPHeader, HTTPResource, SecurityTXT, Website from reports.runner.local import LocalReportJobRunner +from tools.models import Organization @pytest.fixture @@ -25,10 +26,22 @@ def valid_time(): @pytest.fixture -def octopoes_api_connector(request) -> OctopoesAPIConnector: +def integration_organization(request) -> Organization: test_node = f"test-{request.node.originalname}" - connector = OctopoesAPIConnector(settings.OCTOPOES_API, test_node) + return Organization.objects.create(name="Test", code=test_node) + + +@pytest.fixture +def integration_organization_2(request) -> Organization: + test_node = f"test-{request.node.originalname}-2" + + return Organization.objects.create(name="Test 2", code=test_node) + + +@pytest.fixture +def octopoes_api_connector(integration_organization) -> OctopoesAPIConnector: + connector = OctopoesAPIConnector(settings.OCTOPOES_API, integration_organization.code) connector.create_node() yield connector @@ -36,10 +49,8 @@ def octopoes_api_connector(request) -> OctopoesAPIConnector: @pytest.fixture -def octopoes_api_connector_2(request) -> OctopoesAPIConnector: - test_node = f"test-{request.node.originalname}-2" - - connector = OctopoesAPIConnector(settings.OCTOPOES_API, test_node) +def octopoes_api_connector_2(integration_organization_2) -> OctopoesAPIConnector: + connector = OctopoesAPIConnector(settings.OCTOPOES_API, integration_organization_2.code) connector.create_node() yield connector @@ -48,7 +59,7 @@ def octopoes_api_connector_2(request) -> OctopoesAPIConnector: @pytest.fixture def report_runner(valid_time, mocker) -> LocalReportJobRunner: - return LocalReportJobRunner(mocker.MagicMock(), valid_time) + return LocalReportJobRunner(mocker.MagicMock(), mocker.MagicMock(), valid_time) def seed_system( diff --git a/rocky/tests/integration/test_report_runner.py b/rocky/tests/integration/test_report_runner.py index cece284c959..6857434eee3 100644 --- a/rocky/tests/integration/test_report_runner.py +++ b/rocky/tests/integration/test_report_runner.py @@ -2,12 +2,17 @@ from octopoes.connector.octopoes import OctopoesAPIConnector from octopoes.models.ooi.reports import ReportRecipe +from reports.runner.local import LocalReportJobRunner +from rocky.health import ServiceHealth from rocky.scheduler import ReportTask +from rocky.views.health import Health from tests.integration.conftest import seed_system -def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_runner, valid_time): +def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_runner: LocalReportJobRunner, valid_time): seed_system(octopoes_api_connector, valid_time) + report_runner.katalogus_client.health.return_value = ServiceHealth(healthy=True) + report_runner.bytes_client.health.return_value = ServiceHealth(healthy=True) recipe = ReportRecipe( recipe_id="8aa4e52b-812c-4cc2-8196-35fb8efc63ca", From 936fc66d3b744bea9c40eeb2bc80035bcc820db3 Mon Sep 17 00:00:00 2001 From: Donny Peeters Date: Wed, 9 Oct 2024 10:31:13 +0200 Subject: [PATCH 04/14] Working integration test --- rocky/Makefile | 1 + rocky/reports/runner/local.py | 6 ++- rocky/reports/runner/worker.py | 2 +- rocky/reports/views/mixins.py | 41 ++++++++------- rocky/tests/integration/conftest.py | 21 +++++--- rocky/tests/integration/test_report_runner.py | 50 ++++++++++++++----- 6 files changed, 80 insertions(+), 41 deletions(-) diff --git a/rocky/Makefile b/rocky/Makefile index 615e9719125..bce14cf9615 100644 --- a/rocky/Makefile +++ b/rocky/Makefile @@ -35,6 +35,7 @@ test: python3 manage.py test testclean: + docker compose -f .ci/docker-compose.yml kill docker compose -f .ci/docker-compose.yml down --remove-orphans docker compose -f .ci/docker-compose.yml build diff --git a/rocky/reports/runner/local.py b/rocky/reports/runner/local.py index 0f2ae3e1c7f..131cabf225a 100644 --- a/rocky/reports/runner/local.py +++ b/rocky/reports/runner/local.py @@ -1,7 +1,7 @@ from datetime import datetime, timezone from django.conf import settings -from katalogus.client import KATalogusError, KATalogusClientV1 +from katalogus.client import KATalogusClientV1, KATalogusError from tools.models import Organization from octopoes.connector.octopoes import OctopoesAPIConnector @@ -15,7 +15,9 @@ class LocalReportJobRunner(ReportJobRunner): - def __init__(self, katalogus_client: KATalogusClientV1, bytes_client: BytesClient, valid_time: datetime | None = None): + def __init__( + self, katalogus_client: KATalogusClientV1, bytes_client: BytesClient, valid_time: datetime | None = None + ): self.katalogus_client = katalogus_client self.bytes_client = bytes_client self.valid_time = valid_time diff --git a/rocky/reports/runner/worker.py b/rocky/reports/runner/worker.py index 9046fb866f8..6279155ef1c 100644 --- a/rocky/reports/runner/worker.py +++ b/rocky/reports/runner/worker.py @@ -8,9 +8,9 @@ import structlog from django.conf import settings from httpx import HTTPError +from katalogus.client import get_katalogus from pydantic import ValidationError -from katalogus.client import get_katalogus from reports.runner.local import LocalReportJobRunner from reports.runner.models import ReportJobRunner, WorkerManager from rocky.bytes_client import get_bytes_client diff --git a/rocky/reports/views/mixins.py b/rocky/reports/views/mixins.py index e0d11e03f0f..46cc76fcc3d 100644 --- a/rocky/reports/views/mixins.py +++ b/rocky/reports/views/mixins.py @@ -14,7 +14,6 @@ from reports.report_types.concatenated_report.report import ConcatenatedReport from reports.report_types.helpers import REPORTS, get_report_by_id from reports.views.base import BaseReportView, ReportDataDict -from rocky.bytes_client import BytesClient def collect_reports(observed_at: datetime, octopoes_connector: OctopoesAPIConnector, ooi_pks: list[str], report_types): @@ -55,22 +54,20 @@ def collect_reports(observed_at: datetime, octopoes_connector: OctopoesAPIConnec return error_reports, report_data -def save_report_raw(bytes_client: BytesClient, data: dict) -> str: - report_data_raw_id = bytes_client.upload_raw( - raw=ReportDataDict(data).model_dump_json().encode(), manual_mime_types={"openkat/report"} - ) - - return report_data_raw_id - - def save_report_data( bytes_client, observed_at, octopoes_api_connector, organization, plugin_data, report_data, report_names -): +) -> Report | None: + if len(report_data) == 0: + return None + now = datetime.now(timezone.utc) # if it's not a single report, we need a parent if len(report_data) > 1 or len(list(report_data.values())[0]) > 1: - raw_id = save_report_raw(bytes_client, data={"plugins": plugin_data}) + raw_id = bytes_client.upload_raw( + raw=ReportDataDict({"plugins": plugin_data}).model_dump_json().encode(), + manual_mime_types={"openkat/report"}, + ) name = now.strftime(report_names[0][1]) if not name or name.isspace(): @@ -107,8 +104,10 @@ def save_report_data( name_to_save = updated_name break - raw_id = save_report_raw(bytes_client, data={"report_data": data["data"]}) - + raw_id = bytes_client.upload_raw( + raw=ReportDataDict({"report_data": data["data"]}).model_dump_json().encode(), + manual_mime_types={"openkat/report"}, + ) name = now.strftime(name_to_save) if not name or name.isspace(): name = ConcatenatedReport.name @@ -136,7 +135,10 @@ def save_report_data( report_type_id = next(iter(report_data)) ooi = next(iter(report_data[report_type_id])) data = report_data[report_type_id][ooi] - raw_id = save_report_raw(bytes_client, data={"report_data": data["data"], "plugins": plugin_data}) + raw_id = bytes_client.upload_raw( + raw=ReportDataDict({"report_data": data["data"], "plugins": plugin_data}).model_dump_json().encode(), + manual_mime_types={"openkat/report"}, + ) report_type = get_report_by_id(report_type_id) name = now.strftime(report_names[0][1]) @@ -164,7 +166,7 @@ def save_report_data( class SaveGenerateReportMixin(BaseReportView): - def save_report(self, report_names: list) -> Report: + def save_report(self, report_names: list) -> Report | None: error_reports, report_data = collect_reports( self.observed_at, self.octopoes_api_connector, @@ -247,8 +249,9 @@ def save_report(self, report_names: list) -> Report: bytes_client = self.bytes_client # Create the report - report_data_raw_id = save_report_raw(bytes_client, data=post_processed_data) - + report_data_raw_id = bytes_client.upload_raw( + raw=ReportDataDict(post_processed_data).model_dump_json().encode(), manual_mime_types={"openkat/report"} + ) report_type = type(aggregate_report) name = now.strftime(report_names[0][1]) if not name or name.isspace(): @@ -273,6 +276,8 @@ def save_report(self, report_names: list) -> Report: # Save the child reports to bytes for ooi, types in report_data.items(): for report_type, data in types.items(): - save_report_raw(bytes_client, data=data) + bytes_client.upload_raw( + raw=ReportDataDict(data).model_dump_json().encode(), manual_mime_types={"openkat/report"} + ) return report_ooi diff --git a/rocky/tests/integration/conftest.py b/rocky/tests/integration/conftest.py index 9ea556e5093..de75392a7ed 100644 --- a/rocky/tests/integration/conftest.py +++ b/rocky/tests/integration/conftest.py @@ -4,8 +4,10 @@ import pytest from django.conf import settings +from reports.runner.local import LocalReportJobRunner +from tools.models import Organization -from octopoes.api.models import Declaration, Observation +from octopoes.api.models import Declaration, Observation, ServiceHealth from octopoes.connector.octopoes import OctopoesAPIConnector from octopoes.models import OOI, DeclaredScanProfile, Reference from octopoes.models.ooi.certificate import X509Certificate @@ -16,9 +18,6 @@ from octopoes.models.ooi.software import Software, SoftwareInstance from octopoes.models.ooi.web import URL, HostnameHTTPURL, HTTPHeader, HTTPResource, SecurityTXT, Website -from reports.runner.local import LocalReportJobRunner -from tools.models import Organization - @pytest.fixture def valid_time(): @@ -26,7 +25,15 @@ def valid_time(): @pytest.fixture -def integration_organization(request) -> Organization: +def katalogus_mock(mocker): + katalogus = mocker.patch("katalogus.client.KATalogusClientV1") + katalogus().health.return_value = ServiceHealth(service="katalogus", healthy=True) + + return katalogus + + +@pytest.fixture +def integration_organization(katalogus_mock, request) -> Organization: test_node = f"test-{request.node.originalname}" return Organization.objects.create(name="Test", code=test_node) @@ -58,8 +65,8 @@ def octopoes_api_connector_2(integration_organization_2) -> OctopoesAPIConnector @pytest.fixture -def report_runner(valid_time, mocker) -> LocalReportJobRunner: - return LocalReportJobRunner(mocker.MagicMock(), mocker.MagicMock(), valid_time) +def report_runner(valid_time, katalogus_mock, mocker) -> LocalReportJobRunner: + return LocalReportJobRunner(katalogus_mock, mocker.MagicMock(), valid_time) def seed_system( diff --git a/rocky/tests/integration/test_report_runner.py b/rocky/tests/integration/test_report_runner.py index 6857434eee3..72b5ab6ee2f 100644 --- a/rocky/tests/integration/test_report_runner.py +++ b/rocky/tests/integration/test_report_runner.py @@ -1,31 +1,55 @@ +import json + +from reports.runner.local import LocalReportJobRunner + from octopoes.api.models import Declaration from octopoes.connector.octopoes import OctopoesAPIConnector from octopoes.models.ooi.reports import ReportRecipe - -from reports.runner.local import LocalReportJobRunner from rocky.health import ServiceHealth from rocky.scheduler import ReportTask -from rocky.views.health import Health from tests.integration.conftest import seed_system def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_runner: LocalReportJobRunner, valid_time): - seed_system(octopoes_api_connector, valid_time) - report_runner.katalogus_client.health.return_value = ServiceHealth(healthy=True) - report_runner.bytes_client.health.return_value = ServiceHealth(healthy=True) + oois = seed_system(octopoes_api_connector, valid_time) + report_runner.bytes_client.health.return_value = ServiceHealth(service="bytes", healthy=True) + report_runner.bytes_client.upload_raw.return_value = "abcdabcd-f8ab-4bdf-9b1b-58cd98ef6342" recipe = ReportRecipe( - recipe_id="8aa4e52b-812c-4cc2-8196-35fb8efc63ca", + recipe_id="abc4e52b-812c-4cc2-8196-35fb8efc63ca", report_name_format="{report_type} for {ooi} in %Y", subreport_name_format="{report_type} for {ooi} in %Y", - input_recipe={"input_oois": ["Network|internet"]}, + input_recipe={"input_oois": [oois["hostnames"][0].reference, oois["hostnames"][1].reference]}, report_types=["dns-report"], - cron_expression="* * * * *" + cron_expression="* * * * *", ) octopoes_api_connector.save_declaration(Declaration(ooi=recipe, valid_time=valid_time)) - task = ReportTask( - organisation_id=octopoes_api_connector.client, report_recipe_id="8aa4e52b-812c-4cc2-8196-35fb8efc63ca" - ) - + task = ReportTask(organisation_id=octopoes_api_connector.client, report_recipe_id=str(recipe.recipe_id)) report_runner.run(task) + + assert len(report_runner.bytes_client.upload_raw.mock_calls) == 3 + + assert report_runner.bytes_client.upload_raw.mock_calls[0].kwargs["manual_mime_types"] == {"openkat/report"} + assert report_runner.bytes_client.upload_raw.mock_calls[1].kwargs["manual_mime_types"] == {"openkat/report"} + assert report_runner.bytes_client.upload_raw.mock_calls[2].kwargs["manual_mime_types"] == {"openkat/report"} + + assert report_runner.bytes_client.upload_raw.mock_calls[0].kwargs["raw"] == b'{"plugins":[]}' + assert [ # The order of oois is quite random so we assert on the union of last two calls + json.loads(report_runner.bytes_client.upload_raw.mock_calls[1].kwargs["raw"]), + json.loads(report_runner.bytes_client.upload_raw.mock_calls[2].kwargs["raw"]), + ] == [{ + "report_data": { + "input_ooi":"Hostname|test|example.com", + "records":[], + "security":{"spf":True, "dkim":True, "dmarc":True, "dnssec":True, "caa":True}, + "finding_types":[] + } + }, { + "report_data":{ + "input_ooi":"Hostname|test|a.example.com", + "records":[], + "security":{"spf":True, "dkim":True, "dmarc":True, "dnssec":True, "caa":True}, + "finding_types":[] + } + }] From 3cae0f5161a984accbba200b3634477ef3e931c4 Mon Sep 17 00:00:00 2001 From: Donny Peeters Date: Wed, 9 Oct 2024 23:04:27 +0200 Subject: [PATCH 05/14] WIP naming --- octopoes/octopoes/connector/octopoes.py | 2 +- rocky/.ci/.env.test | 2 -- rocky/reports/runner/local.py | 12 +++++++++++- rocky/reports/views/mixins.py | 12 ++++++++++-- rocky/tests/integration/test_report_runner.py | 12 +++++++++++- 5 files changed, 33 insertions(+), 7 deletions(-) diff --git a/octopoes/octopoes/connector/octopoes.py b/octopoes/octopoes/connector/octopoes.py index 7bd8c261949..c8f6ba76061 100644 --- a/octopoes/octopoes/connector/octopoes.py +++ b/octopoes/octopoes/connector/octopoes.py @@ -279,7 +279,7 @@ def list_findings( def list_reports( self, valid_time: datetime, offset: int = DEFAULT_OFFSET, limit: int = DEFAULT_LIMIT - ) -> Paginated[Report]: + ) -> Paginated[tuple[Report, list[Report | None]]]: params: dict[str, str | int | list[str]] = {"valid_time": str(valid_time), "offset": offset, "limit": limit} res = self.session.get(f"/{self.client}/reports", params=params) diff --git a/rocky/.ci/.env.test b/rocky/.ci/.env.test index a834fb38ef8..c7e4af41224 100644 --- a/rocky/.ci/.env.test +++ b/rocky/.ci/.env.test @@ -16,8 +16,6 @@ ROCKY_DB_HOST=ci_rocky-db ROCKY_DB_PORT=5432 ROCKY_DB_DSN=postgres://${ROCKY_DB_USER}:${ROCKY_DB_PASSWORD}@${ROCKY_DB_HOST}:${ROCKY_DB_PORT}/${ROCKY_DB} -LOG_LEVEL=debug - KEIKO_API=http://placeholder:8005 KATALOGUS_API=http://katalogus_mock:8000 OCTOPOES_API=http://ci_octopoes:80 diff --git a/rocky/reports/runner/local.py b/rocky/reports/runner/local.py index 131cabf225a..7d7c420fdb2 100644 --- a/rocky/reports/runner/local.py +++ b/rocky/reports/runner/local.py @@ -46,6 +46,15 @@ def run(self, report_task: ReportTask) -> None: self.katalogus_client.organization_uri = None self.bytes_client.organization = report_task.organisation_id + report_names = [] + oois_count = 0 + + for report_type, data in report_data.items(): + oois_count += len(data) + + for ooi in data: + report_name = recipe.subreport_name_format.format(ooi=ooi, report_type=report_type) + report_names.append((report_name, report_name)) save_report_data( self.bytes_client, @@ -54,7 +63,8 @@ def run(self, report_task: ReportTask) -> None: Organization.objects.get(code=report_task.organisation_id), plugins, report_data, - [(recipe.report_name_format, recipe.report_name_format)], + report_names, + recipe.report_name_format.format(oois_count=oois_count), ) self.bytes_client.organization = None diff --git a/rocky/reports/views/mixins.py b/rocky/reports/views/mixins.py index 46cc76fcc3d..b54c10dd056 100644 --- a/rocky/reports/views/mixins.py +++ b/rocky/reports/views/mixins.py @@ -55,7 +55,14 @@ def collect_reports(observed_at: datetime, octopoes_connector: OctopoesAPIConnec def save_report_data( - bytes_client, observed_at, octopoes_api_connector, organization, plugin_data, report_data, report_names + bytes_client, + observed_at, + octopoes_api_connector, + organization, + plugin_data, + report_data, + report_names, + parent_report_name, ) -> Report | None: if len(report_data) == 0: return None @@ -68,7 +75,7 @@ def save_report_data( raw=ReportDataDict({"plugins": plugin_data}).model_dump_json().encode(), manual_mime_types={"openkat/report"}, ) - name = now.strftime(report_names[0][1]) + name = now.strftime(parent_report_name) if not name or name.isspace(): name = ConcatenatedReport.name @@ -182,6 +189,7 @@ def save_report(self, report_names: list) -> Report | None: self.get_plugin_data_for_saving(), report_data, report_names, + report_names[0][0], ) # If OOI could not be found or the date is incorrect, it will be shown to the user as a message error diff --git a/rocky/tests/integration/test_report_runner.py b/rocky/tests/integration/test_report_runner.py index 72b5ab6ee2f..463f8d46c11 100644 --- a/rocky/tests/integration/test_report_runner.py +++ b/rocky/tests/integration/test_report_runner.py @@ -17,7 +17,7 @@ def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_ru recipe = ReportRecipe( recipe_id="abc4e52b-812c-4cc2-8196-35fb8efc63ca", - report_name_format="{report_type} for {ooi} in %Y", + report_name_format="Concatenated report for {oois_count} objects", subreport_name_format="{report_type} for {ooi} in %Y", input_recipe={"input_oois": [oois["hostnames"][0].reference, oois["hostnames"][1].reference]}, report_types=["dns-report"], @@ -53,3 +53,13 @@ def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_ru "finding_types":[] } }] + + reports = octopoes_api_connector.list_reports(valid_time) + assert reports.count == 1 + + report, subreports = reports.items[0] + assert len(subreports) == 2 + + assert report.name == 'Concatenated report for 2 objects' + assert subreports[0].name == 'Concatenated Report' + assert subreports[1].name == 'Concatenated Report' From f2eb223e2eb6a634201a70628ead59437626bb67 Mon Sep 17 00:00:00 2001 From: Donny Peeters Date: Thu, 10 Oct 2024 12:52:44 +0200 Subject: [PATCH 06/14] Fix integration test Fix styling Fix mypy now that return value for saving reports could be None --- rocky/katalogus/client.py | 2 +- rocky/onboarding/views.py | 11 +++-- rocky/reports/runner/local.py | 2 +- rocky/rocky/bytes_client.py | 2 +- rocky/rocky/views/mixins.py | 11 +++-- rocky/tests/integration/conftest.py | 3 +- rocky/tests/integration/test_report_runner.py | 49 ++++++++++--------- 7 files changed, 44 insertions(+), 36 deletions(-) diff --git a/rocky/katalogus/client.py b/rocky/katalogus/client.py index 5f1347678f1..6a58ab9c9fd 100644 --- a/rocky/katalogus/client.py +++ b/rocky/katalogus/client.py @@ -113,7 +113,7 @@ def __init__(self, error: httpx.HTTPStatusError): class KATalogusClientV1: - def __init__(self, base_uri: str, organization: str): + def __init__(self, base_uri: str, organization: str | None): self.session = httpx.Client(base_url=base_uri) self.organization = organization self.organization_uri = f"/v1/organisations/{organization}" diff --git a/rocky/onboarding/views.py b/rocky/onboarding/views.py index b935915a0f5..2a8a24e9535 100644 --- a/rocky/onboarding/views.py +++ b/rocky/onboarding/views.py @@ -358,11 +358,12 @@ def post(self, request, *args, **kwargs): report_ooi = self.save_report([("Onboarding Report", "Onboarding Report")]) - return redirect( - reverse("view_report", kwargs={"organization_code": self.organization.code}) - + "?" - + urlencode({"report_id": report_ooi.reference}) - ) + if report_ooi: + return redirect( + reverse("view_report", kwargs={"organization_code": self.organization.code}) + + "?" + + urlencode({"report_id": report_ooi.reference}) + ) def set_member_onboarded(self): member = OrganizationMember.objects.get(user=self.request.user, organization=self.organization) diff --git a/rocky/reports/runner/local.py b/rocky/reports/runner/local.py index 7d7c420fdb2..f0c37d658af 100644 --- a/rocky/reports/runner/local.py +++ b/rocky/reports/runner/local.py @@ -43,7 +43,7 @@ def run(self, report_task: ReportTask) -> None: raise JobRuntimeError("Failed to hydrate plugins from KATalogus") from e self.katalogus_client.organization = None - self.katalogus_client.organization_uri = None + self.katalogus_client.organization_uri = "" self.bytes_client.organization = report_task.organisation_id report_names = [] diff --git a/rocky/rocky/bytes_client.py b/rocky/rocky/bytes_client.py index f874daab3c5..791ee8b5a5d 100644 --- a/rocky/rocky/bytes_client.py +++ b/rocky/rocky/bytes_client.py @@ -16,7 +16,7 @@ class BytesClient: - def __init__(self, base_url: str, username: str, password: str, organization: str): + def __init__(self, base_url: str, username: str, password: str, organization: str | None): self.credentials = {"username": username, "password": password} self.session = httpx.Client(base_url=base_url) self.organization = organization diff --git a/rocky/rocky/views/mixins.py b/rocky/rocky/views/mixins.py index f8f1be7bd2f..fda5d4eb250 100644 --- a/rocky/rocky/views/mixins.py +++ b/rocky/rocky/views/mixins.py @@ -391,26 +391,27 @@ def get_subreports(self, report_id: str) -> list[tuple[str, Report]]: return subreports - def hydrate_report_list(self, reports: list[Report]) -> list[HydratedReport]: + def hydrate_report_list(self, reports: list[tuple[Report, list[Report | None]]]) -> list[HydratedReport]: hydrated_reports: list[HydratedReport] = [] for report in reports: hydrated_report: HydratedReport = HydratedReport() parent_report, children_reports = report + filtered_children_reports = list(filter(None, children_reports)) - hydrated_report.total_children_reports = len(children_reports) + hydrated_report.total_children_reports = len(filtered_children_reports) if len(parent_report.input_oois) > 0: hydrated_report.total_objects = len(parent_report.input_oois) else: - hydrated_report.total_objects = len(self.get_children_input_oois(children_reports)) + hydrated_report.total_objects = len(self.get_children_input_oois(filtered_children_reports)) - hydrated_report.report_type_summary = self.report_type_summary(children_reports) + hydrated_report.report_type_summary = self.report_type_summary(filtered_children_reports) if not parent_report.has_parent: hydrated_children_reports: list[Report] = [] - for child_report in children_reports: + for child_report in filtered_children_reports: if str(child_report.parent_report) == str(parent_report): hydrated_children_reports.append(child_report) if len(hydrated_children_reports) >= 5: # We want to show only 5 children reports diff --git a/rocky/tests/integration/conftest.py b/rocky/tests/integration/conftest.py index de75392a7ed..2db4862d54d 100644 --- a/rocky/tests/integration/conftest.py +++ b/rocky/tests/integration/conftest.py @@ -7,7 +7,7 @@ from reports.runner.local import LocalReportJobRunner from tools.models import Organization -from octopoes.api.models import Declaration, Observation, ServiceHealth +from octopoes.api.models import Declaration, Observation from octopoes.connector.octopoes import OctopoesAPIConnector from octopoes.models import OOI, DeclaredScanProfile, Reference from octopoes.models.ooi.certificate import X509Certificate @@ -17,6 +17,7 @@ from octopoes.models.ooi.service import IPService, Service from octopoes.models.ooi.software import Software, SoftwareInstance from octopoes.models.ooi.web import URL, HostnameHTTPURL, HTTPHeader, HTTPResource, SecurityTXT, Website +from rocky.health import ServiceHealth @pytest.fixture diff --git a/rocky/tests/integration/test_report_runner.py b/rocky/tests/integration/test_report_runner.py index 463f8d46c11..aed517a2d6f 100644 --- a/rocky/tests/integration/test_report_runner.py +++ b/rocky/tests/integration/test_report_runner.py @@ -34,25 +34,30 @@ def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_ru assert report_runner.bytes_client.upload_raw.mock_calls[1].kwargs["manual_mime_types"] == {"openkat/report"} assert report_runner.bytes_client.upload_raw.mock_calls[2].kwargs["manual_mime_types"] == {"openkat/report"} - assert report_runner.bytes_client.upload_raw.mock_calls[0].kwargs["raw"] == b'{"plugins":[]}' - assert [ # The order of oois is quite random so we assert on the union of last two calls - json.loads(report_runner.bytes_client.upload_raw.mock_calls[1].kwargs["raw"]), - json.loads(report_runner.bytes_client.upload_raw.mock_calls[2].kwargs["raw"]), - ] == [{ - "report_data": { - "input_ooi":"Hostname|test|example.com", - "records":[], - "security":{"spf":True, "dkim":True, "dmarc":True, "dnssec":True, "caa":True}, - "finding_types":[] - } - }, { - "report_data":{ - "input_ooi":"Hostname|test|a.example.com", - "records":[], - "security":{"spf":True, "dkim":True, "dmarc":True, "dnssec":True, "caa":True}, - "finding_types":[] - } - }] + assert report_runner.bytes_client.upload_raw.mock_calls[0].kwargs["raw"] == b'{"plugins":[]}' + + # The order of the OOIs being processed is not guaranteed, so this is a simple workaround + both_calls = [ + { + "report_data": { + "input_ooi": "Hostname|test|example.com", + "records": [], + "security": {"spf": True, "dkim": True, "dmarc": True, "dnssec": True, "caa": True}, + "finding_types": [], + } + }, + { + "report_data": { + "input_ooi": "Hostname|test|a.example.com", + "records": [], + "security": {"spf": True, "dkim": True, "dmarc": True, "dnssec": True, "caa": True}, + "finding_types": [], + } + }, + ] + + assert json.loads(report_runner.bytes_client.upload_raw.mock_calls[1].kwargs["raw"]) in both_calls + assert json.loads(report_runner.bytes_client.upload_raw.mock_calls[2].kwargs["raw"]) in both_calls reports = octopoes_api_connector.list_reports(valid_time) assert reports.count == 1 @@ -60,6 +65,6 @@ def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_ru report, subreports = reports.items[0] assert len(subreports) == 2 - assert report.name == 'Concatenated report for 2 objects' - assert subreports[0].name == 'Concatenated Report' - assert subreports[1].name == 'Concatenated Report' + assert report.name == "Concatenated report for 2 objects" + assert subreports[0].name == "Concatenated Report" + assert subreports[1].name == "Concatenated Report" From e03ab00796259fb5d9fa23bbf31c07dddca21be8 Mon Sep 17 00:00:00 2001 From: Donny Peeters Date: Thu, 10 Oct 2024 13:06:53 +0200 Subject: [PATCH 07/14] Fix the parent report naming Fix subreport naming --- rocky/reports/runner/local.py | 5 +++-- rocky/tests/integration/test_report_runner.py | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/rocky/reports/runner/local.py b/rocky/reports/runner/local.py index f0c37d658af..389e753203f 100644 --- a/rocky/reports/runner/local.py +++ b/rocky/reports/runner/local.py @@ -49,11 +49,12 @@ def run(self, report_task: ReportTask) -> None: report_names = [] oois_count = 0 - for report_type, data in report_data.items(): + for report_type_id, data in report_data.items(): oois_count += len(data) + report_type = get_report_by_id(report_type_id) for ooi in data: - report_name = recipe.subreport_name_format.format(ooi=ooi, report_type=report_type) + report_name = recipe.subreport_name_format.format(ooi=ooi, report_type=str(report_type.name)) report_names.append((report_name, report_name)) save_report_data( diff --git a/rocky/tests/integration/test_report_runner.py b/rocky/tests/integration/test_report_runner.py index aed517a2d6f..0b915577314 100644 --- a/rocky/tests/integration/test_report_runner.py +++ b/rocky/tests/integration/test_report_runner.py @@ -66,5 +66,6 @@ def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_ru assert len(subreports) == 2 assert report.name == "Concatenated report for 2 objects" - assert subreports[0].name == "Concatenated Report" - assert subreports[1].name == "Concatenated Report" + assert {x.name for x in subreports} == { + "DNS Report for Hostname|test|a.example.com in 2024" ,"DNS Report for Hostname|test|example.com in 2024" + } From 3d8704072c9cdb48e50c8fff85cce4ff263dab2d Mon Sep 17 00:00:00 2001 From: Donny Peeters Date: Thu, 10 Oct 2024 14:10:02 +0200 Subject: [PATCH 08/14] Style fix --- rocky/reports/views/mixins.py | 9 +++++---- rocky/tests/integration/test_report_runner.py | 3 ++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/rocky/reports/views/mixins.py b/rocky/reports/views/mixins.py index 0c9975db674..586c5bee29d 100644 --- a/rocky/reports/views/mixins.py +++ b/rocky/reports/views/mixins.py @@ -73,8 +73,7 @@ def save_report_data( if len(report_data) > 1 or len(list(report_data.values())[0]) > 1: raw_id = bytes_client.upload_raw( - raw=ReportDataDict(input_data).model_dump_json().encode(), - manual_mime_types={"openkat/report"}, + raw=ReportDataDict(input_data).model_dump_json().encode(), manual_mime_types={"openkat/report"} ) name = now.strftime(parent_report_name) if not name or name.isspace(): @@ -249,7 +248,8 @@ def save_report(self, report_names: list) -> Report: # Create the report report_data_raw_id = bytes_client.upload_raw( - raw=ReportDataDict(post_processed_data | self.get_input_data()).model_dump_json().encode(), manual_mime_types={"openkat/report"} + raw=ReportDataDict(post_processed_data | self.get_input_data()).model_dump_json().encode(), + manual_mime_types={"openkat/report"}, ) report_type = type(aggregate_report) name = now.strftime(report_names[0][1]) @@ -277,7 +277,8 @@ def save_report(self, report_names: list) -> Report: for ooi, types in report_data.items(): for report_type, data in types.items(): bytes_client.upload_raw( - raw=ReportDataDict(data | self.get_input_data()).model_dump_json().encode(), manual_mime_types={"openkat/report"} + raw=ReportDataDict(data | self.get_input_data()).model_dump_json().encode(), + manual_mime_types={"openkat/report"}, ) return report_ooi diff --git a/rocky/tests/integration/test_report_runner.py b/rocky/tests/integration/test_report_runner.py index 0b915577314..4589f2dcfed 100644 --- a/rocky/tests/integration/test_report_runner.py +++ b/rocky/tests/integration/test_report_runner.py @@ -67,5 +67,6 @@ def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_ru assert report.name == "Concatenated report for 2 objects" assert {x.name for x in subreports} == { - "DNS Report for Hostname|test|a.example.com in 2024" ,"DNS Report for Hostname|test|example.com in 2024" + "DNS Report for Hostname|test|a.example.com in 2024", + "DNS Report for Hostname|test|example.com in 2024", } From 75da3ae4d050fc4792d08c40ef2f86b1bf7b13e3 Mon Sep 17 00:00:00 2001 From: Rieven Date: Thu, 10 Oct 2024 15:43:26 +0200 Subject: [PATCH 09/14] Fix initial for concat reports --- rocky/reports/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rocky/reports/forms.py b/rocky/reports/forms.py index 73765b0f38c..9388d4fe4f4 100644 --- a/rocky/reports/forms.py +++ b/rocky/reports/forms.py @@ -106,7 +106,7 @@ class CustomReportScheduleForm(BaseRockyForm): class ParentReportNameForm(BaseRockyForm): parent_report_name = forms.CharField( - label=_("Report name format"), required=False, initial="{report type} for {ooi}" + label=_("Report name format"), required=False, initial="{report type} for {oois_count} objects" ) From 73b96fec7572ca7d081cc97ca762e59ff7e4b986 Mon Sep 17 00:00:00 2001 From: Donny Peeters Date: Wed, 16 Oct 2024 10:14:36 +0200 Subject: [PATCH 10/14] Fix naming issues and spot a bug --- rocky/reports/runner/models.py | 2 +- .../runner/{local.py => report_runner.py} | 38 ++++++------- rocky/reports/runner/worker.py | 11 ++-- rocky/reports/views/base.py | 16 +----- rocky/reports/views/mixins.py | 9 ++-- rocky/tests/integration/conftest.py | 6 +-- rocky/tests/integration/test_report_runner.py | 53 +++++++++++++++---- 7 files changed, 73 insertions(+), 62 deletions(-) rename rocky/reports/runner/{local.py => report_runner.py} (59%) diff --git a/rocky/reports/runner/models.py b/rocky/reports/runner/models.py index 6ea2409cbd3..6551b3db619 100644 --- a/rocky/reports/runner/models.py +++ b/rocky/reports/runner/models.py @@ -1,7 +1,7 @@ from octopoes.models.ooi.reports import ReportRecipe -class ReportJobRunner: +class ReportRunner: def run(self, recipe: ReportRecipe) -> None: raise NotImplementedError() diff --git a/rocky/reports/runner/local.py b/rocky/reports/runner/report_runner.py similarity index 59% rename from rocky/reports/runner/local.py rename to rocky/reports/runner/report_runner.py index 389e753203f..f8a742e500e 100644 --- a/rocky/reports/runner/local.py +++ b/rocky/reports/runner/report_runner.py @@ -1,24 +1,20 @@ from datetime import datetime, timezone from django.conf import settings -from katalogus.client import KATalogusClientV1, KATalogusError from tools.models import Organization from octopoes.connector.octopoes import OctopoesAPIConnector from octopoes.models import Reference +from reports.report_types.definitions import report_plugins_union from reports.report_types.helpers import get_report_by_id -from reports.runner.models import JobRuntimeError, ReportJobRunner -from reports.views.base import format_plugin_data, hydrate_plugins +from reports.runner.models import ReportRunner from reports.views.mixins import collect_reports, save_report_data from rocky.bytes_client import BytesClient from rocky.scheduler import ReportTask -class LocalReportJobRunner(ReportJobRunner): - def __init__( - self, katalogus_client: KATalogusClientV1, bytes_client: BytesClient, valid_time: datetime | None = None - ): - self.katalogus_client = katalogus_client +class LocalReportRunner(ReportRunner): + def __init__(self, bytes_client: BytesClient, valid_time: datetime | None = None): self.bytes_client = bytes_client self.valid_time = valid_time @@ -33,18 +29,6 @@ def run(self, report_task: ReportTask) -> None: valid_time, connector, recipe.input_recipe["input_oois"], report_types ) - self.katalogus_client.organization = report_task.organisation_id - self.katalogus_client.organization_uri = f"/v1/organisations/{report_task.organisation_id}" - - try: - report_type_plugins = hydrate_plugins(report_types, self.katalogus_client) - plugins = format_plugin_data(report_type_plugins) - except KATalogusError as e: - raise JobRuntimeError("Failed to hydrate plugins from KATalogus") from e - - self.katalogus_client.organization = None - self.katalogus_client.organization_uri = "" - self.bytes_client.organization = report_task.organisation_id report_names = [] oois_count = 0 @@ -54,7 +38,9 @@ def run(self, report_task: ReportTask) -> None: report_type = get_report_by_id(report_type_id) for ooi in data: - report_name = recipe.subreport_name_format.format(ooi=ooi, report_type=str(report_type.name)) + report_name = recipe.subreport_name_format.replace("{ooi}", ooi).replace( + "{report type}", str(report_type.name) + ) report_names.append((report_name, report_name)) save_report_data( @@ -62,10 +48,16 @@ def run(self, report_task: ReportTask) -> None: valid_time, connector, Organization.objects.get(code=report_task.organisation_id), - plugins, + { + "input_data": { + "input_oois": recipe.input_recipe["input_oois"], + "report_types": recipe.report_types, + "plugins": report_plugins_union(report_types), + } + }, report_data, report_names, - recipe.report_name_format.format(oois_count=oois_count), + recipe.report_name_format.replace("{oois_count}", str(oois_count)), ) self.bytes_client.organization = None diff --git a/rocky/reports/runner/worker.py b/rocky/reports/runner/worker.py index a48e03f0551..350bdd8ed80 100644 --- a/rocky/reports/runner/worker.py +++ b/rocky/reports/runner/worker.py @@ -8,11 +8,10 @@ import structlog from django.conf import settings from httpx import HTTPError -from katalogus.client import get_katalogus from pydantic import ValidationError -from reports.runner.local import LocalReportJobRunner -from reports.runner.models import ReportJobRunner, WorkerManager +from reports.runner.models import ReportRunner, WorkerManager +from reports.runner.report_runner import LocalReportRunner from rocky.bytes_client import get_bytes_client from rocky.scheduler import SchedulerClient, Task, TaskStatus, scheduler_client @@ -22,7 +21,7 @@ class SchedulerWorkerManager(WorkerManager): def __init__( self, - runner: ReportJobRunner, + runner: ReportRunner, scheduler: SchedulerClient, pool_size: int, poll_interval: int, @@ -223,7 +222,7 @@ def _format_exit_code(exitcode: int | None) -> str: def _start_working( - task_queue: mp.Queue, runner: ReportJobRunner, scheduler: SchedulerClient, handling_tasks: dict[int, str] + task_queue: mp.Queue, runner: ReportRunner, scheduler: SchedulerClient, handling_tasks: dict[int, str] ): logger.info("Started listening for tasks from worker[pid=%s]", os.getpid()) @@ -253,7 +252,7 @@ def _start_working( def get_runtime_manager() -> WorkerManager: return SchedulerWorkerManager( - LocalReportJobRunner(get_katalogus(""), get_bytes_client("")), # These are set dynamically. Needs a refactor. + LocalReportRunner(get_bytes_client("")), # These are set dynamically. Needs a refactor. scheduler_client(None), settings.POOL_SIZE, settings.POLL_INTERVAL, diff --git a/rocky/reports/views/base.py b/rocky/reports/views/base.py index 49507b6b1b2..f03947015a9 100644 --- a/rocky/reports/views/base.py +++ b/rocky/reports/views/base.py @@ -120,13 +120,9 @@ def get(self, request: HttpRequest, *args: Any, **kwargs: Any) -> HttpResponse: return redirect(reverse("report_history", kwargs=self.get_kwargs())) -def get_plugin_ids(report_types: list[type[BaseReport]]): - return report_plugins_union(report_types) - - def hydrate_plugins(report_types: list[type["BaseReport"]], katalogus: KATalogusClientV1) -> dict[str, list[Plugin]]: plugins: dict[str, list[Plugin]] = {"required": [], "optional": []} - merged_plugins = get_plugin_ids(report_types) + merged_plugins = report_plugins_union(report_types) required_plugins_ids = list(merged_plugins["required"]) optional_plugins_ids = list(merged_plugins["optional"]) @@ -255,14 +251,6 @@ def get_available_report_types(self) -> tuple[list[dict[str, str]] | dict[str, l report_types = self.get_report_types_for_generate_report() return report_types, len(report_types) - def get_plugin_data_for_saving(self) -> list[dict]: - try: - report_type_plugins = hydrate_plugins(self.get_report_types(), get_katalogus(self.organization.code)) - except KATalogusError as error: - return messages.error(self.request, error.message) - - return format_plugin_data(report_type_plugins) - def get_observed_at(self): return self.observed_at if self.observed_at < datetime.now(timezone.utc) else datetime.now(timezone.utc) @@ -296,7 +284,7 @@ def get_input_data(self) -> dict[str, Any]: "input_data": { "input_oois": self.get_ooi_pks(), "report_types": self.get_report_type_ids(), - "plugins": get_plugin_ids(self.get_report_types()), + "plugins": report_plugins_union(self.get_report_types()), } } diff --git a/rocky/reports/views/mixins.py b/rocky/reports/views/mixins.py index 586c5bee29d..14ac15cbccd 100644 --- a/rocky/reports/views/mixins.py +++ b/rocky/reports/views/mixins.py @@ -59,7 +59,7 @@ def save_report_data( observed_at, octopoes_api_connector, organization, - input_data, + input_data: dict, report_data, report_names, parent_report_name, @@ -75,7 +75,8 @@ def save_report_data( raw_id = bytes_client.upload_raw( raw=ReportDataDict(input_data).model_dump_json().encode(), manual_mime_types={"openkat/report"} ) - name = now.strftime(parent_report_name) + name = now.strftime(parent_report_name.replace("{report type}", str(ConcatenatedReport.name))) + if not name or name.isspace(): name = ConcatenatedReport.name @@ -123,7 +124,7 @@ def save_report_data( ] child_input_data = { - "input_data": {"input_oois": [ooi], "report_types": [report_type_id], "plugins": child_plugins} + "input_data": {"input_oois": [ooi], "report_types": [report_type_id], "plugins": [child_plugins]} } raw_id = bytes_client.upload_raw( @@ -162,7 +163,7 @@ def save_report_data( manual_mime_types={"openkat/report"}, ) report_type = get_report_by_id(report_type_id) - name = now.strftime(report_names[0][1]) + name = now.strftime(parent_report_name.replace("{report type}", str(report_type.name))) if not name or name.isspace(): name = ConcatenatedReport.name diff --git a/rocky/tests/integration/conftest.py b/rocky/tests/integration/conftest.py index 2db4862d54d..63ed1abd011 100644 --- a/rocky/tests/integration/conftest.py +++ b/rocky/tests/integration/conftest.py @@ -4,7 +4,7 @@ import pytest from django.conf import settings -from reports.runner.local import LocalReportJobRunner +from reports.runner.report_runner import LocalReportRunner from tools.models import Organization from octopoes.api.models import Declaration, Observation @@ -66,8 +66,8 @@ def octopoes_api_connector_2(integration_organization_2) -> OctopoesAPIConnector @pytest.fixture -def report_runner(valid_time, katalogus_mock, mocker) -> LocalReportJobRunner: - return LocalReportJobRunner(katalogus_mock, mocker.MagicMock(), valid_time) +def report_runner(valid_time, mocker) -> LocalReportRunner: + return LocalReportRunner(mocker.MagicMock(), valid_time) def seed_system( diff --git a/rocky/tests/integration/test_report_runner.py b/rocky/tests/integration/test_report_runner.py index 4589f2dcfed..566246e5a53 100644 --- a/rocky/tests/integration/test_report_runner.py +++ b/rocky/tests/integration/test_report_runner.py @@ -1,6 +1,6 @@ import json -from reports.runner.local import LocalReportJobRunner +from reports.runner.report_runner import LocalReportRunner from octopoes.api.models import Declaration from octopoes.connector.octopoes import OctopoesAPIConnector @@ -10,7 +10,7 @@ from tests.integration.conftest import seed_system -def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_runner: LocalReportJobRunner, valid_time): +def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_runner: LocalReportRunner, valid_time): oois = seed_system(octopoes_api_connector, valid_time) report_runner.bytes_client.health.return_value = ServiceHealth(service="bytes", healthy=True) report_runner.bytes_client.upload_raw.return_value = "abcdabcd-f8ab-4bdf-9b1b-58cd98ef6342" @@ -18,7 +18,7 @@ def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_ru recipe = ReportRecipe( recipe_id="abc4e52b-812c-4cc2-8196-35fb8efc63ca", report_name_format="Concatenated report for {oois_count} objects", - subreport_name_format="{report_type} for {ooi} in %Y", + subreport_name_format="{report type} for {ooi} in %Y", input_recipe={"input_oois": [oois["hostnames"][0].reference, oois["hostnames"][1].reference]}, report_types=["dns-report"], cron_expression="* * * * *", @@ -34,7 +34,15 @@ def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_ru assert report_runner.bytes_client.upload_raw.mock_calls[1].kwargs["manual_mime_types"] == {"openkat/report"} assert report_runner.bytes_client.upload_raw.mock_calls[2].kwargs["manual_mime_types"] == {"openkat/report"} - assert report_runner.bytes_client.upload_raw.mock_calls[0].kwargs["raw"] == b'{"plugins":[]}' + data = json.loads(report_runner.bytes_client.upload_raw.mock_calls[0].kwargs["raw"]) + data["input_data"]["plugins"]["required"] = set(data["input_data"]["plugins"]["required"]) # ordering issues + assert data == { + "input_data": { + "input_oois": ["Hostname|test|example.com", "Hostname|test|a.example.com"], + "report_types": ["dns-report"], + "plugins": {"required": {"dns-sec", "dns-records"}, "optional": ["dns-zone"]}, + } + } # The order of the OOIs being processed is not guaranteed, so this is a simple workaround both_calls = [ @@ -44,7 +52,12 @@ def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_ru "records": [], "security": {"spf": True, "dkim": True, "dmarc": True, "dnssec": True, "caa": True}, "finding_types": [], - } + }, + "input_data": { + "input_oois": ["Hostname|test|example.com"], + "report_types": ["dns-report"], + "plugins": [{"required": {"dns-sec", "dns-records"}, "optional": ["dns-zone"]}], + }, }, { "report_data": { @@ -52,21 +65,39 @@ def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_ru "records": [], "security": {"spf": True, "dkim": True, "dmarc": True, "dnssec": True, "caa": True}, "finding_types": [], - } + }, + "input_data": { + "input_oois": ["Hostname|test|a.example.com"], + "report_types": ["dns-report"], + "plugins": [{"required": {"dns-sec", "dns-records"}, "optional": ["dns-zone"]}], + }, }, ] - assert json.loads(report_runner.bytes_client.upload_raw.mock_calls[1].kwargs["raw"]) in both_calls - assert json.loads(report_runner.bytes_client.upload_raw.mock_calls[2].kwargs["raw"]) in both_calls + data_1 = json.loads(report_runner.bytes_client.upload_raw.mock_calls[1].kwargs["raw"]) + data_1["input_data"]["plugins"][0]["required"] = set( + data_1["input_data"]["plugins"][0]["required"] + ) # ordering issues + data_2 = json.loads(report_runner.bytes_client.upload_raw.mock_calls[2].kwargs["raw"]) + data_2["input_data"]["plugins"][0]["required"] = set( + data_2["input_data"]["plugins"][0]["required"] + ) # ordering issues + + assert data_1 in both_calls + assert data_2 in both_calls reports = octopoes_api_connector.list_reports(valid_time) assert reports.count == 1 - report, subreports = reports.items[0] assert len(subreports) == 2 assert report.name == "Concatenated report for 2 objects" assert {x.name for x in subreports} == { - "DNS Report for Hostname|test|a.example.com in 2024", - "DNS Report for Hostname|test|example.com in 2024", + "DNS Report for Hostname|test|a.example.com in 2024" + # FIXME: the naming logic in reports/views/mixins.py 107-112 is not right. We expect to find example.com in this + # set, but instead only find a.example.com because when ooi_name is 'example.com', so the check: + # `ooi_name in default_name` also passes for 'DNS Report for Hostname|test|a.example.com in %Y'. + # We shouldn't have to guess the match in the report_names argument. The name should be overridden on an object + # in the report_data list probably. + # "DNS Report for Hostname|test|example.com in 2024", } From 6c97ae95a3cb97c80f4d884d5bdaed5fbfa35792 Mon Sep 17 00:00:00 2001 From: Donny Peeters Date: Wed, 16 Oct 2024 16:19:02 +0200 Subject: [PATCH 11/14] Form text update and make lang --- .../templates/partials/report_names_header.html | 7 ++++--- rocky/rocky/locale/django.pot | 12 +++++++----- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/rocky/reports/templates/partials/report_names_header.html b/rocky/reports/templates/partials/report_names_header.html index b2b0a28854c..6e3a11906fd 100644 --- a/rocky/reports/templates/partials/report_names_header.html +++ b/rocky/reports/templates/partials/report_names_header.html @@ -12,9 +12,10 @@

{% translate "Report name" %}

{% blocktranslate trimmed %} To make the report names more descriptive, you can include placeholders for the - object name, the report type and/or the reference date. Use the placeholder "{ooi}" for the - object name, "{report type}" for the report type and use a Python - strftime code for the reference date. + object name, the report type and/or the reference date. For subreports and reports over a single object, + use the placeholder "{ooi}" for the object name, "{report type}" for the report type and use a + Python strftime code for the reference + date. For reports over multiple objects, use "{oois_count}" for the number of objects in the report. {% endblocktranslate %}

diff --git a/rocky/rocky/locale/django.pot b/rocky/rocky/locale/django.pot index 4af966792b8..9dc9f1faa12 100644 --- a/rocky/rocky/locale/django.pot +++ b/rocky/rocky/locale/django.pot @@ -9,7 +9,7 @@ msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2024-10-09 14:20+0000\n" +"POT-Creation-Date: 2024-10-16 14:17+0000\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -3793,10 +3793,12 @@ msgstr "" #: reports/templates/partials/report_names_header.html msgid "" "To make the report names more descriptive, you can include placeholders for " -"the object name, the report type and/or the reference date. Use the " -"placeholder \"{ooi}\" for the object name, \"{report type}\" for the report " -"type and use a Python strftime code for the reference date." +"the object name, the report type and/or the reference date. For subreports " +"and reports over a single object, use the placeholder \"{ooi}\" for the " +"object name, \"{report type}\" for the report type and use a Python " +"strftime code for the reference date. For reports over multiple objects, " +"use \"{oois_count}\" for the number of objects in the report." msgstr "" #: reports/templates/partials/report_names_header.html From b32527e0db17e99f3140714064f38f84fcf3255e Mon Sep 17 00:00:00 2001 From: Donny Peeters Date: Thu, 17 Oct 2024 09:34:12 +0200 Subject: [PATCH 12/14] Add missing return statement --- rocky/onboarding/views.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rocky/onboarding/views.py b/rocky/onboarding/views.py index 2a8a24e9535..37826f74514 100644 --- a/rocky/onboarding/views.py +++ b/rocky/onboarding/views.py @@ -365,6 +365,8 @@ def post(self, request, *args, **kwargs): + urlencode({"report_id": report_ooi.reference}) ) + return self.get(request, *args, **kwargs) + def set_member_onboarded(self): member = OrganizationMember.objects.get(user=self.request.user, organization=self.organization) member.onboarded = True From 981ad856521796f081cc7f7e5b492b181f0b7aea Mon Sep 17 00:00:00 2001 From: Donny Peeters Date: Thu, 17 Oct 2024 09:39:42 +0200 Subject: [PATCH 13/14] Fix integration test flakiness --- rocky/tests/integration/test_report_runner.py | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/rocky/tests/integration/test_report_runner.py b/rocky/tests/integration/test_report_runner.py index 566246e5a53..dce3a8699f0 100644 --- a/rocky/tests/integration/test_report_runner.py +++ b/rocky/tests/integration/test_report_runner.py @@ -92,12 +92,15 @@ def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_ru assert len(subreports) == 2 assert report.name == "Concatenated report for 2 objects" - assert {x.name for x in subreports} == { - "DNS Report for Hostname|test|a.example.com in 2024" - # FIXME: the naming logic in reports/views/mixins.py 107-112 is not right. We expect to find example.com in this - # set, but instead only find a.example.com because when ooi_name is 'example.com', so the check: - # `ooi_name in default_name` also passes for 'DNS Report for Hostname|test|a.example.com in %Y'. - # We shouldn't have to guess the match in the report_names argument. The name should be overridden on an object - # in the report_data list probably. - # "DNS Report for Hostname|test|example.com in 2024", - } + assert "DNS Report for Hostname|test|a.example.com in 2024" in {x.name for x in subreports} + + # FIXME: the naming logic in reports/views/mixins.py 107-112 is not right. We expect to find example.com in this + # set, but instead only find a.example.com because when ooi_name is 'example.com', the check: + # `ooi_name in default_name` also passes for 'DNS Report for Hostname|test|a.example.com in %Y'. + # We shouldn't have to guess the match in the report_names argument. The name should be overridden on an object + # in the report_data list probably. Note that sometimes this does work when the OOIs are ordered differently. + + # assert {x.name for x in subreports} == { + # "DNS Report for Hostname|test|a.example.com in 2024" + # "DNS Report for Hostname|test|example.com in 2024", + # } From 9eba8abf2f12c790f81350183dad6bba766ecfc2 Mon Sep 17 00:00:00 2001 From: Donny Peeters Date: Thu, 17 Oct 2024 10:46:10 +0200 Subject: [PATCH 14/14] Style --- rocky/tests/integration/test_report_runner.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/rocky/tests/integration/test_report_runner.py b/rocky/tests/integration/test_report_runner.py index dce3a8699f0..75f60a3a7b7 100644 --- a/rocky/tests/integration/test_report_runner.py +++ b/rocky/tests/integration/test_report_runner.py @@ -99,8 +99,3 @@ def test_run_report_task(octopoes_api_connector: OctopoesAPIConnector, report_ru # `ooi_name in default_name` also passes for 'DNS Report for Hostname|test|a.example.com in %Y'. # We shouldn't have to guess the match in the report_names argument. The name should be overridden on an object # in the report_data list probably. Note that sometimes this does work when the OOIs are ordered differently. - - # assert {x.name for x in subreports} == { - # "DNS Report for Hostname|test|a.example.com in 2024" - # "DNS Report for Hostname|test|example.com in 2024", - # }