Merge branch '36385-encrypt-integration-properties-b' into 'master'

Move application to use Integration.encrypted_properties

See merge request gitlab-org/gitlab!80507

app/models/integration.rb

@@ -10,9 +10,11 @@ class Integration < ApplicationRecord
   include FromUnion
   include EachBatch
   include IgnorableColumns
+  extend ::Gitlab::Utils::Override
 
   ignore_column :template, remove_with: '15.0', remove_after: '2022-04-22'
   ignore_column :type, remove_with: '15.0', remove_after: '2022-04-22'
+  ignore_column :properties, remove_with: '15.1', remove_after: '2022-05-22'
 
   UnknownType = Class.new(StandardError)
 
@@ -47,10 +49,7 @@ class Integration < ApplicationRecord
 
   SECTION_TYPE_CONNECTION = 'connection'
 
-  serialize :properties, JSON # rubocop:disable Cop/ActiveRecordSerialize
-
-  attr_encrypted :encrypted_properties_tmp,
-                 attribute: :encrypted_properties,
+  attr_encrypted :properties,
                  mode: :per_attribute_iv,
                  key: Settings.attr_encrypted_db_key_base_32,
                  algorithm: 'aes-256-gcm',
@@ -59,6 +58,15 @@ class Integration < ApplicationRecord
                  encode: false,
                  encode_iv: false
 
+  # Handle assignment of props with symbol keys.
+  # To do this correctly, we need to call the method generated by attr_encrypted.
+  alias_method :attr_encrypted_props=, :properties=
+  private :attr_encrypted_props=
+
+  def properties=(props)
+    self.attr_encrypted_props = props&.with_indifferent_access&.freeze
+  end
+
   alias_attribute :type, :type_new
 
   default_value_for :active, false
@@ -77,8 +85,6 @@ class Integration < ApplicationRecord
   default_value_for :wiki_page_events, true
 
   after_initialize :initialize_properties
-  after_initialize :copy_properties_to_encrypted_properties
-  before_save :copy_properties_to_encrypted_properties
 
   after_commit :reset_updated_properties
 
@@ -165,16 +171,14 @@ class Integration < ApplicationRecord
       class_eval <<~RUBY, __FILE__, __LINE__ + 1
         unless method_defined?(arg)
           def #{arg}
-            properties['#{arg}']
+            properties['#{arg}'] if properties.present?
           end
         end
 
         def #{arg}=(value)
           self.properties ||= {}
-          self.encrypted_properties_tmp = properties
           updated_properties['#{arg}'] = #{arg} unless #{arg}_changed?
-          self.properties['#{arg}'] = value
-          self.encrypted_properties_tmp['#{arg}'] = value
+          self.properties = self.properties.merge('#{arg}' => value)
         end
 
         def #{arg}_changed?
@@ -195,11 +199,13 @@ class Integration < ApplicationRecord
   # Provide convenient boolean accessor methods for each serialized property.
   # Also keep track of updated properties in a similar way as ActiveModel::Dirty
   def self.boolean_accessor(*args)
-    self.prop_accessor(*args)
+    prop_accessor(*args)
 
     args.each do |arg|
       class_eval <<~RUBY, __FILE__, __LINE__ + 1
         def #{arg}
+          return if properties.blank?
+
           Gitlab::Utils.to_boolean(properties['#{arg}'])
         end
 
@@ -318,18 +324,31 @@ class Integration < ApplicationRecord
   def self.build_from_integration(integration, project_id: nil, group_id: nil)
     new_integration = integration.dup
 
-    if integration.supports_data_fields?
-      data_fields = integration.data_fields.dup
-      data_fields.integration = new_integration
-    end
-
     new_integration.instance = false
     new_integration.project_id = project_id
     new_integration.group_id = group_id
-    new_integration.inherit_from_id = integration.id if integration.instance_level? || integration.group_level?
+    new_integration.inherit_from_id = integration.id if integration.inheritable?
     new_integration
   end
 
+  # Duplicating an integration also duplicates the data fields. Duped records have different ciphertexts.
+  override :dup
+  def dup
+    new_integration = super
+    new_integration.assign_attributes(reencrypt_properties)
+
+    if supports_data_fields?
+      fields = data_fields.dup
+      fields.integration = new_integration
+    end
+
+    new_integration
+  end
+
+  def inheritable?
+    instance_level? || group_level?
+  end
+
   def self.instance_exists_for?(type)
     exists?(instance: true, type: type)
   end
@@ -402,13 +421,7 @@ class Integration < ApplicationRecord
   end
 
   def initialize_properties
-    self.properties = {} if has_attribute?(:properties) && properties.nil?
-  end
-
-  def copy_properties_to_encrypted_properties
-    self.encrypted_properties_tmp = properties
-  rescue ActiveModel::MissingAttributeError
-    # ignore - in a record built from using a restricted select list
+    self.properties = {} if has_attribute?(:encrypted_properties) && encrypted_properties.nil?
   end
 
   def title
@@ -445,21 +458,26 @@ class Integration < ApplicationRecord
     %w[active]
   end
 
+  # properties is always nil - ignore it.
+  override :attributes
+  def attributes
+    super.except('properties')
+  end
+
   # return a hash of columns => values suitable for passing to insert_all
   def to_integration_hash
     column = self.class.attribute_aliases.fetch('type', 'type')
-    copy_properties_to_encrypted_properties
 
-    as_json(except: %w[id instance project_id group_id encrypted_properties_tmp])
+    as_json(except: %w[id instance project_id group_id])
       .merge(column => type)
       .merge(reencrypt_properties)
   end
 
   def reencrypt_properties
     unless properties.nil? || properties.empty?
-      alg = self.class.encrypted_attributes[:encrypted_properties_tmp][:algorithm]
+      alg = self.class.encrypted_attributes[:properties][:algorithm]
       iv = generate_iv(alg)
-      ep = self.class.encrypt(:encrypted_properties_tmp, properties, { iv: iv })
+      ep = self.class.encrypt(:properties, properties, { iv: iv })
     end
 
     { 'encrypted_properties' => ep, 'encrypted_properties_iv' => iv }

app/models/integrations/base_chat_notification.rb

@@ -35,8 +35,9 @@ module Integrations
     validates :labels_to_be_notified_behavior, inclusion: { in: LABEL_NOTIFICATION_BEHAVIOURS }, allow_blank: true
 
     def initialize_properties
-      if properties.nil?
-        self.properties = {}
+      super
+
+      if properties.empty?
         self.notify_only_broken_pipelines = true
         self.branches_to_be_notified = "default"
         self.labels_to_be_notified_behavior = MATCH_ANY_LABEL

app/models/integrations/base_issue_tracker.rb

@@ -25,12 +25,15 @@ module Integrations
     def handle_properties
       # this has been moved from initialize_properties and should be improved
       # as part of https://gitlab.com/gitlab-org/gitlab/issues/29404
-      return unless properties
+      return unless properties.present?
+
+      safe_keys = data_fields.attributes.keys.grep_v(/encrypted/) - %w[id service_id created_at]
 
       @legacy_properties_data = properties.dup
-      data_values = properties.slice!('title', 'description')
+
+      data_values = properties.slice(*safe_keys)
       data_values.reject! { |key| data_fields.changed.include?(key) }
-      data_values.slice!(*data_fields.attributes.keys)
+
       data_fields.assign_attributes(data_values) if data_values.present?
 
       self.properties = {}
@@ -68,10 +71,6 @@ module Integrations
       issue_url(iid)
     end
 
-    def initialize_properties
-      {}
-    end
-
     # Initialize with default properties values
     def set_default_data
       return unless issues_tracker.present?

app/models/integrations/buildkite.rb

@@ -27,12 +27,12 @@ module Integrations
     end
 
     # Since SSL verification will always be enabled for Buildkite,
-    # we no longer needs to store the boolean.
+    # we no longer need to store the boolean.
     # This is a stub method to work with deprecated API param.
     # TODO: remove enable_ssl_verification after 14.0
     # https://gitlab.com/gitlab-org/gitlab/-/issues/222808
     def enable_ssl_verification=(_value)
-      self.properties.delete('enable_ssl_verification') # Remove unused key
+      self.properties = properties.except('enable_ssl_verification') # Remove unused key
     end
 
     override :hook_url

