prometheus-community / helm-charts

Prometheus community Helm charts
Apache License 2.0
4.81k stars 4.92k forks source link

Proposal: Use Renovate instead Dependabot as dependency manager #4292

Open jkroepke opened 4 months ago

jkroepke commented 4 months ago

Currently, https://github.com/prometheus-community/helm-charts is using dependabot for tracking dependency.

However, dependabot is unable to track to bump dependencies inside helm values.

While there is no native integration in Renovate, renovate can instrument with regex to take care of dependencies.

Example renovate config: https://github.com/jkroepke/helm-charts/blob/b513065a173454484eecf412aa85032f905baf8d/renovate.json#L11-L19

Example: Dependency

https://github.com/jkroepke/helm-charts/blob/b513065a173454484eecf412aa85032f905baf8d/charts/amazon-eks-pod-identity-webhook/Chart.yaml#L6-L7

Example PR:

https://github.com/jkroepke/helm-charts/pull/51

To avoid duplicate config, I would recommend to disable dependabot here.

Pulling @prometheus-community/helm-charts-maintainers @prometheus-community/helm-charts-admins for additional opinions here.

I guess we also need an org admin for setup the renovate integration.

monotek commented 4 months ago

Does renovate also checks github action versions? If not i would also be ok to use both and use renovate only for docker image updates. I guess the hard part will be the regex as not all charts use "image". Imho some are using "tag" too....

jkroepke commented 4 months ago

Does renovate also checks github action versions?

Yes.

Example: https://github.com/jkroepke/openvpn-auth-oauth2/pull/135 - Thats works without configure regexManagers.

I guess the hard part will be the regex as not all charts use "image": Imhos some are using "tag" too....

You also also work with hints (extra comments): https://github.com/jkroepke/helm-charts/blob/b513065a173454484eecf412aa85032f905baf8d/charts/amazon-eks-pod-identity-webhook/Chart.yaml#L6-L7

Working with hints would be recommendation, because then we have one working renovate.json (in the repo root) that fits for all. Chart Maintainers can conditionally opt-in by enrich the comments.

monotek commented 4 months ago

Cool :) Sounds to be nice addition to have automatic security / minor updates (if we could implement automerge for that too). But i guess also the creation of the pr woould be a step into the right direction so i would like to try it out :)

jkroepke commented 4 months ago

There only downside here is that renovate is unable to bump the chart version, expect for bumping chart dependencies. In this case, I update the version via suggestions and apply them.

The next benefit, as chart maintainer, I can update versions on my own chart, because the contribution is external and I'm able to merge it.

monotek commented 4 months ago

That the appversion cant be raised automatically is kind of a bummer though :(

I still would like to try it out. Maybe we can find some workaround for that...

jkroepke commented 4 months ago

@monotek Maybe it just works now.

https://github.com/renovatebot/renovate/issues/8231 - the issue was resolved 5 weeks ago.

monotek commented 4 months ago

How do we want to proceed? Will you create a pr and add the needed renvovate comments to all charts?

jkroepke commented 4 months ago

How do we want to proceed?

We have to ask an org owner to enable the Renovate App first (https://github.com/apps/renovate)

monotek commented 4 months ago

@SuperQ @brancz

Could you enable renovate app for us please?

jkroepke commented 4 months ago

@monotek Secondly, I would recommend to add the renovate.json to the code owners files, like we do the .github/workflows/.

https://github.com/prometheus-community/helm-charts/pull/4293

SuperQ commented 4 months ago

Ok, I've configured renovate to have access to prometheus-community/helm-charts.

jkroepke commented 4 months ago

Thanks, it works.

The welcome PR with an initial config is availible here: https://github.com/prometheus-community/helm-charts/pull/4295

monotek commented 4 months ago

@SuperQ Thanks a lot :)

@jkroepke will you use the same pr for adjustments or should we just merge it?

jkroepke commented 4 months ago

@monotek I have added dependencyDashboardApproval, to avoid an SPAM of PRs unless we figure out the correct configuration.

After merge, the renovate dashboard will be appear as GitHub Issue and we can use that for precisely observe and open PRs.

However, Renovate will only take are of the configuration on the default branch. As I know, there is no way to test renovate configuration from non-default branches.

jkroepke commented 4 months ago

Hm, using regex manager would not bump the Chart.yaml

Test PR: https://github.com/jkroepke/helm-charts/pull/55

And the postUpgradeTasks are not supported on the hosted renovate.

I created a discussion at renovate for this case: https://github.com/renovatebot/renovate/discussions/27555

morremeyer commented 4 months ago

@jkroepke Since I've been stumbling about this: bumpVersion on values file updates does not work since the feature had to be reverted, see https://github.com/renovatebot/renovate/issues/8231#issuecomment-1968678321

monotek commented 4 months ago

I guess we could come up with an own job, which adds a commit which raises the appversion aftert a renovate run?

morremeyer commented 4 months ago

~@monotek Yes, there are workarounds documented for this in the linked issue already. However, none of the workarounds actually is valid JSON, so any linter you throw at it will refuse it.~

~I've been working on a version that is valid JSON for our org internally, will share in the issue linked above once I got that working.~

~However, as @jkroepke already mentioned, this config only works on self-hosted instances of renovate since it executes commands in the context that renovate is running on. Therefore this is disabled for the public renovate App.~

~So to make this work, it would have to be~

Edit: I misunderstood @monotek's comment, sorry. Useful answer below.

@monotek I've done that in the past and we're doing that in my org in various places, e.g. for pre-commit hooks.

I suggest adding the email address of the CI user to gitIgnoredAuthors so that renovate can override the commits on any newer versions before the PR is merged.

jkroepke commented 4 months ago

Hi there,

I'm having an upstream discussion and the maintainers at least like the proposal: https://github.com/renovatebot/renovate/discussions/27602

monotek commented 4 months ago

I mean it has not to be solved inside of renovate.

We could create a new github action which runs when:

We likly would need some own bot user for this but should be doable.

But maybe lets wait a bit first if there is any outcome of @jkroepke discussion with the renovate guys...

jkroepke commented 4 months ago

We could create a new github action which runs when:

We could also benefit anyways from this:

See: https://github.com/prometheus-community/helm-charts/issues/4088