Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Docker image bumping for .drone.yml files #1447

Closed
wants to merge 1 commit into from

Conversation

JimNero009
Copy link
Contributor

Drone is a CI system that defines a set of steps, each of which contains a Docker image that determines in what container a set of commands should be run. Those images should be fixed for reproducible builds and be updated over time in the same way a Dockerfile should be. This PR then extends the Docker bumping functionality to also include the .drone.yml file and regex replace images as appropriate.

@@ -556,7 +556,7 @@
context "with a non-standard filename" do
let(:dockerfile) do
Dependabot::DependencyFile.new(
name: "custom-name",
name: "custom-dockerfile-name",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was confused by the initial test here. A 'dockerfile' with a name 'custom-name' should never have been picked up by the file_fetcher code which, if I understand correctly, is what will feed the parser code. So the string 'dockerfile' should be in the name somewhere to be picked up and that is what this change is doing and what the larger code change in general uncovered.

Could you confirm?

@JimNero009 JimNero009 changed the title [WIP] Add Docker image bumping for .drone.yml files Add Docker image bumping for .drone.yml files Oct 18, 2019
@rebelagentm rebelagentm requested a review from feelepxyz October 21, 2019 16:55
@rebelagentm
Copy link
Contributor

@JimNero009 👋 Thanks for submitting this! The team is pretty busy scaling Dependabot for GitHub, so it will be a while before we can get to reviewing this.

@feelepxyz feelepxyz added the T: feature-request Requests for new features label Oct 23, 2019
@feelepxyz
Copy link
Contributor

@JimNero009 hey sorry dropping this! Unfortunately can't accept this change as any new package manager makes it harder to debug and maintain for the team which is very under-resourced at the moment 😢

We're hoping to onboard a few more people over the coming months and I'm keen to figure out a plan of supporting community package managers, possibly through a plugin system of sorts.

We've also moved towards separating package managers, e.g. dep and go:modules are separate instead of supporting multiple manifest types in the same one. Combining support has turned out to cause more problems than it solves as package managers tend to diverge over time and makes debugging harder.

@feelepxyz
Copy link
Contributor

@JimNero009 what are your thoughts on building drone support as a separate gem, e.g. dependabot-drone and publishing this yourself? You should be able to use the docker one as a starting point which is a standalone gem but required from dependabot-omnibus: https://github.com/dependabot/dependabot-core/tree/master/docker

You'll then want to require in from your self hosted script, e.g. require "dependabot/drone".

You'll want to add a dependency on https://rubygems.org/gems/dependabot-common and use this gemspec as a starting point: https://github.com/dependabot/dependabot-core/blob/master/common/dependabot-common.gemspec

We could then look at moving generic parts to common if this would help you out over time to make it easier to write custom dependabot-gems. Would be ace if all the basic blocks existed to throw together a language with minimal plumbing, supporting this: #1290

@JimNero009
Copy link
Contributor Author

Sounds like a decent plan! Can't promise I'll get to it any time soon, but I will keep it in mind for those lazy Christmas holidays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: feature-request Requests for new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants