From 1c8eb34876e99d15a3b84e2422aceb34b24a9fe2 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov <zharinov@users.noreply.github.com> Date: Thu, 16 May 2024 06:56:00 -0300 Subject: [PATCH] feat: Support custom artifact notice (#28957) Co-authored-by: Michael Kriese <michael.kriese@visualon.de> Co-authored-by: Rhys Arkins <rhys@arkins.net> --- lib/modules/manager/types.ts | 7 +++ .../__snapshots__/get-updated.spec.ts.snap | 14 +++++ .../update/branch/get-updated.spec.ts | 58 +++++++++++++++++++ .../repository/update/branch/get-updated.ts | 31 ++++++++-- .../repository/update/branch/index.spec.ts | 40 +++++++++++++ lib/workers/repository/update/branch/index.ts | 55 +++++++++++++----- lib/workers/types.ts | 2 + 7 files changed, 188 insertions(+), 19 deletions(-) diff --git a/lib/modules/manager/types.ts b/lib/modules/manager/types.ts index 754d68da8a..b6a93703f2 100644 --- a/lib/modules/manager/types.ts +++ b/lib/modules/manager/types.ts @@ -182,6 +182,11 @@ export interface Upgrade<T = Record<string, any>> extends PackageDependency<T> { replaceString?: string; } +export interface ArtifactNotice { + file: string; + message: string; +} + export interface ArtifactError { lockFile?: string; stderr?: string; @@ -190,10 +195,12 @@ export interface ArtifactError { export type UpdateArtifactsResult = | { file?: FileChange; + notice?: ArtifactNotice; artifactError?: undefined; } | { file?: undefined; + notice?: undefined; artifactError?: ArtifactError; }; diff --git a/lib/workers/repository/update/branch/__snapshots__/get-updated.spec.ts.snap b/lib/workers/repository/update/branch/__snapshots__/get-updated.spec.ts.snap index 55675aa4f8..96c1cbea07 100644 --- a/lib/workers/repository/update/branch/__snapshots__/get-updated.spec.ts.snap +++ b/lib/workers/repository/update/branch/__snapshots__/get-updated.spec.ts.snap @@ -3,6 +3,7 @@ exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() bumps versions in autoReplace managers 1`] = ` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": undefined, "updatedArtifacts": [], "updatedPackageFiles": [ @@ -18,6 +19,7 @@ exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() b exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() bumps versions in updateDependency managers 1`] = ` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": undefined, "updatedArtifacts": [], "updatedPackageFiles": [ @@ -33,6 +35,7 @@ exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() b exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() handles autoreplace base updated 1`] = ` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": undefined, "updatedArtifacts": [], "updatedPackageFiles": [ @@ -48,6 +51,7 @@ exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() h exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() handles autoreplace branch needs update 1`] = ` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": false, "updatedArtifacts": [], "updatedPackageFiles": [ @@ -63,6 +67,7 @@ exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() h exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() handles content change 1`] = ` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": false, "updatedArtifacts": [], "updatedPackageFiles": [ @@ -78,6 +83,7 @@ exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() h exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() handles git submodules 1`] = ` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": undefined, "updatedArtifacts": [], "updatedPackageFiles": [ @@ -93,6 +99,7 @@ exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() h exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() handles isRemediation rebase 1`] = ` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": false, "updatedArtifacts": [], "updatedPackageFiles": [ @@ -108,6 +115,7 @@ exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() h exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() handles isRemediation success 1`] = ` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": undefined, "updatedArtifacts": [], "updatedPackageFiles": [ @@ -128,6 +136,7 @@ exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() h "stderr": "some error", }, ], + "artifactNotices": [], "reuseExistingBranch": true, "updatedArtifacts": [], "updatedPackageFiles": [ @@ -143,6 +152,7 @@ exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() h exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() handles lock files 1`] = ` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": true, "updatedArtifacts": [ { @@ -164,6 +174,7 @@ exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() h exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() handles lockFileMaintenance 1`] = ` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": undefined, "updatedArtifacts": [ { @@ -184,6 +195,7 @@ exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() h "stderr": "some error", }, ], + "artifactNotices": [], "reuseExistingBranch": undefined, "updatedArtifacts": [], "updatedPackageFiles": [], @@ -193,6 +205,7 @@ exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() h exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() update artifacts on update-lockfile strategy 1`] = ` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": undefined, "updatedArtifacts": [ { @@ -214,6 +227,7 @@ exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() u exports[`workers/repository/update/branch/get-updated getUpdatedPackageFiles() update artifacts on update-lockfile strategy with no updateLockedDependency 1`] = ` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": undefined, "updatedArtifacts": [ { diff --git a/lib/workers/repository/update/branch/get-updated.spec.ts b/lib/workers/repository/update/branch/get-updated.spec.ts index 04a20633fd..64a614e517 100644 --- a/lib/workers/repository/update/branch/get-updated.spec.ts +++ b/lib/workers/repository/update/branch/get-updated.spec.ts @@ -1,9 +1,11 @@ +import { mockDeep } from 'jest-mock-extended'; import { git, mocked } from '../../../../../test/util'; import { GitRefsDatasource } from '../../../../modules/datasource/git-refs'; import * as _batectWrapper from '../../../../modules/manager/batect-wrapper'; import * as _bundler from '../../../../modules/manager/bundler'; import * as _composer from '../../../../modules/manager/composer'; import * as _gitSubmodules from '../../../../modules/manager/git-submodules'; +import * as _gomod from '../../../../modules/manager/gomod'; import * as _helmv3 from '../../../../modules/manager/helmv3'; import * as _npm from '../../../../modules/manager/npm'; import * as _pep621 from '../../../../modules/manager/pep621'; @@ -16,6 +18,7 @@ import { getUpdatedPackageFiles } from './get-updated'; const bundler = mocked(_bundler); const composer = mocked(_composer); const gitSubmodules = mocked(_gitSubmodules); +const gomod = mocked(_gomod); const helmv3 = mocked(_helmv3); const npm = mocked(_npm); const batectWrapper = mocked(_batectWrapper); @@ -28,6 +31,7 @@ jest.mock('../../../../modules/manager/composer'); jest.mock('../../../../modules/manager/helmv3'); jest.mock('../../../../modules/manager/npm'); jest.mock('../../../../modules/manager/git-submodules'); +jest.mock('../../../../modules/manager/gomod', () => mockDeep()); jest.mock('../../../../modules/manager/batect-wrapper'); jest.mock('../../../../modules/manager/pep621'); jest.mock('../../../../modules/manager/poetry'); @@ -77,6 +81,7 @@ describe('workers/repository/update/branch/get-updated', () => { reuseExistingBranch: undefined, updatedArtifacts: [], updatedPackageFiles: [], + artifactNotices: [], }); }); @@ -110,6 +115,7 @@ describe('workers/repository/update/branch/get-updated', () => { reuseExistingBranch: undefined, updatedArtifacts: [], updatedPackageFiles: [], + artifactNotices: [], }); }); @@ -178,6 +184,54 @@ describe('workers/repository/update/branch/get-updated', () => { }); }); + it('handles artifact notices', async () => { + config.reuseExistingBranch = true; + config.upgrades.push({ + packageFile: 'go.mod', + manager: 'gomod', + branchName: 'foo/bar', + }); + gomod.updateDependency.mockReturnValue('some new content'); + gomod.updateArtifacts.mockResolvedValueOnce([ + { + file: { + type: 'addition', + path: 'go.mod', + contents: 'some content', + }, + notice: { + file: 'go.mod', + message: 'some notice', + }, + }, + ]); + const res = await getUpdatedPackageFiles(config); + expect(res).toEqual({ + artifactErrors: [], + artifactNotices: [ + { + file: 'go.mod', + message: 'some notice', + }, + ], + reuseExistingBranch: false, + updatedArtifacts: [ + { + contents: 'some content', + path: 'go.mod', + type: 'addition', + }, + ], + updatedPackageFiles: [ + { + contents: 'some new content', + path: 'go.mod', + type: 'addition', + }, + ], + }); + }); + it('handles lockFileMaintenance', async () => { config.upgrades.push({ manager: 'composer', @@ -353,6 +407,7 @@ describe('workers/repository/update/branch/get-updated', () => { expect(res).toMatchInlineSnapshot(` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": undefined, "updatedArtifacts": [], "updatedPackageFiles": [], @@ -533,6 +588,7 @@ describe('workers/repository/update/branch/get-updated', () => { expect(res).toMatchInlineSnapshot(` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": undefined, "updatedArtifacts": [], "updatedPackageFiles": [], @@ -556,6 +612,7 @@ describe('workers/repository/update/branch/get-updated', () => { expect(res).toMatchInlineSnapshot(` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": false, "updatedArtifacts": [], "updatedPackageFiles": [], @@ -581,6 +638,7 @@ describe('workers/repository/update/branch/get-updated', () => { expect(res).toMatchInlineSnapshot(` { "artifactErrors": [], + "artifactNotices": [], "reuseExistingBranch": false, "updatedArtifacts": [], "updatedPackageFiles": [], diff --git a/lib/workers/repository/update/branch/get-updated.ts b/lib/workers/repository/update/branch/get-updated.ts index c79924d2ed..d0f6a57f3e 100644 --- a/lib/workers/repository/update/branch/get-updated.ts +++ b/lib/workers/repository/update/branch/get-updated.ts @@ -5,6 +5,7 @@ import { logger } from '../../../../logger'; import { get } from '../../../../modules/manager'; import type { ArtifactError, + ArtifactNotice, PackageDependency, PackageFile, UpdateArtifact, @@ -21,6 +22,7 @@ export interface PackageFilesResult { reuseExistingBranch?: boolean; updatedPackageFiles: FileChange[]; updatedArtifacts: FileChange[]; + artifactNotices: ArtifactNotice[]; } async function getFileContent( @@ -288,6 +290,7 @@ export async function getUpdatedPackageFiles( })); const updatedArtifacts: FileChange[] = []; const artifactErrors: ArtifactError[] = []; + const artifactNotices: ArtifactNotice[] = []; // istanbul ignore if if (is.nonEmptyArray(updatedPackageFiles)) { logger.debug('updateArtifacts for updatedPackageFiles'); @@ -308,7 +311,12 @@ export async function getUpdatedPackageFiles( packageFile.path, ), }); - processUpdateArtifactResults(results, updatedArtifacts, artifactErrors); + processUpdateArtifactResults( + results, + updatedArtifacts, + artifactErrors, + artifactNotices, + ); } } } @@ -339,7 +347,12 @@ export async function getUpdatedPackageFiles( packageFile.path, ), }); - processUpdateArtifactResults(results, updatedArtifacts, artifactErrors); + processUpdateArtifactResults( + results, + updatedArtifacts, + artifactErrors, + artifactNotices, + ); if (is.nonEmptyArray(results)) { updatedPackageFiles.push(packageFile); } @@ -373,6 +386,7 @@ export async function getUpdatedPackageFiles( results, updatedArtifacts, artifactErrors, + artifactNotices, ); } } @@ -383,6 +397,7 @@ export async function getUpdatedPackageFiles( updatedPackageFiles, updatedArtifacts, artifactErrors, + artifactNotices, }; } @@ -425,16 +440,22 @@ function processUpdateArtifactResults( results: UpdateArtifactsResult[] | null, updatedArtifacts: FileChange[], artifactErrors: ArtifactError[], + artifactNotices: ArtifactNotice[], ): void { if (is.nonEmptyArray(results)) { for (const res of results) { - const { file, artifactError } = res; - // istanbul ignore else + const { file, notice, artifactError } = res; if (file) { updatedArtifacts.push(file); - } else if (artifactError) { + } + + if (artifactError) { artifactErrors.push(artifactError); } + + if (notice) { + artifactNotices.push(notice); + } } } } diff --git a/lib/workers/repository/update/branch/index.spec.ts b/lib/workers/repository/update/branch/index.spec.ts index e71f943e5f..99be0cfac0 100644 --- a/lib/workers/repository/update/branch/index.spec.ts +++ b/lib/workers/repository/update/branch/index.spec.ts @@ -1,3 +1,4 @@ +import { codeBlock } from 'common-tags'; import { fs, git, @@ -103,6 +104,7 @@ describe('workers/repository/update/branch/index', () => { updatedPackageFiles: [], artifactErrors: [], updatedArtifacts: [], + artifactNotices: [], }; beforeEach(() => { @@ -895,6 +897,40 @@ describe('workers/repository/update/branch/index', () => { expect(prWorker.ensurePr).toHaveBeenCalledTimes(0); }); + it('ensures PR and comments notice', async () => { + getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce( + partial<PackageFilesResult>({ + updatedPackageFiles: [partial<FileChange>()], + artifactNotices: [{ file: 'go.mod', message: 'some notice' }], + }), + ); + npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ + artifactErrors: [], + updatedArtifacts: [partial<FileChange>()], + }); + scm.branchExists.mockResolvedValue(true); + automerge.tryBranchAutomerge.mockResolvedValueOnce('failed'); + prWorker.ensurePr.mockResolvedValueOnce({ + type: 'with-pr', + pr: partial<Pr>({ number: 123 }), + }); + prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: true }); + commit.commitFilesToBranch.mockResolvedValueOnce(null); + await branchWorker.processBranch({ ...config, automerge: true }); + expect(prWorker.ensurePr).toHaveBeenCalledTimes(1); + expect(platform.ensureCommentRemoval).toHaveBeenCalledTimes(0); + expect(prAutomerge.checkAutoMerge).toHaveBeenCalledTimes(1); + expect(platform.ensureComment).toHaveBeenCalledWith({ + content: codeBlock` + ##### File name: go.mod + + some notice + `, + number: 123, + topic: 'ℹ Artifact update notice', + }); + }); + it('ensures PR and tries automerge', async () => { getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce( partial<PackageFilesResult>({ @@ -1274,6 +1310,7 @@ describe('workers/repository/update/branch/index', () => { updatedPackageFiles: [partial<FileChange>()], updatedArtifacts: [partial<FileChange>()], artifactErrors: [{}], + artifactNotices: [], }); npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ artifactErrors: [], @@ -1546,6 +1583,7 @@ describe('workers/repository/update/branch/index', () => { updatedPackageFiles: [updatedPackageFile], artifactErrors: [], updatedArtifacts: [], + artifactNotices: [], }); npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ artifactErrors: [], @@ -1643,6 +1681,7 @@ describe('workers/repository/update/branch/index', () => { updatedPackageFiles: [updatedPackageFile], artifactErrors: [], updatedArtifacts: [], + artifactNotices: [], } satisfies PackageFilesResult); npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ artifactErrors: [], @@ -1727,6 +1766,7 @@ describe('workers/repository/update/branch/index', () => { updatedPackageFiles: [updatedPackageFile], artifactErrors: [], updatedArtifacts: [], + artifactNotices: [], }); npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ artifactErrors: [], diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index 08a1fabd2b..c241925cdb 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -147,6 +147,9 @@ export async function processBranch( } const keepUpdatedLabel = config.keepUpdatedLabel; const artifactErrorTopic = emojify(':warning: Artifact update problem'); + const artifactNoticeTopic = emojify( + ':information_source: Artifact update notice', + ); try { // Check if branch already existed const existingPr = @@ -551,6 +554,14 @@ export async function processBranch( number: branchPr.number, topic: artifactErrorTopic, }); + + if (!config.artifactNotices?.length) { + await ensureCommentRemoval({ + type: 'by-topic', + number: branchPr.number, + topic: artifactNoticeTopic, + }); + } } } const forcedManually = userRebaseRequested || !branchExists; @@ -874,22 +885,38 @@ export async function processBranch( }); } } - } else if (config.automerge) { - logger.debug('PR is configured for automerge'); - // skip automerge if there is a new commit since status checks aren't done yet - if (config.ignoreTests === true || !commitSha) { - logger.debug('checking auto-merge'); - const prAutomergeResult = await checkAutoMerge(pr, config); - if (prAutomergeResult?.automerged) { - return { - branchExists, - result: 'automerged', - commitSha, - }; + } else { + if (config.artifactNotices?.length) { + const contentLines: string[] = []; + for (const notice of config.artifactNotices) { + contentLines.push(`##### File name: ${notice.file}`); + contentLines.push(notice.message); } + const content = contentLines.join('\n\n'); + await ensureComment({ + number: pr.number, + topic: artifactNoticeTopic, + content, + }); + } + + if (config.automerge) { + logger.debug('PR is configured for automerge'); + // skip automerge if there is a new commit since status checks aren't done yet + if (config.ignoreTests === true || !commitSha) { + logger.debug('checking auto-merge'); + const prAutomergeResult = await checkAutoMerge(pr, config); + if (prAutomergeResult?.automerged) { + return { + branchExists, + result: 'automerged', + commitSha, + }; + } + } + } else { + logger.debug('PR is not configured for automerge'); } - } else { - logger.debug('PR is not configured for automerge'); } } } catch (err) /* istanbul ignore next */ { diff --git a/lib/workers/types.ts b/lib/workers/types.ts index 013a53cd24..e4c6477b45 100644 --- a/lib/workers/types.ts +++ b/lib/workers/types.ts @@ -10,6 +10,7 @@ import type { import type { Release } from '../modules/datasource/types'; import type { ArtifactError, + ArtifactNotice, ExtractConfig, LookupUpdate, PackageDependency, @@ -30,6 +31,7 @@ export interface BranchUpgradeConfig Partial<LookupUpdate>, RenovateSharedConfig { artifactErrors?: ArtifactError[]; + artifactNotices?: ArtifactNotice[]; autoReplaceStringTemplate?: string; baseDeps?: PackageDependency[]; branchName: string; -- GitLab