Merge branch 'rf-sast-config-update-mr' into 'master'

Add support for updating SAST config

See merge request gitlab-org/gitlab!39269

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

@@ -7,7 +7,7 @@ module Security
         @project = project
         @current_user = current_user
         @params = params
-        @branch_name = @project.repository.next_branch('add-sast-config')
+        @branch_name = @project.repository.next_branch('set-sast-config')
       end
 
       def execute
@@ -23,10 +23,13 @@ module Security
       private
 
       def attributes
-        actions = Security::CiConfiguration::SastBuildActions.new(@project.auto_devops_enabled?, @params).generate
+        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
 
         @project.repository.add_branch(@current_user, @branch_name, @project.default_branch)
-        message = _('Add .gitlab-ci.yml to enable or configure SAST')
+        message = _('Set .gitlab-ci.yml to enable or configure SAST')
 
         {
           commit_message: message,
@@ -37,7 +40,7 @@ module Security
       end
 
       def successful_change_path
-        description = _('Add .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.')
+        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 }
         Gitlab::Routing.url_helpers.project_new_merge_request_url(@project, merge_request: merge_request_params)
       end

ee/changelogs/unreleased/rf-sast-config-update-mr.yml

+---
+title: Add support for updating SAST config
+merge_request: 39269
+author:
+type: added

ee/lib/security/ci_configuration/sast_build_actions.rb

@@ -3,31 +3,43 @@
 module Security
   module CiConfiguration
     class SastBuildActions
-      def initialize(auto_devops_enabled, params)
+      def initialize(auto_devops_enabled, params, existing_gitlab_ci_content)
         @auto_devops_enabled = auto_devops_enabled
         @params = params
+        @existing_gitlab_ci_content = existing_gitlab_ci_content || {}
       end
 
       def generate
-        config = {
-          'stages' => stages,
-          'variables' => parse_variables(global_variables),
-          'sast' => sast_block,
-          'include' => [{ 'template' => template }]
-        }.select { |k, v| v.present? }
+        action = @existing_gitlab_ci_content.present? ? 'update' : 'create'
 
-        content = config.to_yaml
-        content << "# You can override the above template(s) by including variable overrides\n"
-        content << "# See https://docs.gitlab.com/ee/user/application_security/sast/#customizing-the-sast-settings\n"
+        update_existing_content!
 
-        [{ action: 'create', file_path: '.gitlab-ci.yml', content: content }]
+        [{ action: action, file_path: '.gitlab-ci.yml', content: prepare_existing_content }]
       end
 
       private
 
-      def stages
+      def update_existing_content!
+        @existing_gitlab_ci_content['stages'] = set_stages
+        @existing_gitlab_ci_content['variables'] = set_variables(global_variables, @existing_gitlab_ci_content)
+        @existing_gitlab_ci_content['sast'] = set_sast_block
+        @existing_gitlab_ci_content['include'] = set_includes
+
+        @existing_gitlab_ci_content.select! { |k, v| v.present? }
+        @existing_gitlab_ci_content['sast'].select! { |k, v| v.present? }
+      end
+
+      def set_includes
+        includes = @existing_gitlab_ci_content['include'] || []
+        includes = includes.is_a?(Array) ? includes : [includes]
+        includes << { 'template' => template }
+        includes.uniq
+      end
+
+      def set_stages
+        existing_stages = @existing_gitlab_ci_content['stages'] || []
         base_stages = @auto_devops_enabled ? auto_devops_stages : ['test']
-        (base_stages + [sast_stage]).uniq
+        (existing_stages + base_stages + [sast_stage]).uniq
       end
 
       def auto_devops_stages
@@ -40,24 +52,46 @@ module Security
       end
 
       # We only want to write variables that are set
