Merge branch '451662-validate-yaml' into 'master'

Add validation for fallback_behavior

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/154024



Merged-by: Alexander Turinske <aturinske@gitlab.com>
Approved-by: Sharmad Nachnolkar <snachnolkar@gitlab.com>
Approved-by: Deepika Guliani <dguliani@gitlab.com>
Reviewed-by: Deepika Guliani <dguliani@gitlab.com>

ee/app/assets/javascripts/security_orchestration/components/policy_editor/scan_result/lib/from_yaml.js

@@ -2,6 +2,7 @@ import { safeLoad } from 'js-yaml';
 import { isBoolean, isEqual, uniqBy } from 'lodash';
 import { addIdsToPolicy, hasInvalidKey, isValidPolicy } from '../../utils';
 import { PRIMARY_POLICY_KEYS } from '../../constants';
+import { OPEN, CLOSED } from '../constants';
 import {
   VALID_APPROVAL_SETTINGS,
   PERMITTED_INVALID_SETTINGS,
@@ -55,7 +56,7 @@ export const fromYaml = ({ manifest, validateRuleMode = false }) => {
         'enabled',
       ];
 
-      const { actions, approval_settings: settings = {} } = policy;
+      const { actions, approval_settings: settings = {}, fallback_behavior: fallback } = policy;
 
       // Temporary workaround to allow the rule builder to load with wrongly persisted settings
       const hasInvalidApprovalSettings = hasInvalidKey(settings, [
@@ -71,10 +72,13 @@ export const fromYaml = ({ manifest, validateRuleMode = false }) => {
         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
+        !hasInvalidActions &&
+        !hasInvalidFallbackBehavior
         ? policy
         : { error: true };
     }

ee/spec/frontend/security_orchestration/components/policy_editor/scan_result/lib/from_yaml_spec.js

@@ -16,6 +16,7 @@ import {
   duplicateActionsScanResultManifest,
   zeroActionsScanResultManifest,
   zeroActionsScanResultObject,
+  mockFallbackInvalidScanResultManifest,
 } from 'ee_jest/security_orchestration/mocks/mock_scan_result_policy_data';
 import {
   unsupportedManifest,
@@ -30,15 +31,18 @@ jest.mock('lodash/uniqueId', () => jest.fn((prefix) => `${prefix}0`));
 
 describe('fromYaml', () => {
   it.each`
-    title                                                                                                | input                                                                       | output
-    ${'returns the policy object for a supported manifest without approval_settings'}                    | ${{ manifest: mockDefaultBranchesScanResultManifest }}                      | ${mockDefaultBranchesScanResultObject}
-    ${'returns the policy object for a supported manifest with approval_settings'}                       | ${{ manifest: mockApprovalSettingsScanResultManifest }}                     | ${mockApprovalSettingsScanResultObject}
-    ${'returns the error object for a policy with an unsupported attribute'}                             | ${{ manifest: unsupportedManifest, validateRuleMode: true }}                | ${{ error: true }}
-    ${'returns the error object for a policy with colliding self excluded keys'}                         | ${{ manifest: collidingKeysScanResultManifest, validateRuleMode: true }}    | ${{ error: true }}
-    ${'returns the policy object for a policy without actions'}                                          | ${{ manifest: zeroActionsScanResultManifest, validateRuleMode: true }}      | ${zeroActionsScanResultObject}
-    ${'returns the error object for a policy with more than two actions'}                                | ${{ manifest: tooManyActionsScanResultManifest, validateRuleMode: true }}   | ${{ error: true }}
-    ${'returns the error object for a policy with duplicate action types'}                               | ${{ manifest: duplicateActionsScanResultManifest, validateRuleMode: true }} | ${{ error: true }}
-    ${'returns the policy object for a policy with an unsupported attribute when validation is skipped'} | ${{ manifest: unsupportedManifest }}                                        | ${unsupportedManifestObject}
+    title                                                                                                | input                                                                          | output
+    ${'returns the policy object for a supported manifest without approval_settings'}                    | ${{ manifest: mockDefaultBranchesScanResultManifest }}                         | ${mockDefaultBranchesScanResultObject}
+    ${'returns the policy object for a supported manifest with approval_settings'}                       | ${{ manifest: mockApprovalSettingsScanResultManifest }}                        | ${mockApprovalSettingsScanResultObject}
+    ${'returns the error object for a policy with an unsupported attribute'}                             | ${{ manifest: unsupportedManifest, validateRuleMode: true }}                   | ${{ error: true }}
+    ${'returns the error object for a policy with colliding self excluded keys'}                         | ${{ manifest: collidingKeysScanResultManifest, validateRuleMode: true }}       | ${{ error: true }}
+    ${'returns the policy object for a policy without actions'}                                          | ${{ manifest: zeroActionsScanResultManifest, validateRuleMode: true }}         | ${zeroActionsScanResultObject}
+    ${'returns the error object for a policy with more than two actions'}                                | ${{ manifest: tooManyActionsScanResultManifest, validateRuleMode: true }}      | ${{ error: true }}
+    ${'returns the error object for a policy with duplicate action types'}                               | ${{ manifest: duplicateActionsScanResultManifest, validateRuleMode: true }}    | ${{ error: true }}
+    ${'returns the policy object for a policy with fail: open'}                                          | ${{ manifest: zeroActionsScanResultManifest, validateRuleMode: true }}         | ${zeroActionsScanResultObject}
+    ${'returns the policy object for a policy with fail: closed'}                                        | ${{ manifest: zeroActionsScanResultManifest, validateRuleMode: true }}         | ${zeroActionsScanResultObject}
+    ${'returns the error object for a policy with unsupported fallback behavior'}                        | ${{ manifest: mockFallbackInvalidScanResultManifest, validateRuleMode: true }} | ${{ error: true }}
+    ${'returns the policy object for a policy with an unsupported attribute when validation is skipped'} | ${{ manifest: unsupportedManifest }}                                           | ${unsupportedManifestObject}
   `('$title', ({ input, output }) => {
     expect(fromYaml(input)).toStrictEqual(output);
   });

ee/spec/frontend/security_orchestration/mocks/mock_scan_result_policy_data.js

@@ -30,6 +30,8 @@ actions:
     approvals_required: 1
     user_approvers:
       - the.one
+fallback_behavior:
+  fail: open
 `;
 
 export const mockDefaultBranchesScanResultObject = {
@@ -56,6 +58,9 @@ export const mockDefaultBranchesScanResultObject = {
       id: actionId,
     },
   ],
+  fallback_behavior: {
+    fail: 'open',
+  },
 };
 
 export const mockBotMessageScanResultObject = {
@@ -89,6 +94,8 @@ actions:
     approvals_required: 1
     user_approvers:
       - the.one
+fallback_behavior:
+  fail: open
 `;
 
 export const zeroActionsScanResultManifest = `type: approval_policy
@@ -463,3 +470,6 @@ export const createRequiredApprovers = (count) => {
   }
   return approvers;
 };
+
+export const mockFallbackInvalidScanResultManifest = mockDefaultBranchesScanResultManifest.concat(`fallback_behavior:
+  fail: something_else`);