Skip to content

Commit b787704

Browse files
rohitrshcursoragent
andcommitted
refactor(spark): isolate Databricks PAT authentication
Introduce an authentication strategy boundary while preserving the existing PAT resolution and connector behavior, enabling subsequent auth methods to be reviewed independently. Signed-off-by: Rohit Sharma <rohitrsh@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent ca59398 commit b787704

3 files changed

Lines changed: 154 additions & 12 deletions

File tree

plugins/flytekit-spark/flytekitplugins/spark/connector.py

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,8 @@ async def create(
262262
task_execution_metadata: Optional[TaskExecutionMetadata] = None,
263263
**kwargs,
264264
) -> DatabricksJobMetadata:
265+
from .databricks_auth import select_auth
266+
265267
data = json.dumps(_get_databricks_job_spec(task_template))
266268
databricks_instance = task_template.custom.get(
267269
"databricksInstance", os.getenv(DEFAULT_DATABRICKS_INSTANCE_ENV_KEY)
@@ -272,22 +274,13 @@ async def create(
272274
f"Missing databricks instance. Please set the value through the task config or set the {DEFAULT_DATABRICKS_INSTANCE_ENV_KEY} environment variable in the connector."
273275
)
274276

275-
# Get workflow-specific token or fall back to default
276277
namespace = task_execution_metadata.namespace if task_execution_metadata else None
277-
278-
# Extract custom secret name from task template (if provided)
279-
custom_secret_name = task_template.custom.get("databricksTokenSecret")
280-
281-
logger.info(f"Creating Databricks job for namespace: {namespace or 'unknown'}")
282-
if custom_secret_name:
283-
logger.info(f"Using custom secret name: {custom_secret_name}")
284-
285-
auth_token = get_databricks_token(
286-
namespace=namespace, task_template=task_template, secret_name=custom_secret_name
287-
)
278+
auth = await select_auth(task_template=task_template, namespace=namespace)
279+
logger.info("Databricks auth resolved: %s", auth.describe())
288280
databricks_url = f"https://{databricks_instance}{DATABRICKS_API_ENDPOINT}/runs/submit"
289281

290282
async with aiohttp.ClientSession() as session:
283+
auth_token = await auth.get_bearer_token(session)
291284
async with session.post(databricks_url, headers=get_header(auth_token=auth_token), data=data) as resp:
292285
response = await resp.json()
293286
if resp.status != http.HTTPStatus.OK:
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
"""Authentication strategies for the Databricks connector.
2+
3+
This module introduces a small strategy boundary around the connector's
4+
existing Personal Access Token (PAT) flow. It intentionally preserves the
5+
current token resolution behavior so additional authentication methods can be
6+
added independently.
7+
"""
8+
9+
from abc import ABC, abstractmethod
10+
from dataclasses import dataclass
11+
from typing import Optional
12+
13+
from flytekit import lazy_module
14+
from flytekit.models.task import TaskTemplate
15+
16+
aiohttp = lazy_module("aiohttp")
17+
18+
19+
@dataclass
20+
class _Settings:
21+
"""Authentication settings resolved for one Databricks task."""
22+
23+
task_template: Optional[TaskTemplate]
24+
token_secret_name: Optional[str]
25+
namespace: Optional[str]
26+
27+
@staticmethod
28+
def from_task(task_template: Optional[TaskTemplate], namespace: Optional[str]) -> "_Settings":
29+
custom = task_template.custom if task_template is not None else {}
30+
return _Settings(
31+
task_template=task_template,
32+
token_secret_name=custom.get("databricksTokenSecret"),
33+
namespace=namespace,
34+
)
35+
36+
37+
class DatabricksAuth(ABC):
38+
"""Interface for obtaining a bearer token for Databricks API calls."""
39+
40+
auth_type = "unknown"
41+
strategy_name = "DatabricksAuth"
42+
43+
def __init__(self, settings: _Settings):
44+
self.settings = settings
45+
46+
@abstractmethod
47+
async def get_bearer_token(self, session: "aiohttp.ClientSession") -> str: # type: ignore[name-defined]
48+
"""Return a bearer token for a Databricks API request."""
49+
50+
def describe(self) -> str:
51+
"""Return a description that is safe to write to connector logs."""
52+
return (
53+
f"strategy={self.strategy_name} auth_type={self.auth_type} " f"namespace={self.settings.namespace or 'N/A'}"
54+
)
55+
56+
57+
class PATAuth(DatabricksAuth):
58+
"""Delegate to the connector's existing multi-tenant PAT lookup."""
59+
60+
auth_type = "pat"
61+
strategy_name = "PATAuth"
62+
63+
async def get_bearer_token(self, session: "aiohttp.ClientSession") -> str: # type: ignore[name-defined]
64+
from .connector import get_databricks_token
65+
66+
return get_databricks_token(
67+
namespace=self.settings.namespace,
68+
task_template=self.settings.task_template,
69+
secret_name=self.settings.token_secret_name,
70+
)
71+
72+
73+
async def select_auth(
74+
task_template: Optional[TaskTemplate],
75+
namespace: Optional[str],
76+
) -> DatabricksAuth:
77+
"""Select authentication for a task.
78+
79+
PAT remains the only strategy and therefore the unconditional default in
80+
this refactor. Later authentication methods can extend this dispatcher
81+
without changing the connector request lifecycle.
82+
"""
83+
return PATAuth(_Settings.from_task(task_template, namespace))
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
"""Tests for the Databricks authentication strategy boundary."""
2+
3+
from unittest.mock import AsyncMock, MagicMock, patch
4+
5+
import pytest
6+
7+
from flytekitplugins.spark.databricks_auth import (
8+
PATAuth,
9+
_Settings,
10+
select_auth,
11+
)
12+
13+
14+
def _task_template(**custom):
15+
task_template = MagicMock()
16+
task_template.custom = custom
17+
return task_template
18+
19+
20+
def test_settings_use_default_secret_name():
21+
settings = _Settings.from_task(task_template=None, namespace="project-a")
22+
23+
assert settings.token_secret_name is None
24+
assert settings.namespace == "project-a"
25+
26+
27+
def test_settings_use_task_secret_name():
28+
task_template = _task_template()
29+
task_template.custom["databricksTokenSecret"] = "custom-token"
30+
31+
settings = _Settings.from_task(task_template=task_template, namespace="project-a")
32+
33+
assert settings.token_secret_name == "custom-token"
34+
35+
36+
@pytest.mark.asyncio
37+
async def test_select_auth_returns_pat_by_default():
38+
task_template = _task_template()
39+
auth = await select_auth(task_template=task_template, namespace="project-a")
40+
41+
assert isinstance(auth, PATAuth)
42+
assert auth.auth_type == "pat"
43+
44+
45+
@pytest.mark.asyncio
46+
async def test_pat_auth_delegates_to_existing_token_lookup():
47+
task_template = _task_template()
48+
settings = _Settings(
49+
task_template=task_template,
50+
token_secret_name="custom-token",
51+
namespace="project-a",
52+
)
53+
auth = PATAuth(settings)
54+
55+
with patch(
56+
"flytekitplugins.spark.connector.get_databricks_token",
57+
return_value="example-token",
58+
) as get_token:
59+
token = await auth.get_bearer_token(AsyncMock())
60+
61+
assert token == "example-token"
62+
get_token.assert_called_once_with(
63+
namespace="project-a",
64+
task_template=task_template,
65+
secret_name="custom-token",
66+
)

0 commit comments

Comments
 (0)