From dd6a2a0e72bff191f76ba26d571fd276ebca5fd0 Mon Sep 17 00:00:00 2001 From: Alexander Turinske <aturinske@gitlab.com> Date: Thu, 11 Apr 2024 12:40:38 -0700 Subject: [PATCH] Add yaml conversion for pipeline execution - add util methods - update constants - update tests --- .../pipeline_execution/constants.js | 4 +- .../pipeline_execution/editor_component.vue | 17 ++++++ .../policy_editor/pipeline_execution/utils.js | 33 +++++++++-- .../editor_component_spec.js | 56 ++++++++++++++----- .../pipeline_execution/mock_data.js | 29 ++++++++++ .../pipeline_execution/utils_spec.js | 22 ++++---- .../mock_pipeline_execution_policy_data.js | 4 -- 7 files changed, 129 insertions(+), 36 deletions(-) create mode 100644 ee/spec/frontend/security_orchestration/components/policy_editor/pipeline_execution/mock_data.js delete mode 100644 ee/spec/frontend/security_orchestration/mocks/mock_pipeline_execution_policy_data.js diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/pipeline_execution/constants.js b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/pipeline_execution/constants.js index 3f6dd443220a2..dbebcb5f43b34 100644 --- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/pipeline_execution/constants.js +++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/pipeline_execution/constants.js @@ -4,8 +4,8 @@ export const DEFAULT_PIPELINE_EXECUTION_POLICY = `type: pipeline_execution_polic name: '' description: '' enabled: true -actions: - - foo: bar +content: + include: '' `; export const ADD_CONDITION_LABEL = s__('ScanExecutionPolicy|Add condition'); diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/pipeline_execution/editor_component.vue b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/pipeline_execution/editor_component.vue index a7e3081f04139..f25bf3f7877a2 100644 --- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/pipeline_execution/editor_component.vue +++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/pipeline_execution/editor_component.vue @@ -88,6 +88,21 @@ export default { changeEditorMode(mode) { this.mode = mode; }, + handleUpdateProperty(property, value) { + this.policy[property] = value; + this.updateYamlEditorValue(this.policy); + }, + handleUpdateYaml(manifest) { + const { policy, hasParsingError } = createPolicyObject(manifest); + + this.yamlEditorValue = manifest; + this.hasParsingError = hasParsingError; + this.parsingError = hasParsingError ? this.$options.i18n.PARSING_ERROR_MESSAGE : ''; + this.policy = policy; + }, + updateYamlEditorValue(policy) { + this.yamlEditorValue = policyToYaml(policy); + }, }, }; </script> @@ -101,6 +116,8 @@ export default { :policy="policy" :yaml-editor-value="yamlEditorValue" @update-editor-mode="changeEditorMode" + @update-property="handleUpdateProperty" + @update-yaml="handleUpdateYaml" > <template #rules> <dim-disable-container :disabled="hasParsingError"> diff --git a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/pipeline_execution/utils.js b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/pipeline_execution/utils.js index a84d661020158..1acff708c4087 100644 --- a/ee/app/assets/javascripts/security_orchestration/components/policy_editor/pipeline_execution/utils.js +++ b/ee/app/assets/javascripts/security_orchestration/components/policy_editor/pipeline_execution/utils.js @@ -1,12 +1,35 @@ import { safeDump, safeLoad } from 'js-yaml'; -import { addIdsToPolicy, removeIdsFromPolicy } from '../utils'; +import { hasInvalidKey } from '../utils'; /* Construct a policy object expected by the policy editor from a yaml manifest. */ -export const fromYaml = ({ manifest }) => { +export const fromYaml = ({ manifest, validateRuleMode = false }) => { try { - const policy = addIdsToPolicy(safeLoad(manifest, { json: true })); + const policy = safeLoad(manifest, { json: true }); + + if (validateRuleMode) { + /** + * These values are what is supported by rule mode. If the yaml has any other values, + * rule mode will be disabled. This validation should not be used to check whether + * the yaml is a valid policy; that should be done on the backend with the official + * schema. These values should not be retrieved from the backend schema because + * the UI for new attributes may not be available. + */ + const primaryKeys = [ + 'type', + 'name', + 'description', + 'enabled', + 'override_project_ci', + 'content', + ]; + const contentKeys = ['workflow', 'include']; + + return !(hasInvalidKey(policy, primaryKeys) || hasInvalidKey(policy.content, contentKeys)) + ? policy + : { error: true }; + } return policy; } catch { @@ -23,7 +46,7 @@ export const fromYaml = ({ manifest }) => { * @returns {Object} security policy object and any errors */ export const createPolicyObject = (manifest) => { - const policy = fromYaml({ manifest }); + const policy = fromYaml({ manifest, validateRuleMode: true }); return { policy, hasParsingError: Boolean(policy.error) }; }; @@ -32,7 +55,7 @@ export const createPolicyObject = (manifest) => { Return yaml representation of a policy. */ export const policyToYaml = (policy) => { - return safeDump(removeIdsFromPolicy(policy)); + return safeDump(policy); }; export const toYaml = (yaml) => { diff --git a/ee/spec/frontend/security_orchestration/components/policy_editor/pipeline_execution/editor_component_spec.js b/ee/spec/frontend/security_orchestration/components/policy_editor/pipeline_execution/editor_component_spec.js index 339d200824570..1f649b3f31c94 100644 --- a/ee/spec/frontend/security_orchestration/components/policy_editor/pipeline_execution/editor_component_spec.js +++ b/ee/spec/frontend/security_orchestration/components/policy_editor/pipeline_execution/editor_component_spec.js @@ -2,11 +2,12 @@ import { GlEmptyState } from '@gitlab/ui'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import { DEFAULT_ASSIGNED_POLICY_PROJECT } from 'ee/security_orchestration/constants'; import EditorComponent from 'ee/security_orchestration/components/policy_editor/pipeline_execution/editor_component.vue'; -import EditorLayout from 'ee/security_orchestration/components/policy_editor/editor_layout.vue'; import RuleSection from 'ee/security_orchestration/components/policy_editor/pipeline_execution/rule/rule_section.vue'; -import ActionSection from 'ee/security_orchestration/components/policy_editor/pipeline_execution/action/action_section.vue'; +import EditorLayout from 'ee/security_orchestration/components/policy_editor/editor_layout.vue'; +import { DEFAULT_PIPELINE_EXECUTION_POLICY } from 'ee/security_orchestration/components/policy_editor/pipeline_execution/constants'; +import { configFileManifest, configFileObject } from './mock_data'; -describe('RuleSection', () => { +describe('EditorComponent', () => { let wrapper; const policyEditorEmptyStateSvgPath = 'path/to/svg'; const scanPolicyDocumentationPath = 'path/to/docs'; @@ -28,26 +29,53 @@ describe('RuleSection', () => { const findEmptyState = () => wrapper.findComponent(GlEmptyState); const findPolicyEditorLayout = () => wrapper.findComponent(EditorLayout); - const findActionSection = () => wrapper.findComponent(ActionSection); const findRuleSection = () => wrapper.findComponent(RuleSection); - it('renders the editor', () => { - factory(); - expect(findPolicyEditorLayout().exists()).toBe(true); - expect(findActionSection().exists()).toBe(true); - expect(findEmptyState().exists()).toBe(false); + describe('rule mode', () => { + it('renders the editor', () => { + factory(); + expect(findEmptyState().exists()).toBe(false); + }); + + it('renders the rule section', () => { + factory(); + expect(findRuleSection().exists()).toBe(true); + }); + + it('renders the default policy editor layout', () => { + factory(); + const editorLayout = findPolicyEditorLayout(); + expect(editorLayout.exists()).toBe(true); + expect(editorLayout.props()).toEqual( + expect.objectContaining({ + yamlEditorValue: DEFAULT_PIPELINE_EXECUTION_POLICY, + }), + ); + }); + + it('updates the policy', async () => { + const name = 'New name'; + factory(); + expect(findPolicyEditorLayout().props('policy').name).toBe(''); + expect(findPolicyEditorLayout().props('yamlEditorValue')).toContain("name: ''"); + await findPolicyEditorLayout().vm.$emit('update-property', 'name', name); + expect(findPolicyEditorLayout().props('policy').name).toBe(name); + expect(findPolicyEditorLayout().props('yamlEditorValue')).toContain(`name: ${name}`); + }); }); - it('renders the rule section', () => { - factory(); - expect(findRuleSection().exists()).toBe(true); + describe('yaml mode', () => { + it('updates the policy', async () => { + factory(); + await findPolicyEditorLayout().vm.$emit('update-yaml', configFileManifest); + expect(findPolicyEditorLayout().props('policy')).toEqual(configFileObject); + expect(findPolicyEditorLayout().props('yamlEditorValue')).toBe(configFileManifest); + }); }); it('renders the empty page', () => { factory({ provide: { disableScanPolicyUpdate: true } }); expect(findPolicyEditorLayout().exists()).toBe(false); - expect(findActionSection().exists()).toBe(false); - expect(findRuleSection().exists()).toBe(false); const emptyState = findEmptyState(); expect(emptyState.exists()).toBe(true); diff --git a/ee/spec/frontend/security_orchestration/components/policy_editor/pipeline_execution/mock_data.js b/ee/spec/frontend/security_orchestration/components/policy_editor/pipeline_execution/mock_data.js new file mode 100644 index 0000000000000..12c7414ec9560 --- /dev/null +++ b/ee/spec/frontend/security_orchestration/components/policy_editor/pipeline_execution/mock_data.js @@ -0,0 +1,29 @@ +import { fromYaml } from 'ee/security_orchestration/components/policy_editor/pipeline_execution/utils'; + +/** + * Naming convention for mocks: + * mock policy yaml => name ends in `Manifest` + * mock parsed yaml => name ends in `Object` + * mock policy for list/drawer => name ends in `PipelinePolicy` + * + * If you have the same policy in multiple forms (e.g. mock yaml and mock parsed yaml that should + * match), please name them similarly (e.g. fooBarManifest and fooBarObject) + * and keep them near each other. + */ + +export const customYaml = `variable: true +`; + +export const customYamlObject = { variable: true }; + +export const configFileManifest = `name: "Ci config file" +description: "triggers all protected branches except main" +enabled: true +override_project_ci: true +content: + include: + project: pipeline-execution-policy/security-policy-project + file: ci.yml +`; + +export const configFileObject = fromYaml({ manifest: configFileManifest }); diff --git a/ee/spec/frontend/security_orchestration/components/policy_editor/pipeline_execution/utils_spec.js b/ee/spec/frontend/security_orchestration/components/policy_editor/pipeline_execution/utils_spec.js index f5db2ab9409b5..c24d8e4fca227 100644 --- a/ee/spec/frontend/security_orchestration/components/policy_editor/pipeline_execution/utils_spec.js +++ b/ee/spec/frontend/security_orchestration/components/policy_editor/pipeline_execution/utils_spec.js @@ -1,30 +1,30 @@ +import { DEFAULT_PIPELINE_EXECUTION_POLICY } from 'ee/security_orchestration/components/policy_editor/pipeline_execution/constants'; import { createPolicyObject, fromYaml, policyToYaml, toYaml, } from 'ee/security_orchestration/components/policy_editor/pipeline_execution/utils'; -import { - customYaml, - customYamlObject, -} from 'ee_jest/security_orchestration/mocks/mock_pipeline_execution_policy_data'; +import { customYaml, customYamlObject } from './mock_data'; describe('fromYaml', () => { it.each` - title | input | output | features - ${'returns the policy object for a supported manifest'} | ${{ manifest: customYaml }} | ${customYamlObject} | ${{}} - `('$title', ({ input, output, features }) => { - window.gon = { features }; + title | input | output + ${'returns the policy object for a supported manifest'} | ${{ manifest: DEFAULT_PIPELINE_EXECUTION_POLICY }} | ${fromYaml({ manifest: DEFAULT_PIPELINE_EXECUTION_POLICY })} + ${'returns the policy object for a policy with an unsupported attribute without validation'} | ${{ manifest: customYaml }} | ${customYamlObject} + ${'returns the error object for a policy with an unsupported attribute with validation'} | ${{ manifest: customYaml, validateRuleMode: true }} | ${{ error: true }} + `('$title', ({ input, output }) => { expect(fromYaml(input)).toStrictEqual(output); }); }); describe('createPolicyObject', () => { it.each` - title | input | output - ${'returns the policy object and no errors for a supported manifest'} | ${[customYaml]} | ${{ policy: customYamlObject, hasParsingError: false }} + title | input | output + ${'returns the policy object and no errors for a supported manifest'} | ${DEFAULT_PIPELINE_EXECUTION_POLICY} | ${{ policy: fromYaml({ manifest: DEFAULT_PIPELINE_EXECUTION_POLICY }), hasParsingError: false }} + ${'returns the error policy object and the error for an unsupported manifest'} | ${customYaml} | ${{ policy: { error: true }, hasParsingError: true }} `('$title', ({ input, output }) => { - expect(createPolicyObject(...input)).toStrictEqual(output); + expect(createPolicyObject(input)).toStrictEqual(output); }); }); diff --git a/ee/spec/frontend/security_orchestration/mocks/mock_pipeline_execution_policy_data.js b/ee/spec/frontend/security_orchestration/mocks/mock_pipeline_execution_policy_data.js deleted file mode 100644 index 8df22d53f6ccb..0000000000000 --- a/ee/spec/frontend/security_orchestration/mocks/mock_pipeline_execution_policy_data.js +++ /dev/null @@ -1,4 +0,0 @@ -export const customYaml = `variable: true -`; - -export const customYamlObject = { variable: true }; -- GitLab