Pull mirroring with SSH authentication may not update every branch
Summary
Noticed while working on gitaly-proto!236 (merged) <- https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/8116
Gitaly has a RefService.FindAllRemoteBranches RPC. This is used, not in push mirroring as I originally thought, but in pull mirroring, to create local branches matching every branch in the remote repository we are pulling from.
There is no support for SSH authentication in this Gitaly RPC, so if the pull mirror is configured with SSH authentication, this code will never run:
def update_branches
local_branches = repository.branches.each_with_object({}) { |branch, branches| branches[branch.name] = branch }
errors = []
repository.upstream_branches.each do |upstream_branch|
name = upstream_branch.name
next if skip_branch?(name)
local_branch = local_branches[name]
if local_branch.nil?
result = CreateBranchService.new(project, current_user).execute(name, upstream_branch.dereferenced_target.sha, create_master_if_empty: false)
if result[:status] == :error
errors << result[:message]
end
elsif local_branch.dereferenced_target == upstream_branch.dereferenced_target
# Already up to date
elsif repository.diverged_from_upstream?(name)
handle_diverged_branch(upstream_branch, local_branch, name, errors)
else
begin
repository.ff_merge(current_user, upstream_branch.dereferenced_target, name)
rescue Gitlab::Git::PreReceiveError, Gitlab::Git::CommitError => e
errors << e.message
end
end
end
unless errors.empty?
raise UpdateError, errors.join("\n\n")
end
end(The call to upstream_branches there eventually becomes the FetchAllRemoteBranches RPC call, with a ref_name of upstream`)
I'm not actually sure what the result of skipping this code is, offhand, but I suspect it means we won't get new branches, tags, etc?
Perhaps you know more about what the impact would be @tiagonbotelho ?
Does the FetchRemote call update (some of) these for us already? Is this code actually unnecessary?
Steps to reproduce
- Create a pull mirror with SSH authentication
- ???
What is the current bug behavior?
Needs investigation, but we're making an unauthenticated request to a remote when we should be making an authenticated request
What is the expected correct behavior?
We should perform requests to remotes with appropriate credentials
Possible fixes
/cc @jramsay