From 26eadca48cc58e53e5e060efe6424f63377c7663 Mon Sep 17 00:00:00 2001 From: Markus Koller Date: Tue, 19 Sep 2017 17:20:49 +0200 Subject: [PATCH 01/61] Upgrade doorkeeper-openid_connect --- Gemfile | 2 +- Gemfile.lock | 14 +++++++------- .../fix-update-doorkeeper-openid-connect.yml | 5 +++++ .../initializers/doorkeeper_openid_connect.rb | 2 +- config/initializers/secret_token.rb | 2 +- spec/initializers/secret_token_spec.rb | 18 +++++++++--------- 6 files changed, 24 insertions(+), 19 deletions(-) create mode 100644 changelogs/unreleased/fix-update-doorkeeper-openid-connect.yml diff --git a/Gemfile b/Gemfile index fa25d8ded33..2bab0757639 100644 --- a/Gemfile +++ b/Gemfile @@ -23,7 +23,7 @@ gem 'faraday', '~> 0.12' # Authentication libraries gem 'devise', '~> 4.2' gem 'doorkeeper', '~> 4.2.0' -gem 'doorkeeper-openid_connect', '~> 1.1.0' +gem 'doorkeeper-openid_connect', '~> 1.2.0' gem 'omniauth', '~> 1.4.2' gem 'omniauth-auth0', '~> 1.4.1' gem 'omniauth-azure-oauth2', '~> 0.0.6' diff --git a/Gemfile.lock b/Gemfile.lock index 90154d98c9c..e02df394ece 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -83,7 +83,7 @@ GEM coderay (>= 1.0.0) erubis (>= 2.6.6) rack (>= 0.9.0) - bindata (2.3.5) + bindata (2.4.1) binding_of_caller (0.7.2) debug_inspector (>= 0.0.1) bootstrap-sass (3.3.6) @@ -167,9 +167,9 @@ GEM docile (1.1.5) domain_name (0.5.20161021) unf (>= 0.0.5, < 1.0.0) - doorkeeper (4.2.0) + doorkeeper (4.2.6) railties (>= 4.2) - doorkeeper-openid_connect (1.1.2) + doorkeeper-openid_connect (1.2.0) doorkeeper (~> 4.0) json-jwt (~> 1.6) dropzonejs-rails (0.7.2) @@ -416,7 +416,7 @@ GEM railties (>= 4.2.0) thor (>= 0.14, < 2.0) json (1.8.6) - json-jwt (1.7.1) + json-jwt (1.7.2) activesupport bindata multi_json (>= 1.3) @@ -486,7 +486,7 @@ GEM minitest (5.7.0) mmap2 (2.2.7) mousetrap-rails (1.4.6) - multi_json (1.12.1) + multi_json (1.12.2) multi_xml (0.6.0) multipart-post (2.0.0) mustermann (1.0.0) @@ -684,7 +684,7 @@ GEM rainbow (2.2.2) rake raindrops (0.18.0) - rake (12.0.0) + rake (12.1.0) rblineprof (0.3.6) debugger-ruby_core_source (~> 1.3) rbnacl (4.0.2) @@ -1000,7 +1000,7 @@ DEPENDENCIES devise-two-factor (~> 3.0.0) diffy (~> 3.1.0) doorkeeper (~> 4.2.0) - doorkeeper-openid_connect (~> 1.1.0) + doorkeeper-openid_connect (~> 1.2.0) dropzonejs-rails (~> 0.7.1) email_reply_trimmer (~> 0.1) email_spec (~> 1.6.0) diff --git a/changelogs/unreleased/fix-update-doorkeeper-openid-connect.yml b/changelogs/unreleased/fix-update-doorkeeper-openid-connect.yml new file mode 100644 index 00000000000..c57fceec92f --- /dev/null +++ b/changelogs/unreleased/fix-update-doorkeeper-openid-connect.yml @@ -0,0 +1,5 @@ +--- +title: Upgrade doorkeeper-openid_connect +merge_request: 14372 +author: Markus Koller +type: other diff --git a/config/initializers/doorkeeper_openid_connect.rb b/config/initializers/doorkeeper_openid_connect.rb index c58f425b19b..af174def047 100644 --- a/config/initializers/doorkeeper_openid_connect.rb +++ b/config/initializers/doorkeeper_openid_connect.rb @@ -1,7 +1,7 @@ Doorkeeper::OpenidConnect.configure do issuer Gitlab.config.gitlab.url - jws_private_key Rails.application.secrets.jws_private_key + signing_key Rails.application.secrets.openid_connect_signing_key resource_owner_from_access_token do |access_token| User.active.find_by(id: access_token.resource_owner_id) diff --git a/config/initializers/secret_token.rb b/config/initializers/secret_token.rb index f9c1d2165d3..750a5b34f3b 100644 --- a/config/initializers/secret_token.rb +++ b/config/initializers/secret_token.rb @@ -25,7 +25,7 @@ def create_tokens secret_key_base: file_secret_key || generate_new_secure_token, otp_key_base: env_secret_key || file_secret_key || generate_new_secure_token, db_key_base: generate_new_secure_token, - jws_private_key: generate_new_rsa_private_key + openid_connect_signing_key: generate_new_rsa_private_key } missing_secrets = set_missing_keys(defaults) diff --git a/spec/initializers/secret_token_spec.rb b/spec/initializers/secret_token_spec.rb index 84ad55e9f98..d56e14e0e0b 100644 --- a/spec/initializers/secret_token_spec.rb +++ b/spec/initializers/secret_token_spec.rb @@ -36,10 +36,10 @@ describe 'create_tokens' do expect(keys).to all(match(HEX_KEY)) end - it 'generates an RSA key for jws_private_key' do + it 'generates an RSA key for openid_connect_signing_key' do create_tokens - keys = secrets.values_at(:jws_private_key) + keys = secrets.values_at(:openid_connect_signing_key) expect(keys.uniq).to eq(keys) expect(keys).to all(match(RSA_KEY)) @@ -49,7 +49,7 @@ describe 'create_tokens' do expect(self).to receive(:warn_missing_secret).with('secret_key_base') expect(self).to receive(:warn_missing_secret).with('otp_key_base') expect(self).to receive(:warn_missing_secret).with('db_key_base') - expect(self).to receive(:warn_missing_secret).with('jws_private_key') + expect(self).to receive(:warn_missing_secret).with('openid_connect_signing_key') create_tokens end @@ -61,7 +61,7 @@ describe 'create_tokens' do expect(new_secrets['secret_key_base']).to eq(secrets.secret_key_base) expect(new_secrets['otp_key_base']).to eq(secrets.otp_key_base) expect(new_secrets['db_key_base']).to eq(secrets.db_key_base) - expect(new_secrets['jws_private_key']).to eq(secrets.jws_private_key) + expect(new_secrets['openid_connect_signing_key']).to eq(secrets.openid_connect_signing_key) end create_tokens @@ -77,7 +77,7 @@ describe 'create_tokens' do context 'when the other secrets all exist' do before do secrets.db_key_base = 'db_key_base' - secrets.jws_private_key = 'jws_private_key' + secrets.openid_connect_signing_key = 'openid_connect_signing_key' allow(File).to receive(:exist?).with('.secret').and_return(true) allow(File).to receive(:read).with('.secret').and_return('file_key') @@ -88,7 +88,7 @@ describe 'create_tokens' do stub_env('SECRET_KEY_BASE', 'env_key') secrets.secret_key_base = 'secret_key_base' secrets.otp_key_base = 'otp_key_base' - secrets.jws_private_key = 'jws_private_key' + secrets.openid_connect_signing_key = 'openid_connect_signing_key' end it 'does not issue a warning' do @@ -114,7 +114,7 @@ describe 'create_tokens' do before do secrets.secret_key_base = 'secret_key_base' secrets.otp_key_base = 'otp_key_base' - secrets.jws_private_key = 'jws_private_key' + secrets.openid_connect_signing_key = 'openid_connect_signing_key' end it 'does not write any files' do @@ -129,7 +129,7 @@ describe 'create_tokens' do expect(secrets.secret_key_base).to eq('secret_key_base') expect(secrets.otp_key_base).to eq('otp_key_base') expect(secrets.db_key_base).to eq('db_key_base') - expect(secrets.jws_private_key).to eq('jws_private_key') + expect(secrets.openid_connect_signing_key).to eq('openid_connect_signing_key') end it 'deletes the .secret file' do @@ -153,7 +153,7 @@ describe 'create_tokens' do expect(new_secrets['secret_key_base']).to eq('file_key') expect(new_secrets['otp_key_base']).to eq('file_key') expect(new_secrets['db_key_base']).to eq('db_key_base') - expect(new_secrets['jws_private_key']).to eq('jws_private_key') + expect(new_secrets['openid_connect_signing_key']).to eq('openid_connect_signing_key') end create_tokens From 1428f38dd62834f90c2ed3dae17aa673d0585e64 Mon Sep 17 00:00:00 2001 From: Mark Fletcher Date: Tue, 3 Oct 2017 13:14:38 +0700 Subject: [PATCH 02/61] Fix project snippets breadcrumb link --- app/views/projects/snippets/show.html.haml | 2 +- .../38696-fix-project-snippets-breadcrumb-link.yml | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/38696-fix-project-snippets-breadcrumb-link.yml diff --git a/app/views/projects/snippets/show.html.haml b/app/views/projects/snippets/show.html.haml index fda068f08c2..7062c5b765e 100644 --- a/app/views/projects/snippets/show.html.haml +++ b/app/views/projects/snippets/show.html.haml @@ -1,5 +1,5 @@ - @content_class = "limit-container-width limited-inner-width-container" unless fluid_layout -- add_to_breadcrumbs "Snippets", dashboard_snippets_path +- add_to_breadcrumbs "Snippets", project_snippets_path(@project) - breadcrumb_title @snippet.to_reference - page_title "#{@snippet.title} (#{@snippet.to_reference})", "Snippets" diff --git a/changelogs/unreleased/38696-fix-project-snippets-breadcrumb-link.yml b/changelogs/unreleased/38696-fix-project-snippets-breadcrumb-link.yml new file mode 100644 index 00000000000..18b1645d7a9 --- /dev/null +++ b/changelogs/unreleased/38696-fix-project-snippets-breadcrumb-link.yml @@ -0,0 +1,5 @@ +--- +title: Fix project snippets breadcrumb link +merge_request: +author: +type: fixed From ebfb5a50757b65530e9649815619ec206513c65e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 4 Oct 2017 13:01:48 +0200 Subject: [PATCH 03/61] Ensure RSpecFlaky doesn't automatically update flaky examples MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, instantiating a RspecFlaky::FlakyExample object would automatically update its first_flaky_at, last_flaky_at and last_flaky_job. That was wrong because we would overwrite every time the suite report with this false data. We now: - Get the suite report and only read from it - Write only the currently detected flaky examples in the report, so that the final report is only updated with flaky examples that were actually detected in each job. Before, job1 could overwrite the legit report from job2! - Write the newly detected flaky examples by rejecting the already tracked flaky specs instead of using another hash. Signed-off-by: Rémy Coutable --- lib/rspec_flaky/flaky_example.rb | 21 +-- lib/rspec_flaky/listener.rb | 61 ++++--- spec/lib/rspec_flaky/flaky_example_spec.rb | 131 ++++++++++----- spec/lib/rspec_flaky/listener_spec.rb | 181 +++++++++++++++------ 4 files changed, 271 insertions(+), 123 deletions(-) diff --git a/lib/rspec_flaky/flaky_example.rb b/lib/rspec_flaky/flaky_example.rb index f81fb90e870..6be24014d89 100644 --- a/lib/rspec_flaky/flaky_example.rb +++ b/lib/rspec_flaky/flaky_example.rb @@ -9,24 +9,21 @@ module RspecFlaky line: example.line, description: example.description, last_attempts_count: example.attempts, - flaky_reports: 1) + flaky_reports: 0) else super end end - def first_flaky_at - self[:first_flaky_at] || Time.now - end + def update_flakiness!(last_attempts_count: nil) + self.first_flaky_at ||= Time.now + self.last_flaky_at = Time.now + self.flaky_reports += 1 + self.last_attempts_count = last_attempts_count if last_attempts_count - def last_flaky_at - Time.now - end - - def last_flaky_job - return unless ENV['CI_PROJECT_URL'] && ENV['CI_JOB_ID'] - - "#{ENV['CI_PROJECT_URL']}/-/jobs/#{ENV['CI_JOB_ID']}" + if ENV['CI_PROJECT_URL'] && ENV['CI_JOB_ID'] + self.last_flaky_job = "#{ENV['CI_PROJECT_URL']}/-/jobs/#{ENV['CI_JOB_ID']}" + end end def to_h diff --git a/lib/rspec_flaky/listener.rb b/lib/rspec_flaky/listener.rb index ec2fbd9e36c..4752ebe6410 100644 --- a/lib/rspec_flaky/listener.rb +++ b/lib/rspec_flaky/listener.rb @@ -2,11 +2,15 @@ require 'json' module RspecFlaky class Listener - attr_reader :all_flaky_examples, :new_flaky_examples + # - suite_flaky_examples: contains all the currently tracked flacky example + # for the whole RSpec suite + # - flaky_examples: contains the examples detected as flaky during the + # current RSpec run + attr_reader :suite_flaky_examples, :flaky_examples - def initialize - @new_flaky_examples = {} - @all_flaky_examples = init_all_flaky_examples + def initialize(suite_flaky_examples_json = nil) + @flaky_examples = {} + @suite_flaky_examples = init_suite_flaky_examples(suite_flaky_examples_json) end def example_passed(notification) @@ -14,24 +18,16 @@ module RspecFlaky return unless current_example.attempts > 1 - flaky_example_hash = all_flaky_examples[current_example.uid] + flaky_example = suite_flaky_examples.fetch(current_example.uid) { FlakyExample.new(current_example) } + flaky_example.update_flakiness!(last_attempts_count: current_example.attempts) - all_flaky_examples[current_example.uid] = - if flaky_example_hash - FlakyExample.new(flaky_example_hash).tap do |ex| - ex.last_attempts_count = current_example.attempts - ex.flaky_reports += 1 - end - else - FlakyExample.new(current_example).tap do |ex| - new_flaky_examples[current_example.uid] = ex - end - end + flaky_examples[current_example.uid] = flaky_example end def dump_summary(_) - write_report_file(all_flaky_examples, all_flaky_examples_report_path) + write_report_file(flaky_examples, flaky_examples_report_path) + new_flaky_examples = _new_flaky_examples if new_flaky_examples.any? Rails.logger.warn "\nNew flaky examples detected:\n" Rails.logger.warn JSON.pretty_generate(to_report(new_flaky_examples)) @@ -46,12 +42,24 @@ module RspecFlaky private - def init_all_flaky_examples - return {} unless File.exist?(all_flaky_examples_report_path) + def init_suite_flaky_examples(suite_flaky_examples_json = nil) + unless suite_flaky_examples_json + return {} unless File.exist?(suite_flaky_examples_report_path) - all_flaky_examples = JSON.parse(File.read(all_flaky_examples_report_path)) + suite_flaky_examples_json = File.read(suite_flaky_examples_report_path) + end - Hash[(all_flaky_examples || {}).map { |k, ex| [k, FlakyExample.new(ex)] }] + suite_flaky_examples = JSON.parse(suite_flaky_examples_json) + + Hash[(suite_flaky_examples || {}).map { |k, ex| [k, FlakyExample.new(ex)] }].freeze + end + + def _new_flaky_examples + flaky_examples.reject { |uid, _| already_flaky?(uid) } + end + + def already_flaky?(example_uid) + suite_flaky_examples.key?(example_uid) end def write_report_file(examples, file_path) @@ -62,9 +70,14 @@ module RspecFlaky File.write(file_path, JSON.pretty_generate(to_report(examples))) end - def all_flaky_examples_report_path - @all_flaky_examples_report_path ||= ENV['ALL_FLAKY_RSPEC_REPORT_PATH'] || - Rails.root.join("rspec_flaky/all-report.json") + def suite_flaky_examples_report_path + @suite_flaky_examples_report_path ||= ENV['SUITE_FLAKY_RSPEC_REPORT_PATH'] || + Rails.root.join("rspec_flaky/suite-report.json") + end + + def flaky_examples_report_path + @flaky_examples_report_path ||= ENV['FLAKY_RSPEC_REPORT_PATH'] || + Rails.root.join("rspec_flaky/report.json") end def new_flaky_examples_report_path diff --git a/spec/lib/rspec_flaky/flaky_example_spec.rb b/spec/lib/rspec_flaky/flaky_example_spec.rb index cbfc1e538ab..d19c34bebb3 100644 --- a/spec/lib/rspec_flaky/flaky_example_spec.rb +++ b/spec/lib/rspec_flaky/flaky_example_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe RspecFlaky::FlakyExample do +describe RspecFlaky::FlakyExample, :aggregate_failures do let(:flaky_example_attrs) do { example_id: 'spec/foo/bar_spec.rb:2', @@ -9,6 +9,7 @@ describe RspecFlaky::FlakyExample do description: 'hello world', first_flaky_at: 1234, last_flaky_at: 2345, + last_flaky_job: 'https://gitlab.com/gitlab-org/gitlab-ce/-/jobs/12', last_attempts_count: 2, flaky_reports: 1 } @@ -27,57 +28,78 @@ describe RspecFlaky::FlakyExample do end let(:example) { double(example_attrs) } + before do + # Stub these env variables otherwise specs don't behave the same on the CI + stub_env('CI_PROJECT_URL', nil) + stub_env('CI_JOB_ID', nil) + end + describe '#initialize' do shared_examples 'a valid FlakyExample instance' do - it 'returns valid attributes' do - flaky_example = described_class.new(args) + let(:flaky_example) { described_class.new(args) } + it 'returns valid attributes' do expect(flaky_example.uid).to eq(flaky_example_attrs[:uid]) - expect(flaky_example.example_id).to eq(flaky_example_attrs[:example_id]) + expect(flaky_example.file).to eq(flaky_example_attrs[:file]) + expect(flaky_example.line).to eq(flaky_example_attrs[:line]) + expect(flaky_example.description).to eq(flaky_example_attrs[:description]) + expect(flaky_example.first_flaky_at).to eq(expected_first_flaky_at) + expect(flaky_example.last_flaky_at).to eq(expected_last_flaky_at) + expect(flaky_example.last_attempts_count).to eq(flaky_example_attrs[:last_attempts_count]) + expect(flaky_example.flaky_reports).to eq(expected_flaky_reports) end end context 'when given an Rspec::Example' do - let(:args) { example } - - it_behaves_like 'a valid FlakyExample instance' + it_behaves_like 'a valid FlakyExample instance' do + let(:args) { example } + let(:expected_first_flaky_at) { nil } + let(:expected_last_flaky_at) { nil } + let(:expected_flaky_reports) { 0 } + end end context 'when given a hash' do - let(:args) { flaky_example_attrs } - - it_behaves_like 'a valid FlakyExample instance' + it_behaves_like 'a valid FlakyExample instance' do + let(:args) { flaky_example_attrs } + let(:expected_flaky_reports) { flaky_example_attrs[:flaky_reports] } + let(:expected_first_flaky_at) { flaky_example_attrs[:first_flaky_at] } + let(:expected_last_flaky_at) { flaky_example_attrs[:last_flaky_at] } + end end end - describe '#to_h' do - before do - # Stub these env variables otherwise specs don't behave the same on the CI - stub_env('CI_PROJECT_URL', nil) - stub_env('CI_JOB_ID', nil) - end + describe '#update_flakiness!' do + shared_examples 'an up-to-date FlakyExample instance' do + let(:flaky_example) { described_class.new(args) } - shared_examples 'a valid FlakyExample hash' do - let(:additional_attrs) { {} } + it 'updates the first_flaky_at' do + now = Time.now + expected_first_flaky_at = flaky_example.first_flaky_at ? flaky_example.first_flaky_at : now + Timecop.freeze(now) { flaky_example.update_flakiness! } - it 'returns a valid hash' do - flaky_example = described_class.new(args) - final_hash = flaky_example_attrs - .merge(last_flaky_at: instance_of(Time), last_flaky_job: nil) - .merge(additional_attrs) - - expect(flaky_example.to_h).to match(hash_including(final_hash)) + expect(flaky_example.first_flaky_at).to eq(expected_first_flaky_at) end - end - context 'when given an Rspec::Example' do - let(:args) { example } + it 'updates the last_flaky_at' do + now = Time.now + Timecop.freeze(now) { flaky_example.update_flakiness! } - context 'when run locally' do - it_behaves_like 'a valid FlakyExample hash' do - let(:additional_attrs) do - { first_flaky_at: instance_of(Time) } - end + expect(flaky_example.last_flaky_at).to eq(now) + end + + it 'updates the flaky_reports' do + expected_flaky_reports = flaky_example.first_flaky_at ? flaky_example.flaky_reports + 1 : 1 + + expect { flaky_example.update_flakiness! }.to change { flaky_example.flaky_reports }.by(1) + expect(flaky_example.flaky_reports).to eq(expected_flaky_reports) + end + + context 'when passed a :last_attempts_count' do + it 'updates the last_attempts_count' do + flaky_example.update_flakiness!(last_attempts_count: 42) + + expect(flaky_example.last_attempts_count).to eq(42) end end @@ -87,10 +109,45 @@ describe RspecFlaky::FlakyExample do stub_env('CI_JOB_ID', 42) end - it_behaves_like 'a valid FlakyExample hash' do - let(:additional_attrs) do - { first_flaky_at: instance_of(Time), last_flaky_job: "https://gitlab.com/gitlab-org/gitlab-ce/-/jobs/42" } - end + it 'updates the last_flaky_job' do + flaky_example.update_flakiness! + + expect(flaky_example.last_flaky_job).to eq('https://gitlab.com/gitlab-org/gitlab-ce/-/jobs/42') + end + end + end + + context 'when given an Rspec::Example' do + it_behaves_like 'an up-to-date FlakyExample instance' do + let(:args) { example } + end + end + + context 'when given a hash' do + it_behaves_like 'an up-to-date FlakyExample instance' do + let(:args) { flaky_example_attrs } + end + end + end + + describe '#to_h' do + shared_examples 'a valid FlakyExample hash' do + let(:additional_attrs) { {} } + + it 'returns a valid hash' do + flaky_example = described_class.new(args) + final_hash = flaky_example_attrs.merge(additional_attrs) + + expect(flaky_example.to_h).to eq(final_hash) + end + end + + context 'when given an Rspec::Example' do + let(:args) { example } + + it_behaves_like 'a valid FlakyExample hash' do + let(:additional_attrs) do + { first_flaky_at: nil, last_flaky_at: nil, last_flaky_job: nil, flaky_reports: 0 } end end end diff --git a/spec/lib/rspec_flaky/listener_spec.rb b/spec/lib/rspec_flaky/listener_spec.rb index 0e193bf408b..5d04d43c66f 100644 --- a/spec/lib/rspec_flaky/listener_spec.rb +++ b/spec/lib/rspec_flaky/listener_spec.rb @@ -1,22 +1,35 @@ require 'spec_helper' -describe RspecFlaky::Listener do - let(:flaky_example_report) do +describe RspecFlaky::Listener, :aggregate_failures do + let(:already_flaky_example_uid) { '6e869794f4cfd2badd93eb68719371d1' } + let(:suite_flaky_example_report) do { - 'abc123' => { + already_flaky_example_uid => { example_id: 'spec/foo/bar_spec.rb:2', file: 'spec/foo/bar_spec.rb', line: 2, description: 'hello world', first_flaky_at: 1234, - last_flaky_at: instance_of(Time), - last_attempts_count: 2, + last_flaky_at: 4321, + last_attempts_count: 3, flaky_reports: 1, last_flaky_job: nil } } end - let(:example_attrs) do + let(:already_flaky_example_attrs) do + { + id: 'spec/foo/bar_spec.rb:2', + metadata: { + file_path: 'spec/foo/bar_spec.rb', + line_number: 2, + full_description: 'hello world' + }, + execution_result: double(status: 'passed', exception: nil) + } + end + let(:already_flaky_example) { RspecFlaky::FlakyExample.new(suite_flaky_example_report[already_flaky_example_uid]) } + let(:new_example_attrs) do { id: 'spec/foo/baz_spec.rb:3', metadata: { @@ -36,14 +49,15 @@ describe RspecFlaky::Listener do describe '#initialize' do shared_examples 'a valid Listener instance' do - let(:expected_all_flaky_examples) { {} } + let(:expected_suite_flaky_examples) { {} } it 'returns a valid Listener instance' do listener = described_class.new - expect(listener.to_report(listener.all_flaky_examples)) - .to match(hash_including(expected_all_flaky_examples)) - expect(listener.new_flaky_examples).to eq({}) + expect(listener.to_report(listener.suite_flaky_examples)) + .to eq(expected_suite_flaky_examples) + expect(listener.__send__(:_new_flaky_examples)).to eq({}) + expect(listener.flaky_examples).to eq({}) end end @@ -51,16 +65,16 @@ describe RspecFlaky::Listener do it_behaves_like 'a valid Listener instance' end - context 'when a report file exists and set by ALL_FLAKY_RSPEC_REPORT_PATH' do + context 'when a report file exists and set by SUITE_FLAKY_RSPEC_REPORT_PATH' do let(:report_file) do Tempfile.new(%w[rspec_flaky_report .json]).tap do |f| - f.write(JSON.pretty_generate(flaky_example_report)) + f.write(JSON.pretty_generate(suite_flaky_example_report)) f.rewind end end before do - stub_env('ALL_FLAKY_RSPEC_REPORT_PATH', report_file.path) + stub_env('SUITE_FLAKY_RSPEC_REPORT_PATH', report_file.path) end after do @@ -69,19 +83,75 @@ describe RspecFlaky::Listener do end it_behaves_like 'a valid Listener instance' do - let(:expected_all_flaky_examples) { flaky_example_report } + let(:expected_suite_flaky_examples) { suite_flaky_example_report } end end end describe '#example_passed' do - let(:rspec_example) { double(example_attrs) } + let(:rspec_example) { double(new_example_attrs) } let(:notification) { double(example: rspec_example) } + let(:listener) { described_class.new(suite_flaky_example_report.to_json) } shared_examples 'a non-flaky example' do it 'does not change the flaky examples hash' do - expect { subject.example_passed(notification) } - .not_to change { subject.all_flaky_examples } + expect { listener.example_passed(notification) } + .not_to change { listener.flaky_examples } + end + end + + shared_examples 'an existing flaky example' do + let(:expected_flaky_example) do + { + example_id: 'spec/foo/bar_spec.rb:2', + file: 'spec/foo/bar_spec.rb', + line: 2, + description: 'hello world', + first_flaky_at: 1234, + last_attempts_count: 2, + flaky_reports: 2, + last_flaky_job: nil + } + end + + it 'changes the flaky examples hash' do + new_example = RspecFlaky::Example.new(rspec_example) + + now = Time.now + Timecop.freeze(now) do + expect { listener.example_passed(notification) } + .to change { listener.flaky_examples[new_example.uid].to_h } + end + + expect(listener.flaky_examples[new_example.uid].to_h) + .to eq(expected_flaky_example.merge(last_flaky_at: now)) + end + end + + shared_examples 'a new flaky example' do + let(:expected_flaky_example) do + { + example_id: 'spec/foo/baz_spec.rb:3', + file: 'spec/foo/baz_spec.rb', + line: 3, + description: 'hello GitLab', + last_attempts_count: 2, + flaky_reports: 1, + last_flaky_job: nil + } + end + + it 'changes the all flaky examples hash' do + new_example = RspecFlaky::Example.new(rspec_example) + + now = Time.now + Timecop.freeze(now) do + expect { listener.example_passed(notification) } + .to change { listener.flaky_examples[new_example.uid].to_h } + end + + expect(listener.flaky_examples[new_example.uid].to_h) + .to eq(expected_flaky_example.merge(first_flaky_at: now, last_flaky_at: now)) end end @@ -90,53 +160,45 @@ describe RspecFlaky::Listener do end describe 'when the RSpec example has 1 attempt' do - let(:rspec_example) { double(example_attrs.merge(attempts: 1)) } + let(:rspec_example) { double(new_example_attrs.merge(attempts: 1)) } it_behaves_like 'a non-flaky example' end describe 'when the RSpec example has 2 attempts' do - let(:rspec_example) { double(example_attrs.merge(attempts: 2)) } - let(:expected_new_flaky_example) do - { - example_id: 'spec/foo/baz_spec.rb:3', - file: 'spec/foo/baz_spec.rb', - line: 3, - description: 'hello GitLab', - first_flaky_at: instance_of(Time), - last_flaky_at: instance_of(Time), - last_attempts_count: 2, - flaky_reports: 1, - last_flaky_job: nil - } - end + let(:rspec_example) { double(new_example_attrs.merge(attempts: 2)) } - it 'does not change the flaky examples hash' do - expect { subject.example_passed(notification) } - .to change { subject.all_flaky_examples } + it_behaves_like 'a new flaky example' - new_example = RspecFlaky::Example.new(rspec_example) + context 'with an existing flaky example' do + let(:rspec_example) { double(already_flaky_example_attrs.merge(attempts: 2)) } - expect(subject.all_flaky_examples[new_example.uid].to_h) - .to match(hash_including(expected_new_flaky_example)) + it_behaves_like 'an existing flaky example' end end end describe '#dump_summary' do - let(:rspec_example) { double(example_attrs) } - let(:notification) { double(example: rspec_example) } + let(:listener) { described_class.new(suite_flaky_example_report.to_json) } + let(:new_flaky_rspec_example) { double(new_example_attrs.merge(attempts: 2)) } + let(:already_flaky_rspec_example) { double(already_flaky_example_attrs.merge(attempts: 2)) } + let(:notification_new_flaky_rspec_example) { double(example: new_flaky_rspec_example) } + let(:notification_already_flaky_rspec_example) { double(example: already_flaky_rspec_example) } - context 'when a report file path is set by ALL_FLAKY_RSPEC_REPORT_PATH' do + context 'when a report file path is set by FLAKY_RSPEC_REPORT_PATH' do let(:report_file_path) { Rails.root.join('tmp', 'rspec_flaky_report.json') } + let(:new_report_file_path) { Rails.root.join('tmp', 'rspec_flaky_new_report.json') } before do - stub_env('ALL_FLAKY_RSPEC_REPORT_PATH', report_file_path) + stub_env('FLAKY_RSPEC_REPORT_PATH', report_file_path) + stub_env('NEW_FLAKY_RSPEC_REPORT_PATH', new_report_file_path) FileUtils.rm(report_file_path) if File.exist?(report_file_path) + FileUtils.rm(new_report_file_path) if File.exist?(new_report_file_path) end after do FileUtils.rm(report_file_path) if File.exist?(report_file_path) + FileUtils.rm(new_report_file_path) if File.exist?(new_report_file_path) end context 'when FLAKY_RSPEC_GENERATE_REPORT == "false"' do @@ -144,12 +206,13 @@ describe RspecFlaky::Listener do stub_env('FLAKY_RSPEC_GENERATE_REPORT', 'false') end - it 'does not write the report file' do - subject.example_passed(notification) + it 'does not write any report file' do + listener.example_passed(notification_new_flaky_rspec_example) - subject.dump_summary(nil) + listener.dump_summary(nil) expect(File.exist?(report_file_path)).to be(false) + expect(File.exist?(new_report_file_path)).to be(false) end end @@ -158,21 +221,39 @@ describe RspecFlaky::Listener do stub_env('FLAKY_RSPEC_GENERATE_REPORT', 'true') end - it 'writes the report file' do - subject.example_passed(notification) + around do |example| + Timecop.freeze { example.run } + end - subject.dump_summary(nil) + it 'writes the report files' do + listener.example_passed(notification_new_flaky_rspec_example) + listener.example_passed(notification_already_flaky_rspec_example) + + listener.dump_summary(nil) expect(File.exist?(report_file_path)).to be(true) + expect(File.exist?(new_report_file_path)).to be(true) + + expect(File.read(report_file_path)) + .to eq(JSON.pretty_generate(listener.to_report(listener.flaky_examples))) + + new_example = RspecFlaky::Example.new(notification_new_flaky_rspec_example) + new_flaky_example = RspecFlaky::FlakyExample.new(new_example) + new_flaky_example.update_flakiness! + + expect(File.read(new_report_file_path)) + .to eq(JSON.pretty_generate(listener.to_report(new_example.uid => new_flaky_example))) end end end end describe '#to_report' do + let(:listener) { described_class.new(suite_flaky_example_report.to_json) } + it 'transforms the internal hash to a JSON-ready hash' do - expect(subject.to_report('abc123' => RspecFlaky::FlakyExample.new(flaky_example_report['abc123']))) - .to match(hash_including(flaky_example_report)) + expect(listener.to_report(already_flaky_example_uid => already_flaky_example)) + .to match(hash_including(suite_flaky_example_report)) end end end From f286d0978483c912e66257fec299a117b81025c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 4 Oct 2017 13:24:51 +0200 Subject: [PATCH 04/61] Improve the flaky examples detection jobs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- .gitlab-ci.yml | 20 ++++++++++---------- spec/spec_helper.rb | 5 ++++- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 8501911fde4..fb170515c1d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -27,7 +27,7 @@ variables: GET_SOURCES_ATTEMPTS: "3" KNAPSACK_RSPEC_SUITE_REPORT_PATH: knapsack/${CI_PROJECT_NAME}/rspec_report-master.json KNAPSACK_SPINACH_SUITE_REPORT_PATH: knapsack/${CI_PROJECT_NAME}/spinach_report-master.json - FLAKY_RSPEC_SUITE_REPORT_PATH: rspec_flaky/${CI_PROJECT_NAME}/report-master.json + FLAKY_RSPEC_SUITE_REPORT_PATH: rspec_flaky/report-suite.json before_script: - bundle --version @@ -87,12 +87,13 @@ stages: - export CI_NODE_TOTAL=${JOB_NAME[-1]} - export KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json - export KNAPSACK_GENERATE_REPORT=true - - export ALL_FLAKY_RSPEC_REPORT_PATH=rspec_flaky/${CI_PROJECT_NAME}/all_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json - - export NEW_FLAKY_RSPEC_REPORT_PATH=rspec_flaky/${CI_PROJECT_NAME}/new_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json + - export SUITE_FLAKY_RSPEC_REPORT_PATH=${FLAKY_RSPEC_SUITE_REPORT_PATH} + - export FLAKY_RSPEC_REPORT_PATH=rspec_flaky/all_${JOB_NAME[0]}_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json + - export NEW_FLAKY_RSPEC_REPORT_PATH=rspec_flaky/new_${JOB_NAME[0]}_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json - export FLAKY_RSPEC_GENERATE_REPORT=true - export CACHE_CLASSES=true - cp ${KNAPSACK_RSPEC_SUITE_REPORT_PATH} ${KNAPSACK_REPORT_PATH} - - cp ${FLAKY_RSPEC_SUITE_REPORT_PATH} ${ALL_FLAKY_RSPEC_REPORT_PATH} + - '[[ -f $FLAKY_RSPEC_REPORT_PATH ]] || echo "{}" > ${FLAKY_RSPEC_REPORT_PATH}' - '[[ -f $NEW_FLAKY_RSPEC_REPORT_PATH ]] || echo "{}" > ${NEW_FLAKY_RSPEC_REPORT_PATH}' - scripts/gitaly-test-spawn - knapsack rspec "--color --format documentation" @@ -233,7 +234,7 @@ retrieve-tests-metadata: - wget -O $KNAPSACK_SPINACH_SUITE_REPORT_PATH http://${TESTS_METADATA_S3_BUCKET}.s3.amazonaws.com/$KNAPSACK_SPINACH_SUITE_REPORT_PATH || rm $KNAPSACK_SPINACH_SUITE_REPORT_PATH - '[[ -f $KNAPSACK_RSPEC_SUITE_REPORT_PATH ]] || echo "{}" > ${KNAPSACK_RSPEC_SUITE_REPORT_PATH}' - '[[ -f $KNAPSACK_SPINACH_SUITE_REPORT_PATH ]] || echo "{}" > ${KNAPSACK_SPINACH_SUITE_REPORT_PATH}' - - mkdir -p rspec_flaky/${CI_PROJECT_NAME}/ + - mkdir -p rspec_flaky/ - wget -O $FLAKY_RSPEC_SUITE_REPORT_PATH http://${TESTS_METADATA_S3_BUCKET}.s3.amazonaws.com/$FLAKY_RSPEC_SUITE_REPORT_PATH || rm $FLAKY_RSPEC_SUITE_REPORT_PATH - '[[ -f $FLAKY_RSPEC_SUITE_REPORT_PATH ]] || echo "{}" > ${FLAKY_RSPEC_SUITE_REPORT_PATH}' @@ -252,22 +253,21 @@ update-tests-metadata: - retry gem install fog-aws mime-types - scripts/merge-reports ${KNAPSACK_RSPEC_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/rspec-pg_node_*.json - scripts/merge-reports ${KNAPSACK_SPINACH_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/spinach-pg_node_*.json - - scripts/merge-reports ${FLAKY_RSPEC_SUITE_REPORT_PATH} rspec_flaky/${CI_PROJECT_NAME}/all_node_*.json + - scripts/merge-reports ${FLAKY_RSPEC_SUITE_REPORT_PATH} rspec_flaky/all_*_*.json - '[[ -z ${TESTS_METADATA_S3_BUCKET} ]] || scripts/sync-reports put $TESTS_METADATA_S3_BUCKET $KNAPSACK_RSPEC_SUITE_REPORT_PATH $KNAPSACK_SPINACH_SUITE_REPORT_PATH' - '[[ -z ${TESTS_METADATA_S3_BUCKET} ]] || scripts/sync-reports put $TESTS_METADATA_S3_BUCKET $FLAKY_RSPEC_SUITE_REPORT_PATH' - rm -f knapsack/${CI_PROJECT_NAME}/*_node_*.json - - rm -f rspec_flaky/${CI_PROJECT_NAME}/*_node_*.json + - rm -f rspec_flaky/all_*.json rspec_flaky/new_*.json flaky-examples-check: <<: *dedicated-runner image: ruby:2.3-alpine services: [] before_script: [] - cache: {} variables: SETUP_DB: "false" USE_BUNDLE_INSTALL: "false" - NEW_FLAKY_SPECS_REPORT: rspec_flaky/${CI_PROJECT_NAME}/new_rspec_flaky_examples.json + NEW_FLAKY_SPECS_REPORT: rspec_flaky/report-new.json stage: post-test allow_failure: yes only: @@ -281,7 +281,7 @@ flaky-examples-check: - rspec_flaky/ script: - '[[ -f $NEW_FLAKY_SPECS_REPORT ]] || echo "{}" > ${NEW_FLAKY_SPECS_REPORT}' - - scripts/merge-reports $NEW_FLAKY_SPECS_REPORT rspec_flaky/${CI_PROJECT_NAME}/new_node_*.json + - scripts/merge-reports ${NEW_FLAKY_SPECS_REPORT} rspec_flaky/new_*_*.json - scripts/detect-new-flaky-examples $NEW_FLAKY_SPECS_REPORT setup-test-env: diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 576e4ae1d38..48cacba6a8a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -81,7 +81,10 @@ RSpec.configure do |config| if ENV['CI'] # This includes the first try, i.e. tests will be run 4 times before failing. config.default_retry_count = 4 - config.reporter.register_listener(RspecFlaky::Listener.new, :example_passed, :dump_summary) + config.reporter.register_listener( + RspecFlaky::Listener.new, + :example_passed, + :dump_summary) end config.before(:suite) do From 97153e7f62023f304ba76f713e610e635d107aff Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 5 Oct 2017 00:17:54 -0500 Subject: [PATCH 05/61] fix formatting --- app/assets/javascripts/monitoring/components/graph.vue | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/monitoring/components/graph.vue b/app/assets/javascripts/monitoring/components/graph.vue index 6b3e341f936..523cf7c3dda 100644 --- a/app/assets/javascripts/monitoring/components/graph.vue +++ b/app/assets/javascripts/monitoring/components/graph.vue @@ -141,10 +141,12 @@ }, renderAxesPaths() { - this.timeSeries = createTimeSeries(this.graphData.queries[0], - this.graphWidth, - this.graphHeight, - this.graphHeightOffset); + this.timeSeries = createTimeSeries( + this.graphData.queries[0], + this.graphWidth, + this.graphHeight, + this.graphHeightOffset + ); if (this.timeSeries.length > 3) { this.baseGraphHeight = this.baseGraphHeight += (this.timeSeries.length - 3) * 20; From eb31e31478aab52d26df7cb97f075deccd8dd9dc Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 5 Oct 2017 00:37:09 -0500 Subject: [PATCH 06/61] fix spelling ("outter" -> "outer") --- app/assets/javascripts/monitoring/components/graph.vue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/monitoring/components/graph.vue b/app/assets/javascripts/monitoring/components/graph.vue index 523cf7c3dda..2124fc3e8af 100644 --- a/app/assets/javascripts/monitoring/components/graph.vue +++ b/app/assets/javascripts/monitoring/components/graph.vue @@ -65,7 +65,7 @@ }, computed: { - outterViewBox() { + outerViewBox() { return `0 0 ${this.baseGraphWidth} ${this.baseGraphHeight}`; }, @@ -213,7 +213,7 @@ class="prometheus-svg-container" :style="paddingBottomRootSvg"> Date: Thu, 5 Oct 2017 01:07:01 -0500 Subject: [PATCH 07/61] rename graph_paths.vue consistent with all other sub-components of graph.vue --- app/assets/javascripts/monitoring/components/graph.vue | 2 +- .../monitoring/components/{graph_path.vue => graph/path.vue} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename app/assets/javascripts/monitoring/components/{graph_path.vue => graph/path.vue} (100%) diff --git a/app/assets/javascripts/monitoring/components/graph.vue b/app/assets/javascripts/monitoring/components/graph.vue index 2124fc3e8af..eb1eb6f1143 100644 --- a/app/assets/javascripts/monitoring/components/graph.vue +++ b/app/assets/javascripts/monitoring/components/graph.vue @@ -3,7 +3,7 @@ import GraphLegend from './graph/legend.vue'; import GraphFlag from './graph/flag.vue'; import GraphDeployment from './graph/deployment.vue'; - import GraphPath from './graph_path.vue'; + import GraphPath from './graph/path.vue'; import MonitoringMixin from '../mixins/monitoring_mixins'; import eventHub from '../event_hub'; import measurements from '../utils/measurements'; diff --git a/app/assets/javascripts/monitoring/components/graph_path.vue b/app/assets/javascripts/monitoring/components/graph/path.vue similarity index 100% rename from app/assets/javascripts/monitoring/components/graph_path.vue rename to app/assets/javascripts/monitoring/components/graph/path.vue From b0e8d623b79f2a010616c51303c5db1cb25870e1 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 5 Oct 2017 01:08:05 -0500 Subject: [PATCH 08/61] fix x-axis pixel scale to account for transform x-offset --- app/assets/javascripts/monitoring/components/graph.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/monitoring/components/graph.vue b/app/assets/javascripts/monitoring/components/graph.vue index eb1eb6f1143..f1792749936 100644 --- a/app/assets/javascripts/monitoring/components/graph.vue +++ b/app/assets/javascripts/monitoring/components/graph.vue @@ -153,7 +153,7 @@ } const axisXScale = d3.time.scale() - .range([0, this.graphWidth]); + .range([0, this.graphWidth - 70]); const axisYScale = d3.scale.linear() .range([this.graphHeight - this.graphHeightOffset, 0]); From 945c198c532c584e223404ab2fcfdf25127f0405 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 5 Oct 2017 01:08:19 -0500 Subject: [PATCH 09/61] display x-axis tick marks --- app/assets/stylesheets/pages/environments.scss | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index 9362d80d4e6..1e09141665f 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -288,8 +288,13 @@ fill: $black; } - .tick > text { - font-size: 12px; + .tick { + > line { + stroke: $gray-darker; + } + > text { + font-size: 12px; + } } .text-metric-title { From 228ecf3646812f9f7d2811493382da9ad2a25225 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Thu, 5 Oct 2017 01:17:56 -0500 Subject: [PATCH 10/61] add CHANGELOG.md entry for !14258 --- ...ing-graph-axes-labels-are-inaccurate-and-inconsistent.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/37105-monitoring-graph-axes-labels-are-inaccurate-and-inconsistent.yml diff --git a/changelogs/unreleased/37105-monitoring-graph-axes-labels-are-inaccurate-and-inconsistent.yml b/changelogs/unreleased/37105-monitoring-graph-axes-labels-are-inaccurate-and-inconsistent.yml new file mode 100644 index 00000000000..3364b1d46b3 --- /dev/null +++ b/changelogs/unreleased/37105-monitoring-graph-axes-labels-are-inaccurate-and-inconsistent.yml @@ -0,0 +1,5 @@ +--- +title: Fix incorrect X-axis labels in Prometheus graphs +merge_request: 14258 +author: +type: fixed From 1ca4f282a493af89c084038c67feca51f1ec5e23 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 28 Sep 2017 12:34:00 +0200 Subject: [PATCH 11/61] Add specs for methods that count pipeline seeds size --- spec/lib/gitlab/ci/stage/seed_spec.rb | 6 ++++++ spec/models/ci/pipeline_spec.rb | 10 +++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/spec/lib/gitlab/ci/stage/seed_spec.rb b/spec/lib/gitlab/ci/stage/seed_spec.rb index 9ecd128faca..3fe8d50c49a 100644 --- a/spec/lib/gitlab/ci/stage/seed_spec.rb +++ b/spec/lib/gitlab/ci/stage/seed_spec.rb @@ -11,6 +11,12 @@ describe Gitlab::Ci::Stage::Seed do described_class.new(pipeline, 'test', builds) end + describe '#size' do + it 'returns a number of jobs in the stage' do + expect(subject.size).to eq 2 + end + end + describe '#stage' do it 'returns hash attributes of a stage' do expect(subject.stage).to be_a Hash diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 9c1e460ab20..2c9e7013b77 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -238,7 +238,7 @@ describe Ci::Pipeline, :mailer do describe '#stage_seeds' do let(:pipeline) do - create(:ci_pipeline, config: { rspec: { script: 'rake' } }) + build(:ci_pipeline, config: { rspec: { script: 'rake' } }) end it 'returns preseeded stage seeds object' do @@ -247,6 +247,14 @@ describe Ci::Pipeline, :mailer do end end + describe '#seeds_size' do + let(:pipeline) { build(:ci_pipeline_with_one_job) } + + it 'returns number of jobs in stage seeds' do + expect(pipeline.seeds_size).to eq 1 + end + end + describe '#legacy_stages' do subject { pipeline.legacy_stages } From d3762c22473867d8543b0bb91fd0c696c7aa65f9 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 28 Sep 2017 12:28:26 +0200 Subject: [PATCH 12/61] Implement pipeline seeds size methods --- app/models/ci/pipeline.rb | 4 ++++ lib/gitlab/ci/stage/seed.rb | 2 ++ 2 files changed, 6 insertions(+) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 3d5acc00f8f..323c2c4ce7c 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -312,6 +312,10 @@ module Ci @stage_seeds ||= config_processor.stage_seeds(self) end + def seeds_size + @seeds_size ||= stage_seeds.sum(&:size) + end + def has_kubernetes_active? project.kubernetes_service&.active? end diff --git a/lib/gitlab/ci/stage/seed.rb b/lib/gitlab/ci/stage/seed.rb index e19aae35a81..bc97aa63b02 100644 --- a/lib/gitlab/ci/stage/seed.rb +++ b/lib/gitlab/ci/stage/seed.rb @@ -3,7 +3,9 @@ module Gitlab module Stage class Seed attr_reader :pipeline + delegate :project, to: :pipeline + delegate :size, to: :@jobs def initialize(pipeline, stage, jobs) @pipeline = pipeline From f3ec51beba3f1af68d3238489566bb0bbce869a4 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 28 Sep 2017 12:57:16 +0200 Subject: [PATCH 13/61] Add a additional scope related to alive CI/CD statuses --- app/models/concerns/has_status.rb | 1 + spec/models/concerns/has_status_spec.rb | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb index 3803e18a96e..7c3ed96bc28 100644 --- a/app/models/concerns/has_status.rb +++ b/app/models/concerns/has_status.rb @@ -81,6 +81,7 @@ module HasStatus scope :canceled, -> { where(status: 'canceled') } scope :skipped, -> { where(status: 'skipped') } scope :manual, -> { where(status: 'manual') } + scope :alive, -> { where(status: [:created, :pending, :running]) } scope :created_or_pending, -> { where(status: [:created, :pending]) } scope :running_or_pending, -> { where(status: [:running, :pending]) } scope :finished, -> { where(status: [:success, :failed, :canceled]) } diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index a38f2553eb1..6866b43432c 100644 --- a/spec/models/concerns/has_status_spec.rb +++ b/spec/models/concerns/has_status_spec.rb @@ -231,6 +231,18 @@ describe HasStatus do end end + describe '.alive' do + subject { CommitStatus.alive } + + %i[running pending created].each do |status| + it_behaves_like 'containing the job', status + end + + %i[failed success].each do |status| + it_behaves_like 'not containing the job', status + end + end + describe '.created_or_pending' do subject { CommitStatus.created_or_pending } From 8912e99e5f65f148db97688b4d0c99126da222be Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 29 Sep 2017 10:05:11 +0200 Subject: [PATCH 14/61] Add failure_reason column to ci_pipelines Conflicts: db/schema.rb --- .../20170929080234_add_failure_reason_to_pipelines.rb | 9 +++++++++ db/schema.rb | 1 + 2 files changed, 10 insertions(+) create mode 100644 db/migrate/20170929080234_add_failure_reason_to_pipelines.rb diff --git a/db/migrate/20170929080234_add_failure_reason_to_pipelines.rb b/db/migrate/20170929080234_add_failure_reason_to_pipelines.rb new file mode 100644 index 00000000000..82adddbc1ec --- /dev/null +++ b/db/migrate/20170929080234_add_failure_reason_to_pipelines.rb @@ -0,0 +1,9 @@ +class AddFailureReasonToPipelines < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :ci_pipelines, :failure_reason, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index fa1aad257db..5ebcf848709 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -342,6 +342,7 @@ ActiveRecord::Schema.define(version: 20171004121444) do t.integer "source" t.integer "config_source" t.boolean "protected" + t.integer "failure_reason" end add_index "ci_pipelines", ["auto_canceled_by_id"], name: "index_ci_pipelines_on_auto_canceled_by_id", using: :btree From 29f59bdf6f91fd2250fe237a320865e5c836f001 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 29 Sep 2017 10:18:53 +0200 Subject: [PATCH 15/61] Add failure reason enum to CI/CD pipeline model --- app/models/ci/pipeline.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 323c2c4ce7c..857db9622ee 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -58,6 +58,10 @@ module Ci auto_devops_source: 2 } + enum failure_reason: { + unknown_failure: 0 + } + state_machine :status, initial: :created do event :enqueue do transition created: :pending From 90fa4e7e54dc4f272b5f6a853f07b5e57a2a6cc4 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 29 Sep 2017 11:14:00 +0200 Subject: [PATCH 16/61] Move part of pipeline presenter to main CE file --- app/presenters/ci/pipeline_presenter.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/presenters/ci/pipeline_presenter.rb b/app/presenters/ci/pipeline_presenter.rb index a542bdd8295..5e393e95c80 100644 --- a/app/presenters/ci/pipeline_presenter.rb +++ b/app/presenters/ci/pipeline_presenter.rb @@ -1,6 +1,13 @@ module Ci class PipelinePresenter < Gitlab::View::Presenter::Delegated - presents :pipeline + FAILURE_REASONS = {} + + def failure_reason + return unless pipeline.failure_reason? + + FAILURE_REASONS[pipeline.failure_reason.to_sym] || + pipeline.failure_reason + end def status_title if auto_canceled? From 41d8030ec2834da8acc8dc28d1ac57cdbbc0667e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 2 Oct 2017 13:09:07 +0200 Subject: [PATCH 17/61] Pass pipeline failure reason to a transition event --- app/models/ci/pipeline.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 857db9622ee..ddf80433c57 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -113,6 +113,12 @@ module Ci pipeline.auto_canceled_by = nil end + before_transition any => :failed do |pipeline, transition| + transition.args.first.try do |reason| + pipeline.failure_reason = reason + end + end + after_transition [:created, :pending] => :running do |pipeline| pipeline.run_after_commit { PipelineMetricsWorker.perform_async(pipeline.id) } end From 9f639b073394b23c4e559b95dc5ba3ad422352bc Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 2 Oct 2017 13:40:12 +0200 Subject: [PATCH 18/61] Set a pipeline failure reason when it has YAML errors Conflicts: app/models/ci/pipeline.rb --- app/models/ci/pipeline.rb | 3 ++- lib/gitlab/ci/pipeline/chain/validate/config.rb | 2 +- spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb | 4 ++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index ddf80433c57..728ce1d9cc6 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -59,7 +59,8 @@ module Ci } enum failure_reason: { - unknown_failure: 0 + unknown_failure: 0, + config_error: 1 } state_machine :status, initial: :created do diff --git a/lib/gitlab/ci/pipeline/chain/validate/config.rb b/lib/gitlab/ci/pipeline/chain/validate/config.rb index 489bcd79655..075504bcce5 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/config.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/config.rb @@ -13,7 +13,7 @@ module Gitlab end if @command.save_incompleted && @pipeline.has_yaml_errors? - @pipeline.drop + @pipeline.drop!(:config_error) end return error(@pipeline.yaml_errors) diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb index 3740df88f42..8357af38f92 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb @@ -55,6 +55,10 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Config do it 'fails the pipeline' do expect(pipeline.reload).to be_failed end + + it 'sets a config error failure reason' do + expect(pipeline.reload.config_error?).to eq true + end end context 'when saving incomplete pipeline is not allowed' do From ffce9fd53f4e7d400d6f45e00b7e59acd902f3a6 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 3 Oct 2017 13:02:48 +0200 Subject: [PATCH 19/61] Expose failure reason in pipeline error badge Conflicts: app/serializers/pipeline_entity.rb --- .../javascripts/pipelines/components/pipeline_url.vue | 7 +++++++ app/serializers/pipeline_entity.rb | 8 ++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/pipelines/components/pipeline_url.vue b/app/assets/javascripts/pipelines/components/pipeline_url.vue index 76b97af39f1..9da0aac50a1 100644 --- a/app/assets/javascripts/pipelines/components/pipeline_url.vue +++ b/app/assets/javascripts/pipelines/components/pipeline_url.vue @@ -72,6 +72,13 @@ :title="pipeline.yaml_errors"> yaml invalid + + error + (pipeline, _) { pipeline.has_yaml_errors? } + + expose :failure_reason, if: -> (pipeline, _) { pipeline.failure_reason? } do |pipeline| + pipeline.present.failure_reason + end expose :retry_path, if: -> (*) { can_retry? } do |pipeline| retry_project_pipeline_path(pipeline.project, pipeline) @@ -53,8 +59,6 @@ class PipelineEntity < Grape::Entity cancel_project_pipeline_path(pipeline.project, pipeline) end - expose :yaml_errors, if: -> (pipeline, _) { pipeline.has_yaml_errors? } - private alias_method :pipeline, :object From 123da5fbe8f5dee13278b1ef9aff79ff9adcef90 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 3 Oct 2017 13:18:59 +0200 Subject: [PATCH 20/61] Improve specs for pipeline failure reason presenter Conflicts: app/presenters/ci/pipeline_presenter.rb --- app/presenters/ci/pipeline_presenter.rb | 6 +++++- spec/presenters/ci/pipeline_presenter_spec.rb | 17 +++++++++++++++++ spec/serializers/pipeline_entity_spec.rb | 13 +++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/app/presenters/ci/pipeline_presenter.rb b/app/presenters/ci/pipeline_presenter.rb index 5e393e95c80..099b4720fb6 100644 --- a/app/presenters/ci/pipeline_presenter.rb +++ b/app/presenters/ci/pipeline_presenter.rb @@ -1,6 +1,10 @@ module Ci class PipelinePresenter < Gitlab::View::Presenter::Delegated - FAILURE_REASONS = {} + FAILURE_REASONS = { + config_error: 'CI/CD YAML configuration error!' + }.freeze + + presents :pipeline def failure_reason return unless pipeline.failure_reason? diff --git a/spec/presenters/ci/pipeline_presenter_spec.rb b/spec/presenters/ci/pipeline_presenter_spec.rb index e4886a8f019..f7ceaf844be 100644 --- a/spec/presenters/ci/pipeline_presenter_spec.rb +++ b/spec/presenters/ci/pipeline_presenter_spec.rb @@ -51,4 +51,21 @@ describe Ci::PipelinePresenter do end end end + + context '#failure_reason' do + context 'when pipeline has failure reason' do + it 'represents a failure reason sentence' do + pipeline.failure_reason = :config_error + + expect(presenter.failure_reason) + .to eq 'CI/CD YAML configuration error!' + end + end + + context 'when pipeline does not have failure reason' do + it 'returns nil' do + expect(presenter.failure_reason).to be_nil + end + end + end end diff --git a/spec/serializers/pipeline_entity_spec.rb b/spec/serializers/pipeline_entity_spec.rb index f8df461bc81..248552d1858 100644 --- a/spec/serializers/pipeline_entity_spec.rb +++ b/spec/serializers/pipeline_entity_spec.rb @@ -108,5 +108,18 @@ describe PipelineEntity do expect(subject[:ref][:path]).to be_nil end end + + context 'when pipeline has a failure reason set' do + let(:pipeline) { create(:ci_empty_pipeline) } + + before do + pipeline.drop!(:config_error) + end + + it 'has a correct failure reason' do + expect(subject[:failure_reason]) + .to eq 'CI/CD YAML configuration error!' + end + end end end From 599fb7ef7f042594021ca0d26d938683fbe1206d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 3 Oct 2017 13:24:56 +0200 Subject: [PATCH 21/61] Add feature specs for pipeline failure reason badge --- spec/factories/ci/pipelines.rb | 1 + spec/features/projects/pipelines/pipelines_spec.rb | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index e5ea6b41ea3..f994c2df821 100644 --- a/spec/factories/ci/pipelines.rb +++ b/spec/factories/ci/pipelines.rb @@ -47,6 +47,7 @@ FactoryGirl.define do trait :invalid do config(rspec: nil) + failure_reason :config_error end trait :blocked do diff --git a/spec/features/projects/pipelines/pipelines_spec.rb b/spec/features/projects/pipelines/pipelines_spec.rb index f7b40cb1820..92486d2bc57 100644 --- a/spec/features/projects/pipelines/pipelines_spec.rb +++ b/spec/features/projects/pipelines/pipelines_spec.rb @@ -162,6 +162,16 @@ describe 'Pipelines', :js do expect(page).to have_selector( %Q{span[data-original-title="#{pipeline.yaml_errors}"]}) end + + it 'contains badge that indicates failure reason' do + expect(page).to have_content 'error' + end + + it 'contains badge with tooltip which contains failure reason' do + expect(pipeline.failure_reason?).to eq true + expect(page).to have_selector( + %Q{span[data-original-title="#{pipeline.present.failure_reason}"]}) + end end context 'with manual actions' do From 663f3240a95b68e23a0d89291edd0401e7887cb7 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 3 Oct 2017 13:42:51 +0200 Subject: [PATCH 22/61] Make it possible to export pipeline failure reason --- spec/lib/gitlab/import_export/safe_model_attributes.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 7268226112c..48938346577 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -224,6 +224,7 @@ Ci::Pipeline: - auto_canceled_by_id - pipeline_schedule_id - config_source +- failure_reason - protected Ci::Stage: - id From 08cc88f9cecb626ba3f877dd52dd3666add1b434 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 4 Oct 2017 09:32:22 +0200 Subject: [PATCH 23/61] Add js test for pipelines component with error badge --- .../javascripts/pipelines/pipeline_url_spec.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/spec/javascripts/pipelines/pipeline_url_spec.js b/spec/javascripts/pipelines/pipeline_url_spec.js index 256fdbe743c..260eb802bd4 100644 --- a/spec/javascripts/pipelines/pipeline_url_spec.js +++ b/spec/javascripts/pipelines/pipeline_url_spec.js @@ -125,4 +125,22 @@ describe('Pipeline Url Component', () => { component.$el.querySelector('.js-pipeline-url-autodevops').textContent.trim(), ).toEqual('Auto DevOps'); }); + + it('should render error badge when pipeline has a failure reason set', () => { + const component = new PipelineUrlComponent({ + propsData: { + pipeline: { + id: 1, + path: 'foo', + flags: { + failure_reason: true, + }, + failure_reason: 'some reason' + }, + }, + }).$mount(); + + expect(component.$el.querySelector('.js-pipeline-url-failure').textContent).toContain('error'); + expect(component.$el.querySelector('.js-pipeline-url-failure').getAttribute('title').toContain('some reason'); + }); }); From d6e0738098dea1b243280f857ed05c68ad5a8397 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 4 Oct 2017 11:02:52 +0200 Subject: [PATCH 24/61] Fix syntax error in forntend pipeline specs --- spec/javascripts/pipelines/pipeline_url_spec.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/javascripts/pipelines/pipeline_url_spec.js b/spec/javascripts/pipelines/pipeline_url_spec.js index 260eb802bd4..c40842f80b4 100644 --- a/spec/javascripts/pipelines/pipeline_url_spec.js +++ b/spec/javascripts/pipelines/pipeline_url_spec.js @@ -135,12 +135,13 @@ describe('Pipeline Url Component', () => { flags: { failure_reason: true, }, - failure_reason: 'some reason' + failure_reason: 'some reason', }, + autoDevopsHelpPath: 'foo', }, }).$mount(); expect(component.$el.querySelector('.js-pipeline-url-failure').textContent).toContain('error'); - expect(component.$el.querySelector('.js-pipeline-url-failure').getAttribute('title').toContain('some reason'); + expect(component.$el.querySelector('.js-pipeline-url-failure').getAttribute('title')).toContain('some reason'); }); }); From a2e6afdd8c3886ab7b0ac608da6c2952667fe877 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 4 Oct 2017 13:32:54 +0200 Subject: [PATCH 25/61] Fix frotnend pipeline specs for pipeline error badge --- spec/javascripts/pipelines/pipeline_url_spec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/javascripts/pipelines/pipeline_url_spec.js b/spec/javascripts/pipelines/pipeline_url_spec.js index c40842f80b4..cb8e4c32780 100644 --- a/spec/javascripts/pipelines/pipeline_url_spec.js +++ b/spec/javascripts/pipelines/pipeline_url_spec.js @@ -141,7 +141,8 @@ describe('Pipeline Url Component', () => { }, }).$mount(); + expect(component.$el.querySelector('.js-pipeline-url-failure').textContent).toContain('error'); - expect(component.$el.querySelector('.js-pipeline-url-failure').getAttribute('title')).toContain('some reason'); + expect(component.$el.querySelector('.js-pipeline-url-failure').getAttribute('data-original-title')).toContain('some reason'); }); }); From 7db9323de7091b75c3c19b871f3632ed870f9b35 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 4 Oct 2017 14:55:03 +0200 Subject: [PATCH 26/61] Remove a redundant empty line in js pipelines specs --- spec/javascripts/pipelines/pipeline_url_spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/javascripts/pipelines/pipeline_url_spec.js b/spec/javascripts/pipelines/pipeline_url_spec.js index cb8e4c32780..4a4f2259d23 100644 --- a/spec/javascripts/pipelines/pipeline_url_spec.js +++ b/spec/javascripts/pipelines/pipeline_url_spec.js @@ -141,7 +141,6 @@ describe('Pipeline Url Component', () => { }, }).$mount(); - expect(component.$el.querySelector('.js-pipeline-url-failure').textContent).toContain('error'); expect(component.$el.querySelector('.js-pipeline-url-failure').getAttribute('data-original-title')).toContain('some reason'); }); From 83059f130e9049939e3e8251b7ede5fa128073fd Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 3 Oct 2017 10:12:28 +0200 Subject: [PATCH 27/61] Improve pipeline optimistic locking implementation --- app/models/ci/pipeline.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 728ce1d9cc6..cf3ce3c9e54 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -5,6 +5,7 @@ module Ci include Importable include AfterCommitQueue include Presentable + include Gitlab::OptimisticLocking belongs_to :project belongs_to :user @@ -274,7 +275,7 @@ module Ci end def cancel_running - Gitlab::OptimisticLocking.retry_lock(cancelable_statuses) do |cancelable| + retry_optimistic_lock(cancelable_statuses) do |cancelable| cancelable.find_each do |job| yield(job) if block_given? job.cancel @@ -418,7 +419,7 @@ module Ci end def update_status - Gitlab::OptimisticLocking.retry_lock(self) do + retry_optimistic_lock(self) do case latest_builds_status when 'pending' then enqueue when 'running' then run From 62d540340120baac7fa432108a7847cbc2a1cbe5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Tue, 26 Sep 2017 19:42:23 -0500 Subject: [PATCH 28/61] Process and create subkeys when a new GPG key is created --- app/models/gpg_key.rb | 21 +++++++++++++++++-- ...0170926050624_add_parent_id_to_gpg_keys.rb | 10 +++++++++ db/schema.rb | 1 + lib/gitlab/gpg.rb | 21 +++++++++++++++++++ 4 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20170926050624_add_parent_id_to_gpg_keys.rb diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 54bd5b68777..cbf01183969 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -9,6 +9,7 @@ class GpgKey < ActiveRecord::Base belongs_to :user has_many :gpg_signatures + has_many :subkeys, class_name: 'GpgKey', foreign_key: :parent_id, dependent: :destroy validates :user, presence: true @@ -18,7 +19,8 @@ class GpgKey < ActiveRecord::Base format: { with: /\A#{KEY_PREFIX}((?!#{KEY_PREFIX})(?!#{KEY_SUFFIX}).)+#{KEY_SUFFIX}\Z/m, message: "is invalid. A valid public GPG key begins with '#{KEY_PREFIX}' and ends with '#{KEY_SUFFIX}'" - } + }, + unless: :parent_id? validates :fingerprint, presence: true, @@ -34,8 +36,9 @@ class GpgKey < ActiveRecord::Base # the error about the fingerprint unless: -> { errors.has_key?(:key) } - before_validation :extract_fingerprint, :extract_primary_keyid + before_validation :extract_fingerprint, :extract_primary_keyid, unless: :parent_id? after_commit :update_invalid_gpg_signatures, on: :create + after_save :generate_subkeys, unless: :parent_id? def primary_keyid super&.upcase @@ -106,4 +109,18 @@ class GpgKey < ActiveRecord::Base # only allows one key self.primary_keyid = Gitlab::Gpg.primary_keyids_from_key(key).first end + + def generate_subkeys + gpg_subkeys = Gitlab::Gpg.subkeys_from_key(key) + + gpg_subkeys[primary_keyid].each do |subkey_data| + unless subkeys.where(fingerprint: subkey_data[:fingerprint]).exists? + subkeys.create!( + user: user, + primary_keyid: subkey_data[:keyid], + fingerprint: subkey_data[:fingerprint] + ) + end + end + end end diff --git a/db/migrate/20170926050624_add_parent_id_to_gpg_keys.rb b/db/migrate/20170926050624_add_parent_id_to_gpg_keys.rb new file mode 100644 index 00000000000..ef7675be106 --- /dev/null +++ b/db/migrate/20170926050624_add_parent_id_to_gpg_keys.rb @@ -0,0 +1,10 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddParentIdToGpgKeys < ActiveRecord::Migration + DOWNTIME = false + + def change + add_column :gpg_keys, :parent_id, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 17be774e9de..9b1457c9a03 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -586,6 +586,7 @@ ActiveRecord::Schema.define(version: 20171004121444) do t.binary "primary_keyid" t.binary "fingerprint" t.text "key" + t.integer "parent_id" end add_index "gpg_keys", ["fingerprint"], name: "index_gpg_keys_on_fingerprint", unique: true, using: :btree diff --git a/lib/gitlab/gpg.rb b/lib/gitlab/gpg.rb index 0d5039ddf5f..be22a9e0fe2 100644 --- a/lib/gitlab/gpg.rb +++ b/lib/gitlab/gpg.rb @@ -34,6 +34,27 @@ module Gitlab end end + def subkeys_from_key(key) + using_tmp_keychain do + fingerprints = CurrentKeyChain.fingerprints_from_key(key) + raw_keys = GPGME::Key.find(:public, fingerprints) + grouped_subkeys = Hash.new { |h, k| h[k] = [] } + + raw_keys.each_with_object(grouped_subkeys).each do |raw_key, subkeys| + primary_subkey_id = raw_key.primary_subkey.keyid + + raw_key.subkeys.each do |subkey| + # Skip if current subkey is a master key + next if primary_subkey_id == subkey.keyid + # Skip if it isn't a sign key + next if subkey.capability.exclude?(:sign) + + subkeys[primary_subkey_id] << { keyid: subkey.keyid, fingerprint: subkey.fingerprint } + end + end + end + end + def user_infos_from_key(key) using_tmp_keychain do fingerprints = CurrentKeyChain.fingerprints_from_key(key) From 37dcfb6e31a65d487424c0bf73874aabb2e69020 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Tue, 26 Sep 2017 22:10:10 -0500 Subject: [PATCH 29/61] Delegate #key to parent when working with a GPG subkey The content returned by #key is important when veryfying the signature, so given we don't want to repeat it in the database for GPG subkeys we need to delegate it to the parent. --- app/models/gpg_key.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index cbf01183969..683efcebee9 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -8,6 +8,7 @@ class GpgKey < ActiveRecord::Base sha_attribute :fingerprint belongs_to :user + belongs_to :parent, class_name: 'GpgKey' has_many :gpg_signatures has_many :subkeys, class_name: 'GpgKey', foreign_key: :parent_id, dependent: :destroy @@ -48,6 +49,10 @@ class GpgKey < ActiveRecord::Base super&.upcase end + def key + parent_id? ? parent.key : super + end + def key=(value) super(value&.strip) end From 328f4a505bcd2e7b38f907a24df437f8c35bc3af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 27 Sep 2017 12:27:39 -0500 Subject: [PATCH 30/61] Use a separate model to handle GPG subkeys --- app/models/gpg_key.rb | 2 +- app/models/gpg_key_subkey.rb | 3 +++ db/migrate/20170926050624_add_parent_id_to_gpg_keys.rb | 10 ---------- db/migrate/20170927161718_create_gpg_key_subkeys.rb | 9 +++++++++ db/schema.rb | 10 +++++++++- 5 files changed, 22 insertions(+), 12 deletions(-) create mode 100644 app/models/gpg_key_subkey.rb delete mode 100644 db/migrate/20170926050624_add_parent_id_to_gpg_keys.rb create mode 100644 db/migrate/20170927161718_create_gpg_key_subkeys.rb diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 683efcebee9..110ff4ee59c 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -10,7 +10,7 @@ class GpgKey < ActiveRecord::Base belongs_to :user belongs_to :parent, class_name: 'GpgKey' has_many :gpg_signatures - has_many :subkeys, class_name: 'GpgKey', foreign_key: :parent_id, dependent: :destroy + has_many :subkeys, class_name: 'GpgKeySubkey' validates :user, presence: true diff --git a/app/models/gpg_key_subkey.rb b/app/models/gpg_key_subkey.rb new file mode 100644 index 00000000000..4f967f1e47c --- /dev/null +++ b/app/models/gpg_key_subkey.rb @@ -0,0 +1,3 @@ +class GpgKeySubkey < ActiveRecord::Base + belongs_to :gpg_key +end diff --git a/db/migrate/20170926050624_add_parent_id_to_gpg_keys.rb b/db/migrate/20170926050624_add_parent_id_to_gpg_keys.rb deleted file mode 100644 index ef7675be106..00000000000 --- a/db/migrate/20170926050624_add_parent_id_to_gpg_keys.rb +++ /dev/null @@ -1,10 +0,0 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddParentIdToGpgKeys < ActiveRecord::Migration - DOWNTIME = false - - def change - add_column :gpg_keys, :parent_id, :integer - end -end diff --git a/db/migrate/20170927161718_create_gpg_key_subkeys.rb b/db/migrate/20170927161718_create_gpg_key_subkeys.rb new file mode 100644 index 00000000000..d61b669f517 --- /dev/null +++ b/db/migrate/20170927161718_create_gpg_key_subkeys.rb @@ -0,0 +1,9 @@ +class CreateGpgKeySubkeys < ActiveRecord::Migration + def change + create_table :gpg_key_subkeys do |t| + t.binary :keyid + t.binary :fingerprint + t.references :gpg_key, null: false, index: true, foreign_key: { on_delete: :cascade } + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 9b1457c9a03..0d02e584d0c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -579,6 +579,14 @@ ActiveRecord::Schema.define(version: 20171004121444) do add_index "forked_project_links", ["forked_to_project_id"], name: "index_forked_project_links_on_forked_to_project_id", unique: true, using: :btree + create_table "gpg_key_subkeys", force: :cascade do |t| + t.binary "keyid" + t.binary "fingerprint" + t.integer "gpg_key_id", null: false + end + + add_index "gpg_key_subkeys", ["gpg_key_id"], name: "index_gpg_key_subkeys_on_gpg_key_id", using: :btree + create_table "gpg_keys", force: :cascade do |t| t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false @@ -586,7 +594,6 @@ ActiveRecord::Schema.define(version: 20171004121444) do t.binary "primary_keyid" t.binary "fingerprint" t.text "key" - t.integer "parent_id" end add_index "gpg_keys", ["fingerprint"], name: "index_gpg_keys_on_fingerprint", unique: true, using: :btree @@ -1727,6 +1734,7 @@ ActiveRecord::Schema.define(version: 20171004121444) do add_foreign_key "events", "projects", on_delete: :cascade add_foreign_key "events", "users", column: "author_id", name: "fk_edfd187b6f", on_delete: :cascade add_foreign_key "forked_project_links", "projects", column: "forked_to_project_id", name: "fk_434510edb0", on_delete: :cascade + add_foreign_key "gpg_key_subkeys", "gpg_keys", on_delete: :cascade add_foreign_key "gpg_keys", "users", on_delete: :cascade add_foreign_key "gpg_signatures", "gpg_keys", on_delete: :nullify add_foreign_key "gpg_signatures", "projects", on_delete: :cascade From d0572d9aad2c434a040f11956a4c3feac1afdcf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 27 Sep 2017 12:28:50 -0500 Subject: [PATCH 31/61] Refactor the extraction and generation of GPG subkeys --- app/models/gpg_key.rb | 19 ++++--------------- lib/gitlab/gpg.rb | 7 +------ 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 110ff4ee59c..ed09b44027c 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -20,8 +20,7 @@ class GpgKey < ActiveRecord::Base format: { with: /\A#{KEY_PREFIX}((?!#{KEY_PREFIX})(?!#{KEY_SUFFIX}).)+#{KEY_SUFFIX}\Z/m, message: "is invalid. A valid public GPG key begins with '#{KEY_PREFIX}' and ends with '#{KEY_SUFFIX}'" - }, - unless: :parent_id? + } validates :fingerprint, presence: true, @@ -37,9 +36,9 @@ class GpgKey < ActiveRecord::Base # the error about the fingerprint unless: -> { errors.has_key?(:key) } - before_validation :extract_fingerprint, :extract_primary_keyid, unless: :parent_id? + before_validation :extract_fingerprint, :extract_primary_keyid after_commit :update_invalid_gpg_signatures, on: :create - after_save :generate_subkeys, unless: :parent_id? + after_create :generate_subkeys def primary_keyid super&.upcase @@ -49,10 +48,6 @@ class GpgKey < ActiveRecord::Base super&.upcase end - def key - parent_id? ? parent.key : super - end - def key=(value) super(value&.strip) end @@ -119,13 +114,7 @@ class GpgKey < ActiveRecord::Base gpg_subkeys = Gitlab::Gpg.subkeys_from_key(key) gpg_subkeys[primary_keyid].each do |subkey_data| - unless subkeys.where(fingerprint: subkey_data[:fingerprint]).exists? - subkeys.create!( - user: user, - primary_keyid: subkey_data[:keyid], - fingerprint: subkey_data[:fingerprint] - ) - end + subkeys.create!(keyid: subkey_data[:keyid], fingerprint: subkey_data[:fingerprint]) end end end diff --git a/lib/gitlab/gpg.rb b/lib/gitlab/gpg.rb index be22a9e0fe2..343bf54a6ae 100644 --- a/lib/gitlab/gpg.rb +++ b/lib/gitlab/gpg.rb @@ -43,12 +43,7 @@ module Gitlab raw_keys.each_with_object(grouped_subkeys).each do |raw_key, subkeys| primary_subkey_id = raw_key.primary_subkey.keyid - raw_key.subkeys.each do |subkey| - # Skip if current subkey is a master key - next if primary_subkey_id == subkey.keyid - # Skip if it isn't a sign key - next if subkey.capability.exclude?(:sign) - + raw_key.subkeys[1..-1].each do |subkey| subkeys[primary_subkey_id] << { keyid: subkey.keyid, fingerprint: subkey.fingerprint } end end From a41e7e0105e238161ba697ebf26d8554ae59d295 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 27 Sep 2017 16:11:42 -0500 Subject: [PATCH 32/61] Add ability to include subkeys when finding by fingerprint --- app/models/gpg_key.rb | 11 +++++++++++ lib/gitlab/gpg/commit.rb | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index ed09b44027c..6d3537b6fcf 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -40,6 +40,17 @@ class GpgKey < ActiveRecord::Base after_commit :update_invalid_gpg_signatures, on: :create after_create :generate_subkeys + def self.find_with_subkeys(fingerprint) + keys_table = arel_table + subkeys_table = GpgKeySubkey.arel_table + + condition = keys_table[:primary_keyid].eq(fingerprint).or( + subkeys_table[:keyid].eq(fingerprint) + ) + + joins(:subkeys).where(condition).first + end + def primary_keyid super&.upcase end diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb index 86bd9f5b125..40274e13918 100644 --- a/lib/gitlab/gpg/commit.rb +++ b/lib/gitlab/gpg/commit.rb @@ -43,7 +43,7 @@ module Gitlab # key belonging to the keyid. # This way we can add the key to the temporary keychain and extract # the proper signature. - gpg_key = GpgKey.find_by(primary_keyid: verified_signature.fingerprint) + gpg_key = GpgKey.find_with_subkeys(verified_signature.fingerprint) if gpg_key Gitlab::Gpg::CurrentKeyChain.add(gpg_key.key) From 9b4990a4d71b057f0fec14399cd1f2a421901963 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 27 Sep 2017 19:45:19 -0500 Subject: [PATCH 33/61] Associate GgpSignature with GpgKeySubkey if comes from a subkey Additionally we're delegating missing method calls on GpgKeySubkey to GpgKey since most of the info required when verifying a signature is found on GpgKey which is the parent of GpgKeySubkey --- app/models/gpg_key.rb | 11 ----------- app/models/gpg_key_subkey.rb | 15 +++++++++++++++ app/models/gpg_signature.rb | 16 ++++++++++++++++ ...58_add_gpg_key_subkey_id_to_gpg_signatures.rb | 10 ++++++++++ db/schema.rb | 3 +++ lib/gitlab/gpg/commit.rb | 14 ++++++++++++-- 6 files changed, 56 insertions(+), 13 deletions(-) create mode 100644 db/migrate/20170927232658_add_gpg_key_subkey_id_to_gpg_signatures.rb diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 6d3537b6fcf..ed09b44027c 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -40,17 +40,6 @@ class GpgKey < ActiveRecord::Base after_commit :update_invalid_gpg_signatures, on: :create after_create :generate_subkeys - def self.find_with_subkeys(fingerprint) - keys_table = arel_table - subkeys_table = GpgKeySubkey.arel_table - - condition = keys_table[:primary_keyid].eq(fingerprint).or( - subkeys_table[:keyid].eq(fingerprint) - ) - - joins(:subkeys).where(condition).first - end - def primary_keyid super&.upcase end diff --git a/app/models/gpg_key_subkey.rb b/app/models/gpg_key_subkey.rb index 4f967f1e47c..8222e5606aa 100644 --- a/app/models/gpg_key_subkey.rb +++ b/app/models/gpg_key_subkey.rb @@ -1,3 +1,18 @@ class GpgKeySubkey < ActiveRecord::Base + include ShaAttribute + + sha_attribute :keyid + sha_attribute :fingerprint + belongs_to :gpg_key + + def method_missing(m, *a, &b) + return super unless gpg_key.respond_to?(m) + + gpg_key.public_send(m, *a, &b) # rubocop:disable GitlabSecurity/PublicSend + end + + def respond_to_missing?(method, include_private = false) + gpg_key.respond_to?(method, include_private) || super + end end diff --git a/app/models/gpg_signature.rb b/app/models/gpg_signature.rb index 1f047a32c84..c7f75288407 100644 --- a/app/models/gpg_signature.rb +++ b/app/models/gpg_signature.rb @@ -15,11 +15,27 @@ class GpgSignature < ActiveRecord::Base belongs_to :project belongs_to :gpg_key + belongs_to :gpg_key_subkey validates :commit_sha, presence: true validates :project_id, presence: true validates :gpg_key_primary_keyid, presence: true + def gpg_key=(model) + case model + when GpgKey then super + when GpgKeySubkey then write_attribute(:gpg_key_subkey_id, model.id) + end + end + + def gpg_key + if gpg_key_id + super + elsif gpg_key_subkey_id + gpg_key_subkey + end + end + def gpg_key_primary_keyid super&.upcase end diff --git a/db/migrate/20170927232658_add_gpg_key_subkey_id_to_gpg_signatures.rb b/db/migrate/20170927232658_add_gpg_key_subkey_id_to_gpg_signatures.rb new file mode 100644 index 00000000000..74b43d732d5 --- /dev/null +++ b/db/migrate/20170927232658_add_gpg_key_subkey_id_to_gpg_signatures.rb @@ -0,0 +1,10 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddGpgKeySubkeyIdToGpgSignatures < ActiveRecord::Migration + DOWNTIME = false + + def change + add_reference(:gpg_signatures, :gpg_key_subkey, index: true, foreign_key: { on_delete: :nullify }) + end +end diff --git a/db/schema.rb b/db/schema.rb index 0d02e584d0c..0beebc16cbb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -610,11 +610,13 @@ ActiveRecord::Schema.define(version: 20171004121444) do t.text "gpg_key_user_name" t.text "gpg_key_user_email" t.integer "verification_status", limit: 2, default: 0, null: false + t.integer "gpg_key_subkey_id" end add_index "gpg_signatures", ["commit_sha"], name: "index_gpg_signatures_on_commit_sha", unique: true, using: :btree add_index "gpg_signatures", ["gpg_key_id"], name: "index_gpg_signatures_on_gpg_key_id", using: :btree add_index "gpg_signatures", ["gpg_key_primary_keyid"], name: "index_gpg_signatures_on_gpg_key_primary_keyid", using: :btree + add_index "gpg_signatures", ["gpg_key_subkey_id"], name: "index_gpg_signatures_on_gpg_key_subkey_id", using: :btree add_index "gpg_signatures", ["project_id"], name: "index_gpg_signatures_on_project_id", using: :btree create_table "identities", force: :cascade do |t| @@ -1736,6 +1738,7 @@ ActiveRecord::Schema.define(version: 20171004121444) do add_foreign_key "forked_project_links", "projects", column: "forked_to_project_id", name: "fk_434510edb0", on_delete: :cascade add_foreign_key "gpg_key_subkeys", "gpg_keys", on_delete: :cascade add_foreign_key "gpg_keys", "users", on_delete: :cascade + add_foreign_key "gpg_signatures", "gpg_key_subkeys", on_delete: :nullify add_foreign_key "gpg_signatures", "gpg_keys", on_delete: :nullify add_foreign_key "gpg_signatures", "projects", on_delete: :cascade add_foreign_key "issue_assignees", "issues", name: "fk_b7d881734a", on_delete: :cascade diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb index 40274e13918..5cbc836314f 100644 --- a/lib/gitlab/gpg/commit.rb +++ b/lib/gitlab/gpg/commit.rb @@ -43,7 +43,7 @@ module Gitlab # key belonging to the keyid. # This way we can add the key to the temporary keychain and extract # the proper signature. - gpg_key = GpgKey.find_with_subkeys(verified_signature.fingerprint) + gpg_key = find_gpg_key(verified_signature.fingerprint) if gpg_key Gitlab::Gpg::CurrentKeyChain.add(gpg_key.key) @@ -74,7 +74,7 @@ module Gitlab commit_sha: @commit.sha, project: @commit.project, gpg_key: gpg_key, - gpg_key_primary_keyid: gpg_key&.primary_keyid || verified_signature.fingerprint, + gpg_key_primary_keyid: gpg_keyid(gpg_key) || verified_signature.fingerprint, gpg_key_user_name: user_infos[:name], gpg_key_user_email: user_infos[:email], verification_status: verification_status @@ -98,6 +98,16 @@ module Gitlab def user_infos(gpg_key) gpg_key&.verified_user_infos&.first || gpg_key&.user_infos&.first || {} end + + def gpg_keyid(gpg_key) + return nil unless gpg_key + + gpg_key.is_a?(GpgKey) ? gpg_key.primary_keyid : gpg_key.keyid + end + + def find_gpg_key(keyid) + GpgKey.find_by(primary_keyid: keyid) || GpgKeySubkey.find_by(keyid: keyid) + end end end end From c73748e3c4409de7ab945f502d55fe4d62ebd5eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 27 Sep 2017 20:45:08 -0500 Subject: [PATCH 34/61] Render GPG subkeys on index page --- app/assets/stylesheets/pages/profile.scss | 9 +++++++++ app/controllers/profiles/gpg_keys_controller.rb | 2 +- app/models/gpg_key.rb | 2 ++ app/views/profiles/gpg_keys/_key.html.haml | 7 +++++++ 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/pages/profile.scss b/app/assets/stylesheets/pages/profile.scss index 6c521bb06ee..eab39f698c3 100644 --- a/app/assets/stylesheets/pages/profile.scss +++ b/app/assets/stylesheets/pages/profile.scss @@ -108,6 +108,15 @@ } } +.subkeys-list { + @include basic-list; + + li { + padding: 3px 0; + border: none; + } +} + .key-list-item { .key-list-item-info { @media (min-width: $screen-sm-min) { diff --git a/app/controllers/profiles/gpg_keys_controller.rb b/app/controllers/profiles/gpg_keys_controller.rb index 689c76059f6..38e3eacd229 100644 --- a/app/controllers/profiles/gpg_keys_controller.rb +++ b/app/controllers/profiles/gpg_keys_controller.rb @@ -2,7 +2,7 @@ class Profiles::GpgKeysController < Profiles::ApplicationController before_action :set_gpg_key, only: [:destroy, :revoke] def index - @gpg_keys = current_user.gpg_keys + @gpg_keys = current_user.gpg_keys.with_subkeys @gpg_key = GpgKey.new end diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index ed09b44027c..e6c862d9b55 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -12,6 +12,8 @@ class GpgKey < ActiveRecord::Base has_many :gpg_signatures has_many :subkeys, class_name: 'GpgKeySubkey' + scope :with_subkeys, -> { includes(:subkeys) } + validates :user, presence: true validates :key, diff --git a/app/views/profiles/gpg_keys/_key.html.haml b/app/views/profiles/gpg_keys/_key.html.haml index 970e92aadaa..5ed517c1ef6 100644 --- a/app/views/profiles/gpg_keys/_key.html.haml +++ b/app/views/profiles/gpg_keys/_key.html.haml @@ -7,6 +7,13 @@ .description %code= key.fingerprint + - if key.subkeys.present? + .subkeys + %span.bold Subkeys: + %ul.subkeys-list + - key.subkeys.each do |subkey| + %li + %code= subkey.fingerprint .pull-right %span.key-created-at created #{time_ago_with_tooltip(key.created_at)} From 4b6d045c0c3c8d8ab70dd80feb960f193e498d7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 27 Sep 2017 20:47:15 -0500 Subject: [PATCH 35/61] Add missing DOWNTIME declaration --- db/migrate/20170927161718_create_gpg_key_subkeys.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/db/migrate/20170927161718_create_gpg_key_subkeys.rb b/db/migrate/20170927161718_create_gpg_key_subkeys.rb index d61b669f517..9f69e299874 100644 --- a/db/migrate/20170927161718_create_gpg_key_subkeys.rb +++ b/db/migrate/20170927161718_create_gpg_key_subkeys.rb @@ -1,4 +1,6 @@ class CreateGpgKeySubkeys < ActiveRecord::Migration + DOWNTIME = false + def change create_table :gpg_key_subkeys do |t| t.binary :keyid From b27549df97151f773c6fbfac4c9dc3aa491a8b5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Thu, 28 Sep 2017 13:26:16 -0500 Subject: [PATCH 36/61] Add some basic specs and refactor model and validations --- app/models/gpg_key_subkey.rb | 11 +++++ .../20170927161718_create_gpg_key_subkeys.rb | 14 ++++++- ...add_gpg_key_subkey_id_to_gpg_signatures.rb | 10 ----- db/schema.rb | 2 + spec/factories/gpg_key_subkeys.rb | 10 +++++ spec/factories/gpg_keys.rb | 4 ++ spec/models/gpg_key_spec.rb | 9 +++++ spec/models/gpg_key_subkey_spec.rb | 15 +++++++ spec/services/gpg_keys/create_service_spec.rb | 10 +++++ spec/support/gpg_helpers.rb | 40 +++++++++++++++++++ 10 files changed, 114 insertions(+), 11 deletions(-) delete mode 100644 db/migrate/20170927232658_add_gpg_key_subkey_id_to_gpg_signatures.rb create mode 100644 spec/factories/gpg_key_subkeys.rb create mode 100644 spec/models/gpg_key_subkey_spec.rb diff --git a/app/models/gpg_key_subkey.rb b/app/models/gpg_key_subkey.rb index 8222e5606aa..b4f146e5647 100644 --- a/app/models/gpg_key_subkey.rb +++ b/app/models/gpg_key_subkey.rb @@ -6,6 +6,17 @@ class GpgKeySubkey < ActiveRecord::Base belongs_to :gpg_key + validates :gpg_key_id, presence: true + validates :fingerprint, :keyid, presence: true, uniqueness: true + + def keyid + super&.upcase + end + + def fingerprint + super&.upcase + end + def method_missing(m, *a, &b) return super unless gpg_key.respond_to?(m) diff --git a/db/migrate/20170927161718_create_gpg_key_subkeys.rb b/db/migrate/20170927161718_create_gpg_key_subkeys.rb index 9f69e299874..ffe06ce1231 100644 --- a/db/migrate/20170927161718_create_gpg_key_subkeys.rb +++ b/db/migrate/20170927161718_create_gpg_key_subkeys.rb @@ -1,11 +1,23 @@ class CreateGpgKeySubkeys < ActiveRecord::Migration DOWNTIME = false - def change + def up create_table :gpg_key_subkeys do |t| t.binary :keyid t.binary :fingerprint + t.references :gpg_key, null: false, index: true, foreign_key: { on_delete: :cascade } + + t.index :keyid, unique: true, length: Gitlab::Database.mysql? ? 20 : nil + t.index :fingerprint, unique: true, length: Gitlab::Database.mysql? ? 20 : nil end + + add_reference :gpg_signatures, :gpg_key_subkey, index: true, foreign_key: { on_delete: :nullify } + end + + def down + remove_reference(:gpg_signatures, :gpg_key_subkey, index: true, foreign_key: true) + + drop_table :gpg_key_subkeys end end diff --git a/db/migrate/20170927232658_add_gpg_key_subkey_id_to_gpg_signatures.rb b/db/migrate/20170927232658_add_gpg_key_subkey_id_to_gpg_signatures.rb deleted file mode 100644 index 74b43d732d5..00000000000 --- a/db/migrate/20170927232658_add_gpg_key_subkey_id_to_gpg_signatures.rb +++ /dev/null @@ -1,10 +0,0 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddGpgKeySubkeyIdToGpgSignatures < ActiveRecord::Migration - DOWNTIME = false - - def change - add_reference(:gpg_signatures, :gpg_key_subkey, index: true, foreign_key: { on_delete: :nullify }) - end -end diff --git a/db/schema.rb b/db/schema.rb index 0beebc16cbb..b9de70b742a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -585,7 +585,9 @@ ActiveRecord::Schema.define(version: 20171004121444) do t.integer "gpg_key_id", null: false end + add_index "gpg_key_subkeys", ["fingerprint"], name: "index_gpg_key_subkeys_on_fingerprint", unique: true, using: :btree add_index "gpg_key_subkeys", ["gpg_key_id"], name: "index_gpg_key_subkeys_on_gpg_key_id", using: :btree + add_index "gpg_key_subkeys", ["keyid"], name: "index_gpg_key_subkeys_on_keyid", unique: true, using: :btree create_table "gpg_keys", force: :cascade do |t| t.datetime_with_timezone "created_at", null: false diff --git a/spec/factories/gpg_key_subkeys.rb b/spec/factories/gpg_key_subkeys.rb new file mode 100644 index 00000000000..d896b4e217e --- /dev/null +++ b/spec/factories/gpg_key_subkeys.rb @@ -0,0 +1,10 @@ +require_relative '../support/gpg_helpers' + +FactoryGirl.define do + factory :gpg_key_subkey do + gpg_key + + keyid { gpg_key.subkeys.last.keyid } + fingerprint { gpg_key.subkeys.last.fingerprint } + end +end diff --git a/spec/factories/gpg_keys.rb b/spec/factories/gpg_keys.rb index 1258dce8940..93218e5763e 100644 --- a/spec/factories/gpg_keys.rb +++ b/spec/factories/gpg_keys.rb @@ -4,5 +4,9 @@ FactoryGirl.define do factory :gpg_key do key GpgHelpers::User1.public_key user + + factory :gpg_key_with_subkeys do + key GpgHelpers::User1.public_key_with_extra_signing_key + end end end diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index 743f2cfcab5..69d6090e034 100644 --- a/spec/models/gpg_key_spec.rb +++ b/spec/models/gpg_key_spec.rb @@ -3,6 +3,7 @@ require 'rails_helper' describe GpgKey do describe "associations" do it { is_expected.to belong_to(:user) } + it { is_expected.to have_many(:subkeys) } end describe "validation" do @@ -38,6 +39,14 @@ describe GpgKey do expect(gpg_key.primary_keyid).to eq GpgHelpers::User1.primary_keyid end end + + describe 'generate_subkeys' do + it 'extracts the subkeys from the gpg key' do + gpg_key = create(:gpg_key, key: GpgHelpers::User1.public_key_with_extra_signing_key) + + expect(gpg_key.subkeys.count).to eq(2) + end + end end describe '#key=' do diff --git a/spec/models/gpg_key_subkey_spec.rb b/spec/models/gpg_key_subkey_spec.rb new file mode 100644 index 00000000000..3c86837f47f --- /dev/null +++ b/spec/models/gpg_key_subkey_spec.rb @@ -0,0 +1,15 @@ +require 'rails_helper' + +describe GpgKeySubkey do + subject { build(:gpg_key_subkey) } + + describe 'associations' do + it { is_expected.to belong_to(:gpg_key) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:gpg_key_id) } + it { is_expected.to validate_presence_of(:fingerprint) } + it { is_expected.to validate_presence_of(:keyid) } + end +end diff --git a/spec/services/gpg_keys/create_service_spec.rb b/spec/services/gpg_keys/create_service_spec.rb index 20382a3a618..1cd2625531e 100644 --- a/spec/services/gpg_keys/create_service_spec.rb +++ b/spec/services/gpg_keys/create_service_spec.rb @@ -18,4 +18,14 @@ describe GpgKeys::CreateService do it 'creates a gpg key' do expect { subject.execute }.to change { user.gpg_keys.where(params).count }.by(1) end + + context 'when the public key contains subkeys' do + let(:params) { attributes_for(:gpg_key_with_subkeys) } + + it 'generates the gpg subkeys' do + gpg_key = subject.execute + + expect(gpg_key.subkeys.count).to eq(2) + end + end end diff --git a/spec/support/gpg_helpers.rb b/spec/support/gpg_helpers.rb index 65b38626a51..a8cfd72954e 100644 --- a/spec/support/gpg_helpers.rb +++ b/spec/support/gpg_helpers.rb @@ -92,6 +92,46 @@ module GpgHelpers KEY end + def public_key_with_extra_signing_key + <<~KEY.strip + -----BEGIN PGP PUBLIC KEY BLOCK----- + Version: GnuPG v1 + + mI0EWK7VJwEEANSFayuVYenl7sBKUjmIxwDRc3jd+K+FWUZgknLgiLcevaLh/mxV + 98dLxDKGDHHNKc/B7Y4qdlZYv1wfNQVuIbd8dqUQFOOkH7ukbgcGBTxH+2IM67y+ + QBH618luS5Gz1d4bd0YoFf/xZGEh9G5xicz7TiXYzLKjnMjHu2EmbFePABEBAAG0 + LU5hbm5pZSBCZXJuaGFyZCA8bmFubmllLmJlcm5oYXJkQGV4YW1wbGUuY29tPoi4 + BBMBAgAiBQJYrtUnAhsDBgsJCAcDAgYVCAIJCgsEFgIDAQIeAQIXgAAKCRDM++Gf + AKyLHaeSA/99oUWpb02PlfkALcx5RncboMHkgczYEU9wOFIgvXIReswThCMOvPZa + piui+ItyJfV3ijJfO8IvbbFcvU7jjGA073Bb7tbzAEOQLA16mWgBLQlGaRWbHDW4 + uwFxvkJKA0GzEsadEXeniESaZPc4rOXKPO3+/MSQWS2bmvvGsBTEuriNBFiu1ScB + BADIXkITf+kKCkD+n8tMsdTLInefu8KrJ8p7YRYCCabEXnWRsDb5zxUAG2VXCVUh + Yl6QXQybkNiBaduS+uxilz7gtYZUMFJvQ09+fV7D2N9B7u/1bGdIYz+cDFJnEJit + LY4w/nju2Sno5CL5Ead8sZuslKetSXPYHR/kbW462EOw5wARAQABiJ8EGAECAAkF + Aliu1ScCGwwACgkQzPvhnwCsix2WRQQAtOXpBS60myrBUXhlcqabDQgSTw+Spbgb + 61hEMckyzpk7SfMNLz0EbYMvj9SU6znBG8RGeUljPTVMxPGr9yIpoFMSPKAUi/0K + AgRmH3tVpxlMipwXjST1Jukk2eHckt/3jGw3E1ElMSFtULe6u5p4gu578hHukEwT + IKzj0ZyC7DK5AQ0EWcx23AEIANwpAq85bT10JCBuNhOMyF2jKVt5wHbI9wBtjWYG + fgJFBkRvm6IsbmR0Y5DSBvF/of0UX1iGMfx6mvCDJkb1okquhCUef6MONWRpzXYE + CIZDm1TXu6yv0D35tkLfPo+/sY9UHHp1zGRcPAU46e8ztRwoD+zEJwy7lobLHGOL + 9OdWtCGjsutLOTqKRK4jsifr8n3rePU09rejhDkRONNs7ufn9GRcWMN7RWiFDtpU + gNe84AJ38qaXPU8GHNTrDtDtRRPmn68ezMmE1qTNsxQxD4Isexe5Wsfc4+ElaP9s + zaHgij7npX1HS9RpmhnOa2h1ESroM9cqDh3IJVhf+eP6/uMAEQEAAYkBxAQYAQIA + DwUCWcx23AIbAgUJAeEzgAEpCRDM++GfAKyLHcBdIAQZAQIABgUCWcx23AAKCRDk + garE0uOuES7DCAC2Kgl6zO+NqIBIS6McgcEN0sGyvOvZ8Ps4hBiMwCyDAnsIRAUi + v4KZMtQMAyl9njJ3YjPWBsdieuTz45O06DDnrzJpZO5rUGJjAcEue4zvRRWIyu3H + qHC8MsvkslsNCygJHoWlknm+HucroskTNtxHQ+FdKZ6Tey+twl1u+PhV8PQVyFkl + 4G1chO90EP4dvYrye26CC+ik2JkvC7Vy5M+U0PJikme8pFMjcdNks25BnAKcdqKU + AU8RTkSjoYvb8qSmZyldJjYjQRkTPRX1ZdaOID1EdiWl+s5cn0Oypo3z7BChcEMx + IWB/gmXQJQXVr5qNQnJObyMO/RczXYi9qNnyGMED/2EJJERLR0nefjHQalwDKQVP + s5lX1OKAcf2CrV6ZarckqaQgtqjZbuV2C2mrOHUs5uojlXaopj5gA4yJSGDcYhj1 + Rg9jdHWBtkHBj3eL32ZqrHDs3ap8ErZMmPE8A+mn9TTnQS+FY2QF7vBjJKM3qPT7 + DMVGWrg4m1NF8N6yMPMP + =RB1y + -----END PGP PUBLIC KEY BLOCK----- + KEY + end + def primary_keyid fingerprint[-16..-1] end From 866ef2bb2ed601618c77d39f3462ddca9601bc44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Thu, 28 Sep 2017 19:57:49 -0500 Subject: [PATCH 37/61] Add more specs to cover subkeys scenarios --- spec/features/profiles/gpg_keys_spec.rb | 12 ++ spec/lib/gitlab/gpg/commit_spec.rb | 39 +++++ spec/models/gpg_signature_spec.rb | 1 + spec/support/gpg_helpers.rb | 200 ++++++++++++++++++++++++ 4 files changed, 252 insertions(+) diff --git a/spec/features/profiles/gpg_keys_spec.rb b/spec/features/profiles/gpg_keys_spec.rb index b0f6848bc4b..59233e92f93 100644 --- a/spec/features/profiles/gpg_keys_spec.rb +++ b/spec/features/profiles/gpg_keys_spec.rb @@ -20,6 +20,18 @@ feature 'Profile > GPG Keys' do expect(page).to have_content('bette.cartwright@example.net Unverified') expect(page).to have_content(GpgHelpers::User2.fingerprint) end + + scenario 'with multiple subkeys' do + fill_in('Key', with: GpgHelpers::User3.public_key) + click_button('Add key') + + expect(page).to have_content('john.doe@example.com Unverified') + expect(page).to have_content(GpgHelpers::User3.fingerprint) + + GpgHelpers::User3.subkey_fingerprints.each do |fingerprint| + expect(page).to have_content(fingerprint) + end + end end scenario 'User sees their key' do diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb index b07462e4978..a6c99bc07d4 100644 --- a/spec/lib/gitlab/gpg/commit_spec.rb +++ b/spec/lib/gitlab/gpg/commit_spec.rb @@ -63,6 +63,45 @@ describe Gitlab::Gpg::Commit do it_behaves_like 'returns the cached signature on second call' end + context 'commit signed with a subkey' do + let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User3.emails.first } + + let!(:user) { create(:user, email: GpgHelpers::User3.emails.first) } + + let!(:gpg_key) do + create :gpg_key, key: GpgHelpers::User3.public_key, user: user + end + + let(:gpg_key_subkey) do + gpg_key.subkeys.find_by(fingerprint: '0522DD29B98F167CD8421752E38FFCAF75ABD92A') + end + + before do + allow(Rugged::Commit).to receive(:extract_signature) + .with(Rugged::Repository, commit_sha) + .and_return( + [ + GpgHelpers::User3.signed_commit_signature, + GpgHelpers::User3.signed_commit_base_data + ] + ) + end + + it 'returns a valid signature' do + expect(described_class.new(commit).signature).to have_attributes( + commit_sha: commit_sha, + project: project, + gpg_key: gpg_key_subkey, + gpg_key_primary_keyid: gpg_key_subkey.keyid, + gpg_key_user_name: GpgHelpers::User3.names.first, + gpg_key_user_email: GpgHelpers::User3.emails.first, + verification_status: 'verified' + ) + end + + it_behaves_like 'returns the cached signature on second call' + end + context 'user email does not match the committer email, but is the same user' do let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User2.emails.first } diff --git a/spec/models/gpg_signature_spec.rb b/spec/models/gpg_signature_spec.rb index c58fd46762a..91d4efe8e2b 100644 --- a/spec/models/gpg_signature_spec.rb +++ b/spec/models/gpg_signature_spec.rb @@ -4,6 +4,7 @@ RSpec.describe GpgSignature do describe 'associations' do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:gpg_key) } + it { is_expected.to belong_to(:gpg_key_subkey) } end describe 'validation' do diff --git a/spec/support/gpg_helpers.rb b/spec/support/gpg_helpers.rb index a8cfd72954e..4b1ca7ff9c8 100644 --- a/spec/support/gpg_helpers.rb +++ b/spec/support/gpg_helpers.rb @@ -241,4 +241,204 @@ module GpgHelpers ['bette.cartwright@example.com', 'bette.cartwright@example.net'] end end + + # GPG Key with extra signing key + module User3 + extend self + + def signed_commit_signature + <<~SIGNATURE + -----BEGIN PGP SIGNATURE----- + + iQEzBAABCAAdFiEEBSLdKbmPFnzYQhdS44/8r3Wr2SoFAlnNlT8ACgkQ44/8r3Wr + 2SqP1wf9FC4J2S8LIHs/fpxgkYzsyCp5lCbS7JuoD4pqmI2KWyBx+vi9/3mZPCsm + Fj9f0vFEtNOb39GNGZbaA8DdGw30/WAS6kI6yco0WSK53KHrLw9Kqd+3e/NAVSsl + 991Gq4n8X1U5izSH+gZOMtEEUBGqIlZKgRrEh7lhNcz0G7JTF2VCE4NNtZdq7GDA + N6jOQxDGUwi9wQBYORQzIBc3NihfhGloII1hXf0XzrgUY3zNYHTT7QipCxKAmH54 + skwW+wi8RpBedar4saf7fs5xZbP/0yyVz98MJMdHBL68++Xt1AIHoqrb7eWszqnd + PCo/fnz1iHKCig602KLj0/zhADcNkg== + =LsTi + -----END PGP SIGNATURE----- + SIGNATURE + end + + def signed_commit_base_data + <<~SIGNEDDATA + tree 86ec18bfe87ad42a782fdabd8310f9b7ac750f51 + parent 2d1096e3a0ecf1d2baf6dee036cc80775d4940ba + author John Doe 1506645311 -0500 + committer John Doe 1506645311 -0500 + + Commit signed with subkey by John Doe + SIGNEDDATA + end + + def public_key + <<~KEY.strip + -----BEGIN PGP PUBLIC KEY BLOCK----- + + mQENBFnNgbIBCAC9/WblcR4s/pFTwh9cm2iS59YRhtGfbrnfNE6QMIFIRFaK0u6J + LDy+scipXLoGg7X0XNFLW6Y457UUpkaPDVLPuVMONhMXdVqndGVvk9jq3D4oDtRa + ukucuXr9F84lHnjL3EosmAUiK3xBmHOGHm8+bvKbgPOdvre48YxoJ1POTen+chfo + YtLUfnC9EEGY/bn00aaXm3NV+zZK2zio5bFX9YLWSNh/JaXxuJsLoHR/lVrU7CLt + FCaGcPQ9SU46LHPshEYWO7ZsjEYJsYYOIOEzfcfR47T2EDTa6mVc++gC5TCoY3Ql + jccgm+EM0LvyEHwupEpxzCg2VsT0yoedhUhtABEBAAG0H0pvaG4gRG9lIDxqb2hu + LmRvZUBleGFtcGxlLmNvbT6JAVQEEwEIAD4WIQTqP4uIlyqP1HSHLy8RWzrxqtPt + ugUCWc2BsgIbAwUJA8JnAAULCQgHAgYVCAkKCwIEFgIDAQIeAQIXgAAKCRARWzrx + qtPturMDCACc1Pi1sLJFcCnJEc9sCInCO4LH8fntNjRLN3MTPU5YCYmFM3fAl5ly + vXPZ4jNWZxKbQVeFnkDOg5Ti8bzmFEMc8KbZuguktVFizxnLdFCTTRO89i3HDVSN + bIosrs5HJwRKOzul6i2whn3dsr8/P8WJczYjZGiw29hGwH3md4Thn/aAGbzjoCEF + IfIb1kccyHMJkaj79S8B2agsbEJLuTSfsXC3kGZIJyKG1DNmDUHW/ZE6ro/Kkhik + 3w6Jw14cMsKUIOBkOgsD/gXgX9xxWjYHmKrbCXucTKUevNlaCy5tzwrC0Am3prl9 + OJj3macOA8hNaTVDflEHNCwHOwqnVQYyuQENBFnNgbIBCAC59MmKc0cqPRPTpCZ5 + arKRoj23SNKWMDWaxSELdU91Wd/NJk4wF25urk9BtBuwjzaBMqb/xPD88yJC3ucs + 2LwCyAb5C/dHcPOpys8Pd+KrdHDR3zAMjcASsizlW/qFI9MtjhcU9Yd6iTUejAZG + NEC76WALJ3SLYzCaDkHFjWpH3Xq6ck3/9jpL3csn/5GLCsZQUDYGrZSXvHAIigwW + Xo6tMs5LCCO9CZg2qGDpvqlzcmy6CRkf0h/UFYJzGqfbJtxeCIxa93WIPE8eGwao + aneDlNtIoYiP6krC3OLsaPWT58QltNKaQuZSpjwtQBHa4JIt55vx+FcvRb7Kflgf + fT8bABEBAAGJATwEGAEIACYWIQTqP4uIlyqP1HSHLy8RWzrxqtPtugUCWc2BsgIb + DAUJA8JnAAAKCRARWzrxqtPtuqJjCACj+Z4qtgMpJXx3u58wCzkVLl5IylD/tEPA + cNIrj8QS8ec+woTJaMGVCh96VC2FPl8KR4Hjhy0yaupyPbTI6VWib63S/NcDfG7r + tviRFG2Gf8yduERebyC0cpgnmjVgFfJs7N3K3ncz6myOr9idNI05OC9poL73sDUv + jRXhm7uy938bT/R4MQdpYuxucgU3MiwvfG5ht+oJ4Yp+/IrR2PTqRGqMCsodnroa + TBKq2kW565TtCvrFkNub/ytorDbIVN9VrIMcuTiOv8sLoQRDJO9HvWUhYAqMY6Uh + dy12jR9FrquZnGsDKKs9V0Y6J4Wi8vnmdgWVZUc40pfXq3APAB6suQENBFnNgeAB + CADFqQRxLHxLIQ7B72diTMI2tPk9d5c67k+Gzkrg1QYoxBLdRCmhM4ydYqZzvIz4 + 1npkK20w4somOCwvyAOjO46IGb3f/Wk8mY8o5HMpI1miAfze0YTZKzRo2DmrvwbV + /h8jvZNCISwtrOgaaszWSVSuEQQCA1jf4qixfCb3ReETvZc3MTZXhw8IUbszXh5d + a6CYqq/fr5Zw4Dc19ZSoHSTh0Wn03mEm/kaYtia/wt1I+xVvTSaC2Pf/kUtr7UEf + 3NMc0YF0s4KgeW8KwjHa7Sz9wzydLPRH5kJ26SDUGUhrFf1jNLIegtDsoISV/86G + ztXcVq5GY6lXMwmsggRe++7xABEBAAGJAmwEGAEIACAWIQTqP4uIlyqP1HSHLy8R + WzrxqtPtugUCWc2B4AIbAgFACRARWzrxqtPtusB0IAQZAQgAHRYhBAUi3Sm5jxZ8 + 2EIXUuOP/K91q9kqBQJZzYHgAAoJEOOP/K91q9kqlHcH+wbvD14ITYDMfgIfy67O + 4Qcmgf1qzGXhpsABz/i/EPgRD990eNBI0YGuvoKRJfetEGn7LerrrCB8Z+ICFPHF + rzXoe10zm+QTREck0OB8nPFRycJ+Fbl6JX+cnkEx27Mmg1aVb7+H5LMDaWO1KjLs + g2wIDo/jrDfW7NoZzy4XTd7jFCOt4fftL/dhiujQmhLzugZXCxRISOVdmgilDJQo + Tz1sEm34ida98JFjdzSgkUvJ/pFTZ21ThCNxlUf01Hr2Pdcg1e2/97sZocMFTY2J + KwmiW2LG3B05/VrRtdvsCVj8G49coxgPPKty+m71ovAXdS/CvVqE7TefCplsYJ1d + V3abwwf/Xl2SxzbAKbrYMgZfdGzpPg2u6982WvfGIVfjFJh9veHZAbfkPcjdAD2X + e67Y4BeKG2OQQqeOY2y+81A7PaehgHzbFHJG/4RjqB66efrZAg4DgIdbr4oyMoUJ + VVsl0wfYSIvnd4kvWXYICVwk53HLA3wIowZAsJ1LT2byAKbUzayLzTekrTzGcwQk + g2XT798ev2QuR5Ki5x8MULBFX4Lhd03+uGOxjhNPQD6DAAHCQLaXQhaGuyMgt5hD + t0nF3yuw3Eg4Ygcbtm24rZXOHJc9bDKeRiWZz1dIyYYVJmHSQwOVXkAwJlF1sIgy + /jQYeOgFDKq20x86WulkvsUtBoaZJg== + =Q5Z7 + -----END PGP PUBLIC KEY BLOCK----- + KEY + end + + def secret_key + <<~SECRET + -----BEGIN PGP PRIVATE KEY BLOCK----- + + lQPGBFnNgbIBCAC9/WblcR4s/pFTwh9cm2iS59YRhtGfbrnfNE6QMIFIRFaK0u6J + LDy+scipXLoGg7X0XNFLW6Y457UUpkaPDVLPuVMONhMXdVqndGVvk9jq3D4oDtRa + ukucuXr9F84lHnjL3EosmAUiK3xBmHOGHm8+bvKbgPOdvre48YxoJ1POTen+chfo + YtLUfnC9EEGY/bn00aaXm3NV+zZK2zio5bFX9YLWSNh/JaXxuJsLoHR/lVrU7CLt + FCaGcPQ9SU46LHPshEYWO7ZsjEYJsYYOIOEzfcfR47T2EDTa6mVc++gC5TCoY3Ql + jccgm+EM0LvyEHwupEpxzCg2VsT0yoedhUhtABEBAAH+BwMCOqjIWtWBMo3mjIP1 + OnIbZ+YJxSUZ/B8wU2loMv4XiKmeXLbjD6h3jojxYlnreSHA9QvoY8uNaWElL/n2 + jv6bxluivk8tA9FWJVv4HaSlMDd2J2YmUW17r8z9Kvm7b7pFVSrEoYV93Wdj5FJ7 + ciKrFhYNSD7tH1sHwkrFAbiv6aHyk9h48YmR3kx2wBvz+pWk7M2srCJx2b6DXkj/ + fsj1c/vnzUUGooOJgOvYAWrpg/rJUNxSsFypAHf8Xtk+xt8S1aZ9jaCmYk6I1B2L + m00HP43cXUpKcmETW1zXvfMLKjjoUEAJhSJhbCwiEzGL4ojQTarl8qbb+MisakEJ + DkPYtrhiiuVzUIFfqE86yO0UKidtzBmJAW3c6zeiUATvACzU09aGyUY1cJi93oXD + w4PCyVZ+nMvGD1wx+gyYdDINwpX4y6od9RDr06DGCzwu+S2vxsu1T8LdSv52fhBr + U0FY3Z3VN1ytay4SHi/8Y9VBYQFBh7R7Ch0gEMxLVKXVNqOXHUdGrKWV/WmyLKuZ + W9DEnWU4Mpz/di5jU8EDW7EB9DZZhVk3mQw3nuAZrBGD4azmmD5mgSgLeBGmKZ1e + q/9IWO44mRBBUtNv+rAkmmYF3MCNHuc7EMj+c/IgBUC7d5qBzGWA3UJ0vKX4xcIQ + X/PnU+nGxNvBrdqQaMLczeg28SerojxuX79prOsoySctLAbajd9HshW5SfOZ0rvb + BNHPqolQDijYEHGxANh4BbamRMGi60Rop7vJsZOLAemz17x/mvCtAHISOJT77/IM + oWC+IksJ5XsA/klJGe/tkx11aRQDDmKvIJXmMuRdvnIR23UBbzRQlWWq0l6CdoF6 + 6SQ9BJBFq0WY32No9WZAPnDO3buUzWc1Y3uwn/+h7TVYVyTlEqzpYJ9FoJwBHbor + 0663eoyz6+AUtB9Kb2huIERvZSA8am9obi5kb2VAZXhhbXBsZS5jb20+iQFUBBMB + CAA+FiEE6j+LiJcqj9R0hy8vEVs68arT7boFAlnNgbICGwMFCQPCZwAFCwkIBwIG + FQgJCgsCBBYCAwECHgECF4AACgkQEVs68arT7bqzAwgAnNT4tbCyRXApyRHPbAiJ + wjuCx/H57TY0SzdzEz1OWAmJhTN3wJeZcr1z2eIzVmcSm0FXhZ5AzoOU4vG85hRD + HPCm2boLpLVRYs8Zy3RQk00TvPYtxw1UjWyKLK7ORycESjs7peotsIZ93bK/Pz/F + iXM2I2RosNvYRsB95neE4Z/2gBm846AhBSHyG9ZHHMhzCZGo+/UvAdmoLGxCS7k0 + n7Fwt5BmSCcihtQzZg1B1v2ROq6PypIYpN8OicNeHDLClCDgZDoLA/4F4F/ccVo2 + B5iq2wl7nEylHrzZWgsubc8KwtAJt6a5fTiY95mnDgPITWk1Q35RBzQsBzsKp1UG + Mp0DxgRZzYGyAQgAufTJinNHKj0T06QmeWqykaI9t0jSljA1msUhC3VPdVnfzSZO + MBdubq5PQbQbsI82gTKm/8Tw/PMiQt7nLNi8AsgG+Qv3R3DzqcrPD3fiq3Rw0d8w + DI3AErIs5Vv6hSPTLY4XFPWHeok1HowGRjRAu+lgCyd0i2Mwmg5BxY1qR916unJN + //Y6S93LJ/+RiwrGUFA2Bq2Ul7xwCIoMFl6OrTLOSwgjvQmYNqhg6b6pc3JsugkZ + H9If1BWCcxqn2ybcXgiMWvd1iDxPHhsGqGp3g5TbSKGIj+pKwtzi7Gj1k+fEJbTS + mkLmUqY8LUAR2uCSLeeb8fhXL0W+yn5YH30/GwARAQAB/gcDAuYn/gmAA3OC5p5Q + Pat5kE7MtmSvSPmdjVA2o+6RtqZf81JqtAgtDVDwj7SPFsH6ue5P+iAn9938YYek + WQU2+0GXeUbSJt+u4VAchgwA5mYsEnEr1/E5KEfWPWO3jJol1rJG99adrjkMxvug + QJmwieqhu0368w1FU0tKstxYbr3Tz3nPCPDJoigMEUkXiFklDCUgeNk0g+zd5ytE + lXuuLYcGZX7njxL5jD+cMIKqua5zv8WbvNf/BhM1nCarxp4qzKWim8J8jY+iR+/E + qOar4aliGRez0j+qh/r8ywgPwfOO89zrKrMfaclL7dN9yuecmBHKWZvfrP5JKMHj + yTU3nRMhUGbfVUaaZI2Ccz2rNOU4oF9wuzpzQi8qOysZixRmH61Nw3ULIKoQgiWp + 0p5A3L94OaEfZEq3plVaIXI2YWYFSEAlIAc2dq4GxynousLdhNACi9bHhXrfFUhK + ckw1QlbhguO/j63/x8ygsmLZVwHG0fJZtMhT3+EGam9cuMBibIYyu3ufJRy7kMKt + kmyuk02X+hYJ7w8Pu6b8zYHBXbsEKamipMgd4oKtc8WnXILZo4lwDSArgs7ZVCBa + vGBbpTOsr54WjsyuCdX/wv0F2l31J87UxVtTKXx/+nfMfCE02zd+NsTgqvgqmkaA + Sy3qvv326kJNx7p+5hRwDzlAZ7vGJjj5TwCbGYDvctIf6MFrGDRNYwrGwNkPc3TG + rturfeL/ioua0Smj8LIbOv9Ir93gUIseNpxv8tXV/lffdIplcw802b3aXIKyv4fq + b9y3Oq/pDHFukKuBe9WTXJvjT0+ME+a0C8KIb/sts95pmjZsgN1kPmvuT0ReQwUR + eGrqz387bnVUzo4RgM3IERs/0EYzPzE8A2vc1e4/87b5J+1Xnov8Phd29vW8Td5l + ApiFrFO2r+/Np4kBPAQYAQgAJhYhBOo/i4iXKo/UdIcvLxFbOvGq0+26BQJZzYGy + AhsMBQkDwmcAAAoJEBFbOvGq0+26omMIAKP5niq2AyklfHe7nzALORUuXkjKUP+0 + Q8Bw0iuPxBLx5z7ChMlowZUKH3pULYU+XwpHgeOHLTJq6nI9tMjpVaJvrdL81wN8 + buu2+JEUbYZ/zJ24RF5vILRymCeaNWAV8mzs3credzPqbI6v2J00jTk4L2mgvvew + NS+NFeGbu7L3fxtP9HgxB2li7G5yBTcyLC98bmG36gnhin78itHY9OpEaowKyh2e + uhpMEqraRbnrlO0K+sWQ25v/K2isNshU31Wsgxy5OI6/ywuhBEMk70e9ZSFgCoxj + pSF3LXaNH0Wuq5mcawMoqz1XRjonhaLy+eZ2BZVlRzjSl9ercA8AHqydA8YEWc2B + 4AEIAMWpBHEsfEshDsHvZ2JMwja0+T13lzruT4bOSuDVBijEEt1EKaEzjJ1ipnO8 + jPjWemQrbTDiyiY4LC/IA6M7jogZvd/9aTyZjyjkcykjWaIB/N7RhNkrNGjYOau/ + BtX+HyO9k0IhLC2s6BpqzNZJVK4RBAIDWN/iqLF8JvdF4RO9lzcxNleHDwhRuzNe + Hl1roJiqr9+vlnDgNzX1lKgdJOHRafTeYSb+Rpi2Jr/C3Uj7FW9NJoLY9/+RS2vt + QR/c0xzRgXSzgqB5bwrCMdrtLP3DPJ0s9EfmQnbpINQZSGsV/WM0sh6C0OyghJX/ + zobO1dxWrkZjqVczCayCBF777vEAEQEAAf4HAwKESvCIDq5QNeadnSvpkZemItPO + lDf+7Wiue2gt776D5xkVyT7WkgTQv+IGWGtqz7pCCO2rMp/F9u1BghdjY46jtrK6 + MMFKta4YENUhRliH6M2YmRjq5p7xZgH6UOnDlqsafbfyUx30t59tbQj+07aMnH5J + LMm37nVkDvo3wpPQPuo7L6qizYsrHrQKeJZ8636u41UjC99lVH7vXzqXw68FJImi + XdMZbEVBIprYfCDem+fD6gJBA4JBqWJMxuFMfhWp+1WtYoeNojDm4KxBzc2fvYV/ + HOIUfLFBvACD/UwU5ovllHN39/O8SMgyLm9ymx2/qXcdIkUz4l7fhOCY1OW12DMu + 5OFrrTteGK/Sj4Z8pYRdMdaKyjIlxuVzEQGWsU5+J2ALao5atEHguqwlD3cKh3G8 + 1sA/l5eTFDt84erYv1MVStV0BhZaCE4mNL4WpnQGDdW05yoGq9jIyLcurb/k/atU + TUkAF1csgNlJlR3IP+7U9xfHkjMO5+SV82xoNf9nBjz06TRdnvOSKsMNKp0RxC/L + Hbiee9o7Rxqdiyv0ly6bCCymwfvlsEIqo3YKssBfe3XI5yQI2hF9QZaH1ywzmgaH + o+rbME/XxddRJueS79eipT7K05Z3ulSHTXzpDw+jZcdUV0Ac72Q9FTDPMl3xc6NW + DrYwWw/3+kyZ4SkP56l7KlGczTyNPvU9iou4Cj/cAZk/pHx68Chq8ZZNznFm/bIF + gWt3fqE/n+y78B6MI8qTjGJOR0jycxrLH82Z2F+FpMShI2C5NnOa/Ilkv3e2Q5U6 + MOAwaCIz6RHhcI99O/yta2vLelWZqn2g86rLzTG0HlIABTCPYotwetHh0hsrkSv9 + Kh6rOzGB4i8lRqcBVY+alMSiRBlzkwpL4YUcO6f3vEDncQ9evE1AQCpD4jUJjB1H + JSSJAmwEGAEIACAWIQTqP4uIlyqP1HSHLy8RWzrxqtPtugUCWc2B4AIbAgFACRAR + WzrxqtPtusB0IAQZAQgAHRYhBAUi3Sm5jxZ82EIXUuOP/K91q9kqBQJZzYHgAAoJ + EOOP/K91q9kqlHcH+wbvD14ITYDMfgIfy67O4Qcmgf1qzGXhpsABz/i/EPgRD990 + eNBI0YGuvoKRJfetEGn7LerrrCB8Z+ICFPHFrzXoe10zm+QTREck0OB8nPFRycJ+ + Fbl6JX+cnkEx27Mmg1aVb7+H5LMDaWO1KjLsg2wIDo/jrDfW7NoZzy4XTd7jFCOt + 4fftL/dhiujQmhLzugZXCxRISOVdmgilDJQoTz1sEm34ida98JFjdzSgkUvJ/pFT + Z21ThCNxlUf01Hr2Pdcg1e2/97sZocMFTY2JKwmiW2LG3B05/VrRtdvsCVj8G49c + oxgPPKty+m71ovAXdS/CvVqE7TefCplsYJ1dV3abwwf/Xl2SxzbAKbrYMgZfdGzp + Pg2u6982WvfGIVfjFJh9veHZAbfkPcjdAD2Xe67Y4BeKG2OQQqeOY2y+81A7Paeh + gHzbFHJG/4RjqB66efrZAg4DgIdbr4oyMoUJVVsl0wfYSIvnd4kvWXYICVwk53HL + A3wIowZAsJ1LT2byAKbUzayLzTekrTzGcwQkg2XT798ev2QuR5Ki5x8MULBFX4Lh + d03+uGOxjhNPQD6DAAHCQLaXQhaGuyMgt5hDt0nF3yuw3Eg4Ygcbtm24rZXOHJc9 + bDKeRiWZz1dIyYYVJmHSQwOVXkAwJlF1sIgy/jQYeOgFDKq20x86WulkvsUtBoaZ + Jg== + =TKlF + -----END PGP PRIVATE KEY BLOCK----- + SECRET + end + + def fingerprint + 'EA3F8B88972A8FD474872F2F115B3AF1AAD3EDBA' + end + + def subkey_fingerprints + %w(159AD5DDF199591D67D2B87AA3CEC5C0A7C270EC 0522DD29B98F167CD8421752E38FFCAF75ABD92A) + end + + def names + ['John Doe'] + end + + def emails + ['john.doe@example.com'] + end + end end From 59f813998237a3840fd019ef7c552e86f2753dff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Thu, 28 Sep 2017 20:16:53 -0500 Subject: [PATCH 38/61] Remove unused association --- app/models/gpg_key.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index e6c862d9b55..0ddf1245dd1 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -8,7 +8,6 @@ class GpgKey < ActiveRecord::Base sha_attribute :fingerprint belongs_to :user - belongs_to :parent, class_name: 'GpgKey' has_many :gpg_signatures has_many :subkeys, class_name: 'GpgKeySubkey' From c2c35ae7971d19396078bdec6474fdd58f66000c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Fri, 29 Sep 2017 00:24:03 -0500 Subject: [PATCH 39/61] Consider GPG subkeys when trying to update invalid GPG signatures --- lib/gitlab/gpg/invalid_gpg_signature_updater.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/gpg/invalid_gpg_signature_updater.rb b/lib/gitlab/gpg/invalid_gpg_signature_updater.rb index e085eab26c9..9bad914848d 100644 --- a/lib/gitlab/gpg/invalid_gpg_signature_updater.rb +++ b/lib/gitlab/gpg/invalid_gpg_signature_updater.rb @@ -3,13 +3,14 @@ module Gitlab class InvalidGpgSignatureUpdater def initialize(gpg_key) @gpg_key = gpg_key + @gpg_keyids = gpg_key.subkeys.map(&:keyid).push(gpg_key.primary_keyid) end def run GpgSignature .select(:id, :commit_sha, :project_id) .where('gpg_key_id IS NULL OR verification_status <> ?', GpgSignature.verification_statuses[:verified]) - .where(gpg_key_primary_keyid: @gpg_key.primary_keyid) + .where(gpg_key_primary_keyid: @gpg_keyids) .find_each { |sig| sig.gpg_commit.update_signature!(sig) } end end From c50725fecfb776d56c95ef070940ca6c85786ecf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Fri, 29 Sep 2017 17:55:36 -0500 Subject: [PATCH 40/61] Address feedback from last code review --- app/models/gpg_key.rb | 5 +++++ app/models/gpg_key_subkey.rb | 3 +++ app/models/gpg_signature.rb | 2 +- lib/gitlab/gpg.rb | 11 +++++------ lib/gitlab/gpg/commit.rb | 8 +------- lib/gitlab/gpg/invalid_gpg_signature_updater.rb | 3 +-- 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 0ddf1245dd1..27513f4b03f 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -44,6 +44,7 @@ class GpgKey < ActiveRecord::Base def primary_keyid super&.upcase end + alias_method :keyid, :primary_keyid def fingerprint super&.upcase @@ -53,6 +54,10 @@ class GpgKey < ActiveRecord::Base super(value&.strip) end + def keyids + [keyid].concat(subkeys.map(&:keyid)) + end + def user_infos @user_infos ||= Gitlab::Gpg.user_infos_from_key(key) end diff --git a/app/models/gpg_key_subkey.rb b/app/models/gpg_key_subkey.rb index b4f146e5647..1f3ec2a8f68 100644 --- a/app/models/gpg_key_subkey.rb +++ b/app/models/gpg_key_subkey.rb @@ -9,6 +9,9 @@ class GpgKeySubkey < ActiveRecord::Base validates :gpg_key_id, presence: true validates :fingerprint, :keyid, presence: true, uniqueness: true + delegate :key, :user, :user_infos, :verified?, :verified_user_infos, + :verified_and_belongs_to_email?, to: :gpg_key + def keyid super&.upcase end diff --git a/app/models/gpg_signature.rb b/app/models/gpg_signature.rb index c7f75288407..d3cca19cea8 100644 --- a/app/models/gpg_signature.rb +++ b/app/models/gpg_signature.rb @@ -24,7 +24,7 @@ class GpgSignature < ActiveRecord::Base def gpg_key=(model) case model when GpgKey then super - when GpgKeySubkey then write_attribute(:gpg_key_subkey_id, model.id) + when GpgKeySubkey then self.gpg_key_subkey = model end end diff --git a/lib/gitlab/gpg.rb b/lib/gitlab/gpg.rb index 343bf54a6ae..413872d7e08 100644 --- a/lib/gitlab/gpg.rb +++ b/lib/gitlab/gpg.rb @@ -36,15 +36,14 @@ module Gitlab def subkeys_from_key(key) using_tmp_keychain do - fingerprints = CurrentKeyChain.fingerprints_from_key(key) - raw_keys = GPGME::Key.find(:public, fingerprints) - grouped_subkeys = Hash.new { |h, k| h[k] = [] } + fingerprints = CurrentKeyChain.fingerprints_from_key(key) + raw_keys = GPGME::Key.find(:public, fingerprints) - raw_keys.each_with_object(grouped_subkeys).each do |raw_key, subkeys| + raw_keys.each_with_object({}) do |raw_key, grouped_subkeys| primary_subkey_id = raw_key.primary_subkey.keyid - raw_key.subkeys[1..-1].each do |subkey| - subkeys[primary_subkey_id] << { keyid: subkey.keyid, fingerprint: subkey.fingerprint } + grouped_subkeys[primary_subkey_id] = raw_key.subkeys[1..-1].map do |s| + { keyid: s.keyid, fingerprint: s.fingerprint } end end end diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb index 5cbc836314f..961c57ec0e6 100644 --- a/lib/gitlab/gpg/commit.rb +++ b/lib/gitlab/gpg/commit.rb @@ -74,7 +74,7 @@ module Gitlab commit_sha: @commit.sha, project: @commit.project, gpg_key: gpg_key, - gpg_key_primary_keyid: gpg_keyid(gpg_key) || verified_signature.fingerprint, + gpg_key_primary_keyid: gpg_key&.keyid || verified_signature.fingerprint, gpg_key_user_name: user_infos[:name], gpg_key_user_email: user_infos[:email], verification_status: verification_status @@ -99,12 +99,6 @@ module Gitlab gpg_key&.verified_user_infos&.first || gpg_key&.user_infos&.first || {} end - def gpg_keyid(gpg_key) - return nil unless gpg_key - - gpg_key.is_a?(GpgKey) ? gpg_key.primary_keyid : gpg_key.keyid - end - def find_gpg_key(keyid) GpgKey.find_by(primary_keyid: keyid) || GpgKeySubkey.find_by(keyid: keyid) end diff --git a/lib/gitlab/gpg/invalid_gpg_signature_updater.rb b/lib/gitlab/gpg/invalid_gpg_signature_updater.rb index 9bad914848d..b7fb9dde2bc 100644 --- a/lib/gitlab/gpg/invalid_gpg_signature_updater.rb +++ b/lib/gitlab/gpg/invalid_gpg_signature_updater.rb @@ -3,14 +3,13 @@ module Gitlab class InvalidGpgSignatureUpdater def initialize(gpg_key) @gpg_key = gpg_key - @gpg_keyids = gpg_key.subkeys.map(&:keyid).push(gpg_key.primary_keyid) end def run GpgSignature .select(:id, :commit_sha, :project_id) .where('gpg_key_id IS NULL OR verification_status <> ?', GpgSignature.verification_statuses[:verified]) - .where(gpg_key_primary_keyid: @gpg_keyids) + .where(gpg_key_primary_keyid: @gpg_key.keyids) .find_each { |sig| sig.gpg_commit.update_signature!(sig) } end end From 85c6652a387276ca58b900fa34948cf3cba489a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Sun, 1 Oct 2017 21:50:38 -0500 Subject: [PATCH 41/61] Fix some broken specs --- app/models/gpg_key.rb | 2 +- spec/factories/gpg_key_subkeys.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 27513f4b03f..145b957fc88 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -119,7 +119,7 @@ class GpgKey < ActiveRecord::Base def generate_subkeys gpg_subkeys = Gitlab::Gpg.subkeys_from_key(key) - gpg_subkeys[primary_keyid].each do |subkey_data| + gpg_subkeys[primary_keyid]&.each do |subkey_data| subkeys.create!(keyid: subkey_data[:keyid], fingerprint: subkey_data[:fingerprint]) end end diff --git a/spec/factories/gpg_key_subkeys.rb b/spec/factories/gpg_key_subkeys.rb index d896b4e217e..66ecb44d84b 100644 --- a/spec/factories/gpg_key_subkeys.rb +++ b/spec/factories/gpg_key_subkeys.rb @@ -4,7 +4,7 @@ FactoryGirl.define do factory :gpg_key_subkey do gpg_key - keyid { gpg_key.subkeys.last.keyid } - fingerprint { gpg_key.subkeys.last.fingerprint } + sequence(:keyid) { |n| "keyid-#{n}" } + sequence(:fingerprint) { |n| "fingerprint-#{n}" } end end From ecfe6292d3a5d40d7179cf763fb60cf73adb30e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Mon, 2 Oct 2017 20:46:19 -0500 Subject: [PATCH 42/61] Add migration to generate GpgKeySubkeys records and update signatures --- ...e_gpg_key_subkeys_for_existing_gpg_keys.rb | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 db/migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb diff --git a/db/migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb b/db/migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb new file mode 100644 index 00000000000..355fbfbbede --- /dev/null +++ b/db/migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb @@ -0,0 +1,59 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class CreateGpgKeySubkeysForExistingGpgKeys < ActiveRecord::Migration + disable_ddl_transaction! + + DOWNTIME = false + + class GpgKey < ActiveRecord::Base + self.table_name = 'gpg_keys' + + include EachBatch + include ShaAttribute + + sha_attribute :primary_keyid + sha_attribute :fingerprint + + has_many :subkeys, class_name: 'GpgKeySubkey' + end + + class GpgKeySubkey < ActiveRecord::Base + self.table_name = 'gpg_key_subkeys' + + include ShaAttribute + + sha_attribute :keyid + sha_attribute :fingerprint + end + + def up + GpgKey.each_batch do |batch| + batch.each do |gpg_key| + create_subkeys(gpg_key) && update_signatures(gpg_key) + end + end + end + + def down + end + + private + + def create_subkeys(gpg_key) + gpg_subkeys = Gitlab::Gpg.subkeys_from_key(gpg_key.key) + + gpg_subkeys[gpg_key.primary_keyid.upcase]&.each do |subkey_data| + gpg_key.subkeys.build(keyid: subkey_data[:keyid], fingerprint: subkey_data[:fingerprint]) + end + + # Improve latency by doing all INSERTs in a single call + GpgKey.transaction do + gpg_key.save! + end + end + + def update_signatures(gpg_key) + InvalidGpgSignatureUpdateWorker.perform_async(gpg_key.id) + end +end From 8b0397041866c0e2bcbf4c462100fc681d96f893 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Mon, 2 Oct 2017 20:51:49 -0500 Subject: [PATCH 43/61] Add CHANGELOG entry --- .../unreleased/36829-add-ability-to-verify-gpg-subkeys | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/36829-add-ability-to-verify-gpg-subkeys diff --git a/changelogs/unreleased/36829-add-ability-to-verify-gpg-subkeys b/changelogs/unreleased/36829-add-ability-to-verify-gpg-subkeys new file mode 100644 index 00000000000..ee6a7287e86 --- /dev/null +++ b/changelogs/unreleased/36829-add-ability-to-verify-gpg-subkeys @@ -0,0 +1,5 @@ +--- +title: Add support for GPG subkeys in signature verification +merge_request: 14517 +author: +type: added From 8d296b62e4534820feb94cfe0d8b5d3e34f6ee33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Mon, 2 Oct 2017 21:08:59 -0500 Subject: [PATCH 44/61] Convert migration to a post deployment migration This migration can take a bit of time to complete and I don't want to delay the deployment process. --- ...20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename db/{migrate => post_migrate}/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb (100%) diff --git a/db/migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb b/db/post_migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb similarity index 100% rename from db/migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb rename to db/post_migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb From 8be06f20fc64b027911ad75e05afaf0697444d6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Mon, 2 Oct 2017 21:14:44 -0500 Subject: [PATCH 45/61] Remove no longer required methods --- app/models/gpg_key_subkey.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/app/models/gpg_key_subkey.rb b/app/models/gpg_key_subkey.rb index 1f3ec2a8f68..b57922aba30 100644 --- a/app/models/gpg_key_subkey.rb +++ b/app/models/gpg_key_subkey.rb @@ -19,14 +19,4 @@ class GpgKeySubkey < ActiveRecord::Base def fingerprint super&.upcase end - - def method_missing(m, *a, &b) - return super unless gpg_key.respond_to?(m) - - gpg_key.public_send(m, *a, &b) # rubocop:disable GitlabSecurity/PublicSend - end - - def respond_to_missing?(method, include_private = false) - gpg_key.respond_to?(method, include_private) || super - end end From 2577cc99818bd0332aa78018de666579971341c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 4 Oct 2017 10:34:50 -0500 Subject: [PATCH 46/61] Address some feedback from last code review --- app/models/gpg_signature.rb | 9 +++++++-- ...bkeys => 36829-add-ability-to-verify-gpg-subkeys.yml} | 0 db/migrate/20170927161718_create_gpg_key_subkeys.rb | 4 ++-- ...61539_create_gpg_key_subkeys_for_existing_gpg_keys.rb | 4 +++- db/schema.rb | 2 +- lib/gitlab/gpg/commit.rb | 2 ++ 6 files changed, 15 insertions(+), 6 deletions(-) rename changelogs/unreleased/{36829-add-ability-to-verify-gpg-subkeys => 36829-add-ability-to-verify-gpg-subkeys.yml} (100%) diff --git a/app/models/gpg_signature.rb b/app/models/gpg_signature.rb index d3cca19cea8..e783c3b24e4 100644 --- a/app/models/gpg_signature.rb +++ b/app/models/gpg_signature.rb @@ -23,8 +23,13 @@ class GpgSignature < ActiveRecord::Base def gpg_key=(model) case model - when GpgKey then super - when GpgKeySubkey then self.gpg_key_subkey = model + when GpgKey + super + when GpgKeySubkey + self.gpg_key_subkey = model + when NilClass + super + self.gpg_key_subkey = nil end end diff --git a/changelogs/unreleased/36829-add-ability-to-verify-gpg-subkeys b/changelogs/unreleased/36829-add-ability-to-verify-gpg-subkeys.yml similarity index 100% rename from changelogs/unreleased/36829-add-ability-to-verify-gpg-subkeys rename to changelogs/unreleased/36829-add-ability-to-verify-gpg-subkeys.yml diff --git a/db/migrate/20170927161718_create_gpg_key_subkeys.rb b/db/migrate/20170927161718_create_gpg_key_subkeys.rb index ffe06ce1231..c03c40416a8 100644 --- a/db/migrate/20170927161718_create_gpg_key_subkeys.rb +++ b/db/migrate/20170927161718_create_gpg_key_subkeys.rb @@ -3,11 +3,11 @@ class CreateGpgKeySubkeys < ActiveRecord::Migration def up create_table :gpg_key_subkeys do |t| + t.references :gpg_key, null: false, index: true, foreign_key: { on_delete: :cascade } + t.binary :keyid t.binary :fingerprint - t.references :gpg_key, null: false, index: true, foreign_key: { on_delete: :cascade } - t.index :keyid, unique: true, length: Gitlab::Database.mysql? ? 20 : nil t.index :fingerprint, unique: true, length: Gitlab::Database.mysql? ? 20 : nil end diff --git a/db/post_migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb b/db/post_migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb index 355fbfbbede..346dfb1a4b6 100644 --- a/db/post_migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb +++ b/db/post_migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb @@ -28,8 +28,10 @@ class CreateGpgKeySubkeysForExistingGpgKeys < ActiveRecord::Migration end def up - GpgKey.each_batch do |batch| + GpgKey.with_subkeys.each_batch do |batch| batch.each do |gpg_key| + return if gpg_key.subkeys.any? + create_subkeys(gpg_key) && update_signatures(gpg_key) end end diff --git a/db/schema.rb b/db/schema.rb index b9de70b742a..3bcfbcc3fd1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -580,9 +580,9 @@ ActiveRecord::Schema.define(version: 20171004121444) do add_index "forked_project_links", ["forked_to_project_id"], name: "index_forked_project_links_on_forked_to_project_id", unique: true, using: :btree create_table "gpg_key_subkeys", force: :cascade do |t| + t.integer "gpg_key_id", null: false t.binary "keyid" t.binary "fingerprint" - t.integer "gpg_key_id", null: false end add_index "gpg_key_subkeys", ["fingerprint"], name: "index_gpg_key_subkeys_on_fingerprint", unique: true, using: :btree diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb index 961c57ec0e6..0f4ba6f83fc 100644 --- a/lib/gitlab/gpg/commit.rb +++ b/lib/gitlab/gpg/commit.rb @@ -43,6 +43,8 @@ module Gitlab # key belonging to the keyid. # This way we can add the key to the temporary keychain and extract # the proper signature. + # NOTE: the invoked method is #fingerprint but it's only returning + # 16 characters (the format used by keyid) instead of 40. gpg_key = find_gpg_key(verified_signature.fingerprint) if gpg_key From dd139e65b53f30eae2d8bb50dff180e8eab11fe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Wed, 4 Oct 2017 18:44:49 -0500 Subject: [PATCH 47/61] Invalidate GpgSignatures associated to GpgKeySubkeys when revoking the GpgKey --- app/models/gpg_key.rb | 3 ++- app/models/gpg_signature.rb | 13 ++++++++++ ...e_gpg_key_subkeys_for_existing_gpg_keys.rb | 2 +- spec/factories/gpg_signature.rb | 2 +- spec/models/gpg_key_spec.rb | 24 ++++++++++++++++++ spec/models/gpg_signature_spec.rb | 25 +++++++++++++++++++ 6 files changed, 66 insertions(+), 3 deletions(-) diff --git a/app/models/gpg_key.rb b/app/models/gpg_key.rb index 145b957fc88..44eda741679 100644 --- a/app/models/gpg_key.rb +++ b/app/models/gpg_key.rb @@ -91,10 +91,11 @@ class GpgKey < ActiveRecord::Base def revoke GpgSignature - .where(gpg_key: self) + .with_key_and_subkeys(self) .where.not(verification_status: GpgSignature.verification_statuses[:unknown_key]) .update_all( gpg_key_id: nil, + gpg_key_subkey_id: nil, verification_status: GpgSignature.verification_statuses[:unknown_key], updated_at: Time.zone.now ) diff --git a/app/models/gpg_signature.rb b/app/models/gpg_signature.rb index e783c3b24e4..b718e8e1000 100644 --- a/app/models/gpg_signature.rb +++ b/app/models/gpg_signature.rb @@ -21,6 +21,19 @@ class GpgSignature < ActiveRecord::Base validates :project_id, presence: true validates :gpg_key_primary_keyid, presence: true + def self.with_key_and_subkeys(gpg_key) + return none unless gpg_key + + t = arel_table + subkey_ids = gpg_key&.subkeys&.pluck(:id) + + where( + t[:gpg_key_id].eq(gpg_key&.id).or( + t[:gpg_key_subkey_id].in(subkey_ids) + ) + ) + end + def gpg_key=(model) case model when GpgKey diff --git a/db/post_migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb b/db/post_migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb index 346dfb1a4b6..04c81ea684f 100644 --- a/db/post_migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb +++ b/db/post_migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb @@ -30,7 +30,7 @@ class CreateGpgKeySubkeysForExistingGpgKeys < ActiveRecord::Migration def up GpgKey.with_subkeys.each_batch do |batch| batch.each do |gpg_key| - return if gpg_key.subkeys.any? + next if gpg_key.subkeys.any? create_subkeys(gpg_key) && update_signatures(gpg_key) end diff --git a/spec/factories/gpg_signature.rb b/spec/factories/gpg_signature.rb index c0beecf0bea..e9798ff6a41 100644 --- a/spec/factories/gpg_signature.rb +++ b/spec/factories/gpg_signature.rb @@ -5,7 +5,7 @@ FactoryGirl.define do commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } project gpg_key - gpg_key_primary_keyid { gpg_key.primary_keyid } + gpg_key_primary_keyid { gpg_key.keyid } verification_status :verified end end diff --git a/spec/models/gpg_key_spec.rb b/spec/models/gpg_key_spec.rb index 69d6090e034..33e6f1de3d1 100644 --- a/spec/models/gpg_key_spec.rb +++ b/spec/models/gpg_key_spec.rb @@ -191,5 +191,29 @@ describe GpgKey do expect(unrelated_gpg_key.destroyed?).to be false end + + it 'deletes all the associated subkeys' do + gpg_key = create :gpg_key, key: GpgHelpers::User3.public_key + + expect(gpg_key.subkeys).to be_present + + gpg_key.revoke + + expect(gpg_key.subkeys(true)).to be_blank + end + + it 'invalidates all signatures associated to the subkeys' do + gpg_key = create :gpg_key, key: GpgHelpers::User3.public_key + gpg_key_subkey = gpg_key.subkeys.last + gpg_signature = create :gpg_signature, verification_status: :verified, gpg_key: gpg_key_subkey + + gpg_key.revoke + + expect(gpg_signature.reload).to have_attributes( + verification_status: 'unknown_key', + gpg_key: nil, + gpg_key_subkey: nil + ) + end end end diff --git a/spec/models/gpg_signature_spec.rb b/spec/models/gpg_signature_spec.rb index 91d4efe8e2b..db033016c37 100644 --- a/spec/models/gpg_signature_spec.rb +++ b/spec/models/gpg_signature_spec.rb @@ -1,6 +1,9 @@ require 'rails_helper' RSpec.describe GpgSignature do + let(:gpg_key) { create(:gpg_key) } + let(:gpg_key_subkey) { create(:gpg_key_subkey) } + describe 'associations' do it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:gpg_key) } @@ -26,4 +29,26 @@ RSpec.describe GpgSignature do gpg_signature.commit end end + + describe '#gpg_key=' do + it 'supports the assignment of a GpgKey' do + gpg_signature = create(:gpg_signature, gpg_key: gpg_key) + + expect(gpg_signature.gpg_key).to be_an_instance_of(GpgKey) + end + + it 'supports the assignment of a GpgKeySubkey' do + gpg_signature = create(:gpg_signature, gpg_key: gpg_key_subkey) + + expect(gpg_signature.gpg_key).to be_an_instance_of(GpgKeySubkey) + end + + it 'clears gpg_key and gpg_key_subkey_id when passing nil' do + gpg_signature = create(:gpg_signature, gpg_key: gpg_key_subkey) + gpg_signature.update_attribute(:gpg_key, nil) + + expect(gpg_signature.gpg_key_id).to be_nil + expect(gpg_signature.gpg_key_subkey_id).to be_nil + end + end end From 6e0a4fc10e745f8d8be65f8ee420c75d36057c16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Thu, 5 Oct 2017 08:14:34 -0500 Subject: [PATCH 48/61] Convert migrations to generate subkeys to a background migration --- ...le_create_gpg_key_subkeys_from_gpg_keys.rb | 25 +++++++++++++++++++ db/schema.rb | 2 +- .../create_gpg_key_subkeys_from_gpg_keys.rb | 23 +++++------------ 3 files changed, 32 insertions(+), 18 deletions(-) create mode 100644 db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb rename db/post_migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb => lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys.rb (66%) diff --git a/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb b/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb new file mode 100644 index 00000000000..c8bbfbccc08 --- /dev/null +++ b/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb @@ -0,0 +1,25 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class ScheduleCreateGpgKeySubkeysFromGpgKeys < ActiveRecord::Migration + disable_ddl_transaction! + + DOWNTIME = false + + class GpgKey < ActiveRecord::Base + self.table_name = 'gpg_keys' + end + + def up + GpgKey.select(:id).in_batches do |relation| + jobs = relation.pluck(:id).map do |id| + ['CreateGpgKeySubkeysFromGpgKeys', [id]] + end + + BackgroundMigrationWorker.perform_bulk(jobs) + end + end + + def down + end +end diff --git a/db/schema.rb b/db/schema.rb index 3bcfbcc3fd1..3a69e3d3056 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: 20171004121444) do +ActiveRecord::Schema.define(version: 20171005130944) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/db/post_migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb b/lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys.rb similarity index 66% rename from db/post_migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb rename to lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys.rb index 04c81ea684f..3a32134b991 100644 --- a/db/post_migrate/20171002161539_create_gpg_key_subkeys_for_existing_gpg_keys.rb +++ b/lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys.rb @@ -1,11 +1,4 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class CreateGpgKeySubkeysForExistingGpgKeys < ActiveRecord::Migration - disable_ddl_transaction! - - DOWNTIME = false - +class Gitlab::BackgroundMigration::CreateGpgKeySubkeysFromGpgKeys class GpgKey < ActiveRecord::Base self.table_name = 'gpg_keys' @@ -27,17 +20,13 @@ class CreateGpgKeySubkeysForExistingGpgKeys < ActiveRecord::Migration sha_attribute :fingerprint end - def up - GpgKey.with_subkeys.each_batch do |batch| - batch.each do |gpg_key| - next if gpg_key.subkeys.any? + def perform(gpg_key_id) + gpg_key = GpgKey.find_by(id: gpg_key_id) - create_subkeys(gpg_key) && update_signatures(gpg_key) - end - end - end + return if gpg_key.nil? + return if gpg_key.subkeys.any? - def down + create_subkeys(gpg_key) && update_signatures(gpg_key) end private From bd8e3606a202273129b963742aa071a0d2c8812f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Thu, 5 Oct 2017 08:24:27 -0500 Subject: [PATCH 49/61] Don't call update signature worker when there aren't subkeys --- .../create_gpg_key_subkeys_from_gpg_keys.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys.rb b/lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys.rb index 3a32134b991..e94719db72e 100644 --- a/lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys.rb +++ b/lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys.rb @@ -26,7 +26,8 @@ class Gitlab::BackgroundMigration::CreateGpgKeySubkeysFromGpgKeys return if gpg_key.nil? return if gpg_key.subkeys.any? - create_subkeys(gpg_key) && update_signatures(gpg_key) + create_subkeys(gpg_key) + update_signatures(gpg_key) end private @@ -45,6 +46,8 @@ class Gitlab::BackgroundMigration::CreateGpgKeySubkeysFromGpgKeys end def update_signatures(gpg_key) + return unless gpg_key.subkeys.exists? + InvalidGpgSignatureUpdateWorker.perform_async(gpg_key.id) end end From 5595d73763caf224df4109ab2d52e616cd843d49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Thu, 5 Oct 2017 10:17:18 -0500 Subject: [PATCH 50/61] Small refactor and fix for RuboCop --- app/models/gpg_signature.rb | 9 +++------ ...0944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb | 6 ++++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/app/models/gpg_signature.rb b/app/models/gpg_signature.rb index b718e8e1000..675e7a2456d 100644 --- a/app/models/gpg_signature.rb +++ b/app/models/gpg_signature.rb @@ -22,14 +22,11 @@ class GpgSignature < ActiveRecord::Base validates :gpg_key_primary_keyid, presence: true def self.with_key_and_subkeys(gpg_key) - return none unless gpg_key - - t = arel_table - subkey_ids = gpg_key&.subkeys&.pluck(:id) + subkey_ids = gpg_key.subkeys.pluck(:id) where( - t[:gpg_key_id].eq(gpg_key&.id).or( - t[:gpg_key_subkey_id].in(subkey_ids) + arel_table[:gpg_key_id].eq(gpg_key.id).or( + arel_table[:gpg_key_subkey_id].in(subkey_ids) ) ) end diff --git a/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb b/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb index c8bbfbccc08..08c9ce1d3bb 100644 --- a/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb +++ b/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb @@ -8,11 +8,13 @@ class ScheduleCreateGpgKeySubkeysFromGpgKeys < ActiveRecord::Migration class GpgKey < ActiveRecord::Base self.table_name = 'gpg_keys' + + include EachBatch end def up - GpgKey.select(:id).in_batches do |relation| - jobs = relation.pluck(:id).map do |id| + GpgKey.select(:id).each_batch do |gpg_keys| + jobs = gpg_keys.pluck(:id).map do |id| ['CreateGpgKeySubkeysFromGpgKeys', [id]] end From 790bc25dcd727471698838c5f8504fa9dc44fc37 Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas Date: Thu, 5 Oct 2017 12:00:20 -0500 Subject: [PATCH 51/61] fix linter errors and karma specs --- app/assets/javascripts/monitoring/components/graph.vue | 2 +- app/assets/stylesheets/pages/environments.scss | 1 + spec/javascripts/monitoring/graph_path_spec.js | 2 +- spec/javascripts/monitoring/graph_spec.js | 6 +++--- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/monitoring/components/graph.vue b/app/assets/javascripts/monitoring/components/graph.vue index f1792749936..972f4f7adef 100644 --- a/app/assets/javascripts/monitoring/components/graph.vue +++ b/app/assets/javascripts/monitoring/components/graph.vue @@ -145,7 +145,7 @@ this.graphData.queries[0], this.graphWidth, this.graphHeight, - this.graphHeightOffset + this.graphHeightOffset, ); if (this.timeSeries.length > 3) { diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index 1e09141665f..7c0be8cb988 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -292,6 +292,7 @@ > line { stroke: $gray-darker; } + > text { font-size: 12px; } diff --git a/spec/javascripts/monitoring/graph_path_spec.js b/spec/javascripts/monitoring/graph_path_spec.js index a4844636d09..81825a3ae87 100644 --- a/spec/javascripts/monitoring/graph_path_spec.js +++ b/spec/javascripts/monitoring/graph_path_spec.js @@ -1,5 +1,5 @@ import Vue from 'vue'; -import GraphPath from '~/monitoring/components/graph_path.vue'; +import GraphPath from '~/monitoring/components/graph/path.vue'; import createTimeSeries from '~/monitoring/utils/multiple_time_series'; import { singleRowMetricsMultipleSeries, convertDatesMultipleSeries } from './mock_data'; diff --git a/spec/javascripts/monitoring/graph_spec.js b/spec/javascripts/monitoring/graph_spec.js index 7d8b0744af1..2b41635584a 100644 --- a/spec/javascripts/monitoring/graph_spec.js +++ b/spec/javascripts/monitoring/graph_spec.js @@ -44,7 +44,7 @@ describe('Graph', () => { .not.toEqual(-1); }); - it('outterViewBox gets a width and height property based on the DOM size of the element', () => { + it('outerViewBox gets a width and height property based on the DOM size of the element', () => { const component = createComponent({ graphData: convertedMetrics[1], classType: 'col-md-6', @@ -52,8 +52,8 @@ describe('Graph', () => { deploymentData, }); - const viewBoxArray = component.outterViewBox.split(' '); - expect(typeof component.outterViewBox).toEqual('string'); + const viewBoxArray = component.outerViewBox.split(' '); + expect(typeof component.outerViewBox).toEqual('string'); expect(viewBoxArray[2]).toEqual(component.graphWidth.toString()); expect(viewBoxArray[3]).toEqual(component.graphHeight.toString()); }); From a646947bd49c8ad84cf396d99cd5c8fa85472f43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Thu, 5 Oct 2017 14:17:11 -0300 Subject: [PATCH 52/61] Update required Workhorse version to v3.2.0 --- GITLAB_WORKHORSE_VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index fd2a01863fd..944880fa15e 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -3.1.0 +3.2.0 From 60a35e4230404b84d4aee8015fb7821b0b194277 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Mon, 2 Oct 2017 21:52:19 -0300 Subject: [PATCH 53/61] Send API parameters as extra data for sentry errors --- config/initializers/sentry.rb | 4 +++- lib/api/helpers.rb | 2 +- spec/requests/api/helpers_spec.rb | 28 +++++++++++++++++++++++++++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb index 62d0967009a..b2da3b3dc19 100644 --- a/config/initializers/sentry.rb +++ b/config/initializers/sentry.rb @@ -2,7 +2,7 @@ require 'gitlab/current_settings' -if Rails.env.production? +def configure_sentry # allow it to fail: it may do so when create_from_defaults is executed before migrations are actually done begin sentry_enabled = Gitlab::CurrentSettings.current_application_settings.sentry_enabled @@ -23,3 +23,5 @@ if Rails.env.production? end end end + +configure_sentry if Rails.env.production? diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 4964a76bef6..a87297a604c 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -287,7 +287,7 @@ module API if sentry_enabled? && report_exception?(exception) define_params_for_grape_middleware sentry_context - Raven.capture_exception(exception) + Raven.capture_exception(exception, extra: params) end # lifted from https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb#L60 diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 060c8902471..862920ad7c3 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -1,4 +1,6 @@ require 'spec_helper' +require 'raven/transports/dummy' +require_relative '../../../config/initializers/sentry' describe API::Helpers do include API::APIGuard::HelperMethods @@ -476,7 +478,7 @@ describe API::Helpers do allow(exception).to receive(:backtrace).and_return(caller) expect_any_instance_of(self.class).to receive(:sentry_context) - expect(Raven).to receive(:capture_exception).with(exception) + expect(Raven).to receive(:capture_exception).with(exception, extra: {}) handle_api_exception(exception) end @@ -501,6 +503,30 @@ describe API::Helpers do expect(json_response['message']).to start_with("\nRuntimeError (Runtime Error!):") end end + + context 'extra information' do + # Sentry events are an array of the form [auth_header, data, options] + let(:event_data) { Raven.client.transport.events.first[1] } + + before do + stub_application_setting( + sentry_enabled: true, + sentry_dsn: "dummy://12345:67890@sentry.localdomain/sentry/42" + ) + configure_sentry + Raven.client.configuration.encoding = 'json' + end + + it 'sends the params, excluding confidential values' do + expect(Gitlab::Sentry).to receive(:enabled?).twice.and_return(true) + expect(ProjectsFinder).to receive(:new).and_raise('Runtime Error!') + + get api('/projects', user), password: 'dont_send_this', other_param: 'send_this' + + expect(event_data).to include('other_param=send_this') + expect(event_data).to include('password=********') + end + end end describe '.authenticate_non_get!' do From 351fde1b908296802fe65112f2ff1eb4aa6cb5db Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Thu, 5 Oct 2017 11:05:48 -0400 Subject: [PATCH 54/61] Prevent branches or tags from starting with invalid characters (e.g. -, .) Closes #38817 --- changelogs/unreleased/valid-branch-name-dash-bug.yml | 5 +++++ lib/gitlab/git_ref_validator.rb | 2 +- spec/lib/gitlab/git_ref_validator_spec.rb | 5 +++++ 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/valid-branch-name-dash-bug.yml diff --git a/changelogs/unreleased/valid-branch-name-dash-bug.yml b/changelogs/unreleased/valid-branch-name-dash-bug.yml new file mode 100644 index 00000000000..89e4578b3e5 --- /dev/null +++ b/changelogs/unreleased/valid-branch-name-dash-bug.yml @@ -0,0 +1,5 @@ +--- +title: Prevent branches or tags from starting with invalid characters (e.g. -, .) +merge_request: +author: +type: fixed diff --git a/lib/gitlab/git_ref_validator.rb b/lib/gitlab/git_ref_validator.rb index a3c6b21a6a1..2e3e4fc3f1f 100644 --- a/lib/gitlab/git_ref_validator.rb +++ b/lib/gitlab/git_ref_validator.rb @@ -11,7 +11,7 @@ module Gitlab return false if ref_name.start_with?('refs/remotes/') Gitlab::Utils.system_silent( - %W(#{Gitlab.config.git.bin_path} check-ref-format refs/#{ref_name})) + %W(#{Gitlab.config.git.bin_path} check-ref-format --branch #{ref_name})) end end end diff --git a/spec/lib/gitlab/git_ref_validator_spec.rb b/spec/lib/gitlab/git_ref_validator_spec.rb index e1fa8ae03f8..ba7fb168a3b 100644 --- a/spec/lib/gitlab/git_ref_validator_spec.rb +++ b/spec/lib/gitlab/git_ref_validator_spec.rb @@ -4,6 +4,7 @@ describe Gitlab::GitRefValidator do it { expect(described_class.validate('feature/new')).to be_truthy } it { expect(described_class.validate('implement_@all')).to be_truthy } it { expect(described_class.validate('my_new_feature')).to be_truthy } + it { expect(described_class.validate('my-branch')).to be_truthy } it { expect(described_class.validate('#1')).to be_truthy } it { expect(described_class.validate('feature/refs/heads/foo')).to be_truthy } it { expect(described_class.validate('feature/~new/')).to be_falsey } @@ -22,4 +23,8 @@ describe Gitlab::GitRefValidator do it { expect(described_class.validate('refs/remotes/')).to be_falsey } it { expect(described_class.validate('refs/heads/feature')).to be_falsey } it { expect(described_class.validate('refs/remotes/origin')).to be_falsey } + it { expect(described_class.validate('-')).to be_falsey } + it { expect(described_class.validate('-branch')).to be_falsey } + it { expect(described_class.validate('.tag')).to be_falsey } + it { expect(described_class.validate('my branch')).to be_falsey } end From 555f50b3e68e82968ea2eb4916a3f5beeeef7b31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20D=C3=A1vila?= Date: Thu, 5 Oct 2017 22:43:44 -0500 Subject: [PATCH 55/61] Add more specs. --- ...le_create_gpg_key_subkeys_from_gpg_keys.rb | 3 +- ...eate_gpg_key_subkeys_from_gpg_keys_spec.rb | 32 ++++++++ .../gpg/invalid_gpg_signature_updater_spec.rb | 49 +++++++++---- spec/lib/gitlab/gpg_spec.rb | 17 +++++ ...eate_gpg_key_subkeys_from_gpg_keys_spec.rb | 43 +++++++++++ spec/support/gpg_helpers.rb | 73 +++++++++++++++++++ 6 files changed, 203 insertions(+), 14 deletions(-) create mode 100644 spec/lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys_spec.rb create mode 100644 spec/migrations/schedule_create_gpg_key_subkeys_from_gpg_keys_spec.rb diff --git a/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb b/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb index 08c9ce1d3bb..01d56fbd490 100644 --- a/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb +++ b/db/post_migrate/20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys.rb @@ -5,6 +5,7 @@ class ScheduleCreateGpgKeySubkeysFromGpgKeys < ActiveRecord::Migration disable_ddl_transaction! DOWNTIME = false + MIGRATION = 'CreateGpgKeySubkeysFromGpgKeys' class GpgKey < ActiveRecord::Base self.table_name = 'gpg_keys' @@ -15,7 +16,7 @@ class ScheduleCreateGpgKeySubkeysFromGpgKeys < ActiveRecord::Migration def up GpgKey.select(:id).each_batch do |gpg_keys| jobs = gpg_keys.pluck(:id).map do |id| - ['CreateGpgKeySubkeysFromGpgKeys', [id]] + [MIGRATION, [id]] end BackgroundMigrationWorker.perform_bulk(jobs) diff --git a/spec/lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys_spec.rb b/spec/lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys_spec.rb new file mode 100644 index 00000000000..26d48cc8201 --- /dev/null +++ b/spec/lib/gitlab/background_migration/create_gpg_key_subkeys_from_gpg_keys_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Gitlab::BackgroundMigration::CreateGpgKeySubkeysFromGpgKeys, :migration, schema: 20171005130944 do + context 'when GpgKey exists' do + let!(:gpg_key) { create(:gpg_key, key: GpgHelpers::User3.public_key) } + + before do + GpgKeySubkey.destroy_all + end + + it 'generate the subkeys' do + expect do + described_class.new.perform(gpg_key.id) + end.to change { gpg_key.subkeys.count }.from(0).to(2) + end + + it 'schedules the signature update worker' do + expect(InvalidGpgSignatureUpdateWorker).to receive(:perform_async).with(gpg_key.id) + + described_class.new.perform(gpg_key.id) + end + end + + context 'when GpgKey does not exist' do + it 'does not do anything' do + expect(Gitlab::Gpg).not_to receive(:subkeys_from_key) + expect(InvalidGpgSignatureUpdateWorker).not_to receive(:perform_async) + + described_class.new.perform(123) + end + end +end diff --git a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb index b9fd4d02156..d6000af0ecd 100644 --- a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb +++ b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb @@ -2,17 +2,16 @@ require 'rails_helper' RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do describe '#run' do - let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } - let!(:project) { create :project, :repository, path: 'sample-project' } + let(:signature) { [GpgHelpers::User1.signed_commit_signature, GpgHelpers::User1.signed_commit_base_data] } + let(:committer_email) { GpgHelpers::User1.emails.first } + let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } + let!(:project) { create :project, :repository, path: 'sample-project' } let!(:raw_commit) do raw_commit = double( :raw_commit, - signature: [ - GpgHelpers::User1.signed_commit_signature, - GpgHelpers::User1.signed_commit_base_data - ], + signature: signature, sha: commit_sha, - committer_email: GpgHelpers::User1.emails.first + committer_email: committer_email ) allow(raw_commit).to receive :save! @@ -29,12 +28,7 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do allow(Rugged::Commit).to receive(:extract_signature) .with(Rugged::Repository, commit_sha) - .and_return( - [ - GpgHelpers::User1.signed_commit_signature, - GpgHelpers::User1.signed_commit_base_data - ] - ) + .and_return(signature) end context 'gpg signature did have an associated gpg key which was removed later' do @@ -183,5 +177,34 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do ) end end + + context 'gpg signature did not have an associated gpg subkey' do + let(:signature) { [GpgHelpers::User3.signed_commit_signature, GpgHelpers::User3.signed_commit_base_data] } + let(:committer_email) { GpgHelpers::User3.emails.first } + let!(:user) { create :user, email: GpgHelpers::User3.emails.first } + + let!(:invalid_gpg_signature) do + create :gpg_signature, + project: project, + commit_sha: commit_sha, + gpg_key: nil, + gpg_key_primary_keyid: GpgHelpers::User3.subkey_fingerprints.last[24..-1], + verification_status: 'unknown_key' + end + + it 'updates the signature to being valid when the missing gpg key is added' do + # InvalidGpgSignatureUpdater is called by the after_create hook + gpg_key = create(:gpg_key, key: GpgHelpers::User3.public_key, user: user) + subkey = gpg_key.subkeys.last + + expect(invalid_gpg_signature.reload).to have_attributes( + project: project, + commit_sha: commit_sha, + gpg_key_subkey_id: subkey.id, + gpg_key_primary_keyid: subkey.keyid, + verification_status: 'verified' + ) + end + end end end diff --git a/spec/lib/gitlab/gpg_spec.rb b/spec/lib/gitlab/gpg_spec.rb index 11a2aea1915..ab9a166db00 100644 --- a/spec/lib/gitlab/gpg_spec.rb +++ b/spec/lib/gitlab/gpg_spec.rb @@ -28,6 +28,23 @@ describe Gitlab::Gpg do end end + describe '.subkeys_from_key' do + it 'returns the subkeys by primary key' do + all_subkeys = described_class.subkeys_from_key(GpgHelpers::User1.public_key) + subkeys = all_subkeys[GpgHelpers::User1.primary_keyid] + + expect(subkeys).to be_present + expect(subkeys.first[:keyid]).to be_present + expect(subkeys.first[:fingerprint]).to be_present + end + + it 'returns an empty array when there are not subkeys' do + all_subkeys = described_class.subkeys_from_key(GpgHelpers::User4.public_key) + + expect(all_subkeys[GpgHelpers::User4.primary_keyid]).to be_empty + end + end + describe '.user_infos_from_key' do it 'returns the names and emails' do user_infos = described_class.user_infos_from_key(GpgHelpers::User1.public_key) diff --git a/spec/migrations/schedule_create_gpg_key_subkeys_from_gpg_keys_spec.rb b/spec/migrations/schedule_create_gpg_key_subkeys_from_gpg_keys_spec.rb new file mode 100644 index 00000000000..0e884a7d910 --- /dev/null +++ b/spec/migrations/schedule_create_gpg_key_subkeys_from_gpg_keys_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20171005130944_schedule_create_gpg_key_subkeys_from_gpg_keys') + +describe ScheduleCreateGpgKeySubkeysFromGpgKeys, :migration, :sidekiq do + matcher :be_scheduled_migration do |*expected| + match do |migration| + BackgroundMigrationWorker.jobs.any? do |job| + job['args'] == [migration, expected] + end + end + + failure_message do |migration| + "Migration `#{migration}` with args `#{expected.inspect}` not scheduled!" + end + end + + before do + create(:gpg_key, id: 1, key: GpgHelpers::User1.public_key) + create(:gpg_key, id: 2, key: GpgHelpers::User3.public_key) + # Delete all subkeys so they can be recreated + GpgKeySubkey.destroy_all + end + + it 'correctly schedules background migrations' do + Sidekiq::Testing.fake! do + migrate! + + expect(described_class::MIGRATION).to be_scheduled_migration(1) + expect(described_class::MIGRATION).to be_scheduled_migration(2) + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + end + end + + it 'schedules background migrations' do + Sidekiq::Testing.inline! do + expect(GpgKeySubkey.count).to eq(0) + + migrate! + + expect(GpgKeySubkey.count).to eq(3) + end + end +end diff --git a/spec/support/gpg_helpers.rb b/spec/support/gpg_helpers.rb index 4b1ca7ff9c8..3f7279a50e0 100644 --- a/spec/support/gpg_helpers.rb +++ b/spec/support/gpg_helpers.rb @@ -441,4 +441,77 @@ module GpgHelpers ['john.doe@example.com'] end end + + # GPG Key containing just the main key + module User4 + extend self + + def public_key + <<~KEY.strip + -----BEGIN PGP PUBLIC KEY BLOCK----- + + mQENBFnWcesBCAC6Y8FXl9ZJ9HPa6dIYcgQrvjIQcwoQCUEsaXNRpc+206RPCIXK + aIYr0nTD8GeovMuUONXTj+DdueQU2GAAqHHOqvDDVXqRrW3xfWnSwix7sTuhG1Ew + PLHYmjLENqaTsdyliEo3N8VWy2k0QRbC3R6xvop4Ooa87D5vcATIl0gYFtSiHIL+ + TervYvTG9Eq1qSLZHbe2x4IzeqX2luikPKokL7j8FTZaCmC5MezIUur1ulfyYY/j + SkST/1aUFc5QXJJSZA0MYJWZX6x7Y3l7yl0dkHqmK8OTuo8RPWd3ybEiuvRsOL8K + GAv/PmVJRGDAf7GGbwXXsE9MiZ5GzVPxHnexABEBAAG0G0pvaG4gRG9lIDxqb2hu + QGV4YW1wbGUuY29tPokBTgQTAQgAOBYhBAh0izYM0lwuzJnVlAcBbPnhOj+bBQJZ + 1nHrAhsDBQsJCAcCBhUICQoLAgQWAgMBAh4BAheAAAoJEAcBbPnhOj+bkywH/i4w + OwpDxoTjUQlPlqGAGuzvWaPzSJndawgmMTr68oRsD+wlQmQQTR5eqxCpUIyV4aYb + D697RYzoqbT4mlU49ymzfKSAxFe88r1XQWdm81DcofHVPmw2GBrIqaX3Du4Z7xkI + Q9/S43orwknh5FoVwU8Nau7qBuv9vbw2apSkuA1oBj3spQ8hqwLavACyQ+fQloAT + hSDNqPiCZj6L0dwM1HYiqVoN3Q7qjgzzeBzlXzljJoWblhxllvMK20bVoa7H+uR2 + lczFHfsX8VTIMjyTGP7R3oHN91DEahlQybVVNLmNSDKZM2P/0d28BRUmWxQJ4Ws3 + J4hOWDKnLMed3VOIWzM= + =xVuW + -----END PGP PUBLIC KEY BLOCK----- + KEY + end + + def secret_key + <<~KEY.strip + -----BEGIN PGP PRIVATE KEY BLOCK----- + + lQPGBFnWcesBCAC6Y8FXl9ZJ9HPa6dIYcgQrvjIQcwoQCUEsaXNRpc+206RPCIXK + aIYr0nTD8GeovMuUONXTj+DdueQU2GAAqHHOqvDDVXqRrW3xfWnSwix7sTuhG1Ew + PLHYmjLENqaTsdyliEo3N8VWy2k0QRbC3R6xvop4Ooa87D5vcATIl0gYFtSiHIL+ + TervYvTG9Eq1qSLZHbe2x4IzeqX2luikPKokL7j8FTZaCmC5MezIUur1ulfyYY/j + SkST/1aUFc5QXJJSZA0MYJWZX6x7Y3l7yl0dkHqmK8OTuo8RPWd3ybEiuvRsOL8K + GAv/PmVJRGDAf7GGbwXXsE9MiZ5GzVPxHnexABEBAAH+BwMC4UwgHgH5Cp7meY39 + G5Q3GV2xtwADoaAvlOvPOLPK2fQqxQfb4WN4eZECp2wQuMRBMj52c4i9yphab1mQ + vOzoPIRGvkcJoxG++OxQ0kRk0C0gX6wM6SGVdb1nQnfZnoJCCU3IwCaSGktkLDs1 + jwdI+VmXJbSugUbd25bakHQcE2BaNHuRBlQWQfFbhGBy0+uMfNDBZ6FRipBu47hO + f/wm/xXuV8N8BSgvNR/qtAqSQI34CdsnWAhMYm9rqmTNyt0nq4dveX+E0YzVn4lH + lOEa7cpYeuBwIL8L3EvSPNCICiJlF3gVqiYzyqRElnCkv1OGc0x3W5onY/agHgGZ + KYyi/ubOdqqDgBR+eMt0JKSGH2EPxUAGFPY5F37u4erdxH86GzIinAExLSmADiVR + KtxluZP6S2KLbETN5uVbrfa+HVcMbbUZaBHHtL+YbY8PqaFUIvIUR1HM2SK7IrFw + KuQ8ibRgooyP7VgMNiPzlFpY4NXUv+FXIrNJ6ELuIaENi0izJ7aIbVBM8SijDz6u + 5EEmodnDvmU2hmQNZJ17TxggE7oeT0rKdDGHM5zBvqZ3deqE9sgKx/aTKcj61ID3 + M80ZkHPDFazUCohLpYgFN20bYYSmxU4LeNFy8YEiuic8QQKaAFxSf9Lf87UFQwyF + dduI1RWEbjMsbEJXwlmGM02ssQHsgoVKwZxijq5A5R1Ul6LowazQ8obPiwRS4NZ4 + Z+QKDon79MMXiFEeh1jeG/MKKWPxFg3pdtCWhC7WdH4hfkBsCVKf+T58yB2Gzziy + fOHvAl7v3PtdZgf1xikF8spGYGCWo4B2lxC79xIflKAb2U6myb5I4dpUYxzxoMxT + zxHwxEie3NxzZGUyXSt3LqYe2r4CxWnOCXWjIxxRlLue1BE5Za1ycnDRjgUO24+Z + uDQne6KLkhAotBtKb2huIERvZSA8am9obkBleGFtcGxlLmNvbT6JAU4EEwEIADgW + IQQIdIs2DNJcLsyZ1ZQHAWz54To/mwUCWdZx6wIbAwULCQgHAgYVCAkKCwIEFgID + AQIeAQIXgAAKCRAHAWz54To/m5MsB/4uMDsKQ8aE41EJT5ahgBrs71mj80iZ3WsI + JjE6+vKEbA/sJUJkEE0eXqsQqVCMleGmGw+ve0WM6Km0+JpVOPcps3ykgMRXvPK9 + V0FnZvNQ3KHx1T5sNhgayKml9w7uGe8ZCEPf0uN6K8JJ4eRaFcFPDWru6gbr/b28 + NmqUpLgNaAY97KUPIasC2rwAskPn0JaAE4Ugzaj4gmY+i9HcDNR2IqlaDd0O6o4M + 83gc5V85YyaFm5YcZZbzCttG1aGux/rkdpXMxR37F/FUyDI8kxj+0d6BzfdQxGoZ + UMm1VTS5jUgymTNj/9HdvAUVJlsUCeFrNyeITlgypyzHnd1TiFsz + =/37z + -----END PGP PRIVATE KEY BLOCK----- + KEY + end + + def primary_keyid + fingerprint[-16..-1] + end + + def fingerprint + '08748B360CD25C2ECC99D59407016CF9E13A3F9B' + end + end end From 0f6d20e39453b106e42ab53fa3649674b403eba5 Mon Sep 17 00:00:00 2001 From: Tim Zallmann Date: Fri, 6 Oct 2017 10:03:56 +0200 Subject: [PATCH 56/61] Made the insertion of the English Translation File conditional + removed window.translations from the global object to save some memory --- app/assets/javascripts/locale/index.js | 1 + app/views/layouts/_head.html.haml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/locale/index.js b/app/assets/javascripts/locale/index.js index ce05b3eabec..1003b9ba0af 100644 --- a/app/assets/javascripts/locale/index.js +++ b/app/assets/javascripts/locale/index.js @@ -4,6 +4,7 @@ import sprintf from './sprintf'; const langAttribute = document.querySelector('html').getAttribute('lang'); const lang = (langAttribute || 'en').replace(/-/g, '_'); const locale = new Jed(window.translations || {}); +delete window.translations; /** Translates `text` diff --git a/app/views/layouts/_head.html.haml b/app/views/layouts/_head.html.haml index 0d5350f873b..f1b32274664 100644 --- a/app/views/layouts/_head.html.haml +++ b/app/views/layouts/_head.html.haml @@ -37,7 +37,7 @@ - if content_for?(:library_javascripts) = yield :library_javascripts - = javascript_include_tag asset_path("locale/#{I18n.locale.to_s || I18n.default_locale.to_s}/app.js") + = javascript_include_tag asset_path("locale/#{I18n.locale.to_s || I18n.default_locale.to_s}/app.js") unless I18n.locale == :en = webpack_bundle_tag "webpack_runtime" = webpack_bundle_tag "common" = webpack_bundle_tag "main" From 96944979b2a72d3d80f4447d56fb873cbd58fd6c Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Fri, 6 Oct 2017 09:29:24 +0100 Subject: [PATCH 57/61] Fix typo in cycle analytics breaking time component --- .../components/total_time_component.vue | 2 +- .../unreleased/fl-fix-ca-time-component.yml | 5 ++ .../total_time_component_spec.js | 58 +++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/fl-fix-ca-time-component.yml create mode 100644 spec/javascripts/cycle_analytics/total_time_component_spec.js diff --git a/app/assets/javascripts/cycle_analytics/components/total_time_component.vue b/app/assets/javascripts/cycle_analytics/components/total_time_component.vue index 9941b997b3f..62efd4f9c28 100644 --- a/app/assets/javascripts/cycle_analytics/components/total_time_component.vue +++ b/app/assets/javascripts/cycle_analytics/components/total_time_component.vue @@ -20,7 +20,7 @@ - +