Skip to content

Resolve "Auditing of changes to project tags"

What does this MR do?

Implements #5246 . Specifically, whenever a the tag list on a project is edited, an audit event will be created containing which tags were added and/or removed. The audit_events table will contain entries such as

 id |                            details
----+----------------------------------------------------------------
 33 | ---                                                           +
    | :change: tags                                                 +
    | :author_name: Administrator                                   +
    | :target_id: 5                                                 +
    | :target_type: Project                                         +
    | :target_details: documentcloud/underscore                     +
    | :added:                                                       +
    | - my_tag                                                      +
    | :removed:                                                     +
    | - my_taag                                                     +
    | - interesting project

where only the id and details are displayed for brevity.

Are there points in the code the reviewer needs to double check?

  1. The application of the monkey patch was agreed to be done on the module level, not in the model class itself. However, I was not able to make that work. It did not work in the past either. To check what the earlier proposed solution looks like, you can check out on https://gitlab.com/Krijger/gitlab-ee/tree/project-tag-auditing-module-patching. If you read the comments in this MR, you will see many "works for me" statements. This is because (I found out recently) the first tag already produced a project.previous_changes with a value, probably because in this case a setter was called instead of an array modification. However, subsequent calls (which were never tested by the GitLab reviewers) did not. So, even though the patch was not loaded at all, it appeared to work. To be reviewed: including the patch within project.rb, or fix the https://gitlab.com/Krijger/gitlab-ee/tree/project-tag-auditing-module-patching method.
  2. The existing audit events all log from and to. This does not make perfect sense in the case of a list of values. I hcose (as discussed earlier with @zj) to use added and removed and tried to make the code branching for this as clean as possible

Why was this MR needed?

To implement issue #5246 ; specifically:

Project tags are a great way to categorise projects. For on-premise installations, this is especially strong as it can be used in combination with custom added functionality using git hooks; using tags is a simple way to enable or disable such custom functionality.

Screenshots (if relevant)

Does this MR meet the acceptance criteria?

  • Changelog entry added, if necessary CONTRIBUTOR COMMENT: yes
  • Documentation created/updated
  • API support added . - CONTRIBUTOR COMMENT: not added, but verified that API calls also lead to the audit events
  • Tests added for this feature/bug - CONTRIBUTOR COMMENT: only unit tests
  • Review
    • Has been reviewed by UX
    • Has been reviewed by Frontend
    • Has been reviewed by Backend
    • Has been reviewed by Database
  • Conform by the merge request performance guides
  • Conform by the style guides
  • Squashed related commits together
  • Internationalization required/considered
  • If paid feature, have we considered GitLab.com plan and how it works for groups and is there a design for promoting it to users who aren't on the correct plan
  • End-to-end tests pass (package-qa manual pipeline job)

What are the relevant issue numbers?

Closes #5246

Edited by Zeger-Jan van de Weg

Merge request reports