Skip to content

Commit e679214

Browse files
Revert e2e OAuth M2M; use SP PAT instead (keep DBR LTS check on M2M)
The azure-prod DATABRICKS_TOKEN is now a personal access token owned by the PECO service principal, so its identity matches DATABRICKS_USER. That fixes the Personal Staging Location tests (stage://tmp/<SP>/...) with a plain PAT, without the OAuth M2M machinery -- which also broke the retry/HTTP tests, since M2M makes a live token-endpoint call that those tests' urllib3 mocking intercepts. Reverts the e2e auth changes (conftest.auth_connect_kwargs + the consumer call sites + the databricks-sdk dev dep + the code-coverage.yml SP env) back to the plain access_token path. The DBR LTS install check keeps OAuth M2M: it hits the workspace Jobs/SCIM API (which rejects a warehouse-scoped PAT), is proven 15/15 green on M2M, and installs databricks-sdk itself in its own workflow. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
1 parent 4821823 commit e679214

7 files changed

Lines changed: 9 additions & 86 deletions

File tree

.github/workflows/code-coverage.yml

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,6 @@ jobs:
4545
DATABRICKS_SERVER_HOSTNAME: ${{ secrets.DATABRICKS_HOST }}
4646
DATABRICKS_HTTP_PATH: ${{ secrets.TEST_PECO_WAREHOUSE_HTTP_PATH }}
4747
DATABRICKS_TOKEN: ${{ secrets.DATABRICKS_TOKEN }}
48-
# OAuth M2M as the PECO service principal. The e2e suite prefers this
49-
# over the PAT (see conftest.auth_connect_kwargs) so the connecting
50-
# identity IS the service principal -- required for the Personal Staging
51-
# Location tests, whose stage path is stage://tmp/<DATABRICKS_USER>/ and
52-
# DATABRICKS_USER is this same SP. A PAT for a different identity fails
53-
# those with PERMISSION_DENIED on the personal stage.
54-
DATABRICKS_CLIENT_ID: ${{ secrets.TEST_PECO_SP_ID }}
55-
DATABRICKS_CLIENT_SECRET: ${{ secrets.TEST_PECO_SP_OAUTH_SECRET }}
5648
DATABRICKS_CATALOG: peco
5749
DATABRICKS_USER: ${{ secrets.TEST_PECO_SP_ID }}
5850
steps:

conftest.py

Lines changed: 1 addition & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,6 @@ def access_token():
1717
return os.getenv("DATABRICKS_TOKEN")
1818

1919

20-
@pytest.fixture(scope="session")
21-
def client_id():
22-
# OAuth M2M service-principal client id (application id).
23-
return os.getenv("DATABRICKS_CLIENT_ID")
24-
25-
26-
@pytest.fixture(scope="session")
27-
def client_secret():
28-
return os.getenv("DATABRICKS_CLIENT_SECRET")
29-
30-
3120
@pytest.fixture(scope="session")
3221
def ingestion_user():
3322
return os.getenv("DATABRICKS_USER")
@@ -44,58 +33,12 @@ def schema():
4433

4534

4635
@pytest.fixture(scope="session", autouse=True)
47-
def connection_details(
48-
host,
49-
http_path,
50-
access_token,
51-
client_id,
52-
client_secret,
53-
ingestion_user,
54-
catalog,
55-
schema,
56-
):
36+
def connection_details(host, http_path, access_token, ingestion_user, catalog, schema):
5737
return {
5838
"host": host,
5939
"http_path": http_path,
6040
"access_token": access_token,
61-
"client_id": client_id,
62-
"client_secret": client_secret,
6341
"ingestion_user": ingestion_user,
6442
"catalog": catalog,
6543
"schema": schema,
6644
}
67-
68-
69-
def auth_connect_kwargs(details):
70-
"""Return the sql.connect auth kwargs from connection_details.
71-
72-
Prefers OAuth M2M (service principal) when DATABRICKS_CLIENT_ID /
73-
DATABRICKS_CLIENT_SECRET are set, otherwise falls back to a PAT
74-
(DATABRICKS_TOKEN). M2M is required for identity-scoped operations such as
75-
Personal Staging Location tests (stage://tmp/<DATABRICKS_USER>/...), where
76-
the connecting identity must equal DATABRICKS_USER -- the service principal.
77-
A PAT belonging to a different identity cannot access that stage.
78-
"""
79-
client_id = details.get("client_id")
80-
client_secret = details.get("client_secret")
81-
if client_id and client_secret:
82-
host = details["host"]
83-
host_url = host if host.startswith("http") else f"https://{host}"
84-
85-
def credential_provider():
86-
# Imported lazily so a PAT-only environment doesn't require the SDK.
87-
from databricks.sdk.core import Config, oauth_service_principal
88-
89-
return oauth_service_principal(
90-
Config(
91-
host=host_url,
92-
client_id=client_id,
93-
client_secret=client_secret,
94-
# Explicit so an ambient DATABRICKS_TOKEN doesn't collide
95-
# ("more than one authorization method configured").
96-
auth_type="oauth-m2m",
97-
)
98-
)
99-
100-
return {"credentials_provider": credential_provider}
101-
return {"access_token": details.get("access_token")}

pyproject.toml

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,6 @@ black = "^22.3.0"
9393
pytest-dotenv = "^0.5.2"
9494
pytest-cov = "^4.0.0"
9595
pytest-xdist = "^3.0.0"
96-
# Test-only: the e2e suite authenticates to the warehouse via OAuth M2M
97-
# (service principal) using databricks.sdk's oauth_service_principal — see
98-
# conftest.auth_connect_kwargs. The connector itself has no native
99-
# Databricks-OAuth M2M path (its azure-sp-m2m targets Azure AD; its
100-
# databricks-oauth path is interactive U2M), so the SDK provides the
101-
# client-credentials flow against /oidc/v1/token. Not a runtime dependency.
102-
databricks-sdk = ">=0.20"
10396
numpy = [
10497
{ version = ">=1.16.6", python = ">=3.8,<3.11" },
10598
{ version = ">=1.23.4", python = ">=3.11" },

tests/e2e/test_circuit_breaker.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
import databricks.sql as sql
2323
from databricks.sql.telemetry.circuit_breaker_manager import CircuitBreakerManager
24-
from conftest import auth_connect_kwargs
2524

2625

2726
def wait_for_circuit_state(circuit_breaker, expected_states, timeout=5):
@@ -121,7 +120,7 @@ def mock_request(*args, **kwargs):
121120
with sql.connect(
122121
server_hostname=self.arguments["host"],
123122
http_path=self.arguments["http_path"],
124-
**auth_connect_kwargs(self.arguments),
123+
access_token=self.arguments.get("access_token"),
125124
force_enable_telemetry=True,
126125
telemetry_batch_size=1,
127126
_telemetry_circuit_breaker_enabled=True,
@@ -182,7 +181,7 @@ def mock_rate_limited_request(*args, **kwargs):
182181
with sql.connect(
183182
server_hostname=self.arguments["host"],
184183
http_path=self.arguments["http_path"],
185-
**auth_connect_kwargs(self.arguments),
184+
access_token=self.arguments.get("access_token"),
186185
force_enable_telemetry=True,
187186
telemetry_batch_size=1,
188187
_telemetry_circuit_breaker_enabled=False, # Disabled
@@ -216,7 +215,7 @@ def mock_conditional_request(*args, **kwargs):
216215
with sql.connect(
217216
server_hostname=self.arguments["host"],
218217
http_path=self.arguments["http_path"],
219-
**auth_connect_kwargs(self.arguments),
218+
access_token=self.arguments.get("access_token"),
220219
force_enable_telemetry=True,
221220
telemetry_batch_size=1,
222221
_telemetry_circuit_breaker_enabled=True,

tests/e2e/test_driver.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,9 @@ def connection_params(self):
9999
return params
100100

101101
def auth_params(self):
102-
from conftest import auth_connect_kwargs
103-
104-
return auth_connect_kwargs(self.arguments)
102+
return {
103+
"access_token": self.arguments.get("access_token"),
104+
}
105105

106106
@contextmanager
107107
def connection(self, extra_params=()):

tests/e2e/test_telemetry_e2e.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,10 @@ def get_details(self, connection_details):
2424
self.arguments = connection_details.copy()
2525

2626
def connection_params(self):
27-
from conftest import auth_connect_kwargs
28-
2927
return {
3028
"server_hostname": self.arguments["host"],
3129
"http_path": self.arguments["http_path"],
32-
**auth_connect_kwargs(self.arguments),
30+
"access_token": self.arguments.get("access_token"),
3331
}
3432

3533
@contextmanager

tests/e2e/test_transactions.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,10 @@ def _unique_table_name_raw(suffix):
5656
@pytest.fixture
5757
def mst_conn_params(connection_details):
5858
"""Connection parameters with MST enabled."""
59-
from conftest import auth_connect_kwargs
60-
6159
return {
6260
"server_hostname": connection_details["host"],
6361
"http_path": connection_details["http_path"],
64-
**auth_connect_kwargs(connection_details),
62+
"access_token": connection_details.get("access_token"),
6563
"ignore_transactions": False,
6664
}
6765

0 commit comments

Comments
 (0)