[kbn-code-owners] General improvements (#204023)

## Summary
The following improvements have been made:
- Added `--json` flag to CLI command to output result as JSON
- Terminology updated to more accurately reflect object contents
- Code owner teams are always returned as an array
- Search path validation (is under repo root, exists)
- Proper handling of inline comments
- Better logging for `scripts/check_ftr_code_owners.js`

Existing usage of the `@kbn/code-owners` package has been updated
accordingly, without modifying outcomes.

packages/kbn-code-owners/index.ts

@@ -7,9 +7,10 @@
  * License v3.0 only", or the "Server Side Public License, v 1".
  */
 
-export type { PathWithOwners, CodeOwnership } from './src/file_code_owner';
+export type { CodeOwnersEntry } from './src/code_owners';
+export * as cli from './src/cli';
 export {
-  getPathsWithOwnersReversed,
+  getCodeOwnersEntries,
-  getCodeOwnersForFile,
+  findCodeOwnersEntryForPath,
-  runGetOwnersForFileCli,
+  getOwningTeamsForPath,
-} from './src/file_code_owner';
+} from './src/code_owners';

packages/kbn-code-owners/src/cli.ts

@@ -0,0 +1,55 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+import { run } from '@kbn/dev-cli-runner';
+import { findCodeOwnersEntryForPath } from './code_owners';
+import { throwIfPathIsMissing, throwIfPathNotInRepo } from './path';
+
+/**
+ * CLI entrypoint for finding code owners for a given path.
+ */
+export async function findCodeOwnersForPath() {
+  await run(
+    async ({ flagsReader, log }) => {
+      const targetPath = flagsReader.requiredPath('path');
+      throwIfPathIsMissing(targetPath, 'Target path', true);
+      throwIfPathNotInRepo(targetPath, true);
+
+      const codeOwnersEntry = findCodeOwnersEntryForPath(targetPath);
+
+      if (!codeOwnersEntry) {
+        log.warning(`No matching code owners entry found for path ${targetPath}`);
+        return;
+      }
+
+      if (flagsReader.boolean('json')) {
+        // Replacer function that hides irrelevant fields in JSON output
+        const hideIrrelevantFields = (k: string, v: any) => {
+          return ['matcher'].includes(k) ? undefined : v;
+        };
+
+        log.write(JSON.stringify(codeOwnersEntry, hideIrrelevantFields, 2));
+        return;
+      }
+
+      log.write(`Matching pattern: ${codeOwnersEntry.pattern}`);
+      log.write('Teams:', codeOwnersEntry.teams);
+    },
+    {
+      description: `Find code owners for a given path in this local Kibana repository`,
+      flags: {
+        string: ['path'],
+        boolean: ['json'],
+        help: `
+        --path             Path to find owners for (required)
+        --json             Output result as JSON`,
+      },
+    }
+  );
+}

packages/kbn-code-owners/src/code_owners.ts

