diff --git a/ee/app/services/concerns/security/scan_result_policies/policy_logger.rb b/ee/app/services/concerns/security/scan_result_policies/policy_logger.rb new file mode 100644 index 0000000000000000000000000000000000000000..e6fd5587e972dbb328a0120d8bb5963dc3135184 --- /dev/null +++ b/ee/app/services/concerns/security/scan_result_policies/policy_logger.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Security + module ScanResultPolicies + module PolicyLogger + private + + def log_policy_evaluation(event, message, project: nil, **attributes) + default_attributes = { + workflow: 'approval_policy_evaluation', + event: event, + project_path: project&.full_path + }.compact + Gitlab::AppJsonLogger.info(message: message, **default_attributes.merge(attributes)) + end + end + end +end diff --git a/ee/app/services/ee/merge_requests/post_merge_service.rb b/ee/app/services/ee/merge_requests/post_merge_service.rb index 9572f3bab654093a3a02bb007fccb5f40c4d78a5..ba5970f11e146bb5305a920391075b6cffe32b18 100644 --- a/ee/app/services/ee/merge_requests/post_merge_service.rb +++ b/ee/app/services/ee/merge_requests/post_merge_service.rb @@ -4,6 +4,7 @@ module EE module MergeRequests module PostMergeService extend ::Gitlab::Utils::Override + include ::Security::ScanResultPolicies::PolicyLogger override :execute def execute(merge_request, source = nil) @@ -97,9 +98,8 @@ def track_policies_running_violations(merge_request) violations = merge_request.running_scan_result_policy_violations return if violations.none? - ::Gitlab::AppLogger.warn( - message: 'Running scan result policy violations after merge', - project_path: project.full_path, + log_policy_evaluation('post_merge', 'Running scan result policy violations after merge', + project: project, merge_request_id: merge_request.id, merge_request_iid: merge_request.iid, head_pipeline_id: merge_request.diff_head_pipeline&.id, diff --git a/ee/app/services/security/scan_result_policies/sync_any_merge_request_rules_service.rb b/ee/app/services/security/scan_result_policies/sync_any_merge_request_rules_service.rb index 5a1abe10261e7823f67ea72e07d6d8ed4bb9aa91..12af846fced248e5262ca5807161b05a2444481b 100644 --- a/ee/app/services/security/scan_result_policies/sync_any_merge_request_rules_service.rb +++ b/ee/app/services/security/scan_result_policies/sync_any_merge_request_rules_service.rb @@ -5,6 +5,7 @@ module ScanResultPolicies class SyncAnyMergeRequestRulesService include Gitlab::Utils::StrongMemoize include ::Security::ScanResultPolicies::PolicyViolationCommentGenerator + include ::Security::ScanResultPolicies::PolicyLogger def initialize(merge_request) @merge_request = merge_request @@ -32,6 +33,7 @@ def sync_required_approvals violated_policies, unviolated_policies = evaluate_policy_violations(related_policies) + log_message('Evaluating any_merge_request rules from approval policies') violated_rules, unviolated_rules = rules_for_violated_policies(violated_policies) violated_rules, unviolated_rules = update_required_approvals(violated_rules, unviolated_rules) @@ -137,22 +139,18 @@ def log_violated_rules(rules) return unless rules.any? rules.each do |approval_rule| - log_violated_rule( + log_message('Updating MR approval rule', + reason: 'any_merge_request rule violated', approval_rule_id: approval_rule.id, approval_rule_name: approval_rule.name ) end end - def log_violated_rule(**attributes) - default_attributes = { - reason: 'any_merge_request rule violated', - event: 'update_approvals', - merge_request_id: merge_request.id, - merge_request_iid: merge_request.iid, - project_path: merge_request.project.full_path - } - Gitlab::AppJsonLogger.info(message: 'Updating MR approval rule', **default_attributes.merge(attributes)) + def log_message(message, **attributes) + log_policy_evaluation('update_approvals', message, + project: project, merge_request_id: merge_request.id, + merge_request_iid: merge_request.iid, **attributes) end def save_violation_data(violated_policies) diff --git a/ee/app/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service.rb b/ee/app/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service.rb index d0b9cb16b32e96a7babb00156e82d372c523d531..2663537f2bb504ac62eb43668e337e4604af5a53 100644 --- a/ee/app/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service.rb +++ b/ee/app/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service.rb @@ -4,6 +4,7 @@ module Security module ScanResultPolicies class SyncPreexistingStatesApprovalRulesService include VulnerabilityStatesHelper + include ::Security::ScanResultPolicies::PolicyLogger def initialize(merge_request) @merge_request = merge_request @@ -30,11 +31,14 @@ def execute delegate :project, to: :merge_request, private: true def evaluate_rules(approval_rules) + log_message('Evaluating pre_existing scan_finding rules from approval policies') approval_rules.each do |rule| rule_violated, violation_data = preexisting_findings_count_violated?(rule) if rule_violated - log_violated_rule(approval_rule_id: rule.id, approval_rule_name: rule.name) + log_message('Updating MR approval rule with pre_existing states', + reason: 'pre_existing scan_finding rule violated', + approval_rule_id: rule.id, approval_rule_name: rule.name) evaluation.fail!(rule, data: violation_data) else evaluation.pass!(rule) @@ -62,18 +66,9 @@ def vulnerabilities(approval_rule) ::Security::ScanResultPolicies::VulnerabilitiesFinder.new(project, finder_params).execute end - def log_violated_rule(**attributes) - default_attributes = { - reason: 'pre_existing scan_finding_rule violated', - event: 'update_approvals', - merge_request_id: merge_request.id, - merge_request_iid: merge_request.iid, - project_path: merge_request.project.full_path - } - Gitlab::AppJsonLogger.info( - message: 'Updating MR approval rule with pre_existing states', - **default_attributes.merge(attributes) - ) + def log_message(message, **attributes) + log_policy_evaluation('update_approvals', message, + project: project, merge_request_id: merge_request.id, merge_request_iid: merge_request.iid, **attributes) end def evaluation diff --git a/ee/app/services/security/scan_result_policies/update_approvals_service.rb b/ee/app/services/security/scan_result_policies/update_approvals_service.rb index a1510e97ddd512d0854df2e42ade0402162b38c0..f3b5e324d0200cba43165c32b15b5c7ed4e5d530 100644 --- a/ee/app/services/security/scan_result_policies/update_approvals_service.rb +++ b/ee/app/services/security/scan_result_policies/update_approvals_service.rb @@ -5,6 +5,7 @@ module ScanResultPolicies class UpdateApprovalsService include Gitlab::Utils::StrongMemoize include VulnerabilityStatesHelper + include ::Security::ScanResultPolicies::PolicyLogger attr_reader :pipeline, :merge_request, :approval_rules @@ -18,7 +19,11 @@ def execute pipeline_complete = pipeline.complete_or_manual? return unless pipeline_complete - return unless pipeline.can_store_security_reports? + + unless pipeline.can_store_security_reports? + log_update_approval_rule('No security reports found for the pipeline', **validation_context) + return + end approval_rules_with_newly_detected_states = filter_newly_detected_rules(:scan_finding, approval_rules) return if approval_rules_with_newly_detected_states.empty? @@ -40,7 +45,7 @@ def validation_context delegate :project, to: :pipeline def evaluate_rules(approval_rules) - log_update_approval_rule('Evaluating MR approval rules from scan result policies', **validation_context) + log_update_approval_rule('Evaluating scan_finding rules from approval policies', **validation_context) approval_rules.each do |merge_request_approval_rule| approval_rule = merge_request_approval_rule.try(:source_rule) || merge_request_approval_rule @@ -80,13 +85,8 @@ def evaluate_rules(approval_rules) end def log_update_approval_rule(message, **attributes) - default_attributes = { - event: 'update_approvals', - merge_request_id: merge_request.id, - merge_request_iid: merge_request.iid, - project_path: project.full_path - } - Gitlab::AppJsonLogger.info(message: message, **default_attributes.merge(attributes)) + log_policy_evaluation('update_approvals', message, + project: project, merge_request_id: merge_request.id, merge_request_iid: merge_request.iid, **attributes) end def violates_approval_rule?(approval_rule) diff --git a/ee/app/services/security/scan_result_policies/update_license_approvals_service.rb b/ee/app/services/security/scan_result_policies/update_license_approvals_service.rb index 7117bfb1f32afdbeb7dd8be40759d97489b2be06..4cff90e5a53d0a854fd2a3662ad16d0b0e2ef23d 100644 --- a/ee/app/services/security/scan_result_policies/update_license_approvals_service.rb +++ b/ee/app/services/security/scan_result_policies/update_license_approvals_service.rb @@ -4,6 +4,7 @@ module Security module ScanResultPolicies class UpdateLicenseApprovalsService include Gitlab::Utils::StrongMemoize + include ::Security::ScanResultPolicies::PolicyLogger def initialize(merge_request, pipeline, preexisting_states = false) @merge_request = merge_request @@ -20,7 +21,11 @@ def initialize(merge_request, pipeline, preexisting_states = false) def execute return if merge_request.merged? return if approval_rules.empty? - return if !preexisting_states && !scanner.results_available? + + if !preexisting_states && !scanner.results_available? + log_update_approval_rule('No SBOM reports found for the pipeline') + return + end filtered_rules = filter_approval_rules(approval_rules) return if filtered_rules.empty? @@ -42,6 +47,7 @@ def filter_approval_rules(approval_rules) end def evaluate_rules(license_approval_rules) + log_update_approval_rule('Evaluating license_scanning rules from approval policies', **validation_context) license_approval_rules.each do |approval_rule| # We only error for fail-open. Fail closed policy is evaluated as "failing" if !target_branch_pipeline && fail_open?(approval_rule) @@ -53,7 +59,8 @@ def evaluate_rules(license_approval_rules) if rule_violated evaluation.fail!(approval_rule, data: violation_data, context: validation_context) - log_update_approval_rule(approval_rule_id: approval_rule.id, approval_rule_name: approval_rule.name) + log_update_approval_rule('Updating MR approval rule', reason: 'license_finding rule violated', + approval_rule_id: approval_rule.id, approval_rule_name: approval_rule.name) else evaluation.pass!(approval_rule) end @@ -117,19 +124,10 @@ def validation_context { pipeline_ids: [pipeline&.id].compact, target_pipeline_ids: [target_branch_pipeline&.id].compact } end - def log_update_approval_rule(**attributes) - default_attributes = { - reason: 'license_finding rule violated', - event: 'update_approvals', - merge_request_id: merge_request.id, - merge_request_iid: merge_request.iid, - project_path: project.full_path - } - - Gitlab::AppJsonLogger.info( - message: 'Updating MR approval rule', - **default_attributes.merge(attributes).merge(validation_context) - ) + def log_update_approval_rule(message, **attributes) + log_policy_evaluation('update_approvals', message, + project: project, merge_request_id: merge_request.id, + merge_request_iid: merge_request.iid, **attributes.merge(validation_context)) end def build_violation_data(denied_licenses_with_dependencies) diff --git a/ee/app/services/security/unenforceable_policy_rules_notification_service.rb b/ee/app/services/security/unenforceable_policy_rules_notification_service.rb index 9ebf2eae5c5e45f8f6d99a63ff56dacc421b6673..5675115f9d042bc58570616c1d8f5ff63688f3a5 100644 --- a/ee/app/services/security/unenforceable_policy_rules_notification_service.rb +++ b/ee/app/services/security/unenforceable_policy_rules_notification_service.rb @@ -5,6 +5,7 @@ class UnenforceablePolicyRulesNotificationService include Gitlab::Utils::StrongMemoize include ::Security::ScanResultPolicies::RelatedPipelines include ::Security::ScanResultPolicies::VulnerabilityStatesHelper + include ::Security::ScanResultPolicies::PolicyLogger def initialize(merge_request) @merge_request = merge_request @@ -14,8 +15,8 @@ def initialize(merge_request) def execute approval_rules = merge_request.approval_rules.including_scan_result_policy_read - notify_for_report_type(merge_request, :scan_finding, approval_rules.scan_finding) - notify_for_report_type(merge_request, :license_scanning, approval_rules.license_scanning) + update_for_report_type(merge_request, :scan_finding, approval_rules.scan_finding) + update_for_report_type(merge_request, :license_scanning, approval_rules.license_scanning) end private @@ -24,8 +25,13 @@ def execute delegate :project, to: :merge_request, private: true - def notify_for_report_type(merge_request, report_type, approval_rules) - return unless unenforceable_report?(report_type) + def update_for_report_type(merge_request, report_type, approval_rules) + pipelines = pipelines_with_enforceable_reports(report_type) + if pipelines.present? + log_message(report_type, "No unenforceable #{report_type} rules detected, skipping", + pipelines_with_reports_ids: pipelines.map(&:id)) + return + end unblock_fail_open_rules(report_type) @@ -34,6 +40,7 @@ def notify_for_report_type(merge_request, report_type, approval_rules) applicable_rules = filter_newly_detected_rules(report_type, approval_rules) return if applicable_rules.blank? + log_message(report_type, "Unenforceable #{report_type} rules detected") policy_evaluation = Security::SecurityOrchestrationPolicies::PolicyRuleEvaluationService .new(merge_request, approval_rules, report_type) @@ -45,20 +52,22 @@ def notify_for_report_type(merge_request, report_type, approval_rules) policy_evaluation.save end - def unenforceable_report?(report_type) - return true if pipeline.nil? + def pipelines_with_enforceable_reports(report_type) + return [] if pipeline.nil? case report_type when :scan_finding # Pipelines which can store security reports are handled via SyncFindingsToApprovalRulesService - related_pipelines.none?(&:can_store_security_reports?) + related_pipelines.select(&:can_store_security_reports?) when :license_scanning # Pipelines which have scanning results available are handled via SyncLicenseScanningRulesService - related_pipelines.none?(&:can_ingest_sbom_reports?) + related_pipelines.select(&:can_ingest_sbom_reports?) end end def related_pipelines + return [] if pipeline.nil? + project.all_pipelines.id_in(related_pipeline_ids(pipeline)) end strong_memoize_attr :related_pipelines @@ -68,5 +77,11 @@ def unblock_fail_open_rules(report_type) .new(merge_request: merge_request, report_types: [report_type]) .execute end + + def log_message(report_type, message, **attributes) + log_policy_evaluation('unenforceable_rules', message, + project: project, report_type: report_type, merge_request_id: merge_request.id, + merge_request_iid: merge_request.iid, related_pipeline_ids: related_pipelines.map(&:id), **attributes) + end end end diff --git a/ee/spec/services/ee/merge_requests/post_merge_service_spec.rb b/ee/spec/services/ee/merge_requests/post_merge_service_spec.rb index 19ffde8850a3bc8452a193cffb8d5cada4c16be6..74cb8644c822e8af5266fc143ff3b53dfbc3d905 100644 --- a/ee/spec/services/ee/merge_requests/post_merge_service_spec.rb +++ b/ee/spec/services/ee/merge_requests/post_merge_service_spec.rb @@ -219,8 +219,9 @@ scan_result_policy_read: policy) end - it 'logs a warning' do - expect(Gitlab::AppLogger).to receive(:warn).with(a_hash_including( + it 'logs a message' do + expect(Gitlab::AppJsonLogger).to receive(:info).with(a_hash_including( + workflow: 'approval_policy_evaluation', message: 'Running scan result policy violations after merge', merge_request_id: merge_request.id, violation_ids: [violation.id] @@ -232,8 +233,8 @@ context 'when feature is not licensed' do let(:security_orchestration_enabled) { false } - it 'does not log a warning' do - expect(Gitlab::AppLogger).not_to receive(:warn) + it 'does not log a message' do + expect(Gitlab::AppJsonLogger).not_to receive(:info) execute end @@ -241,8 +242,8 @@ end context 'without running_scan_result_policy_violations' do - it 'does log a warning' do - expect(Gitlab::AppLogger).not_to receive(:warn) + it 'does not log a message' do + expect(Gitlab::AppJsonLogger).not_to receive(:info) execute end diff --git a/ee/spec/services/security/scan_result_policies/sync_any_merge_request_rules_service_spec.rb b/ee/spec/services/security/scan_result_policies/sync_any_merge_request_rules_service_spec.rb index c4eea7e33fe8459e9338382fad6c1df1aedf7d8a..6d2ed8cfa556caf5edf9da856bd5ffc48b192431 100644 --- a/ee/spec/services/security/scan_result_policies/sync_any_merge_request_rules_service_spec.rb +++ b/ee/spec/services/security/scan_result_policies/sync_any_merge_request_rules_service_spec.rb @@ -63,6 +63,12 @@ it 'creates no violation records' do expect { execute }.not_to change { merge_request.scan_result_policy_violations.count } end + + it 'does not create a log' do + expect(Gitlab::AppJsonLogger).not_to receive(:info) + + execute + end end describe 'approval rules' do @@ -80,8 +86,10 @@ let(:archived_project) { merge_request.project } end - it 'does not create a log' do - expect(Gitlab::AppJsonLogger).not_to receive(:info) + it 'logs only the evaluation and not a violated rule' do + expect(Gitlab::AppJsonLogger).to receive(:info).with( + hash_including(message: 'Evaluating any_merge_request rules from approval policies') + ) execute end @@ -203,6 +211,8 @@ it_behaves_like 'merge request with scan result violations' it 'logs violated rules' do + expect(Gitlab::AppJsonLogger).to receive(:info).with(hash_including( + message: 'Evaluating any_merge_request rules from approval policies')) expect(Gitlab::AppJsonLogger).to receive(:info).with(hash_including(message: 'Updating MR approval rule')) execute diff --git a/ee/spec/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service_spec.rb b/ee/spec/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service_spec.rb index 4a850110348b2593cab0a60e2575d5cf0d3a3109..02eade06bf0b71733d25197239c6951e860b041c 100644 --- a/ee/spec/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service_spec.rb +++ b/ee/spec/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service_spec.rb @@ -55,6 +55,15 @@ end end + shared_examples_for 'logs only evaluation' do + it 'logs the start of the evaluation' do + expect(Gitlab::AppJsonLogger).to receive(:info).with(hash_including( + message: 'Evaluating pre_existing scan_finding rules from approval policies')) + + execute + end + end + context 'when merge_request is merged' do before do merge_request.update!(state: 'merged') @@ -70,7 +79,7 @@ it_behaves_like 'does not update approval rules' it_behaves_like 'triggers policy bot comment', :scan_finding, false, requires_approval: false - it_behaves_like 'does not log violations' + it_behaves_like 'logs only evaluation' end context 'when rules do not contain pre-existing states' do @@ -116,7 +125,7 @@ it_behaves_like 'sets approvals_required to 0' it_behaves_like 'triggers policy bot comment', :scan_finding, false - it_behaves_like 'does not log violations' + it_behaves_like 'logs only evaluation' it_behaves_like 'merge request without scan result violations' end @@ -136,16 +145,19 @@ it_behaves_like 'triggers policy bot comment', :scan_finding, true it 'logs update' do + expect(Gitlab::AppJsonLogger).to receive(:info).with(hash_including( + message: 'Evaluating pre_existing scan_finding rules from approval policies')) expect(::Gitlab::AppJsonLogger) .to receive(:info).once.ordered .with( + workflow: 'approval_policy_evaluation', event: 'update_approvals', message: 'Updating MR approval rule with pre_existing states', approval_rule_id: approver_rule.id, approval_rule_name: approver_rule.name, merge_request_id: merge_request.id, merge_request_iid: merge_request.iid, - reason: 'pre_existing scan_finding_rule violated', + reason: 'pre_existing scan_finding rule violated', project_path: project.full_path ).and_call_original @@ -167,7 +179,7 @@ context 'when vulnerabilities count does not match the pre-existing states' do it_behaves_like 'sets approvals_required to 0' it_behaves_like 'triggers policy bot comment', :scan_finding, false - it_behaves_like 'does not log violations' + it_behaves_like 'logs only evaluation' it_behaves_like 'merge request without scan result violations' context 'when there are other scan_finding violations' do diff --git a/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb b/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb index 77d50d0958deed712ec8bdf14d1fd3960e1d2203..a1fbcecd7854f688d14307aaaa42b202ae8d1ad4 100644 --- a/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb +++ b/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb @@ -165,10 +165,11 @@ expect(::Gitlab::AppJsonLogger) .to receive(:info).once.ordered .with( + workflow: 'approval_policy_evaluation', event: 'update_approvals', merge_request_id: merge_request.id, merge_request_iid: merge_request.iid, - message: 'Evaluating MR approval rules from scan result policies', + message: 'Evaluating scan_finding rules from approval policies', pipeline_ids: [pipeline.id], target_pipeline_ids: [target_pipeline.id], project_path: project.full_path @@ -177,6 +178,7 @@ expect(::Gitlab::AppJsonLogger) .to receive(:info).once.ordered .with( + workflow: 'approval_policy_evaluation', event: 'update_approvals', approval_rule_id: report_approver_rule.id, approval_rule_name: report_approver_rule.name, @@ -489,10 +491,11 @@ expect(::Gitlab::AppJsonLogger) .to receive(:info).once.ordered .with( + workflow: 'approval_policy_evaluation', event: 'update_approvals', merge_request_id: merge_request.id, merge_request_iid: merge_request.iid, - message: 'Evaluating MR approval rules from scan result policies', + message: 'Evaluating scan_finding rules from approval policies', pipeline_ids: [pipeline.id], target_pipeline_ids: [target_pipeline.id], project_path: project.full_path @@ -501,6 +504,7 @@ expect(::Gitlab::AppJsonLogger) .to receive(:info).once.ordered .with( + workflow: 'approval_policy_evaluation', event: 'update_approvals', approval_rule_id: report_approver_rule.id, approval_rule_name: report_approver_rule.name, @@ -781,6 +785,15 @@ end it_behaves_like 'does not update approvals_required' + + it 'logs a message' do + expect(::Gitlab::AppJsonLogger).to receive(:info).with(a_hash_including( + workflow: 'approval_policy_evaluation', + event: 'update_approvals', + message: 'No security reports found for the pipeline')) + + execute + end end context 'when security scan is removed in related pipeline' do diff --git a/ee/spec/services/security/scan_result_policies/update_license_approvals_service_spec.rb b/ee/spec/services/security/scan_result_policies/update_license_approvals_service_spec.rb index d8fe692df3a5df1e0ddd6f2d2f94183569738c73..cfde7e075fac8900668188cc9642d6597312c017 100644 --- a/ee/spec/services/security/scan_result_policies/update_license_approvals_service_spec.rb +++ b/ee/spec/services/security/scan_result_policies/update_license_approvals_service_spec.rb @@ -81,6 +81,12 @@ end it_behaves_like 'does not trigger policy bot comment' + + it 'does not call logger' do + expect(Gitlab::AppJsonLogger).not_to receive(:info) + + execute + end end describe 'violation data' do @@ -124,6 +130,8 @@ it_behaves_like 'triggers policy bot comment', :license_scanning, true it 'logs the violated rules' do + expect(Gitlab::AppJsonLogger).to receive(:info).with(hash_including( + message: 'Evaluating license_scanning rules from approval policies')) expect(Gitlab::AppJsonLogger).to receive(:info).with(hash_including(message: 'Updating MR approval rule')) execute @@ -156,8 +164,9 @@ it_behaves_like 'triggers policy bot comment', :license_scanning, false it_behaves_like 'merge request without scan result violations' - it 'does not call logger' do - expect(Gitlab::AppJsonLogger).not_to receive(:info) + it 'only logs evaluation' do + expect(Gitlab::AppJsonLogger).to receive(:info).with(hash_including( + message: 'Evaluating license_scanning rules from approval policies')) execute end @@ -196,6 +205,13 @@ it_behaves_like 'requires approval' it_behaves_like 'does not trigger policy bot comment' + + it 'logs a message' do + expect(Gitlab::AppJsonLogger).to receive(:info).with(hash_including( + message: 'No SBOM reports found for the pipeline')) + + execute + end end context 'when there are no violations' do diff --git a/ee/spec/services/security/unenforceable_policy_rules_notification_service_spec.rb b/ee/spec/services/security/unenforceable_policy_rules_notification_service_spec.rb index f14368679f9073318c0b6df0beab1b232b95f6b3..48362fd653101a9f05fa3aabe09ba3ae36c300ed 100644 --- a/ee/spec/services/security/unenforceable_policy_rules_notification_service_spec.rb +++ b/ee/spec/services/security/unenforceable_policy_rules_notification_service_spec.rb @@ -123,6 +123,12 @@ context 'without report approver rules' do it_behaves_like 'does not trigger policy bot comment' + + it 'does not log message' do + expect(Gitlab::AppJsonLogger).not_to receive(:info) + + execute + end end context 'when all reports are enforceable' do @@ -132,6 +138,15 @@ end it_behaves_like 'does not block enforceable rules' + + it 'logs the corresponding message' do + expect(Gitlab::AppJsonLogger).to receive(:info).with( + hash_including(message: 'No unenforceable scan_finding rules detected, skipping')) + expect(Gitlab::AppJsonLogger).to receive(:info).with( + hash_including(message: 'No unenforceable license_scanning rules detected, skipping')) + + execute + end end context 'when merge request has no head pipeline' do @@ -190,6 +205,16 @@ it_behaves_like 'triggers policy bot comment', report_type, true + it 'logs the corresponding message' do + allow(Gitlab::AppJsonLogger).to receive(:info) + expect(Gitlab::AppJsonLogger).to receive(:info).with(hash_including( + event: 'unenforceable_rules', + message: "Unenforceable #{report_type} rules detected" + )) + + execute + end + it 'updates violation status' do expect { execute }.to change { violation.reload.status }.from('running').to('failed') end