feat: API endpoint to validate databases using separate parameters (#14420)

* feat: new endpoint for validating database parameters

* Rebase

* Remove broken tests
This commit is contained in:
Beto Dealmeida 2021-05-12 18:32:10 -07:00 committed by GitHub
parent f1c32b9576
commit 31f406a526
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
22 changed files with 812 additions and 42 deletions

View File

@ -183,3 +183,27 @@ The user doesn't have the proper permissions to connect to the database
```
We were unable to connect to your database. Please confirm that your service account has the Viewer and Job User roles on the project.
## Issue 1018
```
One or more parameters needed to configure a database are missing.
```
Not all parameters required to test, create, or edit a database were present. Please double check which parameters are needed, and that they are present.
## Issue 1019
```
The submitted payload has the incorrect format.
```
Please check that the request payload has the correct format (eg, JSON).
## Issue 1020
```
The submitted payload has the incorrect schema.
```
Please check that the request payload has the expected schema.

View File

@ -38,6 +38,7 @@ export const ErrorTypeEnum = {
CONNECTION_UNKNOWN_DATABASE_ERROR: 'CONNECTION_UNKNOWN_DATABASE_ERROR',
CONNECTION_DATABASE_PERMISSIONS_ERROR:
'CONNECTION_DATABASE_PERMISSIONS_ERROR',
CONNECTION_MISSING_PARAMETERS_ERRORS: 'CONNECTION_MISSING_PARAMETERS_ERRORS',
// Viz errors
VIZ_GET_DF_ERROR: 'VIZ_GET_DF_ERROR',
@ -60,6 +61,10 @@ export const ErrorTypeEnum = {
// Generic errors
GENERIC_COMMAND_ERROR: 'GENERIC_COMMAND_ERROR',
GENERIC_BACKEND_ERROR: 'GENERIC_BACKEND_ERROR',
// API errors
INVALID_PAYLOAD_FORMAT_ERROR: 'INVALID_PAYLOAD_FORMAT_ERROR',
INVALID_PAYLOAD_SCHEMA_ERROR: 'INVALID_PAYLOAD_SCHEMA_ERROR',
} as const;
type ValueOf<T> = T[keyof T];

View File

@ -630,7 +630,7 @@ DISPLAY_MAX_ROW = 10000
# Default row limit for SQL Lab queries. Is overridden by setting a new limit in
# the SQL Lab UI
DEFAULT_SQLLAB_LIMIT = 1000
DEFAULT_SQLLAB_LIMIT = 10000
# Maximum number of tables/views displayed in the dropdown window in SQL Lab.
MAX_TABLE_NAMES = 3000

View File

@ -106,6 +106,7 @@ MODEL_API_RW_METHOD_PERMISSION_MAP = {
"select_star": "read",
"table_metadata": "read",
"test_connection": "read",
"validate_parameters": "read",
"favorite_status": "read",
"thumbnail": "read",
"import_": "write",

View File

@ -47,6 +47,7 @@ from superset.databases.commands.export import ExportDatabasesCommand
from superset.databases.commands.importers.dispatcher import ImportDatabasesCommand
from superset.databases.commands.test_connection import TestConnectionDatabaseCommand
from superset.databases.commands.update import UpdateDatabaseCommand
from superset.databases.commands.validate import ValidateDatabaseParametersCommand
from superset.databases.dao import DatabaseDAO
from superset.databases.decorators import check_datasource_access
from superset.databases.filters import DatabaseFilter
@ -57,6 +58,7 @@ from superset.databases.schemas import (
DatabasePutSchema,
DatabaseRelatedObjectsResponse,
DatabaseTestConnectionSchema,
DatabaseValidateParametersSchema,
get_export_ids_schema,
SchemasResponseSchema,
SelectStarResponseSchema,
@ -65,6 +67,7 @@ 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.db_engine_specs.base import BaseParametersMixin
from superset.exceptions import InvalidPayloadFormatError, InvalidPayloadSchemaError
from superset.extensions import security_manager
from superset.models.core import Database
from superset.typing import FlaskResponse
@ -87,6 +90,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
"related_objects",
"function_names",
"available",
"validate_parameters",
}
resource_name = "database"
class_permission_name = "Database"
@ -176,6 +180,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
DatabaseFunctionNamesResponse,
DatabaseRelatedObjectsResponse,
DatabaseTestConnectionSchema,
DatabaseValidateParametersSchema,
TableMetadataResponseSchema,
SelectStarResponseSchema,
SchemasResponseSchema,
@ -914,3 +919,55 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
)
return self.response(200, databases=available_databases)
@expose("/validate_parameters", methods=["POST"])
@protect()
@statsd_metrics
@event_logger.log_this_with_context(
action=lambda self, *args, **kwargs: f"{self.__class__.__name__}"
f".validate_parameters",
log_to_statsd=False,
)
def validate_parameters( # pylint: disable=too-many-return-statements
self,
) -> FlaskResponse:
"""validates database connection parameters
---
post:
description: >-
Validates parameters used to connect to a database
requestBody:
description: DB-specific parameters
required: true
content:
application/json:
schema:
$ref: "#/components/schemas/DatabaseValidateParametersSchema"
responses:
200:
description: Database Test Connection
content:
application/json:
schema:
type: object
properties:
message:
type: string
400:
$ref: '#/components/responses/400'
422:
$ref: '#/components/responses/422'
500:
$ref: '#/components/responses/500'
"""
if not request.is_json:
raise InvalidPayloadFormatError("Request is not JSON")
try:
payload = DatabaseValidateParametersSchema().load(request.json)
except ValidationError as error:
raise InvalidPayloadSchemaError(error)
command = ValidateDatabaseParametersCommand(g.user, payload)
command.run()
return self.response(200, message="OK")

View File

@ -25,7 +25,7 @@ from superset.commands.exceptions import (
ImportFailedError,
UpdateFailedError,
)
from superset.exceptions import SupersetErrorsException
from superset.exceptions import SupersetErrorException, SupersetErrorsException
class DatabaseInvalidError(CommandInvalidError):
@ -137,3 +137,15 @@ class DatabaseTestConnectionUnexpectedError(SupersetErrorsException):
class DatabaseImportError(ImportFailedError):
message = _("Import database failed for an unknown reason")
class InvalidEngineError(SupersetErrorException):
status = 422
class DatabaseOfflineError(SupersetErrorException):
status = 422
class InvalidParametersError(SupersetErrorsException):
status = 422

View File

@ -0,0 +1,127 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from contextlib import closing
from typing import Any, Dict, Optional
from flask_appbuilder.security.sqla.models import User
from flask_babel import gettext as __
from sqlalchemy.engine.url import make_url
from superset.commands.base import BaseCommand
from superset.databases.commands.exceptions import (
DatabaseOfflineError,
DatabaseTestConnectionFailedError,
InvalidEngineError,
InvalidParametersError,
)
from superset.databases.dao import DatabaseDAO
from superset.db_engine_specs import get_engine_specs
from superset.db_engine_specs.base import BaseParametersMixin
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.models.core import Database
class ValidateDatabaseParametersCommand(BaseCommand):
def __init__(self, user: User, parameters: Dict[str, Any]):
self._actor = user
self._properties = parameters.copy()
self._model: Optional[Database] = None
def run(self) -> None:
engine = self._properties["engine"]
engine_specs = get_engine_specs()
if engine not in engine_specs:
raise InvalidEngineError(
SupersetError(
message=__(
'Engine "%(engine)s" is not a valid engine.', engine=engine,
),
error_type=SupersetErrorType.GENERIC_DB_ENGINE_ERROR,
level=ErrorLevel.ERROR,
extra={"allowed": list(engine_specs), "provided": engine},
),
)
engine_spec = engine_specs[engine]
if not issubclass(engine_spec, BaseParametersMixin):
raise InvalidEngineError(
SupersetError(
message=__(
'Engine "%(engine)s" cannot be configured through parameters.',
engine=engine,
),
error_type=SupersetErrorType.GENERIC_DB_ENGINE_ERROR,
level=ErrorLevel.ERROR,
extra={
"allowed": [
name
for name, engine_spec in engine_specs.items()
if issubclass(engine_spec, BaseParametersMixin)
],
"provided": engine,
},
),
)
# perform initial validation
errors = engine_spec.validate_parameters(self._properties["parameters"])
if errors:
raise InvalidParametersError(errors)
# try to connect
sqlalchemy_uri = engine_spec.build_sqlalchemy_uri(
self._properties["parameters"] # type: ignore
)
if self._model and sqlalchemy_uri == self._model.safe_sqlalchemy_uri():
sqlalchemy_uri = self._model.sqlalchemy_uri_decrypted
database = DatabaseDAO.build_db_for_connection_test(
server_cert=self._properties.get("server_cert", ""),
extra=self._properties.get("extra", "{}"),
impersonate_user=self._properties.get("impersonate_user", False),
encrypted_extra=self._properties.get("encrypted_extra", "{}"),
)
database.set_sqlalchemy_uri(sqlalchemy_uri)
database.db_engine_spec.mutate_db_for_connection_test(database)
username = self._actor.username if self._actor is not None else None
engine = database.get_sqla_engine(user_name=username)
try:
with closing(engine.raw_connection()) as conn:
alive = engine.dialect.do_ping(conn)
except Exception as ex: # pylint: disable=broad-except
url = make_url(sqlalchemy_uri)
context = {
"hostname": url.host,
"password": url.password,
"port": url.port,
"username": url.username,
"database": url.database,
}
errors = database.db_engine_spec.extract_errors(ex, context)
raise DatabaseTestConnectionFailedError(errors)
if not alive:
raise DatabaseOfflineError(
SupersetError(
message=__("Database is offline."),
error_type=SupersetErrorType.GENERIC_DB_ENGINE_ERROR,
level=ErrorLevel.ERROR,
),
)
def validate(self) -> None:
database_name = self._properties.get("database_name")
if database_name is not None:
self._model = DatabaseDAO.get_database_by_name(database_name)

View File

@ -213,9 +213,9 @@ class DatabaseParametersSchemaMixin:
"""
Allow SQLAlchemy URI to be passed as separate parameters.
This mixing is a first step in allowing the users to test, create and
This mixin is a first step in allowing the users to test, create and
edit databases without having to know how to write a SQLAlchemy URI.
Instead, each databases defines the parameters that it takes (eg,
Instead, each database defines the parameters that it takes (eg,
username, password, host, etc.) and the SQLAlchemy URI is built from
these parameters.
@ -223,7 +223,7 @@ class DatabaseParametersSchemaMixin:
"""
parameters = fields.Dict(
keys=fields.Str(),
keys=fields.String(),
values=fields.Raw(),
description="DB-specific parameters for configuration",
)
@ -270,10 +270,34 @@ class DatabaseParametersSchemaMixin:
]
)
data["sqlalchemy_uri"] = engine_spec.build_sqlalchemy_url(parameters)
data["sqlalchemy_uri"] = engine_spec.build_sqlalchemy_uri(parameters)
return data
class DatabaseValidateParametersSchema(Schema):
engine = fields.String(required=True, description="SQLAlchemy engine to use")
parameters = fields.Dict(
keys=fields.String(),
values=fields.Raw(),
description="DB-specific parameters for configuration",
)
database_name = fields.String(
description=database_name_description, allow_none=True, validate=Length(1, 250),
)
impersonate_user = fields.Boolean(description=impersonate_user_description)
extra = fields.String(description=extra_description, validate=extra_validator)
encrypted_extra = fields.String(
description=encrypted_extra_description,
validate=encrypted_extra_validator,
allow_none=True,
)
server_cert = fields.String(
description=server_cert_description,
allow_none=True,
validate=server_cert_validator,
)
class DatabasePostSchema(Schema, DatabaseParametersSchemaMixin):
class Meta: # pylint: disable=too-few-public-methods
unknown = EXCLUDE

View File

@ -63,6 +63,7 @@ from superset.sql_parse import ParsedQuery, Table
from superset.utils import core as utils
from superset.utils.core import ColumnSpec, GenericDataType
from superset.utils.hashing import md5_sha_from_str
from superset.utils.network import is_hostname_valid, is_port_open
if TYPE_CHECKING:
# prevent circular imports
@ -269,7 +270,9 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
max_column_name_length = 0
try_remove_schema_from_table_name = True # pylint: disable=invalid-name
run_multiple_statements_as_one = False
custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType]] = {}
custom_errors: Dict[
Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]
] = {}
@classmethod
def get_dbapi_exception_mapping(cls) -> Dict[Type[Exception], Type[Exception]]:
@ -727,16 +730,17 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
raw_message = cls._extract_error_message(ex)
context = context or {}
for regex, (message, error_type) in cls.custom_errors.items():
for regex, (message, error_type, extra) in cls.custom_errors.items():
match = regex.search(raw_message)
if match:
params = {**context, **match.groupdict()}
extra["engine_name"] = cls.engine_name
return [
SupersetError(
error_type=error_type,
message=message % params,
level=ErrorLevel.ERROR,
extra={"engine_name": cls.engine_name},
extra=extra,
)
]
@ -1274,13 +1278,13 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods
# schema for adding a database by providing parameters instead of the
# full SQLAlchemy URI
class BaseParametersSchema(Schema):
username = fields.String(allow_none=True, description=__("Username"))
username = fields.String(required=True, allow_none=True, description=__("Username"))
password = fields.String(allow_none=True, description=__("Password"))
host = fields.String(required=True, description=__("Hostname or IP address"))
port = fields.Integer(required=True, description=__("Database port"))
database = fields.String(required=True, description=__("Database name"))
query = fields.Dict(
keys=fields.Str(), values=fields.Raw(), description=__("Additinal parameters")
keys=fields.Str(), values=fields.Raw(), description=__("Additional parameters")
)
@ -1318,7 +1322,7 @@ class BaseParametersMixin:
)
@classmethod
def build_sqlalchemy_url(cls, parameters: BaseParametersType) -> str:
def build_sqlalchemy_uri(cls, parameters: BaseParametersType) -> str:
return str(
URL(
cls.drivername,
@ -1331,8 +1335,8 @@ class BaseParametersMixin:
)
)
@classmethod
def get_parameters_from_uri(cls, uri: str) -> BaseParametersType:
@staticmethod
def get_parameters_from_uri(uri: str) -> BaseParametersType:
url = make_url(uri)
return {
"username": url.username,
@ -1343,6 +1347,59 @@ class BaseParametersMixin:
"query": url.query,
}
@classmethod
def validate_parameters(cls, parameters: BaseParametersType) -> List[SupersetError]:
"""
Validates any number of parameters, for progressive validation.
If only the hostname is present it will check if the name is resolvable. As more
parameters are present in the request, more validation is done.
"""
errors: List[SupersetError] = []
required = {"host", "port", "username", "database"}
present = {key for key in parameters if parameters[key]} # type: ignore
missing = sorted(required - present)
if missing:
errors.append(
SupersetError(
message=f'One or more parameters are missing: {", ".join(missing)}',
error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR,
level=ErrorLevel.WARNING,
extra={"missing": missing},
),
)
host = parameters["host"]
if not host:
return errors
if not is_hostname_valid(host):
errors.append(
SupersetError(
message="The hostname provided can't be resolved.",
error_type=SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR,
level=ErrorLevel.ERROR,
extra={"invalid": ["host"]},
),
)
return errors
port = parameters["port"]
if not port:
return errors
if not is_port_open(host, port):
errors.append(
SupersetError(
message="The port is closed.",
error_type=SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR,
level=ErrorLevel.ERROR,
extra={"invalid": ["port"]},
),
)
return errors
@classmethod
def parameters_json_schema(cls) -> Any:
"""

View File

@ -16,7 +16,7 @@
# under the License.
import re
from datetime import datetime
from typing import Any, Dict, List, Optional, Tuple, TYPE_CHECKING
from typing import Any, Dict, List, Optional, Pattern, Tuple, TYPE_CHECKING
import pandas as pd
from flask_babel import gettext as __
@ -95,7 +95,7 @@ class BigQueryEngineSpec(BaseEngineSpec):
"P1Y": "{func}({col}, YEAR)",
}
custom_errors = {
custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]] = {
CONNECTION_DATABASE_PERMISSIONS_REGEX: (
__(
"We were unable to connect to your database. Please "
@ -103,6 +103,7 @@ class BigQueryEngineSpec(BaseEngineSpec):
"and Job User roles on the project."
),
SupersetErrorType.CONNECTION_DATABASE_PERMISSIONS_ERROR,
{},
),
}

View File

@ -20,3 +20,4 @@ from superset.db_engine_specs.postgres import PostgresEngineSpec
class CockroachDbEngineSpec(PostgresEngineSpec):
engine = "cockroachdb"
engine_name = "CockroachDB"
drivername = "cockroach"

View File

@ -17,7 +17,7 @@
import logging
import re
from datetime import datetime
from typing import Any, List, Optional, Tuple
from typing import Any, Dict, List, Optional, Pattern, Tuple
from flask_babel import gettext as __
@ -64,21 +64,24 @@ class MssqlEngineSpec(BaseEngineSpec):
"P1Y": "DATEADD(year, DATEDIFF(year, 0, {col}), 0)",
}
custom_errors = {
custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]] = {
CONNECTION_ACCESS_DENIED_REGEX: (
__(
'Either the username "%(username)s", password, '
'or database name "%(database)s" is incorrect.'
),
SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR,
{},
),
CONNECTION_INVALID_HOSTNAME_REGEX: (
__('The hostname "%(hostname)s" cannot be resolved.'),
SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR,
{},
),
CONNECTION_PORT_CLOSED_REGEX: (
__('Port %(port)s on hostname "%(hostname)s" refused the connection.'),
SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR,
{},
),
CONNECTION_HOST_DOWN_REGEX: (
__(
@ -86,6 +89,7 @@ class MssqlEngineSpec(BaseEngineSpec):
"reached on port %(port)s."
),
SupersetErrorType.CONNECTION_HOST_DOWN_ERROR,
{},
),
}

View File

@ -107,22 +107,26 @@ class MySQLEngineSpec(BaseEngineSpec):
type_code_map: Dict[int, str] = {} # loaded from get_datatype only if needed
custom_errors = {
custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]] = {
CONNECTION_ACCESS_DENIED_REGEX: (
__('Either the username "%(username)s" or the password is incorrect.'),
SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR,
{},
),
CONNECTION_INVALID_HOSTNAME_REGEX: (
__('Unknown MySQL server host "%(hostname)s".'),
SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR,
{},
),
CONNECTION_HOST_DOWN_REGEX: (
__('The host "%(hostname)s" might be down and can\'t be reached.'),
SupersetErrorType.CONNECTION_HOST_DOWN_ERROR,
{},
),
CONNECTION_UNKNOWN_DATABASE_REGEX: (
__('Unable to connect to database "%(database)s".'),
SupersetErrorType.CONNECTION_UNKNOWN_DATABASE_ERROR,
{},
),
}

View File

@ -104,22 +104,27 @@ class PostgresBaseEngineSpec(BaseEngineSpec):
CONNECTION_INVALID_USERNAME_REGEX: (
__('The username "%(username)s" does not exist.'),
SupersetErrorType.CONNECTION_INVALID_USERNAME_ERROR,
{"invalid": ["username"]},
),
CONNECTION_INVALID_PASSWORD_REGEX: (
__('The password provided for username "%(username)s" is incorrect.'),
SupersetErrorType.CONNECTION_INVALID_PASSWORD_ERROR,
{"invalid": ["username", "password"]},
),
CONNECTION_INVALID_PASSWORD_NEEDED_REGEX: (
__("Please re-enter the password."),
SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR,
{"invalid": ["password"]},
),
CONNECTION_INVALID_HOSTNAME_REGEX: (
__('The hostname "%(hostname)s" cannot be resolved.'),
SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR,
{"invalid": ["host"]},
),
CONNECTION_PORT_CLOSED_REGEX: (
__('Port %(port)s on hostname "%(hostname)s" refused the connection.'),
SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR,
{"invalid": ["host", "port"]},
),
CONNECTION_HOST_DOWN_REGEX: (
__(
@ -127,10 +132,12 @@ class PostgresBaseEngineSpec(BaseEngineSpec):
"reached on port %(port)s."
),
SupersetErrorType.CONNECTION_HOST_DOWN_ERROR,
{"invalid": ["host", "port"]},
),
CONNECTION_UNKNOWN_DATABASE_REGEX: (
__('Unable to connect to database "%(database)s".'),
SupersetErrorType.CONNECTION_UNKNOWN_DATABASE_ERROR,
{"invalid": ["database"]},
),
}

View File

@ -165,13 +165,14 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho
"date_add('day', 1, CAST({col} AS TIMESTAMP))))",
}
custom_errors = {
custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]] = {
COLUMN_DOES_NOT_EXIST_REGEX: (
__(
'We can\'t seem to resolve the column "%(column_name)s" at '
"line %(location)s.",
),
SupersetErrorType.COLUMN_DOES_NOT_EXIST_ERROR,
{},
),
TABLE_DOES_NOT_EXIST_REGEX: (
__(
@ -179,6 +180,7 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho
"A valid table must be used to run this query.",
),
SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR,
{},
),
SCHEMA_DOES_NOT_EXIST_REGEX: (
__(
@ -186,14 +188,17 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho
"A valid schema must be used to run this query.",
),
SupersetErrorType.SCHEMA_DOES_NOT_EXIST_ERROR,
{},
),
CONNECTION_ACCESS_DENIED_REGEX: (
__('Either the username "%(username)s" or the password is incorrect.'),
SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR,
{},
),
CONNECTION_INVALID_HOSTNAME_REGEX: (
__('The hostname "%(hostname)s" cannot be resolved.'),
SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR,
{},
),
CONNECTION_HOST_DOWN_REGEX: (
__(
@ -201,14 +206,17 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho
"reached on port %(port)s."
),
SupersetErrorType.CONNECTION_HOST_DOWN_ERROR,
{},
),
CONNECTION_PORT_CLOSED_REGEX: (
__('Port %(port)s on hostname "%(hostname)s" refused the connection.'),
SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR,
{},
),
CONNECTION_UNKNOWN_DATABASE_ERROR: (
__('Unable to connect to catalog named "%(catalog_name)s".'),
SupersetErrorType.CONNECTION_UNKNOWN_DATABASE_ERROR,
{},
),
}

View File

@ -15,6 +15,7 @@
# specific language governing permissions and limitations
# under the License.
import re
from typing import Any, Dict, Pattern, Tuple
from flask_babel import gettext as __
@ -49,18 +50,21 @@ class RedshiftEngineSpec(PostgresBaseEngineSpec):
engine_name = "Amazon Redshift"
max_column_name_length = 127
custom_errors = {
custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]] = {
CONNECTION_ACCESS_DENIED_REGEX: (
__('Either the username "%(username)s" or the password is incorrect.'),
SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR,
{},
),
CONNECTION_INVALID_HOSTNAME_REGEX: (
__('The hostname "%(hostname)s" cannot be resolved.'),
SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR,
{},
),
CONNECTION_PORT_CLOSED_REGEX: (
__('Port %(port)s on hostname "%(hostname)s" refused the connection.'),
SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR,
{},
),
CONNECTION_HOST_DOWN_REGEX: (
__(
@ -68,6 +72,7 @@ class RedshiftEngineSpec(PostgresBaseEngineSpec):
"reached on port %(port)s."
),
SupersetErrorType.CONNECTION_HOST_DOWN_ERROR,
{},
),
CONNECTION_UNKNOWN_DATABASE_REGEX: (
__(
@ -75,6 +80,7 @@ class RedshiftEngineSpec(PostgresBaseEngineSpec):
" Please verify your database name and try again."
),
SupersetErrorType.CONNECTION_UNKNOWN_DATABASE_ERROR,
{},
),
}

View File

@ -48,6 +48,7 @@ class SupersetErrorType(str, Enum):
CONNECTION_ACCESS_DENIED_ERROR = "CONNECTION_ACCESS_DENIED_ERROR"
CONNECTION_UNKNOWN_DATABASE_ERROR = "CONNECTION_UNKNOWN_DATABASE_ERROR"
CONNECTION_DATABASE_PERMISSIONS_ERROR = "CONNECTION_DATABASE_PERMISSIONS_ERROR"
CONNECTION_MISSING_PARAMETERS_ERROR = "CONNECTION_MISSING_PARAMETERS_ERROR"
# Viz errors
VIZ_GET_DF_ERROR = "VIZ_GET_DF_ERROR"
@ -70,6 +71,10 @@ class SupersetErrorType(str, Enum):
GENERIC_COMMAND_ERROR = "GENERIC_COMMAND_ERROR"
GENERIC_BACKEND_ERROR = "GENERIC_BACKEND_ERROR"
# API errors
INVALID_PAYLOAD_FORMAT_ERROR = "INVALID_PAYLOAD_FORMAT_ERROR"
INVALID_PAYLOAD_SCHEMA_ERROR = "INVALID_PAYLOAD_SCHEMA_ERROR"
ERROR_TYPES_TO_ISSUE_CODES_MAPPING = {
SupersetErrorType.BACKEND_TIMEOUT_ERROR: [
@ -220,6 +225,31 @@ ERROR_TYPES_TO_ISSUE_CODES_MAPPING = {
"message": _("Issue 1017 - User doesn't have the proper permissions."),
},
],
SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR: [
{
"code": 1018,
"message": _(
"Issue 1018 - One or more parameters needed to configure a "
"database are missing."
),
},
],
SupersetErrorType.INVALID_PAYLOAD_FORMAT_ERROR: [
{
"code": 1019,
"message": _(
"Issue 1019 - The submitted payload has the incorrect format."
),
}
],
SupersetErrorType.INVALID_PAYLOAD_SCHEMA_ERROR: [
{
"code": 1020,
"message": _(
"Issue 1020 - The submitted payload has the incorrect schema."
),
}
],
}

View File

@ -17,6 +17,7 @@
from typing import Any, Dict, List, Optional
from flask_babel import gettext as _
from marshmallow import ValidationError
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
@ -155,3 +156,29 @@ class DashboardImportException(SupersetException):
class SerializationError(SupersetException):
pass
class InvalidPayloadFormatError(SupersetErrorException):
status = 400
def __init__(self, message: str = "Request payload has incorrect format"):
error = SupersetError(
message=message,
error_type=SupersetErrorType.INVALID_PAYLOAD_FORMAT_ERROR,
level=ErrorLevel.ERROR,
extra={},
)
super().__init__(error)
class InvalidPayloadSchemaError(SupersetErrorException):
status = 422
def __init__(self, error: ValidationError):
error = SupersetError(
message="An error happened when validating the request",
error_type=SupersetErrorType.INVALID_PAYLOAD_SCHEMA_ERROR,
level=ErrorLevel.ERROR,
extra={"messages": error.messages},
)
super().__init__(error)

View File

@ -1290,7 +1290,7 @@ class TestDatabaseApi(SupersetTestCase):
},
"query": {
"additionalProperties": {},
"description": "Additinal parameters",
"description": "Additional parameters",
"type": "object",
},
"username": {
@ -1299,7 +1299,7 @@ class TestDatabaseApi(SupersetTestCase):
"type": "string",
},
},
"required": ["database", "host", "port"],
"required": ["database", "host", "port", "username"],
"type": "object",
},
"preferred": True,
@ -1308,3 +1308,149 @@ class TestDatabaseApi(SupersetTestCase):
{"engine": "mysql", "name": "MySQL", "preferred": False},
]
}
def test_validate_parameters_invalid_payload_format(self):
self.login(username="admin")
url = "api/v1/database/validate_parameters"
rv = self.client.post(url, data="INVALID", content_type="text/plain")
response = json.loads(rv.data.decode("utf-8"))
assert rv.status_code == 400
assert response == {
"errors": [
{
"message": "Request is not JSON",
"error_type": "INVALID_PAYLOAD_FORMAT_ERROR",
"level": "error",
"extra": {
"issue_codes": [
{
"code": 1019,
"message": "Issue 1019 - The submitted payload has the incorrect format.",
}
]
},
}
]
}
def test_validate_parameters_invalid_payload_schema(self):
self.login(username="admin")
url = "api/v1/database/validate_parameters"
payload = {"foo": "bar"}
rv = self.client.post(url, json=payload)
response = json.loads(rv.data.decode("utf-8"))
assert rv.status_code == 422
assert response == {
"errors": [
{
"message": "An error happened when validating the request",
"error_type": "INVALID_PAYLOAD_SCHEMA_ERROR",
"level": "error",
"extra": {
"messages": {
"engine": ["Missing data for required field."],
"foo": ["Unknown field."],
},
"issue_codes": [
{
"code": 1020,
"message": "Issue 1020 - The submitted payload has the incorrect schema.",
}
],
},
}
]
}
def test_validate_parameters_missing_fields(self):
self.login(username="admin")
url = "api/v1/database/validate_parameters"
payload = {
"engine": "postgresql",
"parameters": {
"host": "",
"port": 5432,
"username": "",
"password": "",
"database": "",
"query": {},
},
}
rv = self.client.post(url, json=payload)
response = json.loads(rv.data.decode("utf-8"))
assert rv.status_code == 422
assert response == {
"errors": [
{
"message": "One or more parameters are missing: database, host, username",
"error_type": "CONNECTION_MISSING_PARAMETERS_ERROR",
"level": "warning",
"extra": {
"missing": ["database", "host", "username"],
"issue_codes": [
{
"code": 1018,
"message": "Issue 1018 - One or more parameters needed to configure a database are missing.",
}
],
},
}
]
}
@mock.patch("superset.db_engine_specs.base.is_hostname_valid")
def test_validate_parameters_invalid_host(self, is_hostname_valid):
is_hostname_valid.return_value = False
self.login(username="admin")
url = "api/v1/database/validate_parameters"
payload = {
"engine": "postgresql",
"parameters": {
"host": "localhost",
"port": 5432,
"username": "",
"password": "",
"database": "",
"query": {},
},
}
rv = self.client.post(url, json=payload)
response = json.loads(rv.data.decode("utf-8"))
assert rv.status_code == 422
assert response == {
"errors": [
{
"message": "One or more parameters are missing: database, username",
"error_type": "CONNECTION_MISSING_PARAMETERS_ERROR",
"level": "warning",
"extra": {
"missing": ["database", "username"],
"issue_codes": [
{
"code": 1018,
"message": "Issue 1018 - One or more parameters needed to configure a database are missing.",
}
],
},
},
{
"message": "The hostname provided can't be resolved.",
"error_type": "CONNECTION_INVALID_HOSTNAME_ERROR",
"level": "error",
"extra": {
"invalid": ["host"],
"issue_codes": [
{
"code": 1007,
"message": "Issue 1007 - The hostname provided can't be resolved.",
}
],
},
},
]
}

View File

@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.
# pylint: disable=no-self-use, invalid-name
from unittest import mock
from unittest import mock, skip
from unittest.mock import patch
import pytest
@ -36,9 +36,10 @@ from superset.databases.commands.exceptions import (
from superset.databases.commands.export import ExportDatabasesCommand
from superset.databases.commands.importers.v1 import ImportDatabasesCommand
from superset.databases.commands.test_connection import TestConnectionDatabaseCommand
from superset.databases.commands.validate import ValidateDatabaseParametersCommand
from superset.databases.schemas import DatabaseTestConnectionSchema
from superset.errors import SupersetError, SupersetErrorType
from superset.exceptions import SupersetSecurityException
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import SupersetErrorsException, SupersetSecurityException
from superset.models.core import Database
from superset.utils.core import backend, get_example_database
from tests.base_tests import SupersetTestCase
@ -53,6 +54,7 @@ from tests.fixtures.importexport import (
class TestExportDatabasesCommand(SupersetTestCase):
@skip("Flaky")
@patch("superset.security.manager.g")
@pytest.mark.usefixtures(
"load_birth_names_dashboard_with_slices", "load_energy_table_with_slice"
@ -620,3 +622,122 @@ class TestTestConnectionDatabaseCommand(SupersetTestCase):
)
mock_event_logger.assert_called()
@mock.patch("superset.db_engine_specs.base.is_hostname_valid")
@mock.patch("superset.db_engine_specs.base.is_port_open")
@mock.patch("superset.databases.commands.validate.DatabaseDAO")
def test_validate(DatabaseDAO, is_port_open, is_hostname_valid, app_context):
"""
Test parameter validation.
"""
is_hostname_valid.return_value = True
is_port_open.return_value = True
payload = {
"engine": "postgresql",
"parameters": {
"host": "localhost",
"port": 5432,
"username": "superset",
"password": "superset",
"database": "test",
"query": {},
},
}
command = ValidateDatabaseParametersCommand(None, payload)
command.run()
@mock.patch("superset.db_engine_specs.base.is_hostname_valid")
@mock.patch("superset.db_engine_specs.base.is_port_open")
def test_validate_partial(is_port_open, is_hostname_valid, app_context):
"""
Test parameter validation when only some parameters are present.
"""
is_hostname_valid.return_value = True
is_port_open.return_value = True
payload = {
"engine": "postgresql",
"parameters": {
"host": "localhost",
"port": 5432,
"username": "",
"password": "superset",
"database": "test",
"query": {},
},
}
command = ValidateDatabaseParametersCommand(None, payload)
with pytest.raises(SupersetErrorsException) as excinfo:
command.run()
assert excinfo.value.errors == [
SupersetError(
message="One or more parameters are missing: username",
error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR,
level=ErrorLevel.WARNING,
extra={
"missing": ["username"],
"issue_codes": [
{
"code": 1018,
"message": "Issue 1018 - One or more parameters needed to configure a database are missing.",
}
],
},
)
]
@mock.patch("superset.db_engine_specs.base.is_hostname_valid")
def test_validate_partial_invalid_hostname(is_hostname_valid, app_context):
"""
Test parameter validation when only some parameters are present.
"""
is_hostname_valid.return_value = False
payload = {
"engine": "postgresql",
"parameters": {
"host": "localhost",
"port": None,
"username": "",
"password": "",
"database": "",
"query": {},
},
}
command = ValidateDatabaseParametersCommand(None, payload)
with pytest.raises(SupersetErrorsException) as excinfo:
command.run()
assert excinfo.value.errors == [
SupersetError(
message="One or more parameters are missing: database, port, username",
error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR,
level=ErrorLevel.WARNING,
extra={
"missing": ["database", "port", "username"],
"issue_codes": [
{
"code": 1018,
"message": "Issue 1018 - One or more parameters needed to configure a database are missing.",
}
],
},
),
SupersetError(
message="The hostname provided can't be resolved.",
error_type=SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR,
level=ErrorLevel.ERROR,
extra={
"invalid": ["host"],
"issue_codes": [
{
"code": 1007,
"message": "Issue 1007 - The hostname provided can't be resolved.",
}
],
},
),
]

View File

@ -22,11 +22,13 @@ import pytest
from superset.db_engine_specs import get_engine_specs
from superset.db_engine_specs.base import (
BaseEngineSpec,
BaseParametersMixin,
builtin_time_grains,
LimitMethod,
)
from superset.db_engine_specs.mysql import MySQLEngineSpec
from superset.db_engine_specs.sqlite import SqliteEngineSpec
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.sql_parse import ParsedQuery
from superset.utils.core import get_example_database
from tests.db_engine_specs.base_tests import TestDbEngineSpec
@ -365,3 +367,102 @@ def test_get_time_grain_with_unkown_values():
assert list(time_grains)[-1] == "weird"
app.config = config
@mock.patch("superset.db_engine_specs.base.is_hostname_valid")
@mock.patch("superset.db_engine_specs.base.is_port_open")
def test_validate(is_port_open, is_hostname_valid):
is_hostname_valid.return_value = True
is_port_open.return_value = True
parameters = {
"host": "localhost",
"port": 5432,
"username": "username",
"password": "password",
"database": "dbname",
"query": {"sslmode": "verify-full"},
}
errors = BaseParametersMixin.validate_parameters(parameters)
assert errors == []
def test_validate_parameters_missing():
parameters = {
"host": "",
"port": None,
"username": "",
"password": "",
"database": "",
"query": {},
}
errors = BaseParametersMixin.validate_parameters(parameters)
assert errors == [
SupersetError(
message=(
"One or more parameters are missing: " "database, host, port, username"
),
error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR,
level=ErrorLevel.WARNING,
extra={"missing": ["database", "host", "port", "username"]},
),
]
@mock.patch("superset.db_engine_specs.base.is_hostname_valid")
def test_validate_parameters_invalid_host(is_hostname_valid):
is_hostname_valid.return_value = False
parameters = {
"host": "localhost",
"port": None,
"username": "username",
"password": "password",
"database": "dbname",
"query": {"sslmode": "verify-full"},
}
errors = BaseParametersMixin.validate_parameters(parameters)
assert errors == [
SupersetError(
message="One or more parameters are missing: port",
error_type=SupersetErrorType.CONNECTION_MISSING_PARAMETERS_ERROR,
level=ErrorLevel.WARNING,
extra={"missing": ["port"]},
),
SupersetError(
message="The hostname provided can't be resolved.",
error_type=SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR,
level=ErrorLevel.ERROR,
extra={"invalid": ["host"]},
),
]
@mock.patch("superset.db_engine_specs.base.is_hostname_valid")
@mock.patch("superset.db_engine_specs.base.is_port_open")
def test_validate_parameters_port_closed(is_port_open, is_hostname_valid):
is_hostname_valid.return_value = True
is_port_open.return_value = False
parameters = {
"host": "localhost",
"port": 5432,
"username": "username",
"password": "password",
"database": "dbname",
"query": {"sslmode": "verify-full"},
}
errors = BaseParametersMixin.validate_parameters(parameters)
assert errors == [
SupersetError(
message="The port is closed.",
error_type=SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR,
level=ErrorLevel.ERROR,
extra={
"invalid": ["port"],
"issue_codes": [
{"code": 1008, "message": "Issue 1008 - The port is closed."}
],
},
)
]

View File

@ -23,6 +23,7 @@ from sqlalchemy.dialects import postgresql
from superset.db_engine_specs import get_engine_specs
from superset.db_engine_specs.postgres import PostgresEngineSpec
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.utils.core import get_example_database
from tests.db_engine_specs.base_tests import TestDbEngineSpec
from tests.fixtures.certificates import ssl_certificate
from tests.fixtures.database import default_db_extra
@ -234,6 +235,7 @@ class TestPostgresDbEngineSpec(TestDbEngineSpec):
),
},
],
"invalid": ["username"],
},
)
]
@ -257,6 +259,7 @@ class TestPostgresDbEngineSpec(TestDbEngineSpec):
"can't be resolved.",
}
],
"invalid": ["host"],
},
)
]
@ -282,6 +285,7 @@ could not connect to server: Connection refused
"issue_codes": [
{"code": 1008, "message": "Issue 1008 - The port is closed."}
],
"invalid": ["host", "port"],
},
)
]
@ -311,6 +315,7 @@ psql: error: could not connect to server: Operation timed out
"and can't be reached on the provided port.",
}
],
"invalid": ["host", "port"],
},
)
]
@ -341,6 +346,7 @@ psql: error: could not connect to server: Operation timed out
"and can't be reached on the provided port.",
}
],
"invalid": ["host", "port"],
},
)
]
@ -363,6 +369,7 @@ psql: error: could not connect to server: Operation timed out
),
},
],
"invalid": ["username", "password"],
},
)
]
@ -385,6 +392,7 @@ psql: error: could not connect to server: Operation timed out
),
}
],
"invalid": ["database"],
},
)
]
@ -393,21 +401,20 @@ psql: error: could not connect to server: Operation timed out
result = PostgresEngineSpec.extract_errors(Exception(msg))
assert result == [
SupersetError(
error_type=SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR,
message="Please re-enter the password.",
error_type=SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR,
level=ErrorLevel.ERROR,
extra={
"invalid": ["password"],
"engine_name": "PostgreSQL",
"issue_codes": [
{
"code": 1014,
"message": "Issue 1014 - Either the"
" username or the password is wrong.",
"message": "Issue 1014 - Either the username or the password is wrong.",
},
{
"code": 1015,
"message": "Issue 1015 - Either the database is "
"spelled incorrectly or does not exist.",
"message": "Issue 1015 - Either the database is spelled incorrectly or does not exist.",
},
],
},
@ -424,7 +431,7 @@ def test_base_parameters_mixin():
"database": "dbname",
"query": {"foo": "bar"},
}
sqlalchemy_uri = PostgresEngineSpec.build_sqlalchemy_url(parameters)
sqlalchemy_uri = PostgresEngineSpec.build_sqlalchemy_uri(parameters)
assert (
sqlalchemy_uri
== "postgresql+psycopg2://username:password@localhost:5432/dbname?foo=bar"
@ -437,20 +444,20 @@ def test_base_parameters_mixin():
assert json_schema == {
"type": "object",
"properties": {
"host": {"type": "string", "description": "Hostname or IP address"},
"username": {"type": "string", "nullable": True, "description": "Username"},
"password": {"type": "string", "nullable": True, "description": "Password"},
"database": {"type": "string", "description": "Database name"},
"query": {
"type": "object",
"description": "Additinal parameters",
"additionalProperties": {},
},
"port": {
"type": "integer",
"format": "int32",
"description": "Database port",
},
"password": {"type": "string", "nullable": True, "description": "Password"},
"host": {"type": "string", "description": "Hostname or IP address"},
"username": {"type": "string", "nullable": True, "description": "Username"},
"query": {
"type": "object",
"description": "Additional parameters",
"additionalProperties": {},
},
"database": {"type": "string", "description": "Database name"},
},
"required": ["database", "host", "port"],
"required": ["database", "host", "port", "username"],
}