Commit 2cf5ee38 authored by Alex Kalderimis's avatar Alex Kalderimis

Read encrypted properties

Read the now encrypted properties from `encrypted_properties`, and
ignore the integration.properties column, which will be removed
in a later release.

Changelog: changed
parent 75230eb6
...@@ -10,9 +10,11 @@ class Integration < ApplicationRecord ...@@ -10,9 +10,11 @@ class Integration < ApplicationRecord
include FromUnion include FromUnion
include EachBatch include EachBatch
include IgnorableColumns include IgnorableColumns
extend ::Gitlab::Utils::Override
ignore_column :template, remove_with: '15.0', remove_after: '2022-04-22' 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 :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) UnknownType = Class.new(StandardError)
...@@ -47,10 +49,7 @@ class Integration < ApplicationRecord ...@@ -47,10 +49,7 @@ class Integration < ApplicationRecord
SECTION_TYPE_CONNECTION = 'connection' SECTION_TYPE_CONNECTION = 'connection'
serialize :properties, JSON # rubocop:disable Cop/ActiveRecordSerialize attr_encrypted :properties,
attr_encrypted :encrypted_properties_tmp,
attribute: :encrypted_properties,
mode: :per_attribute_iv, mode: :per_attribute_iv,
key: Settings.attr_encrypted_db_key_base_32, key: Settings.attr_encrypted_db_key_base_32,
algorithm: 'aes-256-gcm', algorithm: 'aes-256-gcm',
...@@ -59,6 +58,15 @@ class Integration < ApplicationRecord ...@@ -59,6 +58,15 @@ class Integration < ApplicationRecord
encode: false, encode: false,
encode_iv: 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 alias_attribute :type, :type_new
default_value_for :active, false default_value_for :active, false
...@@ -77,8 +85,6 @@ class Integration < ApplicationRecord ...@@ -77,8 +85,6 @@ class Integration < ApplicationRecord
default_value_for :wiki_page_events, true default_value_for :wiki_page_events, true
after_initialize :initialize_properties after_initialize :initialize_properties
after_initialize :copy_properties_to_encrypted_properties
before_save :copy_properties_to_encrypted_properties
after_commit :reset_updated_properties after_commit :reset_updated_properties
...@@ -165,16 +171,14 @@ class Integration < ApplicationRecord ...@@ -165,16 +171,14 @@ class Integration < ApplicationRecord
class_eval <<~RUBY, __FILE__, __LINE__ + 1 class_eval <<~RUBY, __FILE__, __LINE__ + 1
unless method_defined?(arg) unless method_defined?(arg)
def #{arg} def #{arg}
properties['#{arg}'] properties['#{arg}'] if properties.present?
end end
end end
def #{arg}=(value) def #{arg}=(value)
self.properties ||= {} self.properties ||= {}
self.encrypted_properties_tmp = properties
updated_properties['#{arg}'] = #{arg} unless #{arg}_changed? updated_properties['#{arg}'] = #{arg} unless #{arg}_changed?
self.properties['#{arg}'] = value self.properties = self.properties.merge('#{arg}' => value)
self.encrypted_properties_tmp['#{arg}'] = value
end end
def #{arg}_changed? def #{arg}_changed?
...@@ -195,11 +199,13 @@ class Integration < ApplicationRecord ...@@ -195,11 +199,13 @@ class Integration < ApplicationRecord
# Provide convenient boolean accessor methods for each serialized property. # Provide convenient boolean accessor methods for each serialized property.
# Also keep track of updated properties in a similar way as ActiveModel::Dirty # Also keep track of updated properties in a similar way as ActiveModel::Dirty
def self.boolean_accessor(*args) def self.boolean_accessor(*args)
self.prop_accessor(*args) prop_accessor(*args)
args.each do |arg| args.each do |arg|
class_eval <<~RUBY, __FILE__, __LINE__ + 1 class_eval <<~RUBY, __FILE__, __LINE__ + 1
def #{arg} def #{arg}
return if properties.blank?
Gitlab::Utils.to_boolean(properties['#{arg}']) Gitlab::Utils.to_boolean(properties['#{arg}'])
end end
...@@ -318,18 +324,31 @@ class Integration < ApplicationRecord ...@@ -318,18 +324,31 @@ class Integration < ApplicationRecord
def self.build_from_integration(integration, project_id: nil, group_id: nil) def self.build_from_integration(integration, project_id: nil, group_id: nil)
new_integration = integration.dup 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.instance = false
new_integration.project_id = project_id new_integration.project_id = project_id
new_integration.group_id = group_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 new_integration
end 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) def self.instance_exists_for?(type)
exists?(instance: true, type: type) exists?(instance: true, type: type)
end end
...@@ -402,13 +421,7 @@ class Integration < ApplicationRecord ...@@ -402,13 +421,7 @@ class Integration < ApplicationRecord
end end
def initialize_properties def initialize_properties
self.properties = {} if has_attribute?(:properties) && properties.nil? self.properties = {} if has_attribute?(:encrypted_properties) && encrypted_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
end end
def title def title
...@@ -445,21 +458,26 @@ class Integration < ApplicationRecord ...@@ -445,21 +458,26 @@ class Integration < ApplicationRecord
%w[active] %w[active]
end 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 # return a hash of columns => values suitable for passing to insert_all
def to_integration_hash def to_integration_hash
column = self.class.attribute_aliases.fetch('type', 'type') 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(column => type)
.merge(reencrypt_properties) .merge(reencrypt_properties)
end end
def reencrypt_properties def reencrypt_properties
unless properties.nil? || properties.empty? 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) iv = generate_iv(alg)
ep = self.class.encrypt(:encrypted_properties_tmp, properties, { iv: iv }) ep = self.class.encrypt(:properties, properties, { iv: iv })
end end
{ 'encrypted_properties' => ep, 'encrypted_properties_iv' => iv } { 'encrypted_properties' => ep, 'encrypted_properties_iv' => iv }
......
...@@ -35,8 +35,9 @@ module Integrations ...@@ -35,8 +35,9 @@ module Integrations
validates :labels_to_be_notified_behavior, inclusion: { in: LABEL_NOTIFICATION_BEHAVIOURS }, allow_blank: true validates :labels_to_be_notified_behavior, inclusion: { in: LABEL_NOTIFICATION_BEHAVIOURS }, allow_blank: true
def initialize_properties def initialize_properties
if properties.nil? super
self.properties = {}
if properties.empty?
self.notify_only_broken_pipelines = true self.notify_only_broken_pipelines = true
self.branches_to_be_notified = "default" self.branches_to_be_notified = "default"
self.labels_to_be_notified_behavior = MATCH_ANY_LABEL self.labels_to_be_notified_behavior = MATCH_ANY_LABEL
......
...@@ -25,12 +25,15 @@ module Integrations ...@@ -25,12 +25,15 @@ module Integrations
def handle_properties def handle_properties
# this has been moved from initialize_properties and should be improved # this has been moved from initialize_properties and should be improved
# as part of https://gitlab.com/gitlab-org/gitlab/issues/29404 # 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 @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.reject! { |key| data_fields.changed.include?(key) }
data_values.slice!(*data_fields.attributes.keys)
data_fields.assign_attributes(data_values) if data_values.present? data_fields.assign_attributes(data_values) if data_values.present?
self.properties = {} self.properties = {}
...@@ -68,10 +71,6 @@ module Integrations ...@@ -68,10 +71,6 @@ module Integrations
issue_url(iid) issue_url(iid)
end end
def initialize_properties
{}
end
# Initialize with default properties values # Initialize with default properties values
def set_default_data def set_default_data
return unless issues_tracker.present? return unless issues_tracker.present?
......
...@@ -27,12 +27,12 @@ module Integrations ...@@ -27,12 +27,12 @@ module Integrations
end end
# Since SSL verification will always be enabled for Buildkite, # 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. # This is a stub method to work with deprecated API param.
# TODO: remove enable_ssl_verification after 14.0 # TODO: remove enable_ssl_verification after 14.0
# https://gitlab.com/gitlab-org/gitlab/-/issues/222808 # https://gitlab.com/gitlab-org/gitlab/-/issues/222808
def enable_ssl_verification=(_value) 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 end
override :hook_url override :hook_url
......
...@@ -94,10 +94,6 @@ module Integrations ...@@ -94,10 +94,6 @@ module Integrations
!!URI(url).hostname&.end_with?(JIRA_CLOUD_HOST) !!URI(url).hostname&.end_with?(JIRA_CLOUD_HOST)
end end
def initialize_properties
{}
end
def data_fields def data_fields
jira_tracker_data || self.build_jira_tracker_data jira_tracker_data || self.build_jira_tracker_data
end end
...@@ -106,7 +102,7 @@ module Integrations ...@@ -106,7 +102,7 @@ module Integrations
return unless reset_password? return unless reset_password?
data_fields.password = nil data_fields.password = nil
properties.delete('password') if properties self.properties = properties.except('password')
end end
def set_default_data def set_default_data
......
...@@ -12,8 +12,9 @@ module Integrations ...@@ -12,8 +12,9 @@ module Integrations
validate :number_of_recipients_within_limit, if: :validate_recipients? validate :number_of_recipients_within_limit, if: :validate_recipients?
def initialize_properties def initialize_properties
if properties.nil? super
self.properties = {}
if properties.blank?
self.notify_only_broken_pipelines = true self.notify_only_broken_pipelines = true
self.branches_to_be_notified = "default" self.branches_to_be_notified = "default"
elsif !self.notify_only_default_branch.nil? elsif !self.notify_only_default_branch.nil?
......
...@@ -32,12 +32,6 @@ module Integrations ...@@ -32,12 +32,6 @@ module Integrations
scope :preload_project, -> { preload(:project) } scope :preload_project, -> { preload(:project) }
scope :with_clusters_with_cilium, -> { joins(project: [:clusters]).merge(Clusters::Cluster.with_available_cilium) } 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? def show_active_box?
false false
end end
......
...@@ -112,8 +112,9 @@ module Projects ...@@ -112,8 +112,9 @@ module Projects
integration = project.find_or_initialize_integration(::Integrations::Prometheus.to_param) integration = project.find_or_initialize_integration(::Integrations::Prometheus.to_param)
integration.assign_attributes(attrs) 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 end
def incident_management_setting_params def incident_management_setting_params
......
# 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
# 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
# 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
beff437160d30bc0cb6577e5b88edb751f1325b316534010844e053a567906ff
\ No newline at end of file
6211f4f1e2708606aa68c139639acdb366cd1f8e4be225800a2e49888f420498
\ No newline at end of file
442300bd5c2f05807bdf752a9c3280a11f1cc84b21c2d61d99fb73268f7a495f
\ No newline at end of file
...@@ -15,7 +15,9 @@ module Integrations ...@@ -15,7 +15,9 @@ module Integrations
default_value_for :pipeline_events, true default_value_for :pipeline_events, true
def initialize_properties def initialize_properties
self.properties ||= { static_context: true } return if properties.present?
self.static_context = true
end end
def title def title
......
...@@ -3,12 +3,12 @@ ...@@ -3,12 +3,12 @@
module Gitlab module Gitlab
module Integrations module Integrations
class StiType < ActiveRecord::Type::String class StiType < ActiveRecord::Type::String
NAMESPACED_INTEGRATIONS = Set.new(%w( NAMESPACED_INTEGRATIONS = %w[
Asana Assembla Bamboo Bugzilla Buildkite Campfire Confluence CustomIssueTracker Datadog Asana Assembla Bamboo Bugzilla Buildkite Campfire Confluence CustomIssueTracker Datadog
Discord DroneCi EmailsOnPush Ewm ExternalWiki Flowdock HangoutsChat Harbor Irker Jenkins Jira Mattermost Discord DroneCi EmailsOnPush Ewm ExternalWiki Flowdock HangoutsChat Harbor Irker Jenkins Jira Mattermost
MattermostSlashCommands MicrosoftTeams MockCi MockMonitoring Packagist PipelinesEmail Pivotaltracker MattermostSlashCommands MicrosoftTeams MockCi MockMonitoring Packagist PipelinesEmail Pivotaltracker
Prometheus Pushover Redmine Shimo Slack SlackSlashCommands Teamcity UnifyCircuit WebexTeams Youtrack Zentao Prometheus Pushover Redmine Shimo Slack SlackSlashCommands Teamcity UnifyCircuit WebexTeams Youtrack Zentao
)).freeze ].to_set.freeze
def self.namespaced_integrations def self.namespaced_integrations
NAMESPACED_INTEGRATIONS NAMESPACED_INTEGRATIONS
......
...@@ -359,10 +359,9 @@ RSpec.describe Projects::ServicesController do ...@@ -359,10 +359,9 @@ RSpec.describe Projects::ServicesController do
def prometheus_integration_as_data def prometheus_integration_as_data
pi = project.prometheus_integration.reload pi = project.prometheus_integration.reload
attrs = pi.attributes.except('encrypted_properties', attrs = pi.attributes.except('encrypted_properties',
'encrypted_properties_iv', 'encrypted_properties_iv')
'encrypted_properties_tmp')
[attrs, pi.encrypted_properties_tmp] [attrs, pi.properties]
end end
end end
......
...@@ -189,7 +189,7 @@ FactoryBot.define do ...@@ -189,7 +189,7 @@ FactoryBot.define do
end end
trait :chat_notification do trait :chat_notification do
webhook { 'https://example.com/webhook' } sequence(:webhook) { |n| "https://example.com/webhook/#{n}" }
end end
trait :inactive do trait :inactive do
......
# 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
...@@ -276,6 +276,20 @@ RSpec.describe Integration do ...@@ -276,6 +276,20 @@ RSpec.describe Integration do
end end
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 describe '.build_from_integration' do
context 'when integration is invalid' do context 'when integration is invalid' do
let(:invalid_integration) do let(:invalid_integration) do
...@@ -644,6 +658,33 @@ RSpec.describe Integration do ...@@ -644,6 +658,33 @@ RSpec.describe Integration do
end end
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 describe "{property}_touched?" do
let(:integration) do let(:integration) do
Integrations::Bamboo.create!( Integrations::Bamboo.create!(
...@@ -896,45 +937,26 @@ RSpec.describe Integration do ...@@ -896,45 +937,26 @@ RSpec.describe Integration do
end end
end end
describe 'encrypted_properties' do describe '#to_integration_hash' do
let(:properties) { { foo: 1, bar: true } } let(:properties) { { foo: 1, bar: true } }
let(:db_props) { properties.stringify_keys } let(:db_props) { properties.stringify_keys }
let(:record) { create(:integration, :instance, properties: properties) } let(:record) { create(:integration, :instance, properties: properties) }
it 'contains the same data as properties' do it 'does not include the properties key' do
expect(record).to have_attributes( hash = record.to_integration_hash
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'
expect(record.encrypted_properties_tmp).to eq({ 'foo' => 'the foo' }) expect(hash).not_to have_key('properties')
end end
it 'saves correctly using insert_all' do it 'saves correctly using insert_all' do
hash = record.to_integration_hash hash = record.to_integration_hash
hash[:project_id] = project hash[:project_id] = project.id
expect do expect do
described_class.insert_all([hash]) described_class.insert_all([hash])
end.to change(described_class, :count).by(1) 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 end
it 'is part of the to_integration_hash' do it 'is part of the to_integration_hash' do
...@@ -944,7 +966,7 @@ RSpec.describe Integration do ...@@ -944,7 +966,7 @@ RSpec.describe Integration do
expect(hash['encrypted_properties']).not_to eq(record.encrypted_properties) expect(hash['encrypted_properties']).not_to eq(record.encrypted_properties)
expect(hash['encrypted_properties_iv']).not_to eq(record.encrypted_properties_iv) 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'], hash['encrypted_properties'],
{ iv: hash['encrypted_properties_iv'] }) { iv: hash['encrypted_properties_iv'] })
...@@ -969,7 +991,7 @@ RSpec.describe Integration do ...@@ -969,7 +991,7 @@ RSpec.describe Integration do
end.to change(described_class, :count).by(1) end.to change(described_class, :count).by(1)
expect(described_class.last).not_to eq record 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 end
end end
...@@ -1094,4 +1116,47 @@ RSpec.describe Integration do ...@@ -1094,4 +1116,47 @@ RSpec.describe Integration do
) )
end end
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 end
...@@ -24,7 +24,7 @@ RSpec.describe Integrations::ExternalWiki do ...@@ -24,7 +24,7 @@ RSpec.describe Integrations::ExternalWiki do
describe 'test' do describe 'test' do
before do before do
subject.properties['external_wiki_url'] = url subject.external_wiki_url = url
end end
let(:url) { 'http://foo' } let(:url) { 'http://foo' }
......
...@@ -187,7 +187,7 @@ RSpec.describe Integrations::Jira do ...@@ -187,7 +187,7 @@ RSpec.describe Integrations::Jira do
subject(:integration) { described_class.create!(params) } subject(:integration) { described_class.create!(params) }
it 'does not store data into properties' do it 'does not store data into properties' do
expect(integration.properties).to be_nil expect(integration.properties).to be_empty
end end
it 'stores data in data_fields correctly' do it 'stores data in data_fields correctly' do
......
...@@ -6,12 +6,12 @@ RSpec.describe Integrations::Slack do ...@@ -6,12 +6,12 @@ RSpec.describe Integrations::Slack do
it_behaves_like Integrations::SlackMattermostNotifier, "Slack" it_behaves_like Integrations::SlackMattermostNotifier, "Slack"
describe '#execute' do describe '#execute' do
let_it_be(:slack_integration) { create(:integrations_slack, branches_to_be_notified: 'all') }
before do before do
stub_request(:post, slack_integration.webhook) stub_request(:post, slack_integration.webhook)
end end
let_it_be(:slack_integration) { create(:integrations_slack, branches_to_be_notified: 'all') }
it 'uses only known events', :aggregate_failures do it 'uses only known events', :aggregate_failures do
described_class::SUPPORTED_EVENTS_FOR_USAGE_LOG.each do |action| 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 expect(Gitlab::UsageDataCounters::HLLRedisCounter.known_event?("i_ecosystem_slack_service_#{action}_notification")).to be true
......
...@@ -9,7 +9,13 @@ RSpec.describe BulkUpdateIntegrationService do ...@@ -9,7 +9,13 @@ RSpec.describe BulkUpdateIntegrationService do
stub_jira_integration_test stub_jira_integration_test
end 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 let(:batch) do
Integration.inherited_descendants_from_self_or_ancestors_from(subgroup_integration).where(id: group_integration.id..integration.id) Integration.inherited_descendants_from_self_or_ancestors_from(subgroup_integration).where(id: group_integration.id..integration.id)
end end
...@@ -50,7 +56,9 @@ RSpec.describe BulkUpdateIntegrationService do ...@@ -50,7 +56,9 @@ RSpec.describe BulkUpdateIntegrationService do
end end
context 'with integration with data fields' do 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 it 'updates the data fields from the integration', :aggregate_failures do
described_class.new(subgroup_integration, batch).execute described_class.new(subgroup_integration, batch).execute
......
...@@ -407,10 +407,11 @@ RSpec.describe Projects::Operations::UpdateService do ...@@ -407,10 +407,11 @@ RSpec.describe Projects::Operations::UpdateService do
context 'prometheus integration' do context 'prometheus integration' do
context 'prometheus params were passed into service' do context 'prometheus params were passed into service' do
let(:prometheus_integration) do let!(:prometheus_integration) do
build_stubbed(:prometheus_integration, project: project, properties: { create(:prometheus_integration, :instance, properties: {
api_url: "http://example.prometheus.com", api_url: "http://example.prometheus.com",
manual_configuration: "0" manual_configuration: "0",
google_iap_audience_client_id: 123
}) })
end end
...@@ -424,21 +425,23 @@ RSpec.describe Projects::Operations::UpdateService do ...@@ -424,21 +425,23 @@ RSpec.describe Projects::Operations::UpdateService do
end end
it 'uses Project#find_or_initialize_integration to include instance defined defaults and pass them to Projects::UpdateService', :aggregate_failures do 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| 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(project_arg).to eq project
expect(user_arg).to eq user 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(prometheus_attrs).to have_key('encrypted_properties')
expect(update_params_hash[:prometheus_integration_attributes]).not_to include(*%w(id project_id created_at updated_at)) expect(prometheus_attrs.keys).not_to include(*%w(id project_id created_at updated_at properties))
end.and_return(project_update_service) expect(prometheus_attrs['encrypted_properties']).not_to eq(prometheus_integration.encrypted_properties)
expect(project_update_service).to receive(:execute) 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
end end
......
...@@ -198,23 +198,23 @@ RSpec.describe Projects::TransferService do ...@@ -198,23 +198,23 @@ RSpec.describe Projects::TransferService do
context 'with a project integration' do context 'with a project integration' do
let_it_be_with_reload(:project) { create(:project, namespace: user.namespace) } 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 context 'when it inherits from instance_integration' do
let_it_be(:project_integration) { create(:integrations_slack, project: project, webhook: 'http://project.slack.com', inherit_from_id: instance_integration.id) } before do
project_integration.update!(inherit_from_id: instance_integration.id, webhook: instance_integration.webhook)
end
it 'replaces inherited integrations', :aggregate_failures do it 'replaces inherited integrations', :aggregate_failures do
execute_transfer expect { execute_transfer }
.to change(Integration, :count).by(0)
expect(project.slack_integration.webhook).to eq(group_integration.webhook) .and change { project.slack_integration.webhook }.to eq(group_integration.webhook)
expect(Integration.count).to eq(3)
end end
end end
context 'with a custom integration' do context 'with a custom integration' do
let_it_be(:project_integration) { create(:integrations_slack, project: project, webhook: 'http://project.slack.com') } it 'does not update the integrations' do
it 'does not updates the integrations' do
expect { execute_transfer }.not_to change { project.slack_integration.webhook } expect { execute_transfer }.not_to change { project.slack_integration.webhook }
end end
end end
......
...@@ -63,7 +63,7 @@ RSpec.describe Projects::PostCreationWorker do ...@@ -63,7 +63,7 @@ RSpec.describe Projects::PostCreationWorker do
end end
it 'cleans invalid record and logs warning', :aggregate_failures do 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) 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 expect(Gitlab::ErrorTracking).to receive(:track_exception).with(an_instance_of(ActiveRecord::RecordInvalid), include(extra: { project_id: a_kind_of(Integer) })).twice
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment