Propagate integration to group descendants

After propagate group-level integrations
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44023, we missed
one scenario where the integration could inherit from another
integration from the ancestors, not the closest one, this is something
that we also fixed finding the closest integration
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45022.

Example: Having a group, a subgroup, and a project belonging to the
subgroup. If we update the group, it propagates the settings to the
subgroup and project (both having inherit_from_id = group.id (pointing
to the group) but if after that, we update the settings of the subgroup,
it doesn't propagate them to the project. This is because the project's
inherit_from_id is still pointing to the group id from the first
propagation.

This always updates the integrations of any descendants for a group
integration.

app/models/service.rb

@@ -274,6 +274,17 @@ class Service < ApplicationRecord
     end
   end
 
+  def self.inherited_descendant_integrations_for(integration)
+    inherit_from_ids =
+      where(type: integration.type, group: integration.group.self_and_ancestors)
+        .or(where(type: integration.type, instance: true)).select(:id)
+
+    from_union([
+      where(type: integration.type, inherit_from_id: inherit_from_ids, group: integration.group.descendants),
+      where(type: integration.type, inherit_from_id: inherit_from_ids, project: Project.in_namespace(integration.group.self_and_descendants))
+    ])
+  end
+
   def activated?
     active
   end

app/services/admin/propagate_integration_service.rb

@@ -5,12 +5,12 @@ module Admin
     include PropagateService
 
     def propagate
-      update_inherited_integrations
-
       if integration.instance?
+        update_inherited_integrations
         create_integration_for_groups_without_integration if Feature.enabled?(:group_level_integrations)
         create_integration_for_projects_without_integration
       else
+        update_inherited_descendant_integrations
         create_integration_for_groups_without_integration_belonging_to_group
         create_integration_for_projects_without_integration_belonging_to_group
       end
@@ -25,6 +25,13 @@ module Admin
         PropagateIntegrationInheritWorker.perform_async(integration.id, min_id, max_id)
       end
     end
+
+    def update_inherited_descendant_integrations
+      Service.inherited_descendant_integrations_for(integration).in_batches(of: BATCH_SIZE) do |services|
+        min_id, max_id = services.pick("MIN(services.id), MAX(services.id)")
+        PropagateIntegrationInheritDescendantWorker.perform_async(integration.id, min_id, max_id)
+      end
+    end
     # rubocop: enable Cop/InBatches
 
     def create_integration_for_groups_without_integration

app/services/bulk_create_integration_service.rb

@@ -47,7 +47,7 @@ class BulkCreateIntegrationService
     if integration.template?
       integration.to_service_hash
     else
-      integration.to_service_hash.tap { |json| json['inherit_from_id'] = integration.id }
+      integration.to_service_hash.tap { |json| json['inherit_from_id'] = integration.inherit_from_id || integration.id }
     end
   end
 

app/services/bulk_update_integration_service.rb

@@ -23,7 +23,7 @@ class BulkUpdateIntegrationService
   attr_reader :integration, :batch
 
   def service_hash
-    integration.to_service_hash.tap { |json| json['inherit_from_id'] = integration.id }
+    integration.to_service_hash.tap { |json| json['inherit_from_id'] = integration.inherit_from_id || integration.id }
   end
 
   def data_fields_hash

app/workers/all_queues.yml

@@ -1839,6 +1839,14 @@
   :weight: 1
   :idempotent: true
   :tags: []
+- :name: propagate_integration_inherit_descendant
+  :feature_category: :integrations
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: propagate_integration_project
   :feature_category: :integrations
   :has_external_dependencies:

app/workers/propagate_integration_inherit_descendant_worker.rb

+# frozen_string_literal: true
+
+class PropagateIntegrationInheritDescendantWorker
+  include ApplicationWorker
+
+  feature_category :integrations
+  idempotent!
+
+  # rubocop: disable CodeReuse/ActiveRecord
+  def perform(integration_id, min_id, max_id)
+    integration = Service.find_by_id(integration_id)
+    return unless integration
+
+    services = Service.inherited_descendant_integrations_for(integration).where(id: min_id..max_id)
+
+    BulkUpdateIntegrationService.new(integration, services).execute
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
+end

config/sidekiq_queues.yml

@@ -252,6 +252,8 @@
   - 1
 - - propagate_integration_inherit
   - 1
+- - propagate_integration_inherit_descendant
+  - 1
 - - propagate_integration_project
   - 1
 - - propagate_service_template

spec/models/service_spec.rb

@@ -599,6 +599,23 @@ RSpec.describe Service do
     end
   end
 
+  describe '.inherited_descendant_integrations_for' do
+    let(:subgroup1) { create(:group, parent: group) }
+    let(:subgroup2) { create(:group, parent: group) }
+    let(:project1) { create(:project, group: subgroup1) }
+    let(:project2) { create(:project, group: subgroup2) }
+    let!(:group_integration) { create(:prometheus_service, group: group, project: nil) }
+    let!(:subgroup_integration1) { create(:prometheus_service, group: subgroup1, project: nil, inherit_from_id: group_integration.id) }
+    let!(:subgroup_integration2) { create(:prometheus_service, group: subgroup2, project: nil) }
+    let!(:project_integration1) { create(:prometheus_service, group: nil, project: project1, inherit_from_id: group_integration.id) }
+    let!(:project_integration2) { create(:prometheus_service, group: nil, project: project2, inherit_from_id: subgroup_integration2.id) }
+
+    it 'returns the groups and projects inheriting from integration ancestors' do
+      expect(described_class.inherited_descendant_integrations_for(group_integration)).to eq([subgroup_integration1, project_integration1])
+      expect(described_class.inherited_descendant_integrations_for(subgroup_integration2)).to eq([project_integration2])
+    end
+  end
+
   describe "{property}_changed?" do
     let(:service) do
       BambooService.create(

spec/services/admin/propagate_integration_service_spec.rb

@@ -67,7 +67,7 @@ RSpec.describe Admin::PropagateIntegrationService do
         end
       end
 
-      context 'with a group without integration' do
+      context 'with a subgroup without integration' do
         let(:subgroup) { create(:group, parent: group) }
 
         it 'calls to PropagateIntegrationGroupWorker' do
@@ -77,6 +77,18 @@ RSpec.describe Admin::PropagateIntegrationService do
           described_class.propagate(group_integration)
         end
       end
+
+      context 'with a subgroup with integration' do
+        let(:subgroup) { create(:group, parent: group) }
+        let(:subgroup_integration) { create(:jira_service, group: subgroup, project: nil, inherit_from_id: group_integration.id) }
+
+        it 'calls to PropagateIntegrationInheritDescendantWorker' do
+          expect(PropagateIntegrationInheritDescendantWorker).to receive(:perform_async)
+            .with(group_integration.id, subgroup_integration.id, subgroup_integration.id)
+
+          described_class.propagate(group_integration)
+        end
+      end
     end
   end
 end

spec/services/bulk_create_integration_service_spec.rb

@@ -37,7 +37,7 @@ RSpec.describe BulkCreateIntegrationService do
     it 'updates inherit_from_id attributes' do
       described_class.new(integration, batch, association).execute
 
-      expect(created_integration.reload.inherit_from_id).to eq(integration.id)
+      expect(created_integration.reload.inherit_from_id).to eq(inherit_from_id)
     end
   end
 
@@ -79,6 +79,7 @@ RSpec.describe BulkCreateIntegrationService do
 
   context 'with an instance-level integration' do
     let(:integration) { instance_integration }
+    let(:inherit_from_id) { integration.id }
 
     context 'with a project association' do
       let!(:project) { create(:project) }
@@ -107,6 +108,19 @@ RSpec.describe BulkCreateIntegrationService do
 
       it_behaves_like 'creates integration from batch ids'
       it_behaves_like 'updates inherit_from_id'
+
+      context 'with a subgroup association' do
+        let!(:group_integration) { create(:jira_service, group: group, project: nil, inherit_from_id: instance_integration.id) }
+        let!(:subgroup) { create(:group, parent: group) }
+        let(:integration) { group_integration }
+        let(:created_integration) { Service.find_by(group: subgroup) }
+        let(:batch) { Group.all }
+        let(:association) { 'group' }
+        let(:inherit_from_id) { instance_integration.id }
+
+        it_behaves_like 'creates integration from batch ids'
+        it_behaves_like 'updates inherit_from_id'
+      end
     end
   end
 
@@ -118,6 +132,7 @@ RSpec.describe BulkCreateIntegrationService do
       let(:created_integration) { project.jira_service }
       let(:batch) { Project.all }
       let(:association) { 'project' }
+      let(:inherit_from_id) { integration.id }
 
       it_behaves_like 'creates integration from batch ids'
       it_behaves_like 'updates project callbacks'

spec/services/bulk_update_integration_service_spec.rb

@@ -5,14 +5,28 @@ require 'spec_helper'
 RSpec.describe BulkUpdateIntegrationService do
   include JiraServiceHelper
 
-  before do
+  before_all do
     stub_jira_service_test
   end
 
   let(:excluded_attributes) { %w[id project_id group_id inherit_from_id instance template created_at updated_at] }
-  let!(:instance_integration) do
+
+  let_it_be(:group) { create(:group) }
+  let_it_be(:group_integration) do
+    JiraService.create!(
+      group: group,
+      active: true,
+      push_events: true,
+      url: 'http://update-jira.instance.com',
+      username: 'user',
+      password: 'secret'
+    )
+  end
+
+  let_it_be(:subgroup_integration) do
     JiraService.create!(
-      instance: true,
+      inherit_from_id: group_integration.id,
+      group: create(:group, parent: group),
       active: true,
       push_events: true,
       url: 'http://update-jira.instance.com',
@@ -21,10 +35,9 @@ RSpec.describe BulkUpdateIntegrationService do
     )
   end
 
-  let!(:integration) do
+  let_it_be(:integration) do
     JiraService.create!(
       project: create(:project),
-      inherit_from_id: instance_integration.id,
       instance: false,
       active: true,
       push_events: false,
@@ -36,21 +49,21 @@ RSpec.describe BulkUpdateIntegrationService do
 
   context 'with inherited integration' do
     it 'updates the integration' do
-      described_class.new(instance_integration, Service.inherit_from_id(instance_integration.id)).execute
+      described_class.new(subgroup_integration, Service.where.not(project: nil)).execute
 
-      expect(integration.reload.inherit_from_id).to eq(instance_integration.id)
+      expect(integration.reload.inherit_from_id).to eq(group_integration.id)
       expect(integration.attributes.except(*excluded_attributes))
-        .to eq(instance_integration.attributes.except(*excluded_attributes))
+        .to eq(subgroup_integration.attributes.except(*excluded_attributes))
     end
 
     context 'with integration with data fields' do
       let(:excluded_attributes) { %w[id service_id created_at updated_at] }
 
       it 'updates the data fields from the integration' do
-        described_class.new(instance_integration, Service.inherit_from_id(instance_integration.id)).execute
+        described_class.new(subgroup_integration, Service.where.not(project: nil)).execute
 
         expect(integration.reload.data_fields.attributes.except(*excluded_attributes))
-          .to eq(instance_integration.data_fields.attributes.except(*excluded_attributes))
+          .to eq(subgroup_integration.data_fields.attributes.except(*excluded_attributes))
       end
     end
   end

spec/workers/propagate_integration_inherit_descendant_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe PropagateIntegrationInheritDescendantWorker do
+  let_it_be(:group) { create(:group) }
+  let_it_be(:subgroup) { create(:group, parent: group) }
+  let_it_be(:group_integration) { create(:redmine_service, group: group, project: nil) }
+  let_it_be(:subgroup_integration) { create(:redmine_service, group: subgroup, project: nil, inherit_from_id: group_integration.id) }
+
+  it_behaves_like 'an idempotent worker' do
+    let(:job_args) { [group_integration.id, subgroup_integration.id, subgroup_integration.id] }
+
+    it 'calls to BulkUpdateIntegrationService' do
+      expect(BulkUpdateIntegrationService).to receive(:new)
+        .with(group_integration, match_array(subgroup_integration)).twice
+        .and_return(double(execute: nil))
+
+      subject
+    end
+  end
+
+  context 'with an invalid integration id' do
+    it 'returns without failure' do
+      expect(BulkUpdateIntegrationService).not_to receive(:new)
+
+      subject.perform(0, subgroup_integration.id, subgroup_integration.id)
+    end
+  end
+end

spec/workers/propagate_integration_inherit_worker_spec.rb

@@ -12,7 +12,7 @@ RSpec.describe PropagateIntegrationInheritWorker do
     it_behaves_like 'an idempotent worker' do
       let(:job_args) { [integration.id, integration1.id, integration3.id] }
 
-      it 'calls to BulkCreateIntegrationService' do
+      it 'calls to BulkUpdateIntegrationService' do
         expect(BulkUpdateIntegrationService).to receive(:new)
           .with(integration, match_array(integration1)).twice
           .and_return(double(execute: nil))