Update in preparation of supporting analyzers

This update is helpful for adding support for analyzers in the
GraphQL mutation. It is also helpful for removing support for the
create action of the SastConfigurationController.

ee/app/graphql/mutations/security/ci_configuration/configure_sast.rb

@@ -28,8 +28,7 @@ module Mutations
           project = authorized_find!(full_path: project_path)
           validate_flag!(project)
 
-          sast_create_service_params = format_for_service(configuration)
-          result = ::Security::CiConfiguration::SastCreateService.new(project, current_user, sast_create_service_params).execute
+          result = ::Security::CiConfiguration::SastCreateService.new(project, current_user, configuration).execute
           prepare_response(result)
         end
 
@@ -45,15 +44,6 @@ module Mutations
           resolve_project(full_path: full_path)
         end
 
-        # Temporary formatting necessary for supporting REST API
-        # Will be removed during the implementation of
-        # https://gitlab.com/gitlab-org/gitlab/-/issues/246737
-        def format_for_service(configuration)
-          global_values = configuration["global"]&.collect {|k| [k["field"], k["value"]]}.to_h
-          pipeline_values = configuration["pipeline"]&.collect {|k| [k["field"], k["value"]]}.to_h
-          global_values.merge!(pipeline_values)
-        end
-
         def prepare_response(result)
           {
             status: result[:status],

ee/app/services/security/ci_configuration/sast_create_service.rb

@@ -25,7 +25,7 @@ module Security
       private
 
       def attributes
-        actions = Security::CiConfiguration::SastBuildActions.new(@project.auto_devops_enabled?, @params, existing_gitlab_ci_content, default_sast_values).generate
+        actions = Security::CiConfiguration::SastBuildActions.new(@project.auto_devops_enabled?, @params, existing_gitlab_ci_content).generate
 
         @project.repository.add_branch(@current_user, @branch_name, @project.default_branch)
         message = _('Set .gitlab-ci.yml to enable or configure SAST')
@@ -43,14 +43,6 @@ module Security
         YAML.safe_load(gitlab_ci_yml) if gitlab_ci_yml
       end
 
-      def default_sast_values
-        result = Security::CiConfiguration::SastParserService.new(@project)
-
-        global_defaults = result.configuration["global"].collect { |k| [k["field"], k["default_value"]] }.to_h
-        pipeline_defaults = result.configuration["pipeline"].collect { |k| [k["field"], k["default_value"]] }.to_h
-        global_defaults.merge!(pipeline_defaults)
-      end
-
       def successful_change_path
         description = _('Set .gitlab-ci.yml to enable or configure SAST security scanning using the GitLab managed template. You can [add variable overrides](https://docs.gitlab.com/ee/user/application_security/sast/#customizing-the-sast-settings) to customize SAST settings.')
         merge_request_params = { source_branch: @branch_name, description: description }

ee/changelogs/unreleased/rf-update-sast-mutation.yml

+---
+title: Update in preparation of supporting analyzers
+merge_request: 42173
+author:
+type: changed

ee/lib/security/ci_configuration/sast_build_actions.rb

@@ -3,11 +3,11 @@
 module Security
   module CiConfiguration
     class SastBuildActions
-      def initialize(auto_devops_enabled, params, existing_gitlab_ci_content, default_sast_values)
+      def initialize(auto_devops_enabled, params, existing_gitlab_ci_content)
         @auto_devops_enabled = auto_devops_enabled
-        @params = params
+        @variables = variables(params)
         @existing_gitlab_ci_content = existing_gitlab_ci_content || {}
-        @default_sast_values = default_sast_values
+        @default_sast_values = default_sast_values(params)
       end
 
       def generate
@@ -20,6 +20,25 @@ module Security
 
       private
 
+      def variables(params)
+        # This early return is necessary for supporting REST API.
+        # Will be removed during the implementation of
+        # https://gitlab.com/gitlab-org/gitlab/-/issues/246737
+        return params unless params['global'].present?
+
+        collect_values(params, 'value')
+      end
+
+      def default_sast_values(params)
+        collect_values(params, 'defaultValue')
+      end
+
+      def collect_values(config, key)
+        global_variables = config['global']&.collect {|k| [k['field'], k[key]]}.to_h
+        pipeline_variables = config['pipeline']&.collect {|k| [k['field'], k[key]]}.to_h
+        global_variables.merge!(pipeline_variables)
+      end
+
       def update_existing_content!
         @existing_gitlab_ci_content['stages'] = set_stages
         @existing_gitlab_ci_content['variables'] = set_variables(global_variables, @existing_gitlab_ci_content)
@@ -49,15 +68,15 @@ module Security
       end
 
       def sast_stage
-        @params['stage'].presence ? @params['stage'] : 'test'
+        @variables['stage'].presence ? @variables['stage'] : 'test'
       end
 
       def set_variables(variables, hash_to_update = {})
         hash_to_update['variables'] ||= {}
 
         variables.each do |key|
-          if @params[key].present? && @params[key].to_s != @default_sast_values[key].to_s
-            hash_to_update['variables'][key] = @params[key]
+          if @variables[key].present? && @variables[key].to_s != @default_sast_values[key].to_s
+            hash_to_update['variables'][key] = @variables[key]
           else
             hash_to_update['variables'].delete(key)
           end

ee/spec/lib/security/ci_configuration/sast_build_actions_spec.rb

@@ -4,18 +4,31 @@ require 'fast_spec_helper'
 
 RSpec.describe Security::CiConfiguration::SastBuildActions do
   let(:default_sast_values) do
-    { "SECURE_ANALYZERS_PREFIX" => "registry.gitlab.com/gitlab-org/security-products/analyzers",
-      "SAST_EXCLUDED_PATHS" => "spec, test, tests, tmp", "SAST_ANALYZER_IMAGE_TAG" => "2",
-      "stage" => "test",
-      "SEARCH_MAX_DEPTH" => "4" }
+    { 'global' =>
+      [
+        { 'field' => 'SECURE_ANALYZERS_PREFIX', 'defaultValue' => 'registry.gitlab.com/gitlab-org/security-products/analyzers', 'value' => 'registry.gitlab.com/gitlab-org/security-products/analyzers' }
+      ],
+      'pipeline' =>
+      [
+        { 'field' => 'stage', 'defaultValue' => 'test', 'value' => 'test' },
+        { 'field' => 'SEARCH_MAX_DEPTH', 'defaultValue' => 4, 'value' => 4 },
+        { 'field' => 'SAST_ANALYZER_IMAGE_TAG', 'defaultValue' => 2, 'value' => 2 },
+        { 'field' => 'SAST_EXCLUDED_PATHS', 'defaultValue' => 'spec, test, tests, tmp', 'value' => 'spec, test, tests, tmp' }
+      ] }
   end
 
   let(:params) do
-    { 'stage' => 'security',
-      'SEARCH_MAX_DEPTH' => 1,
-      'SECURE_ANALYZERS_PREFIX' => 'new_registry',
-      'SAST_ANALYZER_IMAGE_TAG' => 2,
-      'SAST_EXCLUDED_PATHS' => 'spec,docs' }
+    { 'global' =>
+      [
+        { 'field' => 'SECURE_ANALYZERS_PREFIX', 'defaultValue' => 'registry.gitlab.com/gitlab-org/security-products/analyzers', 'value' => 'new_registry' }
+      ],
+      'pipeline' =>
+      [
+        { 'field' => 'stage', 'defaultValue' => 'test', 'value' => 'security' },
+        { 'field' => 'SEARCH_MAX_DEPTH', 'defaultValue' => 4, 'value' => 1 },
+        { 'field' => 'SAST_ANALYZER_IMAGE_TAG', 'defaultValue' => 2, 'value' => 2 },
+        { 'field' => 'SAST_EXCLUDED_PATHS', 'defaultValue' => 'spec, test, tests, tmp', 'value' => 'spec,docs' }
+      ] }
   end
 
   context 'with existing .gitlab-ci.yml' do
@@ -25,7 +38,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
       context 'template includes are array' do
         let(:gitlab_ci_content) { existing_gitlab_ci_and_template_array_without_sast }
 
-        subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
+        subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
 
         it 'generates the correct YML' do
           expect(result.first[:action]).to eq('update')
@@ -36,7 +49,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
       context 'template include is not an array' do
         let(:gitlab_ci_content) { existing_gitlab_ci_and_single_template_without_sast }
 
-        subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
+        subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
 
         it 'generates the correct YML' do
           expect(result.first[:action]).to eq('update')
@@ -48,7 +61,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
     context 'sast template include is not an array' do
       let(:gitlab_ci_content) { existing_gitlab_ci_and_single_template_with_sast_and_default_stage }
 
-      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
+      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
 
       it 'generates the correct YML' do
         expect(result.first[:action]).to eq('update')
@@ -60,7 +73,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
       let(:params) { default_sast_values }
       let(:gitlab_ci_content) { existing_gitlab_ci_and_single_template_with_sast_and_default_stage }
 
-      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
+      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
 
       it 'generates the correct YML' do
         expect(result.first[:content]).to eq(sast_yaml_with_no_variables_set)
@@ -69,15 +82,22 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
 
     context 'with update stage and SEARCH_MAX_DEPTH and set SECURE_ANALYZERS_PREFIX to default' do
       let(:params) do
-        { 'stage' => 'brand_new_stage',
-          'SEARCH_MAX_DEPTH' => 5,
-          'SECURE_ANALYZERS_PREFIX' => 'registry.gitlab.com/gitlab-org/security-products/analyzers',
-          'SAST_EXCLUDED_PATHS' => 'spec,docs' }
+        { 'global' =>
+          [
+            { 'field' => 'SECURE_ANALYZERS_PREFIX', 'defaultValue' => 'registry.gitlab.com/gitlab-org/security-products/analyzers', 'value' => 'registry.gitlab.com/gitlab-org/security-products/analyzers' }
+          ],
+          'pipeline' =>
+          [
+            { 'field' => 'stage', 'defaultValue' => 'test', 'value' => 'brand_new_stage' },
+            { 'field' => 'SEARCH_MAX_DEPTH', 'defaultValue' => 4, 'value' => 5 },
+            { 'field' => 'SAST_ANALYZER_IMAGE_TAG', 'defaultValue' => 2, 'value' => 2 },
+            { 'field' => 'SAST_EXCLUDED_PATHS', 'defaultValue' => 'spec, test, tests, tmp', 'value' => 'spec,docs' }
+          ] }
       end
 
       let(:gitlab_ci_content) { existing_gitlab_ci }
 
-      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
+      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
 
       it 'generates the correct YML' do
         expect(result.first[:action]).to eq('update')
@@ -88,7 +108,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
     context 'with no existing variables' do
       let(:gitlab_ci_content) { existing_gitlab_ci_with_no_variables }
 
-      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
+      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
 
       it 'generates the correct YML' do
         expect(result.first[:action]).to eq('update')
@@ -99,7 +119,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
     context 'with no existing sast config' do
       let(:gitlab_ci_content) { existing_gitlab_ci_with_no_sast_section }
 
-      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
+      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
 
       it 'generates the correct YML' do
         expect(result.first[:action]).to eq('update')
@@ -110,7 +130,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
     context 'with no existing sast variables' do
       let(:gitlab_ci_content) { existing_gitlab_ci_with_no_sast_variables }
 
-      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
+      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
 
       it 'generates the correct YML' do
         expect(result.first[:action]).to eq('update')
@@ -173,9 +193,14 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
       let(:auto_devops_enabled) { false }
 
       context 'with one empty parameter' do
-        let(:params) { { 'SECURE_ANALYZERS_PREFIX' => '' } }
+        let(:params) do
+          { 'global' =>
+            [
+              { 'field' => 'SECURE_ANALYZERS_PREFIX', 'defaultValue' => 'registry.gitlab.com/gitlab-org/security-products/analyzers', 'value' => '' }
+            ] }
+        end
 
-        subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
+        subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
 
         it 'generates the correct YML' do
           expect(result.first[:content]).to eq(sast_yaml_with_no_variables_set)
@@ -183,7 +208,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
       end
 
       context 'with all parameters' do
-        subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
+        subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
 
         it 'generates the correct YML' do
           expect(result.first[:content]).to eq(sast_yaml_all_params)
@@ -194,7 +219,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
     context 'with autodevops enabled' do
       let(:auto_devops_enabled) { true }
 
-      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
+      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
 
       before do
         allow_any_instance_of(described_class).to receive(:auto_devops_stages).and_return(fast_auto_devops_stages)