You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/09/22 01:24:33 UTC
(superset) branch master updated: fix(database): allow metadata edits on an unreachable database (#44000)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new c666a6400c4 fix(database): allow metadata edits on an unreachable database (#44000)
c666a6400c4 is described below
commit c666a6400c4c1fa6567472b3485cfb9163c3e7c9
Author: Endi Monan <65...@users.noreply.github.com>
AuthorDate: Tue Sep 22 03:24:26 2026 +0200
fix(database): allow metadata edits on an unreachable database (#44000)
---
docs/docs/faq.mdx | 13 ++
superset/commands/database/update.py | 60 ++++++
tests/unit_tests/commands/databases/update_test.py | 209 ++++++++++++++++++++-
tests/unit_tests/databases/api_test.py | 77 ++++++++
4 files changed, 358 insertions(+), 1 deletion(-)
diff --git a/docs/docs/faq.mdx b/docs/docs/faq.mdx
index feacb0000ce..82fd422989b 100644
--- a/docs/docs/faq.mdx
+++ b/docs/docs/faq.mdx
@@ -66,6 +66,11 @@ import FAQSchema from '@site/src/components/FAQSchema';
answer:
'Superset is tested using MySQL, PostgreSQL, and SQLite backends for storing its internal metadata. While Superset supports many databases as data sources, only these are recommended for the metadata store in production.',
},
+ {
+ question: 'Can I hide an unavailable Druid database from SQL Lab?',
+ answer:
+ 'Yes. Edit the existing Druid connection and uncheck Expose database in SQL Lab. Keep the database name and connection settings unchanged. If the connection check fails during permission synchronization, Superset saves the option and logs a warning without deleting the connection, datasets, or charts.',
+ },
{
question: 'How can I configure OAuth authentication and authorization?',
answer:
@@ -299,6 +304,14 @@ may work but isn’t tested. It has been reported that [Microsoft SQL Server doe
work as a Superset backend](https://github.com/apache/superset/issues/18961). Column-store,
non-OLTP databases are not designed for this type of workload.
+## Can I hide an unavailable Druid database from SQL Lab?
+
+Yes. Edit the existing Druid connection and uncheck **Expose database in SQL Lab**.
+Keep its database name and connection settings unchanged. If the connection check fails
+during permission synchronization, Superset saves the option and logs a warning without
+deleting the connection, datasets, or charts. Changes to the database name, credentials,
+or other connection settings still require successful synchronization.
+
## How can I configure OAuth authentication and authorization?
You can take a look at this Flask-AppBuilder
diff --git a/superset/commands/database/update.py b/superset/commands/database/update.py
index f0d079b9121..6af1375d39a 100644
--- a/superset/commands/database/update.py
+++ b/superset/commands/database/update.py
@@ -26,6 +26,7 @@ from flask_appbuilder.models.sqla import Model
from superset import db
from superset.commands.base import BaseCommand
from superset.commands.database.exceptions import (
+ DatabaseConnectionFailedError,
DatabaseExistsValidationError,
DatabaseInvalidError,
DatabaseNotFoundError,
@@ -47,6 +48,7 @@ from superset.models.core import Database
from superset.utils import json
from superset.utils.core import get_username
from superset.utils.decorators import on_error, transaction
+from superset.utils.ssh_tunnel import unmask_password_info
logger = logging.getLogger(__name__)
@@ -81,6 +83,9 @@ class UpdateDatabaseCommand(BaseCommand):
# existing personal tokens.
self._handle_oauth2()
+ # The DAO updates the model in place, so compare settings before applying them.
+ can_skip_failed_sync = self._can_skip_failed_sync()
+
# Some DBs require running a query to get the default catalog.
# In these cases, if the current connection is broken then
# `get_default_catalog` would raise an exception. We need to
@@ -122,9 +127,64 @@ class UpdateDatabaseCommand(BaseCommand):
).run()
except (OAuth2RedirectError, MissingOAuth2TokenError):
pass
+ except DatabaseConnectionFailedError:
+ if not can_skip_failed_sync:
+ raise
+ logger.warning(
+ "Skipping permission sync for database %s: connection unavailable "
+ "and connection settings unchanged",
+ self._model_id,
+ )
return database
+ def _can_skip_failed_sync(self) -> bool:
+ """Check that the connection and permission identity remain unchanged."""
+ assert self._model
+
+ connection_fields = {
+ "database_name", # Schema and catalog permissions include this name.
+ "sqlalchemy_uri",
+ "encrypted_extra",
+ "extra",
+ "server_cert",
+ "impersonate_user",
+ "ssh_tunnel",
+ }
+ for key in connection_fields & self._properties.keys():
+ incoming = self._properties[key]
+ original = getattr(self._model, key)
+ if key == "sqlalchemy_uri":
+ try:
+ original = make_url_safe(self._model.sqlalchemy_uri_decrypted)
+ except Exception:
+ # An unavailable old password store must not block repairs.
+ return False
+ incoming = make_url_safe(incoming.strip())
+ if incoming.password == PASSWORD_MASK:
+ incoming = incoming.set(password=original.password)
+ elif key in {"extra", "encrypted_extra"}:
+ try:
+ original = json.loads(original or "{}")
+ incoming = json.loads(incoming or "{}")
+ except json.JSONDecodeError:
+ return False
+ elif key == "server_cert":
+ original = original or None
+ incoming = incoming or None
+ elif key == "ssh_tunnel" and original is not None and incoming is not None:
+ incoming = unmask_password_info(incoming.copy(), original)
+ incoming = {
+ field: incoming.get(field) for field in original.export_fields
+ }
+ original = {
+ field: getattr(original, field) for field in original.export_fields
+ }
+ if incoming != original:
+ return False
+
+ return True
+
def _handle_oauth2(self) -> None:
"""
Handle changes in OAuth2.
diff --git a/tests/unit_tests/commands/databases/update_test.py b/tests/unit_tests/commands/databases/update_test.py
index ced1ab2271c..d21d9d86331 100644
--- a/tests/unit_tests/commands/databases/update_test.py
+++ b/tests/unit_tests/commands/databases/update_test.py
@@ -15,16 +15,22 @@
# specific language governing permissions and limitations
# under the License.
+from typing import Any
from unittest.mock import MagicMock
import pytest
from pytest_mock import MockerFixture
from superset import db
-from superset.commands.database.exceptions import DatabaseInvalidError
+from superset.commands.database.exceptions import (
+ DatabaseConnectionFailedError,
+ DatabaseInvalidError,
+)
from superset.commands.database.update import UpdateDatabaseCommand
from superset.constants import PASSWORD_MASK
+from superset.databases.ssh_tunnel.models import SSHTunnel
from superset.extensions import security_manager
+from superset.models.core import Database
from superset.utils import json
from tests.conftest import with_config
from tests.unit_tests.commands.databases.conftest import oauth2_client_info
@@ -672,6 +678,207 @@ def test_update_broken_connection(mocker: MockerFixture) -> None:
update_catalog_attribute.assert_called_once_with(1, "main")
+@pytest.fixture
+def unreachable_database(mocker: MockerFixture) -> Database:
+ """Set up a database whose permission-sync ping fails, without persistence."""
+ database = Database(
+ id=1,
+ database_name="Druid",
+ expose_in_sqllab=True,
+ impersonate_user=False,
+ extra='{"engine_params": {}, "metadata_params": {}}',
+ encrypted_extra='{"connect_args": {"jwt": "original-token"}}',
+ )
+ database.set_sqlalchemy_uri("druid://user:secret@localhost:8082/druid/v2/sql/")
+ mocker.patch(
+ "superset.commands.database.update.DatabaseDAO.find_by_id",
+ return_value=database,
+ )
+
+ def update(model: Database, properties: dict[str, Any]) -> Database:
+ """Apply the command's properties in place, as the DAO does."""
+ for key, value in properties.items():
+ setattr(model, key, value)
+ return model
+
+ mocker.patch(
+ "superset.commands.database.update.DatabaseDAO.update", side_effect=update
+ )
+ mocker.patch(
+ "superset.commands.database.update.DatabaseDAO.validate_update_uniqueness",
+ return_value=True,
+ )
+ mocker.patch("superset.commands.database.update.get_username", return_value="admin")
+ mocker.patch.object(security_manager, "get_user_by_username")
+ mocker.patch.object(database, "get_sqla_engine")
+ mocker.patch("superset.commands.database.sync_permissions.ping", return_value=False)
+ return database
+
+
+@pytest.mark.parametrize("full_payload", [False, True])
+@pytest.mark.parametrize("async_mode", [False, True])
+def test_update_unreachable_database_metadata(
+ mocker: MockerFixture,
+ unreachable_database: Database,
+ caplog: pytest.LogCaptureFixture,
+ full_payload: bool,
+ async_mode: bool,
+) -> None:
+ """An offline database can be hidden from SQL Lab with unchanged settings."""
+ from flask import current_app
+
+ mocker.patch.dict(
+ current_app.config, {"SYNC_DB_PERMISSIONS_IN_ASYNC_MODE": async_mode}
+ )
+ enqueue = mocker.patch(
+ "superset.commands.database.sync_permissions.sync_database_permissions_task.delay"
+ )
+ update_catalog = mocker.patch.object(
+ UpdateDatabaseCommand, "_update_catalog_attribute"
+ )
+ properties: dict[str, Any] = {"expose_in_sqllab": False}
+ if full_payload:
+ properties.update(
+ database_name="Druid",
+ sqlalchemy_uri=unreachable_database.sqlalchemy_uri,
+ masked_encrypted_extra=json.dumps({"connect_args": {"jwt": PASSWORD_MASK}}),
+ extra='{"metadata_params": {}, "engine_params": {}}',
+ impersonate_user=False,
+ server_cert=None,
+ ssh_tunnel=None,
+ )
+
+ result = UpdateDatabaseCommand(1, properties).run()
+
+ assert result is unreachable_database
+ assert result.expose_in_sqllab is False
+ assert result.password == "secret" # noqa: S105
+ assert json.loads(result.encrypted_extra)["connect_args"]["jwt"] == "original-token"
+ assert "Skipping permission sync for database 1" in caplog.text
+ assert "original-token" not in caplog.text
+ enqueue.assert_not_called()
+ update_catalog.assert_not_called()
+
+
+@pytest.mark.parametrize(
+ "properties",
+ [
+ {"database_name": "Renamed"},
+ {"sqlalchemy_uri": "druid://user:secret@other-host:8082/druid/v2/sql/"},
+ {"sqlalchemy_uri": "druid://user:changed@localhost:8082/druid/v2/sql/"},
+ {"masked_encrypted_extra": '{"connect_args": {"jwt": "changed"}}'},
+ {
+ "sqlalchemy_uri": "druid://user:secret@localhost:8082/druid/v2/sql/",
+ "extra": '{"engine_params": {"connect_args": {"scheme": "https"}}}',
+ },
+ {"extra": '{"allow_multi_catalog": true}'},
+ {"server_cert": "changed-certificate"},
+ {"impersonate_user": True},
+ {"ssh_tunnel": None},
+ ],
+ ids=[
+ "rename",
+ "host",
+ "password",
+ "encrypted-extra",
+ "engine-params",
+ "catalogs",
+ "certificate",
+ "impersonation",
+ "remove-tunnel",
+ ],
+)
+def test_update_unreachable_database_changed_connection_fails(
+ mocker: MockerFixture,
+ unreachable_database: Database,
+ properties: dict[str, Any],
+) -> None:
+ """Changed connection settings or names still require a successful sync."""
+ if "sqlalchemy_uri" in properties:
+ # Keep these cases independent of stored extra credentials.
+ unreachable_database.encrypted_extra = None
+ if "ssh_tunnel" in properties:
+ unreachable_database.ssh_tunnel = SSHTunnel(server_address="localhost")
+ rollback = mocker.patch.object(db.session, "rollback")
+ commit = mocker.patch.object(db.session, "commit")
+
+ with pytest.raises(DatabaseConnectionFailedError):
+ UpdateDatabaseCommand(1, {"expose_in_sqllab": False, **properties}).run()
+
+ rollback.assert_called_once()
+ commit.assert_not_called()
+
+
+@pytest.mark.parametrize("field", ["extra", "encrypted_extra"])
+@pytest.mark.parametrize("invalid_original", [False, True], ids=["incoming", "stored"])
+def test_update_unreachable_database_invalid_json_fails(
+ mocker: MockerFixture,
+ unreachable_database: Database,
+ field: str,
+ invalid_original: bool,
+) -> None:
+ """Invalid JSON must not allow an update to skip a failed permission sync."""
+ if field == "extra":
+ # Reach JSON comparison without reusing secrets across engine-param changes.
+ unreachable_database.password = None
+ unreachable_database.encrypted_extra = None
+ properties: dict[str, Any] = {"expose_in_sqllab": False, field: "{invalid"}
+ if invalid_original:
+ setattr(unreachable_database, field, "{invalid")
+ properties[field] = "{}"
+ rollback = mocker.patch.object(db.session, "rollback")
+ commit = mocker.patch.object(db.session, "commit")
+
+ with pytest.raises(DatabaseConnectionFailedError):
+ UpdateDatabaseCommand(1, properties).run()
+
+ rollback.assert_called_once()
+ commit.assert_not_called()
+
+
+@pytest.mark.parametrize("connection_alive", [False, True])
+def test_update_with_missing_old_password(
+ mocker: MockerFixture,
+ unreachable_database: Database,
+ connection_alive: bool,
+) -> None:
+ """An unavailable old password must not prevent repairing the connection."""
+ from flask import current_app
+ from sqlalchemy.engine.url import URL
+
+ unreachable_database.encrypted_extra = None
+
+ def password_store(uri: URL) -> str:
+ """Only the replacement connection has a stored password."""
+ if uri.host == "localhost":
+ raise KeyError("The old password was removed")
+ return "new-secret"
+
+ mocker.patch.dict(
+ current_app.config, {"SQLALCHEMY_CUSTOM_PASSWORD_STORE": password_store}
+ )
+ mocker.patch(
+ "superset.commands.database.sync_permissions.ping",
+ return_value=connection_alive,
+ )
+ sync = mocker.patch(
+ "superset.commands.database.sync_permissions."
+ "SyncPermissionsCommand.sync_database_permissions"
+ )
+ new_uri = "druid://user:new-secret@replacement:8082/druid/v2/sql/"
+ command = UpdateDatabaseCommand(1, {"sqlalchemy_uri": new_uri})
+
+ if connection_alive:
+ result = command.run()
+ assert result is unreachable_database
+ assert result.sqlalchemy_uri_decrypted == new_uri
+ sync.assert_called_once()
+ else:
+ with pytest.raises(DatabaseConnectionFailedError):
+ command.run()
+ sync.assert_not_called()
+
+
def test_update_host_change_requires_new_credentials(mocker: MockerFixture) -> None:
"""
An update that repoints an existing database at a different host while
diff --git a/tests/unit_tests/databases/api_test.py b/tests/unit_tests/databases/api_test.py
index cedf0b15b03..bc77c10b0c0 100644
--- a/tests/unit_tests/databases/api_test.py
+++ b/tests/unit_tests/databases/api_test.py
@@ -357,6 +357,83 @@ def test_database_connection(
}
+@pytest.mark.parametrize("full_payload", [False, True])
+@pytest.mark.parametrize("change", [None, "password", "database_name"])
+@pytest.mark.parametrize("with_tunnel", [False, True])
+def test_update_unreachable_database(
+ mocker: MockerFixture,
+ session: Session,
+ client: Any,
+ full_api_access: None,
+ full_payload: bool,
+ change: str | None,
+ with_tunnel: bool,
+) -> None:
+ """Persist offline metadata edits, but roll back changed credentials or names."""
+ from superset import security_manager
+ from superset.databases.api import DatabaseRestApi
+ from superset.databases.ssh_tunnel.models import SSHTunnel
+ from superset.models.core import Database
+
+ mocker.patch.object(DatabaseRestApi.datamodel, "_session", session)
+ Database.metadata.create_all(session.get_bind()) # pylint: disable=no-member
+ database = Database(
+ database_name="Druid",
+ expose_in_sqllab=True,
+ encrypted_extra='{"connect_args": {"jwt": "original-token"}}',
+ )
+ database.set_sqlalchemy_uri("druid://user:secret@localhost:8082/druid/v2/sql/")
+ if with_tunnel:
+ database.ssh_tunnel = SSHTunnel(
+ server_address="localhost",
+ server_port=22,
+ username="ssh-user",
+ password="ssh-secret", # noqa: S106
+ )
+ session.add(database)
+ session.commit()
+ database_id = database.id
+
+ mocker.patch("superset.utils.log.DBEventLogger.log")
+ mocker.patch("superset.commands.database.update.get_username", return_value="admin")
+ mocker.patch.object(security_manager, "get_user_by_username")
+ mocker.patch.object(Database, "get_sqla_engine")
+ ping = mocker.patch(
+ "superset.commands.database.sync_permissions.ping",
+ side_effect=ConnectionError("Database unavailable"),
+ )
+ properties: dict[str, Any] = {}
+ if full_payload:
+ response = client.get(f"/api/v1/database/{database_id}/connection")
+ assert response.status_code == 200
+ properties = response.json["result"]
+ properties["expose_in_sqllab"] = False
+ if change == "password":
+ properties["sqlalchemy_uri"] = (
+ "druid://user:changed@localhost:8082/druid/v2/sql/"
+ )
+ elif change == "database_name":
+ properties["database_name"] = "Renamed"
+
+ response = client.put(f"/api/v1/database/{database_id}", json=properties)
+
+ assert response.status_code == (422 if change else 200)
+ if change:
+ assert response.json == {
+ "message": "Connection failed, please check your connection settings"
+ }
+ session.expire_all()
+ stored = session.get(Database, database_id)
+ assert stored is not None
+ assert stored.expose_in_sqllab is bool(change)
+ assert stored.database_name == "Druid"
+ assert stored.password == "secret" # noqa: S105
+ assert json.loads(stored.encrypted_extra)["connect_args"]["jwt"] == "original-token"
+ if with_tunnel:
+ assert stored.ssh_tunnel.password == "ssh-secret" # noqa: S105
+ ping.assert_called_once()
+
+
@pytest.mark.skip(reason="Works locally but fails on CI")
def test_update_with_password_mask(
app: Any,