Merge branch...

Merge branch '238602-integrate-analyzerconfiguration-component-into-sast-configuration-ui-2' into 'master'

Remove sast_configuration_ui_analyzers feature flag

See merge request gitlab-org/gitlab!42593

doc/user/application_security/sast/index.md

@@ -147,6 +147,7 @@ always take the latest SAST artifact available.
 
 > - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/3659) in GitLab Ultimate 13.3.
 > - [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/232862) in GitLab Ultimate 13.4.
+> - [Improved](https://gitlab.com/groups/gitlab-org/-/epics/3635) in GitLab Ultimate 13.5.
 
 You can enable and configure SAST with a basic configuration using the **SAST Configuration**
 page:
@@ -154,9 +155,11 @@ page:
 1. From the project's home page, go to **Security & Compliance** > **Configuration** in the
    left sidebar.
 1. If the project does not have a `gitlab-ci.yml` file, click **Enable** in the Static Application Security Testing (SAST) row, otherwise click **Configure**.
-1. Enter the custom SAST values, then click **Create Merge Request**.
+1. Enter the custom SAST values.
 
     Custom values are stored in the `.gitlab-ci.yml` file. For variables not in the SAST Configuration page, their values are left unchanged. Default values are inherited from the GitLab SAST template.
+1. Optionally, expand the **SAST analyzers** section, select individual [SAST analyzers](./analyzers.md) and enter custom analyzer values.
+1. Click **Create Merge Request**.
 1. Review and merge the merge request.
 
 ### Customizing the SAST settings

ee/app/assets/javascripts/security_configuration/sast/components/app.vue

@@ -3,7 +3,6 @@ import { GlAlert, GlLink, GlLoadingIcon, GlSprintf } from '@gitlab/ui';
 import { s__ } from '~/locale';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import sastCiConfigurationQuery from '../graphql/sast_ci_configuration.query.graphql';
-import sastCiConfigurationWithAnalyzersQuery from '../graphql/sast_ci_configuration_with_analyzers.query.graphql';
 import ConfigurationForm from './configuration_form.vue';
 
 export default {
@@ -27,11 +26,7 @@ export default {
   },
   apollo: {
     sastCiConfiguration: {
-      query() {
-        return this.glFeatures.sastConfigurationUiAnalyzers
-          ? sastCiConfigurationWithAnalyzersQuery
-          : sastCiConfigurationQuery;
-      },
+      query: sastCiConfigurationQuery,
       variables() {
         return {
           fullPath: this.projectPath,

ee/app/assets/javascripts/security_configuration/sast/components/configuration_form.vue

@@ -4,7 +4,6 @@ import * as Sentry from '@sentry/browser';
 import { cloneDeep } from 'lodash';
 import { __, s__ } from '~/locale';
 import { redirectTo } from '~/lib/utils/url_utility';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import AnalyzerConfiguration from './analyzer_configuration.vue';
 import DynamicFields from './dynamic_fields.vue';
 import ExpandableSection from './expandable_section.vue';
@@ -24,7 +23,6 @@ export default {
     GlIcon,
     GlLink,
   },
-  mixins: [glFeatureFlagsMixin()],
   inject: {
     createSastMergeRequestPath: {
       from: 'createSastMergeRequestPath',
@@ -54,18 +52,14 @@ export default {
     return {
       globalConfiguration: cloneDeep(this.sastCiConfiguration.global.nodes),
       pipelineConfiguration: cloneDeep(this.sastCiConfiguration.pipeline.nodes),
-      analyzersConfiguration: this.glFeatures.sastConfigurationUiAnalyzers
-        ? cloneDeep(this.sastCiConfiguration.analyzers.nodes)
-        : [],
+      analyzersConfiguration: cloneDeep(this.sastCiConfiguration.analyzers.nodes),
       hasSubmissionError: false,
       isSubmitting: false,
     };
   },
   computed: {
     shouldRenderAnalyzersSection() {
-      return Boolean(
-        this.glFeatures.sastConfigurationUiAnalyzers && this.analyzersConfiguration.length > 0,
-      );
+      return this.analyzersConfiguration.length > 0;
     },
   },
   methods: {
@@ -100,18 +94,11 @@ export default {
         });
     },
     getMutationConfiguration() {
-      const configuration = {
+      return {
         global: this.globalConfiguration.map(toSastCiConfigurationEntityInput),
         pipeline: this.pipelineConfiguration.map(toSastCiConfigurationEntityInput),
+        analyzers: this.analyzersConfiguration.map(toSastCiConfigurationAnalyzerEntityInput),
       };
-
-      if (this.glFeatures.sastConfigurationUiAnalyzers) {
-        configuration.analyzers = this.analyzersConfiguration.map(
-          toSastCiConfigurationAnalyzerEntityInput,
-        );
-      }
-
-      return configuration;
     },
     onAnalyzerChange(name, updatedAnalyzer) {
       const index = this.analyzersConfiguration.findIndex(analyzer => analyzer.name === name);

ee/app/assets/javascripts/security_configuration/sast/graphql/sast_ci_configuration.fragment.graphql

-#import "./sast_ci_configuration_entity.fragment.graphql"
-
-fragment SastCiConfigurationFragment on SastCiConfiguration {
-  global {
-    nodes {
-      ...SastCiConfigurationEntityFragment
-    }
-  }
-  pipeline {
-    nodes {
-      ...SastCiConfigurationEntityFragment
-    }
-  }
-}

ee/app/assets/javascripts/security_configuration/sast/graphql/sast_ci_configuration.query.graphql

-#import "./sast_ci_configuration.fragment.graphql"
+#import "./sast_ci_configuration_entity.fragment.graphql"
 
 query sastCiConfiguration($fullPath: ID!) {
   project(fullPath: $fullPath) {
     sastCiConfiguration {
-      ...SastCiConfigurationFragment
+      global {
+        nodes {
+          ...SastCiConfigurationEntityFragment
+        }
+      }
+      pipeline {
+        nodes {
+          ...SastCiConfigurationEntityFragment
+        }
+      }
+      analyzers {
+        nodes {
+          description
+          enabled
+          label
+          name
+          variables {
+            nodes {
+              ...SastCiConfigurationEntityFragment
+            }
+          }
+        }
+      }
     }
   }
 }

ee/app/assets/javascripts/security_configuration/sast/graphql/sast_ci_configuration_with_analyzers.query.graphql

-#import "./sast_ci_configuration.fragment.graphql"
-#import "./sast_ci_configuration_entity.fragment.graphql"
-
-query sastCiConfiguration($fullPath: ID!) {
-  project(fullPath: $fullPath) {
-    sastCiConfiguration {
-      ...SastCiConfigurationFragment
-      analyzers {
-        nodes {
-          description
-          enabled
-          label
-          name
-          variables {
-            nodes {
-              ...SastCiConfigurationEntityFragment
-            }
-          }
-        }
-      }
-    }
-  }
-}

ee/app/controllers/projects/security/sast_configuration_controller.rb

@@ -11,10 +11,6 @@ module Projects
       before_action :ensure_sast_configuration_enabled!, except: [:create]
       before_action :authorize_edit_tree!, only: [:create]
 
-      before_action only: [:show] do
-        push_frontend_feature_flag(:sast_configuration_ui_analyzers, project)
-      end
-
       def show
       end
 

ee/changelogs/unreleased/238602-integrate-analyzerconfiguration-component-into-sast-configurati.yml

+---
+title: Expose analyzer configuration in SAST Configuration UI
+merge_request: 42593
+author:
+type: added

ee/config/feature_flags/development/sast_configuration_ui_analyzers.yml

----
-name: sast_configuration_ui_analyzers
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/42214
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/238602
-group: group::static analysis
-type: development
-default_enabled: false

ee/spec/frontend/security_configuration/sast/components/configuration_form_spec.js

@@ -71,7 +71,7 @@ describe('ConfigurationForm component', () => {
   const findAnalyzerConfigurations = () => wrapper.findAll(AnalyzerConfiguration);
   const findAnalyzersSection = () => wrapper.find('[data-testid="analyzers-section"]');
 
-  const expectPayloadForEntities = ({ withAnalyzers = false } = {}) => {
+  const expectPayloadForEntities = () => {
     const expectedPayload = {
       mutation: configureSastMutation,
       variables: {
@@ -92,27 +92,24 @@ describe('ConfigurationForm component', () => {
                 value: 'value1',
               },
             ],
+            analyzers: [
+              {
+                name: 'nameValue0',
+                enabled: true,
+                variables: [
+                  {
+                    field: 'field2',
+                    defaultValue: 'defaultValue2',
+                    value: 'value2',
+                  },
+                ],
+              },
+            ],
           },
         },
       },
     };
 
-    if (withAnalyzers) {
-      expectedPayload.variables.input.configuration.analyzers = [
-        {
-          name: 'nameValue0',
-          enabled: true,
-          variables: [
-            {
-              field: 'field2',
-              defaultValue: 'defaultValue2',
-              value: 'value2',
-            },
-          ],
-        },
-      ];
-    }
-
     expect(wrapper.vm.$apollo.mutate.mock.calls).toEqual([[expectedPayload]]);
   };
 
@@ -162,67 +159,50 @@ describe('ConfigurationForm component', () => {
   });
 
   describe('the analyzers section', () => {
-    describe('given the sastConfigurationUiAnalyzers feature flag is disabled', () => {
-      beforeEach(() => {
-        createComponent();
+    beforeEach(() => {
+      createComponent({
+        stubs: {
+          ExpandableSection,
+        },
       });
+    });
 
-      it('does not render', () => {
-        expect(findAnalyzersSection().exists()).toBe(false);
-      });
+    it('renders', () => {
+      const analyzersSection = findAnalyzersSection();
+      expect(analyzersSection.exists()).toBe(true);
+      expect(analyzersSection.text()).toContain(ConfigurationForm.i18n.analyzersHeading);
+      expect(analyzersSection.text()).toContain(ConfigurationForm.i18n.analyzersSubHeading);
     });
 
-    describe('given the sastConfigurationUiAnalyzers feature flag is enabled', () => {
-      beforeEach(() => {
-        createComponent({
-          provide: {
-            glFeatures: {
-              sastConfigurationUiAnalyzers: true,
-            },
-          },
-          stubs: {
-            ExpandableSection,
-          },
-        });
-      });
+    it('has a link to the documentation', () => {
+      const link = findAnalyzersSection().find(GlLink);
+      expect(link.exists()).toBe(true);
+      expect(link.attributes('href')).toBe(sastAnalyzersDocumentationPath);
+    });
 
-      it('renders', () => {
-        const analyzersSection = findAnalyzersSection();
-        expect(analyzersSection.exists()).toBe(true);
-        expect(analyzersSection.text()).toContain(ConfigurationForm.i18n.analyzersHeading);
-        expect(analyzersSection.text()).toContain(ConfigurationForm.i18n.analyzersSubHeading);
+    it('renders each analyzer', () => {
+      const analyzerEntities = sastCiConfiguration.analyzers.nodes;
+      const analyzerComponents = findAnalyzerConfigurations();
+      analyzerEntities.forEach((entity, i) => {
+        expect(analyzerComponents.at(i).props()).toEqual({ entity });
       });
+    });
 
-      it('has a link to the documentation', () => {
-        const link = findAnalyzersSection().find(GlLink);
-        expect(link.exists()).toBe(true);
-        expect(link.attributes('href')).toBe(sastAnalyzersDocumentationPath);
-      });
+    describe('when an AnalyzerConfiguration emits an input event', () => {
+      let analyzer;
+      let updatedEntity;
 
-      it('renders each analyzer', () => {
-        const analyzerEntities = sastCiConfiguration.analyzers.nodes;
-        const analyzerComponents = findAnalyzerConfigurations();
-        analyzerEntities.forEach((entity, i) => {
-          expect(analyzerComponents.at(i).props()).toEqual({ entity });
-        });
+      beforeEach(() => {
+        analyzer = findAnalyzerConfigurations().at(0);
+        updatedEntity = {
+          ...sastCiConfiguration.analyzers.nodes[0],
+          value: 'new value',
+        };
+        analyzer.vm.$emit('input', updatedEntity);
       });
 
-      describe('when an AnalyzerConfiguration emits an input event', () => {
-        let analyzer;
-        let updatedEntity;
-
-        beforeEach(() => {
-          analyzer = findAnalyzerConfigurations().at(0);
-          updatedEntity = {
-            ...sastCiConfiguration.analyzers.nodes[0],
-            value: 'new value',
-          };
-          analyzer.vm.$emit('input', updatedEntity);
-        });
-
-        it('updates the entity binding', () => {
-          expect(analyzer.props('entity')).toBe(updatedEntity);
-        });
+      it('updates the entity binding', () => {
+        expect(analyzer.props('entity')).toBe(updatedEntity);
       });
     });
   });
@@ -233,126 +213,107 @@ describe('ConfigurationForm component', () => {
     });
 
     describe.each`
-      context             | successPath | errors          | sastConfigurationUiAnalyzers
-      ${'no successPath'} | ${''}       | ${[]}           | ${false}
-      ${'any errors'}     | ${''}       | ${['an error']} | ${false}
-      ${'no successPath'} | ${''}       | ${[]}           | ${true}
-      ${'any errors'}     | ${''}       | ${['an error']} | ${true}
-    `(
-      'given an unsuccessful endpoint response due to $context',
-      ({ successPath, errors, sastConfigurationUiAnalyzers }) => {
-        beforeEach(() => {
-          createComponent({
-            mutationResult: {
-              successPath,
-              errors,
-            },
-            provide: {
-              glFeatures: { sastConfigurationUiAnalyzers },
-            },
-          });
+      context             | successPath | errors
+      ${'no successPath'} | ${''}       | ${[]}
+      ${'any errors'}     | ${''}       | ${['an error']}
+    `('given an unsuccessful endpoint response due to $context', ({ successPath, errors }) => {
+      beforeEach(() => {
+        createComponent({
+          mutationResult: {
+            successPath,
+            errors,
+          },
+        });
+
+        findForm().trigger('submit');
+      });
+
+      it('includes the value of each entity in the payload', () => {
+        expectPayloadForEntities();
+      });
 
-          findForm().trigger('submit');
+      it(`sets the submit button's loading prop to true`, () => {
+        expect(findSubmitButton().props('loading')).toBe(true);
+      });
+
+      describe('after async tasks', () => {
+        beforeEach(fulfillPendingPromises);
+
+        it('does not call redirectTo', () => {
+          expect(redirectTo).not.toHaveBeenCalled();
         });
 
-        it('includes the value of each entity in the payload', () => {
-          expectPayloadForEntities({ withAnalyzers: sastConfigurationUiAnalyzers });
+        it('displays an alert message', () => {
+          expect(findErrorAlert().exists()).toBe(true);
         });
 
-        it(`sets the submit button's loading prop to true`, () => {
-          expect(findSubmitButton().props('loading')).toBe(true);
+        it('sends the error to Sentry', () => {
+          expect(Sentry.captureException.mock.calls).toMatchObject([
+            [{ message: expect.stringMatching(/merge request.*fail/) }],
+          ]);
         });
 
-        describe('after async tasks', () => {
-          beforeEach(fulfillPendingPromises);
+        it(`sets the submit button's loading prop to false`, () => {
+          expect(findSubmitButton().props('loading')).toBe(false);
+        });
 
-          it('does not call redirectTo', () => {
-            expect(redirectTo).not.toHaveBeenCalled();
+        describe('submitting again after a previous error', () => {
+          beforeEach(() => {
+            findForm().trigger('submit');
           });
 
-          it('displays an alert message', () => {
-            expect(findErrorAlert().exists()).toBe(true);
+          it('hides the alert message', () => {
+            expect(findErrorAlert().exists()).toBe(false);
           });
+        });
+      });
+    });
 
-          it('sends the error to Sentry', () => {
-            expect(Sentry.captureException.mock.calls).toMatchObject([
-              [{ message: expect.stringMatching(/merge request.*fail/) }],
-            ]);
-          });
+    describe('given a successful endpoint response', () => {
+      beforeEach(() => {
+        createComponent({
+          mutationResult: {
+            successPath: newMergeRequestPath,
+            errors: [],
+          },
+        });
 
-          it(`sets the submit button's loading prop to false`, () => {
-            expect(findSubmitButton().props('loading')).toBe(false);
-          });
+        findForm().trigger('submit');
+      });
 
-          describe('submitting again after a previous error', () => {
-            beforeEach(() => {
-              findForm().trigger('submit');
-            });
+      it('includes the value of each entity in the payload', () => {
+        expectPayloadForEntities();
+      });
 
-            it('hides the alert message', () => {
-              expect(findErrorAlert().exists()).toBe(false);
-            });
-          });
-        });
-      },
-    );
+      it(`sets the submit button's loading prop to true`, () => {
+        expect(findSubmitButton().props().loading).toBe(true);
+      });
 
-    describe.each([true, false])(
-      'given a successful endpoint response with sastConfigurationUiAnalyzers = %p',
-      sastConfigurationUiAnalyzers => {
-        beforeEach(() => {
-          createComponent({
-            mutationResult: {
-              successPath: newMergeRequestPath,
-              errors: [],
-            },
-            provide: {
-              glFeatures: { sastConfigurationUiAnalyzers },
-            },
-          });
+      describe('after async tasks', () => {
+        beforeEach(fulfillPendingPromises);
 
-          findForm().trigger('submit');
+        it('calls redirectTo', () => {
+          expect(redirectTo).toHaveBeenCalledWith(newMergeRequestPath);
         });
 
-        // See https://github.com/jest-community/eslint-plugin-jest/issues/229
-        // for a similar reason for disabling the rule on the next line
-        // eslint-disable-next-line jest/no-identical-title
-        it('includes the value of each entity in the payload', () => {
-          expectPayloadForEntities({ withAnalyzers: sastConfigurationUiAnalyzers });
+        it('does not display an alert message', () => {
+          expect(findErrorAlert().exists()).toBe(false);
         });
 
-        // eslint-disable-next-line jest/no-identical-title
-        it(`sets the submit button's loading prop to true`, () => {
-          expect(findSubmitButton().props().loading).toBe(true);
+        it('does not call Sentry.captureException', () => {
+          expect(Sentry.captureException).not.toHaveBeenCalled();
         });
 
-        // eslint-disable-next-line jest/no-identical-title
-        describe('after async tasks', () => {
-          beforeEach(fulfillPendingPromises);
-
-          it('calls redirectTo', () => {
-            expect(redirectTo).toHaveBeenCalledWith(newMergeRequestPath);
-          });
-
-          it('does not display an alert message', () => {
-            expect(findErrorAlert().exists()).toBe(false);
-          });
-
-          it('does not call Sentry.captureException', () => {
-            expect(Sentry.captureException).not.toHaveBeenCalled();
-          });
-
-          it('keeps the loading prop set to true', () => {
-            // This is done for UX reasons. If the loading prop is set to false
-            // on success, then there's a period where the button is clickable
-            // again. Instead, we want the button to display a loading indicator
-            // for the remainder of the lifetime of the page (i.e., until the
-            // browser can start painting the new page it's been redirected to).
-            expect(findSubmitButton().props().loading).toBe(true);
-          });
+        it('keeps the loading prop set to true', () => {
+          // This is done for UX reasons. If the loading prop is set to false
+          // on success, then there's a period where the button is clickable
+          // again. Instead, we want the button to display a loading indicator
+          // for the remainder of the lifetime of the page (i.e., until the
+          // browser can start painting the new page it's been redirected to).
+          expect(findSubmitButton().props().loading).toBe(true);
         });
-      },
-    );
+      });
+    });
   });
 
   describe('the cancel button', () => {