Commit 0f90ed3e authored by Zamir Martins's avatar Zamir Martins Committed by Andrew Fontaine

Add vulnerabilities_allowed into rules

for Vulnerability-Check this is a UI change
which affect only one specific type of
project approval rule.

EE: true
Changelog: added
parent d6c18665
...@@ -66,6 +66,7 @@ export default { ...@@ -66,6 +66,7 @@ export default {
return { return {
name: this.defaultRuleName, name: this.defaultRuleName,
approvalsRequired: 1, approvalsRequired: 1,
vulnerabilitiesAllowed: 0,
minApprovalsRequired: 0, minApprovalsRequired: 0,
approvers: [], approvers: [],
approversToAdd: [], approversToAdd: [],
...@@ -154,13 +155,23 @@ export default { ...@@ -154,13 +155,23 @@ export default {
return ''; return '';
}, },
invalidVulnerabilitiesAllowedError() {
if (!isNumber(this.vulnerabilitiesAllowed)) {
return this.$options.APPROVAL_DIALOG_I18N.validations.approvalsRequiredNotNumber;
}
if (this.vulnerabilitiesAllowed < 0) {
return this.$options.APPROVAL_DIALOG_I18N.validations.vulnerabilitiesAllowedMinimum;
}
return '';
},
isValid() { isValid() {
return ( return (
this.isValidName && this.isValidName &&
this.isValidBranches && this.isValidBranches &&
this.isValidApprovalsRequired && this.isValidApprovalsRequired &&
this.isValidApprovers && this.isValidApprovers &&
this.areValidScanners this.areValidScanners &&
this.isValidVulnerabilitiesAllowed
); );
}, },
isValidName() { isValidName() {
...@@ -178,6 +189,13 @@ export default { ...@@ -178,6 +189,13 @@ export default {
areValidScanners() { areValidScanners() {
return !this.showValidation || !this.isVulnerabilityCheck || !this.invalidScanners; return !this.showValidation || !this.isVulnerabilityCheck || !this.invalidScanners;
}, },
isValidVulnerabilitiesAllowed() {
return (
!this.showValidation ||
!this.isVulnerabilityCheck ||
!this.invalidVulnerabilitiesAllowedError
);
},
isMultiSubmission() { isMultiSubmission() {
return this.settings.allowMultiRule && !this.isFallbackSubmission; return this.settings.allowMultiRule && !this.isFallbackSubmission;
}, },
...@@ -208,6 +226,7 @@ export default { ...@@ -208,6 +226,7 @@ export default {
id: this.initRule && this.initRule.id, id: this.initRule && this.initRule.id,
name: this.settings.lockedApprovalsRuleName || this.name || DEFAULT_NAME, name: this.settings.lockedApprovalsRuleName || this.name || DEFAULT_NAME,
approvalsRequired: this.approvalsRequired, approvalsRequired: this.approvalsRequired,
vulnerabilitiesAllowed: this.vulnerabilitiesAllowed,
users: this.userIds, users: this.userIds,
groups: this.groupIds, groups: this.groupIds,
userRecords: this.users, userRecords: this.users,
...@@ -357,6 +376,7 @@ export default { ...@@ -357,6 +376,7 @@ export default {
), ),
branches, branches,
scanners: this.initRule.scanners || [], scanners: this.initRule.scanners || [],
vulnerabilitiesAllowed: this.initRule.vulnerabilitiesAllowed || 0,
}; };
}, },
setAllSelectedScanners() { setAllSelectedScanners() {
...@@ -440,6 +460,23 @@ export default { ...@@ -440,6 +460,23 @@ export default {
data-qa-selector="approvals_required_field" data-qa-selector="approvals_required_field"
/> />
</gl-form-group> </gl-form-group>
<gl-form-group
v-if="isVulnerabilityCheck"
:label="$options.APPROVAL_DIALOG_I18N.form.vulnerabilitiesAllowedLabel"
:description="$options.APPROVAL_DIALOG_I18N.form.vulnerabilitiesAllowedDescription"
:state="isValidVulnerabilitiesAllowed"
:invalid-feedback="invalidVulnerabilitiesAllowedError"
data-testid="vulnerability-amount-group"
>
<gl-form-input
v-model.number="vulnerabilitiesAllowed"
:state="isValidVulnerabilitiesAllowed"
min="0"
class="mw-6em"
type="number"
data-testid="vulnerability-amount"
/>
</gl-form-group>
<gl-form-group <gl-form-group
:label="$options.APPROVAL_DIALOG_I18N.form.approversLabel" :label="$options.APPROVAL_DIALOG_I18N.form.approversLabel"
:state="isValidApprovers" :state="isValidApprovers"
......
...@@ -93,6 +93,10 @@ export const APPROVAL_DIALOG_I18N = { ...@@ -93,6 +93,10 @@ export const APPROVAL_DIALOG_I18N = {
selectAllScannersLabel: s__('ApprovalRule|Select All'), selectAllScannersLabel: s__('ApprovalRule|Select All'),
allScannersSelectedLabel: s__('ApprovalRule|All scanners'), allScannersSelectedLabel: s__('ApprovalRule|All scanners'),
multipleSelectedScannersLabel: s__('ApprovalRule|%{scanner} +%{additionalScanners} more'), multipleSelectedScannersLabel: s__('ApprovalRule|%{scanner} +%{additionalScanners} more'),
vulnerabilitiesAllowedLabel: s__('ApprovalRule|Vulnerabilities allowed'),
vulnerabilitiesAllowedDescription: s__(
'ApprovalRule|Number of vulnerabilities allowed before approval rule is triggered.',
),
}, },
validations: { validations: {
approvalsRequiredNegativeNumber: __('Please enter a non-negative number'), approvalsRequiredNegativeNumber: __('Please enter a non-negative number'),
...@@ -105,5 +109,8 @@ export const APPROVAL_DIALOG_I18N = { ...@@ -105,5 +109,8 @@ export const APPROVAL_DIALOG_I18N = {
ruleNameTaken: __('Rule name is already taken.'), ruleNameTaken: __('Rule name is already taken.'),
ruleNameMissing: __('Please provide a name'), ruleNameMissing: __('Please provide a name'),
scannersRequired: s__('ApprovalRule|Please select at least one security scanner'), scannersRequired: s__('ApprovalRule|Please select at least one security scanner'),
vulnerabilitiesAllowedMinimum: s__(
'ApprovalRule|Please enter a number equal or greater than zero',
),
}, },
}; };
...@@ -32,6 +32,7 @@ export const mapApprovalRuleRequest = (req) => ({ ...@@ -32,6 +32,7 @@ export const mapApprovalRuleRequest = (req) => ({
remove_hidden_groups: req.removeHiddenGroups, remove_hidden_groups: req.removeHiddenGroups,
protected_branch_ids: req.protectedBranchIds, protected_branch_ids: req.protectedBranchIds,
scanners: req.scanners, scanners: req.scanners,
vulnerabilities_allowed: req.vulnerabilitiesAllowed,
}); });
export const mapApprovalFallbackRuleRequest = (req) => ({ export const mapApprovalFallbackRuleRequest = (req) => ({
...@@ -52,6 +53,7 @@ export const mapApprovalRuleResponse = (res) => ({ ...@@ -52,6 +53,7 @@ export const mapApprovalRuleResponse = (res) => ({
protectedBranches: res.protected_branches, protectedBranches: res.protected_branches,
overridden: res.overridden, overridden: res.overridden,
scanners: res.scanners, scanners: res.scanners,
vulnerabilitiesAllowed: res.vulnerabilities_allowed,
}); });
export const mapApprovalSettingsResponse = (res) => ({ export const mapApprovalSettingsResponse = (res) => ({
......
...@@ -31,6 +31,13 @@ const TEST_RULE_WITH_PROTECTED_BRANCHES = { ...@@ -31,6 +31,13 @@ const TEST_RULE_WITH_PROTECTED_BRANCHES = {
...TEST_RULE, ...TEST_RULE,
protectedBranches: TEST_PROTECTED_BRANCHES, protectedBranches: TEST_PROTECTED_BRANCHES,
}; };
const TEST_RULE_VULNERABILITY_CHECK = {
...TEST_RULE,
id: null,
name: VULNERABILITY_CHECK_NAME,
scanners: ['sast', 'dast'],
vulnerabilitiesAllowed: 0,
};
const TEST_APPROVERS = [{ id: 7, type: TYPE_USER }]; const TEST_APPROVERS = [{ id: 7, type: TYPE_USER }];
const TEST_APPROVALS_REQUIRED = 3; const TEST_APPROVALS_REQUIRED = 3;
const TEST_FALLBACK_RULE = { const TEST_FALLBACK_RULE = {
...@@ -87,6 +94,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -87,6 +94,7 @@ describe('EE Approvals RuleForm', () => {
const findProtectedBranchesSelector = () => wrapper.findComponent(ProtectedBranchesSelector); const findProtectedBranchesSelector = () => wrapper.findComponent(ProtectedBranchesSelector);
const findBranchesValidation = () => wrapper.findByTestId('branches-group'); const findBranchesValidation = () => wrapper.findByTestId('branches-group');
const findScannersGroup = () => wrapper.findByTestId('scanners-group'); const findScannersGroup = () => wrapper.findByTestId('scanners-group');
const findVulnerabilityFormGroup = () => wrapper.findByTestId('vulnerability-amount-group');
const inputsAreValid = (inputs) => inputs.every((x) => x.props('state')); const inputsAreValid = (inputs) => inputs.every((x) => x.props('state'));
...@@ -185,6 +193,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -185,6 +193,7 @@ describe('EE Approvals RuleForm', () => {
name: 'Lorem', name: 'Lorem',
approvalsRequired: 2, approvalsRequired: 2,
users, users,
vulnerabilitiesAllowed: 0,
groups, groups,
userRecords, userRecords,
groupRecords, groupRecords,
...@@ -263,6 +272,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -263,6 +272,7 @@ describe('EE Approvals RuleForm', () => {
name: 'Lorem', name: 'Lorem',
approvalsRequired: 2, approvalsRequired: 2,
users, users,
vulnerabilitiesAllowed: 0,
groups, groups,
userRecords, userRecords,
groupRecords, groupRecords,
...@@ -342,6 +352,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -342,6 +352,7 @@ describe('EE Approvals RuleForm', () => {
const expected = { const expected = {
...TEST_RULE, ...TEST_RULE,
users, users,
vulnerabilitiesAllowed: 0,
groups, groups,
userRecords, userRecords,
groupRecords, groupRecords,
...@@ -524,14 +535,19 @@ describe('EE Approvals RuleForm', () => { ...@@ -524,14 +535,19 @@ describe('EE Approvals RuleForm', () => {
describe('with approval suggestions', () => { describe('with approval suggestions', () => {
describe.each` describe.each`
defaultRuleName | expectedDisabledAttribute | expectedDisplayedScanners defaultRuleName | expectedDisabledAttribute | expectedDisplayedScanners | expectedDisplayVulnerabilityAllowed
${VULNERABILITY_CHECK_NAME} | ${true} | ${true} ${VULNERABILITY_CHECK_NAME} | ${true} | ${true} | ${true}
${'License-Check'} | ${true} | ${false} ${'License-Check'} | ${true} | ${false} | ${false}
${'Coverage-Check'} | ${true} | ${false} ${'Coverage-Check'} | ${true} | ${false} | ${false}
${'Foo Bar Baz'} | ${false} | ${false} ${'Foo Bar Baz'} | ${false} | ${false} | ${false}
`( `(
'with defaultRuleName set to $defaultRuleName', 'with defaultRuleName set to $defaultRuleName',
({ defaultRuleName, expectedDisabledAttribute, expectedDisplayedScanners }) => { ({
defaultRuleName,
expectedDisabledAttribute,
expectedDisplayedScanners,
expectedDisplayVulnerabilityAllowed,
}) => {
beforeEach(() => { beforeEach(() => {
createComponent({ createComponent({
initRule: null, initRule: null,
...@@ -546,11 +562,15 @@ describe('EE Approvals RuleForm', () => { ...@@ -546,11 +562,15 @@ describe('EE Approvals RuleForm', () => {
expect(findNameInput().props('disabled')).toBe(expectedDisabledAttribute); expect(findNameInput().props('disabled')).toBe(expectedDisabledAttribute);
}); });
it(`it ${ it(`${expectedDisplayedScanners ? 'shows' : 'does not show'} scanners dropdown`, () => {
expectedDisplayedScanners ? 'shows' : 'does not show'
} scanners dropdown`, () => {
expect(findScannersGroup().exists()).toBe(expectedDisplayedScanners); expect(findScannersGroup().exists()).toBe(expectedDisplayedScanners);
}); });
it(`it ${
expectedDisplayVulnerabilityAllowed ? 'shows' : 'does not show'
} the number of vulnerabilities form group`, () => {
expect(findVulnerabilityFormGroup().exists()).toBe(expectedDisplayVulnerabilityAllowed);
});
}, },
); );
}); });
...@@ -586,9 +606,7 @@ describe('EE Approvals RuleForm', () => { ...@@ -586,9 +606,7 @@ describe('EE Approvals RuleForm', () => {
beforeEach(() => { beforeEach(() => {
createComponent({ createComponent({
initRule: { initRule: {
...TEST_RULE, ...TEST_RULE_VULNERABILITY_CHECK,
id: null,
name: VULNERABILITY_CHECK_NAME,
scanners: [], scanners: [],
}, },
}); });
...@@ -601,23 +619,49 @@ describe('EE Approvals RuleForm', () => { ...@@ -601,23 +619,49 @@ describe('EE Approvals RuleForm', () => {
}); });
describe('and with two scanners selected', () => { describe('and with two scanners selected', () => {
const scanners = ['sast', 'dast']; beforeEach(() => {
createComponent({
initRule: TEST_RULE_VULNERABILITY_CHECK,
});
findForm().trigger('submit');
});
it('dispatches the action on submit', () => {
expect(actions.postRule).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ scanners: ['sast', 'dast'] }),
);
});
});
describe('with invalid number of vulnerabilities', () => {
beforeEach(() => { beforeEach(() => {
createComponent({ createComponent({
initRule: { initRule: {
...TEST_RULE, ...TEST_RULE_VULNERABILITY_CHECK,
id: null, vulnerabilitiesAllowed: -1,
name: VULNERABILITY_CHECK_NAME,
scanners,
}, },
}); });
findForm().trigger('submit'); findForm().trigger('submit');
}); });
it('does not dispatch the action on submit', () => {
expect(actions.postRule).not.toHaveBeenCalled();
});
});
describe('with valid number of vulnerabilities', () => {
beforeEach(() => {
createComponent({
initRule: TEST_RULE_VULNERABILITY_CHECK,
});
findForm().trigger('submit');
});
it('dispatches the action on submit', () => { it('dispatches the action on submit', () => {
expect(actions.postRule).toHaveBeenCalledWith( expect(actions.postRule).toHaveBeenCalledWith(
expect.anything(), expect.anything(),
expect.objectContaining({ scanners }), expect.objectContaining({ vulnerabilitiesAllowed: 0 }),
); );
}); });
......
...@@ -4188,6 +4188,12 @@ msgstr "" ...@@ -4188,6 +4188,12 @@ msgstr ""
msgid "ApprovalRule|Name" msgid "ApprovalRule|Name"
msgstr "" msgstr ""
msgid "ApprovalRule|Number of vulnerabilities allowed before approval rule is triggered."
msgstr ""
msgid "ApprovalRule|Please enter a number equal or greater than zero"
msgstr ""
msgid "ApprovalRule|Please select at least one security scanner" msgid "ApprovalRule|Please select at least one security scanner"
msgstr "" msgstr ""
...@@ -4206,6 +4212,9 @@ msgstr "" ...@@ -4206,6 +4212,9 @@ msgstr ""
msgid "ApprovalRule|Target branch" msgid "ApprovalRule|Target branch"
msgstr "" msgstr ""
msgid "ApprovalRule|Vulnerabilities allowed"
msgstr ""
msgid "ApprovalSettings|Merge request approval settings have been updated." msgid "ApprovalSettings|Merge request approval settings have been updated."
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