feat: extract LGTM processor to external JS file with tests (#6074)

Co-authored-by: Gergely Bräutigam <gergely.brautigam@sap.com>

.github/scripts/lgtm-processor-test.js

@@ -0,0 +1,147 @@
+/**
+ * Tests for lgtm-processor.js helper logic.
+ *
+ * Tests the pure CODEOWNERS parsing and file-pattern matching behaviour
+ * that lives inside lgtm-processor.js, extracted here so they can run
+ * without GitHub Actions context.
+ *
+ * Run with: node .github/scripts/lgtm-processor-test.js
+ */
+
+import assert from 'node:assert/strict';
+
+// ---------------------------------------------------------------------------
+// Helpers duplicated from lgtm-processor.js for unit testing
+// (kept in sync manually; tests will catch drift)
+// ---------------------------------------------------------------------------
+
+function parseCodeowners(content, organization) {
+  const codeownerMappings = [];
+  let wildcardRoles = [];
+
+  content.split('\n').forEach(line => {
+    const trimmed = line.trim();
+    if (!trimmed || trimmed.startsWith('#')) return;
+
+    const match = trimmed.match(/^(\S+)\s+(.+)$/);
+    if (match) {
+      const [, pattern, roles] = match;
+      const rolesList = roles.split(/\s+/).filter(r => r.startsWith('@'));
+
+      if (pattern === '*') {
+        wildcardRoles = rolesList;
+      } else {
+        codeownerMappings.push({ pattern, roles: rolesList });
+      }
+    }
+  });
+
+  const maintainerRoles = wildcardRoles.map(role => role.replace(`@${organization}/`, ''));
+  return { codeownerMappings, wildcardRoles, maintainerRoles };
+}
+
+function fileMatchesPattern(file, pattern) {
+  return file === pattern ||
+    file.startsWith(pattern.endsWith('/') ? pattern : pattern + '/');
+}
+
+function getRequiredReviewerRoles(changedFiles, codeownerMappings, wildcardRoles) {
+  const requiredReviewerRoles = new Set();
+  let hasFilesWithoutSpecificOwners = false;
+
+  changedFiles.forEach(file => {
+    let hasSpecificOwner = false;
+    codeownerMappings.forEach(({ pattern, roles }) => {
+      if (fileMatchesPattern(file, pattern)) {
+        roles.forEach(role => requiredReviewerRoles.add(role));
+        hasSpecificOwner = true;
+      }
+    });
+    if (!hasSpecificOwner) hasFilesWithoutSpecificOwners = true;
+  });
+
+  if (hasFilesWithoutSpecificOwners) {
+    wildcardRoles.forEach(role => requiredReviewerRoles.add(role));
+  }
+
+  return requiredReviewerRoles;
+}
+
+// ---------------------------------------------------------------------------
+// Tests
+// ---------------------------------------------------------------------------
+
+const CODEOWNERS = `
+# Global owners
+* @external-secrets/maintainers
+
+pkg/provider/aws/ @external-secrets/aws-team
+pkg/provider/gcp/ @external-secrets/gcp-team
+docs/ @external-secrets/docs-team
+`;
+
+const ORG = 'external-secrets';
+
+// parseCodeowners: wildcard roles
+{
+  const { wildcardRoles, maintainerRoles } = parseCodeowners(CODEOWNERS, ORG);
+  assert.deepEqual(wildcardRoles, ['@external-secrets/maintainers']);
+  assert.deepEqual(maintainerRoles, ['maintainers']);
+}
+
+// parseCodeowners: directory mappings
+{
+  const { codeownerMappings } = parseCodeowners(CODEOWNERS, ORG);
+  assert.equal(codeownerMappings.length, 3);
+  assert.equal(codeownerMappings[0].pattern, 'pkg/provider/aws/');
+  assert.deepEqual(codeownerMappings[0].roles, ['@external-secrets/aws-team']);
+}
+
+// parseCodeowners: comments and blank lines are ignored
+{
+  const { codeownerMappings } = parseCodeowners('# comment\n\npkg/foo/ @org/team\n', ORG);
+  assert.equal(codeownerMappings.length, 1);
+}
+
+// fileMatchesPattern: exact match
+assert.equal(fileMatchesPattern('pkg/provider/aws/s3.go', 'pkg/provider/aws/s3.go'), true);
+
+// fileMatchesPattern: directory prefix (with trailing slash)
+assert.equal(fileMatchesPattern('pkg/provider/aws/s3.go', 'pkg/provider/aws/'), true);
+
+// fileMatchesPattern: directory prefix (without trailing slash)
+assert.equal(fileMatchesPattern('pkg/provider/aws/s3.go', 'pkg/provider/aws'), true);
+
+// fileMatchesPattern: no match
+assert.equal(fileMatchesPattern('pkg/provider/gcp/storage.go', 'pkg/provider/aws/'), false);
+
+// fileMatchesPattern: partial prefix should not match
+assert.equal(fileMatchesPattern('pkg/provider/aws-extra/file.go', 'pkg/provider/aws'), false);
+
+// getRequiredReviewerRoles: file under specific owner
+{
+  const { codeownerMappings, wildcardRoles } = parseCodeowners(CODEOWNERS, ORG);
+  const roles = getRequiredReviewerRoles(['pkg/provider/aws/ec2.go'], codeownerMappings, wildcardRoles);
+  assert.ok(roles.has('@external-secrets/aws-team'));
+  assert.ok(!roles.has('@external-secrets/maintainers'));
+}
+
+// getRequiredReviewerRoles: file with no specific owner falls back to wildcard
+{
+  const { codeownerMappings, wildcardRoles } = parseCodeowners(CODEOWNERS, ORG);
+  const roles = getRequiredReviewerRoles(['some/unowned/file.go'], codeownerMappings, wildcardRoles);
+  assert.ok(roles.has('@external-secrets/maintainers'));
+}
+
+// getRequiredReviewerRoles: mixed files collect all required roles
+{
+  const { codeownerMappings, wildcardRoles } = parseCodeowners(CODEOWNERS, ORG);
+  const roles = getRequiredReviewerRoles(
+    ['pkg/provider/aws/ec2.go', 'pkg/provider/gcp/storage.go'],
+    codeownerMappings, wildcardRoles
+  );
+  assert.ok(roles.has('@external-secrets/aws-team'));
+  assert.ok(roles.has('@external-secrets/gcp-team'));
+}
+
+console.log('All tests passed.');

.github/scripts/lgtm-processor.js

@@ -0,0 +1,245 @@
+/**
+ * LGTM Command Processor
+ *
+ * Processes /lgtm comments on pull requests. Checks if the commenter has the
+ * required reviewer role(s) based on CODEOWNERS.md, then adds the lgtm label
+ * and posts a confirmation comment.
+ *
+ * @param {object} params
+ * @param {object} params.core    - @actions/core
+ * @param {object} params.github  - @actions/github Octokit instance
+ * @param {object} params.context - GitHub Actions context
+ * @param {object} params.fs      - Node.js fs module
+ */
+export default async function run({ core, github, context, fs }) {
+  const organization = 'external-secrets';
+  const lgtmLabelName = 'lgtm';
+
+  const commenter = context.payload.comment.user.login;
+  const prNumber = context.payload.issue.number;
+  const owner = context.repo.owner;
+  const repo = context.repo.repo;
+
+  // Parse CODEOWNERS.md file
+  let codeownersContent;
+  try {
+    codeownersContent = fs.readFileSync('CODEOWNERS.md', 'utf8');
+  } catch (error) {
+    return;
+  }
+
+  // Extract role mappings from CODEOWNERS.md (including * pattern)
+  const codeownerMappings = [];
+  let wildcardRoles = [];
+  codeownersContent.split('\n').forEach(line => {
+    const trimmed = line.trim();
+    if (!trimmed || trimmed.startsWith('#')) return;
+
+    const match = trimmed.match(/^(\S+)\s+(.+)$/);
+    if (match) {
+      const [, pattern, roles] = match;
+      const rolesList = roles.split(/\s+/).filter(r => r.startsWith('@'));
+
+      if (pattern === '*') {
+        wildcardRoles = rolesList;
+      } else {
+        codeownerMappings.push({
+          pattern,
+          roles: rolesList
+        });
+      }
+    }
+  });
+
+  // Extract maintainer roles from wildcardRoles
+  const maintainerRoles = wildcardRoles.map(role => role.replace(`@${organization}/`, ''));
+
+  // Early check: if user is a maintainer, approve immediately
+  let isMaintainer = false;
+
+  for (const role of maintainerRoles) {
+    try {
+      const response = await github.rest.teams.getMembershipForUserInOrg({
+        org: organization,
+        team_slug: role,
+        username: commenter
+      });
+      if (response.data.state === 'active') {
+        isMaintainer = true;
+        break;
+      }
+    } catch (error) {
+      // User not in this team, continue checking others
+    }
+  }
+
+  if (isMaintainer) {
+    // Check if LGTM label already exists
+    const labels = await github.rest.issues.listLabelsOnIssue({
+      owner, repo, issue_number: prNumber
+    });
+    if (!labels.data.some(l => l.name === lgtmLabelName)) {
+      await github.rest.issues.addLabels({
+        owner, repo, issue_number: prNumber, labels: [lgtmLabelName]
+      });
+    }
+
+    // Simple confirmation for maintainers (no role mentions)
+    await github.rest.issues.createComment({
+      owner, repo, issue_number: prNumber,
+      body: `✅ LGTM by @${commenter} (maintainer)`
+    });
+    return;
+  }
+
+  // Get changed files in PR
+  const filesResponse = await github.rest.pulls.listFiles({
+    owner, repo, pull_number: prNumber
+  });
+  const changedFiles = filesResponse.data.map(f => f.filename);
+
+  // Find all required reviewer roles for the changed files
+  // This includes hierarchical matching (e.g., pkg/provider/ matches pkg/provider/aws/file.go)
+  const requiredReviewerRoles = new Set();
+  let hasFilesWithoutSpecificOwners = false;
+
+  changedFiles.forEach(file => {
+    let hasSpecificOwner = false;
+    codeownerMappings.forEach(mapping => {
+      const { pattern, roles } = mapping;
+      // Match pattern (handle both exact matches and directory patterns)
+      // This handles hierarchical matching where broader patterns can cover more specific paths
+      if (file === pattern ||
+          file.startsWith(pattern.endsWith('/') ? pattern : pattern + '/')) {
+        roles.forEach(role => requiredReviewerRoles.add(role));
+        hasSpecificOwner = true;
+      }
+    });
+
+    if (!hasSpecificOwner) {
+      hasFilesWithoutSpecificOwners = true;
+    }
+  });
+
+  // For files that only match the wildcard pattern, add wildcard roles
+  if (hasFilesWithoutSpecificOwners) {
+    wildcardRoles.forEach(role => {
+      requiredReviewerRoles.add(role);
+    });
+  }
+
+
+  // Find commenter's matching reviewer roles
+  const commenterReviewerRoles = new Set();
+  for (const role of requiredReviewerRoles) {
+    const roleSlug = role.replace(`@${organization}/`, '');
+    
+    try {
+      const response = await github.rest.teams.getMembershipForUserInOrg({
+        org: organization,
+        team_slug: roleSlug,
+        username: commenter
+      });
+      if (response.data.state === 'active') {
+        commenterReviewerRoles.add(role);
+      }
+    } catch (error) {
+      // User not in this role, continue checking others
+    }
+  }
+
+  // Check if user has any required reviewer role
+  if (commenterReviewerRoles.size === 0) {
+    const rolesList = Array.from(requiredReviewerRoles).map(role => role.replace(`@${organization}/`, '')).join(', ');
+    await github.rest.issues.createComment({
+      owner, repo, issue_number: prNumber,
+      body: `@${commenter} You must be a member of one of the required reviewer roles to use /lgtm.\n\nRequired roles for this PR: ${rolesList}`
+    });
+    return;
+  }
+
+  // Check if LGTM label already exists
+  const labels = await github.rest.issues.listLabelsOnIssue({
+    owner, repo, issue_number: prNumber
+  });
+  
+  if (!labels.data.some(l => l.name === lgtmLabelName)) {
+    await github.rest.issues.addLabels({
+      owner, repo, issue_number: prNumber, labels: [lgtmLabelName]
+    });
+    // Added LGTM label
+  }
+
+  // Build confirmation message with role coverage info
+  const mentionRoles = wildcardRoles.join(' ');
+  let confirmationMessage = `${mentionRoles}\n\n✅ LGTM by @${commenter}`;
+
+  // Check for truly uncovered roles using pattern hierarchy
+  const uncoveredRoles = [];
+  for (const requiredRole of requiredReviewerRoles) {
+    let isCovered = commenterReviewerRoles.has(requiredRole);
+
+    // If not directly covered, check if any commenter role has a pattern that covers this required role's pattern
+    if (!isCovered) {
+      // Find the patterns for this required role
+      const requiredRolePatterns = codeownerMappings
+        .filter(mapping => mapping.roles.includes(requiredRole))
+        .map(mapping => mapping.pattern);
+
+      // Check if any commenter role has a pattern that is a parent of any required role pattern
+      for (const commenterRole of commenterReviewerRoles) {
+        const commenterRolePatterns = codeownerMappings
+          .filter(mapping => mapping.roles.includes(commenterRole))
+          .map(mapping => mapping.pattern);
+
+        // Check if any commenter pattern is a parent of any required pattern
+        for (const commenterPattern of commenterRolePatterns) {
+          for (const requiredPattern of requiredRolePatterns) {
+            const commenterPath = commenterPattern.endsWith('/') ? commenterPattern : commenterPattern + '/';
+            const requiredPath = requiredPattern.endsWith('/') ? requiredPattern : requiredPattern + '/';
+
+            // If required pattern starts with commenter pattern, it's hierarchically covered
+            if (requiredPath.startsWith(commenterPath) && commenterPath !== requiredPath) {
+              isCovered = true;
+              break;
+            }
+          }
+          if (isCovered) break;
+        }
+        if (isCovered) break;
+      }
+    }
+
+    if (!isCovered) {
+      uncoveredRoles.push(requiredRole);
+    }
+  }
+
+  // Show role coverage analysis
+  confirmationMessage += `\n\n**Review Coverage:**`;
+
+  if (uncoveredRoles.length > 0) {
+    confirmationMessage += `\n- Commenter has roles:`;
+    Array.from(commenterReviewerRoles).forEach(role => {
+      const roleName = role.replace(`@${organization}/`, '');
+      confirmationMessage += `\n  - ${roleName}`;
+    });
+    confirmationMessage += `\n- Required roles:`;
+    Array.from(requiredReviewerRoles).forEach(role => {
+      const roleName = role.replace(`@${organization}/`, '');
+      confirmationMessage += `\n  - ${roleName}`;
+    });
+    confirmationMessage += `\n- ❌ Additional review may be needed by:`;
+    uncoveredRoles.forEach(role => {
+      const roleName = role.replace(`@${organization}/`, '');
+      confirmationMessage += `\n  - ${roleName}`;
+    });
+  } else {
+    confirmationMessage += `\n- ✅ All required roles covered`;
+  }
+  
+  await github.rest.issues.createComment({
+    owner, repo, issue_number: prNumber,
+    body: confirmationMessage
+  });
+}

.github/workflows/lgtm.yml

@@ -48,234 +48,5 @@ jobs:
       with:
         github-token: ${{ steps.generate_token.outputs.token }}
         script: |
-          const organization = 'external-secrets';
-          const lgtmLabelName = 'lgtm';
-          const fs = require('fs');
-
-          const commenter = context.payload.comment.user.login;
-          const prNumber = context.payload.issue.number;
-          const owner = context.repo.owner;
-          const repo = context.repo.repo;
-
-          // Parse CODEOWNERS.md file
-          let codeownersContent;
-          try {
-            codeownersContent = fs.readFileSync('CODEOWNERS.md', 'utf8');
-          } catch (error) {
-            return;
-          }
-
-          // Extract role mappings from CODEOWNERS.md (including * pattern)
-          const codeownerMappings = [];
-          let wildcardRoles = [];
-          codeownersContent.split('\n').forEach(line => {
-            const trimmed = line.trim();
-            if (!trimmed || trimmed.startsWith('#')) return;
-
-            const match = trimmed.match(/^(\S+)\s+(.+)$/);
-            if (match) {
-              const [, pattern, roles] = match;
-              const rolesList = roles.split(/\s+/).filter(r => r.startsWith('@'));
-
-              if (pattern === '*') {
-                wildcardRoles = rolesList;
-              } else {
-                codeownerMappings.push({
-                  pattern,
-                  roles: rolesList
-                });
-              }
-            }
-          });
-
-          // Extract maintainer roles from wildcardRoles
-          const maintainerRoles = wildcardRoles.map(role => role.replace(`@${organization}/`, ''));
-
-          // Early check: if user is a maintainer, approve immediately
-          let isMaintainer = false;
-
-          for (const role of maintainerRoles) {
-            try {
-              const response = await github.rest.teams.getMembershipForUserInOrg({
-                org: organization,
-                team_slug: role,
-                username: commenter
-              });
-              if (response.data.state === 'active') {
-                isMaintainer = true;
-                break;
-              }
-            } catch (error) {
-              // User not in this team, continue checking others
-            }
-          }
-
-          if (isMaintainer) {
-            // Check if LGTM label already exists
-            const labels = await github.rest.issues.listLabelsOnIssue({
-              owner, repo, issue_number: prNumber
-            });
-            if (!labels.data.some(l => l.name === lgtmLabelName)) {
-              await github.rest.issues.addLabels({
-                owner, repo, issue_number: prNumber, labels: [lgtmLabelName]
-              });
-            }
-
-            // Simple confirmation for maintainers (no role mentions)
-            await github.rest.issues.createComment({
-              owner, repo, issue_number: prNumber,
-              body: `✅ LGTM by @${commenter} (maintainer)`
-            });
-            return;
-          }
-
-          // Get changed files in PR
-          const filesResponse = await github.rest.pulls.listFiles({
-            owner, repo, pull_number: prNumber
-          });
-          const changedFiles = filesResponse.data.map(f => f.filename);
-
-          // Find all required reviewer roles for the changed files
-          // This includes hierarchical matching (e.g., pkg/provider/ matches pkg/provider/aws/file.go)
-          const requiredReviewerRoles = new Set();
-          let hasFilesWithoutSpecificOwners = false;
-
-          changedFiles.forEach(file => {
-            let hasSpecificOwner = false;
-            codeownerMappings.forEach(mapping => {
-              const { pattern, roles } = mapping;
-              // Match pattern (handle both exact matches and directory patterns)
-              // This handles hierarchical matching where broader patterns can cover more specific paths
-              if (file === pattern ||
-                  file.startsWith(pattern.endsWith('/') ? pattern : pattern + '/')) {
-                roles.forEach(role => requiredReviewerRoles.add(role));
-                hasSpecificOwner = true;
-              }
-            });
-
-            if (!hasSpecificOwner) {
-              hasFilesWithoutSpecificOwners = true;
-            }
-          });
-
-          // For files that only match the wildcard pattern, add wildcard roles
-          if (hasFilesWithoutSpecificOwners) {
-            wildcardRoles.forEach(role => {
-              requiredReviewerRoles.add(role);
-            });
-          }
-
-
-          // Find commenter's matching reviewer roles
-          const commenterReviewerRoles = new Set();
-          for (const role of requiredReviewerRoles) {
-            const roleSlug = role.replace(`@${organization}/`, '');
-            
-            try {
-              const response = await github.rest.teams.getMembershipForUserInOrg({
-                org: organization,
-                team_slug: roleSlug,
-                username: commenter
-              });
-              if (response.data.state === 'active') {
-                commenterReviewerRoles.add(role);
-              }
-            } catch (error) {
-              // User not in this role, continue checking others
-            }
-          }
-
-          // Check if user has any required reviewer role
-          if (commenterReviewerRoles.size === 0) {
-            const rolesList = Array.from(requiredReviewerRoles).map(role => role.replace(`@${organization}/`, '')).join(', ');
-            await github.rest.issues.createComment({
-              owner, repo, issue_number: prNumber,
-              body: `@${commenter} You must be a member of one of the required reviewer roles to use /lgtm.\n\nRequired roles for this PR: ${rolesList}`
-            });
-            return;
-          }
-
-          // Check if LGTM label already exists
-          const labels = await github.rest.issues.listLabelsOnIssue({
-            owner, repo, issue_number: prNumber
-          });
-          
-          if (!labels.data.some(l => l.name === lgtmLabelName)) {
-            await github.rest.issues.addLabels({
-              owner, repo, issue_number: prNumber, labels: [lgtmLabelName]
-            });
-            // Added LGTM label
-          }
-
-          // Build confirmation message with role coverage info
-          const mentionRoles = wildcardRoles.join(' ');
-          let confirmationMessage = `${mentionRoles}\n\n✅ LGTM by @${commenter}`;
-
-          // Check for truly uncovered roles using pattern hierarchy
-          const uncoveredRoles = [];
-          for (const requiredRole of requiredReviewerRoles) {
-            let isCovered = commenterReviewerRoles.has(requiredRole);
-
-            // If not directly covered, check if any commenter role has a pattern that covers this required role's pattern
-            if (!isCovered) {
-              // Find the patterns for this required role
-              const requiredRolePatterns = codeownerMappings
-                .filter(mapping => mapping.roles.includes(requiredRole))
-                .map(mapping => mapping.pattern);
-
-              // Check if any commenter role has a pattern that is a parent of any required role pattern
-              for (const commenterRole of commenterReviewerRoles) {
-                const commenterRolePatterns = codeownerMappings
-                  .filter(mapping => mapping.roles.includes(commenterRole))
-                  .map(mapping => mapping.pattern);
-
-                // Check if any commenter pattern is a parent of any required pattern
-                for (const commenterPattern of commenterRolePatterns) {
-                  for (const requiredPattern of requiredRolePatterns) {
-                    const commenterPath = commenterPattern.endsWith('/') ? commenterPattern : commenterPattern + '/';
-                    const requiredPath = requiredPattern.endsWith('/') ? requiredPattern : requiredPattern + '/';
-
-                    // If required pattern starts with commenter pattern, it's hierarchically covered
-                    if (requiredPath.startsWith(commenterPath) && commenterPath !== requiredPath) {
-                      isCovered = true;
-                      break;
-                    }
-                  }
-                  if (isCovered) break;
-                }
-                if (isCovered) break;
-              }
-            }
-
-            if (!isCovered) {
-              uncoveredRoles.push(requiredRole);
-            }
-          }
-
-          // Show role coverage analysis
-          confirmationMessage += `\n\n**Review Coverage:**`;
-
-          if (uncoveredRoles.length > 0) {
-            confirmationMessage += `\n- Commenter has roles:`;
-            Array.from(commenterReviewerRoles).forEach(role => {
-              const roleName = role.replace(`@${organization}/`, '');
-              confirmationMessage += `\n  - ${roleName}`;
-            });
-            confirmationMessage += `\n- Required roles:`;
-            Array.from(requiredReviewerRoles).forEach(role => {
-              const roleName = role.replace(`@${organization}/`, '');
-              confirmationMessage += `\n  - ${roleName}`;
-            });
-            confirmationMessage += `\n- ❌ Additional review may be needed by:`;
-            uncoveredRoles.forEach(role => {
-              const roleName = role.replace(`@${organization}/`, '');
-              confirmationMessage += `\n  - ${roleName}`;
-            });
-          } else {
-            confirmationMessage += `\n- ✅ All required roles covered`;
-          }
-          
-          await github.rest.issues.createComment({
-            owner, repo, issue_number: prNumber,
-            body: confirmationMessage
-          });
+          const { default: run } = await import(`${process.env.GITHUB_WORKSPACE}/.github/scripts/lgtm-processor.js`);
+          await run({ core, github, context, fs: require('fs') });