Merge branch '264393-add-event-timestamp-to-deployments-webhooks' into 'master'

Add event timestamp to deployments webhooks

See merge request gitlab-org/gitlab!60518

app/models/deployment.rb

@@ -87,7 +87,7 @@ class Deployment < ApplicationRecord
 
     after_transition any => :running do |deployment|
       deployment.run_after_commit do
-        Deployments::ExecuteHooksWorker.perform_async(id)
+        Deployments::HooksWorker.perform_async(deployment_id: id, status_changed_at: Time.current)
       end
     end
 
@@ -100,7 +100,7 @@ class Deployment < ApplicationRecord
 
     after_transition any => FINISHED_STATUSES do |deployment|
       deployment.run_after_commit do
-        Deployments::ExecuteHooksWorker.perform_async(id)
+        Deployments::HooksWorker.perform_async(deployment_id: id, status_changed_at: Time.current)
       end
     end
 
@@ -182,8 +182,8 @@ class Deployment < ApplicationRecord
     Commit.truncate_sha(sha)
   end
 
-  def execute_hooks
-    deployment_data = Gitlab::DataBuilder::Deployment.build(self)
+  def execute_hooks(status_changed_at)
+    deployment_data = Gitlab::DataBuilder::Deployment.build(self, status_changed_at)
     project.execute_hooks(deployment_data, :deployment_hooks)
     project.execute_services(deployment_data, :deployment_hooks)
   end

app/services/concerns/integrations/project_test_data.rb

@@ -63,7 +63,7 @@ module Integrations
 
       return { error: s_('TestHooks|Ensure the project has deployments.') } unless deployment.present?
 
-      Gitlab::DataBuilder::Deployment.build(deployment)
+      Gitlab::DataBuilder::Deployment.build(deployment, Time.current)
     end
 
     def releases_events_data

app/workers/all_queues.yml

@@ -642,6 +642,15 @@
   :weight: 3
   :idempotent:
   :tags: []
+- :name: deployment:deployments_hooks
+  :worker_name: Deployments::HooksWorker
+  :feature_category: :continuous_delivery
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 3
+  :idempotent:
+  :tags: []
 - :name: deployment:deployments_link_merge_request
   :worker_name: Deployments::LinkMergeRequestWorker
   :feature_category: :continuous_delivery

app/workers/deployments/execute_hooks_worker.rb

 # frozen_string_literal: true
 
 module Deployments
+  # TODO: remove in https://gitlab.com/gitlab-org/gitlab/-/issues/329360
   class ExecuteHooksWorker # rubocop:disable Scalability/IdempotentWorker
     include ApplicationWorker
 
@@ -10,7 +11,7 @@ module Deployments
 
     def perform(deployment_id)
       if (deploy = Deployment.find_by_id(deployment_id))
-        deploy.execute_hooks
+        deploy.execute_hooks(Time.current)
       end
     end
   end

app/workers/deployments/finished_worker.rb

@@ -13,7 +13,7 @@ module Deployments
     def perform(deployment_id)
       if (deploy = Deployment.find_by_id(deployment_id))
         LinkMergeRequestsService.new(deploy).execute
-        deploy.execute_hooks
+        deploy.execute_hooks(Time.current)
       end
     end
   end

app/workers/deployments/hooks_worker.rb

+# frozen_string_literal: true
+
+module Deployments
+  class HooksWorker # rubocop:disable Scalability/IdempotentWorker
+    include ApplicationWorker
+
+    queue_namespace :deployment
+    feature_category :continuous_delivery
+
+    def perform(params = {})
+      params = params.with_indifferent_access
+
+      if (deploy = Deployment.find_by_id(params[:deployment_id]))
+        deploy.execute_hooks(params[:status_changed_at].to_time)
+      end
+    end
+  end
+end

changelogs/unreleased/264393-add-event-timestamp-to-deployments-webhooks.yml

+---
+title: Add status_changed_at to deployments webhooks
+merge_request: 60518
+author:
+type: added

