diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-04-14 15:09:31 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-04-14 15:09:31 +0300 |
commit | 9769ccf613ec45634ee32efaf1c39763a759a917 (patch) | |
tree | a48a3a73458978a2e9cfe0a0e1b4ace4c7c9da53 /spec | |
parent | 66f492cea7772633bfb6e0cb1352d90d10a67008 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
-rw-r--r-- | spec/components/diffs/overflow_warning_component_spec.rb | 184 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/pipeline/chain/limit/rate_limit_spec.rb | 179 | ||||
-rw-r--r-- | spec/services/ci/create_pipeline_service/rate_limit_spec.rb | 91 | ||||
-rw-r--r-- | spec/services/ci/create_pipeline_service_spec.rb | 4 | ||||
-rw-r--r-- | spec/services/members/create_service_spec.rb | 26 | ||||
-rw-r--r-- | spec/services/members/creator_service_spec.rb | 26 | ||||
-rw-r--r-- | spec/spec_helper.rb | 1 | ||||
-rw-r--r-- | spec/support/shared_examples/models/member_shared_examples.rb | 44 |
8 files changed, 551 insertions, 4 deletions
diff --git a/spec/components/diffs/overflow_warning_component_spec.rb b/spec/components/diffs/overflow_warning_component_spec.rb new file mode 100644 index 00000000000..ee4014ee492 --- /dev/null +++ b/spec/components/diffs/overflow_warning_component_spec.rb @@ -0,0 +1,184 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Diffs::OverflowWarningComponent, type: :component do + include RepoHelpers + + subject(:component) do + described_class.new( + diffs: diffs, + diff_files: diff_files, + project: project, + commit: commit, + merge_request: merge_request + ) + end + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:repository) { project.repository } + let_it_be(:commit) { project.commit(sample_commit.id) } + let_it_be(:diffs) { commit.raw_diffs } + let_it_be(:diff) { diffs.first } + let_it_be(:diff_refs) { commit.diff_refs } + let_it_be(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: diff_refs, repository: repository) } + let_it_be(:diff_files) { [diff_file] } + let_it_be(:merge_request) { create(:merge_request, source_project: project) } + + let(:expected_button_classes) do + "btn gl-alert-action btn-default gl-button btn-default-secondary" + end + + describe "rendered component" do + subject { rendered_component } + + context "on a commit page" do + before do + with_controller_class Projects::CommitController do + render_inline component + end + end + + it { is_expected.to include(component.message) } + + it "links to the diff" do + expect(component.diff_link).to eq( + ActionController::Base.helpers.link_to( + _("Plain diff"), + project_commit_path(project, commit, format: :diff), + class: expected_button_classes + ) + ) + + is_expected.to include(component.diff_link) + end + + it "links to the patch" do + expect(component.patch_link).to eq( + ActionController::Base.helpers.link_to( + _("Email patch"), + project_commit_path(project, commit, format: :patch), + class: expected_button_classes + ) + ) + + is_expected.to include(component.patch_link) + end + end + + context "on a merge request page and the merge request is persisted" do + before do + with_controller_class Projects::MergeRequests::DiffsController do + render_inline component + end + end + + it { is_expected.to include(component.message) } + + it "links to the diff" do + expect(component.diff_link).to eq( + ActionController::Base.helpers.link_to( + _("Plain diff"), + merge_request_path(merge_request, format: :diff), + class: expected_button_classes + ) + ) + + is_expected.to include(component.diff_link) + end + + it "links to the patch" do + expect(component.patch_link).to eq( + ActionController::Base.helpers.link_to( + _("Email patch"), + merge_request_path(merge_request, format: :patch), + class: expected_button_classes + ) + ) + + is_expected.to include(component.patch_link) + end + end + + context "both conditions fail" do + before do + allow(component).to receive(:commit?).and_return(false) + allow(component).to receive(:merge_request?).and_return(false) + render_inline component + end + + it { is_expected.to include(component.message) } + it { is_expected.not_to include(expected_button_classes) } + it { is_expected.not_to include("Plain diff") } + it { is_expected.not_to include("Email patch") } + end + end + + describe "#message" do + subject { component.message } + + it { is_expected.to be_a(String) } + + it "is HTML-safe" do + expect(subject.html_safe?).to be_truthy + end + end + + describe "#diff_link" do + subject { component.diff_link } + + before do + allow(component).to receive(:link_to).and_return("foo") + render_inline component + end + + it "is a string when on a commit page" do + allow(component).to receive(:commit?).and_return(true) + + is_expected.to eq("foo") + end + + it "is a string when on a merge request page" do + allow(component).to receive(:commit?).and_return(false) + allow(component).to receive(:merge_request?).and_return(true) + + is_expected.to eq("foo") + end + + it "is nil in other situations" do + allow(component).to receive(:commit?).and_return(false) + allow(component).to receive(:merge_request?).and_return(false) + + is_expected.to be_nil + end + end + + describe "#patch_link" do + subject { component.patch_link } + + before do + allow(component).to receive(:link_to).and_return("foo") + render_inline component + end + + it "is a string when on a commit page" do + allow(component).to receive(:commit?).and_return(true) + + is_expected.to eq("foo") + end + + it "is a string when on a merge request page" do + allow(component).to receive(:commit?).and_return(false) + allow(component).to receive(:merge_request?).and_return(true) + + is_expected.to eq("foo") + end + + it "is nil in other situations" do + allow(component).to receive(:commit?).and_return(false) + allow(component).to receive(:merge_request?).and_return(false) + + is_expected.to be_nil + end + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/limit/rate_limit_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/limit/rate_limit_spec.rb new file mode 100644 index 00000000000..aa8aec2af4a --- /dev/null +++ b/spec/lib/gitlab/ci/pipeline/chain/limit/rate_limit_spec.rb @@ -0,0 +1,179 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Gitlab::Ci::Pipeline::Chain::Limit::RateLimit, :freeze_time, :clean_gitlab_redis_rate_limiting do + let_it_be(:user) { create(:user) } + let_it_be(:namespace) { create(:namespace) } + let_it_be(:project, reload: true) { create(:project, namespace: namespace) } + + let(:save_incompleted) { false } + let(:throttle_message) do + 'Too many pipelines created in the last minute. Try again later.' + end + + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, + current_user: user, + save_incompleted: save_incompleted + ) + end + + let(:pipeline) { build(:ci_pipeline, project: project, source: source) } + let(:source) { 'push' } + let(:step) { described_class.new(pipeline, command) } + + def perform(count: 2) + count.times { step.perform! } + end + + context 'when the limit is exceeded' do + before do + allow(Gitlab::ApplicationRateLimiter).to receive(:rate_limits) + .and_return(pipelines_create: { threshold: 1, interval: 1.minute }) + + stub_feature_flags(ci_throttle_pipelines_creation_dry_run: false) + end + + it 'does not persist the pipeline' do + perform + + expect(pipeline).not_to be_persisted + expect(pipeline.errors.added?(:base, throttle_message)).to be_truthy + end + + it 'breaks the chain' do + perform + + expect(step.break?).to be_truthy + end + + it 'creates a log entry' do + expect(Gitlab::AppJsonLogger).to receive(:info).with( + a_hash_including( + class: described_class.name, + project_id: project.id, + subscription_plan: project.actual_plan_name, + commit_sha: command.sha + ) + ) + + perform + end + + context 'with child pipelines' do + let(:source) { 'parent_pipeline' } + + it 'does not break the chain' do + perform + + expect(step.break?).to be_falsey + end + + it 'does not invalidate the pipeline' do + perform + + expect(pipeline.errors).to be_empty + end + + it 'does not log anything' do + expect(Gitlab::AppJsonLogger).not_to receive(:info) + + perform + end + end + + context 'when saving incompleted pipelines' do + let(:save_incompleted) { true } + + it 'does not persist the pipeline' do + perform + + expect(pipeline).not_to be_persisted + expect(pipeline.errors.added?(:base, throttle_message)).to be_truthy + end + + it 'breaks the chain' do + perform + + expect(step.break?).to be_truthy + end + end + + context 'when ci_throttle_pipelines_creation is disabled' do + before do + stub_feature_flags(ci_throttle_pipelines_creation: false) + end + + it 'does not break the chain' do + perform + + expect(step.break?).to be_falsey + end + + it 'does not invalidate the pipeline' do + perform + + expect(pipeline.errors).to be_empty + end + + it 'does not log anything' do + expect(Gitlab::AppJsonLogger).not_to receive(:info) + + perform + end + end + + context 'when ci_throttle_pipelines_creation_dry_run is enabled' do + before do + stub_feature_flags(ci_throttle_pipelines_creation_dry_run: true) + end + + it 'does not break the chain' do + perform + + expect(step.break?).to be_falsey + end + + it 'does not invalidate the pipeline' do + perform + + expect(pipeline.errors).to be_empty + end + + it 'creates a log entry' do + expect(Gitlab::AppJsonLogger).to receive(:info).with( + a_hash_including( + class: described_class.name, + project_id: project.id, + subscription_plan: project.actual_plan_name, + commit_sha: command.sha + ) + ) + + perform + end + end + end + + context 'when the limit is not exceeded' do + it 'does not break the chain' do + perform + + expect(step.break?).to be_falsey + end + + it 'does not invalidate the pipeline' do + perform + + expect(pipeline.errors).to be_empty + end + + it 'does not log anything' do + expect(Gitlab::AppJsonLogger).not_to receive(:info) + + perform + end + end +end diff --git a/spec/services/ci/create_pipeline_service/rate_limit_spec.rb b/spec/services/ci/create_pipeline_service/rate_limit_spec.rb new file mode 100644 index 00000000000..caea165cc6c --- /dev/null +++ b/spec/services/ci/create_pipeline_service/rate_limit_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe Ci::CreatePipelineService, :freeze_time, :clean_gitlab_redis_rate_limiting do + describe 'rate limiting' do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { project.first_owner } + + let(:ref) { 'refs/heads/master' } + + before do + stub_ci_pipeline_yaml_file(gitlab_ci_yaml) + stub_feature_flags(ci_throttle_pipelines_creation_dry_run: false) + + allow(Gitlab::ApplicationRateLimiter).to receive(:rate_limits) + .and_return(pipelines_create: { threshold: 1, interval: 1.minute }) + end + + context 'when user is under the limit' do + let(:pipeline) { create_pipelines(count: 1) } + + it 'allows pipeline creation' do + expect(pipeline).to be_created_successfully + expect(pipeline.statuses).not_to be_empty + end + end + + context 'when user is over the limit' do + let(:pipeline) { create_pipelines } + + it 'blocks pipeline creation' do + throttle_message = 'Too many pipelines created in the last minute. Try again later.' + + expect(pipeline).not_to be_persisted + expect(pipeline.statuses).to be_empty + expect(pipeline.errors.added?(:base, throttle_message)).to be_truthy + end + end + + context 'with different users' do + let(:other_user) { create(:user) } + + before do + project.add_maintainer(other_user) + end + + it 'allows other members to create pipelines' do + blocked_pipeline = create_pipelines(user: user) + allowed_pipeline = create_pipelines(count: 1, user: other_user) + + expect(blocked_pipeline).not_to be_persisted + expect(allowed_pipeline).to be_created_successfully + end + end + + context 'with different commits' do + it 'allows user to create pipeline' do + blocked_pipeline = create_pipelines(ref: ref) + allowed_pipeline = create_pipelines(count: 1, ref: 'refs/heads/feature') + + expect(blocked_pipeline).not_to be_persisted + expect(allowed_pipeline).to be_created_successfully + end + end + + context 'with different projects' do + let_it_be(:other_project) { create(:project, :repository) } + + before do + other_project.add_maintainer(user) + end + + it 'allows user to create pipeline' do + blocked_pipeline = create_pipelines(project: project) + allowed_pipeline = create_pipelines(count: 1, project: other_project) + + expect(blocked_pipeline).not_to be_persisted + expect(allowed_pipeline).to be_created_successfully + end + end + end + + def create_pipelines(attrs = {}) + attrs.reverse_merge!(user: user, ref: ref, project: project, count: 2) + + service = described_class.new(attrs[:project], attrs[:user], { ref: attrs[:ref] }) + + attrs[:count].pred.times { service.execute(:push) } + service.execute(:push).payload + end +end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 5a5b3b9d581..943d70ba142 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -12,6 +12,10 @@ RSpec.describe Ci::CreatePipelineService do before do stub_ci_pipeline_to_return_yaml_file + + # Disable rate limiting for pipeline creation + allow(Gitlab::ApplicationRateLimiter).to receive(:rate_limits) + .and_return(pipelines_create: { threshold: 0, interval: 1.minute }) end describe '#execute' do diff --git a/spec/services/members/create_service_spec.rb b/spec/services/members/create_service_spec.rb index 05975d4d875..25437be1e78 100644 --- a/spec/services/members/create_service_spec.rb +++ b/spec/services/members/create_service_spec.rb @@ -143,6 +143,32 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_ end end + context 'when adding a project_bot' do + let_it_be(:project_bot) { create(:user, :project_bot) } + + let(:user_ids) { project_bot.id } + + context 'when project_bot is already a member' do + before do + source.add_developer(project_bot) + end + + it 'does not update the member' do + expect(execute_service[:status]).to eq(:error) + expect(execute_service[:message]).to eq("#{project_bot.username}: not authorized to update member") + expect(OnboardingProgress.completed?(source.namespace, :user_added)).to be(false) + end + end + + context 'when project_bot is not already a member' do + it 'adds the member' do + expect(execute_service[:status]).to eq(:success) + expect(source.users).to include project_bot + expect(OnboardingProgress.completed?(source.namespace, :user_added)).to be(true) + end + end + end + context 'when tracking the invite source', :snowplow do context 'when invite_source is not passed' do let(:additional_params) { {} } diff --git a/spec/services/members/creator_service_spec.rb b/spec/services/members/creator_service_spec.rb new file mode 100644 index 00000000000..ff5bf705b6c --- /dev/null +++ b/spec/services/members/creator_service_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Members::CreatorService do + let_it_be(:source, reload: true) { create(:group, :public) } + let_it_be(:member_type) { GroupMember } + let_it_be(:user) { create(:user) } + let_it_be(:current_user) { create(:user) } + + describe '#execute' do + it 'raises error for new member on authorization check implementation' do + expect do + described_class.new(source, user, :maintainer, current_user: current_user).execute + end.to raise_error(NotImplementedError) + end + + it 'raises error for an existing member on authorization check implementation' do + source.add_developer(user) + + expect do + described_class.new(source, user, :maintainer, current_user: current_user).execute + end.to raise_error(NotImplementedError) + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 32745532d1d..c2825b63452 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -191,6 +191,7 @@ RSpec.configure do |config| config.include MigrationsHelpers, :migration config.include RedisHelpers config.include Rails.application.routes.url_helpers, type: :routing + config.include Rails.application.routes.url_helpers, type: :component config.include PolicyHelpers, type: :policy config.include ExpectRequestWithStatus, type: :request config.include IdempotentWorkerHelper, type: :worker diff --git a/spec/support/shared_examples/models/member_shared_examples.rb b/spec/support/shared_examples/models/member_shared_examples.rb index 98d57e0dd8e..a329a6dca91 100644 --- a/spec/support/shared_examples/models/member_shared_examples.rb +++ b/spec/support/shared_examples/models/member_shared_examples.rb @@ -88,19 +88,55 @@ RSpec.shared_examples_for "member creation" do expect(member).to be_persisted end - context 'when admin mode is enabled', :enable_admin_mode do + context 'when adding a project_bot' do + let_it_be(:project_bot) { create(:user, :project_bot) } + + before_all do + source.add_owner(user) + end + + context 'when project_bot is already a member' do + before do + source.add_developer(project_bot) + end + + it 'does not update the member' do + member = described_class.new(source, project_bot, :maintainer, current_user: user).execute + + expect(source.users.reload).to include(project_bot) + expect(member).to be_persisted + expect(member.access_level).to eq(Gitlab::Access::DEVELOPER) + expect(member.errors.full_messages).to include(/not authorized to update member/) + end + end + + context 'when project_bot is not already a member' do + it 'adds the member' do + member = described_class.new(source, project_bot, :maintainer, current_user: user).execute + + expect(source.users.reload).to include(project_bot) + expect(member).to be_persisted + end + end + end + + context 'when admin mode is enabled', :enable_admin_mode, :aggregate_failures do it 'sets members.created_by to the given admin current_user' do member = described_class.new(source, user, :maintainer, current_user: admin).execute + expect(member).to be_persisted + expect(source.users.reload).to include(user) expect(member.created_by).to eq(admin) end end context 'when admin mode is disabled' do - it 'rejects setting members.created_by to the given admin current_user' do + it 'rejects setting members.created_by to the given admin current_user', :aggregate_failures do member = described_class.new(source, user, :maintainer, current_user: admin).execute - expect(member.created_by).to be_nil + expect(member).not_to be_persisted + expect(source.users.reload).not_to include(user) + expect(member.errors.full_messages).to include(/not authorized to create member/) end end @@ -142,7 +178,7 @@ RSpec.shared_examples_for "member creation" do end context 'when called with an unknown user id' do - it 'adds the user as a member' do + it 'does not add the user as a member' do expect(source.users).not_to include(user) described_class.new(source, non_existing_record_id, :maintainer).execute |