app/models/integrations/jira.rb

@@ -94,10 +94,6 @@ module Integrations
       !!URI(url).hostname&.end_with?(JIRA_CLOUD_HOST)
     end
 
-    def initialize_properties
-      {}
-    end
-
     def data_fields
       jira_tracker_data || self.build_jira_tracker_data
     end
@@ -106,7 +102,7 @@ module Integrations
       return unless reset_password?
 
       data_fields.password = nil
-      properties.delete('password') if properties
+      self.properties = properties.except('password')
     end
 
     def set_default_data

app/models/integrations/pipelines_email.rb

@@ -12,8 +12,9 @@ module Integrations
     validate :number_of_recipients_within_limit, if: :validate_recipients?
 
     def initialize_properties
-      if properties.nil?
-        self.properties = {}
+      super
+
+      if properties.blank?
         self.notify_only_broken_pipelines = true
         self.branches_to_be_notified = "default"
       elsif !self.notify_only_default_branch.nil?

app/models/integrations/prometheus.rb

@@ -32,12 +32,6 @@ module Integrations
     scope :preload_project, -> { preload(:project) }
     scope :with_clusters_with_cilium, -> { joins(project: [:clusters]).merge(Clusters::Cluster.with_available_cilium) }
 
-    def initialize_properties
-      if properties.nil?
-        self.properties = {}
-      end
-    end
-
     def show_active_box?
       false
     end

app/services/projects/operations/update_service.rb

@@ -112,8 +112,9 @@ module Projects
 
         integration = project.find_or_initialize_integration(::Integrations::Prometheus.to_param)
         integration.assign_attributes(attrs)
+        attrs = integration.to_integration_hash.except('created_at', 'updated_at')
 
-        { prometheus_integration_attributes: integration.attributes.except(*%w[id project_id created_at updated_at]) }
+        { prometheus_integration_attributes: attrs }
       end
 
       def incident_management_setting_params

db/post_migrate/20220412143551_add_partial_index_on_unencrypted_integrations.rb

+# frozen_string_literal: true
+#
+class AddPartialIndexOnUnencryptedIntegrations < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_integrations_on_id_where_not_encrypted'
+  INDEX_FILTER_CONDITION = 'properties IS NOT NULL AND encrypted_properties IS NULL'
+
+  def up
+    add_concurrent_index :integrations, [:id],
+      where: INDEX_FILTER_CONDITION,
+      name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :integrations, INDEX_NAME
+  end
+end

db/post_migrate/20220412143552_consume_remaining_encrypt_integration_property_jobs.rb

+# frozen_string_literal: true
+
+class ConsumeRemainingEncryptIntegrationPropertyJobs < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  BATCH_SIZE = 50
+
+  def up
+    Gitlab::BackgroundMigration.steal('EncryptIntegrationProperties')
+
+    model = define_batchable_model('integrations')
+    relation = model.where.not(properties: nil).where(encrypted_properties: nil)
+
+    relation.each_batch(of: BATCH_SIZE) do |batch|
+      range = batch.pluck('MIN(id)', 'MAX(id)').first
+
+      Gitlab::BackgroundMigration::EncryptIntegrationProperties.new.perform(*range)
+    end
+  end
+
+  def down
+  end
+end

db/post_migrate/20220413011328_remove_partial_index_on_unencrypted_integrations.rb

+# frozen_string_literal: true
+#
+# The inverse of 20220412143551_add_partial_index_on_unencrypted_integrations.rb
+class RemovePartialIndexOnUnencryptedIntegrations < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_integrations_on_id_where_not_encrypted'
+  INDEX_FILTER_CONDITION = 'properties IS NOT NULL AND encrypted_properties IS NULL'
+
+  def down
+    add_concurrent_index :integrations, [:id],
+      where: INDEX_FILTER_CONDITION,
+      name: INDEX_NAME
+  end
+
+  def up
+    remove_concurrent_index_by_name :integrations, INDEX_NAME
+  end
+end

db/schema_migrations/20220412143551

+beff437160d30bc0cb6577e5b88edb751f1325b316534010844e053a567906ff
\ No newline at end of file

db/schema_migrations/20220412143552

