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

Show snapshot warnings in the summary #439

Merged
merged 5 commits into from
Apr 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 55 additions & 0 deletions __tests__/summary.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,45 @@ const defaultConfig: ConfigurationOptions = {
comment_summary_in_pr: true
}

const changesWithEmptyManifests: Changes = [
{
change_type: 'added',
manifest: '',
ecosystem: 'unknown',
name: 'castore',
version: '0.1.17',
package_url: 'pkg:hex/castore@0.1.17',
license: null,
source_repository_url: null,
scope: 'runtime',
vulnerabilities: []
},
{
change_type: 'added',
manifest: '',
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ thanks for capturing this invariant in a test!

ecosystem: 'unknown',
name: 'connection',
version: '1.1.0',
package_url: 'pkg:hex/connection@1.1.0',
license: null,
source_repository_url: null,
scope: 'runtime',
vulnerabilities: []
},
{
change_type: 'added',
manifest: 'python/dist-info/METADATA',
ecosystem: 'pip',
name: 'pygments',
version: '2.6.1',
package_url: 'pkg:pypi/pygments@2.6.1',
license: 'BSD-2-Clause',
source_repository_url: 'https://github.com/pygments/pygments',
scope: 'runtime',
vulnerabilities: []
}
]

test('prints headline as h1', () => {
summary.addSummaryToSummary(
emptyChanges,
Expand Down Expand Up @@ -65,6 +104,22 @@ test('only includes "No license issues found"-message if "vulnerability_check" i
expect(text).toContain('✅ No license issues found.')
})

test('groups dependencies with empty manifest paths together', () => {
summary.addSummaryToSummary(
changesWithEmptyManifests,
emptyInvalidLicenseChanges,
defaultConfig
)
summary.addScannedDependencies(changesWithEmptyManifests)
const text = core.summary.stringify()

expect(text).toContain('<summary>Unnamed Manifest</summary>')
expect(text).toContain('castore')
expect(text).toContain('connection')
expect(text).toContain('<summary>python/dist-info/METADATA</summary>')
expect(text).toContain('pygments')
})

test('does not include status section if nothing was found', () => {
summary.addSummaryToSummary(
emptyChanges,
Expand Down
60 changes: 52 additions & 8 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.

30 changes: 26 additions & 4 deletions src/dependency-graph.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import * as core from '@actions/core'
import * as githubUtils from '@actions/github/lib/utils'
import * as retry from '@octokit/plugin-retry'
import {Changes, ChangesSchema} from './schemas'
import {
ChangesSchema,
ComparisonResponse,
ComparisonResponseSchema
} from './schemas'

const retryingOctokit = githubUtils.GitHub.plugin(retry.retry)
const SnapshotWarningsHeader = 'x-github-dependency-graph-snapshot-warnings'
const octo = new retryingOctokit(
githubUtils.getOctokitOptions(core.getInput('repo-token', {required: true}))
)
Expand All @@ -18,14 +23,31 @@ export async function compare({
repo: string
baseRef: string
headRef: string
}): Promise<Changes> {
}): Promise<ComparisonResponse> {
let snapshot_warnings = ''
const changes = await octo.paginate(
'GET /repos/{owner}/{repo}/dependency-graph/compare/{basehead}',
{
method: 'GET',
url: '/repos/{owner}/{repo}/dependency-graph/compare/{basehead}',
owner,
repo,
basehead: `${baseRef}...${headRef}`
},
response => {
if (
response.headers[SnapshotWarningsHeader] &&
typeof response.headers[SnapshotWarningsHeader] === 'string'
) {
snapshot_warnings = Buffer.from(
response.headers[SnapshotWarningsHeader],
'base64'
).toString('utf-8')
}
return ChangesSchema.parse(response.data)
}
)
return ChangesSchema.parse(changes)
return ComparisonResponseSchema.parse({
changes,
snapshot_warnings
})
}
8 changes: 7 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ async function run(): Promise<void> {
const config = await readConfig()
const refs = getRefs(config, github.context)

const changes = await dependencyGraph.compare({
const comparison = await dependencyGraph.compare({
owner: github.context.repo.owner,
repo: github.context.repo.repo,
baseRef: refs.base,
headRef: refs.head
})
const changes = comparison.changes
const snapshot_warnings = comparison.snapshot_warnings

if (!changes) {
core.info('No Dependency Changes found. Skipping Dependency Review.')
Expand Down Expand Up @@ -65,6 +67,10 @@ async function run(): Promise<void> {
config
)

if (snapshot_warnings) {
summary.addSnapshotWarnings(snapshot_warnings)
}

if (config.vulnerability_check) {
summary.addChangeVulnerabilitiesToSummary(vulnerableChanges, minSeverity)
printVulnerabilitiesBlock(vulnerableChanges, minSeverity)
Expand Down
5 changes: 5 additions & 0 deletions src/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,14 @@ export const ConfigurationOptionsSchema = z
})

export const ChangesSchema = z.array(ChangeSchema)
export const ComparisonResponseSchema = z.object({
changes: z.array(ChangeSchema),
snapshot_warnings: z.string()
})

export type Change = z.infer<typeof ChangeSchema>
export type Changes = z.infer<typeof ChangesSchema>
export type ComparisonResponse = z.infer<typeof ComparisonResponseSchema>
export type ConfigurationOptions = z.infer<typeof ConfigurationOptionsSchema>
export type Severity = z.infer<typeof SeveritySchema>
export type Scope = (typeof SCOPES)[number]
17 changes: 17 additions & 0 deletions src/summary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,23 @@ 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.'
)
}

function countLicenseIssues(
invalidLicenseChanges: InvalidLicenseChanges
): number {
Expand Down
4 changes: 3 additions & 1 deletion src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ export function groupDependenciesByManifest(
): Map<string, Changes> {
const dependencies: Map<string, Changes> = new Map()
for (const change of changes) {
const manifestName = change.manifest
// If the manifest is null or empty, give it a name now to avoid
// breaking the HTML rendering later
const manifestName = change.manifest || 'Unnamed Manifest'

if (dependencies.get(manifestName) === undefined) {
dependencies.set(manifestName, [])
Expand Down