Commit 0a9f9c10 authored by Robert Speicher's avatar Robert Speicher
Browse files

Merge branch '22229-use-base-sha-when-downloading-merge-requests' into 'master'

Use base SHA for patches and diffs

## What does this MR do?

Switch from using 'start SHA' to 'base SHA' for patches and diffs

## Are there points in the code the reviewer needs to double check?

## Why was this MR needed?

Makes the downloaded patches and diffs on the merge request page match the frontend-rendered "changes" in these scenarios:

* Unpatched gitlab-workhorse, downloading patchsets of open MRs (https://gitlab.com/gitlab-org/gitlab-workhorse/merge_requests/68)
* Unpatched gitlab-workhorse, downloading diffs of open and merged MRs
* Patched gitlab-workhorse, downloading patchsets of merged merge requests

## What are the relevant issue numbers?

Closes #22229

See merge request !6435
......@@ -60,7 +60,7 @@ def send_git_archive(repository, ref:, format:)
def send_git_diff(repository, diff_refs)
params = {
'RepoPath' => repository.path_to_repo,
'ShaFrom' => diff_refs.start_sha,
'ShaFrom' => diff_refs.base_sha,
'ShaTo' => diff_refs.head_sha
}
......@@ -73,7 +73,7 @@ def send_git_diff(repository, diff_refs)
def send_git_patch(repository, diff_refs)
params = {
'RepoPath' => repository.path_to_repo,
'ShaFrom' => diff_refs.start_sha,
'ShaFrom' => diff_refs.base_sha,
'ShaTo' => diff_refs.head_sha
}
......@@ -107,15 +107,15 @@ def secret
bytes
end
end
def write_secret
bytes = SecureRandom.random_bytes(SECRET_LENGTH)
File.open(secret_path, 'w:BINARY', 0600) do |f|
File.open(secret_path, 'w:BINARY', 0600) do |f|
f.chmod(0600)
f.write(Base64.strict_encode64(bytes))
end
end
def verify_api_request!(request_headers)
JWT.decode(
request_headers[INTERNAL_API_REQUEST_HEADER],
......@@ -128,7 +128,7 @@ def verify_api_request!(request_headers)
def secret_path
Rails.root.join('.gitlab_workhorse_secret')
end
protected
def encode(hash)
......
require 'spec_helper'
describe Gitlab::Workhorse, lib: true do
let(:project) { create(:project) }
let(:subject) { Gitlab::Workhorse }
let(:project) { create(:project) }
let(:repository) { project.repository }
def decode_workhorse_header(array)
key, value = array
command, encoded_params = value.split(":")
params = JSON.parse(Base64.urlsafe_decode64(encoded_params))
[key, command, params]
end
describe ".send_git_archive" do
context "when the repository doesn't have an archive file path" do
......@@ -11,11 +19,37 @@
end
it "raises an error" do
expect { subject.send_git_archive(project.repository, ref: "master", format: "zip") }.to raise_error(RuntimeError)
expect { described_class.send_git_archive(project.repository, ref: "master", format: "zip") }.to raise_error(RuntimeError)
end
end
end
describe '.send_git_patch' do
let(:diff_refs) { double(base_sha: "base", head_sha: "head") }
subject { described_class.send_git_patch(repository, diff_refs) }
it 'sets the header correctly' do
key, command, params = decode_workhorse_header(subject)
expect(key).to eq("Gitlab-Workhorse-Send-Data")
expect(command).to eq("git-format-patch")
expect(params).to eq("RepoPath" => repository.path_to_repo, "ShaFrom" => "base", "ShaTo" => "head")
end
end
describe '.send_git_diff' do
let(:diff_refs) { double(base_sha: "base", head_sha: "head") }
subject { described_class.send_git_patch(repository, diff_refs) }
it 'sets the header correctly' do
key, command, params = decode_workhorse_header(subject)
expect(key).to eq("Gitlab-Workhorse-Send-Data")
expect(command).to eq("git-format-patch")
expect(params).to eq("RepoPath" => repository.path_to_repo, "ShaFrom" => "base", "ShaTo" => "head")
end
end
describe ".secret" do
subject { described_class.secret }
......
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