From f93ec4cb3933e2fe25b90844a6671f2bf312c5a3 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 10 Apr 2023 15:16:36 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../rspec/shared_groups_metadata.yml | 1 - app/models/project.rb | 6 +++++- app/models/protected_branch.rb | 6 +++++- app/models/terraform/state.rb | 2 ++ app/models/terraform/state_version.rb | 2 +- .../protected_branches/cache_service.rb | 7 ++++++- .../terraform/remote_state_handler.rb | 6 +++++- .../allow_protected_branches_for_group.yml | 8 ++++++++ ...129_add_lock_version_to_terraform_state.rb | 9 +++++++++ ..._job_artifacts_on_expire_at_for_removal.rb | 19 +++++++++++++++++++ db/schema_migrations/20230329032129 | 1 + db/schema_migrations/20230404061832 | 1 + db/structure.sql | 5 ++--- .../filtered_search/dropdown_user_spec.js | 7 +++---- .../filtered_search/dropdown_utils_spec.js | 7 +++---- spec/models/project_spec.rb | 2 ++ spec/models/protected_branch_spec.rb | 4 ++++ spec/models/terraform/state_spec.rb | 2 +- spec/models/terraform/state_version_spec.rb | 4 ++-- spec/requests/api/terraform/state_spec.rb | 1 + .../api/terraform/state_version_spec.rb | 2 +- .../protect_default_branch_service_spec.rb | 2 ++ .../protected_branches/cache_service_spec.rb | 1 + 23 files changed, 84 insertions(+), 21 deletions(-) create mode 100644 config/feature_flags/development/allow_protected_branches_for_group.yml create mode 100644 db/migrate/20230329032129_add_lock_version_to_terraform_state.rb create mode 100644 db/post_migrate/20230404061832_drop_sync_index_ci_job_artifacts_on_expire_at_for_removal.rb create mode 100644 db/schema_migrations/20230329032129 create mode 100644 db/schema_migrations/20230404061832 diff --git a/.rubocop_todo/rspec/shared_groups_metadata.yml b/.rubocop_todo/rspec/shared_groups_metadata.yml index a9e4e6defec..b87c851d2f6 100644 --- a/.rubocop_todo/rspec/shared_groups_metadata.yml +++ b/.rubocop_todo/rspec/shared_groups_metadata.yml @@ -3,7 +3,6 @@ RSpec/SharedGroupsMetadata: Details: grace period Exclude: - 'ee/spec/requests/ee/admin/plan_limits_controller_spec.rb' - - 'ee/spec/support/shared_contexts/saas_registration_settings_context.rb' - 'spec/lib/gitlab/ci/config/entry/retry_spec.rb' - 'spec/lib/gitlab/git/merge_base_spec.rb' - 'spec/models/container_repository_spec.rb' diff --git a/app/models/project.rb b/app/models/project.rb index 3af5fc3db35..f50395a4acd 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2843,13 +2843,17 @@ class Project < ApplicationRecord end def all_protected_branches - if Feature.enabled?(:group_protected_branches, group) + if allow_protected_branches_for_group? @all_protected_branches ||= ProtectedBranch.from_union([protected_branches, group_protected_branches]) else protected_branches end end + def allow_protected_branches_for_group? + Feature.enabled?(:group_protected_branches, group) || Feature.enabled?(:allow_protected_branches_for_group, group) + end + def self_monitoring? Gitlab::CurrentSettings.self_monitoring_project_id == id end diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index 64690645d0c..01bdbba1955 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -45,7 +45,7 @@ class ProtectedBranch < ApplicationRecord end def self.allow_force_push?(project, ref_name) - if Feature.enabled?(:group_protected_branches, project.group) + if allow_protected_branches_for_group?(project.group) protected_branches = project.all_protected_branches.matching(ref_name) project_protected_branches, group_protected_branches = protected_branches.partition(&:project_id) @@ -60,6 +60,10 @@ class ProtectedBranch < ApplicationRecord end end + def self.allow_protected_branches_for_group?(group) + Feature.enabled?(:group_protected_branches, group) || Feature.enabled?(:allow_protected_branches_for_group, group) + end + def self.any_protected?(project, ref_names) protected_refs(project).any? do |protected_ref| ref_names.any? do |ref_name| diff --git a/app/models/terraform/state.rb b/app/models/terraform/state.rb index 8a207c891e2..93c128c989c 100644 --- a/app/models/terraform/state.rb +++ b/app/models/terraform/state.rb @@ -8,6 +8,8 @@ module Terraform HEX_REGEXP = %r{\A\h+\z}.freeze UUID_LENGTH = 32 + self.locking_column = :activerecord_lock_version + belongs_to :project belongs_to :locked_by_user, class_name: 'User' diff --git a/app/models/terraform/state_version.rb b/app/models/terraform/state_version.rb index d6a16ad5b99..6727c81f17b 100644 --- a/app/models/terraform/state_version.rb +++ b/app/models/terraform/state_version.rb @@ -5,7 +5,7 @@ module Terraform include EachBatch include FileStoreMounter - belongs_to :terraform_state, class_name: 'Terraform::State', optional: false + belongs_to :terraform_state, class_name: 'Terraform::State', optional: false, touch: true belongs_to :created_by_user, class_name: 'User', optional: true belongs_to :build, class_name: 'Ci::Build', optional: true, foreign_key: :ci_build_id diff --git a/app/services/protected_branches/cache_service.rb b/app/services/protected_branches/cache_service.rb index 2f0d3945014..cb2977796d7 100644 --- a/app/services/protected_branches/cache_service.rb +++ b/app/services/protected_branches/cache_service.rb @@ -74,13 +74,18 @@ module ProtectedBranches def redis_key group = project_or_group.is_a?(Group) ? project_or_group : project_or_group.group - @redis_key ||= if Feature.enabled?(:group_protected_branches, group) + @redis_key ||= if allow_protected_branches_for_group?(group) [CACHE_ROOT_KEY, project_or_group.class.name, project_or_group.id].join(':') else [CACHE_ROOT_KEY, project_or_group.id].join(':') end end + def allow_protected_branches_for_group?(group) + Feature.enabled?(:group_protected_branches, group) || + Feature.enabled?(:allow_protected_branches_for_group, group) + end + def metrics @metrics ||= Gitlab::Cache::Metrics.new(cache_metadata) end diff --git a/app/services/terraform/remote_state_handler.rb b/app/services/terraform/remote_state_handler.rb index d044cf30387..f72bf0390e4 100644 --- a/app/services/terraform/remote_state_handler.rb +++ b/app/services/terraform/remote_state_handler.rb @@ -2,6 +2,8 @@ module Terraform class RemoteStateHandler < BaseService + include Gitlab::OptimisticLocking + StateLockedError = Class.new(StandardError) StateDeletedError = Class.new(StandardError) UnauthorizedError = Class.new(StandardError) @@ -59,7 +61,9 @@ module Terraform private def retrieve_with_lock(find_only: false) - create_or_find!(find_only: find_only).tap { |state| state.with_lock { yield state } } + create_or_find!(find_only: find_only).tap do |state| + retry_lock(state, name: "Terraform state: #{state.id}") { yield state } + end end def create_or_find!(find_only:) diff --git a/config/feature_flags/development/allow_protected_branches_for_group.yml b/config/feature_flags/development/allow_protected_branches_for_group.yml new file mode 100644 index 00000000000..16b06182f7c --- /dev/null +++ b/config/feature_flags/development/allow_protected_branches_for_group.yml @@ -0,0 +1,8 @@ +--- +name: allow_protected_branches_for_group +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116779 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/383178 +milestone: '15.11' +type: development +group: group::compliance +default_enabled: false diff --git a/db/migrate/20230329032129_add_lock_version_to_terraform_state.rb b/db/migrate/20230329032129_add_lock_version_to_terraform_state.rb new file mode 100644 index 00000000000..78f0122c609 --- /dev/null +++ b/db/migrate/20230329032129_add_lock_version_to_terraform_state.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddLockVersionToTerraformState < Gitlab::Database::Migration[2.1] + enable_lock_retries! + + def change + add_column :terraform_states, :activerecord_lock_version, :integer, null: false, default: 0 + end +end diff --git a/db/post_migrate/20230404061832_drop_sync_index_ci_job_artifacts_on_expire_at_for_removal.rb b/db/post_migrate/20230404061832_drop_sync_index_ci_job_artifacts_on_expire_at_for_removal.rb new file mode 100644 index 00000000000..efa0792c457 --- /dev/null +++ b/db/post_migrate/20230404061832_drop_sync_index_ci_job_artifacts_on_expire_at_for_removal.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class DropSyncIndexCiJobArtifactsOnExpireAtForRemoval < Gitlab::Database::Migration[2.1] + INDEX_NAME = 'index_ci_job_artifacts_on_expire_at_for_removal' + CONDITIONS = 'locked = 0 AND expire_at IS NOT NULL' + + disable_ddl_transaction! + + def up + remove_concurrent_index_by_name :ci_job_artifacts, name: INDEX_NAME + end + + def down + add_concurrent_index :ci_job_artifacts, [:expire_at], where: CONDITIONS, name: INDEX_NAME + end +end diff --git a/db/schema_migrations/20230329032129 b/db/schema_migrations/20230329032129 new file mode 100644 index 00000000000..3aec19e9d99 --- /dev/null +++ b/db/schema_migrations/20230329032129 @@ -0,0 +1 @@ +89a256c209e4c402d3162c7c967b7515870e80801e637a372fb9ee670d8e535e \ No newline at end of file diff --git a/db/schema_migrations/20230404061832 b/db/schema_migrations/20230404061832 new file mode 100644 index 00000000000..14cdca25567 --- /dev/null +++ b/db/schema_migrations/20230404061832 @@ -0,0 +1 @@ +51e141580bfb02dbe9ab215bd9283c01598353cb3f59fbe52ab0ab26a7974b49 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index cb57029f218..a09543d9a57 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -22884,7 +22884,8 @@ CREATE TABLE terraform_states ( uuid character varying(32) NOT NULL, name character varying(255) NOT NULL, versioning_enabled boolean DEFAULT true NOT NULL, - deleted_at timestamp with time zone + deleted_at timestamp with time zone, + activerecord_lock_version integer DEFAULT 0 NOT NULL ); CREATE SEQUENCE terraform_states_id_seq @@ -29761,8 +29762,6 @@ CREATE INDEX index_ci_job_artifacts_id_for_terraform_reports ON ci_job_artifacts CREATE INDEX index_ci_job_artifacts_on_expire_at_and_job_id ON ci_job_artifacts USING btree (expire_at, job_id); -CREATE INDEX index_ci_job_artifacts_on_expire_at_for_removal ON ci_job_artifacts USING btree (expire_at) WHERE ((locked = 0) AND (expire_at IS NOT NULL)); - CREATE INDEX index_ci_job_artifacts_on_file_store ON ci_job_artifacts USING btree (file_store); CREATE INDEX index_ci_job_artifacts_on_file_type_for_devops_adoption ON ci_job_artifacts USING btree (file_type, project_id, created_at) WHERE (file_type = ANY (ARRAY[5, 6, 8, 23])); diff --git a/spec/frontend/filtered_search/dropdown_user_spec.js b/spec/frontend/filtered_search/dropdown_user_spec.js index 02ef813883f..8ddf8390431 100644 --- a/spec/frontend/filtered_search/dropdown_user_spec.js +++ b/spec/frontend/filtered_search/dropdown_user_spec.js @@ -1,4 +1,5 @@ -import { loadHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; +import htmlMergeRequestList from 'test_fixtures/merge_requests/merge_request_list.html'; +import { setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; import DropdownUser from '~/filtered_search/dropdown_user'; import DropdownUtils from '~/filtered_search/dropdown_utils'; import FilteredSearchTokenizer from '~/filtered_search/filtered_search_tokenizer'; @@ -71,13 +72,11 @@ describe('Dropdown User', () => { }); describe('hideCurrentUser', () => { - const fixtureTemplate = 'merge_requests/merge_request_list.html'; - let dropdown; let authorFilterDropdownElement; beforeEach(() => { - loadHTMLFixture(fixtureTemplate); + setHTMLFixture(htmlMergeRequestList); authorFilterDropdownElement = document.querySelector('#js-dropdown-author'); const dummyInput = document.createElement('div'); dropdown = new DropdownUser({ diff --git a/spec/frontend/filtered_search/dropdown_utils_spec.js b/spec/frontend/filtered_search/dropdown_utils_spec.js index 2030b45b44c..d8a5b493b7a 100644 --- a/spec/frontend/filtered_search/dropdown_utils_spec.js +++ b/spec/frontend/filtered_search/dropdown_utils_spec.js @@ -1,12 +1,11 @@ -import { loadHTMLFixture, setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; +import htmlMergeRequestList from 'test_fixtures/merge_requests/merge_request_list.html'; +import { setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; import FilteredSearchSpecHelper from 'helpers/filtered_search_spec_helper'; import DropdownUtils from '~/filtered_search/dropdown_utils'; import FilteredSearchDropdownManager from '~/filtered_search/filtered_search_dropdown_manager'; import IssuableFilteredSearchTokenKeys from '~/filtered_search/issuable_filtered_search_token_keys'; describe('Dropdown Utils', () => { - const issuableListFixture = 'merge_requests/merge_request_list.html'; - describe('getEscapedText', () => { it('should return same word when it has no space', () => { const escaped = DropdownUtils.getEscapedText('textWithoutSpace'); @@ -355,7 +354,7 @@ describe('Dropdown Utils', () => { let authorToken; beforeEach(() => { - loadHTMLFixture(issuableListFixture); + setHTMLFixture(htmlMergeRequestList); authorToken = FilteredSearchSpecHelper.createFilterVisualToken('author', '=', '@user'); const searchTermToken = FilteredSearchSpecHelper.createSearchVisualToken('search term'); diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9a201879635..00a94c80198 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -7460,6 +7460,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :projects do context 'when feature flag `group_protected_branches` enabled' do before do stub_feature_flags(group_protected_branches: true) + stub_feature_flags(allow_protected_branches_for_group: true) end it 'return all protected branches' do @@ -7470,6 +7471,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :projects do context 'when feature flag `group_protected_branches` disabled' do before do stub_feature_flags(group_protected_branches: false) + stub_feature_flags(allow_protected_branches_for_group: false) end it 'return only project-level protected branches' do diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index 181a0d8cd88..0a75250b68c 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -335,6 +335,7 @@ RSpec.describe ProtectedBranch, feature_category: :source_code_management do context "when feature flag disabled" do before do stub_feature_flags(group_protected_branches: false) + stub_feature_flags(allow_protected_branches_for_group: false) end let(:subject_branch) { create(:protected_branch, allow_force_push: allow_force_push, name: "foo") } @@ -374,6 +375,7 @@ RSpec.describe ProtectedBranch, feature_category: :source_code_management do with_them do before do stub_feature_flags(group_protected_branches: true) + stub_feature_flags(allow_protected_branches_for_group: true) unless group_level_value.nil? create(:protected_branch, allow_force_push: group_level_value, name: "foo", project: nil, group: group) @@ -427,6 +429,7 @@ RSpec.describe ProtectedBranch, feature_category: :source_code_management do context 'when feature flag enabled' do before do stub_feature_flags(group_protected_branches: true) + stub_feature_flags(allow_protected_branches_for_group: true) end it 'call `all_protected_branches`' do @@ -439,6 +442,7 @@ RSpec.describe ProtectedBranch, feature_category: :source_code_management do context 'when feature flag disabled' do before do stub_feature_flags(group_protected_branches: false) + stub_feature_flags(allow_protected_branches_for_group: false) end it 'call `protected_branches`' do diff --git a/spec/models/terraform/state_spec.rb b/spec/models/terraform/state_spec.rb index 533e6e4bd7b..fc0a6432149 100644 --- a/spec/models/terraform/state_spec.rb +++ b/spec/models/terraform/state_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Terraform::State do +RSpec.describe Terraform::State, feature_category: :infrastructure_as_code do subject { create(:terraform_state, :with_version) } it { is_expected.to belong_to(:project) } diff --git a/spec/models/terraform/state_version_spec.rb b/spec/models/terraform/state_version_spec.rb index 477041117cb..a476b9e79ae 100644 --- a/spec/models/terraform/state_version_spec.rb +++ b/spec/models/terraform/state_version_spec.rb @@ -2,11 +2,11 @@ require 'spec_helper' -RSpec.describe Terraform::StateVersion do +RSpec.describe Terraform::StateVersion, feature_category: :infrastructure_as_code do it { is_expected.to be_a FileStoreMounter } it { is_expected.to be_a EachBatch } - it { is_expected.to belong_to(:terraform_state).required } + it { is_expected.to belong_to(:terraform_state).required.touch } it { is_expected.to belong_to(:created_by_user).class_name('User').optional } it { is_expected.to belong_to(:build).class_name('Ci::Build').optional } diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb index c94643242c9..bce004dcd48 100644 --- a/spec/requests/api/terraform/state_spec.rb +++ b/spec/requests/api/terraform/state_spec.rb @@ -472,6 +472,7 @@ RSpec.describe API::Terraform::State, :snowplow, feature_category: :infrastructu request expect(response).to have_gitlab_http_status(:conflict) + expect(Gitlab::Json.parse(response.body)).to include('Who' => current_user.username) end end diff --git a/spec/requests/api/terraform/state_version_spec.rb b/spec/requests/api/terraform/state_version_spec.rb index 24b3ca94581..94fd2984435 100644 --- a/spec/requests/api/terraform/state_version_spec.rb +++ b/spec/requests/api/terraform/state_version_spec.rb @@ -10,7 +10,7 @@ RSpec.describe API::Terraform::StateVersion, feature_category: :infrastructure_a let_it_be(:maintainer) { create(:user, maintainer_projects: [project]) } let_it_be(:user_without_access) { create(:user) } - let_it_be(:state) { create(:terraform_state, project: project) } + let_it_be_with_reload(:state) { create(:terraform_state, project: project) } let!(:versions) { create_list(:terraform_state_version, 3, terraform_state: state) } diff --git a/spec/services/projects/protect_default_branch_service_spec.rb b/spec/services/projects/protect_default_branch_service_spec.rb index 21743e2a656..a4fdd9983b8 100644 --- a/spec/services/projects/protect_default_branch_service_spec.rb +++ b/spec/services/projects/protect_default_branch_service_spec.rb @@ -247,6 +247,7 @@ RSpec.describe Projects::ProtectDefaultBranchService, feature_category: :source_ context 'when feature flag `group_protected_branches` disabled' do before do stub_feature_flags(group_protected_branches: false) + stub_feature_flags(allow_protected_branches_for_group: false) end it 'return false' do @@ -257,6 +258,7 @@ RSpec.describe Projects::ProtectDefaultBranchService, feature_category: :source_ context 'when feature flag `group_protected_branches` enabled' do before do stub_feature_flags(group_protected_branches: true) + stub_feature_flags(allow_protected_branches_for_group: true) end it 'return true' do diff --git a/spec/services/protected_branches/cache_service_spec.rb b/spec/services/protected_branches/cache_service_spec.rb index 3aa3b56640b..0abf8a673f9 100644 --- a/spec/services/protected_branches/cache_service_spec.rb +++ b/spec/services/protected_branches/cache_service_spec.rb @@ -145,6 +145,7 @@ RSpec.describe ProtectedBranches::CacheService, :clean_gitlab_redis_cache, featu context 'when feature flag disabled' do before do stub_feature_flags(group_protected_branches: false) + stub_feature_flags(allow_protected_branches_for_group: false) end it_behaves_like 'execute with entity'