Draft: Add jobs failed alert to security MR widget
What does this MR do?
This is a PoC/draft of the remaining work for #267504 (closed). It's incomplete feature-wise, and will likely become increasingly stale as time goes on due to design churn of &5710 (closed) and &3534. In fact, it's already stale based on the discussions in #267504 (closed) (e.g., see this thread: #267504 (comment 572527697)).
Alert implementation commit
This adds the alert, as originally designed in #12896 (closed), to the security MR widget.
The design is being rethought at the moment, perhaps in large part due to &5710 (closed), which is an effort to redesign all merge request widgets.
This alert implementation is almost certainly not going to be part of the eventual design (as far as I can tell), but the general structure of how to implement it is here (minus tests).
Note that several unrelated GraphQL queries were modified (id fields were added to various entities) due to a manifestation of #326101 (closed).
"Show when target branch has security reports, but source branch doesn't" implementation commit
Specifically, this makes the security MR widget show when the source branch is based on an old commit of the target branch, such that it isn't configured to run any security scans. Meanwhile, the target branch has advanced and is configured to run security scans. This gives an opportunity to prompt the user to rebase the source branch, in order to run security scans.
It should be noted that this is a rare edge case, and I'm not sure how worthwhile it is to actually handle it. A project is only likely to enable security scanning once, and once it has, any new branches created after it are unaffected. Only a few existing MRs, based on an old commit of the target branch, would show this, until they're rebased. To put this another way, the window when this is likely to happen for a given project is brief, and happens probably only once in a given project's lifetime.
This minimal implementation exposes the security_reports_up_to_date?
field to the frontend, as per the second example in
#275818 (closed).
Unfortunately, more work need to be done, as the MR widget renders in this case with a successful status, and message "Security scanning detected no vulnerabilities" which is not correct - security scanning did not even run!
Exactly what should be rendered instead varies from design to design, so it's not clear what to actually implement right now. As with the previous commit, this may become clearer with &5710 (closed).
Screenshots (strongly suggested)
Job failure alert
As mentioned above, this was the original design, but is now stale (the intended design is TBD).
Widget shown when target branch has security reports, but source branch doesn't
As described above, the implementation doesn't show the right message/status for this case, but the design is TBD.
Does this MR meet the acceptance criteria?
Conformity
-
I have included changelog trailers, or none are needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides. -
This change is backwards compatible across updates, or this does not apply.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) -
I have tested this MR in all supported browsers, or it's not needed. -
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.
Security
Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team
Related to #267504 (closed)