doc/user/project/integrations/webhooks.md

@@ -1368,6 +1368,7 @@ X-Gitlab-Event: Deployment Hook
 {
   "object_kind": "deployment",
   "status": "success",
+  "status_changed_at":"2021-04-28 21:50:00 +0200",
   "deployable_id": 796,
   "deployable_url": "http://10.126.0.2:3000/root/test-deployment-webhooks/-/jobs/796",
   "environment": "staging",

ee/spec/services/ci/process_build_service_spec.rb

@@ -36,7 +36,7 @@ RSpec.describe Ci::ProcessBuildService, '#execute' do
       context 'when user does not have access to the environment' do
         it 'fails the build' do
           allow(Deployments::LinkMergeRequestWorker).to receive(:perform_async)
-          allow(Deployments::ExecuteHooksWorker).to receive(:perform_async)
+          allow(Deployments::HooksWorker).to receive(:perform_async)
           subject
 
           expect(ci_build.failed?).to be_truthy

lib/gitlab/data_builder/deployment.rb

@@ -5,7 +5,7 @@ module Gitlab
     module Deployment
       extend self
 
-      def build(deployment)
+      def build(deployment, status_changed_at)
         # Deployments will not have a deployable when created using the API.
         deployable_url =
           if deployment.deployable
@@ -15,6 +15,7 @@ module Gitlab
         {
           object_kind: 'deployment',
           status: deployment.status,
+          status_changed_at: status_changed_at,
           deployable_id: deployment.deployable_id,
           deployable_url: deployable_url,
           environment: deployment.environment.name,

spec/lib/gitlab/data_builder/deployment_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe Gitlab::DataBuilder::Deployment do
     it 'returns the object kind for a deployment' do
       deployment = build(:deployment, deployable: nil, environment: create(:environment))
 
-      data = described_class.build(deployment)
+      data = described_class.build(deployment, Time.current)
 
       expect(data[:object_kind]).to eq('deployment')
     end
@@ -21,10 +21,12 @@ RSpec.describe Gitlab::DataBuilder::Deployment do
       expected_deployable_url = Gitlab::Routing.url_helpers.project_job_url(deployable.project, deployable)
       expected_user_url = Gitlab::Routing.url_helpers.user_url(deployment.deployed_by)
       expected_commit_url = Gitlab::UrlBuilder.build(commit)
+      status_changed_at = Time.current
 
-      data = described_class.build(deployment)
+      data = described_class.build(deployment, status_changed_at)
 
       expect(data[:status]).to eq('failed')
+      expect(data[:status_changed_at]).to eq(status_changed_at)
       expect(data[:deployable_id]).to eq(deployable.id)
       expect(data[:deployable_url]).to eq(expected_deployable_url)
       expect(data[:environment]).to eq("somewhere")
@@ -38,7 +40,7 @@ RSpec.describe Gitlab::DataBuilder::Deployment do
 
     it 'does not include the deployable URL when there is no deployable' do
       deployment = create(:deployment, status: :failed, deployable: nil)
-      data = described_class.build(deployment)
+      data = described_class.build(deployment, Time.current)
 
       expect(data[:deployable_url]).to be_nil
     end

spec/models/ci/build_spec.rb

@@ -1205,7 +1205,7 @@ RSpec.describe Ci::Build do
 
     before do
       allow(Deployments::LinkMergeRequestWorker).to receive(:perform_async)
-      allow(Deployments::ExecuteHooksWorker).to receive(:perform_async)
+      allow(Deployments::HooksWorker).to receive(:perform_async)
     end
 
     it 'has deployments record with created status' do
@@ -1241,7 +1241,7 @@ RSpec.describe Ci::Build do
 
       before do
         allow(Deployments::UpdateEnvironmentWorker).to receive(:perform_async)
-        allow(Deployments::ExecuteHooksWorker).to receive(:perform_async)
+        allow(Deployments::HooksWorker).to receive(:perform_async)
       end
 
       it_behaves_like 'avoid deadlock'

spec/models/deployment_spec.rb

@@ -107,11 +107,13 @@ RSpec.describe Deployment do
         end
       end
 
-      it 'executes Deployments::ExecuteHooksWorker asynchronously' do
-        expect(Deployments::ExecuteHooksWorker)
-            .to receive(:perform_async).with(deployment.id)
+      it 'executes Deployments::HooksWorker asynchronously' do
+        freeze_time do
+          expect(Deployments::HooksWorker)
+            .to receive(:perform_async).with(deployment_id: deployment.id, status_changed_at: Time.current)
 
-        deployment.run!
+          deployment.run!
+        end
       end
 
       it 'executes Deployments::DropOlderDeploymentsWorker asynchronously' do
@@ -141,11 +143,13 @@ RSpec.describe Deployment do
         deployment.succeed!
       end
 
-      it 'executes Deployments::ExecuteHooksWorker asynchronously' do
-        expect(Deployments::ExecuteHooksWorker)
-          .to receive(:perform_async).with(deployment.id)
+      it 'executes Deployments::HooksWorker asynchronously' do
+        freeze_time do
+          expect(Deployments::HooksWorker)
+          .to receive(:perform_async).with(deployment_id: deployment.id, status_changed_at: Time.current)
 
-        deployment.succeed!
+          deployment.succeed!
+        end
       end
     end
 
@@ -168,11 +172,13 @@ RSpec.describe Deployment do
         deployment.drop!
       end
 
-      it 'executes Deployments::ExecuteHooksWorker asynchronously' do
-        expect(Deployments::ExecuteHooksWorker)
-            .to receive(:perform_async).with(deployment.id)
+      it 'executes Deployments::HooksWorker asynchronously' do
+        freeze_time do
+          expect(Deployments::HooksWorker)
+            .to receive(:perform_async).with(deployment_id: deployment.id, status_changed_at: Time.current)
 
-        deployment.drop!
+          deployment.drop!
+        end
       end
     end
 
@@ -195,11 +201,13 @@ RSpec.describe Deployment do
         deployment.cancel!
       end
 
-      it 'executes Deployments::ExecuteHooksWorker asynchronously' do
-        expect(Deployments::ExecuteHooksWorker)
-            .to receive(:perform_async).with(deployment.id)
+      it 'executes Deployments::HooksWorker asynchronously' do
+        freeze_time do
+          expect(Deployments::HooksWorker)
+            .to receive(:perform_async).with(deployment_id: deployment.id, status_changed_at: Time.current)
 
-        deployment.cancel!
+          deployment.cancel!
+        end
       end
     end
 
@@ -220,11 +228,13 @@ RSpec.describe Deployment do
         deployment.skip!
       end
 
-      it 'does not execute Deployments::ExecuteHooksWorker' do
-        expect(Deployments::ExecuteHooksWorker)
-            .not_to receive(:perform_async).with(deployment.id)
+      it 'does not execute Deployments::HooksWorker' do
+        freeze_time do
+          expect(Deployments::HooksWorker)
+            .not_to receive(:perform_async).with(deployment_id: deployment.id, status_changed_at: Time.current)
 
-        deployment.skip!
+          deployment.skip!
+        end
       end
     end
 
@@ -714,7 +724,7 @@ RSpec.describe Deployment do
     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)
+      expect(Deployments::HooksWorker).to receive(:perform_async)
 
       deploy.update_status('success')
     end

spec/models/project_services/chat_message/deployment_message_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe ChatMessage::DeploymentMessage do
       project = create(:project, :repository)
       commit = project.commit('HEAD')
       deployment = create(:deployment, status: :success, environment: environment, project: project, sha: commit.sha)
-      data = Gitlab::DataBuilder::Deployment.build(deployment)
+      data = Gitlab::DataBuilder::Deployment.build(deployment, Time.current)
 
       message = described_class.new(data)
 
@@ -118,7 +118,7 @@ RSpec.describe ChatMessage::DeploymentMessage do
       job_url = Gitlab::Routing.url_helpers.project_job_url(project, ci_build)
       commit_url = Gitlab::UrlBuilder.build(deployment.commit)
       user_url = Gitlab::Routing.url_helpers.user_url(user)
-      data = Gitlab::DataBuilder::Deployment.build(deployment)
+      data = Gitlab::DataBuilder::Deployment.build(deployment, Time.current)
 
       message = described_class.new(data)
 

spec/models/project_services/slack_service_spec.rb

@@ -59,7 +59,7 @@ RSpec.describe SlackService do
       context 'deployment notification' do
         let_it_be(:deployment) { create(:deployment, user: user) }
 
-        let(:data) { Gitlab::DataBuilder::Deployment.build(deployment) }
+        let(:data) { Gitlab::DataBuilder::Deployment.build(deployment, Time.current) }
 
         it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_deployment_notification'
       end

spec/services/deployments/create_service_spec.rb

@@ -21,7 +21,7 @@ RSpec.describe Deployments::CreateService do
 
       expect(Deployments::UpdateEnvironmentWorker).to receive(:perform_async)
       expect(Deployments::LinkMergeRequestWorker).to receive(:perform_async)
-      expect(Deployments::ExecuteHooksWorker).to receive(:perform_async)
+      expect(Deployments::HooksWorker).to receive(:perform_async)
 
       expect(service.execute).to be_persisted
     end
@@ -37,7 +37,7 @@ RSpec.describe Deployments::CreateService do
 
       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(Deployments::HooksWorker).not_to receive(:perform_async)
 
       expect(service.execute).to be_persisted
     end
@@ -57,7 +57,7 @@ RSpec.describe Deployments::CreateService do
 
         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(Deployments::HooksWorker).not_to receive(:perform_async)
 
         described_class.new(environment.reload, user, params).execute
       end

spec/services/deployments/update_environment_service_spec.rb

@@ -33,7 +33,7 @@ RSpec.describe Deployments::UpdateEnvironmentService do
 
   before do
     allow(Deployments::LinkMergeRequestWorker).to receive(:perform_async)
-    allow(Deployments::ExecuteHooksWorker).to receive(:perform_async)
+    allow(Deployments::HooksWorker).to receive(:perform_async)
     job.success! # Create/Succeed deployment
   end
 

spec/support/shared_examples/models/slack_mattermost_notifications_shared_examples.rb

@@ -201,7 +201,7 @@ RSpec.shared_examples 'slack or mattermost notifications' do |service_name|
 
     context 'deployment events' do
       let_it_be(:deployment) { create(:deployment) }
-      let(:data) { Gitlab::DataBuilder::Deployment.build(deployment) }
+      let(:data) { Gitlab::DataBuilder::Deployment.build(deployment, Time.current) }
 
       it_behaves_like 'calls the service API with the event message', /Deploy to (.*?) created/
     end

spec/workers/deployments/hooks_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Deployments::HooksWorker 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: deployment.id, status_changed_at: Time.current)
+    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: deployment.id, status_changed_at: Time.current)
+    end
+
+    it 'does not execute if a deployment does not exist' do
+      expect(ProjectServiceWorker).not_to receive(:perform_async)
+
+      worker.perform(deployment_id: non_existing_record_id, status_changed_at: Time.current)
+    end
+
+    it 'execute webhooks' do
+      deployment = create(:deployment, :running)
+      project = deployment.project
+      web_hook = create(:project_hook, deployment_events: true, project: project)
+
+      status_changed_at = Time.current
+
+      expect_next_instance_of(WebHookService, web_hook, hash_including(status_changed_at: status_changed_at), "deployment_hooks") do |service|
+        expect(service).to receive(:async_execute)
+      end
+
+      worker.perform(deployment_id: deployment.id, status_changed_at: status_changed_at)
+    end
+  end
+end