Merge branch 'rf-update-sast-mutation' into 'master'

Update in preparation of supporting analyzers

See merge request gitlab-org/gitlab!42173

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

@@ -27,8 +27,7 @@ module Mutations
         def resolve(project_path:, configuration:)
           project = authorized_find!(full_path: project_path)
 
-          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
 
@@ -38,15 +37,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)