From 39da9ce95cbe2a10dbbdfcb2f6ddb7ae9e71a74e Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 24 May 2024 03:13:33 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .rubocop_todo/layout/argument_alignment.yml | 13 ------ app/models/commit_status.rb | 3 ++ app/models/namespace.rb | 1 + app/models/namespace/detail.rb | 6 +++ app/services/groups/create_service.rb | 41 +++++++++---------- db/docs/audit_events.yml | 1 + ...ci_builds_auto_canceled_by_partition_id.rb | 15 +++++++ db/schema_migrations/20240520070447 | 1 + db/structure.sql | 4 +- .../Jobs/Dependency-Scanning.gitlab-ci.yml | 24 ++--------- .../Dependency-Scanning.latest.gitlab-ci.yml | 24 ++--------- lib/gitlab/middleware/path_traversal_check.rb | 2 +- locale/gitlab.pot | 17 ++++++-- .../gitlab_migration_release_spec.rb | 6 ++- spec/factories/groups.rb | 2 + spec/helpers/groups_helper_spec.rb | 9 +++- spec/models/namespace/detail_spec.rb | 13 +++++- spec/models/namespace_spec.rb | 1 + ...ing_for_approval_status_shared_examples.rb | 20 +++++---- 19 files changed, 108 insertions(+), 95 deletions(-) create mode 100644 db/post_migrate/20240520070447_drop_default_value_for_p_ci_builds_auto_canceled_by_partition_id.rb create mode 100644 db/schema_migrations/20240520070447 diff --git a/.rubocop_todo/layout/argument_alignment.yml b/.rubocop_todo/layout/argument_alignment.yml index 20d56b693aa..2c07fc44c98 100644 --- a/.rubocop_todo/layout/argument_alignment.yml +++ b/.rubocop_todo/layout/argument_alignment.yml @@ -51,19 +51,6 @@ Layout/ArgumentAlignment: - 'ee/app/graphql/mutations/security_policy/assign_security_policy_project.rb' - 'ee/app/graphql/mutations/security_policy/commit_scan_execution_policy.rb' - 'ee/app/graphql/mutations/security_policy/create_security_policy_project.rb' - - 'ee/app/graphql/types/deployments/approval_summary_type.rb' - - 'ee/app/graphql/types/deployments/approval_type.rb' - - 'ee/app/graphql/types/epic_descendant_weight_sum_type.rb' - - 'ee/app/graphql/types/epic_tree/epic_tree_node_input_type.rb' - - 'ee/app/graphql/types/epic_type.rb' - - 'ee/app/graphql/types/epics/negated_epic_filter_input_type.rb' - - 'ee/app/graphql/types/epics/unioned_epic_filter_input_type.rb' - - 'ee/app/graphql/types/external_issue_type.rb' - - 'ee/app/graphql/types/geo/geo_node_type.rb' - - 'ee/app/graphql/types/gitlab_subscriptions/preview_billable_user_change_type.rb' - - 'ee/app/graphql/types/group_release_stats_type.rb' - - 'ee/app/graphql/types/group_stats_type.rb' - - 'ee/app/graphql/types/incident_management/escalation_policy_type.rb' - 'ee/app/graphql/types/path_lock_type.rb' - 'ee/app/graphql/types/permission_types/epic.rb' - 'ee/app/graphql/types/permission_types/requirement.rb' diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index bc79ffdffad..693a75ed5ae 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -8,6 +8,7 @@ class CommitStatus < Ci::ApplicationRecord include Presentable include BulkInsertableAssociations include TaggableQueries + include SafelyChangeColumnDefault include IgnorableColumns ignore_columns %i[ @@ -21,6 +22,8 @@ class CommitStatus < Ci::ApplicationRecord user_id_convert_to_bigint ], remove_with: '17.0', remove_after: '2024-04-22' + columns_changing_default :auto_canceled_by_partition_id + self.table_name = :p_ci_builds self.sequence_name = :ci_builds_id_seq self.primary_key = :id diff --git a/app/models/namespace.rb b/app/models/namespace.rb index ed99b1945c8..c5f2e909231 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -165,6 +165,7 @@ class Namespace < ApplicationRecord delegate :math_rendering_limits_enabled?, :lock_math_rendering_limits_enabled?, to: :namespace_settings + delegate :add_creator, to: :namespace_details before_create :sync_share_with_group_lock_with_parent before_update :sync_share_with_group_lock_with_parent, if: :parent_changed? diff --git a/app/models/namespace/detail.rb b/app/models/namespace/detail.rb index a732173dc95..d3a552d788d 100644 --- a/app/models/namespace/detail.rb +++ b/app/models/namespace/detail.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +# rubocop:disable Gitlab/BoundedContexts -- TODO refactor to use bounded context class Namespace::Detail < ApplicationRecord belongs_to :namespace, inverse_of: :namespace_details belongs_to :creator, class_name: "User", optional: true @@ -7,6 +8,11 @@ class Namespace::Detail < ApplicationRecord validates :description, length: { maximum: 255 } self.primary_key = :namespace_id + + def add_creator(user) + update_attribute(:creator, user) + end end +# rubocop:enable Gitlab/BoundedContexts Namespace::Detail.prepend_mod diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index 7abf6a29f56..6f3781140f0 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -2,9 +2,6 @@ module Groups class CreateService < Groups::BaseService - VisibilityError = Class.new(StandardError) - PermissionError = Class.new(StandardError) - def initialize(user, params = {}) @current_user = user @params = params.dup @@ -15,28 +12,30 @@ module Groups build_group after_build_hook - validate_visibility_level! - validate_user_permissions! + return error_response unless valid? @group.name ||= @group.path.dup create_chat_team create_group - if @group.persisted? - after_successful_creation_hook + return error_response unless @group.persisted? - ServiceResponse.success(payload: { group: @group }) - else - ServiceResponse.error(message: 'Group has errors', payload: { group: @group }) - end + after_successful_creation_hook - rescue VisibilityError, PermissionError - ServiceResponse.error(message: 'Group has errors', payload: { group: @group }) + ServiceResponse.success(payload: { group: @group }) end private + def valid? + valid_visibility_level? && valid_user_permissions? + end + + def error_response + ServiceResponse.error(message: 'Group has errors', payload: { group: @group }) + end + def create_chat_team return unless valid_to_create_chat_team? @@ -110,29 +109,29 @@ module Groups Gitlab.config.mattermost.enabled && @chat_team && @group.chat_team.nil? end - def validate_user_permissions! + def valid_user_permissions? if @group.subgroup? unless can?(current_user, :create_subgroup, @group.parent) @group.parent = nil @group.errors.add(:parent_id, s_('CreateGroup|You don’t have permission to create a subgroup in this group.')) - raise PermissionError + return false end else unless can?(current_user, :create_group) @group.errors.add(:base, s_('CreateGroup|You don’t have permission to create groups.')) - raise PermissionError + return false end end - return if organization_setting_valid? + return true if organization_setting_valid? # We are unsetting this here to match behavior of invalid parent_id above and protect against possible # committing to the database of a value that isn't allowed. @group.organization = nil - raise PermissionError + false end def can_create_group_in_organization? @@ -171,12 +170,12 @@ module Groups can_create_group_in_organization? && matches_parent_organization? end - def validate_visibility_level! - return if Gitlab::VisibilityLevel.allowed_for?(current_user, visibility_level) + def valid_visibility_level? + return true if Gitlab::VisibilityLevel.allowed_for?(current_user, visibility_level) deny_visibility_level(@group) - raise VisibilityError, 'Visibility level not allowed' + false end def set_visibility_level diff --git a/db/docs/audit_events.yml b/db/docs/audit_events.yml index ec707e41286..c97212d18e5 100644 --- a/db/docs/audit_events.yml +++ b/db/docs/audit_events.yml @@ -8,3 +8,4 @@ description: TODO introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/commit/cf6b622686eacffa46aba5c8ed6419dc877a6b58 milestone: '7.6' gitlab_schema: gitlab_main +sharding_key_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/454158 diff --git a/db/post_migrate/20240520070447_drop_default_value_for_p_ci_builds_auto_canceled_by_partition_id.rb b/db/post_migrate/20240520070447_drop_default_value_for_p_ci_builds_auto_canceled_by_partition_id.rb new file mode 100644 index 00000000000..c4c5131a986 --- /dev/null +++ b/db/post_migrate/20240520070447_drop_default_value_for_p_ci_builds_auto_canceled_by_partition_id.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class DropDefaultValueForPCiBuildsAutoCanceledByPartitionId < Gitlab::Database::Migration[2.2] + milestone '17.1' + + def up + change_column_default(:p_ci_builds, :auto_canceled_by_partition_id, nil) + change_column_null(:p_ci_builds, :auto_canceled_by_partition_id, true) + end + + def down + change_column_default(:p_ci_builds, :auto_canceled_by_partition_id, 100) + change_column_null(:p_ci_builds, :auto_canceled_by_partition_id, false) + end +end diff --git a/db/schema_migrations/20240520070447 b/db/schema_migrations/20240520070447 new file mode 100644 index 00000000000..ef272eeac2f --- /dev/null +++ b/db/schema_migrations/20240520070447 @@ -0,0 +1 @@ +0a8bf197499d311e2745cbdb85b23cf1c5ea7363fa11cde0d66debe92b337a6e \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 55c92f19284..86a6208be7d 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -1222,7 +1222,7 @@ CREATE TABLE p_ci_builds ( id bigint NOT NULL, stage_id bigint, partition_id bigint NOT NULL, - auto_canceled_by_partition_id bigint DEFAULT 100 NOT NULL, + auto_canceled_by_partition_id bigint, auto_canceled_by_id bigint, commit_id bigint, erased_by_id bigint, @@ -6412,7 +6412,7 @@ CREATE TABLE ci_builds ( id bigint NOT NULL, stage_id bigint, partition_id bigint NOT NULL, - auto_canceled_by_partition_id bigint DEFAULT 100 NOT NULL, + auto_canceled_by_partition_id bigint, auto_canceled_by_id bigint, commit_id bigint, erased_by_id bigint, diff --git a/lib/gitlab/ci/templates/Jobs/Dependency-Scanning.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Dependency-Scanning.gitlab-ci.yml index 4e3a8e396ad..c40e806eafe 100644 --- a/lib/gitlab/ci/templates/Jobs/Dependency-Scanning.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Dependency-Scanning.gitlab-ci.yml @@ -58,16 +58,7 @@ dependency_scanning: .gemnasium-shared-rule: exists: - - '**/Gemfile.lock' - - '**/composer.lock' - - '**/gems.locked' - - '**/go.sum' - - '**/npm-shrinkwrap.json' - - '**/package-lock.json' - - '**/yarn.lock' - - '**/pnpm-lock.yaml' - - '**/packages.lock.json' - - '**/conan.lock' + - '**/{Gemfile.lock,composer.lock,gems.locked,go.sum,npm-shrinkwrap.json,package-lock.json,yarn.lock,pnpm-lock.yaml,packages.lock.json,conan.lock}' gemnasium-dependency_scanning: extends: @@ -94,10 +85,7 @@ gemnasium-dependency_scanning: .gemnasium-maven-shared-rule: exists: - - '**/build.gradle' - - '**/build.gradle.kts' - - '**/build.sbt' - - '**/pom.xml' + - '**/{build.gradle,build.gradle.kts,build.sbt,pom.xml}' gemnasium-maven-dependency_scanning: extends: @@ -122,13 +110,7 @@ gemnasium-maven-dependency_scanning: .gemnasium-python-shared-rule: exists: - - '**/requirements.txt' - - '**/requirements.pip' - - '**/Pipfile' - - '**/Pipfile.lock' - - '**/requires.txt' - - '**/setup.py' - - '**/poetry.lock' + - '**/{requirements.txt,requirements.pip,Pipfile,Pipfile.lock,requires.txt,setup.py,poetry.lock}' gemnasium-python-dependency_scanning: extends: diff --git a/lib/gitlab/ci/templates/Jobs/Dependency-Scanning.latest.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Dependency-Scanning.latest.gitlab-ci.yml index 38d22698792..c5131d5f649 100644 --- a/lib/gitlab/ci/templates/Jobs/Dependency-Scanning.latest.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Dependency-Scanning.latest.gitlab-ci.yml @@ -58,16 +58,7 @@ dependency_scanning: .gemnasium-shared-rule: exists: - - '**/Gemfile.lock' - - '**/composer.lock' - - '**/gems.locked' - - '**/go.sum' - - '**/npm-shrinkwrap.json' - - '**/package-lock.json' - - '**/yarn.lock' - - '**/pnpm-lock.yaml' - - '**/packages.lock.json' - - '**/conan.lock' + - '**/{Gemfile.lock,composer.lock,gems.locked,go.sum,npm-shrinkwrap.json,package-lock.json,yarn.lock,pnpm-lock.yaml,packages.lock.json,conan.lock}' gemnasium-dependency_scanning: extends: @@ -112,10 +103,7 @@ gemnasium-dependency_scanning: .gemnasium-maven-shared-rule: exists: - - '**/build.gradle' - - '**/build.gradle.kts' - - '**/build.sbt' - - '**/pom.xml' + - '**/{build.gradle,build.gradle.kts,build.sbt,pom.xml}' gemnasium-maven-dependency_scanning: extends: @@ -158,13 +146,7 @@ gemnasium-maven-dependency_scanning: .gemnasium-python-shared-rule: exists: - - '**/requirements.txt' - - '**/requirements.pip' - - '**/Pipfile' - - '**/Pipfile.lock' - - '**/requires.txt' - - '**/setup.py' - - '**/poetry.lock' + - '**/{requirements.txt,requirements.pip,Pipfile,Pipfile.lock,requires.txt,setup.py,poetry.lock}' gemnasium-python-dependency_scanning: extends: diff --git a/lib/gitlab/middleware/path_traversal_check.rb b/lib/gitlab/middleware/path_traversal_check.rb index ec17b344777..dbe3db2750d 100644 --- a/lib/gitlab/middleware/path_traversal_check.rb +++ b/lib/gitlab/middleware/path_traversal_check.rb @@ -4,7 +4,7 @@ module Gitlab module Middleware class PathTraversalCheck PATH_TRAVERSAL_MESSAGE = 'Potential path traversal attempt detected' - EXCLUDED_QUERY_PARAM_NAMES = %w[search term name filter].freeze + EXCLUDED_QUERY_PARAM_NAMES = %w[search term name filter filter_projects].freeze def initialize(app) @app = app diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 1315b3c0cee..ece506571f7 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -15361,6 +15361,9 @@ msgstr "" msgid "CreateGroup|You don’t have permission to create groups." msgstr "" +msgid "CreateGroup|You have reached the group limit until you verify your account." +msgstr "" + msgid "CreateTag|Tag" msgstr "" @@ -49688,6 +49691,9 @@ msgstr "" msgid "Some common domains are not allowed. %{learn_more_link}." msgstr "" +msgid "Some friendly information" +msgstr "" + msgid "Someone edited the %{issuableType} at the same time you did. Review %{linkStart}the %{issuableType}%{linkEnd} and make sure you don't unintentionally overwrite their changes." msgstr "" @@ -53375,6 +53381,9 @@ msgstr "" msgid "This is an experimental feature developed by GitLab Incubation Engineering." msgstr "" +msgid "This is really just a warning" +msgstr "" + msgid "This is the highest peak of users on your installation since the license started." msgstr "" @@ -53411,7 +53420,7 @@ msgstr "" msgid "This job depends on upstream jobs that need to succeed in order for this job to be triggered" msgstr "" -msgid "This job deploys to the protected environment \"%{environment}\", which requires approvals. You can approve or reject the deployment on the environment details page." +msgid "This job deploys to the protected environment \"%{environment}\", which requires approvals. You can approve or reject the deployment on the deployment details page." msgstr "" msgid "This job does not have a trace." @@ -57308,6 +57317,9 @@ msgstr "" msgid "View deployment" msgstr "" +msgid "View deployment details page" +msgstr "" + msgid "View details" msgstr "" @@ -57317,9 +57329,6 @@ msgstr "" msgid "View documentation" msgstr "" -msgid "View environment details page" -msgstr "" - msgid "View exposed artifact" msgid_plural "View %d exposed artifacts" msgstr[0] "" diff --git a/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_release_spec.rb b/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_release_spec.rb index e7af292ac69..57efe759a5c 100644 --- a/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_release_spec.rb +++ b/qa/qa/specs/features/api/1_manage/migration/gitlab_migration_release_spec.rb @@ -52,7 +52,11 @@ module QA it( 'successfully imports project release', - testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/360243' + testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/360243', + quarantine: { + type: :investigating, + issue: "https://gitlab.com/gitlab-org/gitlab/-/issues/461458" + } ) do expect_project_import_finished_successfully diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb index 6c4fc218b7d..8b8a058201b 100644 --- a/spec/factories/groups.rb +++ b/spec/factories/groups.rb @@ -15,6 +15,7 @@ FactoryBot.define do developers {} maintainers {} owners {} + creator {} # rubocop:enable Lint/EmptyBlock end @@ -32,6 +33,7 @@ FactoryBot.define do group.add_members(Array.wrap(evaluator.developers), :developer) group.add_members(Array.wrap(evaluator.maintainers), :maintainer) group.add_members(Array.wrap(evaluator.owners), :owner) + group.add_creator(evaluator.creator) if evaluator.creator end trait :with_organization do diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index 83a369eead7..c6d5d426660 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -468,12 +468,17 @@ RSpec.describe GroupsHelper, feature_category: :groups_and_projects do describe '#subgroup_creation_data' do let_it_be(:name) { 'parent group' } + let_it_be(:user) { build(:user) } let_it_be(:group) { build(:group, name: name) } let_it_be(:subgroup) { build(:group, parent: group) } + before do + allow(helper).to receive(:current_user) { user } + end + context 'when group has a parent' do it 'returns expected hash' do - expect(subgroup_creation_data(subgroup)).to include({ + expect(helper.subgroup_creation_data(subgroup)).to include({ import_existing_group_path: '/groups/new#import-group-pane', parent_group_name: name, parent_group_url: group_url(group), @@ -484,7 +489,7 @@ RSpec.describe GroupsHelper, feature_category: :groups_and_projects do context 'when group does not have a parent' do it 'returns expected hash' do - expect(subgroup_creation_data(group)).to include({ + expect(helper.subgroup_creation_data(group)).to include({ import_existing_group_path: '/groups/new#import-group-pane', parent_group_name: nil, parent_group_url: nil, diff --git a/spec/models/namespace/detail_spec.rb b/spec/models/namespace/detail_spec.rb index 27aec38e5c2..8cd58b61f0f 100644 --- a/spec/models/namespace/detail_spec.rb +++ b/spec/models/namespace/detail_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Namespace::Detail, type: :model do +RSpec.describe Namespace::Detail, type: :model, feature_category: :groups_and_projects do describe 'associations' do it { is_expected.to belong_to :namespace } it { is_expected.to belong_to(:creator).class_name('User') } @@ -50,4 +50,15 @@ RSpec.describe Namespace::Detail, type: :model do end end end + + describe '#add_creator' do + let(:namespace) { create(:namespace) } + let_it_be(:user) { create(:user) } + + it 'adds the creator' do + namespace.namespace_details.add_creator(user) + + expect(namespace.namespace_details.creator).to eq(user) + end + end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 3e168f4ac2e..667edd9a61a 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -536,6 +536,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do it { is_expected.to delegate_method(:math_rendering_limits_enabled).to(:namespace_settings) } it { is_expected.to delegate_method(:math_rendering_limits_enabled?).to(:namespace_settings) } it { is_expected.to delegate_method(:lock_math_rendering_limits_enabled?).to(:namespace_settings) } + it { is_expected.to delegate_method(:add_creator).to(:namespace_details) } it do is_expected.to delegate_method(:prevent_sharing_groups_outside_hierarchy=).to(:namespace_settings) diff --git a/spec/support/shared_examples/ci/waiting_for_approval_status_shared_examples.rb b/spec/support/shared_examples/ci/waiting_for_approval_status_shared_examples.rb index 44226345fd2..0bf681c93f6 100644 --- a/spec/support/shared_examples/ci/waiting_for_approval_status_shared_examples.rb +++ b/spec/support/shared_examples/ci/waiting_for_approval_status_shared_examples.rb @@ -43,8 +43,10 @@ RSpec.shared_examples 'a deployment job waiting for approval' do |factory_type| it { expect(subject.illustration).to include(:image, :size) } it { expect(subject.illustration[:title]).to eq('Waiting for approvals') } - it do - expect(subject.illustration[:content]).to include('This job deploys to the protected environment "production"') + it 'has the correct message' do + expected_message = "This job deploys to the protected environment \"production\", which requires approvals. " \ + "You can approve or reject the deployment on the deployment details page." + expect(subject.illustration[:content]).to eq expected_message end end @@ -61,16 +63,18 @@ RSpec.shared_examples 'a deployment job waiting for approval' do |factory_type| end describe '#action_button_title' do - it { expect(subject.action_button_title).to eq('View environment details page') } + it { expect(subject.action_button_title).to eq('View deployment details page') } end describe '#action_path' do - before do - environment = create(:environment, name: 'production', project: project) - create(:deployment, :blocked, project: project, environment: environment, deployable: job) - end + let!(:environment) { create(:environment, name: 'production', project: project) } + let!(:deployment) { create(:deployment, :blocked, project: project, environment: environment, deployable: job) } - it { expect(subject.action_path).to include('environments') } + it 'points to the deployment details page' do + expected_path = Rails.application.routes.url_helpers.project_environment_deployment_path( + project, environment, deployment) + expect(subject.action_path).to eq expected_path + end end describe '#action_method' do