From de1f7725b0b7c48826a16f6d410395707db7a344 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 25 Aug 2021 11:54:49 +0200 Subject: [PATCH 01/20] Skip git submodules when not necessary --- lib/shipit/stack_commands.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/shipit/stack_commands.rb b/lib/shipit/stack_commands.rb index f5d479eca..7182fe0b9 100644 --- a/lib/shipit/stack_commands.rb +++ b/lib/shipit/stack_commands.rb @@ -48,12 +48,12 @@ def fetch_deployed_revision end def build_cacheable_deploy_spec - with_temporary_working_directory do |dir| + with_temporary_working_directory(recursive: false) do |dir| DeploySpec::FileSystem.new(dir, @stack.environment).cacheable end end - def with_temporary_working_directory(commit: nil) + def with_temporary_working_directory(commit: nil, recursive: true) commit ||= @stack.last_deployed_commit.presence || @stack.commits.reachable.last if !commit || !fetched?(commit).tap(&:run).success? @@ -64,10 +64,12 @@ def with_temporary_working_directory(commit: nil) end end + git_args = [] + git_args << '--recursive' if recursive Dir.mktmpdir do |dir| git( 'clone', @stack.git_path, @stack.repo_name, - '--recursive', '--origin', 'cache', + *git_args, '--origin', 'cache', chdir: dir ).run! From d7200a0e7cb781691692e91e1e980becfc4db7e9 Mon Sep 17 00:00:00 2001 From: Ruidan Date: Mon, 5 Feb 2024 16:18:50 -0500 Subject: [PATCH 02/20] Bump Octokit to 5.6.1 --- Gemfile.lock | 4 ++-- shipit-engine.gemspec | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 19e2cf230..7771312d4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -12,7 +12,7 @@ PATH gemoji (~> 2.1) jquery-rails (~> 4.4) lodash-rails (~> 4.17) - octokit (~> 4.20) + octokit (~> 5.6.0) omniauth-github (~> 1.4) paquito pubsubstub (~> 0.2.0) @@ -240,7 +240,7 @@ GEM rack (>= 1.2, < 4) snaky_hash (~> 2.0) version_gem (~> 1.1) - octokit (4.25.1) + octokit (5.6.1) faraday (>= 1, < 3) sawyer (~> 0.9) omniauth (1.9.2) diff --git a/shipit-engine.gemspec b/shipit-engine.gemspec index 99ae08cea..ea7a9cfcb 100644 --- a/shipit-engine.gemspec +++ b/shipit-engine.gemspec @@ -27,7 +27,7 @@ Gem::Specification.new do |s| s.add_dependency('gemoji', '~> 2.1') s.add_dependency('jquery-rails', '~> 4.4') s.add_dependency('lodash-rails', '~> 4.17') - s.add_dependency('octokit', '~> 4.20') + s.add_dependency('octokit', '~> 5.6.0') s.add_dependency('omniauth-github', '~> 1.4') s.add_dependency('pubsubstub', '~> 0.2.0') s.add_dependency('rails', '~> 7.1.1') From 8d386d0408e5c7b50c64ea52c0dbed33d4f72e4a Mon Sep 17 00:00:00 2001 From: Ruidan Date: Mon, 5 Feb 2024 16:26:02 -0500 Subject: [PATCH 03/20] Remove accept header --- app/models/shipit/commit_deployment_status.rb | 1 - lib/shipit/github_app.rb | 1 - lib/shipit/octokit_check_runs.rb | 4 +--- test/models/commit_deployment_status_test.rb | 1 - 4 files changed, 1 insertion(+), 6 deletions(-) diff --git a/app/models/shipit/commit_deployment_status.rb b/app/models/shipit/commit_deployment_status.rb index 85bde2b35..6be2693f5 100644 --- a/app/models/shipit/commit_deployment_status.rb +++ b/app/models/shipit/commit_deployment_status.rb @@ -48,7 +48,6 @@ def create_status_on_github(client) client.create_deployment_status( commit_deployment.api_url, status, - accept: 'application/vnd.github.flash-preview+json', target_url: url_helpers.stack_deploy_url(stack, task), description: description.truncate(DESCRIPTION_CHARACTER_LIMIT_ON_GITHUB), environment_url: stack.deploy_url, diff --git a/lib/shipit/github_app.rb b/lib/shipit/github_app.rb index a8ee5bfba..ae27297e0 100644 --- a/lib/shipit/github_app.rb +++ b/lib/shipit/github_app.rb @@ -103,7 +103,6 @@ def fetch_new_token ) do response = new_client(bearer_token: authentication_payload).create_app_installation_access_token( installation_id, - accept: 'application/vnd.github.machine-man-preview+json', ) token = Token.from_github(response) raise AuthenticationFailed if token.blank? diff --git a/lib/shipit/octokit_check_runs.rb b/lib/shipit/octokit_check_runs.rb index 09d75a016..4b13172b1 100644 --- a/lib/shipit/octokit_check_runs.rb +++ b/lib/shipit/octokit_check_runs.rb @@ -1,9 +1,7 @@ # frozen_string_literal: true module OctokitCheckRuns def check_runs(repo, sha, options = {}) - paginate("#{Octokit::Repository.path(repo)}/commits/#{sha}/check-runs", options.reverse_merge( - accept: 'application/vnd.github.antiope-preview+json', - )) + paginate("#{Octokit::Repository.path(repo)}/commits/#{sha}/check-runs", options) end end diff --git a/test/models/commit_deployment_status_test.rb b/test/models/commit_deployment_status_test.rb index 8bcbc5ab7..9d75fce59 100644 --- a/test/models/commit_deployment_status_test.rb +++ b/test/models/commit_deployment_status_test.rb @@ -15,7 +15,6 @@ class CommitDeploymentStatusTest < ActiveSupport::TestCase @author.github_api.class.any_instance.expects(:create_deployment_status).with( @deployment.api_url, 'in_progress', - accept: "application/vnd.github.flash-preview+json", target_url: "http://shipit.com/shopify/shipit-engine/production/deploys/#{@task.id}", description: "walrus triggered the deploy of shopify/shipit-engine/production to #{@deployment.short_sha}", environment_url: "https://shipit.shopify.com", From 9034b7b8b64591aeceebd4f9a4036dcad5b3af28 Mon Sep 17 00:00:00 2001 From: Kartiki Sharma Date: Mon, 22 Jan 2024 17:41:00 -0500 Subject: [PATCH 04/20] Create secrets.development.json Create secrets.test.json Replace deprecated Rails.application.secrets Add ejson-rails gem Remove deprecation warning Passing the coder as positional argument is deprecated and will be removed in Rails 7.2 Remove deprecated TestFixtures.fixture_path= Remove deprecated check_pending! --- CHANGELOG.md | 6 ++++++ Gemfile | 1 + Gemfile.lock | 5 +++++ app/models/shipit/api_client.rb | 2 +- app/models/shipit/delivery.rb | 2 +- app/models/shipit/hook.rb | 2 +- app/models/shipit/pull_request.rb | 2 +- app/models/shipit/stack.rb | 2 +- app/models/shipit/task.rb | 4 ++-- app/views/shipit/missing_settings.html.erb | 2 +- lib/shipit.rb | 2 +- lib/shipit/engine.rb | 2 +- test/controllers/stacks_controller_test.rb | 2 +- .../0_load_development_secrets.rb | 4 ++-- test/dummy/config/secrets.development.json | 3 +++ test/dummy/config/secrets.test.json | 21 +++++++++++++++++++ test/test_helper.rb | 4 ++-- test/unit/github_app_test.rb | 2 +- 18 files changed, 52 insertions(+), 16 deletions(-) create mode 100644 test/dummy/config/secrets.development.json create mode 100644 test/dummy/config/secrets.test.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 53f3ea8e6..ba8a009f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,10 @@ # Unreleased +* Upgraded Octokit to 5.6.1 (#1327) +* Migrate from legacy Rails secrets to credentials (#1326) + * Rails secrets were [deprecated in Rails 7.1](https://github.com/rails/rails/pull/48472) + * [Guide on credentials](https://guides.rubyonrails.org/security.html#custom-credentials) + +# 0.39.0 * Upgraded to Rails 7.1.1 diff --git a/Gemfile b/Gemfile index 21ebd4a8c..b12bdc854 100644 --- a/Gemfile +++ b/Gemfile @@ -3,6 +3,7 @@ source 'https://rubygems.org' gemspec gem 'sqlite3' +gem 'ejson-rails', require: 'ejson/rails/skip_secrets' group :ci do gem 'mysql2' diff --git a/Gemfile.lock b/Gemfile.lock index 19e2cf230..366772d1c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -144,6 +144,10 @@ GEM docile (1.4.0) drb (2.2.0) ruby2_keywords + ejson (1.4.1) + ejson-rails (0.2.1) + ejson + railties (>= 5.2) equalizer (0.0.11) erubi (1.12.0) execjs (2.8.1) @@ -416,6 +420,7 @@ PLATFORMS DEPENDENCIES byebug + ejson-rails faker mocha mysql2 diff --git a/app/models/shipit/api_client.rb b/app/models/shipit/api_client.rb index 27459c3ec..18f5b73c7 100644 --- a/app/models/shipit/api_client.rb +++ b/app/models/shipit/api_client.rb @@ -8,7 +8,7 @@ class ApiClient < Record validates :creator, :name, presence: true - serialize :permissions, Shipit.serialized_column(:permissions, type: Array) + serialize :permissions, coder: Shipit.serialized_column(:permissions, type: Array) PERMISSIONS = %w( read:stack write:stack diff --git a/app/models/shipit/delivery.rb b/app/models/shipit/delivery.rb index 141c87f8b..9f1fef9ae 100644 --- a/app/models/shipit/delivery.rb +++ b/app/models/shipit/delivery.rb @@ -9,7 +9,7 @@ class Delivery < Record validates :url, presence: true, url: { no_local: true, allow_blank: true } validates :content_type, presence: true - serialize :response_headers, SafeJSON + serialize :response_headers, coder: SafeJSON after_commit :purge_old_deliveries, on: :create diff --git a/app/models/shipit/hook.rb b/app/models/shipit/hook.rb index 9aa6a4e1f..5b20ddeda 100644 --- a/app/models/shipit/hook.rb +++ b/app/models/shipit/hook.rb @@ -87,7 +87,7 @@ def signature validates :content_type, presence: true, inclusion: { in: CONTENT_TYPES.keys } validates :events, presence: true, subset: { of: EVENTS } - serialize :events, Shipit::CSVSerializer + serialize :events, coder: Shipit::CSVSerializer scope :global, -> { where(stack_id: nil) } scope :scoped_to, ->(stack) { where(stack_id: stack.id) } diff --git a/app/models/shipit/pull_request.rb b/app/models/shipit/pull_request.rb index 0083ad9c0..8e993af82 100644 --- a/app/models/shipit/pull_request.rb +++ b/app/models/shipit/pull_request.rb @@ -11,7 +11,7 @@ class PullRequest < Record has_many :pull_request_assignments has_many :assignees, class_name: :User, through: :pull_request_assignments, source: :user - serialize :labels, Shipit.serialized_column(:labels, type: Array) + serialize :labels, coder: Shipit.serialized_column(:labels, type: Array) after_create_commit :emit_create_hooks after_update_commit :emit_update_hooks diff --git a/app/models/shipit/stack.rb b/app/models/shipit/stack.rb index 403f7ff82..4f2642f89 100644 --- a/app/models/shipit/stack.rb +++ b/app/models/shipit/stack.rb @@ -101,7 +101,7 @@ def sync_github_if_necessary validates :lock_reason, length: { maximum: 4096 } - serialize :cached_deploy_spec, DeploySpec + serialize :cached_deploy_spec, coder: DeploySpec delegate( :provisioning_handler_name, :find_task_definition, diff --git a/app/models/shipit/task.rb b/app/models/shipit/task.rb index 9db5e1155..c0baa4688 100644 --- a/app/models/shipit/task.rb +++ b/app/models/shipit/task.rb @@ -58,8 +58,8 @@ def new end end - serialize :definition, TaskDefinition - serialize :env, Shipit.serialized_column(:env, coder: EnvHash) + serialize :definition, coder: TaskDefinition + serialize :env, coder: Shipit.serialized_column(:env, coder: EnvHash) scope :success, -> { where(status: 'success') } scope :completed, -> { where(status: COMPLETED_STATUSES) } diff --git a/app/views/shipit/missing_settings.html.erb b/app/views/shipit/missing_settings.html.erb index 8c874f0c5..59acfb32f 100644 --- a/app/views/shipit/missing_settings.html.erb +++ b/app/views/shipit/missing_settings.html.erb @@ -22,7 +22,7 @@

