[Updater] Remove job_id from the Service/ApiClient's method signatures

updater/lib/dependabot/api_client.rb

@@ -3,18 +3,31 @@
 require "http"
 require "dependabot/job"
 
+# Provides a client to access the internal Dependabot Service's API
+#
+# The Service acts as a relay to Core's GitHub API adapters while providing
+# some co-ordination and enrichment functionality that is only relevant to
+# the integrated service.
+#
+# This API is only available to Dependabot jobs being executed within our
+# hosted infrastructure and is not open to integrators at this time.
+#
 module Dependabot
   class ApiError < StandardError; end
 
   class ApiClient
-    # TODO: instantiate client with job_id?
-    def initialize(base_url, token)
+    def initialize(base_url, job_id, job_token)
       @base_url = base_url
-      @token = token
+      @job_id = job_id
+      @job_token = job_token
     end
 
-    def get_job(job_id)
-      response = fetch_job_details_from_backend(job_id)
+    # TODO: Remove
+    #
+    # We don't seem to use this anymore and always read the job description
+    # from the file system.
+    def fetch_job
+      response = fetch_job_details_from_backend
 
       # If the job has already been accessed then we can safely return quietly.
       # This happens when the backend isn't sure if the updater has enqueued a
@@ -36,10 +49,10 @@ def get_job(job_id)
           :vendor_dependencies, :security_updates_only
         )
 
-      Job.new(job_data.merge(token: token))
+      Job.new(job_data.merge(token: job_token))
     end
 
