Fix bug when creating snippet and default branch not master

When a new snippet repository is created, the ref inside
HEAD is `master` even when the default branch is different.

This also happens with projects and wikis. These resources fix
this by explicitly rewrite the HEAD reference and we're applying
the same behavior to snippets.

When the repository is pushed or pulled we update the reference
inside the HEAD file.
parent f905c33a
...@@ -20,6 +20,7 @@ class Snippet < ApplicationRecord ...@@ -20,6 +20,7 @@ class Snippet < ApplicationRecord
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
MAX_FILE_COUNT = 10 MAX_FILE_COUNT = 10
MASTER_BRANCH = 'master'
cache_markdown_field :title, pipeline: :single_line cache_markdown_field :title, pipeline: :single_line
cache_markdown_field :description cache_markdown_field :description
...@@ -311,13 +312,27 @@ class Snippet < ApplicationRecord ...@@ -311,13 +312,27 @@ class Snippet < ApplicationRecord
override :default_branch override :default_branch
def default_branch def default_branch
super || 'master' super || MASTER_BRANCH
end end
def repository_storage def repository_storage
snippet_repository&.shard_name || self.class.pick_repository_storage snippet_repository&.shard_name || self.class.pick_repository_storage
end end
# Repositories are created by default with the `master` branch.
# This method changes the `HEAD` file to point to the existing
# default branch in case it's not master.
def change_head_to_default_branch
return unless repository.exists?
return if default_branch == MASTER_BRANCH
# All snippets must have at least 1 file. Therefore, if
# `HEAD` is empty is because it's pointing to the wrong
# default branch
return unless repository.empty? || list_files('HEAD').empty?
repository.raw_repository.write_ref('HEAD', "refs/heads/#{default_branch}")
end
def create_repository def create_repository
return if repository_exists? && snippet_repository return if repository_exists? && snippet_repository
......
---
title: Fix bug with snippets in HEAD when default branch is not master
merge_request: 50366
author:
type: fixed
...@@ -30,7 +30,10 @@ module Gitlab ...@@ -30,7 +30,10 @@ module Gitlab
def check(cmd, changes) def check(cmd, changes)
check_snippet_accessibility! check_snippet_accessibility!
super super.tap do |_|
# Ensure HEAD points to the default branch in case it is not master
snippet.change_head_to_default_branch
end
end end
override :download_ability override :download_ability
......
...@@ -390,6 +390,38 @@ RSpec.describe Gitlab::GitAccessSnippet do ...@@ -390,6 +390,38 @@ RSpec.describe Gitlab::GitAccessSnippet do
end end
end end
describe 'HEAD realignment' do
let_it_be(:snippet) { create(:project_snippet, :private, :repository, project: project) }
shared_examples 'HEAD is updated to the snippet default branch' do
let(:actor) { snippet.author }
specify do
expect(snippet).to receive(:change_head_to_default_branch).and_call_original
subject
end
context 'when an error is raised' do
let(:actor) { nil }
it 'does not realign HEAD' do
expect(snippet).not_to receive(:change_head_to_default_branch).and_call_original
expect { subject }.to raise_error(described_class::ForbiddenError)
end
end
end
it_behaves_like 'HEAD is updated to the snippet default branch' do
subject { push_access_check }
end
it_behaves_like 'HEAD is updated to the snippet default branch' do
subject { pull_access_check }
end
end
private private
def raise_snippet_not_found def raise_snippet_not_found
......
...@@ -796,4 +796,90 @@ RSpec.describe Snippet do ...@@ -796,4 +796,90 @@ RSpec.describe Snippet do
it_behaves_like 'can move repository storage' do it_behaves_like 'can move repository storage' do
let_it_be(:container) { create(:snippet, :repository) } let_it_be(:container) { create(:snippet, :repository) }
end end
describe '#change_head_to_default_branch' do
let(:head_path) { Rails.root.join(TestEnv.repos_path, "#{snippet.disk_path}.git", 'HEAD') }
subject { snippet.change_head_to_default_branch }
context 'when repository does not exist' do
let(:snippet) { create(:snippet) }
it 'does nothing' do
expect(snippet.repository_exists?).to eq false
expect(snippet.repository.raw_repository).not_to receive(:write_ref)
subject
end
end
context 'when repository is empty' do
let(:snippet) { create(:snippet, :empty_repo) }
before do
allow(Gitlab::CurrentSettings).to receive(:default_branch_name).and_return(default_branch)
end
context 'when default branch in settings is "master"' do
let(:default_branch) { 'master' }
it 'does nothing' do
expect(File.read(head_path).squish).to eq 'ref: refs/heads/master'
expect(snippet.repository.raw_repository).not_to receive(:write_ref)
subject
end
end
context 'when default branch in settings is different from "master"' do
let(:default_branch) { 'main' }
it 'changes the HEAD reference to the default branch' do
expect(File.read(head_path).squish).to eq 'ref: refs/heads/master'
subject
expect(File.read(head_path).squish).to eq "ref: refs/heads/#{default_branch}"
end
end
end
context 'when repository is not empty' do
let(:snippet) { create(:snippet, :empty_repo) }
before do
populate_snippet_repo
end
context 'when HEAD branch is empty' do
it 'changes HEAD to default branch' do
File.write(head_path, 'ref: refs/heads/non_existen_branch')
expect(File.read(head_path).squish).to eq 'ref: refs/heads/non_existen_branch'
subject
expect(File.read(head_path).squish).to eq 'ref: refs/heads/main'
expect(snippet.list_files('HEAD')).not_to be_empty
end
end
context 'when HEAD branch is not empty' do
it 'does nothing' do
File.write(head_path, 'ref: refs/heads/main')
expect(snippet.repository.raw_repository).not_to receive(:write_ref)
subject
end
end
def populate_snippet_repo
allow(Gitlab::CurrentSettings).to receive(:default_branch_name).and_return('main')
data = [{ file_path: 'new_file_test', content: 'bar' }]
snippet.snippet_repository.multi_files_action(snippet.author, data, branch_name: 'main', message: 'foo')
end
end
end
end end
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