Determine Jira deployment_type based on URL

If API-based methods fail when determining the deployment_type we should
use the URL. Since Jira Cloud does not support custom domains yet we
can pretty much guess whether it's Jira Cloud or Jira Server based on
the domain. If it ends with atlassian.net then we're dealing with a Jira
Cloud, otherwise it's Jira Server.

Changelog: fixed

app/models/integrations/jira.rb

@@ -10,12 +10,7 @@ module Integrations
     include Gitlab::Utils::StrongMemoize
 
     PROJECTS_PER_PAGE = 50
-
-    # TODO: use jira_service.deployment_type enum when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37003 is merged
-    DEPLOYMENT_TYPES = {
-      server: 'SERVER',
-      cloud: 'CLOUD'
-    }.freeze
+    JIRA_CLOUD_HOST = '.atlassian.net'
 
     validates :url, public_url: true, presence: true, if: :activated?
     validates :api_url, public_url: true, allow_blank: true
@@ -517,15 +512,38 @@ module Integrations
     def update_deployment_type
       clear_memoization(:server_info) # ensure we run the request when we try to update deployment type
       results = server_info
-      return data_fields.deployment_unknown! unless results.present?
 
-      case results['deploymentType']
-      when 'Server'
+      unless results.present?
+        Gitlab::AppLogger.warn(message: "Jira API returned no ServerInfo, setting deployment_type from URL", server_info: results, url: client_url)
+
+        return set_deployment_type_from_url
+      end
+
+      if jira_cloud?
+        data_fields.deployment_cloud!
+      else
         data_fields.deployment_server!
-      when 'Cloud'
+      end
+    end
+
+    def jira_cloud?
+      server_info['deploymentType'] == 'Cloud' || URI(client_url).hostname.end_with?(JIRA_CLOUD_HOST)
+    end
+
+    def set_deployment_type_from_url
+      # This shouldn't happen but of course it will happen when an integration is removed.
+      # Instead of deleting the integration we set all fields to null
+      # and mark it as inactive
+      return data_fields.deployment_unknown! unless client_url
+
+      # If API-based detection methods fail here then
+      # we can only assume it's either Cloud or Server
+      # based on the URL being *.atlassian.net
+
+      if URI(client_url).hostname.end_with?(JIRA_CLOUD_HOST)
         data_fields.deployment_cloud!
       else
-        data_fields.deployment_unknown!
+        data_fields.deployment_server!
       end
     end
 

app/services/jira_import/users_importer.rb

@@ -31,21 +31,10 @@ module JiraImport
       @users_mapper_service ||= user_mapper_service_factory
     end
 
-    def deployment_type
-      # TODO: use project.jira_service.deployment_type value when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37003 is merged
-      @deployment_type ||= client.ServerInfo.all.deploymentType
-    end
-
-    def client
-      @client ||= project.jira_service.client
-    end
-
     def user_mapper_service_factory
-      # TODO: use deployment_type enum from jira service when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37003 is merged
-      case deployment_type.upcase
-      when Integrations::Jira::DEPLOYMENT_TYPES[:server]
+      if project.jira_service.data_fields.deployment_server?
         ServerUsersMapperService.new(user, project, start_at)
-      when Integrations::Jira::DEPLOYMENT_TYPES[:cloud]
+      elsif project.jira_service.data_fields.deployment_cloud?
         CloudUsersMapperService.new(user, project, start_at)
       else
         raise ArgumentError

spec/models/integrations/jira_spec.rb

@@ -117,7 +117,8 @@ RSpec.describe Integrations::Jira do
     let(:params) do
       {
         project: project,
-        url: url, api_url: api_url,
+        url: url,
+        api_url: api_url,
         username: username, password: password,
         jira_issue_transition_id: transition_id
       }
@@ -141,9 +142,9 @@ RSpec.describe Integrations::Jira do
     end
 
     context 'when loading serverInfo' do
-      let!(:jira_service) { subject }
+      let(:jira_service) { subject }
 
-      context 'Cloud instance' do
+      context 'from a Cloud instance' do
         let(:server_info_results) { { 'deploymentType' => 'Cloud' } }
 
         it 'is detected' do
@@ -151,7 +152,7 @@ RSpec.describe Integrations::Jira do
         end
       end
 
