Persist runner IP address on contact (#43489)

app/models/ci/runner.rb

@@ -49,7 +49,7 @@ module Ci
       ref_protected: 1
     }
 
-    cached_attr_reader :version, :revision, :platform, :architecture, :contacted_at
+    cached_attr_reader :version, :revision, :platform, :architecture, :contacted_at, :ip_address
 
     # Searches for runners matching the given query.
     #
@@ -157,7 +157,7 @@ module Ci
     end
 
     def update_cached_info(values)
-      values = values&.slice(:version, :revision, :platform, :architecture) || {}
+      values = values&.slice(:version, :revision, :platform, :architecture, :ip_address) || {}
       values[:contacted_at] = Time.now
 
       cache_attributes(values)

db/migrate/20180222043024_add_ip_address_to_runner.rb

+class AddIpAddressToRunner < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :ci_runners, :ip_address, :string
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180216121030) do
+ActiveRecord::Schema.define(version: 20180222043024) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -437,6 +437,7 @@ ActiveRecord::Schema.define(version: 20180216121030) do
     t.boolean "run_untagged", default: true, null: false
     t.boolean "locked", default: false, null: false
     t.integer "access_level", default: 0, null: false
+    t.string "ip_address"
   end
 
   add_index "ci_runners", ["contacted_at"], name: "index_ci_runners_on_contacted_at", using: :btree

lib/api/helpers/runner.rb

@@ -9,16 +9,22 @@ module API
                                                                   Gitlab::CurrentSettings.runners_registration_token)
       end
 
-      def get_runner_version_from_params
-        return unless params['info'].present?
+      def authenticate_runner!
+        forbidden! unless current_runner
 
-        attributes_for_keys(%w(name version revision platform architecture), params['info'])
+        current_runner
+          .update_cached_info(get_runner_details_from_request)
       end
 
-      def authenticate_runner!
-        forbidden! unless current_runner
+      def get_runner_details_from_request
+        return get_runner_ip unless params['info'].present?
+
+        attributes_for_keys(%w(name version revision platform architecture), params['info'])
+          .merge(get_runner_ip)
+      end
 
-        current_runner.update_cached_info(get_runner_version_from_params)
+      def get_runner_ip
+        { ip_address: request.ip }
       end
 
       def current_runner

lib/api/runner.rb

@@ -16,7 +16,8 @@ module API
         optional :tag_list, type: Array[String], desc: %q(List of Runner's tags)
       end
       post '/' do
-        attributes = attributes_for_keys [:description, :locked, :run_untagged, :tag_list]
+        attributes = attributes_for_keys([:description, :locked, :run_untagged, :tag_list])
+          .merge(get_runner_details_from_request)
 
         runner =
           if runner_registration_token_valid?
@@ -30,7 +31,6 @@ module API
         return forbidden! unless runner
 
         if runner.id
-          runner.update(get_runner_version_from_params)
           present runner, with: Entities::RunnerRegistrationDetails
         else
           not_found!

spec/requests/api/runner_spec.rb

@@ -122,6 +122,15 @@ describe API::Runner do
           end
         end
       end
+
+      it "sets the runner's ip_address" do
+        post api('/runners'),
+          { token: registration_token },
+          { 'REMOTE_ADDR' => '123.111.123.111' }
+
+        expect(response).to have_gitlab_http_status 201
+        expect(Ci::Runner.first.ip_address).to eq('123.111.123.111')
+      end
     end
 
     describe 'DELETE /api/v4/runners' do
@@ -422,6 +431,15 @@ describe API::Runner do
             end
           end
 
+          it "sets the runner's ip_address" do
+            post api('/jobs/request'),
+              { token: runner.token },
+              { 'User-Agent' => user_agent, 'REMOTE_ADDR' => '123.222.123.222' }
+
+            expect(response).to have_gitlab_http_status 201
+            expect(runner.reload.ip_address).to eq('123.222.123.222')
+          end
+
           context 'when concurrently updating a job' do
             before do
               expect_any_instance_of(Ci::Build).to receive(:run!)