Merge branch '280595-network-policy-sidebar-updates' into 'master'

Simplify network policy sidebar

See merge request gitlab-org/gitlab!62134

doc/user/application_security/threat_monitoring/index.md

@@ -62,7 +62,7 @@ investigate it for potential threats by
 
 The **Threat Monitoring** page's **Policy** tab displays deployed
 network policies for all available environments. You can check a
-network policy's `yaml` manifest, toggle the policy's enforcement
+network policy's `yaml` manifest, its enforcement
 status, and create and edit deployed policies. This section has the
 following prerequisites:
 
@@ -71,8 +71,7 @@ following prerequisites:
 
 Network policies are fetched directly from the selected environment's
 deployment platform. Changes performed outside of this tab are
-reflected upon refresh. Enforcement status changes are deployed
-directly to a deployment namespace of the selected environment.
+reflected upon refresh.
 
 By default, the network policy list contains predefined policies in a
 disabled state. Once enabled, a predefined policy deploys to the
@@ -89,8 +88,9 @@ users must make changes by following the
 To change a network policy's enforcement status:
 
 - Click the network policy you want to update.
-- Click the **Enforcement status** toggle to update the selected policy.
-- Click the **Apply changes** button to deploy network policy changes.
+- Click the **Edit policy** button.
+- Click the **Policy status** toggle to update the selected policy.
+- Click the **Save changes** button to deploy network policy changes.
 
 Disabled network policies have the `network-policy.gitlab.com/disabled_by: gitlab` selector inside
 the `podSelector` block. This narrows the scope of such a policy and as a result it doesn't affect

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

 <script>
-import {
-  GlTable,
-  GlEmptyState,
-  GlDrawer,
-  GlButton,
-  GlAlert,
-  GlSprintf,
-  GlLink,
-  GlToggle,
-} from '@gitlab/ui';
+import { GlTable, GlEmptyState, GlDrawer, GlButton, GlAlert, GlSprintf, GlLink } from '@gitlab/ui';
 import { mapState, mapActions, mapGetters } from 'vuex';
 import { getTimeago } from '~/lib/utils/datetime_utility';
 import { setUrlFragment, mergeUrlParams } from '~/lib/utils/url_utility';
@@ -19,9 +10,6 @@ import { CiliumNetworkPolicyKind } from './policy_editor/constants';
 import PolicyDrawer from './policy_editor/policy_drawer.vue';
 
 export default {
-  i18n: {
-    enforcementStatus: s__('NetworkPolicies|Enforcement status'),
-  },
   components: {
     GlTable,
     GlEmptyState,
@@ -30,7 +18,6 @@ export default {
     GlAlert,
     GlSprintf,
     GlLink,
-    GlToggle,
     EnvironmentPicker,
     NetworkPolicyEditor: () =>
       import(/* webpackChunkName: 'network_policy_editor' */ './network_policy_editor.vue'),
@@ -50,7 +37,7 @@ export default {
     return { selectedPolicyName: null, initialManifest: null, initialEnforcementStatus: null };
   },
   computed: {
-    ...mapState('networkPolicies', ['policies', 'isLoadingPolicies', 'isUpdatingPolicy']),
+    ...mapState('networkPolicies', ['policies', 'isLoadingPolicies']),
     ...mapState('threatMonitoring', ['currentEnvironmentId', 'allEnvironments']),
     ...mapGetters('networkPolicies', ['policiesWithDefaults']),
     documentationFullPath() {
@@ -64,14 +51,6 @@ export default {
 
       return this.policiesWithDefaults.find((policy) => policy.name === this.selectedPolicyName);
     },
-    hasPolicyChanges() {
-      if (!this.hasSelectedPolicy) return false;
-
-      return (
-        this.selectedPolicy.manifest !== this.initialManifest ||
-        this.selectedPolicy.isEnabled !== this.initialEnforcementStatus
-      );
-    },
     hasAutoDevopsPolicy() {
       return this.policiesWithDefaults.some((policy) => policy.isAutodevops);
     },
@@ -80,12 +59,6 @@ export default {
         ? this.selectedPolicy.manifest.includes(CiliumNetworkPolicyKind)
         : false;
     },
-    shouldShowCiliumDrawer() {
-      return this.hasCiliumSelectedPolicy;
-    },
-    shouldShowEditButton() {
-      return this.hasCiliumSelectedPolicy && Boolean(this.selectedPolicy.creationTimestamp);
-    },
     editPolicyPath() {
       return this.hasSelectedPolicy
         ? mergeUrlParams(
@@ -154,21 +127,6 @@ export default {
       const bTable = this.$refs.policiesTable.$children[0];
       bTable.clearSelected();
     },
-    savePolicy() {
-      const promise = this.selectedPolicy.creationTimestamp ? this.updatePolicy : this.createPolicy;
-      return promise({
-        environmentId: this.currentEnvironmentId,
-        policy: this.selectedPolicy,
-      })
-        .then(() => {
-          this.initialManifest = this.selectedPolicy.manifest;
-          this.initialEnforcementStatus = this.selectedPolicy.isEnabled;
-        })
-        .catch(() => {
-          this.selectedPolicy.manifest = this.initialManifest;
-          this.selectedPolicy.isEnabled = this.initialEnforcementStatus;
-        });
-    },
   },
   emptyStateDescription: s__(
     `NetworkPolicies|Policies are a specification of how groups of pods are allowed to communicate with each other's network endpoints.`,
@@ -259,31 +217,20 @@ export default {
     >
       <template #header>
         <div>
-          <h3 class="gl-mb-3">{{ selectedPolicy.name }}</h3>
+          <h3 class="gl-mb-5 gl-mt-0">{{ 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"
-              variant="success"
-              :loading="isUpdatingPolicy"
-              :disabled="!hasPolicyChanges"
-              @click="savePolicy"
-              >{{ __('Apply changes') }}</gl-button
-            >
           </div>
         </div>
       </template>
       <div v-if="hasSelectedPolicy">
-        <policy-drawer v-if="shouldShowCiliumDrawer" v-model="selectedPolicy.manifest" />
+        <policy-drawer v-if="hasCiliumSelectedPolicy" v-model="selectedPolicy.manifest" />
 
         <div v-else>
           <h5>{{ s__('NetworkPolicies|Policy definition') }}</h5>
@@ -298,14 +245,6 @@ export default {
             />
           </div>
         </div>
-
-        <h5 class="gl-mt-6">{{ $options.i18n.enforcementStatus }}</h5>
-        <p>{{ s__('NetworkPolicies|Choose whether to enforce this policy.') }}</p>
-        <gl-toggle
-          v-model="selectedPolicy.isEnabled"
-          :label="$options.i18n.enforcementStatus"
-          label-position="hidden"
-        />
       </div>
     </gl-drawer>
   </div>

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

 <script>
-import { GlFormTextarea } from '@gitlab/ui';
+import { __ } from '~/locale';
 import fromYaml, { removeUnnecessaryDashes } from './lib/from_yaml';
 import humanizeNetworkPolicy from './lib/humanize';
-import toYaml from './lib/to_yaml';
 import PolicyPreview from './policy_preview.vue';
 
 export default {
   components: {
-    GlFormTextarea,
     PolicyPreview,
   },
   props: {
@@ -18,7 +16,7 @@ export default {
   },
   computed: {
     initialTab() {
-      return this.policy ? 1 : 0;
+      return this.policy ? 0 : 1;
     },
     policy() {
       const policy = fromYaml(this.value);
@@ -30,11 +28,11 @@ export default {
     policyYaml() {
       return removeUnnecessaryDashes(this.value);
     },
-  },
-  methods: {
-    updateManifest(description) {
-      const manifest = toYaml({ ...this.policy, description });
-      this.$emit('input', manifest);
+    enforcementStatusLabel() {
+      if (this.policy) {
+        return this.policy.isEnabled ? __('Enabled') : __('Disabled');
+      }
+      return null;
     },
   },
 };
@@ -42,14 +40,17 @@ export default {
 
 <template>
   <div>
-    <h4>{{ s__('NetworkPolicies|Policy description') }}</h4>
-
-    <h5 class="gl-mt-6">{{ s__('NetworkPolicies|Policy type') }}</h5>
-    <p>{{ s__('NetworkPolicies|Network Policy') }}</p>
+    <h5 class="gl-mt-3">{{ __('Type') }}</h5>
+    <p>{{ s__('NetworkPolicies|Container runtime') }}</p>
 
     <div v-if="policy">
-      <h5 class="gl-mt-6">{{ s__('NetworkPolicies|Description') }}</h5>
-      <gl-form-textarea :value="policy.description" @input="updateManifest" />
+      <template v-if="policy.description">
+        <h5 class="gl-mt-6">{{ s__('NetworkPolicies|Description') }}</h5>
+        <p data-testid="description">{{ policy.description }}</p>
+      </template>
+
+      <h5 class="gl-mt-6">{{ s__('NetworkPolicies|Enforcement status') }}</h5>
+      <p>{{ enforcementStatusLabel }}</p>
     </div>
 
     <policy-preview

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

@@ -39,11 +39,6 @@ export default {
 
 <template>
   <gl-tabs v-model="selectedTab" content-class="gl-pt-0">
-    <gl-tab :title="s__('NetworkPolicies|.yaml')">
-      <pre class="gl-bg-white gl-rounded-top-left-none gl-rounded-top-right-none">{{
-        policyYaml
-      }}</pre>
-    </gl-tab>
     <gl-tab :title="s__('NetworkPolicies|Rule')">
       <div
         v-if="policyDescription"
@@ -56,5 +51,10 @@ export default {
         </gl-alert>
       </div>
     </gl-tab>
+    <gl-tab :title="s__('NetworkPolicies|.yaml')">
+      <pre class="gl-bg-white gl-rounded-top-left-none gl-rounded-top-right-none">{{
+        policyYaml
+      }}</pre>
+    </gl-tab>
   </gl-tabs>
 </template>

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__335__caption_"
+  aria-describedby="__BVID__323__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__335"
+  id="__BVID__323"
   role="table"
 >
   <!---->

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

-import { GlTable, GlToggle } from '@gitlab/ui';
+import { GlTable } from '@gitlab/ui';
 import { mount } from '@vue/test-utils';
 import NetworkPolicyList from 'ee/threat_monitoring/components/network_policy_list.vue';
 import PolicyDrawer from 'ee/threat_monitoring/components/policy_editor/policy_drawer.vue';
-import { PREDEFINED_NETWORK_POLICIES } from 'ee/threat_monitoring/constants';
 import createStore from 'ee/threat_monitoring/store';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import { mockPoliciesResponse } from '../mocks/mock_data';
@@ -41,9 +40,6 @@ describe('NetworkPolicyList component', () => {
   const findTableEmptyState = () => wrapper.find({ ref: 'tableEmptyState' });
   const findEditorDrawer = () => wrapper.find({ ref: 'editorDrawer' });
   const findPolicyEditor = () => wrapper.find({ ref: 'policyEditor' });
-  const findPolicyToggle = () => wrapper.find(GlToggle);
-  const findApplyButton = () => wrapper.find({ ref: 'applyButton' });
-  const findCancelButton = () => wrapper.find({ ref: 'cancelButton' });
   const findAutodevopsAlert = () => wrapper.find('[data-testid="autodevopsAlert"]');
 
   beforeEach(() => {
@@ -174,103 +170,6 @@ spec:
       expect(policyEditor.exists()).toBe(true);
       expect(policyEditor.attributes('value')).toBe(mockData[0].manifest);
     });
-
-    it('renders network policy toggle', () => {
-      const policyToggle = findPolicyToggle();
-      expect(policyToggle.exists()).toBe(true);
-      expect(policyToggle.props()).toMatchObject({
-        label: NetworkPolicyList.i18n.enforcementStatus,
-        value: mockData[0].isEnabled,
-      });
-    });
-
-    it('renders disabled apply button', () => {
-      const applyButton = findApplyButton();
-      expect(applyButton.exists()).toBe(true);
-      expect(applyButton.props('disabled')).toBe(true);
-    });
-
-    it('renders closed editor drawer on Cancel button click', () => {
-      const cancelButton = findCancelButton();
-      expect(cancelButton.exists()).toBe(true);
-      cancelButton.vm.$emit('click');
-
-      return wrapper.vm.$nextTick().then(() => {
-        const editorDrawer = findEditorDrawer();
-        expect(editorDrawer.exists()).toBe(true);
-        expect(editorDrawer.props('open')).toBe(false);
-      });
-    });
-
-    describe('given there is a policy change', () => {
-      beforeEach(() => {
-        findPolicyEditor().vm.$emit('input', 'foo');
-      });
-
-      it('renders enabled apply button', () => {
-        const applyButton = findApplyButton();
-        expect(applyButton.exists()).toBe(true);
-        expect(applyButton.props('disabled')).toBe(false);
-      });
-
-      it('dispatches updatePolicy action on apply button click', () => {
-        findApplyButton().vm.$emit('click');
-
-        expect(store.dispatch).toHaveBeenCalledWith('networkPolicies/updatePolicy', {
-          environmentId: -1,
-          policy: mockData[0],
-        });
-      });
-
-      describe('given there is an updatePolicy error', () => {
-        beforeEach(() => {
-          jest.spyOn(store, 'dispatch').mockRejectedValue();
-        });
-
-        it('reverts isEnabled change', () => {
-          const initial = mockData[0].isEnabled;
-
-          findApplyButton().vm.$emit('click');
-
-          const policyToggle = findPolicyToggle();
-          expect(policyToggle.exists()).toBe(true);
-          expect(policyToggle.props('value')).toBe(initial);
-        });
-      });
-
-      describe('given theres is a predefined policy change', () => {
-        beforeEach(() => {
-          factory({
-            data: () => ({
-              selectedPolicyName: 'drop-outbound',
-              initialManifest: mockData[0].manifest,
-              initialEnforcementStatus: mockData[0].isEnabled,
-            }),
-          });
-        });
-
-        it('dispatches createPolicy action on apply button click', () => {
-          findApplyButton().vm.$emit('click');
-
-          expect(store.dispatch).toHaveBeenCalledWith('networkPolicies/createPolicy', {
-            environmentId: -1,
-            policy: PREDEFINED_NETWORK_POLICIES[0],
-          });
-        });
-      });
-    });
-
-    describe('given there is a policy enforcement status change', () => {
-      beforeEach(() => {
-        findPolicyToggle().vm.$emit('change', false);
-      });
-
-      it('renders enabled apply button', () => {
-        const applyButton = findApplyButton();
-        expect(applyButton.exists()).toBe(true);
-        expect(applyButton.props('disabled')).toBe(false);
-      });
-    });
   });
 
   describe('given there is a default environment with no data to display', () => {

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

@@ -2,18 +2,14 @@
 
 exports[`PolicyDrawer component supported YAML renders policy preview tabs 1`] = `
 <div>
-  <h4>
-    Policy description
-  </h4>
-   
   <h5
-    class="gl-mt-6"
+    class="gl-mt-3"
   >
-    Policy type
+    Type
   </h5>
    
   <p>
-    Network Policy
+    Container runtime
   </p>
    
   <div>
@@ -23,15 +19,26 @@ exports[`PolicyDrawer component supported YAML renders policy preview tabs 1`] =
       Description
     </h5>
      
-    <gl-form-textarea-stub
-      noresize="true"
-      value="test description"
-    />
+    <p
+      data-testid="description"
+    >
+      test description
+    </p>
+     
+    <h5
+      class="gl-mt-6"
+    >
+      Enforcement status
+    </h5>
+     
+    <p>
+      Disabled
+    </p>
   </div>
    
   <policy-preview-stub
     class="gl-mt-4"
-    initialtab="1"
+    initialtab="0"
     policydescription="Deny all traffic"
     policyyaml="apiVersion: cilium.io/v2
 kind: CiliumNetworkPolicy
@@ -49,25 +56,21 @@ spec:
 
 exports[`PolicyDrawer component unsupported YAML renders policy preview tabs 1`] = `
 <div>
-  <h4>
-    Policy description
-  </h4>
-   
   <h5
-    class="gl-mt-6"
+    class="gl-mt-3"
   >
-    Policy type
+    Type
   </h5>
    
   <p>
-    Network Policy
+    Container runtime
   </p>
    
   <!---->
    
   <policy-preview-stub
     class="gl-mt-4"
-    initialtab="0"
+    initialtab="1"
     policyyaml="unsupportedPrimaryKey: test"
   />
 </div>

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

@@ -6,17 +6,6 @@ exports[`PolicyPreview component with policy description renders policy preview
   theme="indigo"
   value="0"
 >
-  <gl-tab-stub
-    title=".yaml"
-    titlelinkclass=""
-  >
-    <pre
-      class="gl-bg-white gl-rounded-top-left-none gl-rounded-top-right-none"
-    >
-      foo
-    </pre>
-  </gl-tab-stub>
-   
   <gl-tab-stub
     title="Rule"
     titlelinkclass=""
@@ -31,5 +20,16 @@ exports[`PolicyPreview component with policy description renders policy preview
       test
     </div>
   </gl-tab-stub>
+   
+  <gl-tab-stub
+    title=".yaml"
+    titlelinkclass=""
+  >
+    <pre
+      class="gl-bg-white gl-rounded-top-left-none gl-rounded-top-right-none"
+    >
+      foo
+    </pre>
+  </gl-tab-stub>
 </gl-tabs-stub>
 `;

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

-import { GlFormTextarea } from '@gitlab/ui';
-import { shallowMount } from '@vue/test-utils';
-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 PolicyDrawer from 'ee/threat_monitoring/components/policy_editor/policy_drawer.vue';
 import PolicyPreview from 'ee/threat_monitoring/components/policy_editor/policy_preview.vue';
+import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
 
 describe('PolicyDrawer component', () => {
   let wrapper;
@@ -16,10 +14,10 @@ describe('PolicyDrawer component', () => {
   const unsupportedYaml = 'unsupportedPrimaryKey: test';
 
   const findPolicyPreview = () => wrapper.findComponent(PolicyPreview);
-  const findTextForm = () => wrapper.findComponent(GlFormTextarea);
+  const findDescription = () => wrapper.findByTestId('description');
 
   const factory = ({ propsData } = {}) => {
-    wrapper = shallowMount(PolicyDrawer, {
+    wrapper = shallowMountExtended(PolicyDrawer, {
       propsData: {
         ...propsData,
       },
@@ -40,26 +38,18 @@ describe('PolicyDrawer component', () => {
     });
 
     it('does render the policy description', () => {
-      expect(findTextForm().exists()).toBe(true);
-      expect(findTextForm().props()).toMatchObject({ value: 'test description' });
+      expect(findDescription().exists()).toBe(true);
+      expect(findDescription().text()).toBe('test description');
     });
 
     it('does render the policy preview', () => {
       expect(findPolicyPreview().exists()).toBe(true);
       expect(findPolicyPreview().props()).toStrictEqual({
-        initialTab: 1,
+        initialTab: 0,
         policyDescription: 'Deny all traffic',
         policyYaml: toYaml(policy),
       });
     });
-
-    it('emits input event on description change', () => {
-      wrapper.find(GlFormTextarea).vm.$emit('input', 'new description');
-
-      expect(wrapper.emitted().input.length).toEqual(1);
-      const updatedPolicy = fromYaml(wrapper.emitted().input[0][0]);
-      expect(updatedPolicy.description).toEqual('new description');
-    });
   });
 
   describe('unsupported YAML', () => {
@@ -72,13 +62,13 @@ describe('PolicyDrawer component', () => {
     });
 
     it('does not render the policy description', () => {
-      expect(findTextForm().exists()).toBe(false);
+      expect(findDescription().exists()).toBe(false);
     });
 
     it('does render the policy preview', () => {
       expect(findPolicyPreview().exists()).toBe(true);
       expect(findPolicyPreview().props()).toStrictEqual({
-        initialTab: 0,
+        initialTab: 1,
         policyDescription: null,
         policyYaml: unsupportedYaml,
       });

locale/gitlab.pot

@@ -4035,9 +4035,6 @@ msgstr ""
 msgid "Apply a template"
 msgstr ""
 
-msgid "Apply changes"
-msgstr ""
-
 msgid "Apply suggestion"
 msgstr ""
 
@@ -21854,7 +21851,7 @@ msgstr ""
 msgid "NetworkPolicies|Are you sure you want to delete this policy? This action cannot be undone."
 msgstr ""
 
-msgid "NetworkPolicies|Choose whether to enforce this policy."
+msgid "NetworkPolicies|Container runtime"
 msgstr ""
 
 msgid "NetworkPolicies|Create policy"