Validate OWNERS/OWNERTEAMS consistency in CI

.github/OWNERS

@@ -13,7 +13,6 @@
 
 # Labs
 /cmd/labs/                  @alexott @asnare
-/acceptance/labs/           @alexott @asnare
 
 # Apps
 /cmd/apps/                  team:eng-apps-devex

.github/scripts/owners.js

@@ -137,4 +137,137 @@ function getOwnershipGroups(filenames, rules) {
   return groups;
 }
 
-module.exports = { parseOwnerTeams, parseOwnersFile, ownersMatch, findOwners, getMaintainers, getOwnershipGroups };
+/**
+ * Parse OWNERS into raw rules WITHOUT expanding team aliases.
+ * Unlike parseOwnersFile, this keeps the original tokens ("team:x", "@user")
+ * so the validator can tell defined from undefined teams and count owners.
+ *
+ * @param {string} filePath - absolute path to the OWNERS file
+ * @returns {Array<{pattern: string, tokens: string[]}>}
+ */
+function parseOwnersRules(filePath) {
+  return readDataLines(filePath).map((parts) => ({
+    pattern: parts[0],
+    tokens: parts.slice(1),
+  }));
+}
+
+/**
+ * Parse the GitHub team-page URLs declared in the OWNERTEAMS header comment.
+ * Format: "#   <name>: https://github.com/orgs/databricks/teams/<slug>"
+ * Returns the set of "team:<name>" aliases that have a documented page.
+ *
+ * @param {string} filePath - absolute path to the OWNERTEAMS file
+ * @returns {Set<string>}
+ */
+function parseTeamPageUrls(filePath) {
+  let content;
+  try {
+    content = fs.readFileSync(filePath, "utf-8");
+  } catch (e) {
+    if (e.code === "ENOENT") return new Set();
+    throw e;
+  }
+  const pages = new Set();
+  for (const raw of content.split("\n")) {
+    const m = raw.match(/^#\s*([a-z0-9-]+):\s*https?:\/\//);
+    if (m) pages.add("team:" + m[1]);
+  }
+  return pages;
+}
+
+/**
+ * Validate OWNERS and OWNERTEAMS for internal consistency.
+ *
+ * Errors (block CI):
+ *   - a rule references a "team:" alias not defined in OWNERTEAMS
+ *   - a rule resolves to zero owners (only a maintainer could ever approve it)
+ *   - a rule maps a path that does not exist in the repository
+ *
+ * Warnings (reported, non-blocking): a defined team has no team-page URL in the
+ * OWNERTEAMS header. A team may legitimately predate its GitHub team page, so
+ * this never blocks a merge.
+ *
+ * fileExists is injectable so tests can validate synthetic rules without a tree.
+ *
+ * @param {string} ownersPath
+ * @param {string} teamsPath
+ * @param {{ repoRoot?: string, fileExists?: (p: string) => boolean }} [opts]
+ * @returns {{ errors: string[], warnings: string[] }}
+ */
+function validateOwners(ownersPath, teamsPath, opts) {
+  const repoRoot = (opts && opts.repoRoot) || process.cwd();
+  const fileExists = (opts && opts.fileExists) || ((p) => fs.existsSync(p));
+  const teams = parseOwnerTeams(teamsPath);
+  const pageUrls = parseTeamPageUrls(teamsPath);
+  const errors = [];
+  const warnings = [];
+
+  for (const { pattern, tokens } of parseOwnersRules(ownersPath)) {
+    let ownerCount = 0;
+    let undefinedTeam = false;
+    for (const token of tokens) {
+      if (token.startsWith("team:")) {
+        if (teams.has(token)) {
+          ownerCount += teams.get(token).length;
+        } else {
+          errors.push(`rule "${pattern}" references undefined team "${token}"; define it in .github/OWNERTEAMS`);
+          undefinedTeam = true;
+        }
+      } else if (token.startsWith("@")) {
+        ownerCount += 1;
+      }
+    }
+    // Skip the zero-owner error when an undefined team already explains it.
+    if (ownerCount === 0 && !undefinedTeam) {
+      errors.push(`rule "${pattern}" resolves to zero owners`);
+    }
+    if (pattern !== "*") {
+      const rel = pattern.replace(/^\//, "").replace(/\/$/, "");
+      if (rel && !fileExists(path.join(repoRoot, rel))) {
+        errors.push(`rule "${pattern}" maps a path that does not exist in the repo`);
+      }
+    }
+  }
+
+  for (const team of teams.keys()) {
+    if (!pageUrls.has(team)) {
+      warnings.push(`team "${team}" has no GitHub team-page URL in the .github/OWNERTEAMS header comment`);
+    }
+  }
+
+  return { errors, warnings };
+}
+
+module.exports = {
+  parseOwnerTeams,
+  parseOwnersFile,
+  parseOwnersRules,
+  parseTeamPageUrls,
+  ownersMatch,
+  findOwners,
+  getMaintainers,
+  getOwnershipGroups,
+  validateOwners,
+};
+
+// CLI entrypoint: `node .github/scripts/owners.js validate`
+if (require.main === module) {
+  if (process.argv[2] !== "validate") {
+    console.error("usage: node .github/scripts/owners.js validate");
+    process.exit(2);
+  }
+  const root = process.cwd();
+  const { errors, warnings } = validateOwners(
+    path.join(root, ".github", "OWNERS"),
+    path.join(root, ".github", "OWNERTEAMS"),
+    { repoRoot: root },
+  );
+  for (const w of warnings) console.warn(`warning: ${w}`);
+  for (const e of errors) console.error(`error: ${e}`);
+  if (errors.length > 0) {
+    console.error(`OWNERS validation failed: ${errors.length} error(s), ${warnings.length} warning(s)`);
+    process.exit(1);
+  }
+  console.log(`OWNERS validation passed: ${warnings.length} warning(s)`);
+}

.github/scripts/owners.test.js

@@ -8,9 +8,12 @@ const {
   parseOwnerTeams,
   ownersMatch,
   parseOwnersFile,
+  parseOwnersRules,
+  parseTeamPageUrls,
   findOwners,
   getMaintainers,
   getOwnershipGroups,
+  validateOwners,
 } = require("./owners");
 
 // --- ownersMatch ---
@@ -348,3 +351,167 @@ describe("getOwnershipGroups", () => {
     assert.ok(groups.has("*"));
   });
 });