-      def parse_variables(variables)
-        variables.map { |var| [var, @params[var]] }
-                 .to_h
-                 .select { |k, v| v.present? }
+      def set_variables(variables, hash_to_update = {})
+        hash_to_update['variables'] ||= {}
+        variables.each do |k, v|
+          hash_to_update['variables'][k] = @params[k]
+        end
+
+        hash_to_update['variables'].select { |k, v| v.present? }
+      end
+
+      def set_sast_block
+        sast_content = @existing_gitlab_ci_content['sast'] || {}
+        sast_content['variables'] = set_variables(sast_variables)
+        sast_content['stage'] = sast_stage
+        sast_content.select { |k, v| v.present? }
+      end
+
+      def prepare_existing_content
+        content = @existing_gitlab_ci_content.to_yaml
+        content = remove_document_delimeter(content)
+
+        content.prepend(sast_comment)
+      end
+
+      def remove_document_delimeter(content)
+        content.gsub(/^---\n/, '')
       end
 
-      def sast_block
-        {
-          'variables' => parse_variables(sast_variables),
-          'stage' => sast_stage,
-          'script' => ['/analyzer run']
-        }.select { |k, v| v.present? }
+      def sast_comment
+        <<~YAML
+          # 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
+          # Note that environment variables can be set in several places
+          # See https://docs.gitlab.com/ee/ci/variables/#priority-of-environment-variables
+        YAML
       end
 
       def template
         return 'Auto-DevOps.gitlab-ci.yml' if @auto_devops_enabled
 
-        'SAST.gitlab-ci.yml'
+        'Security/SAST.gitlab-ci.yml'
       end
 
       def global_variables

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

 # frozen_string_literal: true
 
-require 'spec_helper'
+require 'fast_spec_helper'
 
 RSpec.describe Security::CiConfiguration::SastBuildActions do
-  context 'autodevops disabled' do
+  context 'with existing .gitlab-ci.yml' do
     let(:auto_devops_enabled) { false }
 
-    context 'with empty parameters' do
+    context 'sast has not been included' do
+      context 'template includes are array' do
         let(:params) do
-        { 'stage' => '',
-          'SECURE_ANALYZERS_PREFIX' => '',
-          'SEARCH_MAX_DEPTH' => '' }
+          { 'stage' => 'security',
+            'SEARCH_MAX_DEPTH' => 1,
+            'SECURE_ANALYZERS_PREFIX' => 'new_registry',
+            'SAST_EXCLUDED_PATHS' => 'spec,docs' }
         end
 
-      subject(:result) { described_class.new(auto_devops_enabled, params).generate }
+        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 }
 
         it 'generates the correct YML' do
