diff --git a/CHANGELOG-EE.md b/CHANGELOG-EE.md index a1245419ea..93ea5184a7 100644 --- a/CHANGELOG-EE.md +++ b/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. diff --git a/CHANGELOG.md b/CHANGELOG.md index 53920c6ecc..a10f9b9284 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 1b8bd35d6e..6fe535b5ba 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -13.0.4 +13.0.6 diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index ffca1dc241..594580cb92 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -8.31.1 +8.31.2 diff --git a/VERSION b/VERSION index 1b8bd35d6e..6fe535b5ba 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -13.0.4 +13.0.6 diff --git a/app/models/snippet_repository.rb b/app/models/snippet_repository.rb index 2276851b7a..8151308125 100644 --- a/app/models/snippet_repository.rb +++ b/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 diff --git a/app/services/ci/stop_environments_service.rb b/app/services/ci/stop_environments_service.rb index 14ef744ada..b6c5b398cb 100644 --- a/app/services/ci/stop_environments_service.rb +++ b/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 diff --git a/app/services/snippets/update_service.rb b/app/services/snippets/update_service.rb index 2dc9266dbd..250120c1c1 100644 --- a/app/services/snippets/update_service.rb +++ b/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 diff --git a/app/workers/incident_management/process_alert_worker.rb b/app/workers/incident_management/process_alert_worker.rb index 2ce9fe359b..0af34fa35d 100644 --- a/app/workers/incident_management/process_alert_worker.rb +++ b/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) diff --git a/db/post_migrate/20200511083541_cleanup_projects_with_missing_namespace.rb b/db/post_migrate/20200511083541_cleanup_projects_with_missing_namespace.rb index 442acfc6d1..1ead10a4de 100644 --- a/db/post_migrate/20200511083541_cleanup_projects_with_missing_namespace.rb +++ b/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 diff --git a/doc/administration/operations/moving_repositories.md b/doc/administration/operations/moving_repositories.md index 11cd3fa7b0..960005fe25 100644 --- a/doc/administration/operations/moving_repositories.md +++ b/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. diff --git a/spec/models/snippet_repository_spec.rb b/spec/models/snippet_repository_spec.rb index 255f07ebfa..b86a6f82f0 100644 --- a/spec/models/snippet_repository_spec.rb +++ b/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 diff --git a/spec/services/ci/stop_environments_service_spec.rb b/spec/services/ci/stop_environments_service_spec.rb index 19a6bcc307..ebbe6c37b8 100644 --- a/spec/services/ci/stop_environments_service_spec.rb +++ b/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 diff --git a/spec/services/snippets/update_service_spec.rb b/spec/services/snippets/update_service_spec.rb index 38747ae907..6c3ae52bef 100644 --- a/spec/services/snippets/update_service_spec.rb +++ b/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 diff --git a/spec/workers/incident_management/process_alert_worker_spec.rb b/spec/workers/incident_management/process_alert_worker_spec.rb index 938e72aa0f..2d17a59c76 100644 --- a/spec/workers/incident_management/process_alert_worker_spec.rb +++ b/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'] } )