From 24fa35552cb96e968bcac27f56099b25c6033fd1 Mon Sep 17 00:00:00 2001 From: Alex Kalderimis <akalderimis@gitlab.com> Date: Thu, 3 Dec 2020 18:54:17 +0000 Subject: [PATCH] Sync build information to Jira with JiraConnect This adds support for the `build` module for JiraConnect, and synchronizes pipeline status for linked Jira issues. Specifically: - we add a new section to the application descriptor (requiring an update of our installed JiraConnect app) - we add a new worker triggered by changes in status of pipelines - for every pipeline that is the head-pipeline of a relevant MR (determined by the presence of Jira issue keys in either the branch name or the title) we send that status information to Jira. Some notes: - The concept of a build seems to map most closely to the GitLab concept of a pipeline, so that is the unit of synchronization. - The Jira API allows the number of 'tests' in a build to be reported, with a breakdown by pass/fail/skip. Currently this is implemented to be the number of Ci::Builds in a pipeline - since we cannot guarantee that exact test numbers are available. This may be subject to change, or at least upgrade for test frameworks for which we have breakdown stats. --- .../jira_connect/app_descriptor_controller.rb | 71 +++++--- app/models/ci/pipeline.rb | 10 ++ app/services/jira_connect/sync_service.rb | 12 +- app/workers/all_queues.yml | 12 +- .../jira_connect/sync_branch_worker.rb | 1 + .../jira_connect/sync_builds_worker.rb | 24 +++ .../jira_connect/sync_merge_request_worker.rb | 2 + .../development/jira_sync_builds.yml | 8 + lib/atlassian/jira_connect/client.rb | 75 ++++++-- .../jira_connect/serializers/base_entity.rb | 6 + .../jira_connect/serializers/build_entity.rb | 94 ++++++++++ spec/factories/merge_requests.rb | 8 + spec/factories/sequences.rb | 2 + .../lib/atlassian/jira_connect/client_spec.rb | 163 +++++++++++++++--- .../serializers/build_entity_spec.rb | 52 ++++++ spec/models/ci/pipeline_spec.rb | 34 ++++ .../jira_connect/sync_service_spec.rb | 33 ++-- .../atlassian/jira_connect/schemata.rb | 83 +++++++++ spec/support/helpers/after_next_helpers.rb | 6 +- spec/support/helpers/next_instance_of.rb | 15 +- spec/support/matchers/be_valid_json.rb | 32 ++++ .../jira_connect/sync_branch_worker_spec.rb | 9 +- .../jira_connect/sync_builds_worker_spec.rb | 60 +++++++ .../sync_merge_request_worker_spec.rb | 9 +- .../jira_connect/sync_project_worker_spec.rb | 10 +- 25 files changed, 726 insertions(+), 105 deletions(-) create mode 100644 app/workers/jira_connect/sync_builds_worker.rb create mode 100644 config/feature_flags/development/jira_sync_builds.yml create mode 100644 lib/atlassian/jira_connect/serializers/build_entity.rb create mode 100644 spec/lib/atlassian/jira_connect/serializers/build_entity_spec.rb create mode 100644 spec/support/atlassian/jira_connect/schemata.rb create mode 100644 spec/support/matchers/be_valid_json.rb create mode 100644 spec/workers/jira_connect/sync_builds_worker_spec.rb diff --git a/app/controllers/jira_connect/app_descriptor_controller.rb b/app/controllers/jira_connect/app_descriptor_controller.rb index bf53c61601b..d1ba8a98c64 100644 --- a/app/controllers/jira_connect/app_descriptor_controller.rb +++ b/app/controllers/jira_connect/app_descriptor_controller.rb @@ -27,29 +27,9 @@ class JiraConnect::AppDescriptorController < JiraConnect::ApplicationController authentication: { type: 'jwt' }, + modules: modules, scopes: %w(READ WRITE DELETE), apiVersion: 1, - modules: { - jiraDevelopmentTool: { - key: 'gitlab-development-tool', - application: { - value: 'GitLab' - }, - name: { - value: 'GitLab' - }, - url: 'https://gitlab.com', - logoUrl: view_context.image_url('gitlab_logo.png'), - capabilities: %w(branch commit pull_request) - }, - postInstallPage: { - key: 'gitlab-configuration', - name: { - value: 'GitLab Configuration' - }, - url: relative_to_base_path(jira_connect_subscriptions_path) - } - }, apiMigrations: { gdpr: true } @@ -58,6 +38,55 @@ class JiraConnect::AppDescriptorController < JiraConnect::ApplicationController private + HOME_URL = 'https://gitlab.com' + DOC_URL = 'https://docs.gitlab.com/ee/user/project/integrations/jira.html#gitlab-jira-integration' + + def modules + modules = { + jiraDevelopmentTool: { + key: 'gitlab-development-tool', + application: { + value: 'GitLab' + }, + name: { + value: 'GitLab' + }, + url: HOME_URL, + logoUrl: logo_url, + capabilities: %w(branch commit pull_request) + }, + postInstallPage: { + key: 'gitlab-configuration', + name: { + value: 'GitLab Configuration' + }, + url: relative_to_base_path(jira_connect_subscriptions_path) + } + } + + modules.merge!(build_information_module) + + modules + end + + def logo_url + view_context.image_url('gitlab_logo.png') + end + + # See: https://developer.atlassian.com/cloud/jira/software/modules/build/ + def build_information_module + { + jiraBuildInfoProvider: { + homeUrl: HOME_URL, + logoUrl: logo_url, + documentationUrl: DOC_URL, + actions: {}, + name: { value: "GitLab CI" }, + key: "gitlab-ci" + } + } + end + def relative_to_base_path(full_path) full_path.sub(/^#{jira_connect_base_path}/, '') end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 3a5e745ed2f..a0208a630b9 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -259,6 +259,16 @@ module Ci end end + after_transition any => any do |pipeline| + next unless Feature.enabled?(:jira_sync_builds, pipeline.project) + + pipeline.run_after_commit do + # Passing the seq-id ensures this is idempotent + seq_id = ::Atlassian::JiraConnect::Client.generate_update_sequence_id + ::JiraConnect::SyncBuildsWorker.perform_async(pipeline.id, seq_id) + end + end + after_transition any => [:success, :failed] do |pipeline| ref_status = pipeline.ci_ref&.update_status_by!(pipeline) diff --git a/app/services/jira_connect/sync_service.rb b/app/services/jira_connect/sync_service.rb index f8855fb6deb..b2af284f1f0 100644 --- a/app/services/jira_connect/sync_service.rb +++ b/app/services/jira_connect/sync_service.rb @@ -6,13 +6,15 @@ module JiraConnect self.project = project end - def execute(commits: nil, branches: nil, merge_requests: nil, update_sequence_id: nil) - JiraConnectInstallation.for_project(project).each do |installation| + # Parameters: see Atlassian::JiraConnect::Client#send_info + # Includes: update_sequence_id, commits, branches, merge_requests, pipelines + def execute(**args) + JiraConnectInstallation.for_project(project).flat_map do |installation| client = Atlassian::JiraConnect::Client.new(installation.base_url, installation.shared_secret) - response = client.store_dev_info(project: project, commits: commits, branches: branches, merge_requests: merge_requests, update_sequence_id: update_sequence_id) + responses = client.send_info(project: project, **args) - log_response(response) + responses.each { |r| log_response(r) } end end @@ -29,7 +31,7 @@ module JiraConnect jira_response: response&.to_json } - if response && response['errorMessages'] + if response && (response['errorMessages'] || response['rejectedBuilds'].present?) logger.error(message) else logger.info(message) diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index de0016f64d6..369a769328e 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -877,15 +877,23 @@ :tags: [] - :name: jira_connect:jira_connect_sync_branch :feature_category: :integrations - :has_external_dependencies: + :has_external_dependencies: true :urgency: :low :resource_boundary: :unknown :weight: 1 :idempotent: :tags: [] +- :name: jira_connect:jira_connect_sync_builds + :feature_category: :integrations + :has_external_dependencies: true + :urgency: :low + :resource_boundary: :unknown + :weight: 1 + :idempotent: true + :tags: [] - :name: jira_connect:jira_connect_sync_merge_request :feature_category: :integrations - :has_external_dependencies: + :has_external_dependencies: true :urgency: :low :resource_boundary: :unknown :weight: 1 diff --git a/app/workers/jira_connect/sync_branch_worker.rb b/app/workers/jira_connect/sync_branch_worker.rb index 4c1c987353d..d7e773b0861 100644 --- a/app/workers/jira_connect/sync_branch_worker.rb +++ b/app/workers/jira_connect/sync_branch_worker.rb @@ -7,6 +7,7 @@ module JiraConnect queue_namespace :jira_connect feature_category :integrations loggable_arguments 1, 2 + worker_has_external_dependencies! def perform(project_id, branch_name, commit_shas, update_sequence_id = nil) project = Project.find_by_id(project_id) diff --git a/app/workers/jira_connect/sync_builds_worker.rb b/app/workers/jira_connect/sync_builds_worker.rb new file mode 100644 index 00000000000..c1c749f6041 --- /dev/null +++ b/app/workers/jira_connect/sync_builds_worker.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module JiraConnect + class SyncBuildsWorker + include ApplicationWorker + + idempotent! + worker_has_external_dependencies! + + queue_namespace :jira_connect + feature_category :integrations + + def perform(pipeline_id, sequence_id) + pipeline = Ci::Pipeline.find_by_id(pipeline_id) + + return unless pipeline + return unless Feature.enabled?(:jira_sync_builds, pipeline.project) + + ::JiraConnect::SyncService + .new(pipeline.project) + .execute(pipelines: [pipeline], update_sequence_id: sequence_id) + end + end +end diff --git a/app/workers/jira_connect/sync_merge_request_worker.rb b/app/workers/jira_connect/sync_merge_request_worker.rb index f45ab38f35d..6ef426790b3 100644 --- a/app/workers/jira_connect/sync_merge_request_worker.rb +++ b/app/workers/jira_connect/sync_merge_request_worker.rb @@ -7,6 +7,8 @@ module JiraConnect queue_namespace :jira_connect feature_category :integrations + worker_has_external_dependencies! + def perform(merge_request_id, update_sequence_id = nil) merge_request = MergeRequest.find_by_id(merge_request_id) diff --git a/config/feature_flags/development/jira_sync_builds.yml b/config/feature_flags/development/jira_sync_builds.yml new file mode 100644 index 00000000000..8cb054b848d --- /dev/null +++ b/config/feature_flags/development/jira_sync_builds.yml @@ -0,0 +1,8 @@ +--- +name: jira_sync_builds +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49348 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/292013 +milestone: '13.7' +type: development +group: group::ecosystem +default_enabled: false diff --git a/lib/atlassian/jira_connect/client.rb b/lib/atlassian/jira_connect/client.rb index f81ed462174..da24d0e20ee 100644 --- a/lib/atlassian/jira_connect/client.rb +++ b/lib/atlassian/jira_connect/client.rb @@ -12,31 +12,68 @@ module Atlassian @shared_secret = shared_secret end + def send_info(project:, update_sequence_id: nil, **args) + common = { project: project, update_sequence_id: update_sequence_id } + dev_info = args.slice(:commits, :branches, :merge_requests) + build_info = args.slice(:pipelines) + + responses = [] + + responses << store_dev_info(**common, **dev_info) if dev_info.present? + responses << store_build_info(**common, **build_info) if build_info.present? + raise ArgumentError, 'Invalid arguments' if responses.empty? + + responses.compact + end + + private + + def store_build_info(project:, pipelines:, update_sequence_id: nil) + return unless Feature.enabled?(:jira_sync_builds, project) + + builds = pipelines.map do |pipeline| + build = Serializers::BuildEntity.represent( + pipeline, + update_sequence_id: update_sequence_id + ) + next if build.issue_keys.empty? + + build + end.compact + return if builds.empty? + + post('/rest/builds/0.1/bulk', { builds: builds }) + end + def store_dev_info(project:, commits: nil, branches: nil, merge_requests: nil, update_sequence_id: nil) - dev_info_json = { - repositories: [ - Serializers::RepositoryEntity.represent( - project, - commits: commits, - branches: branches, - merge_requests: merge_requests, - user_notes_count: user_notes_count(merge_requests), - update_sequence_id: update_sequence_id - ) - ] - }.to_json - - uri = URI.join(@base_uri, '/rest/devinfo/0.10/bulk') - - headers = { + repo = Serializers::RepositoryEntity.represent( + project, + commits: commits, + branches: branches, + merge_requests: merge_requests, + user_notes_count: user_notes_count(merge_requests), + update_sequence_id: update_sequence_id + ) + + post('/rest/devinfo/0.10/bulk', { repositories: [repo] }) + end + + def post(path, payload) + uri = URI.join(@base_uri, path) + + self.class.post(uri, headers: headers(uri), body: metadata.merge(payload).to_json) + end + + def headers(uri) + { 'Authorization' => "JWT #{jwt_token('POST', uri)}", 'Content-Type' => 'application/json' } - - self.class.post(uri, headers: headers, body: dev_info_json) end - private + def metadata + { providerMetadata: { product: "GitLab #{Gitlab::VERSION}" } } + end def user_notes_count(merge_requests) return unless merge_requests diff --git a/lib/atlassian/jira_connect/serializers/base_entity.rb b/lib/atlassian/jira_connect/serializers/base_entity.rb index 94deb174a45..640337c0399 100644 --- a/lib/atlassian/jira_connect/serializers/base_entity.rb +++ b/lib/atlassian/jira_connect/serializers/base_entity.rb @@ -11,6 +11,12 @@ module Atlassian expose :update_sequence_id, as: :updateSequenceId + def eql(other) + other.is_a?(self.class) && to_json == other.to_json + end + + alias_method :==, :eql + private def update_sequence_id diff --git a/lib/atlassian/jira_connect/serializers/build_entity.rb b/lib/atlassian/jira_connect/serializers/build_entity.rb new file mode 100644 index 00000000000..3eb8b1f1978 --- /dev/null +++ b/lib/atlassian/jira_connect/serializers/build_entity.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +module Atlassian + module JiraConnect + module Serializers + # A Jira 'build' represents what we call a 'pipeline' + class BuildEntity < Grape::Entity + include Gitlab::Routing + + format_with(:iso8601, &:iso8601) + + expose :schema_version, as: :schemaVersion + expose :pipeline_id, as: :pipelineId + expose :iid, as: :buildNumber + expose :update_sequence_id, as: :updateSequenceNumber + expose :source_ref, as: :displayName + expose :url + expose :state + expose :updated_at, as: :lastUpdated, format_with: :iso8601 + expose :issue_keys, as: :issueKeys + expose :test_info, as: :testInfo + expose :references + + def issue_keys + # extract Jira issue keys from either the source branch/ref or the + # merge request title. + @issue_keys ||= begin + src = "#{pipeline.source_ref} #{pipeline.merge_request&.title}" + JiraIssueKeyExtractor.new(src).issue_keys + end + end + + private + + alias_method :pipeline, :object + delegate :project, to: :object + + def url + project_pipeline_url(project, pipeline) + end + + # translate to Jira status + def state + case pipeline.status + when 'scheduled', 'created', 'pending', 'preparing', 'waiting_for_resource' then 'pending' + when 'running' then 'in_progress' + when 'success' then 'successful' + when 'failed' then 'failed' + when 'canceled', 'skipped' then 'cancelled' + else + 'unknown' + end + end + + def pipeline_id + pipeline.ensure_ci_ref! + + pipeline.ci_ref.id.to_s + end + + def schema_version + '1.0' + end + + def test_info + builds = pipeline.builds.pluck(:status) # rubocop: disable CodeReuse/ActiveRecord + n = builds.size + passed = builds.count { |s| s == 'success' } + failed = builds.count { |s| s == 'failed' } + + { + totalNumber: n, + numberPassed: passed, + numberFailed: failed, + numberSkipped: n - (passed + failed) + } + end + + def references + ref = pipeline.source_ref + + [{ + commit: { id: pipeline.sha, repositoryUri: project_url(project) }, + ref: { name: ref, uri: project_commits_url(project, ref) } + }] + end + + def update_sequence_id + options[:update_sequence_id] || Client.generate_update_sequence_id + end + end + end + end +end diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index 4da384ce61f..e69743122cc 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -24,6 +24,14 @@ FactoryBot.define do trait :with_diffs do end + trait :jira_title do + title { generate(:jira_title) } + end + + trait :jira_branch do + source_branch { generate(:jira_branch) } + end + trait :with_image_diffs do source_branch { "add_images_and_changes" } target_branch { "master" } diff --git a/spec/factories/sequences.rb b/spec/factories/sequences.rb index 7e2836d3688..b338fd99625 100644 --- a/spec/factories/sequences.rb +++ b/spec/factories/sequences.rb @@ -15,4 +15,6 @@ FactoryBot.define do sequence(:sha) { |n| Digest::SHA1.hexdigest("commit-like-#{n}") } sequence(:oid) { |n| Digest::SHA2.hexdigest("oid-like-#{n}") } sequence(:variable) { |n| "var#{n}" } + sequence(:jira_title) { |n| "[PROJ-#{n}]: fix bug" } + sequence(:jira_branch) { |n| "feature/PROJ-#{n}" } end diff --git a/spec/lib/atlassian/jira_connect/client_spec.rb b/spec/lib/atlassian/jira_connect/client_spec.rb index c260c6404f5..6a161854dfb 100644 --- a/spec/lib/atlassian/jira_connect/client_spec.rb +++ b/spec/lib/atlassian/jira_connect/client_spec.rb @@ -7,6 +7,8 @@ RSpec.describe Atlassian::JiraConnect::Client do subject { described_class.new('https://gitlab-test.atlassian.net', 'sample_secret') } + let_it_be(:project) { create_default(:project, :repository) } + around do |example| freeze_time { example.run } end @@ -19,41 +21,158 @@ RSpec.describe Atlassian::JiraConnect::Client do end end - describe '#store_dev_info' do - let_it_be(:project) { create_default(:project, :repository) } - let_it_be(:merge_requests) { create_list(:merge_request, 2, :unique_branches) } + describe '#send_info' do + it 'calls store_build_info and store_dev_info as appropriate' do + expect(subject).to receive(:store_build_info).with( + project: project, + update_sequence_id: :x, + pipelines: :y + ).and_return(:build_stored) + + expect(subject).to receive(:store_dev_info).with( + project: project, + update_sequence_id: :x, + commits: :a, + branches: :b, + merge_requests: :c + ).and_return(:dev_stored) + + args = { + project: project, + update_sequence_id: :x, + commits: :a, + branches: :b, + merge_requests: :c, + pipelines: :y + } + + expect(subject.send_info(**args)).to contain_exactly(:dev_stored, :build_stored) + end - let(:expected_jwt) do - Atlassian::Jwt.encode( - Atlassian::Jwt.build_claims( - Atlassian::JiraConnect.app_key, - '/rest/devinfo/0.10/bulk', - 'POST' - ), - 'sample_secret' - ) + it 'only calls methods that we need to call' do + expect(subject).to receive(:store_dev_info).with( + project: project, + update_sequence_id: :x, + commits: :a + ).and_return(:dev_stored) + + args = { + project: project, + update_sequence_id: :x, + commits: :a + } + + expect(subject.send_info(**args)).to contain_exactly(:dev_stored) + end + + it 'raises an argument error if there is nothing to send (probably a typo?)' do + expect { subject.send_info(project: project, builds: :x) } + .to raise_error(ArgumentError) + end + end + + def expected_headers(path) + expected_jwt = Atlassian::Jwt.encode( + Atlassian::Jwt.build_claims(Atlassian::JiraConnect.app_key, path, 'POST'), + 'sample_secret' + ) + + { + 'Authorization' => "JWT #{expected_jwt}", + 'Content-Type' => 'application/json' + } + end + + describe '#store_build_info' do + let_it_be(:mrs_by_title) { create_list(:merge_request, 4, :unique_branches, :jira_title) } + let_it_be(:mrs_by_branch) { create_list(:merge_request, 2, :jira_branch) } + let_it_be(:red_herrings) { create_list(:merge_request, 1, :unique_branches) } + + let_it_be(:pipelines) do + (red_herrings + mrs_by_branch + mrs_by_title).map do |mr| + create(:ci_pipeline, merge_request: mr) + end + end + + let(:build_info_payload_schema) do + Atlassian::Schemata.build_info_payload + end + + let(:body) do + matcher = be_valid_json.according_to_schema(build_info_payload_schema) + + ->(text) { matcher.matches?(text) } end before do - stub_full_request('https://gitlab-test.atlassian.net/rest/devinfo/0.10/bulk', method: :post) - .with( - headers: { - 'Authorization' => "JWT #{expected_jwt}", - 'Content-Type' => 'application/json' - } - ) + path = '/rest/builds/0.1/bulk' + stub_full_request('https://gitlab-test.atlassian.net' + path, method: :post) + .with(body: body, headers: expected_headers(path)) + end + + it "calls the API with auth headers" do + subject.send(:store_build_info, project: project, pipelines: pipelines) + end + + it 'only sends information about relevant MRs' do + expect(subject).to receive(:post).with('/rest/builds/0.1/bulk', { builds: have_attributes(size: 6) }) + + subject.send(:store_build_info, project: project, pipelines: pipelines) + end + + it 'does not call the API if there is nothing to report' do + expect(subject).not_to receive(:post) + + subject.send(:store_build_info, project: project, pipelines: pipelines.take(1)) + end + + it 'does not call the API if the feature flag is not enabled' do + stub_feature_flags(jira_sync_builds: false) + + expect(subject).not_to receive(:post) + + subject.send(:store_build_info, project: project, pipelines: pipelines) + end + + it 'does call the API if the feature flag enabled for the project' do + stub_feature_flags(jira_sync_builds: project) + + expect(subject).to receive(:post).with('/rest/builds/0.1/bulk', { builds: Array }) + + subject.send(:store_build_info, project: project, pipelines: pipelines) + end + + it 'avoids N+1 database queries' do + baseline = ActiveRecord::QueryRecorder.new do + subject.send(:store_build_info, project: project, pipelines: pipelines) + end + + pipelines << create(:ci_pipeline, head_pipeline_of: create(:merge_request, :jira_branch)) + + expect { subject.send(:store_build_info, project: project, pipelines: pipelines) }.not_to exceed_query_limit(baseline) + end + end + + describe '#store_dev_info' do + let_it_be(:merge_requests) { create_list(:merge_request, 2, :unique_branches) } + + before do + path = '/rest/devinfo/0.10/bulk' + + stub_full_request('https://gitlab-test.atlassian.net' + path, method: :post) + .with(headers: expected_headers(path)) end it "calls the API with auth headers" do - subject.store_dev_info(project: project) + subject.send(:store_dev_info, project: project) end it 'avoids N+1 database queries' do - control_count = ActiveRecord::QueryRecorder.new { subject.store_dev_info(project: project, merge_requests: merge_requests) }.count + control_count = ActiveRecord::QueryRecorder.new { subject.send(:store_dev_info, project: project, merge_requests: merge_requests) }.count merge_requests << create(:merge_request, :unique_branches) - expect { subject.store_dev_info(project: project, merge_requests: merge_requests) }.not_to exceed_query_limit(control_count) + expect { subject.send(:store_dev_info, project: project, merge_requests: merge_requests) }.not_to exceed_query_limit(control_count) end end end diff --git a/spec/lib/atlassian/jira_connect/serializers/build_entity_spec.rb b/spec/lib/atlassian/jira_connect/serializers/build_entity_spec.rb new file mode 100644 index 00000000000..52e475d20ca --- /dev/null +++ b/spec/lib/atlassian/jira_connect/serializers/build_entity_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Atlassian::JiraConnect::Serializers::BuildEntity do + let_it_be(:user) { create_default(:user) } + let_it_be(:project) { create_default(:project) } + + subject { described_class.represent(pipeline) } + + context 'when the pipeline does not belong to any Jira issue' do + let_it_be(:pipeline) { create(:ci_pipeline) } + + describe '#issue_keys' do + it 'is empty' do + expect(subject.issue_keys).to be_empty + end + end + + describe '#to_json' do + it 'can encode the object' do + expect(subject.to_json).to be_valid_json + end + + it 'is invalid, since it has no issue keys' do + expect(subject.to_json).not_to be_valid_json.according_to_schema(Atlassian::Schemata.build_info) + end + end + end + + context 'when the pipeline does belong to a Jira issue' do + let(:pipeline) { create(:ci_pipeline, merge_request: merge_request) } + + %i[jira_branch jira_title].each do |trait| + context "because it belongs to an MR with a #{trait}" do + let(:merge_request) { create(:merge_request, trait) } + + describe '#issue_keys' do + it 'is not empty' do + expect(subject.issue_keys).not_to be_empty + end + end + + describe '#to_json' do + it 'is valid according to the build info schema' do + expect(subject.to_json).to be_valid_json.according_to_schema(Atlassian::Schemata.build_info) + end + end + end + end + end +end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 7c6b9b2e671..472fbb67975 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1206,6 +1206,40 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe 'synching status to Jira' do + let(:worker) { ::JiraConnect::SyncBuildsWorker } + + %i[prepare! run! skip! drop! succeed! cancel! block! delay!].each do |event| + context "when we call pipeline.#{event}" do + it 'triggers a Jira synch worker' do + expect(worker).to receive(:perform_async).with(pipeline.id, Integer) + + pipeline.send(event) + end + + context 'the feature is disabled' do + it 'does not trigger a worker' do + stub_feature_flags(jira_sync_builds: false) + + expect(worker).not_to receive(:perform_async) + + pipeline.send(event) + end + end + + context 'the feature is enabled for this project' do + it 'does trigger a worker' do + stub_feature_flags(jira_sync_builds: pipeline.project) + + expect(worker).to receive(:perform_async) + + pipeline.send(event) + end + end + end + end + end + describe '#duration', :sidekiq_inline do context 'when multiple builds are finished' do before do diff --git a/spec/services/jira_connect/sync_service_spec.rb b/spec/services/jira_connect/sync_service_spec.rb index 83088bb2e79..4b434348146 100644 --- a/spec/services/jira_connect/sync_service_spec.rb +++ b/spec/services/jira_connect/sync_service_spec.rb @@ -3,30 +3,23 @@ require 'spec_helper' RSpec.describe JiraConnect::SyncService do + include AfterNextHelpers + describe '#execute' do let_it_be(:project) { create(:project, :repository) } - let(:branches) { [project.repository.find_branch('master')] } - let(:commits) { project.commits_by(oids: %w[b83d6e3 5a62481]) } - let(:merge_requests) { [create(:merge_request, source_project: project, target_project: project)] } + let(:client) { Atlassian::JiraConnect::Client } + let(:info) { { a: 'Some', b: 'Info' } } subject do - described_class.new(project).execute(commits: commits, branches: branches, merge_requests: merge_requests) + described_class.new(project).execute(**info) end before do create(:jira_connect_subscription, namespace: project.namespace) end - def expect_jira_client_call(return_value = { 'status': 'success' }) - expect_next_instance_of(Atlassian::JiraConnect::Client) do |instance| - expect(instance).to receive(:store_dev_info).with( - project: project, - commits: commits, - branches: [instance_of(Gitlab::Git::Branch)], - merge_requests: merge_requests, - update_sequence_id: anything - ).and_return(return_value) - end + def store_info(return_values = [{ 'status': 'success' }]) + receive(:send_info).with(project: project, **info).and_return(return_values) end def expect_log(type, message) @@ -41,20 +34,22 @@ RSpec.describe JiraConnect::SyncService do end it 'calls Atlassian::JiraConnect::Client#store_dev_info and logs the response' do - expect_jira_client_call + expect_next(client).to store_info expect_log(:info, { 'status': 'success' }) subject end - context 'when request returns an error' do + context 'when a request returns an error' do it 'logs the response as an error' do - expect_jira_client_call({ - 'errorMessages' => ['some error message'] - }) + expect_next(client).to store_info([ + { 'errorMessages' => ['some error message'] }, + { 'rejectedBuilds' => ['x'] } + ]) expect_log(:error, { 'errorMessages' => ['some error message'] }) + expect_log(:error, { 'rejectedBuilds' => ['x'] }) subject end diff --git a/spec/support/atlassian/jira_connect/schemata.rb b/spec/support/atlassian/jira_connect/schemata.rb new file mode 100644 index 00000000000..91f8fe0bb41 --- /dev/null +++ b/spec/support/atlassian/jira_connect/schemata.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module Atlassian + module Schemata + def self.build_info + { + 'type' => 'object', + 'required' => %w(schemaVersion pipelineId buildNumber updateSequenceNumber displayName url state issueKeys testInfo references), + 'properties' => { + 'schemaVersion' => { 'type' => 'string', 'pattern' => '1.0' }, + 'pipelineId' => { 'type' => 'string' }, + 'buildNumber' => { 'type' => 'integer' }, + 'updateSequenceNumber' => { 'type' => 'integer' }, + 'displayName' => { 'type' => 'string' }, + 'url' => { 'type' => 'string' }, + 'state' => { + 'type' => 'string', + 'pattern' => '(pending|in_progress|successful|failed|cancelled)' + }, + 'issueKeys' => { + 'type' => 'array', + 'items' => { 'type' => 'string' }, + 'minItems' => 1 + }, + 'testInfo' => { + 'type' => 'object', + 'required' => %w(totalNumber numberPassed numberFailed numberSkipped), + 'properties' => { + 'totalNumber' => { 'type' => 'integer' }, + 'numberFailed' => { 'type' => 'integer' }, + 'numberPassed' => { 'type' => 'integer' }, + 'numberSkipped' => { 'type' => 'integer' } + } + }, + 'references' => { + 'type' => 'array', + 'items' => { + 'type' => 'object', + 'required' => %w(commit ref), + 'properties' => { + 'commit' => { + 'type' => 'object', + 'required' => %w(id repositoryUri), + 'properties' => { + 'id' => { 'type' => 'string' }, + 'repositoryUri' => { 'type' => 'string' } + } + }, + 'ref' => { + 'type' => 'object', + 'required' => %w(name uri), + 'properties' => { + 'name' => { 'type' => 'string' }, + 'uri' => { 'type' => 'string' } + } + } + } + } + } + } + } + end + + def self.build_info_payload + { + 'type' => 'object', + 'required' => %w(providerMetadata builds), + 'properties' => { + 'providerMetadata' => provider_metadata, + 'builds' => { 'type' => 'array', 'items' => build_info } + } + } + end + + def self.provider_metadata + { + 'type' => 'object', + 'required' => %w(product), + 'properties' => { 'product' => { 'type' => 'string' } } + } + end + end +end diff --git a/spec/support/helpers/after_next_helpers.rb b/spec/support/helpers/after_next_helpers.rb index a681a6b2393..0a7844fdd8f 100644 --- a/spec/support/helpers/after_next_helpers.rb +++ b/spec/support/helpers/after_next_helpers.rb @@ -30,7 +30,11 @@ module AfterNextHelpers msg = asserted ? :to : :not_to case level when :expect - expect_next_instance_of(klass, *args) { |instance| expect(instance).send(msg, condition) } + if asserted + expect_next_instance_of(klass, *args) { |instance| expect(instance).send(msg, condition) } + else + allow_next_instance_of(klass, *args) { |instance| expect(instance).send(msg, condition) } + end when :allow allow_next_instance_of(klass, *args) { |instance| allow(instance).send(msg, condition) } else diff --git a/spec/support/helpers/next_instance_of.rb b/spec/support/helpers/next_instance_of.rb index 7e10ff36887..a8e9ab2bafe 100644 --- a/spec/support/helpers/next_instance_of.rb +++ b/spec/support/helpers/next_instance_of.rb @@ -2,17 +2,26 @@ module NextInstanceOf def expect_next_instance_of(klass, *new_args, &blk) - stub_new(expect(klass), *new_args, &blk) + stub_new(expect(klass), nil, *new_args, &blk) + end + + def expect_next_instances_of(klass, number, *new_args, &blk) + stub_new(expect(klass), number, *new_args, &blk) end def allow_next_instance_of(klass, *new_args, &blk) - stub_new(allow(klass), *new_args, &blk) + stub_new(allow(klass), nil, *new_args, &blk) + end + + def allow_next_instances_of(klass, number, *new_args, &blk) + stub_new(allow(klass), number, *new_args, &blk) end private - def stub_new(target, *new_args, &blk) + def stub_new(target, number, *new_args, &blk) receive_new = receive(:new) + receive_new.exactly(number).times if number receive_new.with(*new_args) if new_args.any? target.to receive_new.and_wrap_original do |method, *original_args| diff --git a/spec/support/matchers/be_valid_json.rb b/spec/support/matchers/be_valid_json.rb new file mode 100644 index 00000000000..f46c35c7198 --- /dev/null +++ b/spec/support/matchers/be_valid_json.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +RSpec::Matchers.define :be_valid_json do + def according_to_schema(schema) + @schema = schema + self + end + + match do |actual| + data = Gitlab::Json.parse(actual) + + if @schema.present? + @validation_errors = JSON::Validator.fully_validate(@schema, data) + @validation_errors.empty? + else + data.present? + end + rescue JSON::ParserError => e + @error = e + false + end + + def failure_message + if @error + "Parse failed with error: #{@error}" + elsif @validation_errors.present? + "Validation failed because #{@validation_errors.join(', and ')}" + else + "Parsing did not return any data" + end + end +end diff --git a/spec/workers/jira_connect/sync_branch_worker_spec.rb b/spec/workers/jira_connect/sync_branch_worker_spec.rb index 4aa2f89de7b..c8453064b0d 100644 --- a/spec/workers/jira_connect/sync_branch_worker_spec.rb +++ b/spec/workers/jira_connect/sync_branch_worker_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe JiraConnect::SyncBranchWorker do + include AfterNextHelpers + describe '#perform' do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :repository, group: group) } @@ -67,7 +69,7 @@ RSpec.describe JiraConnect::SyncBranchWorker do context 'with update_sequence_id' do let(:update_sequence_id) { 1 } - let(:request_url) { 'https://sample.atlassian.net/rest/devinfo/0.10/bulk' } + let(:request_path) { '/rest/devinfo/0.10/bulk' } let(:request_body) do { repositories: [ @@ -78,14 +80,13 @@ RSpec.describe JiraConnect::SyncBranchWorker do update_sequence_id: update_sequence_id ) ] - }.to_json + } end subject { described_class.new.perform(project_id, branch_name, commit_shas, update_sequence_id) } it 'sends the reqeust with custom update_sequence_id' do - expect(Atlassian::JiraConnect::Client).to receive(:post) - .with(URI(request_url), headers: anything, body: request_body) + expect_next(Atlassian::JiraConnect::Client).to receive(:post).with(request_path, request_body) subject end diff --git a/spec/workers/jira_connect/sync_builds_worker_spec.rb b/spec/workers/jira_connect/sync_builds_worker_spec.rb new file mode 100644 index 00000000000..7c58803d778 --- /dev/null +++ b/spec/workers/jira_connect/sync_builds_worker_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::JiraConnect::SyncBuildsWorker do + include AfterNextHelpers + include ServicesHelper + + describe '#perform' do + let_it_be(:pipeline) { create(:ci_pipeline) } + + let(:sequence_id) { Random.random_number(1..10_000) } + let(:pipeline_id) { pipeline.id } + + subject { described_class.new.perform(pipeline_id, sequence_id) } + + context 'when pipeline exists' do + it 'calls the Jira sync service' do + expect_next(::JiraConnect::SyncService, pipeline.project) + .to receive(:execute).with(pipelines: contain_exactly(pipeline), update_sequence_id: sequence_id) + + subject + end + end + + context 'when pipeline does not exist' do + let(:pipeline_id) { non_existing_record_id } + + it 'does not call the sync service' do + expect_next(::JiraConnect::SyncService).not_to receive(:execute) + + subject + end + end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(jira_sync_builds: false) + end + + it 'does not call the sync service' do + expect_next(::JiraConnect::SyncService).not_to receive(:execute) + + subject + end + end + + context 'when the feature flag is enabled for this project' do + before do + stub_feature_flags(jira_sync_builds: pipeline.project) + end + + it 'calls the sync service' do + expect_next(::JiraConnect::SyncService).to receive(:execute) + + subject + end + end + end +end diff --git a/spec/workers/jira_connect/sync_merge_request_worker_spec.rb b/spec/workers/jira_connect/sync_merge_request_worker_spec.rb index b3c0db4f260..1a40aa2b3ad 100644 --- a/spec/workers/jira_connect/sync_merge_request_worker_spec.rb +++ b/spec/workers/jira_connect/sync_merge_request_worker_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe JiraConnect::SyncMergeRequestWorker do + include AfterNextHelpers + describe '#perform' do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :repository, group: group) } @@ -33,7 +35,7 @@ RSpec.describe JiraConnect::SyncMergeRequestWorker do context 'with update_sequence_id' do let(:update_sequence_id) { 1 } - let(:request_url) { 'https://sample.atlassian.net/rest/devinfo/0.10/bulk' } + let(:request_path) { '/rest/devinfo/0.10/bulk' } let(:request_body) do { repositories: [ @@ -43,14 +45,13 @@ RSpec.describe JiraConnect::SyncMergeRequestWorker do update_sequence_id: update_sequence_id ) ] - }.to_json + } end subject { described_class.new.perform(merge_request_id, update_sequence_id) } it 'sends the request with custom update_sequence_id' do - expect(Atlassian::JiraConnect::Client).to receive(:post) - .with(URI(request_url), headers: anything, body: request_body) + expect_next(Atlassian::JiraConnect::Client).to receive(:post).with(request_path, request_body) subject end diff --git a/spec/workers/jira_connect/sync_project_worker_spec.rb b/spec/workers/jira_connect/sync_project_worker_spec.rb index 25210de828c..f7fa565d534 100644 --- a/spec/workers/jira_connect/sync_project_worker_spec.rb +++ b/spec/workers/jira_connect/sync_project_worker_spec.rb @@ -36,7 +36,7 @@ RSpec.describe JiraConnect::SyncProjectWorker, factory_default: :keep do end it_behaves_like 'an idempotent worker' do - let(:request_url) { 'https://sample.atlassian.net/rest/devinfo/0.10/bulk' } + let(:request_path) { '/rest/devinfo/0.10/bulk' } let(:request_body) do { repositories: [ @@ -46,13 +46,13 @@ RSpec.describe JiraConnect::SyncProjectWorker, factory_default: :keep do update_sequence_id: update_sequence_id ) ] - }.to_json + } end it 'sends the request with custom update_sequence_id' do - expect(Atlassian::JiraConnect::Client).to receive(:post) - .exactly(IdempotentWorkerHelper::WORKER_EXEC_TIMES).times - .with(URI(request_url), headers: anything, body: request_body) + allow_next_instances_of(Atlassian::JiraConnect::Client, IdempotentWorkerHelper::WORKER_EXEC_TIMES) do |client| + expect(client).to receive(:post).with(request_path, request_body) + end subject end -- 2.30.9