-        expect(result.first[:content]).to eq(sast_yaml_no_params)
+          expect(result.first[:action]).to eq('update')
+          expect(result.first[:content]).to eq(sast_yaml_two_includes)
+        end
+      end
+
+      context 'template include is not an array' do
+        let(:params) do
+          { 'stage' => 'security',
+            'SEARCH_MAX_DEPTH' => 1,
+            'SECURE_ANALYZERS_PREFIX' => 'new_registry',
+            'SAST_EXCLUDED_PATHS' => 'spec,docs' }
+        end
+
+        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 }
+
+        it 'generates the correct YML' do
+          expect(result.first[:action]).to eq('update')
+          expect(result.first[:content]).to eq(sast_yaml_two_includes)
+        end
+      end
+    end
+
+    context 'sast template include is not an array' do
+      let(:params) do
+        { 'stage' => 'security',
+          'SEARCH_MAX_DEPTH' => 1,
+          'SECURE_ANALYZERS_PREFIX' => 'localhost:5000/analyzers',
+          'SAST_ANALYZER_IMAGE_TAG' => 2,
+          'SAST_EXCLUDED_PATHS' => 'docs' }
+      end
+
+      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 }
+
+      it 'generates the correct YML' do
+        expect(result.first[:action]).to eq('update')
+        expect(result.first[:content]).to eq(sast_yaml_all_params)
+      end
+    end
+
+    context 'with an update to the stage and a variable' do
+      let(:params) do
+        { 'stage' => 'brand_new_stage',
+          'SEARCH_MAX_DEPTH' => 1,
+          'SECURE_ANALYZERS_PREFIX' => 'new_registry',
+          '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 }
+
+      it 'generates the correct YML' do
+        expect(result.first[:action]).to eq('update')
+        expect(result.first[:content]).to eq(sast_yaml_updated_stage)
+      end
+    end
+
+    context 'with no existing variables' do
+      let(:params) do
+        { 'stage' => 'security',
+          'SEARCH_MAX_DEPTH' => 1,
+          'SECURE_ANALYZERS_PREFIX' => 'new_registry',
+          'SAST_EXCLUDED_PATHS' => 'spec,docs' }
+      end
+
+      let(:gitlab_ci_content) { existing_gitlab_ci_with_no_variables }
+
+      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')
+        expect(result.first[:content]).to eq(sast_yaml_variable_section_added)
+      end
+    end
+
+    context 'with no existing sast config' do
+      let(:params) do
+        { 'stage' => 'security',
+          'SEARCH_MAX_DEPTH' => 1,
+          'SECURE_ANALYZERS_PREFIX' => 'new_registry',
+          'SAST_EXCLUDED_PATHS' => 'spec,docs' }
+      end
+
+      let(:gitlab_ci_content) { existing_gitlab_ci_with_no_sast_section }
+
+      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')
+        expect(result.first[:content]).to eq(sast_yaml_sast_section_added)
+      end
+    end
+
+    context 'with no existing sast variables' do
+      let(:params) do
+        { 'stage' => 'security',
+          'SEARCH_MAX_DEPTH' => 1,
+          'SECURE_ANALYZERS_PREFIX' => 'new_registry',
+          'SAST_EXCLUDED_PATHS' => 'spec,docs' }
+      end
+
+      let(:gitlab_ci_content) { existing_gitlab_ci_with_no_sast_variables }
+
+      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')
+        expect(result.first[:content]).to eq(sast_yaml_sast_variables_section_added)
+      end
+    end
+
+    def existing_gitlab_ci_and_template_array_without_sast
+      { "stages" => %w(test security),
+       "variables" => { "RANDOM" => "make sure this persists", "SECURE_ANALYZERS_PREFIX" => "localhost:5000/analyzers" },
+       "sast" => { "variables" => { "SAST_ANALYZER_IMAGE_TAG" => 2, "SEARCH_MAX_DEPTH" => 1 }, "stage" => "security" },
+       "include" => [{ "template" => "existing.yml" }] }
+    end
+
+    def existing_gitlab_ci_and_single_template_with_sast
+      { "stages" => %w(test security),
+       "variables" => { "SECURE_ANALYZERS_PREFIX" => "localhost:5000/analyzers" },
+       "sast" => { "variables" => { "SAST_ANALYZER_IMAGE_TAG" => 2, "SEARCH_MAX_DEPTH" => 1 }, "stage" => "security" },
+       "include" => { "template" => "Security/SAST.gitlab-ci.yml" } }
+    end
+
+    def existing_gitlab_ci_and_single_template_without_sast
+      { "stages" => %w(test security),
+       "variables" => { "RANDOM" => "make sure this persists", "SECURE_ANALYZERS_PREFIX" => "localhost:5000/analyzers" },
+       "sast" => { "variables" => { "SAST_ANALYZER_IMAGE_TAG" => 2, "SEARCH_MAX_DEPTH" => 1 }, "stage" => "security" },
+       "include" => { "template" => "existing.yml" } }
+    end
+
+    def existing_gitlab_ci_with_no_variables
+      { "stages" => %w(test security),
+       "sast" => { "variables" => { "SAST_ANALYZER_IMAGE_TAG" => 2, "SEARCH_MAX_DEPTH" => 1 }, "stage" => "security" },
+       "include" => [{ "template" => "Security/SAST.gitlab-ci.yml" }] }
+    end
+
+    def existing_gitlab_ci_with_no_sast_section
+      { "stages" => %w(test security),
+       "variables" => { "RANDOM" => "make sure this persists", "SECURE_ANALYZERS_PREFIX" => "localhost:5000/analyzers" },
+       "include" => [{ "template" => "Security/SAST.gitlab-ci.yml" }] }
+    end
+
+    def existing_gitlab_ci_with_no_sast_variables
+      { "stages" => %w(test security),
+       "variables" => { "RANDOM" => "make sure this persists", "SECURE_ANALYZERS_PREFIX" => "localhost:5000/analyzers" },
+       "sast" => { "stage" => "security" },
+       "include" => [{ "template" => "Security/SAST.gitlab-ci.yml" }] }
+    end
+
+    def existing_gitlab_ci
+      { "stages" => %w(test security),
+       "variables" => { "RANDOM" => "make sure this persists", "SECURE_ANALYZERS_PREFIX" => "localhost:5000/analyzers" },
+       "sast" => { "variables" => { "SAST_ANALYZER_IMAGE_TAG" => 2, "SEARCH_MAX_DEPTH" => 1 }, "stage" => "security" },
+       "include" => [{ "template" => "Security/SAST.gitlab-ci.yml" }] }
+    end
+  end
+
+  context 'with no .gitlab-ci.yml' do
+    let(:gitlab_ci_content) { nil }
+
+    context 'autodevops disabled' do
+      let(:auto_devops_enabled) { false }
+
+      context 'with one empty parameter' do
+        let(:params) { { 'SECURE_ANALYZERS_PREFIX' => '' } }
+
+        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_nothing_set)
         end
       end
 
@@ -29,7 +208,7 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
             'SAST_EXCLUDED_PATHS' => 'docs' }
         end
 
-      subject(:result) { described_class.new(auto_devops_enabled, params).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)
@@ -41,32 +220,45 @@ 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).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)
+      end
 
       it 'generates the correct YML' do
         expect(result.first[:content]).to eq(auto_devops_with_custom_stage)
       end
     end
+  end
+
+  # stubbing this method allows this spec file to use fast_spec_helper
+  def fast_auto_devops_stages
+    auto_devops_template = YAML.safe_load( File.read('lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml') )
+    auto_devops_template['stages']
+  end
 
-  def sast_yaml_no_params
+  def sast_yaml_with_nothing_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
+    # Note that environment variables can be set in several places
+    # See https://docs.gitlab.com/ee/ci/variables/#priority-of-environment-variables
     stages:
     - test
     sast:
       stage: test
-      script:
-      - "/analyzer run"
     include:
-    - template: SAST.gitlab-ci.yml
-    # You can override the above template(s) by including variable overrides
-    # See https://docs.gitlab.com/ee/user/application_security/sast/#customizing-the-sast-settings
+    - template: Security/SAST.gitlab-ci.yml
     CI_YML
   end
 
   def sast_yaml_all_params
     <<-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
+      # Note that environment variables can be set in several places
+      # See https://docs.gitlab.com/ee/ci/variables/#priority-of-environment-variables
       stages:
       - test
       - security
@@ -78,18 +270,17 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
           SAST_EXCLUDED_PATHS: docs
           SEARCH_MAX_DEPTH: 1
         stage: security
-        script:
-        - "/analyzer run"
       include:
-      - template: SAST.gitlab-ci.yml
-      # You can override the above template(s) by including variable overrides
-      # See https://docs.gitlab.com/ee/user/application_security/sast/#customizing-the-sast-settings
+      - template: Security/SAST.gitlab-ci.yml
     CI_YML
   end
 
   def auto_devops_with_custom_stage
     <<-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
+      # Note that environment variables can be set in several places
+      # See https://docs.gitlab.com/ee/ci/variables/#priority-of-environment-variables
       stages:
       - build
       - test
@@ -108,12 +299,119 @@ RSpec.describe Security::CiConfiguration::SastBuildActions do
       - custom stage
       sast:
         stage: custom stage
