PR feedback

interuss · Sep 13, 2023 · b12444d · b12444d
1 parent 24e4b55
commit b12444d
Show file tree

Hide file tree

Showing 8 changed files with 100 additions and 25 deletions.
diff --git a/monitoring/monitorlib/fetch/__init__.py b/monitoring/monitorlib/fetch/__init__.py
@@ -269,19 +269,23 @@ def describe_query(
     query_type: Optional[QueryType] = None,
     server_id: Optional[str] = None,
 ) -> Query:
-    return Query(
+    query = Query(
         request=describe_request(resp.request, initiated_at),
         response=describe_response(resp),
-        server_id=server_id,
-        query_type=query_type,
     )
+    if query_type is not None:
+        query.query_type = query_type
+    if server_id is not None:
+        query.server_id = server_id
+    return query
 
 
 def query_and_describe(
     client: Optional[infrastructure.UTMClientSession],
     verb: str,
     url: str,
     query_type: Optional[QueryType] = None,
+    server_id: Optional[str] = None,
     **kwargs,
 ) -> Query:
     """Attempt to perform a query, and then describe the results of that attempt.
@@ -305,7 +309,6 @@ def query_and_describe(
     else:
         utm_session = True
     req_kwargs = kwargs.copy()
-    req_kwargs.pop("server_id", None)
     if "timeout" not in req_kwargs:
         req_kwargs["timeout"] = TIMEOUTS
 
@@ -320,7 +323,7 @@ def query_and_describe(
                 client.request(verb, url, **req_kwargs),
                 t0,
                 query_type=query_type,
-                server_id=kwargs.get("server_id", None),
+                server_id=server_id,
             )
         except (requests.Timeout, urllib3.exceptions.ReadTimeoutError) as e:
             failure_message = f"query_and_describe attempt {attempt + 1} from PID {os.getpid()} to {verb} {url} failed with timeout {type(e).__name__}: {str(e)}"
@@ -350,7 +353,7 @@ def query_and_describe(
             elapsed_s=(t1 - t0).total_seconds(),
             reported=StringBasedDateTime(t1),
         ),
-        server_id=kwargs.get("server_id", None),
+        server_id=server_id,
     )
     if query_type is not None:
         result.query_type = query_type

diff --git a/monitoring/uss_qualifier/scenarios/astm/netrid/common/aggregate_checks.py b/monitoring/uss_qualifier/scenarios/astm/netrid/common/aggregate_checks.py
@@ -22,6 +22,8 @@
     NetRIDServiceProvider,
 )
 
+from loguru import logger
+
 
 class AggregateChecks(ReportEvaluationScenario):
     _rid_version: RIDVersion
@@ -141,7 +143,6 @@ def run(self):
         self.end_test_scenario()
 
     def _verify_https_everywhere(self):
-
         for participant_id, participant_queries in self._queries_by_participant.items():
             self._inspect_participant_queries(participant_id, participant_queries)
 
@@ -155,12 +156,9 @@ def _verify_https_everywhere(self):
             # TODO clean this up: this is an internal requirement and not a check,
             #  leaving as-is during development to make sure the test-suite runs but we know about unattributed queries
             #  ultimately this check could go into the constructor and blow things up early
-            self.record_note(
-                "unattributed_queries",
-                f"found {len(unattr_queries)} unattributed queries",
-            )
+
             with self.check(
-                "All interactions happen over https",
+                "No unattributed queries",
                 [],
             ) as check:
                 check.record_failed(
@@ -170,13 +168,12 @@ def _verify_https_everywhere(self):
     def _inspect_participant_queries(
         self, participant_id: str, participant_queries: List[fetch.Query]
     ):
-        found_cleartext_query = False
+        cleartext_queries = []
         for query in participant_queries:
             if query.request.url.startswith("http://"):
-                found_cleartext_query = True
+                cleartext_queries.append(query)
                 if participant_id not in self._debug_mode_usses:
-                    self.record_note(
-                        "cleartext-query",
+                    logger.info(
                         f"query is not https: {participant_id} - {query.request.url}",
                     )
 
@@ -185,9 +182,17 @@ def _inspect_participant_queries(
                 "All interactions happen over https",
                 [participant_id],
             ) as check:
-                if found_cleartext_query:
+                if len(cleartext_queries) > 0:
+                    timestamps = [
+                        q.request.initiated_at.datetime for q in cleartext_queries
+                    ]
+                    urls = set([q.request.url for q in cleartext_queries])
                     check.record_failed(
-                        "found at least one cleartext http query", Severity.Medium
+                        summary=f"found {len(cleartext_queries)} cleartext http queries",
+                        details=f"unique cleartext urls: {urls}",
+                        severity=Severity.Medium,
+                        participants=[participant_id],
+                        query_timestamps=timestamps,
                     )
         else:
             self.record_note(
@@ -203,7 +208,6 @@ def _dp_display_data_details_times_step(self):
         for participant, all_queries in self._queries_by_participant.items():
             relevant_queries: List[fetch.Query] = list()
             for query in all_queries:
-
                 if (
                     query.status_code == 200
                     and query.has_field_with_value("query_type")
@@ -303,7 +307,6 @@ def _dp_display_data_times_step(self):
 
         pattern = re.compile(r"/display_data\?view=(-?\d+(.\d+)?,){3}-?\d+(.\d+)?")
         for participant, all_queries in self._queries_by_participant.items():
-
             # identify successful display_data queries
             relevant_queries: List[fetch.Query] = list()
             for query in all_queries:

diff --git a/monitoring/uss_qualifier/scenarios/astm/netrid/v19/aggregate_checks.md b/monitoring/uss_qualifier/scenarios/astm/netrid/v19/aggregate_checks.md
@@ -76,9 +76,32 @@ TODO: Implement this check
 
 ### Verify https is in use test step
 
-Inspects all record queries for their usage of https. If resources such as a service provide, observer or DSS are marked
-as being in "local debug" mode, they may serve requests over https without breaking the test suite.
+Inspects all record queries for their usage of https. If services such as a service provider, observer or DSS are marked
+as being in "local debug" mode, they may serve requests over http without producing failed checks despite their lack of encryption.
 
 #### All interactions happen over https check
 
 If non-encrypted interactions such as plaintext queries over http are allowed, **[astm.f3411.v19.NET0220](../../../../requirements/astm/f3411/v19.md)** is not satisfied.
+
+#### No unattributed queries check
+
+All queries must have been attributed to a participant: an unattributed query means that one of the test cases has not
+properly recorded to which participant it was made.
+
+This is an internal requirement and does not necessarily imply that there is a problem with the participants under test.
+
+## Mock USS interactions evaluation test case
+
+In this test case, the interactions with a mock_uss instance (if provided) are obtained and then examined to verify
+compliance with requirements.
+
+### Get mock USS interactions test step
+
+### Evaluate mock USS interactions test step
+
+#### No large Display Provider queries check
+
+If one of the Display Provider test participants was found to have sent a query to mock_uss with a larger-than-allowed
+area requested, then that participant will have violated **[astm.f3411.v19.NET0240](../../../../requirements/astm/f3411/v19.md)**.
+
+TODO: Implement this check
diff --git a/monitoring/uss_qualifier/scenarios/astm/netrid/v22a/aggregate_checks.md b/monitoring/uss_qualifier/scenarios/astm/netrid/v22a/aggregate_checks.md
@@ -76,9 +76,32 @@ TODO: Implement this check
 
 ### Verify https is in use test step
 
-Inspects all record queries for their usage of https. If resources such as a service provide, observer or DSS are marked
-as being in "local debug" mode, they may serve requests over https without breaking the test suite.
+Inspects all record queries for their usage of https. If services such as a service provider, observer or DSS are marked
+as being in "local debug" mode, they may serve requests over http without producing failed checks despite their lack of encryption.
+
+#### No unattributed queries check
+
+All queries must have been attributed to a participant: an unattributed query means that one of the test cases has not
+properly recorded to which participant it was made.
+
+This is an internal requirement and does not necessarily imply that there is a problem with the participants under test.
 
 #### All interactions happen over https check
 
 If non-encrypted interactions such as plaintext queries over http are allowed, **[astm.f3411.v19.NET0220](../../../../requirements/astm/f3411/v19.md)** is not satisfied.
+
+## Mock USS interactions evaluation test case
+
+In this test case, the interactions with a mock_uss instance (if provided) are obtained and then examined to verify
+compliance with requirements.
+
+### Get mock USS interactions test step
+
+### Evaluate mock USS interactions test step
+
+#### No large Display Provider queries check
+
+If one of the Display Provider test participants was found to have sent a query to mock_uss with a larger-than-allowed
+area requested, then that participant will have violated **[astm.f3411.v22a.NET0240](../../../../requirements/astm/f3411/v22a.md)**.
+
+TODO: Implement this check
diff --git a/monitoring/uss_qualifier/suites/astm/netrid/f3411_19.md b/monitoring/uss_qualifier/suites/astm/netrid/f3411_19.md
@@ -21,7 +21,7 @@
     <th>Checked in</th>
   </tr>
   <tr>
-    <td rowspan="43" style="vertical-align:top;"><a href="../../../requirements/astm/f3411/v19.md">astm<br>.f3411<br>.v19</a></td>
+    <td rowspan="44" style="vertical-align:top;"><a href="../../../requirements/astm/f3411/v19.md">astm<br>.f3411<br>.v19</a></td>
     <td><a href="../../../requirements/astm/f3411/v19.md">A2-6-1,1a</a></td>
     <td>Implemented</td>
     <td><a href="../../../scenarios/astm/netrid/v19/dss_interoperability.md">ASTM F3411-19 NetRID DSS interoperability</a></td>
@@ -166,6 +166,11 @@
     <td>Implemented</td>
     <td><a href="../../../scenarios/astm/netrid/v19/dss_interoperability.md">ASTM F3411-19 NetRID DSS interoperability</a></td>
   </tr>
+  <tr>
+    <td><a href="../../../requirements/astm/f3411/v19.md">NET0220</a></td>
+    <td>Implemented</td>
+    <td><a href="../../../scenarios/astm/netrid/v19/aggregate_checks.md">ASTM F3411-19 NetRID aggregate checks</a></td>
+  </tr>
   <tr>
     <td><a href="../../../requirements/astm/f3411/v19.md">NET0240</a></td>
     <td>Planned</td>

diff --git a/monitoring/uss_qualifier/suites/astm/netrid/f3411_22a.md b/monitoring/uss_qualifier/suites/astm/netrid/f3411_22a.md
@@ -20,6 +20,12 @@
     <th>Status</th>
     <th>Checked in</th>
   </tr>
+  <tr>
+    <td rowspan="1" style="vertical-align:top;"><a href="../../../requirements/astm/f3411/v19.md">astm<br>.f3411<br>.v19</a></td>
+    <td><a href="../../../requirements/astm/f3411/v19.md">NET0220</a></td>
+    <td>Implemented</td>
+    <td><a href="../../../scenarios/astm/netrid/v22a/aggregate_checks.md">ASTM F3411-22a NetRID aggregate checks</a></td>
+  </tr>
   <tr>
     <td rowspan="52" style="vertical-align:top;"><a href="../../../requirements/astm/f3411/v22a.md">astm<br>.f3411<br>.v22a</a></td>
     <td><a href="../../../requirements/astm/f3411/v22a.md">A2-6-1,1a</a></td>

diff --git a/monitoring/uss_qualifier/suites/uspace/network_identification.md b/monitoring/uss_qualifier/suites/uspace/network_identification.md
@@ -15,6 +15,12 @@
     <th>Status</th>
     <th>Checked in</th>
   </tr>
+  <tr>
+    <td rowspan="1" style="vertical-align:top;"><a href="../../requirements/astm/f3411/v19.md">astm<br>.f3411<br>.v19</a></td>
+    <td><a href="../../requirements/astm/f3411/v19.md">NET0220</a></td>
+    <td>Implemented</td>
+    <td><a href="../../scenarios/astm/netrid/v22a/aggregate_checks.md">ASTM F3411-22a NetRID aggregate checks</a></td>
+  </tr>
   <tr>
     <td rowspan="52" style="vertical-align:top;"><a href="../../requirements/astm/f3411/v22a.md">astm<br>.f3411<br>.v22a</a></td>
     <td><a href="../../requirements/astm/f3411/v22a.md">A2-6-1,1a</a></td>

diff --git a/monitoring/uss_qualifier/suites/uspace/required_services.md b/monitoring/uss_qualifier/suites/uspace/required_services.md
@@ -16,6 +16,12 @@
     <th>Status</th>
     <th>Checked in</th>
   </tr>
+  <tr>
+    <td rowspan="1" style="vertical-align:top;"><a href="../../requirements/astm/f3411/v19.md">astm<br>.f3411<br>.v19</a></td>
+    <td><a href="../../requirements/astm/f3411/v19.md">NET0220</a></td>
+    <td>Implemented</td>
+    <td><a href="../../scenarios/astm/netrid/v22a/aggregate_checks.md">ASTM F3411-22a NetRID aggregate checks</a></td>
+  </tr>
   <tr>
     <td rowspan="52" style="vertical-align:top;"><a href="../../requirements/astm/f3411/v22a.md">astm<br>.f3411<br>.v22a</a></td>
     <td><a href="../../requirements/astm/f3411/v22a.md">A2-6-1,1a</a></td>