diff --git a/Gemfile b/Gemfile index 57853f9f3b5..ad75d89221f 100644 --- a/Gemfile +++ b/Gemfile @@ -25,7 +25,9 @@ gem 'faraday', '~> 1.0' gem 'marginalia', '~> 1.9.0' # Authentication libraries -gem 'devise', '~> 4.6' +gem 'devise', '~> 4.7.2' +# TODO: verify ARM compile issue on 3.1.13+ version (see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/18828) +gem 'bcrypt', '3.1.12' gem 'doorkeeper', '~> 5.3.0' gem 'doorkeeper-openid_connect', '~> 1.7.4' gem 'omniauth', '~> 1.8' @@ -120,7 +122,7 @@ gem 'fog-local', '~> 0.6' gem 'fog-openstack', '~> 1.0' gem 'fog-rackspace', '~> 0.1.1' gem 'fog-aliyun', '~> 0.3' -gem 'gitlab-fog-azure-rm', '~> 0.9', require: false +gem 'gitlab-fog-azure-rm', '~> 1.0', require: false # for Google storage gem 'google-api-client', '~> 0.33' diff --git a/Gemfile.lock b/Gemfile.lock index 142a606faa0..9238920c78c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -235,7 +235,7 @@ GEM thor (>= 0.19, < 2) unicode_plot (>= 0.0.4, < 1.0.0) device_detector (1.0.0) - devise (4.7.1) + devise (4.7.3) bcrypt (~> 3.0) orm_adapter (~> 0.1) railties (>= 4.1.0) @@ -421,7 +421,7 @@ GEM github-markup (1.7.0) gitlab-chronic (0.10.5) numerizer (~> 0.2) - gitlab-fog-azure-rm (0.9.0) + gitlab-fog-azure-rm (1.0.0) azure-storage-blob (~> 2.0) azure-storage-common (~> 2.0) fog-core (= 2.1.0) @@ -1257,6 +1257,7 @@ DEPENDENCIES babosa (~> 1.0.2) base32 (~> 0.3.0) batch-loader (~> 1.4.0) + bcrypt (= 3.1.12) bcrypt_pbkdf (~> 1.0) benchmark-ips (~> 2.3.0) benchmark-memory (~> 0.1) @@ -1282,7 +1283,7 @@ DEPENDENCIES default_value_for (~> 3.3.0) derailed_benchmarks device_detector - devise (~> 4.6) + devise (~> 4.7.2) devise-two-factor (~> 3.1.0) diff_match_patch (~> 0.1.0) diffy (~> 3.3) @@ -1323,7 +1324,7 @@ DEPENDENCIES gitaly (~> 13.3.0.pre.rc1) github-markup (~> 1.7.0) gitlab-chronic (~> 0.10.5) - gitlab-fog-azure-rm (~> 0.9) + gitlab-fog-azure-rm (~> 1.0) gitlab-labkit (= 0.12.1) gitlab-license (~> 1.0) gitlab-mail_room (~> 0.0.6) diff --git a/app/helpers/services_helper.rb b/app/helpers/services_helper.rb index ae59f84e7da..114bbf59ae1 100644 --- a/app/helpers/services_helper.rb +++ b/app/helpers/services_helper.rb @@ -24,7 +24,7 @@ module ServicesHelper when "commit", "commit_events" s_("ProjectService|Event will be triggered when a commit is created/updated") when "deployment" - s_("ProjectService|Event will be triggered when a deployment finishes") + s_("ProjectService|Event will be triggered when a deployment starts or finishes") when "alert" s_("ProjectService|Event will be triggered when a new, unique alert is recorded") end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 3978620c74d..bc4a23bfb68 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -46,6 +46,8 @@ class Deployment < ApplicationRecord scope :older_than, -> (deployment) { where('id < ?', deployment.id) } scope :with_deployable, -> { includes(:deployable).where('deployable_id IS NOT NULL') } + FINISHED_STATUSES = %i[success failed canceled].freeze + state_machine :status, initial: :created do event :run do transition created: :running @@ -63,22 +65,10 @@ class Deployment < ApplicationRecord transition any - [:canceled] => :canceled end - before_transition any => [:success, :failed, :canceled] do |deployment| + before_transition any => FINISHED_STATUSES do |deployment| deployment.finished_at = Time.current end - after_transition any => :success do |deployment| - deployment.run_after_commit do - Deployments::SuccessWorker.perform_async(id) - end - end - - after_transition any => [:success, :failed, :canceled] do |deployment| - deployment.run_after_commit do - Deployments::FinishedWorker.perform_async(id) - end - end - after_transition any => :running do |deployment| next unless deployment.project.forward_deployment_enabled? @@ -86,6 +76,32 @@ class Deployment < ApplicationRecord Deployments::ForwardDeploymentWorker.perform_async(id) end end + + after_transition any => :running do |deployment| + deployment.run_after_commit do + next unless Feature.enabled?(:ci_send_deployment_hook_when_start, deployment.project) + + Deployments::ExecuteHooksWorker.perform_async(id) + end + end + + after_transition any => :success do |deployment| + deployment.run_after_commit do + Deployments::UpdateEnvironmentWorker.perform_async(id) + end + end + + after_transition any => FINISHED_STATUSES do |deployment| + deployment.run_after_commit do + Deployments::LinkMergeRequestWorker.perform_async(id) + end + end + + after_transition any => FINISHED_STATUSES do |deployment| + deployment.run_after_commit do + Deployments::ExecuteHooksWorker.perform_async(id) + end + end end enum status: { @@ -273,7 +289,7 @@ class Deployment < ApplicationRecord SQL end - # Changes the status of a deployment and triggers the correspinding state + # Changes the status of a deployment and triggers the corresponding state # machine events. def update_status(status) case status diff --git a/app/models/project_services/chat_message/deployment_message.rb b/app/models/project_services/chat_message/deployment_message.rb index dae3a56116e..5deb757e60f 100644 --- a/app/models/project_services/chat_message/deployment_message.rb +++ b/app/models/project_services/chat_message/deployment_message.rb @@ -38,7 +38,11 @@ module ChatMessage private def message - "Deploy to #{environment} #{humanized_status}" + if running? + "Starting deploy to #{environment}" + else + "Deploy to #{environment} #{humanized_status}" + end end def color @@ -73,5 +77,9 @@ module ChatMessage def humanized_status status == 'success' ? 'succeeded' : status end + + def running? + status == 'running' + end end end diff --git a/app/services/deployments/after_create_service.rb b/app/services/deployments/update_environment_service.rb similarity index 91% rename from app/services/deployments/after_create_service.rb rename to app/services/deployments/update_environment_service.rb index 3560f9c983b..e9c2f41f626 100644 --- a/app/services/deployments/after_create_service.rb +++ b/app/services/deployments/update_environment_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Deployments - class AfterCreateService + class UpdateEnvironmentService attr_reader :deployment attr_reader :deployable @@ -64,4 +64,4 @@ module Deployments end end -Deployments::AfterCreateService.prepend_if_ee('EE::Deployments::AfterCreateService') +Deployments::UpdateEnvironmentService.prepend_if_ee('EE::Deployments::UpdateEnvironmentService') diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml index 96da5136908..9c60201412c 100644 --- a/app/views/shared/web_hooks/_form.html.haml +++ b/app/views/shared/web_hooks/_form.html.haml @@ -77,7 +77,7 @@ = form.label :deployment_events, class: 'list-label form-check-label ml-1' do %strong= s_('Webhooks|Deployment events') %p.text-muted.ml-1 - = s_('Webhooks|This URL will be triggered when a deployment is finished/failed/canceled') + = s_('Webhooks|This URL is triggered when a deployment starts, finishes, fails, or is canceled') .form-group = form.label :enable_ssl_verification, s_('Webhooks|SSL verification'), class: 'label-bold checkbox' .form-check diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index bdcb31b8d46..4b2d32b47d3 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -427,6 +427,14 @@ :weight: 1 :idempotent: true :tags: [] +- :name: deployment:deployments_execute_hooks + :feature_category: :continuous_delivery + :has_external_dependencies: + :urgency: :low + :resource_boundary: :cpu + :weight: 3 + :idempotent: + :tags: [] - :name: deployment:deployments_finished :feature_category: :continuous_delivery :has_external_dependencies: @@ -443,6 +451,14 @@ :weight: 3 :idempotent: :tags: [] +- :name: deployment:deployments_link_merge_request + :feature_category: :continuous_delivery + :has_external_dependencies: + :urgency: :low + :resource_boundary: :cpu + :weight: 3 + :idempotent: true + :tags: [] - :name: deployment:deployments_success :feature_category: :continuous_delivery :has_external_dependencies: @@ -451,6 +467,14 @@ :weight: 3 :idempotent: :tags: [] +- :name: deployment:deployments_update_environment + :feature_category: :continuous_delivery + :has_external_dependencies: + :urgency: :low + :resource_boundary: :cpu + :weight: 3 + :idempotent: true + :tags: [] - :name: gcp_cluster:cluster_configure_istio :feature_category: :kubernetes_management :has_external_dependencies: true diff --git a/app/workers/deployments/execute_hooks_worker.rb b/app/workers/deployments/execute_hooks_worker.rb new file mode 100644 index 00000000000..6be05232321 --- /dev/null +++ b/app/workers/deployments/execute_hooks_worker.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Deployments + class ExecuteHooksWorker # rubocop:disable Scalability/IdempotentWorker + include ApplicationWorker + + queue_namespace :deployment + feature_category :continuous_delivery + worker_resource_boundary :cpu + + def perform(deployment_id) + if (deploy = Deployment.find_by_id(deployment_id)) + deploy.execute_hooks + end + end + end +end diff --git a/app/workers/deployments/link_merge_request_worker.rb b/app/workers/deployments/link_merge_request_worker.rb new file mode 100644 index 00000000000..4723691a0bb --- /dev/null +++ b/app/workers/deployments/link_merge_request_worker.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Deployments + class LinkMergeRequestWorker + include ApplicationWorker + + queue_namespace :deployment + idempotent! + feature_category :continuous_delivery + worker_resource_boundary :cpu + + def perform(deployment_id) + if (deploy = Deployment.find_by_id(deployment_id)) + LinkMergeRequestsService.new(deploy).execute + end + end + end +end diff --git a/app/workers/deployments/success_worker.rb b/app/workers/deployments/success_worker.rb index 17f790d2f6f..ff1b2d80c1a 100644 --- a/app/workers/deployments/success_worker.rb +++ b/app/workers/deployments/success_worker.rb @@ -12,7 +12,7 @@ module Deployments Deployment.find_by_id(deployment_id).try do |deployment| break unless deployment.success? - Deployments::AfterCreateService.new(deployment).execute + Deployments::UpdateEnvironmentService.new(deployment).execute end end end diff --git a/app/workers/deployments/update_environment_worker.rb b/app/workers/deployments/update_environment_worker.rb new file mode 100644 index 00000000000..2381f9926bc --- /dev/null +++ b/app/workers/deployments/update_environment_worker.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Deployments + class UpdateEnvironmentWorker + include ApplicationWorker + + queue_namespace :deployment + idempotent! + feature_category :continuous_delivery + worker_resource_boundary :cpu + + def perform(deployment_id) + Deployment.find_by_id(deployment_id).try do |deployment| + break unless deployment.success? + + Deployments::UpdateEnvironmentService.new(deployment).execute + end + end + end +end diff --git a/changelogs/unreleased/send-chat-notification-when-deployment-starts.yml b/changelogs/unreleased/send-chat-notification-when-deployment-starts.yml new file mode 100644 index 00000000000..d9330677a85 --- /dev/null +++ b/changelogs/unreleased/send-chat-notification-when-deployment-starts.yml @@ -0,0 +1,5 @@ +--- +title: Send chat notification when deployment starts +merge_request: 41214 +author: Sashi Kumar +type: added diff --git a/changelogs/unreleased/sh-update-fog-azure-rm-gem.yml b/changelogs/unreleased/sh-update-fog-azure-rm-gem.yml new file mode 100644 index 00000000000..25d143a011b --- /dev/null +++ b/changelogs/unreleased/sh-update-fog-azure-rm-gem.yml @@ -0,0 +1,5 @@ +--- +title: Fix large backups not working with Azure Blob storage +merge_request: 44233 +author: +type: fixed diff --git a/config/feature_flags/development/ci_send_deployment_hook_when_start.yml b/config/feature_flags/development/ci_send_deployment_hook_when_start.yml new file mode 100644 index 00000000000..41f8e719b63 --- /dev/null +++ b/config/feature_flags/development/ci_send_deployment_hook_when_start.yml @@ -0,0 +1,7 @@ +--- +name: ci_send_deployment_hook_when_start +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41214 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/247137 +group: group::progressive delivery +type: development +default_enabled: false diff --git a/db/migrate/20200817195628_add_modified_to_approval_merge_request_rule.rb b/db/migrate/20200817195628_add_modified_to_approval_merge_request_rule.rb new file mode 100644 index 00000000000..71a29ae9bc5 --- /dev/null +++ b/db/migrate/20200817195628_add_modified_to_approval_merge_request_rule.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddModifiedToApprovalMergeRequestRule < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + add_column :approval_merge_request_rules, :modified_from_project_rule, :boolean, default: false, null: false + end +end diff --git a/db/post_migrate/20200901170135_backfill_modified_column_for_approval_merge_request_rules.rb b/db/post_migrate/20200901170135_backfill_modified_column_for_approval_merge_request_rules.rb new file mode 100644 index 00000000000..9a9866f38ec --- /dev/null +++ b/db/post_migrate/20200901170135_backfill_modified_column_for_approval_merge_request_rules.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class BackfillModifiedColumnForApprovalMergeRequestRules < ActiveRecord::Migration[6.0] + include Gitlab::Database::Migrations::BackgroundMigrationHelpers + + disable_ddl_transaction! + + class ApprovalMergeRequestRule < ActiveRecord::Base + include ::EachBatch + + self.table_name = 'approval_merge_request_rules' + end + + def change + queue_background_migration_jobs_by_range_at_intervals(ApprovalMergeRequestRule, 'AddModifiedToApprovalMergeRequestRule', 2.minutes, batch_size: 10_000) + end +end diff --git a/db/schema_migrations/20200817195628 b/db/schema_migrations/20200817195628 new file mode 100644 index 00000000000..7d919242d89 --- /dev/null +++ b/db/schema_migrations/20200817195628 @@ -0,0 +1 @@ +4da25ee40eabd81765b562929c819da1fc2b0f8afe1f4eefe6b769fcd8f0d4cd \ No newline at end of file diff --git a/db/schema_migrations/20200901170135 b/db/schema_migrations/20200901170135 new file mode 100644 index 00000000000..4f0f5213a98 --- /dev/null +++ b/db/schema_migrations/20200901170135 @@ -0,0 +1 @@ +fca99780272ca4ceb42fd38d16f67cd4a0a675ecdaf91e51c4c0728205212ed0 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 52369090416..bde64076670 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9338,6 +9338,7 @@ CREATE TABLE approval_merge_request_rules ( rule_type smallint DEFAULT 1 NOT NULL, report_type smallint, section text, + modified_from_project_rule boolean DEFAULT false NOT NULL, CONSTRAINT check_6fca5928b2 CHECK ((char_length(section) <= 255)) ); diff --git a/doc/user/project/integrations/webhooks.md b/doc/user/project/integrations/webhooks.md index 800eb1d3359..7adea5ebcd6 100644 --- a/doc/user/project/integrations/webhooks.md +++ b/doc/user/project/integrations/webhooks.md @@ -1303,7 +1303,12 @@ Note that `commit.id` is the ID of the pipeline, not the ID of the commit. ### Deployment events -Triggered when deployment is finished/failed/canceled. +Triggered when a deployment: + +- Starts ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41214) in GitLab 13.5.) +- Succeeds +- Fails +- Is cancelled **Request Header**: diff --git a/lib/gitlab/background_migration/add_modified_to_approval_merge_request_rule.rb b/lib/gitlab/background_migration/add_modified_to_approval_merge_request_rule.rb new file mode 100644 index 00000000000..2148e96f6b4 --- /dev/null +++ b/lib/gitlab/background_migration/add_modified_to_approval_merge_request_rule.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # Compare all current rules to project rules + class AddModifiedToApprovalMergeRequestRule + # Stubbed class to access the Group table + class Group < ActiveRecord::Base + self.table_name = 'namespaces' + self.inheritance_column = :_type_disabled + end + + # Stubbed class to access the ApprovalMergeRequestRule table + class ApprovalMergeRequestRule < ActiveRecord::Base + self.table_name = 'approval_merge_request_rules' + + has_one :approval_merge_request_rule_source, class_name: 'AddModifiedToApprovalMergeRequestRule::ApprovalMergeRequestRuleSource' + has_one :approval_project_rule, through: :approval_merge_request_rule_source + has_and_belongs_to_many :groups, + class_name: 'AddModifiedToApprovalMergeRequestRule::Group', join_table: "#{self.table_name}_groups" + end + + # Stubbed class to access the ApprovalProjectRule table + class ApprovalProjectRule < ActiveRecord::Base + self.table_name = 'approval_project_rules' + + has_many :approval_merge_request_rule_sources, class_name: 'AddModifiedToApprovalMergeRequestRule::ApprovalMergeRequestRuleSource' + has_and_belongs_to_many :groups, + class_name: 'AddModifiedToApprovalMergeRequestRule::Group', join_table: "#{self.table_name}_groups" + end + + # Stubbed class to access the ApprovalMergeRequestRuleSource table + class ApprovalMergeRequestRuleSource < ActiveRecord::Base + self.table_name = 'approval_merge_request_rule_sources' + + belongs_to :approval_merge_request_rule, class_name: 'AddModifiedToApprovalMergeRequestRule::ApprovalMergeRequestRule' + belongs_to :approval_project_rule, class_name: 'AddModifiedToApprovalMergeRequestRule::ApprovalProjectRule' + end + + def perform(start_id, stop_id) + approval_merge_requests_rules = ApprovalMergeRequestRule + .joins(:groups, :approval_merge_request_rule_source) + .where(id: start_id..stop_id) + .pluck( + 'approval_merge_request_rule_sources.id as ars_id', + 'approval_merge_request_rules_groups.id as amrg_id' + ) + + approval_project_rules = ApprovalProjectRule + .joins(:groups, approval_merge_request_rule_sources: :approval_merge_request_rule) + .where(approval_merge_request_rules: { id: start_id..stop_id }) + .pluck( + 'approval_merge_request_rule_sources.id as ars_id', + 'approval_project_rules_groups.id as apg_id' + ) + + different_names_or_approval_sources = ApprovalMergeRequestRule.joins(:approval_project_rule, :approval_merge_request_rule_source) + .where(id: start_id..stop_id) + .where('approval_merge_request_rules.name != approval_project_rules.name OR ' \ + 'approval_merge_request_rules.approvals_required != approval_project_rules.approvals_required') + .pluck('approval_merge_request_rule_sources.id as ars_id') + + intersected_set = approval_merge_requests_rules.to_set ^ approval_project_rules.to_set + source_ids = intersected_set.collect { |rule| rule[0] }.uniq + + rule_sources = ApprovalMergeRequestRuleSource.where(id: source_ids + different_names_or_approval_sources) + changed_merge_request_rules = ApprovalMergeRequestRule.where(id: rule_sources.select(:approval_merge_request_rule_id)) + + changed_merge_request_rules.update_all(modified_from_project_rule: true) + end + end + end +end diff --git a/lib/gitlab/data_builder/deployment.rb b/lib/gitlab/data_builder/deployment.rb index 70587b3132a..87ebe832862 100644 --- a/lib/gitlab/data_builder/deployment.rb +++ b/lib/gitlab/data_builder/deployment.rb @@ -20,8 +20,8 @@ module Gitlab environment: deployment.environment.name, project: deployment.project.hook_attrs, short_sha: deployment.short_sha, - user: deployment.user.hook_attrs, - user_url: Gitlab::UrlBuilder.build(deployment.user), + user: deployment.deployed_by.hook_attrs, + user_url: Gitlab::UrlBuilder.build(deployment.deployed_by), commit_url: Gitlab::UrlBuilder.build(deployment.commit), commit_title: deployment.commit.title } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d4048b717ff..8fdea2bcb15 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -20015,7 +20015,7 @@ msgstr "" msgid "ProjectService|Event will be triggered when a confidential issue is created/updated/closed" msgstr "" -msgid "ProjectService|Event will be triggered when a deployment finishes" +msgid "ProjectService|Event will be triggered when a deployment starts or finishes" msgstr "" msgid "ProjectService|Event will be triggered when a merge request is created/updated/merged" @@ -28857,15 +28857,15 @@ msgstr "" msgid "Webhooks|Tag push events" msgstr "" +msgid "Webhooks|This URL is triggered when a deployment starts, finishes, fails, or is canceled" +msgstr "" + msgid "Webhooks|This URL will be triggered by a push to the repository" msgstr "" msgid "Webhooks|This URL will be triggered when a confidential issue is created/updated/merged" msgstr "" -msgid "Webhooks|This URL will be triggered when a deployment is finished/failed/canceled" -msgstr "" - msgid "Webhooks|This URL will be triggered when a merge request is created/updated/merged" msgstr "" diff --git a/qa/Rakefile b/qa/Rakefile index 844d8ff898d..1ecce8fdce9 100644 --- a/qa/Rakefile +++ b/qa/Rakefile @@ -2,6 +2,7 @@ require_relative 'qa/tools/revoke_all_personal_access_tokens' require_relative 'qa/tools/delete_subgroups' require_relative 'qa/tools/generate_perf_testdata' require_relative 'qa/tools/delete_test_ssh_keys' +require_relative 'qa/tools/initialize_gitlab_auth' desc "Revokes all personal access tokens" task :revoke_personal_access_tokens do @@ -13,6 +14,11 @@ task :delete_subgroups do QA::Tools::DeleteSubgroups.new.run end +desc "Initialize GitLab with an access token" +task :initialize_gitlab_auth, [:address] do |t, args| + QA::Tools::InitializeGitLabAuth.new(args).run +end + desc "Generate Performance Testdata" task :generate_perf_testdata, :type do |t, args| args.with_defaults(type: :all) diff --git a/qa/qa/tools/initialize_gitlab_auth.rb b/qa/qa/tools/initialize_gitlab_auth.rb new file mode 100644 index 00000000000..b06ddcab040 --- /dev/null +++ b/qa/qa/tools/initialize_gitlab_auth.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require_relative '../../qa' + +module QA + module Tools + # Task to set default password from Runtime::Env.default_password if not set already + # Also creates a personal access token + # @example + # $ bundle exec rake 'initialize_gitlab_auth[http://gitlab.test]' + class InitializeGitLabAuth + attr_reader :address + + def initialize(address:) + @address = address + end + + def run + Runtime::Scenario.define(:gitlab_address, address) + + puts "Signing in and creating the default password for the root user if it's not set already..." + QA::Runtime::Browser.visit(:gitlab, QA::Page::Main::Login) + Flow::Login.sign_in + + puts "Creating an API scoped access token for the root user..." + puts "Token: #{Resource::PersonalAccessToken.fabricate!.access_token}" + end + end + end +end diff --git a/spec/lib/gitlab/background_migration/add_modified_to_approval_merge_request_rule_spec.rb b/spec/lib/gitlab/background_migration/add_modified_to_approval_merge_request_rule_spec.rb new file mode 100644 index 00000000000..81b8b5dde08 --- /dev/null +++ b/spec/lib/gitlab/background_migration/add_modified_to_approval_merge_request_rule_spec.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::AddModifiedToApprovalMergeRequestRule, schema: 20200817195628 do + let(:determine_if_rules_are_modified) { described_class.new } + + let(:namespace) { table(:namespaces).create!(name: 'gitlab', path: 'gitlab') } + let(:projects) { table(:projects) } + let(:normal_project) { projects.create!(namespace_id: namespace.id) } + let(:overridden_project) { projects.create!(namespace_id: namespace.id) } + let(:rules) { table(:approval_merge_request_rules) } + let(:project_rules) { table(:approval_project_rules) } + let(:sources) { table(:approval_merge_request_rule_sources) } + let(:merge_requests) { table(:merge_requests) } + let(:groups) { table(:namespaces) } + let(:mr_groups) { table(:approval_merge_request_rules_groups) } + let(:project_groups) { table(:approval_project_rules_groups) } + + before do + project_rule = project_rules.create!(project_id: normal_project.id, approvals_required: 3, name: 'test rule') + overridden_project_rule = project_rules.create!(project_id: overridden_project.id, approvals_required: 5, name: 'other test rule') + overridden_project_rule_two = project_rules.create!(project_id: overridden_project.id, approvals_required: 7, name: 'super cool rule') + + merge_request = merge_requests.create!(target_branch: 'feature', source_branch: 'default', source_project_id: normal_project.id, target_project_id: normal_project.id) + overridden_merge_request = merge_requests.create!(target_branch: 'feature-2', source_branch: 'default', source_project_id: overridden_project.id, target_project_id: overridden_project.id) + + merge_rule = rules.create!(merge_request_id: merge_request.id, approvals_required: 3, name: 'test rule') + overridden_merge_rule = rules.create!(merge_request_id: overridden_merge_request.id, approvals_required: 6, name: 'other test rule') + overridden_merge_rule_two = rules.create!(merge_request_id: overridden_merge_request.id, approvals_required: 7, name: 'super cool rule') + + sources.create!(approval_project_rule_id: project_rule.id, approval_merge_request_rule_id: merge_rule.id) + sources.create!(approval_project_rule_id: overridden_project_rule.id, approval_merge_request_rule_id: overridden_merge_rule.id) + sources.create!(approval_project_rule_id: overridden_project_rule_two.id, approval_merge_request_rule_id: overridden_merge_rule_two.id) + + group1 = groups.create!(name: "group1", path: "test_group1", type: 'Group') + group2 = groups.create!(name: "group2", path: "test_group2", type: 'Group') + group3 = groups.create!(name: "group3", path: "test_group3", type: 'Group') + + project_groups.create!(approval_project_rule_id: overridden_project_rule_two.id, group_id: group1.id) + project_groups.create!(approval_project_rule_id: overridden_project_rule_two.id, group_id: group2.id) + project_groups.create!(approval_project_rule_id: overridden_project_rule_two.id, group_id: group3.id) + + mr_groups.create!(approval_merge_request_rule_id: overridden_merge_rule.id, group_id: group1.id) + mr_groups.create!(approval_merge_request_rule_id: overridden_merge_rule_two.id, group_id: group2.id) + end + + describe '#perform' do + it 'changes the correct rules' do + original_count = rules.all.count + + determine_if_rules_are_modified.perform(rules.minimum(:id), rules.maximum(:id)) + + results = rules.where(modified_from_project_rule: true) + + expect(results.count).to eq 2 + expect(results.collect(&:name)).to eq(['other test rule', 'super cool rule']) + expect(rules.count).to eq original_count + end + end +end diff --git a/spec/lib/gitlab/data_builder/deployment_spec.rb b/spec/lib/gitlab/data_builder/deployment_spec.rb index 155e66e2fcd..8fb7ab25b17 100644 --- a/spec/lib/gitlab/data_builder/deployment_spec.rb +++ b/spec/lib/gitlab/data_builder/deployment_spec.rb @@ -19,7 +19,7 @@ RSpec.describe Gitlab::DataBuilder::Deployment do deployment = create(:deployment, status: :failed, environment: environment, sha: commit.sha, project: project) deployable = deployment.deployable expected_deployable_url = Gitlab::Routing.url_helpers.project_job_url(deployable.project, deployable) - expected_user_url = Gitlab::Routing.url_helpers.user_url(deployment.user) + expected_user_url = Gitlab::Routing.url_helpers.user_url(deployment.deployed_by) expected_commit_url = Gitlab::UrlBuilder.build(commit) data = described_class.build(deployment) @@ -30,7 +30,7 @@ RSpec.describe Gitlab::DataBuilder::Deployment do expect(data[:environment]).to eq("somewhere") expect(data[:project]).to eq(project.hook_attrs) expect(data[:short_sha]).to eq(deployment.short_sha) - expect(data[:user]).to eq(deployment.user.hook_attrs) + expect(data[:user]).to eq(deployment.deployed_by.hook_attrs) expect(data[:user_url]).to eq(expected_user_url) expect(data[:commit_url]).to eq(expected_commit_url) expect(data[:commit_title]).to eq(commit.title) diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 7c33f29ebf3..45f48cd8a57 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -159,6 +159,8 @@ merge_requests: - assignees - reviews - approval_rules +- approval_merge_request_rule_sources +- approval_project_rules - approvals - approvers - approver_users @@ -467,6 +469,8 @@ project: - feature_usage - approval_rules - approval_merge_request_rules +- approval_merge_request_rule_sources +- approval_project_rules - approvers - approver_users - audit_events diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index cb29cbcbb72..49731757593 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1109,7 +1109,8 @@ RSpec.describe Ci::Build do let(:environment) { deployment.environment } before do - allow(Deployments::FinishedWorker).to receive(:perform_async) + allow(Deployments::LinkMergeRequestWorker).to receive(:perform_async) + allow(Deployments::ExecuteHooksWorker).to receive(:perform_async) end it 'has deployments record with created status' do @@ -1129,7 +1130,8 @@ RSpec.describe Ci::Build do context 'when transits to success' do before do - allow(Deployments::SuccessWorker).to receive(:perform_async) + allow(Deployments::UpdateEnvironmentWorker).to receive(:perform_async) + allow(Deployments::ExecuteHooksWorker).to receive(:perform_async) build.success! end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 1c7b11257ce..80bd47ec102 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -98,16 +98,36 @@ RSpec.describe Deployment do context 'when deployment runs' do let(:deployment) { create(:deployment) } - before do - deployment.run! - end - it 'starts running' do freeze_time do + deployment.run! + expect(deployment).to be_running expect(deployment.finished_at).to be_nil end end + + it 'executes Deployments::ExecuteHooksWorker asynchronously' do + expect(Deployments::ExecuteHooksWorker) + .to receive(:perform_async).with(deployment.id) + + deployment.run! + end + + it 'does not execute Deployments::ExecuteHooksWorker when feature is disabled' do + stub_feature_flags(ci_send_deployment_hook_when_start: false) + expect(Deployments::ExecuteHooksWorker) + .not_to receive(:perform_async).with(deployment.id) + + deployment.run! + end + + it 'executes Deployments::ForwardDeploymentWorker asynchronously' do + expect(Deployments::ForwardDeploymentWorker) + .to receive(:perform_async).once.with(deployment.id) + + deployment.run! + end end context 'when deployment succeeded' do @@ -122,15 +142,15 @@ RSpec.describe Deployment do end end - it 'executes Deployments::SuccessWorker asynchronously' do - expect(Deployments::SuccessWorker) + it 'executes Deployments::UpdateEnvironmentWorker asynchronously' do + expect(Deployments::UpdateEnvironmentWorker) .to receive(:perform_async).with(deployment.id) deployment.succeed! end - it 'executes Deployments::FinishedWorker asynchronously' do - expect(Deployments::FinishedWorker) + it 'executes Deployments::ExecuteHooksWorker asynchronously' do + expect(Deployments::ExecuteHooksWorker) .to receive(:perform_async).with(deployment.id) deployment.succeed! @@ -149,12 +169,19 @@ RSpec.describe Deployment do end end - it 'executes Deployments::FinishedWorker asynchronously' do - expect(Deployments::FinishedWorker) + it 'executes Deployments::LinkMergeRequestWorker asynchronously' do + expect(Deployments::LinkMergeRequestWorker) .to receive(:perform_async).with(deployment.id) deployment.drop! end + + it 'executes Deployments::ExecuteHooksWorker asynchronously' do + expect(Deployments::ExecuteHooksWorker) + .to receive(:perform_async).with(deployment.id) + + deployment.drop! + end end context 'when deployment was canceled' do @@ -169,12 +196,19 @@ RSpec.describe Deployment do end end - it 'executes Deployments::FinishedWorker asynchronously' do - expect(Deployments::FinishedWorker) + it 'executes Deployments::LinkMergeRequestWorker asynchronously' do + expect(Deployments::LinkMergeRequestWorker) .to receive(:perform_async).with(deployment.id) deployment.cancel! end + + it 'executes Deployments::ExecuteHooksWorker asynchronously' do + expect(Deployments::ExecuteHooksWorker) + .to receive(:perform_async).with(deployment.id) + + deployment.cancel! + end end end @@ -580,9 +614,10 @@ RSpec.describe Deployment do expect(deploy).to be_success end - it 'schedules SuccessWorker and FinishedWorker when finishing a deploy' do - expect(Deployments::SuccessWorker).to receive(:perform_async) - expect(Deployments::FinishedWorker).to receive(:perform_async) + it 'schedules workers when finishing a deploy' do + expect(Deployments::UpdateEnvironmentWorker).to receive(:perform_async) + expect(Deployments::LinkMergeRequestWorker).to receive(:perform_async) + expect(Deployments::ExecuteHooksWorker).to receive(:perform_async) deploy.update_status('success') end diff --git a/spec/models/project_services/chat_message/deployment_message_spec.rb b/spec/models/project_services/chat_message/deployment_message_spec.rb index 9c361f90ae0..6bdf2120b36 100644 --- a/spec/models/project_services/chat_message/deployment_message_spec.rb +++ b/spec/models/project_services/chat_message/deployment_message_spec.rb @@ -70,6 +70,17 @@ RSpec.describe ChatMessage::DeploymentMessage do expect(message.pretext).to eq('Deploy to staging unknown') end + + it 'returns a message for a running deployment' do + data = { + status: 'running', + environment: 'production' + } + + message = described_class.new(data) + + expect(message.pretext).to eq('Starting deploy to production') + end end describe '#attachments' do diff --git a/spec/services/deployments/create_service_spec.rb b/spec/services/deployments/create_service_spec.rb index d1f977c28d3..2d157c9d114 100644 --- a/spec/services/deployments/create_service_spec.rb +++ b/spec/services/deployments/create_service_spec.rb @@ -19,8 +19,9 @@ RSpec.describe Deployments::CreateService do status: 'success' ) - expect(Deployments::SuccessWorker).to receive(:perform_async) - expect(Deployments::FinishedWorker).to receive(:perform_async) + expect(Deployments::UpdateEnvironmentWorker).to receive(:perform_async) + expect(Deployments::LinkMergeRequestWorker).to receive(:perform_async) + expect(Deployments::ExecuteHooksWorker).to receive(:perform_async) expect(service.execute).to be_persisted end @@ -34,8 +35,9 @@ RSpec.describe Deployments::CreateService do tag: false ) - expect(Deployments::SuccessWorker).not_to receive(:perform_async) - expect(Deployments::FinishedWorker).not_to receive(:perform_async) + expect(Deployments::UpdateEnvironmentWorker).not_to receive(:perform_async) + expect(Deployments::LinkMergeRequestWorker).not_to receive(:perform_async) + expect(Deployments::ExecuteHooksWorker).not_to receive(:perform_async) expect(service.execute).to be_persisted end diff --git a/spec/services/deployments/after_create_service_spec.rb b/spec/services/deployments/update_environment_service_spec.rb similarity index 95% rename from spec/services/deployments/after_create_service_spec.rb rename to spec/services/deployments/update_environment_service_spec.rb index ee9b008c2f8..66564a1042f 100644 --- a/spec/services/deployments/after_create_service_spec.rb +++ b/spec/services/deployments/update_environment_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Deployments::AfterCreateService do +RSpec.describe Deployments::UpdateEnvironmentService do let(:user) { create(:user) } let(:project) { create(:project, :repository) } let(:options) { { name: 'production' } } @@ -31,7 +31,8 @@ RSpec.describe Deployments::AfterCreateService do subject(:service) { described_class.new(deployment) } before do - allow(Deployments::FinishedWorker).to receive(:perform_async) + allow(Deployments::LinkMergeRequestWorker).to receive(:perform_async) + allow(Deployments::ExecuteHooksWorker).to receive(:perform_async) job.success! # Create/Succeed deployment end @@ -100,8 +101,8 @@ RSpec.describe Deployments::AfterCreateService do end before do - environment.update(name: 'review-apps/master') - job.update(environment: 'review-apps/$CI_COMMIT_REF_NAME') + environment.update!(name: 'review-apps/master') + job.update!(environment: 'review-apps/$CI_COMMIT_REF_NAME') end it 'does not create a new environment' do @@ -241,7 +242,7 @@ RSpec.describe Deployments::AfterCreateService do end it 'does not raise errors if the merge request does not have a metrics record' do - merge_request.metrics.destroy + merge_request.metrics.destroy! expect(merge_request.reload.metrics).to be_nil expect { service.execute }.not_to raise_error diff --git a/spec/workers/deployments/execute_hooks_worker_spec.rb b/spec/workers/deployments/execute_hooks_worker_spec.rb new file mode 100644 index 00000000000..fb1dc8cf290 --- /dev/null +++ b/spec/workers/deployments/execute_hooks_worker_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Deployments::ExecuteHooksWorker do + let(:worker) { described_class.new } + + describe '#perform' do + before do + allow(ProjectServiceWorker).to receive(:perform_async) + end + + it 'executes project services for deployment_hooks' do + deployment = create(:deployment, :running) + project = deployment.project + service = create(:service, type: 'SlackService', project: project, deployment_events: true, active: true) + + expect(ProjectServiceWorker).to receive(:perform_async).with(service.id, an_instance_of(Hash)) + + worker.perform(deployment.id) + end + + it 'does not execute an inactive service' do + deployment = create(:deployment, :running) + project = deployment.project + create(:service, type: 'SlackService', project: project, deployment_events: true, active: false) + + expect(ProjectServiceWorker).not_to receive(:perform_async) + + worker.perform(deployment.id) + end + + it 'does not execute if a deployment does not exist' do + expect(ProjectServiceWorker).not_to receive(:perform_async) + + worker.perform(non_existing_record_id) + end + + it 'execute webhooks' do + deployment = create(:deployment, :running) + project = deployment.project + web_hook = create(:project_hook, deployment_events: true, project: project) + + expect_next_instance_of(WebHookService, web_hook, an_instance_of(Hash), "deployment_hooks") do |service| + expect(service).to receive(:async_execute) + end + + worker.perform(deployment.id) + end + end +end diff --git a/spec/workers/deployments/link_merge_request_worker_spec.rb b/spec/workers/deployments/link_merge_request_worker_spec.rb new file mode 100644 index 00000000000..a55dd897bc7 --- /dev/null +++ b/spec/workers/deployments/link_merge_request_worker_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Deployments::LinkMergeRequestWorker do + subject(:worker) { described_class.new } + + describe '#perform' do + it 'links merge requests to the deployment' do + deployment = create(:deployment) + service = instance_double(Deployments::LinkMergeRequestsService) + + expect(Deployments::LinkMergeRequestsService) + .to receive(:new) + .with(deployment) + .and_return(service) + + expect(service).to receive(:execute) + + worker.perform(deployment.id) + end + + it 'does not link merge requests when the deployment is not found' do + expect(Deployments::LinkMergeRequestsService).not_to receive(:new) + + worker.perform(non_existing_record_id) + end + end + + context 'idempotent' do + include_examples 'an idempotent worker' do + let(:project) { create(:project, :repository) } + let(:environment) { create(:environment, project: project) } + let(:deployment) { create(:deployment, :success, project: project, environment: environment) } + let(:job_args) { deployment.id } + + it 'links merge requests to deployment' do + mr1 = create( + :merge_request, + :merged, + source_project: project, + target_project: project, + source_branch: 'source1', + target_branch: deployment.ref + ) + + mr2 = create( + :merge_request, + :merged, + source_project: project, + target_project: project, + source_branch: 'source2', + target_branch: deployment.ref + ) + + mr3 = create( + :merge_request, + :merged, + source_project: project, + target_project: project, + target_branch: 'foo' + ) + + subject + + expect(deployment.merge_requests).to include(mr1, mr2) + expect(deployment.merge_requests).not_to include(mr3) + end + end + end +end diff --git a/spec/workers/deployments/success_worker_spec.rb b/spec/workers/deployments/success_worker_spec.rb index 7c21a3147a7..d9996e66919 100644 --- a/spec/workers/deployments/success_worker_spec.rb +++ b/spec/workers/deployments/success_worker_spec.rb @@ -8,8 +8,8 @@ RSpec.describe Deployments::SuccessWorker do context 'when successful deployment' do let(:deployment) { create(:deployment, :success) } - it 'executes Deployments::AfterCreateService' do - expect(Deployments::AfterCreateService) + it 'executes Deployments::UpdateEnvironmentService' do + expect(Deployments::UpdateEnvironmentService) .to receive(:new).with(deployment).and_call_original subject @@ -19,8 +19,8 @@ RSpec.describe Deployments::SuccessWorker do context 'when canceled deployment' do let(:deployment) { create(:deployment, :canceled) } - it 'does not execute Deployments::AfterCreateService' do - expect(Deployments::AfterCreateService).not_to receive(:new) + it 'does not execute Deployments::UpdateEnvironmentService' do + expect(Deployments::UpdateEnvironmentService).not_to receive(:new) subject end @@ -29,8 +29,8 @@ RSpec.describe Deployments::SuccessWorker do context 'when deploy record does not exist' do let(:deployment) { nil } - it 'does not execute Deployments::AfterCreateService' do - expect(Deployments::AfterCreateService).not_to receive(:new) + it 'does not execute Deployments::UpdateEnvironmentService' do + expect(Deployments::UpdateEnvironmentService).not_to receive(:new) subject end diff --git a/spec/workers/deployments/update_environment_worker_spec.rb b/spec/workers/deployments/update_environment_worker_spec.rb new file mode 100644 index 00000000000..d67cbd62616 --- /dev/null +++ b/spec/workers/deployments/update_environment_worker_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Deployments::UpdateEnvironmentWorker do + subject(:worker) { described_class.new } + + context 'when successful deployment' do + let(:deployment) { create(:deployment, :success) } + + it 'executes Deployments::UpdateEnvironmentService' do + service = instance_double(Deployments::UpdateEnvironmentService) + + expect(Deployments::UpdateEnvironmentService) + .to receive(:new) + .with(deployment) + .and_return(service) + + expect(service).to receive(:execute) + + worker.perform(deployment.id) + end + end + + context 'when canceled deployment' do + let(:deployment) { create(:deployment, :canceled) } + + it 'does not execute Deployments::UpdateEnvironmentService' do + expect(Deployments::UpdateEnvironmentService).not_to receive(:new) + + worker.perform(deployment.id) + end + end + + context 'when deploy record does not exist' do + it 'does not execute Deployments::UpdateEnvironmentService' do + expect(Deployments::UpdateEnvironmentService).not_to receive(:new) + + worker.perform(non_existing_record_id) + end + end + + context 'idempotent' do + include_examples 'an idempotent worker' do + let(:project) { create(:project, :repository) } + let(:environment) { create(:environment, name: 'production') } + let(:deployment) { create(:deployment, :success, project: project, environment: environment) } + let(:merge_request) { create(:merge_request, target_branch: 'master', source_branch: 'feature', source_project: project) } + let(:job_args) { deployment.id } + + before do + merge_request.metrics.update!(merged_at: 1.hour.ago) + end + + it 'updates merge requests metrics' do + subject + + expect(merge_request.reload.metrics.first_deployed_to_production_at) + .to be_like_time(deployment.finished_at) + end + end + end +end