+6211f4f1e2708606aa68c139639acdb366cd1f8e4be225800a2e49888f420498
\ No newline at end of file

db/schema_migrations/20220413011328

+442300bd5c2f05807bdf752a9c3280a11f1cc84b21c2d61d99fb73268f7a495f
\ No newline at end of file

ee/app/models/integrations/github.rb

@@ -15,7 +15,9 @@ module Integrations
     default_value_for :pipeline_events, true
 
     def initialize_properties
-      self.properties ||= { static_context: true }
+      return if properties.present?
+
+      self.static_context = true
     end
 
     def title

lib/gitlab/integrations/sti_type.rb

@@ -3,12 +3,12 @@
 module Gitlab
   module Integrations
     class StiType < ActiveRecord::Type::String
-      NAMESPACED_INTEGRATIONS = Set.new(%w(
+      NAMESPACED_INTEGRATIONS = %w[
         Asana Assembla Bamboo Bugzilla Buildkite Campfire Confluence CustomIssueTracker Datadog
         Discord DroneCi EmailsOnPush Ewm ExternalWiki Flowdock HangoutsChat Harbor Irker Jenkins Jira Mattermost
         MattermostSlashCommands MicrosoftTeams MockCi MockMonitoring Packagist PipelinesEmail Pivotaltracker
         Prometheus Pushover Redmine Shimo Slack SlackSlashCommands Teamcity UnifyCircuit WebexTeams Youtrack Zentao
-      )).freeze
+      ].to_set.freeze
 
       def self.namespaced_integrations
         NAMESPACED_INTEGRATIONS

spec/controllers/projects/services_controller_spec.rb

@@ -359,10 +359,9 @@ RSpec.describe Projects::ServicesController do
         def prometheus_integration_as_data
           pi = project.prometheus_integration.reload
           attrs = pi.attributes.except('encrypted_properties',
-                                       'encrypted_properties_iv',
-                                       'encrypted_properties_tmp')
+                                       'encrypted_properties_iv')
 
-          [attrs, pi.encrypted_properties_tmp]
+          [attrs, pi.properties]
         end
       end
 

spec/factories/integrations.rb

@@ -189,7 +189,7 @@ FactoryBot.define do
   end
 
   trait :chat_notification do
-    webhook { 'https://example.com/webhook' }
+    sequence(:webhook) { |n| "https://example.com/webhook/#{n}" }
   end
 
   trait :inactive do

spec/migrations/20220412143552_consume_remaining_encrypt_integration_property_jobs_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require_migration!
+
+RSpec.describe ConsumeRemainingEncryptIntegrationPropertyJobs, :migration do
+  subject(:migration) { described_class.new }
+
+  let(:integrations) { table(:integrations) }
+  let(:bg_migration_class) { ::Gitlab::BackgroundMigration::EncryptIntegrationProperties }
+  let(:bg_migration) { instance_double(bg_migration_class) }
+
+  before do
+    stub_const("#{described_class.name}::BATCH_SIZE", 2)
+  end
+
+  it 'performs remaining background migrations', :aggregate_failures do
+    # Already migrated
+    integrations.create!(properties: some_props, encrypted_properties: 'abc')
+    integrations.create!(properties: some_props, encrypted_properties: 'def')
+    integrations.create!(properties: some_props, encrypted_properties: 'xyz')
+    # update required
+    record1 = integrations.create!(properties: some_props)
+    record2 = integrations.create!(properties: some_props)
+    record3 = integrations.create!(properties: some_props)
+    # No update required
+    integrations.create!(properties: nil)
+    integrations.create!(properties: nil)
+
+    expect(Gitlab::BackgroundMigration).to receive(:steal).with(bg_migration_class.name.demodulize)
+    expect(bg_migration_class).to receive(:new).twice.and_return(bg_migration)
+    expect(bg_migration).to receive(:perform).with(record1.id, record2.id)
+    expect(bg_migration).to receive(:perform).with(record3.id, record3.id)
+
+    migrate!
+  end
+
+  def some_props
+    { iid: generate(:iid), url: generate(:url), username: generate(:username) }.to_json
+  end
+end

spec/models/integration_spec.rb

@@ -276,6 +276,20 @@ RSpec.describe Integration do
     end
   end
 
