diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-12-29 18:09:53 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-12-29 18:09:53 +0000 |
commit | 2dcdb22ddf7f36f4f9581737bc4e639655e9f178 (patch) | |
tree | eb90d2340f7da8c9803d51025bb1ee8498887c37 | |
parent | dbdfb3e36dd5ed7b66a029cdd3202d6d7da5ae55 (diff) | |
download | gitlab-ce-2dcdb22ddf7f36f4f9581737bc4e639655e9f178.tar.gz |
Add latest changes from gitlab-org/gitlab@master
27 files changed, 175 insertions, 127 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index e64d3110c3a..36aae42e21f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -24,6 +24,7 @@ class ApplicationController < ActionController::Base include ::Gitlab::EndpointAttributes include FlocOptOut include CheckRateLimit + extend ContentSecurityPolicyPatch before_action :limit_session_time, if: -> { !current_user } before_action :authenticate_user!, except: [:route_not_found] diff --git a/app/controllers/concerns/observability/content_security_policy.rb b/app/controllers/concerns/observability/content_security_policy.rb index eccd1e1e3ef..3865e3b606d 100644 --- a/app/controllers/concerns/observability/content_security_policy.rb +++ b/app/controllers/concerns/observability/content_security_policy.rb @@ -5,8 +5,14 @@ module Observability extend ActiveSupport::Concern included do - content_security_policy do |p| - next if p.directives.blank? || Gitlab::Observability.observability_url.blank? + content_security_policy_with_context do |p| + current_group = if defined?(group) + group + else + defined?(project) ? project&.group : nil + end + + next if p.directives.blank? || !Gitlab::Observability.observability_enabled?(current_user, current_group) default_frame_src = p.directives['frame-src'] || p.directives['default-src'] diff --git a/app/controllers/groups/observability_controller.rb b/app/controllers/groups/observability_controller.rb index 3baa5e830ff..e0332ce6850 100644 --- a/app/controllers/groups/observability_controller.rb +++ b/app/controllers/groups/observability_controller.rb @@ -26,9 +26,7 @@ module Groups end def check_observability_allowed - return render_404 unless Gitlab::Observability.observability_url.present? - - render_404 unless can?(current_user, :read_observability, @group) + render_404 unless Gitlab::Observability.observability_enabled?(current_user, group) end end end diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 7ea15c830f3..1c546d70df9 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -46,8 +46,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic commit: commit, diff_view: diff_view, merge_ref_head_diff: render_merge_ref_head_diff?, - pagination_data: diffs.pagination_data, - merge_conflicts_in_diff: display_merge_conflicts_in_diff? + pagination_data: diffs.pagination_data } # NOTE: Any variables that would affect the resulting json needs to be added to the cache_context to avoid stale cache issues. @@ -59,8 +58,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic params[:expanded], params[:page], params[:per_page], - options[:merge_ref_head_diff], - options[:merge_conflicts_in_diff] + options[:merge_ref_head_diff] ] return unless stale?(etag: [cache_context + diff_options_hash.fetch(:paths, []), diffs]) @@ -84,8 +82,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic options = additional_attributes.merge( only_context_commits: show_only_context_commits?, - merge_ref_head_diff: render_merge_ref_head_diff?, - merge_conflicts_in_diff: display_merge_conflicts_in_diff? + merge_ref_head_diff: render_merge_ref_head_diff? ) render json: DiffsMetadataSerializer.new(project: @merge_request.project, current_user: current_user) @@ -113,8 +110,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic options = additional_attributes.merge( diff_view: "inline", - merge_ref_head_diff: render_merge_ref_head_diff?, - merge_conflicts_in_diff: display_merge_conflicts_in_diff? + merge_ref_head_diff: render_merge_ref_head_diff? ) options[:context_commits] = @merge_request.recent_context_commits @@ -242,8 +238,4 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter .track_mr_diffs_single_file_action(merge_request: @merge_request, user: current_user) end - - def display_merge_conflicts_in_diff? - Feature.enabled?(:display_merge_conflicts_in_diff, @merge_request.project) - end end diff --git a/app/serializers/diffs_entity.rb b/app/serializers/diffs_entity.rb index 759d1e0f10a..5b30c0bb58c 100644 --- a/app/serializers/diffs_entity.rb +++ b/app/serializers/diffs_entity.rb @@ -74,7 +74,7 @@ class DiffsEntity < Grape::Entity options.merge( submodule_links: submodule_links, code_navigation_path: code_navigation_path(diffs), - conflicts: (conflicts(allow_tree_conflicts: true) if options[:merge_conflicts_in_diff]) + conflicts: conflicts(allow_tree_conflicts: true) ) ) end diff --git a/app/serializers/diffs_metadata_entity.rb b/app/serializers/diffs_metadata_entity.rb index ace5105dda5..e55f31a8376 100644 --- a/app/serializers/diffs_metadata_entity.rb +++ b/app/serializers/diffs_metadata_entity.rb @@ -6,7 +6,7 @@ class DiffsMetadataEntity < DiffsEntity DiffFileMetadataEntity.represent( diffs.raw_diff_files(sorted: true), options.merge( - conflicts: (conflicts(allow_tree_conflicts: true) if options[:merge_conflicts_in_diff]) + conflicts: conflicts(allow_tree_conflicts: true) ) ) end diff --git a/app/serializers/paginated_diff_entity.rb b/app/serializers/paginated_diff_entity.rb index b79a0937659..67f4014990c 100644 --- a/app/serializers/paginated_diff_entity.rb +++ b/app/serializers/paginated_diff_entity.rb @@ -17,7 +17,7 @@ class PaginatedDiffEntity < Grape::Entity options.merge( submodule_links: submodule_links, code_navigation_path: code_navigation_path(diffs), - conflicts: (conflicts(allow_tree_conflicts: true) if options[:merge_conflicts_in_diff]) + conflicts: conflicts(allow_tree_conflicts: true) ) ) end diff --git a/config/feature_flags/development/display_merge_conflicts_in_diff.yml b/config/feature_flags/development/display_merge_conflicts_in_diff.yml deleted file mode 100644 index 71146d9236b..00000000000 --- a/config/feature_flags/development/display_merge_conflicts_in_diff.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: display_merge_conflicts_in_diff -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45008 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/276918 -milestone: '13.5' -type: development -group: group::code review -default_enabled: true diff --git a/db/post_migrate/20221228210616_add_index_on_ci_runners_on_runner_type_and_id.rb b/db/post_migrate/20221228210616_add_index_on_ci_runners_on_runner_type_and_id.rb new file mode 100644 index 00000000000..62c40b78945 --- /dev/null +++ b/db/post_migrate/20221228210616_add_index_on_ci_runners_on_runner_type_and_id.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AddIndexOnCiRunnersOnRunnerTypeAndId < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + OLD_INDEX_NAME = 'index_ci_runners_on_runner_type' + NEW_INDEX_NAME = 'index_ci_runners_on_runner_type_and_id' + + def up + add_concurrent_index :ci_runners, [:runner_type, :id], name: NEW_INDEX_NAME + remove_concurrent_index_by_name :ci_runners, OLD_INDEX_NAME + end + + def down + add_concurrent_index :ci_runners, :runner_type, name: OLD_INDEX_NAME + remove_concurrent_index_by_name :ci_runners, NEW_INDEX_NAME + end +end diff --git a/db/schema_migrations/20221228210616 b/db/schema_migrations/20221228210616 new file mode 100644 index 00000000000..69ff81fe265 --- /dev/null +++ b/db/schema_migrations/20221228210616 @@ -0,0 +1 @@ +a248f26495d2fab454538f8dd1c43e73ef4078f1822a900fce97b8c7f6df74c6
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index b252e564872..f549cb1c250 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -28999,7 +28999,7 @@ CREATE INDEX index_ci_runners_on_description_trigram ON ci_runners USING gin (de CREATE INDEX index_ci_runners_on_locked ON ci_runners USING btree (locked); -CREATE INDEX index_ci_runners_on_runner_type ON ci_runners USING btree (runner_type); +CREATE INDEX index_ci_runners_on_runner_type_and_id ON ci_runners USING btree (runner_type, id); CREATE INDEX index_ci_runners_on_token_expires_at_and_id_desc ON ci_runners USING btree (token_expires_at, id DESC); diff --git a/doc/user/project/merge_requests/changes.md b/doc/user/project/merge_requests/changes.md index f6e02dc0dfe..07ee2ef90c4 100644 --- a/doc/user/project/merge_requests/changes.md +++ b/doc/user/project/merge_requests/changes.md @@ -140,6 +140,7 @@ Files marked as viewed are not shown to you again unless either: > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/232484) in GitLab 13.5 [with a flag](../../../administration/feature_flags.md) named `display_merge_conflicts_in_diff`. Disabled by default. > - [Enabled on GitLab.com and self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/276918) in GitLab 15.7. +> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/276918) in GitLab 15.8. Feature flag `display_merge_conflicts_in_diff` removed. To avoid displaying the changes that are already on target branch in the diff, we compare the merge request's source branch with HEAD of the target branch. diff --git a/lib/gitlab/observability.rb b/lib/gitlab/observability.rb index 8dde60a73be..8dbd2f41ccb 100644 --- a/lib/gitlab/observability.rb +++ b/lib/gitlab/observability.rb @@ -11,5 +11,9 @@ module Gitlab 'https://observe.gitlab.com' end + + def observability_enabled?(user, group) + Gitlab::Observability.observability_url.present? && Ability.allowed?(user, :read_observability, group) + end end end diff --git a/lib/sidebars/groups/menus/observability_menu.rb b/lib/sidebars/groups/menus/observability_menu.rb index 656142375af..8cdbfbd3342 100644 --- a/lib/sidebars/groups/menus/observability_menu.rb +++ b/lib/sidebars/groups/menus/observability_menu.rb @@ -23,7 +23,7 @@ module Sidebars override :render? def render? - can?(context.current_user, :read_observability, context.group) + Gitlab::Observability.observability_enabled?(context.current_user, context.group) end private diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index f4fd78730b6..4de724fd6d6 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -213,7 +213,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: nil, latest_diff: true, only_context_commits: false, - merge_conflicts_in_diff: true, merge_ref_head_diff: false } end @@ -281,7 +280,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: nil, latest_diff: true, only_context_commits: false, - merge_conflicts_in_diff: true, merge_ref_head_diff: nil } end @@ -303,33 +301,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: merge_request.diff_head_commit, latest_diff: nil, only_context_commits: false, - merge_conflicts_in_diff: true, - merge_ref_head_diff: nil - } - end - end - end - - context 'when display_merge_conflicts_in_diff is disabled' do - subject { go } - - before do - stub_feature_flags(display_merge_conflicts_in_diff: false) - end - - it_behaves_like 'serializes diffs metadata with expected arguments' do - let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiff } - let(:expected_options) do - { - merge_request: merge_request, - merge_request_diff: merge_request.merge_request_diff, - merge_request_diffs: merge_request.merge_request_diffs, - start_version: nil, - start_sha: nil, - commit: nil, - latest_diff: true, - only_context_commits: false, - merge_conflicts_in_diff: false, merge_ref_head_diff: nil } end @@ -498,7 +469,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: nil, diff_view: :inline, merge_ref_head_diff: nil, - merge_conflicts_in_diff: true, pagination_data: { total_pages: nil }.merge(pagination_data) @@ -617,21 +587,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like 'successful request' end - context 'when display_merge_conflicts_in_diff is disabled' do - before do - stub_feature_flags(display_merge_conflicts_in_diff: false) - end - - subject { go } - - it_behaves_like 'serializes diffs with expected arguments' do - let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch } - let(:expected_options) { collection_arguments(total_pages: 20).merge(merge_conflicts_in_diff: false) } - end - - it_behaves_like 'successful request' - end - it_behaves_like 'forked project with submodules' it_behaves_like 'cached diff collection' diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb index a8ef99eeaec..5b0c88e711d 100644 --- a/spec/finders/ci/runners_finder_spec.rb +++ b/spec/finders/ci/runners_finder_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::RunnersFinder do +RSpec.describe Ci::RunnersFinder, feature_category: :runner_fleet do context 'admin' do let_it_be(:admin) { create(:user, :admin) } diff --git a/spec/lib/gitlab/observability_spec.rb b/spec/lib/gitlab/observability_spec.rb index 2b1d22d9019..8068d2f8ec9 100644 --- a/spec/lib/gitlab/observability_spec.rb +++ b/spec/lib/gitlab/observability_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require 'spec_helper' RSpec.describe Gitlab::Observability do describe '.observability_url' do @@ -30,4 +30,39 @@ RSpec.describe Gitlab::Observability do it { is_expected.to eq(observe_url) } end end + + describe '.observability_enabled?' do + let_it_be(:group) { build(:user) } + let_it_be(:user) { build(:group) } + + subject do + described_class.observability_enabled?(user, group) + end + + it 'checks if read_observability ability is allowed for the given user and group' do + allow(Ability).to receive(:allowed?).and_return(true) + + subject + + expect(Ability).to have_received(:allowed?).with(user, :read_observability, group) + end + + it 'returns true if the read_observability ability is allowed' do + allow(Ability).to receive(:allowed?).and_return(true) + + expect(subject).to eq(true) + end + + it 'returns false if the read_observability ability is not allowed' do + allow(Ability).to receive(:allowed?).and_return(false) + + expect(subject).to eq(false) + end + + it 'returns false if observability url is missing' do + allow(described_class).to receive(:observability_url).and_return("") + + expect(subject).to eq(false) + end + end end diff --git a/spec/lib/sidebars/groups/menus/observability_menu_spec.rb b/spec/lib/sidebars/groups/menus/observability_menu_spec.rb index 3a91b1aea2f..5b993cd6f28 100644 --- a/spec/lib/sidebars/groups/menus/observability_menu_spec.rb +++ b/spec/lib/sidebars/groups/menus/observability_menu_spec.rb @@ -20,23 +20,25 @@ RSpec.describe Sidebars::Groups::Menus::ObservabilityMenu do allow(menu).to receive(:can?).and_call_original end - context 'when user can :read_observability' do + context 'when observability is enabled' do before do - allow(menu).to receive(:can?).with(user, :read_observability, group).and_return(true) + allow(Gitlab::Observability).to receive(:observability_enabled?).and_return(true) end it 'returns true' do expect(menu.render?).to eq true + expect(Gitlab::Observability).to have_received(:observability_enabled?).with(user, group) end end - context 'when user cannot :read_observability' do + context 'when observability is disabled' do before do - allow(menu).to receive(:can?).with(user, :read_observability, group).and_return(false) + allow(Gitlab::Observability).to receive(:observability_enabled?).and_return(false) end it 'returns false' do expect(menu.render?).to eq false + expect(Gitlab::Observability).to have_received(:observability_enabled?).with(user, group) end end end diff --git a/spec/requests/projects/issues_controller_spec.rb b/spec/requests/projects/issues_controller_spec.rb index a943bd6449c..67a73834f2d 100644 --- a/spec/requests/projects/issues_controller_spec.rb +++ b/spec/requests/projects/issues_controller_spec.rb @@ -8,10 +8,14 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do let_it_be(:project) { issue.project } let_it_be(:user) { issue.author } + shared_context 'group project issue' do + let_it_be(:project) { create :project, group: group } + let_it_be(:issue) { create :issue, project: project } + let_it_be(:user) { create(:user) } + end + describe 'GET #new' do - before do - login_as(user) - end + include_context 'group project issue' it_behaves_like "observability csp policy", described_class do let(:tested_path) do @@ -21,9 +25,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do end describe 'GET #show' do - before do - login_as(user) - end + include_context 'group project issue' it_behaves_like "observability csp policy", described_class do let(:tested_path) do @@ -60,7 +62,10 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do end let_it_be(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) } - let_it_be(:discussion_reply) { create(:discussion_note_on_issue, noteable: issue, project: issue.project, in_reply_to: discussion) } + let_it_be(:discussion_reply) do + create(:discussion_note_on_issue, noteable: issue, project: issue.project, in_reply_to: discussion) + end + let_it_be(:state_event) { create(:resource_state_event, issue: issue) } let_it_be(:discussion_2) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) } let_it_be(:discussion_3) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) } @@ -114,7 +119,8 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do context 'when private project' do let_it_be(:private_project) { create(:project, :private) } - it_behaves_like 'authenticates sessionless user for the request spec', 'index atom', public_resource: false, ignore_metrics: true do + it_behaves_like 'authenticates sessionless user for the request spec', 'index atom', public_resource: false, +ignore_metrics: true do let(:url) { project_issues_url(private_project, format: :atom) } before do @@ -122,7 +128,8 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do end end - it_behaves_like 'authenticates sessionless user for the request spec', 'calendar ics', public_resource: false, ignore_metrics: true do + it_behaves_like 'authenticates sessionless user for the request spec', 'calendar ics', public_resource: false, +ignore_metrics: true do let(:url) { project_issues_url(private_project, format: :ics) } before do diff --git a/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb b/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb index 10e57970704..60223a30d28 100644 --- a/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb +++ b/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb @@ -35,7 +35,6 @@ RSpec.describe 'Merge Requests Context Commit Diffs', feature_category: :code_re commit: nil, diff_view: :inline, merge_ref_head_diff: nil, - merge_conflicts_in_diff: true, pagination_data: { total_pages: nil }.merge(pagination_data) diff --git a/spec/requests/projects/merge_requests/creations_spec.rb b/spec/requests/projects/merge_requests/creations_spec.rb index 59e2047e1c7..e299d711cb1 100644 --- a/spec/requests/projects/merge_requests/creations_spec.rb +++ b/spec/requests/projects/merge_requests/creations_spec.rb @@ -26,14 +26,17 @@ RSpec.describe 'merge requests creations', feature_category: :code_review do end it_behaves_like "observability csp policy", Projects::MergeRequests::CreationsController do + let_it_be(:group) { create(:group) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, group: group) } let(:tested_path) do project_new_merge_request_path(project, merge_request: { - title: 'Some feature', - source_branch: 'fix', - target_branch: 'feature', - target_project: project, - source_project: project - }) + title: 'Some feature', + source_branch: 'fix', + target_branch: 'feature', + target_project: project, + source_project: project + }) end end end diff --git a/spec/requests/projects/merge_requests/diffs_spec.rb b/spec/requests/projects/merge_requests/diffs_spec.rb index 858acac7f0d..dfdd372f8ad 100644 --- a/spec/requests/projects/merge_requests/diffs_spec.rb +++ b/spec/requests/projects/merge_requests/diffs_spec.rb @@ -33,7 +33,6 @@ RSpec.describe 'Merge Requests Diffs', feature_category: :code_review do commit: nil, diff_view: :inline, merge_ref_head_diff: nil, - merge_conflicts_in_diff: true, pagination_data: { total_pages: nil }.merge(pagination_data) @@ -112,17 +111,6 @@ RSpec.describe 'Merge Requests Diffs', feature_category: :code_review do it_behaves_like 'serializes diffs with expected arguments' end - context 'with disabled display_merge_conflicts_in_diff feature' do - let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch } - let(:expected_options) { collection_arguments(total_pages: 20).merge(merge_conflicts_in_diff: false) } - - before do - stub_feature_flags(display_merge_conflicts_in_diff: false) - end - - it_behaves_like 'serializes diffs with expected arguments' - end - context 'with diff_head option' do subject { go(page: 0, per_page: 5, diff_head: true) } diff --git a/spec/requests/projects/merge_requests_controller_spec.rb b/spec/requests/projects/merge_requests_controller_spec.rb index aa7a32ef2cf..2e16529b20d 100644 --- a/spec/requests/projects/merge_requests_controller_spec.rb +++ b/spec/requests/projects/merge_requests_controller_spec.rb @@ -8,6 +8,12 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :source_code let_it_be(:user) { merge_request.author } describe 'GET #show' do + let_it_be(:group) { create(:group) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create :project, group: group } + + let_it_be(:merge_request) { create :merge_request, source_project: project, author: user } + before do login_as(user) end @@ -43,7 +49,10 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :source_code describe 'GET #discussions' do let_it_be(:discussion) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } - let_it_be(:discussion_reply) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, in_reply_to: discussion) } + let_it_be(:discussion_reply) do + create(:discussion_note_on_merge_request, noteable: merge_request, project: project, in_reply_to: discussion) + end + let_it_be(:state_event) { create(:resource_state_event, merge_request: merge_request) } let_it_be(:discussion_2) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } let_it_be(:discussion_3) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) } @@ -113,7 +122,8 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :source_code context 'when private project' do let_it_be(:private_project) { create(:project, :private) } - it_behaves_like 'authenticates sessionless user for the request spec', 'index atom', public_resource: false, ignore_metrics: true do + it_behaves_like 'authenticates sessionless user for the request spec', 'index atom', public_resource: false, + ignore_metrics: true do let(:url) { project_merge_requests_url(private_project, format: :atom) } before do diff --git a/spec/serializers/diffs_entity_spec.rb b/spec/serializers/diffs_entity_spec.rb index ba40d538ccb..aa8e7275870 100644 --- a/spec/serializers/diffs_entity_spec.rb +++ b/spec/serializers/diffs_entity_spec.rb @@ -9,13 +9,11 @@ RSpec.describe DiffsEntity do let(:request) { EntityRequest.new(project: project, current_user: user) } let(:merge_request_diffs) { merge_request.merge_request_diffs } - let(:merge_conflicts_in_diff) { false } let(:options) do { request: request, merge_request: merge_request, - merge_request_diffs: merge_request_diffs, - merge_conflicts_in_diff: merge_conflicts_in_diff + merge_request_diffs: merge_request_diffs } end @@ -101,10 +99,9 @@ RSpec.describe DiffsEntity do subject[:diff_files] end - context 'when merge_conflicts_in_diff is true' do + context 'when there are conflicts' do let(:conflict_file) { double(path: diff_files.first.new_path, conflict_type: :both_modified) } let(:conflicts) { double(conflicts: double(files: [conflict_file]), can_be_resolved_in_ui?: false) } - let(:merge_conflicts_in_diff) { true } before do allow(merge_request).to receive(:cannot_be_merged?).and_return(true) diff --git a/spec/serializers/diffs_metadata_entity_spec.rb b/spec/serializers/diffs_metadata_entity_spec.rb index 04db576ffb5..415a0d8e450 100644 --- a/spec/serializers/diffs_metadata_entity_spec.rb +++ b/spec/serializers/diffs_metadata_entity_spec.rb @@ -9,7 +9,6 @@ RSpec.describe DiffsMetadataEntity do let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } let(:merge_request_diffs) { merge_request.merge_request_diffs } let(:merge_request_diff) { merge_request_diffs.last } - let(:merge_conflicts_in_diff) { false } let(:options) { {} } let(:entity) do @@ -18,8 +17,7 @@ RSpec.describe DiffsMetadataEntity do options.merge( request: request, merge_request: merge_request, - merge_request_diffs: merge_request_diffs, - merge_conflicts_in_diff: merge_conflicts_in_diff + merge_request_diffs: merge_request_diffs ) ) end @@ -67,10 +65,9 @@ RSpec.describe DiffsMetadataEntity do subject[:diff_files] end - context 'when merge_conflicts_in_diff is true' do + context 'when there are conflicts' do let(:conflict_file) { double(path: raw_diff_files.first.new_path, conflict_type: :both_modified) } let(:conflicts) { double(conflicts: double(files: [conflict_file]), can_be_resolved_in_ui?: false) } - let(:merge_conflicts_in_diff) { true } before do allow(merge_request).to receive(:cannot_be_merged?).and_return(true) diff --git a/spec/serializers/paginated_diff_entity_spec.rb b/spec/serializers/paginated_diff_entity_spec.rb index 3d77beb9abc..29484d170f8 100644 --- a/spec/serializers/paginated_diff_entity_spec.rb +++ b/spec/serializers/paginated_diff_entity_spec.rb @@ -7,13 +7,11 @@ RSpec.describe PaginatedDiffEntity do let(:request) { double('request', current_user: user) } let(:merge_request) { create(:merge_request) } let(:diff_batch) { merge_request.merge_request_diff.diffs_in_batch(2, 3, diff_options: nil) } - let(:merge_conflicts_in_diff) { false } let(:options) do { request: request, merge_request: merge_request, - pagination_data: diff_batch.pagination_data, - merge_conflicts_in_diff: merge_conflicts_in_diff + pagination_data: diff_batch.pagination_data } end @@ -43,10 +41,9 @@ RSpec.describe PaginatedDiffEntity do subject[:diff_files] end - context 'when merge_conflicts_in_diff is true' do + context 'when there are conflicts' do let(:conflict_file) { double(path: diff_files.first.new_path, conflict_type: :both_modified) } let(:conflicts) { double(conflicts: double(files: [conflict_file]), can_be_resolved_in_ui?: false) } - let(:merge_conflicts_in_diff) { true } before do allow(merge_request).to receive(:cannot_be_merged?).and_return(true) diff --git a/spec/support/shared_examples/observability/csp_shared_examples.rb b/spec/support/shared_examples/observability/csp_shared_examples.rb index 868d7023d14..0cd211f69eb 100644 --- a/spec/support/shared_examples/observability/csp_shared_examples.rb +++ b/spec/support/shared_examples/observability/csp_shared_examples.rb @@ -2,9 +2,17 @@ # Verifies that the proper CSP rules for Observabilty UI are applied to a given controller/path # -# The path under test needs to be declared with `let(:tested_path) { .. }` in the context including this example +# It requires the following variables declared in the context including this example: +# +# - `tested_path`: the path under test +# - `user`: the test user +# - `group`: the test group +# +# e.g. # # ``` +# let_it_be(:group) { create(:group) } +# let_it_be(:user) { create(:user) } # it_behaves_like "observability csp policy" do # let(:tested_path) { ....the path under test } # end @@ -33,6 +41,9 @@ RSpec.shared_examples 'observability csp policy' do |controller_class = describe before do setup_csp_for_controller(controller_class, csp, any_time: true) + group.add_developer(user) + login_as(user) + allow(Gitlab::Observability).to receive(:observability_enabled?).and_return(true) end subject do @@ -48,6 +59,40 @@ RSpec.shared_examples 'observability csp policy' do |controller_class = describe end end + context 'when observability is disabled' do + let(:csp) do + ActionDispatch::ContentSecurityPolicy.new do |p| + p.frame_src 'https://something.test' + end + end + + before do + allow(Gitlab::Observability).to receive(:observability_enabled?).and_return(false) + end + + it 'does not add observability urls to the csp header' do + expect(subject).to include("frame-src https://something.test") + expect(subject).not_to include("#{observability_url} #{signin_url} #{oauth_url}") + end + end + + context 'when checking if observability is enabled' do + let(:csp) do + ActionDispatch::ContentSecurityPolicy.new do |p| + p.frame_src 'https://something.test' + end + end + + it 'check access for a given user and group' do + allow(Gitlab::Observability).to receive(:observability_enabled?) + + get tested_path + + expect(Gitlab::Observability).to have_received(:observability_enabled?) + .with(user, group).at_least(:once) + end + end + context 'when frame-src exists in the CSP config' do let(:csp) do ActionDispatch::ContentSecurityPolicy.new do |p| |