feat(bigquery): Custom message when Service Account doesnt have the correct Roles and Permissions

superset/databases/commands/create.py

@@ -46,6 +46,13 @@ def run(self) -> Model:
         try:
             # Test connection before starting create transaction
             TestConnectionDatabaseCommand(self._properties).run()
+        except DatabaseConnectionFailedError as ex:
+            event_logger.log_with_context(
+                action=f"db_creation_failed.{ex.__class__.__name__}",
+                engine=self._properties.get("sqlalchemy_uri", "").split(":")[0],
+            )
+            # So we can show the original message
+            raise ex
         except Exception as ex:
             event_logger.log_with_context(
                 action=f"db_creation_failed.{ex.__class__.__name__}",

superset/databases/commands/test_connection.py

@@ -27,6 +27,7 @@
 
 from superset.commands.base import BaseCommand
 from superset.databases.commands.exceptions import (
+    DatabaseConnectionFailedError,
     DatabaseSecurityUnsafeError,
     DatabaseTestConnectionDriverError,
     DatabaseTestConnectionFailedError,
@@ -49,6 +50,7 @@ def __init__(self, data: Dict[str, Any]):
 
     def run(self) -> None:  # pylint: disable=too-many-statements
         self.validate()
+        ex_str = ""
         uri = self._properties.get("sqlalchemy_uri", "")
         if self._model and uri == self._model.safe_sqlalchemy_uri():
             uri = self._model.sqlalchemy_uri_decrypted
@@ -117,10 +119,13 @@ def ping(engine: Engine) -> bool:
                     level=ErrorLevel.ERROR,
                     extra={"sqlalchemy_uri": database.sqlalchemy_uri},
                 ) from ex
-            except Exception:  # pylint: disable=broad-except
+            except Exception as ex:  # pylint: disable=broad-except
                 alive = False
+                # So we stop losing the original message if any
+                ex_str = str(ex)
+
             if not alive:
-                raise DBAPIError(None, None, None)
+                raise DBAPIError(ex_str or None, None, None)
 
             # Log succesful connection test with engine
             event_logger.log_with_context(
@@ -145,6 +150,19 @@ def ping(engine: Engine) -> bool:
             )
             # check for custom errors (wrong username, wrong password, etc)
             errors = database.db_engine_spec.extract_errors(ex, context)
+            for error in errors:
+                # Why Adding the message for this error type?
+                # Because it's the type returned for Service Accounts without roles
+                # or permissions when connecting to BigQuery DBs, which is
+                # the change we are introducing at this moment and we don't want to
+                # impact other places (tests for example) at least until we discuss
+                # and standardize the way we bubble up exceptions
+                if (
+                    error.error_type
+                    == SupersetErrorType.CONNECTION_DATABASE_PERMISSIONS_ERROR
+                ):
+                    # Raise original exception with the message
+                    raise DatabaseConnectionFailedError(error.message) from ex
             raise DatabaseTestConnectionFailedError(errors) from ex
         except SupersetSecurityException as ex:
             event_logger.log_with_context(

superset/db_engine_specs/bigquery.py

@@ -46,8 +46,8 @@
 
 
 CONNECTION_DATABASE_PERMISSIONS_REGEX = re.compile(
-    "Access Denied: Project User does not have bigquery.jobs.create "
-    + "permission in project (?P<project>.+?)"
+    "Access Denied: Project (?P<project_name>.+?): User does not have "
+    + "bigquery.jobs.create permission in project (?P<project>.+?)"
 )
 
 TABLE_DOES_NOT_EXIST_REGEX = re.compile(
@@ -154,9 +154,12 @@ class BigQueryEngineSpec(BaseEngineSpec):
     custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]] = {
         CONNECTION_DATABASE_PERMISSIONS_REGEX: (
             __(
-                "We were unable to connect to your database. Please "
-                "confirm that your service account has the Viewer "
-                "and Job User roles on the project."
+                "Unable to connect. Verify that the following roles are set "
+                'on the service account: "BigQuery Data Viewer", '
+                '"BigQuery Metadata Viewer", "BigQuery Job User" '
+                "and the following permissions are set "
+                '"bigquery.readsessions.create", '
+                '"bigquery.readsessions.getData"'
             ),
             SupersetErrorType.CONNECTION_DATABASE_PERMISSIONS_ERROR,
             {},

tests/integration_tests/db_engine_specs/bigquery_tests.py

@@ -246,11 +246,11 @@ def test_df_to_sql(self, mock_get_engine):
         )
 
     def test_extract_errors(self):
-        msg = "403 POST https://bigquery.googleapis.com/bigquery/v2/projects/test-keel-310804/jobs?prettyPrint=false: Access Denied: Project User does not have bigquery.jobs.create permission in project profound-keel-310804"
+        msg = "403 POST https://bigquery.googleapis.com/bigquery/v2/projects/test-keel-310804/jobs?prettyPrint=false: Access Denied: Project profound-keel-310804: User does not have bigquery.jobs.create permission in project profound-keel-310804"
         result = BigQueryEngineSpec.extract_errors(Exception(msg))
         assert result == [
             SupersetError(
-                message="We were unable to connect to your database. Please confirm that your service account has the Viewer and Job User roles on the project.",
+                message='Unable to connect. Verify that the following roles are set on the service account: "BigQuery Data Viewer", "BigQuery Metadata Viewer", "BigQuery Job User" and the following permissions are set "bigquery.readsessions.create", "bigquery.readsessions.getData"',
                 error_type=SupersetErrorType.CONNECTION_DATABASE_PERMISSIONS_ERROR,
                 level=ErrorLevel.ERROR,
                 extra={