Commit 9cbae06e authored by Dheeraj Joshi's avatar Dheeraj Joshi Committed by Tim Zallmann

Display message if MR Security report is outdated

When the last pipeline for the target branch hadn't detected some
security issues, maybe due to jobs failed or artifacts expired, the
MR security report should be considered outdated and the pipeline needs
to be re-run. It also handles the edge case when target branch has
received more commits and the current branch needs to be updated.
parent 2c11ce5a
...@@ -165,21 +165,23 @@ export default { ...@@ -165,21 +165,23 @@ export default {
<template> <template>
<section class="media-section"> <section class="media-section">
<div class="media"> <div class="media">
<status-icon :status="statusIconName" :size="24" /> <status-icon :status="statusIconName" :size="24" class="align-self-center" />
<div class="media-body d-flex flex-align-self-center"> <div class="media-body d-flex flex-align-self-center align-items-center">
<span class="js-code-text code-text"> <div class="js-code-text code-text">
<div>
{{ headerText }} {{ headerText }}
<slot :name="slotName"></slot> <slot :name="slotName"></slot>
<popover v-if="hasPopover" :options="popoverOptions" class="prepend-left-5" /> <popover v-if="hasPopover" :options="popoverOptions" class="prepend-left-5" />
</span> </div>
<slot name="subHeading"></slot>
</div>
<slot name="actionButtons"></slot> <slot name="actionButtons"></slot>
<button <button
v-if="isCollapsible" v-if="isCollapsible"
type="button" type="button"
class="js-collapse-btn btn float-right btn-sm align-self-start qa-expand-report-button" class="js-collapse-btn btn float-right btn-sm align-self-center qa-expand-report-button"
@click="toggleCollapsed" @click="toggleCollapsed"
> >
{{ collapseText }} {{ collapseText }}
......
---
title: Display in MR if security report is outdated
merge_request: 20954
author:
type: other
...@@ -274,6 +274,7 @@ export default { ...@@ -274,6 +274,7 @@ export default {
v-if="shouldRenderSecurityReport" v-if="shouldRenderSecurityReport"
:head-blob-path="mr.headBlobPath" :head-blob-path="mr.headBlobPath"
:source-branch="mr.sourceBranch" :source-branch="mr.sourceBranch"
:target-branch="mr.targetBranch"
:base-blob-path="mr.baseBlobPath" :base-blob-path="mr.baseBlobPath"
:enabled-reports="mr.enabledSecurityReports" :enabled-reports="mr.enabledSecurityReports"
:sast-head-path="mr.sast.head_path" :sast-head-path="mr.sast.head_path"
......
...@@ -9,6 +9,7 @@ import Icon from '~/vue_shared/components/icon.vue'; ...@@ -9,6 +9,7 @@ import Icon from '~/vue_shared/components/icon.vue';
import IssueModal from './components/modal.vue'; import IssueModal from './components/modal.vue';
import securityReportsMixin from './mixins/security_report_mixin'; import securityReportsMixin from './mixins/security_report_mixin';
import createStore from './store'; import createStore from './store';
import { s__, sprintf } from '~/locale';
export default { export default {
store: createStore(), store: createStore(),
...@@ -40,6 +41,11 @@ export default { ...@@ -40,6 +41,11 @@ export default {
required: false, required: false,
default: null, default: null,
}, },
targetBranch: {
type: String,
required: false,
default: null,
},
sastHeadPath: { sastHeadPath: {
type: String, type: String,
required: false, required: false,
...@@ -169,6 +175,7 @@ export default { ...@@ -169,6 +175,7 @@ export default {
'sastContainerStatusIcon', 'sastContainerStatusIcon',
'dastStatusIcon', 'dastStatusIcon',
'dependencyScanningStatusIcon', 'dependencyScanningStatusIcon',
'isBaseSecurityReportOutOfDate',
]), ]),
...mapGetters('sast', ['groupedSastText', 'sastStatusIcon']), ...mapGetters('sast', ['groupedSastText', 'sastStatusIcon']),
securityTab() { securityTab() {
...@@ -191,6 +198,25 @@ export default { ...@@ -191,6 +198,25 @@ export default {
hasSastReports() { hasSastReports() {
return this.hasReportsType('sast'); return this.hasReportsType('sast');
}, },
subHeadingText() {
const mrDivergedCommitsCount =
(gl && gl.mrWidgetData && gl.mrWidgetData.diverged_commits_count) || 0;
const isMRBranchOutdated = mrDivergedCommitsCount > 0;
if (isMRBranchOutdated) {
return sprintf(
s__(
'Security report is out of date. Please incorporate latest changes from %{targetBranchName}',
),
{
targetBranchName: this.targetBranch,
},
);
}
return sprintf(
s__('Security report is out of date. Retry the pipeline for the target branch.'),
);
},
}, },
created() { created() {
...@@ -358,6 +384,10 @@ export default { ...@@ -358,6 +384,10 @@ export default {
</a> </a>
</div> </div>
<div v-if="isBaseSecurityReportOutOfDate" slot="subHeading" class="text-secondary-700 text-1">
<span>{{ subHeadingText }}</span>
</div>
<div slot="body" class="mr-widget-grouped-section report-block"> <div slot="body" class="mr-widget-grouped-section report-block">
<template v-if="hasSastReports"> <template v-if="hasSastReports">
<summary-row <summary-row
......
...@@ -130,5 +130,11 @@ export const anyReportHasIssues = state => ...@@ -130,5 +130,11 @@ export const anyReportHasIssues = state =>
state.sastContainer.newIssues.length > 0 || state.sastContainer.newIssues.length > 0 ||
state.dependencyScanning.newIssues.length > 0; state.dependencyScanning.newIssues.length > 0;
export const isBaseSecurityReportOutOfDate = state =>
state.sast.baseReportOutofDate ||
state.dast.baseReportOutofDate ||
state.sastContainer.baseReportOutofDate ||
state.dependencyScanning.baseReportOutofDate;
// prevent babel-plugin-rewire from generating an invalid default during karma tests // prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {}; export default () => {};
...@@ -61,11 +61,13 @@ export default { ...@@ -61,11 +61,13 @@ export default {
[types.RECEIVE_DIFF_SUCCESS](state, { diff, enrichData }) { [types.RECEIVE_DIFF_SUCCESS](state, { diff, enrichData }) {
const { added, fixed, existing } = parseDiff(diff, enrichData); const { added, fixed, existing } = parseDiff(diff, enrichData);
const baseReportOutofDate = diff.base_report_out_of_date || false;
state.isLoading = false; state.isLoading = false;
state.newIssues = added; state.newIssues = added;
state.resolvedIssues = fixed; state.resolvedIssues = fixed;
state.allIssues = existing; state.allIssues = existing;
state.baseReportOutofDate = baseReportOutofDate;
}, },
[types.RECEIVE_DIFF_ERROR](state) { [types.RECEIVE_DIFF_ERROR](state) {
......
...@@ -11,4 +11,5 @@ export default () => ({ ...@@ -11,4 +11,5 @@ export default () => ({
newIssues: [], newIssues: [],
resolvedIssues: [], resolvedIssues: [],
allIssues: [], allIssues: [],
baseReportOutofDate: false,
}); });
...@@ -108,11 +108,13 @@ export default { ...@@ -108,11 +108,13 @@ export default {
[types.RECEIVE_SAST_CONTAINER_DIFF_SUCCESS](state, { diff, enrichData }) { [types.RECEIVE_SAST_CONTAINER_DIFF_SUCCESS](state, { diff, enrichData }) {
const { added, fixed, existing } = parseDiff(diff, enrichData); const { added, fixed, existing } = parseDiff(diff, enrichData);
const baseReportOutofDate = diff.base_report_out_of_date || false;
Vue.set(state.sastContainer, 'isLoading', false); Vue.set(state.sastContainer, 'isLoading', false);
Vue.set(state.sastContainer, 'newIssues', added); Vue.set(state.sastContainer, 'newIssues', added);
Vue.set(state.sastContainer, 'resolvedIssues', fixed); Vue.set(state.sastContainer, 'resolvedIssues', fixed);
Vue.set(state.sastContainer, 'allIssues', existing); Vue.set(state.sastContainer, 'allIssues', existing);
Vue.set(state.sastContainer, 'baseReportOutofDate', baseReportOutofDate);
}, },
[types.RECEIVE_SAST_CONTAINER_DIFF_ERROR](state) { [types.RECEIVE_SAST_CONTAINER_DIFF_ERROR](state) {
...@@ -164,11 +166,13 @@ export default { ...@@ -164,11 +166,13 @@ export default {
[types.RECEIVE_DAST_DIFF_SUCCESS](state, { diff, enrichData }) { [types.RECEIVE_DAST_DIFF_SUCCESS](state, { diff, enrichData }) {
const { added, fixed, existing } = parseDiff(diff, enrichData); const { added, fixed, existing } = parseDiff(diff, enrichData);
const baseReportOutofDate = diff.base_report_out_of_date || false;
Vue.set(state.dast, 'isLoading', false); Vue.set(state.dast, 'isLoading', false);
Vue.set(state.dast, 'newIssues', added); Vue.set(state.dast, 'newIssues', added);
Vue.set(state.dast, 'resolvedIssues', fixed); Vue.set(state.dast, 'resolvedIssues', fixed);
Vue.set(state.dast, 'allIssues', existing); Vue.set(state.dast, 'allIssues', existing);
Vue.set(state.dast, 'baseReportOutofDate', baseReportOutofDate);
}, },
[types.RECEIVE_DAST_DIFF_ERROR](state) { [types.RECEIVE_DAST_DIFF_ERROR](state) {
...@@ -251,11 +255,13 @@ export default { ...@@ -251,11 +255,13 @@ export default {
[types.RECEIVE_DEPENDENCY_SCANNING_DIFF_SUCCESS](state, { diff, enrichData }) { [types.RECEIVE_DEPENDENCY_SCANNING_DIFF_SUCCESS](state, { diff, enrichData }) {
const { added, fixed, existing } = parseDiff(diff, enrichData); const { added, fixed, existing } = parseDiff(diff, enrichData);
const baseReportOutofDate = diff.base_report_out_of_date || false;
Vue.set(state.dependencyScanning, 'isLoading', false); Vue.set(state.dependencyScanning, 'isLoading', false);
Vue.set(state.dependencyScanning, 'newIssues', added); Vue.set(state.dependencyScanning, 'newIssues', added);
Vue.set(state.dependencyScanning, 'resolvedIssues', fixed); Vue.set(state.dependencyScanning, 'resolvedIssues', fixed);
Vue.set(state.dependencyScanning, 'allIssues', existing); Vue.set(state.dependencyScanning, 'allIssues', existing);
Vue.set(state.dependencyScanning, 'baseReportOutofDate', baseReportOutofDate);
}, },
[types.RECEIVE_DEPENDENCY_SCANNING_DIFF_ERROR](state) { [types.RECEIVE_DEPENDENCY_SCANNING_DIFF_ERROR](state) {
......
...@@ -28,6 +28,7 @@ export default () => ({ ...@@ -28,6 +28,7 @@ export default () => ({
newIssues: [], newIssues: [],
resolvedIssues: [], resolvedIssues: [],
baseReportOutofDate: false,
}, },
dast: { dast: {
paths: { paths: {
...@@ -41,6 +42,7 @@ export default () => ({ ...@@ -41,6 +42,7 @@ export default () => ({
newIssues: [], newIssues: [],
resolvedIssues: [], resolvedIssues: [],
baseReportOutofDate: false,
}, },
dependencyScanning: { dependencyScanning: {
...@@ -56,6 +58,7 @@ export default () => ({ ...@@ -56,6 +58,7 @@ export default () => ({
newIssues: [], newIssues: [],
resolvedIssues: [], resolvedIssues: [],
allIssues: [], allIssues: [],
baseReportOutofDate: false,
}, },
modal: { modal: {
......
...@@ -13,6 +13,7 @@ import { ...@@ -13,6 +13,7 @@ import {
dependencyScanningStatusIcon, dependencyScanningStatusIcon,
anyReportHasError, anyReportHasError,
summaryCounts, summaryCounts,
isBaseSecurityReportOutOfDate,
} from 'ee/vue_shared/security_reports/store/getters'; } from 'ee/vue_shared/security_reports/store/getters';
const BASE_PATH = 'fake/base/path.json'; const BASE_PATH = 'fake/base/path.json';
...@@ -530,4 +531,15 @@ describe('Security reports getters', () => { ...@@ -530,4 +531,15 @@ describe('Security reports getters', () => {
expect(noBaseInAllReports(state)).toEqual(false); expect(noBaseInAllReports(state)).toEqual(false);
}); });
}); });
describe('isBaseSecurityReportOutOfDate', () => {
it('returns false when none reports are out of date', () => {
expect(isBaseSecurityReportOutOfDate(state)).toEqual(false);
});
it('returns true when any of the reports is out of date', () => {
state.dast.baseReportOutofDate = true;
expect(isBaseSecurityReportOutOfDate(state)).toEqual(true);
});
});
}); });
...@@ -197,6 +197,7 @@ describe('sast module mutations', () => { ...@@ -197,6 +197,7 @@ describe('sast module mutations', () => {
], ],
fixed: [createIssue({ cve: 'CVE-4' }), createIssue({ cve: 'CVE-5' })], fixed: [createIssue({ cve: 'CVE-4' }), createIssue({ cve: 'CVE-5' })],
existing: [createIssue({ cve: 'CVE-6' })], existing: [createIssue({ cve: 'CVE-6' })],
base_report_out_of_date: true,
}, },
}; };
state.isLoading = true; state.isLoading = true;
...@@ -207,6 +208,10 @@ describe('sast module mutations', () => { ...@@ -207,6 +208,10 @@ describe('sast module mutations', () => {
expect(state.isLoading).toBe(false); expect(state.isLoading).toBe(false);
}); });
it('should set the `baseReportOutofDate` status to `false`', () => {
expect(state.baseReportOutofDate).toBe(true);
});
it('should have the relevant `new` issues', () => { it('should have the relevant `new` issues', () => {
expect(state.newIssues.length).toBe(3); expect(state.newIssues.length).toBe(3);
}); });
......
...@@ -816,6 +816,7 @@ describe('security reports mutations', () => { ...@@ -816,6 +816,7 @@ describe('security reports mutations', () => {
{ name: 'added vuln 2', report_type: 'container_scanning' }, { name: 'added vuln 2', report_type: 'container_scanning' },
], ],
fixed: [{ name: 'fixed vuln 1', report_type: 'container_scanning' }], fixed: [{ name: 'fixed vuln 1', report_type: 'container_scanning' }],
base_report_out_of_date: true,
}, },
}; };
...@@ -827,6 +828,10 @@ describe('security reports mutations', () => { ...@@ -827,6 +828,10 @@ describe('security reports mutations', () => {
expect(stateCopy.sastContainer.isLoading).toBe(false); expect(stateCopy.sastContainer.isLoading).toBe(false);
}); });
it('should set baseReportOutofDate to true', () => {
expect(stateCopy.sastContainer.baseReportOutofDate).toBe(true);
});
it('should parse and set the added vulnerabilities', () => { it('should parse and set the added vulnerabilities', () => {
reports.diff.added.forEach((vuln, i) => { reports.diff.added.forEach((vuln, i) => {
expect(stateCopy.sastContainer.newIssues[i]).toEqual( expect(stateCopy.sastContainer.newIssues[i]).toEqual(
...@@ -885,6 +890,7 @@ describe('security reports mutations', () => { ...@@ -885,6 +890,7 @@ describe('security reports mutations', () => {
], ],
fixed: [{ name: 'fixed vuln 1', report_type: 'dependency_scanning' }], fixed: [{ name: 'fixed vuln 1', report_type: 'dependency_scanning' }],
existing: [{ name: 'existing vuln 1', report_type: 'dependency_scanning' }], existing: [{ name: 'existing vuln 1', report_type: 'dependency_scanning' }],
base_report_out_of_date: true,
}, },
}; };
mutations[types.RECEIVE_DEPENDENCY_SCANNING_DIFF_SUCCESS](stateCopy, reports); mutations[types.RECEIVE_DEPENDENCY_SCANNING_DIFF_SUCCESS](stateCopy, reports);
...@@ -894,6 +900,10 @@ describe('security reports mutations', () => { ...@@ -894,6 +900,10 @@ describe('security reports mutations', () => {
expect(stateCopy.dependencyScanning.isLoading).toBe(false); expect(stateCopy.dependencyScanning.isLoading).toBe(false);
}); });
it('should set baseReportOutofDate to true', () => {
expect(stateCopy.dependencyScanning.baseReportOutofDate).toBe(true);
});
it('should parse and set the added vulnerabilities', () => { it('should parse and set the added vulnerabilities', () => {
reports.diff.added.forEach((vuln, i) => { reports.diff.added.forEach((vuln, i) => {
expect(stateCopy.dependencyScanning.newIssues[i]).toEqual( expect(stateCopy.dependencyScanning.newIssues[i]).toEqual(
...@@ -952,6 +962,7 @@ describe('security reports mutations', () => { ...@@ -952,6 +962,7 @@ describe('security reports mutations', () => {
], ],
fixed: [{ name: 'fixed vuln 1', report_type: 'dast' }], fixed: [{ name: 'fixed vuln 1', report_type: 'dast' }],
existing: [{ name: 'existing vuln 1', report_type: 'dast' }], existing: [{ name: 'existing vuln 1', report_type: 'dast' }],
base_report_out_of_date: true,
}, },
}; };
mutations[types.RECEIVE_DAST_DIFF_SUCCESS](stateCopy, reports); mutations[types.RECEIVE_DAST_DIFF_SUCCESS](stateCopy, reports);
...@@ -961,6 +972,10 @@ describe('security reports mutations', () => { ...@@ -961,6 +972,10 @@ describe('security reports mutations', () => {
expect(stateCopy.dast.isLoading).toBe(false); expect(stateCopy.dast.isLoading).toBe(false);
}); });
it('should set baseReportOutofDate to true', () => {
expect(stateCopy.dast.baseReportOutofDate).toBe(true);
});
it('should parse and set the added vulnerabilities', () => { it('should parse and set the added vulnerabilities', () => {
reports.diff.added.forEach((vuln, i) => { reports.diff.added.forEach((vuln, i) => {
expect(stateCopy.dast.newIssues[i]).toEqual( expect(stateCopy.dast.newIssues[i]).toEqual(
......
...@@ -478,6 +478,7 @@ describe('Grouped security reports app', () => { ...@@ -478,6 +478,7 @@ describe('Grouped security reports app', () => {
mock.onGet(dastEndpoint).reply(200, { mock.onGet(dastEndpoint).reply(200, {
added: [dockerReport.vulnerabilities[0]], added: [dockerReport.vulnerabilities[0]],
fixed: [dockerReport.vulnerabilities[1], dockerReport.vulnerabilities[2]], fixed: [dockerReport.vulnerabilities[1], dockerReport.vulnerabilities[2]],
base_report_out_of_date: true,
}); });
mock.onGet('vulnerability_feedback_path.json').reply(200, []); mock.onGet('vulnerability_feedback_path.json').reply(200, []);
...@@ -527,6 +528,12 @@ describe('Grouped security reports app', () => { ...@@ -527,6 +528,12 @@ describe('Grouped security reports app', () => {
'DAST detected 1 new, and 2 fixed vulnerabilities', 'DAST detected 1 new, and 2 fixed vulnerabilities',
); );
}); });
it('should display out of date message', () => {
expect(wrapper.vm.$el.textContent).toContain(
'Security report is out of date. Retry the pipeline for the target branch',
);
});
}); });
}); });
...@@ -545,6 +552,7 @@ describe('Grouped security reports app', () => { ...@@ -545,6 +552,7 @@ describe('Grouped security reports app', () => {
canCreateIssue: true, canCreateIssue: true,
canCreateMergeRequest: true, canCreateMergeRequest: true,
canDismissVulnerability: true, canDismissVulnerability: true,
targetBranch: 'master',
}; };
const provide = { const provide = {
glFeatures: { glFeatures: {
...@@ -555,11 +563,13 @@ describe('Grouped security reports app', () => { ...@@ -555,11 +563,13 @@ describe('Grouped security reports app', () => {
beforeEach(() => { beforeEach(() => {
gl.mrWidgetData = gl.mrWidgetData || {}; gl.mrWidgetData = gl.mrWidgetData || {};
gl.mrWidgetData.sast_comparison_path = sastEndpoint; gl.mrWidgetData.sast_comparison_path = sastEndpoint;
gl.mrWidgetData.diverged_commits_count = 100;
mock.onGet(sastEndpoint).reply(200, { mock.onGet(sastEndpoint).reply(200, {
added: [dockerReport.vulnerabilities[0]], added: [dockerReport.vulnerabilities[0]],
fixed: [dockerReport.vulnerabilities[1], dockerReport.vulnerabilities[2]], fixed: [dockerReport.vulnerabilities[1], dockerReport.vulnerabilities[2]],
existing: [dockerReport.vulnerabilities[2]], existing: [dockerReport.vulnerabilities[2]],
base_report_out_of_date: true,
}); });
mock.onGet('vulnerability_feedback_path.json').reply(200, []); mock.onGet('vulnerability_feedback_path.json').reply(200, []);
...@@ -609,6 +619,12 @@ describe('Grouped security reports app', () => { ...@@ -609,6 +619,12 @@ describe('Grouped security reports app', () => {
'SAST detected 1 new, and 2 fixed vulnerabilities', 'SAST detected 1 new, and 2 fixed vulnerabilities',
); );
}); });
it('should display out of date message for Outdated MR ', () => {
expect(wrapper.vm.$el.textContent).toContain(
'Security report is out of date. Please incorporate latest changes from master',
);
});
}); });
}); });
}); });
......
...@@ -16100,6 +16100,12 @@ msgstr "" ...@@ -16100,6 +16100,12 @@ msgstr ""
msgid "Security dashboard" msgid "Security dashboard"
msgstr "" msgstr ""
msgid "Security report is out of date. Please incorporate latest changes from %{targetBranchName}"
msgstr ""
msgid "Security report is out of date. Retry the pipeline for the target branch."
msgstr ""
msgid "SecurityConfiguration|Configured" msgid "SecurityConfiguration|Configured"
msgstr "" msgstr ""
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment