Merge branch 'rf-dont-write-defaults' into 'master'

Do not write SAST defaults via SAST Config UI

See merge request gitlab-org/gitlab!40030

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

@@ -23,10 +23,7 @@ module Security
       private
 
       def attributes
-        gitlab_ci_yml = @project.repository.gitlab_ci_yml_for(@project.repository.root_ref_sha)
-        existing_gitlab_ci_content = YAML.safe_load(gitlab_ci_yml) if gitlab_ci_yml
-
-        actions = Security::CiConfiguration::SastBuildActions.new(@project.auto_devops_enabled?, @params, existing_gitlab_ci_content).generate
+        actions = Security::CiConfiguration::SastBuildActions.new(@project.auto_devops_enabled?, @params, existing_gitlab_ci_content, default_sast_values).generate
 
         @project.repository.add_branch(@current_user, @branch_name, @project.default_branch)
         message = _('Set .gitlab-ci.yml to enable or configure SAST')
@@ -39,6 +36,19 @@ module Security
         }
       end
 
+      def existing_gitlab_ci_content
+        gitlab_ci_yml = @project.repository.gitlab_ci_yml_for(@project.repository.root_ref_sha)
+        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-dont-write-defaults.yml

+---
+title: Do not write SAST defaults via SAST Config UI
+merge_request: 40030
+author:
+type: changed

ee/lib/security/ci_configuration/sast_build_actions.rb

@@ -3,10 +3,11 @@
 module Security
   module CiConfiguration
     class SastBuildActions
-      def initialize(auto_devops_enabled, params, existing_gitlab_ci_content)
+      def initialize(auto_devops_enabled, params, existing_gitlab_ci_content, default_sast_values)
         @auto_devops_enabled = auto_devops_enabled
         @params = params
         @existing_gitlab_ci_content = existing_gitlab_ci_content || {}
+        @default_sast_values = default_sast_values
       end
 
       def generate
@@ -51,14 +52,18 @@ module Security
         @params['stage'].presence ? @params['stage'] : 'test'
       end
 
-      # We only want to write variables that are set
       def set_variables(variables, hash_to_update = {})
         hash_to_update['variables'] ||= {}
-        variables.each do |k, v|
-          hash_to_update['variables'][k] = @params[k]
+
+        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]
+          else
+            hash_to_update['variables'].delete(key)
+          end
         end
 
-        hash_to_update['variables'].select { |k, v| v.present? }
+        hash_to_update['variables']
       end
 
       def set_sast_block

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

@@ -3,6 +3,13 @@
 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" }
+  end
+
   context 'with existing .gitlab-ci.yml' do
     let(:auto_devops_enabled) { false }
 
@@ -17,7 +24,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions 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).generate }
+        subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
 
         it 'generates the correct YML' do
           expect(result.first[:action]).to eq('update')
@@ -35,7 +42,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions 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).generate }
+        subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
 
         it 'generates the correct YML' do
           expect(result.first[:action]).to eq('update')
@@ -55,7 +62,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
 
       let(:gitlab_ci_content) { existing_gitlab_ci_and_single_template_with_sast }
 
-      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
+      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
 
       it 'generates the correct YML' do
         expect(result.first[:action]).to eq('update')
@@ -63,17 +70,28 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
       end
     end
 
-    context 'with an update to the stage and a variable' do
+    context 'with default values' do
+      let(:params) { default_sast_values }
+      let(:gitlab_ci_content) { nil }
+
+      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
+
+      it 'generates the correct YML' do
+        expect(result.first[:content]).to eq(sast_yaml_with_no_variables_set)
+      end
+    end
+
+    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' => 1,
-          'SECURE_ANALYZERS_PREFIX' => 'new_registry',
+          'SEARCH_MAX_DEPTH' => 5,
+          'SECURE_ANALYZERS_PREFIX' => 'registry.gitlab.com/gitlab-org/security-products/analyzers',
           'SAST_EXCLUDED_PATHS' => 'spec,docs' }
       end
 
       let(:gitlab_ci_content) { existing_gitlab_ci }
 
-      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
+      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
 
       it 'generates the correct YML' do
         expect(result.first[:action]).to eq('update')
@@ -91,7 +109,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
 
       let(:gitlab_ci_content) { existing_gitlab_ci_with_no_variables }
 
-      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
+      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
 
       it 'generates the correct YML' do
         expect(result.first[:action]).to eq('update')
