refactor to dedup min summary generation

2024-06-03 15:42:37 -07:00
parent 1988567896
commit f7aca4f481
3 changed files with 85 additions and 134 deletions
@@ -5,6 +5,8 @@ import * as retry from '@octokit/plugin-retry'
 import {RequestError} from '@octokit/request-error'
 import {ConfigurationOptions} from './schemas'

+export const MAX_COMMENT_LENGTH = 65536
+
 const retryingOctokit = githubUtils.GitHub.plugin(retry.retry)
 const octo = new retryingOctokit(
  githubUtils.getOctokitOptions(core.getInput('repo-token', {required: true}))
@@ -12,19 +14,11 @@ const octo = new retryingOctokit(

 // Comment Marker to identify an existing comment to update, so we don't spam the PR with comments
 const COMMENT_MARKER = '<!-- dependency-review-pr-comment-marker -->'
-const MAX_COMMENT_LENGTH = 65536

 export async function commentPr(
-  summary: typeof core.summary,
+  commentContent: string,
  config: ConfigurationOptions,
-  minComment: string
 ): Promise<void> {
-  const commentContent = summary.stringify()
-
-  // this should be truncated for us if it's too long but
-  // we could check len and sub in minSummary instead
-  core.setOutput('comment-content', commentContent)
-
  if (
    !(
      config.comment_summary_in_pr === 'always' ||
@@ -44,13 +38,6 @@ export async function commentPr(

  let commentBody = `${commentContent}\n\n${COMMENT_MARKER}`

-  if (commentBody.length >= MAX_COMMENT_LENGTH) {
-    core.debug(
-      'The comment was too big for the GitHub API. Falling back on a minimum comment'
-    )
-    commentBody = `${minComment}\n\n${COMMENT_MARKER}`
-  }
-
  try {
    const existingCommentId = await findCommentByMarker(COMMENT_MARKER)

@@ -22,9 +22,13 @@ import * as summary from './summary'
 import {getRefs} from './git-refs'

 import {groupDependenciesByManifest} from './utils'
-import {commentPr} from './comment-pr'
+import {
+  commentPr,
+  MAX_COMMENT_LENGTH
+} from './comment-pr'
 import {getDeniedChanges} from './deny'

+
 async function delay(ms: number): Promise<void> {
  return new Promise(resolve => setTimeout(resolve, ms))
 }
@@ -127,14 +131,7 @@ async function run(): Promise<void> {

    const scorecard = await getScorecardLevels(filteredChanges)

-    summary.addSummaryToSummary(
-      vulnerableChanges,
-      invalidLicenseChanges,
-      deniedChanges,
-      scorecard,
-      config
-    )
-    const minSummary = summary.getMinSummaryForComment(
+    const minSummary = summary.addSummaryToSummary(
      vulnerableChanges,
      invalidLicenseChanges,
      deniedChanges,
@@ -173,7 +170,21 @@ async function run(): Promise<void> {
    core.setOutput('dependency-changes', JSON.stringify(changes))
    summary.addScannedDependencies(changes)
    printScannedDependencies(changes)
-    await commentPr(core.summary, config, minSummary)
+
+    // include full summary in output; Actions will truncate if oversized
+    let rendered = core.summary.stringify()
+    core.setOutput('content-comment', rendered)
+
+    // if the summary is oversized, replace with minimal version
+    if (rendered.length >= MAX_COMMENT_LENGTH) {
+      core.debug(
+        'The comment was too big for the GitHub API. Falling back on a minimum comment'
+      )
+      rendered = minSummary
+    }
+
+    // update the PR comment if needed with the right-sized summary
+    await commentPr(rendered, config)
  } catch (error) {
    if (error instanceof RequestError && error.status === 404) {
      core.setFailed(
@@ -10,81 +10,23 @@ const icons = {
  warning: '⚠️'
 }

-export function getMinSummaryForComment(
-  vulnerableChanges: Changes,
-  invalidLicenseChanges: InvalidLicenseChanges,
-  deniedChanges: Changes,
-  scorecard: Scorecard,
-  config: ConfigurationOptions
-): string {
-  const scorecardWarnings = countScorecardWarnings(scorecard, config)
-  const licenseIssues = countLicenseIssues(invalidLicenseChanges)
-
-  let minSummary = '# Dependency Review\n'
-
-  if (
-    vulnerableChanges.length === 0 &&
-    licenseIssues === 0 &&
-    deniedChanges.length === 0 &&
-    scorecardWarnings === 0
-  ) {
-    const issueTypes = [
-      config.vulnerability_check ? 'vulnerabilities' : '',
-      config.license_check ? 'license issues' : '',
-      config.show_openssf_scorecard ? 'OpenSSF Scorecard issues' : ''
-    ]
-    if (issueTypes.filter(Boolean).length === 0) {
-      minSummary += `${icons.check} No issues found.`
-    } else {
-      minSummary += `${icons.check} No ${issueTypes.filter(Boolean).join(' or ')} found.`
-    }
-  }
-
-  minSummary += 'The following issues were found:\n'
-  minSummary += config.vulnerability_check
-    ? `* ${checkOrFailIcon(vulnerableChanges.length)} ${
-        vulnerableChanges.length
-      } vulnerable package(s)\n`
-    : ''
-  minSummary += config.license_check
-    ? `* ${checkOrFailIcon(invalidLicenseChanges.forbidden.length)} ${
-        invalidLicenseChanges.forbidden.length
-      } package(s) with incompatible licenses\n
-* ${checkOrFailIcon(invalidLicenseChanges.unresolved.length)} ${
-        invalidLicenseChanges.unresolved.length
-      } package(s) with invalid SPDX license definitions\n
-* ${checkOrWarnIcon(invalidLicenseChanges.unlicensed.length)} ${
-        invalidLicenseChanges.unlicensed.length
-      } package(s) with unknown licenses.\n`
-    : ''
-  minSummary +=
-    deniedChanges.length > 0
-      ? `* ${checkOrWarnIcon(deniedChanges.length)} ${
-          deniedChanges.length
-        } package(s) denied.\n`
-      : ''
-  minSummary +=
-    config.show_openssf_scorecard && scorecardWarnings > 0
-      ? `* ${checkOrWarnIcon(scorecardWarnings)} ${scorecardWarnings ? scorecardWarnings : 'No'} packages with OpenSSF Scorecard issues.\n`
-      : ''
-
-  // Add the link to the job summary provided by GitHub Actions for this workflow run
-  minSummary += `\n[View full job summary](${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID})`
-
-  return minSummary
-}
-
+// generates the initial DR summmary and applies to the Action's core.summary
+// returns a string array of all the formatted values in the event we need
+// to replace them in the PR comment due to length restrictions
 export function addSummaryToSummary(
  vulnerableChanges: Changes,
  invalidLicenseChanges: InvalidLicenseChanges,
  deniedChanges: Changes,
  scorecard: Scorecard,
-  config: ConfigurationOptions
-): void {
+  config: ConfigurationOptions,
+): string {
+  let out: string[] = [];
+
  const scorecardWarnings = countScorecardWarnings(scorecard, config)
  const licenseIssues = countLicenseIssues(invalidLicenseChanges)

  core.summary.addHeading('Dependency Review', 1)
+  out.push('# Dependency Review')

  if (
    vulnerableChanges.length === 0 &&
@@ -97,54 +39,65 @@ export function addSummaryToSummary(
      config.license_check ? 'license issues' : '',
      config.show_openssf_scorecard ? 'OpenSSF Scorecard issues' : ''
    ]
+
+    let msg = ''
    if (issueTypes.filter(Boolean).length === 0) {
-      core.summary.addRaw(`${icons.check} No issues found.`)
+      msg = `${icons.check} No issues found.`
    } else {
-      core.summary.addRaw(
-        `${icons.check} No ${issueTypes.filter(Boolean).join(' or ')} found.`
-      )
+      msg = `${icons.check} No ${issueTypes.filter(Boolean).join(' or ')} found.`
    }

-    return
+    core.summary.addRaw(msg)
+    out.push(msg)
+    return out.join('\n')
  }

-  core.summary
-    .addRaw('The following issues were found:')
-    .addList([
-      ...(config.vulnerability_check
-        ? [
-            `${checkOrFailIcon(vulnerableChanges.length)} ${
-              vulnerableChanges.length
-            } vulnerable package(s)`
-          ]
-        : []),
-      ...(config.license_check
-        ? [
-            `${checkOrFailIcon(invalidLicenseChanges.forbidden.length)} ${
-              invalidLicenseChanges.forbidden.length
-            } package(s) with incompatible licenses`,
-            `${checkOrFailIcon(invalidLicenseChanges.unresolved.length)} ${
-              invalidLicenseChanges.unresolved.length
-            } package(s) with invalid SPDX license definitions`,
-            `${checkOrWarnIcon(invalidLicenseChanges.unlicensed.length)} ${
-              invalidLicenseChanges.unlicensed.length
-            } package(s) with unknown licenses.`
-          ]
-        : []),
-      ...(deniedChanges.length > 0
-        ? [
-            `${checkOrFailIcon(deniedChanges.length)} ${
-              deniedChanges.length
-            } package(s) denied.`
-          ]
-        : []),
-      ...(config.show_openssf_scorecard && scorecardWarnings > 0
-        ? [
-            `${checkOrWarnIcon(scorecardWarnings)} ${scorecardWarnings ? scorecardWarnings : 'No'} packages with OpenSSF Scorecard issues.`
-          ]
-        : [])
-    ])
-    .addRaw('See the Details below.')
+  const foundIssuesHeader =  'The following issues were found:'
+  core.summary.addRaw(foundIssuesHeader)
+  out.push(foundIssuesHeader)
+
+  const summaryList: string[] = [
+     ...(config.vulnerability_check
+      ? [
+          `${checkOrFailIcon(vulnerableChanges.length)} ${
+            vulnerableChanges.length
+          } vulnerable package(s)`
+        ]
+      : []),
+    ...(config.license_check
+      ? [
+          `${checkOrFailIcon(invalidLicenseChanges.forbidden.length)} ${
+            invalidLicenseChanges.forbidden.length
+          } package(s) with incompatible licenses`,
+          `${checkOrFailIcon(invalidLicenseChanges.unresolved.length)} ${
+            invalidLicenseChanges.unresolved.length
+          } package(s) with invalid SPDX license definitions`,
+          `${checkOrWarnIcon(invalidLicenseChanges.unlicensed.length)} ${
+            invalidLicenseChanges.unlicensed.length
+          } package(s) with unknown licenses.`
+        ]
+      : []),
+    ...(deniedChanges.length > 0
+      ? [
+          `${checkOrWarnIcon(deniedChanges.length)} ${
+            deniedChanges.length
+          } package(s) denied.`
+        ]
+      : []),
+    ...(config.show_openssf_scorecard && scorecardWarnings > 0
+      ? [
+          `${checkOrWarnIcon(scorecardWarnings)} ${scorecardWarnings ? scorecardWarnings : 'No'} packages with OpenSSF Scorecard issues.`
+        ]
+      : [])
+  ];
+
+  core.summary.addList(summaryList)
+  out.push(...summaryList)
+
+  core.summary.addRaw('See the Details below.')
+  out.push(`\n[View full job summary](${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${process.env.GITHUB_RUN_ID})`)
+
+  return out.join('\n')
 }

 function countScorecardWarnings(