Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-11-02

CHANGELOG.md

@@ -2,6 +2,13 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 11.4.4 (2018-10-30)
+
+### Security (1 change)
+
+- Monkey kubeclient to not follow any redirects.
+
+
 ## 11.4.3 (2018-10-26)
 
 - No changes.
@@ -250,6 +257,13 @@ entry.
 - Check frozen string in style builds. (gfyoung)
 
 
+## 11.3.9 (2018-10-31)
+
+### Security (1 change)
+
+- Monkey kubeclient to not follow any redirects.
+
+
 ## 11.3.8 (2018-10-27)
 
 - No changes.
@@ -556,6 +570,13 @@ entry.
 - Creates Vue component for artifacts block on job page.
 
 
+## 11.2.8 (2018-10-31)
+
+### Security (1 change)
+
+- Monkey kubeclient to not follow any redirects.
+
+
 ## 11.2.7 (2018-10-27)
 
 - No changes.

app/assets/javascripts/commons/gitlab_ui.js

 import Vue from 'vue';
-import { GlProgressBar, GlLoadingIcon, GlTooltipDirective } from '@gitlab-org/gitlab-ui';
+import { GlLoadingIcon, GlTooltipDirective } from '@gitlab-org/gitlab-ui';
 
-Vue.component('gl-progress-bar', GlProgressBar);
 Vue.component('gl-loading-icon', GlLoadingIcon);
 
 Vue.directive('gl-tooltip', GlTooltipDirective);

app/assets/javascripts/sidebar/components/time_tracking/comparison_pane.vue

 <script>
 import { parseSeconds, stringifyTime } from '~/lib/utils/datetime_utility';
 import tooltip from '../../../vue_shared/directives/tooltip';