-        script:
-        - "/analyzer run"
       include:
       - template: Auto-DevOps.gitlab-ci.yml
-      # You can override the above template(s) by including variable overrides
+    CI_YML
+  end
+
+  def sast_yaml_two_includes
+    <<-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
+      # Note that environment variables can be set in several places
+      # See https://docs.gitlab.com/ee/ci/variables/#priority-of-environment-variables
+      stages:
+      - test
+      - security
+      variables:
+        RANDOM: make sure this persists
+        SECURE_ANALYZERS_PREFIX: new_registry
+      sast:
+        variables:
+          SAST_EXCLUDED_PATHS: spec,docs
+          SEARCH_MAX_DEPTH: 1
+        stage: security
+      include:
+      - template: existing.yml
+      - template: Security/SAST.gitlab-ci.yml
+    CI_YML
+  end
+
+  def sast_yaml_variable_section_added
+    <<-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
+      # Note that environment variables can be set in several places
+      # See https://docs.gitlab.com/ee/ci/variables/#priority-of-environment-variables
+      stages:
+      - test
+      - security
+      sast:
+        variables:
+          SAST_EXCLUDED_PATHS: spec,docs
+          SEARCH_MAX_DEPTH: 1
+        stage: security
+      include:
+      - template: Security/SAST.gitlab-ci.yml
+      variables:
+        SECURE_ANALYZERS_PREFIX: new_registry
+    CI_YML
+  end
+
+  def sast_yaml_sast_section_added
+    <<-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
+      # Note that environment variables can be set in several places
+      # See https://docs.gitlab.com/ee/ci/variables/#priority-of-environment-variables
+      stages:
+      - test
+      - security
+      variables:
+        RANDOM: make sure this persists
+        SECURE_ANALYZERS_PREFIX: new_registry
+      include:
+      - template: Security/SAST.gitlab-ci.yml
+      sast:
+        variables:
+          SAST_EXCLUDED_PATHS: spec,docs
+          SEARCH_MAX_DEPTH: 1
+        stage: security
+    CI_YML
+  end
+
+  def sast_yaml_sast_variables_section_added
+    <<-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
+      # Note that environment variables can be set in several places
+      # See https://docs.gitlab.com/ee/ci/variables/#priority-of-environment-variables
+      stages:
+      - test
+      - security
+      variables:
+        RANDOM: make sure this persists
+        SECURE_ANALYZERS_PREFIX: new_registry
+      sast:
+        stage: security
+        variables:
+          SAST_EXCLUDED_PATHS: spec,docs
+          SEARCH_MAX_DEPTH: 1
+      include:
+      - template: Security/SAST.gitlab-ci.yml
+    CI_YML
+  end
+
+  def sast_yaml_updated_stage
+    <<-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
+      # Note that environment variables can be set in several places
+      # See https://docs.gitlab.com/ee/ci/variables/#priority-of-environment-variables
+      stages:
+      - test
+      - security
+      - 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
+        stage: brand_new_stage
+      include:
+      - template: Security/SAST.gitlab-ci.yml
     CI_YML
   end
 end

locale/gitlab.pot

@@ -1410,12 +1410,6 @@ msgstr[1] ""
 msgid "Add %{linkStart}assets%{linkEnd} to your Release. GitLab automatically includes read-only assets, like source code and release evidence."
 msgstr ""
 
-msgid "Add .gitlab-ci.yml to enable or configure SAST"
-msgstr ""
-
-msgid "Add .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."
-msgstr ""
-
 msgid "Add CHANGELOG"
 msgstr ""
 
@@ -22200,6 +22194,12 @@ msgstr ""
 msgid "Set %{epic_ref} as the parent epic."
 msgstr ""
 
+msgid "Set .gitlab-ci.yml to enable or configure SAST"
+msgstr ""
+
+msgid "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."
+msgstr ""
+
 msgid "Set a default template for issue descriptions."
 msgstr ""