Skip to content

Commit c1eac83

Browse files
authored
Move eng-apps-devex to OWNERTEAMS for approval resolution (#4976)
## Why The `maintainer-approval` workflow uses `@databricks/eng-apps-devex` team references in OWNERS, then calls `teams.getMembershipForUserInOrg()` to check if a reviewer belongs to that team. This API requires `read:org` scope, which `GITHUB_TOKEN` doesn't support (it only has repository-scoped permissions). The API returns 404 for privacy reasons, the code silently treats that as "not a member", and team-based approvals never resolve. This showed up on #4968 where arsenyinfo (a member of eng-apps-devex) approved, but the check stayed pending until a maintainer stepped in. ## Changes We already have an `OWNERTEAMS` mechanism that expands `team:<name>` references to individual logins at parse time, no API calls needed. `team:bundle` and `team:platform` already use it. This PR adds `team:eng-apps-devex` to the same system: - OWNERTEAMS: added `team:eng-apps-devex` with the full team roster (12 members) - OWNERS: replaced all `@databricks/eng-apps-devex` references with `team:eng-apps-devex` - Tests: updated to use `OWNERTEAMS`-based team resolution instead of mocking the GitHub API ## Test plan - [x] All 20 existing maintainer-approval tests pass - [x] Team member approval now resolves via OWNERTEAMS expansion (no API dependency) - [x] Non-team-member approval correctly stays pending - [x] `make ws` passes This pull request was AI-assisted by Isaac.
1 parent 4fc00e4 commit c1eac83

3 files changed

Lines changed: 25 additions & 12 deletions

File tree

.github/OWNERS

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@
1616
/acceptance/labs/ @alexott @nfx
1717

1818
# Apps
19-
/cmd/apps/ @databricks/eng-apps-devex
20-
/cmd/workspace/apps/ @databricks/eng-apps-devex
21-
/libs/apps/ @databricks/eng-apps-devex
22-
/acceptance/apps/ @databricks/eng-apps-devex
19+
/cmd/apps/ team:eng-apps-devex
20+
/cmd/workspace/apps/ team:eng-apps-devex
21+
/libs/apps/ team:eng-apps-devex
22+
/acceptance/apps/ team:eng-apps-devex
2323

2424
# Auth
2525
/cmd/auth/ team:platform
@@ -60,4 +60,4 @@
6060
/internal/ team:platform
6161

6262
# Experimental
63-
/experimental/aitools/ @databricks/eng-apps-devex @lennartkats-db
63+
/experimental/aitools/ team:eng-apps-devex @lennartkats-db

.github/OWNERTEAMS

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,16 @@
11
# Team aliases for OWNERS file.
22
# Use "team:<name>" in OWNERS to reference a team defined here.
33
# Format: team:<name> @member1 @member2 ...
4+
#
5+
# Keep these in sync with actual GitHub team rosters. GITHUB_TOKEN can't
6+
# resolve org team membership via the API, so this file is the source of
7+
# truth for the maintainer-approval workflow.
8+
#
9+
# GitHub team pages:
10+
# bundle: https://github.com/orgs/databricks/teams/cli-maintainers
11+
# platform: https://github.com/orgs/databricks/teams/cli-platform
12+
# eng-apps-devex: https://github.com/orgs/databricks/teams/eng-apps-devex
413

514
team:bundle @andrewnester @anton-107 @denik @janniklasrose @pietern @shreyas-goenka
615
team:platform @simonfaltum @renaudhartert-db @hectorcast-db @parthban-db @tanmay-db @Divyansh-db @tejaskochar-db @mihaimitrea-db @chrisst @rauchy
16+
team:eng-apps-devex @fjakobs @jamesbroadhead @Shridhad @atilafassina @keugenek @arsenyinfo @igrekun @pkosiec @MarioCadenas @pffigueiredo @ditadi @calvarjorge

.github/workflows/maintainer-approval.test.js

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,23 @@ const runModule = require("./maintainer-approval");
88

99
// --- Test helpers ---
1010

11-
function makeTmpOwners(content) {
11+
function makeTmpOwners(content, ownerTeamsContent) {
1212
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "approval-test-"));
1313
const ghDir = path.join(tmpDir, ".github");
1414
fs.mkdirSync(ghDir);
1515
fs.writeFileSync(path.join(ghDir, "OWNERS"), content);
16+
if (ownerTeamsContent) {
17+
fs.writeFileSync(path.join(ghDir, "OWNERTEAMS"), ownerTeamsContent);
18+
}
1619
return tmpDir;
1720
}
1821

22+
const OWNERTEAMS_CONTENT = "team:eng-apps-devex @teamdev1 @teamdev2\n";
23+
1924
const OWNERS_CONTENT = [
2025
"* @maintainer1 @maintainer2",
2126
"/cmd/pipelines/ @jefferycheng1 @kanterov",
22-
"/cmd/apps/ @databricks/eng-apps-devex",
27+
"/cmd/apps/ team:eng-apps-devex",
2328
"/bundle/ @bundleowner",
2429
].join("\n");
2530

@@ -121,7 +126,7 @@ describe("maintainer-approval", () => {
121126

122127
before(() => {
123128
originalWorkspace = process.env.GITHUB_WORKSPACE;
124-
tmpDir = makeTmpOwners(OWNERS_CONTENT);
129+
tmpDir = makeTmpOwners(OWNERS_CONTENT, OWNERTEAMS_CONTENT);
125130
process.env.GITHUB_WORKSPACE = tmpDir;
126131
});
127132

@@ -283,13 +288,12 @@ describe("maintainer-approval", () => {
283288
assert.equal(github._checkRuns.length, 0);
284289
});
285290

286-
it("team member approved -> success for team-owned path", async () => {
291+
it("OWNERTEAMS member approved -> success for team-owned path", async () => {
287292
const github = makeGithub({
288293
reviews: [
289294
{ state: "APPROVED", user: { login: "teamdev1" } },
290295
],
291296
files: [{ filename: "cmd/apps/main.go" }],
292-
teamMembers: { "eng-apps-devex": ["teamdev1"] },
293297
});
294298
const core = makeCore();
295299
const context = makeContext();
@@ -300,13 +304,12 @@ describe("maintainer-approval", () => {
300304
assert.equal(github._checkRuns[0].conclusion, "success");
301305
});
302306

303-
it("non-team-member approval for team-owned path -> pending", async () => {
307+
it("non-OWNERTEAMS-member approval for team-owned path -> pending", async () => {
304308
const github = makeGithub({
305309
reviews: [
306310
{ state: "APPROVED", user: { login: "outsider" } },
307311
],
308312
files: [{ filename: "cmd/apps/main.go" }],
309-
teamMembers: { "eng-apps-devex": [] },
310313
});
311314
const core = makeCore();
312315
const context = makeContext();

0 commit comments

Comments
 (0)