Merge branch '235135-remove-sast-configuration-ui-feature-flag' into 'master'

Remove sast_configuration_ui feature flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!55680

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

@@ -67,11 +67,6 @@ export default {
       required: false,
       default: false,
     },
-    // TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/241377
-    createSastMergeRequestPath: {
-      type: String,
-      required: true,
-    },
   },
   data: () => ({
     autoDevopsAlertDismissed: 'false',
@@ -202,11 +197,7 @@ export default {
       </template>
 
       <template #cell(manage)="{ item }">
-        <manage-feature
-          :feature="item"
-          :auto-devops-enabled="autoDevopsEnabled"
-          :create-sast-merge-request-path="createSastMergeRequestPath"
-        />
+        <manage-feature :feature="item" :auto-devops-enabled="autoDevopsEnabled" />
       </template>
     </gl-table>
     <auto-fix-settings v-if="glFeatures.securityAutoFix" v-bind="autoFixSettingsProps" />

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

-<script>
-import { GlButton } from '@gitlab/ui';
-import * as Sentry from '@sentry/browser';
-import { deprecatedCreateFlash as createFlash } from '~/flash';
-import axios from '~/lib/utils/axios_utils';
-import { redirectTo } from '~/lib/utils/url_utility';
-import { s__ } from '~/locale';
-
-export default {
-  components: {
-    GlButton,
-  },
-  props: {
-    autoDevopsEnabled: {
-      type: Boolean,
-      required: true,
-    },
-    endpoint: {
-      type: String,
-      required: true,
-    },
-  },
-  data() {
-    return {
-      isCreatingMergeRequest: false,
-    };
-  },
-  computed: {
-    buttonProps() {
-      if (this.autoDevopsEnabled) {
-        return {
-          text: this.$options.i18n.autoDevOps,
-        };
-      }
-
-      return {
-        text: this.$options.i18n.noAutoDevOps,
-        category: 'primary',
-        variant: 'success',
-      };
-    },
-  },
-  methods: {
-    createMergeRequest() {
-      this.isCreatingMergeRequest = true;
-
-      return axios
-        .post(this.endpoint)
-        .then(({ data }) => {
-          const { filePath } = data;
-          if (!filePath) {
-            // eslint-disable-next-line @gitlab/require-i18n-strings
-            throw new Error('SAST merge request creation failed');
-          }
-
-          redirectTo(filePath);
-        })
-        .catch((error) => {
-          this.isCreatingMergeRequest = false;
-          createFlash(
-            s__('SecurityConfiguration|An error occurred while creating the merge request.'),
-          );
-          Sentry.captureException(error);
-        });
-    },
-  },
-  i18n: {
-    autoDevOps: s__('SecurityConfiguration|Configure'),
-    noAutoDevOps: s__('SecurityConfiguration|Enable via Merge Request'),
-  },
-};
-</script>
-
-<template>
-  <gl-button :loading="isCreatingMergeRequest" v-bind="buttonProps" @click="createMergeRequest">{{
-    buttonProps.text
-  }}</gl-button>
-</template>

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

 <script>
 import { GlButton } from '@gitlab/ui';
-import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
-import CreateMergeRequestButton from './create_merge_request_button.vue';
 
 export default {
   components: {
     GlButton,
-    CreateMergeRequestButton,
   },
-  mixins: [glFeatureFlagsMixin()],
   props: {
     feature: {
       type: Object,
@@ -18,18 +14,10 @@ export default {
       type: Boolean,
       required: true,
     },
-    createSastMergeRequestPath: {
-      type: String,
-      required: true,
-    },
   },
   computed: {
     canConfigureFeature() {
-      return Boolean(this.glFeatures.sastConfigurationUi && this.feature.configuration_path);
-    },
-    // TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/241377
-    canCreateSASTMergeRequest() {
-      return Boolean(this.feature.type === 'sast' && this.createSastMergeRequestPath);
+      return Boolean(this.feature.configuration_path);
     },
     canManageProfiles() {
       return this.feature.type === 'dast_profiles';
@@ -61,11 +49,4 @@ export default {
     data-testid="enableButton"
     >{{ s__('SecurityConfiguration|Enable') }}</gl-button
   >
-
-  <!-- TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/241377 -->
-  <create-merge-request-button
-    v-else-if="canCreateSASTMergeRequest"
-    :auto-devops-enabled="autoDevopsEnabled"
-    :endpoint="createSastMergeRequestPath"
-  />
 </template>

ee/app/assets/javascripts/security_configuration/index.js

@@ -19,7 +19,6 @@ export const initSecurityConfiguration = (el) => {
     containerScanningHelpPath,
     dependencyScanningHelpPath,
     toggleAutofixSettingEndpoint,
-    createSastMergeRequestPath,
     gitlabCiHistoryPath,
   } = el.dataset;
 
@@ -36,7 +35,6 @@ export const initSecurityConfiguration = (el) => {
           features: JSON.parse(features),
           helpPagePath,
           latestPipelinePath,
-          createSastMergeRequestPath,
           ...parseBooleanDataAttributes(el, [
             'autoDevopsEnabled',
             'canEnableAutoDevops',

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

@@ -25,10 +25,6 @@ export default {
     GlSprintf,
   },
   inject: {
-    createSastMergeRequestPath: {
-      from: 'createSastMergeRequestPath',
-      default: '',
-    },
     sastAnalyzersDocumentationPath: {
       from: 'sastAnalyzersDocumentationPath',
       default: '',

ee/app/assets/javascripts/security_configuration/sast/index.js

@@ -18,7 +18,6 @@ export default function init() {
 
   const {
     securityConfigurationPath,
-    createSastMergeRequestPath,
     projectPath,
     sastAnalyzersDocumentationPath,
     sastDocumentationPath,
@@ -29,7 +28,6 @@ export default function init() {
     apolloProvider,
     provide: {
       securityConfigurationPath,
-      createSastMergeRequestPath,
       projectPath,
       sastAnalyzersDocumentationPath,
       sastDocumentationPath,

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

@@ -14,7 +14,6 @@ module EE
 
           before_action only: [:show] do
             push_frontend_feature_flag(:security_auto_fix, project, default_enabled: false)
-            push_frontend_feature_flag(:sast_configuration_ui, project, default_enabled: true)
             push_frontend_feature_flag(:api_fuzzing_configuration_ui, project, default_enabled: :yaml)
           end
 

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

@@ -4,38 +4,14 @@ module Projects
   module Security
     class SastConfigurationController < Projects::ApplicationController
       include SecurityAndCompliancePermissions
-      include CreatesCommit
       include SecurityDashboardsPermissions
 
       alias_method :vulnerable, :project
 
-      before_action :ensure_sast_configuration_enabled!, except: [:create]
-      before_action :authorize_edit_tree!, only: [:create]
-
       feature_category :static_application_security_testing
 
       def show
       end
-
-      def create
-        result = ::Security::CiConfiguration::SastCreateService.new(project, current_user, params).execute
-
-        if result[:status] == :success
-          respond_to do |format|
-            format.json { render json: { message: _("success"), filePath: result[:success_path] } }
-          end
-        else
-          respond_to do |format|
-            format.json { render json: { message: _("failed"), filePath: '' } }
-          end
-        end
-      end
-
-      private
-
-      def ensure_sast_configuration_enabled!
-        not_found unless ::Feature.enabled?(:sast_configuration_ui, project, default_enabled: true)
-      end
     end
   end
 end

ee/app/helpers/projects/security/sast_configuration_helper.rb

@@ -3,7 +3,6 @@
 module Projects::Security::SastConfigurationHelper
   def sast_configuration_data(project)
     {
-      create_sast_merge_request_path: project_security_configuration_sast_path(project),
       project_path: project.full_path,
       sast_analyzers_documentation_path: help_page_path('user/application_security/sast/analyzers'),
       sast_documentation_path: help_page_path('user/application_security/sast/index', anchor: 'configuration'),

ee/app/presenters/projects/security/configuration_presenter.rb

@@ -53,7 +53,6 @@ module Projects
         {
           auto_devops_enabled: auto_devops_source?,
           auto_devops_help_page_path: help_page_path('topics/autodevops/index'),
-          create_sast_merge_request_path: project_security_configuration_sast_path(project),
           auto_devops_path: auto_devops_settings_path(project),
           can_enable_auto_devops: can_enable_auto_devops?,
           features: features,

ee/changelogs/unreleased/235135-remove-sast-configuration-ui-feature-flag.yml

+---
+title: Remove sast_configuration_ui feature flag
+merge_request: 55680
+author:
+type: changed

ee/config/feature_flags/development/sast_configuration_ui.yml

----
-name: sast_configuration_ui
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/34947
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/235135
-milestone: '13.2'
-type: development
-group: group::static analysis
-default_enabled: true

ee/config/routes/project.rb

@@ -69,7 +69,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
           resource :configuration, only: [], controller: :configuration do
             post :auto_fix, on: :collection
             resource :corpus_management, only: [:show], controller: :corpus_management
-            resource :sast, only: [:show, :create], controller: :sast_configuration
+            resource :sast, only: [:show], controller: :sast_configuration
             resource :api_fuzzing, only: :show, controller: :api_fuzzing_configuration
             resource :dast_profiles, only: [:show] do
               resources :dast_site_profiles, only: [:new, :edit]

ee/spec/controllers/projects/security/sast_configuration_controller_spec.rb

@@ -52,18 +52,6 @@ RSpec.describe Projects::Security::SastConfigurationController do
 
         expect(response.body).to have_active_sub_navigation('Configuration')
       end
-
-      context 'with feature flag disabled' do
-        before do
-          stub_feature_flags(sast_configuration_ui: false)
-        end
-
-        it 'returns a 404 for an HTML request' do
-          request
-
-          expect(response).to have_gitlab_http_status(:not_found)
-        end
-      end
     end
 
     context 'with unauthorized user' do
@@ -78,40 +66,4 @@ RSpec.describe Projects::Security::SastConfigurationController do
       end
     end
   end
-
-  describe 'POST #create' do
-    let(:params) do
-      {
-        namespace_id: project.namespace.to_param,
-        project_id: project.to_param,
-        sast_configuration: {
-          secure_analyzers_prefix: 'localhost:5000/analyzers',
-          sast_analyzer_image_tag: '1',
-          sast_excluded_paths: 'docs',
-          stage: 'security',
-          search_max_depth: 11
-        },
-        format: :json
-      }
-    end
-
-    subject(:request) { post :create, params: params, as: :json }
-
-    before do
-      sign_in(developer)
-    end
-
-    include_context '"Security & Compliance" permissions' do
-      let(:valid_request) { request }
-    end
-
-    context 'with valid params' do
-      it 'returns the new merge request url' do
-        request
-
-        expect(json_response["message"]).to eq("success")
-        expect(json_response["filePath"]).to match(/#{Gitlab::Routing.url_helpers.project_new_merge_request_url(project, {})}(.*)description(.*)source_branch/)
-      end
-    end
-  end
 end

ee/spec/frontend/security_configuration/components/app_spec.js

@@ -19,7 +19,6 @@ const propsData = {
   gitlabCiPresent: false,
   gitlabCiHistoryPath: '/ci/history',
   autoFixSettingsProps: {},
-  createSastMergeRequestPath: 'http://createSastMergeRequestPath',
 };
 
 describe('Security Configuration App', () => {
@@ -183,7 +182,6 @@ describe('Security Configuration App', () => {
         expect(manage.find(ManageFeature).props()).toEqual({
           feature: features[i],
           autoDevopsEnabled: propsData.autoDevopsEnabled,
-          createSastMergeRequestPath: propsData.createSastMergeRequestPath,
         });
         expect(feature.find(GlLink).props('href')).toBe(features[i].href);
       }

ee/spec/frontend/security_configuration/components/create_merge_request_button_spec.js

-import { GlButton } from '@gitlab/ui';
-import * as Sentry from '@sentry/browser';
-import { shallowMount } from '@vue/test-utils';
-import AxiosMockAdapter from 'axios-mock-adapter';
-import CreateMergeRequestButton from 'ee/security_configuration/components/create_merge_request_button.vue';
-import waitForPromises from 'helpers/wait_for_promises';
-import { deprecatedCreateFlash as createFlash } from '~/flash';
-import axios from '~/lib/utils/axios_utils';
-import { redirectTo } from '~/lib/utils/url_utility';
-
-jest.mock('~/flash.js');
-jest.mock('~/lib/utils/url_utility', () => ({
-  redirectTo: jest.fn(),
-}));
-
-const endpoint = '/endpoint';
-const { i18n } = CreateMergeRequestButton;
-const DEFAULT_BUTTON_PROPS = {
-  category: 'primary',
-  variant: 'default',
-};
-const SUCCESS_BUTTON_PROPS = {
-  category: 'primary',
-  variant: 'success',
-};
-const MERGE_REQUEST_PATH = '/merge_requests/new';
-
-describe('CreateMergeRequestButton component', () => {
-  let axiosMock;
-  let wrapper;
-
-  const createComponent = (props = {}) => {
-    wrapper = shallowMount(CreateMergeRequestButton, {
-      propsData: {
-        endpoint,
-        autoDevopsEnabled: false,
-        ...props,
-      },
-    });
-  };
-
-  beforeEach(() => {
-    axiosMock = new AxiosMockAdapter(axios);
-  });
-
-  afterEach(() => {
-    wrapper.destroy();
-    wrapper = null;
-    axiosMock.restore();
-  });
-
-  const findButton = () => wrapper.find(GlButton);
-
-  describe.each`
-    autoDevopsEnabled | buttonText           | buttonProps
-    ${false}          | ${i18n.noAutoDevOps} | ${SUCCESS_BUTTON_PROPS}
-    ${true}           | ${i18n.autoDevOps}   | ${DEFAULT_BUTTON_PROPS}
-  `(
-    'when autoDevopsEnabled is $autoDevopsEnabled',
-    ({ autoDevopsEnabled, buttonText, buttonProps }) => {
-      beforeEach(() => {
-        createComponent({ autoDevopsEnabled });
-      });
-
-      it('uses the right button label', () => {
-        expect(findButton().text()).toEqual(buttonText);
-      });
-
-      it('passes the correct data to the button', () => {
-        expect(findButton().props()).toMatchObject({
-          loading: false,
-          ...buttonProps,
-        });
-      });
-    },
-  );
-
-  describe('when clicking the button', () => {
-    describe.each`
-      context                    | filePath              | statusCode | partialErrorMessage
-      ${'a response error code'} | ${MERGE_REQUEST_PATH} | ${500}     | ${'500'}
-      ${'no filePath'}           | ${''}                 | ${200}     | ${/merge request.*fail/}
-    `(
-      'given an unsuccessful endpoint response due to $context',
-      ({ filePath, statusCode, partialErrorMessage }) => {
-        beforeEach(() => {
-          axiosMock.onPost(endpoint).replyOnce(statusCode, { filePath });
-          jest.spyOn(Sentry, 'captureException').mockImplementation();
-          createComponent();
-
-          findButton().vm.$emit('click');
-        });
-
-        it('sets the loading prop to true', () => {
-          expect(findButton().props().loading).toBe(true);
-        });
-
-        describe('after async tasks', () => {
-          beforeEach(() => waitForPromises());
-
-          it('does not call redirectTo', () => {
-            expect(redirectTo).not.toHaveBeenCalled();
-          });
-
-          it('creates a flash message', () => {
-            expect(createFlash).toHaveBeenCalledWith(expect.any(String));
-          });
-
-          it('sends the error to Sentry', () => {
-            expect(Sentry.captureException.mock.calls).toMatchObject([
-              [{ message: expect.stringMatching(partialErrorMessage) }],
-            ]);
-          });
-
-          it('sets the loading prop to false', () => {
-            expect(findButton().props().loading).toBe(false);
-          });
-        });
-      },
-    );
-
-    describe('given a successful endpoint response', () => {
-      beforeEach(() => {
-        axiosMock.onPost(endpoint).replyOnce(200, { filePath: MERGE_REQUEST_PATH });
-        jest.spyOn(Sentry, 'captureException').mockImplementation();
-        createComponent();
-
-        findButton().vm.$emit('click');
-      });
-
-      it('sets the loading prop to true', () => {
-        expect(findButton().props().loading).toBe(true);
-      });
-
-      describe('after async tasks', () => {
-        beforeEach(() => waitForPromises());
-
-        it('calls redirectTo', () => {
-          expect(redirectTo).toHaveBeenCalledWith(MERGE_REQUEST_PATH);
-        });
-
-        it('does not create a flash message', () => {
-          expect(createFlash).not.toHaveBeenCalled();
-        });
-
-        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(findButton().props().loading).toBe(true);
-        });
-      });
-    });
-  });
-});

ee/spec/frontend/security_configuration/components/manage_feature_spec.js

 import { shallowMount } from '@vue/test-utils';
 import { merge } from 'lodash';
-import CreateMergeRequestButton from 'ee/security_configuration/components/create_merge_request_button.vue';
 import ManageFeature from 'ee/security_configuration/components/manage_feature.vue';
 import { generateFeatures } from './helpers';
 
-const createSastMergeRequestPath = '/create_sast_merge_request_path';
-
 describe('ManageFeature component', () => {
   let wrapper;
   let feature;
@@ -16,7 +13,6 @@ describe('ManageFeature component', () => {
       merge(
         {
           propsData: {
-            createSastMergeRequestPath,
             autoDevopsEnabled: false,
           },
         },
@@ -29,59 +25,26 @@ describe('ManageFeature component', () => {
     wrapper.destroy();
   });
 
-  const findCreateMergeRequestButton = () => wrapper.find(CreateMergeRequestButton);
   const findTestId = (id) => wrapper.find(`[data-testid="${id}"]`);
 
-  describe('given sastConfigurationUi feature flag is enabled', () => {
-    const featureFlagEnabled = {
-      provide: {
-        glFeatures: {
-          sastConfigurationUi: true,
-        },
-      },
-    };
-
-    describe.each`
-      configured | expectedTestId
-      ${true}    | ${'configureButton'}
-      ${false}   | ${'enableButton'}
-    `('given feature.configured is $configured', ({ configured, expectedTestId }) => {
-      describe('given a configuration path', () => {
-        beforeEach(() => {
-          [feature] = generateFeatures(1, { configured, configuration_path: 'foo' });
-
-          createComponent({
-            ...featureFlagEnabled,
-            propsData: { feature },
-          });
+  describe.each`
+    configured | expectedTestId
+    ${true}    | ${'configureButton'}
+    ${false}   | ${'enableButton'}
+  `('given feature.configured is $configured', ({ configured, expectedTestId }) => {
+    describe('given a configuration path', () => {
+      beforeEach(() => {
+        [feature] = generateFeatures(1, { configured, configuration_path: 'foo' });
+
+        createComponent({
+          propsData: { feature },
         });
-
-        it('shows a button to configure the feature', () => {
-          const button = findTestId(expectedTestId);
-          expect(button.exists()).toBe(true);
-          expect(button.attributes('href')).toBe(feature.configuration_path);
-        });
-      });
-    });
-  });
-
-  describe('given a feature with type "sast"', () => {
-    const autoDevopsEnabled = true;
-
-    beforeEach(() => {
-      [feature] = generateFeatures(1, { type: 'sast' });
-
-      createComponent({
-        propsData: { feature, autoDevopsEnabled },
       });
-    });
 
-    it('shows the CreateMergeRequestButton component', () => {
-      const button = findCreateMergeRequestButton();
-      expect(button.exists()).toBe(true);
-      expect(button.props()).toMatchObject({
-        endpoint: createSastMergeRequestPath,
-        autoDevopsEnabled,
+      it('shows a button to configure the feature', () => {
+        const button = findTestId(expectedTestId);
+        expect(button.exists()).toBe(true);
+        expect(button.attributes('href')).toBe(feature.configuration_path);
       });
     });
   });

ee/spec/helpers/ee/projects/security/sast_configuration_helper_spec.rb

@@ -13,7 +13,6 @@ RSpec.describe Projects::Security::SastConfigurationHelper do
 
     it {
       is_expected.to eq({
-        create_sast_merge_request_path: project_security_configuration_sast_path(project),
         project_path: project_path,
         sast_analyzers_documentation_path: analyzers_docs_path,
         sast_documentation_path: docs_path,

ee/spec/presenters/projects/security/configuration_presenter_spec.rb

@@ -34,10 +34,6 @@ RSpec.describe Projects::Security::ConfigurationPresenter do
       expect(auto_fix['container_scanning']).to be_truthy
     end
 
-    it 'includes the path to create a SAST merge request' do
-      expect(subject[:create_sast_merge_request_path]).to eq(project_security_configuration_sast_path(project))
-    end
-
     it 'includes the path to gitlab_ci history' do
       expect(subject[:gitlab_ci_history_path]).to eq(project_blame_path(project, 'master/.gitlab-ci.yml'))
     end

locale/gitlab.pot

@@ -26597,9 +26597,6 @@ msgstr ""
 msgid "SecurityConfiguration|Enable"
 msgstr ""
 
-msgid "SecurityConfiguration|Enable via Merge Request"
-msgstr ""
-
 msgid "SecurityConfiguration|Enabled"
 msgstr ""