@@ -109,7 +127,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
 
       let(:gitlab_ci_content) { existing_gitlab_ci_with_no_sast_section }
 
-      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
+      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
 
       it 'generates the correct YML' do
         expect(result.first[:action]).to eq('update')
@@ -127,7 +145,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
 
       let(:gitlab_ci_content) { existing_gitlab_ci_with_no_sast_variables }
 
-      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
+      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
 
       it 'generates the correct YML' do
         expect(result.first[:action]).to eq('update')
@@ -177,7 +195,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
 
     def existing_gitlab_ci
       { "stages" => %w(test security),
-       "variables" => { "RANDOM" => "make sure this persists", "SECURE_ANALYZERS_PREFIX" => "localhost:5000/analyzers" },
+       "variables" => { "RANDOM" => "make sure this persists", "SECURE_ANALYZERS_PREFIX" => "bad_prefix" },
        "sast" => { "variables" => { "SAST_ANALYZER_IMAGE_TAG" => 2, "SEARCH_MAX_DEPTH" => 1 }, "stage" => "security" },
        "include" => [{ "template" => "Security/SAST.gitlab-ci.yml" }] }
     end
@@ -192,10 +210,10 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
       context 'with one empty parameter' do
         let(:params) { { 'SECURE_ANALYZERS_PREFIX' => '' } }
 
-        subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
+        subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
 
         it 'generates the correct YML' do
-          expect(result.first[:content]).to eq(sast_yaml_with_nothing_set)
+          expect(result.first[:content]).to eq(sast_yaml_with_no_variables_set)
         end
       end
 
@@ -208,7 +226,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
             'SAST_EXCLUDED_PATHS' => 'docs' }
         end
 
-        subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
+        subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
 
         it 'generates the correct YML' do
           expect(result.first[:content]).to eq(sast_yaml_all_params)
@@ -220,7 +238,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
       let(:auto_devops_enabled) { true }
       let(:params) { { 'stage' => 'custom stage' } }
 
-      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content).generate }
+      subject(:result) { described_class.new(auto_devops_enabled, params, gitlab_ci_content, default_sast_values).generate }
 
       before do
         allow_any_instance_of(described_class).to receive(:auto_devops_stages).and_return(fast_auto_devops_stages)
@@ -238,7 +256,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
     auto_devops_template['stages']
   end
 
-  def sast_yaml_with_nothing_set
+  def sast_yaml_with_no_variables_set
     <<-CI_YML.strip_heredoc
     # You can override the included template(s) by including variable overrides
     # See https://docs.gitlab.com/ee/user/application_security/sast/#customizing-the-sast-settings
@@ -266,7 +284,6 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
         SECURE_ANALYZERS_PREFIX: localhost:5000/analyzers
       sast:
         variables:
-          SAST_ANALYZER_IMAGE_TAG: 2
           SAST_EXCLUDED_PATHS: docs
           SEARCH_MAX_DEPTH: 1
         stage: security
@@ -404,11 +421,10 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
       - brand_new_stage
       variables:
         RANDOM: make sure this persists
-        SECURE_ANALYZERS_PREFIX: new_registry
       sast:
         variables:
           SAST_EXCLUDED_PATHS: spec,docs
-          SEARCH_MAX_DEPTH: 1
+          SEARCH_MAX_DEPTH: 5
         stage: brand_new_stage
       include:
       - template: Security/SAST.gitlab-ci.yml

ee/spec/services/security/ci_configuration/sast_create_service_spec.rb

@@ -21,5 +21,19 @@ RSpec.describe Security::CiConfiguration::SastCreateService do
         expect(result[:success_path]).to match(/#{Gitlab::Routing.url_helpers.project_new_merge_request_url(project, {})}(.*)description(.*)source_branch/)
       end
     end
+
+    context 'with parameters' do
+      let(:params) do
+        { 'stage' => 'security',
+          'SEARCH_MAX_DEPTH' => 1,
+          'SECURE_ANALYZERS_PREFIX' => 'new_registry',
+          'SAST_EXCLUDED_PATHS' => 'spec,docs' }
+      end
+
+      it 'returns the path to create a new merge request' do
+        expect(result[:status]).to eq(:success)
+        expect(result[:success_path]).to match(/#{Gitlab::Routing.url_helpers.project_new_merge_request_url(project, {})}(.*)description(.*)source_branch/)
+      end
+    end
   end
 end