From c85b7360161c31487b18ddfad73ae8247a566aeb Mon Sep 17 00:00:00 2001
From: Kiss Miklos <miklos@roadie.io>
Date: Mon, 6 Mar 2023 16:27:48 +0100
Subject: [PATCH] use more robust matching for CODEOWNERS entries

Signed-off-by: Kiss Miklos <miklos@roadie.io>
---
 .../codeowners/CodeOwnersProcessor.test.ts    |  8 ++---
 .../modules/codeowners/CodeOwnersProcessor.ts |  4 ---
 .../src/modules/codeowners/lib/read.ts        |  3 +-
 .../modules/codeowners/lib/resolve.test.ts    | 17 +++++++++--
 .../src/modules/codeowners/lib/resolve.ts     | 29 +++++++------------
 5 files changed, 29 insertions(+), 32 deletions(-)

diff --git a/plugins/catalog-backend/src/modules/codeowners/CodeOwnersProcessor.test.ts b/plugins/catalog-backend/src/modules/codeowners/CodeOwnersProcessor.test.ts
index a33ce84e0d..8e1fbca726 100644
--- a/plugins/catalog-backend/src/modules/codeowners/CodeOwnersProcessor.test.ts
+++ b/plugins/catalog-backend/src/modules/codeowners/CodeOwnersProcessor.test.ts
@@ -106,15 +106,11 @@ describe('CodeOwnersProcessor', () => {
       });
     });
     it('should use the "backstage.io/managed-by-location" annotation as pattern', async () => {
-      const { entity, processor } = setupTest({
-        metadata: {
-          annotations: { 'backstage.io/managed-by-location': '/docs' },
-        },
-      });
+      const { entity, processor } = setupTest();
 
       const result = await processor.preProcessEntity(
         entity as any,
-        mockLocation(),
+        mockLocation({ basePath: 'docs/' }),
       );
 
       expect(result).toEqual({
diff --git a/plugins/catalog-backend/src/modules/codeowners/CodeOwnersProcessor.ts b/plugins/catalog-backend/src/modules/codeowners/CodeOwnersProcessor.ts
index 9a9c58934c..186b732271 100644
--- a/plugins/catalog-backend/src/modules/codeowners/CodeOwnersProcessor.ts
+++ b/plugins/catalog-backend/src/modules/codeowners/CodeOwnersProcessor.ts
@@ -80,14 +80,10 @@ export class CodeOwnersProcessor implements CatalogProcessor {
       return entity;
     }
 
-    const pattern =
-      entity.metadata?.annotations?.['backstage.io/managed-by-location'];
-
     const owner = await findCodeOwnerByTarget(
       this.reader,
       location.target,
       scmIntegration,
-      pattern,
     );
 
     if (!owner) {
diff --git a/plugins/catalog-backend/src/modules/codeowners/lib/read.ts b/plugins/catalog-backend/src/modules/codeowners/lib/read.ts
index cd15a17997..6a5234c94f 100644
--- a/plugins/catalog-backend/src/modules/codeowners/lib/read.ts
+++ b/plugins/catalog-backend/src/modules/codeowners/lib/read.ts
@@ -52,7 +52,6 @@ export async function findCodeOwnerByTarget(
   reader: UrlReader,
   targetUrl: string,
   scmIntegration: ScmIntegration,
-  pattern?: string,
 ): Promise<string | undefined> {
   const codeownersPaths = scmCodeOwnersPaths[scmIntegration?.type ?? ''];
 
@@ -71,7 +70,7 @@ export async function findCodeOwnerByTarget(
     return undefined;
   }
 
-  const owner = resolveCodeOwner(contents, pattern);
+  const owner = resolveCodeOwner(contents, targetUrl);
 
   return owner;
 }
diff --git a/plugins/catalog-backend/src/modules/codeowners/lib/resolve.test.ts b/plugins/catalog-backend/src/modules/codeowners/lib/resolve.test.ts
index ec9321284f..f9cdf79530 100644
--- a/plugins/catalog-backend/src/modules/codeowners/lib/resolve.test.ts
+++ b/plugins/catalog-backend/src/modules/codeowners/lib/resolve.test.ts
@@ -23,16 +23,29 @@ const mockCodeOwnersText = () => `
 
 describe('resolveCodeOwner', () => {
   it('should parse the codeowners file', () => {
-    expect(resolveCodeOwner(mockCodeOwnersText())).toBe('team-foo');
+    expect(
+      resolveCodeOwner(
+        mockCodeOwnersText(),
+        'https://github.com/can/be/tree/anything/catalog-info.yaml',
+      ),
+    ).toBe('team-foo');
   });
   it('should include the codeowners path into the provided pattern', () => {
     expect(
       resolveCodeOwner(
         mockCodeOwnersText(),
-        'url:https://github.com/acem/repo/main/docs/catalog-info.yaml',
+        'https://github.com/acme/repo/tree/main/docs/catalog-info.yaml',
       ),
     ).toBe('team-docs');
   });
+  it('should match only in resource path', () => {
+    expect(
+      resolveCodeOwner(
+        mockCodeOwnersText(),
+        'https://github.com/acme/repo/tree/docs/catalog-info.yaml',
+      ),
+    ).toBe('team-foo');
+  });
 });
 
 describe('normalizeCodeOwner', () => {
diff --git a/plugins/catalog-backend/src/modules/codeowners/lib/resolve.ts b/plugins/catalog-backend/src/modules/codeowners/lib/resolve.ts
index 139ea881d8..92fa4a0ef6 100644
--- a/plugins/catalog-backend/src/modules/codeowners/lib/resolve.ts
+++ b/plugins/catalog-backend/src/modules/codeowners/lib/resolve.ts
@@ -15,8 +15,7 @@
  */
 
 import * as codeowners from 'codeowners-utils';
-import { CodeOwnersEntry } from 'codeowners-utils';
-import { get, head, pipe, reverse } from 'lodash/fp';
+import parseGitUrl from 'git-url-parse';
 
 const USER_PATTERN = /^@.*/;
 const GROUP_PATTERN = /^@.*\/.*/;
@@ -24,26 +23,20 @@ const EMAIL_PATTERN = /^.*@.*\..*$/;
 
 export function resolveCodeOwner(
   contents: string,
-  pattern?: string,
+  catalogInfoFileUrl: string,
 ): string | undefined {
-  const owners = codeowners.parse(contents);
+  const codeOwnerEntries = codeowners.parse(contents);
 
-  let filteredOwners = owners.filter((e: CodeOwnersEntry) =>
-    pattern?.includes(e.pattern),
-  );
-  if (filteredOwners.length === 0) {
-    filteredOwners = owners.filter((e: CodeOwnersEntry) => e.pattern === '*');
+  const { filepath } = parseGitUrl(catalogInfoFileUrl);
+  const match = codeowners.matchFile(filepath, codeOwnerEntries);
+
+  if (!match) {
+    throw new Error(
+      `There is no matching entry for path: ${filepath} in the CODEOWNERS file`,
+    );
   }
 
-  const result = pipe(
-    reverse,
-    head,
-    get('owners'),
-    head,
-    normalizeCodeOwner,
-  )(filteredOwners);
-
-  return result;
+  return normalizeCodeOwner(match.owners[0]);
 }
 
 export function normalizeCodeOwner(owner: string) {