Merge branch '208746-pypi-package-upload' into 'master'

PyPi: Support package upload

See merge request gitlab-org/gitlab!27632

db/migrate/20200318183553_create_pypi_package_metadata.rb

+# frozen_string_literal: true
+
+class CreatePypiPackageMetadata < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    create_table :packages_pypi_metadata, id: false do |t|
+      t.references :package, primary_key: true, index: false, default: nil, foreign_key: { to_table: :packages_packages, on_delete: :cascade }, type: :bigint
+      t.string "required_python", null: false, limit: 50
+    end
+  end
+end

db/structure.sql

@@ -4493,6 +4493,11 @@ CREATE SEQUENCE public.packages_packages_id_seq
 
 ALTER SEQUENCE public.packages_packages_id_seq OWNED BY public.packages_packages.id;
 
+CREATE TABLE public.packages_pypi_metadata (
+    package_id bigint NOT NULL,
+    required_python character varying(50) NOT NULL
+);
+
 CREATE TABLE public.packages_tags (
     id bigint NOT NULL,
     package_id integer NOT NULL,
@@ -8146,6 +8151,9 @@ ALTER TABLE ONLY public.packages_package_files
 ALTER TABLE ONLY public.packages_packages
     ADD CONSTRAINT packages_packages_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY public.packages_pypi_metadata
+    ADD CONSTRAINT packages_pypi_metadata_pkey PRIMARY KEY (package_id);
+
 ALTER TABLE ONLY public.packages_tags
     ADD CONSTRAINT packages_tags_pkey PRIMARY KEY (id);
 
@@ -11596,6 +11604,9 @@ ALTER TABLE ONLY public.board_labels
 ALTER TABLE ONLY public.scim_identities
     ADD CONSTRAINT fk_rails_9421a0bffb FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.packages_pypi_metadata
+    ADD CONSTRAINT fk_rails_9698717cdd FOREIGN KEY (package_id) REFERENCES public.packages_packages(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.packages_dependency_links
     ADD CONSTRAINT fk_rails_96ef1c00d3 FOREIGN KEY (package_id) REFERENCES public.packages_packages(id) ON DELETE CASCADE;
 
@@ -13032,6 +13043,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200318164448
 20200318165448
 20200318175008
+20200318183553
 20200319071702
 20200319123041
 20200319124127

ee/app/models/packages/package.rb

@@ -10,6 +10,7 @@ class Packages::Package < ApplicationRecord
   has_many :dependency_links, inverse_of: :package, class_name: 'Packages::DependencyLink'
   has_many :tags, inverse_of: :package, class_name: 'Packages::Tag'
   has_one :conan_metadatum, inverse_of: :package
+  has_one :pypi_metadatum, inverse_of: :package
   has_one :maven_metadatum, inverse_of: :package
   has_one :build_info, inverse_of: :package
 

ee/app/models/packages/pypi_metadatum.rb

+# frozen_string_literal: true
+
+class Packages::PypiMetadatum < ApplicationRecord
+  self.primary_key = :package_id
+
+  belongs_to :package, -> { where(package_type: :pypi) }, inverse_of: :pypi_metadatum
+
+  validates :package, presence: true
+end

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

+# frozen_string_literal: true
+
+module Packages
+  module Pypi
+    class CreatePackageService < BaseService
+      include ::Gitlab::Utils::StrongMemoize
+
+      def execute
+        ::Packages::Package.transaction do
+          Packages::PypiMetadatum.upsert(
+            package_id: created_package.id,
+            required_python: params[:requires_python]
+          )
+
+          ::Packages::CreatePackageFileService.new(created_package, file_params).execute
+        end
+      end
+
+      private
+
+      def created_package
+        strong_memoize(:created_package) do
+          project
+            .packages
+            .pypi
+            .safe_find_or_create_by!(name: params[:name], version: params[:version])
+        end
+      end
+
+      def file_params
+        {
+          file: params[:content],
+          file_name: params[:content].original_filename,
+          file_md5: params[:md5_digest],
+          file_sha256: params[:sha256_digest]
+        }
+      end
+    end
+  end
+end

ee/changelogs/unreleased/208746-pypi-package-upload.yml

+---
+title: Support PyPi package upload
+merge_request: 27632
+author:
+type: added

ee/lib/api/pypi_packages.rb

@@ -18,6 +18,10 @@ module API
       render_api_error!(e.message, 400)
     end
 
+    rescue_from ActiveRecord::RecordInvalid do |e|
+      render_api_error!(e.message, 400)
+    end
+
     before do
       require_packages_enabled!
     end
@@ -65,12 +69,21 @@ module API
         end
 
         params do
-          use :workhorse_upload_params
+          requires :content, type: ::API::Validations::Types::WorkhorseFile, desc: 'The package file to be published (generated by Multipart middleware)'
+          requires :requires_python, type: String
+          requires :name, type: String
+          requires :version, type: String
+          optional :md5_digest, type: String
+          optional :sha256_digest, type: String
         end
 
         post do
           authorize_upload!(authorized_user_project)
 
+          ::Packages::Pypi::CreatePackageService
+            .new(authorized_user_project, current_user, declared_params)
+            .execute
+
           created!
         rescue ObjectStorage::RemoteStoreError => e
           Gitlab::ErrorTracking.track_exception(e, extra: { file_name: params[:name], project_id: authorized_user_project.id })

ee/spec/requests/api/pypi_packages_spec.rb

@@ -125,7 +125,9 @@ describe API::PypiPackages do
     let_it_be(:file_name) { 'package.whl' }
     let(:url) { "/projects/#{project.id}/packages/pypi" }
     let(:headers) { {} }
-    let(:params) { { content: temp_file(file_name) } }
+    let(:base_params) { { requires_python: '>=3.7', version: '1.0.0', name: 'sample-project', sha256_digest: '123' } }
+    let(:params) { base_params.merge(content: temp_file(file_name)) }
+    let(:send_rewritten_field) { true }
 
     subject do
       workhorse_finalize(
@@ -133,7 +135,8 @@ describe API::PypiPackages do
         method: :post,
         file_key: :content,
         params: params,
-        headers: headers
+        headers: headers,
+        send_rewritten_field: send_rewritten_field
       )
     end
 
@@ -146,7 +149,7 @@ describe API::PypiPackages do
         using RSpec::Parameterized::TableSyntax
 
         where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do
-          'PUBLIC'  | :developer  | true  | true  | 'process PyPi api request' | :created
+          'PUBLIC'  | :developer  | true  | true  | 'PyPi package creation'    | :created
           'PUBLIC'  | :guest      | true  | true  | 'process PyPi api request' | :forbidden
           'PUBLIC'  | :developer  | true  | false | 'process PyPi api request' | :unauthorized
           'PUBLIC'  | :guest      | true  | false | 'process PyPi api request' | :unauthorized
@@ -179,6 +182,19 @@ describe API::PypiPackages do
         end
       end
 
+      context 'with an invalid package' do
+        let(:token) { personal_access_token.token }
+        let(:user_headers) { build_basic_auth_header(user.username, token) }
+        let(:headers) { user_headers.merge(workhorse_header) }
+
+        before do
+          params[:name] = '.$/@!^*'
+          project.add_developer(user)
+        end
+
+        it_behaves_like 'returning response status', :bad_request
+      end
+
       it_behaves_like 'rejects PyPI access with unknown project id'
     end
 

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

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Packages::Pypi::CreatePackageService do
+  include EE::PackagesManagerApiSpecHelpers
+
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:params) do
+    {
+      name: 'foo',
+      version: '1.0',
+      content: temp_file('foo.tgz'),
+      requires_python: '>=2.7',
+      sha256_digest: '123',
+      md5_digest: '567'
+    }
+  end
+
+  describe '#execute' do
+    subject { described_class.new(project, user, params).execute }
+
+    let(:created_package) { Packages::Package.pypi.last }
+
+    context 'without an existing package' do
+      it 'creates the package' do
+        expect { subject }.to change { Packages::Package.pypi.count }.by(1)
+
+        expect(created_package.name).to eq 'foo'
+        expect(created_package.version).to eq '1.0'
+
+        expect(created_package.pypi_metadatum.required_python).to eq '>=2.7'
+        expect(created_package.package_files.size).to eq 1
+        expect(created_package.package_files.first.file_name).to eq 'foo.tgz'
+        expect(created_package.package_files.first.file_sha256).to eq '123'
+        expect(created_package.package_files.first.file_md5).to eq '567'
+      end
+    end
+
+    context 'with an existing package' do
+      before do
+        described_class.new(project, user, params).execute
+      end
+
+      context 'with an existing file' do
+        before do
+          params[:content] = temp_file('foo.tgz')
+          params[:sha256_digest] = 'abc'
+          params[:md5_digest] = 'def'
+        end
+
+        it 'replaces the file' do
+          expect { subject }
+            .to change { Packages::Package.pypi.count }.by(0)
+            .and change { Packages::PackageFile.count }.by(1)
+
+          expect(created_package.package_files.size).to eq 2
+          expect(created_package.package_files.first.file_name).to eq 'foo.tgz'
+          expect(created_package.package_files.first.file_sha256).to eq '123'
+          expect(created_package.package_files.first.file_md5).to eq '567'
+          expect(created_package.package_files.last.file_name).to eq 'foo.tgz'
+          expect(created_package.package_files.last.file_sha256).to eq 'abc'
+          expect(created_package.package_files.last.file_md5).to eq 'def'
+        end
+      end
+
+      context 'without an existing file' do
+        before do
+          params[:content] = temp_file('another.tgz')
+        end
+
+        it 'adds the file' do
+          expect { subject }
+            .to change { Packages::Package.pypi.count }.by(0)
+            .and change { Packages::PackageFile.count }.by(1)
+
+          expect(created_package.package_files.size).to eq 2
+          expect(created_package.package_files.map(&:file_name).sort).to eq ['another.tgz', 'foo.tgz']
+        end
+      end
+    end
+  end
+end

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

 # frozen_string_literal: true
 
+RSpec.shared_examples 'PyPi package creation' do |user_type, status, add_member = true|
+  RSpec.shared_examples 'creating pypi package files' do
+    it 'creates package files' do
+      expect { subject }
+          .to change { project.packages.pypi.count }.by(1)
+          .and change { Packages::PackageFile.count }.by(1)
+          .and change { Packages::PypiMetadatum.count }.by(1)
+      expect(response).to have_gitlab_http_status(status)
+
+      package = project.reload.packages.pypi.last
+
+      expect(package.name).to eq params[:name]
+      expect(package.version).to eq params[:version]
+      expect(package.pypi_metadatum.required_python).to eq params[:requires_python]
+    end
+  end
+
+  context "for user type #{user_type}" do
+    before do
+      project.send("add_#{user_type}", user) if add_member && user_type != :anonymous
+    end
+
+    it_behaves_like 'creating pypi package files'
+
+    context 'with object storage disabled' do
+      before do
+        stub_package_file_object_storage(enabled: false)
+      end
+
+      context 'without a file from workhorse' do
+        let(:send_rewritten_field) { false }
+
+        it_behaves_like 'returning response status', :bad_request
+      end
+
+      context 'with correct params' do
+        it_behaves_like 'package workhorse uploads'
+        it_behaves_like 'creating pypi package files'
+      end
+    end
+
+    context 'with object storage enabled' do
+      let(:tmp_object) do
+        fog_connection.directories.new(key: 'packages').files.create(
+          key: "tmp/uploads/#{file_name}",
+          body: 'content'
+        )
+      end
+      let(:fog_file) { fog_to_uploaded_file(tmp_object) }
+      let(:params) { base_params.merge(content: fog_file, 'content.remote_id' => file_name) }
+
+      context 'and direct upload enabled' do
+        let(:fog_connection) do
+          stub_package_file_object_storage(direct_upload: true)
+        end
+
+        it_behaves_like 'creating pypi package files'
+
+        ['123123', '../../123123'].each do |remote_id|
+          context "with invalid remote_id: #{remote_id}" do
+            let(:params) { base_params.merge(content: fog_file, 'content.remote_id' => remote_id) }
+
+            it_behaves_like 'returning response status', :forbidden
+          end
+        end
+      end
+
+      context 'and direct upload disabled' do
+        context 'and background upload disabled' do
+          let(:fog_connection) do
+            stub_package_file_object_storage(direct_upload: false, background_upload: false)
+          end
+
+          it_behaves_like 'creating pypi package files'
+        end
+
+        context 'and background upload enabled' do
+          let(:fog_connection) do
+            stub_package_file_object_storage(direct_upload: false, background_upload: true)
+          end
+
+          it_behaves_like 'creating pypi package files'
+        end
+      end
+    end
+
+    it_behaves_like 'background upload schedules a file migration'
+  end
+end
+
 RSpec.shared_examples 'process PyPi api request' do |user_type, status, add_member = true|
   context "for user type #{user_type}" do
     before do