Add latest changes from gitlab-org/gitlab@14-0-stable-eev14.0.0-rc42

5 files changed, 51 insertions, 231 deletions

diff --git a/danger/commit_messages/Dangerfile b/danger/commit_messages/Dangerfile
deleted file mode 100644
index ac3d41adcf4..00000000000
--- a/danger/commit_messages/Dangerfile
+++ /dev/null
@@ -1,147 +0,0 @@
-# frozen_string_literal: true
-
-require 'gitlab/dangerfiles/commit_linter'
-require 'gitlab/dangerfiles/merge_request_linter'
-
-COMMIT_MESSAGE_GUIDELINES = "https://docs.gitlab.com/ee/development/contributing/merge_request_workflow.html#commit-messages-guidelines"
-MORE_INFO = "For more information, take a look at our [Commit message guidelines](#{COMMIT_MESSAGE_GUIDELINES})."
-THE_DANGER_JOB_TEXT = "the `danger-review` job"
-MAX_COMMITS_COUNT = 10
-MAX_COMMITS_COUNT_EXCEEDED_MESSAGE = <<~MSG
-This merge request includes more than %<max_commits_count>d commits. Each commit should meet the following criteria:
-
-1. Have a well-written commit message.
-1. Has all tests passing when used on its own (e.g. when using git checkout SHA).
-1. Can be reverted on its own without also requiring the revert of commit that came before it.
-1. Is small enough that it can be reviewed in isolation in under 30 minutes or so.
-
-If this merge request contains commits that do not meet this criteria and/or contains intermediate work, please rebase these commits into a smaller number of commits or split this merge request into multiple smaller merge requests.
-MSG
-
-def fail_commit(commit, message, more_info: true)
-  self.fail(build_message(commit, message, more_info: more_info))
-end
-
-def warn_commit(commit, message, more_info: true)
-  self.warn(build_message(commit, message, more_info: more_info))
-end
-
-def build_message(commit, message, more_info: true)
-  [message].tap do |full_message|
-    full_message << ". #{MORE_INFO}" if more_info
-    full_message.unshift("#{commit.sha}: ") if commit.sha
-  end.join
-end
-
-def squash_mr?
-  # Locally, we assume the MR is set to be squashed so that the user only sees warnings instead of errors.
-  helper.ci? ? gitlab.mr_json['squash'] : true
-end
-
-def wip_mr?
-  helper.ci? ? gitlab.mr_json['work_in_progress'] : false
-end
-
-def danger_job_link
-  helper.ci? ? "[#{THE_DANGER_JOB_TEXT}](#{ENV['CI_JOB_URL']})" : THE_DANGER_JOB_TEXT
-end
-
-# Perform various checks against commits. We're not using
-# https://github.com/jonallured/danger-commit_lint because its output is not
-# very helpful, and it doesn't offer the means of ignoring merge commits.
-def lint_commit(commit)
-  linter = Gitlab::Dangerfiles::CommitLinter.new(commit)
-
-  # For now we'll ignore merge commits, as getting rid of those is a problem
-  # separate from enforcing good commit messages.
-  return linter if linter.merge?
-
-  # We ignore revert commits as they are well structured by Git already
-  return linter if linter.revert?
-
-  # If MR is set to squash, we ignore fixup commits
-  return linter if linter.fixup? && squash_mr?
-
-  if linter.fixup?
-    msg = "Squash or fixup commits must be squashed before merge, or enable squash merge option and re-run #{danger_job_link}."
-    if wip_mr? || squash_mr?
-      warn_commit(commit, msg, more_info: false)
-    else
-      fail_commit(commit, msg, more_info: false)
-    end
-
-    # Makes no sense to process other rules for fixup commits, they trigger just more noise
-    return linter
-  end
-
-  # Fail if a suggestion commit is used and squash is not enabled
-  if linter.suggestion?
-    unless squash_mr?
-      fail_commit(commit, "If you are applying suggestions, enable squash in the merge request and re-run #{danger_job_link}.", more_info: false)
-    end
-
-    return linter
-  end
-
-  linter.lint
-end
-
-def lint_mr_title(mr_title)
-  commit = Struct.new(:message, :sha).new(mr_title)
-
-  Gitlab::Dangerfiles::MergeRequestLinter.new(commit).lint
-end
-
-def count_non_fixup_commits(commit_linters)
-  commit_linters.count { |commit_linter| !commit_linter.fixup? }
-end
-
-def lint_commits(commits)
-  commit_linters = commits.map { |commit| lint_commit(commit) }
-
-  if squash_mr?
-    multi_line_commit_linter = commit_linters.detect { |commit_linter| !commit_linter.merge? && commit_linter.multi_line? }
-
-    if multi_line_commit_linter && multi_line_commit_linter.failed?
-      warn_or_fail_commits(multi_line_commit_linter)
-      commit_linters.delete(multi_line_commit_linter) # Don't show an error (here) and a warning (below)
-    elsif helper.ci? # We don't have access to the MR title locally
-      title_linter = lint_mr_title(gitlab.mr_json['title'])
-      if title_linter.failed?
-        warn_or_fail_commits(title_linter)
-      end
-    end
-  else
-    if count_non_fixup_commits(commit_linters) > MAX_COMMITS_COUNT
-      self.warn(format(MAX_COMMITS_COUNT_EXCEEDED_MESSAGE, max_commits_count: MAX_COMMITS_COUNT))
-    end
-  end
-
-  failed_commit_linters = commit_linters.select { |commit_linter| commit_linter.failed? }
-  warn_or_fail_commits(failed_commit_linters, default_to_fail: !squash_mr?)
-end
-
-def warn_or_fail_commits(failed_linters, default_to_fail: true)
-  level = default_to_fail ? :fail : :warn
-
-  Array(failed_linters).each do |linter|
-    linter.problems.each do |problem_key, problem_desc|
-      case problem_key
-      when :subject_too_short, :subject_above_warning, :details_too_many_changes, :details_line_too_long
-        warn_commit(linter.commit, problem_desc)
-      else
-        self.__send__("#{level}_commit", linter.commit, problem_desc) # rubocop:disable GitlabSecurity/PublicSend
-      end
-    end
-  end
-end
-
-# As part of https://gitlab.com/groups/gitlab-org/-/epics/4826 we are
-# vendoring workhorse commits from the stand-alone gitlab-workhorse
-# repo. There is no point in linting commits that we want to vendor as
-# is.
-def workhorse_changes?
-  git.diff.any? { |file| file.path.start_with?('workhorse/') }
-end
-
-lint_commits(git.commits) unless workhorse_changes?
diff --git a/danger/feature_flag/Dangerfile b/danger/feature_flag/Dangerfile
index 88ce6393b64..bf2194724fc 100644
--- a/danger/feature_flag/Dangerfile
+++ b/danger/feature_flag/Dangerfile
@@ -53,14 +53,22 @@ def message_for_feature_flag_with_group!(feature_flag:, mr_group_label:)
   end
 end
 
