[core][feat] Maintain account score in metadata (#2030)

* [core][feat] Maintain account score in metadata

* my IDE tries to be clever

Author: aquamatthias

fixcore/fixcore/report/__init__.py

@@ -2,6 +2,7 @@
 
 import logging
 from abc import ABC, abstractmethod
+from collections import defaultdict
 from enum import Enum
 from functools import reduce, total_ordering
 from typing import List, Optional, Dict, ClassVar, AsyncIterator, cast, Set, Tuple, Any
@@ -27,6 +28,7 @@
 
 
 _ReportSeverityPriority: Dict[str, int] = {"info": 0, "low": 1, "medium": 2, "high": 3, "critical": 4}
+_ReportSeverityScore: Dict[str, int] = {"info": 0, "low": 1, "medium": 2, "high": 3, "critical": 4}
 
 
 @total_ordering
@@ -38,12 +40,17 @@ class ReportSeverity(Enum):
     high = "high"
     critical = "critical"
 
+    @property
     def prio(self) -> int:
         return _ReportSeverityPriority[self.value]
 
+    @property
+    def score(self) -> int:
+        return _ReportSeverityScore[self.value]
+
     def __lt__(self, other: Any) -> bool:
         if isinstance(other, ReportSeverity):
-            return self.prio() < other.prio()
+            return self.prio < other.prio
         return NotImplemented
 
     @staticmethod
@@ -359,6 +366,24 @@ def visit_check_collection(collection: CheckCollectionResult) -> None:
         visit_check_collection(self)
         return result
 
+    def score_for(self, account: str) -> int:
+        failing: Dict[ReportSeverity, int] = defaultdict(int)
+        available: Dict[ReportSeverity, int] = defaultdict(int)
+
+        def available_failing(check: CheckCollectionResult) -> None:
+            for result in check.checks:
+                available[result.check.severity] += 1
+                failing[result.check.severity] += (
+                    1 if result.number_of_resources_failing_by_account.get(account, 0) else 0
+                )
+            for child in check.children:
+                available_failing(child)
+
+        available_failing(self)  # walk the benchmark hierarchy
+        missing = sum(severity.score * count for severity, count in failing.items())
+        total = sum(severity.score * count for severity, count in available.items())
+        return int((max(0, total - missing) * 100) // total) if total > 0 else 100
+
 
 class Inspector(ABC):
     """

fixcore/fixcore/report/benchmark_renderer.py

@@ -47,7 +47,7 @@ def render_benchmark_summary(benchmark: CheckCollectionResult, account: str) ->
     def render_result_list(name: str, icon: str, checks: List[CheckResult]) -> str:
         if checks:
             lr = f"## {name} \n\n"
-            for check in sorted(checks, key=lambda x: -x.check.severity.prio()):
+            for check in sorted(checks, key=lambda x: -x.check.severity.prio):
                 lr += f"- {icon} {check.check.severity.name}: {check.check.title}\n"
             return lr
         else:

fixcore/fixcore/report/inspector_service.py

@@ -62,7 +62,7 @@ def includes_severity(self, severity: ReportSeverity) -> bool:
         if self.severity is None:
             return True
         else:
-            return self.severity.prio() <= severity.prio()
+            return self.severity.prio <= severity.prio
 
     def override_values(self) -> Optional[Json]:
         return self.config.override_values
@@ -241,8 +241,10 @@ async def perform_benchmarks(
         report_run_id: Optional[str] = None,
     ) -> Dict[str, BenchmarkResult]:
         config = await self.report_config()
-        context = CheckContext(accounts=accounts, severity=severity, only_failed=only_failing, config=config)
         benchmarks = await self.__benchmarks(benchmark_names)
+        clouds = {c for b in benchmarks.values() for c in b.clouds or []}
+        cloud_accounts = accounts or await self.__list_accounts(list(clouds), graph)
+        context = CheckContext(accounts=cloud_accounts, severity=severity, only_failed=only_failing, config=config)
         # collect all checks
         check_ids = {check for b in benchmarks.values() for check in b.nested_checks() if config.check_allowed(check)}
         checks = await self.list_checks(check_ids=list(check_ids), context=context)
@@ -256,9 +258,22 @@ async def perform_benchmarks(
             model = await self.model_handler.load_model(graph)
             # In case no run_id is provided, we invent a report run id here.
             run_id = report_run_id or uuid_str()
-            await self.db_access.get_graph_db(graph).update_security_section(
-                run_id, self.__benchmarks_to_security_iterator(result), model, accounts
-            )
+            db = self.db_access.get_graph_db(graph)
+            await db.update_security_section(run_id, self.__benchmarks_to_security_iterator(result), model, accounts)
+            # store the security score of an account
+            if account_ids := context.accounts:
+                async with await db.search_list(  # lookup node ids for all given accounts
+                    QueryModel(Query.by("account", P("reported.id").is_in(account_ids)), model)
+                ) as crsr:
+                    async for acc in crsr:
+                        if (node_id := value_in_path(acc, NodePath.node_id)) and (
+                            acc_id := value_in_path(acc, NodePath.reported_id)
+                        ):
+                            bench_score = {br.id: br.score_for(acc_id) for br in result.values()}
+                            account_score = sum(bench_score.values()) / len(bench_score) if bench_score else 100
+                            patch = {"score": account_score, "benchmark": bench_score}
+                            async for _ in db.update_nodes_metadata(model, patch, [node_id]):
+                                pass
         return result
 
     async def perform_checks(
@@ -303,7 +318,7 @@ async def perform_checks(
         )
 
         if context.accounts is None:
-            context.accounts = await self.__list_accounts(benchmark, graph)
+            context.accounts = await self.__list_accounts(benchmark.clouds, graph)
 
         checks_to_perform = await self.list_checks(check_ids=benchmark.nested_checks(), context=context)
         check_by_id = {c.id: c for c in checks_to_perform}
@@ -449,12 +464,12 @@ async def __perform_check(
                 resources_by_account[account_id].append(bend(ReportResourceData, resource))
         return resources_by_account
 
-    async def __list_accounts(self, benchmark: Benchmark, graph: GraphName) -> List[str]:
+    async def __list_accounts(self, clouds: Optional[List[str]], graph: GraphName) -> List[str]:
         model = await self.model_handler.load_model(graph)
         gdb = self.db_access.get_graph_db(graph)
         query = Query.by("account")
-        if benchmark.clouds:
-            query = query.combine(Query.by(P.single("ancestors.cloud.reported.id").is_in(benchmark.clouds)))
+        if clouds:
+            query = query.combine(Query.by(P.single("ancestors.cloud.reported.id").is_in(clouds)))
         async with await gdb.search_list(QueryModel(query, model)) as crs:
             ids = [value_in_path(a, NodePath.reported_id) async for a in crs]
             return [aid for aid in ids if aid is not None]

fixcore/tests/fixcore/cli/cli_test.py

@@ -172,14 +172,14 @@ def test_parse_predecessor_successor_ancestor_descendant_args() -> None:
 
 @pytest.mark.asyncio
 async def test_create_query_parts(cli: CLI) -> None:
-    commands = await cli.evaluate_cli_command('search some_int==0 | search identifier=~"9_" | descendants')
+    commands = await cli.evaluate_cli_command('search some_int==0 | search id=~"9_" | descendants')
     sort = "sort reported.kind asc, reported.name asc, reported.id asc"
     assert len(commands) == 1
     assert len(commands[0].commands) == 2  # list command is added automagically
     assert commands[0].commands[0].name == "execute_search"
     assert (
         commands[0].executable_commands[0].arg
-        == f"'(reported.some_int == 0 and reported.identifier =~ \"9_\") {sort} -default[1:]-> all {sort}'"
+        == f"'(reported.some_int == 0 and reported.id =~ \"9_\") {sort} -default[1:]-> all {sort}'"
     )
     commands = await cli.evaluate_cli_command("search some_int==0 | descendants")
     assert "-default[1:]->" in commands[0].executable_commands[0].arg  # type: ignore

fixcore/tests/fixcore/cli/command_test.py

@@ -141,11 +141,9 @@ async def test_descendants(cli: CLI) -> None:
 
 @pytest.mark.asyncio
 async def test_search_source(cli: CLIService) -> None:
-    result = await cli.execute_cli_command('search is("foo") and some_int==0 --> identifier=~"9_"', list_sink)
+    result = await cli.execute_cli_command('search is("foo") and some_int==0 --> id=~"9_"', list_sink)
     assert len(result[0]) == 10
-    await cli.dependencies.template_expander.put_template(
-        Template("test", 'is(foo) and some_int==0 --> identifier=~"{{fid}}"')
-    )
+    await cli.dependencies.template_expander.put_template(Template("test", 'is(foo) and some_int==0 --> id=~"{{fid}}"'))
     result2 = await cli.execute_cli_command('search expand(test, fid="9_")', list_sink)
     assert len(result2[0]) == 10
 
@@ -502,7 +500,7 @@ async def test_kinds_command(cli: CLI, foo_model: Model) -> None:
         "properties": {
             "age": "duration",
             "ctime": "datetime",
-            "identifier": "string",
+            "id": "string",
             "kind": "string",
             "name": "string",
             "now_is": "datetime",
@@ -538,12 +536,12 @@ async def test_kinds_command(cli: CLI, foo_model: Model) -> None:
 async def test_sort_command(cli: CLI) -> None:
     async def identifiers(query: str) -> List[str]:
         result = await cli.execute_cli_command(query + " | dump", list_sink)
-        return [r["reported"]["identifier"] for r in result[0]]
+        return [r["reported"]["id"] for r in result[0]]
 
-    id_wo = await identifiers("search is(bla) | sort identifier")
-    id_asc = await identifiers("search is(bla) | sort identifier asc")
-    id_desc = await identifiers("search is(bla) | sort identifier desc")
-    id_kind = await identifiers("search is(bla) | sort identifier | sort kind")
+    id_wo = await identifiers("search is(bla) | sort id")
+    id_asc = await identifiers("search is(bla) | sort id asc")
+    id_desc = await identifiers("search is(bla) | sort id desc")
+    id_kind = await identifiers("search is(bla) | sort id | sort kind")
     assert id_wo == id_asc
     assert id_wo == id_kind
     assert id_asc == list(reversed(id_desc))
@@ -553,27 +551,27 @@ async def identifiers(query: str) -> List[str]:
 async def test_limit_command(cli: CLI) -> None:
     async def identifiers(query: str) -> List[str]:
         result = await cli.execute_cli_command(query + " | dump", list_sink)
-        return [r["reported"]["identifier"] for r in result[0]]
+        return [r["reported"]["id"] for r in result[0]]
 
-    assert await identifiers("search is(bla) sort identifier | limit 1") == ["0_0"]
-    assert await identifiers("search is(bla) sort identifier | limit 2") == ["0_0", "0_1"]
-    assert await identifiers("search is(bla) sort identifier | limit 2, 2") == ["0_2", "0_3"]
-    assert await identifiers("search is(bla) sort identifier | limit 10, 2") == ["1_0", "1_1"]
-    assert await identifiers("search is(bla) sort identifier | limit 100, 2") == []
+    assert await identifiers("search is(bla) sort id | limit 1") == ["0_0"]
+    assert await identifiers("search is(bla) sort id | limit 2") == ["0_0", "0_1"]
+    assert await identifiers("search is(bla) sort id | limit 2, 2") == ["0_2", "0_3"]
+    assert await identifiers("search is(bla) sort id | limit 10, 2") == ["1_0", "1_1"]
+    assert await identifiers("search is(bla) sort id | limit 100, 2") == []
 
 
 @pytest.mark.asyncio
 async def test_list_command(cli: CLI) -> None:
-    result = await cli.execute_cli_command('search is (foo) and identifier=="4" sort some_int | list', list_sink)
+    result = await cli.execute_cli_command('search is (foo) and id=="4" sort some_int | list', list_sink)
     assert len(result[0]) == 1
-    assert result[0][0].startswith("kind=foo, identifier=4, some_int=0, age=")
+    assert result[0][0].startswith("kind=foo, id=4, some_int=0, age=")
     list_cmd = "list some_int as si, some_string"
-    result = await cli.execute_cli_command(f'search is (foo) and identifier=="4" | {list_cmd}', list_sink)
+    result = await cli.execute_cli_command(f'search is (foo) and id=="4" | {list_cmd}', list_sink)
     assert result[0] == ["si=0, some_string=hello"]
 
     # list is added automatically when no output renderer is defined and has the same behaviour as if it was given
-    result = await cli.execute_cli_command('search is (foo) and identifier=="4" sort some_int', list_sink)
-    assert result[0][0].startswith("kind=foo, identifier=4, some_int=0, age=")
+    result = await cli.execute_cli_command('search is (foo) and id=="4" sort some_int', list_sink)
+    assert result[0][0].startswith("kind=foo, id=4, some_int=0, age=")
 
     # List is using the correct type
     props = dict(id="test", a="a", b=True, c=False, d=None, e=12, f=1.234, reported={})
@@ -964,7 +962,7 @@ async def test_pagerduty_alias(cli: CLI, echo_http_server: Tuple[int, List[Tuple
             "severity": "warning",
             "component": "Fix",
             "custom_details": {
-                "collector": {"sub_root": {"no-region": {"0_0": {"id": None, "name": "yes or no", "kind": "bla"}}}}
+                "collector": {"sub_root": {"no-region": {"0_0": {"id": "0_0", "name": "yes or no", "kind": "bla"}}}}
             },
         },
         "routing_key": "123",
@@ -1435,7 +1433,7 @@ async def exec(cmd: str) -> List[JsonElement]:
     in_one_min = utc_str(now + timedelta(minutes=1))
     one_min_ago = utc_str(now - timedelta(minutes=1))
     # Create a time series based on all foo entries
-    res = await exec("timeseries snapshot --name test 'search aggregate(reported.some_int, reported.identifier: sum(1)): is(foo)'")  # fmt: skip
+    res = await exec("timeseries snapshot --name test 'search aggregate(reported.some_int, reported.id: sum(1)): is(foo)'")  # fmt: skip
     assert res[0] == "10 entries added to time series test."
     # Get the time series combined with each complete group
     res = await exec(f"timeseries get --name test --start {one_min_ago} --end {in_one_min}")
@@ -1446,11 +1444,11 @@ async def exec(cmd: str) -> List[JsonElement]:
     # Combine over some_int (which has only one) --> only one entry for one timestamp
     res = await exec(f"timeseries get --name test --start {one_min_ago} --end {in_one_min} --group some_int")
     assert len(res) == 1
-    # Combine over identifier (which is unique for each entry) --> 10 entries for one timestamp
-    res = await exec(f"timeseries get --name test --start {one_min_ago} --end {in_one_min} --group identifier")
+    # Combine over id (which is unique for each entry) --> 10 entries for one timestamp
+    res = await exec(f"timeseries get --name test --start {one_min_ago} --end {in_one_min} --group id")
     assert len(res) == 10
-    # Combine over identifier (which is unique for each entry), filter for identifier==2 --> 1 entry for one timestamp
-    res = await exec(f'timeseries get --name test --start {one_min_ago} --end {in_one_min} --group identifier --filter identifier=="2"')  # fmt: skip
+    # Combine over id (which is unique for each entry), filter for id==2 --> 1 entry for one timestamp
+    res = await exec(f'timeseries get --name test --start {one_min_ago} --end {in_one_min} --group id --filter id=="2"')  # fmt: skip
     assert len(res) == 1
 
 

fixcore/tests/fixcore/conftest.py

@@ -263,7 +263,7 @@ def foo_kinds() -> List[Kind]:
         "base",
         [],
         [
-            Property("identifier", "string", required=True),
+            Property("id", "string", required=True),
             Property("kind", "string", required=True),
             Property("ctime", "datetime"),
         ],
@@ -328,7 +328,7 @@ def person_model() -> Model:
         "Base",
         [],
         [
-            Property("id", "string", required=True, description="Some identifier"),
+            Property("id", "string", required=True, description="Some id"),
             Property("kind", "string", required=True, description="Kind of this node."),
             Property("list", "string[]", description="A list of strings."),
             Property("tags", "dictionary[string, string]", description="Key/value pairs."),
@@ -592,7 +592,7 @@ def benchmark() -> Benchmark:
         framework="test",
         documentation="test",
         version="1.5",
-        clouds=["test"],
+        clouds=["collector"],
         title="test_benchmark",
         description="test_benchmark",
         checks=[],