From 57ac93557713805cdb14b565afa1c8d10f401a83 Mon Sep 17 00:00:00 2001 From: Rhys Arkins Date: Mon, 1 Mar 2021 19:35:52 +0100 Subject: [PATCH] fix: Revert "feat(helm-values): Support for bumpVersion" (#8926) This reverts commit 662a60a87e36a92138a98fadddf4235deb03e75b. --- docs/usage/configuration-options.md | 2 +- lib/manager/common.ts | 10 +-- .../__snapshots__/update.spec.ts.snap | 15 ---- lib/manager/helm-values/extract.spec.ts | 60 +++---------- lib/manager/helm-values/extract.ts | 22 +---- lib/manager/helm-values/index.ts | 1 - lib/manager/helm-values/update.spec.ts | 85 ------------------- lib/manager/helm-values/update.ts | 51 ----------- lib/manager/helm-values/util.ts | 52 ------------ .../__snapshots__/get-updated.spec.ts.snap | 60 +++---------- lib/workers/branch/get-updated.spec.ts | 68 +++------------ lib/workers/branch/get-updated.ts | 27 ++---- 12 files changed, 43 insertions(+), 410 deletions(-) delete mode 100644 lib/manager/helm-values/__snapshots__/update.spec.ts.snap delete mode 100644 lib/manager/helm-values/update.spec.ts delete mode 100644 lib/manager/helm-values/update.ts diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index 1fbcc15d1585d4..aa2e68afad5a1b 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -257,7 +257,7 @@ This is an advance field and it's recommend you seek a config review before appl ## bumpVersion -Currently this setting supports `helmv3`, `helm-values`, `npm` and `sbt` only, so raise a feature request if you have a use for it with other package managers. +Currently this setting supports `helmv3`, `npm` and `sbt` only, so raise a feature request if you have a use for it with other package managers. Its purpose is if you want Renovate to update the `version` field within your file's `package.json` any time it updates dependencies within. Usually this is for automatic release purposes, so that you don't need to add another step after Renovate before you can release a new version. diff --git a/lib/manager/common.ts b/lib/manager/common.ts index 9a330aa7a5818f..0ead2c8d019f75 100644 --- a/lib/manager/common.ts +++ b/lib/manager/common.ts @@ -228,13 +228,6 @@ export interface UpdateDependencyConfig> { export interface BumpPackageVersionResult { bumpedContent: string | null; - // describes files that was changed instead of or in addition to the packageFile - bumpedFiles?: BumpedPackageFile[]; -} - -export interface BumpedPackageFile { - fileName: string; - newContent: string; } export interface UpdateLockedConfig { @@ -255,8 +248,7 @@ export interface ManagerApi { bumpPackageVersion?( content: string, currentValue: string, - bumpVersion: ReleaseType | string, - packageFile?: string + bumpVersion: ReleaseType | string ): Result; extractAllPackageFiles?( diff --git a/lib/manager/helm-values/__snapshots__/update.spec.ts.snap b/lib/manager/helm-values/__snapshots__/update.spec.ts.snap deleted file mode 100644 index e1be96d3a072eb..00000000000000 --- a/lib/manager/helm-values/__snapshots__/update.spec.ts.snap +++ /dev/null @@ -1,15 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`lib/manager/helm-values/update .bumpPackageVersion() increments 1`] = ` -"apiVersion: v2 -name: test -version: 0.0.3 -" -`; - -exports[`lib/manager/helm-values/update .bumpPackageVersion() updates 1`] = ` -"apiVersion: v2 -name: test -version: 0.1.0 -" -`; diff --git a/lib/manager/helm-values/extract.spec.ts b/lib/manager/helm-values/extract.spec.ts index a78458e0f4413c..417c3825f5770c 100644 --- a/lib/manager/helm-values/extract.spec.ts +++ b/lib/manager/helm-values/extract.spec.ts @@ -1,5 +1,4 @@ import { readFileSync } from 'fs'; -import { fs } from '../../../test/util'; import { extractPackageFile } from './extract'; const helmDefaultChartInitValues = readFileSync( @@ -16,66 +15,27 @@ describe('lib/manager/helm-values/extract', () => { describe('extractPackageFile()', () => { beforeEach(() => { jest.resetAllMocks(); - fs.readLocalFile = jest.fn(); }); - it('returns null for invalid yaml file content', async () => { - const result = await extractPackageFile('nothing here: ['); + it('returns null for invalid yaml file content', () => { + const result = extractPackageFile('nothing here: ['); expect(result).toBeNull(); }); - it('returns null for empty yaml file content', async () => { - const result = await extractPackageFile(''); + it('returns null for empty yaml file content', () => { + const result = extractPackageFile(''); expect(result).toBeNull(); }); - it('returns null for no file content', async () => { - const result = await extractPackageFile(null); + it('returns null for no file content', () => { + const result = extractPackageFile(null); expect(result).toBeNull(); }); - it('extracts from values.yaml correctly with same structure as "helm create"', async () => { - const result = await extractPackageFile(helmDefaultChartInitValues); + it('extracts from values.yaml correctly with same structure as "helm create"', () => { + const result = extractPackageFile(helmDefaultChartInitValues); expect(result).toMatchSnapshot(); }); - it('extracts from complex values file correctly"', async () => { - const result = await extractPackageFile(helmMultiAndNestedImageValues); + it('extracts from complex values file correctly"', () => { + const result = extractPackageFile(helmMultiAndNestedImageValues); expect(result).toMatchSnapshot(); expect(result.deps).toHaveLength(4); }); - it('returns the package file version from the sibling Chart.yaml"', async () => { - fs.readLocalFile.mockResolvedValueOnce(` - apiVersion: v2 - appVersion: "1.0" - description: A Helm chart for Kubernetes - name: example - version: 0.1.0 - `); - const result = await extractPackageFile( - helmMultiAndNestedImageValues, - 'values.yaml' - ); - expect(result.packageFileVersion).toBe('0.1.0'); - }); - it('does not fail if the sibling Chart.yaml is invalid', async () => { - fs.readLocalFile.mockResolvedValueOnce(` - invalidYaml: [ - `); - const result = await extractPackageFile( - helmMultiAndNestedImageValues, - 'values.yaml' - ); - expect(result).not.toBeNull(); - expect(result.packageFileVersion).toBeUndefined(); - }); - it('does not fail if the sibling Chart.yaml does not contain the required fields', async () => { - fs.readLocalFile.mockResolvedValueOnce(` - apiVersion: v2 - name: test - version-is: missing - `); - const result = await extractPackageFile( - helmMultiAndNestedImageValues, - 'values.yaml' - ); - expect(result).not.toBeNull(); - expect(result.packageFileVersion).toBeUndefined(); - }); }); }); diff --git a/lib/manager/helm-values/extract.ts b/lib/manager/helm-values/extract.ts index e35cb54b10476b..da5b127a65bd02 100644 --- a/lib/manager/helm-values/extract.ts +++ b/lib/manager/helm-values/extract.ts @@ -6,7 +6,6 @@ import { getDep } from '../dockerfile/extract'; import { HelmDockerImageDependency, - getParsedSiblingChartYaml, matchesHelmValuesDockerHeuristic, } from './util'; @@ -54,10 +53,7 @@ function findDependencies( return packageDependencies; } -export async function extractPackageFile( - content: string, - fileName?: string -): Promise { +export function extractPackageFile(content: string): PackageFile { let parsedContent: Record | HelmDockerImageDependency; try { // a parser that allows extracting line numbers would be preferable, with @@ -72,21 +68,7 @@ export async function extractPackageFile( const deps = findDependencies(parsedContent, []); if (deps.length) { logger.debug({ deps }, 'Found dependencies in helm-values'); - - // in Helm, the current package version is the version of the chart. - // This fetches this version by reading it from the Chart.yaml - // found in the same folder as the currently processed values file. - const siblingChart = await getParsedSiblingChartYaml(fileName); - const packageFileVersion = siblingChart?.version; - if (packageFileVersion) { - return { - deps, - packageFileVersion, - }; - } - return { - deps, - }; + return { deps }; } } catch (err) /* istanbul ignore next */ { logger.error({ err }, 'Error parsing helm-values parsed content'); diff --git a/lib/manager/helm-values/index.ts b/lib/manager/helm-values/index.ts index 11d8f2279aefae..1ef75d28735eca 100644 --- a/lib/manager/helm-values/index.ts +++ b/lib/manager/helm-values/index.ts @@ -1,5 +1,4 @@ export { extractPackageFile } from './extract'; -export { bumpPackageVersion } from './update'; export const defaultConfig = { commitMessageTopic: 'helm values {{depName}}', diff --git a/lib/manager/helm-values/update.spec.ts b/lib/manager/helm-values/update.spec.ts deleted file mode 100644 index e90a73547e9f42..00000000000000 --- a/lib/manager/helm-values/update.spec.ts +++ /dev/null @@ -1,85 +0,0 @@ -import yaml from 'js-yaml'; -import { fs } from '../../../test/util'; -import * as helmValuesUpdater from './update'; - -describe('lib/manager/helm-values/update', () => { - describe('.bumpPackageVersion()', () => { - const chartContent = yaml.safeDump({ - apiVersion: 'v2', - name: 'test', - version: '0.0.2', - }); - const helmValuesContent = yaml.safeDump({ - image: { - registry: 'docker.io', - repository: 'docker/whalesay', - tag: '1.0.0', - }, - }); - beforeEach(() => { - jest.resetAllMocks(); - fs.readLocalFile = jest.fn(); - fs.readLocalFile.mockResolvedValueOnce(chartContent); - }); - it('increments', async () => { - const { - bumpedContent, - bumpedFiles, - } = await helmValuesUpdater.bumpPackageVersion( - helmValuesContent, - '0.0.2', - 'patch', - 'values.yaml' - ); - expect(bumpedContent).toEqual(helmValuesContent); - expect(bumpedFiles).toHaveLength(1); - const bumpedFile = bumpedFiles[0]; - expect(bumpedFile.fileName).toEqual('Chart.yaml'); - expect(bumpedFile.newContent).toMatchSnapshot(); - }); - it('no ops', async () => { - const { - bumpedContent, - bumpedFiles, - } = await helmValuesUpdater.bumpPackageVersion( - helmValuesContent, - '0.0.1', - 'patch', - 'values.yaml' - ); - expect(bumpedContent).toEqual(helmValuesContent); - expect(bumpedFiles).toHaveLength(1); - const bumpedFile = bumpedFiles[0]; - expect(bumpedFile.newContent).toEqual(chartContent); - }); - it('updates', async () => { - const { - bumpedContent, - bumpedFiles, - } = await helmValuesUpdater.bumpPackageVersion( - helmValuesContent, - '0.0.1', - 'minor', - 'values.yaml' - ); - expect(bumpedContent).toEqual(helmValuesContent); - expect(bumpedFiles).toHaveLength(1); - const bumpedFile = bumpedFiles[0]; - expect(bumpedFile.fileName).toEqual('Chart.yaml'); - expect(bumpedFile.newContent).toMatchSnapshot(); - }); - it('returns content if bumping errors', async () => { - const { - bumpedContent, - bumpedFiles, - } = await helmValuesUpdater.bumpPackageVersion( - helmValuesContent, - '0.0.2', - true as any, - 'values.yaml' - ); - expect(bumpedContent).toEqual(helmValuesContent); - expect(bumpedFiles).toBeUndefined(); - }); - }); -}); diff --git a/lib/manager/helm-values/update.ts b/lib/manager/helm-values/update.ts deleted file mode 100644 index 25184d07fca5fc..00000000000000 --- a/lib/manager/helm-values/update.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { ReleaseType, inc } from 'semver'; -import { logger } from '../../logger'; -import { getSiblingFileName } from '../../util/fs'; -import { BumpPackageVersionResult } from '../common'; -import { getSiblingChartYamlContent } from './util'; - -export async function bumpPackageVersion( - content: string, - currentValue: string, - bumpVersion: ReleaseType | string, - packageFile: string -): Promise { - logger.debug( - { bumpVersion, currentValue }, - 'Checking if we should bump Chart.yaml version' - ); - const chartFileName = getSiblingFileName(packageFile, 'Chart.yaml'); - const chartYamlContent = await getSiblingChartYamlContent(packageFile); - try { - const newChartVersion = inc(currentValue, bumpVersion as ReleaseType); - if (!newChartVersion) { - throw new Error('semver inc failed'); - } - logger.debug({ newChartVersion }); - const bumpedContent = chartYamlContent.replace( - /^(version:\s*).*$/m, - `$1${newChartVersion}` - ); - if (bumpedContent === chartYamlContent) { - logger.debug('Version was already bumped'); - } else { - logger.debug('Bumped Chart.yaml version'); - } - return { - bumpedContent: content, - bumpedFiles: [{ fileName: chartFileName, newContent: bumpedContent }], - }; - } catch (err) { - logger.warn( - { - chartYamlContent, - currentValue, - bumpVersion, - }, - 'Failed to bumpVersion' - ); - return { - bumpedContent: content, - }; - } -} diff --git a/lib/manager/helm-values/util.ts b/lib/manager/helm-values/util.ts index cc9a21f7a7bd11..1aeabaf30b37f4 100644 --- a/lib/manager/helm-values/util.ts +++ b/lib/manager/helm-values/util.ts @@ -1,6 +1,3 @@ -import yaml from 'js-yaml'; -import { logger } from '../../logger'; -import { getSiblingFileName, readLocalFile } from '../../util/fs'; import { hasKey } from '../../util/object'; export type HelmDockerImageDependency = { @@ -38,52 +35,3 @@ export function matchesHelmValuesDockerHeuristic( hasKey('tag', data) ); } - -/** - * This function looks for a Chart.yaml in the same directory as @param fileName and - * returns its raw contents. - * - * @param fileName - */ -export async function getSiblingChartYamlContent( - fileName: string -): Promise { - try { - const chartFileName = getSiblingFileName(fileName, 'Chart.yaml'); - return await readLocalFile(chartFileName, 'utf8'); - } catch (err) { - logger.debug({ fileName }, 'Failed to read helm Chart.yaml'); - return null; - } -} - -/** - * This function looks for a Chart.yaml in the same directory as @param fileName and - * if it looks like a valid Helm Chart.yaml, it is parsed and returned as an object. - * - * @param fileName - */ -export async function getParsedSiblingChartYaml( - fileName: string -): Promise { - try { - const chartContents = await getSiblingChartYamlContent(fileName); - if (!chartContents) { - logger.debug({ fileName }, 'Failed to find helm Chart.yaml'); - return null; - } - // TODO: fix me - const chart = yaml.safeLoad(chartContents, { json: true }) as any; - if (!(chart?.apiVersion && chart.name && chart.version)) { - logger.debug( - { fileName }, - 'Failed to find required fields in Chart.yaml' - ); - return null; - } - return chart; - } catch (err) { - logger.debug({ fileName }, 'Failed to parse helm Chart.yaml'); - return null; - } -} diff --git a/lib/workers/branch/__snapshots__/get-updated.spec.ts.snap b/lib/workers/branch/__snapshots__/get-updated.spec.ts.snap index 28862513be364b..bd99b33978a014 100644 --- a/lib/workers/branch/__snapshots__/get-updated.spec.ts.snap +++ b/lib/workers/branch/__snapshots__/get-updated.spec.ts.snap @@ -1,5 +1,19 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`workers/branch/get-updated getUpdatedPackageFiles() bumps versions in autoReplace managers 1`] = ` +Object { + "artifactErrors": Array [], + "reuseExistingBranch": undefined, + "updatedArtifacts": Array [], + "updatedPackageFiles": Array [ + Object { + "contents": "version: 0.0.2", + "name": "undefined", + }, + ], +} +`; + exports[`workers/branch/get-updated getUpdatedPackageFiles() bumps versions in updateDependency managers 1`] = ` Object { "artifactErrors": Array [], @@ -200,49 +214,3 @@ Object { ], } `; - -exports[`workers/branch/get-updated getUpdatedPackageFiles() in autoReplace managers bumps versions 1`] = ` -Object { - "artifactErrors": Array [], - "reuseExistingBranch": undefined, - "updatedArtifacts": Array [], - "updatedPackageFiles": Array [ - Object { - "contents": "version: 0.0.2", - "name": "undefined", - }, - ], -} -`; - -exports[`workers/branch/get-updated getUpdatedPackageFiles() in autoReplace managers bumps versions in all files if multiple files were bumped 1`] = ` -Object { - "artifactErrors": Array [], - "reuseExistingBranch": undefined, - "updatedArtifacts": Array [], - "updatedPackageFiles": Array [ - Object { - "contents": "version: 0.0.2", - "name": "/test/Chart.yaml", - }, - Object { - "contents": "# Version 0.0.2", - "name": "/test/README.md", - }, - ], -} -`; - -exports[`workers/branch/get-updated getUpdatedPackageFiles() in autoReplace managers bumps versions with a bumpPackageFile different from the packageFile 1`] = ` -Object { - "artifactErrors": Array [], - "reuseExistingBranch": undefined, - "updatedArtifacts": Array [], - "updatedPackageFiles": Array [ - Object { - "contents": "version: 0.0.2", - "name": "/test/Chart.yaml", - }, - ], -} -`; diff --git a/lib/workers/branch/get-updated.spec.ts b/lib/workers/branch/get-updated.spec.ts index 21adf701cec036..9c6171a115caa4 100644 --- a/lib/workers/branch/get-updated.spec.ts +++ b/lib/workers/branch/get-updated.spec.ts @@ -2,7 +2,6 @@ import { defaultConfig, git, mocked } from '../../../test/util'; import * as datasourceGitSubmodules from '../../datasource/git-submodules'; import * as _composer from '../../manager/composer'; import * as _gitSubmodules from '../../manager/git-submodules'; -import * as _helmValues from '../../manager/helm-values'; import * as _helmv3 from '../../manager/helmv3'; import * as _npm from '../../manager/npm'; import { BranchConfig } from '../common'; @@ -12,13 +11,11 @@ import { getUpdatedPackageFiles } from './get-updated'; const composer = mocked(_composer); const gitSubmodules = mocked(_gitSubmodules); const helmv3 = mocked(_helmv3); -const helmValues = mocked(_helmValues); const npm = mocked(_npm); const autoReplace = mocked(_autoReplace); jest.mock('../../manager/composer'); jest.mock('../../manager/helmv3'); -jest.mock('../../manager/helm-values'); jest.mock('../../manager/npm'); jest.mock('../../manager/git-submodules'); jest.mock('../../util/git'); @@ -210,63 +207,18 @@ describe('workers/branch/get-updated', () => { const res = await getUpdatedPackageFiles(config); expect(res).toMatchSnapshot(); }); - - describe('in autoReplace managers', () => { - it('bumps versions', async () => { - config.upgrades.push({ - branchName: undefined, - bumpVersion: 'patch', - manager: 'helmv3', - }); - autoReplace.doAutoReplace.mockResolvedValueOnce('version: 0.0.1'); - helmv3.bumpPackageVersion.mockReturnValue({ - bumpedContent: 'version: 0.0.2', - }); - const res = await getUpdatedPackageFiles(config); - expect(res).toMatchSnapshot(); - }); - it('bumps versions with a bumpPackageFile different from the packageFile', async () => { - config.upgrades.push({ - branchName: undefined, - bumpVersion: 'patch', - manager: 'helm-values', - }); - autoReplace.doAutoReplace.mockResolvedValueOnce('existing content'); - helmValues.bumpPackageVersion.mockResolvedValue({ - bumpedContent: 'existing content', - bumpedFiles: [ - { - fileName: '/test/Chart.yaml', - newContent: 'version: 0.0.2', - }, - ], - }); - const res = await getUpdatedPackageFiles(config); - expect(res).toMatchSnapshot(); + it('bumps versions in autoReplace managers', async () => { + config.upgrades.push({ + branchName: undefined, + bumpVersion: 'patch', + manager: 'helmv3', }); - it('bumps versions in all files if multiple files were bumped', async () => { - config.upgrades.push({ - branchName: undefined, - bumpVersion: 'patch', - manager: 'helm-values', - }); - autoReplace.doAutoReplace.mockResolvedValueOnce('existing content'); - helmValues.bumpPackageVersion.mockResolvedValue({ - bumpedContent: 'existing content', - bumpedFiles: [ - { - fileName: '/test/Chart.yaml', - newContent: 'version: 0.0.2', - }, - { - fileName: '/test/README.md', - newContent: '# Version 0.0.2', - }, - ], - }); - const res = await getUpdatedPackageFiles(config); - expect(res).toMatchSnapshot(); + autoReplace.doAutoReplace.mockResolvedValueOnce('version: 0.0.1'); + helmv3.bumpPackageVersion.mockReturnValue({ + bumpedContent: 'version: 0.0.2', }); + const res = await getUpdatedPackageFiles(config); + expect(res).toMatchSnapshot(); }); }); }); diff --git a/lib/workers/branch/get-updated.ts b/lib/workers/branch/get-updated.ts index fa5edf2dd9b2d5..5e79c12d0ca27c 100644 --- a/lib/workers/branch/get-updated.ts +++ b/lib/workers/branch/get-updated.ts @@ -3,7 +3,7 @@ import { WORKER_FILE_UPDATE_FAILED } from '../../constants/error-messages'; import * as datasourceGitSubmodules from '../../datasource/git-submodules'; import { logger } from '../../logger'; import { get } from '../../manager'; -import { ArtifactError, BumpedPackageFile } from '../../manager/common'; +import { ArtifactError } from '../../manager/common'; import { File, getFile } from '../../util/git'; import { BranchConfig } from '../common'; import { doAutoReplace } from './auto-replace'; @@ -102,18 +102,14 @@ export async function getUpdatedPackageFiles( packageFileContent, reuseExistingBranch ); - if (res) { - let bumpedPackageFiles: BumpedPackageFile[]; if (bumpPackageVersion && upgrade.bumpVersion) { - const bumpResult = await bumpPackageVersion( + const { bumpedContent } = await bumpPackageVersion( res, upgrade.packageFileVersion, - upgrade.bumpVersion, - packageFile + upgrade.bumpVersion ); - res = bumpResult.bumpedContent; - bumpedPackageFiles = bumpResult.bumpedFiles; + res = bumpedContent; } if (res === packageFileContent) { logger.debug({ packageFile, depName }, 'No content changed'); @@ -125,18 +121,6 @@ export async function getUpdatedPackageFiles( logger.debug({ packageFile, depName }, 'Contents updated'); updatedFileContents[packageFile] = res; } - // indicates that the version was bumped in one or more files in - // addition to or instead of the packageFile - if (bumpedPackageFiles) { - for (const bumpedPackageFile of bumpedPackageFiles) { - logger.debug( - { bumpedPackageFile, depName }, - 'Updating bumpedPackageFile content' - ); - updatedFileContents[bumpedPackageFile.fileName] = - bumpedPackageFile.newContent; - } - } continue; // eslint-disable-line no-continue } else if (reuseExistingBranch) { return getUpdatedPackageFiles({ @@ -155,8 +139,7 @@ export async function getUpdatedPackageFiles( const { bumpedContent } = await bumpPackageVersion( newContent, upgrade.packageFileVersion, - upgrade.bumpVersion, - packageFile + upgrade.bumpVersion ); newContent = bumpedContent; }