Add latest changes from gitlab-org/gitlab@master

4 files changed, 178 insertions, 12 deletions

diff --git a/spec/services/ci/create_job_artifacts_service_spec.rb b/spec/services/ci/create_job_artifacts_service_spec.rb
index e1146fc3df6..03106687678 100644
--- a/spec/services/ci/create_job_artifacts_service_spec.rb
+++ b/spec/services/ci/create_job_artifacts_service_spec.rb
@@ -3,8 +3,9 @@
 require 'spec_helper'
 
 describe Ci::CreateJobArtifactsService do
-  let(:service) { described_class.new }
-  let(:job) { create(:ci_build) }
+  let_it_be(:project) { create(:project) }
+  let(:service) { described_class.new(project) }
+  let(:job) { create(:ci_build, project: project) }
   let(:artifacts_sha256) { '0' * 64 }
   let(:metadata_file) { nil }
 
@@ -64,7 +65,7 @@ describe Ci::CreateJobArtifactsService do
         it 'sets expiration date according to application settings' do
           expected_expire_at = 1.day.from_now
 
-          expect(subject).to be_truthy
+          expect(subject).to match(a_hash_including(status: :success))
           archive_artifact, metadata_artifact = job.job_artifacts.last(2)
 
           expect(job.artifacts_expire_at).to be_within(1.minute).of(expected_expire_at)
@@ -80,7 +81,7 @@ describe Ci::CreateJobArtifactsService do
           it 'sets expiration date according to the parameter' do
             expected_expire_at = 2.hours.from_now
 
-            expect(subject).to be_truthy
+            expect(subject).to match(a_hash_including(status: :success))
             archive_artifact, metadata_artifact = job.job_artifacts.last(2)
 
             expect(job.artifacts_expire_at).to be_within(1.minute).of(expected_expire_at)
@@ -101,21 +102,50 @@ describe Ci::CreateJobArtifactsService do
 
         it 'ignores the changes' do
           expect { subject }.not_to change { Ci::JobArtifact.count }
-          expect(subject).to be_truthy
+          expect(subject).to match(a_hash_including(status: :success))
         end
       end
 
       context 'when sha256 of uploading artifact is different than the existing one' do
         let(:existing_sha256) { '1' * 64 }
 
-        it 'returns false and logs the error' do
+        it 'returns error status' do
           expect(Gitlab::ErrorTracking).to receive(:track_exception).and_call_original
 
           expect { subject }.not_to change { Ci::JobArtifact.count }
-          expect(subject).to be_falsey
-          expect(job.errors[:base]).to contain_exactly('another artifact of the same type already exists')
+          expect(subject).to match(
+            a_hash_including(http_status: :bad_request,
+              message: 'another artifact of the same type already exists',
+              status: :error))
         end
       end
     end
+
+    shared_examples 'rescues object storage error' do |klass, message, expected_message|
+      it "handles #{klass}" do
+        allow_next_instance_of(JobArtifactUploader) do |uploader|
+          allow(uploader).to receive(:store!).and_raise(klass, message)
+        end
+
+        expect(Gitlab::ErrorTracking)
+          .to receive(:track_exception)
+          .and_call_original
+
+        expect(subject).to match(
+          a_hash_including(
+            http_status: :service_unavailable,
+            message: expected_message || message,
+            status: :error))
+      end
+    end
+
+    it_behaves_like 'rescues object storage error',
+      Errno::EIO, 'some/path', 'Input/output error - some/path'
+
+    it_behaves_like 'rescues object storage error',
+      Google::Apis::ServerError, 'Server error'
+
+    it_behaves_like 'rescues object storage error',
+      Signet::RemoteServerError, 'The service is currently unavailable'
   end
 end
diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb
index 8ca9ce86574..188271f4a75 100644
--- a/spec/services/ci/retry_build_service_spec.rb
+++ b/spec/services/ci/retry_build_service_spec.rb
@@ -36,7 +36,7 @@ describe Ci::RetryBuildService do
        job_artifacts_performance job_artifacts_lsif
        job_artifacts_codequality job_artifacts_metrics scheduled_at
        job_variables waiting_for_resource_at job_artifacts_metrics_referee
-       job_artifacts_network_referee].freeze
+       job_artifacts_network_referee needs].freeze
 
   IGNORE_ACCESSORS =
     %i[type lock_version target_url base_tags trace_sections
@@ -46,7 +46,8 @@ describe Ci::RetryBuildService do
        sourced_pipelines artifacts_file_store artifacts_metadata_store
        metadata runner_session trace_chunks upstream_pipeline_id
        artifacts_file artifacts_metadata artifacts_size commands
-       resource resource_group_id processed security_scans].freeze
+       resource resource_group_id processed security_scans author
+       pipeline_id].freeze
 
   shared_examples 'build duplication' do
     let(:another_pipeline) { create(:ci_empty_pipeline, project: project) }
@@ -79,8 +80,15 @@ describe Ci::RetryBuildService do
     end
 
     describe 'clone accessors' do
+      let(:forbidden_associations) do
+        Ci::Build.reflect_on_all_associations.each_with_object(Set.new) do |assoc, memo|
+          memo << assoc.name unless assoc.macro == :belongs_to
+        end
+      end
+
       CLONE_ACCESSORS.each do |attribute|
         it "clones #{attribute} build attribute" do
+          expect(attribute).not_to be_in(forbidden_associations), "association #{attribute} must be `belongs_to`"
           expect(build.send(attribute)).not_to be_nil
           expect(new_build.send(attribute)).not_to be_nil
           expect(new_build.send(attribute)).to eq build.send(attribute)
@@ -97,9 +105,17 @@ describe Ci::RetryBuildService do
           expect(new_build.protected).to eq build.protected
         end
       end
