Commit a5657a3e authored by Shinya Maeda's avatar Shinya Maeda

Merge branch '193170-fix-deployment-ref-validation' into 'master'

Update deployment ref validation

Closes #193170

See merge request gitlab-org/gitlab!23170
parents 6b006eba fa0b4321
...@@ -135,7 +135,7 @@ class Deployment < ApplicationRecord ...@@ -135,7 +135,7 @@ class Deployment < ApplicationRecord
end end
def create_ref def create_ref
project.repository.create_ref(ref, ref_path) project.repository.create_ref(sha, ref_path)
end end
def invalidate_cache def invalidate_cache
...@@ -280,12 +280,12 @@ class Deployment < ApplicationRecord ...@@ -280,12 +280,12 @@ class Deployment < ApplicationRecord
errors.add(:ref, _('The branch or tag does not exist')) errors.add(:ref, _('The branch or tag does not exist'))
end end
private
def ref_path def ref_path
File.join(environment.ref_path, 'deployments', iid.to_s) File.join(environment.ref_path, 'deployments', iid.to_s)
end end
private
def legacy_finished_at def legacy_finished_at
self.created_at if success? && !read_attribute(:finished_at) self.created_at if success? && !read_attribute(:finished_at)
end end
......
...@@ -193,15 +193,6 @@ class Environment < ApplicationRecord ...@@ -193,15 +193,6 @@ class Environment < ApplicationRecord
folder_name == "production" folder_name == "production"
end end
def first_deployment_for(commit_sha)
ref = project.repository.ref_name_for_sha(ref_path, commit_sha)
return unless ref
deployment_iid = ref.split('/').last
deployments.find_by(iid: deployment_iid)
end
def ref_path def ref_path
"refs/#{Repository::REF_ENVIRONMENTS}/#{slug}" "refs/#{Repository::REF_ENVIRONMENTS}/#{slug}"
end end
......
---
title: Use of sha instead of ref when creating a new ref on deployment creation.
merge_request: 23170
author:
type: fixed
...@@ -520,6 +520,21 @@ describe Deployment do ...@@ -520,6 +520,21 @@ describe Deployment do
end end
end end
describe '#create_ref' do
let(:deployment) { build(:deployment) }
subject { deployment.create_ref }
it 'creates a ref using the sha' do
expect(deployment.project.repository).to receive(:create_ref).with(
deployment.sha,
"refs/environments/#{deployment.environment.name}/deployments/#{deployment.iid}"
)
subject
end
end
describe '#playable_build' do describe '#playable_build' do
subject { deployment.playable_build } subject { deployment.playable_build }
......
...@@ -325,26 +325,6 @@ describe Environment, :use_clean_rails_memory_store_caching do ...@@ -325,26 +325,6 @@ describe Environment, :use_clean_rails_memory_store_caching do
end end
end end
describe '#first_deployment_for' do
let(:project) { create(:project, :repository) }
let!(:deployment) { create(:deployment, :succeed, environment: environment, ref: commit.parent.id) }
let!(:deployment1) { create(:deployment, :succeed, environment: environment, ref: commit.id) }
let(:head_commit) { project.commit }
let(:commit) { project.commit.parent }
it 'returns deployment id for the environment', :sidekiq_might_not_need_inline do
expect(environment.first_deployment_for(commit.id)).to eq deployment1
end
it 'return nil when no deployment is found' do
expect(environment.first_deployment_for(head_commit.id)).to eq nil
end
it 'returns a UTF-8 ref', :sidekiq_might_not_need_inline do
expect(environment.first_deployment_for(commit.id).ref).to be_utf8
end
end
describe '#environment_type' do describe '#environment_type' do
subject { environment.environment_type } subject { environment.environment_type }
......
...@@ -49,7 +49,7 @@ describe Deployments::AfterCreateService do ...@@ -49,7 +49,7 @@ describe Deployments::AfterCreateService do
it 'creates ref' do it 'creates ref' do
expect_any_instance_of(Repository) expect_any_instance_of(Repository)
.to receive(:create_ref) .to receive(:create_ref)
.with(deployment.ref, deployment.send(:ref_path)) .with(deployment.sha, deployment.send(:ref_path))
service.execute service.execute
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