From b76b295fc3a2a4d5a7edac16037b357ce570392a Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Sat, 19 Apr 2025 00:08:58 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../lint/safe_navigation_consistency.yml | 17 +----- app/controllers/projects/jobs_controller.rb | 2 +- app/models/ci/build.rb | 1 - app/models/concerns/ci/deployable.rb | 2 +- .../policies/dependency_proxy/group_policy.rb | 2 +- .../work_items/callbacks/assignees.rb | 2 +- app/views/admin/groups/_group.html.haml | 2 +- .../_marked_for_deletion_badge.html.haml | 4 ++ .../cleanup_container_repository_worker.rb | 2 +- doc/api/openapi/openapi_v2.yaml | 2 +- lib/api/ci/jobs.rb | 2 +- lib/api/ci/pipeline_schedules.rb | 2 +- lib/api/helpers.rb | 2 +- .../importers/pull_request_note_importer.rb | 2 +- .../importers/pull_request_notes_importer.rb | 2 +- lib/gitlab/submodule_links.rb | 2 +- spec/requests/api/ci/jobs_spec.rb | 56 ++++++------------- .../work_items/callbacks/assignees_spec.rb | 14 +++++ 18 files changed, 50 insertions(+), 68 deletions(-) create mode 100644 app/views/admin/groups/_marked_for_deletion_badge.html.haml diff --git a/.rubocop_todo/lint/safe_navigation_consistency.yml b/.rubocop_todo/lint/safe_navigation_consistency.yml index 73bf57b92d8..8eb302a2894 100644 --- a/.rubocop_todo/lint/safe_navigation_consistency.yml +++ b/.rubocop_todo/lint/safe_navigation_consistency.yml @@ -1,16 +1 @@ ---- -# Cop supports --autocorrect. -Lint/SafeNavigationConsistency: - Exclude: - - 'app/models/concerns/ci/deployable.rb' - - 'app/policies/packages/policies/dependency_proxy/group_policy.rb' - - 'app/workers/container_expiration_policies/cleanup_container_repository_worker.rb' - - 'ee/app/controllers/concerns/ee/groups/params.rb' - - 'ee/app/models/ee/issue.rb' - - 'ee/app/services/ee/members/create_service.rb' - - 'ee/lib/ee/gitlab/auth/saml/user.rb' - - 'ee/lib/gitlab/status_page/storage.rb' - - 'lib/api/helpers.rb' - - 'lib/gitlab/bitbucket_server_import/importers/pull_request_note_importer.rb' - - 'lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer.rb' - - 'lib/gitlab/submodule_links.rb' +# Empty after resolving Lint/SafeNavigationConsistency diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 69ceb8116b9..4fc30eec61f 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -242,7 +242,7 @@ class Projects::JobsController < Projects::ApplicationController end def force_param - %w[1 t true y yes].include?(params[:force].to_s.downcase) + params[:force] == "true" end def play_params diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 3b062b14709..a187e81c882 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -47,7 +47,6 @@ module Ci DEGRADATION_THRESHOLD_VARIABLE_NAME = 'DEGRADATION_THRESHOLD' RUNNERS_STATUS_CACHE_EXPIRATION = 1.minute - CANCELABLE_STATUSES = (HasStatus::CANCELABLE_STATUSES + ['canceling']).freeze DEPLOYMENT_NAMES = %w[deploy release rollout].freeze TOKEN_PREFIX = 'glcbt-' diff --git a/app/models/concerns/ci/deployable.rb b/app/models/concerns/ci/deployable.rb index 3abb70547e4..0956a9eb324 100644 --- a/app/models/concerns/ci/deployable.rb +++ b/app/models/concerns/ci/deployable.rb @@ -38,7 +38,7 @@ module Ci project.ci_forward_deployment_enabled? && (!project.ci_forward_deployment_rollback_allowed? || incomplete?) && deployment&.persisted? && - deployment&.older_than_last_successful_deployment? + deployment.older_than_last_successful_deployment? end strong_memoize_attr :has_outdated_deployment? diff --git a/app/policies/packages/policies/dependency_proxy/group_policy.rb b/app/policies/packages/policies/dependency_proxy/group_policy.rb index b0aed7dfcaf..e19bf29bfed 100644 --- a/app/policies/packages/policies/dependency_proxy/group_policy.rb +++ b/app/policies/packages/policies/dependency_proxy/group_policy.rb @@ -21,7 +21,7 @@ module Packages desc "Deploy token with read access to dependency proxy" condition(:read_dependency_proxy_deploy_token) do - deploy_token_user? && @user&.valid_for_dependency_proxy? && @user&.has_access_to_group?(@subject.group) + deploy_token_user? && @user&.valid_for_dependency_proxy? && @user.has_access_to_group?(@subject.group) end # TODO: Remove the deploy token check when we create a deploy token policy diff --git a/app/services/work_items/callbacks/assignees.rb b/app/services/work_items/callbacks/assignees.rb index 0febcfb2e79..aad81a3f06d 100644 --- a/app/services/work_items/callbacks/assignees.rb +++ b/app/services/work_items/callbacks/assignees.rb @@ -27,7 +27,7 @@ module WorkItems assignee_ids = assignee_ids.first(1) unless work_item.allows_multiple_assignees? assignees = User.id_in(assignee_ids) - assignees.select { |assignee| assignee.can?(:read_work_item, work_item) }.map(&:id) + assignees.select { |assignee| assignee.can?(:read_work_item, work_item.resource_parent) }.map(&:id) end end end diff --git a/app/views/admin/groups/_group.html.haml b/app/views/admin/groups/_group.html.haml index d18ff5a9e4e..080449d2e89 100644 --- a/app/views/admin/groups/_group.html.haml +++ b/app/views/admin/groups/_group.html.haml @@ -17,7 +17,7 @@ = gl_badge_tag storage_counter(group.storage_size) = render_if_exists 'admin/namespace_plan_badge', namespace: group, css_class: 'gl-ml-5 gl-mr-0' - = render_if_exists 'admin/groups/marked_for_deletion_badge', group: group, css_class: 'gl-ml-5' + = render 'admin/groups/marked_for_deletion_badge', group: group, css_class: 'gl-ml-5' %span.gl-ml-5.has-tooltip{ title: _('Projects') } = sprite_icon('project', css_class: 'gl-align-text-bottom') diff --git a/app/views/admin/groups/_marked_for_deletion_badge.html.haml b/app/views/admin/groups/_marked_for_deletion_badge.html.haml new file mode 100644 index 00000000000..23b9c6db757 --- /dev/null +++ b/app/views/admin/groups/_marked_for_deletion_badge.html.haml @@ -0,0 +1,4 @@ +- css_class = local_assigns.fetch(:css_class, '') + +- if group.marked_for_deletion? + = render Pajamas::BadgeComponent.new(_('Pending deletion'), variant: 'warning', class: css_class) diff --git a/app/workers/container_expiration_policies/cleanup_container_repository_worker.rb b/app/workers/container_expiration_policies/cleanup_container_repository_worker.rb index aea2e7df727..6c1709fcc26 100644 --- a/app/workers/container_expiration_policies/cleanup_container_repository_worker.rb +++ b/app/workers/container_expiration_policies/cleanup_container_repository_worker.rb @@ -121,7 +121,7 @@ module ContainerExpirationPolicies end def allowed_to_run? - return false unless policy&.enabled && policy&.next_run_at + return false unless policy&.enabled && policy.next_run_at now = Time.zone.now diff --git a/doc/api/openapi/openapi_v2.yaml b/doc/api/openapi/openapi_v2.yaml index cb17b3dbafb..50fbcfc5a44 100644 --- a/doc/api/openapi/openapi_v2.yaml +++ b/doc/api/openapi/openapi_v2.yaml @@ -49498,7 +49498,7 @@ definitions: properties: force: type: boolean - description: Whether to force cancellation for a job in `canceling` state + description: Force cancellation for a job with a state of `canceling` example: true description: Cancel a specific job of a project postApiV4ProjectsIdJobsJobIdPlay: diff --git a/lib/api/ci/jobs.rb b/lib/api/ci/jobs.rb index 051cda0d723..8ebff7694ee 100644 --- a/lib/api/ci/jobs.rb +++ b/lib/api/ci/jobs.rb @@ -123,7 +123,7 @@ module API end params do requires :job_id, type: Integer, desc: 'The ID of a job', documentation: { example: 88 } - optional :force, type: Boolean, desc: 'Whether to force cancellation for a job in `canceling` state', documentation: { example: true } + optional :force, type: Boolean, desc: 'Force cancellation for a job with a state of `canceling`', documentation: { example: true } end post ':id/jobs/:job_id/cancel', urgency: :low, feature_category: :continuous_integration do authorize_cancel_builds! diff --git a/lib/api/ci/pipeline_schedules.rb b/lib/api/ci/pipeline_schedules.rb index 90d7c68f16c..dc2da00799e 100644 --- a/lib/api/ci/pipeline_schedules.rb +++ b/lib/api/ci/pipeline_schedules.rb @@ -35,7 +35,7 @@ module API authorize! :read_pipeline_schedule, user_project schedules = ::Ci::PipelineSchedulesFinder.new(user_project).execute(scope: params[:scope]) - .preload([:owner, :last_pipeline, :inputs]) + .preload([:owner, :inputs]) present paginate(schedules), with: Entities::Ci::PipelineSchedule end # rubocop: enable CodeReuse/ActiveRecord diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 5aca5fe7d76..3da1311b9f1 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -1026,7 +1026,7 @@ module API end def handle_job_token_failure!(project) - if current_user&.from_ci_job_token? && current_user&.ci_job_token_scope + if current_user&.from_ci_job_token? && current_user.ci_job_token_scope source_project = current_user.ci_job_token_scope.current_project error_message = format("Authentication by CI/CD job token not allowed from %{source_project_path} to %{target_project_path}.", source_project_path: source_project.path, target_project_path: project.path) diff --git a/lib/gitlab/bitbucket_server_import/importers/pull_request_note_importer.rb b/lib/gitlab/bitbucket_server_import/importers/pull_request_note_importer.rb index 663dc977344..9dbd429efe7 100644 --- a/lib/gitlab/bitbucket_server_import/importers/pull_request_note_importer.rb +++ b/lib/gitlab/bitbucket_server_import/importers/pull_request_note_importer.rb @@ -60,7 +60,7 @@ module Gitlab end def import_data_valid? - project.import_data&.credentials && project.import_data&.data + project.import_data&.credentials && project.import_data.data end end end diff --git a/lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer.rb b/lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer.rb index 6ec26b620bf..32f754312eb 100644 --- a/lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer.rb +++ b/lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer.rb @@ -50,7 +50,7 @@ module Gitlab end def import_data_valid? - project.import_data&.credentials && project.import_data&.data + project.import_data&.credentials && project.import_data.data end # rubocop: disable CodeReuse/ActiveRecord diff --git a/lib/gitlab/submodule_links.rb b/lib/gitlab/submodule_links.rb index 38b10c5892d..303d58c342e 100644 --- a/lib/gitlab/submodule_links.rb +++ b/lib/gitlab/submodule_links.rb @@ -35,7 +35,7 @@ module Gitlab end def old_submodule_id(submodule_url, diff_file) - return unless diff_file&.old_blob && diff_file&.old_content_sha + return unless diff_file&.old_blob && diff_file.old_content_sha # if the submodule url has changed from old_sha to sha, a compare link does not make sense # diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index 3cb0a2a8d7d..01a66915840 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -866,49 +866,29 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do end end - [ - [:job, false, 'success', :user, :created], - [:running_job, false, 'canceled', :user, :created], - [:canceling_job, false, 'canceling', :user, :created], - [:job, true, 'success', :user, :forbidden], - [:running_job, true, 'running', :user, :forbidden], - [:running_job, true, 'running', :maintainer, :created], # Force cancel does not default to regular cancel - [:canceling_job, true, 'canceling', :user, :forbidden], - [:canceling_job, true, 'canceled', :maintainer, :created] - ].each do |job, force_param, expected_job_status, user_sym, http_status| - describe "POST /projects/:id/jobs/:#{job}_id/cancel?force=#{force_param}" do - let(:api_user) { send(user_sym) } + describe "POST /projects/:id/jobs/:job_id/cancel?force" do + where(:test_job, :test_user, :force, :expected_job_status, :expected_http_status) do + ref(:job) | ref(:user) | false | 'success' | :created + ref(:running_job) | ref(:user) | false | 'canceled' | :created + ref(:canceling_job) | ref(:user) | false | 'canceling' | :created + ref(:job) | ref(:user) | true | 'success' | :forbidden + ref(:running_job) | ref(:user) | true | 'running' | :forbidden + ref(:running_job) | ref(:maintainer) | true | 'running' | :created # Force cancel does not default to regular cancel + ref(:canceling_job) | ref(:user) | true | 'canceling' | :forbidden + ref(:canceling_job) | ref(:maintainer) | true | 'canceled' | :created + end + with_them do before do - post api("/projects/#{project.id}/jobs/#{send(job).id}/cancel?force=#{force_param}", api_user) + post api("/projects/#{project.id}/jobs/#{test_job.id}/cancel?force=#{force}", test_user) end - context 'authorized user' do - context "#{user_sym} with :cancel_build permission" do - it "cancels :#{job}" do - expect(response).to have_gitlab_http_status(http_status) + it "responds as expected" do + expect(response).to have_gitlab_http_status(expected_http_status) - if http_status != :forbidden - json_response = Gitlab::Json.parse(response.body) - expect(json_response['status']).to eq(expected_job_status) - end - end - end - - context 'user without :cancel_build permission' do - let(:api_user) { reporter } - - it 'does not cancel job' do - expect(response).to have_gitlab_http_status(:forbidden) - end - end - end - - context 'unauthorized user' do - let(:api_user) { nil } - - it 'does not cancel job' do - expect(response).to have_gitlab_http_status(:unauthorized) + if expected_http_status == :created + json_response = Gitlab::Json.parse(response.body) + expect(json_response['status']).to eq(expected_job_status) end end end diff --git a/spec/services/work_items/callbacks/assignees_spec.rb b/spec/services/work_items/callbacks/assignees_spec.rb index 12f7e3818ec..e6939eec182 100644 --- a/spec/services/work_items/callbacks/assignees_spec.rb +++ b/spec/services/work_items/callbacks/assignees_spec.rb @@ -46,6 +46,20 @@ RSpec.describe WorkItems::Callbacks::Assignees, :freeze_time, feature_category: end end + context 'when user has permission on parent' do + let(:public_project) { create(:project, :public, reporters: reporter) } + let(:confidential_wi) { create(:work_item, :confidential, project: public_project, updated_at: 1.day.ago) } + + let(:params) { { assignee_ids: [new_assignee.id] } } + let(:service) { described_class.new(issuable: confidential_wi, current_user: current_user, params: params) } + + it 'updates the assignees' do + assignees_callback + + expect(confidential_wi.assignee_ids).to contain_exactly(new_assignee.id) + end + end + context 'when multiple assignees are given' do let(:params) { { assignee_ids: [new_assignee.id, reporter.id] } }