diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-02-18 10:34:06 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-02-18 10:34:06 +0000 |
commit | 859a6fb938bb9ee2a317c46dfa4fcc1af49608f0 (patch) | |
tree | d7f2700abe6b4ffcb2dcfc80631b2d87d0609239 /tooling | |
parent | 446d496a6d000c73a304be52587cd9bbc7493136 (diff) | |
download | gitlab-ce-859a6fb938bb9ee2a317c46dfa4fcc1af49608f0.tar.gz |
Add latest changes from gitlab-org/gitlab@13-9-stable-eev13.9.0-rc42
Diffstat (limited to 'tooling')
-rw-r--r-- | tooling/danger/base_linter.rb | 96 | ||||
-rw-r--r-- | tooling/danger/changelog.rb | 92 | ||||
-rw-r--r-- | tooling/danger/commit_linter.rb | 150 | ||||
-rw-r--r-- | tooling/danger/emoji_checker.rb | 45 | ||||
-rw-r--r-- | tooling/danger/feature_flag.rb | 44 | ||||
-rw-r--r-- | tooling/danger/helper.rb | 294 | ||||
-rw-r--r-- | tooling/danger/merge_request_linter.rb | 30 | ||||
-rw-r--r-- | tooling/danger/request_helper.rb | 23 | ||||
-rw-r--r-- | tooling/danger/roulette.rb | 169 | ||||
-rw-r--r-- | tooling/danger/sidekiq_queues.rb | 37 | ||||
-rw-r--r-- | tooling/danger/teammate.rb | 121 | ||||
-rw-r--r-- | tooling/danger/title_linting.rb | 38 | ||||
-rw-r--r-- | tooling/danger/weightage.rb | 10 | ||||
-rw-r--r-- | tooling/danger/weightage/maintainers.rb | 33 | ||||
-rw-r--r-- | tooling/danger/weightage/reviewers.rb | 65 | ||||
-rw-r--r-- | tooling/gitlab_danger.rb | 59 | ||||
-rw-r--r-- | tooling/lib/tooling/kubernetes_client.rb | 2 |
17 files changed, 1306 insertions, 2 deletions
diff --git a/tooling/danger/base_linter.rb b/tooling/danger/base_linter.rb new file mode 100644 index 00000000000..c58f2d84dc8 --- /dev/null +++ b/tooling/danger/base_linter.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require_relative 'title_linting' + +module Tooling + module Danger + class BaseLinter + MIN_SUBJECT_WORDS_COUNT = 3 + MAX_LINE_LENGTH = 72 + + attr_reader :commit, :problems + + def self.problems_mapping + { + subject_too_short: "The %s must contain at least #{MIN_SUBJECT_WORDS_COUNT} words", + subject_too_long: "The %s may not be longer than #{MAX_LINE_LENGTH} characters", + subject_starts_with_lowercase: "The %s must start with a capital letter", + subject_ends_with_a_period: "The %s must not end with a period" + } + end + + def self.subject_description + 'commit subject' + end + + def initialize(commit) + @commit = commit + @problems = {} + end + + def failed? + problems.any? + end + + def add_problem(problem_key, *args) + @problems[problem_key] = sprintf(self.class.problems_mapping[problem_key], *args) + end + + def lint_subject + if subject_too_short? + add_problem(:subject_too_short, self.class.subject_description) + end + + if subject_too_long? + add_problem(:subject_too_long, self.class.subject_description) + end + + if subject_starts_with_lowercase? + add_problem(:subject_starts_with_lowercase, self.class.subject_description) + end + + if subject_ends_with_a_period? + add_problem(:subject_ends_with_a_period, self.class.subject_description) + end + + self + end + + private + + def subject + TitleLinting.remove_draft_flag(message_parts[0]) + end + + def subject_too_short? + subject.split(' ').length < MIN_SUBJECT_WORDS_COUNT + end + + def subject_too_long? + line_too_long?(subject) + end + + def line_too_long?(line) + line.length > MAX_LINE_LENGTH + end + + def subject_starts_with_lowercase? + return false if ('A'..'Z').cover?(subject[0]) + + first_char = subject.sub(/\A(\[.+\]|\w+:)\s/, '')[0] + first_char_downcased = first_char.downcase + return true unless ('a'..'z').cover?(first_char_downcased) + + first_char.downcase == first_char + end + + def subject_ends_with_a_period? + subject.end_with?('.') + end + + def message_parts + @message_parts ||= commit.message.split("\n", 3) + end + end + end +end diff --git a/tooling/danger/changelog.rb b/tooling/danger/changelog.rb new file mode 100644 index 00000000000..f7f505f51a6 --- /dev/null +++ b/tooling/danger/changelog.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +require_relative 'title_linting' + +module Tooling + module Danger + module Changelog + NO_CHANGELOG_LABELS = [ + 'tooling', + 'tooling::pipelines', + 'tooling::workflow', + 'ci-build', + 'meta' + ].freeze + NO_CHANGELOG_CATEGORIES = %i[docs none].freeze + CREATE_CHANGELOG_COMMAND = 'bin/changelog -m %<mr_iid>s "%<mr_title>s"' + CREATE_EE_CHANGELOG_COMMAND = 'bin/changelog --ee -m %<mr_iid>s "%<mr_title>s"' + CHANGELOG_MODIFIED_URL_TEXT = "**CHANGELOG.md was edited.** Please remove the additions and create a CHANGELOG entry.\n\n" + CHANGELOG_MISSING_URL_TEXT = "**[CHANGELOG missing](https://docs.gitlab.com/ee/development/changelog.html)**:\n\n" + + OPTIONAL_CHANGELOG_MESSAGE = <<~MSG + If you want to create a changelog entry for GitLab FOSS, run the following: + + #{CREATE_CHANGELOG_COMMAND} + + If you want to create a changelog entry for GitLab EE, run the following instead: + + #{CREATE_EE_CHANGELOG_COMMAND} + + If this merge request [doesn't need a CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry), feel free to ignore this message. + MSG + + REQUIRED_CHANGELOG_MESSAGE = <<~MSG + To create a changelog entry, run the following: + + #{CREATE_CHANGELOG_COMMAND} + + This merge request requires a changelog entry because it [introduces a database migration](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry). + MSG + + def required? + git.added_files.any? { |path| path =~ %r{\Adb/(migrate|post_migrate)/} } + end + alias_method :db_changes?, :required? + + def optional? + categories_need_changelog? && without_no_changelog_label? + end + + def found + @found ||= git.added_files.find { |path| path =~ %r{\A(ee/)?(changelogs/unreleased)(-ee)?/} } + end + + def ee_changelog? + found.start_with?('ee/') + end + + def modified_text + CHANGELOG_MODIFIED_URL_TEXT + + format(OPTIONAL_CHANGELOG_MESSAGE, mr_iid: mr_iid, mr_title: sanitized_mr_title) + end + + def required_text + CHANGELOG_MISSING_URL_TEXT + + format(REQUIRED_CHANGELOG_MESSAGE, mr_iid: mr_iid, mr_title: sanitized_mr_title) + end + + def optional_text + CHANGELOG_MISSING_URL_TEXT + + format(OPTIONAL_CHANGELOG_MESSAGE, mr_iid: mr_iid, mr_title: sanitized_mr_title) + end + + private + + def mr_iid + gitlab.mr_json["iid"] + end + + def sanitized_mr_title + TitleLinting.sanitize_mr_title(gitlab.mr_json["title"]) + end + + def categories_need_changelog? + (helper.changes_by_category.keys - NO_CHANGELOG_CATEGORIES).any? + end + + def without_no_changelog_label? + (gitlab.mr_labels & NO_CHANGELOG_LABELS).empty? + end + end + end +end diff --git a/tooling/danger/commit_linter.rb b/tooling/danger/commit_linter.rb new file mode 100644 index 00000000000..905031ec881 --- /dev/null +++ b/tooling/danger/commit_linter.rb @@ -0,0 +1,150 @@ +# frozen_string_literal: true + +require_relative 'base_linter' +require_relative 'emoji_checker' + +module Tooling + module Danger + class CommitLinter < BaseLinter + MAX_CHANGED_FILES_IN_COMMIT = 3 + MAX_CHANGED_LINES_IN_COMMIT = 30 + SHORT_REFERENCE_REGEX = %r{([\w\-\/]+)?(?<!`)(#|!|&|%)\d+(?<!`)}.freeze + + def self.problems_mapping + super.merge( + { + separator_missing: "The commit subject and body must be separated by a blank line", + details_too_many_changes: "Commits that change #{MAX_CHANGED_LINES_IN_COMMIT} or more lines across " \ + "at least #{MAX_CHANGED_FILES_IN_COMMIT} files must describe these changes in the commit body", + details_line_too_long: "The commit body should not contain more than #{MAX_LINE_LENGTH} characters per line", + message_contains_text_emoji: "Avoid the use of Markdown Emoji such as `:+1:`. These add limited value " \ + "to the commit message, and are displayed as plain text outside of GitLab", + message_contains_unicode_emoji: "Avoid the use of Unicode Emoji. These add no value to the commit " \ + "message, and may not be displayed properly everywhere", + message_contains_short_reference: "Use full URLs instead of short references (`gitlab-org/gitlab#123` or " \ + "`!123`), as short references are displayed as plain text outside of GitLab" + } + ) + end + + def initialize(commit) + super + + @linted = false + end + + def fixup? + commit.message.start_with?('fixup!', 'squash!') + end + + def suggestion? + commit.message.start_with?('Apply suggestion to') + end + + def merge? + commit.message.start_with?('Merge branch') + end + + def revert? + commit.message.start_with?('Revert "') + end + + def multi_line? + !details.nil? && !details.empty? + end + + def lint + return self if @linted + + @linted = true + lint_subject + lint_separator + lint_details + lint_message + + self + end + + private + + def lint_separator + return self unless separator && !separator.empty? + + add_problem(:separator_missing) + + self + end + + def lint_details + if !multi_line? && many_changes? + add_problem(:details_too_many_changes) + end + + details&.each_line do |line| + line_without_urls = line.strip.gsub(%r{https?://\S+}, '') + + # If the line includes a URL, we'll allow it to exceed MAX_LINE_LENGTH characters, but + # only if the line _without_ the URL does not exceed this limit. + next unless line_too_long?(line_without_urls) + + add_problem(:details_line_too_long) + break + end + + self + end + + def lint_message + if message_contains_text_emoji? + add_problem(:message_contains_text_emoji) + end + + if message_contains_unicode_emoji? + add_problem(:message_contains_unicode_emoji) + end + + if message_contains_short_reference? + add_problem(:message_contains_short_reference) + end + + self + end + + def files_changed + commit.diff_parent.stats[:total][:files] + end + + def lines_changed + commit.diff_parent.stats[:total][:lines] + end + + def many_changes? + files_changed > MAX_CHANGED_FILES_IN_COMMIT && lines_changed > MAX_CHANGED_LINES_IN_COMMIT + end + + def separator + message_parts[1] + end + + def details + message_parts[2]&.gsub(/^Signed-off-by.*$/, '') + end + + def message_contains_text_emoji? + emoji_checker.includes_text_emoji?(commit.message) + end + + def message_contains_unicode_emoji? + emoji_checker.includes_unicode_emoji?(commit.message) + end + + def message_contains_short_reference? + commit.message.match?(SHORT_REFERENCE_REGEX) + end + + def emoji_checker + @emoji_checker ||= Tooling::Danger::EmojiChecker.new + end + end + end +end diff --git a/tooling/danger/emoji_checker.rb b/tooling/danger/emoji_checker.rb new file mode 100644 index 00000000000..9d8ff93037c --- /dev/null +++ b/tooling/danger/emoji_checker.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'json' + +module Tooling + module Danger + class EmojiChecker + DIGESTS = File.expand_path('../../fixtures/emojis/digests.json', __dir__) + ALIASES = File.expand_path('../../fixtures/emojis/aliases.json', __dir__) + + # A regex that indicates a piece of text _might_ include an Emoji. The regex + # alone is not enough, as we'd match `:foo:bar:baz`. Instead, we use this + # regex to save us from having to check for all possible emoji names when we + # know one definitely is not included. + LIKELY_EMOJI = /:[\+a-z0-9_\-]+:/.freeze + + UNICODE_EMOJI_REGEX = %r{( + [\u{1F300}-\u{1F5FF}] | + [\u{1F1E6}-\u{1F1FF}] | + [\u{2700}-\u{27BF}] | + [\u{1F900}-\u{1F9FF}] | + [\u{1F600}-\u{1F64F}] | + [\u{1F680}-\u{1F6FF}] | + [\u{2600}-\u{26FF}] + )}x.freeze + + def initialize + names = JSON.parse(File.read(DIGESTS)).keys + + JSON.parse(File.read(ALIASES)).keys + + @emoji = names.map { |name| ":#{name}:" } + end + + def includes_text_emoji?(text) + return false unless text.match?(LIKELY_EMOJI) + + @emoji.any? { |emoji| text.include?(emoji) } + end + + def includes_unicode_emoji?(text) + text.match?(UNICODE_EMOJI_REGEX) + end + end + end +end diff --git a/tooling/danger/feature_flag.rb b/tooling/danger/feature_flag.rb new file mode 100644 index 00000000000..2e65831ef9f --- /dev/null +++ b/tooling/danger/feature_flag.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'yaml' + +module Tooling + module Danger + module FeatureFlag + # `change_type` can be: + # - :added + # - :modified + # - :deleted + def feature_flag_files(change_type:) + files = git.public_send("#{change_type}_files") # rubocop:disable GitlabSecurity/PublicSend + files.select { |path| path =~ %r{\A(ee/)?config/feature_flags/} }.map { |path| Found.new(path) } + end + + class Found + attr_reader :path + + def initialize(path) + @path = path + end + + def raw + @raw ||= File.read(path) + end + + def group + @group ||= yaml['group'] + end + + def group_match_mr_label?(mr_group_label) + mr_group_label == group + end + + private + + def yaml + @yaml ||= YAML.safe_load(raw) + end + end + end + end +end diff --git a/tooling/danger/helper.rb b/tooling/danger/helper.rb new file mode 100644 index 00000000000..60026ee9c70 --- /dev/null +++ b/tooling/danger/helper.rb @@ -0,0 +1,294 @@ +# frozen_string_literal: true + +require_relative 'teammate' +require_relative 'title_linting' + +module Tooling + module Danger + module Helper + RELEASE_TOOLS_BOT = 'gitlab-release-tools-bot' + + # Returns a list of all files that have been added, modified or renamed. + # `git.modified_files` might contain paths that already have been renamed, + # so we need to remove them from the list. + # + # Considering these changes: + # + # - A new_file.rb + # - D deleted_file.rb + # - M modified_file.rb + # - R renamed_file_before.rb -> renamed_file_after.rb + # + # it will return + # ``` + # [ 'new_file.rb', 'modified_file.rb', 'renamed_file_after.rb' ] + # ``` + # + # @return [Array<String>] + def all_changed_files + Set.new + .merge(git.added_files.to_a) + .merge(git.modified_files.to_a) + .merge(git.renamed_files.map { |x| x[:after] }) + .subtract(git.renamed_files.map { |x| x[:before] }) + .to_a + .sort + end + + # Returns a string containing changed lines as git diff + # + # Considering changing a line in lib/gitlab/usage_data.rb it will return: + # + # [ "--- a/lib/gitlab/usage_data.rb", + # "+++ b/lib/gitlab/usage_data.rb", + # "+ # Test change", + # "- # Old change" ] + def changed_lines(changed_file) + diff = git.diff_for_file(changed_file) + return [] unless diff + + diff.patch.split("\n").select { |line| %r{^[+-]}.match?(line) } + end + + def all_ee_changes + all_changed_files.grep(%r{\Aee/}) + end + + def ee? + # Support former project name for `dev` and support local Danger run + %w[gitlab gitlab-ee].include?(ENV['CI_PROJECT_NAME']) || Dir.exist?(File.expand_path('../../../ee', __dir__)) + end + + def gitlab_helper + # Unfortunately the following does not work: + # - respond_to?(:gitlab) + # - respond_to?(:gitlab, true) + gitlab + rescue NameError + nil + end + + def release_automation? + gitlab_helper&.mr_author == RELEASE_TOOLS_BOT + end + + def project_name + ee? ? 'gitlab' : 'gitlab-foss' + end + + def markdown_list(items) + list = items.map { |item| "* `#{item}`" }.join("\n") + + if items.size > 10 + "\n<details>\n\n#{list}\n\n</details>\n" + else + list + end + end + + # @return [Hash<String,Array<String>>] + def changes_by_category + all_changed_files.each_with_object(Hash.new { |h, k| h[k] = [] }) do |file, hash| + categories_for_file(file).each { |category| hash[category] << file } + end + end + + # Determines the categories a file is in, e.g., `[:frontend]`, `[:backend]`, or `%i[frontend engineering_productivity]` + # using filename regex and specific change regex if given. + # + # @return Array<Symbol> + def categories_for_file(file) + _, categories = CATEGORIES.find do |key, _| + filename_regex, changes_regex = Array(key) + + found = filename_regex.match?(file) + found &&= changed_lines(file).any? { |changed_line| changes_regex.match?(changed_line) } if changes_regex + + found + end + + Array(categories || :unknown) + end + + # Returns the GFM for a category label, making its best guess if it's not + # a category we know about. + # + # @return[String] + def label_for_category(category) + CATEGORY_LABELS.fetch(category, "~#{category}") + end + + CATEGORY_LABELS = { + docs: "~documentation", # Docs are reviewed along DevOps stages, so don't need roulette for now. + none: "", + qa: "~QA", + test: "~test ~Quality for `spec/features/*`", + engineering_productivity: '~"Engineering Productivity" for CI, Danger', + ci_template: '~"ci::templates"' + }.freeze + # 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], + + %r{\Adoc/.*(\.(md|png|gif|jpg))\z} => :docs, + %r{\A(CONTRIBUTING|LICENSE|MAINTENANCE|PHILOSOPHY|PROCESS|README)(\.md)?\z} => :docs, + + %r{\A(ee/)?app/(assets|views)/} => :frontend, + %r{\A(ee/)?public/} => :frontend, + %r{\A(ee/)?spec/(javascripts|frontend)/} => :frontend, + %r{\A(ee/)?vendor/assets/} => :frontend, + %r{\A(ee/)?scripts/frontend/} => :frontend, + %r{(\A|/)( + \.babelrc | + \.eslintignore | + \.eslintrc(\.yml)? | + \.nvmrc | + \.prettierignore | + \.prettierrc | + \.scss-lint.yml | + \.stylelintrc | + \.haml-lint.yml | + \.haml-lint_todo.yml | + babel\.config\.js | + jest\.config\.js | + package\.json | + yarn\.lock | + config/.+\.js + )\z}x => :frontend, + + %r{(\A|/)( + \.gitlab/ci/frontend\.gitlab-ci\.yml + )\z}x => %i[frontend engineering_productivity], + + %r{\A(ee/)?db/(?!fixtures)[^/]+} => :database, + %r{\A(ee/)?lib/gitlab/(database|background_migration|sql|github_import)(/|\.rb)} => :database, + %r{\A(app/models/project_authorization|app/services/users/refresh_authorized_projects_service)(/|\.rb)} => :database, + %r{\A(ee/)?app/finders/} => :database, + %r{\Arubocop/cop/migration(/|\.rb)} => :database, + + %r{\A(\.gitlab-ci\.yml\z|\.gitlab\/ci)} => :engineering_productivity, + %r{\A\.codeclimate\.yml\z} => :engineering_productivity, + %r{\Alefthook.yml\z} => :engineering_productivity, + %r{\A\.editorconfig\z} => :engineering_productivity, + %r{Dangerfile\z} => :engineering_productivity, + %r{\A(ee/)?(danger/|tooling/danger/)} => :engineering_productivity, + %r{\A(ee/)?scripts/} => :engineering_productivity, + %r{\Atooling/} => :engineering_productivity, + %r{(CODEOWNERS)} => :engineering_productivity, + %r{(tests.yml)} => :engineering_productivity, + + %r{\Alib/gitlab/ci/templates} => :ci_template, + + %r{\A(ee/)?spec/features/} => :test, + %r{\A(ee/)?spec/support/shared_examples/features/} => :test, + %r{\A(ee/)?spec/support/shared_contexts/features/} => :test, + %r{\A(ee/)?spec/support/helpers/features/} => :test, + + %r{\A(ee/)?app/(?!assets|views)[^/]+} => :backend, + %r{\A(ee/)?(bin|config|generator_templates|lib|rubocop)/} => :backend, + %r{\A(ee/)?spec/} => :backend, + %r{\A(ee/)?vendor/} => :backend, + %r{\A(Gemfile|Gemfile.lock|Rakefile)\z} => :backend, + %r{\A[A-Z_]+_VERSION\z} => :backend, + %r{\A\.rubocop((_manual)?_todo)?\.yml\z} => :backend, + %r{\Afile_hooks/} => :backend, + + %r{\A(ee/)?qa/} => :qa, + + # Files that don't fit into any category are marked with :none + %r{\A(ee/)?changelogs/} => :none, + %r{\Alocale/gitlab\.pot\z} => :none, + + # GraphQL auto generated doc files and schema + %r{\Adoc/api/graphql/reference/} => :backend, + + # Fallbacks in case the above patterns miss anything + %r{\.rb\z} => :backend, + %r{( + \.(md|txt)\z | + \.markdownlint\.json + )}x => :none, # To reinstate roulette for documentation, set to `:docs`. + %r{\.js\z} => :frontend + }.freeze + + def new_teammates(usernames) + usernames.map { |u| Tooling::Danger::Teammate.new('username' => u) } + end + + def mr_title + return '' unless gitlab_helper + + gitlab_helper.mr_json['title'] + end + + def mr_web_url + return '' unless gitlab_helper + + gitlab_helper.mr_json['web_url'] + end + + def mr_target_branch + return '' unless gitlab_helper + + gitlab_helper.mr_json['target_branch'] + end + + def draft_mr? + TitleLinting.has_draft_flag?(mr_title) + end + + def security_mr? + mr_web_url.include?('/gitlab-org/security/') + end + + def cherry_pick_mr? + TitleLinting.has_cherry_pick_flag?(mr_title) + end + + def run_all_rspec_mr? + TitleLinting.has_run_all_rspec_flag?(mr_title) + end + + def run_as_if_foss_mr? + TitleLinting.has_run_as_if_foss_flag?(mr_title) + end + + def stable_branch? + /\A\d+-\d+-stable-ee/i.match?(mr_target_branch) + end + + def mr_has_labels?(*labels) + return false unless gitlab_helper + + labels = labels.flatten.uniq + (labels & gitlab_helper.mr_labels) == labels + end + + def labels_list(labels, sep: ', ') + labels.map { |label| %Q{~"#{label}"} }.join(sep) + end + + def prepare_labels_for_mr(labels) + return '' unless labels.any? + + "/label #{labels_list(labels, sep: ' ')}" + end + + def changed_files(regex) + all_changed_files.grep(regex) + end + + def has_database_scoped_labels?(labels) + labels.any? { |label| label.start_with?('database::') } + end + + def has_ci_changes? + changed_files(%r{\A(\.gitlab-ci\.yml|\.gitlab/ci/)}).any? + end + + def group_label(labels) + labels.find { |label| label.start_with?('group::') } + end + end + end +end diff --git a/tooling/danger/merge_request_linter.rb b/tooling/danger/merge_request_linter.rb new file mode 100644 index 00000000000..ddeb9cc2981 --- /dev/null +++ b/tooling/danger/merge_request_linter.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require_relative 'base_linter' + +module Tooling + module Danger + class MergeRequestLinter < BaseLinter + alias_method :lint, :lint_subject + + def self.subject_description + 'merge request title' + end + + def self.mr_run_options_regex + [ + 'RUN AS-IF-FOSS', + 'UPDATE CACHE', + 'RUN ALL RSPEC', + 'SKIP RSPEC FAIL-FAST' + ].join('|') + end + + private + + def subject + super.gsub(/\[?(#{self.class.mr_run_options_regex})\]?/, '').strip + end + end + end +end diff --git a/tooling/danger/request_helper.rb b/tooling/danger/request_helper.rb new file mode 100644 index 00000000000..d6b99f562f9 --- /dev/null +++ b/tooling/danger/request_helper.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'net/http' +require 'json' + +module Tooling + module Danger + module RequestHelper + HTTPError = Class.new(RuntimeError) + + # @param [String] url + def self.http_get_json(url) + rsp = Net::HTTP.get_response(URI.parse(url)) + + unless rsp.is_a?(Net::HTTPOK) + raise HTTPError, "Failed to read #{url}: #{rsp.code} #{rsp.message}" + end + + JSON.parse(rsp.body) + end + end + end +end diff --git a/tooling/danger/roulette.rb b/tooling/danger/roulette.rb new file mode 100644 index 00000000000..c928fb2b655 --- /dev/null +++ b/tooling/danger/roulette.rb @@ -0,0 +1,169 @@ +# frozen_string_literal: true + +require_relative 'teammate' +require_relative 'request_helper' +require_relative 'weightage/reviewers' +require_relative 'weightage/maintainers' + +module Tooling + module Danger + module Roulette + ROULETTE_DATA_URL = 'https://gitlab-org.gitlab.io/gitlab-roulette/roulette.json' + HOURS_WHEN_PERSON_CAN_BE_PICKED = (6..14).freeze + + INCLUDE_TIMEZONE_FOR_CATEGORY = { + database: false + }.freeze + + Spin = Struct.new(:category, :reviewer, :maintainer, :optional_role, :timezone_experiment) + + def team_mr_author + team.find { |person| person.username == mr_author_username } + end + + # Assigns GitLab team members to be reviewer and maintainer + # for each change category that a Merge Request contains. + # + # @return [Array<Spin>] + def spin(project, categories, timezone_experiment: false) + spins = categories.sort.map do |category| + including_timezone = INCLUDE_TIMEZONE_FOR_CATEGORY.fetch(category, timezone_experiment) + + spin_for_category(project, category, timezone_experiment: including_timezone) + end + + backend_spin = spins.find { |spin| spin.category == :backend } + + spins.each do |spin| + including_timezone = INCLUDE_TIMEZONE_FOR_CATEGORY.fetch(spin.category, timezone_experiment) + case spin.category + when :qa + # MR includes QA changes, but also other changes, and author isn't an SET + if categories.size > 1 && !team_mr_author&.any_capability?(project, spin.category) + spin.optional_role = :maintainer + end + when :test + spin.optional_role = :maintainer + + if spin.reviewer.nil? + # Fetch an already picked backend reviewer, or pick one otherwise + spin.reviewer = backend_spin&.reviewer || spin_for_category(project, :backend, timezone_experiment: including_timezone).reviewer + end + when :engineering_productivity + if spin.maintainer.nil? + # Fetch an already picked backend maintainer, or pick one otherwise + spin.maintainer = backend_spin&.maintainer || spin_for_category(project, :backend, timezone_experiment: including_timezone).maintainer + end + when :ci_template + if spin.maintainer.nil? + # Fetch an already picked backend maintainer, or pick one otherwise + spin.maintainer = backend_spin&.maintainer || spin_for_category(project, :backend, timezone_experiment: including_timezone).maintainer + end + end + end + + spins + end + + # Looks up the current list of GitLab team members and parses it into a + # useful form + # + # @return [Array<Teammate>] + def team + @team ||= + begin + data = Tooling::Danger::RequestHelper.http_get_json(ROULETTE_DATA_URL) + data.map { |hash| ::Tooling::Danger::Teammate.new(hash) } + rescue JSON::ParserError + raise "Failed to parse JSON response from #{ROULETTE_DATA_URL}" + end + end + + # Like +team+, but only returns teammates in the current project, based on + # project_name. + # + # @return [Array<Teammate>] + def project_team(project_name) + team.select { |member| member.in_project?(project_name) } + rescue => err + warn("Reviewer roulette failed to load team data: #{err.message}") + [] + end + + # Known issue: If someone is rejected due to OOO, and then becomes not OOO, the + # selection will change on next spin + # @param [Array<Teammate>] people + def spin_for_person(people, random:, timezone_experiment: false) + shuffled_people = people.shuffle(random: random) + + if timezone_experiment + shuffled_people.find(&method(:valid_person_with_timezone?)) + else + shuffled_people.find(&method(:valid_person?)) + end + end + + private + + # @param [Teammate] person + # @return [Boolean] + def valid_person?(person) + !mr_author?(person) && person.available + end + + # @param [Teammate] person + # @return [Boolean] + def valid_person_with_timezone?(person) + valid_person?(person) && HOURS_WHEN_PERSON_CAN_BE_PICKED.cover?(person.local_hour) + end + + # @param [Teammate] person + # @return [Boolean] + def mr_author?(person) + person.username == mr_author_username + end + + def mr_author_username + helper.gitlab_helper&.mr_author || `whoami` + end + + def mr_source_branch + return `git rev-parse --abbrev-ref HEAD` unless helper.gitlab_helper&.mr_json + + helper.gitlab_helper.mr_json['source_branch'] + end + + def mr_labels + helper.gitlab_helper&.mr_labels || [] + end + + def new_random(seed) + Random.new(Digest::MD5.hexdigest(seed).to_i(16)) + end + + def spin_role_for_category(team, role, project, category) + team.select do |member| + member.public_send("#{role}?", project, category, mr_labels) # rubocop:disable GitlabSecurity/PublicSend + end + end + + def spin_for_category(project, category, timezone_experiment: false) + team = project_team(project) + reviewers, traintainers, maintainers = + %i[reviewer traintainer maintainer].map do |role| + spin_role_for_category(team, role, project, category) + end + + random = new_random(mr_source_branch) + + weighted_reviewers = Weightage::Reviewers.new(reviewers, traintainers).execute + weighted_maintainers = Weightage::Maintainers.new(maintainers).execute + + reviewer = spin_for_person(weighted_reviewers, random: random, timezone_experiment: timezone_experiment) + maintainer = spin_for_person(weighted_maintainers, random: random, timezone_experiment: timezone_experiment) + + Spin.new(category, reviewer, maintainer, false, timezone_experiment) + end + end + end +end diff --git a/tooling/danger/sidekiq_queues.rb b/tooling/danger/sidekiq_queues.rb new file mode 100644 index 00000000000..ae32b128682 --- /dev/null +++ b/tooling/danger/sidekiq_queues.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Tooling + module Danger + module SidekiqQueues + def changed_queue_files + @changed_queue_files ||= git.modified_files.grep(%r{\A(ee/)?app/workers/all_queues\.yml}) + end + + def added_queue_names + @added_queue_names ||= new_queues.keys - old_queues.keys + end + + def changed_queue_names + @changed_queue_names ||= + (new_queues.values_at(*old_queues.keys) - old_queues.values) + .compact.map { |queue| queue[:name] } + end + + private + + def old_queues + @old_queues ||= queues_for(gitlab.base_commit) + end + + def new_queues + @new_queues ||= queues_for(gitlab.head_commit) + end + + def queues_for(branch) + changed_queue_files + .flat_map { |file| YAML.safe_load(`git show #{branch}:#{file}`, permitted_classes: [Symbol]) } + .to_h { |queue| [queue[:name], queue] } + end + end + end +end diff --git a/tooling/danger/teammate.rb b/tooling/danger/teammate.rb new file mode 100644 index 00000000000..bcd33bebdc9 --- /dev/null +++ b/tooling/danger/teammate.rb @@ -0,0 +1,121 @@ +# frozen_string_literal: true + +module Tooling + module Danger + class Teammate + attr_reader :options, :username, :name, :role, :projects, :available, :hungry, :reduced_capacity, :tz_offset_hours + + # The options data are produced by https://gitlab.com/gitlab-org/gitlab-roulette/-/blob/master/lib/team_member.rb + def initialize(options = {}) + @options = options + @username = options['username'] + @name = options['name'] + @markdown_name = options['markdown_name'] + @role = options['role'] + @projects = options['projects'] + @available = options['available'] + @hungry = options['hungry'] + @reduced_capacity = options['reduced_capacity'] + @tz_offset_hours = options['tz_offset_hours'] + end + + def to_h + options + end + + def ==(other) + return false unless other.respond_to?(:username) + + other.username == username + end + + def in_project?(name) + projects&.has_key?(name) + end + + def any_capability?(project, category) + capabilities(project).any? { |capability| capability.end_with?(category.to_s) } + end + + def reviewer?(project, category, labels) + has_capability?(project, category, :reviewer, labels) + end + + def traintainer?(project, category, labels) + has_capability?(project, category, :trainee_maintainer, labels) + end + + def maintainer?(project, category, labels) + has_capability?(project, category, :maintainer, labels) + end + + def markdown_name(author: nil) + "#{@markdown_name} (#{utc_offset_text(author)})" + end + + def local_hour + (Time.now.utc + tz_offset_hours * 3600).hour + end + + protected + + def floored_offset_hours + floored_offset = tz_offset_hours.floor(0) + + floored_offset == tz_offset_hours ? floored_offset : tz_offset_hours + end + + private + + def utc_offset_text(author = nil) + offset_text = + if floored_offset_hours >= 0 + "UTC+#{floored_offset_hours}" + else + "UTC#{floored_offset_hours}" + end + + return offset_text unless author + + "#{offset_text}, #{offset_diff_compared_to_author(author)}" + end + + def offset_diff_compared_to_author(author) + diff = floored_offset_hours - author.floored_offset_hours + return "same timezone as `@#{author.username}`" if diff == 0 + + ahead_or_behind = diff < 0 ? 'behind' : 'ahead of' + pluralized_hours = pluralize(diff.abs, 'hour', 'hours') + + "#{pluralized_hours} #{ahead_or_behind} `@#{author.username}`" + end + + def has_capability?(project, category, kind, labels) + case category + when :test + area = role[/Software Engineer in Test(?:.*?, (\w+))/, 1] + + area && labels.any?("devops::#{area.downcase}") if kind == :reviewer + when :engineering_productivity + return false unless role[/Engineering Productivity/] + return true if kind == :reviewer + return true if capabilities(project).include?("#{kind} engineering_productivity") + + capabilities(project).include?("#{kind} backend") + else + capabilities(project).include?("#{kind} #{category}") + end + end + + def capabilities(project) + Array(projects.fetch(project, [])) + end + + def pluralize(count, singular, plural) + word = count == 1 || count.to_s =~ /^1(\.0+)?$/ ? singular : plural + + "#{count || 0} #{word}" + end + end + end +end diff --git a/tooling/danger/title_linting.rb b/tooling/danger/title_linting.rb new file mode 100644 index 00000000000..dcd83df7d93 --- /dev/null +++ b/tooling/danger/title_linting.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Tooling + module Danger + module TitleLinting + DRAFT_REGEX = /\A*#{Regexp.union(/(?i)(\[WIP\]\s*|WIP:\s*|WIP$)/, /(?i)(\[draft\]|\(draft\)|draft:|draft\s\-\s|draft$)/)}+\s*/i.freeze + CHERRY_PICK_REGEX = /cherry[\s-]*pick/i.freeze + RUN_ALL_RSPEC_REGEX = /RUN ALL RSPEC/i.freeze + RUN_AS_IF_FOSS_REGEX = /RUN AS-IF-FOSS/i.freeze + + module_function + + def sanitize_mr_title(title) + remove_draft_flag(title).gsub(/`/, '\\\`') + end + + def remove_draft_flag(title) + title.gsub(DRAFT_REGEX, '') + end + + def has_draft_flag?(title) + DRAFT_REGEX.match?(title) + end + + def has_cherry_pick_flag?(title) + CHERRY_PICK_REGEX.match?(title) + end + + def has_run_all_rspec_flag?(title) + RUN_ALL_RSPEC_REGEX.match?(title) + end + + def has_run_as_if_foss_flag?(title) + RUN_AS_IF_FOSS_REGEX.match?(title) + end + end + end +end diff --git a/tooling/danger/weightage.rb b/tooling/danger/weightage.rb new file mode 100644 index 00000000000..cf8d17410dc --- /dev/null +++ b/tooling/danger/weightage.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module Tooling + module Danger + module Weightage + CAPACITY_MULTIPLIER = 2 # change this number to change what it means to be a reduced capacity reviewer 1/this number + BASE_REVIEWER_WEIGHT = 1 + end + end +end diff --git a/tooling/danger/weightage/maintainers.rb b/tooling/danger/weightage/maintainers.rb new file mode 100644 index 00000000000..068b24e7913 --- /dev/null +++ b/tooling/danger/weightage/maintainers.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require_relative '../weightage' + +module Tooling + module Danger + module Weightage + class Maintainers + def initialize(maintainers) + @maintainers = maintainers + end + + def execute + maintainers.each_with_object([]) do |maintainer, weighted_maintainers| + add_weighted_reviewer(weighted_maintainers, maintainer, BASE_REVIEWER_WEIGHT) + end + end + + private + + attr_reader :maintainers + + def add_weighted_reviewer(reviewers, reviewer, weight) + if reviewer.reduced_capacity + reviewers.fill(reviewer, reviewers.size, weight) + else + reviewers.fill(reviewer, reviewers.size, weight * CAPACITY_MULTIPLIER) + end + end + end + end + end +end diff --git a/tooling/danger/weightage/reviewers.rb b/tooling/danger/weightage/reviewers.rb new file mode 100644 index 00000000000..e74fce37187 --- /dev/null +++ b/tooling/danger/weightage/reviewers.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require_relative '../weightage' + +module Tooling + module Danger + module Weightage + # Weights after (current multiplier of 2) + # + # +------------------------------+--------------------------------+ + # | reviewer type | weight(times in reviewer pool) | + # +------------------------------+--------------------------------+ + # | reduced capacity reviewer | 1 | + # | reviewer | 2 | + # | hungry reviewer | 4 | + # | reduced capacity traintainer | 3 | + # | traintainer | 6 | + # | hungry traintainer | 8 | + # +------------------------------+--------------------------------+ + # + class Reviewers + DEFAULT_REVIEWER_WEIGHT = CAPACITY_MULTIPLIER * BASE_REVIEWER_WEIGHT + TRAINTAINER_WEIGHT = 3 + + def initialize(reviewers, traintainers) + @reviewers = reviewers + @traintainers = traintainers + end + + def execute + # TODO: take CODEOWNERS into account? + # https://gitlab.com/gitlab-org/gitlab/issues/26723 + + weighted_reviewers + weighted_traintainers + end + + private + + attr_reader :reviewers, :traintainers + + def weighted_reviewers + reviewers.each_with_object([]) do |reviewer, total_reviewers| + add_weighted_reviewer(total_reviewers, reviewer, BASE_REVIEWER_WEIGHT) + end + end + + def weighted_traintainers + traintainers.each_with_object([]) do |reviewer, total_traintainers| + add_weighted_reviewer(total_traintainers, reviewer, TRAINTAINER_WEIGHT) + end + end + + def add_weighted_reviewer(reviewers, reviewer, weight) + if reviewer.reduced_capacity + reviewers.fill(reviewer, reviewers.size, weight) + elsif reviewer.hungry + reviewers.fill(reviewer, reviewers.size, weight * CAPACITY_MULTIPLIER + DEFAULT_REVIEWER_WEIGHT) + else + reviewers.fill(reviewer, reviewers.size, weight * CAPACITY_MULTIPLIER) + end + end + end + end + end +end diff --git a/tooling/gitlab_danger.rb b/tooling/gitlab_danger.rb new file mode 100644 index 00000000000..d20d3499641 --- /dev/null +++ b/tooling/gitlab_danger.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +# rubocop:todo Gitlab/NamespacedClass +class GitlabDanger + LOCAL_RULES ||= %w[ + changes_size + commit_messages + database + documentation + duplicate_yarn_dependencies + eslint + karma + pajamas + pipeline + prettier + product_intelligence + utility_css + ].freeze + + CI_ONLY_RULES ||= %w[ + ce_ee_vue_templates + changelog + ci_templates + metadata + feature_flag + roulette + sidekiq_queues + specialization_labels + specs + ].freeze + + MESSAGE_PREFIX = '==>'.freeze + + attr_reader :gitlab_danger_helper + + def initialize(gitlab_danger_helper) + @gitlab_danger_helper = gitlab_danger_helper + end + + def self.local_warning_message + "#{MESSAGE_PREFIX} Only the following Danger rules can be run locally: #{LOCAL_RULES.join(', ')}" + end + + def self.success_message + "#{MESSAGE_PREFIX} No Danger rule violations!" + end + + def rule_names + ci? ? LOCAL_RULES | CI_ONLY_RULES : LOCAL_RULES + end + + def html_link(str) + self.ci? ? gitlab_danger_helper.html_link(str) : str + end + + def ci? + !gitlab_danger_helper.nil? + end +end diff --git a/tooling/lib/tooling/kubernetes_client.rb b/tooling/lib/tooling/kubernetes_client.rb index f7abc5ac4cf..35605fd493c 100644 --- a/tooling/lib/tooling/kubernetes_client.rb +++ b/tooling/lib/tooling/kubernetes_client.rb @@ -43,7 +43,6 @@ module Tooling %(--namespace "#{namespace}"), '--now', '--ignore-not-found', - '--include-uninitialized', %(--wait=#{wait}), selector ] @@ -58,7 +57,6 @@ module Tooling %(--namespace "#{namespace}"), '--now', '--ignore-not-found', - '--include-uninitialized', %(--wait=#{wait}), resource_names.join(' ') ] |