Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: split database information #24067

Merged
merged 4 commits into from
May 22, 2023
Merged

feat: split database information #24067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fe7ccec
feat: split database information
betodealmeida May 11, 2023
f4ee6e4
Fix unit test
betodealmeida May 18, 2023
f5cd423
Add OpenAPI spec
betodealmeida May 18, 2023
cae6057
Address comments
betodealmeida May 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions superset-frontend/src/features/databases/DatabaseModal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
'database',
t('database'),
addDangerToast,
'connection',
);

const [tabKey, setTabKey] = useState<string>(DEFAULT_TAB_KEY);
Expand Down
6 changes: 5 additions & 1 deletion superset-frontend/src/views/CRUD/hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ export function useSingleViewResource<D extends object = any>(
resourceName: string,
resourceLabel: string, // resourceLabel for translations
handleErrorMsg: (errorMsg: string) => void,
path_suffix = '',
) {
const [state, setState] = useState<SingleViewResourceState<D>>({
loading: false,
Expand All @@ -242,8 +243,11 @@ export function useSingleViewResource<D extends object = any>(
loading: true,
});

const baseEndpoint = `/api/v1/${resourceName}/${resourceID}`;
const endpoint =
path_suffix !== '' ? `${baseEndpoint}/${path_suffix}` : baseEndpoint;
return SupersetClient.get({
endpoint: `/api/v1/${resourceName}/${resourceID}`,
endpoint,
})
.then(
({ json = {} }) => {
Expand Down
1 change: 1 addition & 0 deletions superset/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ class RouteMethod: # pylint: disable=too-few-public-methods
"add_objects": "write",
"delete_object": "write",
"copy_dash": "write",
"get_connection": "write",
}

EXTRA_FORM_DATA_APPEND_KEYS = {
Expand Down
53 changes: 47 additions & 6 deletions superset/databases/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
from superset.databases.schemas import (
database_schemas_query_schema,
database_tables_query_schema,
DatabaseConnectionSchema,
DatabaseFunctionNamesResponse,
DatabasePostSchema,
DatabasePutSchema,
Expand Down Expand Up @@ -122,6 +123,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
"validate_sql",
"delete_ssh_tunnel",
"schemas_access_for_file_upload",
"get_connection",
}
resource_name = "database"
class_permission_name = "Database"
Expand All @@ -144,12 +146,6 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
"driver",
"force_ctas_schema",
"impersonate_user",
"masked_encrypted_extra",
"extra",
"parameters",
"parameters_schema",
"server_cert",
"sqlalchemy_uri",
"is_managed_externally",
"engine_information",
]
Expand Down Expand Up @@ -223,6 +219,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):

openapi_spec_tag = "Database"
openapi_spec_component_schemas = (
DatabaseConnectionSchema,
DatabaseFunctionNamesResponse,
DatabaseSchemaAccessForFileUploadResponse,
DatabaseRelatedObjectsResponse,
Expand All @@ -237,6 +234,50 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
ValidateSQLResponse,
)

@expose("/<int:pk>/connection", methods=("GET",))
@protect()
@safe
def get_connection(self, pk: int) -> Response:
"""Get database connection info.
---
get:
summary: >-
Get a database connection info
parameters:
- in: path
schema:
type: integer
description: The database id
name: pk
responses:
200:
description: Database with connection info
content:
application/json:
schema:
$ref: "#/components/schemas/DatabaseConnectionSchema"
400:
$ref: '#/components/responses/400'
401:
$ref: '#/components/responses/401'
422:
$ref: '#/components/responses/422'
500:
$ref: '#/components/responses/500'
"""
database = DatabaseDAO.find_by_id(pk)
database_connection_schema = DatabaseConnectionSchema()
response = {
"id": pk,
"result": database_connection_schema.dump(database, many=False),
}
try:
if ssh_tunnel := DatabaseDAO.get_ssh_tunnel(pk):
response["result"]["ssh_tunnel"] = ssh_tunnel.data
return self.response(200, **response)
except SupersetException as ex:
return self.response(ex.status, message=ex.message)

@expose("/<int:pk>", methods=("GET",))
@protect()
@safe
Expand Down
83 changes: 83 additions & 0 deletions superset/databases/schemas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,3 +880,86 @@ class DatabaseSchemaAccessForFileUploadResponse(Schema):
"information"
},
)


