Merge tag 'debian/13.0.6-1' into buster-fasttrack

gitlab Debian release 13.0.6-1

CHANGELOG-EE.md

@@ -1,5 +1,13 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
+## 13.0.5 (2020-06-04)
+
+- No changes.
+
+## 13.0.4 (2020-06-03)
+
+- No changes.
+
 ## 13.0.3 (2020-05-29)
 
 - No changes.

CHANGELOG.md

@@ -2,6 +2,24 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 13.0.6 (2020-06-10)
+
+- No changes.
+
+## 13.0.5 (2020-06-04)
+
+### Fixed (4 changes)
+
+- Fix NoMethodError by using the correct method to report exceptions to Sentry. !33260
+- Fix bug in snippets updating only file_name or content. !33375
+- Fix ambiguous string concatenation on CleanupProjectsWithMissingNamespace. !33497
+- Fix linking alerts to created issues for the Generic alerts intergration. !33647
+
+### Other (1 change)
+
+- Update GitLab Workhorse to v8.31.2. !33818
+
+
 ## 13.0.4 (2020-06-03)
 
 ### Security (1 change)

GITALY_SERVER_VERSION

@@ -1 +1 @@
-13.0.4
+13.0.6

GITLAB_WORKHORSE_VERSION

@@ -1 +1 @@
-8.31.1
+8.31.2

VERSION

@@ -1 +1 @@
-13.0.4
+13.0.6

app/models/snippet_repository.rb

@@ -53,10 +53,21 @@ class SnippetRepository < ApplicationRecord
   def transform_file_entries(files)
     next_index = get_last_empty_file_index + 1
 
-    files.each do |file_entry|
+    files.map do |file_entry|
       file_entry[:file_path] = file_path_for(file_entry, next_index) { next_index += 1 }
       file_entry[:action] = infer_action(file_entry) unless file_entry[:action]
-    end
+      file_entry[:action] = file_entry[:action].to_sym
+
+      if only_rename_action?(file_entry)
+        file_entry[:infer_content] = true
+      elsif empty_update_action?(file_entry)
+        # There is no need to perform a repository operation
+        # When the update action has no content
+        file_entry = nil
+      end
+
+      file_entry
+    end.compact
   end
 
   def file_path_for(file_entry, next_index)
@@ -111,4 +122,12 @@ class SnippetRepository < ApplicationRecord
     err.is_a?(ArgumentError) &&
       err.message.downcase.match?(/failed to parse signature/)
   end
+
+  def only_rename_action?(action)
+    action[:action] == :move && action[:content].nil?
+  end
+
+  def empty_update_action?(action)
+    action[:action] == :update && action[:content].nil?
+  end
 end

app/services/ci/stop_environments_service.rb

@@ -28,7 +28,7 @@ module Ci
       stop_actions.each do |stop_action|
         stop_action.play(stop_action.user)
       rescue => e
-        Gitlab::ErrorTracking.track_error(e, deployable_id: stop_action.id)
+        Gitlab::ErrorTracking.track_exception(e, deployable_id: stop_action.id)
       end
     end
 

app/services/snippets/update_service.rb

@@ -85,8 +85,10 @@ module Snippets
     end
 
     def snippet_files(snippet)
