Merge branch '10io-fix-nuget-upload' into 'master'

Fix nuget uploads

See merge request gitlab-org/gitlab!23599

ee/app/services/packages/nuget/create_package_service.rb

@@ -3,15 +3,21 @@
 module Packages
   module Nuget
     class CreatePackageService < BaseService
-      PACKAGE_NAME = 'NuGet.Package'
+      TEMPORARY_PACKAGE_NAME = 'NuGet.Temporary.Package'
       PACKAGE_VERSION = '0.0.0'
 
       def execute
         project.packages.nuget.create!(
-          name: PACKAGE_NAME,
-          version: PACKAGE_VERSION
+          name: TEMPORARY_PACKAGE_NAME,
+          version: "#{PACKAGE_VERSION}-#{uuid}"
         )
       end
+
+      private
+
+      def uuid
+        SecureRandom.uuid
+      end
     end
   end
 end

ee/lib/api/conan_packages.rb

@@ -250,7 +250,7 @@ module API
           end
           route_setting :authentication, job_token_allowed: true
           put 'authorize' do
-            authorize_workhorse!(project)
+            authorize_workhorse!(subject: project)
           end
         end
 
@@ -273,7 +273,7 @@ module API
           end
           route_setting :authentication, job_token_allowed: true
           put 'authorize' do
-            authorize_workhorse!(project)
+            authorize_workhorse!(subject: project)
           end
 
           desc 'Upload package files' do

ee/lib/api/helpers/packages_helpers.rb

@@ -3,6 +3,8 @@
 module API
   module Helpers
     module PackagesHelpers
+      MAX_PACKAGE_FILE_SIZE = 50.megabytes.freeze
+
       def require_packages_enabled!
         not_found! unless ::Gitlab.config.packages.enabled
       end
@@ -29,14 +31,17 @@ module API
         authorize_read_package!(subject)
       end
 
-      def authorize_workhorse!(subject = user_project)
+      def authorize_workhorse!(subject: user_project, has_length: true, maximum_size: MAX_PACKAGE_FILE_SIZE)
         authorize_upload!(subject)
 
         Gitlab::Workhorse.verify_api_request!(headers)
 
         status 200
         content_type Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE
-        ::Packages::PackageFileUploader.workhorse_authorize(has_length: true)
+
+        params = { has_length: has_length }
+        params[:maximum_size] = maximum_size unless has_length
+        ::Packages::PackageFileUploader.workhorse_authorize(params)
       end
 
       def authorize_upload!(subject = user_project)

ee/lib/api/helpers/packages_manager_clients_helpers.rb

@@ -36,8 +36,8 @@ module API
         ::Ci::Build.find_by_token(token)
       end
 
-      def uploaded_package_file
-        uploaded_file = UploadedFile.from_params(params, :file, ::Packages::PackageFileUploader.workhorse_local_upload_path)
+      def uploaded_package_file(param_name = :file)
+        uploaded_file = UploadedFile.from_params(params, param_name, ::Packages::PackageFileUploader.workhorse_local_upload_path)
         bad_request!('Missing package file!') unless uploaded_file
         uploaded_file
       end

ee/lib/api/nuget_packages.rb

@@ -96,7 +96,7 @@ module API
           package = ::Packages::Nuget::CreatePackageService.new(authorized_user_project, current_user).execute
 
           file_params = params.merge(
-            file: uploaded_package_file,
+            file: uploaded_package_file(:package),
             file_name: PACKAGE_FILENAME,
             file_type: PACKAGE_FILETYPE
           )
@@ -112,7 +112,7 @@ module API
           forbidden!
         end
         put 'authorize' do
-          authorize_workhorse!(authorized_user_project)
+          authorize_workhorse!(subject: authorized_user_project, has_length: false)
         end
       end
     end

ee/spec/lib/api/helpers/packages_helpers_spec.rb

@@ -89,7 +89,7 @@ describe API::Helpers::PackagesHelpers do
   describe '#authorize_workhorse!' do
     let_it_be(:headers) { {} }
 
