Merge branch 'sk/fix-release-api-internal-server-error' into 'master'

Fix 500 error on relase API for invalid tag_name

Closes #204718

See merge request gitlab-org/gitlab!28969

app/services/tags/create_service.rb

@@ -4,7 +4,7 @@ module Tags
   class CreateService < BaseService
     def execute(tag_name, target, message)
       valid_tag = Gitlab::GitRefValidator.validate(tag_name)
-      return error('Tag name invalid') unless valid_tag
+      return error('Tag name invalid', 400) unless valid_tag
 
       repository = project.repository
       message = message&.strip
@@ -14,7 +14,7 @@ module Tags
       begin
         new_tag = repository.add_tag(current_user, tag_name, target, message)
       rescue Gitlab::Git::Repository::TagExistsError
-        return error("Tag #{tag_name} already exists")
+        return error("Tag #{tag_name} already exists", 409)
       rescue Gitlab::Git::PreReceiveError => ex
         return error(ex.message)
       end
@@ -24,7 +24,7 @@ module Tags
 
         success.merge(tag: new_tag)
       else
-        error("Target #{target} is invalid")
+        error("Target #{target} is invalid", 400)
       end
     end
   end

changelogs/unreleased/fix-release-api-internal-server-error.yml

+---
+title: Fix 500 error on create release API when providing an invalid tag_name
+merge_request: 28969
+author: Sashi Kumar
+type: fixed

spec/requests/api/releases_spec.rb

@@ -702,10 +702,10 @@ describe API::Releases do
       context 'when tag name is HEAD' do
         let(:tag_name) { 'HEAD' }
 
-        it 'returns an error as failure on tag creation' do
+        it 'returns a 400 error as failure on tag creation' do
           post api("/projects/#{project.id}/releases", maintainer), params: params
 
-          expect(response).to have_gitlab_http_status(:internal_server_error)
+          expect(response).to have_gitlab_http_status(:bad_request)
           expect(json_response['message']).to eq('Tag name invalid')
         end
       end
@@ -713,10 +713,10 @@ describe API::Releases do
       context 'when tag name is empty' do
         let(:tag_name) { '' }
 
-        it 'returns an error as failure on tag creation' do
+        it 'returns a 400 error as failure on tag creation' do
           post api("/projects/#{project.id}/releases", maintainer), params: params
 
-          expect(response).to have_gitlab_http_status(:internal_server_error)
+          expect(response).to have_gitlab_http_status(:bad_request)
           expect(json_response['message']).to eq('Tag name invalid')
         end
       end

spec/services/tags/create_service_spec.rb

@@ -21,8 +21,9 @@ describe Tags::CreateService do
       it 'returns an error' do
         response = service.execute('v1.1.0', 'foo', 'Foo')
 
-        expect(response).to eq(status: :error,
-                               message: 'Target foo is invalid')
+        expect(response[:status]).to eq(:error)
+        expect(response[:http_status]).to eq(400)
+        expect(response[:message]).to eq('Target foo is invalid')
       end
     end
 
@@ -34,8 +35,19 @@ describe Tags::CreateService do
 
         response = service.execute('v1.1.0', 'master', 'Foo')
 
-        expect(response).to eq(status: :error,
-                               message: 'Tag v1.1.0 already exists')
+        expect(response[:status]).to eq(:error)
+        expect(response[:http_status]).to eq(409)
+        expect(response[:message]).to eq('Tag v1.1.0 already exists')
+      end
+    end
+
+    context 'when tag name is invalid' do
+      it 'returns an error' do
+        response = service.execute('HEAD', 'master', 'Foo')
+
+        expect(response[:status]).to eq(:error)
+        expect(response[:http_status]).to eq(400)
+        expect(response[:message]).to eq('Tag name invalid')
       end
     end
 
@@ -47,8 +59,8 @@ describe Tags::CreateService do
 
         response = service.execute('v1.1.0', 'master', 'Foo')
 
-        expect(response).to eq(status: :error,
-                               message: 'something went wrong')
+        expect(response[:status]).to eq(:error)
+        expect(response[:message]).to eq('something went wrong')
       end
     end
   end