-      context 'Server instance' do
+      context 'from a Server instance' do
         let(:server_info_results) { { 'deploymentType' => 'Server' } }
 
         it 'is detected' do
@@ -159,11 +160,45 @@ RSpec.describe Integrations::Jira do
         end
       end
 
-      context 'Unknown instance' do
+      context 'from an Unknown instance' do
         let(:server_info_results) { { 'deploymentType' => 'FutureCloud' } }
 
-        it 'is detected' do
-          expect(jira_service.jira_tracker_data.deployment_unknown?).to be_truthy
+        context 'and URL ends in .atlassian.net' do
+          let(:api_url) { 'http://example-api.atlassian.net' }
+
+          it 'deployment_type is set to cloud' do
+            expect(jira_service.jira_tracker_data.deployment_cloud?).to be_truthy
+          end
+        end
+
+        context 'and URL is something else' do
+          let(:api_url) { 'http://my-jira-api.someserver.com' }
+
+          it 'deployment_type is set to server' do
+            expect(jira_service.jira_tracker_data.deployment_server?).to be_truthy
+          end
+        end
+      end
+
+      context 'and no ServerInfo response is received' do
+        let(:server_info_results) { {} }
+
+        context 'and URL ends in .atlassian.net' do
+          let(:api_url) { 'http://example-api.atlassian.net' }
+
+          it 'deployment_type is set to cloud' do
+            expect(Gitlab::AppLogger).to receive(:warn).with(message: "Jira API returned no ServerInfo, setting deployment_type from URL", server_info: server_info_results, url: api_url)
+            expect(jira_service.jira_tracker_data.deployment_cloud?).to be_truthy
+          end
+        end
+
+        context 'and URL is something else' do
+          let(:api_url) { 'http://my-jira-api.someserver.com' }
+
+          it 'deployment_type is set to server' do
+            expect(Gitlab::AppLogger).to receive(:warn).with(message: "Jira API returned no ServerInfo, setting deployment_type from URL", server_info: server_info_results, url: api_url)
+            expect(jira_service.jira_tracker_data.deployment_server?).to be_truthy
+          end
         end
       end
     end
@@ -229,17 +264,31 @@ RSpec.describe Integrations::Jira do
         end
 
         context 'when updating the url, api_url, username, or password' do
-          it 'updates deployment type' do
-            service.update!(url: 'http://first.url')
-            service.jira_tracker_data.update!(deployment_type: 'server')
+          context 'when updating the integration' do
+            it 'updates deployment type' do
+              service.update!(url: 'http://first.url')
+              service.jira_tracker_data.update!(deployment_type: 'server')
 
-            expect(service.jira_tracker_data.deployment_server?).to be_truthy
+              expect(service.jira_tracker_data.deployment_server?).to be_truthy
 
-            service.update!(api_url: 'http://another.url')
-            service.jira_tracker_data.reload
+              service.update!(api_url: 'http://another.url')
+              service.jira_tracker_data.reload
 
-            expect(service.jira_tracker_data.deployment_cloud?).to be_truthy
-            expect(WebMock).to have_requested(:get, /serverInfo/).twice
+              expect(service.jira_tracker_data.deployment_cloud?).to be_truthy
+              expect(WebMock).to have_requested(:get, /serverInfo/).twice
+            end
+          end
+
+          context 'when removing the integration' do
+            let(:server_info_results) { {} }
+
+            it 'updates deployment type' do
+              service.update!(url: nil, api_url: nil, active: false)
+
+              service.jira_tracker_data.reload
+
+              expect(service.jira_tracker_data.deployment_unknown?).to be_truthy
+            end
           end
 
           it 'calls serverInfo for url' do

spec/services/jira_import/users_importer_spec.rb

@@ -43,39 +43,37 @@ RSpec.describe JiraImport::UsersImporter do
       end
     end
 
-    RSpec.shared_examples 'maps jira users to gitlab users' do
+    RSpec.shared_examples 'maps Jira users to GitLab users' do |users_mapper_service:|
       context 'when Jira import is configured correctly' do
-        let_it_be(:jira_service) { create(:jira_service, project: project, active: true) }
-        let(:client) { double }
+        let_it_be(:jira_service) { create(:jira_service, project: project, active: true, url: "http://jira.example.net") }
 
