diff options
Diffstat (limited to 'spec/services/projects')
16 files changed, 517 insertions, 273 deletions
diff --git a/spec/services/projects/alerting/notify_service_spec.rb b/spec/services/projects/alerting/notify_service_spec.rb index b88f0ef5149..2f8c2049f85 100644 --- a/spec/services/projects/alerting/notify_service_spec.rb +++ b/spec/services/projects/alerting/notify_service_spec.rb @@ -8,21 +8,21 @@ describe Projects::Alerting::NotifyService do before do # We use `let_it_be(:project)` so we make sure to clear caches project.clear_memoization(:licensed_feature_available) + allow(ProjectServiceWorker).to receive(:perform_async) end - shared_examples 'processes incident issues' do |amount| + shared_examples 'processes incident issues' do let(:create_incident_service) { spy } - let(:new_alert) { instance_double(AlertManagement::Alert, id: 503, persisted?: true) } - it 'processes issues' do - expect(AlertManagement::Alert) - .to receive(:create) - .and_return(new_alert) + before do + allow_any_instance_of(AlertManagement::Alert).to receive(:execute_services) + end + it 'processes issues' do expect(IncidentManagement::ProcessAlertWorker) .to receive(:perform_async) - .with(project.id, kind_of(Hash), new_alert.id) - .exactly(amount).times + .with(project.id, kind_of(Hash), kind_of(Integer)) + .once Sidekiq::Testing.inline! do expect(subject).to be_success @@ -73,6 +73,7 @@ describe Projects::Alerting::NotifyService do describe '#execute' do let(:token) { 'invalid-token' } let(:starts_at) { Time.current.change(usec: 0) } + let(:fingerprint) { 'testing' } let(:service) { described_class.new(project, nil, payload) } let(:payload_raw) do { @@ -82,7 +83,8 @@ describe Projects::Alerting::NotifyService do monitoring_tool: 'GitLab RSpec', service: 'GitLab Test Suite', description: 'Very detailed description', - hosts: ['1.1.1.1', '2.2.2.2'] + hosts: ['1.1.1.1', '2.2.2.2'], + fingerprint: fingerprint }.with_indifferent_access end let(:payload) { ActionController::Parameters.new(payload_raw).permit! } @@ -131,11 +133,37 @@ describe Projects::Alerting::NotifyService do description: payload_raw.fetch(:description), monitoring_tool: payload_raw.fetch(:monitoring_tool), service: payload_raw.fetch(:service), - fingerprint: nil, + fingerprint: Digest::SHA1.hexdigest(fingerprint), ended_at: nil ) end + it 'executes the alert service hooks' do + slack_service = create(:service, type: 'SlackService', project: project, alert_events: true, active: true) + subject + + expect(ProjectServiceWorker).to have_received(:perform_async).with(slack_service.id, an_instance_of(Hash)) + end + + context 'existing alert with same fingerprint' do + let(:fingerprint_sha) { Digest::SHA1.hexdigest(fingerprint) } + let!(:existing_alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint_sha) } + + it 'does not create AlertManagement::Alert' do + expect { subject }.not_to change(AlertManagement::Alert, :count) + end + + it 'increments the existing alert count' do + expect { subject }.to change { existing_alert.reload.events }.from(1).to(2) + end + + it 'does not executes the alert service hooks' do + subject + + expect(ProjectServiceWorker).not_to have_received(:perform_async) + end + end + context 'with a minimal payload' do let(:payload_raw) do { @@ -176,7 +204,7 @@ describe Projects::Alerting::NotifyService do context 'issue enabled' do let(:issue_enabled) { true } - it_behaves_like 'processes incident issues', 1 + it_behaves_like 'processes incident issues' context 'with an invalid payload' do before do @@ -188,6 +216,21 @@ describe Projects::Alerting::NotifyService do it_behaves_like 'does not process incident issues due to error', http_status: :bad_request it_behaves_like 'NotifyService does not create alert' end + + context 'when alert already exists' do + let(:fingerprint_sha) { Digest::SHA1.hexdigest(fingerprint) } + let!(:existing_alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint_sha) } + + context 'when existing alert does not have an associated issue' do + it_behaves_like 'processes incident issues' + end + + context 'when existing alert has an associated issue' do + let!(:existing_alert) { create(:alert_management_alert, :with_issue, project: project, fingerprint: fingerprint_sha) } + + it_behaves_like 'does not process incident issues' + end + end end context 'with emails turned on' do diff --git a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb index 01f09f208fd..11ea7d51673 100644 --- a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb +++ b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' describe Projects::ContainerRepository::CleanupTagsService do let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, :private) } + let_it_be(:project, reload: true) { create(:project, :private) } let_it_be(:repository) { create(:container_repository, :root, project: project) } let(:service) { described_class.new(project, user, params) } @@ -72,6 +72,47 @@ describe Projects::ContainerRepository::CleanupTagsService do end end + context 'with invalid regular expressions' do + RSpec.shared_examples 'handling an invalid regex' do + it 'keeps all tags' do + expect(Projects::ContainerRepository::DeleteTagsService) + .not_to receive(:new) + subject + end + + it 'returns an error' do + response = subject + + expect(response[:status]).to eq(:error) + expect(response[:message]).to eq('invalid regex') + end + + it 'calls error tracking service' do + expect(Gitlab::ErrorTracking).to receive(:log_exception).and_call_original + + subject + end + end + + context 'when name_regex_delete is invalid' do + let(:params) { { 'name_regex_delete' => '*test*' } } + + it_behaves_like 'handling an invalid regex' + end + + context 'when name_regex is invalid' do + let(:params) { { 'name_regex' => '*test*' } } + + it_behaves_like 'handling an invalid regex' + end + + context 'when name_regex_keep is invalid' do + let(:params) { { 'name_regex_keep' => '*test*' } } + + it_behaves_like 'handling an invalid regex' + end + end + context 'when delete regex matching specific tags is used' do let(:params) do { 'name_regex_delete' => 'C|D' } diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index e542f1e9108..e70ee05ed31 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -43,10 +43,10 @@ describe Projects::CreateService, '#execute' do create_project(user, opts) end - it 'creates associated project settings' do + it 'builds associated project settings' do project = create_project(user, opts) - expect(project.project_setting).to be_persisted + expect(project.project_setting).to be_new_record end end @@ -88,6 +88,116 @@ describe Projects::CreateService, '#execute' do end end + context 'group sharing', :sidekiq_inline do + let_it_be(:group) { create(:group) } + let_it_be(:shared_group) { create(:group) } + let_it_be(:shared_group_user) { create(:user) } + let(:opts) do + { + name: 'GitLab', + namespace_id: shared_group.id + } + end + + before do + create(:group_group_link, shared_group: shared_group, shared_with_group: group) + + shared_group.add_maintainer(shared_group_user) + group.add_developer(user) + end + + it 'updates authorization' do + shared_group_project = create_project(shared_group_user, opts) + + expect( + Ability.allowed?(shared_group_user, :read_project, shared_group_project) + ).to be_truthy + expect( + Ability.allowed?(user, :read_project, shared_group_project) + ).to be_truthy + end + end + + context 'membership overrides', :sidekiq_inline do + let_it_be(:group) { create(:group, :private) } + let_it_be(:subgroup_for_projects) { create(:group, :private, parent: group) } + let_it_be(:subgroup_for_access) { create(:group, :private, parent: group) } + let_it_be(:group_maintainer) { create(:user) } + let(:group_access_level) { Gitlab::Access::REPORTER } + let(:subgroup_access_level) { Gitlab::Access::DEVELOPER } + let(:share_max_access_level) { Gitlab::Access::MAINTAINER } + let(:opts) do + { + name: 'GitLab', + namespace_id: subgroup_for_projects.id + } + end + + before do + group.add_maintainer(group_maintainer) + + create(:group_group_link, shared_group: subgroup_for_projects, + shared_with_group: subgroup_for_access, + group_access: share_max_access_level) + end + + context 'membership is higher from group hierarchy' do + let(:group_access_level) { Gitlab::Access::MAINTAINER } + + it 'updates authorization' do + create(:group_member, access_level: subgroup_access_level, group: subgroup_for_access, user: user) + create(:group_member, access_level: group_access_level, group: group, user: user) + + subgroup_project = create_project(group_maintainer, opts) + + project_authorization = ProjectAuthorization.where( + project_id: subgroup_project.id, + user_id: user.id, + access_level: group_access_level) + + expect(project_authorization).to exist + end + end + + context 'membership is higher from group share' do + let(:subgroup_access_level) { Gitlab::Access::MAINTAINER } + + context 'share max access level is not limiting' do + it 'updates authorization' do + create(:group_member, access_level: group_access_level, group: group, user: user) + create(:group_member, access_level: subgroup_access_level, group: subgroup_for_access, user: user) + + subgroup_project = create_project(group_maintainer, opts) + + project_authorization = ProjectAuthorization.where( + project_id: subgroup_project.id, + user_id: user.id, + access_level: subgroup_access_level) + + expect(project_authorization).to exist + end + end + + context 'share max access level is limiting' do + let(:share_max_access_level) { Gitlab::Access::DEVELOPER } + + it 'updates authorization' do + create(:group_member, access_level: group_access_level, group: group, user: user) + create(:group_member, access_level: subgroup_access_level, group: subgroup_for_access, user: user) + + subgroup_project = create_project(group_maintainer, opts) + + project_authorization = ProjectAuthorization.where( + project_id: subgroup_project.id, + user_id: user.id, + access_level: share_max_access_level) + + expect(project_authorization).to exist + end + end + end + end + context 'error handling' do it 'handles invalid options' do opts[:default_branch] = 'master' @@ -339,29 +449,39 @@ describe Projects::CreateService, '#execute' do end end - context 'when there is an active service template' do - before do - create(:prometheus_service, project: nil, template: true, active: true) - end + describe 'create service for the project' do + subject(:project) { create_project(user, opts) } - it 'creates a service from this template' do - project = create_project(user, opts) + context 'when there is an active instance-level and an active template integration' do + let!(:template_integration) { create(:prometheus_service, :template, api_url: 'https://prometheus.template.com/') } + let!(:instance_integration) { create(:prometheus_service, :instance, api_url: 'https://prometheus.instance.com/') } - expect(project.services.count).to eq 1 - expect(project.errors).to be_empty + it 'creates a service from the instance-level integration' do + expect(project.services.count).to eq(1) + expect(project.services.first.api_url).to eq(instance_integration.api_url) + expect(project.services.first.inherit_from_id).to eq(instance_integration.id) + end end - end - context 'when a bad service template is created' do - it 'sets service to be inactive' do - opts[:import_url] = 'http://www.gitlab.com/gitlab-org/gitlab-foss' - create(:service, type: 'DroneCiService', project: nil, template: true, active: true) + context 'when there is an active service template' do + let!(:template_integration) { create(:prometheus_service, :template, api_url: 'https://prometheus.template.com/') } - project = create_project(user, opts) - service = project.services.first + it 'creates a service from the template' do + expect(project.services.count).to eq(1) + expect(project.services.first.api_url).to eq(template_integration.api_url) + expect(project.services.first.inherit_from_id).to be_nil + end + end - expect(project).to be_persisted - expect(service.active).to be false + context 'when there is an invalid integration' do + before do + create(:service, :template, type: 'DroneCiService', active: true) + end + + it 'creates an inactive service' do + expect(project).to be_persisted + expect(project.services.first.active).to be false + end end end @@ -547,7 +667,9 @@ describe Projects::CreateService, '#execute' do ) expect(AuthorizedProjectUpdate::UserRefreshWithLowUrgencyWorker).to( receive(:bulk_perform_in) - .with(1.hour, array_including([user.id], [other_user.id])) + .with(1.hour, + array_including([user.id], [other_user.id]), + batch_delay: 30.seconds, batch_size: 100) .and_call_original ) diff --git a/spec/services/projects/group_links/create_service_spec.rb b/spec/services/projects/group_links/create_service_spec.rb index 92667184be8..22f7c8bdcb4 100644 --- a/spec/services/projects/group_links/create_service_spec.rb +++ b/spec/services/projects/group_links/create_service_spec.rb @@ -3,16 +3,17 @@ require 'spec_helper' describe Projects::GroupLinks::CreateService, '#execute' do - let(:user) { create :user } - let(:group) { create :group } - let(:project) { create :project } + let_it_be(:user) { create :user } + let_it_be(:group) { create :group } + let_it_be(:project) { create :project } let(:opts) do { link_group_access: '30', expires_at: nil } end - let(:subject) { described_class.new(project, user, opts) } + + subject { described_class.new(project, user, opts) } before do group.add_developer(user) @@ -22,6 +23,12 @@ describe Projects::GroupLinks::CreateService, '#execute' do expect { subject.execute(group) }.to change { project.project_group_links.count }.from(0).to(1) end + it 'updates authorization' do + expect { subject.execute(group) }.to( + change { Ability.allowed?(user, :read_project, project) } + .from(false).to(true)) + end + it 'returns false if group is blank' do expect { subject.execute(nil) }.not_to change { project.project_group_links.count } end diff --git a/spec/services/projects/group_links/destroy_service_spec.rb b/spec/services/projects/group_links/destroy_service_spec.rb index 0fd1fcfe1a5..0a8c9580e70 100644 --- a/spec/services/projects/group_links/destroy_service_spec.rb +++ b/spec/services/projects/group_links/destroy_service_spec.rb @@ -3,15 +3,25 @@ require 'spec_helper' describe Projects::GroupLinks::DestroyService, '#execute' do - let(:project) { create(:project, :private) } - let!(:group_link) { create(:project_group_link, project: project) } - let(:user) { create :user } - let(:subject) { described_class.new(project, user) } + let_it_be(:user) { create :user } + let_it_be(:project) { create(:project, :private) } + let_it_be(:group) { create(:group) } + let!(:group_link) { create(:project_group_link, project: project, group: group) } + + subject { described_class.new(project, user) } it 'removes group from project' do expect { subject.execute(group_link) }.to change { project.project_group_links.count }.from(1).to(0) end + it 'updates authorization' do + group.add_maintainer(user) + + expect { subject.execute(group_link) }.to( + change { Ability.allowed?(user, :read_project, project) } + .from(true).to(false)) + end + it 'returns false if group_link is blank' do expect { subject.execute(nil) }.not_to change { project.project_group_links.count } end diff --git a/spec/services/projects/group_links/update_service_spec.rb b/spec/services/projects/group_links/update_service_spec.rb new file mode 100644 index 00000000000..5be2ae1e0f7 --- /dev/null +++ b/spec/services/projects/group_links/update_service_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::GroupLinks::UpdateService, '#execute' do + let_it_be(:user) { create :user } + let_it_be(:group) { create :group } + let_it_be(:project) { create :project } + let!(:link) { create(:project_group_link, project: project, group: group) } + + let(:expiry_date) { 1.month.from_now.to_date } + let(:group_link_params) do + { group_access: Gitlab::Access::GUEST, + expires_at: expiry_date } + end + + subject { described_class.new(link).execute(group_link_params) } + + before do + group.add_developer(user) + end + + it 'updates existing link' do + expect(link.group_access).to eq(Gitlab::Access::DEVELOPER) + expect(link.expires_at).to be_nil + + subject + + link.reload + + expect(link.group_access).to eq(Gitlab::Access::GUEST) + expect(link.expires_at).to eq(expiry_date) + end + + it 'updates project permissions' do + expect { subject }.to change { user.can?(:create_release, project) }.from(true).to(false) + end + + it 'executes UserProjectAccessChangedService' do + expect_next_instance_of(UserProjectAccessChangedService) do |service| + expect(service).to receive(:execute) + end + + subject + end + + context 'with only param not requiring authorization refresh' do + let(:group_link_params) { { expires_at: Date.tomorrow } } + + it 'does not execute UserProjectAccessChangedService' do + expect(UserProjectAccessChangedService).not_to receive(:new) + + subject + end + end +end diff --git a/spec/services/projects/import_export/export_service_spec.rb b/spec/services/projects/import_export/export_service_spec.rb index 5f496cb1e56..19891341311 100644 --- a/spec/services/projects/import_export/export_service_spec.rb +++ b/spec/services/projects/import_export/export_service_spec.rb @@ -119,9 +119,7 @@ describe Projects::ImportExport::ExportService do end it 'notifies logger' do - allow(Rails.logger).to receive(:error) - - expect(Rails.logger).to receive(:error) + expect(service.instance_variable_get(:@logger)).to receive(:error) end end end @@ -149,7 +147,7 @@ describe Projects::ImportExport::ExportService do end it 'notifies logger' do - expect(Rails.logger).to receive(:error) + expect(service.instance_variable_get(:@logger)).to receive(:error) end it 'does not call the export strategy' do diff --git a/spec/services/projects/lsif_data_service_spec.rb b/spec/services/projects/lsif_data_service_spec.rb deleted file mode 100644 index 4866f848121..00000000000 --- a/spec/services/projects/lsif_data_service_spec.rb +++ /dev/null @@ -1,126 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Projects::LsifDataService do - let(:artifact) { create(:ci_job_artifact, :lsif) } - let(:project) { build_stubbed(:project) } - let(:path) { 'main.go' } - let(:commit_id) { Digest::SHA1.hexdigest(SecureRandom.hex) } - - let(:service) { described_class.new(artifact.file, project, commit_id) } - - describe '#execute' do - def highlighted_value(value) - [{ language: 'go', value: Gitlab::Highlight.highlight(nil, value, language: 'go') }] - end - - context 'fetched lsif file', :use_clean_rails_memory_store_caching do - it 'is cached' do - service.execute(path) - - cached_data = Rails.cache.fetch("project:#{project.id}:lsif:#{commit_id}") - - expect(cached_data.keys).to eq(%w[def_refs doc_ranges docs hover_refs ranges]) - end - end - - context 'for main.go' do - let(:path_prefix) { "/#{project.full_path}/-/blob/#{commit_id}" } - - it 'returns lsif ranges for the file' do - expect(service.execute(path)).to eq([ - { - end_char: 9, - end_line: 6, - start_char: 5, - start_line: 6, - definition_url: "#{path_prefix}/main.go#L7", - hover: highlighted_value('func main()') - }, - { - end_char: 36, - end_line: 3, - start_char: 1, - start_line: 3, - definition_url: "#{path_prefix}/main.go#L4", - hover: highlighted_value('package "github.com/user/hello/morestrings" ("github.com/user/hello/morestrings")') - }, - { - end_char: 12, - end_line: 7, - start_char: 1, - start_line: 7, - definition_url: "#{path_prefix}/main.go#L4", - hover: highlighted_value('package "github.com/user/hello/morestrings" ("github.com/user/hello/morestrings")') - }, - { - end_char: 20, - end_line: 7, - start_char: 13, - start_line: 7, - definition_url: "#{path_prefix}/morestrings/reverse.go#L11", - hover: highlighted_value('func Reverse(s string) string') + [{ value: "This method reverses a string \n\n" }] - }, - { - end_char: 12, - end_line: 8, - start_char: 1, - start_line: 8, - definition_url: "#{path_prefix}/main.go#L4", - hover: highlighted_value('package "github.com/user/hello/morestrings" ("github.com/user/hello/morestrings")') - }, - { - end_char: 18, - end_line: 8, - start_char: 13, - start_line: 8, - definition_url: "#{path_prefix}/morestrings/reverse.go#L5", - hover: highlighted_value('func Func2(i int) string') - } - ]) - end - end - - context 'for morestring/reverse.go' do - let(:path) { 'morestrings/reverse.go' } - - it 'returns lsif ranges for the file' do - expect(service.execute(path).first).to eq({ - end_char: 2, - end_line: 11, - start_char: 1, - start_line: 11, - definition_url: "/#{project.full_path}/-/blob/#{commit_id}/morestrings/reverse.go#L12", - hover: highlighted_value('var a string') - }) - end - end - - context 'for an unknown file' do - let(:path) { 'unknown.go' } - - it 'returns nil' do - expect(service.execute(path)).to eq(nil) - end - end - end - - describe '#doc_id' do - context 'when the passed path matches multiple files' do - let(:path) { 'check/main.go' } - let(:docs) do - { - 1 => 'cmd/check/main.go', - 2 => 'cmd/command.go', - 3 => 'check/main.go', - 4 => 'cmd/nested/check/main.go' - } - end - - it 'fetches the document with the shortest absolute path' do - expect(service.__send__(:find_doc_id, docs, path)).to eq(3) - end - end - end -end diff --git a/spec/services/projects/operations/update_service_spec.rb b/spec/services/projects/operations/update_service_spec.rb index 99a9fdd4184..f4d62b48fe5 100644 --- a/spec/services/projects/operations/update_service_spec.rb +++ b/spec/services/projects/operations/update_service_spec.rb @@ -96,7 +96,8 @@ describe Projects::Operations::UpdateService do let(:params) do { metrics_setting_attributes: { - external_dashboard_url: 'http://gitlab.com' + external_dashboard_url: 'http://gitlab.com', + dashboard_timezone: 'utc' } } end @@ -108,6 +109,7 @@ describe Projects::Operations::UpdateService do expect(project.reload.metrics_setting.external_dashboard_url).to eq( 'http://gitlab.com' ) + expect(project.metrics_setting.dashboard_timezone).to eq('utc') end end @@ -122,22 +124,25 @@ describe Projects::Operations::UpdateService do expect(project.reload.metrics_setting.external_dashboard_url).to eq( 'http://gitlab.com' ) + expect(project.metrics_setting.dashboard_timezone).to eq('utc') end + end - context 'with blank external_dashboard_url in params' do - let(:params) do - { - metrics_setting_attributes: { - external_dashboard_url: '' - } + context 'with blank external_dashboard_url' do + let(:params) do + { + metrics_setting_attributes: { + external_dashboard_url: '', + dashboard_timezone: 'utc' } - end + } + end - it 'destroys the metrics_setting entry in DB' do - expect(result[:status]).to eq(:success) + it 'updates dashboard_timezone' do + expect(result[:status]).to eq(:success) - expect(project.reload.metrics_setting).to be_nil - end + expect(project.reload.metrics_setting.external_dashboard_url).to be(nil) + expect(project.metrics_setting.dashboard_timezone).to eq('utc') end end end diff --git a/spec/services/projects/prometheus/alerts/create_events_service_spec.rb b/spec/services/projects/prometheus/alerts/create_events_service_spec.rb index 35f23afd7a2..61236b5bbdb 100644 --- a/spec/services/projects/prometheus/alerts/create_events_service_spec.rb +++ b/spec/services/projects/prometheus/alerts/create_events_service_spec.rb @@ -89,9 +89,9 @@ describe Projects::Prometheus::Alerts::CreateEventsService do context 'with a resolved payload' do let(:started_at) { truncate_to_second(Time.current) } let(:ended_at) { started_at + 1 } - let(:payload_key) { PrometheusAlertEvent.payload_key_for(alert.prometheus_metric_id, utc_rfc3339(started_at)) } let(:resolved_event) { alert_payload(status: 'resolved', started_at: started_at, ended_at: ended_at) } let(:alerts_payload) { { 'alerts' => [resolved_event] } } + let(:payload_key) { Gitlab::Alerting::Alert.new(project: project, payload: resolved_event).gitlab_fingerprint } context 'with a matching firing event' do before do diff --git a/spec/services/projects/prometheus/alerts/notify_service_spec.rb b/spec/services/projects/prometheus/alerts/notify_service_spec.rb index 009543f9016..95acedb1e76 100644 --- a/spec/services/projects/prometheus/alerts/notify_service_spec.rb +++ b/spec/services/projects/prometheus/alerts/notify_service_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Projects::Prometheus::Alerts::NotifyService do + include PrometheusHelpers + let_it_be(:project, reload: true) { create(:project) } let(:service) { described_class.new(project, nil, payload) } @@ -92,9 +94,10 @@ describe Projects::Prometheus::Alerts::NotifyService do end context 'with valid payload' do - let(:alert_firing) { create(:prometheus_alert, project: project) } - let(:alert_resolved) { create(:prometheus_alert, project: project) } - let(:payload_raw) { payload_for(firing: [alert_firing], resolved: [alert_resolved]) } + let_it_be(:alert_firing) { create(:prometheus_alert, project: project) } + let_it_be(:alert_resolved) { create(:prometheus_alert, project: project) } + let_it_be(:cluster) { create(:cluster, :provided_by_user, projects: [project]) } + let(:payload_raw) { prometheus_alert_payload(firing: [alert_firing], resolved: [alert_resolved]) } let(:payload) { ActionController::Parameters.new(payload_raw).permit! } let(:payload_alert_firing) { payload_raw['alerts'].first } let(:token) { 'token' } @@ -116,9 +119,7 @@ describe Projects::Prometheus::Alerts::NotifyService do with_them do before do - cluster = create(:cluster, :provided_by_user, - projects: [project], - enabled: cluster_enabled) + cluster.update!(enabled: cluster_enabled) if status create(:clusters_applications_prometheus, status, @@ -179,6 +180,39 @@ describe Projects::Prometheus::Alerts::NotifyService do end end + context 'with generic alerts integration' do + using RSpec::Parameterized::TableSyntax + + where(:alerts_service, :token, :result) do + :active | :valid | :success + :active | :invalid | :failure + :active | nil | :failure + :inactive | :valid | :failure + nil | nil | :failure + end + + with_them do + let(:valid) { project.alerts_service.token } + let(:invalid) { 'invalid token' } + let(:token_input) { public_send(token) if token } + + before do + if alerts_service + create(:alerts_service, alerts_service, project: project) + end + end + + case result = params[:result] + when :success + it_behaves_like 'notifies alerts' + when :failure + it_behaves_like 'no notifications', http_status: :unauthorized + else + raise "invalid result: #{result.inspect}" + end + end + end + context 'alert emails' do before do create(:prometheus_service, project: project) @@ -227,7 +261,7 @@ describe Projects::Prometheus::Alerts::NotifyService do context 'with multiple firing alerts and resolving alerts' do let(:payload_raw) do - payload_for(firing: [alert_firing, alert_firing], resolved: [alert_resolved]) + prometheus_alert_payload(firing: [alert_firing, alert_firing], resolved: [alert_resolved]) end it 'processes Prometheus alerts' do @@ -258,7 +292,7 @@ describe Projects::Prometheus::Alerts::NotifyService do context 'multiple firing alerts' do let(:payload_raw) do - payload_for(firing: [alert_firing, alert_firing], resolved: []) + prometheus_alert_payload(firing: [alert_firing, alert_firing], resolved: []) end it_behaves_like 'processes incident issues', 2 @@ -266,7 +300,7 @@ describe Projects::Prometheus::Alerts::NotifyService do context 'without firing alerts' do let(:payload_raw) do - payload_for(firing: [], resolved: [alert_resolved]) + prometheus_alert_payload(firing: [], resolved: [alert_resolved]) end it_behaves_like 'processes incident issues', 1 @@ -284,24 +318,17 @@ describe Projects::Prometheus::Alerts::NotifyService do end context 'with invalid payload' do - context 'without version' do + context 'when payload is not processable' do let(:payload) { {} } - it_behaves_like 'no notifications', http_status: :unprocessable_entity - end - - context 'when version is not "4"' do - let(:payload) { { 'version' => '5' } } + before do + allow(described_class).to receive(:processable?).with(payload) + .and_return(false) + end it_behaves_like 'no notifications', http_status: :unprocessable_entity end - context 'with missing alerts' do - let(:payload) { { 'version' => '4' } } - - it_behaves_like 'no notifications', http_status: :unauthorized - end - context 'when the payload is too big' do let(:payload) { { 'the-payload-is-too-big' => true } } let(:deep_size_object) { instance_double(Gitlab::Utils::DeepSize, valid?: false) } @@ -328,50 +355,39 @@ describe Projects::Prometheus::Alerts::NotifyService do end end - private - - def payload_for(firing: [], resolved: []) - status = firing.any? ? 'firing' : 'resolved' - alerts = firing + resolved - alert_name = alerts.first.title - prometheus_metric_id = alerts.first.prometheus_metric_id.to_s - - alerts_map = \ - firing.map { |alert| map_alert_payload('firing', alert) } + - resolved.map { |alert| map_alert_payload('resolved', alert) } - - # See https://prometheus.io/docs/alerting/configuration/#%3Cwebhook_config%3E - { - 'version' => '4', - 'receiver' => 'gitlab', - 'status' => status, - 'alerts' => alerts_map, - 'groupLabels' => { - 'alertname' => alert_name - }, - 'commonLabels' => { - 'alertname' => alert_name, - 'gitlab' => 'hook', - 'gitlab_alert_id' => prometheus_metric_id - }, - 'commonAnnotations' => {}, - 'externalURL' => '', - 'groupKey' => "{}:{alertname=\'#{alert_name}\'}" - } - end + describe '.processable?' do + let(:valid_payload) { prometheus_alert_payload } + + subject { described_class.processable?(payload) } + + context 'with valid payload' do + let(:payload) { valid_payload } + + it { is_expected.to eq(true) } + + context 'containing unrelated keys' do + let(:payload) { valid_payload.merge('unrelated' => 'key') } - def map_alert_payload(status, alert) - { - 'status' => status, - 'labels' => { - 'alertname' => alert.title, - 'gitlab' => 'hook', - 'gitlab_alert_id' => alert.prometheus_metric_id.to_s - }, - 'annotations' => {}, - 'startsAt' => '2018-09-24T08:57:31.095725221Z', - 'endsAt' => '0001-01-01T00:00:00Z', - 'generatorURL' => 'http://prometheus-prometheus-server-URL' - } + it { is_expected.to eq(true) } + end + end + + context 'with invalid payload' do + where(:missing_key) do + described_class::REQUIRED_PAYLOAD_KEYS.to_a + end + + with_them do + let(:payload) { valid_payload.except(missing_key) } + + it { is_expected.to eq(false) } + end + end + + context 'with unsupported version' do + let(:payload) { valid_payload.merge('version' => '5') } + + it { is_expected.to eq(false) } + end end end diff --git a/spec/services/projects/propagate_service_template_spec.rb b/spec/services/projects/propagate_service_template_spec.rb index 7188ac5f733..ddc27c037f8 100644 --- a/spec/services/projects/propagate_service_template_spec.rb +++ b/spec/services/projects/propagate_service_template_spec.rb @@ -62,8 +62,8 @@ describe Projects::PropagateServiceTemplate do } ) - Service.build_from_template(project.id, service_template).save! - Service.build_from_template(project.id, other_service).save! + Service.build_from_integration(project.id, service_template).save! + Service.build_from_integration(project.id, other_service).save! expect { described_class.propagate(service_template) } .not_to change { Service.count } diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index f561a303be4..29c3c300d1b 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -158,6 +158,23 @@ describe Projects::UpdatePagesService do expect(project.pages_metadatum).not_to be_deployed end end + + context 'with background jobs running', :sidekiq_inline do + where(:ci_atomic_processing) do + [true, false] + end + + with_them do + before do + stub_feature_flags(ci_atomic_processing: ci_atomic_processing) + end + + it 'succeeds' do + expect(project.pages_deployed?).to be_falsey + expect(execute).to eq(:success) + end + end + end end end diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb index 38c2dc0780e..418973fb0a6 100644 --- a/spec/services/projects/update_remote_mirror_service_spec.rb +++ b/spec/services/projects/update_remote_mirror_service_spec.rb @@ -10,6 +10,10 @@ describe Projects::UpdateRemoteMirrorService do subject(:service) { described_class.new(project, project.creator) } + before do + stub_feature_flags(gitaly_ruby_remote_branches_ls_remote: false) + end + describe '#execute' do subject(:execute!) { service.execute(remote_mirror, 0) } @@ -102,6 +106,19 @@ describe Projects::UpdateRemoteMirrorService do expect(remote_mirror.last_error).to include("refs/heads/develop") end end + + # https://gitlab.com/gitlab-org/gitaly/-/issues/2670 + context 'when `gitaly_ruby_remote_branches_ls_remote` is enabled' do + before do + stub_feature_flags(gitaly_ruby_remote_branches_ls_remote: true) + end + + it 'does not perform a fetch' do + expect(project.repository).not_to receive(:fetch_remote) + + execute! + end + end end def stub_fetch_remote(project, remote_name:, ssh_auth:) diff --git a/spec/services/projects/update_repository_storage_service_spec.rb b/spec/services/projects/update_repository_storage_service_spec.rb index 28b79bc61d9..e37580e7367 100644 --- a/spec/services/projects/update_repository_storage_service_spec.rb +++ b/spec/services/projects/update_repository_storage_service_spec.rb @@ -16,7 +16,7 @@ describe Projects::UpdateRepositoryStorageService do end context 'without wiki and design repository' do - let(:project) { create(:project, :repository, repository_read_only: true, wiki_enabled: false) } + let(:project) { create(:project, :repository, wiki_enabled: false) } let(:destination) { 'test_second_storage' } let(:repository_storage_move) { create(:project_repository_storage_move, :scheduled, project: project, destination_storage_name: destination) } let!(:checksum) { project.repository.checksum } @@ -131,7 +131,7 @@ describe Projects::UpdateRepositoryStorageService do context 'with wiki repository' do include_examples 'moves repository to another storage', 'wiki' do - let(:project) { create(:project, :repository, repository_read_only: true, wiki_enabled: true) } + let(:project) { create(:project, :repository, wiki_enabled: true) } let(:repository) { project.wiki.repository } let(:destination) { 'test_second_storage' } let(:repository_storage_move) { create(:project_repository_storage_move, :scheduled, project: project, destination_storage_name: destination) } @@ -144,7 +144,7 @@ describe Projects::UpdateRepositoryStorageService do context 'with design repository' do include_examples 'moves repository to another storage', 'design' do - let(:project) { create(:project, :repository, repository_read_only: true) } + let(:project) { create(:project, :repository) } let(:repository) { project.design_repository } let(:destination) { 'test_second_storage' } let(:repository_storage_move) { create(:project_repository_storage_move, :scheduled, project: project, destination_storage_name: destination) } diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index ce9765a36ba..8a17884f641 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -254,7 +254,7 @@ describe Projects::UpdateService do it 'logs an error and creates a metric when wiki can not be created' do project.project_feature.update(wiki_access_level: ProjectFeature::DISABLED) - expect_any_instance_of(ProjectWiki).to receive(:wiki).and_raise(ProjectWiki::CouldNotCreateWikiError) + expect_any_instance_of(ProjectWiki).to receive(:wiki).and_raise(Wiki::CouldNotCreateWikiError) expect_any_instance_of(described_class).to receive(:log_error).with("Could not create wiki for #{project.full_name}") counter = double(:counter) @@ -552,6 +552,63 @@ describe Projects::UpdateService do end end end + + describe 'when changing repository_storage' do + let(:repository_read_only) { false } + let(:project) { create(:project, :repository, repository_read_only: repository_read_only) } + let(:opts) { { repository_storage: 'test_second_storage' } } + + before do + stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/extra_storage' }) + end + + shared_examples 'the transfer was not scheduled' do + it 'does not schedule the transfer' do + expect do + update_project(project, user, opts) + end.not_to change(project.repository_storage_moves, :count) + end + end + + context 'authenticated as admin' do + let(:user) { create(:admin) } + + it 'schedules the transfer of the repository to the new storage and locks the project' do + update_project(project, admin, opts) + + expect(project).to be_repository_read_only + expect(project.repository_storage_moves.last).to have_attributes( + state: ::ProjectRepositoryStorageMove.state_machines[:state].states[:scheduled].value, + source_storage_name: 'default', + destination_storage_name: 'test_second_storage' + ) + end + + context 'the repository is read-only' do + let(:repository_read_only) { true } + + it_behaves_like 'the transfer was not scheduled' + end + + context 'the storage has not changed' do + let(:opts) { { repository_storage: 'default' } } + + it_behaves_like 'the transfer was not scheduled' + end + + context 'the storage does not exist' do + let(:opts) { { repository_storage: 'nonexistent' } } + + it_behaves_like 'the transfer was not scheduled' + end + end + + context 'authenticated as user' do + let(:user) { create(:user) } + + it_behaves_like 'the transfer was not scheduled' + end + end end describe '#run_auto_devops_pipeline?' do @@ -611,25 +668,6 @@ describe Projects::UpdateService do end end - describe 'repository_storage' do - let(:admin) { create(:admin) } - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } - let(:opts) { { repository_storage: 'test_second_storage' } } - - it 'calls the change repository storage method if the storage changed' do - expect(project).to receive(:change_repository_storage).with('test_second_storage') - - update_project(project, admin, opts).inspect - end - - it "doesn't call the change repository storage for non-admin users" do - expect(project).not_to receive(:change_repository_storage) - - update_project(project, user, opts).inspect - end - end - def update_project(project, user, opts) described_class.new(project, user, opts).execute end |