From d657831613232fb95226c076343cd320c6573886 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 15 Sep 2022 21:12:27 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../projects/environments_controller.rb | 19 +-- app/models/concerns/enums/ci/commit_status.rb | 3 +- app/presenters/commit_status_presenter.rb | 1 + .../ci/create_downstream_pipeline_service.rb | 13 ++ .../dashboard/milestones/index.html.haml | 6 +- .../settings/operations/show.html.haml | 13 -- ...l => ci_limit_complete_hierarchy_size.yml} | 8 +- ...change_namespace_id_not_null_in_members.rb | 13 ++ ...espace_foreign_key_on_delete_constraint.rb | 30 +++++ db/schema_migrations/20220914005141 | 1 + db/schema_migrations/20220914010233 | 1 + db/structure.sql | 6 + lib/gitlab/ci/status/build/failed.rb | 1 + locale/gitlab.pot | 9 -- .../projects/environments_controller_spec.rb | 125 ++++++------------ spec/factories/group_members.rb | 1 + spec/factories/project_members.rb | 1 + spec/features/dashboard/milestones_spec.rb | 7 +- spec/frontend/lib/utils/text_utility_spec.js | 35 ++--- spec/helpers/members_helper_spec.rb | 8 +- spec/models/member_spec.rb | 3 +- spec/models/members/group_member_spec.rb | 7 +- spec/models/members/project_member_spec.rb | 2 +- ...create_downstream_pipeline_service_spec.rb | 41 ++++++ .../members_notifications_shared_example.rb | 4 +- 25 files changed, 194 insertions(+), 164 deletions(-) rename config/feature_flags/development/{environments_search.yml => ci_limit_complete_hierarchy_size.yml} (66%) create mode 100644 db/migrate/20220914005141_change_namespace_id_not_null_in_members.rb create mode 100644 db/migrate/20220914010233_change_members_namespace_foreign_key_on_delete_constraint.rb create mode 100644 db/schema_migrations/20220914005141 create mode 100644 db/schema_migrations/20220914010233 diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 21e64affcf7..4f037cc843e 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -50,16 +50,8 @@ class Projects::EnvironmentsController < Projects::ApplicationController respond_to do |format| format.html format.json do - environments_count_by_state = {} - - if Feature.enabled?(:environments_search, project) - @environments = search_environments.with_state(params[:scope] || :available) - environments_count_by_state = search_environments.count_by_state - else - @environments = project.environments - .with_state(params[:scope] || :available) - environments_count_by_state = project.environments.count_by_state - end + @environments = search_environments.with_state(params[:scope] || :available) + environments_count_by_state = search_environments.count_by_state Gitlab::PollingInterval.set_header(response, interval: 3_000) render json: { @@ -80,12 +72,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController respond_to do |format| format.html format.json do - folder_environments = - if Feature.enabled?(:environments_search, project) - search_environments(type: params[:id]) - else - project.environments.where(environment_type: params[:id]) - end + folder_environments = search_environments(type: params[:id]) @environments = folder_environments.with_state(params[:scope] || :available) .order(:name) diff --git a/app/models/concerns/enums/ci/commit_status.rb b/app/models/concerns/enums/ci/commit_status.rb index a8b38096360..9de2da5aac3 100644 --- a/app/models/concerns/enums/ci/commit_status.rb +++ b/app/models/concerns/enums/ci/commit_status.rb @@ -40,7 +40,8 @@ module Enums downstream_pipeline_creation_failed: 1_007, secrets_provider_not_found: 1_008, reached_max_descendant_pipelines_depth: 1_009, - ip_restriction_failure: 1_010 + ip_restriction_failure: 1_010, + reached_max_pipeline_hierarchy_size: 1_011 } end end diff --git a/app/presenters/commit_status_presenter.rb b/app/presenters/commit_status_presenter.rb index 5361a6c2643..059d6d06be2 100644 --- a/app/presenters/commit_status_presenter.rb +++ b/app/presenters/commit_status_presenter.rb @@ -26,6 +26,7 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated downstream_pipeline_creation_failed: 'The downstream pipeline could not be created', secrets_provider_not_found: 'The secrets provider can not be found', reached_max_descendant_pipelines_depth: 'You reached the maximum depth of child pipelines', + reached_max_pipeline_hierarchy_size: 'The downstream pipeline tree is too large', project_deleted: 'The job belongs to a deleted project', user_blocked: 'The user who created this job is blocked', ci_quota_exceeded: 'No more CI minutes available', diff --git a/app/services/ci/create_downstream_pipeline_service.rb b/app/services/ci/create_downstream_pipeline_service.rb index c68d990ae2e..25cc9045052 100644 --- a/app/services/ci/create_downstream_pipeline_service.rb +++ b/app/services/ci/create_downstream_pipeline_service.rb @@ -11,6 +11,7 @@ module Ci DuplicateDownstreamPipelineError = Class.new(StandardError) MAX_NESTED_CHILDREN = 2 + MAX_HIERARCHY_SIZE = 1000 def execute(bridge) @bridge = bridge @@ -86,6 +87,11 @@ module Ci return false end + if Feature.enabled?(:ci_limit_complete_hierarchy_size) && pipeline_tree_too_large? + @bridge.drop!(:reached_max_pipeline_hierarchy_size) + return false + end + unless can_create_downstream_pipeline?(target_ref) @bridge.drop!(:insufficient_bridge_permissions) return false @@ -141,6 +147,13 @@ module Ci ancestors_of_new_child.count > MAX_NESTED_CHILDREN end + def pipeline_tree_too_large? + return false unless @bridge.triggers_downstream_pipeline? + + # Applies to the entire pipeline tree across all projects + @bridge.pipeline.complete_hierarchy_count >= MAX_HIERARCHY_SIZE + end + def config_checksum(pipeline) [pipeline.project_id, pipeline.ref, pipeline.source].hash end diff --git a/app/views/dashboard/milestones/index.html.haml b/app/views/dashboard/milestones/index.html.haml index 39fbd9bc097..bc8e3e6ab69 100644 --- a/app/views/dashboard/milestones/index.html.haml +++ b/app/views/dashboard/milestones/index.html.haml @@ -9,7 +9,7 @@ - if current_user .page-title-controls = render 'shared/new_project_item_select', - path: '-/milestones/new', label: 'New milestone', + path: '-/milestones/new', label: _('Milestone'), include_groups: true, type: :milestones - if @milestone_states.any? { |name, count| count > 0 } @@ -23,7 +23,7 @@ - if current_user .page-title-controls = render 'shared/new_project_item_select', - path: '-/milestones/new', label: 'New milestone', + path: '-/milestones/new', label: _('Milestone'), include_groups: true, type: :milestones - else .milestones @@ -36,5 +36,5 @@ - if current_user .page-title-controls = render 'shared/new_project_item_select', - path: '-/milestones/new', label: 'New milestone', + path: '-/milestones/new', label: _('Milestone'), include_groups: true, type: :milestones diff --git a/app/views/projects/settings/operations/show.html.haml b/app/views/projects/settings/operations/show.html.haml index 87e3e03099c..90e0ccce8b4 100644 --- a/app/views/projects/settings/operations/show.html.haml +++ b/app/views/projects/settings/operations/show.html.haml @@ -2,19 +2,6 @@ - page_title _('Monitor Settings') - breadcrumb_title _('Monitor Settings') -= render Pajamas::AlertComponent.new(variant: :danger, - dismissible: false, - title: s_('Deprecations|Feature deprecation and removal')) do |c| - = c.body do - - removal_epic_link_url = 'https://gitlab.com/groups/gitlab-org/-/epics/7188' - - removal_epic_link_start = ''.html_safe % { url: removal_epic_link_url } - - opstrace_link_url = 'https://gitlab.com/groups/gitlab-org/-/epics/6976' - - opstrace_link_start = ''.html_safe % { url: opstrace_link_url } - - link_end = ''.html_safe - = html_escape(s_('Deprecations|The metrics feature was deprecated in GitLab 14.7.')) - = html_escape(s_('Deprecations|The logs and tracing features were also deprecated in GitLab 14.7, and are %{removal_link_start} scheduled for removal %{link_end} in GitLab 15.0.')) % {removal_link_start: removal_epic_link_start, link_end: link_end } - = html_escape(s_('Deprecations|For information on a possible replacement, %{opstrace_link_start} learn more about Opstrace %{link_end}.')) % {opstrace_link_start: opstrace_link_start, link_end: link_end } - = render 'projects/settings/operations/metrics_dashboard' = render 'projects/settings/operations/error_tracking' = render 'projects/settings/operations/alert_management' diff --git a/config/feature_flags/development/environments_search.yml b/config/feature_flags/development/ci_limit_complete_hierarchy_size.yml similarity index 66% rename from config/feature_flags/development/environments_search.yml rename to config/feature_flags/development/ci_limit_complete_hierarchy_size.yml index 50b9946d21a..ad0dd85a25a 100644 --- a/config/feature_flags/development/environments_search.yml +++ b/config/feature_flags/development/ci_limit_complete_hierarchy_size.yml @@ -1,8 +1,8 @@ --- -name: environments_search -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/97015 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/372541 +name: ci_limit_complete_hierarchy_size +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/95857 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/373719 milestone: '15.4' type: development -group: group::release +group: group::pipeline execution default_enabled: false diff --git a/db/migrate/20220914005141_change_namespace_id_not_null_in_members.rb b/db/migrate/20220914005141_change_namespace_id_not_null_in_members.rb new file mode 100644 index 00000000000..250746b95b8 --- /dev/null +++ b/db/migrate/20220914005141_change_namespace_id_not_null_in_members.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class ChangeNamespaceIdNotNullInMembers < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + def up + add_not_null_constraint :members, :member_namespace_id, validate: false + end + + def down + remove_not_null_constraint :members, :member_namespace_id + end +end diff --git a/db/migrate/20220914010233_change_members_namespace_foreign_key_on_delete_constraint.rb b/db/migrate/20220914010233_change_members_namespace_foreign_key_on_delete_constraint.rb new file mode 100644 index 00000000000..2ee98d59c3e --- /dev/null +++ b/db/migrate/20220914010233_change_members_namespace_foreign_key_on_delete_constraint.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class ChangeMembersNamespaceForeignKeyOnDeleteConstraint < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + TARGET_COLUMN = :member_namespace_id + + def up + # add the new FK before removing the old one + add_concurrent_foreign_key( + :members, + :namespaces, + column: TARGET_COLUMN, + name: fk_name("#{TARGET_COLUMN}_new"), + on_delete: :cascade, + validate: false + ) + end + + def down + with_lock_retries do + remove_foreign_key_if_exists(:members, column: TARGET_COLUMN, name: fk_name("#{TARGET_COLUMN}_new")) + end + end + + def fk_name(column_name) + # generate a FK name + concurrent_foreign_key_name(:members, column_name) + end +end diff --git a/db/schema_migrations/20220914005141 b/db/schema_migrations/20220914005141 new file mode 100644 index 00000000000..88859155884 --- /dev/null +++ b/db/schema_migrations/20220914005141 @@ -0,0 +1 @@ +df7862d3bab250feb867ecf60134bbfdffdfd6ea4f3a5a9b2c7e546e0aa89e3f \ No newline at end of file diff --git a/db/schema_migrations/20220914010233 b/db/schema_migrations/20220914010233 new file mode 100644 index 00000000000..777c73c0be6 --- /dev/null +++ b/db/schema_migrations/20220914010233 @@ -0,0 +1 @@ +be86548616ce5b4e6f0caf6db79c49ac523766257d20c6f5465d21a0e53f46d0 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 3274c47b9c9..5389ee0ff84 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -24983,6 +24983,9 @@ ALTER TABLE ONLY chat_teams ALTER TABLE vulnerability_scanners ADD CONSTRAINT check_37608c9db5 CHECK ((char_length(vendor) <= 255)) NOT VALID; +ALTER TABLE members + ADD CONSTRAINT check_508774aac0 CHECK ((member_namespace_id IS NOT NULL)) NOT VALID; + ALTER TABLE sprints ADD CONSTRAINT check_ccd8a1eae0 CHECK ((start_date IS NOT NULL)) NOT VALID; @@ -32415,6 +32418,9 @@ ALTER TABLE ONLY lfs_objects_projects ALTER TABLE ONLY vulnerability_merge_request_links ADD CONSTRAINT fk_2ef3954596 FOREIGN KEY (vulnerability_id) REFERENCES vulnerabilities(id) ON DELETE CASCADE; +ALTER TABLE ONLY members + ADD CONSTRAINT fk_2f85abf8f1 FOREIGN KEY (member_namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE NOT VALID; + ALTER TABLE ONLY analytics_cycle_analytics_group_stages ADD CONSTRAINT fk_3078345d6d FOREIGN KEY (stage_event_hash_id) REFERENCES analytics_cycle_analytics_stage_event_hashes(id) ON DELETE CASCADE; diff --git a/lib/gitlab/ci/status/build/failed.rb b/lib/gitlab/ci/status/build/failed.rb index 5412965d5dc..a136044c124 100644 --- a/lib/gitlab/ci/status/build/failed.rb +++ b/lib/gitlab/ci/status/build/failed.rb @@ -31,6 +31,7 @@ module Gitlab downstream_pipeline_creation_failed: 'downstream pipeline can not be created', secrets_provider_not_found: 'secrets provider can not be found', reached_max_descendant_pipelines_depth: 'reached maximum depth of child pipelines', + reached_max_pipeline_hierarchy_size: 'downstream pipeline tree is too large', project_deleted: 'pipeline project was deleted', user_blocked: 'pipeline user was blocked', ci_quota_exceeded: 'no more CI minutes available', diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 60cc8306c6d..f432f69fed2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -13331,18 +13331,9 @@ msgstr "" msgid "Deprecated API rate limits" msgstr "" -msgid "Deprecations|Feature deprecation and removal" -msgstr "" - msgid "Deprecations|For information on a possible replacement %{epicStart} learn more about Opstrace %{epicEnd}." msgstr "" -msgid "Deprecations|For information on a possible replacement, %{opstrace_link_start} learn more about Opstrace %{link_end}." -msgstr "" - -msgid "Deprecations|The logs and tracing features were also deprecated in GitLab 14.7, and are %{removal_link_start} scheduled for removal %{link_end} in GitLab 15.0." -msgstr "" - msgid "Deprecations|The metrics feature was deprecated in GitLab 14.7." msgstr "" diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 5ed56609265..16a43bae674 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -18,8 +18,27 @@ RSpec.describe Projects::EnvironmentsController do sign_in(user) end - # TODO: inline when FF is removed https://gitlab.com/gitlab-org/gitlab/-/issues/372541 - shared_examples 'index examples' do + describe 'GET index' do + context 'when a request for the HTML is made' do + it 'responds with status code 200' do + get :index, params: environment_params + + expect(response).to have_gitlab_http_status(:ok) + end + + it 'expires etag cache to force reload environments list' do + expect_any_instance_of(Gitlab::EtagCaching::Store) + .to receive(:touch).with(project_environments_path(project, format: :json)) + + get :index, params: environment_params + end + + it_behaves_like 'tracking unique visits', :index do + let(:request_params) { environment_params } + let(:target_id) { 'users_visiting_environments_pages' } + end + end + context 'when requesting JSON response for folders' do before do allow_any_instance_of(Environment).to receive(:has_terminals?).and_return(true) @@ -57,16 +76,8 @@ RSpec.describe Projects::EnvironmentsController do it 'handles search option properly' do get :index, params: environment_params(format: :json, search: 'staging/r') - if Feature.enabled?(:environments_search, project) - expect(environments.map { |env| env['name'] } ).to contain_exactly('staging/review-1', 'staging/review-2') - expect(json_response['available_count']).to eq 2 - else - expect(environments.map { |env| env['name'] } ).to contain_exactly('production', - 'staging/review-1', - 'staging/review-2') - expect(json_response['available_count']).to eq 3 - end - + expect(environments.map { |env| env['name'] } ).to contain_exactly('staging/review-1', 'staging/review-2') + expect(json_response['available_count']).to eq 2 expect(json_response['stopped_count']).to eq 1 end @@ -160,40 +171,32 @@ RSpec.describe Projects::EnvironmentsController do end end - describe 'GET index' do - context 'when a request for the HTML is made' do - it 'responds with status code 200' do - get :index, params: environment_params + describe 'GET folder' do + context 'when using default format' do + it 'responds with HTML' do + get :folder, params: { + namespace_id: project.namespace, + project_id: project, + id: 'staging-1.0' + } expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template 'folder' end - it 'expires etag cache to force reload environments list' do - expect_any_instance_of(Gitlab::EtagCaching::Store) - .to receive(:touch).with(project_environments_path(project, format: :json)) + it_behaves_like 'tracking unique visits', :folder do + let(:request_params) do + { + namespace_id: project.namespace, + project_id: project, + id: 'staging-1.0' + } + end - get :index, params: environment_params - end - - it_behaves_like 'tracking unique visits', :index do - let(:request_params) { environment_params } let(:target_id) { 'users_visiting_environments_pages' } end end - include_examples 'index examples' - - context 'when environments_search feature flag is disabled' do - before do - stub_feature_flags(environments_search: false) - end - - include_examples 'index examples' - end - end - - # TODO: inline when FF is removed https://gitlab.com/gitlab-org/gitlab/-/issues/372541 - shared_examples 'folder examples' do context 'when using JSON format' do before do create(:environment, project: project, @@ -230,57 +233,13 @@ RSpec.describe Projects::EnvironmentsController do search: 'staging-1.0/z' }, format: :json) - if Feature.enabled?(:environments_search, project) - expect(environments.map { |env| env['name'] } ).to eq(['staging-1.0/zzz']) - expect(json_response['available_count']).to eq 1 - else - expect(environments.map { |env| env['name'] } ).to contain_exactly('staging-1.0/review', - 'staging-1.0/zzz') - expect(json_response['available_count']).to eq 2 - end - + expect(environments.map { |env| env['name'] } ).to eq(['staging-1.0/zzz']) + expect(json_response['available_count']).to eq 1 expect(json_response['stopped_count']).to eq 0 end end end - describe 'GET folder' do - context 'when using default format' do - it 'responds with HTML' do - get :folder, params: { - namespace_id: project.namespace, - project_id: project, - id: 'staging-1.0' - } - - expect(response).to be_ok - expect(response).to render_template 'folder' - end - - it_behaves_like 'tracking unique visits', :folder do - let(:request_params) do - { - namespace_id: project.namespace, - project_id: project, - id: 'staging-1.0' - } - end - - let(:target_id) { 'users_visiting_environments_pages' } - end - end - - include_examples 'folder examples' - - context 'when environments_search feature flag is disabled' do - before do - stub_feature_flags(environments_search: false) - end - - include_examples 'folder examples' - end - end - describe 'GET show' do context 'with valid id' do it 'responds with a status code 200' do diff --git a/spec/factories/group_members.rb b/spec/factories/group_members.rb index 4b1bf9a7d11..702db45554e 100644 --- a/spec/factories/group_members.rb +++ b/spec/factories/group_members.rb @@ -4,6 +4,7 @@ FactoryBot.define do factory :group_member do access_level { GroupMember::OWNER } source { association(:group) } + member_namespace_id { source.id } user trait(:guest) { access_level { GroupMember::GUEST } } diff --git a/spec/factories/project_members.rb b/spec/factories/project_members.rb index ab1e45acc15..57f228650a1 100644 --- a/spec/factories/project_members.rb +++ b/spec/factories/project_members.rb @@ -4,6 +4,7 @@ FactoryBot.define do factory :project_member do user source { association(:project) } + member_namespace_id { source.id } maintainer trait(:guest) { access_level { ProjectMember::GUEST } } diff --git a/spec/features/dashboard/milestones_spec.rb b/spec/features/dashboard/milestones_spec.rb index 3f89955b12b..08cb95979ac 100644 --- a/spec/features/dashboard/milestones_spec.rb +++ b/spec/features/dashboard/milestones_spec.rb @@ -41,7 +41,12 @@ RSpec.describe 'Dashboard > Milestones' do first('.select2-result-label').click end - find('.js-new-project-item-link').click + a_el = find('.js-new-project-item-link') + + expect(a_el).to have_content('New Milestone in ') + expect(a_el).to have_no_content('New New Milestone in ') + + a_el.click expect(page).to have_current_path(new_group_milestone_path(group), ignore_query: true) end diff --git a/spec/frontend/lib/utils/text_utility_spec.js b/spec/frontend/lib/utils/text_utility_spec.js index 8e31fc792c5..49a160c9f23 100644 --- a/spec/frontend/lib/utils/text_utility_spec.js +++ b/spec/frontend/lib/utils/text_utility_spec.js @@ -45,29 +45,18 @@ describe('text_utility', () => { }); describe('slugify', () => { - it('should remove accents and convert to lower case', () => { - expect(textUtils.slugify('João')).toEqual('jo-o'); - }); - it('should replaces whitespaces with hyphens and convert to lower case', () => { - expect(textUtils.slugify('My Input String')).toEqual('my-input-string'); - }); - it('should remove trailing whitespace and replace whitespaces within string with a hyphen', () => { - expect(textUtils.slugify(' a new project ')).toEqual('a-new-project'); - }); - it('should only remove non-allowed special characters', () => { - expect(textUtils.slugify('test!_pro-ject~')).toEqual('test-_pro-ject'); - }); - it('should squash multiple hypens', () => { - expect(textUtils.slugify('test!!!!_pro-ject~')).toEqual('test-_pro-ject'); - }); - it('should return empty string if only non-allowed characters', () => { - expect(textUtils.slugify('здрасти')).toEqual(''); - }); - it('should squash multiple separators', () => { - expect(textUtils.slugify('Test:-)')).toEqual('test'); - }); - it('should trim any separators from the beginning and end of the slug', () => { - expect(textUtils.slugify('-Test:-)-')).toEqual('test'); + it.each` + title | input | output + ${'should remove accents and convert to lower case'} | ${'João'} | ${'jo-o'} + ${'should replaces whitespaces with hyphens and convert to lower case'} | ${'My Input String'} | ${'my-input-string'} + ${'should remove trailing whitespace and replace whitespaces within string with a hyphen'} | ${' a new project '} | ${'a-new-project'} + ${'should only remove non-allowed special characters'} | ${'test!_pro-ject~'} | ${'test-_pro-ject'} + ${'should squash to multiple non-allowed special characters'} | ${'test!!!!_pro-ject~'} | ${'test-_pro-ject'} + ${'should return empty string if only non-allowed characters'} | ${'дружба'} | ${''} + ${'should squash multiple separators'} | ${'Test:-)'} | ${'test'} + ${'should trim any separators from the beginning and end of the slug'} | ${'-Test:-)-'} | ${'test'} + `('$title', ({ input, output }) => { + expect(textUtils.slugify(input)).toBe(output); }); }); diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb index 4a3a623ce77..005fce1730f 100644 --- a/spec/helpers/members_helper_spec.rb +++ b/spec/helpers/members_helper_spec.rb @@ -6,12 +6,12 @@ RSpec.describe MembersHelper do describe '#remove_member_message' do let(:requester) { create(:user) } let(:project) { create(:project, :public) } - let(:project_member) { build(:project_member, project: project) } - let(:project_member_invite) { build(:project_member, project: project).tap { |m| m.generate_invite_token! } } + let(:project_member) { create(:project_member, project: project) } + let(:project_member_invite) { create(:project_member, project: project).tap { |m| m.generate_invite_token! } } let(:project_member_request) { project.request_access(requester) } let(:group) { create(:group) } - let(:group_member) { build(:group_member, group: group) } - let(:group_member_invite) { build(:group_member, group: group).tap { |m| m.generate_invite_token! } } + let(:group_member) { create(:group_member, group: group) } + let(:group_member_invite) { create(:group_member, group: group).tap { |m| m.generate_invite_token! } } let(:group_member_request) { group.request_access(requester) } it { expect(remove_member_message(project_member)).to eq "Are you sure you want to remove #{project_member.user.name} from the #{project.full_name} project?" } diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index fbf510a419f..7b75a6ee1c2 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -889,7 +889,8 @@ RSpec.describe Member do end describe 'generate invite token on create' do - let!(:member) { build(:project_member, invite_email: "user@example.com") } + let(:project) { create(:project) } + let!(:member) { build(:project_member, invite_email: "user@example.com", project: project) } it 'sets the invite token' do expect { member.save! }.to change { member.invite_token }.to(kind_of(String)) diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index c6266f15340..363830d21dd 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -74,7 +74,8 @@ RSpec.describe GroupMember do describe '#update_two_factor_requirement' do it 'is called after creation and deletion' do user = build :user - group_member = build :group_member, user: user + group = create :group + group_member = build :group_member, user: user, group: group expect(user).to receive(:update_two_factor_requirement) @@ -151,8 +152,8 @@ RSpec.describe GroupMember do context 'when importing' do it 'does not refresh' do expect(UserProjectAccessChangedService).not_to receive(:new) - - member = build(:group_member) + group = create(:group) + member = build(:group_member, group: group) member.importing = true member.save! end diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 99fc5dc14df..ad6f3ca5428 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -201,7 +201,7 @@ RSpec.describe ProjectMember do it 'does not refresh' do expect(AuthorizedProjectUpdate::ProjectRecalculatePerUserWorker).not_to receive(:bulk_perform_and_wait) - member = build(:project_member) + member = build(:project_member, project: project) member.importing = true member.save! end diff --git a/spec/services/ci/create_downstream_pipeline_service_spec.rb b/spec/services/ci/create_downstream_pipeline_service_spec.rb index 10aa839214c..9c02c5218f1 100644 --- a/spec/services/ci/create_downstream_pipeline_service_spec.rb +++ b/spec/services/ci/create_downstream_pipeline_service_spec.rb @@ -833,5 +833,46 @@ RSpec.describe Ci::CreateDownstreamPipelineService, '#execute' do end end end + + context 'when the pipeline tree is too large' do + let_it_be(:parent) { create(:ci_pipeline) } + let_it_be(:child) { create(:ci_pipeline, child_of: parent) } + let_it_be(:sibling) { create(:ci_pipeline, child_of: parent) } + + before do + stub_const("#{described_class}::MAX_HIERARCHY_SIZE", 3) + end + + let(:bridge) do + create(:ci_bridge, status: :pending, user: user, options: trigger, pipeline: child) + end + + it 'does not create a new pipeline' do + expect { subject }.not_to change { Ci::Pipeline.count } + end + + it 'drops the trigger job with an explanatory reason' do + subject + + expect(bridge.reload).to be_failed + expect(bridge.failure_reason).to eq('reached_max_pipeline_hierarchy_size') + end + + context 'with :ci_limit_complete_hierarchy_size disabled' do + before do + stub_feature_flags(ci_limit_complete_hierarchy_size: false) + end + + it 'creates a new pipeline' do + expect { subject }.to change { Ci::Pipeline.count }.by(1) + end + + it 'marks the bridge job as successful' do + subject + + expect(bridge.reload).to be_success + end + end + end end end diff --git a/spec/support/shared_examples/models/members_notifications_shared_example.rb b/spec/support/shared_examples/models/members_notifications_shared_example.rb index 75eed0203a7..e74aab95e46 100644 --- a/spec/support/shared_examples/models/members_notifications_shared_example.rb +++ b/spec/support/shared_examples/models/members_notifications_shared_example.rb @@ -8,7 +8,7 @@ RSpec.shared_examples 'members notifications' do |entity_type| end describe "#after_create" do - let(:member) { build(:"#{entity_type}_member") } + let(:member) { build(:"#{entity_type}_member", "#{entity_type}": create(entity_type.to_s)) } it "sends email to user" do expect(notification_service).to receive(:"new_#{entity_type}_member").with(member) @@ -35,7 +35,7 @@ RSpec.shared_examples 'members notifications' do |entity_type| describe '#after_commit' do context 'on creation of a member requesting access' do - let(:member) { build(:"#{entity_type}_member", :access_request) } + let(:member) { build(:"#{entity_type}_member", :access_request, "#{entity_type}": create(entity_type.to_s)) } it "calls NotificationService.new_access_request" do expect(notification_service).to receive(:new_access_request).with(member)