From 16c28ee0b82576d9fc5992b68ab9d21908c62640 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Mon, 21 Jan 2019 17:08:27 +0100 Subject: [PATCH] Add initial support for Golang repos --- package.json | 3 +- src/annotation-result.ts | 3 +- src/dependency-check-gopkg.ts | 118 ++++++++++++++++++++++++++++++++++ src/dependency-check-json.ts | 80 ++++++++++++++++++++++- src/dependency-check.ts | 100 ++++++---------------------- src/dependency.ts | 1 + src/index.ts | 40 ++++++++---- yarn.lock | 5 ++ 8 files changed, 254 insertions(+), 96 deletions(-) create mode 100644 src/dependency-check-gopkg.ts diff --git a/package.json b/package.json index 68d8eea..8a67b76 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,8 @@ "@types/nock": "^9.3.0", "humanize-plus": "^1.8.2", "nock": "^10.0.0", - "probot": "^7.2.0" + "probot": "^7.2.0", + "toml": "^2.3.5" }, "description": "checks changes to packages.json to ensure that URL schemes match intended pattern and that forks are referenced with a tag, instead of a branch.", "devDependencies": { diff --git a/src/annotation-result.ts b/src/annotation-result.ts index f9205aa..6bbcd61 100644 --- a/src/annotation-result.ts +++ b/src/annotation-result.ts @@ -18,7 +18,7 @@ export class AnnotationResult { path: string, startLine: number, endLine: number, - rawDetails: string, + rawDetails?: string, ) { this.title = title this.message = message @@ -27,6 +27,5 @@ export class AnnotationResult { this.path = path this.startLine = startLine this.endLine = endLine - this.rawDetails = rawDetails } } diff --git a/src/dependency-check-gopkg.ts b/src/dependency-check-gopkg.ts new file mode 100644 index 0000000..b973c5c --- /dev/null +++ b/src/dependency-check-gopkg.ts @@ -0,0 +1,118 @@ +import { Context } from 'probot' // eslint-disable-line no-unused-vars +import toml from 'toml' +import { AnalysisResult } from './analysis-result' +import { Dependency } from './dependency' +import { AnnotationSource, + createAnnotation, + findLineInFileContent } from './dependency-check' + +export async function checkGopkgFileAsync( + analysisResult: AnalysisResult, + context: Context, + gopkgTomlFilename: string, + gopkgLockFilename: string, + headSHA: string, +) { + const gopkgTomlContentsResponse: any = + await context.github.repos.getContents(context.repo({ path: gopkgTomlFilename, ref: headSHA })) + context.log.debug(`get contents response for ${gopkgTomlFilename}: ${gopkgTomlContentsResponse.status}`) + + const gopkgLockContentsResponse: any = + await context.github.repos.getContents(context.repo({ path: gopkgLockFilename, ref: headSHA })) + context.log.debug(`get contents response for ${gopkgLockFilename}: ${gopkgLockContentsResponse.status}`) + + const gopkgTomlContents = Buffer.from(gopkgTomlContentsResponse.data.content, 'base64').toString('utf8') + const gopkgTomlContentsToml = toml.parse(gopkgTomlContents) + const gopkgLockContents = Buffer.from(gopkgLockContentsResponse.data.content, 'base64').toString('utf8') + const gopkgLockContentsToml = toml.parse(gopkgLockContents) + + await checkGoDependenciesAsync( + gopkgTomlContents, gopkgLockContents, + getDependenciesFromGopkg(gopkgTomlContentsToml, gopkgLockContentsToml), + gopkgTomlFilename, gopkgLockFilename, + analysisResult) +} + +function getDependenciesFromGopkg(gopkgTomlContentsToml: any, gopkgLockContentsToml: any): Dependency[] { + const dependencies: Dependency[] = [] + + for (const tomlDep of gopkgLockContentsToml.projects) { + dependencies.push({ + name: tomlDep.name, + url: tomlDep.source ? tomlDep.source : tomlDep.name, + + refType: getRefType(gopkgTomlContentsToml, tomlDep), + }) + } + + return dependencies +} + +function getRefType(gopkgTomlContentsToml: any, tomlDep: any): 'commit' | 'tag' | 'branch' | 'unknown' { + if (tomlDep.version) { + return 'tag' + } else if (tomlDep.branch) { + return 'branch' + } else { + const override: any = gopkgTomlContentsToml.override.find((o: any) => o.name === tomlDep.name) + if (override && override.revision) { + return 'commit' + } + } + + return 'unknown' +} + +export async function checkGoDependenciesAsync( + gopkgTomlContents: string, gopkgLockContents: string, + dependencies: Dependency[], + gopkgTomlFilename: string, gopkgLockFilename: string, + result: AnalysisResult, +) { + if (!dependencies || dependencies.length === 0) { + return + } + + // tslint:disable-next-line:max-line-length + result.checkedDependencyCount += dependencies.length + + for (const dependency of dependencies) { + const url = dependency.url + let line = findLineInFileContent(gopkgTomlContents, `name = "${url}"`) + let filename = gopkgTomlFilename + if (line < 0) { + line = findLineInFileContent(gopkgLockContents, `name = "${url}"`) + filename = gopkgLockFilename + } + const refType = dependency.refType + if (!refType) { + continue + } + + const annotation: AnnotationSource = { + dependency, + filename, + line, + } + const newAnnotation = (level: 'notice' | 'warning' | 'failure', title: string, message: string) => { + result.annotations.push(createAnnotation(annotation, level, title, message)) + } + switch (refType) { + case 'tag': + continue + case 'commit': + newAnnotation('notice', `Dependency '${url}' is not locked with a tag/release.`, + `A commit SHA is not a deterministic dependency locator. +If the commit is overwritten by a force-push, it will be impossible to rebuild the same output in the future.`, + ) + break + case 'branch': + newAnnotation('notice', // TODO: change this to 'failure' once we've fixed issues in the codebase + `Dependency '${url}' is not locked with a tag/release.`, + `A branch is not a deterministic dependency locator. +If the branch advances, it will be impossible to rebuild the same output in the future.`, + ) + break + } + } +} diff --git a/src/dependency-check-json.ts b/src/dependency-check-json.ts index 024e059..f27cb70 100644 --- a/src/dependency-check-json.ts +++ b/src/dependency-check-json.ts @@ -1,7 +1,10 @@ import { Context } from 'probot' // eslint-disable-line no-unused-vars import { AnalysisResult } from './analysis-result' import { Dependency } from './dependency' -import { checkDependenciesAsync } from './dependency-check' +import { AnnotationSource, + createAnnotation, + findLineInFileContent, + slowGetRefTypeAsync } from './dependency-check' export async function checkPackageFileAsync( analysisResult: AnalysisResult, @@ -41,3 +44,78 @@ function getDependenciesFromJSON(dependenciesJSON: any): Dependency[] { return dependencies } + +async function checkDependenciesAsync( + context: Context, + contents: string, + dependencies: Dependency[], + filename: string, + result: AnalysisResult, +) { + if (!dependencies || dependencies.length === 0) { + return + } + + // tslint:disable-next-line:max-line-length + const urlRegex = /^(http:\/\/|https:\/\/|git\+http:\/\/|git\+https:\/\/|ssh:\/\/|git\+ssh:\/\/|github:)([a-zA-Z0-9_\-./]+)(#(.*))?$/gm + const requiredProtocol = 'git+https://' + + result.checkedDependencyCount += dependencies.length + + for (const dependency of dependencies) { + const url = dependency.url + const match = urlRegex.exec(url) + if (!match) { + continue + } + + const protocol = match[1] + const address = protocol === 'github:' ? `github.com/${match[2]}` : match[2] + const tag = match.length > 4 ? match[4] : '' + const line = findLineInFileContent(contents, url) + const optimalAddress = address.endsWith('.git') ? address : address.concat('.git') + const refType = dependency.refType ? dependency.refType : await slowGetRefTypeAsync(context, address, tag) + const optimalTag = refType === 'tag' ? tag : '#' + const suggestedUrl = `${requiredProtocol}${optimalAddress}${optimalTag}` + + const annotation: AnnotationSource = { + dependency, + filename, + line, + } + const newAnnotation = (level: 'notice' | 'warning' | 'failure', title: string, message: string) => { + result.annotations.push(createAnnotation(annotation, + level, title, + message.concat(`\r\n\r\nSuggested URL: ${suggestedUrl}`))) + } + if (protocol !== requiredProtocol) { + newAnnotation('warning', `Found protocol ${protocol} being used in dependency`, + `Protocol should be ${requiredProtocol}.`) + } + if (protocol !== 'github:' && !address.endsWith('.git')) { + newAnnotation('warning', 'Address should end with .git for consistency.', + `Android builds have been known to fail when dependency addresses don't end with .git.`, + ) + } + if (!tag) { + newAnnotation('failure', 'Dependency is not locked with a tag/release.', + `${url} is not a deterministic dependency locator. +If the branch advances, it will be impossible to rebuild the same output in the future.`, + ) + } else if (refType === 'unknown') { + newAnnotation('failure', `Dependency is locked with an unknown ref-spec (\`${tag}\`).`, + `Please check that the tag \`${tag}\` exists in the target repository ${address}.`, + ) + } else if (refType === 'commit') { + newAnnotation('notice', 'Dependency is locked with a commit instead of a tag/release.', + `${url} is not a deterministic dependency locator. +If the commit is overwritten by a force-push, it will be impossible to rebuild the same output in the future.`, + ) + } else if (refType === 'branch') { + newAnnotation('failure', 'Dependency is locked with a branch instead of a tag/release.', + `${url} is not a deterministic dependency locator. +If the branch advances, it will be impossible to rebuild the same output in the future.`, + ) + } + } +} diff --git a/src/dependency-check.ts b/src/dependency-check.ts index 517d4f2..150ab6e 100644 --- a/src/dependency-check.ts +++ b/src/dependency-check.ts @@ -1,84 +1,20 @@ import Octokit from '@octokit/rest' import { Context } from 'probot' -import { AnalysisResult } from './analysis-result' import { AnnotationResult } from './annotation-result' import { Dependency } from './dependency' -export async function checkDependenciesAsync( - context: Context, - contents: string, - dependencies: Dependency[], - filename: string, - result: AnalysisResult, -) { - if (!dependencies || dependencies.length === 0) { - return - } - - // tslint:disable-next-line:max-line-length - const urlRegex = /^(http:\/\/|https:\/\/|git\+http:\/\/|git\+https:\/\/|ssh:\/\/|git\+ssh:\/\/|github:)([a-zA-Z0-9_\-./]+)(#(.*))?$/gm - const requiredProtocol = 'git+https://' - - result.checkedDependencyCount += dependencies.length - - for (const dependency of dependencies) { - const url = dependency.url - const match = urlRegex.exec(url) - if (!match) { - continue - } - - const protocol = match[1] - const address = protocol === 'github:' ? `github.com/${match[2]}` : match[2] - const tag = match.length > 4 ? match[4] : '' - const { line } = findLineColumn(contents, contents.indexOf(url)) - const optimalAddress = address.endsWith('.git') ? address : address.concat('.git') - const refType = await getRefTypeAsync(context, address, tag) - const optimalTag = refType === 'tag' ? tag : '#' - const suggestedUrl = `${requiredProtocol}${optimalAddress}${optimalTag}` - - const annotation: AnnotationSource = { - dependency, - filename, - line, - } - const newAnnotation = (level: 'notice' | 'warning' | 'failure', title: string, message: string) => { - result.annotations.push(createAnnotation(annotation, suggestedUrl, level, title, message)) - } - if (protocol !== requiredProtocol) { - newAnnotation('warning', `Found protocol ${protocol} being used in dependency`, - `Protocol should be ${requiredProtocol}.`) - } - if (protocol !== 'github:' && !address.endsWith('.git')) { - newAnnotation('warning', 'Address should end with .git for consistency.', - `Android builds have been known to fail when dependency addresses don't end with .git.`, - ) - } - if (!tag) { - newAnnotation('failure', 'Dependency is not locked with a tag/release.', - `${url} is not a deterministic dependency locator. -If the branch advances, it will be impossible to rebuild the same output in the future.`, - ) - } else if (refType === 'unknown') { - newAnnotation('failure', `Dependency is locked with an unknown ref-spec (\`${tag}\`).`, - `Please check that the tag \`${tag}\` exists in the target repository ${address}.`, - ) - } else if (refType !== 'tag') { - newAnnotation('failure', 'Dependency is locked with a branch instead of a tag/release.', - `${url} is not a deterministic dependency locator. -If the branch advances, it will be impossible to rebuild the same output in the future.`, - ) - } - } -} - -interface AnnotationSource { +export interface AnnotationSource { dependency: Dependency filename: string line: number } -function findLineColumn(contents: string, index: number) { +export function findLineInFileContent(contents: string, substring: string): number { + const index = contents.indexOf(substring) + if (index < 0) { + return -1 + } + const lines = contents.split('\n') const line = contents.substr(0, index).split('\n').length @@ -90,12 +26,11 @@ function findLineColumn(contents: string, index: number) { const col = index - startOfLineIndex - return { line, col } + return line } -function createAnnotation( +export function createAnnotation( annotationSource: AnnotationSource, - suggestedUrl: string, annotationLevel: 'notice' | 'warning' | 'failure', title: string, message: string, @@ -104,26 +39,25 @@ function createAnnotation( return new AnnotationResult( title, - message.concat(`\r\n\r\nSuggested URL: ${suggestedUrl}`), + message, annotationLevel, dependency, filename, line, line, - `{suggestedUrl: ${suggestedUrl}}`, ) } -async function getRefTypeAsync( +export async function slowGetRefTypeAsync( context: Context, address: string, tag: string, -): Promise<'tag' | 'branch' | 'unknown'> { +): Promise<'commit' | 'tag' | 'branch' | 'unknown'> { if (!tag) { return 'branch' } - // 'github.com/status-im/bignumber.js' + // e.g. 'github.com/status-im/bignumber.js' const parts = address.split('/') if (parts[0] === 'github.com') { const params: Octokit.GitdataGetRefParams = { @@ -149,6 +83,14 @@ async function getRefTypeAsync( context.log.trace(error) } + // check if it is a commit + try { + await context.github.gitdata.getCommit({ ...params, commit_sha: tag }) + return 'commit' + } catch (error) { + context.log.trace(error) + } + // probably not existing? return 'unknown' } diff --git a/src/dependency.ts b/src/dependency.ts index cc20bb1..4657f06 100644 --- a/src/dependency.ts +++ b/src/dependency.ts @@ -1,4 +1,5 @@ export interface Dependency { name: string url: string + refType?: 'commit' | 'tag' | 'branch' | 'unknown' } diff --git a/src/index.ts b/src/index.ts index db5a0a6..a479046 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,6 +5,7 @@ import Humanize from 'humanize-plus' import { Application, Context } from 'probot' // eslint-disable-line no-unused-vars import { AnalysisResult } from './analysis-result' import { AnnotationResult } from './annotation-result' +import { checkGopkgFileAsync } from './dependency-check-gopkg' import { checkPackageFileAsync } from './dependency-check-json' const pendingChecks: any = [] @@ -120,7 +121,7 @@ async function queueCheckAsync(context: Context, checkSuite: Octokit.ChecksCreat } const packageJsonFilenameRegex = /^(.*\/)?package\.json(.orig)?$/g - const gopkgFilenameRegex = /^Gopkg.toml$/g + const gopkgFilenameRegex = /^(.*\/)?Gopkg\.toml$/g if (!check.output) { check.output = { summary: '' } @@ -136,6 +137,13 @@ async function queueCheckAsync(context: Context, checkSuite: Octokit.ChecksCreat if (packageJsonFilenameRegex.test(file.filename)) { analysisResult.addPackageFilename(file.filename) await checkPackageFileAsync(analysisResult, context, file.filename, headSHA) + } else { + const match = gopkgFilenameRegex.exec(file.filename) + if (match) { + const path = match[1] ? match[1] : '' + analysisResult.addPackageFilename(file.filename) + await checkGopkgFileAsync(analysisResult, context, file.filename, `${path}Gopkg.lock`, headSHA) + } } break } @@ -210,7 +218,10 @@ function prepareCheckRunUpdate(check: Octokit.ChecksUpdateParams, analysisResult check.output.title = 'No changes to dependencies' check.output.summary = 'No changes detected to package.json files' } - } else if (analysisResult.annotations.length === 0) { + } else if (analysisResult.annotations + .map((a) => a.annotationLevel) + .filter((l) => l === 'warning' || l === 'failure') + .length === 0) { check.conclusion = 'success' if (check.output) { check.output.title = 'All dependencies are good!' @@ -225,18 +236,21 @@ function prepareCheckRunUpdate(check: Octokit.ChecksUpdateParams, analysisResult analysisResult.annotations.filter((a) => a.annotationLevel === level).length const warnings = getAnnotationCount('warning') const failures = getAnnotationCount('failure') + const notices = getAnnotationCount('notice') const uniqueProblemDependencies = [ ...new Set(analysisResult.annotations.map((a) => a.dependency.name)) ] - check.output.title = `${Humanize.boundedNumber(failures + warnings, 10)} ${Humanize.pluralize( - failures + warnings, 'problem', - )} detected` - check.output.summary = `Checked ${analysisResult.checkedDependencyCount} ${Humanize.pluralize( - analysisResult.checkedDependencyCount, 'dependency', 'dependencies', - )} in ${Humanize.oxford(analysisResult.sourceFilenames.map((f) => `\`${f}\``), 3)}. -${Humanize.boundedNumber(failures, 10)} ${Humanize.pluralize( - failures, 'failure', - )}, ${Humanize.boundedNumber(warnings, 10)} ${Humanize.pluralize( - warnings, 'warning')} in ${Humanize.oxford( - uniqueProblemDependencies.map((f) => `\`${f}\``), 3)} need your attention!` + const humanizedFilenames = Humanize.oxford(analysisResult.sourceFilenames.map((f) => `\`${f}\``), 3) + const problemSummary = [ + failures > 0 ? `${failures} ${Humanize.pluralize(failures, 'failure')}` : undefined, + warnings > 0 ? `${warnings} ${Humanize.pluralize(warnings, 'warning')}` : undefined, + notices > 0 ? `${notices} ${Humanize.pluralize(notices, 'notice')}` : undefined, + ] + const humanizedProblemDeps = Humanize.oxford(uniqueProblemDependencies.map((f) => `\`${f}\``), 3) + const humanizeItemCount = (count: number, singular: string, plural: string) => + `${Humanize.boundedNumber(count, 10)} ${Humanize.pluralize(count, singular, plural)}` + const humanizedDepCount = humanizeItemCount(analysisResult.checkedDependencyCount, 'dependency', 'dependencies') + check.output.title = `${humanizeItemCount(failures + warnings, 'problem', 'problems')} detected` + check.output.summary = `Checked ${humanizedDepCount} in ${humanizedFilenames}. +${Humanize.oxford(problemSummary.filter((p) => p !== undefined))} in ${humanizedProblemDeps} need your attention!` } } } diff --git a/yarn.lock b/yarn.lock index 0b58e5d..281aa26 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4725,6 +4725,11 @@ to-regex@^3.0.1, to-regex@^3.0.2: regex-not "^1.0.2" safe-regex "^1.1.0" +toml@^2.3.5: + version "2.3.5" + resolved "https://registry.yarnpkg.com/toml/-/toml-2.3.5.tgz#a1f5d7f7efd300fa426258f3e74374536191e3db" + integrity sha512-ulY/Z2yPWKl/3JvGJvnEe7mXqVt2+TtDoRxJNgTAwO+3lwXefeCHS697NN0KRy6q7U/b1MnSnj/UGF/4U0U2WQ== + touch@^3.1.0: version "3.1.0" resolved "https://registry.yarnpkg.com/touch/-/touch-3.1.0.tgz#fe365f5f75ec9ed4e56825e0bb76d24ab74af83b"