Merge branch 'update-gitlab-dangerfiles-2' into 'master'

Update gitlab-dangerfiles to 3.0.0 which import plugins/rules automatically

See merge request gitlab-org/gitlab!82026

.gitlab/ci/frontend.gitlab-ci.yml

@@ -335,10 +335,13 @@ bundle-size-review:
   stage: test
   needs: ["compile-production-assets"]
   script:
+    - source scripts/utils.sh
     - mkdir -p bundle-size-review
     - cp webpack-report/index.html bundle-size-review/bundle-report.html
     - yarn global add https://gitlab.com/gitlab-org/frontend/playground/webpack-memory-metrics.git
-    - danger --dangerfile=danger/bundle_size/Dangerfile --fail-on-errors=true --verbose --danger_id=bundle-size-review
+    - |
+      danger_id=$(echo -n ${DANGER_GITLAB_API_TOKEN} | md5sum | awk '{print $1}' | cut -c5-10)
+      run_timed_command "danger --dangerfile=danger/Dangerfile-bundle_size --fail-on-errors=true --verbose --danger_id=bundle-size-review-${danger_id}"
   artifacts:
     when: always
     name: bundle-size-review

Dangerfile

@@ -11,16 +11,8 @@ project_name = ee? ? 'gitlab' : 'gitlab-foss'
 
 Gitlab::Dangerfiles.for_project(self, project_name) do |gitlab_dangerfiles|
   gitlab_dangerfiles.import_plugins
+  gitlab_dangerfiles.config.ci_only_rules = ProjectHelper::CI_ONLY_RULES
+  gitlab_dangerfiles.config.files_to_category = ProjectHelper::CATEGORIES
 
-  unless helper.release_automation?
-    danger.import_plugin('danger/plugins/*.rb')
-    gitlab_dangerfiles.import_dangerfiles(except: %w[simple_roulette])
-    gitlab_dangerfiles.config.files_to_category = ProjectHelper::CATEGORIES
-  end
-end
-
-return if helper.release_automation?
-
-project_helper.rule_names.each do |rule|
-  danger.import_dangerfile(path: File.join('danger', rule))
+  gitlab_dangerfiles.import_dangerfiles(except: %w[simple_roulette])
 end

Gemfile

@@ -403,7 +403,7 @@ group :development, :test do
 end
 
 group :development, :test, :danger do
-  gem 'gitlab-dangerfiles', '~> 2.11.0', require: false
+  gem 'gitlab-dangerfiles', '~> 3.0', require: false
 end
 
 group :development, :test, :coverage do

Gemfile.lock

@@ -224,7 +224,7 @@ GEM
     css_parser (1.7.0)
       addressable
     daemons (1.3.1)
-    danger (8.4.5)
+    danger (8.5.0)
       claide (~> 1.0)
       claide-plugins (>= 0.9.2)
       colored2 (~> 3.1)
@@ -463,9 +463,10 @@ GEM
       terminal-table (~> 1.5, >= 1.5.1)
     gitlab-chronic (0.10.5)
       numerizer (~> 0.2)
-    gitlab-dangerfiles (2.11.0)
+    gitlab-dangerfiles (3.0.0)
       danger (>= 8.4.5)
       danger-gitlab (>= 8.0.0)
+      rake
     gitlab-experiment (0.7.0)
       activesupport (>= 3.0)
       request_store (>= 1.0)
@@ -1494,7 +1495,7 @@ DEPENDENCIES
   gitaly (~> 14.9.0.pre.rc4)
   github-markup (~> 1.7.0)
   gitlab-chronic (~> 0.10.5)
-  gitlab-dangerfiles (~> 2.11.0)
+  gitlab-dangerfiles (~> 3.0)
   gitlab-experiment (~> 0.7.0)
   gitlab-fog-azure-rm (~> 1.2.0)
   gitlab-labkit (~> 0.22.0)

Rakefile

