Skip to content

Commit

Permalink
Don't resolve digest to versions unnecessarily
Browse files Browse the repository at this point in the history
* If the Dockerfile specifies a version, we don't need to do this,
  since we already have a previous version.

* If the Dockerfile only specifies a SHA, I don't think we need it
  either because we will be updating to another SHA, so the user seems
  uninterested in specific versions. And this case doesn't really work
  in most cases anyways because of how inefficient this is.

With this patch, we can find updates to Dockerfiles specifiying only
SHAs much faster.
  • Loading branch information
deivid-rodriguez committed Apr 5, 2023
1 parent 8dd3592 commit 666c100
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 670 deletions.
62 changes: 1 addition & 61 deletions docker/lib/dependabot/docker/file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
require "dependabot/file_parsers"
require "dependabot/file_parsers/base"
require "dependabot/errors"
require "dependabot/docker/utils/credentials_finder"

module Dependabot
module Docker
Expand Down Expand Up @@ -77,13 +76,7 @@ def dockerfiles
end

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

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

def source_from(parsed_from_line)
Expand All @@ -98,59 +91,6 @@ def source_from(parsed_from_line)
source
end

def version_from_digest(registry:, image:, digest:)
return unless digest

registry_details = fetch_registry_details(registry)
repo = docker_repo_name(image, registry_details["registry"])
client = docker_registry_client(registry_details["registry"], registry_details["credentials"])
client.tags(repo, auto_paginate: true).fetch("tags").find do |tag|
digest == client.digest(repo, tag)
rescue DockerRegistry2::NotFound
# Shouldn't happen, but it does. Example of existing tag with
# no manifest is "library/python", "2-windowsservercore".
false
end
rescue DockerRegistry2::RegistryAuthenticationException,
RestClient::Forbidden
raise PrivateSourceAuthenticationFailure, registry_details["registry"]
rescue RestClient::Exceptions::OpenTimeout,
RestClient::Exceptions::ReadTimeout
raise if credentials_finder.using_dockerhub?(registry_details["registry"])

raise PrivateSourceTimedOut, registry_details["registry"]
end

def docker_repo_name(image, registry)
return image if image.include? "/"
return "library/#{image}" if credentials_finder.using_dockerhub?(registry)

image
end

def docker_registry_client(registry_hostname, registry_credentials)
unless credentials_finder.using_dockerhub?(registry_hostname)
return DockerRegistry2::Registry.new("https://#{registry_hostname}")
end

DockerRegistry2::Registry.new(
"https://#{registry_hostname}",
user: registry_credentials&.fetch("username", nil),
password: registry_credentials&.fetch("password", nil),
read_timeout: 10
)
end

def fetch_registry_details(registry)
registry ||= credentials_finder.base_registry
credentials = credentials_finder.credentials_for_registry(registry)
{ "registry" => registry, "credentials" => credentials }
end

def credentials_finder
@credentials_finder ||= Utils::CredentialsFinder.new(credentials)
end

def check_required_files
# Just check if there are any files at all.
return if dependency_files.any?
Expand Down
6 changes: 6 additions & 0 deletions docker/lib/dependabot/docker/tag.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require "dependabot/docker/file_parser"

module Dependabot
module Docker
class Tag
Expand All @@ -24,6 +26,10 @@ def to_s
name
end

def digest?
name.match?(FileParser::DIGEST)
end

def comparable?
name.match?(NAME_WITH_VERSION)
end
Expand Down
9 changes: 8 additions & 1 deletion docker/lib/dependabot/docker/update_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
module Dependabot
module Docker
class UpdateChecker < Dependabot::UpdateCheckers::Base
DIGEST = /^sha256:[0-9a-f]{64}$/

def latest_version
latest_version_from(dependency.version)
end
Expand Down Expand Up @@ -109,6 +111,7 @@ def latest_tag_from(version)
# NOTE: It's important that this *always* returns a version (even if
# it's the existing one) as it is what we later check the digest of.
def fetch_latest_tag(version_tag)
return latest_digest if version_tag.digest?
return version_tag unless version_tag.comparable?

# Prune out any downgrade tags before checking for pre-releases
Expand Down Expand Up @@ -199,7 +202,11 @@ def version_of_latest_tag
end

def updated_digest
@updated_digest ||= digest_of(latest_version)
@updated_digest ||= if latest_version.match?(DIGEST)
latest_digest
else
digest_of(latest_version)
end
end

def tags_from_registry
Expand Down
Loading

0 comments on commit 666c100

Please sign in to comment.