Add a dependabot implementation for Kubernetes YAML files.
This addresses #2217
~This is heavily based off of the Dockerfile updater. I basically copied the code + tests and then edited as necessary.~
~There's a lot of duplication, so if there's a better way to factor this (make it part of the docker package?) I'm open to suggestions.~
It is added onto the Docker package. If there is a better way to structure the code, let me know.
Thanks!
We've talked about turning this functionality off with a feature-flag initially, so some pointers on how to do it:
- Most top-level base classes take an optional
optionsargument, wish accepts a Hash: https://github.com/dependabot/dependabot-core/blob/f614de480d65a4c110815c1c13df8374f6c47f86/common/lib/dependabot/update_checkers/base.rb#L19 https://github.com/dependabot/dependabot-core/blob/f614de480d65a4c110815c1c13df8374f6c47f86/common/lib/dependabot/file_updaters/base.rb#L14
We can pass some sort of kube_manifests_enabled flag to them, and check for it in the code. Here's another piece of functionality (in the npm ecosystem) that's shielded off in that way:
https://github.com/dependabot/dependabot-core/blob/f614de480d65a4c110815c1c13df8374f6c47f86/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb#L132
PS: I noticed the FileFetcher::Base class currently doesn't take this arg, feel free to add it, as long as it takes a default empty hash def initialize(some:, other:, :args, options: {}) it won't break any code not passing it yet
Comments addressed. Please take another look.
@brendandburns thanks! the linting errors should be auto-correctable using bundle exec rubocop -A -c ../.rubocop.yaml from the docker directory, taking a look at the rest now
Here's a little change I had to make to get the dry-run script to accept this new arg:
diff --git a/bin/dry-run.rb b/bin/dry-run.rb
index 96c792f06..af1c1e70d 100755
--- a/bin/dry-run.rb
+++ b/bin/dry-run.rb
@@ -489,7 +489,8 @@ $repo_contents_path = File.expand_path(File.join("tmp", $repo_name.split("/")))
fetcher_args = {
source: $source,
credentials: $options[:credentials],
- repo_contents_path: $repo_contents_path
+ repo_contents_path: $repo_contents_path,
+ options: $options[:updater_options]
}
$config_file = begin
cfg_file = Dependabot::Config::FileFetcher.new(**fetcher_args).config_file
But with that change, it seems to work:
[dependabot-core-dev] ~/dependabot-core $ bin/dry-run.rb docker dsp-testing/dependabot-all-updates-test --dir=/config/kubernetes/default/deployments --updater-options=kubernetes_updates
=> fetching dependency files
=> dumping fetched dependency files: ./dry-run/dsp-testing/dependabot-all-updates-test/config/kubernetes/default/deployments
=> parsing dependency files
=> updating 1 dependencies: nginx
=== nginx (1.14.2)
=> checking for updates 1/1
=> latest available version is 1.23.1
=> latest allowed version is 1.23.1
=> requirements to unlock: own
=> requirements update strategy:
=> updating nginx from 1.14.2 to 1.23.1
± nginx.yaml
~~~
19c19
< image: nginx:1.14.2
---
> image: nginx:1.23.1
~~~
We have a set of (still) internal integration tests, so I'm setting those up now to make it a bit easier to keep verifying any changes made to this, but overall it looks pretty good
@brendandburns I'll extract the changes to pass options to the FileFetcher into a separate PR so I can make some changes to our internal systems that run Dependabot to properly deal with this, do you mind if I push a few small changes to this branch and rebase it on the latest main? I think I should have access to your fork
@jurre please go ahead and push your commits to this branch. I will address the style/constant extraction comments after that
I actually went ahead and addressed your comments since they were so small. Please add your commits and then hopefully we can merge this!
Looks like I don't have commit access to the fork actually, all I think we need to do for now is rebase this on main with the changes you've already made, and the bits I pulled out into a separate PR.
After that I'll get those e2e tests set up, test a few more edge cases and I think that's it!
@jurre rebased, ptal. Thanks!
Did some extra testing against https://github.com/dsp-testing/dependabot-all-updates-test/blob/main/config/kubernetes/default/multiple-resources.yaml (thanks @bdragon for setting that up), which it seems to handle fine:
bin/dry-run.rb docker dsp-testing/dependabot-all-updates-test --dir=/config/kubernetes/default --updater-options=kubernetes_updates
To use retry middleware with Faraday v2.0+, install `faraday-retry` gem
=> fetching dependency files
=> dumping fetched dependency files: ./dry-run/dsp-testing/dependabot-all-updates-test/config/kubernetes/default
=> parsing dependency files
=> updating 3 dependencies: ruby, nginx, busybox
=== ruby (3.1)
=> checking for updates 1/3
=> latest available version is 3.1.2
=> latest allowed version is 3.1.2
(no update needed as it's already up-to-date)
=== nginx (1.23)
=> checking for updates 2/3
=> latest available version is 1.23.1
=> latest allowed version is 1.23.1
(no update needed as it's already up-to-date)
=== busybox (1.35)
=> checking for updates 3/3
=> latest available version is 1.35.0
=> latest allowed version is 1.35.0
(no update needed as it's already up-to-date)
🌍 Total requests made: '0'
Given that this is behind a feature-gate, I'm inclined to merge it. @bdragon do you see any reason not to?
We'll want to be mindful when writing release notes for dependabot-core, since this is not enabled by default
The python spec failure was addressed in https://github.com/dependabot/dependabot-core/pull/5546 and is not related, I'm going to go ahead and merge this change, thanks @brendandburns!