Skip to content

Commit ec01acd

Browse files
committed
Better handling of projects with a disabled repository module.
We now treat this case exactly the same as an archived project/repository. Before, it was handled inconsistently. Three changes: 1) When a repository module is disabled/enabled through settings page call update_repositories to update state. A disabling of the repository module will call update_repositories with :archive. 2) Existing repositories will be left alone and marked with a dummy redmine key of "redmine_disabled_project". Reenabling of the repository module will restore project and keys to accessibility. 3) When removing redmine keys from a gitolite.conf entry, don't report in the log unless removing real readmine keys. This avoids a weird circumstance in which every fetch_changesets operation reports that it is deleting redmine keys (caused when repo entry exists but the "delete_repositories" flag is disabled at the time that the repo is deleted. This fix is entirely about removing confusing messages from the log.
1 parent 44cec8e commit ec01acd

File tree

3 files changed

+75
-40
lines changed

3 files changed

+75
-40
lines changed

Diff for: lib/git_hosting.rb

+48-39
Original file line numberDiff line numberDiff line change
@@ -662,20 +662,20 @@ def self.update_repositories(*args)
662662
args.each {|arg| flags.merge!(arg) if arg.is_a?(Hash)}
663663
if flags[:resync_all]
664664
logger.info "Executing RESYNC_ALL operation on gitolite configuration"
665-
projects = Project.active_or_archived.has_module(:repository).find(:all, :include => :repository)
665+
projects = Project.active_or_archived.find(:all, :include => :repository)
666666
elsif flags[:delete]
667667
# When delete, want to recompute users, so need to go through all projects
668668
logger.info "Executing DELETE operation (resync keys, remove dead repositories)"
669-
projects = Project.active_or_archived.has_module(:repository).find(:all, :include => :repository)
669+
projects = Project.active_or_archived.find(:all, :include => :repository)
670670
elsif flags[:archive]
671671
# When archive, want to recompute users, so need to go through all projects
672672
logger.info "Executing ARCHIVE operation (remove keys)"
673-
projects = Project.active_or_archived.has_module(:repository).find(:all, :include => :repository)
673+
projects = Project.active_or_archived.find(:all, :include => :repository)
674674
elsif flags[:descendants]
675675
if Project.method_defined?(:self_and_descendants)
676676
projects = (args.flatten.select{|p| p.is_a?(Project)}).collect{|p| p.self_and_descendants}.flatten
677677
else
678-
projects = Project.active_or_archived.has_module(:repository).find(:all, :include => :repository)
678+
projects = Project.active_or_archived.find(:all, :include => :repository)
679679
end
680680
else
681681
projects = args.flatten.select{|p| p.is_a?(Project)}
@@ -719,7 +719,7 @@ def self.update_repositories(*args)
719719
end
720720

721721
# Collect relevant users into hash with user as key and activity (in some active project) as value
722-
(git_projects.select{|proj| proj.active?}.map{|proj| proj.member_principals.map(&:user).compact}.flatten.uniq << GitolitePublicKey::DEPLOY_PSEUDO_USER).each do |cur_user|
722+
(git_projects.select{|proj| proj.active? && proj.module_enabled?(:repository)}.map{|proj| proj.member_principals.map(&:user).compact}.flatten.uniq << GitolitePublicKey::DEPLOY_PSEUDO_USER).each do |cur_user|
723723
if cur_user == GitolitePublicKey::DEPLOY_PSEUDO_USER
724724
active_keys = DeploymentCredential.active.select(&:honored?).map(&:gitolite_public_key).uniq
725725
cur_token = cur_user
@@ -882,41 +882,47 @@ def self.update_repositories(*args)
882882

883883
# If this is an active (non-archived) project, then update gitolite entry. Add GIT_DAEMON_KEY.
884884
if proj.active?
885-
# Get deployment keys (could be empty)
886-
write_user_keys = myrepo.deployment_credentials.active.select{|cred| cred.honored? && cred.allowed_to?(:commit_access)}.map{|x| x.gitolite_public_key.identifier}
887-
read_user_keys = myrepo.deployment_credentials.active.select{|cred| cred.honored? && cred.allowed_to?(:view_changesets) && !cred.allowed_to?(:commit_access)}.map{|x| x.gitolite_public_key.identifier}
888-
889-
# fetch users
890-
users = proj.member_principals.map(&:user).compact.uniq
891-
write_users = users.select{ |user| user.allowed_to?( :commit_access, proj ) }
892-
read_users = users.select{ |user| user.allowed_to?( :view_changesets, proj ) && !user.allowed_to?( :commit_access, proj ) }
893-
894-
read_users.map{|u| u.gitolite_public_keys.active.user_key}.flatten.compact.uniq.each do |key|
895-
read_user_keys.push key.identifier
896-
end
897-
write_users.map{|u| u.gitolite_public_keys.active.user_key}.flatten.compact.uniq.each do |key|
898-
write_user_keys.push key.identifier
899-
end
900-
901-
#git daemon support
902-
if (proj.repository.extra.git_daemon == 1 || proj.repository.extra.git_daemon == nil ) && proj.is_public
903-
read_user_keys.push GitoliteConfig::GIT_DAEMON_KEY
904-
end
885+
if proj.module_enabled?(:repository)
886+
# Get deployment keys (could be empty)
887+
write_user_keys = myrepo.deployment_credentials.active.select{|cred| cred.honored? && cred.allowed_to?(:commit_access)}.map{|x| x.gitolite_public_key.identifier}
888+
read_user_keys = myrepo.deployment_credentials.active.select{|cred| cred.honored? && cred.allowed_to?(:view_changesets) && !cred.allowed_to?(:commit_access)}.map{|x| x.gitolite_public_key.identifier}
889+
890+
# fetch users
891+
users = proj.member_principals.map(&:user).compact.uniq
892+
write_users = users.select{ |user| user.allowed_to?( :commit_access, proj ) }
893+
read_users = users.select{ |user| user.allowed_to?( :view_changesets, proj ) && !user.allowed_to?( :commit_access, proj ) }
894+
895+
read_users.map{|u| u.gitolite_public_keys.active.user_key}.flatten.compact.uniq.each do |key|
896+
read_user_keys.push key.identifier
897+
end
898+
write_users.map{|u| u.gitolite_public_keys.active.user_key}.flatten.compact.uniq.each do |key|
899+
write_user_keys.push key.identifier
900+
end
905901

906-
# Remove previous redmine keys, then add new keys
907-
# By doing things this way, we leave non-redmine keys alone
908-
# Note -- delete_redmine_keys() will also remove the GIT_DAEMON_KEY for repos with redmine keys
909-
# (to be put back as above, when appropriate).
910-
conf.delete_redmine_keys repo_name
911-
conf.add_read_user repo_name, read_user_keys.uniq
912-
conf.add_write_user repo_name, write_user_keys.uniq
902+
#git daemon support
903+
if (proj.repository.extra.git_daemon == 1 || proj.repository.extra.git_daemon == nil ) && proj.is_public
904+
read_user_keys.push GitoliteConfig::GIT_DAEMON_KEY
905+
end
913906

914-
# If no redmine keys, mark with dummy key
915-
if (read_user_keys+write_user_keys).empty?
916-
conf.mark_with_dummy_key repo_name
907+
# Remove previous redmine keys, then add new keys
908+
# By doing things this way, we leave non-redmine keys alone
909+
# Note -- delete_redmine_keys() will also remove the GIT_DAEMON_KEY for repos with redmine keys
910+
# (to be put back as above, when appropriate).
911+
conf.delete_redmine_keys repo_name
912+
conf.add_read_user repo_name, read_user_keys.uniq
913+
conf.add_write_user repo_name, write_user_keys.uniq
914+
915+
# If no redmine keys, mark with dummy key
916+
if (read_user_keys+write_user_keys).empty?
917+
conf.mark_with_dummy_key repo_name
918+
end
919+
else
920+
# Must be a project that has repositories disabled. Mark as disabled project.
921+
conf.delete_redmine_keys repo_name
922+
conf.mark_disabled repo_name
917923
end
918924
else
919-
# Must be an archived project! Clear out redmine keys. Mark as an archived project.
925+
# Must be an archived project! Clear out redmine keys. Mark as an archived project.
920926
conf.delete_redmine_keys repo_name
921927
conf.mark_archived repo_name
922928
end
@@ -938,20 +944,23 @@ def self.update_repositories(*args)
938944
redmine_repos.delete_if{|basename,values| proj_ids.index(basename)}
939945
end
940946
redmine_repos.values.flatten.each do |repo_name|
941-
# First, delete redmine keys for this repository
947+
# First, check if there are any redmine keys other than the DUMMY or ARCHIVED key
948+
has_keys = conf.has_actual_redmine_keys? repo_name
949+
950+
# Next, delete redmine keys for this repository
942951
conf.delete_redmine_keys repo_name
943952
if (Setting.plugin_redmine_git_hosting['deleteGitRepositories'] == "true")
944953
if conf.repo_has_no_keys? repo_name
945954
logger.warn "Deleting #{orphanString}entry '#{repo_name}' from #{gitolite_conf}"
946955
conf.delete_repo repo_name
947956
GitoliteRecycle.move_repository_to_recycle repo_name
948-
else
957+
elsif has_keys # Something changed when we deleted keys
949958
logger.info "Deleting redmine keys from #{orphanString}entry '#{repo_name}' in #{gitolite_conf}"
950959
if git_repository_exists? repo_name
951960
logger.info " Not removing #{repo_name}.git from gitolite repository, because non-redmine keys remain."
952961
end
953962
end
954-
else
963+
elsif has_keys # Something changed when we deleted keys
955964
logger.info "Deleting redmine keys from #{orphanString}entry '#{repo_name}' in #{gitolite_conf}"
956965
end
957966
end

Diff for: lib/git_hosting/patches/projects_controller_patch.rb

+16
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,21 @@ def unarchive_with_disable_update
108108
GitHostingObserver.set_update_active(@project);
109109
end
110110

111+
def settings_with_disable_update
112+
# Turn of updates during repository update
113+
GitHostingObserver.set_update_active(false);
114+
115+
# Do actual update
116+
settings_without_disable_update
117+
118+
# Reenable updates to perform a single update
119+
if @project.module_enabled?(:repository)
120+
GitHostingObserver.set_update_active(@project);
121+
else
122+
GitHostingObserver.set_update_active(:archive);
123+
end
124+
end
125+
111126
def self.included(base)
112127
base.class_eval do
113128
unloadable
@@ -117,6 +132,7 @@ def self.included(base)
117132
base.send(:alias_method_chain, :destroy, :disable_update)
118133
base.send(:alias_method_chain, :archive, :disable_update)
119134
base.send(:alias_method_chain, :unarchive, :disable_update)
135+
base.send(:alias_method_chain, :settings, :disable_update)
120136
end
121137
end
122138
end

Diff for: lib/gitolite_conf.rb

+11-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ module GitHosting
22
class GitoliteConfig
33
DUMMY_REDMINE_KEY="redmine_dummy_key"
44
ARCHIVED_REDMINE_KEY="redmine_archived_project"
5+
DISABLED_REDMINE_KEY="redmine_disabled_project"
56
GIT_DAEMON_KEY="daemon"
67
ADMIN_REPO = "gitolite-admin"
78
PRIMARY_CONF_FILE = "gitolite.conf"
@@ -64,6 +65,10 @@ def mark_archived repo_name
6465
add_read_user repo_name, [ARCHIVED_REDMINE_KEY]
6566
end
6667

68+
def mark_disabled repo_name
69+
add_read_user repo_name, [DISABLED_REDMINE_KEY]
70+
end
71+
6772
# Grab admin key (assuming it exists)
6873
def get_admin_key
6974
return (repository(ADMIN_REPO).get "RW+").first
@@ -91,6 +96,11 @@ def is_redmine_repo? repo_name
9196
repository(repo_name).rights.detect {|perm, users| users.detect {|key| is_redmine_key? key}} || (repo_has_no_keys? repo_name)
9297
end
9398

99+
# Return true if repository has any redmine keys other than the DUMMY_REDMINE_KEY
100+
def has_actual_redmine_keys? repo_name
101+
repository(repo_name).rights.detect {|perm, users| users.detect {|key| GitolitePublicKey::ident_to_user_token(key)}}
102+
end
103+
94104
# Delete all of the redmine keys from a repository
95105
# In addition, if there are any redmine keys, delete the GIT_DAEMON_KEY as well,
96106
# since we assume this under control of redmine server.
@@ -107,7 +117,7 @@ def repo_has_no_keys? repo_name
107117
end
108118

109119
def is_redmine_key? keyname
110-
(GitolitePublicKey::ident_to_user_token(keyname) || keyname == DUMMY_REDMINE_KEY || keyname == ARCHIVED_REDMINE_KEY) ? true : false
120+
(GitolitePublicKey::ident_to_user_token(keyname) || keyname == DUMMY_REDMINE_KEY || keyname == ARCHIVED_REDMINE_KEY || keyname == DISABLED_REDMINE_KEY) ? true : false
111121
end
112122

113123
def is_daemon_key? keyname

0 commit comments

Comments
 (0)