search

dependabot / dependabot-core

🤖 Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.68k stars 1.01k forks source link

Make Dependabot recursively check folders for Dockerfiles / Helm Charts / K8s files #6047

Open jeffwidman opened 1 year ago

jeffwidman commented 1 year ago

Is there an existing issue for this?

Feature description

While we don't currently support bumping versions of Helm Charts / K8s YAML files, we did recently extend our Docker version updater to check for docker versions in these file formats.

Helm Charts / K8s YAML files are often stored in nested directories.

So I tried specifying a docker stanza targeting a parent directory:

  - package-ecosystem: "docker"
    directory: "/manifest_staging"
    schedule:
      interval: "weekly"

And got the following error:

updater | ERROR <job_499677747> Found neither Kubernetes YAML nor Dockerfiles in /manifest_staging
updater | ERROR <job_499677747> /home/dependabot/docker/lib/dependabot/docker/file_fetcher.rb:42:in `fetch_files'
updater | ERROR <job_499677747> /home/dependabot/common/lib/dependabot/file_fetchers/base.rb:69:in `files'
updater | ERROR <job_499677747> /home/dependabot/dependabot-updater/lib/dependabot/file_fetcher_job.rb:52:in `dependency_files'
updater | ERROR <job_499677747> /home/dependabot/dependabot-updater/lib/dependabot/file_fetcher_job.rb:17:in `perform_job'
updater | ERROR <job_499677747> /home/dependabot/dependabot-updater/lib/dependabot/base_job.rb:50:in `run'
updater | ERROR <job_499677747> bin/fetch_files.rb:23:in `<main>'

So it's not recursively searching child folders if the top level dir doesn't have a chart present. This was fine for a world where we only checked docker files, but for checking charts/k8s templates, we should re-evaluate and possibly recursively search for matching files. I recall at a previous employer, we'd have chart/template repos with 50+ subdirectories.

There's some edge cases in here with deeply nested subdirs/tons of files going haywire on us, so we may need to limit the depth/breadth that we walk.

Keep in mind any changes here will affect not only parsing Yaml files, but also searching for Dockerfile, unless we explicitly limit the scope of that. I think the user experience gets better if we search recursively for all of them, but not everyone may agree.

jeffwidman commented 1 year ago

I did some digging based on some internal alerts and noticed that there are a lot of repos configured for the docker check that check the / directory but don't have a Dockerfile in there. Given the volume of errors, its clear users are intuitively expecting that even for plain Dockerfiles that the check will be recursive.

jeffwidman commented 1 year ago

Since I last commented, i realized this one is a little more complicated because some users will want recursive search, some will want only search the root directory, etc... so I think we need to be explicit about / => root dir, * => recursive to allow disambiguating user intent.

The * recursive case is already tracked under:

So I'm going to close this as a duplicate.

jeffwidman commented 1 year ago

I realize this is a semi-duplicate, but I just spent a bunch of time searching for it and was tougher to find because it was closed. So I'm going to re-open, and treat this as a specific sub-case of #2178 .