Merge branch 'master' into preselected_milestone_while_create_new_issue

app/controllers/application_controller.rb

@@ -14,10 +14,6 @@ class ApplicationController < ActionController::Base
     render "errors/gitolite", layout: "error"
   end
 
-  rescue_from Gitlab::Gitolite::InvalidKey do |exception|
-    render "errors/invalid_ssh_key", layout: "error"
-  end
-
   rescue_from Encoding::CompatibilityError do |exception|
     render "errors/encoding", layout: "error", status: 404
   end

app/views/errors/invalid_ssh_key.html.haml

-%h1 Git Error
-%hr
-%p Seems like SSH Key you provided is not a valid SSH key.

lib/gitlab/backend/gitolite.rb

-require 'gitolite'
-require 'timeout'
-require 'fileutils'
+require_relative 'gitolite_config'
 
-# TODO: refactor & cleanup 
 module Gitlab
   class Gitolite
     class AccessDenied < StandardError; end
-    class InvalidKey < StandardError; end
+
+    def config
+      Gitlab::GitoliteConfig.new
+    end
 
     def set_key key_id, key_content, projects
-      configure do |c|
-        c.update_keys(key_id, key_content)
-        c.update_projects(projects)
+      config.apply do |config|
+        config.write_key(key_id, key_content)
+        config.update_projects(projects)
       end
     end
 
     def remove_key key_id, projects
-      configure do |c|
-        c.delete_key(key_id)
-        c.update_projects(projects)
+      config.apply do |config|
+        config.rm_key(key_id)
+        config.update_projects(projects)
       end
     end
 
     def update_repository project
-      configure do |c|
-        c.update_project(project.path, project)
+      config.update_project!(project.path, project)
     end
-    end
-
-    alias_method :create_repository, :update_repository
 
     def remove_repository project
-      configure do |c|
-        c.destroy_project(project)
-      end
+      config.destroy_project!(project)
     end
 
     def url_to_repo path
       Gitlab.config.ssh_path + "#{path}.git"
     end
 
-    def initialize
-      # create tmp dir
-      @local_dir = File.join(Rails.root, 'tmp',"gitlabhq-gitolite-#{Time.now.to_i}")
-    end
-
     def enable_automerge
-      configure do |git|
-        git.admin_all_repo
-      end
-    end
-
-    protected
-
-    def destroy_project(project)
-      FileUtils.rm_rf(project.path_to_repo)
-
-      ga_repo = ::Gitolite::GitoliteAdmin.new(File.join(@local_dir,'gitolite'))
-      conf = ga_repo.config
-      conf.rm_repo(project.path)
-      ga_repo.save
-    end
-
-    #update or create
-    def update_keys(user, key)
-      File.open(File.join(@local_dir, 'gitolite/keydir',"#{user}.pub"), 'w') {|f| f.write(key.gsub(/\n/,'')) }
+      config.admin_all_repo!(project)
     end
 
-    def delete_key(user)
-      File.unlink(File.join(@local_dir, 'gitolite/keydir',"#{user}.pub"))
-      `cd #{File.join(@local_dir,'gitolite')} ; git rm keydir/#{user}.pub`
-    end
-
-    # update or create
-    def update_project(repo_name, project)
-      ga_repo = ::Gitolite::GitoliteAdmin.new(File.join(@local_dir,'gitolite'))
-      conf = ga_repo.config
-      repo = update_project_config(project, conf)
-      conf.add_repo(repo, true)
-      
-      ga_repo.save
-    end
-
-    # Updates many projects and uses project.path as the repo path
-    # An order of magnitude faster than update_project
-    def update_projects(projects)
-      ga_repo = ::Gitolite::GitoliteAdmin.new(File.join(@local_dir,'gitolite'))
-      conf = ga_repo.config
-
-      projects.each do |project|
-        repo = update_project_config(project, conf)
-        conf.add_repo(repo, true)
-      end
-
-      ga_repo.save
-    end
-
-    def update_project_config(project, conf)
-      repo_name = project.path
-
-      repo = if conf.has_repo?(repo_name)
-               conf.get_repo(repo_name)
-             else 
-               ::Gitolite::Config::Repo.new(repo_name)
-             end
-
-      name_readers = project.repository_readers
-      name_writers = project.repository_writers
-      name_masters = project.repository_masters
-
-      pr_br = project.protected_branches.map(&:name).join("$ ")
-
-      repo.clean_permissions
-
-      # Deny access to protected branches for writers
-      unless name_writers.blank? || pr_br.blank?
-        repo.add_permission("-", pr_br.strip + "$ ", name_writers)
-      end
-
-      # Add read permissions
-      repo.add_permission("R", "", name_readers) unless name_readers.blank?
-
-      # Add write permissions
-      repo.add_permission("RW+", "", name_writers) unless name_writers.blank?
-      repo.add_permission("RW+", "", name_masters) unless name_masters.blank?
-
-      repo
-    end
-
-    def admin_all_repo
-      ga_repo = ::Gitolite::GitoliteAdmin.new(File.join(@local_dir,'gitolite'))
-      conf = ga_repo.config
-      owner_name = ""
-
-      # Read gitolite-admin user
-      #
-      begin 
-        repo = conf.get_repo("gitolite-admin")
-        owner_name = repo.permissions[0]["RW+"][""][0]
-        raise StandardError if owner_name.blank?
-      rescue => ex
-        puts "Can't determine gitolite-admin owner".red
-        raise StandardError
-      end
-
-      # @ALL repos premission for gitolite owner
-      repo_name = "@all"
-      repo = if conf.has_repo?(repo_name)
-               conf.get_repo(repo_name)
-             else 
-               ::Gitolite::Config::Repo.new(repo_name)
-             end
-
-      repo.add_permission("RW+", "", owner_name)
-      conf.add_repo(repo, true)
-      ga_repo.save
-    end
-
-    private
-
-    def pull
-      # create tmp dir
-      @local_dir = File.join(Rails.root, 'tmp',"gitlabhq-gitolite-#{Time.now.to_i}")
-      Dir.mkdir @local_dir
-
-      `git clone #{Gitlab.config.gitolite_admin_uri} #{@local_dir}/gitolite`
-    end
-
-    def push
-      Dir.chdir(File.join(@local_dir, "gitolite"))
-      `git add -A`
-      `git commit -am "GitLab"`
-      `git push`
-      Dir.chdir(Rails.root)
-
-      FileUtils.rm_rf(@local_dir)
-    end
-
-    def configure
-      Timeout::timeout(30) do
-        File.open(File.join(Rails.root, 'tmp', "gitlabhq-gitolite.lock"), "w+") do |f|
-          begin 
-            f.flock(File::LOCK_EX)
-            pull
-            yield(self)
-            push
-          ensure
-            f.flock(File::LOCK_UN)
-          end
-        end
-      end
-    rescue Exception => ex
-      if ex.message =~ /is not a valid SSH key string/
-        raise Gitolite::InvalidKey.new("ssh key is not valid")
-      else
-        Gitlab::Logger.error(ex.message)
-        raise Gitolite::AccessDenied.new("gitolite timeout")
-      end
-    end
+    alias_method :create_repository, :update_repository
   end
 end

lib/gitlab/backend/gitolite_config.rb

+require 'gitolite'
+require 'timeout'
+require 'fileutils'
+
+module Gitlab
+  class GitoliteConfig
+    attr_reader :config_tmp_dir
+
+    def reset_config_tmp_dir
+      @config_tmp_dir = File.join(Rails.root, 'tmp',"gitlabhq-gitolite-#{Time.now.to_i}")
+    end
+
+    def apply
+      Timeout::timeout(30) do
+        File.open(File.join(Rails.root, 'tmp', "gitlabhq-gitolite.lock"), "w+") do |f|
+          begin
+            f.flock(File::LOCK_EX)
+            reset_config_tmp_dir
+            pull
+            yield(self)
+            push
+            FileUtils.rm_rf(config_tmp_dir)
+          ensure
+            f.flock(File::LOCK_UN)
+          end
+        end
+      end
+    rescue Exception => ex
+      Gitlab::Logger.error(ex.message)
+      raise Gitolite::AccessDenied.new("gitolite timeout")
+    end
+
+    def destroy_project(project)
+      FileUtils.rm_rf(project.path_to_repo)
+
+      ga_repo = ::Gitolite::GitoliteAdmin.new(File.join(config_tmp_dir,'gitolite'))
+      conf = ga_repo.config
+      conf.rm_repo(project.path)
+      ga_repo.save
+    end
+
+    def destroy_project!(project)
+      apply do |config|
+        config.destroy_project(project)
+      end
+    end
+
+    def write_key(id, key)
+      File.open(File.join(config_tmp_dir, 'gitolite/keydir',"#{id}.pub"), 'w') do |f|
+        f.write(key.gsub(/\n/,''))
+      end
+    end
+
+    def rm_key(user)
+      File.unlink(File.join(config_tmp_dir, 'gitolite/keydir',"#{user}.pub"))
+      `cd #{File.join(config_tmp_dir,'gitolite')} ; git rm keydir/#{user}.pub`
+    end
+
+    # update or create
+    def update_project(repo_name, project)
+      ga_repo = ::Gitolite::GitoliteAdmin.new(File.join(config_tmp_dir,'gitolite'))
+      conf = ga_repo.config
+      repo = update_project_config(project, conf)
+      conf.add_repo(repo, true)
+
+      ga_repo.save
+    end
+
+    def update_project!(repo_name, project)
+      apply do |config|
+        config.update_project(repo_name, project)
+      end
+    end
+
+    # Updates many projects and uses project.path as the repo path
+    # An order of magnitude faster than update_project
+    def update_projects(projects)
+      ga_repo = ::Gitolite::GitoliteAdmin.new(File.join(config_tmp_dir,'gitolite'))
+      conf = ga_repo.config
+
+      projects.each do |project|
+        repo = update_project_config(project, conf)
+        conf.add_repo(repo, true)
+      end
+
+      ga_repo.save
+    end
+
+    def update_project_config(project, conf)
+      repo_name = project.path
+
+      repo = if conf.has_repo?(repo_name)
+               conf.get_repo(repo_name)
+             else
+               ::Gitolite::Config::Repo.new(repo_name)
+             end
+
+      name_readers = project.repository_readers
+      name_writers = project.repository_writers
+      name_masters = project.repository_masters
+
+      pr_br = project.protected_branches.map(&:name).join("$ ")
+
+      repo.clean_permissions
+
+      # Deny access to protected branches for writers
+      unless name_writers.blank? || pr_br.blank?
+        repo.add_permission("-", pr_br.strip + "$ ", name_writers)
+      end
+
+      # Add read permissions
+      repo.add_permission("R", "", name_readers) unless name_readers.blank?
+
+      # Add write permissions
+      repo.add_permission("RW+", "", name_writers) unless name_writers.blank?
+      repo.add_permission("RW+", "", name_masters) unless name_masters.blank?
+
+      repo
+    end
+
+    def admin_all_repo
+      ga_repo = ::Gitolite::GitoliteAdmin.new(File.join(config_tmp_dir,'gitolite'))
+      conf = ga_repo.config
+      owner_name = ""
+
+      # Read gitolite-admin user
+      #
+      begin
+        repo = conf.get_repo("gitolite-admin")
+        owner_name = repo.permissions[0]["RW+"][""][0]
+        raise StandardError if owner_name.blank?
+      rescue => ex
+        puts "Can't determine gitolite-admin owner".red
+        raise StandardError
+      end
+
+      # @ALL repos premission for gitolite owner
+      repo_name = "@all"
+      repo = if conf.has_repo?(repo_name)
+               conf.get_repo(repo_name)
+             else
+               ::Gitolite::Config::Repo.new(repo_name)
+             end
+
+      repo.add_permission("RW+", "", owner_name)
+      conf.add_repo(repo, true)
+      ga_repo.save
+    end
+
+    def admin_all_repo!
+      apply { |config| config.admin_all_repo }
+    end
+
+    private
+
+    def pull
+      Dir.mkdir config_tmp_dir
+      `git clone #{Gitlab.config.gitolite_admin_uri} #{config_tmp_dir}/gitolite`
+    end
+
+    def push
+      Dir.chdir(File.join(config_tmp_dir, "gitolite"))
+      `git add -A`
+      `git commit -am "GitLab"`
+      `git push`
+      Dir.chdir(Rails.root)
+    end
+  end
+end
+

spec/lib/gitolite_config_spec.rb

+require 'spec_helper'
+
+describe Gitlab::GitoliteConfig do
+  let(:gitolite) { Gitlab::GitoliteConfig.new }
+
+  it { should respond_to :write_key }
+  it { should respond_to :rm_key }
+  it { should respond_to :update_project }
+  it { should respond_to :update_project! }
+  it { should respond_to :update_projects }
+  it { should respond_to :destroy_project }
+  it { should respond_to :destroy_project! }
+  it { should respond_to :apply }
+  it { should respond_to :admin_all_repo }
+  it { should respond_to :admin_all_repo! }
+end

spec/lib/gitolite_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Gitolite do
+  let(:project) { double('Project', path: 'diaspora') }
+  let(:gitolite_config) { double('Gitlab::GitoliteConfig') }
+  let(:gitolite) { Gitlab::Gitolite.new }
+
+  before do
+    gitolite.stub(config: gitolite_config)
+  end
+
+  it { should respond_to :set_key }
+  it { should respond_to :remove_key }
+
+  it { should respond_to :update_repository }
+  it { should respond_to :create_repository }
+  it { should respond_to :remove_repository }
+
+  it { gitolite.url_to_repo('diaspora').should == Gitlab.config.ssh_path + "diaspora.git" }
+
+  it "should call config update" do
+    gitolite_config.should_receive(:update_project!)
+    gitolite.update_repository project
+  end
+end

spec/support/gitolite_stub.rb

@@ -27,9 +27,21 @@ module GitoliteStub
   end
 
   def stub_gitlab_gitolite
-    gitlab_gitolite = Gitlab::Gitolite.new
-    Gitlab::Gitolite.stub(new: gitlab_gitolite)
-    gitlab_gitolite.stub(configure: ->() { yield(self) })
-    gitlab_gitolite.stub(update_keys: true)
+    gitolite_config = double('Gitlab::GitoliteConfig')
+    gitolite_config.stub(
+      apply: ->() { yield(self) },
+      write_key: true,
+      rm_key: true,
+      update_projects: true,
+      update_project: true,
+      update_project!: true,
+      destroy_project: true,
+      destroy_project!: true,
+      admin_all_repo: true,
+      admin_all_repo!: true,
+
+    )
+
+    Gitlab::GitoliteConfig.stub(new: gitolite_config)
   end
 end