Skip to content

Commit c048d2e

Browse files
fix(bots): give the engineer-bot e2e repro a token the env scrub can't strip
The bug-fix flow's REQUIRED tests/e2e repro authenticates through conftest.py, which reads the token from DATABRICKS_TOKEN. But the engineer-bot runs pytest in an agent-driven subprocess whose environment is scrubbed of every credential- shaped variable (the engine's shared/env_scrub.py strips *TOKEN*/*SECRET*/... ), so DATABRICKS_TOKEN is gone before pytest starts. The connection is then built with access_token=None and hangs — observed on issue #791: every e2e test (incl. existing, unmodified ones) stalled ~10 min per attempt until the 45-min job timeout. The same warehouse/env is used by code-coverage.yml's e2e job, which passes because it runs pytest directly (no agent, no scrub). Fix (per the engine's centralize-bot-workflows design doc): pass the connection details through a file instead of a credential-named env var. - engineer-bot.yml: a new step writes host/http_path/token/user/catalog to $RUNNER_TEMP/e2e-connection.json (built with jq so the secret is never shell- interpolated; chmod 600) and exports its PATH to the author step as DATABRICKS_TEST_CONFIG_FILE — a name env_scrub deliberately preserves. - conftest.py: each connection fixture now falls back to that file when its env var is absent (env var still wins). Normal CI and local dev leave the path var unset, so the file dict is empty and behavior is byte-for-byte unchanged. Author phase only: the follow-up phase deliberately runs mocked tests/unit and forbids tests/e2e, so it needs no live token (left untouched). Signed-off-by: eric-wang-1990 <e.wang@databricks.com> Co-authored-by: Isaac
1 parent fb25502 commit c048d2e

2 files changed

Lines changed: 81 additions & 12 deletions

File tree

.github/workflows/engineer-bot.yml

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,36 @@ jobs:
173173
fi
174174
{ echo "ISSUE_TITLE<<$DELIM"; printf '%s\n' "$TITLE"; echo "$DELIM"; } >> "$GITHUB_ENV"
175175
176+
- name: Write E2E connection config (token survives the agent env scrub)
177+
id: e2e_config
178+
# The bug-fix flow's REQUIRED tests/e2e repro authenticates via the repo's
179+
# conftest, which reads DATABRICKS_TOKEN. But the author agent runs pytest
180+
# in a subprocess whose env is scrubbed of every credential-shaped var
181+
# (engine shared/env_scrub.py strips *TOKEN*/*SECRET*/…), so DATABRICKS_TOKEN
182+
# never reaches the test — the connection then has access_token=None and
183+
# hangs. Workaround (per the engine's centralize-bot-workflows design doc):
184+
# write the connection details to a file and pass its PATH in
185+
# DATABRICKS_TEST_CONFIG_FILE, a var the scrub deliberately preserves; the
186+
# conftest reads it as a fallback. Written here (an ordinary step, token in
187+
# scope) with jq so the secret is never shell-interpolated.
188+
env:
189+
DATABRICKS_SERVER_HOSTNAME: ${{ secrets.DATABRICKS_HOST }}
190+
DATABRICKS_HTTP_PATH: ${{ secrets.TEST_PECO_WAREHOUSE_HTTP_PATH }}
191+
DATABRICKS_TOKEN: ${{ secrets.DATABRICKS_TOKEN }}
192+
DATABRICKS_USER: ${{ secrets.TEST_PECO_SP_ID }}
193+
run: |
194+
CONFIG_PATH="$RUNNER_TEMP/e2e-connection.json"
195+
jq -n \
196+
--arg host "$DATABRICKS_SERVER_HOSTNAME" \
197+
--arg http_path "$DATABRICKS_HTTP_PATH" \
198+
--arg access_token "$DATABRICKS_TOKEN" \
199+
--arg ingestion_user "$DATABRICKS_USER" \
200+
--arg catalog "peco" \
201+
'{host: $host, http_path: $http_path, access_token: $access_token, ingestion_user: $ingestion_user, catalog: $catalog}' \
202+
> "$CONFIG_PATH"
203+
chmod 600 "$CONFIG_PATH"
204+
echo "path=$CONFIG_PATH" >> "$GITHUB_OUTPUT"
205+
176206
- name: Run author
177207
id: author
178208
# Run from RUNNER_TEMP so the engine-rendered prompt's context file
@@ -201,6 +231,12 @@ jobs:
201231
DATABRICKS_HTTP_PATH: ${{ secrets.TEST_PECO_WAREHOUSE_HTTP_PATH }}
202232
DATABRICKS_CATALOG: peco
203233
DATABRICKS_USER: ${{ secrets.TEST_PECO_SP_ID }}
234+
# The connection TOKEN can't ride a plain env var into the agent's test
235+
# subprocess (env_scrub strips *TOKEN*). Pass the path to the config file
236+
# written above instead — this var name survives the scrub, and the
237+
# conftest reads access_token from it. (host/http_path/catalog/user are
238+
# not credential-shaped, so they still arrive via the env vars above.)
239+
DATABRICKS_TEST_CONFIG_FILE: ${{ steps.e2e_config.outputs.path }}
204240
REPO_ROOT: ${{ github.workspace }}
205241
RUNNER_TEMP: ${{ runner.temp }}
206242
FLOW: ${{ steps.ctx.outputs.flow }}