+  describe '#inheritable?' do
+    it 'is true for an instance integration' do
+      expect(create(:integration, :instance)).to be_inheritable
+    end
+
+    it 'is true for a group integration' do
+      expect(create(:integration, :group)).to be_inheritable
+    end
+
+    it 'is false for a project integration' do
+      expect(create(:integration)).not_to be_inheritable
+    end
+  end
+
   describe '.build_from_integration' do
     context 'when integration is invalid' do
       let(:invalid_integration) do
@@ -644,6 +658,33 @@ RSpec.describe Integration do
     end
   end
 
+  describe '#properties=' do
+    let(:integration_type) do
+      Class.new(described_class) do
+        field :foo
+        field :bar
+      end
+    end
+
+    it 'supports indifferent access' do
+      integration = integration_type.new
+
+      integration.properties = { foo: 1, 'bar' => 2 }
+
+      expect(integration).to have_attributes(foo: 1, bar: 2)
+    end
+  end
+
+  describe '#properties' do
+    it 'is not mutable' do
+      integration = described_class.new
+
+      integration.properties = { foo: 1, bar: 2 }
+
+      expect { integration.properties[:foo] = 3 }.to raise_error
+    end
+  end
+
   describe "{property}_touched?" do
     let(:integration) do
       Integrations::Bamboo.create!(
@@ -896,45 +937,26 @@ RSpec.describe Integration do
     end
   end
 
-  describe 'encrypted_properties' do
+  describe '#to_integration_hash' do
     let(:properties) { { foo: 1, bar: true } }
     let(:db_props) { properties.stringify_keys }
     let(:record) { create(:integration, :instance, properties: properties) }
 
-    it 'contains the same data as properties' do
-      expect(record).to have_attributes(
-        properties: db_props,
-        encrypted_properties_tmp: db_props
-      )
-    end
-
-    it 'is persisted' do
-      encrypted_properties = described_class.id_in(record.id)
-
-      expect(encrypted_properties).to contain_exactly have_attributes(encrypted_properties_tmp: db_props)
-    end
-
-    it 'is updated when using prop_accessors' do
-      some_integration = Class.new(described_class) do
-        prop_accessor :foo
-      end
-
-      record = some_integration.new
-
-      record.foo = 'the foo'
+    it 'does not include the properties key' do
+      hash = record.to_integration_hash
 
-      expect(record.encrypted_properties_tmp).to eq({ 'foo' => 'the foo' })
+      expect(hash).not_to have_key('properties')
     end
 
     it 'saves correctly using insert_all' do
       hash = record.to_integration_hash
-      hash[:project_id] = project
+      hash[:project_id] = project.id
 
       expect do
         described_class.insert_all([hash])
       end.to change(described_class, :count).by(1)
 
-      expect(described_class.last).to have_attributes(encrypted_properties_tmp: db_props)
+      expect(described_class.last).to have_attributes(properties: db_props)
     end
 
     it 'is part of the to_integration_hash' do
@@ -944,7 +966,7 @@ RSpec.describe Integration do
       expect(hash['encrypted_properties']).not_to eq(record.encrypted_properties)
       expect(hash['encrypted_properties_iv']).not_to eq(record.encrypted_properties_iv)
 
-      decrypted = described_class.decrypt(:encrypted_properties_tmp,
+      decrypted = described_class.decrypt(:properties,
                                           hash['encrypted_properties'],
                                           { iv: hash['encrypted_properties_iv'] })
 
@@ -969,7 +991,7 @@ RSpec.describe Integration do
         end.to change(described_class, :count).by(1)
 
         expect(described_class.last).not_to eq record
-        expect(described_class.last).to have_attributes(encrypted_properties_tmp: db_props)
+        expect(described_class.last).to have_attributes(properties: db_props)
       end
     end
   end
@@ -1094,4 +1116,47 @@ RSpec.describe Integration do
       )
     end
   end
+
+  describe '#attributes' do
+    it 'does not include properties' do
+      expect(create(:integration).attributes).not_to have_key('properties')
+    end
+
+    it 'can be used in assign_attributes without nullifying properties' do
+      record = create(:integration, :instance, properties: { url: generate(:url) })
+
+      attrs = record.attributes
+
+      expect { record.assign_attributes(attrs) }.not_to change(record, :properties)
+    end
+  end
+
+  describe '#dup' do
+    let(:original) { create(:integration, properties: { one: 1, two: 2, three: 3 }) }
+
+    it 'results in distinct ciphertexts, but identical properties' do
+      copy = original.dup
+
+      expect(copy).to have_attributes(properties: eq(original.properties))
+
+      expect(copy).not_to have_attributes(
+        encrypted_properties: eq(original.encrypted_properties)
+      )
+    end
+
+    context 'when the model supports data-fields' do
+      let(:original) { create(:jira_integration, username: generate(:username), url: generate(:url)) }
+
+      it 'creates distinct but identical data-fields' do
+        copy = original.dup
+
+        expect(copy).to have_attributes(
+          username: original.username,
+          url: original.url
+        )
+
+        expect(copy.data_fields).not_to eq(original.data_fields)
+      end
+    end
+  end
 end

spec/models/integrations/external_wiki_spec.rb

@@ -24,7 +24,7 @@ RSpec.describe Integrations::ExternalWiki do
 
   describe 'test' do
     before do
-      subject.properties['external_wiki_url'] = url
+      subject.external_wiki_url = url
     end
 
     let(:url) { 'http://foo' }

spec/models/integrations/jira_spec.rb

@@ -187,7 +187,7 @@ RSpec.describe Integrations::Jira do
     subject(:integration) { described_class.create!(params) }
 
     it 'does not store data into properties' do
-      expect(integration.properties).to be_nil
+      expect(integration.properties).to be_empty
     end
 
     it 'stores data in data_fields correctly' do

spec/models/integrations/slack_spec.rb

@@ -6,12 +6,12 @@ RSpec.describe Integrations::Slack do
   it_behaves_like Integrations::SlackMattermostNotifier, "Slack"
 
   describe '#execute' do
+    let_it_be(:slack_integration) { create(:integrations_slack, branches_to_be_notified: 'all') }
+
     before do
       stub_request(:post, slack_integration.webhook)
     end
 
-    let_it_be(:slack_integration) { create(:integrations_slack, branches_to_be_notified: 'all') }
-
     it 'uses only known events', :aggregate_failures do
       described_class::SUPPORTED_EVENTS_FOR_USAGE_LOG.each do |action|
         expect(Gitlab::UsageDataCounters::HLLRedisCounter.known_event?("i_ecosystem_slack_service_#{action}_notification")).to be true

spec/services/bulk_update_integration_service_spec.rb

@@ -9,7 +9,13 @@ RSpec.describe BulkUpdateIntegrationService do
     stub_jira_integration_test
   end
 
-  let(:excluded_attributes) { %w[id project_id group_id inherit_from_id instance template created_at updated_at] }
+  let(:excluded_attributes) do
+    %w[
+       id project_id group_id inherit_from_id instance template
+       created_at updated_at encrypted_properties encrypted_properties_iv
+    ]
+  end
+
   let(:batch) do
     Integration.inherited_descendants_from_self_or_ancestors_from(subgroup_integration).where(id: group_integration.id..integration.id)
   end
@@ -50,7 +56,9 @@ RSpec.describe BulkUpdateIntegrationService do
     end
 
     context 'with integration with data fields' do
-      let(:excluded_attributes) { %w[id service_id created_at updated_at] }
+      let(:excluded_attributes) do
+        %w[id service_id created_at updated_at encrypted_properties encrypted_properties_iv]
+      end
 
       it 'updates the data fields from the integration', :aggregate_failures do
         described_class.new(subgroup_integration, batch).execute

spec/services/projects/operations/update_service_spec.rb

@@ -407,10 +407,11 @@ RSpec.describe Projects::Operations::UpdateService do
 
     context 'prometheus integration' do
       context 'prometheus params were passed into service' do
-        let(:prometheus_integration) do
-          build_stubbed(:prometheus_integration, project: project, properties: {
+        let!(:prometheus_integration) do
+          create(:prometheus_integration, :instance, properties: {
             api_url: "http://example.prometheus.com",
-            manual_configuration: "0"
+            manual_configuration: "0",
+            google_iap_audience_client_id: 123
           })
         end
 
@@ -424,21 +425,23 @@ RSpec.describe Projects::Operations::UpdateService do
         end
 
         it 'uses Project#find_or_initialize_integration to include instance defined defaults and pass them to Projects::UpdateService', :aggregate_failures do
-          project_update_service = double(Projects::UpdateService)
-
-          expect(project)
-            .to receive(:find_or_initialize_integration)
-            .with('prometheus')
-            .and_return(prometheus_integration)
           expect(Projects::UpdateService).to receive(:new) do |project_arg, user_arg, update_params_hash|
+            prometheus_attrs = update_params_hash[:prometheus_integration_attributes]
+
             expect(project_arg).to eq project
             expect(user_arg).to eq user
-            expect(update_params_hash[:prometheus_integration_attributes]).to include('properties' => { 'api_url' => 'http://new.prometheus.com', 'manual_configuration' => '1' })
-            expect(update_params_hash[:prometheus_integration_attributes]).not_to include(*%w(id project_id created_at updated_at))
-          end.and_return(project_update_service)
-          expect(project_update_service).to receive(:execute)
+            expect(prometheus_attrs).to have_key('encrypted_properties')
+            expect(prometheus_attrs.keys).not_to include(*%w(id project_id created_at updated_at properties))
+            expect(prometheus_attrs['encrypted_properties']).not_to eq(prometheus_integration.encrypted_properties)
+          end.and_call_original
 
-          subject.execute
+          expect { subject.execute }.to change(Integrations::Prometheus, :count).by(1)
+
+          expect(Integrations::Prometheus.last).to have_attributes(
+            api_url: 'http://new.prometheus.com',
+            manual_configuration: true,
+            google_iap_audience_client_id: 123
+          )
         end
       end
 

spec/services/projects/transfer_service_spec.rb

@@ -198,23 +198,23 @@ RSpec.describe Projects::TransferService do
 
     context 'with a project integration' do
       let_it_be_with_reload(:project) { create(:project, namespace: user.namespace) }
-      let_it_be(:instance_integration) { create(:integrations_slack, :instance, webhook: 'http://project.slack.com') }
+      let_it_be(:instance_integration) { create(:integrations_slack, :instance) }
+      let_it_be(:project_integration) { create(:integrations_slack, project: project) }
 
-      context 'with an inherited integration' do
-        let_it_be(:project_integration) { create(:integrations_slack, project: project, webhook: 'http://project.slack.com', inherit_from_id: instance_integration.id) }
+      context 'when it inherits from instance_integration' do
+        before do
+          project_integration.update!(inherit_from_id: instance_integration.id, webhook: instance_integration.webhook)
+        end
 
         it 'replaces inherited integrations', :aggregate_failures do
-          execute_transfer
-
-          expect(project.slack_integration.webhook).to eq(group_integration.webhook)
-          expect(Integration.count).to eq(3)
+          expect { execute_transfer }
+            .to change(Integration, :count).by(0)
+            .and change { project.slack_integration.webhook }.to eq(group_integration.webhook)
         end
       end
 
       context 'with a custom integration' do
-        let_it_be(:project_integration) { create(:integrations_slack, project: project, webhook: 'http://project.slack.com') }
-
-        it 'does not updates the integrations' do
+        it 'does not update the integrations' do
           expect { execute_transfer }.not_to change { project.slack_integration.webhook }
         end
       end

spec/workers/projects/post_creation_worker_spec.rb

@@ -63,7 +63,7 @@ RSpec.describe Projects::PostCreationWorker do
           end
 
           it 'cleans invalid record and logs warning', :aggregate_failures do
-            invalid_integration_record = build(:prometheus_integration, properties: { api_url: nil, manual_configuration: true }.to_json)
+            invalid_integration_record = build(:prometheus_integration, properties: { api_url: nil, manual_configuration: true })
             allow(::Integrations::Prometheus).to receive(:new).and_return(invalid_integration_record)
 
             expect(Gitlab::ErrorTracking).to receive(:track_exception).with(an_instance_of(ActiveRecord::RecordInvalid), include(extra: { project_id: a_kind_of(Integer) })).twice