From 32a0878dbbd91ce4c832c7894b98ac71db27e39b Mon Sep 17 00:00:00 2001 From: Tyler Garner Date: Tue, 18 Apr 2023 12:26:37 -0400 Subject: [PATCH] Multiple improvements related to the kick process and general code centralization. * Centralizes logic for retrieving members. * Centralizes dry run logic. * Safely checks for ISSUE_WITHOUT_MENTION env var. * Fixes bug where kick action was not checking out the repo, which is required to read the db.json file. --- .github/actions/audit/action.yml | 2 +- .github/actions/kick/action.yml | 19 ++++++++++++++-- .github/actions/report/action.yml | 2 +- lib/inactiveUsers.js | 22 ++++++++++++++++++- lib/kick.js | 36 ++++++++++--------------------- lib/report.js | 26 +++++++--------------- lib/utils.js | 2 ++ 7 files changed, 61 insertions(+), 48 deletions(-) diff --git a/.github/actions/audit/action.yml b/.github/actions/audit/action.yml index a1f2286..9e0b820 100644 --- a/.github/actions/audit/action.yml +++ b/.github/actions/audit/action.yml @@ -22,7 +22,7 @@ runs: with: path: main repository: oss-tooling/user-inactivity - ref: v1.1.4 + ref: v1.1.5 - uses: actions/checkout@v3 with: diff --git a/.github/actions/kick/action.yml b/.github/actions/kick/action.yml index a6eae97..7d03ede 100644 --- a/.github/actions/kick/action.yml +++ b/.github/actions/kick/action.yml @@ -5,6 +5,9 @@ inputs: description: the github organization to audit repo: description: name of repo where inactive user issues are created + branch: + description: branch to store activity data on + default: main inactivity-duration: description: the amount of days a user can be inactive for default: 30 @@ -16,12 +19,24 @@ inputs: description: the app installation id runs: using: composite - steps: + steps: + - uses: actions/checkout@v3 + with: + ref: ${{ inputs.branch }} + path: activity + - uses: actions/checkout@v3 with: path: main repository: oss-tooling/user-inactivity - ref: v1.1.4 + ref: v1.1.5 + + - name: Move db.json + run: | + if test -f "${{ github.workspace }}/activity/db.json"; then + mv ${{ github.workspace }}/activity/db.json ${{ github.workspace }}/main/db.json + fi + shell: bash - name: Install dependencies run: npm install diff --git a/.github/actions/report/action.yml b/.github/actions/report/action.yml index b3cc691..9a31282 100644 --- a/.github/actions/report/action.yml +++ b/.github/actions/report/action.yml @@ -24,7 +24,7 @@ runs: with: path: main repository: oss-tooling/user-inactivity - ref: v1.1.4 + ref: v1.1.5 - uses: actions/checkout@v3 with: diff --git a/lib/inactiveUsers.js b/lib/inactiveUsers.js index 6a17e5b..4396d61 100644 --- a/lib/inactiveUsers.js +++ b/lib/inactiveUsers.js @@ -7,7 +7,27 @@ exports.inactiveUsers = (octokit, options) => { inactiveUsers: { audit: audit(octokit, options), report: report(octokit, options), - kick: kick(octokit, options) + kick: kick(octokit, options), + getOrgMemberData: async (org, issueRepo) => { + const [ members, userIssues, outsideCollaborators ] = await Promise.all([ + octokit.paginate(octokit.orgs.listMembers, { + org, + role: 'member', + per_page: 100 + }), + octokit.paginate(octokit.issues.listForRepo, { + owner: org, + repo: issueRepo, + state: 'open', + per_page: 100, + }), + octokit.paginate(octokit.orgs.listOutsideCollaborators, { + org, + per_page: 100 + }) + ]) + return { members, userIssues, outsideCollaborators } + } } } } diff --git a/lib/kick.js b/lib/kick.js index 659a4de..cda454e 100644 --- a/lib/kick.js +++ b/lib/kick.js @@ -1,8 +1,8 @@ const database = require('./database') -const { inactiveUserLabel } = require('./utils') +const { dryRun } = require('./utils') const preserveAccountComment = (user) => { - if (process.env.ISSUE_WITHOUT_MENTION.toLowerCase() === 'true') { + if (process.env.ISSUE_WITHOUT_MENTION && process.env.ISSUE_WITHOUT_MENTION.toLowerCase() === 'true') { return `Thank you, I will mark your account active. Please remember this ${database.INACTIVE_DURATION} day audit will be part of the continuous monitoring for GitHub, so please maintain activity of one of the following to avoid be pulled as inactive again. 1. Commits 2. Created issue(s) @@ -40,7 +40,7 @@ const removeCollaboratorFromOrg = async (client, owner, login) => { } const createComment = async (client, owner, repo, number, comment) => { - if (process.env.DRY_RUN.toLowerCase() !== 'true') { + if (!dryRun) { await client.issues.createComment({ owner: owner, repo: repo, @@ -51,7 +51,7 @@ const createComment = async (client, owner, repo, number, comment) => { } const closeIssue = async (client, owner, repo, number) => { - if (process.env.DRY_RUN.toLowerCase() !== 'true') { + if (!dryRun) { await client.issues.update({ owner: owner, repo: repo, @@ -62,7 +62,7 @@ const closeIssue = async (client, owner, repo, number) => { } const addLabels = async (client, owner, repo, number, labels) => { - if (process.env.DRY_RUN.toLowerCase() !== 'true') { + if (!dryRun) { await client.issues.addLabels({ owner: owner, repo: repo, @@ -73,30 +73,16 @@ const addLabels = async (client, owner, repo, number, labels) => { } exports.kick = (octokit) => async (org, repo) => { - const [members, outsideCollaborators, issues] = await Promise.all([ - octokit.paginate(octokit.orgs.listMembers, { - org, - role: 'member', - per_page: 100 - }), - octokit.paginate(octokit.orgs.listOutsideCollaborators, { - org, - per_page: 100 - }, (response) => response.data.map((collaborator) => collaborator.login)), - octokit.paginate(octokit.issues.listForRepo, { - owner: org, - repo, - state: 'open', - labels: [inactiveUserLabel], - sort: 'created', - direction: 'desc', - per_page: 100 - }) - ]) + const { members: orgMembers, userIssues: issues, outsideCollaborators: outsideCollaboratorsRaw} = await octokit.inactiveUsers.getOrgMemberData(org, repo) + + const members = orgMembers.concat(outsideCollaboratorsRaw) + const outsideCollaborators = outsideCollaboratorsRaw.map(collaborator => collaborator.login) const _expiredUsers = await database.getExpiredUsers(members) const expiredUsers = _expiredUsers.map(user => user.login) + console.log(expiredUsers) + const expirationDate = new Date() expirationDate.setDate(expirationDate.getDate() - 3) diff --git a/lib/report.js b/lib/report.js index bda3580..faf27cb 100644 --- a/lib/report.js +++ b/lib/report.js @@ -3,22 +3,12 @@ const database = require('./database') const utils = require('./utils') exports.report = (octokit) => async (org, repo, body) => { - console.log('Retrieving list of all organization users') - const members = await octokit.paginate(octokit.orgs.listMembers, { - org, - role: 'member', - per_page: 100 - }) - - console.log('Retrieving all active issues') - const _issues = await octokit.paginate(octokit.issues.listForRepo, { - owner: org, - repo, - state: 'open', - per_page: 100 - }) - - const usernames = _issues.map(issue => issue.title.toLowerCase()) + const { members: orgMembers, userIssues, outsideCollaborators } = await octokit.inactiveUsers.getOrgMemberData(org, repo) + + const usersWithIssues = userIssues.map(issue => issue.title.toLowerCase()) + const members = orgMembers.concat(outsideCollaborators) + + console.log(`${members.length} users found in the organization (${orgMembers.length} members and ${outsideCollaborators.length} outside collaborators)`) const expiredUsers = await database.getExpiredUsers(members) console.log(`${expiredUsers.length} users found with no activity in the last ${database.INACTIVE_DURATION} days`) @@ -28,12 +18,12 @@ exports.report = (octokit) => async (org, repo, body) => { console.log(`Skipping: ${expiredUser.login} is a bot account`) continue } - if (usernames.includes(expiredUser.login.toLowerCase())) { + if (usersWithIssues.includes(expiredUser.login.toLowerCase())) { console.log(`Skipping: ${expiredUser.login} already has an issue open`) continue } console.log(`Sending notification for ${expiredUser.login}`) - if (process.env.DRY_RUN.toLowerCase() !== 'true') { + if (!utils.dryRun) { await octokit.issues.create({ owner: org, repo, diff --git a/lib/utils.js b/lib/utils.js index 040c2ae..46197f8 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -53,3 +53,5 @@ exports.registerSecrets = async () => { } exports.inactiveUserLabel = 'inactive-user' + +exports.dryRun = process.env.DRY_RUN ? process.env.DRY_RUN.toLowerCase() == 'true' : false