(superset) 01/01: fix(import): catch KeyError for missing uuid/ssh_tunnel in load_configs

[el...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

eschutho pushed a commit to branch fix-keyerror-import-config-uuid
in repository https://gitbox.apache.org/repos/asf/superset.git

commit d746e42332be35487509decd2afa577e10506f77
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Tue Aug 18 16:43:37 2026 +0000

    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.
---
 superset/commands/importers/v1/utils.py            | 19 +++++
 .../unit_tests/commands/importers/v1/utils_test.py | 82 ++++++++++++++++++++++
 2 files changed, 101 insertions(+)

diff --git a/superset/commands/importers/v1/utils.py b/superset/commands/importers/v1/utils.py
index 28936341f13..98e7a2d6e43 100644
--- a/superset/commands/importers/v1/utils.py
+++ b/superset/commands/importers/v1/utils.py
@@ -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
 
diff --git a/tests/unit_tests/commands/importers/v1/utils_test.py b/tests/unit_tests/commands/importers/v1/utils_test.py
index 58def5bebef..0e0419e693e 100644
--- a/tests/unit_tests/commands/importers/v1/utils_test.py
+++ b/tests/unit_tests/commands/importers/v1/utils_test.py
@@ -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 == []