diff --git a/Dangerfile b/Dangerfile
index 103e38fdd332c5516f4bdd36e12134914ff88ba9..34e0efa027a3dc1a310c4e42a039717f5e8d240a 100644
--- a/Dangerfile
+++ b/Dangerfile
@@ -3,10 +3,7 @@
require_relative 'tooling/gitlab_danger'
require_relative 'tooling/danger/request_helper'
-danger.import_plugin('danger/plugins/helper.rb')
-danger.import_plugin('danger/plugins/roulette.rb')
-danger.import_plugin('danger/plugins/changelog.rb')
-danger.import_plugin('danger/plugins/sidekiq_queues.rb')
+Dir["danger/plugins/*.rb"].sort.each { |f| danger.import_plugin(f) }
return if helper.release_automation?
diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile
index 06593a040932ea36560fde978f803deb363943eb..c8474157fa56ad051baaa558048f86ec1a077750 100644
--- a/danger/changelog/Dangerfile
+++ b/danger/changelog/Dangerfile
@@ -35,7 +35,7 @@ def check_changelog_yaml(path)
elsif yaml["merge_request"] != gitlab.mr_json["iid"] && !cherry_pick_against_stable_branch
fail "Merge request ID was not set to #{gitlab.mr_json["iid"]}! #{SEE_DOC}"
end
-rescue Psych::SyntaxError, Psych::DisallowedClass, Psych::BadAlias
+rescue Psych::Exception
# YAML could not be parsed, fail the build.
fail "#{gitlab.html_link(path)} isn't valid YAML! #{SEE_DOC}"
rescue StandardError => e
diff --git a/danger/feature_flag/Dangerfile b/danger/feature_flag/Dangerfile
new file mode 100644
index 0000000000000000000000000000000000000000..089ee2dc50bf1fb8a90a258f2f39504fb7710649
--- /dev/null
+++ b/danger/feature_flag/Dangerfile
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+# rubocop:disable Style/SignalException
+
+SEE_DOC = "See the [feature flag documentation](https://docs.gitlab.com/ee/development/feature_flags/development.html#feature-flag-definition-and-validation)."
+
+SUGGEST_MR_COMMENT = <<~SUGGEST_COMMENT
+```suggestion
+group: "%<group>s"
+```
+
+#{SEE_DOC}
+SUGGEST_COMMENT
+
+def check_feature_flag_yaml(feature_flag)
+ mr_group_label = helper.group_label(gitlab.mr_labels)
+
+ if feature_flag.group.nil?
+ message_for_feature_flag_missing_group!(feature_flag: feature_flag, mr_group_label: mr_group_label)
+ else
+ message_for_feature_flag_with_group!(feature_flag: feature_flag, mr_group_label: mr_group_label)
+ 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}"
+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}"
+ 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})
+ end
+ end
+end
+
+def message_for_feature_flag_with_group!(feature_flag:, mr_group_label:)
+ return if feature_flag.group_match_mr_label?(mr_group_label)
+
+ if mr_group_label.nil?
+ gitlab.api.update_merge_request(gitlab.mr_json['project_id'],
+ gitlab.mr_json['iid'],
+ add_labels: 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!)
+ end
+end
+
+feature_flag.feature_flag_files.each do |feature_flag|
+ check_feature_flag_yaml(feature_flag)
+end
diff --git a/danger/plugins/feature_flag.rb b/danger/plugins/feature_flag.rb
new file mode 100644
index 0000000000000000000000000000000000000000..8db8daba6d971adba628aa9a37c6b8ddd807eed4
--- /dev/null
+++ b/danger/plugins/feature_flag.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+require_relative '../../tooling/danger/feature_flag'
+
+module Danger
+ class FeatureFlag < Plugin
+ # Put the helper code somewhere it can be tested
+ include Tooling::Danger::FeatureFlag
+ end
+end
diff --git a/spec/tooling/danger/feature_flag_spec.rb b/spec/tooling/danger/feature_flag_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..1ee79e3c74e51b82e47d56080e2a1560fd353dd1
--- /dev/null
+++ b/spec/tooling/danger/feature_flag_spec.rb
@@ -0,0 +1,112 @@
+# frozen_string_literal: true
+
+require_relative 'danger_spec_helper'
+
+require_relative '../../../tooling/danger/feature_flag'
+
+RSpec.describe Tooling::Danger::FeatureFlag do
+ include DangerSpecHelper
+
+ let(:added_files) { nil }
+ let(:fake_git) { double('fake-git', added_files: added_files) }
+
+ let(:mr_labels) { nil }
+ let(:mr_json) { nil }
+ let(:fake_gitlab) { double('fake-gitlab', mr_labels: mr_labels, mr_json: mr_json) }
+
+ let(:changes_by_category) { nil }
+ let(:sanitize_mr_title) { nil }
+ let(:ee?) { false }
+ let(:fake_helper) { double('fake-helper', changes_by_category: changes_by_category, sanitize_mr_title: sanitize_mr_title, ee?: ee?) }
+
+ let(:fake_danger) { new_fake_danger.include(described_class) }
+
+ subject(:feature_flag) { fake_danger.new(git: fake_git, gitlab: fake_gitlab, helper: fake_helper) }
+
+ describe '#feature_flag_files' do
+ context 'added files contain several feature flags' do
+ let(:added_files) do
+ [
+ 'config/feature_flags/development/entry.yml',
+ 'ee/config/feature_flags/ops/entry.yml'
+ ]
+ end
+
+ it 'returns an array of Found objects' do
+ expect(feature_flag.feature_flag_files).to contain_exactly(an_instance_of(described_class::Found), an_instance_of(described_class::Found))
+ end
+ end
+
+ context 'added files do not contain a feature_flag' do
+ let(:added_files) do
+ [
+ 'app/models/model.rb',
+ 'app/assets/javascripts/file.js'
+ ]
+ end
+
+ it 'returns the feature flag file path' do
+ expect(feature_flag.feature_flag_files).to be_empty
+ end
+ end
+ end
+
+ describe described_class::Found do
+ let(:feature_flag_path) { 'config/feature_flags/development/entry.yml' }
+ let(:group) { 'group::source code' }
+ let(:raw_yaml) do
+ YAML.dump('group' => group)
+ end
+
+ subject(:found) { described_class.new(feature_flag_path) }
+
+ before do
+ allow(File).to receive(:read).and_call_original
+ expect(File).to receive(:read).with(feature_flag_path).and_return(raw_yaml)
+ end
+
+ describe '#raw' do
+ it 'returns the raw YAML' do
+ expect(found.raw).to eq(raw_yaml)
+ end
+ end
+
+ describe '#group' do
+ it 'returns the group found in the YAML' do
+ expect(found.group).to eq(group)
+ end
+ end
+
+ describe '#group_match_mr_label?' do
+ subject(:result) { found.group_match_mr_label?(mr_group_label) }
+
+ context 'when MR labels match FF group' do
+ let(:mr_group_label) { 'group::source code' }
+
+ specify { expect(result).to eq(true) }
+ end
+
+ context 'when MR labels does not match FF group' do
+ let(:mr_group_label) { 'group::access' }
+
+ specify { expect(result).to eq(false) }
+ end
+
+ context 'when group is nil' do
+ let(:group) { nil }
+
+ context 'and MR has no group label' do
+ let(:mr_group_label) { nil }
+
+ specify { expect(result).to eq(true) }
+ end
+
+ context 'and MR has a group label' do
+ let(:mr_group_label) { 'group::source code' }
+
+ specify { expect(result).to eq(false) }
+ end
+ end
+ end
+ end
+end
diff --git a/spec/tooling/danger/helper_spec.rb b/spec/tooling/danger/helper_spec.rb
index 69aefddc53639cf6dd0dbfb13ed918aa311953f6..40170dcd355059c0c8f16c3429c8aae90dad6db0 100644
--- a/spec/tooling/danger/helper_spec.rb
+++ b/spec/tooling/danger/helper_spec.rb
@@ -599,4 +599,14 @@
end
end
end
+
+ describe '#group_label' do
+ it 'returns nil when no group label is present' do
+ expect(helper.group_label(%w[foo bar])).to be_nil
+ end
+
+ it 'returns the group label when a group label is present' do
+ expect(helper.group_label(['foo', 'group::source code', 'bar'])).to eq('group::source code')
+ end
+ end
end
diff --git a/tooling/danger/feature_flag.rb b/tooling/danger/feature_flag.rb
new file mode 100644
index 0000000000000000000000000000000000000000..27352457f95aa4e7d18af3262215171aaf4b5531
--- /dev/null
+++ b/tooling/danger/feature_flag.rb
@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+require 'yaml'
+
+module Tooling
+ module Danger
+ module FeatureFlag
+ def feature_flag_files
+ @feature_flag_files ||= git.added_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
index 70df0dd82c75d46a99c92aa509a9a7eaa13a643f..afb6220e1168b7974e73e7327a12200e13d4c1a5 100644
--- a/tooling/danger/helper.rb
+++ b/tooling/danger/helper.rb
@@ -260,13 +260,17 @@ def changed_files(regex)
all_changed_files.grep(regex)
end
- def has_database_scoped_labels?(current_mr_labels)
- current_mr_labels.any? { |label| label.start_with?('database::') }
+ 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/gitlab_danger.rb b/tooling/gitlab_danger.rb
index ca62e93a59d53b3ba382d6731f610fa547243256..d20d3499641778eff002a2951262f9db90e16098 100644
--- a/tooling/gitlab_danger.rb
+++ b/tooling/gitlab_danger.rb
@@ -4,28 +4,29 @@
class GitlabDanger
LOCAL_RULES ||= %w[
changes_size
+ commit_messages
+ database
documentation
duplicate_yarn_dependencies
- prettier
eslint
karma
- database
- commit_messages
- product_intelligence
- utility_css
pajamas
pipeline
+ prettier
+ product_intelligence
+ utility_css
].freeze
CI_ONLY_RULES ||= %w[
- metadata
+ ce_ee_vue_templates
changelog
- specs
+ ci_templates
+ metadata
+ feature_flag
roulette
- ce_ee_vue_templates
sidekiq_queues
specialization_labels
- ci_templates
+ specs
].freeze
MESSAGE_PREFIX = '==>'.freeze