Compare commits

...
Author SHA1 Message Date
Elizabeth Thompson d746e42332 fix(import): catch KeyError for missing uuid/ssh_tunnel in load_configs
A hand-edited or third-party database export YAML that omits the `uuid`
key (or `ssh_tunnel`) hit `config["uuid"]`/`config["ssh_tunnel"]`
indexing in load_configs() before schema.load() ran, raising a raw
KeyError that escaped the enclosing `except ValidationError` and
surfaced as an opaque 500 from the *//import/ endpoints instead of a
clean validation error. Add a sibling `except KeyError` that logs and
appends a ValidationError, routing the failure into the same aggregated
per-file error path as every other validation failure.
2026-08-18 16:43:37 +00:00
2 changed files with 101 additions and 0 deletions
+19
View File
@@ -247,6 +247,25 @@ def load_configs(
)
exc.messages = {file_name: exc.messages}
exceptions.append(exc)
except KeyError as exc:
# Some config fields (e.g. `uuid`, `ssh_tunnel`) are read
# directly from the imported YAML before schema validation runs;
# a config missing one of these keys raises a raw KeyError
# instead of failing validation cleanly like every other
# per-file error. Convert it into a ValidationError so it flows
# into the same aggregated error path.
field = str(exc).strip("'\"")
logger.error(
"Missing required key %s in config for %s (prefix: %s)",
exc,
file_name,
prefix,
)
exceptions.append(
ValidationError(
{file_name: {field: ["Missing data for required field."]}}
)
)
return configs
@@ -138,3 +138,85 @@ class TestLoadYaml:
with pytest.raises(ValidationError):
load_yaml("test.yaml", 'key: "unterminated string')
class TestLoadConfigs:
def _database_schemas(self) -> dict[str, object]:
from marshmallow import fields, Schema
class DatabaseSchema(Schema):
uuid = fields.UUID(required=True)
database_name = fields.String(required=True)
sqlalchemy_uri = fields.String(required=True)
password = fields.String(required=False, allow_none=True)
return {"databases/": DatabaseSchema()}
@patch("superset.commands.importers.v1.utils.db")
def test_missing_uuid_appends_validation_error(self, mock_db: object) -> None:
"""A databases config missing `uuid` must not raise a raw KeyError;
it should be excluded from the returned configs and a ValidationError
appended to the exceptions list instead."""
from marshmallow.exceptions import ValidationError
from superset.commands.importers.v1.utils import load_configs
mock_db.session.query.return_value.all.return_value = []
# No `uuid` and no `password`, so the code reaches
# `config["uuid"] in db_passwords` and would raise KeyError pre-fix.
contents = {
"databases/bad.yaml": (
"database_name: bad\nsqlalchemy_uri: postgres://localhost\n"
),
}
exceptions: list[ValidationError] = []
configs = load_configs(
contents,
self._database_schemas(),
{},
exceptions,
{},
{},
{},
{},
)
assert "databases/bad.yaml" not in configs
assert len(exceptions) == 1
assert isinstance(exceptions[0], ValidationError)
assert "databases/bad.yaml" in exceptions[0].messages
@patch("superset.commands.importers.v1.utils.db")
def test_uuid_present_loads_successfully(self, mock_db: object) -> None:
"""Control: a well-formed databases config loads with no exceptions."""
from marshmallow.exceptions import ValidationError
from superset.commands.importers.v1.utils import load_configs
mock_db.session.query.return_value.all.return_value = []
contents = {
"databases/good.yaml": (
"uuid: 6ff1d5b3-4b0f-4c6a-9d2f-9c8b7a6e5d4c\n"
"database_name: good\n"
"sqlalchemy_uri: postgres://localhost\n"
"password: secret\n"
),
}
exceptions: list[ValidationError] = []
configs = load_configs(
contents,
self._database_schemas(),
{},
exceptions,
{},
{},
{},
{},
)
assert "databases/good.yaml" in configs
assert exceptions == []