conftest.py

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,68 @@
1+
import json
12
import os
3+
24
import pytest
35

46

7+
def _test_config_from_file():
8+
"""Connection details from the JSON file named by ``DATABRICKS_TEST_CONFIG_FILE``,
9+
or ``{}`` when the variable is unset/empty/unreadable.
10+
11+
Why this indirection exists: the engineer-bot (databricks-bot-engine) runs
12+
this e2e suite inside an agent-driven subprocess whose environment has every
13+
credential-shaped variable — anything matching ``*TOKEN*`` / ``*SECRET*`` /
14+
``*PASSWORD*`` etc. — stripped for safety (the engine's ``shared/env_scrub.py``).
15+
``DATABRICKS_TOKEN`` is therefore removed before pytest starts, so the token
16+
can't reach the agent's test run as a plain env var. The bot instead writes the
17+
connection details (token included) to a file and points at it with
18+
``DATABRICKS_TEST_CONFIG_FILE`` — a name the scrub deliberately preserves.
19+
20+
Normal CI (and local dev) leaves the variable unset, so this returns ``{}`` and
21+
every fixture below resolves purely from its env var, unchanged.
22+
"""
23+
path = os.getenv("DATABRICKS_TEST_CONFIG_FILE")
24+
if not path:
25+
return {}
26+
try:
27+
with open(path) as f:
28+
return json.load(f)
29+
except (OSError, ValueError):
30+
return {}
31+
32+
33+
@pytest.fixture(scope="session")
34+
def _test_config():
35+
return _test_config_from_file()
36+
37+
538
@pytest.fixture(scope="session")
6-
def host():
7-
return os.getenv("DATABRICKS_SERVER_HOSTNAME")
39+
def host(_test_config):
40+
return os.getenv("DATABRICKS_SERVER_HOSTNAME") or _test_config.get("host")
841

942

1043
@pytest.fixture(scope="session")
11-
def http_path():
12-
return os.getenv("DATABRICKS_HTTP_PATH")
44+
def http_path(_test_config):
45+
return os.getenv("DATABRICKS_HTTP_PATH") or _test_config.get("http_path")
1346

1447

1548
@pytest.fixture(scope="session")
16-
def access_token():
17-
return os.getenv("DATABRICKS_TOKEN")
49+
def access_token(_test_config):
50+
return os.getenv("DATABRICKS_TOKEN") or _test_config.get("access_token")
1851

1952

2053
@pytest.fixture(scope="session")
21-
def ingestion_user():
22-
return os.getenv("DATABRICKS_USER")
54+
def ingestion_user(_test_config):
55+
return os.getenv("DATABRICKS_USER") or _test_config.get("ingestion_user")
2356

2457

2558
@pytest.fixture(scope="session")
26-
def catalog():
27-
return os.getenv("DATABRICKS_CATALOG")
59+
def catalog(_test_config):
60+
return os.getenv("DATABRICKS_CATALOG") or _test_config.get("catalog")
2861

2962

3063
@pytest.fixture(scope="session")
31-
def schema():
32-
return os.getenv("DATABRICKS_SCHEMA", "default")
64+
def schema(_test_config):
65+
return os.getenv("DATABRICKS_SCHEMA") or _test_config.get("schema") or "default"
3366

3467

3568
@pytest.fixture(scope="session", autouse=True)

0 commit comments

Comments
 (0)