diff --git a/app/models/shipit/deploy_spec.rb b/app/models/shipit/deploy_spec.rb index b9d2fec70..920c88537 100644 --- a/app/models/shipit/deploy_spec.rb +++ b/app/models/shipit/deploy_spec.rb @@ -76,7 +76,18 @@ def directory def dependencies_steps around_steps('dependencies') do - config('dependencies', 'override') { discover_dependencies_steps || [] } + config('dependencies', 'override') do + if skip_dependencies_for_production_platform? + Rails.logger.warn( + "Skipping dependency installation: stack uses production_platform " \ + "and has no deploy steps requiring local dependencies. " \ + "To override, set `dependencies.override` in your shipit.yml." + ) + [] + else + discover_dependencies_steps || [] + end + end end end alias dependencies_steps! dependencies_steps @@ -264,6 +275,44 @@ def links private + def production_platform? + config('production_platform').present? + end + + def skip_dependencies_for_production_platform? + return false unless production_platform? + return false if Shipit.safe_deploy_command_prefixes.empty? + + # Only check explicitly configured steps. If deploy/rollback rely on auto-discovery + # (no override), we conservatively assume dependencies may be needed. + # Similarly, discovered task definitions (e.g., kubernetes-restart) are inherently + # safe commands and don't need to be checked here. + all_steps = Array(config('deploy', 'override')) + + Array(config('deploy', 'pre')) + + Array(config('deploy', 'post')) + + Array(config('rollback', 'override')) + + Array(config('rollback', 'pre')) + + Array(config('rollback', 'post')) + + all_task_steps + + all_steps = all_steps.compact + return false if all_steps.empty? + + all_steps.all? { |step| safe_deploy_command?(step) } + end + + def all_task_steps + task_configs = config('tasks') || {} + task_configs.values.flat_map { |td| Array(td['steps']) } + end + + def safe_deploy_command?(step) + step = step.to_s.strip + return true if step.empty? + + Shipit.safe_deploy_command_prefixes.any? { |prefix| step == prefix || step.start_with?("#{prefix} ") } + end + def around_steps(section) steps = yield return unless steps diff --git a/lib/shipit.rb b/lib/shipit.rb index 1b6f738f2..332111ff3 100644 --- a/lib/shipit.rb +++ b/lib/shipit.rb @@ -69,6 +69,7 @@ module Shipit attr_writer( :internal_hook_receivers, :preferred_org_emails, + :safe_deploy_command_prefixes, :task_execution_strategy, :task_logger, :use_git_askpass @@ -293,6 +294,10 @@ def committer_email secrets.committer_email.presence || "#{app_name.underscore.dasherize}@#{host}" end + def safe_deploy_command_prefixes + @safe_deploy_command_prefixes ||= [] + end + def internal_hook_receivers @internal_hook_receivers ||= [] end diff --git a/test/models/deploy_spec_test.rb b/test/models/deploy_spec_test.rb index e492331ec..18731c324 100644 --- a/test/models/deploy_spec_test.rb +++ b/test/models/deploy_spec_test.rb @@ -9,6 +9,16 @@ class DeploySpecTest < ActiveSupport::TestCase @stack = shipit_stacks(:shipit) @spec = DeploySpec::FileSystem.new(@app_dir, @stack) @spec.stubs(:load_config).returns({}) + @original_safe_deploy_command_prefixes = Shipit.safe_deploy_command_prefixes + Shipit.safe_deploy_command_prefixes = %w[ + production-platform-next + kubernetes-deploy + kubernetes-restart + ] + end + + teardown do + Shipit.safe_deploy_command_prefixes = @original_safe_deploy_command_prefixes end test '#supports_fetch_deployed_revision? returns false by default' do @@ -52,6 +62,140 @@ class DeploySpecTest < ActiveSupport::TestCase assert_equal ['before', 'bundle install', 'after'], @spec.dependencies_steps end + test '#dependencies_steps returns empty when production_platform is configured and all steps are safe' do + @spec.stubs(:load_config).returns( + 'production_platform' => { 'application' => 'my-app', 'runtime_ids' => ['production-unrestricted'] }, + 'deploy' => { 'override' => ['production-platform-next deploy my-app production-unrestricted'] } + ) + assert_equal [], @spec.dependencies_steps + end + + test '#dependencies_steps still discovers deps when production_platform has unsafe deploy steps' do + @spec.stubs(:load_config).returns( + 'production_platform' => { 'application' => 'my-app', 'runtime_ids' => ['production-unrestricted'] }, + 'deploy' => { 'override' => ['bundle exec rake deploy'] } + ) + @spec.expects(:bundler?).returns(true).at_least_once + @spec.expects(:bundle_install).returns(['bundle install']) + assert_equal ['bundle install'], @spec.dependencies_steps + end + + test '#dependencies_steps still discovers deps when production_platform is absent' do + @spec.stubs(:load_config).returns({}) + @spec.expects(:bundler?).returns(true).at_least_once + @spec.expects(:bundle_install).returns(['bundle install']) + assert_equal ['bundle install'], @spec.dependencies_steps + end + + test '#dependencies_steps respects explicit override even with production_platform' do + @spec.stubs(:load_config).returns( + 'production_platform' => { 'application' => 'my-app', 'runtime_ids' => ['production-unrestricted'] }, + 'dependencies' => { 'override' => ['custom-install'] } + ) + assert_equal ['custom-install'], @spec.dependencies_steps + end + + test '#dependencies_steps preserves pre/post steps when skipping for production_platform' do + @spec.stubs(:load_config).returns( + 'production_platform' => { 'application' => 'my-app', 'runtime_ids' => ['production-unrestricted'] }, + 'deploy' => { 'override' => ['production-platform-next deploy my-app production-unrestricted'] }, + 'dependencies' => { 'pre' => ['echo before'], 'post' => ['echo after'] } + ) + assert_equal ['echo before', 'echo after'], @spec.dependencies_steps + end + + test '#dependencies_steps skips deps when production_platform tasks only use safe commands' do + @spec.stubs(:load_config).returns( + 'production_platform' => { 'application' => 'my-app', 'runtime_ids' => ['production-unrestricted'] }, + 'tasks' => { + 'restart' => { 'steps' => ['production-platform-next run-once my-app production-unrestricted restart'] } + } + ) + assert_equal [], @spec.dependencies_steps + end + + test '#dependencies_steps does not skip when production_platform task has unsafe steps' do + @spec.stubs(:load_config).returns( + 'production_platform' => { 'application' => 'my-app', 'runtime_ids' => ['production-unrestricted'] }, + 'tasks' => { + 'migrate' => { 'steps' => ['bundle exec rake db:migrate'] } + } + ) + @spec.expects(:bundler?).returns(true).at_least_once + @spec.expects(:bundle_install).returns(['bundle install']) + assert_equal ['bundle install'], @spec.dependencies_steps + end + + test '#dependencies_steps skips deps when production_platform uses kubernetes-deploy' do + @spec.stubs(:load_config).returns( + 'production_platform' => { 'application' => 'my-app', 'runtime_ids' => ['production-unrestricted'] }, + 'deploy' => { 'override' => ['kubernetes-deploy --max-watch-seconds 900 my-namespace my-context'] } + ) + assert_equal [], @spec.dependencies_steps + end + + test '#dependencies_steps falls through to discovery when deploy step has unknown command' do + @spec.stubs(:load_config).returns( + 'production_platform' => { 'application' => 'my-app', 'runtime_ids' => ['production-unrestricted'] }, + 'deploy' => { 'override' => ['some-unknown-deploy-tool --flag'] } + ) + @spec.expects(:discover_dependencies_steps).returns(nil).once + assert_equal [], @spec.dependencies_steps + end + + test '#dependencies_steps does not skip when deploy is safe but rollback is unsafe' do + @spec.stubs(:load_config).returns( + 'production_platform' => { 'application' => 'my-app', 'runtime_ids' => ['production-unrestricted'] }, + 'deploy' => { 'override' => ['production-platform-next deploy my-app production-unrestricted'] }, + 'rollback' => { 'override' => ['bundle exec rake rollback'] } + ) + @spec.expects(:bundler?).returns(true).at_least_once + @spec.expects(:bundle_install).returns(['bundle install']) + assert_equal ['bundle install'], @spec.dependencies_steps + end + + test '#dependencies_steps does not skip when deploy.pre has unsafe steps' do + @spec.stubs(:load_config).returns( + 'production_platform' => { 'application' => 'my-app', 'runtime_ids' => ['production-unrestricted'] }, + 'deploy' => { + 'pre' => ['bundle exec rake before_deploy'], + 'override' => ['production-platform-next deploy my-app production-unrestricted'] + } + ) + @spec.expects(:bundler?).returns(true).at_least_once + @spec.expects(:bundle_install).returns(['bundle install']) + assert_equal ['bundle install'], @spec.dependencies_steps + end + + test '#dependencies_steps does not skip when production_platform has no deploy override configured' do + @spec.stubs(:load_config).returns( + 'production_platform' => { 'application' => 'my-app', 'runtime_ids' => ['production-unrestricted'] } + ) + @spec.expects(:bundler?).returns(true).at_least_once + @spec.expects(:bundle_install).returns(['bundle install']) + assert_equal ['bundle install'], @spec.dependencies_steps + end + + test '#dependencies_steps does not skip when safe_deploy_command_prefixes is empty' do + Shipit.safe_deploy_command_prefixes = [] + @spec.stubs(:load_config).returns( + 'production_platform' => { 'application' => 'my-app', 'runtime_ids' => ['production-unrestricted'] }, + 'deploy' => { 'override' => ['production-platform-next deploy my-app production-unrestricted'] } + ) + @spec.expects(:bundler?).returns(true).at_least_once + @spec.expects(:bundle_install).returns(['bundle install']) + assert_equal ['bundle install'], @spec.dependencies_steps + end + + test '#dependencies_steps skips deps when custom safe_deploy_command_prefixes match' do + Shipit.safe_deploy_command_prefixes = %w[my-custom-deployer] + @spec.stubs(:load_config).returns( + 'production_platform' => { 'application' => 'my-app', 'runtime_ids' => ['production-unrestricted'] }, + 'deploy' => { 'override' => ['my-custom-deployer deploy my-app'] } + ) + assert_equal [], @spec.dependencies_steps + end + test '#fetch_deployed_revision_steps! is unknown by default' do assert_raises DeploySpec::Error do @spec.fetch_deployed_revision_steps! diff --git a/test/unit/shipit_test.rb b/test/unit/shipit_test.rb index c5c25936c..0cc8e3a09 100644 --- a/test/unit/shipit_test.rb +++ b/test/unit/shipit_test.rb @@ -30,6 +30,15 @@ class ShipitTest < ActiveSupport::TestCase assert_equal(['shopify/developers'], Shipit.github_teams.map(&:handle)) end + test ".safe_deploy_command_prefixes defaults to empty array" do + assert_equal [], Shipit.safe_deploy_command_prefixes + end + + test ".safe_deploy_command_prefixes can be set" do + Shipit.safe_deploy_command_prefixes = %w[foo bar] + assert_equal %w[foo bar], Shipit.safe_deploy_command_prefixes + end + test ".presence_check_timeout defaults to 30" do assert_equal 30, Shipit.presence_check_timeout end