Commit 613208c3 authored by Grzegorz Bizon's avatar Grzegorz Bizon

Recover from renaming project that has container images

parent e691f5b8
...@@ -50,10 +50,13 @@ class ProjectsController < Projects::ApplicationController ...@@ -50,10 +50,13 @@ class ProjectsController < Projects::ApplicationController
respond_to do |format| respond_to do |format|
if result[:status] == :success if result[:status] == :success
flash[:notice] = _("Project '%{project_name}' was successfully updated.") % { project_name: @project.name } flash[:notice] = _("Project '%{project_name}' was successfully updated.") % { project_name: @project.name }
format.html do format.html do
redirect_to(edit_project_path(@project)) redirect_to(edit_project_path(@project))
end end
else else
flash[:alert] = result[:message]
format.html { render 'edit' } format.html { render 'edit' }
end end
......
...@@ -977,8 +977,6 @@ class Project < ActiveRecord::Base ...@@ -977,8 +977,6 @@ class Project < ActiveRecord::Base
Rails.logger.error "Attempting to rename #{old_path_with_namespace} -> #{new_path_with_namespace}" Rails.logger.error "Attempting to rename #{old_path_with_namespace} -> #{new_path_with_namespace}"
expire_caches_before_rename(old_path_with_namespace)
if has_container_registry_tags? if has_container_registry_tags?
Rails.logger.error "Project #{old_path_with_namespace} cannot be renamed because container registry tags are present!" Rails.logger.error "Project #{old_path_with_namespace} cannot be renamed because container registry tags are present!"
...@@ -986,6 +984,8 @@ class Project < ActiveRecord::Base ...@@ -986,6 +984,8 @@ class Project < ActiveRecord::Base
raise StandardError.new('Project cannot be renamed, because images are present in its container registry') raise StandardError.new('Project cannot be renamed, because images are present in its container registry')
end end
expire_caches_before_rename(old_path_with_namespace)
if gitlab_shell.mv_repository(repository_storage_path, old_path_with_namespace, new_path_with_namespace) if gitlab_shell.mv_repository(repository_storage_path, old_path_with_namespace, new_path_with_namespace)
# If repository moved successfully we need to send update instructions to users. # If repository moved successfully we need to send update instructions to users.
# However we cannot allow rollback since we moved repository # However we cannot allow rollback since we moved repository
......
module Projects module Projects
class UpdateService < BaseService class UpdateService < BaseService
def execute def execute
# check that user is allowed to set specified visibility_level unless visibility_level_allowed?
new_visibility = params[:visibility_level] return error('New visibility level not allowed!')
if new_visibility && new_visibility.to_i != project.visibility_level
unless can?(current_user, :change_visibility_level, project) &&
Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
deny_visibility_level(project, new_visibility)
return error('Visibility level unallowed')
end
end end
new_branch = params[:default_branch] if project.has_container_registry_tags?
return error('Cannot rename project because it contains container registry tags!')
end
if project.repository.exists? && new_branch && new_branch != project.default_branch if changing_default_branch?
project.change_head(new_branch) project.change_head(new_branch)
end end
...@@ -28,8 +22,33 @@ module Projects ...@@ -28,8 +22,33 @@ module Projects
success success
else else
error('Project could not be updated') error('Project could not be updated!')
end
end end
private
def visibility_level_allowed?
# check that user is allowed to set specified visibility_level
new_visibility = params[:visibility_level]
if new_visibility && new_visibility.to_i != project.visibility_level
unless can?(current_user, :change_visibility_level, project) &&
Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility)
deny_visibility_level(project, new_visibility)
return false
end
end
true
end
def changing_default_branch?
new_branch = params[:default_branch]
project.repository.exists? &&
new_branch && new_branch != project.default_branch
end end
end end
end end
...@@ -211,27 +211,47 @@ describe ProjectsController do ...@@ -211,27 +211,47 @@ describe ProjectsController do
let(:admin) { create(:admin) } let(:admin) { create(:admin) }
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:new_path) { 'renamed_path' }
let(:project_params) { { path: new_path } }
before do before do
sign_in(admin) sign_in(admin)
end end
context 'when only renaming a project path' do
it "sets the repository to the right path after a rename" do it "sets the repository to the right path after a rename" do
controller.instance_variable_set(:@project, project) expect { update_project path: 'renamed_path' }
.to change { project.reload.path }
put :update, expect(project.reload.path).to include 'renamed_path'
namespace_id: project.namespace, expect(assigns(:repository).path).to include project.reload.path
id: project.id,
project: project_params
expect(project.repository.path).to include(new_path)
expect(assigns(:repository).path).to eq(project.repository.path)
expect(response).to have_http_status(302) expect(response).to have_http_status(302)
end end
end end
context 'when project has container repositories with tags' do
before do
stub_container_registry_config(enabled: true)
stub_container_registry_tags(repository: /image/, tags: %w[rc1])
create(:container_repository, project: project, name: :image)
end
it 'does not allow to rename the project' do
expect { update_project path: 'renamed_path' }
.not_to change { project.reload.path }
expect(project.reload.path).to_not include 'renamed_path'
expect(controller).to set_flash[:alert].to(/container registry tags/)
expect(response).to have_http_status(200)
end
end
def update_project(**parameters)
put :update,
namespace_id: project.namespace.path,
id: project.path,
project: parameters
end
end
describe '#transfer' do describe '#transfer' do
render_views render_views
......
...@@ -1236,7 +1236,7 @@ describe Project, models: true do ...@@ -1236,7 +1236,7 @@ describe Project, models: true do
subject { project.rename_repo } subject { project.rename_repo }
it { expect{subject}.to raise_error(Exception) } it { expect{subject}.to raise_error(StandardError) }
end end
end end
......
...@@ -40,7 +40,7 @@ describe Projects::UpdateService, services: true do ...@@ -40,7 +40,7 @@ describe Projects::UpdateService, services: true do
it 'does not update the project to public' do it 'does not update the project to public' do
result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC) result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC)
expect(result).to eq({ status: :error, message: 'Visibility level unallowed' }) expect(result).to eq({ status: :error, message: 'New visibility level not allowed!' })
expect(project).to be_private expect(project).to be_private
end end
...@@ -92,7 +92,8 @@ describe Projects::UpdateService, services: true do ...@@ -92,7 +92,8 @@ describe Projects::UpdateService, services: true do
it 'returns an error result when record cannot be updated' do it 'returns an error result when record cannot be updated' do
result = update_project(project, admin, { name: 'foo&bar' }) result = update_project(project, admin, { name: 'foo&bar' })
expect(result).to eq({ status: :error, message: 'Project could not be updated' }) expect(result).to eq({ status: :error,
message: 'Project could not be updated!' })
end end
def update_project(project, user, opts) def update_project(project, user, opts)
......
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