-        before do
-          expect(importer).to receive(:client).at_least(1).and_return(client)
-          allow(client).to receive_message_chain(:ServerInfo, :all, :deploymentType).and_return(deployment_type)
-        end
-
-        context 'when jira client raises an error' do
+        context 'when users mapper service raises an error' do
           let(:error) { Timeout::Error.new }
 
           it 'returns an error response' do
-            expect(client).to receive(:get).and_raise(error)
-            expect(Gitlab::ErrorTracking).to receive(:log_exception).with(error, project_id: project.id)
+            expect_next_instance_of(users_mapper_service) do |instance|
+              expect(instance).to receive(:execute).and_raise(error)
+            end
 
+            expect(Gitlab::ErrorTracking).to receive(:log_exception).with(error, project_id: project.id)
             expect(subject.error?).to be_truthy
             expect(subject.message).to include('There was an error when communicating to Jira')
           end
         end
 
-        context 'when jira client returns result' do
-          context 'when jira client returns an empty array' do
-            let(:jira_users) { [] }
-
+        context 'when users mapper service returns result' do
+          context 'when users mapper service returns an empty array' do
             it 'returns nil payload' do
+              expect_next_instance_of(users_mapper_service) do |instance|
+                expect(instance).to receive(:execute).and_return([])
+              end
+
               expect(subject.success?).to be_truthy
               expect(subject.payload).to be_empty
             end
           end
 
-          context 'when jira client returns an results' do
+          context 'when Jira client returns any users' do
             let_it_be(:project_member) { create(:user, email: 'sample@jira.com') }
             let_it_be(:group_member) { create(:user, name: 'user-name2') }
             let_it_be(:other_user) { create(:user) }
@@ -86,6 +84,10 @@ RSpec.describe JiraImport::UsersImporter do
             end
 
             it 'returns the mapped users' do
+              expect_next_instance_of(users_mapper_service) do |instance|
+                expect(instance).to receive(:execute).and_return(mapped_users)
+              end
+
               expect(subject.success?).to be_truthy
               expect(subject.payload).to eq(mapped_users)
             end
@@ -95,43 +97,23 @@ RSpec.describe JiraImport::UsersImporter do
     end
 
     context 'when Jira instance is of Server deployment type' do
-      let(:deployment_type) { 'Server' }
-      let(:url) { "/rest/api/2/user/search?username=''&maxResults=50&startAt=#{start_at}" }
-      let(:jira_users) do
-        [
-          { 'key' => 'acc1', 'name' => 'user-name1', 'emailAddress' => 'sample@jira.com' },
-          { 'key' => 'acc2', 'name' => 'user-name2' }
-        ]
-      end
-
       before do
-        allow_next_instance_of(JiraImport::ServerUsersMapperService) do |instance|
-          allow(instance).to receive(:client).and_return(client)
-          allow(client).to receive(:get).with(url).and_return(jira_users)
-        end
+        allow(project).to receive(:jira_service).and_return(jira_service)
+
+        jira_service.data_fields.deployment_server!
       end
 
-      it_behaves_like 'maps jira users to gitlab users'
+      it_behaves_like 'maps Jira users to GitLab users', users_mapper_service: JiraImport::ServerUsersMapperService
     end
 
-    context 'when Jira instance is of Cloud deploymet type' do
-      let(:deployment_type) { 'Cloud' }
-      let(:url) { "/rest/api/2/users?maxResults=50&startAt=#{start_at}" }
-      let(:jira_users) do
-        [
-          { 'accountId' => 'acc1', 'displayName' => 'user-name1', 'emailAddress' => 'sample@jira.com' },
-          { 'accountId' => 'acc2', 'displayName' => 'user-name2' }
-        ]
-      end
-
+    context 'when Jira instance is of Cloud deployment type' do
       before do
-        allow_next_instance_of(JiraImport::CloudUsersMapperService) do |instance|
-          allow(instance).to receive(:client).and_return(client)
-          allow(client).to receive(:get).with(url).and_return(jira_users)
-        end
+        allow(project).to receive(:jira_service).and_return(jira_service)
+
+        jira_service.data_fields.deployment_cloud!
       end
 
-      it_behaves_like 'maps jira users to gitlab users'
+      it_behaves_like 'maps Jira users to GitLab users', users_mapper_service: JiraImport::CloudUsersMapperService
     end
   end
 end