feat(bigquery): Custom message when Service Account doesnt have the correct Roles and Permissions (#21838)
This commit is contained in:
parent
059e53a39f
commit
203b289021
|
|
@ -75,10 +75,12 @@ from superset.databases.schemas import (
|
|||
from superset.databases.utils import get_table_metadata
|
||||
from superset.db_engine_specs import get_available_engine_specs
|
||||
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
|
||||
from superset.exceptions import SupersetErrorsException
|
||||
from superset.extensions import security_manager
|
||||
from superset.models.core import Database
|
||||
from superset.superset_typing import FlaskResponse
|
||||
from superset.utils.core import error_msg_from_exception, parse_js_uri_path_item
|
||||
from superset.views.base import json_errors_response
|
||||
from superset.views.base_api import (
|
||||
BaseSupersetModelRestApi,
|
||||
requires_form_data,
|
||||
|
|
@ -224,7 +226,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
|
|||
log_to_statsd=False,
|
||||
)
|
||||
@requires_json
|
||||
def post(self) -> Response:
|
||||
def post(self) -> FlaskResponse:
|
||||
"""Creates a new Database
|
||||
---
|
||||
post:
|
||||
|
|
@ -281,6 +283,8 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
|
|||
return self.response_422(message=ex.normalized_messages())
|
||||
except DatabaseConnectionFailedError as ex:
|
||||
return self.response_422(message=str(ex))
|
||||
except SupersetErrorsException as ex:
|
||||
return json_errors_response(errors=ex.errors, status=ex.status)
|
||||
except DatabaseCreateFailedError as ex:
|
||||
logger.error(
|
||||
"Error creating model %s: %s",
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ from superset.databases.commands.exceptions import (
|
|||
)
|
||||
from superset.databases.commands.test_connection import TestConnectionDatabaseCommand
|
||||
from superset.databases.dao import DatabaseDAO
|
||||
from superset.exceptions import SupersetErrorsException
|
||||
from superset.extensions import db, event_logger, security_manager
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
|
@ -46,6 +47,13 @@ class CreateDatabaseCommand(BaseCommand):
|
|||
try:
|
||||
# Test connection before starting create transaction
|
||||
TestConnectionDatabaseCommand(self._properties).run()
|
||||
except SupersetErrorsException 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__}",
|
||||
|
|
|
|||
|
|
@ -29,13 +29,16 @@ from superset.commands.base import BaseCommand
|
|||
from superset.databases.commands.exceptions import (
|
||||
DatabaseSecurityUnsafeError,
|
||||
DatabaseTestConnectionDriverError,
|
||||
DatabaseTestConnectionFailedError,
|
||||
DatabaseTestConnectionUnexpectedError,
|
||||
)
|
||||
from superset.databases.dao import DatabaseDAO
|
||||
from superset.databases.utils import make_url_safe
|
||||
from superset.errors import ErrorLevel, SupersetErrorType
|
||||
from superset.exceptions import SupersetSecurityException, SupersetTimeoutException
|
||||
from superset.exceptions import (
|
||||
SupersetErrorsException,
|
||||
SupersetSecurityException,
|
||||
SupersetTimeoutException,
|
||||
)
|
||||
from superset.extensions import event_logger
|
||||
from superset.models.core import Database
|
||||
|
||||
|
|
@ -49,6 +52,7 @@ class TestConnectionDatabaseCommand(BaseCommand):
|
|||
|
||||
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 +121,13 @@ class TestConnectionDatabaseCommand(BaseCommand):
|
|||
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,7 +152,7 @@ class TestConnectionDatabaseCommand(BaseCommand):
|
|||
)
|
||||
# check for custom errors (wrong username, wrong password, etc)
|
||||
errors = database.db_engine_spec.extract_errors(ex, context)
|
||||
raise DatabaseTestConnectionFailedError(errors) from ex
|
||||
raise SupersetErrorsException(errors) from ex
|
||||
except SupersetSecurityException as ex:
|
||||
event_logger.log_with_context(
|
||||
action=f"test_connection_error.{ex.__class__.__name__}",
|
||||
|
|
|
|||
|
|
@ -46,8 +46,8 @@ if TYPE_CHECKING:
|
|||
|
||||
|
||||
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,
|
||||
{},
|
||||
|
|
|
|||
|
|
@ -525,11 +525,55 @@ class TestDatabaseApi(SupersetTestCase):
|
|||
self.login(username="admin")
|
||||
response = self.client.post(uri, json=database_data)
|
||||
response_data = json.loads(response.data.decode("utf-8"))
|
||||
expected_response = {
|
||||
"message": "Connection failed, please check your connection settings"
|
||||
superset_error_mysql = SupersetError(
|
||||
message='Either the username "superset" or the password is incorrect.',
|
||||
error_type="CONNECTION_ACCESS_DENIED_ERROR",
|
||||
level="error",
|
||||
extra={
|
||||
"engine_name": "MySQL",
|
||||
"invalid": ["username", "password"],
|
||||
"issue_codes": [
|
||||
{
|
||||
"code": 1014,
|
||||
"message": (
|
||||
"Issue 1014 - Either the username or the password is wrong."
|
||||
),
|
||||
},
|
||||
{
|
||||
"code": 1015,
|
||||
"message": (
|
||||
"Issue 1015 - Issue 1015 - Either the database is spelled incorrectly or does not exist."
|
||||
),
|
||||
},
|
||||
],
|
||||
},
|
||||
)
|
||||
superset_error_postgres = SupersetError(
|
||||
message='The password provided for username "superset" is incorrect.',
|
||||
error_type="CONNECTION_INVALID_PASSWORD_ERROR",
|
||||
level="error",
|
||||
extra={
|
||||
"engine_name": "PostgreSQL",
|
||||
"invalid": ["username", "password"],
|
||||
"issue_codes": [
|
||||
{
|
||||
"code": 1013,
|
||||
"message": (
|
||||
"Issue 1013 - The password provided when connecting to a database is not valid."
|
||||
),
|
||||
}
|
||||
],
|
||||
},
|
||||
)
|
||||
expected_response_mysql = {"errors": [dataclasses.asdict(superset_error_mysql)]}
|
||||
expected_response_postgres = {
|
||||
"errors": [dataclasses.asdict(superset_error_postgres)]
|
||||
}
|
||||
self.assertEqual(response.status_code, 422)
|
||||
self.assertEqual(response_data, expected_response)
|
||||
self.assertEqual(response.status_code, 500)
|
||||
if example_db.backend == "mysql":
|
||||
self.assertEqual(response_data, expected_response_mysql)
|
||||
else:
|
||||
self.assertEqual(response_data, expected_response_postgres)
|
||||
|
||||
def test_update_database(self):
|
||||
"""
|
||||
|
|
@ -1516,7 +1560,7 @@ class TestDatabaseApi(SupersetTestCase):
|
|||
url = "api/v1/database/test_connection/"
|
||||
rv = self.post_assert_metric(url, data, "test_connection")
|
||||
|
||||
assert rv.status_code == 422
|
||||
assert rv.status_code == 500
|
||||
assert rv.headers["Content-Type"] == "application/json; charset=utf-8"
|
||||
response = json.loads(rv.data.decode("utf-8"))
|
||||
expected_response = {"errors": [dataclasses.asdict(superset_error)]}
|
||||
|
|
|
|||
|
|
@ -32,7 +32,6 @@ from superset.databases.commands.exceptions import (
|
|||
DatabaseNotFoundError,
|
||||
DatabaseSecurityUnsafeError,
|
||||
DatabaseTestConnectionDriverError,
|
||||
DatabaseTestConnectionFailedError,
|
||||
DatabaseTestConnectionUnexpectedError,
|
||||
)
|
||||
from superset.databases.commands.export import ExportDatabasesCommand
|
||||
|
|
@ -683,7 +682,7 @@ class TestTestConnectionDatabaseCommand(SupersetTestCase):
|
|||
json_payload = {"sqlalchemy_uri": db_uri}
|
||||
command_without_db_name = TestConnectionDatabaseCommand(json_payload)
|
||||
|
||||
with pytest.raises(DatabaseTestConnectionFailedError) as excinfo:
|
||||
with pytest.raises(SupersetErrorsException) as excinfo:
|
||||
command_without_db_name.run()
|
||||
assert (
|
||||
excinfo.value.errors[0].error_type
|
||||
|
|
@ -757,7 +756,7 @@ class TestTestConnectionDatabaseCommand(SupersetTestCase):
|
|||
json_payload = {"sqlalchemy_uri": db_uri}
|
||||
command_without_db_name = TestConnectionDatabaseCommand(json_payload)
|
||||
|
||||
with pytest.raises(DatabaseTestConnectionFailedError) as excinfo:
|
||||
with pytest.raises(SupersetErrorsException) as excinfo:
|
||||
command_without_db_name.run()
|
||||
assert str(excinfo.value) == (
|
||||
"Connection failed, please check your connection settings"
|
||||
|
|
|
|||
|
|
@ -246,11 +246,11 @@ class TestBigQueryDbEngineSpec(TestDbEngineSpec):
|
|||
)
|
||||
|
||||
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={
|
||||
|
|
|
|||
Loading…
Reference in New Issue