@@ -0,0 +1,127 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+import { REPO_ROOT } from '@kbn/repo-info';
+import fs from 'node:fs';
+import path from 'node:path';
+
+import ignore, { Ignore } from 'ignore';
+import { CODE_OWNERS_FILE, throwIfPathIsMissing, throwIfPathNotInRepo } from './path';
+
+export interface CodeOwnersEntry {
+  pattern: string;
+  matcher: Ignore;
+  teams: string[];
+  comment?: string;
+}
+
+/**
+ * Generator function that yields lines from the CODEOWNERS file
+ */
+export function* getCodeOwnersLines(): Generator<string> {
+  const codeOwnerLines = fs
+    .readFileSync(CODE_OWNERS_FILE, { encoding: 'utf8', flag: 'r' })
+    .split(/\r?\n/);
+
+  for (const line of codeOwnerLines) {
+    // Empty line
+    if (line.length === 0) continue;
+
+    // Comment
+    if (line.startsWith('#')) continue;
+
+    // Assignment override on backport branches to avoid review requests
+    if (line.includes('@kibanamachine')) continue;
+
+    yield line.trim();
+  }
+}
+
+/**
+ * Get all code owner entries from the CODEOWNERS file
+ *
+ * Entries are ordered in reverse relative to how they're defined in the CODEOWNERS file
+ * as patterns defined lower in the CODEOWNERS file can override earlier entries.
+ */
+export function getCodeOwnersEntries(): CodeOwnersEntry[] {
+  const entries: CodeOwnersEntry[] = [];
+
+  for (const line of getCodeOwnersLines()) {
+    const comment = line
+      .match(/#(.+)$/)
+      ?.at(1)
+      ?.trim();
+
+    const [rawPathPattern, ...rawTeams] = line
+      .replace(/#.+$/, '') // drop comment
+      .split(/\s+/);
+
+    const pathPattern = rawPathPattern.replace(/\/$/, '');
+
+    entries.push({
+      pattern: pathPattern,
+      teams: rawTeams.map((t) => t.replace('@', '')).filter((t) => t.length > 0),
+      comment,
+
+      // Register code owner entry with the `ignores` lib for easy pattern matching later on
+      matcher: ignore().add(pathPattern),
+    });
+  }
+
+  // Reverse entry order as patterns defined lower in the CODEOWNERS file can override earlier entries
+  entries.reverse();
+
+  return entries;
+}
+
+/**
+ * Get a list of matching code owners for a given path
+ *
+ * Tip:
+ *   If you're making a lot of calls to this function, fetch the code owner paths once using
+ *   `getCodeOwnersEntries` and pass it in the `getCodeOwnersEntries` parameter to speed up your queries..
+ *
+ * @param searchPath The path to find code owners for
+ * @param codeOwnersEntries Pre-defined list of code owner paths to search in
+ *
+ * @returns Code owners entry if a match is found.
+ * @throws Error if `searchPath` does not exist or is not part of this repository
+ */
+export function findCodeOwnersEntryForPath(
+  searchPath: string,
+  codeOwnersEntries?: CodeOwnersEntry[]
+): CodeOwnersEntry | undefined {
+  throwIfPathIsMissing(CODE_OWNERS_FILE, 'Code owners file');
+  throwIfPathNotInRepo(searchPath);
+  const searchPathRelativeToRepo = path.relative(REPO_ROOT, searchPath);
+
+  return (codeOwnersEntries || getCodeOwnersEntries()).find(
+    (p) => p.matcher.test(searchPathRelativeToRepo).ignored
+  );
+}
+
+/**
+ * Get a list of matching code owners for a given path
+ *
+ * Tip:
+ *   If you're making a lot of calls to this function, fetch the code owner paths once using
+ *   `getCodeOwnersEntries` and pass it in the `getCodeOwnersEntries` parameter to speed up your queries.
+ *
+ * @param searchPath The path to find code owners for
+ * @param codeOwnersEntries Pre-defined list of code owner entries
+ *
+ * @returns List of code owners for the given path. Empty list if no matching entry is found.
+ * @throws Error if `searchPath` does not exist or is not part of this repository
+ */
+export function getOwningTeamsForPath(
+  searchPath: string,
+  codeOwnersEntries?: CodeOwnersEntry[]
+): string[] {
+  return findCodeOwnersEntryForPath(searchPath, codeOwnersEntries)?.teams || [];
+}

packages/kbn-code-owners/src/file_code_owner.ts

@@ -1,106 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the "Elastic License
- * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
- * Public License v 1"; you may not use this file except in compliance with, at
- * your election, the "Elastic License 2.0", the "GNU Affero General Public
- * License v3.0 only", or the "Server Side Public License, v 1".
- */
-
-import { REPO_ROOT } from '@kbn/repo-info';
-import { createFailError, createFlagError } from '@kbn/dev-cli-errors';
-import { join as joinPath } from 'path';
-import { existsSync, readFileSync } from 'fs';
-import { run } from '@kbn/dev-cli-runner';
-
-import type { Ignore } from 'ignore';
-import ignore from 'ignore';
-
-export interface PathWithOwners {
-  path: string;
-  teams: string;
-  ignorePattern: Ignore;
-}
-export type CodeOwnership = Partial<Pick<PathWithOwners, 'path' | 'teams'>> | undefined;
-
-const existOrThrow = (targetFile: string) => {
-  if (existsSync(targetFile) === false)
-    throw createFailError(`Unable to determine code owners: file ${targetFile} Not Found`);
-};
-
-/**
- * Get the .github/CODEOWNERS entries, prepared for path matching.
- * The last matching CODEOWNERS entry has highest precedence:
- * https://help.github.com/articles/about-codeowners/
- * so entries are returned in reversed order to later search for the first match.
- */
-export function getPathsWithOwnersReversed(): PathWithOwners[] {
-  const codeownersPath = joinPath(REPO_ROOT, '.github', 'CODEOWNERS');
-  existOrThrow(codeownersPath);
-  const codeownersContent = readFileSync(codeownersPath, { encoding: 'utf8', flag: 'r' });
-  const codeownersLines = codeownersContent.split(/\r?\n/);
-  const codeowners = codeownersLines
-    .map((line) => line.trim())
-    .filter((line) => line && line[0] !== '#')
-    // kibanamachine is an assignment override on backport branches to avoid review requests
-    .filter((line) => line && !line.includes('@kibanamachine'));
-
-  const pathsWithOwners: PathWithOwners[] = codeowners.map((c) => {
-    const [path, ...ghTeams] = c.split(/\s+/);
-    const cleanedPath = path.replace(/\/$/, ''); // remove trailing slash
-    return {
-      path: cleanedPath,
-      teams: ghTeams.map((t) => t.replace('@', '')).join(),
-      // register CODEOWNERS entries with the `ignores` lib for later path matching
-      ignorePattern: ignore().add([cleanedPath]),
-    };
-  });
-
-  return pathsWithOwners.reverse();
-}
-
-/**
- * Get the GitHub CODEOWNERS for a file in the repository
- * @param filePath the file to get code owners for
- * @param reversedCodeowners a cached reversed code owners list, use to speed up multiple requests
- */
-export function getCodeOwnersForFile(
-  filePath: string,
-  reversedCodeowners?: PathWithOwners[]
-): CodeOwnership {
-  const pathsWithOwners = reversedCodeowners ?? getPathsWithOwnersReversed();
-  const match = pathsWithOwners.find((p) => p.ignorePattern.test(filePath).ignored);
-  if (match?.path && match.teams) return { path: match.path, teams: match.teams };
-  return;
-}
-const trimFrontSlash = (x: string): string => x.replace(/^\//, '');
-/**
- * Run the getCodeOwnersForFile() method above.
- * Report back to the cli with either success and the owner(s), or a failure.
- *
- * This function depends on a --file param being passed on the cli, like this:
- * $ node scripts/get_owners_for_file.js --file SOME-FILE
- */
-export async function runGetOwnersForFileCli() {
-  run(
-    async ({ flags, log }) => {
-      const targetFile = flags.file as string;
-      if (!targetFile) throw createFlagError(`Missing --file argument`);
-      existOrThrow(targetFile); // This call is duplicated in getPathsWithOwnersReversed(), so this is a short circuit
-      const result = getCodeOwnersForFile(targetFile);
-      if (result)
-        log.success(`Found matching entry in .github/CODEOWNERS:
-${trimFrontSlash(result?.path ? result.path : '')} ${result.teams}`);
-      else log.error(`Ownership of file [${targetFile}] is UNKNOWN`);
-    },
-    {
-      description: 'Report file ownership from GitHub CODEOWNERS file.',
-      flags: {
-        string: ['file'],
-        help: `
-      --file             Required, path to the file to report owners for.
-              `,
-      },
-    }
-  );
-}

packages/kbn-code-owners/src/path.ts

@@ -0,0 +1,48 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+import fs from 'node:fs';
+import path from 'node:path';
+import { createFailError } from '@kbn/dev-cli-errors';
+import { REPO_ROOT } from '@kbn/repo-info';
+
+/** CODEOWNERS file path **/
+export const CODE_OWNERS_FILE = path.join(REPO_ROOT, '.github', 'CODEOWNERS');
+
+/**
+ * Throw an error if the given path does not exist
+ *
+ * @param targetPath Path to check
+ * @param description Path description used in the error message if an exception is thrown
+ * @param cli Whether this function is called from a CLI context
+ */
+export function throwIfPathIsMissing(
+  targetPath: fs.PathLike,
+  description = 'File',
+  cli: boolean = false
+) {
+  if (fs.existsSync(targetPath)) return;
+  const msg = `${description} ${targetPath} does not exist`;
+  throw cli ? createFailError(msg) : new Error(msg);
+}
+
+/**
+ * Throw an error if the given path does not reside in this repo
+ *
+ * @param targetPath Path to check
+ * @param cli Whether this function is called from a CLI context
+ */
+export function throwIfPathNotInRepo(targetPath: fs.PathLike, cli: boolean = false) {
+  const relativePath = path.relative(REPO_ROOT, targetPath.toString());
+
+  if (relativePath.includes('../')) {
+    const msg = `Path ${targetPath} is not part of this repository.`;
+    throw cli ? createFailError(msg) : new Error(msg);
+  }
+}

packages/kbn-scout-reporting/src/reporting/playwright.ts

@@ -24,9 +24,9 @@ import { SCOUT_REPORT_OUTPUT_ROOT } from '@kbn/scout-info';
 import stripANSI from 'strip-ansi';
 import { REPO_ROOT } from '@kbn/repo-info';
 import {
-  type PathWithOwners,
+  type CodeOwnersEntry,
-  getPathsWithOwnersReversed,
+  getCodeOwnersEntries,
-  getCodeOwnersForFile,
+  getOwningTeamsForPath,
 } from '@kbn/code-owners';
 import { generateTestRunId, getTestIDForTitle, ScoutReport, ScoutReportEventAction } from '.';
 import { environmentMetadata } from '../datasources';
@@ -47,7 +47,7 @@ export class ScoutPlaywrightReporter implements Reporter {
   readonly name: string;
   readonly runId: string;
   private report: ScoutReport;
-  private readonly pathsWithOwners: PathWithOwners[];
+  private readonly codeOwnersEntries: CodeOwnersEntry[];
 
   constructor(private reporterOptions: ScoutPlaywrightReporterOptions = {}) {
     this.log = new ToolingLog({
@@ -60,20 +60,11 @@ export class ScoutPlaywrightReporter implements Reporter {
     this.log.info(`Scout test run ID: ${this.runId}`);
 
     this.report = new ScoutReport(this.log);
-    this.pathsWithOwners = getPathsWithOwnersReversed();
+    this.codeOwnersEntries = getCodeOwnersEntries();
   }
 
   private getFileOwners(filePath: string): string[] {
-    const concatenatedOwners = getCodeOwnersForFile(filePath, this.pathsWithOwners)?.teams;
+    return getOwningTeamsForPath(filePath, this.codeOwnersEntries);
-
-    if (concatenatedOwners === undefined) {
-      return [];
-    }
-
-    return concatenatedOwners
-      .replace(/#.+$/, '')
-      .split(',')
-      .filter((value) => value.length > 0);
   }
 
   /**

packages/kbn-test/src/functional_test_runner/lib/mocha/reporter/scout_ftr_reporter.ts

@@ -19,9 +19,9 @@ import {
   datasources,
 } from '@kbn/scout-reporting';
 import {
-  getCodeOwnersForFile,
+  getOwningTeamsForPath,
-  getPathsWithOwnersReversed,
+  getCodeOwnersEntries,
-  type PathWithOwners,
+  type CodeOwnersEntry,
 } from '@kbn/code-owners';
 import { Runner, Test } from '../../../fake_mocha_types';
 
@@ -41,7 +41,7 @@ export class ScoutFTRReporter {
   readonly name: string;
   readonly runId: string;
   private report: ScoutReport;
-  private readonly pathsWithOwners: PathWithOwners[];
+  private readonly codeOwnersEntries: CodeOwnersEntry[];
 
   constructor(private runner: Runner, private reporterOptions: ScoutFTRReporterOptions = {}) {
     this.log = new ToolingLog({
@@ -54,7 +54,7 @@ export class ScoutFTRReporter {
     this.log.info(`Scout test run ID: ${this.runId}`);
 
     this.report = new ScoutReport(this.log);
-    this.pathsWithOwners = getPathsWithOwnersReversed();
+    this.codeOwnersEntries = getCodeOwnersEntries();
 
     // Register event listeners
     for (const [eventName, listener] of Object.entries({
@@ -68,16 +68,7 @@ export class ScoutFTRReporter {
   }
 
   private getFileOwners(filePath: string): string[] {
-    const concatenatedOwners = getCodeOwnersForFile(filePath, this.pathsWithOwners)?.teams;
+    return getOwningTeamsForPath(filePath, this.codeOwnersEntries);
-
-    if (concatenatedOwners === undefined) {
-      return [];
-    }
-
-    return concatenatedOwners
-      .replace(/#.+$/, '')
-      .split(',')
-      .filter((value) => value.length > 0);
   }
 
   /**

packages/kbn-test/src/functional_test_runner/run_check_ftr_code_owners.ts

@@ -10,11 +10,7 @@
 import { run } from '@kbn/dev-cli-runner';
 import { createFailError } from '@kbn/dev-cli-errors';
 import { getRepoFiles } from '@kbn/get-repo-files';
-import {
+import { getOwningTeamsForPath, getCodeOwnersEntries } from '@kbn/code-owners';
-  getCodeOwnersForFile,
-  getPathsWithOwnersReversed,
-  type CodeOwnership,
-} from '@kbn/code-owners';
 
 const TEST_DIRECTORIES = ['test', 'x-pack/test', 'x-pack/test_serverless'];
 
@@ -36,15 +32,22 @@ export async function runCheckFtrCodeOwnersCli() {
       const missingOwners = new Set<string>();
 
       // cache codeowners for quicker lookup
-      const reversedCodeowners = getPathsWithOwnersReversed();
+      log.info('Reading CODEOWNERS file');
+      const codeOwnersEntries = getCodeOwnersEntries();
 
       const testFiles = await getRepoFiles(TEST_DIRECTORIES);
+      log.info(`Checking ownership for ${testFiles.length} test files (this will take a while)`);
+
       for (const { repoRel } of testFiles) {
-        const owners: CodeOwnership = getCodeOwnersForFile(repoRel, reversedCodeowners);
+        const owners = getOwningTeamsForPath(repoRel, codeOwnersEntries);
-        if (owners === undefined || owners.teams === '') missingOwners.add(repoRel);
+
+        if (owners.length === 0) {
+          missingOwners.add(repoRel);
+        }
       }
 
       const timeSpent = fmtMs(performance.now() - start);
+      log.info(`Ownership check complete (took ${timeSpent})`);
 
       if (missingOwners.size) {
         log.error(
@@ -55,9 +58,7 @@ export async function runCheckFtrCodeOwnersCli() {
         );
       }
 
-      log.success(
+      log.success(`All test files have a code owner. 🥳`);
-        `All test files have a code owner (checked ${testFiles.length} test files in ${timeSpent})`
-      );
     },
     {
       description: 'Check that all test files are covered by GitHub CODEOWNERS',

packages/kbn-test/src/mocha/junit_report_generation.js

@@ -8,7 +8,7 @@
  */
 
 import { REPO_ROOT } from '@kbn/repo-info';
-import { getCodeOwnersForFile, getPathsWithOwnersReversed } from '@kbn/code-owners';
+import { getOwningTeamsForPath, getCodeOwnersEntries } from '@kbn/code-owners';
 import { dirname, relative } from 'path';
 import { writeFileSync, mkdirSync } from 'fs';
 import { inspect } from 'util';
@@ -94,10 +94,10 @@ export function setupJUnitReportGeneration(runner, options = {}) {
       .filter((node) => node.pending || !results.find((result) => result.node === node))
       .map((node) => ({ skipped: true, node }));
 
-    // cache codeowners for quicker lookup
+    // cache codeowner entries for quicker lookup
-    let reversedCodeowners = [];
+    let codeOwnersEntries = [];
     try {
-      reversedCodeowners = getPathsWithOwnersReversed();
+      codeOwnersEntries = getCodeOwnersEntries();
     } catch {
       /* no-op */
     }
@@ -143,8 +143,8 @@ export function setupJUnitReportGeneration(runner, options = {}) {
       if (failed) {
         const testCaseRelativePath = getPath(node);
 
-        const owners = getCodeOwnersForFile(testCaseRelativePath, reversedCodeowners);
+        // Comma-separated list of owners. Empty string if no owners are found.
-        attrs.owners = owners?.teams || ''; // empty string when no codeowners are defined
+        attrs.owners = getOwningTeamsForPath(testCaseRelativePath, codeOwnersEntries).join(',');
       }
 
       return testsuitesEl.ele('testcase', attrs);

scripts/get_owners_for_file.js

@@ -8,4 +8,4 @@
  */
 
 require('../src/setup_node_env');
-require('@kbn/code-owners').runGetOwnersForFileCli();
+void require('@kbn/code-owners').cli.findCodeOwnersForPath();