From 125a8198799fb6971076b47871b198999dc76f55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yves=20Desgagn=C3=A9?= Date: Sun, 17 May 2026 16:00:57 -0400 Subject: [PATCH] Refactor builders: extract long methods, dedupe property arrays, STDERR --- bin/github-build.rb | 4 +- lib/ghb/application.rb | 4 +- lib/ghb/aws_job_builder.rb | 12 +- lib/ghb/code_deploy_job_builder.rb | 22 ++- lib/ghb/dependabot_manager.rb | 2 +- lib/ghb/file_scanner.rb | 12 +- lib/ghb/language_job_builder.rb | 237 +++++++++++++++++------------ lib/ghb/licenses_job_builder.rb | 4 +- lib/ghb/linter_job_builder.rb | 4 +- lib/ghb/repository_configurator.rb | 153 ++++++++++--------- lib/ghb/slack_job_builder.rb | 4 +- lib/ghb/workflow/job.rb | 9 +- lib/ghb/workflow/step.rb | 9 +- lib/ghb/workflow/workflow.rb | 21 ++- 14 files changed, 287 insertions(+), 210 deletions(-) diff --git a/bin/github-build.rb b/bin/github-build.rb index ac86dca..331991d 100755 --- a/bin/github-build.rb +++ b/bin/github-build.rb @@ -7,10 +7,10 @@ begin exit(GHB::Application.new(ARGV).execute) rescue GHB::ConfigError => e - puts("Error: #{e.message}") + warn("Error: #{e.message}") exit(1) rescue StandardError => e - puts("Error: #{e.message}") + warn("Error: #{e.message}") warn(e.backtrace.join("\n")) if ENV['DEBUG'] exit(1) end diff --git a/lib/ghb/application.rb b/lib/ghb/application.rb index 581fdf5..7b640f9 100644 --- a/lib/ghb/application.rb +++ b/lib/ghb/application.rb @@ -234,12 +234,12 @@ def workflow_set_defaults @new_workflow.run_name = @old_workflow.run_name unless @old_workflow.run_name.nil? @new_workflow.permissions = - if @old_workflow.permissions.is_a?(Hash) && @old_workflow.permissions.any? + if @old_workflow.permissions.any? @old_workflow.permissions else { contents: 'read', 'pull-requests': 'read' } end - @new_workflow.env = @old_workflow.env.is_a?(Hash) ? @old_workflow.env : {} + @new_workflow.env = @old_workflow.env @new_workflow.defaults = @old_workflow.defaults || {} @new_workflow.concurrency = @old_workflow.concurrency || {} end diff --git a/lib/ghb/aws_job_builder.rb b/lib/ghb/aws_job_builder.rb index 1923d68..eddfd1c 100644 --- a/lib/ghb/aws_job_builder.rb +++ b/lib/ghb/aws_job_builder.rb @@ -15,21 +15,19 @@ def build return unless File.exist?('.aws') puts(' Adding aws commands...') - needs = @new_workflow.jobs.keys.map(&:to_s) - base_condition = "always() && (needs.variables.outputs.DEPLOY_ON_BETA == '1' || needs.variables.outputs.DEPLOY_ON_RC == '1' || needs.variables.outputs.DEPLOY_ON_PROD == '1')" - job_conditions = @new_workflow.jobs.keys.map { |job_name| "needs.#{job_name}.result != 'failure'" } - if_statement = ([base_condition] + job_conditions).join(' && ') + needs = @new_workflow.deploy_needs + if_statement = @new_workflow.deploy_if_statement old_workflow = @old_workflow @new_workflow.do_job(:aws) do - copy_properties(old_workflow.jobs[id], %i[name permissions needs if runs_on environment concurrency outputs env defaults timeout_minutes strategy continue_on_error container services uses with secrets]) + copy_properties(old_workflow.jobs[id]) do_name('AWS') do_runs_on(DEFAULT_UBUNTU_VERSION) do_needs(needs) - do_if("${{#{if_statement}}}") + do_if(if_statement) do_step('AWS Commands') do - copy_properties(find_step(old_workflow.jobs[:aws]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) + copy_properties(find_step(old_workflow.jobs[:aws]&.steps, name)) do_uses("cloud-officer/ci-actions/aws@#{CI_ACTIONS_VERSION}") if with.empty? diff --git a/lib/ghb/code_deploy_job_builder.rb b/lib/ghb/code_deploy_job_builder.rb index 13d52b9..714e6d7 100644 --- a/lib/ghb/code_deploy_job_builder.rb +++ b/lib/ghb/code_deploy_job_builder.rb @@ -23,23 +23,21 @@ def build private def build_codedeploy_job - needs = @new_workflow.jobs.keys.map(&:to_s) - base_condition = "always() && (needs.variables.outputs.DEPLOY_ON_BETA == '1' || needs.variables.outputs.DEPLOY_ON_RC == '1' || needs.variables.outputs.DEPLOY_ON_PROD == '1')" - job_conditions = @new_workflow.jobs.keys.map { |job_name| "needs.#{job_name}.result != 'failure'" } - if_statement = ([base_condition] + job_conditions).join(' && ') + needs = @new_workflow.deploy_needs + if_statement = @new_workflow.deploy_if_statement code_deploy_pre_steps = @code_deploy_pre_steps old_workflow = @old_workflow @new_workflow.do_job(:codedeploy) do - copy_properties(old_workflow.jobs[id], %i[name permissions needs if runs_on environment concurrency outputs env defaults timeout_minutes strategy continue_on_error container services uses with secrets]) + copy_properties(old_workflow.jobs[id]) do_name('Code Deploy') do_runs_on(DEFAULT_UBUNTU_VERSION) do_needs(needs) - do_if("${{#{if_statement}}}") + do_if(if_statement) if code_deploy_pre_steps.empty? do_step('Checkout') do - copy_properties(find_step(old_workflow.jobs[:codedeploy]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) + copy_properties(find_step(old_workflow.jobs[:codedeploy]&.steps, name)) do_uses("cloud-officer/ci-actions/codedeploy/checkout@#{CI_ACTIONS_VERSION}") do_with({ 'ssh-key': '${{secrets.SSH_KEY}}', 'github-token': '${{secrets.GH_PAT}}' }) if with.empty? end @@ -52,20 +50,20 @@ def build_codedeploy_job end do_step('Update Packages') do - copy_properties(find_step(old_workflow.jobs[:codedeploy]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) + copy_properties(find_step(old_workflow.jobs[:codedeploy]&.steps, name)) do_if("${{needs.variables.outputs.UPDATE_PACKAGES == '1'}}") do_shell('bash') do_run('touch update-packages') end do_step('Zip') do - copy_properties(find_step(old_workflow.jobs[:codedeploy]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) + copy_properties(find_step(old_workflow.jobs[:codedeploy]&.steps, name)) do_shell('bash') do_run('zip --quiet --recurse-paths "${{needs.variables.outputs.BUILD_NAME}}.zip" ./*') if run.nil? end do_step('S3Copy') do - copy_properties(find_step(old_workflow.jobs[:codedeploy]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) + copy_properties(find_step(old_workflow.jobs[:codedeploy]&.steps, name)) do_uses("cloud-officer/ci-actions/codedeploy/s3copy@#{CI_ACTIONS_VERSION}") if with.empty? @@ -89,14 +87,14 @@ def build_environment_jobs %w[beta rc prod].each do |environment| @new_workflow.do_job(:"#{environment}_deploy") do - copy_properties(old_workflow.jobs[id], %i[name permissions needs if runs_on environment concurrency outputs env defaults timeout_minutes strategy continue_on_error container services uses with secrets]) + copy_properties(old_workflow.jobs[id]) do_name("#{environment.capitalize} Deploy") do_runs_on(DEFAULT_UBUNTU_VERSION) do_needs(%w[variables codedeploy]) do_if("${{always() && needs.codedeploy.result == 'success' && needs.variables.outputs.DEPLOY_ON_#{environment.upcase} == '1'}}") do_step("#{environment.capitalize} Deploy") do - copy_properties(find_step(old_workflow.jobs[:"#{environment}_deploy"]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) + copy_properties(find_step(old_workflow.jobs[:"#{environment}_deploy"]&.steps, name)) do_uses("cloud-officer/ci-actions/codedeploy/deploy@#{CI_ACTIONS_VERSION}") if with.empty? diff --git a/lib/ghb/dependabot_manager.rb b/lib/ghb/dependabot_manager.rb index a3beacf..a68d7f2 100644 --- a/lib/ghb/dependabot_manager.rb +++ b/lib/ghb/dependabot_manager.rb @@ -94,7 +94,7 @@ def save_dependencies_workflow end do_step('Licenses') do - copy_properties(new_workflow.jobs[:licenses]&.steps&.first, %i[id if uses run shell with env continue_on_error timeout_minutes]) + copy_properties(new_workflow.jobs[:licenses]&.steps&.first) do_uses("cloud-officer/ci-actions/soup@#{CI_ACTIONS_VERSION}") if with.empty? diff --git a/lib/ghb/file_scanner.rb b/lib/ghb/file_scanner.rb index 96c7a95..e83a777 100644 --- a/lib/ghb/file_scanner.rb +++ b/lib/ghb/file_scanner.rb @@ -12,12 +12,6 @@ def cached_file_read(path) @file_cache[path] ||= File.read(path) end - # Pure Ruby file finder - avoids shell command injection (SEC-001, SEC-002) - # @param path [String] starting directory path - # @param pattern [Regexp] file pattern to match - # @param excluded_paths [Array] paths to exclude (partial matches) - # @param max_depth [Integer, nil] maximum directory depth (nil for unlimited) - # @return [Array] list of matching file paths # Builds the list of excluded directory patterns from languages.yaml. # Combines install_dirs from all dependency entries with the top-level excluded_dirs. # @return [Array] directory names to exclude (e.g., ['node_modules', 'vendor', '.git']) @@ -42,6 +36,12 @@ def excluded_dirs_from_config end end + # Pure Ruby file finder - avoids shell command injection (SEC-001, SEC-002) + # @param path [String] starting directory path + # @param pattern [Regexp] file pattern to match + # @param excluded_paths [Array] paths to exclude (partial matches) + # @param max_depth [Integer, nil] maximum directory depth (nil for unlimited) + # @return [Array] list of matching file paths def find_files_matching(path, pattern, excluded_paths = [], max_depth: nil) matches = [] base_depth = path.count(File::SEPARATOR) diff --git a/lib/ghb/language_job_builder.rb b/lib/ghb/language_job_builder.rb index f1db721..eb957f1 100644 --- a/lib/ghb/language_job_builder.rb +++ b/lib/ghb/language_job_builder.rb @@ -7,6 +7,12 @@ module GHB class LanguageJobBuilder include FileScanner + # Deploy flags that extend the Swift unit-test `if:` so the job also runs on deploy triggers. + SWIFT_DEPLOY_CHECK_FLAGS = %w[DEPLOY_ON_BETA DEPLOY_ON_RC DEPLOY_ON_PROD DEPLOY_MACOS DEPLOY_TVOS].freeze + # Languages whose dependency steps must also be staged as CodeDeploy pre-steps. + CODEDEPLOY_SETUP_LANGUAGES = %w[go php].freeze + private_constant :SWIFT_DEPLOY_CHECK_FLAGS, :CODEDEPLOY_SETUP_LANGUAGES + attr_reader :code_deploy_pre_steps, :dependencies_steps, :dependencies_commands def initialize(options:, submodules:, old_workflow:, new_workflow:, unit_tests_conditions:, file_cache:, dependencies_commands:) @@ -123,132 +129,173 @@ def detect_language(language, service_options) add_language_job(language, setup_options, version_file, mono_dependency_locations) end - def add_language_job(language, setup_options, version_file, mono_dependency_locations) - additional_checks = - if language[:short_name] == 'swift' - " || (needs.variables.outputs.DEPLOY_ON_BETA == '1') || (needs.variables.outputs.DEPLOY_ON_RC == '1') || (needs.variables.outputs.DEPLOY_ON_PROD == '1') || (needs.variables.outputs.DEPLOY_MACOS == '1') || (needs.variables.outputs.DEPLOY_TVOS == '1')" - else - '' - end + # True for languages that get the extended Swift deploy `if:` checks. + def extra_deploy_checks?(language) + language[:short_name] == 'swift' + end + + # Swift + Xcode Cloud: collect dependency info but drop the unit-test job. + def xcode_cloud_unit_tests?(language) + language[:short_name] == 'swift' && Dir.exist?('ci_scripts') + end + + # Whether this language's dependency steps must also be staged as CodeDeploy pre-steps. + def needs_codedeploy_setup?(language) + CODEDEPLOY_SETUP_LANGUAGES.include?(language[:short_name]) || @options.force_codedeploy_setup + end + + # The extra `|| (...)` deploy checks appended to the Swift unit-test `if:`. + def additional_unit_test_checks(language) + return '' unless extra_deploy_checks?(language) + checks = SWIFT_DEPLOY_CHECK_FLAGS.map { |flag| "(needs.variables.outputs.#{flag} == '1')" } + + " || #{checks.join(' || ')}" + end + + def add_language_job(language, setup_options, version_file, mono_dependency_locations) + additional_checks = additional_unit_test_checks(language) skip_license_check = @options.skip_license_check - force_codedeploy_setup = @options.force_codedeploy_setup + needs_codedeploy_setup = needs_codedeploy_setup?(language) old_workflow = @old_workflow unit_tests_conditions = @unit_tests_conditions - code_deploy_pre_steps = @code_deploy_pre_steps - dependencies_steps = @dependencies_steps - dependencies_commands_additions = @dependencies_commands_additions - - # For Swift with Xcode Cloud (ci_scripts), skip the unit test job but still collect dependency info - skip_unit_test_job = language[:short_name] == 'swift' && Dir.exist?('ci_scripts') + # Swift with Xcode Cloud (ci_scripts): build the job to collect dependency info, then delete it below. + skip_unit_test_job = xcode_cloud_unit_tests?(language) + builder = self @new_workflow.do_job(:"#{language[:short_name]}_unit_tests") do - copy_properties(old_workflow.jobs[id], %i[name permissions needs if runs_on environment concurrency outputs env defaults timeout_minutes strategy continue_on_error container services uses with secrets]) + copy_properties(old_workflow.jobs[id]) do_name("#{language[:long_name]} Unit Tests") do_runs_on(old_workflow.jobs[:"#{language[:short_name]}_unit_tests"]&.runs_on || language[:'runs-on'] || DEFAULT_UBUNTU_VERSION) do_needs(%w[variables]) do_if("${{#{unit_tests_conditions}#{additional_checks}}}") - do_step('Setup') do - copy_properties(find_step(old_workflow.jobs[:"#{language[:short_name]}_unit_tests"]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) - do_uses("cloud-officer/ci-actions/setup@#{CI_ACTIONS_VERSION}") + builder.__send__(:build_setup_step, self, language, version_file, setup_options, needs_codedeploy_setup) + dependency_detected = builder.__send__(:build_dependency_steps, self, language, needs_codedeploy_setup) + builder.__send__(:build_mono_dependency_steps, self, language, mono_dependency_locations) - # Remove version parameter from with if version file exists (version file takes precedence) - if version_file - version_option_key = (version_file == '.nvmrc' ? 'node-version' : version_file.delete_prefix('.')).to_sym - with.delete(version_option_key) - end + next unless dependency_detected || mono_dependency_locations.any? - if with.empty? - do_with( - { - 'ssh-key': '${{secrets.SSH_KEY}}', - 'github-token': '${{secrets.GH_PAT}}', - 'aws-access-key-id': '${{secrets.AWS_ACCESS_KEY_ID}}', - 'aws-secret-access-key': '${{secrets.AWS_SECRET_ACCESS_KEY}}', - 'aws-region': '${{secrets.AWS_DEFAULT_REGION}}' - }.merge(setup_options) - ) - end + builder.__send__(:build_licenses_step, self, language) if File.exist?('Podfile.lock') && skip_license_check == false + end - with[:'github-token'] = '${{secrets.GH_PAT}}' + # Remove the unit test job from the workflow when Xcode Cloud handles tests, + # but dependency info (dependencies_steps, dependencies_commands) was still collected above + return unless skip_unit_test_job - code_deploy_pre_steps << duplicate(self) if language[:short_name] == 'go' or language[:short_name] == 'php' or force_codedeploy_setup - dependencies_steps << duplicate(self) + @new_workflow.jobs.delete(:"#{language[:short_name]}_unit_tests") + puts(" Skipping #{language[:long_name]} Unit Tests job (Xcode Cloud handles tests via ci_scripts)") + end + + def build_setup_step(job, language, version_file, setup_options, needs_codedeploy_setup) + old_workflow = @old_workflow + code_deploy_pre_steps = @code_deploy_pre_steps + dependencies_steps = @dependencies_steps + + job.do_step('Setup') do + copy_properties(find_step(old_workflow.jobs[:"#{language[:short_name]}_unit_tests"]&.steps, name)) + do_uses("cloud-officer/ci-actions/setup@#{CI_ACTIONS_VERSION}") + + # Remove version parameter from with if version file exists (version file takes precedence) + if version_file + version_option_key = (version_file == '.nvmrc' ? 'node-version' : version_file.delete_prefix('.')).to_sym + with.delete(version_option_key) end - dependency_detected = false + if with.empty? + do_with( + { + 'ssh-key': '${{secrets.SSH_KEY}}', + 'github-token': '${{secrets.GH_PAT}}', + 'aws-access-key-id': '${{secrets.AWS_ACCESS_KEY_ID}}', + 'aws-secret-access-key': '${{secrets.AWS_SECRET_ACCESS_KEY}}', + 'aws-region': '${{secrets.AWS_DEFAULT_REGION}}' + }.merge(setup_options) + ) + end - language[:dependencies].each do |dependency| - next unless File.file?(dependency[:dependency_file]) + with[:'github-token'] = '${{secrets.GH_PAT}}' - dependency_detected = true + code_deploy_pre_steps << duplicate(self) if needs_codedeploy_setup + dependencies_steps << duplicate(self) + end + end - do_step(dependency[:package_manager_name]) do - copy_properties(find_step(old_workflow.jobs[:"#{language[:short_name]}_unit_tests"]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) - do_shell('bash') - do_run(dependency[:package_manager_default]) if run.nil? - env['GITHUB_TOKEN'] = '${{secrets.GH_PAT}}' - code_deploy_pre_steps << duplicate(self) if language[:short_name] == 'go' or language[:short_name] == 'php' or force_codedeploy_setup - dependencies_commands_additions << dependency[:package_manager_update] if dependency[:package_manager_update] - end + def build_dependency_steps(job, language, needs_codedeploy_setup) + old_workflow = @old_workflow + code_deploy_pre_steps = @code_deploy_pre_steps + dependencies_commands_additions = @dependencies_commands_additions + dependency_detected = false + + language[:dependencies].each do |dependency| + next unless File.file?(dependency[:dependency_file]) + + dependency_detected = true + + job.do_step(dependency[:package_manager_name]) do + copy_properties(find_step(old_workflow.jobs[:"#{language[:short_name]}_unit_tests"]&.steps, name)) + do_shell('bash') + do_run(dependency[:package_manager_default]) if run.nil? + env['GITHUB_TOKEN'] = '${{secrets.GH_PAT}}' + code_deploy_pre_steps << duplicate(self) if needs_codedeploy_setup + dependencies_commands_additions << dependency[:package_manager_update] if dependency[:package_manager_update] end + end - if dependency_detected - do_step(language[:unit_test_framework_name]) do - copy_properties(find_step(old_workflow.jobs[:"#{language[:short_name]}_unit_tests"]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) - do_shell('bash') - do_run(language[:unit_test_framework_default]) if run.nil? - env['GITHUB_TOKEN'] = '${{secrets.GH_PAT}}' - end + if dependency_detected + job.do_step(language[:unit_test_framework_name]) do + copy_properties(find_step(old_workflow.jobs[:"#{language[:short_name]}_unit_tests"]&.steps, name)) + do_shell('bash') + do_run(language[:unit_test_framework_default]) if run.nil? + env['GITHUB_TOKEN'] = '${{secrets.GH_PAT}}' end + end - mono_dependency_locations.each do |loc| - dep = loc[:dependency] - subdir = loc[:subdir] - - do_step("#{dep[:package_manager_name]} (#{subdir})") do - copy_properties(find_step(old_workflow.jobs[:"#{language[:short_name]}_unit_tests"]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) - do_shell('bash') - do_run("cd #{subdir} && #{dep[:package_manager_default]}") if run.nil? - env['GITHUB_TOKEN'] = '${{secrets.GH_PAT}}' - dependencies_commands_additions << dep[:package_manager_update] if dep[:package_manager_update] - end + dependency_detected + end - do_step("#{language[:unit_test_framework_name]} (#{subdir})") do - copy_properties(find_step(old_workflow.jobs[:"#{language[:short_name]}_unit_tests"]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) - do_shell('bash') - do_run("cd #{subdir} && #{language[:unit_test_framework_default]}") if run.nil? - env['GITHUB_TOKEN'] = '${{secrets.GH_PAT}}' - end - end + def build_mono_dependency_steps(job, language, mono_dependency_locations) + old_workflow = @old_workflow + dependencies_commands_additions = @dependencies_commands_additions - next unless dependency_detected || mono_dependency_locations.any? + mono_dependency_locations.each do |loc| + dep = loc[:dependency] + subdir = loc[:subdir] - if File.exist?('Podfile.lock') and skip_license_check == false - do_step('Licenses') do - copy_properties(find_step(old_workflow.jobs[:"#{language[:short_name]}_unit_tests"]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) - do_uses("cloud-officer/ci-actions/soup@#{CI_ACTIONS_VERSION}") - - if with.empty? - do_with( - { - 'ssh-key': '${{secrets.SSH_KEY}}', - 'github-token': '${{secrets.GH_PAT}}', - parameters: '--no_prompt' - } - ) - end - end + job.do_step("#{dep[:package_manager_name]} (#{subdir})") do + copy_properties(find_step(old_workflow.jobs[:"#{language[:short_name]}_unit_tests"]&.steps, name)) + do_shell('bash') + do_run("cd #{subdir} && #{dep[:package_manager_default]}") if run.nil? + env['GITHUB_TOKEN'] = '${{secrets.GH_PAT}}' + dependencies_commands_additions << dep[:package_manager_update] if dep[:package_manager_update] + end + + job.do_step("#{language[:unit_test_framework_name]} (#{subdir})") do + copy_properties(find_step(old_workflow.jobs[:"#{language[:short_name]}_unit_tests"]&.steps, name)) + do_shell('bash') + do_run("cd #{subdir} && #{language[:unit_test_framework_default]}") if run.nil? + env['GITHUB_TOKEN'] = '${{secrets.GH_PAT}}' end end + end - # Remove the unit test job from the workflow when Xcode Cloud handles tests, - # but dependency info (dependencies_steps, dependencies_commands) was still collected above - return unless skip_unit_test_job + def build_licenses_step(job, language) + old_workflow = @old_workflow - @new_workflow.jobs.delete(:"#{language[:short_name]}_unit_tests") - puts(" Skipping #{language[:long_name]} Unit Tests job (Xcode Cloud handles tests via ci_scripts)") + job.do_step('Licenses') do + copy_properties(find_step(old_workflow.jobs[:"#{language[:short_name]}_unit_tests"]&.steps, name)) + do_uses("cloud-officer/ci-actions/soup@#{CI_ACTIONS_VERSION}") + + if with.empty? + do_with( + { + 'ssh-key': '${{secrets.SSH_KEY}}', + 'github-token': '${{secrets.GH_PAT}}', + parameters: '--no_prompt' + } + ) + end + end end def add_setup_options(setup_options, options, version_file = nil) diff --git a/lib/ghb/licenses_job_builder.rb b/lib/ghb/licenses_job_builder.rb index e5a39dd..325436b 100644 --- a/lib/ghb/licenses_job_builder.rb +++ b/lib/ghb/licenses_job_builder.rb @@ -26,14 +26,14 @@ def build old_workflow = @old_workflow @new_workflow.do_job(:licenses) do - copy_properties(old_workflow.jobs[id], %i[name permissions needs if runs_on environment concurrency outputs env defaults timeout_minutes strategy continue_on_error container services uses with secrets]) + copy_properties(old_workflow.jobs[id]) do_name('Licenses Check') do_runs_on(old_workflow.jobs[:licenses]&.runs_on || DEFAULT_UBUNTU_VERSION) do_needs(%w[variables]) do_if("${{needs.variables.outputs.SKIP_LICENSES != '1'}}") do_step('Licenses') do - copy_properties(find_step(old_workflow.jobs[:licenses]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) + copy_properties(find_step(old_workflow.jobs[:licenses]&.steps, name)) do_uses("cloud-officer/ci-actions/soup@#{CI_ACTIONS_VERSION}") if with.empty? diff --git a/lib/ghb/linter_job_builder.rb b/lib/ghb/linter_job_builder.rb index 651d808..5ecd69e 100644 --- a/lib/ghb/linter_job_builder.rb +++ b/lib/ghb/linter_job_builder.rb @@ -138,7 +138,7 @@ def add_linter_job(short_name, linter) old_workflow = @old_workflow @new_workflow.do_job(short_name) do - copy_properties(old_workflow.jobs[id], %i[name permissions needs if runs_on environment concurrency outputs env defaults timeout_minutes strategy continue_on_error container services uses with secrets]) + copy_properties(old_workflow.jobs[id]) do_name(linter[:long_name]) do_runs_on(old_workflow.jobs[short_name]&.runs_on || DEFAULT_UBUNTU_VERSION) do_needs(%w[variables]) @@ -151,7 +151,7 @@ def add_linter_job(short_name, linter) end do_step(linter[:short_name]) do - copy_properties(find_step(old_workflow.jobs[short_name]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) + copy_properties(find_step(old_workflow.jobs[short_name]&.steps, name)) do_uses("#{linter[:uses]}@#{CI_ACTIONS_VERSION}") if with.empty? diff --git a/lib/ghb/repository_configurator.rb b/lib/ghb/repository_configurator.rb index a6bd41f..ed82cf3 100644 --- a/lib/ghb/repository_configurator.rb +++ b/lib/ghb/repository_configurator.rb @@ -48,6 +48,32 @@ def configure private def configure_branch_protection(github_client, repo_url, current_protection, protection_exists) + augment_required_status_checks + log_codeql_languages(github_client, repo_url) + + # CodeQL checks are NOT included because they use "smart mode" which only runs + # when relevant files change. CodeQL still blocks PRs through code scanning alerts. + expected_checks = @required_status_checks.dup + actual_checks = current_protection.dig('required_status_checks', 'contexts') || [] + xcode_checks = discover_xcode_cloud_checks(github_client, repo_url, actual_checks, expected_checks, protection_exists) + expected_checks.concat(xcode_checks) + + validate_required_checks!(expected_checks, actual_checks, protection_exists) + sync = protection_exists && @options.sync_required_status_checks && required_checks_differ?(expected_checks, actual_checks) + branch_protection = build_branch_protection_payload(current_protection, expected_checks, protection_exists, sync) + + puts(' Setting branch protection...') + github_client.put("#{repo_url}/branches/#{@default_branch}/protection", body: branch_protection) + + # Enable required signatures (separate endpoint) + puts(' Enabling required signatures...') + github_client.post( + "#{repo_url}/branches/#{@default_branch}/protection/required_signatures", + expected_codes: [200, 204] + ) + end + + def augment_required_status_checks # Add Vercel check if Next.js project @required_status_checks << 'Vercel' if File.exist?('package.json') && File.read('package.json').include?('"next"') @@ -58,83 +84,77 @@ def configure_branch_protection(github_client, repo_url, current_protection, pro # across regenerations. Job names are read dynamically so renaming a # smoke job needs no generator change. smoke_workflow = '.github/workflows/smoke.yml' - if File.exist?(smoke_workflow) - smoke_jobs = (Psych.safe_load(File.read(smoke_workflow)) || {})['jobs'] || {} - smoke_jobs.each { |job_id, job| @required_status_checks << ((job && job['name']) || job_id.to_s) } - end + return unless File.exist?(smoke_workflow) - # Check for CodeQL default setup + smoke_jobs = (Psych.safe_load(File.read(smoke_workflow)) || {})['jobs'] || {} + smoke_jobs.each { |job_id, job| @required_status_checks << ((job && job['name']) || job_id.to_s) } + end + + def log_codeql_languages(github_client, repo_url) codeql_response = github_client.get("#{repo_url}/code-scanning/default-setup", expected_codes: nil) + return unless codeql_response.code == 200 - if codeql_response.code == 200 - codeql_setup = JSON.parse(codeql_response.body) + codeql_setup = JSON.parse(codeql_response.body) + return unless codeql_setup['state'] == 'configured' && codeql_setup['languages'].is_a?(Array) - if codeql_setup['state'] == 'configured' && codeql_setup['languages'].is_a?(Array) - # Filter out redundant languages from API response - # The API returns 'javascript', 'javascript-typescript', and 'typescript' but - # only 'javascript' check actually runs (it covers both JS and TS) - redundant_languages = %w[javascript-typescript typescript] - languages = codeql_setup['languages'].reject { |lang| redundant_languages.include?(lang) } - puts(" CodeQL languages detected: #{languages.join(', ')} (#{languages.length})") - end - end - - # Build complete list of expected checks - # Note: CodeQL checks are NOT included because they use "smart mode" which only runs - # when relevant files change. CodeQL still blocks PRs through code scanning alerts. - expected_checks = @required_status_checks.dup + # Filter out redundant languages from API response: the API returns 'javascript', + # 'javascript-typescript', and 'typescript' but only 'javascript' check actually + # runs (it covers both JS and TS). + redundant_languages = %w[javascript-typescript typescript] + languages = codeql_setup['languages'].reject { |lang| redundant_languages.include?(lang) } + puts(" CodeQL languages detected: #{languages.join(', ')} (#{languages.length})") + end - # Get actual checks from branch protection - actual_checks = current_protection.dig('required_status_checks', 'contexts') || [] + def discover_xcode_cloud_checks(github_client, repo_url, actual_checks, expected_checks, protection_exists) + return [] unless Dir.exist?('ci_scripts') - # Discover Xcode Cloud checks when ci_scripts directory exists - if Dir.exist?('ci_scripts') - xcode_checks = - if protection_exists - # Extract Xcode Cloud checks from existing protection (checks not from GitHub Actions workflows) - discover_xcode_cloud_checks_from_protection(actual_checks, expected_checks) - else - # For new repos, try to discover from commit statuses on the default branch - discover_xcode_cloud_checks_from_statuses(github_client, repo_url) - end - - expected_checks.concat(xcode_checks) + if protection_exists + # Extract Xcode Cloud checks from existing protection (checks not from GitHub Actions workflows) + discover_xcode_cloud_checks_from_protection(actual_checks, expected_checks) + else + # For new repos, try to discover from commit statuses on the default branch + discover_xcode_cloud_checks_from_statuses(github_client, repo_url) end + end - puts(' Checking required status checks...') + # True when expected and actual required-check lists differ in either direction. + def required_checks_differ?(expected_checks, actual_checks) + (expected_checks - actual_checks).any? || (actual_checks - expected_checks).any? + end - # Only validate mismatch if protection already exists (skip for new repos) - if protection_exists - # Compare expected vs actual - missing_checks = expected_checks - actual_checks - extra_checks = actual_checks - expected_checks + # Prints the check status and raises on an unsynced mismatch. No return value. + def validate_required_checks!(expected_checks, actual_checks, protection_exists) + puts(' Checking required status checks...') - if missing_checks.any? || extra_checks.any? - if missing_checks.any? - puts(' MISSING (expected but not in branch protection):') - missing_checks.each { |check| puts(" ✗ #{check}") } - end + unless protection_exists + puts(' No existing branch protection, will create with expected checks') + return + end - if extra_checks.any? - puts(' EXTRA (in branch protection but not expected):') - extra_checks.each { |check| puts(" + #{check}") } - end + missing_checks = expected_checks - actual_checks + extra_checks = actual_checks - expected_checks + return if missing_checks.none? && extra_checks.none? - raise('Error: branch protection checks mismatch!') unless @options.sync_required_status_checks + if missing_checks.any? + warn(' MISSING (expected but not in branch protection):') + missing_checks.each { |check| warn(" ✗ #{check}") } + end - puts(' --sync_required_status_checks set: overwriting remote check list with expected') - sync_required_status_checks = true - end - else - puts(' No existing branch protection, will create with expected checks') + if extra_checks.any? + warn(' EXTRA (in branch protection but not expected):') + extra_checks.each { |check| warn(" + #{check}") } end - sync_required_status_checks ||= false - # Preserve existing dismissal restrictions or use empty defaults + raise('Error: branch protection checks mismatch!') unless @options.sync_required_status_checks + + puts(' --sync_required_status_checks set: overwriting remote check list with expected') + nil + end + + def build_branch_protection_payload(current_protection, expected_checks, protection_exists, sync_required_status_checks) + # Preserve existing dismissal restrictions / bypass allowances or use empty defaults dismissal_users = current_protection.dig('required_pull_request_reviews', 'dismissal_restrictions', 'users')&.map { |u| u['login'] } || [] dismissal_teams = current_protection.dig('required_pull_request_reviews', 'dismissal_restrictions', 'teams')&.map { |t| t['slug'] } || [] - - # Preserve existing bypass allowances or use empty defaults bypass_users = current_protection.dig('required_pull_request_reviews', 'bypass_pull_request_allowances', 'users')&.map { |u| u['login'] } || [] bypass_teams = current_protection.dig('required_pull_request_reviews', 'bypass_pull_request_allowances', 'teams')&.map { |t| t['slug'] } || [] @@ -154,9 +174,7 @@ def configure_branch_protection(github_client, repo_url, current_protection, pro expected_checks.map { |check| { context: check, app_id: nil } } end - # Set branch protection - puts(' Setting branch protection...') - branch_protection = { + { required_status_checks: { strict: false, checks: status_checks @@ -183,15 +201,6 @@ def configure_branch_protection(github_client, repo_url, current_protection, pro block_creations: false, required_conversation_resolution: true } - - github_client.put("#{repo_url}/branches/#{@default_branch}/protection", body: branch_protection) - - # Enable required signatures (separate endpoint) - puts(' Enabling required signatures...') - github_client.post( - "#{repo_url}/branches/#{@default_branch}/protection/required_signatures", - expected_codes: [200, 204] - ) end def discover_xcode_cloud_checks_from_protection(actual_checks, expected_checks) diff --git a/lib/ghb/slack_job_builder.rb b/lib/ghb/slack_job_builder.rb index 2bdedbf..3260ccc 100644 --- a/lib/ghb/slack_job_builder.rb +++ b/lib/ghb/slack_job_builder.rb @@ -17,14 +17,14 @@ def build old_workflow = @old_workflow @new_workflow.do_job(:slack) do - copy_properties(old_workflow.jobs[id], %i[name permissions needs if runs_on environment concurrency outputs env defaults timeout_minutes strategy continue_on_error container services uses with secrets]) + copy_properties(old_workflow.jobs[id]) do_name('Publish Statuses') do_runs_on(DEFAULT_UBUNTU_VERSION) do_needs(needs) do_if('always()') do_step('Publish Statuses') do - copy_properties(find_step(old_workflow.jobs[:slack]&.steps, name), %i[id if uses run shell with env continue_on_error timeout_minutes]) + copy_properties(find_step(old_workflow.jobs[:slack]&.steps, name)) do_uses("cloud-officer/ci-actions/slack@#{CI_ACTIONS_VERSION}") if with.empty? diff --git a/lib/ghb/workflow/job.rb b/lib/ghb/workflow/job.rb index 4b07943..04edb4a 100644 --- a/lib/ghb/workflow/job.rb +++ b/lib/ghb/workflow/job.rb @@ -4,8 +4,13 @@ require_relative 'step' module GHB - # Data model for GitHub Actions job - instance variables map to YAML schema + # Data model for GitHub Actions job - instance variables map to YAML schema. + # Any new copyable ivar must be added to COPYABLE_PROPERTIES. class Job + # Properties carried over from a previously-generated job by copy_properties. + COPYABLE_PROPERTIES = %i[name permissions needs if runs_on environment concurrency outputs env defaults timeout_minutes strategy continue_on_error container services uses with secrets].freeze + public_constant :COPYABLE_PROPERTIES + def initialize(id) @id = id @name = nil @@ -31,7 +36,7 @@ def initialize(id) attr_accessor :id, :name, :permissions, :needs, :if, :runs_on, :environment, :concurrency, :outputs, :env, :defaults, :steps, :timeout_minutes, :strategy, :continue_on_error, :container, :services, :uses, :with, :secrets - def copy_properties(object, properties) + def copy_properties(object, properties = COPYABLE_PROPERTIES) return if object.nil? properties.each do |property| diff --git a/lib/ghb/workflow/step.rb b/lib/ghb/workflow/step.rb index 59704c3..cd4f8c0 100644 --- a/lib/ghb/workflow/step.rb +++ b/lib/ghb/workflow/step.rb @@ -1,8 +1,13 @@ # frozen_string_literal: true module GHB - # Data model for GitHub Actions step - instance variables map to YAML schema + # Data model for GitHub Actions step - instance variables map to YAML schema. + # Any new copyable ivar must be added to COPYABLE_PROPERTIES. class Step + # Properties carried over from a previously-generated step by copy_properties. + COPYABLE_PROPERTIES = %i[id if uses run shell with env continue_on_error timeout_minutes].freeze + public_constant :COPYABLE_PROPERTIES + def initialize(name, options = {}) @id = options[:id] @if = options[:if] @@ -18,7 +23,7 @@ def initialize(name, options = {}) attr_accessor :id, :if, :name, :uses, :run, :shell, :with, :env, :continue_on_error, :timeout_minutes - def copy_properties(object, properties) + def copy_properties(object, properties = COPYABLE_PROPERTIES) return if object.nil? properties.each do |property| diff --git a/lib/ghb/workflow/workflow.rb b/lib/ghb/workflow/workflow.rb index 373ffea..f3da3b9 100644 --- a/lib/ghb/workflow/workflow.rb +++ b/lib/ghb/workflow/workflow.rb @@ -58,6 +58,21 @@ def do_job(id, &block) @jobs[id] = job end + # Job names this workflow's deploy jobs must wait on (all current jobs). + def deploy_needs + @jobs.keys.map(&:to_s) + end + + # The shared `if:` expression gating deploy jobs (aws / codedeploy): + # run on a deploy trigger, only if no prior job failed. Returns the + # ${{ }}-wrapped expression ready to pass to do_if. + def deploy_if_statement + base_condition = "always() && (needs.variables.outputs.DEPLOY_ON_BETA == '1' || needs.variables.outputs.DEPLOY_ON_RC == '1' || needs.variables.outputs.DEPLOY_ON_PROD == '1')" + job_conditions = @jobs.keys.map { |job_name| "needs.#{job_name}.result != 'failure'" } + + "${{#{([base_condition] + job_conditions).join(' && ')}}}" + end + def read(file) content = File.read(file) @@ -69,9 +84,9 @@ def read(file) @name = workflow_data[:name] @run_name = workflow_data[:'run-name'] - @on = workflow_data[:on] || [] - @permissions = workflow_data[:permissions] || [] - @env = workflow_data[:env] || [] + @on = workflow_data[:on] || {} + @permissions = workflow_data[:permissions] || {} + @env = workflow_data[:env] || {} @defaults = workflow_data[:defaults] || {} @concurrency = workflow_data[:concurrency] || {} @jobs = {}