Restore DS_ANALYZER_NAME for deprecated jobs
What does this MR do and why?
CI configs using the removed jobs bundler-audit and retire.js might have unexpected failure as the job definition will not point to the right location for the docker image to be downloaded.
This doesn't harm pipelines as the job is allowed to fail.
This pb was introduced with this MR: !87563 (diffs)
Here's an example: https://gitlab.com/gitlab-org/security-products/analyzers/container-scanning/-/jobs/2470461129
Describe in detail what your merge request does and why.
Screenshots or screen recordings
These are strongly recommended to assist reviewers and reduce the time to merge your change.
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
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.
Merge request reports
Activity
changed milestone to %15.0
assigned to @gonzoyumo
requested review from @ifrenkel and @brytannia
Suggested Reviewers (beta)
The individuals below may be good candidates to participate in the review based on various factors.
You can use slash commands in comments to quickly assign
/assign_reviewer @user1
.Suggested Reviewers @mayra-cabrera
,@matteeyah
,@brytannia
,@hfyngvason
,@bwill
If you do not believe these suggestions are useful, please apply the label Bad Suggested Reviewer. You can also provide feedback for this feature on this issue:
https://gitlab.com/gitlab-org/gitlab/-/issues/357923
.Automatically generated by Suggested Reviewers Bot - an experimental ML-based recommendation engine created by ~"group::applied ml".
added 1 commit
- 78f01b7f - Set DS_MAJOR_VERSION:2 for bundler-audit/retire-js
1 Warning You've made some app changes, but didn't add any tests.
That's OK as long as you're refactoring existing code,
but please consider adding any of the maintenancepipelines, maintenanceworkflow, documentation, QA labels.2 Messages 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.
This merge request adds or changes files that require a review from the CI/CD Templates maintainers. If needed, you can retry the
danger-review
job that generated this comment.This merge request requires a CI/CD Template review. To make sure these changes are reviewed, take the following steps:
- Ensure the merge request has the citemplates label. If the merge request modifies CI/CD Template files, Danger will do this for you.
- Prepare your MR for a CI/CD Template review according to the template development guide.
- Assign and
@
mention the CI/CD Template reviewer suggested by Reviewer Roulette.
The following files require a review from the CI/CD Templates maintainers:
lib/gitlab/ci/templates/Jobs/Dependency-Scanning.gitlab-ci.yml
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 citemplates Tiger Watson ( @tigerwnz
) (UTC+10, 14 hours ahead of@gonzoyumo
)Hordur Freyr Yngvason ( @hfyngvason
) (UTC-4, same timezone as@gonzoyumo
)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.
Generated by
Danger - Resolved by Olivier Gonzalez
- Resolved by Tetiana Chupryna
@gonzoyumo this has been tested here, however, the job fails because we exit with status 1 in the template. Shouldn't we implement something similar to Make License-Scanning backward compatible (!32002 - merged) to avoid "punishing users who don't follow all our deprecation notices"?
@gonzoyumo tests are currently failing because of 2022-05-18: CI Gateway in us-east1-d experienci... (gitlab-com/gl-infra/production#7084 - closed)
@fcatteau
, thanks for approving this merge request.This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.
For more info, please refer to the following links:
Allure report
allure-report-publisher
generated test report!review-qa-blocking:
test report for 78f01b7f expand test summary
+-------------------------------------------------------------------+ | suites summary | +----------------------+--------+--------+---------+-------+--------+ | | passed | failed | skipped | flaky | result | +----------------------+--------+--------+---------+-------+--------+ | Verify | 12 | 0 | 1 | 6 | ❗ | | Plan | 41 | 0 | 1 | 9 | ❗ | | Create | 24 | 0 | 2 | 8 | ❗ | | Manage | 34 | 0 | 2 | 8 | ❗ | | Configure | 0 | 0 | 1 | 0 | ➖ | | Protect | 2 | 0 | 0 | 2 | ❗ | | Package | 0 | 0 | 1 | 0 | ➖ | | Version sanity check | 0 | 0 | 1 | 0 | ➖ | +----------------------+--------+--------+---------+-------+--------+ | Total | 113 | 0 | 9 | 33 | ❗ | +----------------------+--------+--------+---------+-------+--------+
Thanks for making this MR @gonzoyumo - I re-ran the edge-case tests collected over the past week using the template from this MR and all pass:
test pipeline MR status normal DS detection tests/ruby-bundler/-/pipelines/542113448 custom stage for DS tests/ruby-bundler/-/pipelines/542100105 removed job w/o script section tests/ruby-bundler/-/pipelines/542119327 retire.js sanity check tests/js-npm/-/pipelines/542117041 bundler-audit job w/script section tests/ruby-bundler/-/pipelines/542102785 The last test row is the one that fails without the current MR (e.g. using the master template):
Edited by Igor Frenkel- Resolved by Igor Frenkel
@gonzoyumo one non-blocking suggestion: Perhaps it's better to use a generic image (like
ruby
oralpine
) rather than thebundler-audit
one since we might run into another failure if that image goes away. If someone overrides the image then they keep using the analyzer. If it's just the job they get the error message. WDYT?
requested review from @hfyngvason
removed review request for @brytannia