feat: helm requirements.yaml support #3805

nchashch · 2019-05-24T09:09:22Z

Todo

Implement extractPackageFile
Implement updateDependencies
Add tests
Add datasource for helm chart repositories
Test with actual charts (version without caching)
Add caching for helm datasource
Get test coverage to 100%
Test with actual charts (version with caching)

This PR adds helm manager.

* Change fileMatch in definitions.js from Chart.ya?ml to requirements.ya?ml

* Fix some unrelated test failures

* Get test coverage to 100%

* index.yaml is cached for every helm repository for 10 minutes * rename repository field to helmRepository in order to avoid conflict with existing configuration field

rarkins · 2019-06-11T05:28:39Z

@nchashch thanks for this contribution. Can you educate me a little on how this works? I had assumed that Helm charts would just reference Docker images, so was surprised to see a new datasource. When I look at the extract function, it appears to reference only other charts. Can you describe to me how it all works?

nchashch · 2019-06-11T18:40:31Z

@rarkins There is a requrements.yaml file, that lists all dependencies (other charts) specifying dependency name, version, and repository url (see reference 1). A repository specified by the url is an HTTP server that provides an index.yaml file which lists all available packages and their versions (see reference 2).

Helm datasource downloads and caches the index.yaml file (it is around 5 MB for the official repository: https://kubernetes-charts.storage.googleapis.com/index.yaml), and then extracts a list of versions for the lookupName package. Then the manager just extracts and updates the values in requirements.yaml as usual.

From the docs I have read it looks like an image (or images) can be specified with a docker container name and tag in values.yaml file like here, and then used for lookup in another file (see reference 3). There might be other ways to specify docker images, like specifying them directly in Chart.yaml, but I am not sure.

It might be possible to also update docker images for helm charts (maybe in a separate manager?), but I think it would be pretty difficult, because it looks like values.yaml can have arbitrary structure. So this PR only handles dependencies in requirements.yaml.

rarkins · 2019-06-12T05:18:06Z

Thanks, I understand better now. As you suggest, we may need to do the image renovating for Helm in another PR

rarkins · 2019-06-12T05:18:55Z

lib/config/definitions.js

+    stage: 'package',
+    type: 'object',
+    default: {
+      fileMatch: ['(^|/)requirements.ya?ml$'],


Suggested change

fileMatch: ['(^|/)requirements.ya?ml$'],

fileMatch: ['(^|/)requirements\\.ya?ml$'],

rarkins · 2019-06-12T05:19:30Z

lib/datasource/helm/index.js

+  getPkgReleases,
+};
+
+async function getPkgReleases({ lookupName, helmRepository }) {


Instead of this custom new helmRepository field, can we reuse our existing registryUrls concept?

There can be multiple repositories in a single requirements.yaml file, and each dependency is associated with it's own repository. So if registryUrls is used, we will have to pass helmRepository with every dependency anyway in order to identify which registry url to use.

Getting index.yaml for every registry in registryUrls and searching for lookupName in them without helmRepository wouldn't work, because packages can be duplicated in repositories, and there is no guarantee that the right one will be found. Also it would be more expensive.

rarkins · 2019-06-12T05:20:22Z

lib/datasource/helm/index.js

+      if (!res || !res.body) {
+        logger.warn(
+          { dependency: lookupName },
+          `Received invalid index.yaml from ${helmRepository}`


Technically we haven't tried parsing the yaml yet so this message might confuse people.

rarkins · 2019-06-12T05:21:11Z

lib/datasource/helm/index.js

+    }
+  }
+  let doc;
+  if (!cachedIndex) {


This is the second if (!cachedIndex) check in this block, they should probably be combined

rarkins · 2019-06-12T05:22:48Z

lib/manager/helm/extract.js

+      deps = doc.dependencies.map(dep => ({
+        depName: dep.name,
+        currentValue: dep.version,
+        helmRepository: dep.repository,


Switch to registryUrls here

rarkins · 2019-06-12T05:23:13Z

lib/manager/helm/extract.js

+        depName: dep.name,
+        currentValue: dep.version,
+        helmRepository: dep.repository,
+        datasource: 'helm',


Is there any possibility of non-helm datasources here? If not then datasource: 'helm' could be pushed one level up into the response, rather than be inside each dep.

rarkins · 2019-06-12T05:25:16Z

@nchashch could you also make sure that the data we are caching is the most efficient/optimal dataset? e.g. if the file is 5MB raw, then it's probably even more when parsed, but we maybe only need 200kb of it and could cache only that.

rarkins · 2019-06-12T05:39:03Z

I'm still trying to consider if we should have one package manager (helm) or two eventually (helm-charts and helm-images).

Also, I'm wondering if instead of looking for requirements.yaml - for which there could be a lot of false hits, we instead look for directories with Chart.yaml and if found then we will look for requirements.yaml and values.yaml next to it to confirm. This will make it easier to add the values.yaml support in future.

What do you think?

* Factor out retrieving index.yaml into getRepositoryData function

… into feat/helm-manager-3414

nchashch · 2019-06-17T19:16:58Z

@rarkins This PR is ready for review. Now only versions are cached by the datasource, instead of the whole parsed index.yaml.

rarkins · 2019-06-18T04:02:36Z

@nchashch can you suggest some demo repos I can fork and test against?

ig0rsky · 2019-09-16T21:38:27Z

@pharindoko @rarkins any updates?

pharindoko · 2019-09-16T21:44:17Z

Sorry I do not have the time to test it.

JamieMagee · 2019-09-25T20:47:58Z

I'm testing this against the helm/charts repository and I'm encountering some errors. I'm getting a lot of unsupported version messages, for values that should be supported semver versions

DEBUG: Dependency elasticsearch has unsupported value ^1.10.0 (repository=helm/charts)
DEBUG: Dependency kibana has unsupported value ^0.16.0 (repository=helm/charts)
DEBUG: Dependency filebeat has unsupported value ^1.0.0 (repository=helm/charts)
DEBUG: Dependency logstash has unsupported value ^1.1.0 (repository=helm/charts)
DEBUG: Dependency fluentd has unsupported value ^1.0.0 (repository=helm/charts)
DEBUG: Dependency fluent-bit has unsupported value ^0.13.0 (repository=helm/charts)
DEBUG: Dependency fluentd-elasticsearch has unsupported value ^1.0.0 (repository=helm/charts)
DEBUG: Dependency nginx-ldapauth-proxy has unsupported value ^0.1.0 (repository=helm/charts)

JamieMagee · 2019-09-30T20:28:50Z

test/manager/helm-requirements/extract.spec.ts

@@ -0,0 +1,123 @@
+import { extractPackageFile } from '../../../lib/manager/helm-requirements/extract';
+
+const platform: any = global.platform;


Can you change this to

import { platform as _platform } from '../../../lib/platform'; const platform: any = _platform;

lib/manager/helm-requirements/extract.ts

JamieMagee · 2019-09-30T20:54:43Z

@nchashch If you're interested in getting this merged, there should be just a few changes required:

Merge in changes from latest master
Make changes around platform no longer being global

You can see how it works on my fork of the helm/charts repo here.

As for the unsupported version messages I mentioned above, I realise now that your changes don't have support for version ranges. We open an issue for that, and add it later.

* platform is no longer global, and has to be imported * Import platform in extract.spec.js

nchashch · 2019-10-01T13:54:30Z

@JamieMagee I have merged master, and added platform imports instead of using it as a global.

JamieMagee · 2019-10-03T20:15:52Z

@nchashch Thanks for the update.

I'll review this a little more thoroughly tomorrow, but initially this looks fine. We can open another issue to support pinning/version ranges and I can do the TypeScript conversion for lib/datasource/helm/index.js a separately.

nchashch · 2019-10-03T20:38:46Z

@JamieMagee Thanks!

JamieMagee · 2019-10-04T19:20:19Z

lib/datasource/helm/index.js

+    }
+    if (
+      gotErr.statusCode === 429 ||
+      (gotErr.statusCode > 500 && gotErr.statusCode < 600)


Suggested change

(gotErr.statusCode > 500 && gotErr.statusCode < 600)

(gotErr.statusCode >= 500 && gotErr.statusCode < 600)

JamieMagee · 2019-10-04T19:28:45Z

lib/datasource/helm/index.js

+    });
+    const cacheMinutes = 20;
+    await renovateCache.set(cacheNamespace, cacheKey, result, cacheMinutes);
+  } catch (yamlErr) {


Suggested change

} catch (yamlErr) {

} catch (err) {

Also add a logger.warn here with text about loading YAML, and logger.debug with the full error

JamieMagee · 2019-10-04T19:29:00Z

lib/datasource/helm/index.js

+      logger.warn(`Received invalid response from ${repository}`);
+      return null;
+    }
+  } catch (gotErr) {


Suggested change

} catch (gotErr) {

} catch (err) {

JamieMagee · 2019-10-04T19:44:36Z

lib/manager/helm-requirements/update.ts

+  const searchString = `${oldVersion}`;
+  const newString = `${newValue}`;
+  let newFileContent = fileContent;
+  // Search starts at 'dependencies' instaed of `${depName}` because fields in a YAML record


Suggested change

// Search starts at 'dependencies' instaed of `${depName}` because fields in a YAML record

// Search starts at 'dependencies' instead of `${depName}` because fields in a YAML record

JamieMagee · 2019-10-04T19:47:31Z

lib/manager/helm-requirements/update.ts

+        searchString,
+        newString
+      );
+      // Compare the parsed toml structure of old and new


TOML or YAML?

* Treat 500 response as server error in getRepositoryData in helm datasource

renovate-bot · 2019-10-14T14:02:33Z

🎉 This PR is included in version 19.61.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

nchashch added 2 commits May 24, 2019 11:28

helm: Add stub implementation of helm manager

1c43e85

helm: Add initial implementation of extractPackage file

9ffbce2

* Change fileMatch in definitions.js from Chart.ya?ml to requirements.ya?ml

nchashch changed the title ~~feat: Add helm (kubernetes package manager) support~~ feat: Add helm (kubernetes package manager) support (WIP) May 24, 2019

nchashch and others added 13 commits May 27, 2019 08:34

helm: Fix failing stub tests, fix linting errors

13c27c6

helm: Add initial implementation of updateDependency function

76ad25c

* Fix some unrelated test failures

Merge branch 'master' into feat/helm-manager-3414

09cfcb8

helm: Fix repository/extract/index test failure

0fef5e0

helm: Add more tests for updateDependency and extractPackageFile

0b7d5df

* Get test coverage to 100%

Merge branch 'master' into feat/helm-manager-3414

57f6d1c

helm: Add helm datasource

f2da3d6

Merge branch 'master' into feat/helm-manager-3414

3c2bf02

helm: Add tests for helm datasource

f6e2fab

Merge branch 'master' into feat/helm-manager-3414

d0a12a2

Merge branch 'master' into feat/helm-manager-3414

4f75b4b

Merge branch 'master' into feat/helm-manager-3414

62a6840

helm: Add caching to helm datasource

d859647

* index.yaml is cached for every helm repository for 10 minutes * rename repository field to helmRepository in order to avoid conflict with existing configuration field

nchashch changed the title ~~feat: Add helm (kubernetes package manager) support (WIP)~~ feat: Add helm (kubernetes package manager) support Jun 4, 2019

Merge branch 'master' into feat/helm-manager-3414

480d221

rarkins requested changes Jun 12, 2019

View reviewed changes

nchashch and others added 4 commits June 17, 2019 21:55

helm: Cache only version data from index.yaml

db24020

* Factor out retrieving index.yaml into getRepositoryData function

Merge branch 'feat/helm-manager-3414' of github.com:nchashch/renovate…

65fa233

… into feat/helm-manager-3414

Merge branch 'master' into feat/helm-manager-3414

922bca6

helm: Move datasource field to response in extractPackageFile

76adfc5

JamieMagee reviewed Sep 30, 2019

View reviewed changes

lib/manager/helm-requirements/extract.ts Show resolved Hide resolved

nchashch added 2 commits October 1, 2019 11:36

Merge branch 'master' into feat/helm-manager-3414

2de2bb7

helm: Import platform in extract.js

7572ba3

* platform is no longer global, and has to be imported * Import platform in extract.spec.js

rarkins requested a review from JamieMagee October 3, 2019 06:09

Merge branch 'master' into feat/helm-manager-3414

9bcdcff

JamieMagee reviewed Oct 4, 2019

View reviewed changes

JamieMagee previously approved these changes Oct 4, 2019

View reviewed changes

Fix comments, variable names, add logger messages

737e9b4

* Treat 500 response as server error in getRepositoryData in helm datasource

nchashch dismissed JamieMagee’s stale review via 737e9b4 October 8, 2019 10:38

Merge branch 'master' into feat/helm-manager-3414

9fa9bc9

nchashch requested a review from JamieMagee October 9, 2019 09:57

JamieMagee approved these changes Oct 11, 2019

View reviewed changes

rarkins changed the title ~~feat: Add helm (kubernetes package manager) support~~ feat: helm requirements.yaml support Oct 14, 2019

rarkins merged commit 0fffbae into renovatebot:master Oct 14, 2019

renovate-bot added the released label Oct 14, 2019

Morishiri mentioned this pull request Oct 17, 2019

Helm requirements.yaml updater - option to specify credentials renovatebot/config-help#384

Closed

github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: helm requirements.yaml support #3805

feat: helm requirements.yaml support #3805

nchashch commented May 24, 2019 •

edited

rarkins commented Jun 11, 2019

nchashch commented Jun 11, 2019

rarkins commented Jun 12, 2019

rarkins Jun 12, 2019

rarkins Jun 12, 2019

nchashch Jun 17, 2019

rarkins Jun 12, 2019

rarkins Jun 12, 2019

rarkins Jun 12, 2019

rarkins Jun 12, 2019

rarkins commented Jun 12, 2019

rarkins commented Jun 12, 2019

nchashch commented Jun 17, 2019

rarkins commented Jun 18, 2019

ig0rsky commented Sep 16, 2019

pharindoko commented Sep 16, 2019

JamieMagee commented Sep 25, 2019

JamieMagee Sep 30, 2019

JamieMagee commented Sep 30, 2019

nchashch commented Oct 1, 2019

JamieMagee commented Oct 3, 2019

nchashch commented Oct 3, 2019

JamieMagee Oct 4, 2019

JamieMagee Oct 4, 2019

JamieMagee Oct 4, 2019

JamieMagee Oct 4, 2019

JamieMagee Oct 4, 2019

renovate-bot commented Oct 14, 2019

	fileMatch: ['(^\|/)requirements.ya?ml$'],
	fileMatch: ['(^\|/)requirements\\.ya?ml$'],

		@@ -0,0 +1,123 @@
		import { extractPackageFile } from '../../../lib/manager/helm-requirements/extract';

		const platform: any = global.platform;

	(gotErr.statusCode > 500 && gotErr.statusCode < 600)
	(gotErr.statusCode >= 500 && gotErr.statusCode < 600)

	// Search starts at 'dependencies' instaed of `${depName}` because fields in a YAML record
	// Search starts at 'dependencies' instead of `${depName}` because fields in a YAML record

feat: helm requirements.yaml support #3805

feat: helm requirements.yaml support #3805

Conversation

nchashch commented May 24, 2019 • edited

Todo

rarkins commented Jun 11, 2019

nchashch commented Jun 11, 2019

rarkins commented Jun 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rarkins commented Jun 12, 2019

rarkins commented Jun 12, 2019

nchashch commented Jun 17, 2019

rarkins commented Jun 18, 2019

ig0rsky commented Sep 16, 2019

pharindoko commented Sep 16, 2019

JamieMagee commented Sep 25, 2019

Choose a reason for hiding this comment

JamieMagee commented Sep 30, 2019

nchashch commented Oct 1, 2019

JamieMagee commented Oct 3, 2019

nchashch commented Oct 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

renovate-bot commented Oct 14, 2019

nchashch commented May 24, 2019 •

edited