+def feature_flag_file_added?
+  feature_flag.feature_flag_files(change_type: :added).any?
+end
+
 def feature_flag_file_added_or_removed?
-  feature_flag.feature_flag_files(change_type: :added).any? || feature_flag.feature_flag_files(change_type: :deleted).any?
+  feature_flag_file_added? || feature_flag.feature_flag_files(change_type: :deleted).any?
 end
 
 feature_flag.feature_flag_files(change_type: :added).each do |feature_flag|
   check_feature_flag_yaml(feature_flag)
 end
 
+if helper.security_mr? && feature_flag_file_added?
+  fail "Feature flags are discouraged from security merge requests. Read the [security documentation](https://gitlab.com/gitlab-org/release/docs/-/blob/master/general/security/utilities/feature_flags.md) for details."
+end
+
 if feature_flag_file_added_or_removed?
   new_mr_title = helper.mr_title.dup
   new_mr_title << ' [RUN ALL RSPEC]' unless helper.run_all_rspec_mr?
diff --git a/danger/plugins/product_intelligence.rb b/danger/plugins/product_intelligence.rb
new file mode 100644
index 00000000000..91551a8312f
--- /dev/null
+++ b/danger/plugins/product_intelligence.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+require_relative '../../tooling/danger/product_intelligence'
+
+module Danger
+  class ProductIntelligence < ::Danger::Plugin
+    # Put the helper code somewhere it can be tested
+    include Tooling::Danger::ProductIntelligence
+  end
+end
diff --git a/danger/product_intelligence/Dangerfile b/danger/product_intelligence/Dangerfile
index 3867aed84d5..5469e8a87a5 100644
--- a/danger/product_intelligence/Dangerfile
+++ b/danger/product_intelligence/Dangerfile
@@ -18,82 +18,16 @@ UPDATE_DICTIONARY_MESSAGE = <<~MSG
   ```
 MSG
 
-all_changed_files = helper.all_changed_files
+# exit if not matching files
+matching_changed_files = product_intelligence.matching_changed_files
+return unless matching_changed_files.any?
 
-tracking_files = [
-  'lib/gitlab/tracking.rb',
-  'spec/lib/gitlab/tracking_spec.rb',
-  'app/helpers/tracking_helper.rb',
-  'spec/helpers/tracking_helper_spec.rb',
-  'app/assets/javascripts/tracking.js',
-  'spec/frontend/tracking_spec.js',
-  'generator_templates/usage_metric_definition/metric_definition.yml',
-  'lib/generators/gitlab/usage_metric_definition_generator.rb',
-  'lib/generators/gitlab/usage_metric_definition/redis_hll_generator.rb',
-  'spec/lib/generators/gitlab/usage_metric_definition_generator_spec.rb',
-  'spec/lib/generators/gitlab/usage_metric_definition/redis_hll_generator_spec.rb',
-  'config/metrics/schema.json'
-]
+warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(matching_changed_files))
+fail format(UPDATE_DICTIONARY_MESSAGE) if product_intelligence.need_dictionary_changes?
 
-tracking_changed_files   = all_changed_files & tracking_files
-usage_data_changed_files = all_changed_files.grep(%r{(usage_data)})
-dictionary_changed_file  = all_changed_files.grep(%r{(doc/development/usage_ping/dictionary.md)})
-metrics_changed_files    = all_changed_files.grep(%r{((ee/)?config/metrics/.*\.yml)})
+labels = product_intelligence.missing_labels
+return unless labels.any?
 
-def matching_files?(file, extension:, pattern:)
-  return unless file.end_with?(extension)
-
-  helper.changed_lines(file).grep(pattern).any?
-end
-
-js_patterns = Regexp.union(
-  'Tracking.event',
-  /\btrack\(/,
-  'data-track-event',
-  'data-track-action'
-)
-
-dictionary_pattern = Regexp.union(
-  'key_path:',
-  'description:',
-  'product_section:',
-  'product_stage:',
-  'product_group:',
-  'status:',
-  'tier:'
-)
-
-snowplow_changed_files = all_changed_files.select do |file|
-  matching_files?(file, extension: '.rb', pattern: %r{Gitlab::Tracking\.event}) ||
-    matching_files?(file, extension: '.js', pattern: js_patterns) ||
-    matching_files?(file, extension: '.vue', pattern: js_patterns) ||
-    matching_files?(file, extension: '.haml', pattern: %r{data: \{ track})
-end
-
-required_dictionary_update_changed_files = metrics_changed_files.select do |file|
-  matching_files?(file, extension: '.yml', pattern: dictionary_pattern)
-end
-
-matching_changed_files = usage_data_changed_files +
-                         tracking_changed_files +
-                         metrics_changed_files +
-                         dictionary_changed_file +
-                         snowplow_changed_files
-
-if matching_changed_files.any?
-  warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(matching_changed_files))
-
-  fail format(UPDATE_DICTIONARY_MESSAGE) if required_dictionary_update_changed_files.any? && dictionary_changed_file.empty?
-
-  return unless helper.ci?
-
-  labels = []
-  labels << 'product intelligence' unless helper.mr_has_labels?('product intelligence')
-  labels << 'product intelligence::review pending' unless helper.mr_has_labels?(['product intelligence::approved', 'product intelligence::review pending'])
-
-  if labels.any?
-    gitlab.api.update_merge_request(gitlab.mr_json['project_id'],
+gitlab.api.update_merge_request(gitlab.mr_json['project_id'],
                                     gitlab.mr_json['iid'],
                                     add_labels: labels)
-  end
-end
diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile
index 06d2929cd54..c8ea494bc41 100644
--- a/danger/roulette/Dangerfile
+++ b/danger/roulette/Dangerfile
@@ -2,15 +2,21 @@
 
 require 'digest/md5'
 
-MESSAGE = <<MARKDOWN
+REVIEW_ROULETTE_SECTION = <<MARKDOWN
 ## Reviewer roulette
+MARKDOWN
+
+CATEGORY_TABLE = <<MARKDOWN
+
+Changes that require review have been detected!
 
-Changes that require review have been detected! A merge request is normally
-reviewed by both a reviewer and a maintainer in its primary category (e.g.
-~frontend or ~backend), and by a maintainer in all other categories.
+Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
+
+| Category | Reviewer | Maintainer |
+| -------- | -------- | ---------- |
 MARKDOWN
 
-CATEGORY_TABLE_HEADER = <<MARKDOWN
+POST_TABLE_MESSAGE = <<MARKDOWN
 
 To spread load more evenly across eligible reviewers, Danger has picked a candidate for each
 review slot, based on their timezone. Feel free to
@@ -26,12 +32,15 @@ Please consider assigning a reviewer or maintainer who is a
 
 Once you've decided who will review this merge request, assign them as a reviewer!
 Danger does not automatically notify them for you.
+MARKDOWN
 
-| Category | Reviewer | Maintainer |
-| -------- | -------- | ---------- |
+NO_SUGGESTIONS = <<MARKDOWN
+
+There are no reviewer and maintainer suggestions for the changes in this MR.
 MARKDOWN
 
 UNKNOWN_FILES_MESSAGE = <<MARKDOWN
+### Uncategorised files
 
 These files couldn't be categorised, so Danger was unable to suggest a reviewer.
 Please consider creating a merge request to
@@ -99,8 +108,14 @@ if changes.any?
     markdown_row_for_spins(spin.category, [spin])
   end
 
-  markdown(MESSAGE)
-  markdown(CATEGORY_TABLE_HEADER + rows.join("\n")) unless rows.empty?
+  markdown(REVIEW_ROULETTE_SECTION)
+
+  if rows.empty?
+    markdown(NO_SUGGESTIONS)
+  else
+    markdown(CATEGORY_TABLE + rows.join("\n"))
+    markdown(POST_TABLE_MESSAGE)
+  end
 
   unknown = changes.fetch(:unknown, [])
   markdown(UNKNOWN_FILES_MESSAGE + helper.markdown_list(unknown)) unless unknown.empty?