Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dr Snaps launch PR #556

Merged
merged 27 commits into from Sep 7, 2023
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion __tests__/summary.test.ts
Expand Up @@ -26,7 +26,9 @@ const defaultConfig: ConfigurationOptions = {
deny_licenses: [],
deny_packages: [],
deny_groups: [],
comment_summary_in_pr: true
comment_summary_in_pr: true,
retry_on_snapshot_warnings: false,
retry_on_snapshot_warnings_timeout: 120
}

const changesWithEmptyManifests: Changes = [
Expand Down
8 changes: 8 additions & 0 deletions action.yml
Expand Up @@ -53,6 +53,14 @@ inputs:
deny-groups:
description: A comma-separated list of package URLs for group(s)/namespace(s) to deny (e.g. "pkg:npm/express, pkg:pip/pycrypto")
required: false
retry-on-snapshot-warnings:
description: Whether to retry on snapshot warnings
required: false
default: true
febuiles marked this conversation as resolved.
Show resolved Hide resolved
retry-on-snapshot-warnings-timeout:
description: How many seconds before bailing on retries
juxtin marked this conversation as resolved.
Show resolved Hide resolved
required: false
default: 120
runs:
using: 'node16'
main: 'dist/index.js'
67 changes: 51 additions & 16 deletions dist/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion scripts/create_summary.ts
Expand Up @@ -30,7 +30,9 @@ const defaultConfig: ConfigurationOptions = {
'pkg:pip/certifi',
'pkg:pip/pycrypto@2.6.1'
],
comment_summary_in_pr: 'never'
comment_summary_in_pr: true,
retry_on_snapshot_warnings: false,
retry_on_snapshot_warnings_timeout: 120
}

const tmpDir = path.resolve(__dirname, '../tmp')
Expand Down
16 changes: 15 additions & 1 deletion src/config.ts
Expand Up @@ -41,6 +41,12 @@ function readInlineConfig(): ConfigurationOptionsPartial {
const base_ref = getOptionalInput('base-ref')
const head_ref = getOptionalInput('head-ref')
const comment_summary_in_pr = getOptionalInput('comment-summary-in-pr')
const retry_on_snapshot_warnings = getOptionalBoolean(
'retry-on-snapshot-warnings'
)
const retry_on_snapshot_warnings_timeout = getOptionalNumber(
'retry-on-snapshot-warnings-timeout'
)

validatePURL(allow_dependencies_licenses)
validateLicenses('allow-licenses', allow_licenses)
Expand All @@ -59,14 +65,22 @@ function readInlineConfig(): ConfigurationOptionsPartial {
vulnerability_check,
base_ref,
head_ref,
comment_summary_in_pr
comment_summary_in_pr,
retry_on_snapshot_warnings,
retry_on_snapshot_warnings_timeout
}

return Object.fromEntries(
Object.entries(keys).filter(([_, value]) => value !== undefined)
)
}

function getOptionalNumber(name: string): number | undefined {
const value = core.getInput(name)
const parsed = z.string().regex(/^\d+$/).transform(Number).safeParse(value)
return parsed.success ? parsed.data : undefined
}

function getOptionalBoolean(name: string): boolean | undefined {
const value = core.getInput(name)
return value.length > 0 ? core.getBooleanInput(name) : undefined
Expand Down
54 changes: 48 additions & 6 deletions src/main.ts
Expand Up @@ -18,18 +18,60 @@ import {groupDependenciesByManifest} from './utils'
import {commentPr} from './comment-pr'
import {getDeniedChanges} from './deny'

async function delay(ms: number): Promise<void> {
return new Promise(resolve => setTimeout(resolve, ms))
}

async function getComparison(
baseRef: string,
headRef: string,
retryOpts?: {
retryUntil: number
retryDelay: number
}
): ReturnType<typeof dependencyGraph.compare> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Q) why do we do this instead of specifying an explicit type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess is that it's because this is just a wrapper function, so passing through the type as well as the value could simplify things when refactoring.

const comparison = await dependencyGraph.compare({
owner: github.context.repo.owner,
repo: github.context.repo.repo,
baseRef,
headRef
})

if (comparison.snapshot_warnings.trim() !== '') {
core.info(comparison.snapshot_warnings)
if (retryOpts !== undefined) {
if (retryOpts.retryUntil < Date.now()) {
core.info(`Retry timeout exceeded. Proceeding...`)
return comparison
} else {
core.info(`Retrying in ${retryOpts.retryDelay} seconds...`)
await delay(retryOpts.retryDelay * 1000)
return getComparison(baseRef, headRef, retryOpts)
}
}
}

return comparison
}

async function run(): Promise<void> {
try {
const config = await readConfig()

const refs = getRefs(config, github.context)

const comparison = await dependencyGraph.compare({
owner: github.context.repo.owner,
repo: github.context.repo.repo,
baseRef: refs.base,
headRef: refs.head
})
const comparison = await getComparison(
refs.base,
refs.head,
config.retry_on_snapshot_warnings
? {
retryUntil:
Date.now() + config.retry_on_snapshot_warnings_timeout * 1000,
retryDelay: 10
}
: undefined
)

const changes = comparison.changes
const snapshot_warnings = comparison.snapshot_warnings

Expand Down
2 changes: 2 additions & 0 deletions src/schemas.ts
Expand Up @@ -49,6 +49,8 @@ export const ConfigurationOptionsSchema = z
config_file: z.string().optional(),
base_ref: z.string().optional(),
head_ref: z.string().optional(),
retry_on_snapshot_warnings: z.boolean().default(false),
retry_on_snapshot_warnings_timeout: z.number().default(120),
comment_summary_in_pr: z
.union([
z.preprocess(
Expand Down
11 changes: 1 addition & 10 deletions src/summary.ts
Expand Up @@ -232,19 +232,10 @@ export function addScannedDependencies(changes: Changes): void {
}

export function addSnapshotWarnings(warnings: string): void {
// For now, we want to ignore warnings that just complain
// about missing snapshots on the head SHA. This is a product
// decision to avoid presenting warnings to users who simply
// don't use snapshots.
const ignore_regex = new RegExp(/No.*snapshot.*found.*head.*/, 'i')
if (ignore_regex.test(warnings)) {
return
}

core.summary.addHeading('Snapshot Warnings', 2)
core.summary.addQuote(`${icons.warning}: ${warnings}`)
core.summary.addRaw(
'Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice.'
'Re-running this action after a short time may resolve the issue. See <a href="https://docs.github.com/en/code-security/supply-chain-security/understanding-your-software-supply-chain/about-dependency-review#best-practices-for-using-the-dependency-review-api-and-the-dependency-submission-api-together">the documentation</a> for more information and troubleshooting advice.'
)
}

Expand Down