-    def create_pull_request(job_id, dependencies, updated_dependency_files,
+    def create_pull_request(dependencies, updated_dependency_files,
                             base_commit_sha, pr_message)
       api_url = "#{base_url}/update_jobs/#{job_id}/create_pull_request"
       data = create_pull_request_data(dependencies, updated_dependency_files, base_commit_sha, pr_message)
@@ -53,8 +66,8 @@ def create_pull_request(job_id, dependencies, updated_dependency_files,
       sleep(rand(3.0..10.0)) && retry
     end
 
-    def update_pull_request(job_id, dependencies, updated_dependency_files,
-                            base_commit_sha)
+    # TODO: Determine if we should regenerate the PR message within core for updates
+    def update_pull_request(dependencies, updated_dependency_files, base_commit_sha)
       api_url = "#{base_url}/update_jobs/#{job_id}/update_pull_request"
       body = {
         data: {
@@ -73,7 +86,7 @@ def update_pull_request(job_id, dependencies, updated_dependency_files,
       sleep(rand(3.0..10.0)) && retry
     end
 
-    def close_pull_request(job_id, dependency_name, reason)
+    def close_pull_request(dependency_name, reason)
       api_url = "#{base_url}/update_jobs/#{job_id}/close_pull_request"
       body = { data: { "dependency-names": dependency_name, reason: reason } }
       response = http_client.post(api_url, json: body)
@@ -86,7 +99,7 @@ def close_pull_request(job_id, dependency_name, reason)
       sleep(rand(3.0..10.0)) && retry
     end
 
-    def record_update_job_error(job_id, error_type:, error_details:)
+    def record_update_job_error(error_type:, error_details:)
       api_url = "#{base_url}/update_jobs/#{job_id}/record_update_job_error"
       body = {
         data: {
@@ -104,7 +117,7 @@ def record_update_job_error(job_id, error_type:, error_details:)
       sleep(rand(3.0..10.0)) && retry
     end
 
-    def mark_job_as_processed(job_id, base_commit_sha)
+    def mark_job_as_processed(base_commit_sha)
       api_url = "#{base_url}/update_jobs/#{job_id}/mark_as_processed"
       body = { data: { "base-commit-sha": base_commit_sha } }
       response = http_client.patch(api_url, json: body)
@@ -117,7 +130,7 @@ def mark_job_as_processed(job_id, base_commit_sha)
       sleep(rand(3.0..10.0)) && retry
     end
 
-    def update_dependency_list(job_id, dependencies, dependency_files)
+    def update_dependency_list(dependencies, dependency_files)
       api_url = "#{base_url}/update_jobs/#{job_id}/update_dependency_list"
       body = {
         data: {
@@ -135,7 +148,7 @@ def update_dependency_list(job_id, dependencies, dependency_files)
       sleep(rand(3.0..10.0)) && retry
     end
 
-    def record_package_manager_version(job_id, ecosystem, package_managers)
+    def record_package_manager_version(ecosystem, package_managers)
       api_url = "#{base_url}/update_jobs/#{job_id}/record_package_manager_version"
       body = {
         data: {
@@ -155,10 +168,10 @@ def record_package_manager_version(job_id, ecosystem, package_managers)
 
     private
 
-    attr_reader :token, :base_url
+    attr_reader :base_url, :job_id, :job_token
 
     def http_client
-      client = HTTP.auth(token)
+      client = HTTP.auth(job_token)
       proxy = URI(base_url).find_proxy
       unless proxy.nil?
         args = [proxy.host, proxy.port, proxy.user, proxy.password].compact
@@ -167,7 +180,7 @@ def http_client
       client
     end
 
-    def fetch_job_details_from_backend(job_id)
+    def fetch_job_details_from_backend
       api_url = "#{base_url}/update_jobs/#{job_id}"
       http_client.get(api_url)
     rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError

updater/lib/dependabot/base_command.rb

@@ -51,7 +51,7 @@ def run
       logger_info("Finished job processing")
     rescue StandardError => e
       handle_exception(e)
-      service.mark_job_as_processed(job_id, base_commit_sha)
+      service.mark_job_as_processed(base_commit_sha)
     ensure
       Dependabot.logger.info(service.summary) unless service.noop?
       raise Dependabot::RunFailure if Dependabot::Environment.github_actions? && service.failure?
@@ -63,27 +63,23 @@ def handle_exception(err)
 
       Raven.capture_exception(err, raven_context)
 
-      service.record_update_job_error(
-        job_id,
-        error_type: "unknown_error",
-        error_details: { message: err.message }
-      )
-    end
-
-    def job_id
-      Environment.job_id
+      service.record_update_job_error(error_type: "unknown_error", error_details: { message: err.message })
     end
 
     def api_url
       Environment.api_url
     end
 
-    def token
-      Environment.token
+    def job_id
+      Environment.job_id
+    end
+
+    def job_token
+      Environment.job_token
     end
 
     def api_client
-      @api_client ||= Dependabot::ApiClient.new(api_url, token)
+      @api_client ||= Dependabot::ApiClient.new(api_url, job_id, job_token)
     end
 
     def service

updater/lib/dependabot/environment.rb

@@ -6,8 +6,8 @@ def self.job_id
       @job_id ||= environment_variable("DEPENDABOT_JOB_ID")
     end
 
-    def self.token
-      @token ||= environment_variable("DEPENDABOT_JOB_TOKEN")
+    def self.job_token
+      @job_token ||= environment_variable("DEPENDABOT_JOB_TOKEN")
     end
 
     def self.api_url

updater/lib/dependabot/file_fetcher_command.rb

@@ -22,11 +22,7 @@ def perform_job
         raise "base commit SHA not found" unless @base_commit_sha
 
         version = file_fetcher.package_manager_version
-        unless version.nil?
-          api_client.record_package_manager_version(
-            Dependabot::Environment.job_id, version[:ecosystem], version[:package_managers]
-          )
-        end
+        api_client.record_package_manager_version(version[:ecosystem], version[:package_managers]) unless version.nil?
 
         dependency_files
       rescue StandardError => e
@@ -39,7 +35,7 @@ def perform_job
           logger_error("Error during file fetching; aborting")
         end
         handle_file_fetcher_error(e)
-        service.mark_job_as_processed(job_id, @base_commit_sha)
+        service.mark_job_as_processed(@base_commit_sha)
         return
       end
 
@@ -191,7 +187,6 @@ def rate_limit_error_remaining(error)
 
     def record_error(error_details)
       service.record_update_job_error(
-        job_id,
         error_type: error_details.fetch(:"error-type"),
         error_details: error_details[:"error-detail"]
       )

updater/lib/dependabot/job.rb

@@ -4,6 +4,16 @@
 require "dependabot/source"
 require "wildcard_matcher"
 
+# Describes a single Dependabot workload within the GitHub-integrated Service
+#
+# This primarily acts as a value class to hold inputs for various Core objects
+# and is an approximate data structure for the 'job description file' used by
+# the CLI tool.
+#
+# See: https://github.com/dependabot/cli#job-description-file
+#
+# This class should evenually be promoted to common/lib and augmented to
+# validate job description files.
 module Dependabot
   class Job
     TOP_LEVEL_DEPENDENCY_TYPES = %w(direct production development).freeze

updater/lib/dependabot/service.rb

@@ -17,26 +17,26 @@ def initialize(client:)
       @errors = []
     end
 
-    def_delegators :client, :get_job, :mark_job_as_processed, :update_dependency_list, :record_package_manager_version
+    def_delegators :client, :fetch_job, :mark_job_as_processed, :update_dependency_list, :record_package_manager_version
 
-    def create_pull_request(job_id, dependencies, updated_dependency_files, base_commit_sha, pr_message)
-      client.create_pull_request(job_id, dependencies, updated_dependency_files, base_commit_sha, pr_message)
+    def create_pull_request(dependencies, updated_dependency_files, base_commit_sha, pr_message)
+      client.create_pull_request(dependencies, updated_dependency_files, base_commit_sha, pr_message)
       @pull_requests << [humanize(dependencies), :created]
     end
 
-    def update_pull_request(job_id, dependencies, updated_dependency_files, base_commit_sha)
-      client.update_pull_request(job_id, dependencies, updated_dependency_files, base_commit_sha)
+    def update_pull_request(dependencies, updated_dependency_files, base_commit_sha)
+      client.update_pull_request(dependencies, updated_dependency_files, base_commit_sha)
       @pull_requests << [humanize(dependencies), :updated]
     end
 
-    def close_pull_request(job_id, dependency_name, reason)
-      client.close_pull_request(job_id, dependency_name, reason)
+    def close_pull_request(dependency_name, reason)
+      client.close_pull_request(dependency_name, reason)
       @pull_requests << [dependency_name, "closed: #{reason}"]
     end
 
-    def record_update_job_error(job_id, error_type:, error_details:, dependency: nil)
+    def record_update_job_error(error_type:, error_details:, dependency: nil)
       @errors << [error_type.to_s, dependency]
-      client.record_update_job_error(job_id, error_type: error_type, error_details: error_details)
+      client.record_update_job_error(error_type: error_type, error_details: error_details)
     end
 
     def noop?

updater/lib/dependabot/update_files_command.rb

@@ -16,7 +16,7 @@ def perform_job
         base_commit_sha: base_commit_sha
       ).run
 
-      service.mark_job_as_processed(job_id, base_commit_sha)
+      service.mark_job_as_processed(base_commit_sha)
     end
 
     def job

updater/lib/dependabot/updater.rb

@@ -746,7 +746,6 @@ def create_pull_request(dependencies, updated_dependency_files, pr_message)
                   "pull request for creation")
 
       service.create_pull_request(
-        job_id,
         dependencies,
         updated_dependency_files.map(&:to_h),
         base_commit_sha,
@@ -767,7 +766,6 @@ def update_pull_request(dependencies, updated_dependency_files)
                   "pull request for update")
 
       service.update_pull_request(
-        job_id,
         dependencies,
         updated_dependency_files.map(&:to_h),
         base_commit_sha
@@ -778,7 +776,7 @@ def close_pull_request(reason:)
       reason_string = reason.to_s.tr("_", " ")
       logger_info("Telling backend to close pull request for " \
                   "#{job.dependencies.join(', ')} - #{reason_string}")
-      service.close_pull_request(job_id, job.dependencies, reason)
+      service.close_pull_request(job.dependencies, reason)
     end
 
     # rubocop:disable Metrics/MethodLength
@@ -978,7 +976,6 @@ def pr_message(dependencies, files)
 
     def update_dependency_list(dependencies)
       service.update_dependency_list(
-        job_id,
         dependencies.map do |dep|
           {
             name: dep.name,
@@ -1016,7 +1013,6 @@ def log_prefix
 
     def record_error(error_details, dependency: nil)
       service.record_update_job_error(
-        job_id,
         error_type: error_details.fetch(:"error-type"),
         error_details: error_details[:"error-detail"],
         dependency: dependency