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
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions docker/lib/dependabot/docker/file_fetcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,19 @@ module Dependabot
module Docker
class FileFetcher < Dependabot::FileFetchers::Base
def self.required_files_in?(filenames)
filenames.any? { |f| f.match?(/dockerfile/i) }
filenames.any? { |f| f.match?(/dockerfile|.drone.yml/i) }
end

def self.required_files_message
"Repo must contain a Dockerfile."
"Repo must contain a Dockerfile and/or a .drone.yml"
end

private

def fetch_files
fetched_files = []
fetched_files += correctly_encoded_dockerfiles
fetched_files += drone_files

return fetched_files if fetched_files.any?

Expand Down Expand Up @@ -49,6 +50,13 @@ def correctly_encoded_dockerfiles
def incorrectly_encoded_dockerfiles
dockerfiles.reject { |f| f.content.valid_encoding? }
end

def drone_files
@drone_files ||=
repo_contents(raise_errors: false).
select { |f| f.type == "file" && f.name.match?(/.drone.yml/i) }.
map { |f| fetch_file_from_host(f.name) }
end
end
end
end
Expand Down
71 changes: 40 additions & 31 deletions docker/lib/dependabot/docker/file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,77 +24,86 @@ class FileParser < Dependabot::FileParsers::Base
IMAGE = %r{(?<image>#{NAME_COMPONENT}(?:/#{NAME_COMPONENT})*)}.freeze

FROM = /FROM/i.freeze
DRONE_IMAGE = /\s+image\:/i.freeze
TAG = /:(?<tag>[\w][\w.-]{0,127})/.freeze
DIGEST = /@(?<digest>[^\s]+)/.freeze
NAME = /\s+AS\s+(?<name>[\w-]+)/.freeze
FROM_LINE =
%r{^#{FROM}\s+(#{REGISTRY}/)?#{IMAGE}#{TAG}?#{DIGEST}?#{NAME}?}.freeze
DRONE_IMAGE_LINE =
%r{^#{DRONE_IMAGE}\s+(#{REGISTRY}/)?#{IMAGE}#{TAG}?#{DIGEST}?#{NAME}?}.
freeze

AWS_ECR_URL = /dkr\.ecr\.(?<region>[^.]+).amazonaws\.com/.freeze

def parse
dependency_set = DependencySet.new

dockerfiles.each do |dockerfile|
dockerfile.content.each_line do |line|
next unless FROM_LINE.match?(line)
parse_docker_files(dependency_set, dockerfiles, FROM_LINE)
parse_docker_files(dependency_set, drone_files, DRONE_IMAGE_LINE)

parsed_from_line = FROM_LINE.match(line).named_captures
if parsed_from_line["registry"] == "docker.io"
parsed_from_line["registry"] = nil
dependency_set.dependencies
end

private

def parse_docker_files(dependency_set, files, line_regex)
files.each do |docker_file|
docker_file.content.each_line do |line|
next unless line_regex.match?(line)

parsed_image_line = line_regex.match(line).named_captures
if parsed_image_line["registry"] == "docker.io"
parsed_image_line["registry"] = nil
end

version = version_from(parsed_from_line)
version = version_from(parsed_image_line)
next unless version

dependency_set << Dependency.new(
name: parsed_from_line.fetch("image"),
name: parsed_image_line.fetch("image"),
version: version,
package_manager: "docker",
requirements: [
requirement: nil,
groups: [],
file: dockerfile.name,
source: source_from(parsed_from_line)
file: docker_file.name,
source: source_from(parsed_image_line)
]
)
end
end

dependency_set.dependencies
end

private

def dockerfiles
# The Docker file fetcher only fetches Dockerfiles, so no need to
# filter here
dependency_files
dependency_files.select { |f| f.name.match?(/dockerfile/i) }
end

def version_from(parsed_from_line)
return parsed_from_line.fetch("tag") if parsed_from_line.fetch("tag")
def drone_files
dependency_files.select { |f| f.name.match?(/.drone.yml/i) }
end

def version_from(parsed_line)
return parsed_line.fetch("tag") if parsed_line.fetch("tag")

version_from_digest(
registry: parsed_from_line.fetch("registry"),
image: parsed_from_line.fetch("image"),
digest: parsed_from_line.fetch("digest")
registry: parsed_line.fetch("registry"),
image: parsed_line.fetch("image"),
digest: parsed_line.fetch("digest")
)
end

def source_from(parsed_from_line)
def source_from(parsed_line)
source = {}

if parsed_from_line.fetch("registry")
source[:registry] = parsed_from_line.fetch("registry")
if parsed_line.fetch("registry")
source[:registry] = parsed_line.fetch("registry")
end

if parsed_from_line.fetch("tag")
source[:tag] = parsed_from_line.fetch("tag")
end
source[:tag] = parsed_line.fetch("tag") if parsed_line.fetch("tag")

if parsed_from_line.fetch("digest")
source[:digest] = parsed_from_line.fetch("digest")
if parsed_line.fetch("digest")
source[:digest] = parsed_line.fetch("digest")
end

source
Expand Down Expand Up @@ -158,7 +167,7 @@ def check_required_files
# Just check if there are any files at all.
return if dependency_files.any?

raise "No Dockerfile!"
raise "No Dockerfile or .drone.yml!"
end
end
end
Expand Down
18 changes: 12 additions & 6 deletions docker/lib/dependabot/docker/file_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,13 @@ module Dependabot
module Docker
class FileUpdater < Dependabot::FileUpdaters::Base
FROM_REGEX = /FROM/i.freeze
DRONE_REGEX = /\s+image\:/i.freeze

def self.updated_files_regex
[/dockerfile/i]
[
/dockerfile/i,
/.drone.yml/i
]
end

def updated_dependency_files
Expand All @@ -22,7 +26,7 @@ def updated_dependency_files
updated_files <<
updated_file(
file: file,
content: updated_dockerfile_content(file)
content: updated_file_content(file)
)
end

Expand All @@ -43,10 +47,10 @@ def check_required_files
# Just check if there are any files at all.
return if dependency_files.any?

raise "No Dockerfile!"
raise "No Dockerfile or .drone.yml!"
end

def updated_dockerfile_content(file)
def updated_file_content(file)
updated_content =
if specified_with_digest?(file)
update_digest_and_tag(file)
Expand All @@ -60,7 +64,8 @@ def updated_dockerfile_content(file)
end

def update_digest_and_tag(file)
old_declaration_regex = /^#{FROM_REGEX}\s+.*@#{old_digest(file)}/
old_declaration_regex =
/^(#{FROM_REGEX}|#{DRONE_REGEX})\s+.*@#{old_digest(file)}/

file.content.gsub(old_declaration_regex) do |old_dec|
old_dec.
Expand All @@ -81,7 +86,8 @@ def update_tag(file)
escaped_declaration = Regexp.escape(old_declaration)

old_declaration_regex =
%r{^#{FROM_REGEX}\s+(docker\.io/)?#{escaped_declaration}(?=\s|$)}
%r{^(#{FROM_REGEX}|#{DRONE_REGEX})
\s+(docker\.io/)?#{escaped_declaration}(?=\s|$)}x

file.content.gsub(old_declaration_regex) do |old_dec|
old_dec.gsub(":#{old_tag(file)}", ":#{new_tag(file)}")
Expand Down
37 changes: 37 additions & 0 deletions docker/spec/dependabot/docker/file_fetcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,4 +157,41 @@
to raise_error(Dependabot::DependencyFileNotFound)
end
end

context "with a Drone file" do
before do
stub_request(:get, url + "?ref=sha").
with(headers: { "Authorization" => "token token" }).
to_return(
status: 200,
body: fixture("github", "contents_docker_and_drone_repo.json"),
headers: { "content-type" => "application/json" }
)

stub_request(:get, File.join(url, "Dockerfile?ref=sha")).
with(headers: { "Authorization" => "token token" }).
to_return(
status: 200,
body: dockerfile_fixture,
headers: { "content-type" => "application/json" }
)

stub_request(:get, File.join(url, ".drone.yml?ref=sha")).
with(headers: { "Authorization" => "token token" }).
to_return(
status: 200,
body: drone_fixture,
headers: { "content-type" => "application/json" }
)
end

let(:dockerfile_fixture) { fixture("github", "contents_dockerfile.json") }
let(:drone_fixture) { fixture("github", "contents_drone.json") }

it "fetches the Dockerfile and .drone.yml" do
expect(file_fetcher_instance.files.count).to eq(2)
expect(file_fetcher_instance.files.map(&:name)).
to match_array(["Dockerfile", ".drone.yml"])
end
end
end
40 changes: 36 additions & 4 deletions docker/spec/dependabot/docker/file_parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?

content: dockerfile_body
)
end
Expand All @@ -569,7 +569,7 @@
[{
requirement: nil,
groups: [],
file: "custom-name",
file: "custom-dockerfile-name",
source: { tag: "17.04" }
}]
end
Expand All @@ -582,7 +582,7 @@
let(:files) { [dockerfile, dockefile2] }
let(:dockefile2) do
Dependabot::DependencyFile.new(
name: "custom-name",
name: "custom-dockerfile-name",
content: dockerfile_body2
)
end
Expand Down Expand Up @@ -615,7 +615,7 @@
[{
requirement: nil,
groups: [],
file: "custom-name",
file: "custom-dockerfile-name",
source: { tag: "17.04" }
}]
end
Expand All @@ -628,5 +628,37 @@
end
end
end

context "with a drone file" do
let(:files) { [drone_file] }
let(:drone_file) do
Dependabot::DependencyFile.new(
name: ".drone.yml",
content: drone_body
)
end
let(:drone_body) { fixture("docker", "drone", "simple_drone.yml") }

its(:length) { is_expected.to eq(1) }

describe "the dependency" do
subject(:dependency) { dependencies.first }
let(:expected_requirements) do
[{
requirement: nil,
groups: [],
file: ".drone.yml",
source: { tag: "3.10.0" }
}]
end

it "has the right details" do
expect(dependency).to be_a(Dependabot::Dependency)
expect(dependency.name).to eq("alpine")
expect(dependency.version).to eq("3.10.0")
expect(dependency.requirements).to eq(expected_requirements)
end
end
end
end
end
51 changes: 50 additions & 1 deletion docker/spec/dependabot/docker/file_updater_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
)
end

describe "#updated_dependency_files" do
describe "#updated_dependency_files for Dockerfile" do
subject(:updated_files) { updater.updated_dependency_files }

it "returns DependencyFile objects" do
Expand Down Expand Up @@ -435,4 +435,53 @@
end
end
end

describe "drone file updates" do
let(:files) { [drone_file] }
let(:drone_file) do
Dependabot::DependencyFile.new(
content: drone_body,
name: ".drone.yml"
)
end
let(:drone_body) { fixture("docker", "drone", "simple_drone.yml") }
let(:dependency) do
Dependabot::Dependency.new(
name: "alpine",
version: "3.10.2",
previous_version: "3.10.0",
requirements: [{
requirement: nil,
groups: [],
file: ".drone.yml",
source: { tag: "3.10.2" }
}],
previous_requirements: [{
requirement: nil,
groups: [],
file: ".drone.yml",
source: { tag: "3.10.0" }
}],
package_manager: "docker"
)
end
describe "#updated_dependency_files" do
subject(:updated_files) { updater.updated_dependency_files }

it "returns DependencyFile objects" do
updated_files.each { |f| expect(f).to be_a(Dependabot::DependencyFile) }
end

its(:length) { is_expected.to eq(1) }
describe "the updated drone file" do
subject(:updated_drone_file) do
updated_files.find { |f| f.name == ".drone.yml" }
end

its(:content) { is_expected.to include "image: alpine:3.10.2\n" }
its(:content) { is_expected.to include "pipeline:\n" }
its(:content) { is_expected.to include "echo \"Heya!\"" }
end
end
end
end
Loading