From 2c8831c7706a2bf2fa2eaa22ede9eea4c593f799 Mon Sep 17 00:00:00 2001 From: Kamil Bukum Date: Thu, 5 Dec 2024 10:58:38 -0800 Subject: [PATCH 1/2] add package manager with version for docker --- docker/lib/dependabot/docker.rb | 1 + docker/lib/dependabot/docker/file_fetcher.rb | 14 ++-- docker/lib/dependabot/docker/file_parser.rb | 73 +++++++++++++------ docker/lib/dependabot/docker/file_updater.rb | 20 ++--- .../lib/dependabot/docker/package_manager.rb | 64 ++++++++++++++++ 5 files changed, 131 insertions(+), 41 deletions(-) create mode 100644 docker/lib/dependabot/docker/package_manager.rb diff --git a/docker/lib/dependabot/docker.rb b/docker/lib/dependabot/docker.rb index 209d19f805..e00bf3e688 100644 --- a/docker/lib/dependabot/docker.rb +++ b/docker/lib/dependabot/docker.rb @@ -10,6 +10,7 @@ require "dependabot/docker/metadata_finder" require "dependabot/docker/requirement" require "dependabot/docker/version" +require "dependabot/docker/package_manager" require "dependabot/pull_request_creator/labeler" Dependabot::PullRequestCreator::Labeler diff --git a/docker/lib/dependabot/docker/file_fetcher.rb b/docker/lib/dependabot/docker/file_fetcher.rb index 62a319739c..1923d91971 100644 --- a/docker/lib/dependabot/docker/file_fetcher.rb +++ b/docker/lib/dependabot/docker/file_fetcher.rb @@ -5,6 +5,7 @@ require "dependabot/docker/utils/helpers" require "dependabot/file_fetchers" require "dependabot/file_fetchers/base" +require "dependabot/docker/package_manager" module Dependabot module Docker @@ -12,9 +13,6 @@ class FileFetcher < Dependabot::FileFetchers::Base extend T::Sig extend T::Helpers - YAML_REGEXP = /^[^\.].*\.ya?ml$/i - DOCKER_REGEXP = /dockerfile/i - sig { override.params(filenames: T::Array[String]).returns(T::Boolean) } def self.required_files_in?(filenames) filenames.any? { |f| f.match?(DOCKER_REGEXP) } or @@ -36,7 +34,7 @@ def fetch_files if incorrectly_encoded_dockerfiles.none? && incorrectly_encoded_yamlfiles.none? raise Dependabot::DependencyFileNotFound.new( - File.join(directory, "Dockerfile"), + File.join(directory, MANIFEST_FILE), "No Dockerfiles nor Kubernetes YAML found in #{directory}" ) elsif incorrectly_encoded_dockerfiles.none? @@ -62,7 +60,7 @@ def dockerfiles sig { returns(T::Array[Dependabot::DependencyFile]) } def fetch_dockerfiles repo_contents(raise_errors: false) - .select { |f| f.type == "file" && f.name.match?(DOCKER_REGEXP) } + .select { |f| f.type == FILE_TYPE && f.name.match?(DOCKER_REGEXP) } .map { |f| fetch_file_from_host(f.name) } end @@ -80,7 +78,7 @@ def incorrectly_encoded_dockerfiles def yamlfiles @yamlfiles ||= T.let( repo_contents(raise_errors: false) - .select { |f| f.type == "file" && f.name.match?(YAML_REGEXP) } + .select { |f| f.type == FILE_TYPE && f.name.match?(YAML_REGEXP) } .map { |f| fetch_file_from_host(f.name) }, T.nilable(T::Array[DependencyFile]) ) @@ -89,14 +87,14 @@ def yamlfiles sig { params(resource: Object).returns(T.nilable(T::Boolean)) } def likely_kubernetes_resource?(resource) # Heuristic for being a Kubernetes resource. We could make this tighter but this probably works well. - resource.is_a?(::Hash) && resource.key?("apiVersion") && resource.key?("kind") + resource.is_a?(::Hash) && resource.key?(API_VERSION_KEY) && resource.key?(RESOURCE_KEY) end sig { returns(T::Array[Dependabot::DependencyFile]) } def correctly_encoded_yamlfiles candidate_files = yamlfiles.select { |f| f.content&.valid_encoding? } candidate_files.select do |f| - if f.type == "file" && Utils.likely_helm_chart?(f) + if f.type == FILE_TYPE && Utils.likely_helm_chart?(f) true else # This doesn't handle multi-resource files, but it shouldn't matter, since the first resource diff --git a/docker/lib/dependabot/docker/file_parser.rb b/docker/lib/dependabot/docker/file_parser.rb index f30ed67037..23eb15eca1 100644 --- a/docker/lib/dependabot/docker/file_parser.rb +++ b/docker/lib/dependabot/docker/file_parser.rb @@ -7,6 +7,7 @@ require "dependabot/file_parsers" require "dependabot/file_parsers/base" require "dependabot/errors" +require "dependabot/docker/package_manager" require "sorbet-runtime" module Dependabot @@ -16,8 +17,6 @@ class FileParser < Dependabot::FileParsers::Base require "dependabot/file_parsers/base/dependency_set" - YAML_REGEXP = /^[^\.].*\.ya?ml$/i - # Details of Docker regular expressions is at # https://github.com/docker/distribution/blob/master/reference/regexp.go DOMAIN_COMPONENT = /(?:[[:alnum:]]|[[:alnum:]][[[:alnum:]]-]*[[:alnum:]])/ @@ -51,15 +50,15 @@ def parse next unless FROM_LINE.match?(line) parsed_from_line = T.must(FROM_LINE.match(line)).named_captures - parsed_from_line["registry"] = nil if parsed_from_line["registry"] == "docker.io" + parsed_from_line[REGISTERY_KEY] = nil if parsed_from_line[REGISTERY_KEY] == REGISTERY_DOMAIN version = version_from(parsed_from_line) next unless version dependency_set << Dependency.new( - name: T.must(parsed_from_line.fetch("image")), + name: T.must(parsed_from_line.fetch(IMAGE_KEY)), version: version, - package_manager: "docker", + package_manager: PACKAGE_MANAGER, requirements: [ requirement: nil, groups: [], @@ -77,28 +76,60 @@ def parse dependency_set.dependencies end + sig { returns(Ecosystem) } + def ecosystem + @ecosystem ||= T.let( + Ecosystem.new( + name: ECOSYSTEM, + package_manager: package_manager + ), + T.nilable(Ecosystem) + ) + end + private + sig { returns(Ecosystem::VersionManager) } + def package_manager + @package_manager ||= T.let( + PackageManager.new(docker_version || "latest"), + T.nilable(Dependabot::Docker::PackageManager) + ) + end + + sig { returns(T.nilable(String)) } + def docker_version + @docker_version ||= T.let( + begin + dockerfile = dockerfiles.find { |f| f.name == "Dockerfile" } + return unless dockerfile + + dockerfile.content&.match(/FROM docker:(?[0-9.]+)/)&.named_captures&.fetch(VERSION_KEY) + end, + T.nilable(String) + ) + end + sig { returns(T::Array[Dependabot::DependencyFile]) } def dockerfiles # The Docker file fetcher fetches Dockerfiles and yaml files. Reject yaml files. - dependency_files.reject { |f| f.type == "file" && f.name.match?(YAML_REGEXP) } + dependency_files.reject { |f| f.type == FILE_TYPE && f.name.match?(YAML_REGEXP) } end sig { params(parsed_from_line: T::Hash[String, T.nilable(String)]).returns(T.nilable(String)) } def version_from(parsed_from_line) - parsed_from_line.fetch("tag") || parsed_from_line.fetch("digest") + parsed_from_line.fetch(TAG_KEY) || parsed_from_line.fetch(DIGEST_KEY) end sig { params(parsed_from_line: T::Hash[String, T.nilable(String)]).returns(T::Hash[String, T.nilable(String)]) } def source_from(parsed_from_line) source = {} - source[:registry] = parsed_from_line.fetch("registry") if parsed_from_line.fetch("registry") + source[:registry] = parsed_from_line.fetch(REGISTERY_KEY) if parsed_from_line.fetch(REGISTERY_KEY) - source[:tag] = parsed_from_line.fetch("tag") if parsed_from_line.fetch("tag") + source[:tag] = parsed_from_line.fetch(TAG_KEY) if parsed_from_line.fetch(TAG_KEY) - source[:digest] = parsed_from_line.fetch("digest") if parsed_from_line.fetch("digest") + source[:digest] = parsed_from_line.fetch(DIGEST_KEY) if parsed_from_line.fetch(DIGEST_KEY) source end @@ -108,7 +139,7 @@ def check_required_files # Just check if there are any files at all. return if dependency_files.any? - raise "No Dockerfile!" + raise "No #{MANIFEST_FILE}!" end sig { params(file: T.untyped).returns(Dependabot::FileParsers::Base::DependencySet) } @@ -125,7 +156,7 @@ def workfile_file_dependencies(file) details = string.match(IMAGE_SPEC)&.named_captures next if details.nil? - details["registry"] = nil if details["registry"] == "docker.io" + details[REGISTERY_KEY] = nil if details[REGISTERY_KEY] == REGISTERY_DOMAIN version = version_from(details) next unless version @@ -145,9 +176,9 @@ def workfile_file_dependencies(file) end def build_image_dependency(file, details, version) Dependency.new( - name: details.fetch("image"), + name: details.fetch(IMAGE_KEY), version: version, - package_manager: "docker", + package_manager: PACKAGE_MANAGER, requirements: [ requirement: nil, groups: [], @@ -168,7 +199,7 @@ def deep_fetch_images(json_obj) sig { params(json_object: T.untyped).returns(T::Array[T.untyped]) } def deep_fetch_images_from_hash(json_object) - img = json_object.fetch("image", nil) + img = json_object.fetch(IMAGE_KEY, nil) images = if !img.nil? && img.is_a?(String) && !img.empty? @@ -185,23 +216,23 @@ def deep_fetch_images_from_hash(json_object) sig { returns(T::Array[Dependabot::DependencyFile]) } def manifest_files # Dependencies include both Dockerfiles and yaml, select yaml. - dependency_files.select { |f| f.type == "file" && f.name.match?(YAML_REGEXP) } + dependency_files.select { |f| f.type == FILE_TYPE && f.name.match?(YAML_REGEXP) } end sig { params(img_hash: T::Hash[String, T.nilable(String)]).returns(T::Array[String]) } def parse_helm(img_hash) - tag_value = img_hash.key?("tag") ? img_hash.fetch("tag", nil) : img_hash.fetch("version", nil) + tag_value = img_hash.key?(TAG_KEY) ? img_hash.fetch(TAG_KEY, nil) : img_hash.fetch(VERSION_KEY, nil) return [] unless tag_value - repo = img_hash.fetch("repository", nil) + repo = img_hash.fetch(REPOSITORY_KEY, nil) return [] unless repo tag_details = T.must(tag_value.to_s.match(TAG_WITH_DIGEST)).named_captures - tag = tag_details["tag"] + tag = tag_details[TAG_KEY] return [repo] unless tag - registry = img_hash.fetch("registry", nil) - digest = tag_details["digest"] + registry = img_hash.fetch(REGISTERY_KEY, nil) + digest = tag_details[DIGEST_KEY] image = "#{repo}:#{tag}" image.prepend("#{registry}/") if registry diff --git a/docker/lib/dependabot/docker/file_updater.rb b/docker/lib/dependabot/docker/file_updater.rb index c0100cffe9..054347bfa3 100644 --- a/docker/lib/dependabot/docker/file_updater.rb +++ b/docker/lib/dependabot/docker/file_updater.rb @@ -5,6 +5,7 @@ require "dependabot/file_updaters" require "dependabot/file_updaters/base" require "dependabot/errors" +require "dependabot/docker/package_manager" require "sorbet-runtime" module Dependabot @@ -12,11 +13,6 @@ module Docker class FileUpdater < Dependabot::FileUpdaters::Base extend T::Sig - FROM_REGEX = /FROM(\s+--platform\=\S+)?/i - - YAML_REGEXP = /^[^\.].*\.ya?ml$/i - DOCKER_REGEXP = /dockerfile/i - sig { override.returns(T::Array[Regexp]) } def self.updated_files_regex [ @@ -63,7 +59,7 @@ def check_required_files # Just check if there are any files at all. return if dependency_files.any? - raise "No Dockerfile!" + raise "No #{MANIFEST_FILE}!" end sig { params(file: Dependabot::DependencyFile).returns(String) } @@ -105,7 +101,7 @@ def update_digest_and_tag(previous_content, old_source, new_source) "" end old_declaration += - if specified_with_digest?(old_source) then "@sha256:#{old_digest}" + if specified_with_digest?(old_source) then "@#{SHA256_KEY}:#{old_digest}" else "" end @@ -116,7 +112,7 @@ def update_digest_and_tag(previous_content, old_source, new_source) previous_content.gsub(old_declaration_regex) do |old_dec| old_dec - .gsub("@sha256:#{old_digest}", "@sha256:#{new_digest}") + .gsub("@#{SHA256_KEY}:#{old_digest}", "@#{SHA256_KEY}:#{new_digest}") .gsub(":#{old_tag}", ":#{new_tag}") end end @@ -206,7 +202,7 @@ def update_image(file, content) def new_yaml_image(file) element = T.must(dependency).requirements.find { |r| r[:file] == file.name } prefix = element&.dig(:source, :registry) ? "#{element.fetch(:source)[:registry]}/" : "" - digest = element&.dig(:source, :digest) ? "@sha256:#{element.fetch(:source)[:digest]}" : "" + digest = element&.dig(:source, :digest) ? "@#{SHA256_KEY}:#{element.fetch(:source)[:digest]}" : "" tag = element&.dig(:source, :tag) ? ":#{element.fetch(:source)[:tag]}" : "" "#{prefix}#{T.must(dependency).name}#{tag}#{digest}" end @@ -215,7 +211,7 @@ def new_yaml_image(file) def old_yaml_images(file) T.must(previous_requirements(file)).map do |r| prefix = r.fetch(:source)[:registry] ? "#{r.fetch(:source)[:registry]}/" : "" - digest = r.fetch(:source)[:digest] ? "@sha256:#{r.fetch(:source)[:digest]}" : "" + digest = r.fetch(:source)[:digest] ? "@#{SHA256_KEY}:#{r.fetch(:source)[:digest]}" : "" tag = r.fetch(:source)[:tag] ? ":#{r.fetch(:source)[:tag]}" : "" "#{prefix}#{T.must(dependency).name}#{tag}#{digest}" end @@ -225,7 +221,7 @@ def old_yaml_images(file) def old_helm_tags(file) T.must(previous_requirements(file)).map do |r| tag = r.fetch(:source)[:tag] || "" - digest = r.fetch(:source)[:digest] ? "@sha256:#{r.fetch(:source)[:digest]}" : "" + digest = r.fetch(:source)[:digest] ? "@#{SHA256_KEY}:#{r.fetch(:source)[:digest]}" : "" "#{tag}#{digest}" end end @@ -234,7 +230,7 @@ def old_helm_tags(file) def new_helm_tag(file) element = T.must(dependency).requirements.find { |r| r[:file] == file.name } tag = T.must(element).dig(:source, :tag) || "" - digest = T.must(element).dig(:source, :digest) ? "@sha256:#{T.must(element).dig(:source, :digest)}" : "" + digest = T.must(element).dig(:source, :digest) ? "@#{SHA256_KEY}:#{T.must(element).dig(:source, :digest)}" : "" "#{tag}#{digest}" end diff --git a/docker/lib/dependabot/docker/package_manager.rb b/docker/lib/dependabot/docker/package_manager.rb new file mode 100644 index 0000000000..699e33d3ef --- /dev/null +++ b/docker/lib/dependabot/docker/package_manager.rb @@ -0,0 +1,64 @@ +# typed: strong +# frozen_string_literal: true + +require "sorbet-runtime" +require "dependabot/docker/version" +require "dependabot/ecosystem" +require "dependabot/docker/requirement" + +module Dependabot + module Docker + ECOSYSTEM = T.let("docker", String) + PACKAGE_MANAGER = T.let("docker", String) + + MANIFEST_FILE = T.let("Dockerfile", String) + + FILE_TYPE = "file" + API_VERSION_KEY = "apiVersion" + RESOURCE_KEY = "kind" + REGISTERY_KEY = "registry" + IMAGE_KEY = "image" + REPOSITORY_KEY = "repository" + TAG_KEY = "tag" + VERSION_KEY = "version" + DIGEST_KEY = "digest" + + REGISTERY_DOMAIN = "docker.io" + + YAML_REGEXP = /^[^\.].*\.ya?ml$/i + DOCKER_REGEXP = /dockerfile/i + FROM_REGEX = /FROM(\s+--platform\=\S+)?/i + + SHA256_KEY = "sha256" + + class PackageManager < Dependabot::Ecosystem::VersionManager + extend T::Sig + + sig do + params( + raw_version: String, + requirement: T.nilable(Requirement) + ).void + end + def initialize(raw_version, requirement = nil) + super( + PACKAGE_MANAGER, + Version.new(raw_version), + [], + [], + requirement, + ) + end + + sig { override.returns(T::Boolean) } + def deprecated? + false + end + + sig { override.returns(T::Boolean) } + def unsupported? + false + end + end + end +end From d9c96357982e552e81184af27af821953d7df99a Mon Sep 17 00:00:00 2001 From: Kamil Bukum Date: Tue, 10 Dec 2024 10:38:14 -0800 Subject: [PATCH 2/2] fix --- docker/lib/dependabot/docker/file_parser.rb | 37 +++++++++++++------ .../lib/dependabot/docker/package_manager.rb | 7 ++-- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/docker/lib/dependabot/docker/file_parser.rb b/docker/lib/dependabot/docker/file_parser.rb index 23eb15eca1..5bfac10f24 100644 --- a/docker/lib/dependabot/docker/file_parser.rb +++ b/docker/lib/dependabot/docker/file_parser.rb @@ -92,22 +92,37 @@ def ecosystem sig { returns(Ecosystem::VersionManager) } def package_manager @package_manager ||= T.let( - PackageManager.new(docker_version || "latest"), + PackageManager.new(docker_versions), T.nilable(Dependabot::Docker::PackageManager) ) end - sig { returns(T.nilable(String)) } - def docker_version - @docker_version ||= T.let( - begin - dockerfile = dockerfiles.find { |f| f.name == "Dockerfile" } - return unless dockerfile + sig { returns(T::Array[T::Hash[Symbol, T.nilable(String)]]) } + def docker_versions + @docker_versions ||= T.let(begin + versions = [] - dockerfile.content&.match(/FROM docker:(?[0-9.]+)/)&.named_captures&.fetch(VERSION_KEY) - end, - T.nilable(String) - ) + dockerfiles.each do |dockerfile| + T.must(dockerfile.content).each_line do |line| + next unless FROM_LINE.match?(line) + + parsed_from_line = T.must(FROM_LINE.match(line)).named_captures + version = version_from(parsed_from_line) + name = parsed_from_line.fetch("name", nil) # Alias name, if present + image = parsed_from_line.fetch("image", nil) + + next unless version && image + + versions << { + image: image, + version: version, + name: name || "none" + } + end + end + + versions + end, T.nilable(T::Array[T.untyped])) end sig { returns(T::Array[Dependabot::DependencyFile]) } diff --git a/docker/lib/dependabot/docker/package_manager.rb b/docker/lib/dependabot/docker/package_manager.rb index 699e33d3ef..6dccd6c1f8 100644 --- a/docker/lib/dependabot/docker/package_manager.rb +++ b/docker/lib/dependabot/docker/package_manager.rb @@ -36,14 +36,15 @@ class PackageManager < Dependabot::Ecosystem::VersionManager sig do params( - raw_version: String, + versions: T::Array[T::Hash[Symbol, T.nilable(String)]], requirement: T.nilable(Requirement) ).void end - def initialize(raw_version, requirement = nil) + def initialize(versions, requirement = nil) + # TODO: needs to get the first one and return it for metrics collection. super( PACKAGE_MANAGER, - Version.new(raw_version), + Version.new("latest"), [], [], requirement,