+
+// --- parseOwnersRules ---
+
+describe("parseOwnersRules", () => {
+  let tmpDir;
+  let ownersPath;
+
+  before(() => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "owners-raw-"));
+    ownersPath = path.join(tmpDir, "OWNERS");
+  });
+
+  after(() => {
+    fs.rmSync(tmpDir, { recursive: true });
+  });
+
+  it("keeps team aliases and @users un-expanded", () => {
+    fs.writeFileSync(ownersPath, "/cmd/auth/ team:platform @carol\n* @alice\n");
+    const rules = parseOwnersRules(ownersPath);
+    assert.equal(rules.length, 2);
+    assert.deepEqual(rules[0], {
+      pattern: "/cmd/auth/",
+      tokens: ["team:platform", "@carol"],
+    });
+    assert.deepEqual(rules[1], { pattern: "*", tokens: ["@alice"] });
+  });
+});
+
+// --- parseTeamPageUrls ---
+
+describe("parseTeamPageUrls", () => {
+  let tmpDir;
+  let teamsPath;
+
+  before(() => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "team-urls-"));
+    teamsPath = path.join(tmpDir, "OWNERTEAMS");
+  });
+
+  after(() => {
+    fs.rmSync(tmpDir, { recursive: true });
+  });
+
+  it("collects team-page URLs from header comments", () => {
+    fs.writeFileSync(
+      teamsPath,
+      [
+        "# GitHub team pages:",
+        "#   platform:  https://github.com/orgs/databricks/teams/cli-platform",
+        "#   bundle:    https://github.com/orgs/databricks/teams/cli-maintainers",
+        "team:platform @alice",
+      ].join("\n")
+    );
+    const pages = parseTeamPageUrls(teamsPath);
+    assert.equal(pages.size, 2);
+    assert.ok(pages.has("team:platform"));
+    assert.ok(pages.has("team:bundle"));
+  });
+
+  it("ignores header lines that are not team-page URLs", () => {
+    fs.writeFileSync(
+      teamsPath,
+      [
+        '# Use "team:<name>" in OWNERS to reference a team.',
+        "# Format: team:<name> @member1",
+        "team:platform @alice",
+      ].join("\n")
+    );
+    assert.equal(parseTeamPageUrls(teamsPath).size, 0);
+  });
+});
+
+// --- validateOwners ---
+
+describe("validateOwners", () => {
+  let tmpDir;
+  let ownersPath;
+  let teamsPath;
+  const allExist = () => true;
+
+  before(() => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "owners-validate-"));
+    ownersPath = path.join(tmpDir, "OWNERS");
+    teamsPath = path.join(tmpDir, "OWNERTEAMS");
+  });
+
+  after(() => {
+    fs.rmSync(tmpDir, { recursive: true });
+  });
+
+  function write(owners, teams) {
+    fs.writeFileSync(ownersPath, owners);
+    fs.writeFileSync(teamsPath, teams);
+  }
+
+  it("passes a consistent OWNERS/OWNERTEAMS pair", () => {
+    write(
+      "* @alice\n/cmd/auth/ team:platform\n",
+      "#   platform: https://github.com/orgs/databricks/teams/cli-platform\nteam:platform @bob @carol\n"
+    );
+    const { errors, warnings } = validateOwners(ownersPath, teamsPath, {
+      fileExists: allExist,
+    });
+    assert.deepEqual(errors, []);
+    assert.deepEqual(warnings, []);
+  });
+
+  it("errors on a team alias not defined in OWNERTEAMS", () => {
+    write("* @alice\n/cmd/auth/ team:platfrom\n", "team:platform @bob\n");
+    const { errors } = validateOwners(ownersPath, teamsPath, {
+      fileExists: allExist,
+    });
+    assert.equal(errors.length, 1);
+    assert.match(errors[0], /undefined team "team:platfrom"/);
+  });
+
+  it("does not also report zero owners when a team is undefined", () => {
+    write("/cmd/auth/ team:nope\n", "team:platform @bob\n");
+    const { errors } = validateOwners(ownersPath, teamsPath, {
+      fileExists: allExist,
+    });
+    assert.equal(errors.length, 1);
+    assert.match(errors[0], /undefined team/);
+  });
+
+  it("errors when a rule resolves to zero owners (org team missing @)", () => {
+    write("/cmd/auth/ databricks/eng-apps-devex\n", "team:platform @bob\n");
+    const { errors } = validateOwners(ownersPath, teamsPath, {
+      fileExists: allExist,
+    });
+    assert.equal(errors.length, 1);
+    assert.match(errors[0], /zero owners/);
+  });
+
+  it("errors when a rule maps a path that does not exist", () => {
+    write(
+      "* @alice\n/acceptance/ghost/ team:platform\n",
+      "#   platform: https://github.com/orgs/databricks/teams/cli-platform\nteam:platform @bob\n"
+    );
+    const { errors } = validateOwners(ownersPath, teamsPath, {
+      fileExists: (p) => !p.includes("ghost"),
+    });
+    assert.equal(errors.length, 1);
+    assert.match(errors[0], /does not exist/);
+  });
+
+  it("does not path-check the * catch-all rule", () => {
+    write("* @alice\n", "team:platform @bob\n");
+    const { errors } = validateOwners(ownersPath, teamsPath, {
+      fileExists: () => false,
+    });
+    assert.deepEqual(errors, []);
+  });
+
+  it("warns (does not error) when a defined team has no team-page URL", () => {
+    write("* @alice\n/cmd/auth/ team:newteam\n", "team:newteam @bob\n");
+    const { errors, warnings } = validateOwners(ownersPath, teamsPath, {
+      fileExists: allExist,
+    });
+    assert.deepEqual(errors, []);
+    assert.equal(warnings.length, 1);
+    assert.match(warnings[0], /team-page URL/);
+  });
+});

.github/workflows/test-owners-scripts.yml

@@ -6,7 +6,9 @@ on:
       - '.github/scripts/**'
       - '.github/workflows/maintainer-approval.js'
       - '.github/workflows/maintainer-approval.test.js'
+      - '.github/workflows/test-owners-scripts.yml'
       - '.github/OWNERS'
+      - '.github/OWNERTEAMS'
 
 jobs:
   test:
@@ -15,11 +17,10 @@ jobs:
       labels: ubuntu-latest-deco
     timeout-minutes: 5
     steps:
+      # Full checkout (no sparse-checkout): `owners.js validate` verifies that
+      # every OWNERS path exists in the tree, so it needs the whole repo.
       - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
-        with:
-          sparse-checkout: |
-            .github/scripts
-            .github/workflows
-            .github/OWNERS
       - name: Run OWNERS script tests
         run: node --test .github/scripts/owners.test.js .github/workflows/maintainer-approval.test.js
+      - name: Validate OWNERS and OWNERTEAMS
+        run: node .github/scripts/owners.js validate