Skip to content

Commit

Permalink
feat: API endpoint to validate databases using separate parameters (a…
Browse files Browse the repository at this point in the history
…pache#14420)

* feat: new endpoint for validating database parameters

* Rebase

* Remove broken tests
  • Loading branch information
betodealmeida committed May 13, 2021
1 parent bd06c32 commit ae953d4
Show file tree
Hide file tree
Showing 22 changed files with 811 additions and 41 deletions.
24 changes: 24 additions & 0 deletions docs/src/pages/docs/Miscellaneous/issue_codes.mdx
Expand Up @@ -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.
5 changes: 5 additions & 0 deletions superset-frontend/src/components/ErrorMessage/types.ts
Expand Up @@ -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',
Expand All @@ -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];
Expand Down
2 changes: 1 addition & 1 deletion superset/config.py
Expand Up @@ -630,7 +630,7 @@ def _try_json_readsha( # pylint: disable=unused-argument

# 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
Expand Down
1 change: 1 addition & 0 deletions superset/constants.py
Expand Up @@ -106,6 +106,7 @@ class RouteMethod: # pylint: disable=too-few-public-methods
"select_star": "read",
"table_metadata": "read",
"test_connection": "read",
"validate_parameters": "read",
"favorite_status": "read",
"thumbnail": "read",
"import_": "write",
Expand Down
57 changes: 57 additions & 0 deletions superset/databases/api.py
Expand Up @@ -47,6 +47,7 @@
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
Expand All @@ -57,6 +58,7 @@
DatabasePutSchema,
DatabaseRelatedObjectsResponse,
DatabaseTestConnectionSchema,
DatabaseValidateParametersSchema,
get_export_ids_schema,
SchemasResponseSchema,
SelectStarResponseSchema,
Expand All @@ -65,6 +67,7 @@
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
Expand All @@ -87,6 +90,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
"related_objects",
"function_names",
"available",
"validate_parameters",
}
resource_name = "database"
class_permission_name = "Database"
Expand Down Expand Up @@ -176,6 +180,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
DatabaseFunctionNamesResponse,
DatabaseRelatedObjectsResponse,
DatabaseTestConnectionSchema,
DatabaseValidateParametersSchema,
TableMetadataResponseSchema,
SelectStarResponseSchema,
SchemasResponseSchema,
Expand Down Expand Up @@ -914,3 +919,55 @@ def available(self) -> Response:
)

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")
14 changes: 13 additions & 1 deletion superset/databases/commands/exceptions.py
Expand Up @@ -25,7 +25,7 @@
ImportFailedError,
UpdateFailedError,
)
from superset.exceptions import SupersetErrorsException
from superset.exceptions import SupersetErrorException, SupersetErrorsException


class DatabaseInvalidError(CommandInvalidError):
Expand Down Expand Up @@ -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
127 changes: 127 additions & 0 deletions superset/databases/commands/validate.py
@@ -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)
32 changes: 28 additions & 4 deletions superset/databases/schemas.py
Expand Up @@ -213,17 +213,17 @@ 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.
When using this mixin make sure that `sqlalchemy_uri` is not required.
"""

parameters = fields.Dict(
keys=fields.Str(),
keys=fields.String(),
values=fields.Raw(),
description="DB-specific parameters for configuration",
)
Expand Down Expand Up @@ -270,10 +270,34 @@ def build_sqlalchemy_uri(
]
)

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
Expand Down

0 comments on commit ae953d4

Please sign in to comment.