From 6af823619d1f9b6e6686281a1b9fb930bbde3c8d Mon Sep 17 00:00:00 2001 From: Maxwell Weru Date: Mon, 16 Sep 2024 08:04:59 +0200 Subject: [PATCH] Sync updater files to version 0.275.0 (#1343) --- update-files.ps1 | 2 + updater/lib/dependabot/api_client.rb | 42 +++++- updater/lib/dependabot/dependency_change.rb | 26 ++-- .../dependabot/dependency_change_builder.rb | 20 ++- updater/lib/dependabot/dependency_snapshot.rb | 35 ++++- updater/lib/dependabot/job.rb | 6 +- updater/lib/dependabot/notices_helpers.rb | 75 ++++++++++ updater/lib/dependabot/opentelemetry.rb | 30 +++- updater/lib/dependabot/pull_request.rb | 108 +++++++++++++++ updater/lib/dependabot/service.rb | 15 ++ .../updater/group_update_creation.rb | 15 +- updater/lib/dependabot/updater/operations.rb | 3 +- .../create_security_update_pull_request.rb | 66 +++++---- .../operations/group_update_all_versions.rb | 57 +++----- .../refresh_security_update_pull_request.rb | 118 ++++++++++------ .../refresh_version_update_pull_request.rb | 128 +++++++++++++----- .../updater/operations/update_all_versions.rb | 122 +++++++++++------ .../updater/security_update_helpers.rb | 57 +++++++- updater/spec/dependabot/api_client_spec.rb | 41 ++++++ .../spec/dependabot/dependency_change_spec.rb | 33 +++-- updater/spec/dependabot/service_spec.rb | 25 +++- .../dependabot/updater/operations_spec.rb | 18 +-- 22 files changed, 791 insertions(+), 251 deletions(-) create mode 100644 updater/lib/dependabot/notices_helpers.rb create mode 100644 updater/lib/dependabot/pull_request.rb diff --git a/update-files.ps1 b/update-files.ps1 index 4341ddbe..83a81ae9 100644 --- a/update-files.ps1 +++ b/update-files.ps1 @@ -51,7 +51,9 @@ $files = @( "updater/lib/dependabot/environment.rb" "updater/lib/dependabot/file_fetcher_command.rb" "updater/lib/dependabot/job.rb" + "updater/lib/dependabot/notices_helpers.rb" "updater/lib/dependabot/opentelemetry.rb" + "updater/lib/dependabot/pull_request.rb" "updater/lib/dependabot/sentry.rb" "updater/lib/dependabot/service.rb" "updater/lib/dependabot/setup.rb" diff --git a/updater/lib/dependabot/api_client.rb b/updater/lib/dependabot/api_client.rb index 65be0c1d..24b40822 100644 --- a/updater/lib/dependabot/api_client.rb +++ b/updater/lib/dependabot/api_client.rb @@ -102,8 +102,11 @@ def close_pull_request(dependency_names, reason) sig { params(error_type: T.any(String, Symbol), error_details: T.nilable(T::Hash[T.untyped, T.untyped])).void } def record_update_job_error(error_type:, error_details:) ::Dependabot::OpenTelemetry.tracer.in_span("record_update_job_error", kind: :internal) do |_span| - ::Dependabot::OpenTelemetry.record_update_job_error(job_id: job_id, error_type: error_type, - error_details: error_details) + ::Dependabot::OpenTelemetry.record_update_job_error( + job_id: job_id, + error_type: error_type, + error_details: error_details + ) api_url = "#{base_url}/update_jobs/#{job_id}/record_update_job_error" body = { data: { @@ -123,6 +126,41 @@ def record_update_job_error(error_type:, error_details:) end end + sig do + params( + warn_type: T.any(String, Symbol), + warn_title: String, + warn_description: String + ).void + end + def record_update_job_warning(warn_type:, warn_title:, warn_description:) + ::Dependabot::OpenTelemetry.tracer.in_span("record_update_job_message", kind: :internal) do |_span| + ::Dependabot::OpenTelemetry.record_update_job_warning( + job_id: job_id, + warn_type: warn_type, + warn_title: warn_title, + warn_description: warn_description + ) + api_url = "#{base_url}/update_jobs/#{job_id}/record_update_job_warning" + body = { + data: { + "warn-type": warn_type, + "warn-title": warn_title, + "warn-description": warn_description + } + } + response = http_client.post(api_url, json: body) + raise ApiError, response.body if response.code >= 400 + rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError + retry_count ||= 0 + retry_count += 1 + raise if retry_count > 3 + + sleep(rand(3.0..10.0)) + retry + end + end + sig { params(error_type: T.any(Symbol, String), error_details: T.nilable(T::Hash[T.untyped, T.untyped])).void } def record_update_job_unknown_error(error_type:, error_details:) error_type = "unknown_error" if error_type.nil? diff --git a/updater/lib/dependabot/dependency_change.rb b/updater/lib/dependabot/dependency_change.rb index 161cbb50..a7065986 100644 --- a/updater/lib/dependabot/dependency_change.rb +++ b/updater/lib/dependabot/dependency_change.rb @@ -45,15 +45,19 @@ def initialize(deps_no_previous_version:, deps_no_change:) sig { returns(T.nilable(Dependabot::DependencyGroup)) } attr_reader :dependency_group + sig { returns(T::Array[Dependabot::Notice]) } + attr_reader :notices + sig do params( job: Dependabot::Job, updated_dependencies: T::Array[Dependabot::Dependency], updated_dependency_files: T::Array[Dependabot::DependencyFile], - dependency_group: T.nilable(Dependabot::DependencyGroup) + dependency_group: T.nilable(Dependabot::DependencyGroup), + notices: T::Array[Dependabot::Notice] ).void end - def initialize(job:, updated_dependencies:, updated_dependency_files:, dependency_group: nil) + def initialize(job:, updated_dependencies:, updated_dependency_files:, dependency_group: nil, notices: []) @job = job @updated_dependencies = updated_dependencies @updated_dependency_files = updated_dependency_files @@ -61,6 +65,7 @@ def initialize(job:, updated_dependencies:, updated_dependency_files:, dependenc @pr_message = T.let(nil, T.nilable(Dependabot::PullRequestCreator::Message)) ensure_dependencies_have_directories + @notices = notices end sig { returns(Dependabot::PullRequestCreator::Message) } @@ -90,7 +95,8 @@ def pr_message dependency_group: dependency_group, pr_message_max_length: pr_message_max_length, pr_message_encoding: pr_message_encoding, - ignore_conditions: job.ignore_conditions + ignore_conditions: job.ignore_conditions, + notices: notices ).message @pr_message = message @@ -135,9 +141,11 @@ def merge_changes!(dependency_changes) dependency_changes.each do |dependency_change| updated_dependencies.concat(dependency_change.updated_dependencies) updated_dependency_files.concat(dependency_change.updated_dependency_files) + notices.concat(dependency_change.notices) end updated_dependencies.compact! updated_dependency_files.compact! + notices.compact! end sig { returns(T::Boolean) } @@ -171,11 +179,7 @@ def matches_existing_pr? Set.new(pr["dependencies"]) == updated_dependencies_set(should_consider_directory: directories_in_use) end else - job.existing_pull_requests.any? do |pr| - directories_in_use = pr.all? { |dep| dep["directory"] } - - Set.new(pr) == updated_dependencies_set(should_consider_directory: directories_in_use) - end + job.existing_pull_requests.any?(new_pr) end end @@ -197,6 +201,12 @@ def updated_dependencies_set(should_consider_directory:) ) end + sig { returns(PullRequest) } + def new_pr + @new_pr ||= T.let(PullRequest.create_from_updated_dependencies(updated_dependencies), + T.nilable(Dependabot::PullRequest)) + end + sig { returns(T::Array[Dependabot::Dependency]) } def ensure_dependencies_have_directories updated_dependencies.each do |dep| diff --git a/updater/lib/dependabot/dependency_change_builder.rb b/updater/lib/dependabot/dependency_change_builder.rb index 6e09dcf2..7ae3894e 100644 --- a/updater/lib/dependabot/dependency_change_builder.rb +++ b/updater/lib/dependabot/dependency_change_builder.rb @@ -30,15 +30,17 @@ class DependencyChangeBuilder job: Dependabot::Job, dependency_files: T::Array[Dependabot::DependencyFile], updated_dependencies: T::Array[Dependabot::Dependency], - change_source: T.any(Dependabot::Dependency, Dependabot::DependencyGroup) + change_source: T.any(Dependabot::Dependency, Dependabot::DependencyGroup), + notices: T::Array[Dependabot::Notice] ).returns(Dependabot::DependencyChange) end - def self.create_from(job:, dependency_files:, updated_dependencies:, change_source:) + def self.create_from(job:, dependency_files:, updated_dependencies:, change_source:, notices: []) new( job: job, dependency_files: dependency_files, updated_dependencies: updated_dependencies, - change_source: change_source + change_source: change_source, + notices: notices ).run end @@ -47,10 +49,11 @@ def self.create_from(job:, dependency_files:, updated_dependencies:, change_sour job: Dependabot::Job, dependency_files: T::Array[Dependabot::DependencyFile], updated_dependencies: T::Array[Dependabot::Dependency], - change_source: T.any(Dependabot::Dependency, Dependabot::DependencyGroup) + change_source: T.any(Dependabot::Dependency, Dependabot::DependencyGroup), + notices: T::Array[Dependabot::Notice] ).void end - def initialize(job:, dependency_files:, updated_dependencies:, change_source:) + def initialize(job:, dependency_files:, updated_dependencies:, change_source:, notices: []) @job = job dir = Pathname.new(job.source.directory).cleanpath @@ -61,6 +64,7 @@ def initialize(job:, dependency_files:, updated_dependencies:, change_source:) @updated_dependencies = updated_dependencies @change_source = change_source + @notices = notices end sig { returns(Dependabot::DependencyChange) } @@ -84,7 +88,8 @@ def run job: job, updated_dependencies: updated_deps, updated_dependency_files: updated_files, - dependency_group: source_dependency_group + dependency_group: source_dependency_group, + notices: notices ) end @@ -102,6 +107,9 @@ def run sig { returns(T.any(Dependabot::Dependency, Dependabot::DependencyGroup)) } attr_reader :change_source + sig { returns(T::Array[Dependabot::Notice]) } + attr_reader :notices + sig { returns(T.nilable(String)) } def source_dependency_name return nil unless change_source.is_a? Dependabot::Dependency diff --git a/updater/lib/dependabot/dependency_snapshot.rb b/updater/lib/dependabot/dependency_snapshot.rb index 9fe29693..30f3a0b7 100644 --- a/updater/lib/dependabot/dependency_snapshot.rb +++ b/updater/lib/dependabot/dependency_snapshot.rb @@ -5,6 +5,7 @@ require "sorbet-runtime" require "dependabot/file_parsers" +require "dependabot/notices_helpers" # This class describes the dependencies obtained from a project at a specific commit SHA # including both the Dependabot::DependencyFile objects at that reference as well as @@ -15,6 +16,7 @@ module Dependabot class DependencySnapshot extend T::Sig + include NoticesHelpers sig do params(job: Dependabot::Job, job_definition: T::Hash[String, T.untyped]).returns(Dependabot::DependencySnapshot) @@ -65,6 +67,18 @@ def dependencies T.must(@dependencies[@current_directory]) end + sig { returns(T.nilable(Dependabot::PackageManagerBase)) } + def package_manager + @package_manager[@current_directory] + end + + sig { returns(T::Array[Dependabot::Notice]) } + def notices + # The notices array in dependency snapshot stay immutable, + # so we can return a copy + @notices[@current_directory]&.dup || [] + end + # Returns the subset of all project dependencies which are permitted # by the project configuration. sig { returns(T::Array[Dependabot::Dependency]) } @@ -167,6 +181,9 @@ def initialize(job:, base_commit_sha:, dependency_files:) # rubocop:disable Metr @current_directory = T.let("", String) @dependencies = T.let({}, T::Hash[String, T::Array[Dependabot::Dependency]]) + @package_manager = T.let({}, T::Hash[String, T.nilable(Dependabot::PackageManagerBase)]) + @notices = T.let({}, T::Hash[String, T::Array[Dependabot::Notice]]) + directories.each do |dir| @current_directory = dir @dependencies[dir] = parse_files! @@ -216,7 +233,7 @@ def parse_files! def dependency_file_parser assert_current_directory_set! job.source.directory = @current_directory - Dependabot::FileParsers.for_package_manager(job.package_manager).new( + parser = Dependabot::FileParsers.for_package_manager(job.package_manager).new( dependency_files: dependency_files, repo_contents_path: job.repo_contents_path, source: job.source, @@ -224,6 +241,22 @@ def dependency_file_parser reject_external_code: job.reject_external_code?, options: job.experiments ) + # Add 'package_manager' to the depedency_snapshopt to use it in operations' + package_manager_for_current_directory = parser.package_manager + @package_manager[@current_directory] = package_manager_for_current_directory + + # Log deprecation notices if the package manager is deprecated + # and add them to the notices array + notices_for_current_directory = [] + + # add deprecation notices for the package manager + add_deprecation_notice( + notices: notices_for_current_directory, + package_manager: package_manager_for_current_directory + ) + @notices[@current_directory] = notices_for_current_directory + + parser end sig { params(group: Dependabot::DependencyGroup).returns(T::Array[T::Hash[String, String]]) } diff --git a/updater/lib/dependabot/job.rb b/updater/lib/dependabot/job.rb index 99fdc2d5..75f052e9 100644 --- a/updater/lib/dependabot/job.rb +++ b/updater/lib/dependabot/job.rb @@ -11,6 +11,7 @@ require "dependabot/experiments" require "dependabot/requirements_update_strategy" require "dependabot/source" +require "dependabot/pull_request" # Describes a single Dependabot workload within the GitHub-integrated Service # @@ -60,7 +61,7 @@ class Job sig { returns(T.nilable(T::Array[String])) } attr_reader :dependencies - sig { returns(T::Array[T::Array[T::Hash[String, String]]]) } + sig { returns(T::Array[PullRequest]) } attr_reader :existing_pull_requests sig { returns(T::Array[T::Hash[String, T.untyped]]) } @@ -139,8 +140,7 @@ def initialize(attributes) # rubocop:disable Metrics/AbcSize end, T::Array[Dependabot::Credential]) @dependencies = T.let(attributes.fetch(:dependencies), T.nilable(T::Array[T.untyped])) - @existing_pull_requests = T.let(attributes.fetch(:existing_pull_requests), - T::Array[T::Array[T::Hash[String, String]]]) + @existing_pull_requests = T.let(PullRequest.create_from_job_definition(attributes), T::Array[PullRequest]) # TODO: Make this hash required # # We will need to do a pass updating the CLI and smoke tests before this is possible, diff --git a/updater/lib/dependabot/notices_helpers.rb b/updater/lib/dependabot/notices_helpers.rb new file mode 100644 index 00000000..b5a646c0 --- /dev/null +++ b/updater/lib/dependabot/notices_helpers.rb @@ -0,0 +1,75 @@ +# typed: strong +# frozen_string_literal: true + +require "sorbet-runtime" +require "dependabot/notices" +require "dependabot/package_manager" + +# This module extracts helpers for notice generations that can be used +# for showing notices in logs, pr messages and alert ui page. +module Dependabot + module NoticesHelpers + extend T::Sig + extend T::Helpers + + abstract! + + # Add a deprecation notice to the notice list if the package manager is deprecated + # if the package manager is deprecated. + # notices << deprecation_notices if deprecation_notices + sig do + params( + notices: T::Array[Dependabot::Notice], + package_manager: T.nilable(PackageManagerBase) + ) + .void + end + def add_deprecation_notice(notices:, package_manager:) + # Create a deprecation notice if the package manager is deprecated + deprecation_notice = create_deprecation_notice(package_manager) + + return unless deprecation_notice + + log_notice(deprecation_notice) + + notices << deprecation_notice + end + + sig { params(notice: Dependabot::Notice).void } + def log_notice(notice) + logger = Dependabot.logger + # Log each non-empty line of the deprecation notice description + notice.description.each_line do |line| + line = line.strip + next if line.empty? + + case notice.mode + when Dependabot::Notice::NoticeMode::INFO + logger.info(line) + when Dependabot::Notice::NoticeMode::WARN + logger.warn(line) + when Dependabot::Notice::NoticeMode::ERROR + logger.error(line) + else + logger.info(line) + end + end + end + + private + + sig { params(package_manager: T.nilable(PackageManagerBase)).returns(T.nilable(Dependabot::Notice)) } + def create_deprecation_notice(package_manager) + # Feature flag check if deprecation notice should be added to notices. + return unless Dependabot::Experiments.enabled?(:add_deprecation_warn_to_pr_message) + + return unless package_manager + + return unless package_manager.is_a?(PackageManagerBase) + + Notice.generate_pm_deprecation_notice( + package_manager + ) + end + end +end diff --git a/updater/lib/dependabot/opentelemetry.rb b/updater/lib/dependabot/opentelemetry.rb index 8862f143..9a933b9f 100644 --- a/updater/lib/dependabot/opentelemetry.rb +++ b/updater/lib/dependabot/opentelemetry.rb @@ -10,6 +10,9 @@ module OpenTelemetry module Attributes JOB_ID = "dependabot.job.id" + WARN_TYPE = "dependabot.job.warn_type" + WARN_TITLE = "dependabot.job.warn_title" + WARN_DESCRIPTION = "dependabot.job.warn_description" ERROR_TYPE = "dependabot.job.error_type" ERROR_DETAILS = "dependabot.job.error_details" METRIC = "dependabot.metric" @@ -30,7 +33,6 @@ def self.configure puts "OpenTelemetry is enabled, configuring..." require "opentelemetry/exporter/otlp" - require "sentry-opentelemetry" # OpenTelemetry instrumentation expects the related gem to be loaded. # While most are already loaded by this point in initialization, some are not. @@ -50,14 +52,8 @@ def self.configure config.use "OpenTelemetry::Instrumentation::Faraday" config.use "OpenTelemetry::Instrumentation::HTTP" config.use "OpenTelemetry::Instrumentation::Net::HTTP" - - # https://docs.sentry.io/platforms/ruby/tracing/instrumentation/opentelemetry/ - config.add_span_processor(::Sentry::OpenTelemetry::SpanProcessor.instance) end - # https://docs.sentry.io/platforms/ruby/tracing/instrumentation/opentelemetry/ - ::OpenTelemetry.propagation = ::Sentry::OpenTelemetry::Propagator.new - tracer end @@ -96,6 +92,26 @@ def self.record_update_job_error(job_id:, error_type:, error_details:) current_span.add_event(error_type, attributes: attributes) end + sig do + params( + job_id: T.any(String, Integer), + warn_type: T.any(String, Symbol), + warn_title: String, + warn_description: String + ).void + end + def self.record_update_job_warning(job_id:, warn_type:, warn_title:, warn_description:) + current_span = ::OpenTelemetry::Trace.current_span + + attributes = { + Attributes::JOB_ID => job_id, + Attributes::WARN_TYPE => warn_type, + Attributes::WARN_TITLE => warn_title, + Attributes::WARN_DESCRIPTION => warn_description + } + current_span.add_event(warn_type, attributes: attributes) + end + sig do params( error: StandardError, diff --git a/updater/lib/dependabot/pull_request.rb b/updater/lib/dependabot/pull_request.rb new file mode 100644 index 00000000..f3f4e3a1 --- /dev/null +++ b/updater/lib/dependabot/pull_request.rb @@ -0,0 +1,108 @@ +# typed: strict +# frozen_string_literal: true + +require "sorbet-runtime" + +module Dependabot + class PullRequest + extend T::Sig + + class Dependency + extend T::Sig + + sig { returns(String) } + attr_reader :name + + sig { returns(T.nilable(String)) } + attr_reader :version + + sig { returns(T::Boolean) } + attr_reader :removed + + sig { returns(T.nilable(String)) } + attr_reader :directory + + sig { params(name: String, version: T.nilable(String), removed: T::Boolean, directory: T.nilable(String)).void } + def initialize(name:, version:, removed: false, directory: nil) + @name = name + @version = version + @removed = removed + @directory = directory + end + + sig { returns(T::Hash[Symbol, T.untyped]) } + def to_h + { + name: name, + version: version, + removed: removed? ? true : nil, + directory: directory + }.compact + end + + sig { returns(T::Boolean) } + def removed? + removed + end + end + + sig { returns(T::Array[Dependency]) } + attr_reader :dependencies + + sig { params(attributes: T::Hash[Symbol, T.untyped]).returns(T::Array[Dependabot::PullRequest]) } + def self.create_from_job_definition(attributes) + attributes.fetch(:existing_pull_requests).map do |pr| + new( + pr.map do |dep| + Dependency.new( + name: dep.fetch("dependency-name"), + version: dep.fetch("dependency-version", nil), + removed: dep.fetch("dependency-removed", false), + directory: dep.fetch("directory", nil) + ) + end + ) + end + end + + sig { params(updated_dependencies: T::Array[Dependabot::Dependency]).returns(Dependabot::PullRequest) } + def self.create_from_updated_dependencies(updated_dependencies) + new( + updated_dependencies.filter_map do |dep| + Dependency.new( + name: dep.name, + version: dep.version, + removed: dep.removed?, + directory: dep.directory + ) + end + ) + end + + sig { params(dependencies: T::Array[PullRequest::Dependency]).void } + def initialize(dependencies) + @dependencies = dependencies + end + + sig { params(other: PullRequest).returns(T::Boolean) } + def ==(other) + if using_directory? && other.using_directory? + dependencies.map(&:to_h).difference(other.dependencies.map(&:to_h)).none? + else + dependencies.map { |dep| dep.to_h.except(:directory) }.difference( + other.dependencies.map { |dep| dep.to_h.except(:directory) } + ).none? + end + end + + sig { params(name: String, version: String).returns(T::Boolean) } + def contains_dependency?(name, version) + dependencies.any? { |dep| dep.name == name && dep.version == version } + end + + sig { returns(T::Boolean) } + def using_directory? + dependencies.all? { |dep| !!dep.directory } + end + end +end diff --git a/updater/lib/dependabot/service.rb b/updater/lib/dependabot/service.rb index 5e69e3d2..c8cb0f2e 100644 --- a/updater/lib/dependabot/service.rb +++ b/updater/lib/dependabot/service.rb @@ -81,6 +81,21 @@ def record_update_job_error(error_type:, error_details:, dependency: nil) client.record_update_job_error(error_type: error_type, error_details: error_details) end + sig do + params( + warn_type: T.any(String, Symbol), + warn_title: String, + warn_description: String + ).void + end + def record_update_job_warning(warn_type:, warn_title:, warn_description:) + client.record_update_job_warning( + warn_type: warn_type, + warn_title: warn_title, + warn_description: warn_description + ) + end + sig { params(error_type: T.any(String, Symbol), error_details: T.nilable(T::Hash[T.untyped, T.untyped])).void } def record_update_job_unknown_error(error_type:, error_details:) client.record_update_job_unknown_error(error_type: error_type, error_details: error_details) diff --git a/updater/lib/dependabot/updater/group_update_creation.rb b/updater/lib/dependabot/updater/group_update_creation.rb index 50d5ddca..abf23f64 100644 --- a/updater/lib/dependabot/updater/group_update_creation.rb +++ b/updater/lib/dependabot/updater/group_update_creation.rb @@ -6,6 +6,8 @@ require "dependabot/dependency_change_builder" require "dependabot/updater/dependency_group_change_batch" require "dependabot/workspace" +require "dependabot/updater/security_update_helpers" +require "dependabot/notices" # This module contains the methods required to build a DependencyChange for # a single DependencyGroup. @@ -22,6 +24,7 @@ class Updater module GroupUpdateCreation extend T::Sig extend T::Helpers + include PullRequestHelpers abstract! @@ -52,6 +55,9 @@ def compile_all_dependency_changes_for(group) ) original_dependencies = dependency_snapshot.dependencies + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + notices = dependency_snapshot.notices + Dependabot.logger.info("Updating the #{job.source.directory} directory.") group.dependencies.each do |dependency| # We still want to update a dependency if it's been updated in another manifest files, @@ -89,6 +95,8 @@ def compile_all_dependency_changes_for(group) dep.name.casecmp(dependency.name)&.zero? end + next unless lead_dependency + dependency_change = create_change_for(T.must(lead_dependency), updated_dependencies, dependency_files, group) # Move on to the next dependency using the existing files if we @@ -106,7 +114,8 @@ def compile_all_dependency_changes_for(group) job: job, updated_dependencies: group_changes.updated_dependencies, updated_dependency_files: group_changes.updated_dependency_files, - dependency_group: group + dependency_group: group, + notices: notices ) if Experiments.enabled?("dependency_change_validation") && !dependency_change.all_have_previous_version? @@ -114,6 +123,10 @@ def compile_all_dependency_changes_for(group) return nil end + # Send warning alerts to the API if any warning notices are present. + # Note that only notices with notice.show_alert set to true will be sent. + record_warning_notices(notices) if notices.any? + dependency_change ensure cleanup_workspace diff --git a/updater/lib/dependabot/updater/operations.rb b/updater/lib/dependabot/updater/operations.rb index b1b59187..9b33014d 100644 --- a/updater/lib/dependabot/updater/operations.rb +++ b/updater/lib/dependabot/updater/operations.rb @@ -34,8 +34,7 @@ module Operations RefreshGroupUpdatePullRequest, CreateSecurityUpdatePullRequest, RefreshSecurityUpdatePullRequest, - RefreshVersionUpdatePullRequest, - UpdateAllVersions + RefreshVersionUpdatePullRequest ].freeze def self.class_for(job:) diff --git a/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb b/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb index 941f469b..156a48f3 100644 --- a/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb @@ -2,6 +2,7 @@ # frozen_string_literal: true require "dependabot/updater/security_update_helpers" +require "dependabot/notices" # This class implements our strategy for updating a single, insecure dependency # to a secure version. We attempt to make the smallest version update possible, @@ -12,6 +13,7 @@ module Operations class CreateSecurityUpdatePullRequest extend T::Sig include SecurityUpdateHelpers + include PullRequestHelpers sig { params(job: Job).returns(T::Boolean) } def self.applies_to?(job:) @@ -42,7 +44,9 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:) @dependency_snapshot = dependency_snapshot @error_handler = error_handler # TODO: Collect @created_pull_requests on the Job object? - @created_pull_requests = T.let([], T::Array[T::Array[T::Hash[String, String]]]) + @created_pull_requests = T.let([], T::Array[PullRequest]) + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + @notices = T.let([], T::Array[Dependabot::Notice]) end # TODO: We currently tolerate multiple dependencies for this operation @@ -55,6 +59,10 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:) def perform Dependabot.logger.info("Starting security update job for #{job.source.repo}") + # Retrieve the list of initial notices from dependency snapshot + @notices = dependency_snapshot.notices + # More notices can be added during the update process + target_dependencies = dependency_snapshot.job_dependencies if target_dependencies.empty? @@ -74,8 +82,11 @@ def perform attr_reader :dependency_snapshot sig { returns(Dependabot::Updater::ErrorHandler) } attr_reader :error_handler - sig { returns(T::Array[T::Array[T::Hash[String, String]]]) } + sig { returns(T::Array[PullRequest]) } attr_reader :created_pull_requests + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + sig { returns(T::Array[Dependabot::Notice]) } + attr_reader :notices sig { params(dependency: Dependabot::Dependency).void } def check_and_create_pr_with_error_handling(dependency) @@ -152,11 +163,11 @@ def check_and_create_pull_request(dependency) # request) record_pull_request_exists_for_security_update(existing_pr) - deps = existing_pr.map do |dep| - if dep.fetch("dependency-removed", false) - "#{dep.fetch('dependency-name')}@removed" + deps = existing_pr.dependencies.map do |dep| + if dep.removed? + "#{dep.name}@removed" else - "#{dep.fetch('dependency-name')}@#{dep.fetch('dependency-version')}" + "#{dep.name}@#{dep.version}" end end @@ -169,9 +180,15 @@ def check_and_create_pull_request(dependency) job: job, dependency_files: dependency_snapshot.dependency_files, updated_dependencies: updated_deps, - change_source: checker.dependency + change_source: checker.dependency, + # Sending notices to the pr message builder to be used in the PR message if show_in_pr is true + notices: @notices ) + # Send warning alerts to the API if any warning notices are present. + # Note that only notices with notice.show_alert set to true will be sent. + record_warning_notices(notices) if notices.any? + create_pull_request(dependency_change) rescue Dependabot::AllVersionsIgnored Dependabot.logger.info("All updates for #{dependency.name} were ignored") @@ -243,29 +260,19 @@ def pr_exists_for_latest_version?(checker) return false if latest_version.nil? job.existing_pull_requests - .select { |pr| pr.count == 1 } - .map(&:first) - .select { |pr| pr && pr.fetch("dependency-name") == checker.dependency.name } - .any? { |pr| pr && pr.fetch("dependency-version", nil) == latest_version } + .any? { |pr| pr.contains_dependency?(checker.dependency.name, latest_version) } || + created_pull_requests.any? { |pr| pr.contains_dependency?(checker.dependency.name, latest_version) } end sig do params(updated_dependencies: T::Array[Dependabot::Dependency]) - .returns(T.nilable(T::Array[T::Hash[String, String]])) + .returns(T.nilable(PullRequest)) end def existing_pull_request(updated_dependencies) - new_pr_set = Set.new( - updated_dependencies.map do |dep| - { - "dependency-name" => dep.name, - "dependency-version" => dep.version, - "dependency-removed" => dep.removed? ? true : nil - }.compact - end - ) + new_pr = PullRequest.create_from_updated_dependencies(updated_dependencies) - job.existing_pull_requests.find { |pr| Set.new(pr) == new_pr_set } || - created_pull_requests.find { |pr| Set.new(pr) == new_pr_set } + job.existing_pull_requests.find { |pr| pr == new_pr } || + created_pull_requests.find { |pr| pr == new_pr } end sig { params(checker: Dependabot::UpdateCheckers::Base).returns(Symbol) } @@ -289,18 +296,7 @@ def create_pull_request(dependency_change) service.create_pull_request(dependency_change, dependency_snapshot.base_commit_sha) - created_pull_requests << dependency_change.updated_dependencies.map do |dep| - create_pull_request_for_dependency(dep) - end - end - - sig { params(dependency: Dependabot::Dependency).returns(T::Hash[String, String]) } - def create_pull_request_for_dependency(dependency) - { - "dependency-name" => dependency.name, - "dependency-version" => dependency.version, - "dependency-removed" => dependency.removed? ? true : nil - }.compact + created_pull_requests << PullRequest.create_from_updated_dependencies(dependency_change.updated_dependencies) end end end diff --git a/updater/lib/dependabot/updater/operations/group_update_all_versions.rb b/updater/lib/dependabot/updater/operations/group_update_all_versions.rb index afb90751..9eaf7a79 100644 --- a/updater/lib/dependabot/updater/operations/group_update_all_versions.rb +++ b/updater/lib/dependabot/updater/operations/group_update_all_versions.rb @@ -22,14 +22,12 @@ class GroupUpdateAllVersions include GroupUpdateCreation sig { params(job: Dependabot::Job).returns(T::Boolean) } - def self.applies_to?(job:) # rubocop:disable Metrics/PerceivedComplexity + def self.applies_to?(job:) return false if job.updating_a_pull_request? if Dependabot::Experiments.enabled?(:grouped_security_updates_disabled) && job.security_updates_only? return false end - return true if job.source.directories && T.must(job.source.directories).count > 1 - if job.security_updates_only? return true if job.dependencies && T.must(job.dependencies).count > 1 return true if job.dependency_groups.any? { |group| group["applies-to"] == "security-updates" } @@ -37,7 +35,7 @@ def self.applies_to?(job:) # rubocop:disable Metrics/PerceivedComplexity return false end - job.dependency_groups.any? + true end sig { returns(Symbol) } @@ -63,25 +61,7 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:) sig { void } def perform - if dependency_snapshot.groups.any? - run_grouped_dependency_updates - else - # We shouldn't have selected this operation if no groups were defined - # due to the rules in `::applies_to?`, but if it happens it isn't - # enough reasons to fail the job. - Dependabot.logger.warn( - "No dependency groups defined!" - ) - - # We should warn our exception tracker in case this represents an - # unexpected problem hydrating groups we have swallowed and then - # delegate everything to run_ungrouped_dependency_updates. - service.capture_exception( - error: DependabotError.new("Attempted a grouped update with no groups defined."), - job: job - ) - end - + run_grouped_dependency_updates if dependency_snapshot.groups.any? run_ungrouped_dependency_updates end @@ -139,8 +119,15 @@ def run_grouped_update_for(group) sig { void } def run_ungrouped_dependency_updates - if job.source.directories.nil? - return if dependency_snapshot.ungrouped_dependencies.empty? + directories.each do |directory| + job.source.directory = directory + dependency_snapshot.current_directory = directory + next unless dependency_snapshot.dependencies.any? + + if dependency_snapshot.ungrouped_dependencies.empty? + Dependabot.logger.info("Found no dependencies to update after filtering allowed updates in #{directory}") + next + end Dependabot::Updater::Operations::UpdateAllVersions.new( service: service, @@ -148,19 +135,15 @@ def run_ungrouped_dependency_updates dependency_snapshot: dependency_snapshot, error_handler: error_handler ).perform + end + end + + sig { returns(T::Array[String]) } + def directories + if job.source.directories.nil? + [T.must(job.source.directory)] else - T.must(job.source.directories).each do |directory| - job.source.directory = directory - dependency_snapshot.current_directory = directory - next if dependency_snapshot.ungrouped_dependencies.empty? - - Dependabot::Updater::Operations::UpdateAllVersions.new( - service: service, - job: job, - dependency_snapshot: dependency_snapshot, - error_handler: error_handler - ).perform - end + T.must(job.source.directories) end end end diff --git a/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb b/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb index 1b1aefa6..a70b4662 100644 --- a/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb @@ -1,6 +1,9 @@ -# typed: true +# typed: strong # frozen_string_literal: true +require "dependabot/updater/security_update_helpers" +require "dependabot/notices" + # This class implements our strategy for 'refreshing' an existing Pull Request # that updates an insecure dependency. # @@ -14,8 +17,11 @@ module Dependabot class Updater module Operations class RefreshSecurityUpdatePullRequest + extend T::Sig include SecurityUpdateHelpers + include PullRequestHelpers + sig { params(job: Job).returns(T::Boolean) } def self.applies_to?(job:) return false unless job.security_updates_only? # If we haven't been given metadata about the dependencies present @@ -25,31 +31,53 @@ def self.applies_to?(job:) job.updating_a_pull_request? end + sig { returns(Symbol) } def self.tag_name :update_security_pr end + sig do + params(service: Service, job: Job, dependency_snapshot: DependencySnapshot, error_handler: ErrorHandler) + .void + end def initialize(service:, job:, dependency_snapshot:, error_handler:) @service = service @job = job @dependency_snapshot = dependency_snapshot @error_handler = error_handler + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + @notices = T.let([], T::Array[Dependabot::Notice]) end + sig { void } def perform - dependency = dependencies.last + Dependabot.logger.info("Starting update job for #{job.source.repo}") + Dependabot.logger.info("Checking and updating security pull requests...") + + # Retrieve the list of initial notices from dependency snapshot + @notices = dependency_snapshot.notices + # More notices can be added during the update process + check_and_update_pull_request(dependencies) rescue StandardError => e - error_handler.handle_dependency_error(error: e, dependency: dependency) + error_handler.handle_dependency_error(error: e, dependency: dependencies.last) end private + sig { returns(Dependabot::Job) } attr_reader :job + sig { returns(Dependabot::Service) } attr_reader :service + sig { returns(Dependabot::DependencySnapshot) } attr_reader :dependency_snapshot + sig { returns(Dependabot::Updater::ErrorHandler) } attr_reader :error_handler + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + sig { returns(T::Array[Dependabot::Notice]) } + attr_reader :notices + sig { returns(T::Array[Dependabot::Dependency]) } def dependencies dependency_snapshot.job_dependencies end @@ -57,8 +85,18 @@ def dependencies # rubocop:disable Metrics/AbcSize # rubocop:disable Metrics/PerceivedComplexity # rubocop:disable Metrics/MethodLength + # rubocop:disable Metrics/CyclomaticComplexity + sig { params(dependencies: T::Array[Dependabot::Dependency]).void } def check_and_update_pull_request(dependencies) - if dependencies.count != job.dependencies.count + # If the job dependencies are empty, then we should close the PR + job_dependencies = job.dependencies + unless job_dependencies + Dependabot.logger.info("No dependencies to update") + close_pull_request(reason: :dependencies_removed) + return + end + + if dependencies.count != job_dependencies.count # If the job dependencies mismatch the parsed dependencies, then # we should close the PR as at least one thing we changed has been # removed from the project. @@ -72,12 +110,14 @@ def check_and_update_pull_request(dependencies) # pull request is rebased. if dependencies.none? { |d| job.allowed_update?(d) } lead_dependency = dependencies.first - if job.vulnerable?(lead_dependency) + if lead_dependency && job.vulnerable?(lead_dependency) Dependabot.logger.info( "Dependency no longer allowed to update #{lead_dependency.name} #{lead_dependency.version}" ) - else + elsif lead_dependency Dependabot.logger.info("No longer vulnerable #{lead_dependency.name} #{lead_dependency.version}") + else + Dependabot.logger.info("No dependencies to update") end close_pull_request(reason: :up_to_date) return @@ -89,10 +129,13 @@ def check_and_update_pull_request(dependencies) # Note: Gradle, Maven and Nuget dependency names can be case-insensitive # and the dependency name in the security advisory often doesn't match # what users have specified in their manifest. - lead_dep_name = job.dependencies.first.downcase + lead_dep_name = job_dependencies.first&.downcase lead_dependency = dependencies.find do |dep| dep.name.downcase == lead_dep_name end + + return close_pull_request(reason: :update_no_longer_possible) unless lead_dependency + checker = update_checker_for(lead_dependency) log_checking_for_update(lead_dependency) @@ -115,13 +158,19 @@ def check_and_update_pull_request(dependencies) job: job, dependency_files: dependency_snapshot.dependency_files, updated_dependencies: updated_deps, - change_source: checker.dependency + change_source: checker.dependency, + # Sending notices to the pr message builder to be used in the PR message if show_in_pr is true + notices: @notices ) + # Send warning alerts to the API if any warning notices are present. + # Note that only notices with notice.show_alert set to true will be sent. + record_warning_notices(notices) if notices.any? + # NOTE: Gradle, Maven and Nuget dependency names can be case-insensitive # and the dependency name in the security advisory often doesn't match # what users have specified in their manifest. - job_dependencies = job.dependencies.map(&:downcase) + job_dependencies = job_dependencies.map(&:downcase) if dependency_change.updated_dependencies.map { |x| x.name.downcase } != job_dependencies # The dependencies being updated have changed. Close the existing # multi-dependency PR and try creating a new one. @@ -135,7 +184,7 @@ def check_and_update_pull_request(dependencies) create_pull_request(dependency_change) end rescue Dependabot::AllVersionsIgnored - Dependabot.logger.info("All updates for #{job.dependencies.first} were ignored") + Dependabot.logger.info("All updates for #{job_dependencies&.first} were ignored") # Report this error to the backend to create an update job error raise @@ -143,7 +192,9 @@ def check_and_update_pull_request(dependencies) # rubocop:enable Metrics/AbcSize # rubocop:enable Metrics/PerceivedComplexity # rubocop:enable Metrics/MethodLength + # rubocop:enable Metrics/CyclomaticComplexity + sig { params(checker: Dependabot::UpdateCheckers::Base).returns(Symbol) } def requirements_to_unlock(checker) if !checker.requirements_unlocked_or_can_be? if checker.can_update?(requirements_to_unlock: :none) then :none @@ -157,6 +208,7 @@ def requirements_to_unlock(checker) end end + sig { params(dependency: Dependabot::Dependency).returns(Dependabot::UpdateCheckers::Base) } def update_checker_for(dependency) Dependabot::UpdateCheckers.for_package_manager(job.package_manager).new( dependency: dependency, @@ -171,6 +223,7 @@ def update_checker_for(dependency) ) end + sig { params(dependency: Dependabot::Dependency).void } def log_checking_for_update(dependency) Dependabot.logger.info( "Checking if #{dependency.name} #{dependency.version} needs updating" @@ -178,12 +231,14 @@ def log_checking_for_update(dependency) job.log_ignore_conditions_for(dependency) end + sig { params(dependency: Dependabot::Dependency).void } def log_up_to_date(dependency) Dependabot.logger.info( "No update needed for #{dependency.name} #{dependency.version}" ) end + sig { params(requirements_to_unlock: Symbol, checker: Dependabot::UpdateCheckers::Base).void } def log_requirements_for_update(requirements_to_unlock, checker) Dependabot.logger.info("Requirements to unlock #{requirements_to_unlock}") @@ -194,35 +249,16 @@ def log_requirements_for_update(requirements_to_unlock, checker) ) end + sig do + params(updated_dependencies: T::Array[Dependabot::Dependency]) + .returns(T.nilable(PullRequest)) + end def existing_pull_request(updated_dependencies) - new_pr_set = Set.new( - updated_dependencies.map do |dep| - { - "dependency-name" => dep.name, - "dependency-version" => dep.version, - "dependency-removed" => dep.removed? ? true : nil, - "directory" => dep.directory - }.compact - end - ) - - existing = job.existing_pull_requests.find { |pr| Set.new(pr) == new_pr_set } - return existing if existing - - # try the search again without directory - new_pr_set = Set.new( - updated_dependencies.map do |dep| - { - "dependency-name" => dep.name, - "dependency-version" => dep.version, - "dependency-removed" => dep.removed? ? true : nil - }.compact - end - ) - - job.existing_pull_requests.find { |pr| Set.new(pr) == new_pr_set } + new_pr = PullRequest.create_from_updated_dependencies(updated_dependencies) + job.existing_pull_requests.find { |pr| pr == new_pr } end + sig { params(dependency_change: Dependabot::DependencyChange).void } def create_pull_request(dependency_change) Dependabot.logger.info("Submitting #{dependency_change.updated_dependencies.map(&:name).join(', ')} " \ "pull request for creation") @@ -230,6 +266,7 @@ def create_pull_request(dependency_change) service.create_pull_request(dependency_change, dependency_snapshot.base_commit_sha) end + sig { params(dependency_change: Dependabot::DependencyChange).void } def update_pull_request(dependency_change) Dependabot.logger.info("Submitting #{dependency_change.updated_dependencies.map(&:name).join(', ')} " \ "pull request for update") @@ -237,11 +274,16 @@ def update_pull_request(dependency_change) service.update_pull_request(dependency_change, dependency_snapshot.base_commit_sha) end + sig { params(reason: Symbol).void } def close_pull_request(reason:) reason_string = reason.to_s.tr("_", " ") + + job_dependencies = job.dependencies || [] + Dependabot.logger.info("Telling backend to close pull request for " \ - "#{job.dependencies.join(', ')} - #{reason_string}") - service.close_pull_request(job.dependencies, reason) + "#{job_dependencies.join(', ')} - #{reason_string}") + + service.close_pull_request(job_dependencies, reason) end end end diff --git a/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb b/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb index 81c1d660..aafa2b2e 100644 --- a/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb @@ -1,6 +1,9 @@ -# typed: true +# typed: strong # frozen_string_literal: true +require "dependabot/updater/security_update_helpers" +require "dependabot/notices" + # This class implements our strategy for 'refreshing' an existing Pull Request # that updates a dependnency to the latest permitted version. # @@ -12,6 +15,10 @@ module Dependabot class Updater module Operations class RefreshVersionUpdatePullRequest + extend T::Sig + include PullRequestHelpers + + sig { params(job: Dependabot::Job).returns(T::Boolean) } def self.applies_to?(job:) return false if job.security_updates_only? # If we haven't been given metadata about the dependencies present @@ -21,24 +28,42 @@ def self.applies_to?(job:) job.updating_a_pull_request? end + sig { returns(Symbol) } def self.tag_name :update_version_pr end + sig do + params( + service: Dependabot::Service, + job: Dependabot::Job, + dependency_snapshot: Dependabot::DependencySnapshot, + error_handler: ErrorHandler + ).void + end def initialize(service:, job:, dependency_snapshot:, error_handler:) @service = service @job = job @dependency_snapshot = dependency_snapshot @error_handler = error_handler + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + @notices = T.let([], T::Array[Dependabot::Notice]) - return unless job.source.directory.nil? && job.source.directories.count == 1 + return unless job.source.directory.nil? && job.source.directories&.count == 1 - job.source.directory = job.source.directories.first + job.source.directory = job.source.directories&.first end + sig { void } def perform - Dependabot.logger.info("Starting PR update job for #{job.source.repo}") + Dependabot.logger.info("Starting update job for #{job.source.repo}") + Dependabot.logger.info("Checking and updating versions pull requests...") dependency = dependencies.last + + # Retrieve the list of initial notices from dependency snapshot + @notices = dependency_snapshot.notices + # More notices can be added during the update process + check_and_update_pull_request(dependencies) rescue StandardError => e error_handler.handle_dependency_error(error: e, dependency: dependency) @@ -46,20 +71,33 @@ def perform private + sig { returns(Dependabot::Job) } attr_reader :job + sig { returns(Dependabot::Service) } attr_reader :service + sig { returns(Dependabot::DependencySnapshot) } attr_reader :dependency_snapshot + sig { returns(Dependabot::Updater::ErrorHandler) } attr_reader :error_handler - attr_reader :created_pull_requests + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + sig { returns(T::Array[Dependabot::Notice]) } + attr_reader :notices + sig { returns(T::Array[Dependabot::Dependency]) } def dependencies dependency_snapshot.job_dependencies end # rubocop:disable Metrics/AbcSize # rubocop:disable Metrics/PerceivedComplexity + # rubocop:disable Metrics/MethodLength + sig do + params(dependencies: T::Array[Dependabot::Dependency]).void + end def check_and_update_pull_request(dependencies) - if dependencies.count != job.dependencies.count + job_dependencies = T.must(job.dependencies) + + if job_dependencies.count.zero? || dependencies.count != job_dependencies.count # If the job dependencies mismatch the parsed dependencies, then # we should close the PR as at least one thing we changed has been # removed from the project. @@ -73,10 +111,19 @@ def check_and_update_pull_request(dependencies) # Note: Gradle, Maven and Nuget dependency names can be case-insensitive # and the dependency name in the security advisory often doesn't match # what users have specified in their manifest. - lead_dep_name = job.dependencies.first.downcase + lead_dep_name = T.must(job_dependencies.first).downcase lead_dependency = dependencies.find do |dep| dep.name.downcase == lead_dep_name end + + if lead_dependency.nil? + # If the lead dependency is not found, it indicates that one of the dependencies + # we attempted to update has been removed from the project. Therefore, we should + # close the PR. + close_pull_request(reason: :dependency_removed) + return + end + checker = update_checker_for(lead_dependency, raise_on_ignored: raise_on_ignored?(lead_dependency)) log_checking_for_update(lead_dependency) @@ -99,13 +146,19 @@ def check_and_update_pull_request(dependencies) job: job, dependency_files: dependency_snapshot.dependency_files, updated_dependencies: updated_deps, - change_source: checker.dependency + change_source: checker.dependency, + # Sending notices to the pr message builder to be used in the PR message if show_in_pr is true + notices: @notices ) + # Send warning alerts to the API if any warning notices are present. + # Note that only notices with notice.show_alert set to true will be sent. + record_warning_notices(notices) if notices.any? + # NOTE: Gradle, Maven and Nuget dependency names can be case-insensitive # and the dependency name in the security advisory often doesn't match # what users have specified in their manifest. - job_dependencies = job.dependencies.map(&:downcase) + job_dependencies = job_dependencies.map(&:downcase) if dependency_change.updated_dependencies.map { |x| x.name.downcase } != job_dependencies # The dependencies being updated have changed. Close the existing # multi-dependency PR and try creating a new one. @@ -121,7 +174,9 @@ def check_and_update_pull_request(dependencies) end # rubocop:enable Metrics/AbcSize # rubocop:enable Metrics/PerceivedComplexity + # rubocop:enable Metrics/MethodLength + sig { params(dependency_change: Dependabot::DependencyChange).void } def create_pull_request(dependency_change) Dependabot.logger.info("Submitting #{dependency_change.updated_dependencies.map(&:name).join(', ')} " \ "pull request for creation") @@ -129,6 +184,7 @@ def create_pull_request(dependency_change) service.create_pull_request(dependency_change, dependency_snapshot.base_commit_sha) end + sig { params(dependency_change: Dependabot::DependencyChange).void } def update_pull_request(dependency_change) Dependabot.logger.info("Submitting #{dependency_change.updated_dependencies.map(&:name).join(', ')} " \ "pull request for update") @@ -136,17 +192,25 @@ def update_pull_request(dependency_change) service.update_pull_request(dependency_change, dependency_snapshot.base_commit_sha) end + sig { params(reason: Symbol).void } def close_pull_request(reason:) + job_dependencies = T.must(job.dependencies) + reason_string = reason.to_s.tr("_", " ") Dependabot.logger.info("Telling backend to close pull request for " \ - "#{job.dependencies.join(', ')} - #{reason_string}") - service.close_pull_request(job.dependencies, reason) + "#{job_dependencies.join(', ')} - #{reason_string}") + service.close_pull_request(job_dependencies, reason) end + sig { params(dependency: Dependabot::Dependency).returns(T::Boolean) } def raise_on_ignored?(dependency) job.ignore_conditions_for(dependency).any? end + sig do + params(dependency: Dependabot::Dependency, raise_on_ignored: T::Boolean) + .returns(Dependabot::UpdateCheckers::Base) + end def update_checker_for(dependency, raise_on_ignored:) Dependabot::UpdateCheckers.for_package_manager(job.package_manager).new( dependency: dependency, @@ -161,6 +225,7 @@ def update_checker_for(dependency, raise_on_ignored:) ) end + sig { params(dependency: Dependabot::Dependency).void } def log_checking_for_update(dependency) Dependabot.logger.info( "Checking if #{dependency.name} #{dependency.version} needs updating" @@ -168,6 +233,9 @@ def log_checking_for_update(dependency) job.log_ignore_conditions_for(dependency) end + sig do + params(dependency: Dependabot::Dependency, checker: Dependabot::UpdateCheckers::Base).returns(T::Boolean) + end def all_versions_ignored?(dependency, checker) Dependabot.logger.info("Latest version is #{checker.latest_version}") false @@ -176,6 +244,9 @@ def all_versions_ignored?(dependency, checker) true end + sig do + params(checker: Dependabot::UpdateCheckers::Base).returns(Symbol) + end def requirements_to_unlock(checker) if !checker.requirements_unlocked_or_can_be? if checker.can_update?(requirements_to_unlock: :none) then :none @@ -189,6 +260,9 @@ def requirements_to_unlock(checker) end end + sig do + params(requirements_to_unlock: Symbol, checker: Dependabot::UpdateCheckers::Base).void + end def log_requirements_for_update(requirements_to_unlock, checker) Dependabot.logger.info("Requirements to unlock #{requirements_to_unlock}") @@ -199,33 +273,13 @@ def log_requirements_for_update(requirements_to_unlock, checker) ) end + sig do + params(updated_dependencies: T::Array[Dependabot::Dependency]) + .returns(T.nilable(Dependabot::PullRequest)) + end def existing_pull_request(updated_dependencies) - new_pr_set = Set.new( - updated_dependencies.map do |dep| - { - "dependency-name" => dep.name, - "dependency-version" => dep.version, - "dependency-removed" => dep.removed? ? true : nil, - "directory" => dep.directory - }.compact - end - ) - - existing = job.existing_pull_requests.find { |pr| Set.new(pr) == new_pr_set } - return existing if existing - - # try the search again without directory - new_pr_set = Set.new( - updated_dependencies.map do |dep| - { - "dependency-name" => dep.name, - "dependency-version" => dep.version, - "dependency-removed" => dep.removed? ? true : nil - }.compact - end - ) - - job.existing_pull_requests.find { |pr| Set.new(pr) == new_pr_set } + new_pr = PullRequest.create_from_updated_dependencies(updated_dependencies) + job.existing_pull_requests.find { |pr| pr == new_pr } end end end diff --git a/updater/lib/dependabot/updater/operations/update_all_versions.rb b/updater/lib/dependabot/updater/operations/update_all_versions.rb index 453f5249..bdcc9a20 100644 --- a/updater/lib/dependabot/updater/operations/update_all_versions.rb +++ b/updater/lib/dependabot/updater/operations/update_all_versions.rb @@ -1,6 +1,9 @@ -# typed: true +# typed: strong # frozen_string_literal: true +require "dependabot/updater/security_update_helpers" +require "dependabot/notices" + # This class implements our strategy for iterating over all of the dependencies # for a specific project folder to find those that are out of date and create # a single PR per Dependency. @@ -8,45 +11,71 @@ module Dependabot class Updater module Operations class UpdateAllVersions - def self.applies_to?(job:) - return false if job.security_updates_only? - return false if job.updating_a_pull_request? - return false if job.dependencies&.any? + extend T::Sig + include PullRequestHelpers - true + sig { params(_job: Dependabot::Job).returns(T::Boolean) } + def self.applies_to?(_job:) + false # only called elsewhere end + sig { returns(Symbol) } def self.tag_name :update_all_versions end + sig do + params( + service: Dependabot::Service, + job: Dependabot::Job, + dependency_snapshot: Dependabot::DependencySnapshot, + error_handler: ErrorHandler + ).void + end def initialize(service:, job:, dependency_snapshot:, error_handler:) @service = service @job = job @dependency_snapshot = dependency_snapshot @error_handler = error_handler # TODO: Collect @created_pull_requests on the Job object? - @created_pull_requests = [] + @created_pull_requests = T.let([], T::Array[PullRequest]) + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + @notices = T.let([], T::Array[Dependabot::Notice]) - return unless job.source.directory.nil? && job.source.directories.count == 1 + return unless job.source.directory.nil? && job.source.directories&.count == 1 - job.source.directory = job.source.directories.first + job.source.directory = job.source.directories&.first end + sig { void } def perform Dependabot.logger.info("Starting update job for #{job.source.repo}") Dependabot.logger.info("Checking all dependencies for version updates...") + + # Retrieve the list of initial notices from dependency snapshot + @notices = dependency_snapshot.notices + # More notices can be added during the update process + dependencies.each { |dep| check_and_create_pr_with_error_handling(dep) } end private + sig { returns(Dependabot::Job) } attr_reader :job + sig { returns(Dependabot::Service) } attr_reader :service + sig { returns(Dependabot::DependencySnapshot) } attr_reader :dependency_snapshot + sig { returns(Dependabot::Updater::ErrorHandler) } attr_reader :error_handler + sig { returns(T::Array[PullRequest]) } attr_reader :created_pull_requests + # A list of notices that will be used in PR messages and/or sent to the dependabot github alerts. + sig { returns(T::Array[Dependabot::Notice]) } + attr_reader :notices + sig { returns(T::Array[Dependabot::Dependency]) } def dependencies if dependency_snapshot.dependencies.any? && dependency_snapshot.allowed_dependencies.none? Dependabot.logger.info("Found no dependencies to update after filtering allowed updates") @@ -60,12 +89,12 @@ def dependencies end end + sig { params(dependency: Dependabot::Dependency).void } def check_and_create_pr_with_error_handling(dependency) check_and_create_pull_request(dependency) rescue URI::InvalidURIError => e - msg = e.class.to_s + " with message: " + e.message - e = Dependabot::DependencyFileNotResolvable.new(msg) - error_handler.handle_dependency_error(error: e, dependency: dependency) + error_handler.handle_dependency_error(error: Dependabot::DependencyFileNotResolvable.new(e.message), + dependency: dependency) rescue Dependabot::InconsistentRegistryResponse => e error_handler.log_dependency_error( dependency: dependency, @@ -80,6 +109,7 @@ def check_and_create_pr_with_error_handling(dependency) # rubocop:disable Metrics/AbcSize # rubocop:disable Metrics/MethodLength # rubocop:disable Metrics/PerceivedComplexity + sig { params(dependency: Dependabot::Dependency).void } def check_and_create_pull_request(dependency) checker = update_checker_for(dependency, raise_on_ignored: raise_on_ignored?(dependency)) @@ -113,11 +143,11 @@ def check_and_create_pull_request(dependency) end if (existing_pr = existing_pull_request(updated_deps)) - deps = existing_pr.map do |dep| - if dep.fetch("dependency-removed", false) - "#{dep.fetch('dependency-name')}@removed" + deps = existing_pr.dependencies.map do |dep| + if dep.removed? + "#{dep.name}@removed" else - "#{dep.fetch('dependency-name')}@#{dep.fetch('dependency-version')}" + "#{dep.name}@#{dep.version}" end end @@ -137,29 +167,41 @@ def check_and_create_pull_request(dependency) job: job, dependency_files: dependency_snapshot.dependency_files, updated_dependencies: updated_deps, - change_source: checker.dependency + change_source: checker.dependency, + # Sending notices to the pr message builder to be used in the PR message if show_in_pr is true + notices: @notices ) if dependency_change.updated_dependency_files.empty? raise "UpdateChecker found viable dependencies to be updated, but FileUpdater failed to update any files" end + # Send warning alerts to the API if any warning notices are present. + # Note that only notices with notice.show_alert set to true will be sent. + record_warning_notices(notices) if notices.any? + create_pull_request(dependency_change) end # rubocop:enable Metrics/PerceivedComplexity # rubocop:enable Metrics/MethodLength # rubocop:enable Metrics/AbcSize + sig { params(dependency: Dependabot::Dependency).void } def log_up_to_date(dependency) Dependabot.logger.info( "No update needed for #{dependency.name} #{dependency.version}" ) end + sig { params(dependency: Dependabot::Dependency).returns(T::Boolean) } def raise_on_ignored?(dependency) job.ignore_conditions_for(dependency).any? end + sig do + params(dependency: Dependabot::Dependency, raise_on_ignored: T::Boolean) + .returns(Dependabot::UpdateCheckers::Base) + end def update_checker_for(dependency, raise_on_ignored:) Dependabot::UpdateCheckers.for_package_manager(job.package_manager).new( dependency: dependency, @@ -174,6 +216,7 @@ def update_checker_for(dependency, raise_on_ignored:) ) end + sig { params(dependency: Dependabot::Dependency).void } def log_checking_for_update(dependency) Dependabot.logger.info( "Checking if #{dependency.name} #{dependency.version} needs updating" @@ -181,6 +224,7 @@ def log_checking_for_update(dependency) job.log_ignore_conditions_for(dependency) end + sig { params(error: StandardError, dependency: Dependabot::Dependency).returns(T.untyped) } def process_dependency_error(error, dependency) if error.class.to_s.include?("RegistryError") ex = Dependabot::DependencyFileNotResolvable.new(error.message) @@ -190,6 +234,10 @@ def process_dependency_error(error, dependency) end end + sig do + params(dependency: Dependabot::Dependency, checker: Dependabot::UpdateCheckers::Base) + .returns(T::Boolean) + end def all_versions_ignored?(dependency, checker) Dependabot.logger.info("Latest version is #{checker.latest_version}") false @@ -198,32 +246,28 @@ def all_versions_ignored?(dependency, checker) true end + sig { params(checker: Dependabot::UpdateCheckers::Base).returns(T::Boolean) } def pr_exists_for_latest_version?(checker) latest_version = checker.latest_version&.to_s return false if latest_version.nil? job.existing_pull_requests - .select { |pr| pr.count == 1 } - .map(&:first) - .select { |pr| pr.fetch("dependency-name") == checker.dependency.name } - .any? { |pr| pr.fetch("dependency-version", nil) == latest_version } + .any? { |pr| pr.contains_dependency?(checker.dependency.name, latest_version) } || + created_pull_requests.any? { |pr| pr.contains_dependency?(checker.dependency.name, latest_version) } end + sig do + params(updated_dependencies: T::Array[Dependabot::Dependency]) + .returns(T.nilable(Dependabot::PullRequest)) + end def existing_pull_request(updated_dependencies) - new_pr_set = Set.new( - updated_dependencies.map do |dep| - { - "dependency-name" => dep.name, - "dependency-version" => dep.version, - "dependency-removed" => dep.removed? ? true : nil - }.compact - end - ) + new_pr = PullRequest.create_from_updated_dependencies(updated_dependencies) - job.existing_pull_requests.find { |pr| Set.new(pr) == new_pr_set } || - created_pull_requests.find { |pr| Set.new(pr) == new_pr_set } + job.existing_pull_requests.find { |pr| pr == new_pr } || + created_pull_requests.find { |pr| pr == new_pr } end + sig { params(checker: Dependabot::UpdateCheckers::Base).returns(Symbol) } def requirements_to_unlock(checker) if !checker.requirements_unlocked_or_can_be? if checker.can_update?(requirements_to_unlock: :none) then :none @@ -237,6 +281,7 @@ def requirements_to_unlock(checker) end end + sig { params(requirements_to_unlock: Symbol, checker: Dependabot::UpdateCheckers::Base).void } def log_requirements_for_update(requirements_to_unlock, checker) Dependabot.logger.info("Requirements to unlock #{requirements_to_unlock}") @@ -249,17 +294,19 @@ def log_requirements_for_update(requirements_to_unlock, checker) # If a version update for a peer dependency is possible we should # defer to the PR that will be created for it to avoid duplicate PRs. + sig { params(dependency_name: String, updated_deps: T::Array[Dependabot::Dependency]).returns(T::Boolean) } def peer_dependency_should_update_instead?(dependency_name, updated_deps) updated_deps .reject { |dep| dep.name == dependency_name } .any? do |dep| next true if existing_pull_request([dep]) + next false if dep.previous_requirements.nil? original_peer_dep = ::Dependabot::Dependency.new( name: dep.name, version: dep.previous_version, - requirements: dep.previous_requirements, + requirements: T.must(dep.previous_requirements), package_manager: dep.package_manager ) update_checker_for(original_peer_dep, raise_on_ignored: false) @@ -267,19 +314,14 @@ def peer_dependency_should_update_instead?(dependency_name, updated_deps) end end + sig { params(dependency_change: Dependabot::DependencyChange).void } def create_pull_request(dependency_change) Dependabot.logger.info("Submitting #{dependency_change.updated_dependencies.map(&:name).join(', ')} " \ "pull request for creation") service.create_pull_request(dependency_change, dependency_snapshot.base_commit_sha) - created_pull_requests << dependency_change.updated_dependencies.map do |dep| - { - "dependency-name" => dep.name, - "dependency-version" => dep.version, - "dependency-removed" => dep.removed? ? true : nil - }.compact - end + created_pull_requests << PullRequest.create_from_updated_dependencies(dependency_change.updated_dependencies) end end end diff --git a/updater/lib/dependabot/updater/security_update_helpers.rb b/updater/lib/dependabot/updater/security_update_helpers.rb index c409a521..e4827eea 100644 --- a/updater/lib/dependabot/updater/security_update_helpers.rb +++ b/updater/lib/dependabot/updater/security_update_helpers.rb @@ -120,13 +120,13 @@ def record_pull_request_exists_for_latest_version(checker) ) end - sig { params(existing_pull_request: T::Array[T::Hash[String, String]]).void } + sig { params(existing_pull_request: PullRequest).void } def record_pull_request_exists_for_security_update(existing_pull_request) - updated_dependencies = existing_pull_request.map do |dep| + updated_dependencies = existing_pull_request.dependencies.map do |dep| { - "dependency-name": dep.fetch("dependency-name"), - "dependency-version": dep.fetch("dependency-version", nil), - "dependency-removed": dep.fetch("dependency-removed", nil) + "dependency-name": dep.name, + "dependency-version": dep.version, + "dependency-removed": dep.removed? ? true : nil }.compact end @@ -181,5 +181,52 @@ def security_update_not_possible_message(checker, latest_allowed_version, confli end end end + + module PullRequestHelpers + extend T::Sig + extend T::Helpers + + sig { returns(Dependabot::Service) } + attr_reader :service + + abstract! + + sig { params(notices: T.nilable(T::Array[Dependabot::Notice])).void } + def record_warning_notices(notices) + return if !notices || notices.empty? + + # Find unique warning notices which are going to be shown on insight page. + warn_notices = unique_warn_notices(notices) + + warn_notices.each do |notice| + # If alert is enabled, sending the deprecation notice to the service for showing on the UI insight page + send_alert_notice(notice) if notice.show_alert + end + rescue StandardError => e + Dependabot.logger.error( + "Failed to send notice warning: #{e.message}" + ) + end + + private + + # Resurns unique warning notices which are going to be shown on insight page. + sig { params(notices: T::Array[Dependabot::Notice]).returns(T::Array[Dependabot::Notice]) } + def unique_warn_notices(notices) + notices + .select { |notice| notice.mode == Dependabot::Notice::NoticeMode::WARN } + .uniq { |notice| [notice.type, notice.package_manager_name] } + end + + sig { params(notice: Dependabot::Notice).void } + def send_alert_notice(notice) + # Sending the notice to the service for showing on the dependabot insight page + service.record_update_job_warning( + warn_type: notice.type, + warn_title: notice.title, + warn_description: notice.description + ) + end + end end end diff --git a/updater/spec/dependabot/api_client_spec.rb b/updater/spec/dependabot/api_client_spec.rb index f40c6229..36779ba6 100644 --- a/updater/spec/dependabot/api_client_spec.rb +++ b/updater/spec/dependabot/api_client_spec.rb @@ -379,6 +379,47 @@ end end + describe "record_update_job_warning" do + let(:record_update_job_warning_url) { "http://example.com/update_jobs/1/record_update_job_warning" } + + let(:warn_type) { "test_warning_type" } + let(:warn_title) { "Test Warning Title" } + let(:warn_description) { "Test Warning Description" } + + before do + stub_request(:post, record_update_job_warning_url) + .to_return(status: 204, headers: headers) + end + + it "hits the correct endpoint" do + client.record_update_job_warning( + warn_type: warn_type, + warn_title: warn_title, + warn_description: warn_description + ) + + expect(WebMock) + .to have_requested(:post, record_update_job_warning_url) + .with(headers: { "Authorization" => "token" }) + end + + it "encodes the payload correctly" do + client.record_update_job_warning( + warn_type: warn_type, + warn_title: warn_title, + warn_description: warn_description + ) + + expect(WebMock).to(have_requested(:post, record_update_job_warning_url).with do |req| + data = JSON.parse(req.body)["data"] + + expect(data["warn-type"]).to eq(warn_type) + expect(data["warn-title"]).to eq(warn_title) + expect(data["warn-description"]).to eq(warn_description) + end) + end + end + describe "mark_job_as_processed" do let(:url) { "http://example.com/update_jobs/1/mark_as_processed" } let(:base_commit) { "sha" } diff --git a/updater/spec/dependabot/dependency_change_spec.rb b/updater/spec/dependabot/dependency_change_spec.rb index 61d6fe0e..4b6acbc3 100644 --- a/updater/spec/dependabot/dependency_change_spec.rb +++ b/updater/spec/dependabot/dependency_change_spec.rb @@ -7,6 +7,7 @@ require "dependabot/pull_request_creator" require "dependabot/dependency_change" require "dependabot/job" +require "dependabot/pull_request" RSpec.describe Dependabot::DependencyChange do subject(:dependency_change) do @@ -114,7 +115,8 @@ dependency_group: nil, pr_message_encoding: nil, pr_message_max_length: 65_535, - ignore_conditions: [] + ignore_conditions: [], + notices: [] ) expect(dependency_change.pr_message.pr_message).to eql("Hello World!") @@ -141,7 +143,8 @@ dependency_group: group, pr_message_encoding: nil, pr_message_max_length: 65_535, - ignore_conditions: [] + ignore_conditions: [], + notices: [] ) expect(dependency_change.pr_message&.pr_message).to eql("Hello World!") @@ -297,11 +300,15 @@ end let(:existing_pull_requests) do [ - updated_dependencies.map do |dep| - { "dependency-name" => dep.name, - "dependency-version" => dep.version, - "directory" => dep.directory } - end + Dependabot::PullRequest.new( + updated_dependencies.map do |dep| + Dependabot::PullRequest::Dependency.new( + name: dep.name, + version: dep.version, + directory: dep.directory + ) + end + ) ] end let(:dependency_change) do @@ -319,10 +326,14 @@ context "when there's no directory in an existing PR that otherwise matches" do let(:existing_pull_requests) do [ - updated_dependencies.map do |dep| - { "dependency-name" => dep.name, - "dependency-version" => dep.version } - end + Dependabot::PullRequest.new( + updated_dependencies.map do |dep| + Dependabot::PullRequest::Dependency.new( + name: dep.name, + version: dep.version + ) + end + ) ] end diff --git a/updater/spec/dependabot/service_spec.rb b/updater/spec/dependabot/service_spec.rb index a0f9572d..2af5f2a5 100644 --- a/updater/spec/dependabot/service_spec.rb +++ b/updater/spec/dependabot/service_spec.rb @@ -23,7 +23,8 @@ update_pull_request: nil, close_pull_request: nil, record_update_job_error: nil, - record_update_job_unknown_error: nil + record_update_job_unknown_error: nil, + record_update_job_warning: nil }) allow(api_client).to receive(:is_a?).with(Dependabot::ApiClient).and_return(true) api_client @@ -305,6 +306,28 @@ end end + describe "#record_update_job_warning" do + let(:warn_type) { :deprecated_dependency } + let(:warn_title) { "Deprecated Dependency Used" } + let(:warn_description) { "The dependency xyz is deprecated and should be updated or removed." } + + before do + service.record_update_job_warning( + warn_type: warn_type, + warn_title: warn_title, + warn_description: warn_description + ) + end + + it "delegates to @client" do + expect(mock_client).to have_received(:record_update_job_warning).with( + warn_type: warn_type, + warn_title: warn_title, + warn_description: warn_description + ) + end + end + describe "#capture_exception" do before do allow(Dependabot::Experiments).to receive(:enabled?).with(:record_update_job_unknown_error).and_return(true) diff --git a/updater/spec/dependabot/updater/operations_spec.rb b/updater/spec/dependabot/updater/operations_spec.rb index 6afb24fe..fb110a28 100644 --- a/updater/spec/dependabot/updater/operations_spec.rb +++ b/updater/spec/dependabot/updater/operations_spec.rb @@ -12,22 +12,6 @@ Dependabot::Experiments.reset! end - it "returns nil if no operation matches" do - # We always expect jobs that update a pull request to specify their - # existing dependency changes, a job with this set of conditions - # should never exist. - source = instance_double(Dependabot::Source, directory: "/.", directories: nil) - job = instance_double(Dependabot::Job, - source: source, - security_updates_only?: false, - updating_a_pull_request?: true, - dependencies: [], - dependency_groups: [], - is_a?: true) - - expect(described_class.class_for(job: job)).to be_nil - end - it "returns the UpdateAllVersions class when the Job is for a fresh, non-security update with no dependencies" do source = instance_double(Dependabot::Source, directory: "/.", directories: nil) job = instance_double(Dependabot::Job, @@ -38,7 +22,7 @@ dependency_groups: [], is_a?: true) - expect(described_class.class_for(job: job)).to be(Dependabot::Updater::Operations::UpdateAllVersions) + expect(described_class.class_for(job: job)).to be(Dependabot::Updater::Operations::GroupUpdateAllVersions) end it "returns the GroupUpdateAllVersions class when the Job is for a fresh, version update with no dependencies" do