Merge branch 'fix/gb/recover-from-renaming-project-with-container-images' into 'master'
Recover from renaming project that has container images Closes #23019 See merge request !12840
This commit is contained in:
commit
0618ad12e2
|
@ -50,10 +50,13 @@ class ProjectsController < Projects::ApplicationController
|
|||
respond_to do |format|
|
||||
if result[:status] == :success
|
||||
flash[:notice] = _("Project '%{project_name}' was successfully updated.") % { project_name: @project.name }
|
||||
|
||||
format.html do
|
||||
redirect_to(edit_project_path(@project))
|
||||
end
|
||||
else
|
||||
flash[:alert] = result[:message]
|
||||
|
||||
format.html { render 'edit' }
|
||||
end
|
||||
|
||||
|
|
|
@ -486,7 +486,9 @@ class Project < ActiveRecord::Base
|
|||
end
|
||||
|
||||
def has_container_registry_tags?
|
||||
container_repositories.to_a.any?(&:has_tags?) ||
|
||||
return @images if defined?(@images)
|
||||
|
||||
@images = container_repositories.to_a.any?(&:has_tags?) ||
|
||||
has_root_container_repository_tags?
|
||||
end
|
||||
|
||||
|
@ -977,8 +979,6 @@ class Project < ActiveRecord::Base
|
|||
|
||||
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?
|
||||
Rails.logger.error "Project #{old_path_with_namespace} cannot be renamed because container registry tags are present!"
|
||||
|
||||
|
@ -986,6 +986,8 @@ class Project < ActiveRecord::Base
|
|||
raise StandardError.new('Project cannot be renamed, because images are present in its container registry')
|
||||
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 repository moved successfully we need to send update instructions to users.
|
||||
# However we cannot allow rollback since we moved repository
|
||||
|
|
|
@ -1,22 +1,16 @@
|
|||
module Projects
|
||||
class UpdateService < BaseService
|
||||
def execute
|
||||
# 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 error('Visibility level unallowed')
|
||||
end
|
||||
unless visibility_level_allowed?
|
||||
return error('New visibility level not allowed!')
|
||||
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
|
||||
project.change_head(new_branch)
|
||||
if changing_default_branch?
|
||||
project.change_head(params[:default_branch])
|
||||
end
|
||||
|
||||
if project.update_attributes(params.except(:default_branch))
|
||||
|
@ -28,8 +22,33 @@ module Projects
|
|||
|
||||
success
|
||||
else
|
||||
error('Project could not be updated')
|
||||
error('Project could not be updated!')
|
||||
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
|
||||
|
|
|
@ -0,0 +1,4 @@
|
|||
---
|
||||
title: Recover from renaming project that has container images
|
||||
merge_request: 12840
|
||||
author:
|
|
@ -211,24 +211,43 @@ describe ProjectsController do
|
|||
|
||||
let(:admin) { create(:admin) }
|
||||
let(:project) { create(:project, :repository) }
|
||||
let(:new_path) { 'renamed_path' }
|
||||
let(:project_params) { { path: new_path } }
|
||||
|
||||
before do
|
||||
sign_in(admin)
|
||||
end
|
||||
|
||||
it "sets the repository to the right path after a rename" do
|
||||
controller.instance_variable_set(:@project, project)
|
||||
context 'when only renaming a project path' do
|
||||
it "sets the repository to the right path after a rename" do
|
||||
expect { update_project path: 'renamed_path' }
|
||||
.to change { project.reload.path }
|
||||
|
||||
expect(project.path).to include 'renamed_path'
|
||||
expect(assigns(:repository).path).to include project.path
|
||||
expect(response).to have_http_status(302)
|
||||
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(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,
|
||||
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)
|
||||
namespace_id: project.namespace.path,
|
||||
id: project.path,
|
||||
project: parameters
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -1236,7 +1236,7 @@ describe Project, models: true do
|
|||
|
||||
subject { project.rename_repo }
|
||||
|
||||
it { expect{subject}.to raise_error(Exception) }
|
||||
it { expect{subject}.to raise_error(StandardError) }
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -1,11 +1,14 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe Projects::UpdateService, services: true do
|
||||
describe Projects::UpdateService, '#execute', :services do
|
||||
let(:user) { create(:user) }
|
||||
let(:admin) { create(:admin) }
|
||||
let(:project) { create(:empty_project, creator_id: user.id, namespace: user.namespace) }
|
||||
|
||||
describe 'update_by_user' do
|
||||
let(:project) do
|
||||
create(:empty_project, creator: user, namespace: user.namespace)
|
||||
end
|
||||
|
||||
context 'when changing visibility level' do
|
||||
context 'when visibility_level is INTERNAL' do
|
||||
it 'updates the project to internal' do
|
||||
result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL)
|
||||
|
@ -40,7 +43,7 @@ describe Projects::UpdateService, services: true do
|
|||
it 'does not update the project to public' do
|
||||
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
|
||||
end
|
||||
|
||||
|
@ -55,12 +58,13 @@ describe Projects::UpdateService, services: true do
|
|||
end
|
||||
end
|
||||
|
||||
describe 'visibility_level' do
|
||||
describe 'when updating project that has forks' do
|
||||
let(:project) { create(:empty_project, :internal) }
|
||||
let(:forked_project) { create(:forked_project_with_submodules, :internal) }
|
||||
|
||||
before do
|
||||
forked_project.build_forked_project_link(forked_to_project_id: forked_project.id, forked_from_project_id: project.id)
|
||||
forked_project.build_forked_project_link(forked_to_project_id: forked_project.id,
|
||||
forked_from_project_id: project.id)
|
||||
forked_project.save
|
||||
end
|
||||
|
||||
|
@ -89,10 +93,38 @@ describe Projects::UpdateService, services: true do
|
|||
end
|
||||
end
|
||||
|
||||
it 'returns an error result when record cannot be updated' do
|
||||
result = update_project(project, admin, { name: 'foo&bar' })
|
||||
context 'when updating a default branch' do
|
||||
let(:project) { create(:project, :repository) }
|
||||
|
||||
expect(result).to eq({ status: :error, message: 'Project could not be updated' })
|
||||
it 'changes a default branch' do
|
||||
update_project(project, admin, default_branch: 'feature')
|
||||
|
||||
expect(Project.find(project.id).default_branch).to eq 'feature'
|
||||
end
|
||||
end
|
||||
|
||||
context 'when renaming project that contains container images' 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
|
||||
result = update_project(project, admin, path: 'renamed')
|
||||
|
||||
expect(result).to include(status: :error)
|
||||
expect(result[:message]).to match(/contains container registry tags/)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when passing invalid parameters' do
|
||||
it 'returns an error result when record cannot be updated' do
|
||||
result = update_project(project, admin, { name: 'foo&bar' })
|
||||
|
||||
expect(result).to eq({ status: :error,
|
||||
message: 'Project could not be updated!' })
|
||||
end
|
||||
end
|
||||
|
||||
def update_project(project, user, opts)
|
||||
|
|
Loading…
Reference in New Issue