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

Don't try to resolve pinned sha's to versions when updating docker images #6150

Merged
Merged
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
4 changes: 2 additions & 2 deletions common/lib/dependabot/dependency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def humanized_previous_version
return ref_changed? ? previous_ref : nil
end

if previous_version.match?(/^[0-9a-f]{40}$/)
if previous_version.match?(/^[0-9a-f]{40}/)
return previous_ref if ref_changed? && previous_ref

"`#{previous_version[0..6]}`"
Expand All @@ -134,7 +134,7 @@ def humanized_previous_version
def humanized_version
return if removed?

if version.match?(/^[0-9a-f]{40}$/)
if version.match?(/^[0-9a-f]{40}/)
return new_ref if ref_changed? && new_ref

"`#{version[0..6]}`"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ def commits_details(base:, head:)
groups: [],
source: {
type: "digest",
digest: "sha256:18305429afa14ea462f810146ba44d4363ae76e4c8d" \
digest: "18305429afa14ea462f810146ba44d4363ae76e4c8d" \
"fc38288cf73aa07485005"
}
}],
Expand All @@ -441,7 +441,7 @@ def commits_details(base:, head:)
groups: [],
source: {
type: "digest",
digest: "sha256:2167a21baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" \
digest: "2167a21baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" \
"aaaaaaaaaaaaaaaaaaaaa"
}
}]
Expand Down
68 changes: 4 additions & 64 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 All @@ -25,15 +24,15 @@ class FileParser < Dependabot::FileParsers::Base
FROM = /FROM/i
PLATFORM = /--platform\=(?<platform>\S+)/
TAG = /:(?<tag>[\w][\w.-]{0,127})/
DIGEST = /@(?<digest>[^\s]+)/
DIGEST = /(?<digest>[0-9a-f]{64})/
NAME = /\s+AS\s+(?<name>[\w-]+)/
FROM_LINE =
%r{^#{FROM}\s+(#{PLATFORM}\s+)?(#{REGISTRY}/)?
#{IMAGE}#{TAG}?#{DIGEST}?#{NAME}?}x
#{IMAGE}#{TAG}?(?:@sha256:#{DIGEST})?#{NAME}?}x

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

IMAGE_SPEC = %r{^(#{REGISTRY}/)?#{IMAGE}#{TAG}?#{DIGEST}?#{NAME}?}x
IMAGE_SPEC = %r{^(#{REGISTRY}/)?#{IMAGE}#{TAG}?(?:@sha256:#{DIGEST})?#{NAME}?}x

def parse
dependency_set = DependencySet.new
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
8 changes: 4 additions & 4 deletions docker/lib/dependabot/docker/file_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,11 @@ def update_digest_and_tag(previous_content, old_source, new_source)
old_tag = old_source[:tag]
new_tag = new_source[:tag]

old_declaration_regex = /^#{FROM_REGEX}\s+.*@#{old_digest}/
old_declaration_regex = /^#{FROM_REGEX}\s+.*@sha256:#{old_digest}/

previous_content.gsub(old_declaration_regex) do |old_dec|
old_dec.
gsub("@#{old_digest}", "@#{new_digest}").
gsub("@sha256:#{old_digest}", "@sha256:#{new_digest}").
gsub(":#{old_tag}", ":#{new_tag}")
end
end
Expand Down Expand Up @@ -181,7 +181,7 @@ def update_image(file, content)
def new_yaml_image(file)
element = dependency.requirements.find { |r| r[:file] == file.name }
prefix = element.fetch(:source)[:registry] ? "#{element.fetch(:source)[:registry]}/" : ""
digest = element.fetch(:source)[:digest] ? "@#{element.fetch(:source)[:digest]}" : ""
digest = element.fetch(:source)[:digest] ? "@sha256:#{element.fetch(:source)[:digest]}" : ""
tag = element.fetch(:source)[:tag] ? ":#{element.fetch(:source)[:tag]}" : ""
"#{prefix}#{dependency.name}#{tag}#{digest}"
end
Expand All @@ -194,7 +194,7 @@ def new_yaml_tag(file)
def old_yaml_images(file)
previous_requirements(file).map do |r|
prefix = r.fetch(:source)[:registry] ? "#{r.fetch(:source)[:registry]}/" : ""
digest = r.fetch(:source)[:digest] ? "@#{r.fetch(:source)[:digest]}" : ""
digest = r.fetch(:source)[:digest] ? "@sha256:#{r.fetch(:source)[:digest]}" : ""
tag = r.fetch(:source)[:tag] ? ":#{r.fetch(:source)[:tag]}" : ""
"#{prefix}#{dependency.name}#{tag}#{digest}"
end
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
39 changes: 22 additions & 17 deletions docker/lib/dependabot/docker/update_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
require "dependabot/update_checkers/base"
require "dependabot/errors"
require "dependabot/docker/tag"
require "dependabot/docker/file_parser"
require "dependabot/docker/version"
require "dependabot/docker/requirement"
require "dependabot/docker/utils/credentials_finder"
Expand Down Expand Up @@ -106,9 +107,10 @@ def latest_tag_from(version)
@tags[version] = fetch_latest_tag(Tag.new(version))
end

# NOTE: It's important that this *always* returns a version (even if
# NOTE: It's important that this *always* returns a tag (even if
# it's the existing one) as it is what we later check the digest of.
def fetch_latest_tag(version_tag)
return Tag.new(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 @@ -164,9 +166,10 @@ def comparable_tags_from_registry(original_tag)
end

def remove_version_downgrades(candidate_tags, version_tag)
current_version = comparable_version_from(version_tag)

candidate_tags.select do |tag|
comparable_version_from(tag) >=
comparable_version_from(version_tag)
comparable_version_from(tag) >= current_version
end
end

Expand Down Expand Up @@ -198,7 +201,11 @@ def version_of_latest_tag
end

def updated_digest
@updated_digest ||= digest_of(latest_version)
@updated_digest ||= if latest_tag_from(dependency.version).digest?
latest_digest
else
digest_of(latest_version)
end
end

def tags_from_registry
Expand Down Expand Up @@ -234,20 +241,18 @@ def digest_of(tag)
@digests ||= {}
return @digests[tag] if @digests.key?(tag)

@digests[tag] =
begin
docker_registry_client.digest(docker_repo_name, tag)
rescue *transient_docker_errors => e
attempt ||= 1
attempt += 1
if attempt > 3 && e.is_a?(DockerRegistry2::NotFound)
nil
else
raise if attempt > 3
@digests[tag] = fetch_digest_of(tag)
end

retry
end
end
def fetch_digest_of(tag)
Nishnha marked this conversation as resolved.
Show resolved Hide resolved
docker_registry_client.digest(docker_repo_name, tag)&.delete_prefix("sha256:")
rescue *transient_docker_errors => e
attempt ||= 1
attempt += 1
return if attempt > 3 && e.is_a?(DockerRegistry2::NotFound)
raise if attempt > 3

retry
rescue DockerRegistry2::RegistryAuthenticationException,
RestClient::Forbidden
raise PrivateSourceAuthenticationFailure, registry_hostname
Expand Down
Loading