Merge branch 'network-policy-editor-edit' into 'master'

Network policy editor edit

See merge request gitlab-org/gitlab!40809

ee/app/assets/javascripts/pages/projects/threat_monitoring/edit/index.js

+import initPolicyEditorApp from 'ee/threat_monitoring/policy_editor';
+
+document.addEventListener('DOMContentLoaded', initPolicyEditorApp);

ee/app/assets/javascripts/threat_monitoring/components/network_policy_list.vue

@@ -12,7 +12,7 @@ import {
 } from '@gitlab/ui';
 import { s__ } from '~/locale';
 import { getTimeago } from '~/lib/utils/datetime_utility';
-import { setUrlFragment } from '~/lib/utils/url_utility';
+import { setUrlFragment, mergeUrlParams } from '~/lib/utils/url_utility';
 import EnvironmentPicker from './environment_picker.vue';
 import NetworkPolicyEditor from './network_policy_editor.vue';
 import PolicyDrawer from './policy_editor/policy_drawer.vue';
@@ -74,14 +74,29 @@ export default {
     hasAutoDevopsPolicy() {
       return this.policiesWithDefaults.some(policy => policy.isAutodevops);
     },
+    hasCiliumSelectedPolicy() {
+      return this.hasSelectedPolicy
+        ? this.selectedPolicy.manifest.includes(CiliumNetworkPolicyKind)
+        : false;
+    },
     shouldShowCiliumDrawer() {
-      if (!this.hasSelectedPolicy) return false;
-
+      return this.glFeatures.networkPolicyEditor && this.hasCiliumSelectedPolicy;
+    },
+    shouldShowEditButton() {
       return (
         this.glFeatures.networkPolicyEditor &&
-        this.selectedPolicy.manifest.includes(CiliumNetworkPolicyKind)
+        this.hasCiliumSelectedPolicy &&
+        Boolean(this.selectedPolicy.creationTimestamp)
       );
     },
+    editPolicyPath() {
+      return this.hasSelectedPolicy
+        ? mergeUrlParams(
+            { environment_id: this.currentEnvironmentId },
+            this.newPolicyPath.replace('new', `${this.selectedPolicyName}/edit`),
+          )
+        : '';
+    },
   },
   methods: {
     ...mapActions('networkPolicies', ['createPolicy', 'updatePolicy']),
@@ -230,6 +245,14 @@ export default {
           <h3 class="gl-mb-3">{{ selectedPolicy.name }}</h3>
           <div>
             <gl-button ref="cancelButton" @click="deselectPolicy">{{ __('Cancel') }}</gl-button>
+            <gl-button
+              v-if="shouldShowEditButton"
+              data-testid="edit-button"
+              category="primary"
+              variant="info"
+              :href="editPolicyPath"
+              >{{ s__('NetworkPolicies|Edit policy') }}</gl-button
+            >
             <gl-button
               ref="applyButton"
               category="primary"

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/lib/from_yaml.js

@@ -113,6 +113,7 @@ function parseRule(item, direction) {
 */
 export default function fromYaml(manifest) {
   const { description, metadata, spec } = safeLoad(manifest, { json: true });
+  const { name, resourceVersion } = metadata;
   const { endpointSelector = {}, ingress = [], egress = [] } = spec;
   const matchLabels = endpointSelector.matchLabels || {};
 
@@ -130,7 +131,8 @@ export default function fromYaml(manifest) {
     .filter(rule => Boolean(rule));
 
   return {
-    name: metadata.name,
+    name,
+    resourceVersion,
     description,
     isEnabled: !Object.keys(matchLabels).includes(DisabledByLabel),
     endpointMatchMode: endpointLabels.length > 0 ? EndpointMatchModeLabel : EndpointMatchModeAny,

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/lib/to_yaml.js

@@ -33,7 +33,11 @@ function spec({ rules, isEnabled, endpointMatchMode, endpointLabels }) {
  Return yaml representation of a policy.
 */
 export default function toYaml(policy) {
-  const { name, description } = policy;
+  const { name, resourceVersion, description } = policy;
+  const metadata = { name };
+  if (resourceVersion) {
+    metadata.resourceVersion = resourceVersion;
+  }
 
   const policySpec = {
     apiVersion: 'cilium.io/v2',
@@ -46,7 +50,7 @@ export default function toYaml(policy) {
 
   // We want description at a specific position to have yaml in a common form.
   Object.assign(policySpec, {
-    metadata: { name },
+    metadata,
     spec: spec(policy),
   });
 

ee/app/assets/javascripts/threat_monitoring/components/policy_editor/policy_editor.vue

@@ -49,20 +49,29 @@ export default {
       type: String,
       required: true,
     },
+    existingPolicy: {
+      type: Object,
+      required: false,
+      default: null,
+    },
   },
   data() {
-    return {
-      editorMode: EditorModeRule,
-      yamlEditorValue: '',
-      yamlEditorError: null,
-      policy: {
+    const policy = this.existingPolicy
+      ? fromYaml(this.existingPolicy.manifest)
+      : {
           name: '',
           description: '',
           isEnabled: false,
           endpointMatchMode: EndpointMatchModeAny,
           endpointLabels: '',
           rules: [],
-      },
+        };
+
+    return {
+      editorMode: EditorModeRule,
+      yamlEditorValue: '',
+      yamlEditorError: null,
+      policy,
     };
   },
   computed: {
@@ -83,13 +92,21 @@ export default {
     hasParsingError() {
       return Boolean(this.yamlEditorError);
     },
+    isEditing() {
+      return Boolean(this.existingPolicy);
+    },
+    saveButtonText() {
+      return this.isEditing
+        ? s__('NetworkPolicies|Save changes')
+        : s__('NetworkPolicies|Create policy');
+    },
   },
   created() {
     this.fetchEnvironments();
   },
   methods: {
     ...mapActions('threatMonitoring', ['fetchEnvironments']),
-    ...mapActions('networkPolicies', ['createPolicy']),
+    ...mapActions('networkPolicies', ['createPolicy', 'updatePolicy']),
     addRule() {
       this.policy.rules.push(buildRule(RuleTypeEndpoint));
     },
@@ -121,11 +138,13 @@ export default {
       this.editorMode = mode;
     },
     savePolicy() {
+      const saveFn = this.isEditing ? this.updatePolicy : this.createPolicy;
       const policy = { manifest: toYaml(this.policy) };
-      return this.createPolicy({
-        environmentId: this.currentEnvironmentId,
-        policy,
-      }).then(() => {
+      if (this.isEditing) {
+        policy.name = this.existingPolicy.name;
+      }
+
+      return saveFn({ environmentId: this.currentEnvironmentId, policy }).then(() => {
         if (!this.errorUpdatingPolicy) redirectTo(this.threatMonitoringPath);
       });
     },
@@ -262,9 +281,9 @@ export default {
           type="submit"
           category="primary"
           variant="success"
-          data-testid="create-policy"
+          data-testid="save-policy"
           @click="savePolicy"
-          >{{ s__('NetworkPolicies|Create policy') }}</gl-button
+          >{{ saveButtonText }}</gl-button
         >
         <gl-button category="secondary" variant="default" :href="threatMonitoringPath">{{
           __('Cancel')

ee/app/assets/javascripts/threat_monitoring/policy_editor.js

@@ -4,7 +4,12 @@ import createStore from './store';
 
 export default () => {
   const el = document.querySelector('#js-policy-builder-app');
-  const { environmentsEndpoint, networkPoliciesEndpoint, threatMonitoringPath } = el.dataset;
+  const {
+    environmentsEndpoint,
+    networkPoliciesEndpoint,
+    threatMonitoringPath,
+    policy,
+  } = el.dataset;
 
   const store = createStore();
   store.dispatch('threatMonitoring/setEndpoints', {
@@ -14,13 +19,16 @@ export default () => {
     networkPoliciesEndpoint,
   });
 
+  const props = { threatMonitoringPath };
+  if (policy) {
+    props.existingPolicy = JSON.parse(policy);
+  }
+
   return new Vue({
     el,
     store,
     render(createElement) {
-      return createElement(PolicyEditorApp, {
-        props: { threatMonitoringPath },
-      });
+      return createElement(PolicyEditorApp, { props });
     },
   });
 };

ee/app/helpers/ee/projects_helper.rb

@@ -169,6 +169,7 @@ module EE
         projects/licenses#index
         projects/threat_monitoring#show
         projects/threat_monitoring#new
+        projects/threat_monitoring#edit
       ]
     end
 

ee/spec/frontend/threat_monitoring/components/__snapshots__/network_policy_list_spec.js.snap

@@ -6,10 +6,10 @@ exports[`NetworkPolicyList component renders policies table 1`] = `
 <table
   aria-busy="false"
   aria-colcount="3"
-  aria-describedby="__BVID__334__caption_"
+  aria-describedby="__BVID__529__caption_"
   aria-multiselectable="false"
   class="table b-table gl-table table-hover b-table-stacked-md b-table-selectable b-table-select-single"
-  id="__BVID__334"
+  id="__BVID__529"
   role="table"
 >
   <!---->

ee/spec/frontend/threat_monitoring/components/network_policy_list_spec.js

@@ -29,7 +29,7 @@ describe('NetworkPolicyList component', () => {
     wrapper = mount(NetworkPolicyList, {
       propsData: {
         documentationPath: 'documentation_path',
-        newPolicyPath: 'new_policy_path',
+        newPolicyPath: '/policies/new',
         ...propsData,
       },
       data,
@@ -86,7 +86,31 @@ describe('NetworkPolicyList component', () => {
       expect(wrapper.find(PolicyDrawer).exists()).toBe(false);
     });
 
+    it('does not render edit button', () => {
+      expect(wrapper.find('[data-testid="edit-button"]').exists()).toBe(false);
+    });
+
+    describe('given there is a selected policy', () => {
+      beforeEach(() => {
+        factory({
+          provide: {
+            glFeatures: {
+              networkPolicyEditor: true,
+            },
+          },
+          data: () => ({ selectedPolicyName: 'policy' }),
+        });
+      });
+    });
+
     describe('given selected policy is a cilium policy', () => {
+      const manifest = `apiVersion: cilium.io/v2
+kind: CiliumNetworkPolicy
+metadata:
+  name: policy
+spec:
+  endpointSelector: {}`;
+
       beforeEach(() => {
         factory({
           provide: {
@@ -94,20 +118,13 @@ describe('NetworkPolicyList component', () => {
               networkPolicyEditor: true,
             },
           },
-          data: () => ({
-            selectedPolicyName: 'policy',
-          }),
+          data: () => ({ selectedPolicyName: 'policy' }),
           state: {
             policies: [
               {
                 name: 'policy',
-                isEnabled: false,
-                manifest: `apiVersion: cilium.io/v2
-kind: CiliumNetworkPolicy
-metadata:
-  name: test-policy
-spec:
-  endpointSelector: {}`,
+                creationTimestamp: new Date(),
+                manifest,
               },
             ],
           },
@@ -117,6 +134,12 @@ spec:
       it('renders the new policy drawer', () => {
         expect(wrapper.find(PolicyDrawer).exists()).toBe(true);
       });
+
+      it('renders edit button', () => {
+        const button = wrapper.find('[data-testid="edit-button"]');
+        expect(button.exists()).toBe(true);
+        expect(button.attributes().href).toBe('/policies/policy/edit?environment_id=-1');
+      });
     });
   });
 

ee/spec/frontend/threat_monitoring/components/policy_editor/__snapshots__/policy_editor_spec.js.snap

@@ -217,7 +217,7 @@ spec:
     >
       <gl-button-stub
         category="primary"
-        data-testid="create-policy"
+        data-testid="save-policy"
         icon=""
         size="medium"
         type="submit"

ee/spec/frontend/threat_monitoring/components/policy_editor/lib/from_yaml_spec.js

@@ -52,6 +52,18 @@ describe('fromYaml', () => {
     });
   });
 
+  describe('when resourceVersion is not empty', () => {
+    beforeEach(() => {
+      policy.resourceVersion = '1234';
+    });
+
+    it('returns policy object', () => {
+      expect(fromYaml(toYaml(policy))).toMatchObject({
+        resourceVersion: '1234',
+      });
+    });
+  });
+
   describe('when policy is disabled', () => {
     beforeEach(() => {
       policy.isEnabled = false;

ee/spec/frontend/threat_monitoring/components/policy_editor/lib/to_yaml_spec.js

@@ -40,6 +40,25 @@ spec:
     });
   });
 
+  describe('when resourceVersion is not empty', () => {
+    beforeEach(() => {
+      policy.resourceVersion = '1234';
+    });
+
+    it('returns yaml representation', () => {
+      expect(toYaml(policy)).toEqual(`apiVersion: cilium.io/v2
+kind: CiliumNetworkPolicy
+metadata:
+  name: test-policy
+  resourceVersion: '1234'
+spec:
+  endpointSelector:
+    matchLabels:
+      network-policy.gitlab.com/disabled_by: gitlab
+`);
+    });
+  });
+
   describe('when policy is enabled', () => {
     beforeEach(() => {
       policy.isEnabled = true;

ee/spec/frontend/threat_monitoring/components/policy_editor/policy_editor_spec.js

@@ -13,6 +13,7 @@ import {
 } from 'ee/threat_monitoring/components/policy_editor/constants';
 import fromYaml from 'ee/threat_monitoring/components/policy_editor/lib/from_yaml';
 import toYaml from 'ee/threat_monitoring/components/policy_editor/lib/to_yaml';
+import { buildRule } from 'ee/threat_monitoring/components/policy_editor/lib/rules';
 import { redirectTo } from '~/lib/utils/url_utility';
 
 jest.mock('~/lib/utils/url_utility');
@@ -48,6 +49,8 @@ describe('PolicyEditorApp component', () => {
   const findAddRuleButton = () => wrapper.find('[data-testid="add-rule"]');
   const findYAMLParsingAlert = () => wrapper.find('[data-testid="parsing-alert"]');
   const findNetworkPolicyEditor = () => wrapper.find(NetworkPolicyEditor);
+  const findPolicyName = () => wrapper.find("[id='policyName']");
+  const findSavePolicy = () => wrapper.find("[data-testid='save-policy']");
 
   beforeEach(() => {
     factory();
@@ -126,7 +129,7 @@ spec:
 
     beforeEach(() => {
       initialValue = findPreview().props('policyYaml');
-      wrapper.find("[id='policyName']").vm.$emit('input', 'new');
+      findPolicyName().vm.$emit('input', 'new');
     });
 
     it('updates policy yaml preview', () => {
@@ -169,7 +172,7 @@ spec:
   });
 
   it('updates yaml editor value on switch to yaml editor', async () => {
-    wrapper.find("[id='policyName']").vm.$emit('input', 'test-policy');
+    findPolicyName().vm.$emit('input', 'test-policy');
     wrapper.find("[data-testid='editor-mode']").vm.$emit('input', EditorModeYAML);
     await wrapper.vm.$nextTick();
 
@@ -199,7 +202,7 @@ spec:
   });
 
   it('creates policy and redirects to a threat monitoring path', async () => {
-    wrapper.find("[data-testid='create-policy']").vm.$emit('click');
+    findSavePolicy().vm.$emit('click');
 
     await wrapper.vm.$nextTick();
     expect(store.dispatch).toHaveBeenCalledWith('networkPolicies/createPolicy', {
@@ -219,10 +222,64 @@ spec:
     });
 
     it('it does not redirect', async () => {
-      wrapper.find("[data-testid='create-policy']").vm.$emit('click');
+      findSavePolicy().vm.$emit('click');
 
       await wrapper.vm.$nextTick();
       expect(redirectTo).not.toHaveBeenCalledWith('/threat-monitoring');
     });
   });
+
+  describe('given existingPolicy property was provided', () => {
+    const manifest = toYaml({
+      name: 'policy',
+      endpointLabels: '',
+      rules: [buildRule()],
+    });
+
+    beforeEach(() => {
+      factory({
+        propsData: {
+          existingPolicy: { name: 'policy', manifest },
+        },
+      });
+    });
+
+    it('presents existing policy', () => {
+      expect(findPolicyName().attributes().value).toEqual('policy');
+      expect(wrapper.findAll(PolicyRuleBuilder).length).toEqual(1);
+    });
+
+    it('updates existing policy and redirects to a threat monitoring path', async () => {
+      const saveButton = findSavePolicy();
+      expect(saveButton.text()).toEqual('Save changes');
+      saveButton.vm.$emit('click');
+
+      await wrapper.vm.$nextTick();
+      expect(store.dispatch).toHaveBeenCalledWith('networkPolicies/updatePolicy', {
+        environmentId: -1,
+        policy: { name: 'policy', manifest: toYaml(wrapper.vm.policy) },
+      });
+      expect(redirectTo).toHaveBeenCalledWith('/threat-monitoring');
+    });
+
+    describe('given there is a updatePolicy error', () => {
+      beforeEach(() => {
+        factory({
+          propsData: {
+            existingPolicy: { name: 'policy', manifest },
+          },
+          state: {
+            errorUpdatingPolicy: true,
+          },
+        });
+      });
+
+      it('it does not redirect', async () => {
+        findSavePolicy().vm.$emit('click');
+
+        await wrapper.vm.$nextTick();
+        expect(redirectTo).not.toHaveBeenCalledWith('/threat-monitoring');
+      });
+    });
+  });
 });

ee/spec/helpers/projects_helper_spec.rb

@@ -182,6 +182,7 @@ RSpec.describe ProjectsHelper do
         projects/licenses#index
         projects/threat_monitoring#show
         projects/threat_monitoring#new
+        projects/threat_monitoring#edit
       ]
     end
 

locale/gitlab.pot

@@ -16352,6 +16352,9 @@ msgstr ""
 msgid "NetworkPolicies|Description"
 msgstr ""
 
+msgid "NetworkPolicies|Edit policy"
+msgstr ""
+
 msgid "NetworkPolicies|Editor mode"
 msgstr ""
 
@@ -16430,6 +16433,9 @@ msgstr ""
 msgid "NetworkPolicies|Rules"
 msgstr ""
 
+msgid "NetworkPolicies|Save changes"
+msgstr ""
+
 msgid "NetworkPolicies|Something went wrong, failed to update policy"
 msgstr ""