2011 distributed fides better handle children with no access results (#…

…2012)
ethyca · Dec 9, 2022 · ec7aee8 · ec7aee8
1 parent de0f1fb
commit ec7aee8
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,9 +18,11 @@ The types of changes are:
 
 ## [Unreleased](https://github.com/ethyca/fides/compare/2.2.1...main)
 
-### Fixes
+### Fixed
 
 * Fixed mypy and pylint errors [#2013](https://github.com/ethyca/fides/pull/2013)
+* Update connection test endpoint to be effectively non-blocking [#2000](https://github.com/ethyca/fides/pull/2000)
+* Update Fides connector to better handle children with no access results [#2012](https://github.com/ethyca/fides/pull/2012)
 
 
 ## [2.2.1](https://github.com/ethyca/fides/compare/2.2.0...2.2.1)

diff --git a/src/fides/api/ops/service/connectors/fides/fides_client.py b/src/fides/api/ops/service/connectors/fides/fides_client.py
@@ -51,6 +51,9 @@ def __init__(
 
     def login(self) -> None:
         ul: UserLogin = UserLogin(username=self.username, password=self.password)
+        log.info(
+            f"Logging in to remote fides {self.uri} with username '{self.username}'..."
+        )
         try:
             response = requests.post(
                 f"{self.uri}{urls.V1_URL_PREFIX}{urls.LOGIN}", json=ul.dict()
@@ -61,6 +64,9 @@ def login(self) -> None:
 
         if response.ok:
             self.token = response.json()["token_data"]["access_token"]
+            log.info(
+                f"Successfully logged in to remote fides {self.uri} with username '{self.username}'"
+            )
         else:
             log.error(f"Error logging in on remote Fides {self.uri}")
             response.raise_for_status()
@@ -104,6 +110,9 @@ def create_privacy_request(
             policy_key=policy_key,
         )
 
+        log.info(
+            f"Creating privacy request with external_id {external_id} on remote fides {self.uri}..."
+        )
         request: PreparedRequest = self.authenticated_request(
             method="POST",
             path=urls.V1_URL_PREFIX + urls.PRIVACY_REQUEST_AUTHENTICATED,
@@ -118,7 +127,12 @@ def create_privacy_request(
             raise FidesError(
                 f"Failed privacy request creation on remote Fides {self.uri} with failure message: {response.json()['failed'][0]['message']}"
             )
-        return response.json()["succeeded"][0]["id"]
+
+        pr_id = response.json()["succeeded"][0]["id"]
+        log.info(
+            f"Successfully created privacy request with id {pr_id} and external_id {external_id} on remote fides {self.uri}"
+        )
+        return pr_id
 
     @sync
     async def poll_for_request_completion(
@@ -137,6 +151,9 @@ async def poll_for_request_completion(
                 f"Unable to poll for request completion. No token for Fides connector for server {self.uri}"
             )
 
+        log.info(
+            f"Polling remote fides {self.uri} for completion of privacy request with id {privacy_request_id}..."
+        )
         status: PrivacyRequestResponse = await poll_server_for_completion(
             privacy_request_id=privacy_request_id,
             server_url=self.uri,
@@ -157,7 +174,7 @@ async def poll_for_request_completion(
                 f"Privacy request [{privacy_request_id}] on remote Fides {self.uri} was denied. Look at the remote Fides for more information."
             )
         if status.status == PrivacyRequestStatus.complete:
-            log.debug(
+            log.info(
                 f"Privacy request [{privacy_request_id}] is complete on remote Fides {self.uri}!",
             )
             return status
@@ -170,6 +187,15 @@ def request_status(self, privacy_request_id: str = None) -> List[Dict[str, Any]]
         """
         Return privacy request object that tracks its status
         """
+        if privacy_request_id:
+            log.info(
+                f"Retrieving request status for privacy request {privacy_request_id if privacy_request_id else ''} on remote fides {self.uri}..."
+            )
+        else:
+            log.info(
+                f"Retrieving request status for all privacy requests on remote fides {self.uri}..."
+            )
+
         request: PreparedRequest = self.authenticated_request(
             method="GET",
             path=urls.V1_URL_PREFIX + urls.PRIVACY_REQUESTS,
@@ -184,6 +210,14 @@ def request_status(self, privacy_request_id: str = None) -> List[Dict[str, Any]]
             )
             response.raise_for_status()
 
+        if privacy_request_id:
+            log.info(
+                f"Retrieved request status for privacy request {privacy_request_id if privacy_request_id else ''} on remote fides {self.uri}"
+            )
+        else:
+            log.info(
+                f"Retrieved request status for all privacy requests on remote fides {self.uri}"
+            )
         return response.json()["items"]
 
     def retrieve_request_results(
@@ -197,6 +231,9 @@ def retrieve_request_results(
         Returns the filtered access results as a `Dict[str, List[Row]]
         """
         try:
+            log.info(
+                f"Retrieving request results for privacy request {privacy_request_id} on remote fides {self.uri}..."
+            )
             request = self.authenticated_request(
                 method="get",
                 path=f"{urls.V1_URL_PREFIX}{urls.PRIVACY_REQUEST_TRANSFER_TO_PARENT.format(privacy_request_id=privacy_request_id, rule_key=rule_key)}",
@@ -205,16 +242,16 @@ def retrieve_request_results(
             response = self.session.send(request)
         except requests.exceptions.HTTPError as e:
             log.error(
-                "Error retrieving data from child server for privacy request %s: %s",
-                privacy_request_id,
-                e,
+                f"Error retrieving data from child server for privacy request {privacy_request_id}: {e}"
             )
 
         if response.status_code != 200:
             log.error(
-                "Error retrieving data from child server for privacy request %s: %s",
-                privacy_request_id,
-                response.json(),
+                f"Error retrieving data from child server for privacy request {privacy_request_id}: {response.text}"
             )
+            return {}
 
+        log.info(
+            f"Retrieved request results for privacy request {privacy_request_id} on remote fides {self.uri}"
+        )
         return response.json()
diff --git a/src/fides/api/ops/service/connectors/fides_connector.py b/src/fides/api/ops/service/connectors/fides_connector.py
@@ -69,6 +69,7 @@ def test_connection(self) -> Optional[ConnectionTestStatus]:
             log.error(f"Error testing connection to remote Fides {str(e)}")
             return ConnectionTestStatus.failed
 
+        log.info(f"Successful connection test for {self.configuration.key}")
         return ConnectionTestStatus.succeeded
 
     def retrieve_data(
@@ -84,6 +85,9 @@ def retrieve_data(
             raise FidesError(
                 f"No identity data found for privacy request {privacy_request.id}, cannot execute Fides connector!"
             )
+        log.info(
+            f"{self.configuration.key} starting retrieve_data for privacy request {privacy_request.id}..."
+        )
 
         client: FidesClient = self.client()
 
@@ -110,6 +114,9 @@ def retrieve_data(
             for rule in policy.get_rules_for_action(action_type=ActionType.access)
         }
 
+        log.info(
+            f"{self.configuration.key} finished retrieve_data for privacy request {privacy_request.id}"
+        )
         return [results]
 
     def mask_data(
@@ -126,6 +133,9 @@ def mask_data(
             raise FidesError(
                 f"No identity data found for privacy request {privacy_request.id}, cannot execute Fides connector!"
             )
+        log.info(
+            f"{self.configuration.key} starting mask_data for privacy request {privacy_request.id}..."
+        )
 
         client: FidesClient = self.client()
         update_ct = 0
@@ -142,6 +152,9 @@ def mask_data(
             )
             update_ct += 1
 
+        log.info(
+            f"{self.configuration.key} finished mask_data for privacy request {privacy_request.id}"
+        )
         return update_ct
 
     def close(self) -> None:

diff --git a/tests/ops/service/connectors/fides/test_fides_client.py b/tests/ops/service/connectors/fides/test_fides_client.py
@@ -355,3 +355,20 @@ def test_request_status_privacy_request(
         # or environment-specific issues,
         # let's not assume anything about the status here.
         assert statuses[0]["status"] is not None
+
+    def test_retrieve_request_results_nonexistent_request(
+        self, authenticated_fides_client: FidesClient, policy
+    ):
+        """
+        Tests that retrieving requests results for a nonexistent request
+        properly returns an empty dict.
+
+        At this point, a nonexistent request behaves the same as a
+        legitimate request that does not output data. So we use this to
+        ensure that these requests are handled well by the client,
+        and simply return no data.
+        """
+        result = authenticated_fides_client.retrieve_request_results(
+            "some_nonexistent_request", "some_nonexistent_rule"
+        )
+        assert result == {}