+import { GlProgressBar } from '@gitlab-org/gitlab-ui';
 
 export default {
   name: 'TimeTrackingComparisonPane',
+  components: {
+    GlProgressBar,
+  },
   directives: {
     tooltip,
   },

app/assets/javascripts/vue_shared/components/markdown/header.vue

 <script>
 import $ from 'jquery';
-import Tooltip from '../../directives/tooltip';
+import { GlTooltipDirective } from '@gitlab-org/gitlab-ui';
 import ToolbarButton from './toolbar_button.vue';
 import Icon from '../icon.vue';
 
 export default {
-  directives: {
-    Tooltip,
-  },
   components: {
     ToolbarButton,
     Icon,
   },
+  directives: {
+    GlTooltip: GlTooltipDirective,
+  },
   props: {
     previewMarkdown: {
       type: Boolean,
@@ -147,7 +147,7 @@ export default {
           icon="table"
         />
         <button
-          v-tooltip
+          v-gl-tooltip
           aria-label="Go full screen"
           class="toolbar-btn toolbar-fullscreen-btn js-zen-enter"
           data-container="body"

app/assets/javascripts/vue_shared/components/markdown/toolbar_button.vue

 <script>
-import tooltip from '../../directives/tooltip';
-import icon from '../icon.vue';
+import { GlTooltipDirective } from '@gitlab-org/gitlab-ui';
+import Icon from '../icon.vue';
 
 export default {
   components: {
-    icon,
+    Icon,
   },
   directives: {
-    tooltip,
+    GlTooltip: GlTooltipDirective,
   },
   props: {
     buttonTitle: {
@@ -43,7 +43,7 @@ export default {
 
 <template>
   <button
-    v-tooltip
+    v-gl-tooltip
     :data-md-tag="tag"
     :data-md-select="tagSelect"
     :data-md-block="tagBlock"

app/helpers/labels_helper.rb

@@ -143,7 +143,7 @@ module LabelsHelper
 
   def labels_filter_path(options = {})
     project = @target_project || @project
-    format = options.delete(:format) || :html
+    format = options.delete(:format)
 
     if project
       project_labels_path(project, format, options)

app/services/merge_requests/reload_diffs_service.rb

@@ -36,7 +36,10 @@ module MergeRequests
       # Remove cache for all diffs on this MR. Do not use the association on the
       # model, as that will interfere with other actions happening when
       # reloading the diff.
-      MergeRequestDiff.where(merge_request: merge_request).each do |merge_request_diff|
+      MergeRequestDiff
+        .where(merge_request: merge_request)
+        .preload(merge_request: :target_project)
+        .find_each do |merge_request_diff|
         next if merge_request_diff == new_diff
 
         cacheable_collection(merge_request_diff).clear_cache

changelogs/unreleased/52548-links-in-tabs-of-the-labels-index-pages-ends-with-html.yml

+---
+title: Fix bug when links in tabs of the labels index pages ends with .html
+merge_request: 22716
+author:
+type: fixed

changelogs/unreleased/ab-45608-stuck-mr-query.yml

+---
+title: Add index to find stuck merge requests.
+merge_request: 22749
+author:
+type: performance

changelogs/unreleased/gl-ui-progress-bar.yml

+---
+title: Remove gitlab-ui's progress bar from global
+merge_request:
+author:
+type: performance

changelogs/unreleased/kinolaev-master-patch-91872.yml

+---
+title: Change HELM_HOST in Auto-DevOps template to work behind proxy
+merge_request: 22596
+author: Sergej Nikolaev <kinolaev@gmail.com>
+type: fixed

changelogs/unreleased/replace-tooltip-in-markdown-component.yml

+---
+title: Replace tooltip in markdown component with gl-tooltip
+merge_request: 21989
+author: George Tsiolis
+type: other

changelogs/unreleased/security-kubeclient-ssrf.yml

+---
+title: Monkey kubeclient to not follow any redirects.
+merge_request:
+author:
+type: security

changelogs/unreleased/sh-optimize-reload-diffs-service.yml

+---
+title: Significantly cut memory usage and SQL queries when reloading diffs
+merge_request: 22725
+author:
+type: performance

config/initializers/8_metrics.rb

@@ -127,7 +127,11 @@ end
 # check: https://github.com/rspec/rspec-mocks#settings-mocks-or-stubs-on-any-instance-of-a-class
 #
 # Related issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/33587
-if Gitlab::Metrics.enabled? && !Rails.env.test?
+#
+# In development mode, we turn off eager loading when we're running
+# `rails generate migration` because eager loading short-circuits the
+# loading of our custom migration templates.
+if Gitlab::Metrics.enabled? && !Rails.env.test? && !(Rails.env.development? && defined?(Rails::Generators))
   require 'pathname'
   require 'influxdb'
   require 'connection_pool'

config/initializers/kubeclient.rb

@@ -13,4 +13,25 @@ class Kubeclient::Client
     ns_prefix = build_namespace_prefix(namespace)
     rest_client["#{ns_prefix}#{entity_name_plural}/#{name}:#{port}/proxy"].url
   end
+
+  # Monkey patch to set `max_redirects: 0`, so that kubeclient
+  # does not follow redirects and expose internal services.
+  # See https://gitlab.com/gitlab-org/gitlab-ce/issues/53158
+  def create_rest_client(path = nil)
+    path ||= @api_endpoint.path
+    options = {
+      ssl_ca_file: @ssl_options[:ca_file],
+      ssl_cert_store: @ssl_options[:cert_store],
+      verify_ssl: @ssl_options[:verify_ssl],
+      ssl_client_cert: @ssl_options[:client_cert],
+      ssl_client_key: @ssl_options[:client_key],
+      proxy: @http_proxy_uri,
+      user: @auth_options[:username],
+      password: @auth_options[:password],
+      open_timeout: @timeouts[:open],
+      read_timeout: @timeouts[:read],
+      max_redirects: 0
+    }
+    RestClient::Resource.new(@api_endpoint.merge(path).to_s, options)
+  end
 end

db/migrate/20181101144347_add_index_for_stuck_mr_query.rb

+# frozen_string_literal: true
+class AddIndexForStuckMrQuery < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :merge_requests, [:id, :merge_jid], where: "merge_jid IS NOT NULL and state = 'locked'"
+  end
+
+  def down
+    remove_concurrent_index :merge_requests, [:id, :merge_jid], where: "merge_jid IS NOT NULL and state = 'locked'"
+  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: 20181031190559) do
+ActiveRecord::Schema.define(version: 20181101144347) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1759,6 +1759,7 @@ ActiveRecord::Schema.define(version: 20181031190559) do
   add_index "merge_requests", ["created_at"], name: "index_merge_requests_on_created_at", using: :btree
   add_index "merge_requests", ["description"], name: "index_merge_requests_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
   add_index "merge_requests", ["head_pipeline_id"], name: "index_merge_requests_on_head_pipeline_id", using: :btree
+  add_index "merge_requests", ["id", "merge_jid"], name: "index_merge_requests_on_id_and_merge_jid", where: "((merge_jid IS NOT NULL) AND ((state)::text = 'locked'::text))", using: :btree
   add_index "merge_requests", ["latest_merge_request_diff_id"], name: "index_merge_requests_on_latest_merge_request_diff_id", using: :btree
   add_index "merge_requests", ["merge_user_id"], name: "index_merge_requests_on_merge_user_id", where: "(merge_user_id IS NOT NULL)", using: :btree
   add_index "merge_requests", ["milestone_id"], name: "index_merge_requests_on_milestone_id", using: :btree

doc/user/project/import/index.md

 # Migrating projects to a GitLab instance
 
-1. [From Bitbucket.org](bitbucket.md)
+1. [From Bitbucket Cloud (aka bitbucket.org)](bitbucket.md)
+1. [From Bitbucket Server (aka Stash)](bitbucket_server.md)
 1. [From ClearCase](clearcase.md)
 1. [From CVS](cvs.md)
 1. [From FogBugz](fogbugz.md)

lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml

@@ -823,7 +823,7 @@ rollout 100%:
   function initialize_tiller() {
     echo "Checking Tiller..."
 
-    export HELM_HOST=":44134"
+    export HELM_HOST="localhost:44134"
     tiller -listen ${HELM_HOST} -alsologtostderr > /dev/null 2>&1 &
     echo "Tiller is listening on ${HELM_HOST}"
 

lib/gitlab/diff/file_collection/base.rb

@@ -17,7 +17,6 @@ module Gitlab
 
           @diffable = diffable
           @include_stats = diff_options.delete(:include_stats)
-          @diffs = diffable.raw_diffs(diff_options)
           @project = project
           @diff_options = diff_options
           @diff_refs = diff_refs
@@ -25,8 +24,12 @@ module Gitlab
           @repository = project.repository
         end
 
+        def diffs
+          @diffs ||= diffable.raw_diffs(diff_options)
+        end
+
         def diff_files
-          @diff_files ||= @diffs.decorate! { |diff| decorate_diff!(diff) }
+          @diff_files ||= diffs.decorate! { |diff| decorate_diff!(diff) }
         end
 
         def diff_file_with_old_path(old_path)

spec/helpers/labels_helper_spec.rb

@@ -211,4 +211,29 @@ describe LabelsHelper do
       end
     end
   end
+
+  describe 'labels_filter_path' do
+    let(:group) { create(:group) }
+    let(:project) { create(:project) }
+
+    it 'links to the dashboard labels page' do
+      expect(labels_filter_path).to eq(dashboard_labels_path)
+    end
+
+    it 'links to the group labels page' do
+      assign(:group, group)
+
+      expect(helper.labels_filter_path).to eq(group_labels_path(group))
+    end
+
+    it 'links to the project labels page' do
+      assign(:project, project)
+
+      expect(helper.labels_filter_path).to eq(project_labels_path(project))
+    end
+
+    it 'supports json format' do
+      expect(labels_filter_path(format: :json)).to eq(dashboard_labels_path(format: :json))
+    end
+  end
 end

spec/models/merge_request_diff_spec.rb

@@ -52,9 +52,9 @@ describe MergeRequestDiff do
 
     context 'when it was not cleaned by the system' do
       it 'returns persisted diffs' do
-        expect(diff).to receive(:load_diffs)
+        expect(diff).to receive(:load_diffs).and_call_original
 
-        diff.diffs
+        diff.diffs.diff_files
       end
     end
 
@@ -76,19 +76,19 @@ describe MergeRequestDiff do
       end
 
       it 'returns persisted diffs if cannot compare with diff refs' do
-        expect(diff).to receive(:load_diffs)
+        expect(diff).to receive(:load_diffs).and_call_original
 
         diff.update!(head_commit_sha: 'invalid-sha')
 
-        diff.diffs
+        diff.diffs.diff_files
       end
 
       it 'returns persisted diffs if diff refs does not exist' do
-        expect(diff).to receive(:load_diffs)
+        expect(diff).to receive(:load_diffs).and_call_original
 
         diff.update!(start_commit_sha: nil, base_commit_sha: nil)
 
-        diff.diffs
+        diff.diffs.diff_files
       end
     end
   end

spec/models/merge_request_spec.rb

@@ -553,9 +553,9 @@ describe MergeRequest do
       it 'delegates to the MR diffs' do
         merge_request.save
 
-        expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(hash_including(options))
+        expect(merge_request.merge_request_diff).to receive(:raw_diffs).with(hash_including(options)).and_call_original
 
-        merge_request.diffs(options)
+        merge_request.diffs(options).diff_files
       end
     end
 

spec/services/merge_requests/reload_diffs_service_spec.rb

@@ -60,6 +60,17 @@ describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_cachin
 
         subject.execute
       end
+
+      it 'avoids N+1 queries', :request_store do
+        current_user
+        merge_request
+
+        control_count = ActiveRecord::QueryRecorder.new do
+          subject.execute
+        end.count
+
+        expect { subject.execute }.not_to exceed_query_limit(control_count)
+      end
     end
   end
 end