diff --git a/danger/pipeline/Dangerfile b/danger/pipeline/Dangerfile index c61fca86bebd14905218518a9d22a1771915527d..090faf620ee12ccdfb294024f160efdd3383b5ca 100644 --- a/danger/pipeline/Dangerfile +++ b/danger/pipeline/Dangerfile @@ -19,4 +19,7 @@ MSG if helper.has_ci_changes? markdown(PIPELINE_CHANGES_MESSAGE) + + dependency_validation_message = ci_jobs_dependency_validation.output_message + markdown(dependency_validation_message) unless dependency_validation_message.empty? end diff --git a/danger/plugins/ci_jobs_dependency_validation.rb b/danger/plugins/ci_jobs_dependency_validation.rb new file mode 100644 index 0000000000000000000000000000000000000000..0d35eb62ed285f9d40aee10decc449b78e4eac5d --- /dev/null +++ b/danger/plugins/ci_jobs_dependency_validation.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require_relative '../../tooling/danger/ci_jobs_dependency_validation' + +module Danger + class CiJobsDependencyValidation < ::Danger::Plugin + include Tooling::Danger::CiJobsDependencyValidation + end +end diff --git a/spec/tooling/danger/ci_jobs_dependency_validation_spec.rb b/spec/tooling/danger/ci_jobs_dependency_validation_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..192bc7699929253f52afa6c6b8432f84d702fd64 --- /dev/null +++ b/spec/tooling/danger/ci_jobs_dependency_validation_spec.rb @@ -0,0 +1,364 @@ +# frozen_string_literal: true + +require 'gitlab/dangerfiles/spec_helper' +require 'fast_spec_helper' +require 'webmock/rspec' + +require_relative '../../../tooling/danger/ci_jobs_dependency_validation' + +RSpec.describe Tooling::Danger::CiJobsDependencyValidation, feature_category: :tooling do + include_context 'with dangerfile' + + let(:ci) { true } + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + + let(:rules_base) do + [ + { + 'if' => '$CI_MERGE_REQUEST_EVENT_TYPE == "merged_result" || $CI_MERGE_REQUEST_EVENT_TYPE == "detached"', + 'changes' => ["doc/index.md"] + } + ] + end + + let(:job_config_base) { { 'rules' => rules_base, 'needs' => [] } } + let(:new_condition) { { 'if' => '$NEW_VAR == "true"' } } + + let(:rules_with_new_condition) { [*rules_base, new_condition] } + + let(:source_branch_jobs_base) do + described_class::VALIDATED_JOB_NAMES.index_with { job_config_base } + end + + let(:source_branch_merged_yaml) { YAML.dump(source_branch_jobs_base) } + + let(:master_merged_yaml) do + YAML.dump({ + 'job1' => job_config_base + }) + end + + let(:query) do + { + content_ref: 'feature_branch', + dry_run_ref: 'feature_branch', + include_jobs: true, + dry_run: true + } + end + + subject(:ci_jobs_dependency_validation) { fake_danger.new(helper: fake_helper) } + + before do + allow(ci_jobs_dependency_validation).to receive_message_chain(:gitlab, :api, :get).with("/projects/1/ci/lint", + query: {}) do + { 'merged_yaml' => master_merged_yaml } + end + + allow(ci_jobs_dependency_validation).to receive_message_chain(:gitlab, :api, :get).with("/projects/1/ci/lint", + query: query) do + { 'merged_yaml' => source_branch_merged_yaml } + end + + allow(ci_jobs_dependency_validation.helper).to receive(:ci?).and_return(ci) + allow(ci_jobs_dependency_validation.helper).to receive(:has_ci_changes?).and_return(true) + allow(ci_jobs_dependency_validation.helper).to receive(:mr_source_branch).and_return('feature_branch') + allow(ci_jobs_dependency_validation.helper).to receive(:mr_target_branch).and_return('master') + allow(ci_jobs_dependency_validation.helper).to receive(:mr_source_project_id).and_return('1') + allow(ci_jobs_dependency_validation.helper).to receive(:mr_target_project_id).and_return('1') + end + + describe '#output_message' do + shared_examples 'empty message' do |output, num_of_jobs_in_target_branch, num_of_jobs_in_source_branch| + it 'returns empty string and prints the correct messages to stdout' do + default_output = <<~OUTPUT + Initializing #{num_of_jobs_in_target_branch} jobs from master ci config... + Initializing #{num_of_jobs_in_source_branch} jobs from feature_branch ci config... + Looking for misconfigured dependent jobs for setup-test-env... + Detected 0 dependent jobs with misconfigured rules. + Looking for misconfigured dependent jobs for compile-test-assets... + Detected 0 dependent jobs with misconfigured rules. + Looking for misconfigured dependent jobs for retrieve-tests-metadata... + Detected 0 dependent jobs with misconfigured rules. + Looking for misconfigured dependent jobs for build-gdk-image... + Detected 0 dependent jobs with misconfigured rules. + OUTPUT + + expect { expect(ci_jobs_dependency_validation.output_message).to eq('') }.tap do |expectation| + expected_output = output == :default_stdout_output ? default_output : output + expectation.to output(expected_output).to_stdout unless expected_output.nil? + end + end + end + + context 'when not in ci environment' do + let(:ci) { false } + + it_behaves_like 'empty message', nil + end + + context 'when in ci environment' do + context 'with no ci changes' do + before do + allow(ci_jobs_dependency_validation.helper).to receive(:has_ci_changes?).and_return(false) + end + + it_behaves_like 'empty message' + end + + context 'when target branch jobs is empty' do + let(:source_branch_merged_yaml) { YAML.dump({}) } + + it_behaves_like 'empty message' + end + + context 'when source branch jobs is empty' do + let(:master_merged_yaml) { YAML.dump({}) } + + it_behaves_like 'empty message' + end + + context 'when jobs do not have dependencies' do + it_behaves_like 'empty message', :default_stdout_output, 1, 4 + end + + context 'when needed jobs are missing is source branch' do + let(:source_branch_merged_yaml) do + YAML.dump({ 'job1' => job_config_base.merge({ 'rules' => rules_with_new_condition }) }) + end + + it 'returns warning for the missing jobs' do + expect(ci_jobs_dependency_validation.output_message).to eq( + <<~MARKDOWN.chomp + Unable to find job setup-test-env in feature_branch. Skipping. + Unable to find job compile-test-assets in feature_branch. Skipping. + Unable to find job retrieve-tests-metadata in feature_branch. Skipping. + Unable to find job build-gdk-image in feature_branch. Skipping. + MARKDOWN + ) + end + end + + context 'when job1 in branch needs one other job to run' do + let(:job_name) { 'job1' } + let(:needed_job_name) { 'setup-test-env' } + let(:needed_job_config) { job_config_base } + let(:needed_job) { { needed_job_name => needed_job_config } } + + let(:source_branch_merged_yaml) do + YAML.dump(source_branch_jobs_base.merge( + { + job_name => { + 'rules' => rules_with_new_condition, + 'needs' => [needed_job_name] + } + } + )) + end + + context 'with a hidden job' do + let(:job_name) { '.job1' } + + it_behaves_like 'empty message', :default_stdout_output, 1, 5 + end + + context 'with a ignored job' do + let(:job_name) { 'default' } + + it_behaves_like 'empty message', :default_stdout_output, 1, 5 + end + + context 'when the dependent job config has not changed (identical in master and in branch)' do + let(:master_merged_yaml) { source_branch_merged_yaml } + + it_behaves_like 'empty message', :default_stdout_output, 5, 5 + end + + context 'when VALIDATED_JOB_NAMES does not contain the needed job' do + let(:needed_job_name) { 'not-recognized' } + + it_behaves_like 'empty message', :default_stdout_output, 1, 5 + end + + context 'when VALIDATED_JOB_NAMES contains the needed job and dependent job config changed' do + context 'when the added rule is also present in its needed job' do + let(:needed_job_config) { job_config_base.merge({ 'rules' => rules_with_new_condition }) } + + it_behaves_like 'empty message' + end + + context 'when an added rule is missing in its needed job' do + it 'returns warning' do + expect(ci_jobs_dependency_validation.output_message).to eq( + <<~MARKDOWN + **This MR adds new rules to the following dependent jobs for `setup-test-env`:** + + `job1`: + + ```yaml + - if: $NEW_VAR == "true" + ``` + + Please ensure the changes are included in the rules for `setup-test-env` to avoid yaml syntax error! + + <details><summary>Click to expand rules for setup-test-env to confirm if the new conditions are present</summary> + + ```yaml + - if: $CI_MERGE_REQUEST_EVENT_TYPE == "merged_result" || $CI_MERGE_REQUEST_EVENT_TYPE + == "detached" + changes: + - doc/index.md + ``` + + </details> + MARKDOWN + ) + end + end + end + end + + context 'when job configs are malformatted' do + let(:source_branch_merged_yaml) do + YAML.dump(source_branch_jobs_base.merge( + { + 'job1' => 'not a hash', + 'job2' => ['array'], + 'job3' => { 'key' => 'missing needs and rules' } + } + )) + end + + it_behaves_like 'empty message', :default_stdout_output, 1, 7 + end + + context 'when dependent job has a rule that is not a hash' do + let(:source_branch_merged_yaml) do + YAML.dump( + source_branch_jobs_base.merge({ + 'job1' => { + 'rules' => ['this is a malformatted rule'], + 'needs' => 'this is a malformatted needs' + } + }) + ) + end + + it_behaves_like 'empty message', :default_stdout_output, 1, 5 + end + + context 'when dependent job have an added rule but condition reads "when: never"' do + let(:new_condition) { { 'if' => "$NEW_VAR == true", 'when' => 'never' } } + let(:source_branch_merged_yaml) do + YAML.dump( + source_branch_jobs_base.merge({ + 'job1' => { + 'rules' => [new_condition], + 'needs' => ['setup-test-env'] + } + }) + ) + end + + it_behaves_like 'empty message', <<~OUTPUT + Initializing 1 jobs from master ci config... + Initializing 5 jobs from feature_branch ci config... + Looking for misconfigured dependent jobs for setup-test-env... + Detected 0 jobs with applicable rule changes. + Detected 0 dependent jobs with misconfigured rules. + Looking for misconfigured dependent jobs for compile-test-assets... + Detected 0 dependent jobs with misconfigured rules. + Looking for misconfigured dependent jobs for retrieve-tests-metadata... + Detected 0 dependent jobs with misconfigured rules. + Looking for misconfigured dependent jobs for build-gdk-image... + Detected 0 dependent jobs with misconfigured rules. + OUTPUT + end + + context 'when dependent job have modified rules, but its attributes has nested arrays' do + let(:source_branch_merged_yaml) do + YAML.dump( + source_branch_jobs_base.merge({ + 'job1' => { + 'rules' => [{ 'if' => 'true', 'when' => 'always' }, [new_condition]], + 'needs' => ['setup-test-env', %w[compile-test-assets retrieve-tests-metadata]] + } + }) + ) + end + + it 'correctly parses input yaml and returns warning' do + expected_markdown = %w[setup-test-env compile-test-assets retrieve-tests-metadata].map do |job_name| + <<~MARKDOWN + **This MR adds new rules to the following dependent jobs for `#{job_name}`:** + + `job1`: + + ```yaml + - if: 'true' + when: always + - if: $NEW_VAR == "true" + ``` + + Please ensure the changes are included in the rules for `#{job_name}` to avoid yaml syntax error! + + <details><summary>Click to expand rules for #{job_name} to confirm if the new conditions are present</summary> + + ```yaml + - if: $CI_MERGE_REQUEST_EVENT_TYPE == "merged_result" || $CI_MERGE_REQUEST_EVENT_TYPE + == "detached" + changes: + - doc/index.md + ``` + + </details> + + MARKDOWN + end.join('').chomp + + expect(ci_jobs_dependency_validation.output_message).to eq(expected_markdown) + end + end + end + end + + describe '#fetch_jobs_yaml' do + context 'with api returns error' do + before do + allow( + ci_jobs_dependency_validation + ).to receive_message_chain(:gitlab, :api, :get).with("/projects/1/ci/lint", query: {}).and_raise('') + end + + it 'returns jobs yaml' do + expect(ci_jobs_dependency_validation.send(:fetch_jobs_yaml, '1', 'master')).to eq([]) + end + end + + context 'with returned payload missing merged_yaml' do + before do + allow( + ci_jobs_dependency_validation + ).to receive_message_chain(:gitlab, :api, :get).with("/projects/1/ci/lint", query: {}).and_return({}) + end + + it 'returns jobs yaml' do + expect(ci_jobs_dependency_validation.send(:fetch_jobs_yaml, '1', 'master')).to eq([]) + end + end + + context 'with returned merged_yaml cannot be parsed' do + before do + allow( + ci_jobs_dependency_validation + ).to receive_message_chain(:gitlab, :api, :get).with("/projects/1/ci/lint", query: {}).and_return( + { 'merged_yaml' => 'date: 2024-04-04' } + ) + end + + it 'returns jobs yaml' do + expect(ci_jobs_dependency_validation.send(:fetch_jobs_yaml, '1', 'master')).to eq([]) + end + end + end +end diff --git a/tooling/danger/ci_jobs_dependency_validation.rb b/tooling/danger/ci_jobs_dependency_validation.rb new file mode 100644 index 0000000000000000000000000000000000000000..3d02d6dd12767122bd844ea93279750903e28ba3 --- /dev/null +++ b/tooling/danger/ci_jobs_dependency_validation.rb @@ -0,0 +1,211 @@ +# frozen_string_literal: true + +module Tooling + module Danger + module CiJobsDependencyValidation + VALIDATED_JOB_NAMES = %w[setup-test-env compile-test-assets retrieve-tests-metadata build-gdk-image].freeze + GLOBAL_KEYWORDS = %w[workflow variables stages default].freeze + DEFAULT_BRANCH_NAME = 'master' + + Job = Struct.new(:name, :rules, :needs, keyword_init: true) do + def self.parse_rules_from_yaml(name, jobs_yaml) + attribute_values(jobs_yaml, name, 'rules').filter_map do |rule| + next if rule['when'] == 'manual' || rule['when'] == 'never' + + rule.is_a?(Hash) ? rule.slice('if', 'changes', 'when') : rule + end + end + + def self.parse_needs_from_yaml(name, jobs_yaml) + attribute_values(jobs_yaml, name, 'needs').map { |need| need.is_a?(Hash) ? need['job'] : need } + end + + def self.attribute_values(jobs_yaml, name, attribute) + return [] if jobs_yaml.nil? || jobs_yaml.empty? || !jobs_yaml[name].is_a?(Hash) + + values = jobs_yaml.dig(name, attribute) + values.nil? ? [] : Array(values).flatten + end + + def self.ignore?(job_name) + # hidden jobs are extended by other jobs thus their rules will be verified in the extending jobs + GLOBAL_KEYWORDS.include?(job_name) || job_name.start_with?('.') + end + + def dependent_jobs(jobs) + jobs.select do |job| + !Job.ignore?(job.name) && job.needs.include?(name) + end + end + end + + def output_message + return '' if !helper.ci? || !helper.has_ci_changes? || target_branch_jobs.empty? || source_branch_jobs.empty? + + VALIDATED_JOB_NAMES.filter_map do |needed_job_name| + construct_warning_message(needed_job_name) + end.join("\n") + end + + private + + def target_branch_jobs + @target_branch_jobs ||= build_jobs_from_yaml(target_branch_jobs_yaml, target_branch) + end + + def source_branch_jobs + @source_branch_jobs ||= build_jobs_from_yaml(source_branch_jobs_yaml, source_branch) + end + + def target_branch_jobs_yaml + @target_branch_jobs_yaml ||= fetch_jobs_yaml(target_project_id, target_branch) + end + + def source_branch_jobs_yaml + @source_branch_jobs_yaml ||= fetch_jobs_yaml(source_project_id, source_branch) + end + + def fetch_jobs_yaml(project_id, ref) + api_response = gitlab.api.get(lint_path(project_id), query: query_params(ref)) + + YAML.load(api_response['merged_yaml'], aliases: true) + rescue StandardError => e + puts e.message + [] + end + + def build_jobs_from_yaml(jobs_yaml, ref) + puts "Initializing #{jobs_yaml.keys.count} jobs from #{ref} ci config..." + + jobs_yaml.filter_map do |job_name, _job_data| + next if Job.ignore?(job_name) + + Job.new( + name: job_name, + rules: Job.parse_rules_from_yaml(job_name, jobs_yaml), + needs: Job.parse_needs_from_yaml(job_name, jobs_yaml) + ) + end + end + + def query_params(ref) + ref_query_params = { + content_ref: ref, + dry_run_ref: ref, + include_jobs: true, + dry_run: true + } + + ref == DEFAULT_BRANCH_NAME ? {} : ref_query_params + end + + def construct_warning_message(needed_job_name) + needed_job_in_source_branch = source_branch_jobs.find { |job| job.name == needed_job_name } + needed_job_in_target_branch = target_branch_jobs.find { |job| job.name == needed_job_name } + + if needed_job_in_source_branch.nil? + return "Unable to find job #{needed_job_name} in #{source_branch}. Skipping." + end + + puts "Looking for misconfigured dependent jobs for #{needed_job_name}..." + + warnings = changed_jobs_warnings( + needed_job_in_source_branch: needed_job_in_source_branch, + needed_job_in_target_branch: needed_job_in_target_branch + ) + + puts "Detected #{warnings.count} dependent jobs with misconfigured rules." + + return if warnings.empty? + + <<~MSG + **This MR adds new rules to the following dependent jobs for `#{needed_job_name}`:** + + #{warnings.join("\n")} + + Please ensure the changes are included in the rules for `#{needed_job_name}` to avoid yaml syntax error! + + <details><summary>Click to expand rules for #{needed_job_name} to confirm if the new conditions are present</summary> + + ```yaml + #{dump_yaml(needed_job_in_source_branch.rules)} + ``` + + </details> + MSG + end + + def changed_jobs_warnings(needed_job_in_source_branch:, needed_job_in_target_branch:) + dependent_jobs_new = needed_job_in_source_branch&.dependent_jobs(source_branch_jobs) || [] + dependent_jobs_old = needed_job_in_target_branch&.dependent_jobs(target_branch_jobs) || [] + + (dependent_jobs_new - dependent_jobs_old).filter_map do |dependent_job_with_rule_change| + dependent_job_old = dependent_jobs_old.find do |target_branch_job| + target_branch_job.name == dependent_job_with_rule_change.name + end + + report_candidates = if dependent_job_old.nil? + dependent_job_with_rule_change.rules + else + dependent_job_with_rule_change.rules - dependent_job_old.rules + end + + puts "Detected #{report_candidates.count} jobs with applicable rule changes." + + rules_to_report = exact_rules_missing_in_needed_job( + needed_job: needed_job_in_source_branch, + rules: report_candidates + ) + + next if rules_to_report.empty? + + <<~MARKDOWN.chomp + `#{dependent_job_with_rule_change.name}`: + + ```yaml + #{dump_yaml(rules_to_report)} + ``` + MARKDOWN + end + end + + def dump_yaml(yaml) + YAML.dump(yaml).delete_prefix("---\n").chomp + end + + # Limitation: "exact" rules missing does not always mean the needed_job is missing the rules + # needed_jobs can have very generic rules, for example + # - rule-for-job1: + # - <<: *if-merge-request-targeting-stable-branch + # - rule-for-needed_job: + # - <<: *if-merge-request-targeting-all-branches + # The above config is still valid, but danger will still print a warning because the exact rule is missing. + # We will have to manually identify that this config is fine and the warning should be ignored. + def exact_rules_missing_in_needed_job(rules:, needed_job:) + return [] if rules.empty? + + rules.select { |rule| !needed_job.rules.include?(rule) } + end + + def lint_path(project_id) + "/projects/#{project_id}/ci/lint" + end + + def source_project_id + helper.mr_source_project_id + end + + def target_project_id + helper.mr_target_project_id + end + + def source_branch + helper.mr_source_branch + end + + def target_branch + helper.mr_target_branch + end + end + end +end