From 5f8473753973398f07a548642578a81bc0da632e Mon Sep 17 00:00:00 2001 From: Florian Greinacher Date: Sat, 20 Mar 2021 22:36:43 +0100 Subject: [PATCH] feat: expose post upgrade command errors in PRs (#8944) --- lib/workers/branch/index.spec.ts | 94 ++++++++++++++++++++++++++++++++ lib/workers/branch/index.ts | 44 +++++++++++---- 2 files changed, 126 insertions(+), 12 deletions(-) diff --git a/lib/workers/branch/index.spec.ts b/lib/workers/branch/index.spec.ts index 298434d13caef1..51ff0568bcf091 100644 --- a/lib/workers/branch/index.spec.ts +++ b/lib/workers/branch/index.spec.ts @@ -9,6 +9,7 @@ import * as _npmPostExtract from '../../manager/npm/post-update'; import { PrState } from '../../types'; import * as _exec from '../../util/exec'; import { File, StatusResult } from '../../util/git'; +import * as _sanitize from '../../util/sanitize'; import * as _limits from '../global/limits'; import * as _prWorker from '../pr'; import { BranchConfig, PrResult, ProcessBranchResult } from '../types'; @@ -29,6 +30,7 @@ jest.mock('./automerge'); jest.mock('./commit'); jest.mock('../pr'); jest.mock('../../util/exec'); +jest.mock('../../util/sanitize'); jest.mock('../../util/git'); jest.mock('fs-extra'); jest.mock('../global/limits'); @@ -42,6 +44,7 @@ const automerge = mocked(_automerge); const commit = mocked(_commit); const prWorker = mocked(_prWorker); const exec = mocked(_exec); +const sanitize = mocked(_sanitize); const fs = mocked(_fs); const limits = mocked(_limits); @@ -65,7 +68,19 @@ describe('workers/branch', () => { } as never; schedule.isScheduledNow.mockReturnValue(true); commit.commitFilesToBranch.mockResolvedValue('abc123'); + + platform.massageMarkdown.mockImplementation((prBody) => prBody); + prWorker.ensurePr.mockResolvedValue({ + prResult: PrResult.Created, + pr: { + title: '', + sourceBranch: '', + state: '', + body: '', + }, + }); setAdminConfig(); + sanitize.sanitize.mockImplementation((input) => input); }); afterEach(() => { platform.ensureComment.mockClear(); @@ -743,6 +758,85 @@ describe('workers/branch', () => { expect(exec.exec).toHaveBeenCalledWith('echo semver', { cwd: '/localDir', }); + const errorMessage = expect.stringContaining( + "Post-upgrade command 'disallowed task' does not match allowed pattern '^echo {{{versioning}}}$'" + ); + expect(platform.ensureComment).toHaveBeenCalledWith( + expect.objectContaining({ + content: errorMessage, + }) + ); + expect(sanitize.sanitize).toHaveBeenCalledWith(errorMessage); + }); + + it('handles post-upgrade task exec errors', async () => { + const updatedPackageFile: File = { + name: 'pom.xml', + contents: 'pom.xml file contents', + }; + getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({ + updatedPackageFiles: [updatedPackageFile], + artifactErrors: [], + } as never); + npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ + artifactErrors: [], + updatedArtifacts: [ + { + name: 'yarn.lock', + contents: Buffer.from([1, 2, 3]) /* Binary content */, + }, + ], + } as never); + git.branchExists.mockReturnValueOnce(true); + platform.getBranchPr.mockResolvedValueOnce({ + title: 'rebase!', + state: PrState.Open, + body: `- [x] `, + } as never); + git.isBranchModified.mockResolvedValueOnce(true); + git.getRepoStatus.mockResolvedValueOnce({ + modified: ['modified_file'], + not_added: [], + deleted: ['deleted_file'], + } as StatusResult); + + fs.outputFile.mockReturnValue(); + fs.readFile.mockResolvedValueOnce(Buffer.from('modified file content')); + + schedule.isScheduledNow.mockReturnValueOnce(false); + commit.commitFilesToBranch.mockResolvedValueOnce(null); + + const adminConfig = { + allowedPostUpgradeCommands: ['^exit 1$'], + allowPostUpgradeCommandTemplating: true, + trustLevel: 'high', + }; + setAdminConfig(adminConfig); + + exec.exec.mockRejectedValue(new Error('Meh, this went wrong!')); + + await branchWorker.processBranch({ + ...config, + localDir: '/localDir', + upgrades: [ + { + ...defaultConfig, + depName: 'some-dep-name', + postUpgradeTasks: { + commands: ['exit 1'], + fileFilters: ['modified_file', 'deleted_file'], + }, + } as never, + ], + }); + + const errorMessage = expect.stringContaining('Meh, this went wrong!'); + expect(platform.ensureComment).toHaveBeenCalledWith( + expect.objectContaining({ + content: errorMessage, + }) + ); + expect(sanitize.sanitize).toHaveBeenCalledWith(errorMessage); }); it('executes post-upgrade tasks with disabled post-upgrade command templating', async () => { diff --git a/lib/workers/branch/index.ts b/lib/workers/branch/index.ts index 34df7533bade98..ae4eb0ccc9573d 100644 --- a/lib/workers/branch/index.ts +++ b/lib/workers/branch/index.ts @@ -32,6 +32,7 @@ import { isBranchModified, } from '../../util/git'; import { regEx } from '../../util/regex'; +import { sanitize } from '../../util/sanitize'; import * as template from '../../util/template'; import { Limit, isLimitReached } from '../global/limits'; import { checkAutoMerge, ensurePr, getPlatformPrOptions } from '../pr'; @@ -396,20 +397,29 @@ export async function processBranch( regEx(pattern).test(cmd) ) ) { - const compiledCmd = allowPostUpgradeCommandTemplating - ? template.compile(cmd, upgrade) - : cmd; + try { + const compiledCmd = allowPostUpgradeCommandTemplating + ? template.compile(cmd, upgrade) + : cmd; - logger.debug({ cmd: compiledCmd }, 'Executing post-upgrade task'); - - const execResult = await exec(compiledCmd, { - cwd: config.localDir, - }); + logger.debug( + { cmd: compiledCmd }, + 'Executing post-upgrade task' + ); + const execResult = await exec(compiledCmd, { + cwd: config.localDir, + }); - logger.debug( - { cmd: compiledCmd, ...execResult }, - 'Executed post-upgrade task' - ); + logger.debug( + { cmd: compiledCmd, ...execResult }, + 'Executed post-upgrade task' + ); + } catch (error) { + config.artifactErrors.push({ + lockFile: upgrade.packageFile, + stderr: sanitize(error.message), + }); + } } else { logger.warn( { @@ -418,6 +428,16 @@ export async function processBranch( }, 'Post-upgrade task did not match any on allowed list' ); + config.artifactErrors.push({ + lockFile: upgrade.packageFile, + stderr: sanitize( + `Post-upgrade command '${cmd}' does not match allowed pattern${ + allowedPostUpgradeCommands.length === 1 ? '' : 's' + } ${allowedPostUpgradeCommands + .map((x) => `'${x}'`) + .join(', ')}` + ), + }); } }