On MR merge, save a list of compliance violations that occurred at the point of merging
What does this MR do and why?
This MR adds a new service worker to the EE PostMergeService
so when an MR is merged, we will asynchronously review the merged MR for any compliance violations and save these to the merge_requests_compliance_violations
table. These saved violations will be used by #347325 (closed).
We add a new feature flag so we can easily incrementally roll this out or roll this back to protect the merge process.
During testing it became apparent that the initial assumptions on data locations were incorrect so I have had to update the violation classes with:
- Updated
ApprovedByCommitter
andApprovedByMergeRequestAuthor
to useapproved_by_users
rather thanapprover_users
- Updated
ApprovedByInsufficientUsers
to use the merge request metrics data if it exists
How to set up and validate locally
- Enable the feature flag
compliance_violations_graphql_type
:echo "Feature.enable(:compliance_violations_graphql_type)" | bundle exec rails c
- Go to a projects general settings and make sure
Prevent approval by author.
is unticked under theMerge request approvals
section - Edit a file in the project and create a new merge request
- Use the merge requests author to approve the merge request and then merge it
- Wait for merging to be completed
- Check the
merge_request_compliance_violations
table to see it has been filled with three violations (reasons0
,1
and2
) for your merge request- Use a database GUI
- In your terminal enter
gdk psql
and then run the following query:SELECT * FROM merge_requests_compliance_violations;
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #347324 (closed)
Merge request reports
Activity
changed milestone to %14.7
assigned to @rob.hunt
removed workflowin dev label
added 1 commit
- 7a31ea10 - Started adding a worker to trigger the violations processing.
1 Message CHANGELOG missing: If you want to create a changelog entry for GitLab FOSS, add the
Changelog
trailer to the commit message you want to add to the changelog.If you want to create a changelog entry for GitLab EE, also add the
EE: true
trailer to your commit message.If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer backend Tyler Amos ( @tyleramos
) (UTC-5, 5 hours behind@rob.hunt
)James Fargher ( @proglottis
) (UTC+13, 13 hours ahead of@rob.hunt
)To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
Sidekiq queue changes
This merge request contains changes to Sidekiq queues. Please follow the documentation on changing a queue's urgency.
These queues were added:
compliance_management_merge_requests_compliance_violations
If needed, you can retry the
danger-review
job that generated this comment.Generated by
Danger- Resolved by Robert Hunt
@rob.hunt Is there a way for the user to see the list of violations?
added 689 commits
-
7a31ea10...9f7ece4a - 687 commits from branch
master
- 2b124cda - Started adding a worker to trigger the violations processing.
- 62d799b9 - Adding to the PostMergeService and added a feature flag
-
7a31ea10...9f7ece4a - 687 commits from branch
changed milestone to %14.8
- A deleted user
added feature flag label
Allure report
allure-report-publisher
generated test report for 437b3feb!package-and-qa-ff-disabled:
test report
package-and-qa-ff-enabled: test report
review-qa-smoke: test report
review-qa-reliable: test reportadded 137 commits
-
62d799b9...15c73d5d - 134 commits from branch
master
- 6f85265b - Started adding a worker to trigger the violations processing.
- a2c299fa - Adding to the PostMergeService and added a feature flag
- 1484ef70 - Move to EE PostMergeService and updated YAML configs for worker
Toggle commit list-
62d799b9...15c73d5d - 134 commits from branch
added 221 commits
-
1484ef70...b59b523f - 217 commits from branch
master
- cd91372f - Started adding a worker to trigger the violations processing.
- a19d14c8 - Adding to the PostMergeService and added a feature flag
- 3562355a - Move to EE PostMergeService and updated YAML configs for worker
- 935b52fe - WIP: Working on getting the worker to function by testing using specs
Toggle commit list-
1484ef70...b59b523f - 217 commits from branch
added 137 commits
-
935b52fe...e2aaae9c - 136 commits from branch
master
- 41c399b4 - Add service and worker to save compliance violations when MR is merged
-
935b52fe...e2aaae9c - 136 commits from branch
added 19 commits
-
41c399b4...7cf74b8f - 18 commits from branch
master
- b5654664 - Add service and worker to save compliance violations when MR is merged
-
41c399b4...7cf74b8f - 18 commits from branch
added 1 commit
- c90f29a9 - Add service and worker to save compliance violations when MR is merged
added 223 commits
-
c90f29a9...a36e640b - 221 commits from branch
master
- d29331fa - Add service and worker to save compliance violations when MR is merged
- 58f4978d - Fixed violations so they actually work!
-
c90f29a9...a36e640b - 221 commits from branch