summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2016-10-13 16:33:19 +0000
committerRémy Coutable <remy@rymai.me>2016-10-17 14:13:42 +0200
commitb777bd73f30bc5a2cf03e73512ec31bbe747b320 (patch)
treedf3f5600f9c3bca5c18ac8aa1a943ed92a7bacf0
parent42195ff6598349d2d45813eedd2184ec3f9b5175 (diff)
downloadgitlab-ce-b777bd73f30bc5a2cf03e73512ec31bbe747b320.tar.gz
Merge branch '22655-deployments-don-t-always-have-keep-around-refs' into 'master'
Handle case where deployment ref no longer exists ## What does this MR do? In 8.9, we didn't create keep-around refs for deployments. So it's possible that someone created a deployment (say, for testing), and then deleted the branch and all other references to that commit. That commit could then get GCed, and trying to view MRs on 8.11+ will show a 500. See https://gitlab.com/gitlab-org/gitlab-ce/issues/22655#note_16575020 for more details. ## Why was this MR needed? If someone created a deployment on 8.9, then deleted all references to the commit for that deployment, we will throw an exception when checking if the deployment includes a commit. Closes #22655. See merge request !6855 Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r--CHANGELOG1
-rw-r--r--app/models/deployment.rb9
-rw-r--r--spec/models/deployment_spec.rb9
3 files changed, 18 insertions, 1 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 3a3f3e35664..7abf3bed61b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -8,6 +8,7 @@ v 8.12.7
- Fix JS bug with select2 because of missing `data-field` attribute in select box. !6812
- Do not alter 'force_remove_source_branch' options on MergeRequest unless specified. !6817
- Fix GFM autocomplete setup being called several times. !6840
+ - Handle case where deployment ref no longer exists. !6855
v 8.12.6
- Update mailroom to 0.8.1 in Gemfile.lock !6814
diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index 07d7e19e70d..dcefab68ff6 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -40,7 +40,14 @@ class Deployment < ActiveRecord::Base
def includes_commit?(commit)
return false unless commit
- project.repository.is_ancestor?(commit.id, sha)
+ # Before 8.10, deployments didn't have keep-around refs. Any deployment
+ # created before then could have a `sha` referring to a commit that no
+ # longer exists in the repository, so just ignore those.
+ begin
+ project.repository.is_ancestor?(commit.id, sha)
+ rescue Rugged::OdbError
+ false
+ end
end
def update_merge_request_metrics!
diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb
index bfff639ad78..01a4a53a264 100644
--- a/spec/models/deployment_spec.rb
+++ b/spec/models/deployment_spec.rb
@@ -38,5 +38,14 @@ describe Deployment, models: true do
expect(deployment.includes_commit?(commit)).to be true
end
end
+
+ context 'when the SHA for the deployment does not exist in the repo' do
+ it 'returns false' do
+ deployment.update(sha: Gitlab::Git::BLANK_SHA)
+ commit = project.commit
+
+ expect(deployment.includes_commit?(commit)).to be false
+ end
+ end
end
end