Delay pinging AppSec team for JiHu contributions after first approval
Following up triage-serverless#125 (closed), this is brought up from gitlab-com/www-gitlab-com!90253 (comment 677426770)
Problem
If we ping the AppSec team too early in the life cycle of a merge request, chances are AppSec team can review too early and the review becomes outdated quickly, therefore a complete re-review might be needed later.
Proposal
Given that it's difficult to shift the ping after the "last" approval, we can at least shift the ping after the "first" approval.
Below was copied and edited from gitlab-com/www-gitlab-com!90253 (comment 677426770)
- MR gets submitted
- Bot makes the unresolved thread that acts as a blocker to merging until AppSec can review it (but does not
@-mention
the AppSec team yet) - Reviewers/maintainers go through the MR, request some changes, eventually approve it
- Once approved by the reviewers/maintainers, automation comments in that unresolved thread and pings the AppSec team for a
finalfollow up approval
Implementation
With triage-serverless!153 (merged) where it can leave a unique mark in the comment, and a way to detect if the unique mark is already being left, we can extend this so that it can find the original thread and attach new comments in the thread.
- Wrap the first AppSec thread with a unique mark
- When we receive an approval, find the above thread with the unique mark
- Comment and ping AppSec team on the thread
- (Bonus) Make sure the thread cannot be resolved before AppSec team member is pinged!
- (Bonus bonus) Make sure that only team members can resolve the thread!
Question: Final approval?
If we may find a good way to determine what is the "last" approval, we can as well make AppSec get pinged after that so AppSec team can be the last one to approve. This will be more ideal because we can make sure there's no loophole that AppSec team review can get outdated.
We can as well think about making this process more formal so maintainers can just follow that rule manually.
Question: triage-ops?
Can we do the same for triage-ops?