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

CE upstream - 2018-05-25 09:26 UTC

Closes charts/gitlab#390

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

app/controllers/projects/clusters/applications_controller.rb

@@ -5,9 +5,10 @@ class Projects::Clusters::ApplicationsController < Projects::ApplicationControll
   before_action :authorize_create_cluster!, only: [:create]
 
   def create
-    Clusters::Applications::ScheduleInstallationService.new(project, current_user,
-                                                            application_class: @application_class,
-                                                            cluster: @cluster).execute
+    application = @application_class.find_or_create_by!(cluster: @cluster)
+
+    Clusters::Applications::ScheduleInstallationService.new(project, current_user).execute(application)
+
     head :no_content
   rescue StandardError
     head :bad_request

app/helpers/commits_helper.rb

@@ -63,7 +63,7 @@ module CommitsHelper
   # Returns a link formatted as a commit branch link
   def commit_branch_link(url, text)
     link_to(url, class: 'label label-gray ref-name branch-link') do
-      sprite_icon('fork', size: 12, css_class: 'fork-svg') + "#{text}"
+      sprite_icon('branch', size: 12, css_class: 'fork-svg') + "#{text}"
     end
   end
 
@@ -77,7 +77,7 @@ module CommitsHelper
   # Returns a link formatted as a commit tag link
   def commit_tag_link(url, text)
     link_to(url, class: 'label label-gray ref-name') do
-      icon('tag', class: 'append-right-5') + "#{text}"
+      sprite_icon('tag', size: 12, css_class: 'append-right-5 vertical-align-middle') + "#{text}"
     end
   end
 

app/models/project.rb

@@ -1449,8 +1449,8 @@ class Project < ActiveRecord::Base
     self.runners_token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.runners_token)
   end
 
-  def open_issues_count
-    Projects::OpenIssuesCountService.new(self).count
+  def open_issues_count(current_user = nil)
+    Projects::OpenIssuesCountService.new(self, current_user).count
   end
 
   def open_merge_requests_count

app/services/clusters/applications/schedule_installation_service.rb

 module Clusters
   module Applications
     class ScheduleInstallationService < ::BaseService
-      def execute
-        application_class.find_or_create_by!(cluster: cluster).try do |application|
-          application.make_scheduled!
-          ClusterInstallAppWorker.perform_async(application.name, application.id)
-        end
-      end
-
-      private
-
-      def application_class
-        params[:application_class]
-      end
+      def execute(application)
+        application.make_scheduled!
 
-      def cluster
-        params[:cluster]
+        ClusterInstallAppWorker.perform_async(application.name, application.id)
       end
     end
   end

app/services/projects/open_issues_count_service.rb

@@ -2,14 +2,42 @@ module Projects
   # Service class for counting and caching the number of open issues of a
   # project.
   class OpenIssuesCountService < Projects::CountService
+    include Gitlab::Utils::StrongMemoize
+
+    def initialize(project, user = nil)
+      @user = user
+
+      super(project)
+    end
+
     def cache_key_name
-      'open_issues_count'
+      public_only? ? 'public_open_issues_count' : 'total_open_issues_count'
+    end
+
+    def public_only?
+      !user_is_at_least_reporter?
+    end
+
+    def relation_for_count
+      self.class.query(@project, public_only: public_only?)
+    end
+
+    def user_is_at_least_reporter?
+      strong_memoize(:user_is_at_least_reporter) do
+        @user && @project.team.member?(@user, Gitlab::Access::REPORTER)
+      end
     end
 
-    def self.query(project_ids)
-      # We don't include confidential issues in this number since this would
-      # expose the number of confidential issues to non project members.
-      Issue.opened.public_only.where(project: project_ids)
+    # We only show total issues count for reporters
+    # which are allowed to view confidential issues
+    # This will still show a discrepancy on issues number but should be less than before.
+    # Check https://gitlab.com/gitlab-org/gitlab-ce/issues/38418 description.
+    def self.query(projects, public_only: true)
+      if public_only
+        Issue.opened.public_only.where(project: projects)
+      else
+        Issue.opened.where(project: projects)
+      end
     end
   end
 end

app/views/layouts/nav/sidebar/_project.html.haml

@@ -94,7 +94,7 @@
               = _('Issues')
             - if @project.issues_enabled?
               %span.badge.count.issue_counter
-                = number_with_delimiter(@project.open_issues_count)
+                = number_with_delimiter(@project.open_issues_count(current_user))
 
           %ul.sidebar-sub-level-items
             = nav_link(controller: :issues, action: :index, html_options: { class: "fly-out-top-item" } ) do
@@ -103,7 +103,7 @@
                   = _('Issues')
                 - if @project.issues_enabled?
                   %span.badge.count.issue_counter.fly-out-badge
