Merge pull request #1105 from uc-cdis/fix/stop-letting-google-errors-win

fence/resources/google/access_utils.py

@@ -38,6 +38,10 @@
 logger = get_logger(__name__)
 
 
+class GoogleUpdateException(Exception):
+    pass
+
+
 def bulk_update_google_groups(google_bulk_mapping):
     """
     Update Google Groups based on mapping provided from Group -> Users.
@@ -116,7 +120,7 @@ def bulk_update_google_groups(google_bulk_mapping):
                     google_update_failures = True
 
             if google_update_failures:
-                raise Exception(
+                raise GoogleUpdateException(
                     f"FAILED TO UPDATE GOOGLE GROUPS (see previous errors)."
                 )
 

fence/sync/sync_users.py

@@ -39,6 +39,7 @@
 )
 from fence.resources.storage import StorageManager
 from fence.resources.google.access_utils import bulk_update_google_groups
+from fence.resources.google.access_utils import GoogleUpdateException
 from fence.sync import utils
 from fence.sync.passport_sync.ras_sync import RASVisa
 from fence.utils import get_SQLAlchemyDriver
@@ -1552,13 +1553,21 @@ def _sync(self, sess):
                 user_projects, user_yaml.project_to_resource
             )
 
-        # update the Fence DB
-        if user_projects:
-            self.logger.info("Sync to db and storage backend")
-            self.sync_to_db_and_storage_backend(user_projects, user_info, sess)
-            self.logger.info("Finish syncing to db and storage backend")
-        else:
-            self.logger.info("No users for syncing")
+        google_update_ex = None
+
+        try:
+            # update the Fence DB
+            if user_projects:
+                self.logger.info("Sync to db and storage backend")
+                self.sync_to_db_and_storage_backend(user_projects, user_info, sess)
+                self.logger.info("Finish syncing to db and storage backend")
+            else:
+                self.logger.info("No users for syncing")
+        except GoogleUpdateException as ex:
+            # save this to reraise later after all non-Google syncing has finished
+            # this way, any issues with Google only affect Google data access and don't
+            # cascade problems into non-Google AWS or Azure access
+            google_update_ex = ex
 
         # update the Arborist DB (resources, roles, policies, groups)
         if user_yaml.authz:
@@ -1601,6 +1610,8 @@ def _sync(self, sess):
             f"Persisting authz mapping to database: {user_yaml.project_to_resource}"
         )
         user_yaml.persist_project_to_resource(db_session=sess)
+        if google_update_ex is not None:
+            raise google_update_ex
 
     def _grant_all_consents_to_c999_users(
         self, user_projects, user_yaml_project_to_resources

tests/dbgap_sync/test_user_sync.py

@@ -5,10 +5,15 @@
 
 import asyncio
 import flask
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, patch
 import mock
 
 from fence import models
+from fence.resources.google import access_utils
+from fence.resources.google.access_utils import (
+    GoogleUpdateException,
+    bulk_update_google_groups,
+)
 from fence.sync.sync_users import _format_policy_id
 from fence.config import config
 from fence.job.visa_update_cronjob import Visa_Token_Update
@@ -418,6 +423,25 @@ def test_dbgap_consent_codes(
         assert resource_to_parent_paths["phs000179"] == ["/orgA/programs/"]
 
 
+@pytest.mark.parametrize("syncer", ["google", "cleversafe"], indirect=True)
+def test_sync_with_google_errors(syncer, monkeypatch):
+    """
+    Verifies that errors from the bulk_update_google_groups method, specifically ones relating to Google APIs, do not
+    prevent arborist updates from occuring.
+    """
+    monkeypatch.setitem(config, "GOOGLE_BULK_UPDATES", True)
+    syncer._update_arborist = MagicMock()
+    syncer._update_authz_in_arborist = MagicMock()
+
+    with patch("fence.sync.sync_users.bulk_update_google_groups") as mock_bulk_update:
+        mock_bulk_update.side_effect = GoogleUpdateException("Something's Wrong!")
+        with pytest.raises(GoogleUpdateException):
+            syncer.sync()
+
+    syncer._update_arborist.assert_called()
+    syncer._update_authz_in_arborist.assert_called()
+
+
 @pytest.mark.parametrize("syncer", ["google", "cleversafe"], indirect=True)
 def test_sync_from_files(syncer, db_session, storage_client):
     sess = db_session