From b5a5fdf0e3b97622789db444bf0bf697e78dbb47 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Thu, 22 Feb 2018 17:03:00 +1100 Subject: [PATCH] Persist runner IP address on contact (#43489) --- app/models/ci/runner.rb | 4 ++-- ...20180222043024_add_ip_address_to_runner.rb | 9 +++++++++ db/schema.rb | 3 ++- lib/api/helpers/runner.rb | 20 ++++++++++++------- lib/api/runner.rb | 4 ++-- spec/requests/api/runner_spec.rb | 18 +++++++++++++++++ 6 files changed, 46 insertions(+), 12 deletions(-) create mode 100644 db/migrate/20180222043024_add_ip_address_to_runner.rb diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 13c784bea0d..609620a62bb 100644 --- a/app/models/ci/runner.rb +++ b/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) diff --git a/db/migrate/20180222043024_add_ip_address_to_runner.rb b/db/migrate/20180222043024_add_ip_address_to_runner.rb new file mode 100644 index 00000000000..bf00560b5a8 --- /dev/null +++ b/db/migrate/20180222043024_add_ip_address_to_runner.rb @@ -0,0 +1,9 @@ +class AddIpAddressToRunner < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :ci_runners, :ip_address, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 5bb461169f1..3fb80065ff5 100644 --- a/db/schema.rb +++ b/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 diff --git a/lib/api/helpers/runner.rb b/lib/api/helpers/runner.rb index fbe30192a16..35ac0b4cbca 100644 --- a/lib/api/helpers/runner.rb +++ b/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? - - attributes_for_keys(%w(name version revision platform architecture), params['info']) - end - def authenticate_runner! forbidden! unless current_runner - current_runner.update_cached_info(get_runner_version_from_params) + current_runner + .update_cached_info(get_runner_details_from_request) + end + + 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 + + def get_runner_ip + { ip_address: request.ip } end def current_runner diff --git a/lib/api/runner.rb b/lib/api/runner.rb index 5469cba69a6..91cdc564002 100644 --- a/lib/api/runner.rb +++ b/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! diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index f10b6e43d09..4021e537efc 100644 --- a/spec/requests/api/runner_spec.rb +++ b/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!)