-                    = number_with_delimiter(@project.open_issues_count)
+                    = number_with_delimiter(@project.open_issues_count(current_user))
             %li.divider.fly-out-top-item
             = nav_link(controller: :issues, action: :index) do
               = link_to project_issues_path(@project), title: 'Issues' do

app/views/projects/commit/branches.html.haml

@@ -6,7 +6,8 @@
 
 - if @branches.any? || @tags.any? || @tags_limit_exceeded
   %span
-    = link_to "…", "#", class: "js-details-expand label label-gray"
+    = link_to "#", class: "js-details-expand label label-gray ref-name" do
+      = sprite_icon('ellipsis_h', size: 12, css_class: 'vertical-align-middle')
     %span.js-details-content.hide
       = commit_branches_links(@project, @branches)
       - if @tags_limit_exceeded

changelogs/unreleased/commit-branch-tag-icon-update.yml

+---
+title: Updated icons for branch and tag names in commit details
+merge_request: 18953
+author: Constance Okoghenun
+type: changed

changelogs/unreleased/issue_38418.yml

+---
+title: Fix issue count on sidebar
+merge_request:
+author:
+type: other

changelogs/unreleased/migrate-restore-repo-to-gitaly.yml

+---
+title: Support restoring repositories into gitaly
+merge_request:
+author:
+type: changed

config/initializers/7_prometheus_metrics.rb

@@ -25,7 +25,7 @@ Sidekiq.configure_server do |config|
   end
 end
 
-if Gitlab::Metrics.prometheus_metrics_enabled?
+if !Rails.env.test? && Gitlab::Metrics.prometheus_metrics_enabled?
   unless Sidekiq.server?
     Gitlab::Metrics::Samplers::UnicornSampler.initialize_instance(Settings.monitoring.unicorn_sampler_interval).start
   end

lib/backup/repository.rb

@@ -73,6 +73,9 @@ module Backup
     end
 
     def prepare_directories
+      # TODO: Need to find a way to do this for gitaly
+      # Gitaly discussion issue: https://gitlab.com/gitlab-org/gitaly/issues/1194
+
       Gitlab.config.repositories.storages.each do |name, repository_storage|
         path = repository_storage.legacy_disk_path
         next unless File.exist?(path)
@@ -93,70 +96,65 @@ module Backup
       end
     end
 