@@ -16,3 +16,6 @@ require File.expand_path('config/initializers/01_active_record_database_tasks_co
 Gitlab::Application.load_tasks
 
 Knapsack.load_tasks if defined?(Knapsack)
+
+require 'gitlab-dangerfiles'
+Gitlab::Dangerfiles.load_tasks

danger/bundle_size/Dangerfile → danger/Dangerfile-bundle_size

 # frozen_string_literal: true
+# This file isn't named "Dangerfile" so that it's not imported by default since it's only meant to be run in the `bundle-size-review` job.
 
 analysis_result = "./bundle-size-review/analysis.json"
 markdown_result = "./bundle-size-review/comparison.md"

danger/ci_templates/Dangerfile

@@ -19,7 +19,7 @@ return unless helper.ci?
 
 template_paths_to_review = helper.changes_by_category[:ci_template]
 
-if gitlab.mr_labels.include?('ci::templates') || template_paths_to_review.any?
+if helper.mr_labels.include?('ci::templates') || template_paths_to_review.any?
   message 'This merge request adds or changes files that require a ' \
     'review from the CI/CD Templates maintainers.'
 

danger/database/Dangerfile

@@ -49,11 +49,11 @@ if geo_migration_created && !geo_db_schema_updated
 end
 
 return unless helper.ci?
-return if gitlab.mr_labels.include?(DATABASE_APPROVED_LABEL)
+return if helper.mr_labels.include?(DATABASE_APPROVED_LABEL)
 
 db_paths_to_review = helper.changes_by_category[:database]
 
-if gitlab.mr_labels.include?('database') || db_paths_to_review.any?
+if helper.mr_labels.include?('database') || db_paths_to_review.any?
   message 'This merge request adds or changes files that require a ' \
     'review from the [Database team](https://gitlab.com/groups/gl-database/-/group_members).'
 

danger/feature_flag/Dangerfile

@@ -22,21 +22,21 @@ def check_feature_flag_yaml(feature_flag)
   end
 rescue Psych::Exception
   # YAML could not be parsed, fail the build.
-  fail "#{gitlab.html_link(feature_flag.path)} isn't valid YAML! #{SEE_DOC}"
+  fail "#{helper.html_link(feature_flag.path)} isn't valid YAML! #{SEE_DOC}"
 rescue StandardError => e
   warn "There was a problem trying to check the Feature Flag file. Exception: #{e.class.name} - #{e.message}"
 end
 
 def message_for_feature_flag_missing_group!(feature_flag:, mr_group_label:)
   if mr_group_label.nil?
-    warn "Consider setting `group` in #{gitlab.html_link(feature_flag.path)}. #{SEE_DOC}"
+    warn "Consider setting `group` in #{helper.html_link(feature_flag.path)}. #{SEE_DOC}"
   else
     mr_line = feature_flag.raw.lines.find_index("group:\n")
 
     if mr_line
       markdown(format(SUGGEST_MR_COMMENT, group: mr_group_label), file: feature_flag.path, line: mr_line.succ)
     else
-      warn %(Consider setting `group: "#{mr_group_label}"` in #{gitlab.html_link(feature_flag.path)}. #{SEE_DOC})
+      warn %(Consider setting `group: "#{mr_group_label}"` in #{helper.html_link(feature_flag.path)}. #{SEE_DOC})
     end
   end
 end
@@ -60,7 +60,7 @@ def message_for_feature_flag_with_group!(feature_flag:, mr_group_label:)
   if mr_group_label.nil?
     helper.labels_to_add << feature_flag.group
   else
-    fail %(`group` is set to ~"#{feature_flag.group}" in #{gitlab.html_link(feature_flag.path)}, which does not match ~"#{mr_group_label}" set on the MR!)
+    fail %(`group` is set to ~"#{feature_flag.group}" in #{helper.html_link(feature_flag.path)}, which does not match ~"#{mr_group_label}" set on the MR!)
   end
 end
 

danger/specialization_labels/Dangerfile

@@ -16,11 +16,11 @@ SPECIALIZATIONS = {
 labels_to_add = helper.changes_by_category.each_with_object([]) do |(category, _changes), memo|
   label = SPECIALIZATIONS[category]
   next unless label
-  next if gitlab.mr_labels.include?(label)
+  next if helper.mr_labels.include?(label)
 
   # Don't override already-set scoped labels.
   label_scope = label.split('::')[0...-1].join('::')
-  next if !label_scope.empty? && gitlab.mr_labels.any? { |mr_label| mr_label.start_with?(label_scope) }
+  next if !label_scope.empty? && helper.has_scoped_label_with_scope?(label_scope)
 
   memo << label
 end

danger/specs/Dangerfile

@@ -37,7 +37,7 @@ has_ee_app_changes = all_changed_files.grep(%r{\Aee/(app|lib|db/(geo/)?(post_)?m
 spec_changes = specs.changed_specs_files(ee: :exclude)
 has_spec_changes = spec_changes.any?
 has_ee_spec_changes = specs.changed_specs_files(ee: :only).any?
-new_specs_needed = (gitlab.mr_labels & NO_SPECS_LABELS).empty?
+new_specs_needed = (helper.mr_labels & NO_SPECS_LABELS).empty?
 
 if (has_app_changes || has_ee_app_changes) && !(has_spec_changes || has_ee_spec_changes) && new_specs_needed
   warn format(NO_NEW_SPEC_MESSAGE, labels: helper.labels_list(NO_SPECS_LABELS)), sticky: false

danger/z_metadata/Dangerfile

@@ -14,12 +14,12 @@ end
 
 has_milestone = !gitlab.mr_json["milestone"].nil?
 
-unless has_milestone || (helper.security_mr? && gitlab.branch_for_base == DEFAULT_BRANCH)
+unless has_milestone || (helper.security_mr? && helper.mr_target_branch == DEFAULT_BRANCH)
   warn "This merge request does not refer to an existing milestone.", sticky: false
 end
 
-has_pick_into_stable_label = gitlab.mr_labels.find { |label| label.start_with?('Pick into') }
+has_pick_into_stable_label = helper.mr_labels.find { |label| label.start_with?('Pick into') }
 
-if gitlab.branch_for_base != DEFAULT_BRANCH && !has_pick_into_stable_label && !helper.security_mr?
+if helper.mr_target_branch != DEFAULT_BRANCH && !has_pick_into_stable_label && !helper.security_mr?
   warn "Most of the time, merge requests should target `#{DEFAULT_BRANCH}`. Otherwise, please set the relevant `Pick into X.Y` label."
 end

lefthook.yml

@@ -2,7 +2,7 @@ pre-push:
   parallel: true
   commands:
     danger:
-      run: CI_PROJECT_DIR=. bundle exec danger dry_run
+      run: bundle exec rake danger_local
     eslint:
       tags: frontend style
       files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD

lib/tasks/gitlab_danger.rake

-# frozen_string_literal: true
-
-desc 'Run local Danger rules'
-task :danger_local do
-  require_relative '../../tooling/danger/project_helper'
-  require 'gitlab/popen'
-
-  puts("#{Tooling::Danger::ProjectHelper.local_warning_message}\n")
-
-  # _status will _always_ be 0, regardless of failure or success :(
-  output, _status = Gitlab::Popen.popen(%w{danger dry_run})
-
-  if output.empty?
-    puts(Tooling::Danger::ProjectHelper.success_message)
-  else
-    puts(output)
-    exit(1)
-  end
-end

spec/tooling/danger/project_helper_spec.rb

@@ -276,40 +276,6 @@ RSpec.describe Tooling::Danger::ProjectHelper do
     end
   end
 
-  describe '.local_warning_message' do
-    it 'returns an informational message with rules that can run' do
-      expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: ci_config, database, documentation, duplicate_yarn_dependencies, eslint, gitaly, pajamas, pipeline, prettier, product_intelligence, utility_css, vue_shared_documentation, datateam')
-    end
-  end
-
-  describe '.success_message' do
-    it 'returns an informational success message' do
-      expect(described_class.success_message).to eq('==> No Danger rule violations!')
-    end
-  end
-
-  describe '#rule_names' do
-    context 'when running locally' do
-      before do
-        expect(fake_helper).to receive(:ci?).and_return(false)
-      end
-
-      it 'returns local only rules' do
-        expect(project_helper.rule_names).to match_array(described_class::LOCAL_RULES)
-      end
-    end
-
-    context 'when running under CI' do
-      before do
-        expect(fake_helper).to receive(:ci?).and_return(true)
-      end
-
-      it 'returns all rules' do
-        expect(project_helper.rule_names).to eq(described_class::LOCAL_RULES | described_class::CI_ONLY_RULES)
-      end
-    end
-  end
-
   describe '#file_lines' do
     let(:filename) { 'spec/foo_spec.rb' }
     let(:file_spy) { spy }

tooling/danger/project_helper.rb

@@ -3,22 +3,6 @@
 module Tooling
   module Danger
     module ProjectHelper
-      LOCAL_RULES ||= %w[
-        ci_config
-        database
-        documentation
-        duplicate_yarn_dependencies
-        eslint
-        gitaly
-        pajamas
-        pipeline
-        prettier
-        product_intelligence
-        utility_css
-        vue_shared_documentation
-        datateam
-      ].freeze
-
       CI_ONLY_RULES ||= %w[
         ce_ee_vue_templates
         ci_templates
@@ -31,8 +15,6 @@ module Tooling
         z_metadata
       ].freeze
 
-      MESSAGE_PREFIX = '==>'
-
       # First-match win, so be sure to put more specific regex at the top...
       CATEGORIES = {
         [%r{usage_data\.rb}, %r{^(\+|-).*\s+(count|distinct_count|estimate_batch_distinct_count)\(.*\)(.*)$}] => [:database, :backend, :product_intelligence],
@@ -181,20 +163,6 @@ module Tooling
         %r{\.js\z} => :frontend
       }.freeze
 
-      def local_warning_message
-        "#{MESSAGE_PREFIX} Only the following Danger rules can be run locally: #{LOCAL_RULES.join(', ')}"
-      end
-      module_function :local_warning_message # rubocop:disable Style/AccessModifierDeclarations
-
-      def success_message
-        "#{MESSAGE_PREFIX} No Danger rule violations!"
-      end
-      module_function :success_message # rubocop:disable Style/AccessModifierDeclarations
-
-      def rule_names
-        helper.ci? ? LOCAL_RULES | CI_ONLY_RULES : LOCAL_RULES
-      end
-
       def file_lines(filename)
         read_file(filename).lines(chomp: true)
       end