naiduv commented on code in PR #44203:
URL: https://github.com/apache/superset/pull/44203#discussion_r3996865518
##########
superset/db_engine_specs/databricks.py:
##########
@@ -311,13 +398,23 @@ class DatabricksODBCEngineSpec(DatabricksBaseEngineSpec):
class DatabricksDynamicBaseEngineSpec(BasicParametersMixin,
DatabricksBaseEngineSpec):
default_driver = ""
encryption_parameters = {"ssl": "1"}
- required_parameters = {"access_token", "host", "port"}
+ # ``access_token`` is required unless Encrypted Extra has M2M credentials.
+ required_parameters = {"host", "port"}
context_key_mapping = {
"access_token": "password",
"host": "hostname",
"port": "port",
}
+ # Mask the service-principal secret; leave ``auth_method`` / ``client_id``
+ # visible when the database is edited (Snowflake / Redshift pattern).
+ # ``$.oauth2_client_info.secret`` is always added by the base class.
+ # pylint: disable=invalid-name
+ encrypted_extra_sensitive_fields = {
+ "$.client_secret": "OAuth Client Secret",
+ "$.azure_client_secret": "Azure Client Secret",
+ }
Review Comment:
Fixed in 0833524: restored inherited `$.*` so unrelated Encrypted Extra
secrets stay masked, while keeping named paths for `client_secret` /
`azure_client_secret`.
##########
superset/db_engine_specs/databricks.py:
##########
@@ -478,7 +578,47 @@ def update_params_from_encrypted_extra(
logger.error(ex, exc_info=True)
raise
encrypted_extra.pop("oauth2_client_info", None)
- params.update(encrypted_extra)
+
+ auth_method = encrypted_extra.pop("auth_method", None)
+ client_id = encrypted_extra.pop("client_id", None) or
encrypted_extra.pop(
+ "azure_client_id", None
+ )
+ client_secret = encrypted_extra.pop("client_secret", None) or (
+ encrypted_extra.pop("azure_client_secret", None)
+ )
+ # Always drop the Azure-named aliases so they never reach
create_engine.
+ encrypted_extra.pop("azure_client_id", None)
+ encrypted_extra.pop("azure_client_secret", None)
+ azure_tenant_id = encrypted_extra.pop("azure_tenant_id", None)
+
+ if auth_method == AUTH_METHOD_OAUTH_M2M:
+ if not client_id or not client_secret:
+ raise ValueError(
+ "Databricks OAuth M2M requires both client_id and "
+ "client_secret in Secure Extra."
+ )
+ host = ""
+ if getattr(database, "url_object", None) is not None:
+ host = database.url_object.host or ""
+ connect_args = params.setdefault("connect_args", {})
+ connect_args["credentials_provider"] = (
+ _build_oauth_m2m_credentials_provider(host, client_id,
client_secret)
+ )
Review Comment:
Fixed in 0833524: replaced the per-build nested function with
`_M2MCredentialsProvider` and a secret-free, identity-stable `__repr__` so the
engine cache key is reusable. Secret rotation still evicts the cache on
database save.
##########
superset/db_engine_specs/databricks.py:
##########
@@ -478,7 +578,47 @@ def update_params_from_encrypted_extra(
logger.error(ex, exc_info=True)
raise
encrypted_extra.pop("oauth2_client_info", None)
- params.update(encrypted_extra)
+
+ auth_method = encrypted_extra.pop("auth_method", None)
+ client_id = encrypted_extra.pop("client_id", None) or
encrypted_extra.pop(
+ "azure_client_id", None
+ )
+ client_secret = encrypted_extra.pop("client_secret", None) or (
+ encrypted_extra.pop("azure_client_secret", None)
+ )
+ # Always drop the Azure-named aliases so they never reach
create_engine.
+ encrypted_extra.pop("azure_client_id", None)
+ encrypted_extra.pop("azure_client_secret", None)
+ azure_tenant_id = encrypted_extra.pop("azure_tenant_id", None)
+
+ if auth_method == AUTH_METHOD_OAUTH_M2M:
+ if not client_id or not client_secret:
+ raise ValueError(
+ "Databricks OAuth M2M requires both client_id and "
+ "client_secret in Secure Extra."
+ )
+ host = ""
+ if getattr(database, "url_object", None) is not None:
+ host = database.url_object.host or ""
+ connect_args = params.setdefault("connect_args", {})
+ connect_args["credentials_provider"] = (
+ _build_oauth_m2m_credentials_provider(host, client_id,
client_secret)
+ )
+ elif auth_method == AUTH_METHOD_AZURE_SP_M2M:
+ if not client_id or not client_secret:
+ raise ValueError(
+ "Databricks Azure service-principal M2M requires both "
+ "client_id and client_secret in Secure Extra."
+ )
+ connect_args = params.setdefault("connect_args", {})
+ connect_args["auth_type"] = AUTH_METHOD_AZURE_SP_M2M
Review Comment:
Fixed in 0833524: Azure SP M2M now uses `credentials_provider` +
`azure_service_principal` (requires `azure_tenant_id` and `databricks-sdk`). We
no longer pass the unsupported `auth_type=azure-sp-m2m` to the connector.
##########
superset/db_engine_specs/databricks.py:
##########
@@ -539,10 +696,11 @@ def validate_parameters( # type: ignore
],
) -> list[SupersetError]:
errors: list[SupersetError] = []
- connect_args: dict[str, Any] = {}
- if extra := json.loads(properties.get("extra")): # type: ignore
- engine_params = extra.get("engine_params", {})
- connect_args = engine_params.get("connect_args", {})
+ extra_raw = properties.get("extra")
+ extra = json.loads(extra_raw) if extra_raw else {} # type: ignore
Review Comment:
Fixed in 0833524: malformed Extra JSON now returns a `SupersetError` from
`validate_parameters` instead of raising `JSONDecodeError`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
