Skip to content

Resolve audit event missing attribute error

Bala Kumar requested to merge 402173-auditevents-missing-attribute into master

What does this MR do and why?

Fixes missing attribute error when author user is not available during the audit event of approval rules creation.

The user is nil because we are using policy_configuration.policy_last_updated_by which can be nil when the policy configuration commit user Git email does not exist in GitLab. Related finding: #402173 (comment 1348099239)

In another MR we'll include documentation changes for the other cases that do not work as intented because policy_last_updated_by is empty. Related discussion: #402173 (comment 1350359197)

How to set up and validate locally

  1. Create a Simple scan result policy like
scan_result_policy:
- name: License Policy 1
  description: ''
  enabled: true
  actions:
  - type: require_approval
    approvals_required: 1
    users_approvers_ids:
    - 1
  rules:
  - type: license_finding
    branches: []
    match_on_inclusion: true
    license_types:
    - GNU General Public License v3.0 or later
    license_states:
    - newly_detected
  1. Amend (git commit --amend --author="Author Name <email@address.com>" --no-edit , git push -f) policy merge commit with a Git user say test@example.com or remove the policy merge committed user from GitLab by deleting user.
  2. Create a new protected branch which invokes Security::ProcessScanResultPolicyWorker and we should not observe the missing attribute error in Sidekiq logs.
  AuditEvents::BuildService::MissingAttributeError: AuditEvents::BuildService::MissingAttributeError
  from app/services/audit_events/build_service.rb:14:in `initialize'
  from lib/gitlab/audit/auditor.rb:169:in `new'
  from lib/gitlab/audit/auditor.rb:169:in `build_event'
  from ee/lib/ee/gitlab/audit/auditor.rb:16:in `block in multiple_audit'
  from ee/lib/ee/gitlab/audit/auditor.rb:16:in `map'
  from ee/lib/ee/gitlab/audit/auditor.rb:16:in `multiple_audit'
  from lib/gitlab/audit/auditor.rb:56:in `audit'
  from ee/app/services/concerns/approval_rules/updater.rb:33:in `with_audit_logged'
  from ee/app/services/concerns/approval_rules/updater.rb:12:in `action'
  from ee/app/services/approval_rules/base_service.rb:8:in `execute'
  from ee/app/services/security/security_orchestration_policies/process_scan_result_policy_service.rb:37:in `block in create_new_approval_rules'
  from ee/app/services/security/security_orchestration_policies/process_scan_result_policy_service.rb:26:in `each'
  from ee/app/services/security/security_orchestration_policies/process_scan_result_policy_service.rb:26:in `each_with_index'
  from ee/app/services/security/security_orchestration_policies/process_scan_result_policy_service.rb:26:in `create_new_approval_rules'
  from ee/app/services/security/security_orchestration_policies/process_scan_result_policy_service.rb:15:in `execute'`

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #402173 (closed)

Edited by Bala Kumar

Merge request reports

Loading