Commit 9561328b authored by Rajendra Kadam's avatar Rajendra Kadam

This MR uses AppLogger in

users, group and external users spec
files and adds changelog
for the changelog

Add changelog

Update changelog

Fix spec failures

Fix spec failures
parent b720d175
---
title: Use applogger
merge_request: 41055
author: Rajendra Kadam
type: other
...@@ -15,7 +15,7 @@ module EE ...@@ -15,7 +15,7 @@ module EE
begin begin
group.start_ldap_sync group.start_ldap_sync
Rails.logger.debug { "Started syncing all providers for '#{group.name}' group" } # rubocop:disable Gitlab/RailsLogger ::Gitlab::AppLogger.debug "Started syncing all providers for '#{group.name}' group"
# Shuffle providers to prevent a scenario where sync fails after a time # Shuffle providers to prevent a scenario where sync fails after a time
# and only the first provider or two get synced. This shuffles the order # and only the first provider or two get synced. This shuffles the order
...@@ -28,9 +28,9 @@ module EE ...@@ -28,9 +28,9 @@ module EE
end end
group.finish_ldap_sync group.finish_ldap_sync
Rails.logger.debug { "Finished syncing all providers for '#{group.name}' group" } # rubocop:disable Gitlab/RailsLogger ::Gitlab::AppLogger.debug "Finished syncing all providers for '#{group.name}' group"
rescue ::Gitlab::Auth::Ldap::LdapConnectionError rescue ::Gitlab::Auth::Ldap::LdapConnectionError
Rails.logger.warn("Error syncing all providers for '#{group.name}' group") # rubocop:disable Gitlab/RailsLogger ::Gitlab::AppLogger.warn("Error syncing all providers for '#{group.name}' group")
group.fail_ldap_sync group.fail_ldap_sync
end end
end end
...@@ -41,15 +41,15 @@ module EE ...@@ -41,15 +41,15 @@ module EE
begin begin
group.start_ldap_sync group.start_ldap_sync
Rails.logger.debug { "Started syncing '#{proxy.provider}' provider for '#{group.name}' group" } # rubocop:disable Gitlab/RailsLogger ::Gitlab::AppLogger.debug "Started syncing '#{proxy.provider}' provider for '#{group.name}' group"
sync_group = new(group, proxy) sync_group = new(group, proxy)
sync_group.update_permissions sync_group.update_permissions
group.finish_ldap_sync group.finish_ldap_sync
Rails.logger.debug { "Finished syncing '#{proxy.provider}' provider for '#{group.name}' group" } # rubocop:disable Gitlab/RailsLogger ::Gitlab::AppLogger.debug "Finished syncing '#{proxy.provider}' provider for '#{group.name}' group"
rescue ::Gitlab::Auth::Ldap::LdapConnectionError rescue ::Gitlab::Auth::Ldap::LdapConnectionError
Rails.logger.warn("Error syncing '#{proxy.provider}' provider for '#{group.name}' group") # rubocop:disable Gitlab/RailsLogger ::Gitlab::AppLogger.warn("Error syncing '#{proxy.provider}' provider for '#{group.name}' group")
group.fail_ldap_sync group.fail_ldap_sync
end end
end end
...@@ -59,7 +59,7 @@ module EE ...@@ -59,7 +59,7 @@ module EE
return true unless group.ldap_sync_started? return true unless group.ldap_sync_started?
Rails.logger.warn "Group '#{group.name}' is not ready for LDAP sync. Skipping" # rubocop:disable Gitlab/RailsLogger ::Gitlab::AppLogger.warn "Group '#{group.name}' is not ready for LDAP sync. Skipping"
false false
end end
...@@ -111,13 +111,13 @@ module EE ...@@ -111,13 +111,13 @@ module EE
# so we must propagate any higher inherited permissions unconditionally. # so we must propagate any higher inherited permissions unconditionally.
inherit_higher_access_levels(group, access_levels) inherit_higher_access_levels(group, access_levels)
logger.debug do logger.debug(
<<-MSG.strip_heredoc.tr("\n", ' ') <<-MSG.strip_heredoc.tr("\n", ' ')
Resolved '#{group.name}' group member access, Resolved '#{group.name}' group member access,
propagating any higher access inherited from a parent group: propagating any higher access inherited from a parent group:
#{access_levels.to_hash} #{access_levels.to_hash}
MSG MSG
end )
update_existing_group_membership(group, access_levels) update_existing_group_membership(group, access_levels)
add_new_members(group, access_levels) add_new_members(group, access_levels)
...@@ -129,9 +129,7 @@ module EE ...@@ -129,9 +129,7 @@ module EE
if member_dns = get_member_dns(group_link) if member_dns = get_member_dns(group_link)
access_levels.set(member_dns, to: group_link.group_access) access_levels.set(member_dns, to: group_link.group_access)
logger.debug do logger.debug "Resolved '#{group.name}' group member access: #{access_levels.to_hash}"
"Resolved '#{group.name}' group member access: #{access_levels.to_hash}"
end
end end
end end
...@@ -141,7 +139,7 @@ module EE ...@@ -141,7 +139,7 @@ module EE
def dns_for_group_cn(group_cn) def dns_for_group_cn(group_cn)
if config.group_base.blank? if config.group_base.blank?
logger.debug { "No `group_base` configured for '#{provider}' provider and group link CN #{group_cn}. Skipping" } logger.debug "No `group_base` configured for '#{provider}' provider and group link CN #{group_cn}. Skipping"
return return
end end
...@@ -172,7 +170,7 @@ module EE ...@@ -172,7 +170,7 @@ module EE
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def update_existing_group_membership(group, access_levels) def update_existing_group_membership(group, access_levels)
logger.debug { "Updating existing membership for '#{group.name}' group" } logger.debug "Updating existing membership for '#{group.name}' group"
multiple_ldap_providers = ::Gitlab::Auth::Ldap::Config.providers.count > 1 multiple_ldap_providers = ::Gitlab::Auth::Ldap::Config.providers.count > 1
existing_members = select_and_preload_group_members(group) existing_members = select_and_preload_group_members(group)
...@@ -228,7 +226,7 @@ module EE ...@@ -228,7 +226,7 @@ module EE
end end
def add_new_members(group, access_levels) def add_new_members(group, access_levels)
logger.debug { "Adding new members to '#{group.name}' group" } logger.debug "Adding new members to '#{group.name}' group"
return unless access_levels.present? return unless access_levels.present?
...@@ -247,14 +245,14 @@ module EE ...@@ -247,14 +245,14 @@ module EE
access_level access_level
) )
else else
logger.debug do logger.debug(
<<-MSG.strip_heredoc.tr("\n", ' ') <<-MSG.strip_heredoc.tr("\n", ' ')
#{self.class.name}: User with DN `#{member_dn}` should have access #{self.class.name}: User with DN `#{member_dn}` should have access
to '#{group.name}' group but there is no user in GitLab with that to '#{group.name}' group but there is no user in GitLab with that
identity. Membership will be updated once the user signs in for identity. Membership will be updated once the user signs in for
the first time. the first time.
MSG MSG
end )
end end
end end
end end
...@@ -276,13 +274,13 @@ module EE ...@@ -276,13 +274,13 @@ module EE
end end
def warn_cannot_remove_last_owner(user, group) def warn_cannot_remove_last_owner(user, group)
logger.warn do logger.warn(
<<-MSG.strip_heredoc.tr("\n", ' ') <<-MSG.strip_heredoc.tr("\n", ' ')
#{self.class.name}: LDAP group sync cannot remove #{user.name} #{self.class.name}: LDAP group sync cannot remove #{user.name}
(#{user.id}) from group #{group.name} (#{group.id}) as this is (#{user.id}) from group #{group.name} (#{group.id}) as this is
the group's last owner the group's last owner
MSG MSG
end )
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
...@@ -310,7 +308,7 @@ module EE ...@@ -310,7 +308,7 @@ module EE
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
def logger def logger
Rails.logger # rubocop:disable Gitlab/RailsLogger ::Gitlab::AppLogger
end end
def config def config
......
...@@ -39,7 +39,7 @@ module EE ...@@ -39,7 +39,7 @@ module EE
true true
rescue ::Gitlab::Auth::Ldap::LdapConnectionError rescue ::Gitlab::Auth::Ldap::LdapConnectionError
Rails.logger.warn("Error syncing #{attribute} users for provider '#{provider}'. LDAP connection Error") # rubocop:disable Gitlab/RailsLogger ::Gitlab::AppLogger.warn("Error syncing #{attribute} users for provider '#{provider}'. LDAP connection Error")
false false
end end
...@@ -64,13 +64,13 @@ module EE ...@@ -64,13 +64,13 @@ module EE
user user
else else
Rails.logger.debug do # rubocop:disable Gitlab/RailsLogger ::Gitlab::AppLogger.debug(
<<-MSG.strip_heredoc.tr("\n", ' ') <<-MSG.strip_heredoc.tr("\n", ' ')
#{self.class.name}: User with DN `#{member_dn}` should be marked as #{self.class.name}: User with DN `#{member_dn}` should be marked as
#{attribute} but there is no user in GitLab with that identity. #{attribute} but there is no user in GitLab with that identity.
Membership will be updated once the user signs in for the first time. Membership will be updated once the user signs in for the first time.
MSG MSG
end )
nil nil
end end
......
...@@ -61,7 +61,7 @@ RSpec.describe EE::Gitlab::Auth::Ldap::Sync::AdminUsers do ...@@ -61,7 +61,7 @@ RSpec.describe EE::Gitlab::Auth::Ldap::Sync::AdminUsers do
end end
it 'logs a debug message' do it 'logs a debug message' do
expect(Rails.logger) expect(Gitlab::AppLogger)
.to receive(:warn) .to receive(:warn)
.with("Error syncing admin users for provider 'ldapmain'. LDAP connection Error") .with("Error syncing admin users for provider 'ldapmain'. LDAP connection Error")
.at_least(:once) .at_least(:once)
......
...@@ -62,7 +62,7 @@ RSpec.describe EE::Gitlab::Auth::Ldap::Sync::ExternalUsers do ...@@ -62,7 +62,7 @@ RSpec.describe EE::Gitlab::Auth::Ldap::Sync::ExternalUsers do
end end
it 'logs a debug message' do it 'logs a debug message' do
expect(Rails.logger) expect(Gitlab::AppLogger)
.to receive(:warn) .to receive(:warn)
.with("Error syncing external users for provider 'ldapmain'. LDAP connection Error") .with("Error syncing external users for provider 'ldapmain'. LDAP connection Error")
.at_least(:once) .at_least(:once)
......
...@@ -42,7 +42,7 @@ RSpec.describe EE::Gitlab::Auth::Ldap::Sync::Group do ...@@ -42,7 +42,7 @@ RSpec.describe EE::Gitlab::Auth::Ldap::Sync::Group do
it 'logs a debug message' do it 'logs a debug message' do
group.start_ldap_sync group.start_ldap_sync
expect(Rails.logger) expect(Gitlab::AppLogger)
.to receive(:warn) .to receive(:warn)
.with(/^Group '\w*' is not ready for LDAP sync. Skipping/) .with(/^Group '\w*' is not ready for LDAP sync. Skipping/)
.at_least(:once) .at_least(:once)
...@@ -67,7 +67,7 @@ RSpec.describe EE::Gitlab::Auth::Ldap::Sync::Group do ...@@ -67,7 +67,7 @@ RSpec.describe EE::Gitlab::Auth::Ldap::Sync::Group do
end end
it 'logs a debug message' do it 'logs a debug message' do
expect(Rails.logger) expect(Gitlab::AppLogger)
.to receive(:warn).at_least(:once) .to receive(:warn).at_least(:once)
execute execute
...@@ -187,7 +187,7 @@ RSpec.describe EE::Gitlab::Auth::Ldap::Sync::Group do ...@@ -187,7 +187,7 @@ RSpec.describe EE::Gitlab::Auth::Ldap::Sync::Group do
it 'does not update permissions unless ldap sync status is started' do it 'does not update permissions unless ldap sync status is started' do
group.finish_ldap_sync group.finish_ldap_sync
expect(Rails.logger) expect(Gitlab::AppLogger)
.to receive(:warn).with(/status must be 'started' before updating permissions/) .to receive(:warn).with(/status must be 'started' before updating permissions/)
sync_group.update_permissions sync_group.update_permissions
...@@ -611,7 +611,7 @@ RSpec.describe EE::Gitlab::Auth::Ldap::Sync::Group do ...@@ -611,7 +611,7 @@ RSpec.describe EE::Gitlab::Auth::Ldap::Sync::Group do
it 'does not update permissions unless ldap sync status is started' do it 'does not update permissions unless ldap sync status is started' do
group.finish_ldap_sync group.finish_ldap_sync
expect(Rails.logger) expect(Gitlab::AppLogger)
.to receive(:warn).with(/status must be 'started' before updating permissions/) .to receive(:warn).with(/status must be 'started' before updating permissions/)
sync_group.update_permissions sync_group.update_permissions
......
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