-
Notifications
You must be signed in to change notification settings - Fork 55
chore: cache results of vulnerability scans #621
Changes from 16 commits
c221ed0
374fce2
f585290
144aaa7
e66b554
25a8a62
9e3f0fc
a662a54
a74b4c9
af24263
ba74944
6c3861c
19922c9
b0ce4b0
63adf2e
30d805e
4537020
29e1e63
67ce5d4
73ca81c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
import * as fs from 'fs' | ||
import { task } from 'gulp' | ||
import * as path from 'path' | ||
import debug from 'debug' | ||
|
||
import config from '../../../config' | ||
import sh from '../sh' | ||
|
||
const { paths } = config | ||
|
||
const SCAN_RESULTS_DIR_NAME = '.vuln-scans' | ||
|
||
const log = message => debug.log(message) | ||
log.success = message => debug.log(`✔ ${message}`) | ||
|
||
const ensureDirExists = path => { | ||
if (!fs.existsSync(path)) { | ||
sh(`mkdir -p ${path}`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for that we need Node LTS v10 (and this is, actually, the step we should make). Agreed to defer it to the follow-up PR, to be absolutely sure that all the necessary accompanying adjustments to the code will be made |
||
} | ||
} | ||
|
||
const getScanResultsDirPath = () => { | ||
return paths.base(SCAN_RESULTS_DIR_NAME) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we actually need it? As I see it can be a simple variable |
||
|
||
const getTodayScanFilePath = () => { | ||
const now = new Date() | ||
|
||
const fileName = `snyk-scanned-${now.getUTCFullYear()}-${now.getUTCMonth() + | ||
1}-${now.getUTCDate()}` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we split this to multiple variables? To avoid multiline there |
||
|
||
return path.resolve(getScanResultsDirPath(), fileName) | ||
} | ||
|
||
const recentlyChecked = () => { | ||
const recentCheckFilePath = getTodayScanFilePath() | ||
return fs.existsSync(recentCheckFilePath) | ||
} | ||
|
||
const registerRecentSucessfulScan = async () => { | ||
ensureDirExists(getScanResultsDirPath()) | ||
|
||
const recentScanFilePath = getTodayScanFilePath() | ||
await sh(`touch ${recentScanFilePath}`) | ||
} | ||
|
||
/** | ||
* The following strategy is used to perform vulnerabilites scan | ||
* - check if there is marker of recent sucessful scan | ||
* - if this marker exists, skip checks | ||
* - if there is no marker, perform check | ||
* - if check is successful, create successful check marker | ||
*/ | ||
task('test:vulns', async () => { | ||
if (recentlyChecked()) { | ||
log.success('Vulnerabilities check was already performed recently, skipping..') | ||
return | ||
} | ||
|
||
log('Scanning dependency packages for vulnerabilities..') | ||
await sh(`yarn snyk test`) | ||
log.success('Vulnerability scan is successfully passed.') | ||
|
||
registerRecentSucessfulScan() | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it okay that we restoring a cache with a key without the
epoch
variable?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is necessary because otherwise Circle CI won't update the cache entry if it has existed before (this is its feature). To avoid this 'rewrite ban' the following strategy was suggested: https://discuss.circleci.com/t/add-mechanism-to-update-existing-cache-key/9014/12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed, please add a small comment with this link before the
key
line 👍There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed to introduce comment for that