Merge branch 'ce-to-ee-2018-05-08' into 'master'

CE upstream - 2018-05-08 21:28 UTC

See merge request gitlab-org/gitlab-ee!5620

Gemfile

@@ -449,4 +449,4 @@ gem 'lograge', '~> 0.5'
 gem 'grape_logging', '~> 1.7'
 
 # Asset synchronization
-gem 'asset_sync', '~> 2.2.0'
+gem 'asset_sync', '~> 2.4'

Gemfile.lock

@@ -59,7 +59,7 @@ GEM
     asciidoctor (1.5.6.2)
     asciidoctor-plantuml (0.0.8)
       asciidoctor (~> 1.5)
-    asset_sync (2.2.0)
+    asset_sync (2.4.0)
       activemodel (>= 4.1.0)
       fog-core
       mime-types (>= 2.99)
@@ -218,7 +218,7 @@ GEM
     et-orbi (1.0.3)
       tzinfo
     eventmachine (1.0.8)
-    excon (0.60.0)
+    excon (0.62.0)
     execjs (2.6.0)
     expression_parser (0.9.0)
     factory_bot (4.8.2)
@@ -1027,7 +1027,7 @@ DEPENDENCIES
   asana (~> 0.6.0)
   asciidoctor (~> 1.5.6)
   asciidoctor-plantuml (= 0.0.8)
-  asset_sync (~> 2.2.0)
+  asset_sync (~> 2.4)
   attr_encrypted (~> 3.1.0)
   awesome_print (~> 1.2.0)
   aws-sdk

app/assets/javascripts/ide/components/mr_file_icon.vue

@@ -16,8 +16,8 @@ export default {
   <icon
     name="git-merge"
     v-tooltip
-    title="__('Part of merge request changes')"
-    css-classes="ide-file-changed-icon"
+    :title="__('Part of merge request changes')"
+    css-classes="append-right-8"
     :size="12"
   />
 </template>

app/assets/stylesheets/framework/buttons.scss

@@ -106,10 +106,6 @@
   @include btn-color($red-500, $red-600, $red-600, $red-700, $red-700, $red-800, $white-light);
 }
 
-@mixin btn-gray {
-  @include btn-color($gray-light, $border-gray-normal, $gray-normal, $border-gray-normal, $gray-dark, $border-gray-dark, $gl-text-color);
-}
-
 @mixin btn-white {
   @include btn-color($white-light, $border-color, $white-normal, $border-white-normal, $white-dark, $border-gray-dark, $gl-text-color);
 }
@@ -183,10 +179,6 @@
     }
   }
 
-  &.btn-gray {
-    @include btn-gray;
-  }
-
   &.btn-info,
   &.btn-primary,
   &.btn-register {

app/assets/stylesheets/pages/projects.scss

@@ -205,7 +205,6 @@
 .project-repo-buttons,
 .group-buttons {
   .btn {
-    @include btn-gray;
     padding: 3px 10px;
 
     &:last-child {
@@ -294,7 +293,7 @@
     }
 
     .count {
-      @include btn-gray;
+      @include btn-white;
       display: inline-block;
       background: $white-light;
       border-radius: 2px;

app/models/concerns/reactive_caching.rb

@@ -63,13 +63,16 @@ module ReactiveCaching
     end
 
     def with_reactive_cache(*args, &blk)
-      within_reactive_cache_lifetime(*args) do
+      bootstrap = !within_reactive_cache_lifetime?(*args)
+      Rails.cache.write(alive_reactive_cache_key(*args), true, expires_in: self.class.reactive_cache_lifetime)
+
+      if bootstrap
+        ReactiveCachingWorker.perform_async(self.class, id, *args)
+        nil
+      else
         data = Rails.cache.read(full_reactive_cache_key(*args))
         yield data if data.present?
       end
-    ensure
-      Rails.cache.write(alive_reactive_cache_key(*args), true, expires_in: self.class.reactive_cache_lifetime)
-      ReactiveCachingWorker.perform_async(self.class, id, *args)
     end
 
     def clear_reactive_cache!(*args)
@@ -78,7 +81,7 @@ module ReactiveCaching
 
     def exclusively_update_reactive_cache!(*args)
       locking_reactive_cache(*args) do
-        within_reactive_cache_lifetime(*args) do
+        if within_reactive_cache_lifetime?(*args)
           enqueuing_update(*args) do
             key = full_reactive_cache_key(*args)
             new_value = calculate_reactive_cache(*args)
@@ -111,8 +114,8 @@ module ReactiveCaching
       Gitlab::ExclusiveLease.cancel(full_reactive_cache_key(*args), uuid)
     end
 
-    def within_reactive_cache_lifetime(*args)
-      yield if Rails.cache.read(alive_reactive_cache_key(*args))
+    def within_reactive_cache_lifetime?(*args)
+      !!Rails.cache.read(alive_reactive_cache_key(*args))
     end
 
     def enqueuing_update(*args)

app/views/help/_shortcuts.html.haml

 #modal-shortcuts.modal{ tabindex: -1 }
-  .modal-dialog
+  .modal-dialog.modal-lg
     .modal-content
       .modal-header
         %a.close{ href: "#", "data-dismiss" => "modal" } ×

app/views/help/ui.html.haml

@@ -74,10 +74,10 @@
         = lorem
 
       .cover-controls
-        = link_to '#', class: 'btn btn-gray' do
+        = link_to '#', class: 'btn btn-default' do
           = icon('pencil')
          
-        = link_to '#', class: 'btn btn-gray' do
+        = link_to '#', class: 'btn btn-default' do
           = icon('rss')
 
   %h2#lists Lists
@@ -206,7 +206,6 @@
 
   .example
     %button.btn.btn-default{ :type => "button" } Default
-    %button.btn.btn-gray{ :type => "button" } Gray
     %button.btn.btn-primary{ :type => "button" } Primary
     %button.btn.btn-success{ :type => "button" } Success
     %button.btn.btn-info{ :type => "button" } Info

app/views/users/show.html.haml

@@ -12,7 +12,7 @@
   .cover-block.user-cover-block.top-area
     .cover-controls
       - if @user == current_user
-        = link_to profile_path, class: 'btn btn-gray has-tooltip', title: 'Edit profile', 'aria-label': 'Edit profile' do
+        = link_to profile_path, class: 'btn btn-default has-tooltip', title: 'Edit profile', 'aria-label': 'Edit profile' do
           = icon('pencil')
       - elsif current_user
         - if @user.abuse_report
@@ -20,13 +20,13 @@
             data: { toggle: 'tooltip', placement: 'bottom', container: 'body' } }
             = icon('exclamation-circle')
         - else
-          = link_to new_abuse_report_path(user_id: @user.id, ref_url: request.referrer), class: 'btn btn-gray',
+          = link_to new_abuse_report_path(user_id: @user.id, ref_url: request.referrer), class: 'btn',
             title: 'Report abuse', data: { toggle: 'tooltip', placement: 'bottom', container: 'body' } do
             = icon('exclamation-circle')
-      = link_to user_path(@user, rss_url_options), class: 'btn btn-gray has-tooltip', title: 'Subscribe', 'aria-label': 'Subscribe' do
+      = link_to user_path(@user, rss_url_options), class: 'btn btn-default has-tooltip', title: 'Subscribe', 'aria-label': 'Subscribe' do
         = icon('rss')
       - if current_user && current_user.admin?
-        = link_to [:admin, @user], class: 'btn btn-gray', title: 'View user in admin area',
+        = link_to [:admin, @user], class: 'btn btn-default', title: 'View user in admin area',
           data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
           = icon('users')
 

changelogs/unreleased/44319-remove-gray-buttons.yml

+---
+title: Remove gray button styles
+merge_request:
+author:
+type: fixed

changelogs/unreleased/5750-backport-checksum-git-commanderror-exit-status-128.yml

+---
+title: Raise NoRepository error for non-valid repositories when calculating repository
+  checksum
+merge_request: 18594
+author:
+type: fixed

changelogs/unreleased/feature-expose-runner-ip-to-api.yml

+---
+title: Expose runner ip address to runners API
+merge_request: 18799
+author: Lars Greiss
+type: changed

changelogs/unreleased/fix-gb-add-pipeline-builds-foreign-key.yml

+---
+title: Add database foreign key constraint between pipelines and build
+merge_request: 18822
+author:
+type: fixed

changelogs/unreleased/fix-reactive-cache-retry-rate.yml

+---
+title: Update commit status from external CI services less aggressively
+merge_request: 18802
+author:
+type: fixed

changelogs/unreleased/fix-shorcut-modal.yml

+---
+title: Fix modal width of shorcuts help page
+merge_request: 18766
+author: Lars Greiss
+type: fixed

db/migrate/20180420010016_add_pipeline_build_foreign_key.rb

+class AddPipelineBuildForeignKey < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    execute <<~SQL
+      DELETE FROM ci_builds WHERE project_id IS NULL OR commit_id IS NULL
+    SQL
+
+    execute <<~SQL
+      DELETE FROM ci_builds WHERE NOT EXISTS
+        (SELECT true FROM ci_pipelines WHERE ci_pipelines.id = ci_builds.commit_id)
+        AND stage_id IS NULL
+    SQL
+
+    add_concurrent_foreign_key(:ci_builds, :ci_pipelines, column: :commit_id)
+  end
+
+  def down
+    return unless foreign_key_exists?(:ci_builds, :ci_pipelines, column: :commit_id)
+
+    remove_foreign_key(:ci_builds, column: :commit_id)
+  end
+end

db/migrate/20180508055821_make_remote_mirrors_disabled_by_default.rb

+class MakeRemoteMirrorsDisabledByDefault < ActiveRecord::Migration
+  DOWNTIME = false
+
+  def up
+    change_column_default :remote_mirrors, :enabled, false
+  end
+
+  def down
+    change_column_default :remote_mirrors, :enabled, true
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180503200320) do
+ActiveRecord::Schema.define(version: 20180508055821) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"

doc/development/README.md

@@ -19,6 +19,7 @@ description: 'Learn how to contribute to GitLab.'
 - [Code review guidelines](code_review.md) for reviewing code and having code reviewed.
 - [Automatic CE->EE merge](automatic_ce_ee_merge.md)
 - [Guidelines for implementing Enterprise Edition features](ee_features.md)
+- [Security process for developers](https://gitlab.com/gitlab-org/release/docs/blob/master/general/security/developer.md#security-releases-critical-non-critical-as-a-developer)
 
 ## UX and frontend guides
 

ee/db/migrate/20160530214349_make_remote_mirrors_disabled_by_default.rb → ee/db/migrate/20160530214349_make_remote_mirrors_disabled_by_default_ee.rb

 # See http://doc.gitlab.com/ce/development/migration_style_guide.html
 # for more information on how to write migrations for GitLab.
 
-class MakeRemoteMirrorsDisabledByDefault < ActiveRecord::Migration
+class MakeRemoteMirrorsDisabledByDefaultEE < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
   # When using the methods "add_concurrent_index" or "add_column_with_default"

lib/api/entities.rb

@@ -995,6 +995,7 @@ module API
     class Runner < Grape::Entity
       expose :id
       expose :description
+      expose :ip_address
       expose :active
       expose :is_shared
       expose :name

lib/gitlab/git/repository.rb

@@ -1601,7 +1601,7 @@ module Gitlab
 
       def checksum
         gitaly_migrate(:calculate_checksum,
-                      status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
+                       status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
           if is_enabled
             gitaly_repository_client.calculate_checksum
           else

lib/gitlab/gitaly_client/repository_service.rb

@@ -292,6 +292,8 @@ module Gitlab
         request  = Gitaly::CalculateChecksumRequest.new(repository: @gitaly_repo)
         response = GitalyClient.call(@storage, :repository_service, :calculate_checksum, request)
         response.checksum.presence
+      rescue GRPC::DataLoss => e
+        raise Gitlab::Git::Repository::InvalidRepository.new(e)
       end
 
       def raw_changes_between(from, to)

spec/helpers/projects_helper_spec.rb

@@ -292,7 +292,7 @@ describe ProjectsHelper do
     end
 
     it 'removes the repo path' do
-      repo = "#{storage_path}/namespace/test.git"
+      repo = File.join(storage_path, 'namespace/test.git')
       import_error = "Could not clone #{repo}\n"
 
       expect(sanitize_repo_path(project, import_error)).to eq('Could not clone [REPOS PATH]/namespace/test.git')

spec/migrations/add_pipeline_build_foreign_key_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20180420010016_add_pipeline_build_foreign_key.rb')
+
+describe AddPipelineBuildForeignKey, :migration do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:pipelines) { table(:ci_pipelines) }
+  let(:builds) { table(:ci_builds) }
+
+  before do
+    namespaces.create(id: 10, name: 'gitlab-org', path: 'gitlab-org')
+    projects.create!(id: 11, namespace_id: 10, name: 'gitlab', path: 'gitlab')
+    pipelines.create!(id: 12, project_id: 11, ref: 'master', sha: 'adf43c3a')
+
+    builds.create!(id: 101, commit_id: 12, project_id: 11)
+    builds.create!(id: 102, commit_id: 222, project_id: 11)
+    builds.create!(id: 103, commit_id: 333, project_id: 11)
+    builds.create!(id: 104, commit_id: 12, project_id: 11)
+    builds.create!(id: 106, commit_id: nil, project_id: 11)
+    builds.create!(id: 107, commit_id: 12, project_id: nil)
+  end
+
+  it 'adds foreign key after removing orphans' do
+    expect(builds.all.count).to eq 6
+    expect(foreign_key_exists?(:ci_builds, :ci_pipelines, column: :commit_id)).to be_falsey
+
+    migrate!
+
+    expect(builds.all.pluck(:id)).to eq [101, 104]
+    expect(foreign_key_exists?(:ci_builds, :ci_pipelines, column: :commit_id)).to be_truthy
+  end
+end

spec/models/ci/runner_spec.rb

@@ -198,7 +198,7 @@ describe Ci::Runner do
   end
 
   describe '#assign_to' do
-    let!(:project) { FactoryBot.create :project }
+    let!(:project) { FactoryBot.create(:project) }
     let!(:shared_runner) { FactoryBot.create(:ci_runner, :shared) }
 
     before do

spec/models/concerns/reactive_caching_spec.rb

@@ -29,12 +29,6 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do
     end
   end
 
-  let(:now) { Time.now.utc }
-
-  around do |example|
-    Timecop.freeze(now) { example.run }
-  end
-
   let(:calculation) { -> { 2 + 2 } }
   let(:cache_key) { "foo:666" }
   let(:instance) { CacheTest.new(666, &calculation) }
@@ -49,13 +43,15 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do
     context 'when cache is empty' do
       it { is_expected.to be_nil }
 
-      it 'queues a background worker' do
+      it 'enqueues a background worker to bootstrap the cache' do
         expect(ReactiveCachingWorker).to receive(:perform_async).with(CacheTest, 666)
 
         go!
       end
 
       it 'updates the cache lifespan' do
+        expect(reactive_cache_alive?(instance)).to be_falsy
+
         go!
 
         expect(reactive_cache_alive?(instance)).to be_truthy
@@ -69,6 +65,18 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do
 
       it { is_expected.to eq(2) }
 
+      it 'does not enqueue a background worker' do
+        expect(ReactiveCachingWorker).not_to receive(:perform_async)
+
+        go!
+      end
+
+      it 'updates the cache lifespan' do
+        expect(Rails.cache).to receive(:write).with(alive_reactive_cache_key(instance), true, expires_in: anything)
+
+        go!
+      end
+
       context 'and expired' do
         before do
           invalidate_reactive_cache(instance)

spec/models/project_spec.rb

@@ -344,12 +344,12 @@ describe Project do
 
   describe 'project token' do
     it 'sets an random token if none provided' do
-      project = FactoryBot.create :project, runners_token: ''
+      project = FactoryBot.create(:project, runners_token: '')
       expect(project.runners_token).not_to eq('')
     end
 
     it 'does not set an random token if one provided' do
-      project = FactoryBot.create :project, runners_token: 'my-token'
+      project = FactoryBot.create(:project, runners_token: 'my-token')
       expect(project.runners_token).to eq('my-token')
     end
   end
@@ -682,7 +682,7 @@ describe Project do
   describe '#to_param' do
     context 'with namespace' do
       before do
-        @group = create :group, name: 'gitlab'
+        @group = create(:group, name: 'gitlab')
         @project = create(:project, name: 'gitlabhq', namespace: @group)
       end
 
@@ -995,8 +995,8 @@ describe Project do
 
   describe '#star_count' do
     it 'counts stars from multiple users' do
-      user1 = create :user
-      user2 = create :user
+      user1 = create(:user)
+      user2 = create(:user)
       project = create(:project, :public)
 
       expect(project.star_count).to eq(0)
@@ -1018,7 +1018,7 @@ describe Project do
     end
 
     it 'counts stars on the right project' do
-      user = create :user
+      user = create(:user)
       project1 = create(:project, :public)
       project2 = create(:project, :public)
 
@@ -1277,9 +1277,9 @@ describe Project do
 
   describe '#any_runners?' do
     context 'shared runners' do
-      let(:project) { create :project, shared_runners_enabled: shared_runners_enabled }
-      let(:specific_runner) { create :ci_runner }
-      let(:shared_runner) { create :ci_runner, :shared }
+      let(:project) { create(:project, shared_runners_enabled: shared_runners_enabled) }
+      let(:specific_runner) { create(:ci_runner) }
+      let(:shared_runner) { create(:ci_runner, :shared) }
 
       context 'for shared runners disabled' do
         let(:shared_runners_enabled) { false }
@@ -1337,9 +1337,9 @@ describe Project do
     end
 
     context 'group runners' do
-      let(:project) { create :project, group_runners_enabled: group_runners_enabled }
-      let(:group) { create :group, projects: [project] }
-      let(:group_runner) { create :ci_runner, groups: [group] }
+      let(:project) { create(:project, group_runners_enabled: group_runners_enabled) }
+      let(:group) { create(:group, projects: [project]) }
+      let(:group_runner) { create(:ci_runner, groups: [group]) }
 
       context 'for group runners disabled' do
         let(:group_runners_enabled) { false }
@@ -1421,7 +1421,7 @@ describe Project do
   end
 
   describe '#pages_deployed?' do
-    let(:project) { create :project }
+    let(:project) { create(:project) }
 
     subject { project.pages_deployed? }
 
@@ -1439,8 +1439,8 @@ describe Project do
   end
 
   describe '#pages_url' do
-    let(:group) { create :group, name: group_name }
-    let(:project) { create :project, namespace: group, name: project_name }
+    let(:group) { create(:group, name: group_name) }
+    let(:project) { create(:project, namespace: group, name: project_name) }
     let(:domain) { 'Example.com' }
 
     subject { project.pages_url }
@@ -1466,8 +1466,8 @@ describe Project do
   end
 
   describe '#pages_group_url' do
-    let(:group) { create :group, name: group_name }
-    let(:project) { create :project, namespace: group, name: project_name }
+    let(:group) { create(:group, name: group_name) }
+    let(:project) { create(:project, namespace: group, name: project_name) }
     let(:domain) { 'Example.com' }
     let(:port) { 1234 }
 
@@ -1584,8 +1584,8 @@ describe Project do
     let(:private_group)    { create(:group, visibility_level: 0)  }
     let(:internal_group)   { create(:group, visibility_level: 10) }
 
-    let(:private_project)  { create :project, :private, group: private_group }
-    let(:internal_project) { create :project, :internal, group: internal_group }
+    let(:private_project)  { create(:project, :private, group: private_group) }
+    let(:internal_project) { create(:project, :internal, group: internal_group) }
 
     context 'when group is private project can not be internal' do
       it { expect(private_project.visibility_level_allowed?(Gitlab::VisibilityLevel::INTERNAL)).to be_falsey }
@@ -2074,8 +2074,6 @@ describe Project do
       update_remote_mirrors
     end
 
-    # TODO: study if remote_mirror_available_overridden is still a necessary attribute considering that
-    # it is no longer under any license
     it 'does nothing when remote mirror is disabled globally and not overridden' do
       stub_application_setting(mirror_available: false)
       project.remote_mirror_available_overridden = false
@@ -2815,8 +2813,8 @@ describe Project do
   end
 
   describe '#pages_url' do
-    let(:group) { create :group, name: 'Group' }
-    let(:nested_group) { create :group, parent: group }
+    let(:group) { create(:group, name: 'Group') }
+    let(:nested_group) { create(:group, parent: group) }
     let(:domain) { 'Example.com' }
 
     subject { project.pages_url }
@@ -2827,7 +2825,7 @@ describe Project do
     end
 
     context 'top-level group' do
-      let(:project) { create :project, namespace: group, name: project_name }
+      let(:project) { create(:project, namespace: group, name: project_name) }
 
       context 'group page' do
         let(:project_name) { 'group.example.com' }
@@ -2843,7 +2841,7 @@ describe Project do
     end
 
     context 'nested group' do
-      let(:project) { create :project, namespace: nested_group, name: project_name }
+      let(:project) { create(:project, namespace: nested_group, name: project_name) }
       let(:expected_url) { "http://group.example.com/#{nested_group.path}/#{project.path}" }
 
       context 'group page' do
@@ -2861,7 +2859,7 @@ describe Project do
   end
 
   describe '#http_url_to_repo' do
-    let(:project) { create :project }
+    let(:project) { create(:project) }
 
     it 'returns the url to the repo without a username' do
       expect(project.http_url_to_repo).to eq("#{project.web_url}.git")

spec/requests/api/runners_spec.rb

@@ -46,6 +46,7 @@ describe API::Runners do
         expect(response).to have_gitlab_http_status(200)
         expect(response).to include_pagination_headers
         expect(json_response).to be_an Array
+        expect(json_response[0]).to have_key('ip_address')
         expect(descriptions).to contain_exactly(
           'Project runner', 'Two projects runner'
         )
@@ -59,6 +60,7 @@ describe API::Runners do
         expect(response).to have_gitlab_http_status(200)
         expect(response).to include_pagination_headers
         expect(json_response).to be_an Array
+        expect(json_response[0]).to have_key('ip_address')
         expect(shared).to be_falsey
       end
 
@@ -87,6 +89,7 @@ describe API::Runners do
           expect(response).to have_gitlab_http_status(200)
           expect(response).to include_pagination_headers
           expect(json_response).to be_an Array
+          expect(json_response[0]).to have_key('ip_address')
           expect(shared).to be_truthy
         end
       end
@@ -106,6 +109,7 @@ describe API::Runners do
         expect(response).to have_gitlab_http_status(200)
         expect(response).to include_pagination_headers
         expect(json_response).to be_an Array
+        expect(json_response[0]).to have_key('ip_address')
         expect(shared).to be_falsey
       end
 
@@ -515,6 +519,7 @@ describe API::Runners do
         expect(response).to have_gitlab_http_status(200)
         expect(response).to include_pagination_headers
         expect(json_response).to be_an Array
+        expect(json_response[0]).to have_key('ip_address')
         expect(shared).to be_truthy
       end
     end

spec/support/helpers/migrations_helpers.rb

@@ -30,6 +30,16 @@ module MigrationsHelpers
     end
   end
 
+  def foreign_key_exists?(source, target = nil, column: nil)
+    ActiveRecord::Base.connection.foreign_keys(source).any? do |key|
+      if column
+        key.options[:column].to_s == column.to_s
+      else
+        key.to_table.to_s == target.to_s
+      end
+    end
+  end
+
   def reset_column_in_all_models
     clear_schema_cache!