dependabot-core icon indicating copy to clipboard operation
dependabot-core copied to clipboard

Add support for helm files.

Open brendandburns opened this issue 2 years ago • 8 comments

This PR extends the recent Kubernetes support for Docker containers to common Helm chart formats:

foo:
  image:
    repository: sql/sql
    tag: 1.2.3
    registry: docker.io

or alternatively:

foo:
  image:
    repository: sql/sql
    version: 1.2.3

brendandburns avatar Sep 16 '22 17:09 brendandburns

@brendandburns : The docker CI / CI (docker, docker) (pull_request) build failed. There are few failures due to the naming convention of the method and variable names (Naming/MethodName:). Request you to fix that. Thank you.

honeyankit avatar Sep 16 '22 18:09 honeyankit

@honeyankit thanks I'll take a look. (is there a way that I can run these locally?)

brendandburns avatar Sep 16 '22 20:09 brendandburns

@honeyankit style issues addressed (I hope) ptal

brendandburns avatar Sep 16 '22 20:09 brendandburns

is there a way that I can run these locally?

https://github.com/dependabot/dependabot-core#running-the-tests describes how to run the Rubocop style checks. I highly suggest running those commands within the docker container as described in the section just above that one.

jeffwidman avatar Sep 16 '22 21:09 jeffwidman

Rebased.

brendandburns avatar Sep 20 '22 19:09 brendandburns

@honeyankit this seems to be failing because of a permission problem unrelated to this PR.

If it is something in this PR please let me know and I will try to fix it.

brendandburns avatar Sep 20 '22 20:09 brendandburns

@honeyankit this seems to be failing because of a permission problem unrelated to this PR.

If it is something in this PR please let me know and I will try to fix it.

Sorry for the confusion there. That's not a required check, so it's not a blocker for merging.

The workflow in question pushes images built for PRs to GitHub Container Registry, so that we can easily deploy changes before merging. This step currently fails on PRs from forks, because forks won't have write access to GHCR (see #5668). This is a known issue and we're working on a solution.

(Also, thanks for your contribution! I'm really excited to add support in Dependabot for Helm charts)

mattt avatar Sep 20 '22 21:09 mattt

Also, can you please squash the lint change commit into the original commit?

We currently can't merge via squash due to a limitation in our release script so whatever the commit history is on this branch is what makes it into main...

jeffwidman avatar Sep 21 '22 08:09 jeffwidman

Comments addressed, commits squashed and rebased.

Please take another look.

Thanks!

brendandburns avatar Sep 27 '22 19:09 brendandburns

Comment addressed. Please take another look.

brendandburns avatar Sep 28 '22 22:09 brendandburns

@jeffwidman

newlines added. fwiw, the JSON formatter for VS Code/Codespaces removes the newline at the end of JSON files, so I'm not sure that newlines for JSON is standard, but nonetheless they should be there now.

Please take another look.

Thanks!

brendandburns avatar Oct 04 '22 22:10 brendandburns

fwiw, the JSON formatter for VS Code/Codespaces removes the newline at the end of JSON files, so I'm not sure that newlines for JSON is standard, but nonetheless they should be there now.

Oh interesting. I'm surprised by that, I'll have to research that a bit more.

jeffwidman avatar Oct 05 '22 23:10 jeffwidman

Hmm... something wrong here, I clicked the Update via Rebase and now it says:

dependabot-ci authored and jeffwidman committed 1 minute ago

Hmm... I'm going to amend the commit to ensure Brendan is the author here.

jeffwidman avatar Oct 06 '22 02:10 jeffwidman

That looks better, @brendandburns see ☝️ I have no idea what went wrong, something in GitHub's Update with Rebase command... Anyway, I wasn't sure what email you preferred to use, and there was no history due to the force push so I used your @microsoft.com email, as that's what it looks like you used elsewhere on other commits on your profile.

I'll deploy this once CI goes green, and assuming everything looks good in production will merge this later tonight.

jeffwidman avatar Oct 06 '22 02:10 jeffwidman

@jeffwidman Thanks and yes, the @microsoft.com address is generally what I use.

Great to see this merge!

brendandburns avatar Oct 06 '22 18:10 brendandburns

That looks better, @brendandburns see ☝️ I have no idea what went wrong, something in GitHub's Update with Rebase command... Anyway, I wasn't sure what email you preferred to use, and there was no history due to the force push so I used your @microsoft.com email, as that's what it looks like you used elsewhere on other commits on your profile.

I'll deploy this once CI goes green, and assuming everything looks good in production will merge this later tonight.

Hi @jeffwidman, Do we need to cut a new tag to use this feature?

nilekhc avatar Oct 11 '22 22:10 nilekhc

It is already deployed to GitHub prod, but requires a feature flag. If you are using it somewhere other than GitHub than you'll need to use mainas not yet on a tagged release.

jeffwidman avatar Oct 12 '22 00:10 jeffwidman

It is already deployed to GitHub prod, but requires a feature flag. If you are using it somewhere other than GitHub than you'll need to use mainas not yet on a tagged release.

I am using it outside of GH. When it'll be available on tagged release?

nilekhc avatar Oct 12 '22 00:10 nilekhc

@jeffwidman Any timeline on when the tag release will cut?

nilekhc avatar Oct 17 '22 17:10 nilekhc

I am using it outside of GH. When it'll be available on tagged release?

We don't really do tagged releases anymore as our deployment model has changed a bit and we don't need them anymore, you can just pull the changes in from a sha

jurre avatar Oct 25 '22 07:10 jurre

cc @mburumaxwell

nilekhc avatar Oct 25 '22 17:10 nilekhc

Long term, I think we probably should still make tagged releases available on RubyGems to make life easier for those who run Dependabot in custom ways.

But since we are now deploying multiple times a day straight from PR branches before they merge back to main we'll probably move away from manually running the release scripts that push to RubyGems and instead automate it to cut versions on a monthly or weekly basis. This also means we'll likely need to move away from semver guarantees.

That said the above is my personal opinion and others on the team may feel differently... So just overall there's some planning / internal discussion that needs to happen and no one has had the bandwidth to drive that yet.. I'm hopeful I might have time before end of quarter but I can't guarantee it.

jeffwidman avatar Oct 26 '22 09:10 jeffwidman

Currently this is buried behind a feature flag for folks on GitHub.com...

We'd like to start slowly rolling it out for everyone, but before I do that, I'd like to test on a few repos to ensure no unexpected issues. I can / will setup a fake test repo, but there's nothing like live data to confirm things.

Anyone watching this PR care to volunteer to have their repo(s) be part of the beta test?

Repos must match the following criteria:

  • on GitHub.com
  • public, because for security reasons we can't access private repos w/o jumping through some hoops
  • have helm files... ideally with a combination of up-to-date and outdated docker image tags

If interested, please comment on this thread with the URLs of any repos you own/maintain.

jeffwidman avatar Oct 28 '22 07:10 jeffwidman

@nilekhc This is now live on RubyGems: https://rubygems.org/gems/dependabot-omnibus/versions/0.213.0

jeffwidman avatar Oct 31 '22 16:10 jeffwidman

Currently this is buried behind a feature flag for folks on GitHub.com...

We'd like to start slowly rolling it out for everyone, but before I do that, I'd like to test on a few repos to ensure no unexpected issues. I can / will setup a fake test repo, but there's nothing like live data to confirm things.

Anyone watching this PR care to volunteer to have their repo(s) be part of the beta test?

Repos must match the following criteria:

  • on GitHub.com
  • public, because for security reasons we can't access private repos w/o jumping through some hoops
  • have helm files... ideally with a combination of up-to-date and outdated docker image tags

If interested, please comment on this thread with the URLs of any repos you own/maintain.

Hey @jeffwidman, we can test with our repo if you are still looking for volunteers. https://github.com/Azure/secrets-store-csi-driver-provider-azure

Let me know what I need to do.

cc @aramase

nilekhc avatar Oct 31 '22 17:10 nilekhc

Awesome thanks @nilekhc

I will enable the feature flag on your repo tomorrow, and then since it's a public repo I think I can internally trigger a dry-run to test it. I'll post a follow-up once the flag is enabled.

jeffwidman avatar Nov 03 '22 03:11 jeffwidman

Also @nilekhc you'll need to enable a "docker" ecosystem check in https://github.com/Azure/secrets-store-csi-driver-provider-azure/blob/master/.github/dependabot.yml for the whatever folder holds these files. IIRC, the check will recursively search for files, so if you've got nested subdirs of charts, targeting the "docker" check at the parent dir should be fine.

jeffwidman avatar Nov 03 '22 03:11 jeffwidman

Also @nilekhc you'll need to enable a "docker" ecosystem check in https://github.com/Azure/secrets-store-csi-driver-provider-azure/blob/master/.github/dependabot.yml for the whatever folder holds these files. IIRC, the check will recursively search for files, so if you've got nested subdirs of charts, targeting the "docker" check at the parent dir should be fine.

PR - https://github.com/Azure/secrets-store-csi-driver-provider-azure/pull/1014

nilekhc avatar Nov 04 '22 05:11 nilekhc

@jeffwidman It's working on our repo - https://github.com/Azure/secrets-store-csi-driver-provider-azure/pulls

nilekhc avatar Nov 11 '22 04:11 nilekhc

Awesome news thanks for circling back!

jeffwidman avatar Nov 11 '22 05:11 jeffwidman