Config: - <% if Rails.application.secrets.github.present? %> + <% if Rails.application.credentials.github.present? %> Success! <% else %> diff --git a/lib/shipit.rb b/lib/shipit.rb index 885f6b53f..8360dbb0a 100644 --- a/lib/shipit.rb +++ b/lib/shipit.rb @@ -291,7 +291,7 @@ def revision_file end def secrets - Rails.application.secrets + Rails.application.credentials end end diff --git a/lib/shipit/engine.rb b/lib/shipit/engine.rb index 0d90d124c..b1279137f 100644 --- a/lib/shipit/engine.rb +++ b/lib/shipit/engine.rb @@ -21,7 +21,7 @@ class Engine < ::Rails::Engine Shipit::Engine.routes.default_url_options[:host] = Shipit.host Pubsubstub.redis_url = Shipit.redis_url.to_s - Rails.application.secrets.deep_symbolize_keys! + Rails.application.credentials.deep_symbolize_keys! app.config.assets.paths << Emoji.images_path app.config.assets.precompile += %w( diff --git a/test/controllers/stacks_controller_test.rb b/test/controllers/stacks_controller_test.rb index 2a7c2fd15..7b8912937 100644 --- a/test/controllers/stacks_controller_test.rb +++ b/test/controllers/stacks_controller_test.rb @@ -10,7 +10,7 @@ class StacksControllerTest < ActionController::TestCase end test "validates that Shipit.github is present" do - Rails.application.secrets.stubs(:github).returns(nil) + Rails.application.credentials.stubs(:github).returns(nil) get :index assert_select "#github_app .missing" assert_select ".missing", count: 1 diff --git a/test/dummy/config/initializers/0_load_development_secrets.rb b/test/dummy/config/initializers/0_load_development_secrets.rb index 863be78d5..f3b83652b 100644 --- a/test/dummy/config/initializers/0_load_development_secrets.rb +++ b/test/dummy/config/initializers/0_load_development_secrets.rb @@ -2,8 +2,8 @@ if local_secrets.exist? secrets = YAML.load(local_secrets.read).deep_symbolize_keys if Rails.env.development? - Rails.application.secrets.deep_merge!(secrets) + Rails.application.credentials.deep_merge!(secrets) elsif Rails.env.test? - Rails.application.secrets.merge!(redis_url: secrets[:redis_url]) + Rails.application.credentials.merge!(redis_url: secrets[:redis_url]) end end diff --git a/test/dummy/config/secrets.development.json b/test/dummy/config/secrets.development.json new file mode 100644 index 000000000..2c22e2b66 --- /dev/null +++ b/test/dummy/config/secrets.development.json @@ -0,0 +1,3 @@ +{ + "secret_key_base": "s3cr3ts3cr3ts3cr3ts3cr3ts3cr3ts3cr3t" +} \ No newline at end of file diff --git a/test/dummy/config/secrets.test.json b/test/dummy/config/secrets.test.json new file mode 100644 index 000000000..a08afdab4 --- /dev/null +++ b/test/dummy/config/secrets.test.json @@ -0,0 +1,21 @@ +{ + "host": "shipit.com", + "secret_key_base": "s3cr3ts3cr3ts3cr3ts3cr3ts3cr3ts3cr3t", + "github_api": { + "token": "t0k3n" + }, + "github": { + "domain": null, + "app_id": 42, + "installation_id": 43, + "bot_login": "shipit[bot]", + "webhook_secret": null, + "private_key": "-----BEGIN RSA PRIVATE KEY-----\nMIIEpAIBAAKCAQEA7iUQC2uUq/gtQg0gxtyaccuicYgmq1LUr1mOWbmwM1Cv63+S\n73qo8h87FX+YyclY5fZF6SMXIys02JOkImGgbnvEOLcHnImCYrWs03msOzEIO/pG\nM0YedAPtQ2MEiLIu4y8htosVxeqfEOPiq9kQgFxNKyETzjdIA9q1md8sofuJUmPv\nibacW1PecuAMnn+P8qf0XIDp7uh6noB751KvhCaCNTAPtVE9NZ18OmNG9GOyX/pu\npQHIrPgTpTG6KlAe3r6LWvemzwsMtuRGU+K+KhK9dFIlSE+v9rA32KScO8efOh6s\nGu3rWorV4iDu14U62rzEfdzzc63YL94sUbZxbwIDAQABAoIBADLJ8r8MxZtbhYN1\nu0zOFZ45WL6v09dsBfITvnlCUeLPzYUDIzoxxcBFittN6C744x3ARS6wjimw+EdM\nTZALlCSb/sA9wMDQzt7wchhz9Zh2H5RzDu+2f54sjDh38KqancdT8PO2fAFGxX/b\nqicOVyeZB9gv6MJtJc20olBbuXAeBNfcDABF9oxF+0i+Ssg7B4VXiqgcjtGbr/Og\nqRll7AqyTArVx2xEcVfZxeZ4zGnigzcJq4te7yYpxzwk+RxblkPh54Yt4WxZ+8DI\nRsn3r6ajlpwzpwvsJFU2Txq7xBTzGQMFmy/Pnjk83kP2cogxB2+tRyjITGqTwD8b\ngg9PFCkCgYEA+7u8A0l0Cz6p0SI6c7ftVePVRiIhpawWN7og/wEmI6zUjm/3rA+R\nhrhaVKuOD8QF/HdDsqTck5gjGAjTmJz6r33/cl1Tz+pr62znsrB4r0yMKvQbKN81\nWGaWOsi2+ZXqLNv5h5wpUF0MTKlXHeKnwP5kuEvGwVn6WURFCh6PhLMCgYEA8i5e\nJjulJVGyd5HuoY3xyO7E6DjidsqRnVRq+hYpORjnHvTmSwe4+tH4ha2p9Kv2Y6k3\nC1NYY/fSMQoYCCRaYyJleI+la/9tsZqAmtms4ZB8KhFmPHf9fW75i6G0xKWyZ8K+\nE2Ft/UaEiM282593cguV6+Kt5uExnyPxLLK4FlUCgYEAwRJ/JGI8/7bjFkTTYheq\nj5q75BufhOrU6471acAe2XPgXxLfefdC3Xodxh0CS3NESBvNL4Ikr4sbN37lk4Kq\n/th7iOKtuqUIeru/hZy2I3VpeDRbdGCmEJQ2GwYA2LKztg5Nd0Y9paaIHXAwIfrK\nQUqcQ4HTAk8ZpUeoUBeaaeMCgYANLmbjb9WiPVsYVPIHCwHA7PX8qbPxwT7BsGmO\nKQyfVfKmZa/vH4F67Vi4deZNMdrcO8aKMEQcVM2065a5QrlEsgeR00eupB1lUEJ1\nqylUsZeAdqf43JMIc7TTW77KATa/nQLZbTEeWus1wvTngztuEqFbUGAks9cOkVc8\nFpIcbQKBgQDVIL8gPLmn0f+4oLF8MBC+oxtKpz14X5iJ1saGFkzW5I+nIEskpS0S\nqtirnTCnJFGdCrFwctnxiuiCmyGwpBYdjIfHyvYAHnqAtMnESzCUyeSFZiquVW5W\nMvbMmDPoV27XOHU9kIq6NXtfrkpufiyo6/VEYWozXalxKLNuqLYfPQ==\n-----END RSA PRIVATE KEY-----\n", + "oauth": { + "id": "Iv1.bf2c2c45b449bfd9", + "secret": "ef694cd6e45223075d78d138ef014049052665f1", + "teams": null + } + }, + "redis_url": "redis://127.0.0.1:6379/7" +} \ No newline at end of file diff --git a/test/test_helper.rb b/test/test_helper.rb index a5ba4068e..6c75d7f97 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -23,7 +23,7 @@ # Load fixtures from the engine if ActiveSupport::TestCase.respond_to?(:fixture_path=) - ActiveSupport::TestCase.fixture_path = File.expand_path("../fixtures", __FILE__) + ActiveSupport::TestCase.fixture_paths << File.expand_path("../fixtures", __FILE__) ActiveSupport::TestCase.fixtures(:all) end @@ -71,7 +71,7 @@ class TestCase end end - ActiveRecord::Migration.check_pending! + ActiveRecord::Migration.check_all_pending! # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order. # diff --git a/test/unit/github_app_test.rb b/test/unit/github_app_test.rb index 5beda14d5..4e2aab2eb 100644 --- a/test/unit/github_app_test.rb +++ b/test/unit/github_app_test.rb @@ -202,7 +202,7 @@ def app(extra_config = {}) end def default_config - Rails.application.secrets.github.deep_dup + Rails.application.credentials.github.deep_dup end end end From cda478927faee56d4dff70854e3cb6494e870d6f Mon Sep 17 00:00:00 2001 From: Erik Ryhorchuk Date: Wed, 6 Mar 2024 14:39:10 -0800 Subject: [PATCH 05/20] rescue forbidden user github error --- app/models/shipit/user.rb | 2 ++ test/models/users_test.rb | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/app/models/shipit/user.rb b/app/models/shipit/user.rb index 0e283e143..c94c29dfa 100644 --- a/app/models/shipit/user.rb +++ b/app/models/shipit/user.rb @@ -95,6 +95,8 @@ def refresh_from_github! update!(github_user: Shipit.github.api.user(github_id)) rescue Octokit::NotFound identify_renamed_user! + rescue Octokit::Forbidden + Rails.logger.info("User #{name}, github_id #{github_id} has forbidden access to their GitHub, likely deleted.") end def github_user=(github_user) diff --git a/test/models/users_test.rb b/test/models/users_test.rb index 1cbe57296..b62083c05 100644 --- a/test/models/users_test.rb +++ b/test/models/users_test.rb @@ -203,6 +203,14 @@ class UsersTest < ActiveSupport::TestCase assert_equal 'george@cyclim.se', user.email end + test "#refresh_from_github! logs deleted users" do + Shipit.github.api.expects(:user).with(@user.github_id).raises(Octokit::Forbidden) + + Rails.logger.expects(:info).with("User #{@user.name}, github_id #{@user.github_id} has forbidden access to their GitHub, likely deleted.") + + @user.refresh_from_github! + end + test "#github_api uses the user's access token" do assert_equal @user.github_access_token, @user.github_api.access_token end From c2c77ea26069b6d3390b5fd60f8464efadc9c485 Mon Sep 17 00:00:00 2001 From: Kartiki Sharma Date: Tue, 19 Mar 2024 11:19:39 -0400 Subject: [PATCH 06/20] Stack: update `build_deploy` to pass `allow_concurrency` Add unit tests --- CHANGELOG.md | 2 ++ README.md | 5 ++++ .../shipit/api/deploys_controller.rb | 6 ++++- app/models/shipit/stack.rb | 4 ++-- .../api/deploys_controller_test.rb | 23 +++++++++++++++++++ 5 files changed, 37 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba8a009f4..0a5c3c017 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ * Migrate from legacy Rails secrets to credentials (#1326) * Rails secrets were [deprecated in Rails 7.1](https://github.com/rails/rails/pull/48472) * [Guide on credentials](https://guides.rubyonrails.org/security.html#custom-credentials) +* For deployments, `allow_concurrency` defaults to the same value as `force`. If wanted, it can be set separately by passing the intended value for `allow_concurrency` to `build_deploy` method + # 0.39.0 diff --git a/README.md b/README.md index e8a6f17f1..a48fa65e7 100644 --- a/README.md +++ b/README.md @@ -357,6 +357,11 @@ For example: fetch: curl --silent https://app.example.com/services/ping/version ``` + +**Note:** Currently, deployments in emergency mode are configured to occur concurrently via [the `build_deploy` method](https://github.com/Shopify/shipit-engine/blob/main/app/models/shipit/stack.rb), +whose `allow_concurrency` keyword argument defaults to `force`, where `force` is true when emergency mode is enabled. +If you'd like to separate these two from one another, override this method as desired in your service. +

Kubernetes

**kubernetes** allows to specify a Kubernetes namespace and context to deploy to. diff --git a/app/controllers/shipit/api/deploys_controller.rb b/app/controllers/shipit/api/deploys_controller.rb index d80dedbfb..3ccb3f077 100644 --- a/app/controllers/shipit/api/deploys_controller.rb +++ b/app/controllers/shipit/api/deploys_controller.rb @@ -11,6 +11,7 @@ def index params do requires :sha, String, length: { in: 6..40 } accepts :force, Boolean, default: false + accepts :allow_concurrency, Boolean accepts :require_ci, Boolean, default: false accepts :env, Hash, default: {} end @@ -18,7 +19,10 @@ def create commit = stack.commits.by_sha(params.sha) || param_error!(:sha, 'Unknown revision') param_error!(:force, "Can't deploy a locked stack") if !params.force && stack.locked? param_error!(:require_ci, "Commit is not deployable") if params.require_ci && !commit.deployable? - deploy = stack.trigger_deploy(commit, current_user, env: params.env, force: params.force) + + allow_concurrency = params.allow_concurrency.nil? ? params.force : params.allow_concurrency + deploy = stack.trigger_deploy(commit, current_user, env: params.env, force: params.force, + allow_concurrency: allow_concurrency) render_resource(deploy, status: :accepted) end end diff --git a/app/models/shipit/stack.rb b/app/models/shipit/stack.rb index 4f2642f89..23767f254 100644 --- a/app/models/shipit/stack.rb +++ b/app/models/shipit/stack.rb @@ -150,14 +150,14 @@ def trigger_task(definition_id, user, env: nil, force: false) task end - def build_deploy(until_commit, user, env: nil, force: false) + def build_deploy(until_commit, user, env: nil, force: false, allow_concurrency: force) since_commit = last_deployed_commit.presence || commits.first deploys.build( user_id: user.id, until_commit: until_commit, since_commit: since_commit, env: filter_deploy_envs(env&.to_h || {}), - allow_concurrency: force, + allow_concurrency: allow_concurrency, ignored_safeties: force || !until_commit.deployable?, max_retries: retries_on_deploy, ) diff --git a/test/controllers/api/deploys_controller_test.rb b/test/controllers/api/deploys_controller_test.rb index 7a044b8ff..c7ebbf02a 100644 --- a/test/controllers/api/deploys_controller_test.rb +++ b/test/controllers/api/deploys_controller_test.rb @@ -120,6 +120,29 @@ class DeploysControllerTest < ApiControllerTestCase assert_response :accepted assert_json 'status', 'pending' end + + test "#create uses allow_concurrency param when provided" do + @stack.update!(lock_reason: 'Something broken') + + assert_difference -> { @stack.deploys.count }, 1 do + post :create, params: { stack_id: @stack.to_param, sha: @commit.sha, force: 'true', allow_concurrency: 'false' } + end + assert_response :accepted + assert_json 'status', 'pending' + refute @stack.deploys.last.allow_concurrency + end + + test "#create defaults allow_concurrency to force param when not provided" do + @stack.update!(lock_reason: 'Something broken') + expected_force = true + + assert_difference -> { @stack.deploys.count }, 1 do + post :create, params: { stack_id: @stack.to_param, sha: @commit.sha, force: expected_force } + end + assert_response :accepted + assert_json 'status', 'pending' + assert_equal expected_force, @stack.deploys.last.allow_concurrency + end end end end From 57a05ef62fd93fa761989bb0a301d3e88f99704c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 25 Mar 2024 19:47:51 +0000 Subject: [PATCH 07/20] Bump rdoc from 6.6.2 to 6.6.3.1 Bumps [rdoc](https://github.com/ruby/rdoc) from 6.6.2 to 6.6.3.1. - [Release notes](https://github.com/ruby/rdoc/releases) - [Changelog](https://github.com/ruby/rdoc/blob/master/History.rdoc) - [Commits](https://github.com/ruby/rdoc/compare/v6.6.2...v6.6.3.1) --- updated-dependencies: - dependency-name: rdoc dependency-type: indirect ... Signed-off-by: dependabot[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 8cace7f4c..ebfaad276 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -318,7 +318,7 @@ GEM zeitwerk (~> 2.6) rainbow (3.1.1) rake (13.1.0) - rdoc (6.6.2) + rdoc (6.6.3.1) psych (>= 4.0.0) redis (4.8.1) redis-objects (1.7.0) From 37cface5d59050015364231258a56e48f87e999b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 26 Mar 2024 20:34:18 +0000 Subject: [PATCH 08/20] Bump rack from 2.2.8 to 2.2.9 Bumps [rack](https://github.com/rack/rack) from 2.2.8 to 2.2.9. - [Release notes](https://github.com/rack/rack/releases) - [Changelog](https://github.com/rack/rack/blob/main/CHANGELOG.md) - [Commits](https://github.com/rack/rack/compare/v2.2.8...v2.2.9) --- updated-dependencies: - dependency-name: rack dependency-type: indirect ... Signed-off-by: dependabot[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index ebfaad276..fe5e2b9c3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -272,7 +272,7 @@ GEM rack redis (~> 4.0) racc (1.7.3) - rack (2.2.8) + rack (2.2.9) rack-session (1.0.2) rack (< 3) rack-test (2.1.0) From 047dbabbfec702ede1862642944661e4d0eeb9ae Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 3 Apr 2024 11:00:18 +0200 Subject: [PATCH 09/20] Release 0.39.0 --- .github/workflows/main.yml | 10 +++++----- .rubocop.yml | 2 +- CHANGELOG.md | 10 +++++----- Gemfile.lock | 17 +++++++---------- dev.yml | 2 +- lib/shipit/version.rb | 2 +- template.rb | 4 ++-- 7 files changed, 22 insertions(+), 25 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6efee0c9f..1a1f81ecc 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -9,10 +9,10 @@ jobs: steps: - uses: actions/checkout@v1 - - name: Set up Ruby 2.7 + - name: Set up Ruby 3.0 uses: ruby/setup-ruby@v1 with: - ruby-version: '2.7' + ruby-version: '3.0' bundler-cache: true - name: rubocop run: | @@ -25,7 +25,7 @@ jobs: fail-fast: false matrix: ruby_version: - - '2.7' + - '3.0' services: db: @@ -68,7 +68,7 @@ jobs: fail-fast: false matrix: ruby_version: - - '2.7' + - '3.0' services: db: @@ -107,10 +107,10 @@ jobs: fail-fast: false matrix: ruby_version: - - '2.7' - '3.0' - '3.1' - '3.2' + - '3.3' services: redis: diff --git a/.rubocop.yml b/.rubocop.yml index 979b3f24a..da85c1c48 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,5 +1,5 @@ AllCops: - TargetRubyVersion: 2.7 + TargetRubyVersion: 3.0 Exclude: - tmp/* - bin/* diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a5c3c017..679df9436 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,15 +1,15 @@ # Unreleased + +# 0.39.0 + +* Minimum Ruby version is now Ruby 3.0 +* Upgraded to Rails 7.1.1 * Upgraded Octokit to 5.6.1 (#1327) * Migrate from legacy Rails secrets to credentials (#1326) * Rails secrets were [deprecated in Rails 7.1](https://github.com/rails/rails/pull/48472) * [Guide on credentials](https://guides.rubyonrails.org/security.html#custom-credentials) * For deployments, `allow_concurrency` defaults to the same value as `force`. If wanted, it can be set separately by passing the intended value for `allow_concurrency` to `build_deploy` method - -# 0.39.0 - -* Upgraded to Rails 7.1.1 - # 0.38.0 * Convert `commit_deployment_statuses.github_id` to bigint (#1312) diff --git a/Gemfile.lock b/Gemfile.lock index fe5e2b9c3..89c198c3d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - shipit-engine (0.38.0) + shipit-engine (0.39.0) active_model_serializers (~> 0.9.3) ansi_stream (~> 0.0.6) autoprefixer-rails (~> 6.4.1) @@ -231,11 +231,9 @@ GEM net-smtp (0.4.0.1) net-protocol nio4r (2.7.0) - nokogiri (1.15.5-arm64-darwin) + nokogiri (1.16.3-arm64-darwin) racc (~> 1.4) - nokogiri (1.15.5-x86_64-darwin) - racc (~> 1.4) - nokogiri (1.15.5-x86_64-linux) + nokogiri (1.16.3-x86_64-linux) racc (~> 1.4) oauth2 (2.0.9) faraday (>= 0.17.3, < 3.0) @@ -265,7 +263,7 @@ GEM pry (0.14.1) coderay (~> 1.1) method_source (~> 1.0) - psych (5.1.2) + psych (4.0.6) stringio public_suffix (4.0.6) pubsubstub (0.2.2) @@ -377,7 +375,8 @@ GEM activesupport (>= 5.2) sprockets (>= 3.0.0) spy (1.0.2) - sqlite3 (1.4.2) + sqlite3 (1.7.3-arm64-darwin) + sqlite3 (1.7.3-x86_64-linux) state_machines (0.5.0) state_machines-activemodel (0.8.0) activemodel (>= 5.1) @@ -413,9 +412,7 @@ GEM zeitwerk (2.6.12) PLATFORMS - arm64-darwin-21 - arm64-darwin-22 - x86_64-darwin-20 + arm64-darwin x86_64-linux DEPENDENCIES diff --git a/dev.yml b/dev.yml index 6d9736d38..c56e7facb 100644 --- a/dev.yml +++ b/dev.yml @@ -8,7 +8,7 @@ up: - packages: - sqlite - ruby: - version: 2.7.5 + version: 3.0.3 - isogun - bundler: without: ci diff --git a/lib/shipit/version.rb b/lib/shipit/version.rb index 5e104fc24..528579f6b 100644 --- a/lib/shipit/version.rb +++ b/lib/shipit/version.rb @@ -1,4 +1,4 @@ # frozen_string_literal: true module Shipit - VERSION = '0.38.0' + VERSION = '0.39.0' end diff --git a/template.rb b/template.rb index 7f7b2cea4..2f80764ff 100644 --- a/template.rb +++ b/template.rb @@ -1,7 +1,7 @@ # Template for rails new app # Run this like `rails new shipit -m template.rb` -if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.7') - raise Thor::Error, "You need at least Ruby 2.7 to install shipit" +if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('3.0') + raise Thor::Error, "You need at least Ruby 3.0 to install shipit" end if Gem::Version.new(Rails::VERSION::STRING) < Gem::Version.new('7.1') raise Thor::Error, "You need Rails 7.1 to install shipit" From c200e3069a3ad980ba2ab79b67e9002fec4605f7 Mon Sep 17 00:00:00 2001 From: Jenny Shen Date: Tue, 9 Apr 2024 11:14:06 -0400 Subject: [PATCH 10/20] Set .ruby-version to 3.0.3 dev.yml reads from .ruby-version --- .ruby-version | 1 + dev.yml | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) create mode 100644 .ruby-version diff --git a/.ruby-version b/.ruby-version new file mode 100644 index 000000000..75a22a26a --- /dev/null +++ b/.ruby-version @@ -0,0 +1 @@ +3.0.3 diff --git a/dev.yml b/dev.yml index c56e7facb..2dfd802fa 100644 --- a/dev.yml +++ b/dev.yml @@ -7,8 +7,7 @@ type: rails up: - packages: - sqlite - - ruby: - version: 3.0.3 + - ruby - isogun - bundler: without: ci From 69e3eb9090da9d16135ee1d7d84a2202e59127bd Mon Sep 17 00:00:00 2001 From: Jenny Shen Date: Tue, 9 Apr 2024 11:15:39 -0400 Subject: [PATCH 11/20] Set required_ruby_version to >= 3.0.0 --- shipit-engine.gemspec | 2 ++ 1 file changed, 2 insertions(+) diff --git a/shipit-engine.gemspec b/shipit-engine.gemspec index ea7a9cfcb..9eed3b91d 100644 --- a/shipit-engine.gemspec +++ b/shipit-engine.gemspec @@ -17,6 +17,8 @@ Gem::Specification.new do |s| s.files = Dir["{app,config,db,lib,vendor}/**/*", "LICENSE", "Rakefile", "README.md"] s.test_files = Dir["test/**/*"] - Dir["test/dummy/tmp/**/*"] - Dir["test/dummy/log/**/*"] + s.required_ruby_version = '>= 3.0.0' + s.add_dependency('active_model_serializers', '~> 0.9.3') s.add_dependency('ansi_stream', '~> 0.0.6') s.add_dependency('autoprefixer-rails', '~> 6.4.1') From 42c242c795eedd3406697d5c0c2848e388d92f38 Mon Sep 17 00:00:00 2001 From: Jenny Shen Date: Tue, 9 Apr 2024 11:17:08 -0400 Subject: [PATCH 12/20] Remove ruby version definition in CI/CD it will be read from .ruby-version --- .github/workflows/main.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1a1f81ecc..211e28835 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -9,10 +9,9 @@ jobs: steps: - uses: actions/checkout@v1 - - name: Set up Ruby 3.0 + - name: Set up Ruby uses: ruby/setup-ruby@v1 with: - ruby-version: '3.0' bundler-cache: true - name: rubocop run: | @@ -140,8 +139,6 @@ jobs: - uses: actions/checkout@v1 - name: Set up Ruby uses: ruby/setup-ruby@v1 - with: - ruby-version: '3.0' - name: Run setup script run: | git config --global user.email "you@example.com" From c62f05bb7d921c36d6cc3929abed4350814bcaa1 Mon Sep 17 00:00:00 2001 From: Kate Boyd Date: Thu, 11 Apr 2024 15:09:38 -0600 Subject: [PATCH 13/20] Make RefreshCheckRunsJob and RefreshStatusesJob enqueue one of themselves per commit --- app/jobs/shipit/refresh_check_runs_job.rb | 4 +++- app/jobs/shipit/refresh_statuses_job.rb | 4 +++- test/jobs/refresh_check_runs_job_test.rb | 23 +++++++++++++++++++ ...b_test.rb => refresh_statuses_job_test.rb} | 8 +++---- 4 files changed, 33 insertions(+), 6 deletions(-) create mode 100644 test/jobs/refresh_check_runs_job_test.rb rename test/jobs/{refresh_status_job_test.rb => refresh_statuses_job_test.rb} (67%) diff --git a/app/jobs/shipit/refresh_check_runs_job.rb b/app/jobs/shipit/refresh_check_runs_job.rb index 885783112..26cf8f31f 100644 --- a/app/jobs/shipit/refresh_check_runs_job.rb +++ b/app/jobs/shipit/refresh_check_runs_job.rb @@ -8,7 +8,9 @@ def perform(params) Commit.find(params[:commit_id]).refresh_check_runs! else stack = Stack.find(params[:stack_id]) - stack.commits.order(id: :desc).limit(30).each(&:refresh_check_runs!) + stack.commits.order(id: :desc).limit(30).each do |commit| + RefreshCheckRunsJob.perform_later(commit_id: commit.id) + end end end end diff --git a/app/jobs/shipit/refresh_statuses_job.rb b/app/jobs/shipit/refresh_statuses_job.rb index 0a94b5733..f21b609d0 100644 --- a/app/jobs/shipit/refresh_statuses_job.rb +++ b/app/jobs/shipit/refresh_statuses_job.rb @@ -8,7 +8,9 @@ def perform(params) Commit.find(params[:commit_id]).refresh_statuses! else stack = Stack.find(params[:stack_id]) - stack.commits.order(id: :desc).limit(30).each(&:refresh_statuses!) + stack.commits.order(id: :desc).limit(30).each do |commit| + RefreshStatusesJob.perform_later(commit_id: commit.id) + end end end end diff --git a/test/jobs/refresh_check_runs_job_test.rb b/test/jobs/refresh_check_runs_job_test.rb new file mode 100644 index 000000000..38f5cf3a5 --- /dev/null +++ b/test/jobs/refresh_check_runs_job_test.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true +require 'test_helper' + +module Shipit + class RefreshCheckRunsJobTest < ActiveSupport::TestCase + setup do + @stack = shipit_stacks(:shipit) + @job = RefreshCheckRunsJob.new + end + + test "#perform enqueues RefreshCheckRunsJob for the last 30 commits on the stack" do + assert_enqueued_jobs @stack.commits.count, only: RefreshCheckRunsJob do + @job.perform(stack_id: @stack.id) + end + end + + test "if :commit_id param is present only this commit is refreshed" do + Commit.any_instance.expects(:refresh_check_runs!).once + + @job.perform(stack_id: @stack.id, commit_id: shipit_commits(:first).id) + end + end +end diff --git a/test/jobs/refresh_status_job_test.rb b/test/jobs/refresh_statuses_job_test.rb similarity index 67% rename from test/jobs/refresh_status_job_test.rb rename to test/jobs/refresh_statuses_job_test.rb index d60e477f8..5ccf711eb 100644 --- a/test/jobs/refresh_status_job_test.rb +++ b/test/jobs/refresh_statuses_job_test.rb @@ -8,10 +8,10 @@ class RefreshStatusesJobTest < ActiveSupport::TestCase @job = RefreshStatusesJob.new end - test "#perform call #refresh_statuses! on the last 30 commits of the stack" do - Commit.any_instance.expects(:refresh_statuses!).times(@stack.commits.count) - - @job.perform(stack_id: @stack.id) + test "#perform enqueues RefreshStatusesJob for the last 30 commits on the stack" do + assert_enqueued_jobs @stack.commits.count, only: RefreshStatusesJob do + @job.perform(stack_id: @stack.id) + end end test "if :commit_id param is present only this commit is refreshed" do From 891d34158713dd12224155b598a540b00e56f134 Mon Sep 17 00:00:00 2001 From: Kate Boyd <77460334+kwboyd-shopify@users.noreply.github.com> Date: Wed, 17 Apr 2024 12:48:47 -0600 Subject: [PATCH 14/20] Revert "Make RefreshCheckRunsJob and RefreshStatusesJob enqueue one of themselves per commit" --- app/jobs/shipit/refresh_check_runs_job.rb | 4 +--- app/jobs/shipit/refresh_statuses_job.rb | 4 +--- test/jobs/refresh_check_runs_job_test.rb | 23 ------------------- ...job_test.rb => refresh_status_job_test.rb} | 8 +++---- 4 files changed, 6 insertions(+), 33 deletions(-) delete mode 100644 test/jobs/refresh_check_runs_job_test.rb rename test/jobs/{refresh_statuses_job_test.rb => refresh_status_job_test.rb} (67%) diff --git a/app/jobs/shipit/refresh_check_runs_job.rb b/app/jobs/shipit/refresh_check_runs_job.rb index 26cf8f31f..885783112 100644 --- a/app/jobs/shipit/refresh_check_runs_job.rb +++ b/app/jobs/shipit/refresh_check_runs_job.rb @@ -8,9 +8,7 @@ def perform(params) Commit.find(params[:commit_id]).refresh_check_runs! else stack = Stack.find(params[:stack_id]) - stack.commits.order(id: :desc).limit(30).each do |commit| - RefreshCheckRunsJob.perform_later(commit_id: commit.id) - end + stack.commits.order(id: :desc).limit(30).each(&:refresh_check_runs!) end end end diff --git a/app/jobs/shipit/refresh_statuses_job.rb b/app/jobs/shipit/refresh_statuses_job.rb index f21b609d0..0a94b5733 100644 --- a/app/jobs/shipit/refresh_statuses_job.rb +++ b/app/jobs/shipit/refresh_statuses_job.rb @@ -8,9 +8,7 @@ def perform(params) Commit.find(params[:commit_id]).refresh_statuses! else stack = Stack.find(params[:stack_id]) - stack.commits.order(id: :desc).limit(30).each do |commit| - RefreshStatusesJob.perform_later(commit_id: commit.id) - end + stack.commits.order(id: :desc).limit(30).each(&:refresh_statuses!) end end end diff --git a/test/jobs/refresh_check_runs_job_test.rb b/test/jobs/refresh_check_runs_job_test.rb deleted file mode 100644 index 38f5cf3a5..000000000 --- a/test/jobs/refresh_check_runs_job_test.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true -require 'test_helper' - -module Shipit - class RefreshCheckRunsJobTest < ActiveSupport::TestCase - setup do - @stack = shipit_stacks(:shipit) - @job = RefreshCheckRunsJob.new - end - - test "#perform enqueues RefreshCheckRunsJob for the last 30 commits on the stack" do - assert_enqueued_jobs @stack.commits.count, only: RefreshCheckRunsJob do - @job.perform(stack_id: @stack.id) - end - end - - test "if :commit_id param is present only this commit is refreshed" do - Commit.any_instance.expects(:refresh_check_runs!).once - - @job.perform(stack_id: @stack.id, commit_id: shipit_commits(:first).id) - end - end -end diff --git a/test/jobs/refresh_statuses_job_test.rb b/test/jobs/refresh_status_job_test.rb similarity index 67% rename from test/jobs/refresh_statuses_job_test.rb rename to test/jobs/refresh_status_job_test.rb index 5ccf711eb..d60e477f8 100644 --- a/test/jobs/refresh_statuses_job_test.rb +++ b/test/jobs/refresh_status_job_test.rb @@ -8,10 +8,10 @@ class RefreshStatusesJobTest < ActiveSupport::TestCase @job = RefreshStatusesJob.new end - test "#perform enqueues RefreshStatusesJob for the last 30 commits on the stack" do - assert_enqueued_jobs @stack.commits.count, only: RefreshStatusesJob do - @job.perform(stack_id: @stack.id) - end + test "#perform call #refresh_statuses! on the last 30 commits of the stack" do + Commit.any_instance.expects(:refresh_statuses!).times(@stack.commits.count) + + @job.perform(stack_id: @stack.id) end test "if :commit_id param is present only this commit is refreshed" do From ee4bab34f06bb97228e4861a0ba658c360272f01 Mon Sep 17 00:00:00 2001 From: Erik Ryhorchuk Date: Wed, 17 Apr 2024 13:37:00 -0700 Subject: [PATCH 15/20] add github secondary rate limit retries --- app/jobs/shipit/background_job.rb | 6 +++++ test/jobs/shipit/background_job_test.rb | 34 +++++++++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 test/jobs/shipit/background_job_test.rb diff --git a/app/jobs/shipit/background_job.rb b/app/jobs/shipit/background_job.rb index ee7d69e91..eb86c8850 100644 --- a/app/jobs/shipit/background_job.rb +++ b/app/jobs/shipit/background_job.rb @@ -5,9 +5,15 @@ class << self attr_accessor :timeout end + DEFAULT_RETRY_TIME_IN_SECONDS = 30 + # Write actions can sometimes fail intermittently, particulary for large and/or busy repositories retry_on(Octokit::BadGateway, Octokit::InternalServerError) + rescue_from(Octokit::TooManyRequests, Octokit::AbuseDetected) do |exception| + retry_job wait: exception.response_headers.fetch("Retry-After", DEFAULT_RETRY_TIME_IN_SECONDS) + end + def perform(*) with_timeout do super diff --git a/test/jobs/shipit/background_job_test.rb b/test/jobs/shipit/background_job_test.rb new file mode 100644 index 000000000..3c8be33b6 --- /dev/null +++ b/test/jobs/shipit/background_job_test.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true +require 'test_helper' + +module Shipit + class BackgroundJobTest < ActiveSupport::TestCase + setup do + @stack = shipit_stacks(:shipit) + @last_commit = @stack.commits.last + @job = CacheDeploySpecJob.new + @user = shipit_users(:walrus) + end + + test "#perform retries on Octokit secondary rate limit exceptions" do + freeze_time do + Octokit::Forbidden.any_instance.expects(:response_headers) + .returns({ "Retry-After" => 45 }) + + Shipit.github.api.expects(:user).with(@user.github_id).raises(Octokit::TooManyRequests) + + assert_enqueued_with(job: BackgroundStubJob, at: Time.now + 45.seconds) do + BackgroundStubJob.perform_now(@user) + end + end + end + + class BackgroundStubJob < BackgroundJob + queue_as :default + + def perform(user) + Shipit.github.api.user(user.github_id) + end + end + end +end From 139b1c3b5343a3c9740c4d7f903f28063061d243 Mon Sep 17 00:00:00 2001 From: Ruidan Date: Thu, 18 Apr 2024 11:22:57 -0400 Subject: [PATCH 16/20] Convert retry-after header string to seconds --- app/jobs/shipit/background_job.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/jobs/shipit/background_job.rb b/app/jobs/shipit/background_job.rb index eb86c8850..f4c0a0718 100644 --- a/app/jobs/shipit/background_job.rb +++ b/app/jobs/shipit/background_job.rb @@ -11,7 +11,7 @@ class << self retry_on(Octokit::BadGateway, Octokit::InternalServerError) rescue_from(Octokit::TooManyRequests, Octokit::AbuseDetected) do |exception| - retry_job wait: exception.response_headers.fetch("Retry-After", DEFAULT_RETRY_TIME_IN_SECONDS) + retry_job wait: exception.response_headers.fetch("Retry-After", DEFAULT_RETRY_TIME_IN_SECONDS).to_i.seconds end def perform(*) From 8c7a4e4d2205f6d65d380b019705bfb62280900a Mon Sep 17 00:00:00 2001 From: Simon Jagoe Date: Mon, 29 Jan 2024 10:38:31 +0200 Subject: [PATCH 17/20] Paginate check runs refresh --- app/models/shipit/commit.rb | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/app/models/shipit/commit.rb b/app/models/shipit/commit.rb index a1417cc5b..9e152b2ed 100644 --- a/app/models/shipit/commit.rb +++ b/app/models/shipit/commit.rb @@ -168,10 +168,26 @@ def create_status_from_github!(github_status) end end - def refresh_check_runs! + def paginated_check_runs response = stack.handle_github_redirections do - stack.github_api.check_runs(github_repo_name, sha) + stack.github_api.check_runs(github_repo_name, sha, per_page: 100) + end + + return response if stack.github_api.last_response.rels[:next].nil? + + loop do + page = stack.handle_github_redirections do + stack.github_api.get(stack.github_api.last_response.rels[:next].href) + end + response.check_runs.concat(page.check_runs) + break if stack.github_api.last_response.rels[:next].nil? end + + response + end + + def refresh_check_runs! + response = paginated_check_runs response.check_runs.each do |check_run| create_or_update_check_run_from_github!(check_run) end From e18a10ad32e5270af48fb49b91b1164869c18c86 Mon Sep 17 00:00:00 2001 From: Darren Worrall Date: Mon, 20 May 2024 13:49:11 +0100 Subject: [PATCH 18/20] Round the `finished in` second count --- app/models/shipit/task_execution_strategy/default.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/shipit/task_execution_strategy/default.rb b/app/models/shipit/task_execution_strategy/default.rb index 42af6c80a..3cd2998b9 100644 --- a/app/models/shipit/task_execution_strategy/default.rb +++ b/app/models/shipit/task_execution_strategy/default.rb @@ -94,7 +94,7 @@ def capture!(command) @task.write(line) end finished_at = Process.clock_gettime(Process::CLOCK_MONOTONIC) - @task.write("pid: #{command.pid} finished in: #{finished_at - started_at} seconds\n") + @task.write("pid: #{command.pid} finished in: #{(finished_at - started_at).round(3)} seconds\n") command.success? end From 3623994fcaa3de52080558ff03ed20e6453cce79 Mon Sep 17 00:00:00 2001 From: Javier Cervantes Date: Mon, 27 May 2024 13:34:07 -0400 Subject: [PATCH 19/20] Fix tests for paginate check runs refresh --- test/models/commits_test.rb | 7 ++++--- test/models/merge_request_test.rb | 4 +++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/test/models/commits_test.rb b/test/models/commits_test.rb index 0965167e5..55449bdc5 100644 --- a/test/models/commits_test.rb +++ b/test/models/commits_test.rb @@ -353,10 +353,11 @@ class CommitsTest < ActiveSupport::TestCase completed_at: Time.now, started_at: Time.now - 1.minute, ) - response = mock( + response = stub(rels: {}, data: mock( check_runs: [check_run], - ) - Shipit.github.api.expects(:check_runs).with(@stack.github_repo_name, @commit.sha).returns(response) + )) + Shipit.github.api.expects(:check_runs).with(@stack.github_repo_name, @commit.sha, per_page: 100).returns(response.data) + Shipit.github.api.expects(:last_response).returns(response) assert_difference -> { @commit.check_runs.count }, 1 do @commit.refresh_check_runs! diff --git a/test/models/merge_request_test.rb b/test/models/merge_request_test.rb index dc09ebcb6..a0140d4a6 100644 --- a/test/models/merge_request_test.rb +++ b/test/models/merge_request_test.rb @@ -125,7 +125,7 @@ class MergeRequestTest < ActiveSupport::TestCase created_at: 1.day.ago, )]) - Shipit.github.api.expects(:check_runs).with(@stack.github_repo_name, head_sha).returns(stub( + response = stub(rels: {}, data: stub( check_runs: [stub( id: 123456, name: 'check run', @@ -140,6 +140,8 @@ class MergeRequestTest < ActiveSupport::TestCase )] )) + Shipit.github.api.expects(:last_response).returns(response) + Shipit.github.api.expects(:check_runs).with(@stack.github_repo_name, head_sha, per_page: 100).returns(response.data) merge_request.refresh! assert_predicate merge_request, :mergeable? From 5ff2c97257280bf23fe27ad0f048508b251c0972 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 4 Jun 2024 22:28:50 +0000 Subject: [PATCH 20/20] Bump the bundler group across 1 directory with 5 updates Bumps the bundler group with 2 updates in the / directory: [rails](https://github.com/rails/rails) and [rexml](https://github.com/ruby/rexml). Updates `rails` from 7.1.1 to 7.1.3.4 - [Release notes](https://github.com/rails/rails/releases) - [Commits](https://github.com/rails/rails/compare/v7.1.1...v7.1.3.4) Updates `actionpack` from 7.1.1 to 7.1.3.4 - [Release notes](https://github.com/rails/rails/releases) - [Changelog](https://github.com/rails/rails/blob/v7.1.3.4/actionpack/CHANGELOG.md) - [Commits](https://github.com/rails/rails/compare/v7.1.1...v7.1.3.4) Updates `actiontext` from 7.1.1 to 7.1.3.4 - [Release notes](https://github.com/rails/rails/releases) - [Changelog](https://github.com/rails/rails/blob/v7.1.3.4/actiontext/CHANGELOG.md) - [Commits](https://github.com/rails/rails/compare/v7.1.1...v7.1.3.4) Updates `nokogiri` from 1.16.3 to 1.16.5 - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/main/CHANGELOG.md) - [Commits](https://github.com/sparklemotion/nokogiri/compare/v1.16.3...v1.16.5) Updates `rexml` from 3.2.5 to 3.2.8 - [Release notes](https://github.com/ruby/rexml/releases) - [Changelog](https://github.com/ruby/rexml/blob/master/NEWS.md) - [Commits](https://github.com/ruby/rexml/compare/v3.2.5...v3.2.8) --- updated-dependencies: - dependency-name: rails dependency-type: direct:production dependency-group: bundler - dependency-name: actionpack dependency-type: indirect dependency-group: bundler - dependency-name: actiontext dependency-type: indirect dependency-group: bundler - dependency-name: nokogiri dependency-type: indirect dependency-group: bundler - dependency-name: rexml dependency-type: indirect dependency-group: bundler ... Signed-off-by: dependabot[bot] --- Gemfile.lock | 152 ++++++++++++++++++++++++++------------------------- 1 file changed, 77 insertions(+), 75 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 89c198c3d..f1021ac8a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -33,50 +33,51 @@ PATH GEM remote: https://rubygems.org/ specs: - actioncable (7.1.1) - actionpack (= 7.1.1) - activesupport (= 7.1.1) + actioncable (7.1.3.4) + actionpack (= 7.1.3.4) + activesupport (= 7.1.3.4) nio4r (~> 2.0) websocket-driver (>= 0.6.1) zeitwerk (~> 2.6) - actionmailbox (7.1.1) - actionpack (= 7.1.1) - activejob (= 7.1.1) - activerecord (= 7.1.1) - activestorage (= 7.1.1) - activesupport (= 7.1.1) + actionmailbox (7.1.3.4) + actionpack (= 7.1.3.4) + activejob (= 7.1.3.4) + activerecord (= 7.1.3.4) + activestorage (= 7.1.3.4) + activesupport (= 7.1.3.4) mail (>= 2.7.1) net-imap net-pop net-smtp - actionmailer (7.1.1) - actionpack (= 7.1.1) - actionview (= 7.1.1) - activejob (= 7.1.1) - activesupport (= 7.1.1) + actionmailer (7.1.3.4) + actionpack (= 7.1.3.4) + actionview (= 7.1.3.4) + activejob (= 7.1.3.4) + activesupport (= 7.1.3.4) mail (~> 2.5, >= 2.5.4) net-imap net-pop net-smtp rails-dom-testing (~> 2.2) - actionpack (7.1.1) - actionview (= 7.1.1) - activesupport (= 7.1.1) + actionpack (7.1.3.4) + actionview (= 7.1.3.4) + activesupport (= 7.1.3.4) nokogiri (>= 1.8.5) + racc rack (>= 2.2.4) rack-session (>= 1.0.1) rack-test (>= 0.6.3) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) - actiontext (7.1.1) - actionpack (= 7.1.1) - activerecord (= 7.1.1) - activestorage (= 7.1.1) - activesupport (= 7.1.1) + actiontext (7.1.3.4) + actionpack (= 7.1.3.4) + activerecord (= 7.1.3.4) + activestorage (= 7.1.3.4) + activesupport (= 7.1.3.4) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (7.1.1) - activesupport (= 7.1.1) + actionview (7.1.3.4) + activesupport (= 7.1.3.4) builder (~> 3.1) erubi (~> 1.11) rails-dom-testing (~> 2.2) @@ -84,22 +85,22 @@ GEM active_model_serializers (0.9.9) activemodel (>= 3.2) concurrent-ruby (~> 1.0) - activejob (7.1.1) - activesupport (= 7.1.1) + activejob (7.1.3.4) + activesupport (= 7.1.3.4) globalid (>= 0.3.6) - activemodel (7.1.1) - activesupport (= 7.1.1) - activerecord (7.1.1) - activemodel (= 7.1.1) - activesupport (= 7.1.1) + activemodel (7.1.3.4) + activesupport (= 7.1.3.4) + activerecord (7.1.3.4) + activemodel (= 7.1.3.4) + activesupport (= 7.1.3.4) timeout (>= 0.4.0) - activestorage (7.1.1) - actionpack (= 7.1.1) - activejob (= 7.1.1) - activerecord (= 7.1.1) - activesupport (= 7.1.1) + activestorage (7.1.3.4) + actionpack (= 7.1.3.4) + activejob (= 7.1.3.4) + activerecord (= 7.1.3.4) + activesupport (= 7.1.3.4) marcel (~> 1.0) - activesupport (7.1.1) + activesupport (7.1.3.4) base64 bigdecimal concurrent-ruby (~> 1.0, >= 1.0.2) @@ -120,7 +121,7 @@ GEM ice_nine (~> 0.11.0) thread_safe (~> 0.3, >= 0.3.1) base64 (0.2.0) - bigdecimal (3.1.6) + bigdecimal (3.1.8) builder (3.2.4) byebug (11.1.3) coderay (1.1.3) @@ -133,7 +134,7 @@ GEM coffee-script-source execjs coffee-script-source (1.12.2) - concurrent-ruby (1.2.3) + concurrent-ruby (1.3.1) connection_pool (2.4.1) crack (0.4.5) rexml @@ -142,8 +143,7 @@ GEM descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) docile (1.4.0) - drb (2.2.0) - ruby2_keywords + drb (2.2.1) ejson (1.4.1) ejson-rails (0.2.1) ejson @@ -188,12 +188,12 @@ GEM activesupport (>= 6.1) hashdiff (1.0.1) hashie (5.0.0) - i18n (1.14.1) + i18n (1.14.5) concurrent-ruby (~> 1.0) ice_nine (0.11.2) io-console (0.7.2) - irb (1.11.1) - rdoc + irb (1.13.1) + rdoc (>= 4.0.0) reline (>= 0.4.2) jquery-rails (4.6.0) rails-dom-testing (>= 1, < 3) @@ -210,10 +210,10 @@ GEM net-imap net-pop net-smtp - marcel (1.0.2) + marcel (1.0.4) method_source (1.0.0) mini_mime (1.1.5) - minitest (5.21.2) + minitest (5.23.1) mocha (2.1.0) ruby2_keywords (>= 0.0.5) msgpack (1.7.1) @@ -221,19 +221,19 @@ GEM multipart-post (2.3.0) mutex_m (0.2.0) mysql2 (0.5.3) - net-imap (0.4.9.1) + net-imap (0.4.12) date net-protocol net-pop (0.1.2) net-protocol net-protocol (0.2.2) timeout - net-smtp (0.4.0.1) + net-smtp (0.5.0) net-protocol - nio4r (2.7.0) - nokogiri (1.16.3-arm64-darwin) + nio4r (2.7.3) + nokogiri (1.16.5-arm64-darwin) racc (~> 1.4) - nokogiri (1.16.3-x86_64-linux) + nokogiri (1.16.5-x86_64-linux) racc (~> 1.4) oauth2 (2.0.9) faraday (>= 0.17.3, < 3.0) @@ -263,13 +263,13 @@ GEM pry (0.14.1) coderay (~> 1.1) method_source (~> 1.0) - psych (4.0.6) + psych (5.1.2) stringio public_suffix (4.0.6) pubsubstub (0.2.2) rack redis (~> 4.0) - racc (1.7.3) + racc (1.8.0) rack (2.2.9) rack-session (1.0.2) rack (< 3) @@ -278,20 +278,20 @@ GEM rackup (1.0.0) rack (< 3) webrick - rails (7.1.1) - actioncable (= 7.1.1) - actionmailbox (= 7.1.1) - actionmailer (= 7.1.1) - actionpack (= 7.1.1) - actiontext (= 7.1.1) - actionview (= 7.1.1) - activejob (= 7.1.1) - activemodel (= 7.1.1) - activerecord (= 7.1.1) - activestorage (= 7.1.1) - activesupport (= 7.1.1) + rails (7.1.3.4) + actioncable (= 7.1.3.4) + actionmailbox (= 7.1.3.4) + actionmailer (= 7.1.3.4) + actionpack (= 7.1.3.4) + actiontext (= 7.1.3.4) + actionview (= 7.1.3.4) + activejob (= 7.1.3.4) + activemodel (= 7.1.3.4) + activerecord (= 7.1.3.4) + activestorage (= 7.1.3.4) + activesupport (= 7.1.3.4) bundler (>= 1.15.0) - railties (= 7.1.1) + railties (= 7.1.3.4) rails-dom-testing (2.2.0) activesupport (>= 5.0.0) minitest @@ -306,28 +306,29 @@ GEM actionview (> 3.1) activesupport (> 3.1) railties (> 3.1) - railties (7.1.1) - actionpack (= 7.1.1) - activesupport (= 7.1.1) + railties (7.1.3.4) + actionpack (= 7.1.3.4) + activesupport (= 7.1.3.4) irb rackup (>= 1.0.0) rake (>= 12.2) thor (~> 1.0, >= 1.2.2) zeitwerk (~> 2.6) rainbow (3.1.1) - rake (13.1.0) - rdoc (6.6.3.1) + rake (13.2.1) + rdoc (6.7.0) psych (>= 4.0.0) redis (4.8.1) redis-objects (1.7.0) redis regexp_parser (2.2.1) - reline (0.4.2) + reline (0.5.8) io-console (~> 0.5) responders (3.1.0) actionpack (>= 5.2) railties (>= 5.2) - rexml (3.2.5) + rexml (3.2.8) + strscan (>= 3.0.9) rubocop (1.18.3) parallel (~> 1.10) parser (>= 3.0.0.0) @@ -385,7 +386,8 @@ GEM activerecord (>= 5.1) state_machines-activemodel (>= 0.8.0) stringio (3.1.0) - thor (1.3.0) + strscan (3.1.0) + thor (1.3.1) thread_safe (0.3.6) tilt (2.2.0) timeout (0.4.1) @@ -409,7 +411,7 @@ GEM websocket-driver (0.7.6) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) - zeitwerk (2.6.12) + zeitwerk (2.6.15) PLATFORMS arm64-darwin