+    def restore_custom_hooks(project)
+      # TODO: Need to find a way to do this for gitaly
+      # Gitaly migration issue: https://gitlab.com/gitlab-org/gitaly/issues/1195
+      in_path(path_to_tars(project)) do |dir|
+        path_to_project_repo = path_to_repo(project)
+        cmd = %W(tar -xf #{path_to_tars(project, dir)} -C #{path_to_project_repo} #{dir})
+
+        output, status = Gitlab::Popen.popen(cmd)
+        unless status.zero?
+          progress_warn(project, cmd.join(' '), output)
+        end
+      end
+    end
+
     def restore
       prepare_directories
+      gitlab_shell = Gitlab::Shell.new
       Project.find_each(batch_size: 1000) do |project|
-        progress.print " * #{display_repo_path(project)} ... "
-        path_to_project_repo = path_to_repo(project)
+        progress.print " * #{project.full_path} ... "
         path_to_project_bundle = path_to_bundle(project)
-
         project.ensure_storage_path_exists
 
-        cmd = if File.exist?(path_to_project_bundle)
-                %W(#{Gitlab.config.git.bin_path} clone --bare --mirror #{path_to_project_bundle} #{path_to_project_repo})
-              else
-                %W(#{Gitlab.config.git.bin_path} init --bare #{path_to_project_repo})
-              end
+        restore_repo_success = nil
+        if File.exist?(path_to_project_bundle)
+          begin
+            gitlab_shell.remove_repository(project.repository_storage, project.disk_path) if project.repository_exists?
+            project.repository.create_from_bundle path_to_project_bundle
+            restore_repo_success = true
+          rescue => e
+            restore_repo_success = false
+            progress.puts "Error: #{e}".color(:red)
+          end
+        else
+          restore_repo_success = gitlab_shell.create_repository(project.repository_storage, project.disk_path)
+        end
 
-        output, status = Gitlab::Popen.popen(cmd)
-        if status.zero?
+        if restore_repo_success
           progress.puts "[DONE]".color(:green)
         else
-          progress_warn(project, cmd.join(' '), output)
+          progress.puts "[Failed] restoring #{project.full_path} repository".color(:red)
         end
 
-        in_path(path_to_tars(project)) do |dir|
-          cmd = %W(tar -xf #{path_to_tars(project, dir)} -C #{path_to_project_repo} #{dir})
-
-          output, status = Gitlab::Popen.popen(cmd)
-          unless status.zero?
-            progress_warn(project, cmd.join(' '), output)
-          end
-        end
+        restore_custom_hooks(project)
 
         wiki = ProjectWiki.new(project)
-        path_to_wiki_repo = path_to_repo(wiki)
         path_to_wiki_bundle = path_to_bundle(wiki)
 
         if File.exist?(path_to_wiki_bundle)
-          progress.print " * #{display_repo_path(wiki)} ... "
-
-          # If a wiki bundle exists, first remove the empty repo
-          # that was initialized with ProjectWiki.new() and then
-          # try to restore with 'git clone --bare'.
-          FileUtils.rm_rf(path_to_wiki_repo)
-          cmd = %W(#{Gitlab.config.git.bin_path} clone --bare #{path_to_wiki_bundle} #{path_to_wiki_repo})
-
-          output, status = Gitlab::Popen.popen(cmd)
-          if status.zero?
-            progress.puts " [DONE]".color(:green)
-          else
-            progress_warn(project, cmd.join(' '), output)
+          progress.print " * #{wiki.full_path} ... "
+          begin
+            gitlab_shell.remove_repository(wiki.repository_storage, wiki.disk_path) if wiki.repository_exists?
+            wiki.repository.create_from_bundle(path_to_wiki_bundle)
+            progress.puts "[DONE]".color(:green)
+          rescue => e
+            progress.puts "[Failed] restoring #{wiki.full_path} wiki".color(:red)
+            progress.puts "Error #{e}".color(:red)
           end
         end
       end
-
-      progress.print 'Put GitLab hooks in repositories dirs'.color(:yellow)
-      cmd = %W(#{Gitlab.config.gitlab_shell.path}/bin/create-hooks) + repository_storage_paths_args
-
-      output, status = Gitlab::Popen.popen(cmd)
-      if status.zero?
-        progress.puts " [DONE]".color(:green)
-      else
-        puts " [FAILED]".color(:red)
-        puts "failed: #{cmd}"
-        puts output
-      end
     end
     # rubocop:enable Metrics/AbcSize
 

package.json

@@ -16,7 +16,7 @@
     "webpack-prod": "NODE_ENV=production webpack --config config/webpack.config.js"
   },
   "dependencies": {
-    "@gitlab-org/gitlab-svgs": "^1.22.0",
+    "@gitlab-org/gitlab-svgs": "^1.23.0",
     "autosize": "^4.0.0",
     "axios": "^0.17.1",
     "babel-core": "^6.26.3",

spec/lib/backup/repository_spec.rb

@@ -48,14 +48,14 @@ describe Backup::Repository do
 
     describe 'command failure' do
       before do
-        allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1])
+        allow_any_instance_of(Gitlab::Shell).to receive(:create_repository).and_return(false)
       end
 
       context 'hashed storage' do
         it 'shows the appropriate error' do
           subject.restore
 
-          expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} (#{project.disk_path}) - error")
+          expect(progress).to have_received(:puts).with("[Failed] restoring #{project.full_path} repository")
         end
       end
 
@@ -65,33 +65,10 @@ describe Backup::Repository do
         it 'shows the appropriate error' do
           subject.restore
 
-          expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} - error")
+          expect(progress).to have_received(:puts).with("[Failed] restoring #{project.full_path} repository")
         end
       end
     end
-
-    describe 'folders without permissions' do
-      before do
-        allow(FileUtils).to receive(:mv).and_raise(Errno::EACCES)
-      end
-
-      it 'shows error message' do
-        expect(subject).to receive(:access_denied_error)
-        subject.restore
-      end
-    end
-
-    describe 'folder that is a mountpoint' do
-      before do
-        allow(FileUtils).to receive(:mv).and_raise(Errno::EBUSY)
-      end
-
-      it 'shows error message' do
-        expect(subject).to receive(:resource_busy_error).and_call_original
-
-        expect { subject.restore }.to raise_error(/is a mountpoint/)
-      end
-    end
   end
 
   describe '#empty_repo?' do

spec/services/clusters/applications/schedule_installation_service_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Clusters::Applications::ScheduleInstallationService do
   def count_scheduled
-    application_class&.with_status(:scheduled)&.count || 0
+    application&.class&.with_status(:scheduled)&.count || 0
   end
 
   shared_examples 'a failing service' do
@@ -10,45 +10,42 @@ describe Clusters::Applications::ScheduleInstallationService do
       expect(ClusterInstallAppWorker).not_to receive(:perform_async)
       count_before = count_scheduled
 
-      expect { service.execute }.to raise_error(StandardError)
+      expect { service.execute(application) }.to raise_error(StandardError)
       expect(count_scheduled).to eq(count_before)
     end
   end
 
   describe '#execute' do
-    let(:application_class) { Clusters::Applications::Helm }
-    let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
-    let(:project) { cluster.project }
-    let(:service) { described_class.new(project, nil, cluster: cluster, application_class: application_class) }
+    let(:project) { double(:project) }
+    let(:service) { described_class.new(project, nil) }
 
-    it 'creates a new application' do
-      allow(ClusterInstallAppWorker).to receive(:perform_async)
+    context 'when application is installable' do
+      let(:application) { create(:clusters_applications_helm, :installable) }
 
-      expect { service.execute }.to change { application_class.count }.by(1)
-    end
-
-    it 'make the application scheduled' do
-      expect(ClusterInstallAppWorker).to receive(:perform_async).with(application_class.application_name, kind_of(Numeric)).once
+      it 'make the application scheduled' do
+        expect(ClusterInstallAppWorker).to receive(:perform_async).with(application.name, kind_of(Numeric)).once
 
-      expect { service.execute }.to change { application_class.with_status(:scheduled).count }.by(1)
+        expect { service.execute(application) }.to change { application.class.with_status(:scheduled).count }.by(1)
+      end
     end
 
     context 'when installation is already in progress' do
       let(:application) { create(:clusters_applications_helm, :installing) }
-      let(:cluster) { application.cluster }
 
       it_behaves_like 'a failing service'
     end
 
-    context 'when application_class is nil' do
-      let(:application_class) { nil }
+    context 'when application is nil' do
+      let(:application) { nil }
 
       it_behaves_like 'a failing service'
     end
 
     context 'when application cannot be persisted' do
+      let(:application) { create(:clusters_applications_helm) }
+
       before do
-        expect_any_instance_of(application_class).to receive(:make_scheduled!).once.and_raise(ActiveRecord::RecordInvalid)
+        expect(application).to receive(:make_scheduled!).once.and_raise(ActiveRecord::RecordInvalid)
       end
 
       it_behaves_like 'a failing service'

spec/services/projects/open_issues_count_service_spec.rb

@@ -2,20 +2,53 @@ require 'spec_helper'
 
 describe Projects::OpenIssuesCountService do
   describe '#count' do
-    it 'returns the number of open issues' do
-      project = create(:project)
-      create(:issue, :opened, project: project)
+    let(:project) { create(:project) }
 
-      expect(described_class.new(project).count).to eq(1)
+    context 'when user is nil' do
+      it 'does not include confidential issues in the issue count' do
+        create(:issue, :opened, project: project)
+        create(:issue, :opened, confidential: true, project: project)
+
+        expect(described_class.new(project).count).to eq(1)
+      end
     end
 
-    it 'does not include confidential issues in the issue count' do
-      project = create(:project)
+    context 'when user is provided' do
+      let(:user) { create(:user) }
+
+      context 'when user can read confidential issues' do
+        before do
+          project.add_reporter(user)
+        end
+
+        it 'returns the right count with confidential issues' do
+          create(:issue, :opened, project: project)
+          create(:issue, :opened, confidential: true, project: project)
+
+          expect(described_class.new(project, user).count).to eq(2)
+        end
+
+        it 'uses total_open_issues_count cache key' do
+          expect(described_class.new(project, user).cache_key_name).to eq('total_open_issues_count')
+        end
+      end
+
+      context 'when user cannot read confidential issues' do
+        before do
+          project.add_guest(user)
+        end
+
+        it 'does not include confidential issues' do
+          create(:issue, :opened, project: project)
+          create(:issue, :opened, confidential: true, project: project)
 
-      create(:issue, :opened, project: project)
-      create(:issue, :opened, confidential: true, project: project)
+          expect(described_class.new(project, user).count).to eq(1)
+        end
 
-      expect(described_class.new(project).count).to eq(1)
+        it 'uses public_open_issues_count cache key' do
+          expect(described_class.new(project, user).cache_key_name).to eq('public_open_issues_count')
+        end
+      end
     end
   end
 end

yarn.lock

@@ -54,9 +54,9 @@
     lodash "^4.2.0"
     to-fast-properties "^2.0.0"
 
-"@gitlab-org/gitlab-svgs@^1.22.0":
-  version "1.22.0"
-  resolved "https://registry.yarnpkg.com/@gitlab-org/gitlab-svgs/-/gitlab-svgs-1.22.0.tgz#9f2daefebcda911cba8341313c8c464c8087fe44"
+"@gitlab-org/gitlab-svgs@^1.23.0":
+  version "1.23.0"
+  resolved "https://registry.yarnpkg.com/@gitlab-org/gitlab-svgs/-/gitlab-svgs-1.23.0.tgz#42047aeedcc06bc12d417ed1efadad1749af9670"
 
 "@mrmlnc/readdir-enhanced@^2.2.1":
   version "2.2.1"