-    subject { helper.authorize_workhorse!(project) }
+    subject { helper.authorize_workhorse!(subject: project) }
 
     before do
       allow(helper).to receive(:headers).and_return(headers)
@@ -104,6 +104,20 @@ describe API::Helpers::PackagesHelpers do
 
       expect(subject).to eq nil
     end
+
+    context 'without length' do
+      subject { helper.authorize_workhorse!(subject: project, has_length: false) }
+
+      it 'authorizes workhorse' do
+        expect(helper).to receive(:authorize_upload!).with(project)
+        expect(helper).to receive(:status).with(200)
+        expect(helper).to receive(:content_type).with(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+        expect(Gitlab::Workhorse).to receive(:verify_api_request!).with(headers)
+        expect(::Packages::PackageFileUploader).to receive(:workhorse_authorize).with(has_length: false, maximum_size: ::API::Helpers::PackagesHelpers::MAX_PACKAGE_FILE_SIZE)
+
+        expect(subject).to eq nil
+      end
+    end
   end
 
   describe '#authorize_upload!' do

ee/spec/requests/api/nuget_packages_spec.rb

@@ -177,13 +177,13 @@ describe API::NugetPackages do
     let_it_be(:file_name) { 'package.nupkg' }
     let(:url) { "/projects/#{project.id}/packages/nuget" }
     let(:headers) { {} }
-    let(:params) { { file: temp_file(file_name) } }
+    let(:params) { { package: temp_file(file_name) } }
 
     subject do
       workhorse_finalize(
         api(url),
         method: :put,
-        file_key: :file,
+        file_key: :package,
         params: params,
         headers: headers
       )

ee/spec/services/packages/nuget/create_package_service_spec.rb

@@ -14,8 +14,20 @@ describe Packages::Nuget::CreatePackageService do
       package = Packages::Package.last
 
       expect(package).to be_valid
-      expect(package.name).to eq(Packages::Nuget::CreatePackageService::PACKAGE_NAME)
-      expect(package.version).to eq(Packages::Nuget::CreatePackageService::PACKAGE_VERSION)
+      expect(package.name).to eq(Packages::Nuget::CreatePackageService::TEMPORARY_PACKAGE_NAME)
+      expect(package.version).to start_with(Packages::Nuget::CreatePackageService::PACKAGE_VERSION)
+      expect(package.package_type).to eq('nuget')
+    end
+
+    it 'can create two packages in a row' do
+      expect { subject }.to change { Packages::Package.count }.by(1)
+      expect { described_class.new(project, user, params).execute }.to change { Packages::Package.count }.by(1)
+
+      package = Packages::Package.last
+
+      expect(package).to be_valid
+      expect(package.name).to eq(Packages::Nuget::CreatePackageService::TEMPORARY_PACKAGE_NAME)
+      expect(package.version).to start_with(Packages::Nuget::CreatePackageService::PACKAGE_VERSION)
       expect(package.package_type).to eq('nuget')
     end
   end

ee/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb

@@ -98,7 +98,7 @@ RSpec.shared_examples 'process nuget upload' do |user_type, status, add_member =
 
     context 'with object storage disabled' do
       context 'without a file from workhorse' do
-        let(:params) { { file: nil } }
+        let(:params) { { package: nil } }
 
         it_behaves_like 'returning response status', :bad_request
       end
@@ -121,7 +121,7 @@ RSpec.shared_examples 'process nuget upload' do |user_type, status, add_member =
           )
         end
         let(:fog_file) { fog_to_uploaded_file(tmp_object) }
-        let(:params) { { file: fog_file, 'file.remote_id' => file_name } }
+        let(:params) { { package: fog_file, 'package.remote_id' => file_name } }
 
         it_behaves_like 'creates nuget package files'
 
@@ -129,8 +129,8 @@ RSpec.shared_examples 'process nuget upload' do |user_type, status, add_member =
           context "with invalid remote_id: #{remote_id}" do
             let(:params) do
               {
-                file: fog_file,
-                'file.remote_id' => remote_id
+                package: fog_file,
+                'package.remote_id' => remote_id
               }
             end