Skip to content

Make sure that keys are deleted.

gitlab-qa-bot requested to merge github/fork/dosire/delete_ssh_keys into master

Created by: dosire

This solves a problem when deleting ssh keys. The problem is that the save function restored keys that had to be deleted. This commit fixes it by not calling save when a key is deleted.

For reference here are the steps of the problematic original situation:

The key observer after_destroy in Gitlab calls remove_key in gitolite_config.rb https://github.com/gitlabhq/gitlabhq/blob/master/lib/gitlab/backend/gitolite.rb#L18

The remove_key is wrapped in the the apply method in gitolite_config.rb that sets lock on the repo, pulls the gitolite-admin repo and calls ga_repo.config. This calls ::Gitolite::GitoliteAdmin.new. This calls initialize, which has load_data, this calls load_keys, which calls list_keys which sets @ssh_keys to the contents of the working copy (!!! which still contains the ssh key we will remove later!!!!). Then apply yields to remove_key that it wraps. https://github.com/gitlabhq/gitlabhq/blob/master/lib/gitlab/backend/gitolite_config.rb#L17 https://github.com/gitlabhq/gitlabhq/blob/master/lib/gitlab/backend/gitolite_config.rb#L20

The remove_key calls the rm_key method https://github.com/gitlabhq/gitlabhq/blob/master/lib/gitlab/backend/gitolite_config.rb#L98

The rm_key methods calls File.unlink that removes the .pub file

The rm_key then removes the user(key) from the gitolite-admin repo

The remove_key calls update_projects which updates the project configuration. At this point we have a key that is removed and changes set on staging.

    # On branch master
    # Changes to be committed:
    #   (use "git reset HEAD <file>..." to unstage)
    #
    # deleted:    keydir/admin_local_host_1349087204.pub
    #

Apply continues and calls ga_repo.save to save the changes to the repo before commiting the changes

@gl_admin.remove is called with all keys that exist in the file system now but didn't exist on the filesystem on initialization (@ssh_keys), this is only usefull if you removed the keys from @ssh_keys explicitly. This is done in https://github.com/gitlabhq/gitolite-client/blob/master/lib/gitolite/gitolite_admin.rb#L141

In ga_repo.save all the keys that existed in the working copy on initialization that where stored in @ssh_keys are written to the working copy with to_file and added again to the staging area @gl_admin.add. This cancels out the staging of the deleted file. https://github.com/gitlabhq/gitolite-client/blob/master/lib/gitolite/gitolite_admin.rb#L81

  # On branch master
     nothing to commit (working directory clean)

Apply calls the push method that adds the changed, deleted and untracked files system('git add -A') and commits (there is nothing to commit) and nothing is being pushed. https://github.com/gitlabhq/gitlabhq/blob/master/lib/gitlab/backend/gitolite_config.rb#L188

The removed key is still in the working copy (/home/git/.gitolite/keydir/x.pub) and in the repo.

Merge request reports