+
+      it 'clones only the needs attributes' do
+        expect(new_build.needs.exists?).to be_truthy
+        expect(build.needs.exists?).to be_truthy
+
+        expect(new_build.needs_attributes).to match(build.needs_attributes)
+        expect(new_build.needs).not_to match(build.needs)
+      end
     end
 
-    describe 'reject acessors' do
+    describe 'reject accessors' do
       REJECT_ACCESSORS.each do |attribute|
         it "does not clone #{attribute} build attribute" do
           expect(new_build.send(attribute)).not_to eq build.send(attribute)
@@ -117,8 +133,9 @@ describe Ci::RetryBuildService do
       #
       current_accessors =
         Ci::Build.attribute_names.map(&:to_sym) +
+        Ci::Build.attribute_aliases.keys.map(&:to_sym) +
         Ci::Build.reflect_on_all_associations.map(&:name) +
-        [:tag_list]
+        [:tag_list, :needs_attributes]
 
       current_accessors.uniq!
 
diff --git a/spec/services/error_tracking/issue_update_service_spec.rb b/spec/services/error_tracking/issue_update_service_spec.rb
index 78388328a22..ffb15901f80 100644
--- a/spec/services/error_tracking/issue_update_service_spec.rb
+++ b/spec/services/error_tracking/issue_update_service_spec.rb
@@ -43,6 +43,12 @@ describe ErrorTracking::IssueUpdateService do
           update_service.execute
         end
 
+        it 'clears the reactive cache' do
+          expect(error_tracking_setting).to receive(:expire_issues_cache)
+
+          result
+        end
+
         context 'related issue and resolving' do
           let(:issue) { create(:issue, project: project) }
           let(:sentry_issue) { create(:sentry_issue, issue: issue) }
diff --git a/spec/services/metrics/dashboard/update_dashboard_service_spec.rb b/spec/services/metrics/dashboard/update_dashboard_service_spec.rb
new file mode 100644
index 00000000000..2bb08579fb9
--- /dev/null
+++ b/spec/services/metrics/dashboard/update_dashboard_service_spec.rb
@@ -0,0 +1,113 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Metrics::Dashboard::UpdateDashboardService, :use_clean_rails_memory_store_caching do
+  include MetricsDashboardHelpers
+
+  set(:user) { create(:user) }
+  set(:project) { create(:project, :repository) }
+  set(:environment) { create(:environment, project: project) }
+
+  describe '#execute' do
+    subject(:service_call) { described_class.new(project, user, params).execute }
+
+    let(:commit_message) { 'test' }
+    let(:branch) { 'dashboard_new_branch' }
+    let(:dashboard) { 'config/prometheus/common_metrics.yml' }
+    let(:file_name) { 'custom_dashboard.yml' }
+    let(:file_content_hash) { YAML.safe_load(File.read(dashboard)) }
+    let(:params) do
+      {
+        file_name: file_name,
+        file_content: file_content_hash,
+        commit_message: commit_message,
+        branch: branch
+      }
+    end
+
+    context 'user does not have push right to repository' do
+      it_behaves_like 'misconfigured dashboard service response', :forbidden, "You can't commit to this project"
+    end
+
+    context 'with rights to push to the repository' do
+      before do
+        project.add_maintainer(user)
+      end
+
+      context 'path traversal attack attempt' do
+        context 'with a yml extension' do
+          let(:file_name) { 'config/prometheus/../database.yml' }
+
+          it_behaves_like 'misconfigured dashboard service response', :bad_request, "A file with this name doesn't exist"
+        end
+
+        context 'without a yml extension' do
+          let(:file_name) { '../../..../etc/passwd' }
+
+          it_behaves_like 'misconfigured dashboard service response', :bad_request, "The file name should have a .yml extension"
+        end
+      end
+
+      context 'valid parameters' do
+        it_behaves_like 'valid dashboard update process'
+      end
+
+      context 'selected branch already exists' do
+        let(:branch) { 'existing_branch' }
+
+        before do
+          project.repository.add_branch(user, branch, 'master')
+        end
+
+        it_behaves_like 'misconfigured dashboard service response', :bad_request, "There was an error updating the dashboard, branch named: existing_branch already exists."
+      end
+
+      context 'Files::UpdateService success' do
+        before do
+          allow(::Files::UpdateService).to receive(:new).and_return(double(execute: { status: :success }))
+        end
+
+        it 'returns success', :aggregate_failures do
+          dashboard_details = {
+            path: '.gitlab/dashboards/custom_dashboard.yml',
+            display_name: 'custom_dashboard.yml',
+            default: false,
+            system_dashboard: false
+          }
+
+          expect(service_call[:status]).to be :success
+          expect(service_call[:http_status]).to be :created
+          expect(service_call[:dashboard]).to match dashboard_details
+        end
+
+        context 'with escaped characters in file name' do
+          let(:file_name) { "custom_dashboard%26copy.yml" }
+
+          it 'escapes the special characters', :aggregate_failures do
+            dashboard_details = {
+              path: '.gitlab/dashboards/custom_dashboard&copy.yml',
+              display_name: 'custom_dashboard&copy.yml',
+              default: false,
+              system_dashboard: false
+            }
+
+            expect(service_call[:status]).to be :success
+            expect(service_call[:http_status]).to be :created
+            expect(service_call[:dashboard]).to match dashboard_details
+          end
+        end
+      end
+
+      context 'Files::UpdateService fails' do
+        before do
+          allow(::Files::UpdateService).to receive(:new).and_return(double(execute: { status: :error }))
+        end
+
+        it 'returns error' do
+          expect(service_call[:status]).to be :error
+        end
+      end
+    end
+  end
+end