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

Get the right SHA for the snapshot depending on the event type #42

Merged
merged 2 commits into from
Mar 16, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions package-lock.json

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

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"@actions/exec": "^1.1.1",
"@actions/github": "^5.0.0",
"@octokit/rest": "^18.12.0",
"@octokit/webhooks-types": "^6.10.0",
"openapi-typescript": "^5.2.0",
"packageurl-js": "0.0.6"
},
Expand Down
87 changes: 77 additions & 10 deletions src/snapshot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { context } from '@actions/github'

import { Manifest } from './manifest'
import { PackageCache } from './package-cache'
import { Snapshot } from './snapshot'
import { shaFromContext, Snapshot } from './snapshot'

function roundTripJSON(obj: any): object {
return JSON.parse(JSON.stringify(obj))
Expand All @@ -20,20 +20,17 @@ manifest.addDirectDependency(
manifest.addIndirectDependency(cache.package('pkg:npm/%40actions/core@1.6.0'))

// add bogus git data to the context
context.sha = '0000000000000000000000000000000000000000'
context.sha = '1000000000000000000000000000000000000000'
context.ref = 'foo/bar/baz'
context.eventName = 'push'

describe('Snapshot', () => {
it('renders expected JSON', () => {
const snapshot = new Snapshot(
{
name: 'test detector',
url: 'https://github.com/github/dependency-submission-toolkit',
version: '0.0.1'
},
exampleDetector,
context,
{ id: '42', correlator: 'test' },
new Date('2022-06-04T05:07:06.457Z')
exampleJob,
exampleDate
)
snapshot.addManifest(manifest)
expect(roundTripJSON(snapshot)).toEqual({
Expand All @@ -49,7 +46,7 @@ describe('Snapshot', () => {
},
ref: 'foo/bar/baz',
scanned: '2022-06-04T05:07:06.457Z',
sha: '0000000000000000000000000000000000000000',
sha: '1000000000000000000000000000000000000000',
manifests: {
test: {
resolved: {
Expand All @@ -73,4 +70,74 @@ describe('Snapshot', () => {
}
})
})

it('gets the correct sha from the context when given a pull request', () => {
const prContext = context
const expectedSha = 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2'
prContext.eventName = 'pull_request'
prContext.payload.pull_request = {
number: 1,
head: {
sha: expectedSha
}
}

const snapshot = new Snapshot(
exampleDetector,
prContext,
exampleJob,
exampleDate
)

expect(snapshot.sha).toEqual(expectedSha)
})
})

describe('shaFromContext', () => {
it('gets the right sha from the context when given a pull_request event', () => {
const expectedSha = '1234567890123456789012345678901234567890'
const prContext = context
prContext.eventName = 'pull_request'
prContext.payload.pull_request = {
number: 1,
head: {
sha: expectedSha
}
}
expect(shaFromContext(prContext)).toEqual(expectedSha)
})

it('gets the right sha from the context when given a pull_request_review event', () => {
const expectedSha = 'abcdef1234567890123456789012345678901234'
const prReviewContext = context
prReviewContext.eventName = 'pull_request_review'
prReviewContext.payload.pull_request = {
number: 1,
head: {
sha: expectedSha
}
}
expect(shaFromContext(prReviewContext)).toEqual(expectedSha)
})

it('uses the primary sha from the context when given a push event', () => {
const expectedSha = 'def1234567890123456789012345678901234567'
const pushContext = context
pushContext.eventName = 'push'
pushContext.sha = expectedSha
expect(shaFromContext(pushContext)).toEqual(expectedSha)
})
})

const exampleDetector = {
name: 'test detector',
url: 'https://github.com/github/dependency-submission-toolkit',
version: '0.0.1'
}

const exampleJob = {
id: '42',
correlator: 'test'
}

const exampleDate = new Date('2022-06-04T05:07:06.457Z')
31 changes: 30 additions & 1 deletion src/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as core from '@actions/core'
import * as github from '@actions/github'
import { Octokit } from '@octokit/rest'
import { RequestError } from '@octokit/request-error'
import { PullRequestEvent } from '@octokit/webhooks-types'

import { Manifest } from './manifest'

Expand Down Expand Up @@ -33,6 +34,34 @@ export function jobFromContext(context: Context): Job {
}
}

/**
* shaFromContext returns the sha of the commit that triggered the action, or the head sha of the PR.
*
* See https://docs.github.com/en/actions/reference/events-that-trigger-workflows#pull_request for more details
* about why this function is necessary, but the short reason is that GITHUB_SHA is _not_ necessarily the head sha
* of the PR when the event is pull_request (or some other related event types).
*
Copy link
Contributor

Choose a reason for hiding this comment

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

☝️ This is such a gotcha for folks building DG snapshot Actions I'm glad to see us call it out in the toolkit code here! ❤️

🤔 I wonder what else we could do to document the "so you're building a DG snapshot Action" gotchas somewhere public for folks that don't choose to use the toolkit under the hood? Maybe a blog post about the quirk?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was thinking about sneaking something about this into the API docs, since I think that's all we really have now!

* @param {Context} context
* @returns {string}
*/
export function shaFromContext(context: Context): string {
const pullRequestEvents = [
'pull_request',
'pull_request_comment',
'pull_request_review',
'pull_request_review_comment'
// Note that pull_request_target is omitted here.
// That event runs in the context of the base commit of the PR,
// so the snapshot should not be associated with the head commit.
]
if (pullRequestEvents.includes(context.eventName)) {
const pr = (context.payload as PullRequestEvent).pull_request
return pr.head.sha
} else {
return context.sha
}
}

/**
* Detector provides metadata details about the detector used to generate the snapshot
*/
Expand Down Expand Up @@ -104,7 +133,7 @@ export class Snapshot {
this.detector = detector
this.version = version
this.job = job || jobFromContext(context)
this.sha = context.sha
this.sha = shaFromContext(context)
this.ref = context.ref
this.scanned = date.toISOString()
this.manifests = {}
Expand Down