diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/constants.js b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/constants.js
index 2ea21d9c594a5ff13497e4452d528e01138f768a..a2b07926529a66b1fd962434155e4254acedbb8a 100644
--- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/constants.js
+++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/constants.js
@@ -106,6 +106,7 @@ export const RULE_MODE_SCANNERS = {
};
export const MAX_ALLOWED_RULES_LENGTH = 5;
+export const MAX_ALLOWED_APPROVER_ACTION_LENGTH = 5;
export const PRIMARY_POLICY_KEYS = [
'type',
diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/action/action_section.vue b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/action/action_section.vue
index 57c72f167b495066444e3126983aff22150053d4..bc92d3317067636001a93b3e9e707a4699852a19 100644
--- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/action/action_section.vue
+++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/action/action_section.vue
@@ -1,18 +1,14 @@
<script>
import { GlButton } from '@gitlab/ui';
import { ACTION_AND_LABEL } from '../../constants';
-import { REQUIRE_APPROVAL_TYPE, DISABLED_BOT_MESSAGE_ACTION } from '../lib';
import ApproverAction from './approver_action.vue';
-import BotMessageAction from './bot_message_action.vue';
export default {
ACTION_AND_LABEL,
- DISABLED_BOT_MESSAGE_ACTION,
name: 'ActionSection',
components: {
GlButton,
ApproverAction,
- BotMessageAction,
},
props: {
actionIndex: {
@@ -34,20 +30,13 @@ export default {
},
},
computed: {
- isApproverAction() {
- return this.initAction.type === REQUIRE_APPROVAL_TYPE;
- },
isFirstAction() {
return this.actionIndex === 0;
},
},
methods: {
remove() {
- if (this.isApproverAction) {
- this.$emit('remove');
- } else {
- this.$emit('changed', this.$options.DISABLED_BOT_MESSAGE_ACTION);
- }
+ this.$emit('remove');
},
},
};
@@ -66,7 +55,6 @@ export default {
<div class="gl-flex gl-w-full">
<div class="gl-flex-1">
<approver-action
- v-if="isApproverAction"
:init-action="initAction"
:errors="errors"
:existing-approvers="existingApprovers"
@@ -74,7 +62,6 @@ export default {
@updateApprovers="$emit('updateApprovers', $event)"
@changed="$emit('changed', $event)"
/>
- <bot-message-action v-else :init-action="initAction" />
</div>
<div class="security-policies-bg-subtle">
<gl-button
diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/action/bot_message_action.vue b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/action/bot_message_action.vue
index 448a16c24267b958c3014675fad33bccdd4317b0..2289457b31e4844e5f218d28a024575c59593b89 100644
--- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/action/bot_message_action.vue
+++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/action/bot_message_action.vue
@@ -3,8 +3,10 @@ import { GlIcon, GlLink, GlSprintf } from '@gitlab/ui';
import { helpPagePath } from '~/helpers/help_page_helper';
import { s__ } from '~/locale';
import SectionLayout from '../../section_layout.vue';
+import { DISABLED_BOT_MESSAGE_ACTION } from '../lib';
export default {
+ DISABLED_BOT_MESSAGE_ACTION,
documentationLink: helpPagePath(
'user/application_security/policies/merge_request_approval_policies',
{
@@ -24,11 +26,21 @@ export default {
GlSprintf,
SectionLayout,
},
+ methods: {
+ remove() {
+ this.$emit('changed', this.$options.DISABLED_BOT_MESSAGE_ACTION);
+ },
+ },
};
</script>
<template>
- <section-layout :show-remove-button="false">
+ <section-layout
+ class="gl-pr-0 gl-pt-0"
+ content-classes="gl-pt-5"
+ :show-remove-button="true"
+ @remove="remove"
+ >
<template #content>
<div class="gl-mt-3 gl-flex-col">
<gl-sprintf :message="$options.i18n.contentText">
diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/editor_component.vue b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/editor_component.vue
index 031d31f82d6fcb8310ac9db1b9aff8b36ceb07a9..c608974d13d7a0309c3bd372ff85dd7c7f1e134d 100644
--- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/editor_component.vue
+++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/editor_component.vue
@@ -1,5 +1,5 @@
<script>
-import { isEmpty, uniqBy } from 'lodash';
+import { isEmpty } from 'lodash';
import { GlAlert, GlEmptyState, GlButton } from '@gitlab/ui';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { setUrlFragment } from '~/lib/utils/url_utility';
@@ -17,12 +17,14 @@ import {
ADD_RULE_LABEL,
RULES_LABEL,
MAX_ALLOWED_RULES_LENGTH,
+ MAX_ALLOWED_APPROVER_ACTION_LENGTH,
} from '../constants';
import EditorLayout from '../editor_layout.vue';
import DimDisableContainer from '../dim_disable_container.vue';
import ScanFilterSelector from '../scan_filter_selector.vue';
import SettingsSection from './settings/settings_section.vue';
import ActionSection from './action/action_section.vue';
+import BotCommentAction from './action/bot_message_action.vue';
import RuleSection from './rule/rule_section.vue';
import FallbackSection from './fallback_section.vue';
import { CLOSED } from './constants';
@@ -79,9 +81,16 @@ export default {
settingErrorDescription: s__(
"SecurityOrchestration|This policy doesn't contain any actions or override project approval settings. You cannot create an empty policy.",
),
+ approverActionTooltip: s__(
+ 'SecurityOrchestration|Merge request approval policies allow a maximum of 5 approver actions.',
+ ),
+ botActionTooltip: s__(
+ 'SecurityOrchestration|Merge request approval policies allow a maximum 1 bot message action.',
+ ),
},
components: {
ActionSection,
+ BotCommentAction,
DimDisableContainer,
FallbackSection,
GlAlert,
@@ -122,6 +131,7 @@ export default {
},
mixins: [glFeatureFlagsMixin()],
inject: [
+ 'actionApprovers',
'disableScanPolicyUpdate',
'namespaceId',
'namespacePath',
@@ -167,6 +177,10 @@ export default {
const { policy, hasParsingError } = createPolicyObject(yamlEditorValue);
+ const existingApprovers = isEmpty(this.scanResultPolicyApprovers)
+ ? this.actionApprovers
+ : [this.scanResultPolicyApprovers];
+
return {
errors: { action: [] },
invalidBranches: [],
@@ -178,7 +192,7 @@ export default {
'scan-result-policy-editor',
),
mode: EDITOR_MODE_RULE,
- existingApprovers: this.scanResultPolicyApprovers,
+ existingApprovers,
yamlEditorValue,
};
},
@@ -193,23 +207,26 @@ export default {
return this.errors;
},
- actionsForRuleMode() {
- const actions = this.policy.actions || [];
+ actions() {
+ const actions = this.policy?.actions || [];
+ const hasBotAction = actions.some(({ type }) => type === BOT_MESSAGE_TYPE);
// If the yaml does not have a bot message action, then the bot message will be created as if
// the bot message action exists and is enabled. Thus, we add it into the actions for rule
// mode so the user can remove it
- const policiesWithBotMessageAction = uniqBy(
- [...actions, buildAction(BOT_MESSAGE_TYPE)],
- 'type',
- );
- // If the bot message action is disabled, it should not appear in rule mode
- return policiesWithBotMessageAction.filter(
- (action) => action.type !== BOT_MESSAGE_TYPE || action.enabled,
- );
+ if (hasBotAction) {
+ return actions;
+ }
+
+ return [...actions, buildAction(BOT_MESSAGE_TYPE)];
},
- availableActionListboxItems() {
- const usedActionTypes = this.actionsForRuleMode.map((action) => action.type);
- return ACTION_LISTBOX_ITEMS.filter((item) => !usedActionTypes.includes(item.value));
+ approversActions() {
+ return this.actions
+ .filter(({ type }) => type === REQUIRE_APPROVAL_TYPE)
+ .slice(0, MAX_ALLOWED_APPROVER_ACTION_LENGTH);
+ },
+ botActions() {
+ const botActions = this.actions.filter(({ type }) => type === BOT_MESSAGE_TYPE);
+ return botActions.filter(({ enabled }) => enabled);
},
fallbackBehaviorSetting() {
return this.policy.fallback_behavior?.fail || CLOSED;
@@ -298,42 +315,48 @@ export default {
},
},
methods: {
+ getExistingApprover(index) {
+ return this.existingApprovers[index] || {};
+ },
ruleHasBranchesProperty(rule) {
return BRANCHES_KEY in rule;
},
addAction(type) {
- if (type === BOT_MESSAGE_TYPE && this.hasDisabledBotMessageAction) {
- // If the bot message action is in the yaml and is disabled, then we do not want to add
- // a new bot message action, but instead enable the existing one
- this.updateAction(buildAction(BOT_MESSAGE_TYPE));
+ if (type === REQUIRE_APPROVAL_TYPE) {
+ this.addApproverAction();
} else {
- const newAction = buildAction(type);
- this.policy = {
- ...this.policy,
- actions: this.policy.actions ? [...this.policy.actions, newAction] : [newAction],
- };
- this.updateYamlEditorValue(this.policy);
+ this.addBotAction();
}
},
- removeApproverAction() {
- const { actions, ...newPolicy } = this.policy;
- const updatedActions = actions.filter((action) => action.type !== REQUIRE_APPROVAL_TYPE);
- this.policy = { ...newPolicy, ...(updatedActions.length ? { actions: updatedActions } : {}) };
- this.updateYamlEditorValue(this.policy);
- this.updatePolicyApprovers({});
+ addApproverAction() {
+ const lastApproverActionIndex = this.policy.actions.findLastIndex(
+ ({ type }) => type === REQUIRE_APPROVAL_TYPE,
+ );
+ const nextIndex = lastApproverActionIndex + 1;
+ this.policy.actions.splice(nextIndex, 0, buildAction(REQUIRE_APPROVAL_TYPE));
},
- updateAction(values) {
- const actionType = values.type;
- const actions = this.policy.actions || [];
- const indexOfActionToUpdate = actions.findIndex((a) => a.type === actionType);
-
- if (indexOfActionToUpdate >= 0) {
- actions.splice(indexOfActionToUpdate, 1, values);
+ addBotAction() {
+ if (this.hasDisabledBotMessageAction) {
+ this.updateBotAction(buildAction(BOT_MESSAGE_TYPE));
} else {
- actions.push(values);
+ this.policy.actions.push(buildAction(BOT_MESSAGE_TYPE));
}
+ },
+ removeApproverAction(index) {
+ this.policy.actions?.splice(index, 1);
+ this.updateYamlEditorValue(this.policy);
+ this.updatePolicyApprovers({}, index);
+ },
+ updateAction(values, index) {
+ this.policy.actions.splice(index, 1, values);
+ this.errors.action = [];
+ this.updateYamlEditorValue(this.policy);
+ },
+ updateBotAction(values) {
+ const actions = this.policy.actions || [];
+ const indexOfActionToUpdate = actions.findIndex((a) => a.type === BOT_MESSAGE_TYPE);
- this.policy.actions = actions;
+ actions.splice(indexOfActionToUpdate, 1, values);
this.errors.action = [];
this.updateYamlEditorValue(this.policy);
},
@@ -404,13 +427,14 @@ export default {
}
}
},
- updatePolicyApprovers(values) {
- this.existingApprovers = values;
+ updatePolicyApprovers(values, index) {
+ this.existingApprovers[index] = values;
},
invalidForRuleMode() {
- const { actions, rules } = this.policy;
- const approvalAction = actions?.find((action) => action.type === REQUIRE_APPROVAL_TYPE);
- const invalidApprovers = approversOutOfSync(approvalAction, this.existingApprovers);
+ const { rules } = this.policy;
+ const invalidApprovers = this.existingApprovers.some((existingApprovers, index) =>
+ approversOutOfSync(this.approversActions[index], existingApprovers),
+ );
return (
invalidApprovers ||
@@ -423,6 +447,20 @@ export default {
invalidVulnerabilityAttributes(rules)
);
},
+ shouldDisableActionSelector(filter) {
+ if (filter === BOT_MESSAGE_TYPE) {
+ return this.botActions.length > 0;
+ }
+
+ return this.approversActions.length >= MAX_ALLOWED_APPROVER_ACTION_LENGTH;
+ },
+ customFilterSelectorTooltip(filter) {
+ if (filter.value === BOT_MESSAGE_TYPE) {
+ return this.$options.i18n.botActionTooltip;
+ }
+
+ return this.$options.i18n.approverActionTooltip;
+ },
},
};
</script>
@@ -488,26 +526,35 @@ export default {
</template>
<action-section
- v-for="(action, index) in actionsForRuleMode"
- :key="action.id"
+ v-for="(action, index) in approversActions"
+ :key="`${action.id}_${index}`"
:data-testid="`action-${index}`"
class="gl-mb-4"
:action-index="index"
:init-action="action"
:errors="actionError.action"
- :existing-approvers="existingApprovers"
+ :existing-approvers="getExistingApprover(index)"
@error="handleParsingError"
- @updateApprovers="updatePolicyApprovers"
- @changed="updateAction"
- @remove="removeApproverAction"
+ @updateApprovers="updatePolicyApprovers($event, index)"
+ @changed="updateAction($event, index)"
+ @remove="removeApproverAction(index)"
+ />
+
+ <bot-comment-action
+ v-for="action in botActions"
+ :key="action.id"
+ class="gl-mb-4"
+ :init-action="action"
+ @changed="updateBotAction($event)"
/>
<scan-filter-selector
- v-if="availableActionListboxItems.length"
class="gl-w-full"
:button-text="$options.i18n.ADD_ACTION_LABEL"
:header="$options.i18n.filterHeaderText"
- :filters="availableActionListboxItems"
+ :custom-filter-tooltip="customFilterSelectorTooltip"
+ :should-disable-filter="shouldDisableActionSelector"
+ :filters="$options.ACTION_LISTBOX_ITEMS"
@select="addAction"
/>
</dim-disable-container>
diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/lib/from_yaml.js b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/lib/from_yaml.js
index 3169b66506ead3e2adee7a2d535c25fce8caa1d0..f2d5515acf5750d0a4128d4724c27dfa3949502c 100644
--- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/lib/from_yaml.js
+++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/lib/from_yaml.js
@@ -1,5 +1,5 @@
import { safeLoad } from 'js-yaml';
-import { isBoolean, isEqual, uniqBy } from 'lodash';
+import { isBoolean, isEqual } from 'lodash';
import { addIdsToPolicy, hasInvalidKey, isValidPolicy } from '../../utils';
import { PRIMARY_POLICY_KEYS } from '../../constants';
import { OPEN, CLOSED } from '../constants';
@@ -63,7 +63,7 @@ export const fromYaml = ({ manifest, validateRuleMode = false }) => {
'enabled',
];
- const { actions, approval_settings: settings = {}, fallback_behavior: fallback } = policy;
+ const { approval_settings: settings = {}, fallback_behavior: fallback } = policy;
// Temporary workaround to allow the rule builder to load with wrongly persisted settings
const hasInvalidApprovalSettings = hasInvalidKey(settings, [
@@ -77,16 +77,11 @@ export const fromYaml = ({ manifest, validateRuleMode = false }) => {
)
: false;
- const hasInvalidActions =
- actions?.length > 2 ||
- (actions?.length && actions.length !== uniqBy(actions, 'type').length);
-
const hasInvalidFallbackBehavior = fallback && ![OPEN, CLOSED].includes(fallback.fail);
return isValidPolicy({ policy, primaryKeys, rulesKeys, actionsKeys }) &&
!hasInvalidApprovalSettings &&
!hasInvalidSettingStructure &&
- !hasInvalidActions &&
!hasInvalidFallbackBehavior
? policy
: { error: true };
diff --git a/ee/app/assets/javascripts/security_orchestration/policy_editor.js b/ee/app/assets/javascripts/security_orchestration/policy_editor.js
index cb4fb73b3730cf76b364a9e8fc8ebc30316cb0f4..dbcb266203f6f1ed3a154fe2a190620d5508c755 100644
--- a/ee/app/assets/javascripts/security_orchestration/policy_editor.js
+++ b/ee/app/assets/javascripts/security_orchestration/policy_editor.js
@@ -31,6 +31,7 @@ export default (el, namespaceType) => {
scanResultApprovers,
softwareLicenses,
timezones,
+ actionApprovers,
maxScanExecutionPolicyActions,
} = el.dataset;
@@ -54,6 +55,7 @@ export default (el, namespaceType) => {
}
let scanResultPolicyApprovers;
+ let parsedActionApprovers;
try {
scanResultPolicyApprovers = decomposeApprovers(
@@ -65,6 +67,12 @@ export default (el, namespaceType) => {
scanResultPolicyApprovers = {};
}
+ try {
+ parsedActionApprovers = JSON.parse(actionApprovers);
+ } catch {
+ parsedActionApprovers = [];
+ }
+
try {
parsedTimezones = JSON.parse(timezones);
} catch {
@@ -80,6 +88,7 @@ export default (el, namespaceType) => {
el,
apolloProvider,
provide: {
+ actionApprovers: parsedActionApprovers,
createAgentHelpPath,
disableScanPolicyUpdate: parseBoolean(disableScanPolicyUpdate),
globalGroupApproversEnabled: parseBoolean(globalGroupApproversEnabled),
diff --git a/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/action/action_section_spec.js b/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/action/action_section_spec.js
index a55c841a701066ac74975f030599ac76cc673994..1a3e075d976493a37939c1b3a0c6a87218a6a76f 100644
--- a/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/action/action_section_spec.js
+++ b/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/action/action_section_spec.js
@@ -2,11 +2,7 @@ import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
import ActionSection from 'ee/security_orchestration/components/policy_editor/scan_result/action/action_section.vue';
import ApproverAction from 'ee/security_orchestration/components/policy_editor/scan_result/action/approver_action.vue';
import BotCommentAction from 'ee/security_orchestration/components/policy_editor/scan_result/action/bot_message_action.vue';
-import {
- BOT_MESSAGE_TYPE,
- REQUIRE_APPROVAL_TYPE,
- DISABLED_BOT_MESSAGE_ACTION,
-} from 'ee/security_orchestration/components/policy_editor/scan_result/lib';
+import { REQUIRE_APPROVAL_TYPE } from 'ee/security_orchestration/components/policy_editor/scan_result/lib';
describe('ActionSection', () => {
let wrapper;
@@ -72,21 +68,4 @@ describe('ActionSection', () => {
});
});
});
-
- describe('Bot Comment Action', () => {
- it('renders a bot comment action for that type of action', () => {
- factory({ props: { initAction: { type: BOT_MESSAGE_TYPE } } });
- expect(findBotCommentAction().exists()).toBe(true);
- expect(findApproverAction().exists()).toBe(false);
- expect(findActionSeperator().exists()).toBe(false);
- });
-
- it('emits "changed" on removal', () => {
- factory({ props: { initAction: { type: BOT_MESSAGE_TYPE } } });
- findRemoveButton().vm.$emit('click');
- expect(wrapper.emitted('changed')).toEqual([
- [expect.objectContaining(DISABLED_BOT_MESSAGE_ACTION)],
- ]);
- });
- });
});
diff --git a/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/action/bot_message_action_spec.js b/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/action/bot_message_action_spec.js
index 82ac7a9e9c82ced653f6014be866b5ef508f7688..d7567bfc29a0f8a9fb99aa0c6bd1a891b2fb2992 100644
--- a/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/action/bot_message_action_spec.js
+++ b/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/action/bot_message_action_spec.js
@@ -2,6 +2,7 @@ import { GlLink, GlSprintf } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils';
import SectionLayout from 'ee/security_orchestration/components/policy_editor/section_layout.vue';
import BotCommentAction from 'ee/security_orchestration/components/policy_editor/scan_result/action/bot_message_action.vue';
+import { DISABLED_BOT_MESSAGE_ACTION } from 'ee/security_orchestration/components/policy_editor/scan_result/lib';
describe('BotCommentAction', () => {
let wrapper;
@@ -29,4 +30,11 @@ describe('BotCommentAction', () => {
'/help/user/application_security/policies/merge_request_approval_policies#example-bot-messages',
);
});
+
+ it('emits changed event when action is removed', () => {
+ factory();
+ findSectionLayout().vm.$emit('remove');
+
+ expect(wrapper.emitted('changed')).toEqual([[DISABLED_BOT_MESSAGE_ACTION]]);
+ });
});
diff --git a/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/editor_component_spec.js b/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/editor_component_spec.js
index 1d8f54e54905b94ed548eeb7536520d707b95ffc..998a2dc283c9c1a918064a355f359c944b84eead 100644
--- a/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/editor_component_spec.js
+++ b/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/editor_component_spec.js
@@ -14,6 +14,7 @@ import {
} from 'ee/security_orchestration/components/policy_editor/scan_result/constants';
import ScanFilterSelector from 'ee/security_orchestration/components/policy_editor/scan_filter_selector.vue';
import EditorLayout from 'ee/security_orchestration/components/policy_editor/editor_layout.vue';
+import BotCommentAction from 'ee/security_orchestration/components/policy_editor/scan_result/action/bot_message_action.vue';
import {
ACTION_LISTBOX_ITEMS,
BLOCK_GROUP_BRANCH_MODIFICATION,
@@ -36,6 +37,7 @@ import {
import {
mockDefaultBranchesScanResultManifest,
mockDefaultBranchesScanResultObject,
+ mockDefaultBranchesScanResultObjectWithoutBotAction,
mockDeprecatedScanResultManifest,
mockDeprecatedScanResultObject,
} from 'ee_jest/security_orchestration/mocks/mock_scan_result_policy_data';
@@ -109,6 +111,7 @@ describe('EditorComponent', () => {
...propsData,
},
provide: {
+ actionApprovers: [],
disableScanPolicyUpdate: false,
policyEditorEmptyStateSvgPath,
namespaceId: 1,
@@ -155,6 +158,8 @@ describe('EditorComponent', () => {
const findSettingsSection = () => wrapper.findComponent(SettingsSection);
const findEmptyActionsAlert = () => wrapper.findByTestId('empty-actions-alert');
const findScanFilterSelector = () => wrapper.findComponent(ScanFilterSelector);
+ const findBotCommentAction = () => wrapper.findComponent(BotCommentAction);
+ const findBotCommentActions = () => wrapper.findAllComponents(BotCommentAction);
const verifiesParsingError = () => {
expect(findPolicyEditorLayout().props('hasParsingError')).toBe(true);
@@ -221,8 +226,10 @@ describe('EditorComponent', () => {
expect(findPolicyEditorLayout().props('yamlEditorValue')).toBe(
mockDefaultBranchesScanResultManifest,
);
+
expect(findAllRuleSections()).toHaveLength(1);
- expect(findAllActionSections()).toHaveLength(2);
+ expect(findAllActionSections()).toHaveLength(1);
+ expect(findBotCommentActions()).toHaveLength(1);
});
it('displays a scan result policy', () => {
@@ -232,7 +239,8 @@ describe('EditorComponent', () => {
mockDeprecatedScanResultManifest,
);
expect(findAllRuleSections()).toHaveLength(1);
- expect(findAllActionSections()).toHaveLength(2);
+ expect(findAllActionSections()).toHaveLength(1);
+ expect(findBotCommentActions()).toHaveLength(1);
});
});
});
@@ -327,7 +335,8 @@ describe('EditorComponent', () => {
it('displays the approver action and the add action button on the group-level', () => {
factory({ provide: { namespaceType } });
expect(findActionSection().exists()).toBe(true);
- expect(findAllActionSections()).toHaveLength(2);
+ expect(findAllActionSections()).toHaveLength(1);
+ expect(findBotCommentActions()).toHaveLength(1);
});
});
@@ -344,28 +353,37 @@ describe('EditorComponent', () => {
});
it('displays a bot message action section if there is no bot message action in the policy', () => {
- factoryWithExistingPolicy({ policy: mockDefaultBranchesScanResultObject });
- const actionSections = findAllActionSections();
- expect(actionSections).toHaveLength(2);
- expect(actionSections.at(1).props('initAction')).toEqual(
- expect.objectContaining({ type: BOT_MESSAGE_TYPE, enabled: true }),
- );
+ factoryWithExistingPolicy({
+ policy: mockDefaultBranchesScanResultObjectWithoutBotAction,
+ });
+ expect(findBotCommentActions()).toHaveLength(1);
});
});
});
describe('add', () => {
- it('hides the scan filter selector by default, when all action types are used', () => {
+ it('always display rule selector with approver option, when all bot message is selected', () => {
factoryWithExistingPolicy({ policy: mockDefaultBranchesScanResultObject });
- expect(findScanFilterSelector().exists()).toBe(false);
+ expect(findScanFilterSelector().exists()).toBe(true);
+ expect(
+ findScanFilterSelector().props('customFilterTooltip')({ value: BOT_MESSAGE_TYPE }),
+ ).toBe('Merge request approval policies allow a maximum 1 bot message action.');
+ expect(findScanFilterSelector().props('filters')).toEqual([
+ { value: REQUIRE_APPROVAL_TYPE, text: 'Require Approvers' },
+ { value: BOT_MESSAGE_TYPE, text: 'Send bot message' },
+ ]);
});
it('shows the scan filter selector if there are action types not shown', async () => {
factoryWithExistingPolicy({ policy: mockDefaultBranchesScanResultObject });
await findAllActionSections().at(0).vm.$emit('remove');
expect(findScanFilterSelector().exists()).toBe(true);
+ expect(
+ findScanFilterSelector().props('customFilterTooltip')({ value: REQUIRE_APPROVAL_TYPE }),
+ ).toBe('Merge request approval policies allow a maximum of 5 approver actions.');
expect(findScanFilterSelector().props('filters')).toEqual([
{ text: 'Require Approvers', value: REQUIRE_APPROVAL_TYPE },
+ { text: 'Send bot message', value: BOT_MESSAGE_TYPE },
]);
});
@@ -381,20 +399,59 @@ describe('EditorComponent', () => {
expect.objectContaining(disabledAction),
]);
await findScanFilterSelector().vm.$emit('select', BOT_MESSAGE_TYPE);
- expect(findAllActionSections()).toHaveLength(1);
+ expect(findAllActionSections()).toHaveLength(0);
+ expect(findBotCommentActions()).toHaveLength(1);
const { id, ...action } = buildBotMessageAction();
expect(findPolicyEditorLayout().props('policy').actions).toEqual([
expect.objectContaining(action),
]);
});
+
+ it('adds multiple approval rules', async () => {
+ factoryWithExistingPolicy({ policy: mockDefaultBranchesScanResultObject });
+
+ expect(findAllActionSections()).toHaveLength(1);
+ expect(findBotCommentActions()).toHaveLength(1);
+
+ await findScanFilterSelector().vm.$emit('select', REQUIRE_APPROVAL_TYPE);
+
+ expect(findAllActionSections()).toHaveLength(2);
+ expect(findBotCommentActions()).toHaveLength(1);
+
+ await findScanFilterSelector().vm.$emit('select', REQUIRE_APPROVAL_TYPE);
+
+ expect(findAllActionSections()).toHaveLength(3);
+ expect(findBotCommentActions()).toHaveLength(1);
+ });
+
+ it('add maximum of 5 approval actions', async () => {
+ factoryWithExistingPolicy({ policy: mockDefaultBranchesScanResultObject });
+
+ expect(findAllActionSections()).toHaveLength(1);
+
+ await findScanFilterSelector().vm.$emit('select', REQUIRE_APPROVAL_TYPE);
+ expect(findAllActionSections()).toHaveLength(2);
+
+ await findScanFilterSelector().vm.$emit('select', REQUIRE_APPROVAL_TYPE);
+ expect(findAllActionSections()).toHaveLength(3);
+
+ await findScanFilterSelector().vm.$emit('select', REQUIRE_APPROVAL_TYPE);
+ expect(findAllActionSections()).toHaveLength(4);
+
+ await findScanFilterSelector().vm.$emit('select', REQUIRE_APPROVAL_TYPE);
+ expect(findAllActionSections()).toHaveLength(5);
+
+ await findScanFilterSelector().vm.$emit('select', REQUIRE_APPROVAL_TYPE);
+ expect(findAllActionSections()).toHaveLength(5);
+ });
});
describe('remove', () => {
it('removes the approver action', async () => {
factory();
- expect(findAllActionSections()).toHaveLength(2);
- await findActionSection().vm.$emit('remove');
expect(findAllActionSections()).toHaveLength(1);
+ await findActionSection().vm.$emit('remove');
+ expect(findAllActionSections()).toHaveLength(0);
expect(findPolicyEditorLayout().props('policy').actions).not.toContainEqual(
buildApprovalAction(),
);
@@ -402,9 +459,11 @@ describe('EditorComponent', () => {
it('disables the bot message action', async () => {
factory();
- expect(findAllActionSections()).toHaveLength(2);
- await findActionSection().vm.$emit('changed', DISABLED_BOT_MESSAGE_ACTION);
expect(findAllActionSections()).toHaveLength(1);
+ expect(findBotCommentActions()).toHaveLength(1);
+ await findBotCommentAction().vm.$emit('changed', DISABLED_BOT_MESSAGE_ACTION);
+ expect(findAllActionSections()).toHaveLength(1);
+ expect(findBotCommentActions()).toHaveLength(0);
expect(findPolicyEditorLayout().props('policy').actions).toContainEqual(
DISABLED_BOT_MESSAGE_ACTION,
);
@@ -418,9 +477,10 @@ describe('EditorComponent', () => {
);
await findAllActionSections().at(0).vm.$emit('remove');
await findScanFilterSelector().vm.$emit('select', REQUIRE_APPROVAL_TYPE);
+
expect(removeIdsFromPolicy(findPolicyEditorLayout().props('policy')).actions).toEqual([
- { type: BOT_MESSAGE_TYPE, enabled: true },
{ approvals_required: 1, type: REQUIRE_APPROVAL_TYPE },
+ { type: BOT_MESSAGE_TYPE, enabled: true },
]);
expect(findActionSection().props('existingApprovers')).toEqual({});
});
@@ -428,7 +488,12 @@ describe('EditorComponent', () => {
describe('update', () => {
beforeEach(() => {
- factory();
+ factory({
+ provide: {
+ scanResultPolicyApprovers: {},
+ actionApprovers: [{ role: ['owner'] }],
+ },
+ });
});
it('updates policy action when edited', async () => {
@@ -445,12 +510,9 @@ describe('EditorComponent', () => {
it('updates the policy approvers', async () => {
const newApprover = ['owner'];
- await findActionSection().vm.$emit('updateApprovers', {
- ...scanResultPolicyApprovers,
- role: newApprover,
- });
+ await findActionSection().vm.$emit('updateApprovers', { role: newApprover });
- expect(findActionSection().props('existingApprovers')).toMatchObject({
+ expect(findActionSection().props('existingApprovers')).toEqual({
role: newApprover,
});
});
diff --git a/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/lib/from_yaml_spec.js b/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/lib/from_yaml_spec.js
index 093eee1f5d6854209a8c49bf467b00af01537741..337fed7ee702d360235c84aa4920317c04c9f6fe 100644
--- a/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/lib/from_yaml_spec.js
+++ b/ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/lib/from_yaml_spec.js
@@ -12,12 +12,10 @@ import {
mockGroupApprovalSettingsScanResultObject,
mockApprovalSettingsPermittedInvalidScanResultManifest,
mockApprovalSettingsPermittedInvalidScanResultObject,
- mockPolicyScopeScanResultManifest,
- mockPolicyScopeScanResultObject,
mockProjectFallbackClosedScanResultManifest,
mockProjectFallbackClosedScanResultObject,
- tooManyActionsScanResultManifest,
- duplicateActionsScanResultManifest,
+ multipleApproverActionsScanResultManifest,
+ multipleApproverActionsScanResultObject,
zeroActionsScanResultManifest,
zeroActionsScanResultObject,
mockFallbackInvalidScanResultManifest,
@@ -43,7 +41,7 @@ describe('fromYaml', () => {
${'without actions'} | ${zeroActionsScanResultManifest} | ${zeroActionsScanResultObject}
${'with fail: closed'} | ${mockProjectFallbackClosedScanResultManifest} | ${mockProjectFallbackClosedScanResultObject}
${'with `approval_settings` containing permitted invalid settings'} | ${mockApprovalSettingsPermittedInvalidScanResultManifest} | ${mockApprovalSettingsPermittedInvalidScanResultObject}
- ${'with `policy_scope` by default'} | ${mockPolicyScopeScanResultManifest} | ${mockPolicyScopeScanResultObject}
+ ${'with multiple actions'} | ${multipleApproverActionsScanResultManifest} | ${multipleApproverActionsScanResultObject}
`('returns the policy object for a manifest $title', ({ manifest, output }) => {
expect(fromYaml({ manifest, validateRuleMode: true })).toStrictEqual(output);
});
@@ -53,8 +51,6 @@ describe('fromYaml', () => {
it.each`
title | manifest
${'unsupported fallback behavior'} | ${mockFallbackInvalidScanResultManifest}
- ${'more than two actions'} | ${tooManyActionsScanResultManifest}
- ${'duplicate action types'} | ${duplicateActionsScanResultManifest}
${'an unsupported attribute'} | ${unsupportedManifest}
${'colliding self excluded keys'} | ${collidingKeysScanResultManifest}
`('returns the error object for a policy with $title', ({ manifest }) => {
diff --git a/ee/spec/frontend/security_orchestration/mocks/mock_scan_result_policy_data.js b/ee/spec/frontend/security_orchestration/mocks/mock_scan_result_policy_data.js
index 01769895bc1afac97ae4df528d7cfe208c60a20a..0be655310abcae895344e0ce3ffe9ca540429a1a 100644
--- a/ee/spec/frontend/security_orchestration/mocks/mock_scan_result_policy_data.js
+++ b/ee/spec/frontend/security_orchestration/mocks/mock_scan_result_policy_data.js
@@ -69,6 +69,18 @@ export const mockDefaultBranchesScanResultObject = {
},
};
+export const mockDefaultBranchesScanResultObjectWithoutBotAction = {
+ ...mockDefaultBranchesScanResultObject,
+ actions: [
+ {
+ type: 'require_approval',
+ approvals_required: 1,
+ user_approvers: ['the.one'],
+ id: actionId,
+ },
+ ],
+};
+
export const mockDeprecatedScanResultManifest = `type: scan_result_policy
name: critical vulnerability CS approvals
description: This policy enforces critical vulnerability CS approvals
@@ -126,22 +138,39 @@ export const zeroActionsScanResultObject = {
],
};
-export const tooManyActionsScanResultManifest = zeroActionsScanResultManifest.concat(`
+export const multipleApproverActionsScanResultManifest = zeroActionsScanResultManifest.concat(`
actions:
- type: require_approval
approvals_required: 1
- type: send_bot_message
enabled: true
- - type: other_type
-`);
-
-export const duplicateActionsScanResultManifest = zeroActionsScanResultManifest.concat(`actions:
- - type: require_approval
- approvals_required: 1
- type: require_approval
approvals_required: 1
`);
+export const multipleApproverActionsScanResultObject = {
+ type: 'approval_policy',
+ name: 'critical vulnerability CS approvals',
+ description: 'This policy enforces critical vulnerability CS approvals',
+ enabled: true,
+ rules: [
+ {
+ type: 'scan_finding',
+ branches: [],
+ scanners: ['container_scanning'],
+ vulnerabilities_allowed: 1,
+ severity_levels: ['critical'],
+ vulnerability_states: ['newly_detected'],
+ id: ruleId,
+ },
+ ],
+ actions: [
+ { type: 'require_approval', approvals_required: 1, id: actionId },
+ { type: 'send_bot_message', enabled: true, id: actionId },
+ { type: 'require_approval', approvals_required: 1, id: actionId },
+ ],
+};
+
export const enabledSendBotMessageActionScanResultManifest = zeroActionsScanResultManifest.concat(`
actions:
- type: send_bot_message
diff --git a/ee/spec/frontend_integration/security_orchestration/policy_editor/mocks/mocks.js b/ee/spec/frontend_integration/security_orchestration/policy_editor/mocks/mocks.js
index 482562eae64bd6f55247ccb15d239e58972db140..8ab094fd8568a6990989eeacce0838e4c126d210 100644
--- a/ee/spec/frontend_integration/security_orchestration/policy_editor/mocks/mocks.js
+++ b/ee/spec/frontend_integration/security_orchestration/policy_editor/mocks/mocks.js
@@ -1,6 +1,7 @@
import { NAMESPACE_TYPES } from 'ee/security_orchestration/constants';
export const DEFAULT_PROVIDE = {
+ actionApprovers: [],
disableScanPolicyUpdate: false,
disableSecurityPolicyProject: false,
policyEditorEmptyStateSvgPath: 'path/to/svg',
diff --git a/ee/spec/frontend_integration/security_orchestration/policy_editor/scan_result/actions_spec.js b/ee/spec/frontend_integration/security_orchestration/policy_editor/scan_result/actions_spec.js
index 7805007ff294e232b58a2d2ebea0399044a24b65..939c1aeb311940449a015e77746250835c4eaddc 100644
--- a/ee/spec/frontend_integration/security_orchestration/policy_editor/scan_result/actions_spec.js
+++ b/ee/spec/frontend_integration/security_orchestration/policy_editor/scan_result/actions_spec.js
@@ -49,10 +49,6 @@ describe('Scan result policy actions', () => {
.mockReturnValue(POLICY_TYPE_COMPONENT_OPTIONS.approval.urlParameter);
});
- afterEach(() => {
- window.gon = {};
- });
-
const findApprovalsInput = () => wrapper.findByTestId('approvals-required-input');
const findAvailableTypeListBox = () => wrapper.findByTestId('available-types');
const findApproverAction = () => wrapper.findComponent(ApproverAction);
@@ -121,7 +117,7 @@ describe('Scan result policy actions', () => {
describe('groups', () => {
beforeEach(() => {
- createWrapper({ provide: { existingPolicy: null, namespaceType: 'group' } });
+ createWrapper({ provide: { namespaceType: 'group', actionApprovers: [] } });
});
it('selects group approvers', async () => {
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index d318cf7f47ca3f66800e9cb5ad8475108904a766..ffd449a9f6e9506ea82da5b27cf3479da92cb4ff 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -49802,6 +49802,12 @@ msgstr ""
msgid "SecurityOrchestration|Merge request approval"
msgstr ""
+msgid "SecurityOrchestration|Merge request approval policies allow a maximum 1 bot message action."
+msgstr ""
+
+msgid "SecurityOrchestration|Merge request approval policies allow a maximum of 5 approver actions."
+msgstr ""
+
msgid "SecurityOrchestration|Merge request approval policies can only be created by project owners."
msgstr ""