Commit 1b1e64d5 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'dz-blacklist--names' into 'master'

Blacklist nested groups and project names that can cause ambiguous routing for HA

Closes #29324

See merge request !9898
parents a6f9913f cd4db7b4
......@@ -36,7 +36,8 @@ class NamespaceValidator < ActiveModel::EachValidator
].freeze
WILDCARD_ROUTES = %w[tree commits wikis new edit create update logs_tree
preview blob blame raw files create_dir find_file].freeze
preview blob blame raw files create_dir find_file
artifacts graphs refs badges].freeze
STRICT_RESERVED = (RESERVED + WILDCARD_ROUTES).freeze
......
---
title: Reserve few project and nested group paths that have wildcard routes associated
merge_request: 9898
author:
require 'thread'
class RenameMoreReservedProjectNames < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
include Gitlab::ShellAdapter
DOWNTIME = false
THREAD_COUNT = 8
KNOWN_PATHS = %w(artifacts graphs refs badges).freeze
def up
queues = Array.new(THREAD_COUNT) { Queue.new }
start = false
threads = Array.new(THREAD_COUNT) do |index|
Thread.new do
queue = queues[index]
# Wait until we have input to process.
until start; end
rename_projects(queue.pop) until queue.empty?
end
end
enum = queues.each
reserved_projects.each_slice(100) do |slice|
begin
queue = enum.next
rescue StopIteration
enum.rewind
retry
end
queue << slice
end
start = true
threads.each(&:join)
end
def down
# nothing to do here
end
private
def reserved_projects
Project.unscoped.
includes(:namespace).
where('EXISTS (SELECT 1 FROM namespaces WHERE projects.namespace_id = namespaces.id)').
where('projects.path' => KNOWN_PATHS)
end
def route_exists?(full_path)
quoted_path = ActiveRecord::Base.connection.quote_string(full_path)
ActiveRecord::Base.connection.
select_all("SELECT id, path FROM routes WHERE path = '#{quoted_path}'").present?
end
# Adds number to the end of the path that is not taken by other route
def rename_path(namespace_path, path_was)
counter = 0
path = "#{path_was}#{counter}"
while route_exists?("#{namespace_path}/#{path}")
counter += 1
path = "#{path_was}#{counter}"
end
path
end
def rename_projects(projects)
projects.each do |project|
id = project.id
path_was = project.path
namespace_path = project.namespace.path
path = rename_path(namespace_path, path_was)
begin
# Because project path update is quite complex operation we can't safely
# copy-paste all code from GitLab. As exception we use Rails code here
project.rename_repo if rename_project_row(project, path)
rescue Exception => e # rubocop: disable Lint/RescueException
Rails.logger.error "Exception when renaming project #{id}: #{e.message}"
end
end
end
def rename_project_row(project, path)
project.respond_to?(:update_attributes) &&
project.update_attributes(path: path) &&
project.respond_to?(:rename_repo)
end
end
......@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20170306170512) do
ActiveRecord::Schema.define(version: 20170313133418) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
......
# encoding: utf-8
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20170313133418_rename_more_reserved_project_names.rb')
# This migration uses multiple threads, and thus different transactions. This
# means data created in this spec may not be visible to some threads. To work
# around this we use the TRUNCATE cleaning strategy.
describe RenameMoreReservedProjectNames, truncate: true do
let(:migration) { described_class.new }
let!(:project) { create(:empty_project) }
before do
project.path = 'artifacts'
project.save!(validate: false)
end
describe '#up' do
context 'when project repository exists' do
before { project.create_repository }
context 'when no exception is raised' do
it 'renames project with reserved names' do
migration.up
expect(project.reload.path).to eq('artifacts0')
end
end
context 'when exception is raised during rename' do
before do
allow(project).to receive(:rename_repo).and_raise(StandardError)
end
it 'captures exception from project rename' do
expect { migration.up }.not_to raise_error
end
end
end
context 'when project repository does not exist' do
it 'does not raise error' do
expect { migration.up }.not_to raise_error
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