-      [{ previous_path: snippet.file_name_on_repo,
-         file_path: params[:file_name],
+      file_name_on_repo = snippet.file_name_on_repo
+
+      [{ previous_path: file_name_on_repo,
+         file_path: params[:file_name] || file_name_on_repo,
          content: params[:content] }]
     end
 

app/workers/incident_management/process_alert_worker.rb

@@ -12,7 +12,7 @@ module IncidentManagement
       return unless project
 
       new_issue = create_issue(project, alert_payload)
-      return unless am_alert_id && new_issue.persisted?
+      return unless am_alert_id && new_issue&.persisted?
 
       link_issue_with_alert(am_alert_id, new_issue.id)
     end
@@ -27,6 +27,7 @@ module IncidentManagement
       IncidentManagement::CreateIssueService
         .new(project, alert_payload)
         .execute
+        .dig(:issue)
     end
 
     def link_issue_with_alert(alert_id, issue_id)

db/post_migrate/20200511083541_cleanup_projects_with_missing_namespace.rb

@@ -249,8 +249,8 @@ class CleanupProjectsWithMissingNamespace < ActiveRecord::Migration[6.0]
         -- Names are expected to be unique inside their namespace
         --  (uniqueness validation on namespace_id, name)
         -- Attach the id to the name and path to make sure that they are unique
-        name = name || '_' || id,
-        path = path || '_' || id
+        name = name || '_' || id::text,
+        path = path || '_' || id::text
       SQL
     end
   end

debian/changelog

@@ -1,3 +1,10 @@
+gitlab (13.0.6-1) experimental; urgency=medium
+
+  * New upstream version 13.0.6 with a security fix (CVE ID not
+    assigned yet)
+
+ -- Pirate Praveen <praveen@debian.org>  Thu, 11 Jun 2020 17:01:47 +0530
+
 gitlab (13.0.4-1+fto10+1) buster-fasttrack; urgency=medium
 
   * Rebuild for buster-fasttrack.

doc/administration/operations/moving_repositories.md

@@ -85,8 +85,8 @@ not included in GitLab so you need to install it yourself with apt
 or yum. Also note that the GitLab scripts we used below were added
 in GitLab 8.1.
 
-** This process does not clean up repositories at the target location that no
-longer exist at the source. ** If you start using your GitLab instance with
+**This process does not clean up repositories at the target location that no
+longer exist at the source.** If you start using your GitLab instance with
 `/mnt/gitlab/repositories`, you need to run `gitlab-rake gitlab:cleanup:repos`
 after switching to the new repository storage directory.
 

spec/models/snippet_repository_spec.rb

@@ -116,17 +116,81 @@ describe SnippetRepository do
       end
 
       context 'when commit actions are present' do
-        let(:file_action) { { file_path: 'foo.txt', content: 'foo', action: :foobar } }
-        let(:data) { [file_action] }
+        shared_examples 'uses the expected action' do |action, expected_action|
+          let(:file_action) { { file_path: 'foo.txt', content: 'foo', action: action } }
+          let(:data) { [file_action] }
 
-        it 'does not change commit action' do
-          expect(repo).to(
-            receive(:multi_action).with(
-              user,
-              hash_including(actions: array_including(hash_including(action: :foobar)))))
+          specify do
+            expect(repo).to(
+              receive(:multi_action).with(
+                user,
+                hash_including(actions: array_including(hash_including(action: expected_action)))))
 
-          snippet_repository.multi_files_action(user, data, commit_opts)
+            snippet_repository.multi_files_action(user, data, commit_opts)
+          end
         end
+
+        it_behaves_like 'uses the expected action', :foobar, :foobar
+
+        context 'when action is a string' do
+          it_behaves_like 'uses the expected action', 'foobar', :foobar
+        end
+      end
+    end
+
+    context 'when move action does not include content' do
+      let(:previous_path) { 'CHANGELOG' }
+      let(:new_path) { 'CHANGELOG_new' }
+      let(:move_action) { { previous_path: previous_path, file_path: new_path, action: action } }
+
+      shared_examples 'renames file and does not update content' do
+        specify do
+          existing_content = blob_at(snippet, previous_path).data
+
+          snippet_repository.multi_files_action(user, [move_action], commit_opts)
+
+          blob = blob_at(snippet, new_path)
+          expect(blob).not_to be_nil
+          expect(blob.data).to eq existing_content
+        end
+      end
+
+      context 'when action is not set' do
+        let(:action) { nil }
+
+        it_behaves_like 'renames file and does not update content'
+      end
+
+      context 'when action is set' do
+        let(:action) { :move }
+
+        it_behaves_like 'renames file and does not update content'
+      end
+    end
+
+    context 'when update action does not include content' do
+      let(:update_action) { { previous_path: 'CHANGELOG', file_path: 'CHANGELOG', action: action } }
+
+      shared_examples 'does not commit anything' do
+        specify do
+          last_commit_id = snippet.repository.head_commit.id
+
+          snippet_repository.multi_files_action(user, [update_action], commit_opts)
+
+          expect(snippet.repository.head_commit.id).to eq last_commit_id
+        end
+      end
+
+      context 'when action is not set' do
+        let(:action) { nil }
+
+        it_behaves_like 'does not commit anything'
+      end
+
+      context 'when action is set' do
+        let(:action) { :update }
+
+        it_behaves_like 'does not commit anything'
       end
     end
 

spec/services/ci/stop_environments_service_spec.rb

@@ -222,8 +222,10 @@ describe Ci::StopEnvironmentsService do
 
       it 'tracks the exception' do
         expect(Gitlab::ErrorTracking)
-          .to receive(:track_error)
-          .with(Gitlab::Access::AccessDeniedError, anything).twice
+          .to receive(:track_exception)
+          .with(Gitlab::Access::AccessDeniedError, anything)
+          .twice
+          .and_call_original
 
         subject
       end

spec/services/snippets/update_service_spec.rb

@@ -302,6 +302,60 @@ describe Snippets::UpdateService do
       end
     end
 
+    shared_examples 'only file_name is present' do
+      let(:base_opts) do
+        {
+          file_name: file_name
+        }
+      end
+
+      shared_examples 'content is not updated' do
+        specify do
+          existing_content = snippet.blobs.first.data
+          response = subject
+          snippet = response.payload[:snippet]
+
+          blob = snippet.repository.blob_at('master', file_name)
+
+          expect(blob).not_to be_nil
+          expect(response).to be_success
+          expect(blob.data).to eq existing_content
+        end
+      end
+
+      context 'when renaming the file_name' do
+        let(:file_name) { 'new_file_name' }
+
+        it_behaves_like 'content is not updated'
+      end
+
+      context 'when file_name does not change' do
+        let(:file_name) { snippet.blobs.first.path }
+
+        it_behaves_like 'content is not updated'
+      end
+    end
+
+    shared_examples 'only content is present' do
+      let(:content) { 'new_content' }
+      let(:base_opts) do
+        {
+          content: content
+        }
+      end
+
+      it 'updates the content' do
+        response = subject
+        snippet = response.payload[:snippet]
+
+        blob = snippet.repository.blob_at('master', snippet.blobs.first.path)
+
+        expect(blob).not_to be_nil
+        expect(response).to be_success
+        expect(blob.data).to eq content
+      end
+    end
+
     context 'when Project Snippet' do
       let_it_be(:project) { create(:project) }
       let!(:snippet) { create(:project_snippet, :repository, author: user, project: project) }
@@ -316,6 +370,8 @@ describe Snippets::UpdateService do
       it_behaves_like 'updates repository content'
       it_behaves_like 'commit operation fails'
       it_behaves_like 'committable attributes'
+      it_behaves_like 'only file_name is present'
+      it_behaves_like 'only content is present'
       it_behaves_like 'snippets spam check is performed' do
         before do
           subject
@@ -340,6 +396,8 @@ describe Snippets::UpdateService do
       it_behaves_like 'updates repository content'
       it_behaves_like 'commit operation fails'
       it_behaves_like 'committable attributes'
+      it_behaves_like 'only file_name is present'
+      it_behaves_like 'only content is present'
       it_behaves_like 'snippets spam check is performed' do
         before do
           subject

spec/workers/incident_management/process_alert_worker_spec.rb

@@ -7,40 +7,39 @@ describe IncidentManagement::ProcessAlertWorker do
 
   describe '#perform' do
     let(:alert_management_alert_id) { nil }
-    let(:alert_payload) { { alert: 'payload' } }
-    let(:new_issue) { create(:issue, project: project) }
-    let(:create_issue_service) { instance_double(IncidentManagement::CreateIssueService, execute: new_issue) }
+    let(:alert_payload) do
+      {
+        'annotations' => { 'title' => 'title' },
+        'startsAt' => Time.now.rfc3339
+      }
+    end
+
+    let(:created_issue) { Issue.last }
 
     subject { described_class.new.perform(project.id, alert_payload, alert_management_alert_id) }
 
     before do
       allow(IncidentManagement::CreateIssueService)
         .to receive(:new).with(project, alert_payload)
-        .and_return(create_issue_service)
+        .and_call_original
     end
 
-    it 'calls create issue service' do
-      expect(Project).to receive(:find_by_id).and_call_original
-
+    it 'creates an issue' do
       expect(IncidentManagement::CreateIssueService)
         .to receive(:new).with(project, alert_payload)
-        .and_return(create_issue_service)
 
-      expect(create_issue_service).to receive(:execute)
-
-      subject
+      expect { subject }.to change { Issue.count }.by(1)
     end
 
     context 'with invalid project' do
-      let(:invalid_project_id) { 0 }
+      let(:invalid_project_id) { non_existing_record_id }
 
       subject { described_class.new.perform(invalid_project_id, alert_payload) }
 
       it 'does not create issues' do
-        expect(Project).to receive(:find_by_id).and_call_original
         expect(IncidentManagement::CreateIssueService).not_to receive(:new)
 
-        subject
+        expect { subject }.not_to change { Issue.count }
       end
     end
 
@@ -59,7 +58,9 @@ describe IncidentManagement::ProcessAlertWorker do
 
       context 'when alert can be updated' do
         it 'updates AlertManagement::Alert#issue_id' do
-          expect { subject }.to change { alert.reload.issue_id }.to(new_issue.id)
+          subject
+
+          expect(alert.reload.issue_id).to eq(created_issue.id)
         end
 
         it 'does not write a warning to log' do
@@ -80,12 +81,12 @@ describe IncidentManagement::ProcessAlertWorker do
           expect { subject }.not_to change { alert.reload.issue_id }
         end
 
-        it 'writes a worning to log' do
+        it 'logs a warning' do
           subject
 
           expect(Gitlab::AppLogger).to have_received(:warn).with(
             message: 'Cannot link an Issue with Alert',
-            issue_id: new_issue.id,
+            issue_id: created_issue.id,
             alert_id: alert_management_alert_id,
             alert_errors: { hosts: ['hosts array is over 255 chars'] }
           )