From 7b370b16c252d957a5f33b0ebda5dc503f25857c Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 17 Jun 2021 12:25:10 -0700 Subject: [PATCH 1/3] fix: do not use Marshmallow validation in partial params validation --- docs/src/pages/docs/Miscellaneous/issue_codes.mdx | 8 ++++++++ .../src/components/ErrorMessage/types.ts | 1 + superset/databases/schemas.py | 11 ----------- superset/db_engine_specs/base.py | 11 ++++++++++- superset/errors.py | 4 ++++ tests/databases/api_tests.py | 10 +++++----- 6 files changed, 28 insertions(+), 17 deletions(-) diff --git a/docs/src/pages/docs/Miscellaneous/issue_codes.mdx b/docs/src/pages/docs/Miscellaneous/issue_codes.mdx index ad26c70a798e9..fd098312cc4a4 100644 --- a/docs/src/pages/docs/Miscellaneous/issue_codes.mdx +++ b/docs/src/pages/docs/Miscellaneous/issue_codes.mdx @@ -207,3 +207,11 @@ The submitted payload has the incorrect schema. ``` Please check that the request payload has the expected schema. + +## Issue 1021 + +``` +The database port provided is invalid. +``` + +Please check that the provided database port is an integer between 0 and 65535 (inclusive). diff --git a/superset-frontend/src/components/ErrorMessage/types.ts b/superset-frontend/src/components/ErrorMessage/types.ts index 5c345e04db24f..d6669da9013de 100644 --- a/superset-frontend/src/components/ErrorMessage/types.ts +++ b/superset-frontend/src/components/ErrorMessage/types.ts @@ -33,6 +33,7 @@ export const ErrorTypeEnum = { CONNECTION_INVALID_PASSWORD_ERROR: 'CONNECTION_INVALID_PASSWORD_ERROR', CONNECTION_INVALID_HOSTNAME_ERROR: 'CONNECTION_INVALID_HOSTNAME_ERROR', CONNECTION_PORT_CLOSED_ERROR: 'CONNECTION_PORT_CLOSED_ERROR', + CONNECTION_INVALID_PORT_ERROR: 'CONNECTION_INVALID_PORT_ERROR', CONNECTION_HOST_DOWN_ERROR: 'CONNECTION_HOST_DOWN_ERROR', CONNECTION_ACCESS_DENIED_ERROR: 'CONNECTION_ACCESS_DENIED_ERROR', CONNECTION_UNKNOWN_DATABASE_ERROR: 'CONNECTION_UNKNOWN_DATABASE_ERROR', diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py index 93da007ba86f9..f0509d50c701d 100644 --- a/superset/databases/schemas.py +++ b/superset/databases/schemas.py @@ -346,17 +346,6 @@ class Meta: # pylint: disable=too-few-public-methods description=configuration_method_description, ) - @validates_schema - def validate_parameters( # pylint: disable=no-self-use - self, data: Dict[str, Any], **kwargs: Any # pylint: disable=unused-argument - ) -> None: - """ - Validate the DB engine spec specific parameters schema. - """ - # TODO (aafghahi): Move this onCreate instead of validation - engine_spec = get_engine_spec(data.get("engine") or data.get("backend")) - engine_spec.parameters_schema.load(data["parameters"]) # type: ignore - class DatabasePostSchema(Schema, DatabaseParametersSchemaMixin): class Meta: # pylint: disable=too-few-public-methods diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index 3d1898d11a370..010f71e690ffb 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -1445,7 +1445,16 @@ def validate_parameters( port = parameters.get("port", None) if not port: return errors - if not is_port_open(host, port): + if not (isinstance(port, int) and 0 <= port < 2 ** 16): + errors.append( + SupersetError( + message="The port must be an integer between 0 and 65535 (inclusive).", + error_type=SupersetErrorType.CONNECTION_INVALID_PORT_ERROR, + level=ErrorLevel.ERROR, + extra={"invalid": ["port"]}, + ), + ) + elif not is_port_open(host, port): errors.append( SupersetError( message="The port is closed.", diff --git a/superset/errors.py b/superset/errors.py index 961cca444c982..9b40d71e70f9d 100644 --- a/superset/errors.py +++ b/superset/errors.py @@ -44,6 +44,7 @@ class SupersetErrorType(str, Enum): CONNECTION_INVALID_PASSWORD_ERROR = "CONNECTION_INVALID_PASSWORD_ERROR" CONNECTION_INVALID_HOSTNAME_ERROR = "CONNECTION_INVALID_HOSTNAME_ERROR" CONNECTION_PORT_CLOSED_ERROR = "CONNECTION_PORT_CLOSED_ERROR" + CONNECTION_INVALID_PORT_ERROR = "CONNECTION_INVALID_PORT_ERROR" CONNECTION_HOST_DOWN_ERROR = "CONNECTION_HOST_DOWN_ERROR" CONNECTION_ACCESS_DENIED_ERROR = "CONNECTION_ACCESS_DENIED_ERROR" CONNECTION_UNKNOWN_DATABASE_ERROR = "CONNECTION_UNKNOWN_DATABASE_ERROR" @@ -156,6 +157,9 @@ class SupersetErrorType(str, Enum): SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR: [ {"code": 1008, "message": _("Issue 1008 - The port is closed.")}, ], + SupersetErrorType.CONNECTION_INVALID_PORT_ERROR: [ + {"code": 1021, "message": _("Issue 1021 - Port number is invalid.")}, + ], SupersetErrorType.CONNECTION_HOST_DOWN_ERROR: [ { "code": 1009, diff --git a/tests/databases/api_tests.py b/tests/databases/api_tests.py index bf8feb44fb82f..5ca5eee26a189 100644 --- a/tests/databases/api_tests.py +++ b/tests/databases/api_tests.py @@ -1807,18 +1807,18 @@ def test_validate_parameters_invalid_port(self): assert response == { "errors": [ { - "message": "Not a valid integer.", - "error_type": "INVALID_PAYLOAD_SCHEMA_ERROR", - "level": "error", + "error_type": "CONNECTION_INVALID_PORT_ERROR", "extra": { "invalid": ["port"], "issue_codes": [ { - "code": 1020, - "message": "Issue 1020 - The submitted payload has the incorrect schema.", + "code": 1021, + "message": "Issue 1021 - Port number is invalid.", } ], }, + "level": "error", + "message": "The port must be an integer between 0 and 65535 (inclusive).", } ] } From a6f283a0496900d1f3f955246482dd3d260f1669 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 17 Jun 2021 13:45:46 -0700 Subject: [PATCH 2/3] Fix lint --- superset/db_engine_specs/base.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index 010f71e690ffb..fe75a70392c6f 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -1448,7 +1448,10 @@ def validate_parameters( if not (isinstance(port, int) and 0 <= port < 2 ** 16): errors.append( SupersetError( - message="The port must be an integer between 0 and 65535 (inclusive).", + message=( + "The port must be an integer between 0 and 65535 " + "(inclusive)." + ), error_type=SupersetErrorType.CONNECTION_INVALID_PORT_ERROR, level=ErrorLevel.ERROR, extra={"invalid": ["port"]}, From 71aeb1863e3bd3eebe0ffbc9890da4d5037a30c3 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Thu, 17 Jun 2021 14:12:45 -0700 Subject: [PATCH 3/3] Update test --- tests/databases/api_tests.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/tests/databases/api_tests.py b/tests/databases/api_tests.py index 5ca5eee26a189..d091264d7ea66 100644 --- a/tests/databases/api_tests.py +++ b/tests/databases/api_tests.py @@ -1909,18 +1909,32 @@ def test_validate_parameters_invalid_port_range(self, is_hostname_valid): assert response == { "errors": [ { - "message": "Must be greater than or equal to 0 and less than 65536.", - "error_type": "INVALID_PAYLOAD_SCHEMA_ERROR", + "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 port must be an integer between 0 and 65535 (inclusive).", + "error_type": "CONNECTION_INVALID_PORT_ERROR", "level": "error", "extra": { "invalid": ["port"], "issue_codes": [ { - "code": 1020, - "message": "Issue 1020 - The submitted payload has the incorrect schema.", + "code": 1021, + "message": "Issue 1021 - Port number is invalid.", } ], }, - } + }, ] }