Merge branch 'replace_checkbox_by_toggle_for_modsecurity' into 'master'

Replace checkbox by toggle for modsecurity setup

See merge request gitlab-org/gitlab!26720

app/assets/javascripts/clusters/clusters_bundle.js

@@ -256,6 +256,7 @@ export default class Clusters {
     eventHub.$on('uninstallApplication', data => this.uninstallApplication(data));
     eventHub.$on('setCrossplaneProviderStack', data => this.setCrossplaneProviderStack(data));
     eventHub.$on('setIngressModSecurityEnabled', data => this.setIngressModSecurityEnabled(data));
+    eventHub.$on('resetIngressModSecurityEnabled', id => this.resetIngressModSecurityEnabled(id));
     // Add event listener to all the banner close buttons
     this.addBannerCloseHandler(this.unreachableContainer, 'unreachable');
     this.addBannerCloseHandler(this.authenticationFailureContainer, 'authentication_failure');
@@ -270,6 +271,7 @@ export default class Clusters {
     eventHub.$off('setCrossplaneProviderStack');
     eventHub.$off('uninstallApplication');
     eventHub.$off('setIngressModSecurityEnabled');
+    eventHub.$off('resetIngressModSecurityEnabled');
   }
 
   initPolling(method, successCallback, errorCallback) {
@@ -523,6 +525,10 @@ export default class Clusters {
     this.store.updateAppProperty(id, 'modsecurity_enabled', modSecurityEnabled);
   }
 
+  resetIngressModSecurityEnabled(id) {
+    this.store.updateAppProperty(id, 'isEditingModSecurityEnabled', false);
+  }
+
   destroy() {
     this.destroyed = true;
 

app/assets/javascripts/clusters/components/applications.vue

@@ -119,9 +119,6 @@ export default {
     ingressInstalled() {
       return this.applications.ingress.status === APPLICATION_STATUS.INSTALLED;
     },
-    ingressEnableModsecurity() {
-      return this.applications.ingress.modsecurity_enabled;
-    },
     ingressExternalEndpoint() {
       return this.applications.ingress.externalIp || this.applications.ingress.externalHostname;
     },

app/assets/javascripts/clusters/components/ingress_modsecurity_settings.vue

 <script>
 import _ from 'lodash';
 import { __ } from '../../locale';
-import LoadingButton from '~/vue_shared/components/loading_button.vue';
 import { APPLICATION_STATUS, INGRESS } from '~/clusters/constants';
-import { GlAlert, GlSprintf, GlLink } from '@gitlab/ui';
+import { GlAlert, GlSprintf, GlLink, GlToggle, GlButton } from '@gitlab/ui';
 import eventHub from '~/clusters/event_hub';
+import modSecurityLogo from 'images/cluster_app_logos/modsecurity.png';
 
-const { UPDATING, UNINSTALLING } = APPLICATION_STATUS;
+const { UPDATING, UNINSTALLING, INSTALLING, INSTALLED, UPDATED } = APPLICATION_STATUS;
 
 export default {
+  title: 'ModSecurity Web Application Firewall',
+  modsecurityUrl: 'https://modsecurity.org/about.html',
   components: {
-    LoadingButton,
     GlAlert,
     GlSprintf,
     GlLink,
+    GlToggle,
+    GlButton,
   },
   props: {
     ingress: {
@@ -26,6 +29,10 @@ export default {
       default: '',
     },
   },
+  data: () => ({
+    modSecurityLogo,
+    hasValueChanged: false,
+  }),
   computed: {
     modSecurityEnabled: {
       get() {
@@ -36,6 +43,11 @@ export default {
           id: INGRESS,
           modSecurityEnabled: isEnabled,
         });
+        if (this.hasValueChanged) {
+          this.resetStatus();
+        } else {
+          this.hasValueChanged = true;
+        }
       },
     },
     ingressModSecurityDescription() {
@@ -45,13 +57,21 @@ export default {
       return [UPDATING].includes(this.ingress.status);
     },
     saveButtonDisabled() {
-      return [UNINSTALLING, UPDATING].includes(this.ingress.status);
+      return [UNINSTALLING, UPDATING, INSTALLING].includes(this.ingress.status);
     },
     saveButtonLabel() {
       return this.saving ? __('Saving') : __('Save changes');
     },
-    ingressInstalled() {
-      return this.ingress.installed;
+    /**
+     * Returns true either when:
+     *   - The application is getting updated.
+     *   - The user has changed some of the settings for an application which is
+     *     neither getting installed nor updated.
+     */
+    showButtons() {
+      return (
+        this.saving || (this.hasValueChanged && [INSTALLED, UPDATED].includes(this.ingress.status))
+      );
     },
   },
   methods: {
@@ -60,6 +80,11 @@ export default {
         id: INGRESS,
         params: { modsecurity_enabled: this.ingress.modsecurity_enabled },
       });
+      this.resetStatus();
+    },
+    resetStatus() {
+      eventHub.$emit('resetIngressModSecurityEnabled', INGRESS);
+      this.hasValueChanged = false;
     },
   },
 };
@@ -75,42 +100,65 @@ export default {
       @dismiss="alert = null"
     >
       {{
-        s__('ClusterIntegration|Something went wrong while updating the Web Application Firewall.')
+        s__(
+          'ClusterIntegration|Something went wrong while trying to save your settings. Please try again.',
+        )
       }}
     </gl-alert>
-    <div class="form-group">
-      <div class="form-check form-check-inline">
-        <input
-          v-model="modSecurityEnabled"
-          type="checkbox"
-          autocomplete="off"
-          class="form-check-input"
+    <div class="gl-responsive-table-row-layout" role="row">
+      <div class="table-section append-right-8 section-align-top" role="gridcell">
+        <img
+          :src="modSecurityLogo"
+          :alt="`${$options.title} logo`"
+          class="cluster-application-logo avatar s40"
         />
-        <label class="form-check-label label-bold" for="ingress-enable-modsecurity">
-          {{ s__('ClusterIntegration|Enable Web Application Firewall') }}
-        </label>
       </div>
-      <p class="form-text text-muted">
+      <div class="table-section section-wrap" role="gridcell">
         <strong>
-          <gl-sprintf
-            :message="s__('ClusterIntegration|Learn more about %{linkStart}ModSecurity%{linkEnd}')"
-          >
-            <template #link="{ content }">
-              <gl-link :href="ingressModSecurityDescription" target="_blank"
-                >{{ content }}
-              </gl-link>
-            </template>
-          </gl-sprintf>
+          <gl-link :href="$options.modsecurityUrl" target="_blank">{{ $options.title }} </gl-link>
         </strong>
-      </p>
-      <loading-button
-        v-if="ingressInstalled"
-        class="btn-success mt-1"
-        :loading="saving"
-        :disabled="saveButtonDisabled"
-        :label="saveButtonLabel"
-        @click="updateApplication"
-      />
+        <div class="form-group">
+          <p class="form-text text-muted">
+            <strong>
+              <gl-sprintf
+                :message="
+                  s__(
+                    'ClusterIntegration|Real-time web application monitoring, logging and access control. %{linkStart}More information%{linkEnd}',
+                  )
+                "
+              >
+                <template #link="{ content }">
+                  <gl-link :href="ingressModSecurityDescription" target="_blank"
+                    >{{ content }}
+                  </gl-link>
+                </template>
+              </gl-sprintf>
+            </strong>
+          </p>
+          <div class="form-check form-check-inline mt-3">
+            <gl-toggle
+              v-model="modSecurityEnabled"
+              :label-on="__('Enabled')"
+              :label-off="__('Disabled')"
+              :disabled="saveButtonDisabled"
+              label-position="right"
+            />
+          </div>
+          <div v-if="showButtons">
+            <gl-button
+              class="btn-success inline mr-1"
+              :loading="saving"
+              :disabled="saveButtonDisabled"
+              @click="updateApplication"
+            >
+              {{ saveButtonLabel }}
+            </gl-button>
+            <gl-button :disabled="saveButtonDisabled" @click="resetStatus">
+              {{ __('Cancel') }}
+            </gl-button>
+          </div>
+        </div>
+      </div>
     </div>
   </div>
 </template>

app/assets/javascripts/clusters/stores/clusters_store.js

@@ -211,9 +211,7 @@ export default class ClusterStore {
         this.state.applications.ingress.externalIp = serverAppEntry.external_ip;
         this.state.applications.ingress.externalHostname = serverAppEntry.external_hostname;
         if (!this.state.applications.ingress.isEditingModSecurityEnabled) {
-          this.state.applications.ingress.modsecurity_enabled =
-            serverAppEntry.modsecurity_enabled ||
-            this.state.applications.ingress.modsecurity_enabled;
+          this.state.applications.ingress.modsecurity_enabled = serverAppEntry.modsecurity_enabled;
         }
       } else if (appId === CERT_MANAGER) {
         this.state.applications.cert_manager.email =

app/models/clusters/applications/ingress.rb

@@ -16,7 +16,7 @@ module Clusters
       include AfterCommitQueue
 
       default_value_for :ingress_type, :nginx
-      default_value_for :modsecurity_enabled, false
+      default_value_for :modsecurity_enabled, true
       default_value_for :version, VERSION
 
       enum ingress_type: {

changelogs/unreleased/replace_checkbox_by_toggle_for_modsecurity.yml

+---
+title: Replace checkbox by toggle for ModSecurity on Cluster App Page
+merge_request: 26720
+author:
+type: changed

locale/gitlab.pot

@@ -4249,9 +4249,6 @@ msgstr ""
 msgid "ClusterIntegration|Enable Cloud Run for Anthos"
 msgstr ""
 
-msgid "ClusterIntegration|Enable Web Application Firewall"
-msgstr ""
-
 msgid "ClusterIntegration|Enable or disable GitLab's connection to your Kubernetes cluster."
 msgstr ""
 
@@ -4429,9 +4426,6 @@ msgstr ""
 msgid "ClusterIntegration|Learn more about %{help_link_start}zones%{help_link_end}."
 msgstr ""
 
-msgid "ClusterIntegration|Learn more about %{linkStart}ModSecurity%{linkEnd}"
-msgstr ""
-
 msgid "ClusterIntegration|Learn more about %{startLink}Regions %{externalLinkIcon}%{endLink}."
 msgstr ""
 
@@ -4564,6 +4558,9 @@ msgstr ""
 msgid "ClusterIntegration|Read our %{link_start}help page%{link_end} on Kubernetes cluster integration."
 msgstr ""
 
+msgid "ClusterIntegration|Real-time web application monitoring, logging and access control. %{linkStart}More information%{linkEnd}"
+msgstr ""
+
 msgid "ClusterIntegration|Region"
 msgstr ""
 
@@ -4711,13 +4708,13 @@ msgstr ""
 msgid "ClusterIntegration|Something went wrong while installing %{title}"
 msgstr ""
 
-msgid "ClusterIntegration|Something went wrong while uninstalling %{title}"
+msgid "ClusterIntegration|Something went wrong while trying to save your settings. Please try again."
 msgstr ""
 
-msgid "ClusterIntegration|Something went wrong while updating Knative domain name."
+msgid "ClusterIntegration|Something went wrong while uninstalling %{title}"
 msgstr ""
 
-msgid "ClusterIntegration|Something went wrong while updating the Web Application Firewall."
+msgid "ClusterIntegration|Something went wrong while updating Knative domain name."
 msgstr ""
 
 msgid "ClusterIntegration|Specifying a domain will allow you to use Auto Review Apps and Auto Deploy stages for %{auto_devops_start}Auto DevOps%{auto_devops_end}. The domain should have a wildcard DNS configured matching the domain."

spec/frontend/clusters/components/ingress_modsecurity_settings_spec.js

 import { shallowMount } from '@vue/test-utils';
 import IngressModsecuritySettings from '~/clusters/components/ingress_modsecurity_settings.vue';
-import LoadingButton from '~/vue_shared/components/loading_button.vue';
 import { APPLICATION_STATUS, INGRESS } from '~/clusters/constants';
-import { GlAlert } from '@gitlab/ui';
+import { GlAlert, GlToggle } from '@gitlab/ui';
 import eventHub from '~/clusters/event_hub';
 
 const { UPDATING } = APPLICATION_STATUS;
@@ -27,32 +26,55 @@ describe('IngressModsecuritySettings', () => {
     });
   };
 
-  const findSaveButton = () => wrapper.find(LoadingButton);
-  const findModSecurityCheckbox = () => wrapper.find('input').element;
+  const findSaveButton = () => wrapper.find('.btn-success');
+  const findCancelButton = () => wrapper.find('[variant="secondary"]');
+  const findModSecurityToggle = () => wrapper.find(GlToggle);
 
   describe('when ingress is installed', () => {
     beforeEach(() => {
-      createComponent({ installed: true });
+      createComponent({ installed: true, status: 'installed' });
       jest.spyOn(eventHub, '$emit');
     });
 
-    it('renders save button', () => {
-      expect(findSaveButton().exists()).toBe(true);
-      expect(findModSecurityCheckbox().checked).toBe(false);
+    it('does not render save and cancel buttons', () => {
+      expect(findSaveButton().exists()).toBe(false);
+      expect(findCancelButton().exists()).toBe(false);
     });
 
-    describe('and the save changes button is clicked', () => {
+    describe('with toggle changed by the user', () => {
       beforeEach(() => {
-        findSaveButton().vm.$emit('click');
+        findModSecurityToggle().vm.$emit('change');
+      });
+
+      it('renders both save and cancel buttons', () => {
+        expect(findSaveButton().exists()).toBe(true);
+        expect(findCancelButton().exists()).toBe(true);
       });
 
-      it('triggers save event and pass current modsecurity value', () =>
-        wrapper.vm.$nextTick().then(() => {
+      describe('and the save changes button is clicked', () => {
+        beforeEach(() => {
+          findSaveButton().vm.$emit('click');
+        });
+
+        it('triggers save event and pass current modsecurity value', () => {
           expect(eventHub.$emit).toHaveBeenCalledWith('updateApplication', {
             id: INGRESS,
             params: { modsecurity_enabled: false },
           });
-        }));
+        });
+      });
+
+      describe('and the cancel button is clicked', () => {
+        beforeEach(() => {
+          findCancelButton().vm.$emit('click');
+        });
+
+        it('triggers reset event and hides both cancel and save changes button', () => {
+          expect(eventHub.$emit).toHaveBeenCalledWith('resetIngressModSecurityEnabled', INGRESS);
+          expect(findSaveButton().exists()).toBe(false);
+          expect(findCancelButton().exists()).toBe(false);
+        });
+      });
     });
 
     it('triggers set event to be propagated with the current modsecurity value', () => {
@@ -79,7 +101,7 @@ describe('IngressModsecuritySettings', () => {
       });
 
       it('renders save button with "Saving" label', () => {
-        expect(findSaveButton().props('label')).toBe('Saving');
+        expect(findSaveButton().text()).toBe('Saving');
       });
     });
 
@@ -101,7 +123,7 @@ describe('IngressModsecuritySettings', () => {
 
     it('does not render the save button', () => {
       expect(findSaveButton().exists()).toBe(false);
-      expect(findModSecurityCheckbox().checked).toBe(false);
+      expect(findModSecurityToggle().props('value')).toBe(false);
     });
   });
 });

spec/frontend/clusters/services/mock_data.js

@@ -20,6 +20,7 @@ const CLUSTERS_MOCK_DATA = {
             external_ip: null,
             external_hostname: null,
             can_uninstall: false,
+            modsecurity_enabled: false,
           },
           {
             name: 'runner',

spec/models/clusters/applications/ingress_spec.rb

@@ -177,6 +177,7 @@ describe Clusters::Applications::Ingress do
     context 'when modsecurity_enabled is disabled' do
       before do
         allow(subject).to receive(:cluster).and_return(cluster)
+        allow(subject).to receive(:modsecurity_enabled).and_return(false)
       end
 
       it 'excludes modsecurity module enablement' do