Merge branch 'handle-invalid-kept-around-references' into 'master'

Gracefully handle case when keep-around references are corrupted or exist already

We were seeing a number of error messages when attempting to create a keep-around ref:

1. Failed to create locked file `refs/keep-around/XYZ`: File exists
2. Failed to write reference `refs/keep-around/XYZ`: a reference with that name already exists.

I'm not sure how these happen, but I suspect when multiple workers attempt to write the same file we may have an issue. The force parameter should help ensure the file gets created,
as well as the rescues to prevent 500 Errors.

Rugged/libgit2 unfortunately does not allow you to delete or re-create a reference that has been corrupted, even with the force parameter. A truncated reference will stay that way until manual intervention.

Closes #20109

See merge request !5430

Signed-off-by: Rémy Coutable <remy@rymai.me>

3 files changed, 32 insertions, 2 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 35b50eadd19..9610284f548 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.10.1 (unreleased)
   - Refactor repository storages documentation. !5428
+  - Gracefully handle case when keep-around references are corrupted or exist already. !5430
 
 v 8.10.0
   - Fix profile activity heatmap to show correct day name (eanplatter)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 6130e40cb2b..b6ecd3fa232 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -206,11 +206,20 @@ class Repository
 
     return if kept_around?(sha)
 
-    rugged.references.create(keep_around_ref_name(sha), sha)
+    # This will still fail if the file is corrupted (e.g. 0 bytes)
+    begin
+      rugged.references.create(keep_around_ref_name(sha), sha, force: true)
+    rescue Rugged::ReferenceError => ex
+      Rails.logger.error "Unable to create keep-around reference for repository #{path}: #{ex}"
+    end
   end
 
   def kept_around?(sha)
-    ref_exists?(keep_around_ref_name(sha))
+    begin
+      ref_exists?(keep_around_ref_name(sha))
+    rescue Rugged::ReferenceError
+      false
+    end
   end
 
   def tag_names
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 38b0c345b48..881ab5ff8dc 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1173,11 +1173,31 @@ describe Repository, models: true do
   end
 
   describe "#keep_around" do
+    it "does not fail if we attempt to reference bad commit" do
+      expect(repository.kept_around?('abc1234')).to be_falsey
+    end
+
     it "stores a reference to the specified commit sha so it isn't garbage collected" do
       repository.keep_around(sample_commit.id)
 
       expect(repository.kept_around?(sample_commit.id)).to be_truthy
     end
+
+    it "attempting to call keep_around on truncated ref does not fail" do
+      repository.keep_around(sample_commit.id)
+      ref = repository.send(:keep_around_ref_name, sample_commit.id)
+      path = File.join(repository.path, ref)
+      # Corrupt the reference
+      File.truncate(path, 0)
+
+      expect(repository.kept_around?(sample_commit.id)).to be_falsey
+
+      repository.keep_around(sample_commit.id)
+
+      expect(repository.kept_around?(sample_commit.id)).to be_falsey
+
+      File.delete(path)
+    end
   end
 
   def create_remote_branch(remote_name, branch_name, target)