class DatabaseConnectionSchema(Schema):
"""
Schema with database connection information.

This is only for admins (who have ``can_create`` on ``Database``).
"""

allow_ctas = fields.Boolean(metadata={"description": allow_ctas_description})
allow_cvas = fields.Boolean(metadata={"description": allow_cvas_description})
allow_dml = fields.Boolean(metadata={"description": allow_dml_description})
allow_file_upload = fields.Boolean(
metadata={"description": allow_file_upload_description}
)
allow_run_async = fields.Boolean(
metadata={"description": allow_run_async_description}
)
backend = fields.String(
allow_none=True, metadata={"description": "SQLAlchemy engine to use"}
)
cache_timeout = fields.Integer(
metadata={"description": cache_timeout_description}, allow_none=True
)
configuration_method = fields.String(
metadata={"description": configuration_method_description},
)
database_name = fields.String(
metadata={"description": database_name_description},
allow_none=True,
validate=Length(1, 250),
)
driver = fields.String(
allow_none=True, metadata={"description": "SQLAlchemy driver to use"}
)
engine_information = fields.Dict(keys=fields.String(), values=fields.Raw())
expose_in_sqllab = fields.Boolean(
metadata={"description": expose_in_sqllab_description}
)
extra = fields.String(
metadata={"description": extra_description}, validate=extra_validator
)
force_ctas_schema = fields.String(
metadata={"description": force_ctas_schema_description},
allow_none=True,
validate=Length(0, 250),
)
id = fields.Integer(metadata={"description": "Database ID (for updates)"})
impersonate_user = fields.Boolean(
metadata={"description": impersonate_user_description}
)
is_managed_externally = fields.Boolean(allow_none=True, dump_default=False)
server_cert = fields.String(
metadata={"description": server_cert_description},
allow_none=True,
validate=server_cert_validator,
)
uuid = fields.String(required=False)
ssh_tunnel = fields.Nested(DatabaseSSHTunnel, allow_none=True)
masked_encrypted_extra = fields.String(
metadata={"description": encrypted_extra_description},
validate=encrypted_extra_validator,
allow_none=True,
)
parameters = fields.Dict(
keys=fields.String(),
values=fields.Raw(),
metadata={"description": "DB-specific parameters for configuration"},
)
parameters_schema = fields.Dict(
keys=fields.String(),
values=fields.Raw(),
metadata={
"description": (
"JSONSchema for configuring the database by "
"parameters instead of SQLAlchemy URI"
),
},
)
sqlalchemy_uri = fields.String(
metadata={"description": sqlalchemy_uri_description},
validate=[Length(1, 1024), sqlalchemy_uri_validator],
)
150 changes: 149 additions & 1 deletion tests/unit_tests/databases/api_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ def test_password_mask(
Database.metadata.create_all(session.get_bind()) # pylint: disable=no-member

database = Database(
uuid=UUID("02feae18-2dd6-4bb4-a9c0-49e9d4f29d58"),
database_name="my_database",
sqlalchemy_uri="gsheets://",
encrypted_extra=json.dumps(
Expand All @@ -103,14 +104,161 @@ def test_password_mask(
mocker.patch("sqlalchemy.engine.URL.get_driver_name", return_value="gsheets")
mocker.patch("superset.utils.log.DBEventLogger.log")

response = client.get("/api/v1/database/1")
response = client.get("/api/v1/database/1/connection")

# check that private key is masked
assert (
response.json["result"]["parameters"]["service_account_info"]["private_key"]
== "XXXXXXXXXX"
)
assert "encrypted_extra" not in response.json["result"]


def test_database_connection(
mocker: MockFixture,
app: Any,
session: Session,
client: Any,
full_api_access: None,
) -> None:
"""
Test that connection info is only returned in ``api/v1/database/${id}/connection``.
"""
from superset.databases.api import DatabaseRestApi
from superset.models.core import Database

DatabaseRestApi.datamodel.session = session

# create table for databases
Database.metadata.create_all(session.get_bind()) # pylint: disable=no-member

database = Database(
uuid=UUID("02feae18-2dd6-4bb4-a9c0-49e9d4f29d58"),
database_name="my_database",
sqlalchemy_uri="gsheets://",
encrypted_extra=json.dumps(
{
"service_account_info": {
"type": "service_account",
"project_id": "black-sanctum-314419",
"private_key_id": "259b0d419a8f840056158763ff54d8b08f7b8173",
"private_key": "SECRET",
"client_email": "google-spreadsheets-demo-servi@black-sanctum-314419.iam.gserviceaccount.com",
"client_id": "114567578578109757129",
"auth_uri": "https://accounts.google.com/o/oauth2/auth",
"token_uri": "https://oauth2.googleapis.com/token",
"auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",
"client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/google-spreadsheets-demo-servi%40black-sanctum-314419.iam.gserviceaccount.com",
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a real google account?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, with the private key omitted.

}
),
)
session.add(database)
session.commit()

# mock the lookup so that we don't need to include the driver
mocker.patch("sqlalchemy.engine.URL.get_driver_name", return_value="gsheets")
mocker.patch("superset.utils.log.DBEventLogger.log")

response = client.get("/api/v1/database/1/connection")
assert response.json == {
"id": 1,
"result": {
"allow_ctas": False,
"allow_cvas": False,
"allow_dml": False,
"allow_file_upload": False,
"allow_run_async": False,
"backend": "gsheets",
"cache_timeout": None,
"configuration_method": "sqlalchemy_form",
"database_name": "my_database",
"driver": "gsheets",
"engine_information": {
"disable_ssh_tunneling": True,
"supports_file_upload": False,
},
"expose_in_sqllab": True,
"extra": '{\n "metadata_params": {},\n "engine_params": {},\n "metadata_cache_timeout": {},\n "schemas_allowed_for_file_upload": []\n}\n',
"force_ctas_schema": None,
"id": 1,
"impersonate_user": False,
"is_managed_externally": False,
"masked_encrypted_extra": json.dumps(
{
"service_account_info": {
"type": "service_account",
"project_id": "black-sanctum-314419",
"private_key_id": "259b0d419a8f840056158763ff54d8b08f7b8173",
"private_key": "XXXXXXXXXX",
"client_email": "google-spreadsheets-demo-servi@black-sanctum-314419.iam.gserviceaccount.com",
"client_id": "114567578578109757129",
"auth_uri": "https://accounts.google.com/o/oauth2/auth",
"token_uri": "https://oauth2.googleapis.com/token",
"auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",
"client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/google-spreadsheets-demo-servi%40black-sanctum-314419.iam.gserviceaccount.com",
}
}
),
"parameters": {
"service_account_info": {
"auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",
"auth_uri": "https://accounts.google.com/o/oauth2/auth",
"client_email": "google-spreadsheets-demo-servi@black-sanctum-314419.iam.gserviceaccount.com",
"client_id": "114567578578109757129",
"client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/google-spreadsheets-demo-servi%40black-sanctum-314419.iam.gserviceaccount.com",
"private_key": "XXXXXXXXXX",
"private_key_id": "259b0d419a8f840056158763ff54d8b08f7b8173",
"project_id": "black-sanctum-314419",
"token_uri": "https://oauth2.googleapis.com/token",
"type": "service_account",
}
},
"parameters_schema": {
"properties": {
"catalog": {"type": "object"},
"service_account_info": {
"description": "Contents of GSheets JSON credentials.",
"type": "string",
"x-encrypted-extra": True,
},
},
"type": "object",
},
"server_cert": None,
"sqlalchemy_uri": "gsheets://",
"uuid": "02feae18-2dd6-4bb4-a9c0-49e9d4f29d58",
},
}

response = client.get("/api/v1/database/1")
assert response.json == {
"id": 1,
"result": {
"allow_ctas": False,
"allow_cvas": False,
"allow_dml": False,
"allow_file_upload": False,
"allow_run_async": False,
"backend": "gsheets",
"cache_timeout": None,
"configuration_method": "sqlalchemy_form",
"database_name": "my_database",
"driver": "gsheets",
"engine_information": {
"disable_ssh_tunneling": True,
"supports_file_upload": False,
},
"expose_in_sqllab": True,
"force_ctas_schema": None,
"id": 1,
"impersonate_user": False,
"is_managed_externally": False,
"uuid": "02feae18-2dd6-4bb4-a9c0-49e9d4f29d58",
},
}


@pytest.mark.skip(reason="Works locally but fails on CI")
def test_update_with_password_mask(
app: Any,
Expand Down