Merge branch '330818-services-api-return-404-for-unsaved-services' into 'master'

Fix services API returning nonexistent services

See merge request gitlab-org/gitlab!61646

changelogs/unreleased/330818-services-api-return-404-for-unsaved-services.yml

+---
+title: Fix services API returning non-existing services
+merge_request: 61646
+author:
+type: fixed

lib/api/services.rb

@@ -125,8 +125,11 @@ module API
         requires :service_slug, type: String, values: SERVICES.keys, desc: 'The name of the service'
       end
       get ":id/services/:service_slug" do
-        service = user_project.find_or_initialize_service(params[:service_slug].underscore)
-        present service, with: Entities::ProjectService
+        integration = user_project.find_or_initialize_service(params[:service_slug].underscore)
+
+        not_found!('Service') unless integration&.persisted?
+
+        present integration, with: Entities::ProjectService
       end
     end
 

spec/requests/api/services_spec.rb

@@ -114,21 +114,61 @@ RSpec.describe API::Services do
     describe "GET /projects/:id/services/#{service.dasherize}" do
       include_context service
 
-      # inject some properties into the service
-      let!(:initialized_service) { initialize_service(service) }
+      let!(:initialized_service) { initialize_service(service, active: true) }
+
+      let_it_be(:project2) do
+        create(:project, creator_id: user.id, namespace: user.namespace)
+      end
+
+      def deactive_service!
+        return initialized_service.update!(active: false) unless initialized_service.is_a?(PrometheusService)
+
+        # PrometheusService sets `#active` itself within a `before_save`:
+        initialized_service.manual_configuration = false
+        initialized_service.save!
+      end
 
       it 'returns authentication error when unauthenticated' do
         get api("/projects/#{project.id}/services/#{dashed_service}")
         expect(response).to have_gitlab_http_status(:unauthorized)
       end
 
-      it "returns all properties of service #{service}" do
+      it "returns all properties of active service #{service}" do
+        get api("/projects/#{project.id}/services/#{dashed_service}", user)
+
+        expect(initialized_service).to be_active
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['properties'].keys).to match_array(service_instance.api_field_names)
+      end
+
+      it "returns all properties of inactive service #{service}" do
+        deactive_service!
+
         get api("/projects/#{project.id}/services/#{dashed_service}", user)
 
+        expect(initialized_service).not_to be_active
         expect(response).to have_gitlab_http_status(:ok)
         expect(json_response['properties'].keys).to match_array(service_instance.api_field_names)
       end
 
+      it "returns not found if service does not exist" do
+        get api("/projects/#{project2.id}/services/#{dashed_service}", user)
+
+        expect(response).to have_gitlab_http_status(:not_found)
+        expect(json_response['message']).to eq('404 Service Not Found')
+      end
+
+      it "returns not found if service exists but is in `Project#disabled_services`" do
+        expect_next_found_instance_of(Project) do |project|
+          expect(project).to receive(:disabled_services).at_least(:once).and_return([service])
+        end
+
+        get api("/projects/#{project.id}/services/#{dashed_service}", user)
+
+        expect(response).to have_gitlab_http_status(:not_found)
+        expect(json_response['message']).to eq('404 Service Not Found')
+      end
+
       it "returns error when authenticated but not a project owner" do
         project.add_developer(user2)
         get api("/projects/#{project.id}/services/#{dashed_service}", user2)

spec/support/shared_contexts/services_shared_context.rb

@@ -49,8 +49,9 @@ Integration.available_services_names.each do |service|
       stub_jira_service_test if service == 'jira'
     end
 
-    def initialize_service(service)
+    def initialize_service(service, attrs = {})
       service_item = project.find_or_initialize_service(service)
+      service_item.attributes = attrs
       service_item.properties = service_attrs
       service_item.save!
       service_item