From 58eee015c22f98def286d09a0186064976733e10 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 20 Jul 2018 15:15:40 +0300 Subject: [PATCH 01/55] Generate models necessary for maven repository backend Signed-off-by: Dmitriy Zaporozhets --- app/models/packages.rb | 5 +++++ app/models/packages/maven_metadatum.rb | 3 +++ app/models/packages/package.rb | 3 +++ app/models/packages/package_file.rb | 3 +++ ...180720120026_create_packages_package_files.rb | 16 ++++++++++++++++ .../20180720120716_create_packages_packages.rb | 11 +++++++++++ ...80720121404_create_packages_maven_metadata.rb | 12 ++++++++++++ spec/models/packages/maven_metadatum_spec.rb | 5 +++++ spec/models/packages/package_file_spec.rb | 5 +++++ spec/models/packages/package_spec.rb | 5 +++++ 10 files changed, 68 insertions(+) create mode 100644 app/models/packages.rb create mode 100644 app/models/packages/maven_metadatum.rb create mode 100644 app/models/packages/package.rb create mode 100644 app/models/packages/package_file.rb create mode 100644 db/migrate/20180720120026_create_packages_package_files.rb create mode 100644 db/migrate/20180720120716_create_packages_packages.rb create mode 100644 db/migrate/20180720121404_create_packages_maven_metadata.rb create mode 100644 spec/models/packages/maven_metadatum_spec.rb create mode 100644 spec/models/packages/package_file_spec.rb create mode 100644 spec/models/packages/package_spec.rb diff --git a/app/models/packages.rb b/app/models/packages.rb new file mode 100644 index 00000000000000..a3a12aec720b56 --- /dev/null +++ b/app/models/packages.rb @@ -0,0 +1,5 @@ +module Packages + def self.table_name_prefix + 'packages_' + end +end diff --git a/app/models/packages/maven_metadatum.rb b/app/models/packages/maven_metadatum.rb new file mode 100644 index 00000000000000..64cd0c820ac8cf --- /dev/null +++ b/app/models/packages/maven_metadatum.rb @@ -0,0 +1,3 @@ +class Packages::MavenMetadatum < ActiveRecord::Base + belongs_to :package +end diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb new file mode 100644 index 00000000000000..85f5d8e0ce7ad5 --- /dev/null +++ b/app/models/packages/package.rb @@ -0,0 +1,3 @@ +class Packages::Package < ActiveRecord::Base + belongs_to :project +end diff --git a/app/models/packages/package_file.rb b/app/models/packages/package_file.rb new file mode 100644 index 00000000000000..6d92cef3706564 --- /dev/null +++ b/app/models/packages/package_file.rb @@ -0,0 +1,3 @@ +class Packages::PackageFile < ActiveRecord::Base + belongs_to :package +end diff --git a/db/migrate/20180720120026_create_packages_package_files.rb b/db/migrate/20180720120026_create_packages_package_files.rb new file mode 100644 index 00000000000000..13d019122e2cc1 --- /dev/null +++ b/db/migrate/20180720120026_create_packages_package_files.rb @@ -0,0 +1,16 @@ +class CreatePackagesPackageFiles < ActiveRecord::Migration + DOWNTIME = false + + def change + create_table :packages_package_files do |t| + t.references :package, index: true, foreign_key: true, null: false + t.string :file + t.integer :file_type + t.integer :size + t.binary :file_md5 + t.binary :file_sha1 + + t.timestamps null: false + end + end +end diff --git a/db/migrate/20180720120716_create_packages_packages.rb b/db/migrate/20180720120716_create_packages_packages.rb new file mode 100644 index 00000000000000..a797a5e8cc70a9 --- /dev/null +++ b/db/migrate/20180720120716_create_packages_packages.rb @@ -0,0 +1,11 @@ +class CreatePackagesPackages < ActiveRecord::Migration + def change + create_table :packages_packages do |t| + t.references :project, index: true, foreign_key: true, null: false + t.string :name + t.string :version + + t.timestamps null: false + end + end +end diff --git a/db/migrate/20180720121404_create_packages_maven_metadata.rb b/db/migrate/20180720121404_create_packages_maven_metadata.rb new file mode 100644 index 00000000000000..f5e27bfdc5a56e --- /dev/null +++ b/db/migrate/20180720121404_create_packages_maven_metadata.rb @@ -0,0 +1,12 @@ +class CreatePackagesMavenMetadata < ActiveRecord::Migration + def change + create_table :packages_maven_metadata do |t| + t.references :package, index: true, foreign_key: true, null: false + t.string :app_group, null: false + t.string :app_name, null: false + t.string :app_version, null: false + + t.timestamps null: false + end + end +end diff --git a/spec/models/packages/maven_metadatum_spec.rb b/spec/models/packages/maven_metadatum_spec.rb new file mode 100644 index 00000000000000..b94af42ef80a3f --- /dev/null +++ b/spec/models/packages/maven_metadatum_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe Packages::MavenMetadatum, type: :model do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb new file mode 100644 index 00000000000000..cb60e64edab2d2 --- /dev/null +++ b/spec/models/packages/package_file_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe Packages::PackageFile, type: :model do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb new file mode 100644 index 00000000000000..45000e11a0bcca --- /dev/null +++ b/spec/models/packages/package_spec.rb @@ -0,0 +1,5 @@ +require 'rails_helper' + +RSpec.describe Packages::Package, type: :model do + pending "add some examples to (or delete) #{__FILE__}" +end -- GitLab From 6d8c8ac7074a8fbe9d52fd3bd4a8de67b4228970 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 20 Jul 2018 15:24:40 +0300 Subject: [PATCH 02/55] Add associations and cascade delete for foreign keys Signed-off-by: Dmitriy Zaporozhets --- app/models/packages/package.rb | 2 ++ db/migrate/20180720120026_create_packages_package_files.rb | 2 +- db/migrate/20180720120716_create_packages_packages.rb | 2 +- db/migrate/20180720121404_create_packages_maven_metadata.rb | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb index 85f5d8e0ce7ad5..a132038e427f35 100644 --- a/app/models/packages/package.rb +++ b/app/models/packages/package.rb @@ -1,3 +1,5 @@ class Packages::Package < ActiveRecord::Base belongs_to :project + has_many :package_files + has_one :maven_metadatum end diff --git a/db/migrate/20180720120026_create_packages_package_files.rb b/db/migrate/20180720120026_create_packages_package_files.rb index 13d019122e2cc1..bf92b9556e7aa5 100644 --- a/db/migrate/20180720120026_create_packages_package_files.rb +++ b/db/migrate/20180720120026_create_packages_package_files.rb @@ -3,7 +3,7 @@ class CreatePackagesPackageFiles < ActiveRecord::Migration def change create_table :packages_package_files do |t| - t.references :package, index: true, foreign_key: true, null: false + t.references :package, index: true, foreign_key: { on_delete: :cascade }, null: false t.string :file t.integer :file_type t.integer :size diff --git a/db/migrate/20180720120716_create_packages_packages.rb b/db/migrate/20180720120716_create_packages_packages.rb index a797a5e8cc70a9..c73d171941c886 100644 --- a/db/migrate/20180720120716_create_packages_packages.rb +++ b/db/migrate/20180720120716_create_packages_packages.rb @@ -1,7 +1,7 @@ class CreatePackagesPackages < ActiveRecord::Migration def change create_table :packages_packages do |t| - t.references :project, index: true, foreign_key: true, null: false + t.references :project, index: true, foreign_key: { on_delete: :cascade }, null: false t.string :name t.string :version diff --git a/db/migrate/20180720121404_create_packages_maven_metadata.rb b/db/migrate/20180720121404_create_packages_maven_metadata.rb index f5e27bfdc5a56e..ac0e7f01a06475 100644 --- a/db/migrate/20180720121404_create_packages_maven_metadata.rb +++ b/db/migrate/20180720121404_create_packages_maven_metadata.rb @@ -1,7 +1,7 @@ class CreatePackagesMavenMetadata < ActiveRecord::Migration def change create_table :packages_maven_metadata do |t| - t.references :package, index: true, foreign_key: true, null: false + t.references :package, index: true, foreign_key: { on_delete: :cascade }, null: false t.string :app_group, null: false t.string :app_name, null: false t.string :app_version, null: false -- GitLab From de2f836b23f379318803099e7dca7ec8bc30f6e3 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 20 Jul 2018 15:36:49 +0300 Subject: [PATCH 03/55] Add package uploader [ci skip] Signed-off-by: Dmitriy Zaporozhets --- app/models/packages/package_file.rb | 2 ++ app/uploaders/packages/package_file_uploader.rb | 6 ++++++ db/migrate/20180720120026_create_packages_package_files.rb | 1 + 3 files changed, 9 insertions(+) create mode 100644 app/uploaders/packages/package_file_uploader.rb diff --git a/app/models/packages/package_file.rb b/app/models/packages/package_file.rb index 6d92cef3706564..e1535a2c0365cd 100644 --- a/app/models/packages/package_file.rb +++ b/app/models/packages/package_file.rb @@ -1,3 +1,5 @@ class Packages::PackageFile < ActiveRecord::Base belongs_to :package + + mount_uploader :file, PackageFileUploader end diff --git a/app/uploaders/packages/package_file_uploader.rb b/app/uploaders/packages/package_file_uploader.rb new file mode 100644 index 00000000000000..bd7db8f4360080 --- /dev/null +++ b/app/uploaders/packages/package_file_uploader.rb @@ -0,0 +1,6 @@ +class Packages::PackageFileUploader < GitlabUploader + extend Workhorse::UploadPath + include ObjectStorage::Concern + + # TODO: Implement me +end diff --git a/db/migrate/20180720120026_create_packages_package_files.rb b/db/migrate/20180720120026_create_packages_package_files.rb index bf92b9556e7aa5..381df0ec2776e5 100644 --- a/db/migrate/20180720120026_create_packages_package_files.rb +++ b/db/migrate/20180720120026_create_packages_package_files.rb @@ -6,6 +6,7 @@ def change t.references :package, index: true, foreign_key: { on_delete: :cascade }, null: false t.string :file t.integer :file_type + t.integer :file_store t.integer :size t.binary :file_md5 t.binary :file_sha1 -- GitLab From 5ffae25a852305559d9912e438e59b2fe1c04785 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 23 Jul 2018 17:10:55 +0300 Subject: [PATCH 04/55] Add packages to config files [ci skip] Signed-off-by: Dmitriy Zaporozhets --- .../packages/package_file_uploader.rb | 2 ++ config/gitlab.yml.example | 20 +++++++++++++++++++ config/initializers/1_settings.rb | 18 +++++++++++++++++ 3 files changed, 40 insertions(+) diff --git a/app/uploaders/packages/package_file_uploader.rb b/app/uploaders/packages/package_file_uploader.rb index bd7db8f4360080..84e756ad063955 100644 --- a/app/uploaders/packages/package_file_uploader.rb +++ b/app/uploaders/packages/package_file_uploader.rb @@ -2,5 +2,7 @@ class Packages::PackageFileUploader < GitlabUploader extend Workhorse::UploadPath include ObjectStorage::Concern + storage_options Gitlab.config.packages + # TODO: Implement me end diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 745bb6d24e0205..cc0a07600bc5de 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -207,6 +207,26 @@ production: &base # endpoint: 'http://127.0.0.1:9000' # default: nil # path_style: true # Use 'host/bucket_name/object' instead of 'bucket_name.host/object' + ## Packages (maven repository so far) + packages: + enabled: true + # The location where build packages are stored (default: shared/packages). + # storage_path: shared/packages + object_store: + enabled: false + remote_directory: packages # The bucket name + # direct_upload: false # Use Object Storage directly for uploads instead of background uploads if enabled (Default: false) + # background_upload: false # Temporary option to limit automatic upload (Default: true) + # proxy_download: false # Passthrough all downloads via GitLab instead of using Redirects to Object Storage + connection: + provider: AWS + aws_access_key_id: AWS_ACCESS_KEY_ID + aws_secret_access_key: AWS_SECRET_ACCESS_KEY + region: us-east-1 + # host: 'localhost' # default: s3.amazonaws.com + # endpoint: 'http://127.0.0.1:9000' # default: nil + # path_style: true # Use 'host/bucket_name/object' instead of 'bucket_name.host/object' + ## GitLab Pages pages: enabled: false diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index e17788dccb468c..2a89103087c54e 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -252,6 +252,24 @@ Settings.uploads['object_store'] = ObjectStoreSettings.parse(Settings.uploads['object_store']) Settings.uploads['object_store']['remote_directory'] ||= 'uploads' +# +# Packages +# +Settings['packages'] ||= Settingslogic.new({}) +Settings.packages['enabled'] = true if Settings.packages['enabled'].nil? +Settings.packages['storage_path'] = Settings.absolute(Settings.packages['storage_path'] || File.join(Settings.shared['path'], "packages")) +# Settings.artifact['path'] is deprecated, use `storage_path` instead +Settings.packages['path'] = Settings.packages['storage_path'] +Settings.packages['max_size'] ||= 100 # in megabytes +Settings.packages['object_store'] ||= Settingslogic.new({}) +Settings.packages['object_store']['enabled'] = false if Settings.packages['object_store']['enabled'].nil? +Settings.packages['object_store']['remote_directory'] ||= nil +Settings.packages['object_store']['direct_upload'] = false if Settings.packages['object_store']['direct_upload'].nil? +Settings.packages['object_store']['background_upload'] = true if Settings.packages['object_store']['background_upload'].nil? +Settings.packages['object_store']['proxy_download'] = false if Settings.packages['object_store']['proxy_download'].nil? +# Convert upload connection settings to use string keys, to make Fog happy +Settings.packages['object_store']['connection']&.deep_stringify_keys! + # # Mattermost # -- GitLab From 87ff4029be861e1d342034980c9b3046fae5446c Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 24 Jul 2018 18:13:23 +0300 Subject: [PATCH 05/55] Make package migrations pass [ci skip] Signed-off-by: Dmitriy Zaporozhets --- ... 20180720120726_create_packages_package_files.rb} | 10 +++++++++- .../20180720121404_create_packages_maven_metadata.rb | 12 +++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) rename db/migrate/{20180720120026_create_packages_package_files.rb => 20180720120726_create_packages_package_files.rb} (56%) diff --git a/db/migrate/20180720120026_create_packages_package_files.rb b/db/migrate/20180720120726_create_packages_package_files.rb similarity index 56% rename from db/migrate/20180720120026_create_packages_package_files.rb rename to db/migrate/20180720120726_create_packages_package_files.rb index 381df0ec2776e5..0c51587c04115c 100644 --- a/db/migrate/20180720120026_create_packages_package_files.rb +++ b/db/migrate/20180720120726_create_packages_package_files.rb @@ -1,9 +1,13 @@ class CreatePackagesPackageFiles < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + DOWNTIME = false + disable_ddl_transaction! + def change create_table :packages_package_files do |t| - t.references :package, index: true, foreign_key: { on_delete: :cascade }, null: false + t.references :package, index: true, null: false t.string :file t.integer :file_type t.integer :file_store @@ -13,5 +17,9 @@ def change t.timestamps null: false end + + add_concurrent_foreign_key :packages_package_files, :packages_packages, + column: :package_id, + on_delete: :cascade end end diff --git a/db/migrate/20180720121404_create_packages_maven_metadata.rb b/db/migrate/20180720121404_create_packages_maven_metadata.rb index ac0e7f01a06475..f4cd9574894b73 100644 --- a/db/migrate/20180720121404_create_packages_maven_metadata.rb +++ b/db/migrate/20180720121404_create_packages_maven_metadata.rb @@ -1,12 +1,22 @@ class CreatePackagesMavenMetadata < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + def change create_table :packages_maven_metadata do |t| - t.references :package, index: true, foreign_key: { on_delete: :cascade }, null: false + t.references :package, index: true, null: false t.string :app_group, null: false t.string :app_name, null: false t.string :app_version, null: false t.timestamps null: false end + + add_concurrent_foreign_key :packages_maven_metadata, :packages_packages, + column: :package_id, + on_delete: :cascade end end -- GitLab From c45ecaf909e02b84ab5f382a042d7014f011b317 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 25 Jul 2018 15:21:10 +0300 Subject: [PATCH 06/55] Add support for maven package download via API [ci skip] Signed-off-by: Dmitriy Zaporozhets --- .../packages/package_file_uploader.rb | 15 +++++- lib/api/api.rb | 1 + lib/api/packages.rb | 48 +++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 lib/api/packages.rb diff --git a/app/uploaders/packages/package_file_uploader.rb b/app/uploaders/packages/package_file_uploader.rb index 84e756ad063955..57ccfaf4f8ce98 100644 --- a/app/uploaders/packages/package_file_uploader.rb +++ b/app/uploaders/packages/package_file_uploader.rb @@ -4,5 +4,18 @@ class Packages::PackageFileUploader < GitlabUploader storage_options Gitlab.config.packages - # TODO: Implement me + def store_dir + dynamic_segment + end + + private + + def dynamic_segment + File.join(disk_hash[0..1], disk_hash[2..3], disk_hash, + 'packages', model.package.id.to_s, 'files', model.id.to_s) + end + + def disk_hash + @disk_hash ||= Digest::SHA2.hexdigest(model.package.project_id.to_s) + end end diff --git a/lib/api/api.rb b/lib/api/api.rb index 03e61cb7b31f84..4c1d4770dde42e 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -172,6 +172,7 @@ class API < Grape::API mount ::API::License mount ::API::ProjectMirror mount ::API::ProjectPushRule + mount ::API::Packages ## EE-specific API V4 endpoints END route :any, '*path' do diff --git a/lib/api/packages.rb b/lib/api/packages.rb new file mode 100644 index 00000000000000..d35771e7c1446a --- /dev/null +++ b/lib/api/packages.rb @@ -0,0 +1,48 @@ +module API + class Packages < Grape::API + content_type :md5, 'text/plain' + content_type :sha1, 'text/plain' + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do + desc 'Download the maven package file' do + detail 'This feature was introduced in GitLab 11.3' + end + get ':id/maven/*file_path' do + checksum = %w(md5 sha1).include?(params[:format]) + + file_path, _, file_name = params[:file_path].rpartition('/') + file_name = file_name + '.' + params[:format] unless checksum + + app_path, _, version = file_path.rpartition('/') + app_group, _, app_name = app_path.rpartition('/') + + metadata = ::Packages::MavenMetadatum.find_by(app_group: app_group, + app_name: app_name, + app_version: version) + + package_file = metadata.package.package_files.find_by(file: file_name) + + if checksum + case params[:format] + when 'md5' + package_file.file_md5 + when 'sha1' + package_file.file_sha1 + end + else + present_carrierwave_file!(package_file.file) + end + end + + desc 'Upload the maven package file' do + detail 'This feature was introduced in GitLab 11.3' + end + post ':id/maven/*file_path' do + # parse file path, create an upload, save record in database + end + end + end +end -- GitLab From 10ef04d47892f1518f252dd2b729c28d5e8a47d2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 25 Jul 2018 18:42:42 +0300 Subject: [PATCH 07/55] Add minimal UI for project packages [ci skip] Signed-off-by: Dmitriy Zaporozhets --- .../projects/packages/packages_controller.rb | 11 +++++++ app/models/project.rb | 1 + app/views/projects/packages/index.html.haml | 26 ++++++++++++++++ .../packages/packages/index.html.haml | 30 +++++++++++++++++++ config/routes/project.rb | 4 +++ 5 files changed, 72 insertions(+) create mode 100644 app/controllers/projects/packages/packages_controller.rb create mode 100644 app/views/projects/packages/index.html.haml create mode 100644 app/views/projects/packages/packages/index.html.haml diff --git a/app/controllers/projects/packages/packages_controller.rb b/app/controllers/projects/packages/packages_controller.rb new file mode 100644 index 00000000000000..d897f7ed8338f5 --- /dev/null +++ b/app/controllers/projects/packages/packages_controller.rb @@ -0,0 +1,11 @@ +module Projects + module Packages + class PackagesController < ApplicationController + before_action :authorize_admin_project! + + def index + @packages = project.packages.all + end + end + end +end diff --git a/app/models/project.rb b/app/models/project.rb index bfc0a50646c3dc..18fddb53cf5f9a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -131,6 +131,7 @@ class Project < ActiveRecord::Base has_one :last_event, -> {order 'events.created_at DESC'}, class_name: 'Event' has_many :boards, before_add: :validate_board_limit + has_many :packages, class_name: 'Packages::Package' # Project services has_one :campfire_service diff --git a/app/views/projects/packages/index.html.haml b/app/views/projects/packages/index.html.haml new file mode 100644 index 00000000000000..8b3c9c07d6fc5e --- /dev/null +++ b/app/views/projects/packages/index.html.haml @@ -0,0 +1,26 @@ +- @no_container = true +- page_title "Packages" + +%div{ class: container_class } + %table.table + %thead + %tr + %th Name + %th Version + %th Files + %th Created at + + %tbody + - @packages.each do |package| + %tr + %td + = package.name + %td + = package.version + %td + %ul + - package.package_files.each do |package_file| + %li + = package_file.file + %td + = package.created_at diff --git a/app/views/projects/packages/packages/index.html.haml b/app/views/projects/packages/packages/index.html.haml new file mode 100644 index 00000000000000..e41e0a354763f5 --- /dev/null +++ b/app/views/projects/packages/packages/index.html.haml @@ -0,0 +1,30 @@ +- @no_container = true +- page_title "Packages" + + +%div{ class: container_class } + %table.table + %thead + %tr + %th ID + %th Name + %th Version + %th Files + %th Created at + + %tbody + - @packages.each do |package| + %tr + %td + = package.id + %td + = package.name + %td + = package.version + %td + %ul.content-list + - package.package_files.each do |package_file| + %li + = package_file.file.identifier + %td + = package.created_at diff --git a/config/routes/project.rb b/config/routes/project.rb index 52f899d10d52ca..c2f7e01f04b788 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -310,6 +310,10 @@ scope '-' do get 'archive/*id', constraints: { format: Gitlab::PathRegex.archive_formats_regex, id: /.+?/ }, to: 'repositories#archive', as: 'archive' + ## EE-specific + resources :packages, only: :index, module: 'packages' + ## EE-specific + resources :jobs, only: [:index, :show], constraints: { id: /\d+/ } do collection do post :cancel_all -- GitLab From 52ffee740a625003a7b5ce7d0f2a2d06ca8b3abf Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 26 Jul 2018 18:59:22 +0300 Subject: [PATCH 08/55] Refactor maven repository API Signed-off-by: Dmitriy Zaporozhets --- lib/api/api.rb | 2 +- lib/api/maven_packages.rb | 66 +++++++++++++++++++++++++++++++++++++++ lib/api/packages.rb | 48 ---------------------------- 3 files changed, 67 insertions(+), 49 deletions(-) create mode 100644 lib/api/maven_packages.rb delete mode 100644 lib/api/packages.rb diff --git a/lib/api/api.rb b/lib/api/api.rb index 4c1d4770dde42e..ea42b50d80c8e6 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -172,7 +172,7 @@ class API < Grape::API mount ::API::License mount ::API::ProjectMirror mount ::API::ProjectPushRule - mount ::API::Packages + mount ::API::MavenPackages ## EE-specific API V4 endpoints END route :any, '*path' do diff --git a/lib/api/maven_packages.rb b/lib/api/maven_packages.rb new file mode 100644 index 00000000000000..6afe7b2d19dff7 --- /dev/null +++ b/lib/api/maven_packages.rb @@ -0,0 +1,66 @@ +module API + class MavenPackages < Grape::API + MAVEN_ENDPOINT_REQUIREMENTS = { + app_name: API::NO_SLASH_URL_PART_REGEX, + app_version: API::NO_SLASH_URL_PART_REGEX, + file_name: API::NO_SLASH_URL_PART_REGEX + }.freeze + + content_type :md5, 'text/plain' + content_type :sha1, 'text/plain' + content_type :binary, 'application/octet-stream' + + helpers do + def extract_format(file_name) + name, _, format = file_name.rpartition('.') + + if %w(md5 sha1).include?(format) + [name, format] + else + [file_name, nil] + end + end + end + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do + desc 'Download the maven package file' do + detail 'This feature was introduced in GitLab 11.3' + end + params do + requires :app_group, type: String, desc: 'Package group id' + requires :app_name, type: String, desc: 'Package artifact id' + requires :app_version, type: String, desc: 'Package version' + requires :file_name, type: String, desc: 'Package file name' + end + get ':id/packages/maven/*app_group/:app_name/:app_version/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do + file_name, format = extract_format(params[:file_name]) + + metadata = ::Packages::MavenMetadatum.find_by!(app_group: params[:app_group], + app_name: params[:app_name], + app_version: params[:app_version]) + + + package_file = metadata.package.package_files.find_by!(file: file_name) + + case format + when 'md5' + package_file.file_md5 + when 'sha1' + package_file.file_sha1 + else + present_carrierwave_file!(package_file.file) + end + end + + desc 'Upload the maven package file' do + detail 'This feature was introduced in GitLab 11.3' + end + put ':id/packages/maven/*app_group/:app_name/:app_version/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do + # TODO: Implement me + end + end + end +end diff --git a/lib/api/packages.rb b/lib/api/packages.rb deleted file mode 100644 index d35771e7c1446a..00000000000000 --- a/lib/api/packages.rb +++ /dev/null @@ -1,48 +0,0 @@ -module API - class Packages < Grape::API - content_type :md5, 'text/plain' - content_type :sha1, 'text/plain' - - params do - requires :id, type: String, desc: 'The ID of a project' - end - resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do - desc 'Download the maven package file' do - detail 'This feature was introduced in GitLab 11.3' - end - get ':id/maven/*file_path' do - checksum = %w(md5 sha1).include?(params[:format]) - - file_path, _, file_name = params[:file_path].rpartition('/') - file_name = file_name + '.' + params[:format] unless checksum - - app_path, _, version = file_path.rpartition('/') - app_group, _, app_name = app_path.rpartition('/') - - metadata = ::Packages::MavenMetadatum.find_by(app_group: app_group, - app_name: app_name, - app_version: version) - - package_file = metadata.package.package_files.find_by(file: file_name) - - if checksum - case params[:format] - when 'md5' - package_file.file_md5 - when 'sha1' - package_file.file_sha1 - end - else - present_carrierwave_file!(package_file.file) - end - end - - desc 'Upload the maven package file' do - detail 'This feature was introduced in GitLab 11.3' - end - post ':id/maven/*file_path' do - # parse file path, create an upload, save record in database - end - end - end -end -- GitLab From f933ace6777f1a3317dcb76abd311ecdbde69ef2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 29 Jul 2018 12:22:04 +0300 Subject: [PATCH 09/55] Implement upload package logic without workhorse for now Signed-off-by: Dmitriy Zaporozhets --- app/models/packages/maven_metadatum.rb | 2 + app/models/packages/package.rb | 2 + app/models/packages/package_file.rb | 2 + .../packages/package_file_uploader.rb | 4 ++ ...720120726_create_packages_package_files.rb | 1 + db/schema.rb | 36 ++++++++++++ lib/api/maven_packages.rb | 55 ++++++++++++++++++- spec/models/packages/maven_metadatum_spec.rb | 8 ++- spec/models/packages/package_file_spec.rb | 8 ++- spec/models/packages/package_spec.rb | 9 ++- 10 files changed, 121 insertions(+), 6 deletions(-) diff --git a/app/models/packages/maven_metadatum.rb b/app/models/packages/maven_metadatum.rb index 64cd0c820ac8cf..92efdb8d1080d0 100644 --- a/app/models/packages/maven_metadatum.rb +++ b/app/models/packages/maven_metadatum.rb @@ -1,3 +1,5 @@ class Packages::MavenMetadatum < ActiveRecord::Base belongs_to :package + + validates :package, presence: true end diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb index a132038e427f35..3fa35313217fa5 100644 --- a/app/models/packages/package.rb +++ b/app/models/packages/package.rb @@ -2,4 +2,6 @@ class Packages::Package < ActiveRecord::Base belongs_to :project has_many :package_files has_one :maven_metadatum + + validates :project, presence: true end diff --git a/app/models/packages/package_file.rb b/app/models/packages/package_file.rb index e1535a2c0365cd..4f859c270dd4d3 100644 --- a/app/models/packages/package_file.rb +++ b/app/models/packages/package_file.rb @@ -1,5 +1,7 @@ class Packages::PackageFile < ActiveRecord::Base belongs_to :package + validates :package, presence: true + mount_uploader :file, PackageFileUploader end diff --git a/app/uploaders/packages/package_file_uploader.rb b/app/uploaders/packages/package_file_uploader.rb index 57ccfaf4f8ce98..c7bc41a9437975 100644 --- a/app/uploaders/packages/package_file_uploader.rb +++ b/app/uploaders/packages/package_file_uploader.rb @@ -4,6 +4,10 @@ class Packages::PackageFileUploader < GitlabUploader storage_options Gitlab.config.packages + def filename + model.file_name + end + def store_dir dynamic_segment end diff --git a/db/migrate/20180720120726_create_packages_package_files.rb b/db/migrate/20180720120726_create_packages_package_files.rb index 0c51587c04115c..1b9145e4b41f40 100644 --- a/db/migrate/20180720120726_create_packages_package_files.rb +++ b/db/migrate/20180720120726_create_packages_package_files.rb @@ -9,6 +9,7 @@ def change create_table :packages_package_files do |t| t.references :package, index: true, null: false t.string :file + t.string :file_name, null: false t.integer :file_type t.integer :file_store t.integer :size diff --git a/db/schema.rb b/db/schema.rb index 5b3527ae2ce79d..5a0e79622b005d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1921,6 +1921,42 @@ t.string "nonce", null: false end + create_table "packages_maven_metadata", force: :cascade do |t| + t.integer "package_id", null: false + t.string "app_group", null: false + t.string "app_name", null: false + t.string "app_version", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "packages_maven_metadata", ["package_id"], name: "index_packages_maven_metadata_on_package_id", using: :btree + + create_table "packages_package_files", force: :cascade do |t| + t.integer "package_id", null: false + t.string "file" + t.string "file_name", null: false + t.integer "file_type" + t.integer "file_store" + t.integer "size" + t.binary "file_md5" + t.binary "file_sha1" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "packages_package_files", ["package_id"], name: "index_packages_package_files_on_package_id", using: :btree + + create_table "packages_packages", force: :cascade do |t| + t.integer "project_id", null: false + t.string "name" + t.string "version" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "packages_packages", ["project_id"], name: "index_packages_packages_on_project_id", using: :btree + create_table "pages_domains", force: :cascade do |t| t.integer "project_id" t.text "certificate" diff --git a/lib/api/maven_packages.rb b/lib/api/maven_packages.rb index 6afe7b2d19dff7..4a43a3c950bd46 100644 --- a/lib/api/maven_packages.rb +++ b/lib/api/maven_packages.rb @@ -6,6 +6,8 @@ class MavenPackages < Grape::API file_name: API::NO_SLASH_URL_PART_REGEX }.freeze + MAVEN_METADATA_FILE = 'maven-metadata.xml'.freeze + content_type :md5, 'text/plain' content_type :sha1, 'text/plain' content_type :binary, 'application/octet-stream' @@ -42,8 +44,7 @@ def extract_format(file_name) app_name: params[:app_name], app_version: params[:app_version]) - - package_file = metadata.package.package_files.find_by!(file: file_name) + package_file = metadata.package.package_files.find_by!(file_name: file_name) case format when 'md5' @@ -59,7 +60,55 @@ def extract_format(file_name) detail 'This feature was introduced in GitLab 11.3' end put ':id/packages/maven/*app_group/:app_name/:app_version/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do - # TODO: Implement me + file_name, format = extract_format(params[:file_name]) + + metadata = ::Packages::MavenMetadatum.find_by(app_group: params[:app_group], + app_name: params[:app_name], + app_version: params[:app_version]) + + if metadata + # Everything seems legit. We can proceed to file uploading + else + if file_name == MAVEN_METADATA_FILE + xml = env['api.request.input'] + version = Nokogiri::XML(xml).css('metadata:root > version').text + + # Skip handling top level maven-metadata.xml for now + # Also stop request if version in metadata file differs from one in URL + return unless version || version != params[:app_version] + end + + package = Packages::Package.crate(project: user_project) + + metadata = ::Packages::MavenMetadatum.crate_by!( + project: package.project, + app_group: params[:app_group], + app_name: params[:app_name], + app_version: params[:app_version] + ) + end + + # Convert string into CarrierWave compatible StringIO object + string_file = CarrierWaveStringFile.new(env['api.request.input']) + + if format + package_file = metadata.package.package_files.find_by!(file_name: file_name) + + case format + when 'md5' + package_file.file_md5 = string_file + when 'sha1' + package_file.file_sha1 = string_file + end + + package_file.save! + else + package_file = metadata.package.package_files.new + package_file.file_name = file_name + package_file.file_type = file_name.rpartition('.').last + package_file.file = string_file + package_file.save! + end end end end diff --git a/spec/models/packages/maven_metadatum_spec.rb b/spec/models/packages/maven_metadatum_spec.rb index b94af42ef80a3f..3a1c1154ed9502 100644 --- a/spec/models/packages/maven_metadatum_spec.rb +++ b/spec/models/packages/maven_metadatum_spec.rb @@ -1,5 +1,11 @@ require 'rails_helper' RSpec.describe Packages::MavenMetadatum, type: :model do - pending "add some examples to (or delete) #{__FILE__}" + describe 'relationships' do + it { is_expected.to belong_to(:package) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:package) } + end end diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index cb60e64edab2d2..619b005df53c04 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -1,5 +1,11 @@ require 'rails_helper' RSpec.describe Packages::PackageFile, type: :model do - pending "add some examples to (or delete) #{__FILE__}" + describe 'relationships' do + it { is_expected.to belong_to(:package) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:package) } + end end diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 45000e11a0bcca..4d76665b0ea6a2 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -1,5 +1,12 @@ require 'rails_helper' RSpec.describe Packages::Package, type: :model do - pending "add some examples to (or delete) #{__FILE__}" + describe 'relationships' do + it { is_expected.to belong_to(:project) } + it { is_expected.to have_many(:package_files) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + end end -- GitLab From d07b8d1759a463ae12b53db2bdbc3f5df1657cfa Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 29 Jul 2018 14:32:28 +0300 Subject: [PATCH 10/55] Require auth for non get requests to maven packages Signed-off-by: Dmitriy Zaporozhets --- lib/api/maven_packages.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/api/maven_packages.rb b/lib/api/maven_packages.rb index 4a43a3c950bd46..7837a846320966 100644 --- a/lib/api/maven_packages.rb +++ b/lib/api/maven_packages.rb @@ -12,6 +12,8 @@ class MavenPackages < Grape::API content_type :sha1, 'text/plain' content_type :binary, 'application/octet-stream' + before { authenticate_non_get! } + helpers do def extract_format(file_name) name, _, format = file_name.rpartition('.') -- GitLab From a76fb6d8ea5adb22b7959c61a1f39f125123263b Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 29 Jul 2018 15:07:36 +0300 Subject: [PATCH 11/55] Make package uploading work and add some docs about feature Signed-off-by: Dmitriy Zaporozhets --- doc/user/project/maven_packages.md | 55 ++++++++++++++++++++++++++++++ lib/api/maven_packages.rb | 8 ++--- 2 files changed, 59 insertions(+), 4 deletions(-) create mode 100644 doc/user/project/maven_packages.md diff --git a/doc/user/project/maven_packages.md b/doc/user/project/maven_packages.md new file mode 100644 index 00000000000000..3ff70135efaa57 --- /dev/null +++ b/doc/user/project/maven_packages.md @@ -0,0 +1,55 @@ +# GitLab Maven Packages repository + +## Configure project to use GitLab Maven Repository URL + +To download packages from GitLab, you need `repository` section in your `pom.xml`. + +```xml + + + gitlab-maven + https://gitlab.com/api/v4/projects/PROJECT_ID/packages/maven + + +``` + +To upload packages to GitLab, you need a `distributionManagement` section in your `pom.xml`. + +```xml + + + gitlab-maven + https://gitlab.com/api/v4/projects/PROJECT_ID/packages/maven + + +``` + +In both examples, replace `PROJECT_ID` with your project ID. +If you have a private GitLab installation, replace `gitlab.com` with your domain name. + +## Configure repository access + +If a project is private, credentials will need to be provided for authorization. +The preferred way to do this, is by using a [personal access tokens][pat]. +You can add a corresponding section to your `settings.xml` file: + + +```xml + + + + gitlab-maven + + + + Private-Token + REPLACE_WITH_YOUR_PRIVATE_TOKEN + + + + + + +``` + +[pat]: ../profile/personal_access_tokens.md diff --git a/lib/api/maven_packages.rb b/lib/api/maven_packages.rb index 7837a846320966..a2add87a9d5bde 100644 --- a/lib/api/maven_packages.rb +++ b/lib/api/maven_packages.rb @@ -77,13 +77,13 @@ def extract_format(file_name) # Skip handling top level maven-metadata.xml for now # Also stop request if version in metadata file differs from one in URL - return unless version || version != params[:app_version] + return if version.blank? || version != params[:app_version] end - package = Packages::Package.crate(project: user_project) + package = Packages::Package.create(project: user_project) - metadata = ::Packages::MavenMetadatum.crate_by!( - project: package.project, + metadata = ::Packages::MavenMetadatum.create!( + package: package, app_group: params[:app_group], app_name: params[:app_name], app_version: params[:app_version] -- GitLab From 747d901c9a336b1bd54a765836d225a22378491c Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 30 Jul 2018 11:58:21 +0300 Subject: [PATCH 12/55] Refactor maven packages code [ci skip] Signed-off-by: Dmitriy Zaporozhets --- .../packages/create_maven_package_service.rb | 14 +++++ config/initializers/1_settings.rb | 9 +--- ...20180720120716_create_packages_packages.rb | 4 +- ...720120726_create_packages_package_files.rb | 2 +- ...20121404_create_packages_maven_metadata.rb | 2 +- lib/api/maven_packages.rb | 53 ++++++++++--------- 6 files changed, 48 insertions(+), 36 deletions(-) create mode 100644 app/services/packages/create_maven_package_service.rb diff --git a/app/services/packages/create_maven_package_service.rb b/app/services/packages/create_maven_package_service.rb new file mode 100644 index 00000000000000..115de9e032181c --- /dev/null +++ b/app/services/packages/create_maven_package_service.rb @@ -0,0 +1,14 @@ +module Packages + class CreateMavenPackageService < BaseService + def execute + package = Packages::Package.create(project: project) + + Packages::MavenMetadatum.create!( + package: package, + app_group: params[:app_group], + app_name: params[:app_name], + app_version: params[:app_version] + ) + end + end +end diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 2a89103087c54e..52e0def1148569 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -261,14 +261,7 @@ # Settings.artifact['path'] is deprecated, use `storage_path` instead Settings.packages['path'] = Settings.packages['storage_path'] Settings.packages['max_size'] ||= 100 # in megabytes -Settings.packages['object_store'] ||= Settingslogic.new({}) -Settings.packages['object_store']['enabled'] = false if Settings.packages['object_store']['enabled'].nil? -Settings.packages['object_store']['remote_directory'] ||= nil -Settings.packages['object_store']['direct_upload'] = false if Settings.packages['object_store']['direct_upload'].nil? -Settings.packages['object_store']['background_upload'] = true if Settings.packages['object_store']['background_upload'].nil? -Settings.packages['object_store']['proxy_download'] = false if Settings.packages['object_store']['proxy_download'].nil? -# Convert upload connection settings to use string keys, to make Fog happy -Settings.packages['object_store']['connection']&.deep_stringify_keys! +Settings.packages['object_store'] = ObjectStoreSettings.parse(Settings.packages['object_store']) # # Mattermost diff --git a/db/migrate/20180720120716_create_packages_packages.rb b/db/migrate/20180720120716_create_packages_packages.rb index c73d171941c886..8a1413f05610d2 100644 --- a/db/migrate/20180720120716_create_packages_packages.rb +++ b/db/migrate/20180720120716_create_packages_packages.rb @@ -1,11 +1,13 @@ class CreatePackagesPackages < ActiveRecord::Migration + DOWNTIME = false + def change create_table :packages_packages do |t| t.references :project, index: true, foreign_key: { on_delete: :cascade }, null: false t.string :name t.string :version - t.timestamps null: false + t.timestamps_with_timezone null: false end end end diff --git a/db/migrate/20180720120726_create_packages_package_files.rb b/db/migrate/20180720120726_create_packages_package_files.rb index 1b9145e4b41f40..f283bdf0b8fe7f 100644 --- a/db/migrate/20180720120726_create_packages_package_files.rb +++ b/db/migrate/20180720120726_create_packages_package_files.rb @@ -16,7 +16,7 @@ def change t.binary :file_md5 t.binary :file_sha1 - t.timestamps null: false + t.timestamps_with_timezone null: false end add_concurrent_foreign_key :packages_package_files, :packages_packages, diff --git a/db/migrate/20180720121404_create_packages_maven_metadata.rb b/db/migrate/20180720121404_create_packages_maven_metadata.rb index f4cd9574894b73..71e19e03ecfc1e 100644 --- a/db/migrate/20180720121404_create_packages_maven_metadata.rb +++ b/db/migrate/20180720121404_create_packages_maven_metadata.rb @@ -12,7 +12,7 @@ def change t.string :app_name, null: false t.string :app_version, null: false - t.timestamps null: false + t.timestamps_with_timezone null: false end add_concurrent_foreign_key :packages_maven_metadata, :packages_packages, diff --git a/lib/api/maven_packages.rb b/lib/api/maven_packages.rb index a2add87a9d5bde..dfbb8f703ae3d3 100644 --- a/lib/api/maven_packages.rb +++ b/lib/api/maven_packages.rb @@ -24,6 +24,14 @@ def extract_format(file_name) [file_name, nil] end end + + def valid_metadata_xml?(xml) + version = Nokogiri::XML(xml).css('metadata:root > version').text + + # Skip handling top level maven-metadata.xml (one without the version) for now. + # Also make sure version in the metadata file is equal to one in the URL + version.present? && version == params[:app_version] + end end params do @@ -53,7 +61,7 @@ def extract_format(file_name) package_file.file_md5 when 'sha1' package_file.file_sha1 - else + when nil present_carrierwave_file!(package_file.file) end end @@ -61,39 +69,33 @@ def extract_format(file_name) desc 'Upload the maven package file' do detail 'This feature was introduced in GitLab 11.3' end + params do + requires :app_group, type: String, desc: 'Package group id' + requires :app_name, type: String, desc: 'Package artifact id' + requires :app_version, type: String, desc: 'Package version' + requires :file_name, type: String, desc: 'Package file name' + end put ':id/packages/maven/*app_group/:app_name/:app_version/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do file_name, format = extract_format(params[:file_name]) + string_file = env['api.request.input'] metadata = ::Packages::MavenMetadatum.find_by(app_group: params[:app_group], app_name: params[:app_name], app_version: params[:app_version]) - if metadata - # Everything seems legit. We can proceed to file uploading - else + unless metadata if file_name == MAVEN_METADATA_FILE - xml = env['api.request.input'] - version = Nokogiri::XML(xml).css('metadata:root > version').text - - # Skip handling top level maven-metadata.xml for now - # Also stop request if version in metadata file differs from one in URL - return if version.blank? || version != params[:app_version] + return unless valid_metadata_xml?(string_file) end - package = Packages::Package.create(project: user_project) - - metadata = ::Packages::MavenMetadatum.create!( - package: package, - app_group: params[:app_group], - app_name: params[:app_name], - app_version: params[:app_version] - ) + # There is no metadata for this upload. We need to create a package + # record and corresponding maven metadata record + metadata = Packages::CreateMavenPackageService.new(user_project, current_user, params).execute end - # Convert string into CarrierWave compatible StringIO object - string_file = CarrierWaveStringFile.new(env['api.request.input']) - if format + # Maven tries to create a md5 and sha1 files for each package file. + # Instead, we update existing package file record with such data. package_file = metadata.package.package_files.find_by!(file_name: file_name) case format @@ -102,15 +104,16 @@ def extract_format(file_name) when 'sha1' package_file.file_sha1 = string_file end - - package_file.save! else package_file = metadata.package.package_files.new package_file.file_name = file_name package_file.file_type = file_name.rpartition('.').last - package_file.file = string_file - package_file.save! + + # Convert string into CarrierWave compatible StringIO object + package_file.file = CarrierWaveStringFile.new(string_file) end + + package_file.save! end end end -- GitLab From 5f681af8f3f8703e56aed330f620a0f522980bdc Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 30 Jul 2018 12:42:52 +0300 Subject: [PATCH 13/55] Fix wrong file serve when deploy new maven package build with same version [ci skip] Signed-off-by: Dmitriy Zaporozhets --- app/models/packages/package_file.rb | 2 ++ .../packages/create_maven_package_service.rb | 12 +++++++++++- lib/api/maven_packages.rb | 6 ++++-- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/app/models/packages/package_file.rb b/app/models/packages/package_file.rb index 4f859c270dd4d3..8bedeceba95ee7 100644 --- a/app/models/packages/package_file.rb +++ b/app/models/packages/package_file.rb @@ -4,4 +4,6 @@ class Packages::PackageFile < ActiveRecord::Base validates :package, presence: true mount_uploader :file, PackageFileUploader + + scope :recent, -> { reorder(id: :desc) } end diff --git a/app/services/packages/create_maven_package_service.rb b/app/services/packages/create_maven_package_service.rb index 115de9e032181c..38426de50da3bd 100644 --- a/app/services/packages/create_maven_package_service.rb +++ b/app/services/packages/create_maven_package_service.rb @@ -1,7 +1,11 @@ module Packages class CreateMavenPackageService < BaseService def execute - package = Packages::Package.create(project: project) + package = Packages::Package.create!( + project: project, + name: full_app_name, + version: params[:app_version] + ) Packages::MavenMetadatum.create!( package: package, @@ -10,5 +14,11 @@ def execute app_version: params[:app_version] ) end + + private + + def full_app_name + params[:app_group] + '/' + params[:app_name] + end end end diff --git a/lib/api/maven_packages.rb b/lib/api/maven_packages.rb index dfbb8f703ae3d3..b62a04202cb133 100644 --- a/lib/api/maven_packages.rb +++ b/lib/api/maven_packages.rb @@ -54,7 +54,7 @@ def valid_metadata_xml?(xml) app_name: params[:app_name], app_version: params[:app_version]) - package_file = metadata.package.package_files.find_by!(file_name: file_name) + package_file = metadata.package.package_files.recent.find_by!(file_name: file_name) case format when 'md5' @@ -96,7 +96,7 @@ def valid_metadata_xml?(xml) if format # Maven tries to create a md5 and sha1 files for each package file. # Instead, we update existing package file record with such data. - package_file = metadata.package.package_files.find_by!(file_name: file_name) + package_file = metadata.package.package_files.recent.find_by!(file_name: file_name) case format when 'md5' @@ -105,6 +105,8 @@ def valid_metadata_xml?(xml) package_file.file_sha1 = string_file end else + # TODO: If maven-metadata.xml file is present for this package, we + # need to update it instead of creating a new one package_file = metadata.package.package_files.new package_file.file_name = file_name package_file.file_type = file_name.rpartition('.').last -- GitLab From 0a18081d1d5fcf53518c24814224794e3c834b99 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 30 Jul 2018 14:13:11 +0300 Subject: [PATCH 14/55] Add more UI to browse maven packages [ci skip] Signed-off-by: Dmitriy Zaporozhets --- .../projects/packages/packages_controller.rb | 15 ++++- .../packages/packages/index.html.haml | 43 ++++++-------- .../projects/packages/packages/show.html.haml | 56 +++++++++++++++++++ config/routes/project.rb | 2 +- 4 files changed, 89 insertions(+), 27 deletions(-) create mode 100644 app/views/projects/packages/packages/show.html.haml diff --git a/app/controllers/projects/packages/packages_controller.rb b/app/controllers/projects/packages/packages_controller.rb index d897f7ed8338f5..3c12349b1719a8 100644 --- a/app/controllers/projects/packages/packages_controller.rb +++ b/app/controllers/projects/packages/packages_controller.rb @@ -4,7 +4,20 @@ class PackagesController < ApplicationController before_action :authorize_admin_project! def index - @packages = project.packages.all + @packages = project.packages.all.page(params[:page]) + end + + def show + @package = project.packages.find(params[:id]) + @package_files = @package.package_files.recent + @maven_metadatum = @package.maven_metadatum + end + + def destroy + @package = project.packages.find(params[:id]) + @package.destroy + + redirect_to project_packages_path(@project), notice: _('Package was removed') end end end diff --git a/app/views/projects/packages/packages/index.html.haml b/app/views/projects/packages/packages/index.html.haml index e41e0a354763f5..465c46a02bcd64 100644 --- a/app/views/projects/packages/packages/index.html.haml +++ b/app/views/projects/packages/packages/index.html.haml @@ -1,30 +1,23 @@ -- @no_container = true - page_title "Packages" +%table.table + %thead + %tr + %th Name + %th Version + %th Type + %th Created at -%div{ class: container_class } - %table.table - %thead + %tbody + - @packages.each do |package| %tr - %th ID - %th Name - %th Version - %th Files - %th Created at + %td + = link_to package.name, project_package_path(@project, package) + %td + = package.version + %td + = _('Maven package') + %td + = package.created_at - %tbody - - @packages.each do |package| - %tr - %td - = package.id - %td - = package.name - %td - = package.version - %td - %ul.content-list - - package.package_files.each do |package_file| - %li - = package_file.file.identifier - %td - = package.created_at += paginate @packages, theme: "gitlab" diff --git a/app/views/projects/packages/packages/show.html.haml b/app/views/projects/packages/packages/show.html.haml new file mode 100644 index 00000000000000..da8808f816c955 --- /dev/null +++ b/app/views/projects/packages/packages/show.html.haml @@ -0,0 +1,56 @@ +- add_to_breadcrumbs _("Packages"), project_packages_path(@project) +- add_to_breadcrumbs @package.name, project_packages_path(@project) +- breadcrumb_title @package.version +- page_title "Packages" + +.detail-page-header + .detail-page-header-body + .detail-page-header-actions + - if can?(current_user, :admin_project, @project) + = link_to project_package_path(@project, @package), method: :delete, data: { confirm: _("Are you sure?") }, class: "btn btn-grouped btn-inverted btn-remove", title: _('Delete Package') do + = _('Delete') +.row.prepend-top-default + .col-md-6 + .card + .card-header + Package info: + %ul.content-list + %li + %span.light Name: + %strong + = @package.name + %li + %span.light Version: + %strong + = @package.version + %li + %span.light Created at: + %strong + = @package.created_at + - if @maven_metadatum + .card + .card-header + Maven Package info: + %ul.content-list + %li + %span.light App group: + %strong + = @maven_metadatum.app_group + %li + %span.light App name: + %strong + = @maven_metadatum.app_name + %li + %span.light App version: + %strong + = @maven_metadatum.app_version + .col-md-6 + .card + .card-header + Package files: + %ul.content-list + - @package_files.each do |package_file| + %li + = package_file.file.identifier + .pull-right.text-muted + = package_file.created_at diff --git a/config/routes/project.rb b/config/routes/project.rb index c2f7e01f04b788..e6274a1e42c895 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -311,7 +311,7 @@ get 'archive/*id', constraints: { format: Gitlab::PathRegex.archive_formats_regex, id: /.+?/ }, to: 'repositories#archive', as: 'archive' ## EE-specific - resources :packages, only: :index, module: 'packages' + resources :packages, only: [:index, :show, :destroy], module: 'packages' ## EE-specific resources :jobs, only: [:index, :show], constraints: { id: /\d+/ } do -- GitLab From 4945249325d85ddbdb21ebb6ace1f2d2d9a981db Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 30 Jul 2018 19:57:48 +0300 Subject: [PATCH 15/55] Use workhorse for maven package file upload [ci skip] Signed-off-by: Dmitriy Zaporozhets --- lib/api/maven_packages.rb | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/lib/api/maven_packages.rb b/lib/api/maven_packages.rb index b62a04202cb133..ded588f92805a4 100644 --- a/lib/api/maven_packages.rb +++ b/lib/api/maven_packages.rb @@ -75,14 +75,46 @@ def valid_metadata_xml?(xml) requires :app_version, type: String, desc: 'Package version' requires :file_name, type: String, desc: 'Package file name' end + put ':id/packages/maven/*app_group/:app_name/:app_version/:file_name/authorize', requirements: MAVEN_ENDPOINT_REQUIREMENTS do + not_allowed! unless Gitlab.config.packages.enabled + require_gitlab_workhorse! + Gitlab::Workhorse.verify_api_request!(headers) + + status 200 + content_type Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE + ::Packages::PackageFileUploader.workhorse_authorize(has_length: true) + end + + desc 'Upload the maven package file' do + detail 'This feature was introduced in GitLab 11.3' + end + params do + requires :app_group, type: String, desc: 'Package group id' + requires :app_name, type: String, desc: 'Package artifact id' + requires :app_version, type: String, desc: 'Package version' + requires :file_name, type: String, desc: 'Package file name' + optional 'file.path', type: String, desc: %q(path to locally stored body (generated by Workhorse)) + optional 'file.name', type: String, desc: %q(real filename as send in Content-Disposition (generated by Workhorse)) + optional 'file.type', type: String, desc: %q(real content type as send in Content-Type (generated by Workhorse)) + optional 'file.size', type: Integer, desc: %q(real size of file (generated by Workhorse)) + optional 'file.sha256', type: String, desc: %q(sha256 checksum of the file (generated by Workhorse)) + end put ':id/packages/maven/*app_group/:app_name/:app_version/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do + not_allowed! unless Gitlab.config.packages.enabled + require_gitlab_workhorse! + file_name, format = extract_format(params[:file_name]) - string_file = env['api.request.input'] + + uploaded_file = UploadedFile.from_params(params, :file, ::Packages::PackageFileUploader.workhorse_local_upload_path) + bad_request!('Missing package file!') unless uploaded_file metadata = ::Packages::MavenMetadatum.find_by(app_group: params[:app_group], app_name: params[:app_name], app_version: params[:app_version]) + # TODO: Refactor. We don't need to read file for every request, only metadata and md5/sha1 + string_file = File.read(uploaded_file) + unless metadata if file_name == MAVEN_METADATA_FILE return unless valid_metadata_xml?(string_file) @@ -112,7 +144,7 @@ def valid_metadata_xml?(xml) package_file.file_type = file_name.rpartition('.').last # Convert string into CarrierWave compatible StringIO object - package_file.file = CarrierWaveStringFile.new(string_file) + package_file.file = uploaded_file end package_file.save! -- GitLab From 07c401cbb65fbcb6efe8f25c881260907ea14233 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 2 Aug 2018 15:36:09 +0300 Subject: [PATCH 16/55] Specify namespace for PackageFileUploader [ci skip] Signed-off-by: Dmitriy Zaporozhets --- app/models/packages/package_file.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/packages/package_file.rb b/app/models/packages/package_file.rb index 8bedeceba95ee7..68c0e9bc8b69b5 100644 --- a/app/models/packages/package_file.rb +++ b/app/models/packages/package_file.rb @@ -3,7 +3,7 @@ class Packages::PackageFile < ActiveRecord::Base validates :package, presence: true - mount_uploader :file, PackageFileUploader + mount_uploader :file, Packages::PackageFileUploader scope :recent, -> { reorder(id: :desc) } end -- GitLab From 10d65e1be60bf18ac20a2dfe58cbd70f7fb791f3 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 2 Aug 2018 18:27:30 +0300 Subject: [PATCH 17/55] Add packages to navigation and policies [ci skip] Signed-off-by: Dmitriy Zaporozhets --- .../projects/packages/packages_controller.rb | 10 +++++++++- app/helpers/projects_helper.rb | 4 ++++ app/policies/project_policy.rb | 1 + app/views/layouts/nav/sidebar/_project.html.haml | 13 +++++++++++++ 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects/packages/packages_controller.rb b/app/controllers/projects/packages/packages_controller.rb index 3c12349b1719a8..9eb7c4a6a8a2ba 100644 --- a/app/controllers/projects/packages/packages_controller.rb +++ b/app/controllers/projects/packages/packages_controller.rb @@ -1,7 +1,9 @@ module Projects module Packages class PackagesController < ApplicationController - before_action :authorize_admin_project! + before_action :verify_packages_enabled! + before_action :authorize_read_packages! + before_action :authorize_admin_project!, only: [:destroy] def index @packages = project.packages.all.page(params[:page]) @@ -19,6 +21,12 @@ def destroy redirect_to project_packages_path(@project), notice: _('Package was removed') end + + private + + def verify_packages_enabled! + render_404 unless Gitlab.config.packages.enabled + end end end end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 539e6f5b10afec..b67337e7b79d32 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -268,6 +268,10 @@ def get_project_nav_tabs(project, current_user) nav_tabs << :container_registry end + if Gitlab.config.packages.enabled && can?(current_user, :read_packages, project) + nav_tabs << :packages + end + if project.builds_enabled? && can?(current_user, :read_pipeline, project) nav_tabs << :pipelines end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 44f3046d8d24d2..1317010a653614 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -183,6 +183,7 @@ class ProjectPolicy < BasePolicy enable :read_commit_status enable :read_build enable :read_container_image + enable :read_packages enable :read_pipeline enable :read_pipeline_schedule enable :read_environment diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index 34f47806205c40..a27b88ffeedb40 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -262,6 +262,19 @@ %strong.fly-out-top-item-name = _('Registry') + - if project_nav_tab? :packages + = nav_link(controller: %w[projects/packages]) do + = link_to project_packages_path(@project) do + .nav-icon-container + = sprite_icon('disk') + %span.nav-item-name + = _('Packages') + %ul.sidebar-sub-level-items.is-fly-out-only + = nav_link(controller: %w[projects/packages], html_options: { class: "fly-out-top-item" } ) do + = link_to project_packages_path(@project) do + %strong.fly-out-top-item-name + = _('Packages') + - if project_nav_tab? :wiki = nav_link(controller: :wikis) do = link_to get_project_wiki_path(@project), class: 'shortcuts-wiki' do -- GitLab From db1a6daab42dbf448a9c89a2caf77054fae3bbb9 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 2 Aug 2018 20:37:46 +0300 Subject: [PATCH 18/55] Add some basic feature specs on UI [ci skip] Signed-off-by: Dmitriy Zaporozhets --- app/views/projects/packages/index.html.haml | 26 ---------- .../packages/packages/index.html.haml | 43 +++++++++------- spec/factories/packages.rb | 46 +++++++++++++++++ spec/features/projects/packages_spec.rb | 51 +++++++++++++++++++ spec/support/helpers/stub_gitlab_calls.rb | 4 ++ 5 files changed, 125 insertions(+), 45 deletions(-) delete mode 100644 app/views/projects/packages/index.html.haml create mode 100644 spec/factories/packages.rb create mode 100644 spec/features/projects/packages_spec.rb diff --git a/app/views/projects/packages/index.html.haml b/app/views/projects/packages/index.html.haml deleted file mode 100644 index 8b3c9c07d6fc5e..00000000000000 --- a/app/views/projects/packages/index.html.haml +++ /dev/null @@ -1,26 +0,0 @@ -- @no_container = true -- page_title "Packages" - -%div{ class: container_class } - %table.table - %thead - %tr - %th Name - %th Version - %th Files - %th Created at - - %tbody - - @packages.each do |package| - %tr - %td - = package.name - %td - = package.version - %td - %ul - - package.package_files.each do |package_file| - %li - = package_file.file - %td - = package.created_at diff --git a/app/views/projects/packages/packages/index.html.haml b/app/views/projects/packages/packages/index.html.haml index 465c46a02bcd64..5254abdcb6ae40 100644 --- a/app/views/projects/packages/packages/index.html.haml +++ b/app/views/projects/packages/packages/index.html.haml @@ -1,23 +1,28 @@ - page_title "Packages" -%table.table - %thead - %tr - %th Name - %th Version - %th Type - %th Created at - - %tbody - - @packages.each do |package| +- if @packages.any? + %table.table + %thead %tr - %td - = link_to package.name, project_package_path(@project, package) - %td - = package.version - %td - = _('Maven package') - %td - = package.created_at + %th Name + %th Version + %th Type + %th Created at + + %tbody + - @packages.each do |package| + %tr + %td + = link_to package.name, project_package_path(@project, package) + %td + = package.version + %td + = _('Maven package') + %td + = package.created_at + + = paginate @packages, theme: "gitlab" -= paginate @packages, theme: "gitlab" +- else + .nothing-here-block + = _('No packages stored for this project.') diff --git a/spec/factories/packages.rb b/spec/factories/packages.rb new file mode 100644 index 00000000000000..ec03b3ac18ac0c --- /dev/null +++ b/spec/factories/packages.rb @@ -0,0 +1,46 @@ +FactoryBot.define do + factory :package, class: Packages::Package do + project + name 'my/company/app/my-app' + version '1-0-SNAPSHOT' + + factory :maven_package do + maven_metadatum + + after :create do |package| + create :package_file, :xml, package: package + create :package_file, :jar, package: package + create :package_file, :pom, package: package + end + end + end + + factory :package_file, class: Packages::PackageFile do + package + + trait(:jar) do + file Tempfile.new('my-app-1.0-20180724.124855-1.jar') + file_name 'my-app-1.0-20180724.124855-1.jar' + file_type 'jar' + end + + trait(:pom) do + file Tempfile.new('my-app-1.0-20180724.124855-1.pom') + file_name 'my-app-1.0-20180724.124855-1.pom' + file_type 'pom' + end + + trait(:xml) do + file Tempfile.new('maven-metadata.xml') + file_name 'maven-metadata.xml' + file_type 'xml' + end + end + + factory :maven_metadatum, class: Packages::MavenMetadatum do + package + app_group 'my/company/app' + app_name 'my-app' + app_version '1-0-SNAPSHOT' + end +end diff --git a/spec/features/projects/packages_spec.rb b/spec/features/projects/packages_spec.rb new file mode 100644 index 00000000000000..d2b2380109c1fb --- /dev/null +++ b/spec/features/projects/packages_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe 'Packages' do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:package) { create(:maven_package, project: project) } + + before do + sign_in(user) + project.add_developer(user) + stub_packages_config(enabled: true) + end + + context 'when there are no packages' do + it 'shows no packages message' do + visit_project_packages + + expect(page).to have_content 'No packages stored for this project.' + end + end + + context 'when there are packages' do + before do + package + end + + it 'shows list of packages' do + visit_project_packages + + expect(page).to have_content(package.name) + expect(page).to have_content(package.version) + end + + it 'shows a single package' do + visit_project_packages + + click_on package.name + + expect(page).to have_content(package.name) + expect(page).to have_content(package.version) + + package.package_files.each do |package_file| + expect(page).to have_content(package_file.file_name) + end + end + end + + def visit_project_packages + visit project_packages_path(project) + end +end diff --git a/spec/support/helpers/stub_gitlab_calls.rb b/spec/support/helpers/stub_gitlab_calls.rb index 2933f2c78dc8cd..0bf947fa2b00b6 100644 --- a/spec/support/helpers/stub_gitlab_calls.rb +++ b/spec/support/helpers/stub_gitlab_calls.rb @@ -30,6 +30,10 @@ def stub_ci_builds_disabled allow_any_instance_of(Project).to receive(:builds_enabled?).and_return(false) end + def stub_packages_config(settings) + allow(Gitlab.config.packages).to receive_messages(settings) + end + def stub_container_registry_config(registry_settings) allow(Gitlab.config.registry).to receive_messages(registry_settings) allow(Auth::ContainerRegistryAuthenticationService) -- GitLab From de0f19050ad6ccff5c1606850e48f581f4cfb9fd Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 3 Aug 2018 11:39:14 +0300 Subject: [PATCH 19/55] Add specs for maven packages api [ci skip] Signed-off-by: Dmitriy Zaporozhets --- spec/factories/packages.rb | 9 +- spec/fixtures/maven/maven-metadata.xml | 25 +++ .../maven/my-app-1.0-20180724.124855-1.jar | Bin 0 -> 2526 bytes .../maven/my-app-1.0-20180724.124855-1.pom | 34 +++++ spec/requests/api/maven_packages_spec.rb | 142 ++++++++++++++++++ 5 files changed, 207 insertions(+), 3 deletions(-) create mode 100644 spec/fixtures/maven/maven-metadata.xml create mode 100644 spec/fixtures/maven/my-app-1.0-20180724.124855-1.jar create mode 100644 spec/fixtures/maven/my-app-1.0-20180724.124855-1.pom create mode 100644 spec/requests/api/maven_packages_spec.rb diff --git a/spec/factories/packages.rb b/spec/factories/packages.rb index ec03b3ac18ac0c..ec323a3e670251 100644 --- a/spec/factories/packages.rb +++ b/spec/factories/packages.rb @@ -19,20 +19,23 @@ package trait(:jar) do - file Tempfile.new('my-app-1.0-20180724.124855-1.jar') + file { fixture_file_upload('spec/fixtures/maven/my-app-1.0-20180724.124855-1.jar') } file_name 'my-app-1.0-20180724.124855-1.jar' + file_sha1 '4f0bfa298744d505383fbb57c554d4f5c12d88b3' file_type 'jar' end trait(:pom) do - file Tempfile.new('my-app-1.0-20180724.124855-1.pom') + file { fixture_file_upload('spec/fixtures/maven/my-app-1.0-20180724.124855-1.pom') } file_name 'my-app-1.0-20180724.124855-1.pom' + file_sha1 '19c975abd49e5102ca6c74a619f21e0cf0351c57' file_type 'pom' end trait(:xml) do - file Tempfile.new('maven-metadata.xml') + file { fixture_file_upload('spec/fixtures/maven/maven-metadata.xml') } file_name 'maven-metadata.xml' + file_sha1 '42b1bdc80de64953b6876f5a8c644f20204011b0' file_type 'xml' end end diff --git a/spec/fixtures/maven/maven-metadata.xml b/spec/fixtures/maven/maven-metadata.xml new file mode 100644 index 00000000000000..7d7549df2277cb --- /dev/null +++ b/spec/fixtures/maven/maven-metadata.xml @@ -0,0 +1,25 @@ + + + com.mycompany.app + my-app + 1.0-SNAPSHOT + + + 20180724.124855 + 1 + + 20180724124855 + + + jar + 1.0-20180724.124855-1 + 20180724124855 + + + pom + 1.0-20180724.124855-1 + 20180724124855 + + + + diff --git a/spec/fixtures/maven/my-app-1.0-20180724.124855-1.jar b/spec/fixtures/maven/my-app-1.0-20180724.124855-1.jar new file mode 100644 index 0000000000000000000000000000000000000000..ea3903cf6d9f4953eb1a9678612016e29522d5b4 GIT binary patch literal 2526 zcmWIWW@h1H0D(ERKYTzm40AFtF!;KLIO=-(x#`1{aWKrT{oy0V%qmjIz`)SPz`!7a ztjyQZ&(qB{I7H9a&GqC)u0sX_3>Rz*{ukI!SR5*I=Grb%F28r~#gh^|PpRtOS}!hA zSeySu37Mf337#J9m^K;>zz*NBpRgqhn zoS$2em{*BMtq_LV#DW62dJeGd9#>?SL@+Wis4_7yNMlv+SWuvsoRe5woEv!B>u`X8 z?e_IHJ=?Y|`ev$6A#-6!e<7rtT^zogMc|*E7!}c;Wn|+^pht4f3jfmz)GA z9hG@@=FCjvo%Z|V?=$!nW-MyrWj32-6{TjjacN@oVLs=lPHjdzQ#8u@fAK8MHWlFs z4R3ptC&lCP=LEZ*VQF~fo%f$3mi=tfQPnRxbfL&r)Z$=I(vib64=+Bkvujat7RL@A z$ys;P%g@B`eEhlK$(KnHZHqtW$o9LdTsWa=Jj2$s*S1jka;O#Oyd}P3CNHGQbl)V- zKe$VD-}+nrTHo4bdD+Eoy)d}3vri)HwC(rijUOh}hvb`ix;XrI@?Dv5-q-V~&4k4* zHJox2*1PeYUErGe?DEz!^Y;-Y0n5TY_A^CPvA8xHwNDV7)^C6C-lk2S8z)Va3e$-C z%yuZ$i8*_PUfX#i!$k-KTJz?`Br`X)Y zveZ0y>@hH)D^|o*oSdJl2Tv$^$dQk!U4t0yxs|%erf@LKsQuxivC`8IvWFn zJ_)82*pwG+7wMS2K(Ff_liRHmB}dpjPWA8C z_dIE~^n{z@DygsPG2XrHf5$or)-EJ(^UTtlnWP2m;e$dg_ zvvJAg%my=R9-ID9>lEH)`+V*9e+uiabh3P&|GNId`*X*)>MnYjVP$As#V6GK@`t{~ z>t$JNmu12h|7%`%FupnX<2P-gbyLL6Zmc>c^5FeJbFX6yFPa?pVa+pH+q`J*ma3T6 z`QCM(`C-|D1Dw)do85E<733uh3=C!@CbEK}{DRb?lFZcN;4=TS-nw2op1y$|t3p2b z>Uf`h9K@&T8~Q=(Y@mTrpn=I&Bg2hGrlAF;ynKNMMh06RctEbnp zOk(Y?e!eu3<2JZ zOd`yAiShe9XTwJDm7$_vDGpl-3$yMi%&sIeSA7V zwGKkZ8>kL=(V~w}FQ^tm=#^lE)NlyBkOBqaUyy6jYbA(&28Jb#9$59m>nLQ0pcjy! z(iCEl0K@5mdS&G_x^dXvS97;x-Oc+9Hf|CB`^-*$Xojd+7@? x6% + 4.0.0 + com.mycompany.app + my-app + jar + 1.0-SNAPSHOT + my-app + http://maven.apache.org + + + junit + junit + 3.8.1 + test + + + + + local + file:///tmp/maven + + + + + local + file:///tmp/maven + + + + 1.6 + 1.6 + + diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb new file mode 100644 index 00000000000000..c868731e66dcdb --- /dev/null +++ b/spec/requests/api/maven_packages_spec.rb @@ -0,0 +1,142 @@ +require 'spec_helper' + +describe API::MavenPackages do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:personal_access_token) { create(:personal_access_token, user: user) } + let(:jwt_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } + let(:headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => jwt_token } } + let(:headers_with_token) { headers.merge('Private-Token' => personal_access_token.token) } + + before do + project.add_developer(user) + end + + describe 'GET /api/v4/projects/:id/packages/maven/*app_group/:app_name/:app_version/:file_name' do + let(:package) { create(:maven_package, project: project) } + let(:maven_metadatum) { package.maven_metadatum } + let(:package_file_xml) { package.package_files.find_by(file_type: 'xml') } + + context 'a public project' do + it 'returns the file' do + download_file(package_file_xml.file_name) + + expect(response).to have_gitlab_http_status(200) + expect(response.content_type.to_s).to eq('application/octet-stream') + end + + it 'returns sha1 of the file' do + download_file(package_file_xml.file_name + '.sha1') + + expect(response).to have_gitlab_http_status(200) + expect(response.content_type.to_s).to eq('text/plain') + expect(response.body).to eq(package_file_xml.file_sha1) + end + end + + context 'private project' do + # Auth required, read permissions required + end + + def download_file(file_name, params = {}, request_headers = headers) + get api("/projects/#{project.id}/packages/maven/" \ + "#{maven_metadatum.app_group}/#{maven_metadatum.app_name}/" \ + "#{maven_metadatum.app_version}/#{file_name}"), params, request_headers + end + + def download_file_with_token(params = {}, request_headers = headers_with_token) + download_file(params, request_headers) + end + end + + describe 'PUT /api/v4/projects/:id/packages/maven/*app_group/:app_name/:app_version/:file_name/authorize' do + it 'authorizes posting package with a valid token' do + authorize_upload_with_token + + expect(response).to have_gitlab_http_status(200) + expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + expect(json_response['TempPath']).not_to be_nil + end + + it 'rejects request without a valid token' do + headers_with_token['Private-Token'] = 'foo' + + authorize_upload_with_token + + expect(response).to have_gitlab_http_status(401) + end + + it 'rejects request without a valid permission' do + project.add_guest(user) + + authorize_upload_with_token + + expect(response).to have_gitlab_http_status(401) + end + + it 'rejects requests that did not go through gitlab-workhorse' do + headers.delete(Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER) + + authorize_upload_with_token + + expect(response).to have_gitlab_http_status(500) + end + + def authorize_upload(params = {}, request_headers = headers) + put api("/projects/#{project.id}/packages/maven/com/example/my-app/1-0-SNAPSHOT/maven-metadata.xml/authorize"), params, request_headers + end + + def authorize_upload_with_token(params = {}, request_headers = headers_with_token) + authorize_upload(params, request_headers) + end + end + + describe 'PUT /api/v4/projects/:id/packages/maven/*app_group/:app_name/:app_version/:file_name' do + let(:file_upload) { fixture_file_upload('spec/fixtures/maven/maven-metadata.xml') } + + before do + # by configuring this path we allow to pass temp file from any path + allow(Packages::PackageFileUploader).to receive(:workhorse_upload_path).and_return('/') + end + + it 'rejects requests without a file from workhorse' do + upload_file_with_token + + expect(response).to have_gitlab_http_status(400) + end + + it 'rejects request without a token' do + upload_file + + expect(response).to have_gitlab_http_status(401) + end + + context 'when params from workhorse are correct' do + let(:package) { project.packages.reload.last } + let(:package_file) { package.package_files.reload.last } + let(:params) do + { + 'file.path' => file_upload.path, + 'file.name' => file_upload.original_filename + } + end + + it 'creates package and stores package file' do + expect { upload_file_with_token(params) }.to change { project.packages.count }.by(1) + #.and change { Packages::MavenMetadatum.count }.by(1) + #.and change { Packages::PackageFile.count }.by(1) + + expect(response).to have_gitlab_http_status(200) + expect(package_file.original_filename).to eq(file_upload.original_filename) + end + end + + def upload_file(params = {}, request_headers = headers) + put api("/projects/#{project.id}/packages/maven/com/example/my-app/1-0-SNAPSHOT/maven-metadata.xml"), params, request_headers + end + + def upload_file_with_token(params = {}, request_headers = headers_with_token) + upload_file(params, request_headers) + end + end +end -- GitLab From b049ba6b83194cbd8dc040b476a69adc2b29c446 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 3 Aug 2018 12:46:07 +0300 Subject: [PATCH 20/55] Move packages specs and API to ee/ directory Signed-off-by: Dmitriy Zaporozhets --- {lib => ee/lib}/api/maven_packages.rb | 0 {spec => ee/spec}/factories/packages.rb | 0 .../spec}/features/projects/packages_spec.rb | 0 {spec => ee/spec}/fixtures/maven/maven-metadata.xml | 0 .../fixtures/maven/my-app-1.0-20180724.124855-1.jar | Bin .../fixtures/maven/my-app-1.0-20180724.124855-1.pom | 0 .../spec}/models/packages/maven_metadatum_spec.rb | 0 .../spec}/models/packages/package_file_spec.rb | 0 {spec => ee/spec}/models/packages/package_spec.rb | 0 .../spec}/requests/api/maven_packages_spec.rb | 0 10 files changed, 0 insertions(+), 0 deletions(-) rename {lib => ee/lib}/api/maven_packages.rb (100%) rename {spec => ee/spec}/factories/packages.rb (100%) rename {spec => ee/spec}/features/projects/packages_spec.rb (100%) rename {spec => ee/spec}/fixtures/maven/maven-metadata.xml (100%) rename {spec => ee/spec}/fixtures/maven/my-app-1.0-20180724.124855-1.jar (100%) rename {spec => ee/spec}/fixtures/maven/my-app-1.0-20180724.124855-1.pom (100%) rename {spec => ee/spec}/models/packages/maven_metadatum_spec.rb (100%) rename {spec => ee/spec}/models/packages/package_file_spec.rb (100%) rename {spec => ee/spec}/models/packages/package_spec.rb (100%) rename {spec => ee/spec}/requests/api/maven_packages_spec.rb (100%) diff --git a/lib/api/maven_packages.rb b/ee/lib/api/maven_packages.rb similarity index 100% rename from lib/api/maven_packages.rb rename to ee/lib/api/maven_packages.rb diff --git a/spec/factories/packages.rb b/ee/spec/factories/packages.rb similarity index 100% rename from spec/factories/packages.rb rename to ee/spec/factories/packages.rb diff --git a/spec/features/projects/packages_spec.rb b/ee/spec/features/projects/packages_spec.rb similarity index 100% rename from spec/features/projects/packages_spec.rb rename to ee/spec/features/projects/packages_spec.rb diff --git a/spec/fixtures/maven/maven-metadata.xml b/ee/spec/fixtures/maven/maven-metadata.xml similarity index 100% rename from spec/fixtures/maven/maven-metadata.xml rename to ee/spec/fixtures/maven/maven-metadata.xml diff --git a/spec/fixtures/maven/my-app-1.0-20180724.124855-1.jar b/ee/spec/fixtures/maven/my-app-1.0-20180724.124855-1.jar similarity index 100% rename from spec/fixtures/maven/my-app-1.0-20180724.124855-1.jar rename to ee/spec/fixtures/maven/my-app-1.0-20180724.124855-1.jar diff --git a/spec/fixtures/maven/my-app-1.0-20180724.124855-1.pom b/ee/spec/fixtures/maven/my-app-1.0-20180724.124855-1.pom similarity index 100% rename from spec/fixtures/maven/my-app-1.0-20180724.124855-1.pom rename to ee/spec/fixtures/maven/my-app-1.0-20180724.124855-1.pom diff --git a/spec/models/packages/maven_metadatum_spec.rb b/ee/spec/models/packages/maven_metadatum_spec.rb similarity index 100% rename from spec/models/packages/maven_metadatum_spec.rb rename to ee/spec/models/packages/maven_metadatum_spec.rb diff --git a/spec/models/packages/package_file_spec.rb b/ee/spec/models/packages/package_file_spec.rb similarity index 100% rename from spec/models/packages/package_file_spec.rb rename to ee/spec/models/packages/package_file_spec.rb diff --git a/spec/models/packages/package_spec.rb b/ee/spec/models/packages/package_spec.rb similarity index 100% rename from spec/models/packages/package_spec.rb rename to ee/spec/models/packages/package_spec.rb diff --git a/spec/requests/api/maven_packages_spec.rb b/ee/spec/requests/api/maven_packages_spec.rb similarity index 100% rename from spec/requests/api/maven_packages_spec.rb rename to ee/spec/requests/api/maven_packages_spec.rb -- GitLab From d67219f4235130486a3de23c534f638fe28334d4 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 3 Aug 2018 12:50:14 +0300 Subject: [PATCH 21/55] Move packages migrations to ee/ Signed-off-by: Dmitriy Zaporozhets --- {db => ee/db}/migrate/20180720120716_create_packages_packages.rb | 0 .../db}/migrate/20180720120726_create_packages_package_files.rb | 0 .../db}/migrate/20180720121404_create_packages_maven_metadata.rb | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename {db => ee/db}/migrate/20180720120716_create_packages_packages.rb (100%) rename {db => ee/db}/migrate/20180720120726_create_packages_package_files.rb (100%) rename {db => ee/db}/migrate/20180720121404_create_packages_maven_metadata.rb (100%) diff --git a/db/migrate/20180720120716_create_packages_packages.rb b/ee/db/migrate/20180720120716_create_packages_packages.rb similarity index 100% rename from db/migrate/20180720120716_create_packages_packages.rb rename to ee/db/migrate/20180720120716_create_packages_packages.rb diff --git a/db/migrate/20180720120726_create_packages_package_files.rb b/ee/db/migrate/20180720120726_create_packages_package_files.rb similarity index 100% rename from db/migrate/20180720120726_create_packages_package_files.rb rename to ee/db/migrate/20180720120726_create_packages_package_files.rb diff --git a/db/migrate/20180720121404_create_packages_maven_metadata.rb b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb similarity index 100% rename from db/migrate/20180720121404_create_packages_maven_metadata.rb rename to ee/db/migrate/20180720121404_create_packages_maven_metadata.rb -- GitLab From 2f75375fe75a19bcb4cbf8ef7a8792e29c15e41f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 3 Aug 2018 13:01:24 +0300 Subject: [PATCH 22/55] Move more package files to ee/ directory Signed-off-by: Dmitriy Zaporozhets --- app/models/project.rb | 1 - app/policies/project_policy.rb | 1 - ee/app/models/ee/project.rb | 1 + {app => ee/app}/models/packages.rb | 0 {app => ee/app}/models/packages/maven_metadatum.rb | 0 {app => ee/app}/models/packages/package.rb | 0 {app => ee/app}/models/packages/package_file.rb | 0 ee/app/policies/ee/project_policy.rb | 1 + ee/spec/factories/packages.rb | 6 +++--- ee/spec/features/projects/packages_spec.rb | 1 - spec/support/helpers/stub_gitlab_calls.rb | 4 ---- 11 files changed, 5 insertions(+), 10 deletions(-) rename {app => ee/app}/models/packages.rb (100%) rename {app => ee/app}/models/packages/maven_metadatum.rb (100%) rename {app => ee/app}/models/packages/package.rb (100%) rename {app => ee/app}/models/packages/package_file.rb (100%) diff --git a/app/models/project.rb b/app/models/project.rb index 18fddb53cf5f9a..bfc0a50646c3dc 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -131,7 +131,6 @@ class Project < ActiveRecord::Base has_one :last_event, -> {order 'events.created_at DESC'}, class_name: 'Event' has_many :boards, before_add: :validate_board_limit - has_many :packages, class_name: 'Packages::Package' # Project services has_one :campfire_service diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 1317010a653614..44f3046d8d24d2 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -183,7 +183,6 @@ class ProjectPolicy < BasePolicy enable :read_commit_status enable :read_build enable :read_container_image - enable :read_packages enable :read_pipeline enable :read_pipeline_schedule enable :read_environment diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 79a14ba64721cd..1c442047bc8ab6 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -38,6 +38,7 @@ module Project has_many :protected_environments has_many :software_license_policies, inverse_of: :project, class_name: 'SoftwareLicensePolicy' accepts_nested_attributes_for :software_license_policies, allow_destroy: true + has_many :packages, class_name: 'Packages::Package' has_many :sourced_pipelines, class_name: 'Ci::Sources::Pipeline', foreign_key: :source_project_id diff --git a/app/models/packages.rb b/ee/app/models/packages.rb similarity index 100% rename from app/models/packages.rb rename to ee/app/models/packages.rb diff --git a/app/models/packages/maven_metadatum.rb b/ee/app/models/packages/maven_metadatum.rb similarity index 100% rename from app/models/packages/maven_metadatum.rb rename to ee/app/models/packages/maven_metadatum.rb diff --git a/app/models/packages/package.rb b/ee/app/models/packages/package.rb similarity index 100% rename from app/models/packages/package.rb rename to ee/app/models/packages/package.rb diff --git a/app/models/packages/package_file.rb b/ee/app/models/packages/package_file.rb similarity index 100% rename from app/models/packages/package_file.rb rename to ee/app/models/packages/package_file.rb diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 640c28f8e23ac7..0c25299e0cf976 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -89,6 +89,7 @@ module ProjectPolicy enable :read_deploy_board enable :admin_issue_link enable :admin_epic_issue + enable :read_packages end rule { can?(:developer_access) }.policy do diff --git a/ee/spec/factories/packages.rb b/ee/spec/factories/packages.rb index ec323a3e670251..e7de98802836aa 100644 --- a/ee/spec/factories/packages.rb +++ b/ee/spec/factories/packages.rb @@ -19,21 +19,21 @@ package trait(:jar) do - file { fixture_file_upload('spec/fixtures/maven/my-app-1.0-20180724.124855-1.jar') } + file { fixture_file_upload('ee/spec/fixtures/maven/my-app-1.0-20180724.124855-1.jar') } file_name 'my-app-1.0-20180724.124855-1.jar' file_sha1 '4f0bfa298744d505383fbb57c554d4f5c12d88b3' file_type 'jar' end trait(:pom) do - file { fixture_file_upload('spec/fixtures/maven/my-app-1.0-20180724.124855-1.pom') } + file { fixture_file_upload('ee/spec/fixtures/maven/my-app-1.0-20180724.124855-1.pom') } file_name 'my-app-1.0-20180724.124855-1.pom' file_sha1 '19c975abd49e5102ca6c74a619f21e0cf0351c57' file_type 'pom' end trait(:xml) do - file { fixture_file_upload('spec/fixtures/maven/maven-metadata.xml') } + file { fixture_file_upload('ee/spec/fixtures/maven/maven-metadata.xml') } file_name 'maven-metadata.xml' file_sha1 '42b1bdc80de64953b6876f5a8c644f20204011b0' file_type 'xml' diff --git a/ee/spec/features/projects/packages_spec.rb b/ee/spec/features/projects/packages_spec.rb index d2b2380109c1fb..bc05d1120aed21 100644 --- a/ee/spec/features/projects/packages_spec.rb +++ b/ee/spec/features/projects/packages_spec.rb @@ -8,7 +8,6 @@ before do sign_in(user) project.add_developer(user) - stub_packages_config(enabled: true) end context 'when there are no packages' do diff --git a/spec/support/helpers/stub_gitlab_calls.rb b/spec/support/helpers/stub_gitlab_calls.rb index 0bf947fa2b00b6..2933f2c78dc8cd 100644 --- a/spec/support/helpers/stub_gitlab_calls.rb +++ b/spec/support/helpers/stub_gitlab_calls.rb @@ -30,10 +30,6 @@ def stub_ci_builds_disabled allow_any_instance_of(Project).to receive(:builds_enabled?).and_return(false) end - def stub_packages_config(settings) - allow(Gitlab.config.packages).to receive_messages(settings) - end - def stub_container_registry_config(registry_settings) allow(Gitlab.config.registry).to receive_messages(registry_settings) allow(Auth::ContainerRegistryAuthenticationService) -- GitLab From 1e412a2b22313b9d91690ad3e041a844456d6458 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 3 Aug 2018 13:07:07 +0300 Subject: [PATCH 23/55] Remove UI related code for packages [ci skip] Signed-off-by: Dmitriy Zaporozhets --- .../projects/packages/packages_controller.rb | 32 ----------- app/helpers/projects_helper.rb | 4 -- .../packages/packages/index.html.haml | 28 ---------- .../projects/packages/packages/show.html.haml | 56 ------------------- .../packages/create_maven_package_service.rb | 0 ee/spec/features/projects/packages_spec.rb | 50 ----------------- 6 files changed, 170 deletions(-) delete mode 100644 app/controllers/projects/packages/packages_controller.rb delete mode 100644 app/views/projects/packages/packages/index.html.haml delete mode 100644 app/views/projects/packages/packages/show.html.haml rename {app => ee/app}/services/packages/create_maven_package_service.rb (100%) delete mode 100644 ee/spec/features/projects/packages_spec.rb diff --git a/app/controllers/projects/packages/packages_controller.rb b/app/controllers/projects/packages/packages_controller.rb deleted file mode 100644 index 9eb7c4a6a8a2ba..00000000000000 --- a/app/controllers/projects/packages/packages_controller.rb +++ /dev/null @@ -1,32 +0,0 @@ -module Projects - module Packages - class PackagesController < ApplicationController - before_action :verify_packages_enabled! - before_action :authorize_read_packages! - before_action :authorize_admin_project!, only: [:destroy] - - def index - @packages = project.packages.all.page(params[:page]) - end - - def show - @package = project.packages.find(params[:id]) - @package_files = @package.package_files.recent - @maven_metadatum = @package.maven_metadatum - end - - def destroy - @package = project.packages.find(params[:id]) - @package.destroy - - redirect_to project_packages_path(@project), notice: _('Package was removed') - end - - private - - def verify_packages_enabled! - render_404 unless Gitlab.config.packages.enabled - end - end - end -end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index b67337e7b79d32..539e6f5b10afec 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -268,10 +268,6 @@ def get_project_nav_tabs(project, current_user) nav_tabs << :container_registry end - if Gitlab.config.packages.enabled && can?(current_user, :read_packages, project) - nav_tabs << :packages - end - if project.builds_enabled? && can?(current_user, :read_pipeline, project) nav_tabs << :pipelines end diff --git a/app/views/projects/packages/packages/index.html.haml b/app/views/projects/packages/packages/index.html.haml deleted file mode 100644 index 5254abdcb6ae40..00000000000000 --- a/app/views/projects/packages/packages/index.html.haml +++ /dev/null @@ -1,28 +0,0 @@ -- page_title "Packages" - -- if @packages.any? - %table.table - %thead - %tr - %th Name - %th Version - %th Type - %th Created at - - %tbody - - @packages.each do |package| - %tr - %td - = link_to package.name, project_package_path(@project, package) - %td - = package.version - %td - = _('Maven package') - %td - = package.created_at - - = paginate @packages, theme: "gitlab" - -- else - .nothing-here-block - = _('No packages stored for this project.') diff --git a/app/views/projects/packages/packages/show.html.haml b/app/views/projects/packages/packages/show.html.haml deleted file mode 100644 index da8808f816c955..00000000000000 --- a/app/views/projects/packages/packages/show.html.haml +++ /dev/null @@ -1,56 +0,0 @@ -- add_to_breadcrumbs _("Packages"), project_packages_path(@project) -- add_to_breadcrumbs @package.name, project_packages_path(@project) -- breadcrumb_title @package.version -- page_title "Packages" - -.detail-page-header - .detail-page-header-body - .detail-page-header-actions - - if can?(current_user, :admin_project, @project) - = link_to project_package_path(@project, @package), method: :delete, data: { confirm: _("Are you sure?") }, class: "btn btn-grouped btn-inverted btn-remove", title: _('Delete Package') do - = _('Delete') -.row.prepend-top-default - .col-md-6 - .card - .card-header - Package info: - %ul.content-list - %li - %span.light Name: - %strong - = @package.name - %li - %span.light Version: - %strong - = @package.version - %li - %span.light Created at: - %strong - = @package.created_at - - if @maven_metadatum - .card - .card-header - Maven Package info: - %ul.content-list - %li - %span.light App group: - %strong - = @maven_metadatum.app_group - %li - %span.light App name: - %strong - = @maven_metadatum.app_name - %li - %span.light App version: - %strong - = @maven_metadatum.app_version - .col-md-6 - .card - .card-header - Package files: - %ul.content-list - - @package_files.each do |package_file| - %li - = package_file.file.identifier - .pull-right.text-muted - = package_file.created_at diff --git a/app/services/packages/create_maven_package_service.rb b/ee/app/services/packages/create_maven_package_service.rb similarity index 100% rename from app/services/packages/create_maven_package_service.rb rename to ee/app/services/packages/create_maven_package_service.rb diff --git a/ee/spec/features/projects/packages_spec.rb b/ee/spec/features/projects/packages_spec.rb deleted file mode 100644 index bc05d1120aed21..00000000000000 --- a/ee/spec/features/projects/packages_spec.rb +++ /dev/null @@ -1,50 +0,0 @@ -require 'spec_helper' - -describe 'Packages' do - let(:user) { create(:user) } - let(:project) { create(:project) } - let(:package) { create(:maven_package, project: project) } - - before do - sign_in(user) - project.add_developer(user) - end - - context 'when there are no packages' do - it 'shows no packages message' do - visit_project_packages - - expect(page).to have_content 'No packages stored for this project.' - end - end - - context 'when there are packages' do - before do - package - end - - it 'shows list of packages' do - visit_project_packages - - expect(page).to have_content(package.name) - expect(page).to have_content(package.version) - end - - it 'shows a single package' do - visit_project_packages - - click_on package.name - - expect(page).to have_content(package.name) - expect(page).to have_content(package.version) - - package.package_files.each do |package_file| - expect(page).to have_content(package_file.file_name) - end - end - end - - def visit_project_packages - visit project_packages_path(project) - end -end -- GitLab From 8f12f9882c4e2cd17780d4c629378e9cc3f611ef Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 3 Aug 2018 13:10:48 +0300 Subject: [PATCH 24/55] Move package uploader to ee dir [ci skip] Signed-off-by: Dmitriy Zaporozhets --- config/routes/project.rb | 4 ---- {app => ee/app}/uploaders/packages/package_file_uploader.rb | 0 2 files changed, 4 deletions(-) rename {app => ee/app}/uploaders/packages/package_file_uploader.rb (100%) diff --git a/config/routes/project.rb b/config/routes/project.rb index e6274a1e42c895..52f899d10d52ca 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -310,10 +310,6 @@ scope '-' do get 'archive/*id', constraints: { format: Gitlab::PathRegex.archive_formats_regex, id: /.+?/ }, to: 'repositories#archive', as: 'archive' - ## EE-specific - resources :packages, only: [:index, :show, :destroy], module: 'packages' - ## EE-specific - resources :jobs, only: [:index, :show], constraints: { id: /\d+/ } do collection do post :cancel_all diff --git a/app/uploaders/packages/package_file_uploader.rb b/ee/app/uploaders/packages/package_file_uploader.rb similarity index 100% rename from app/uploaders/packages/package_file_uploader.rb rename to ee/app/uploaders/packages/package_file_uploader.rb -- GitLab From 78514b79f9ead8b7ef1ef9873552d3bff96327d2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 3 Aug 2018 13:13:07 +0300 Subject: [PATCH 25/55] Remove packages tab from sidebar Signed-off-by: Dmitriy Zaporozhets --- app/views/layouts/nav/sidebar/_project.html.haml | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index a27b88ffeedb40..34f47806205c40 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -262,19 +262,6 @@ %strong.fly-out-top-item-name = _('Registry') - - if project_nav_tab? :packages - = nav_link(controller: %w[projects/packages]) do - = link_to project_packages_path(@project) do - .nav-icon-container - = sprite_icon('disk') - %span.nav-item-name - = _('Packages') - %ul.sidebar-sub-level-items.is-fly-out-only - = nav_link(controller: %w[projects/packages], html_options: { class: "fly-out-top-item" } ) do - = link_to project_packages_path(@project) do - %strong.fly-out-top-item-name - = _('Packages') - - if project_nav_tab? :wiki = nav_link(controller: :wikis) do = link_to get_project_wiki_path(@project), class: 'shortcuts-wiki' do -- GitLab From 540dface90fc303229cf9ffd9022a3102ac94678 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 3 Aug 2018 14:28:37 +0300 Subject: [PATCH 26/55] Make package migrations reversible Signed-off-by: Dmitriy Zaporozhets --- .../20180720120726_create_packages_package_files.rb | 12 +++++++++++- .../20180720121404_create_packages_maven_metadata.rb | 12 +++++++++++- ee/spec/requests/api/maven_packages_spec.rb | 4 ++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/ee/db/migrate/20180720120726_create_packages_package_files.rb b/ee/db/migrate/20180720120726_create_packages_package_files.rb index f283bdf0b8fe7f..94b7eefe4feef6 100644 --- a/ee/db/migrate/20180720120726_create_packages_package_files.rb +++ b/ee/db/migrate/20180720120726_create_packages_package_files.rb @@ -5,7 +5,7 @@ class CreatePackagesPackageFiles < ActiveRecord::Migration disable_ddl_transaction! - def change + def up create_table :packages_package_files do |t| t.references :package, index: true, null: false t.string :file @@ -23,4 +23,14 @@ def change column: :package_id, on_delete: :cascade end + + def down + if foreign_keys_for(:packages_package_files, :package_id).any? + remove_foreign_key :packages_package_files, column: :package_id + end + + if table_exists?(:packages_package_files) + drop_table :packages_package_files + end + end end diff --git a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb index 71e19e03ecfc1e..276e1e70cd1de1 100644 --- a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb +++ b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb @@ -5,7 +5,7 @@ class CreatePackagesMavenMetadata < ActiveRecord::Migration disable_ddl_transaction! - def change + def up create_table :packages_maven_metadata do |t| t.references :package, index: true, null: false t.string :app_group, null: false @@ -19,4 +19,14 @@ def change column: :package_id, on_delete: :cascade end + + def down + if foreign_keys_for(:packages_maven_metadata, :package_id).any? + remove_foreign_key :packages_maven_metadata, column: :package_id + end + + if table_exists?(:packages_maven_metadata) + drop_table :packages_maven_metadata + end + end end diff --git a/ee/spec/requests/api/maven_packages_spec.rb b/ee/spec/requests/api/maven_packages_spec.rb index c868731e66dcdb..7d0db86c0633b5 100644 --- a/ee/spec/requests/api/maven_packages_spec.rb +++ b/ee/spec/requests/api/maven_packages_spec.rb @@ -123,8 +123,8 @@ def authorize_upload_with_token(params = {}, request_headers = headers_with_toke it 'creates package and stores package file' do expect { upload_file_with_token(params) }.to change { project.packages.count }.by(1) - #.and change { Packages::MavenMetadatum.count }.by(1) - #.and change { Packages::PackageFile.count }.by(1) + .and change { Packages::MavenMetadatum.count }.by(1) + .and change { Packages::PackageFile.count }.by(1) expect(response).to have_gitlab_http_status(200) expect(package_file.original_filename).to eq(file_upload.original_filename) -- GitLab From b4e8bca64de861f5ac63c55c0c273d72c14e6fa1 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 3 Aug 2018 16:41:08 +0300 Subject: [PATCH 27/55] Add frozen literals and permission checks Signed-off-by: Dmitriy Zaporozhets --- ee/app/models/license.rb | 1 + ee/app/models/packages.rb | 1 + ee/app/models/packages/maven_metadatum.rb | 1 + ee/app/models/packages/package.rb | 1 + ee/app/models/packages/package_file.rb | 1 + ee/app/policies/ee/project_policy.rb | 5 +++ .../packages/create_maven_package_service.rb | 1 + .../packages/package_file_uploader.rb | 1 + ...20180720120716_create_packages_packages.rb | 1 + ...720120726_create_packages_package_files.rb | 1 + ...20121404_create_packages_maven_metadata.rb | 1 + ee/lib/api/maven_packages.rb | 7 ++++ ee/spec/factories/packages.rb | 1 + .../models/packages/maven_metadatum_spec.rb | 1 + ee/spec/models/packages/package_file_spec.rb | 1 + ee/spec/models/packages/package_spec.rb | 1 + ee/spec/requests/api/maven_packages_spec.rb | 32 +++++++++++++++++-- 17 files changed, 55 insertions(+), 3 deletions(-) diff --git a/ee/app/models/license.rb b/ee/app/models/license.rb index 3c9c4238eaaf29..341fb416c91734 100644 --- a/ee/app/models/license.rb +++ b/ee/app/models/license.rb @@ -64,6 +64,7 @@ class License < ActiveRecord::Base protected_environments system_header_footer custom_project_templates + packages ].freeze EEU_FEATURES = EEP_FEATURES + %i[ diff --git a/ee/app/models/packages.rb b/ee/app/models/packages.rb index a3a12aec720b56..e14c9290093f25 100644 --- a/ee/app/models/packages.rb +++ b/ee/app/models/packages.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true module Packages def self.table_name_prefix 'packages_' diff --git a/ee/app/models/packages/maven_metadatum.rb b/ee/app/models/packages/maven_metadatum.rb index 92efdb8d1080d0..58ba74d9debc5e 100644 --- a/ee/app/models/packages/maven_metadatum.rb +++ b/ee/app/models/packages/maven_metadatum.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class Packages::MavenMetadatum < ActiveRecord::Base belongs_to :package diff --git a/ee/app/models/packages/package.rb b/ee/app/models/packages/package.rb index 3fa35313217fa5..8a655745132f25 100644 --- a/ee/app/models/packages/package.rb +++ b/ee/app/models/packages/package.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class Packages::Package < ActiveRecord::Base belongs_to :project has_many :package_files diff --git a/ee/app/models/packages/package_file.rb b/ee/app/models/packages/package_file.rb index 68c0e9bc8b69b5..abd709d5f29703 100644 --- a/ee/app/models/packages/package_file.rb +++ b/ee/app/models/packages/package_file.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class Packages::PackageFile < ActiveRecord::Base belongs_to :package diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 0c25299e0cf976..54f85f70e9f2c1 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -84,6 +84,10 @@ module ProjectPolicy rule { can?(:read_issue) }.enable :read_issue_link + rule { can?(:public_access) }.policy do + enable :read_packages + end + rule { can?(:reporter_access) }.policy do enable :admin_board enable :read_deploy_board @@ -95,6 +99,7 @@ module ProjectPolicy rule { can?(:developer_access) }.policy do enable :admin_board enable :admin_vulnerability_feedback + enable :write_packages end rule { can?(:developer_access) & security_reports_feature_available }.enable :read_project_security_dashboard diff --git a/ee/app/services/packages/create_maven_package_service.rb b/ee/app/services/packages/create_maven_package_service.rb index 38426de50da3bd..46bb5918a5d4b8 100644 --- a/ee/app/services/packages/create_maven_package_service.rb +++ b/ee/app/services/packages/create_maven_package_service.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true module Packages class CreateMavenPackageService < BaseService def execute diff --git a/ee/app/uploaders/packages/package_file_uploader.rb b/ee/app/uploaders/packages/package_file_uploader.rb index c7bc41a9437975..6bd3bd7407df1b 100644 --- a/ee/app/uploaders/packages/package_file_uploader.rb +++ b/ee/app/uploaders/packages/package_file_uploader.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class Packages::PackageFileUploader < GitlabUploader extend Workhorse::UploadPath include ObjectStorage::Concern diff --git a/ee/db/migrate/20180720120716_create_packages_packages.rb b/ee/db/migrate/20180720120716_create_packages_packages.rb index 8a1413f05610d2..8779e2f6d220d9 100644 --- a/ee/db/migrate/20180720120716_create_packages_packages.rb +++ b/ee/db/migrate/20180720120716_create_packages_packages.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class CreatePackagesPackages < ActiveRecord::Migration DOWNTIME = false diff --git a/ee/db/migrate/20180720120726_create_packages_package_files.rb b/ee/db/migrate/20180720120726_create_packages_package_files.rb index 94b7eefe4feef6..15696f29a34d34 100644 --- a/ee/db/migrate/20180720120726_create_packages_package_files.rb +++ b/ee/db/migrate/20180720120726_create_packages_package_files.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class CreatePackagesPackageFiles < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb index 276e1e70cd1de1..c868b4b6cc1bc2 100644 --- a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb +++ b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class CreatePackagesMavenMetadata < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/ee/lib/api/maven_packages.rb b/ee/lib/api/maven_packages.rb index ded588f92805a4..da82406cdb2700 100644 --- a/ee/lib/api/maven_packages.rb +++ b/ee/lib/api/maven_packages.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true module API class MavenPackages < Grape::API MAVEN_ENDPOINT_REQUIREMENTS = { @@ -48,6 +49,8 @@ def valid_metadata_xml?(xml) requires :file_name, type: String, desc: 'Package file name' end get ':id/packages/maven/*app_group/:app_name/:app_version/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do + unauthorized! unless can?(current_user, :read_package, user_project) + file_name, format = extract_format(params[:file_name]) metadata = ::Packages::MavenMetadatum.find_by!(app_group: params[:app_group], @@ -77,6 +80,8 @@ def valid_metadata_xml?(xml) end put ':id/packages/maven/*app_group/:app_name/:app_version/:file_name/authorize', requirements: MAVEN_ENDPOINT_REQUIREMENTS do not_allowed! unless Gitlab.config.packages.enabled + unauthorized! unless can?(current_user, :write_package, user_project) + require_gitlab_workhorse! Gitlab::Workhorse.verify_api_request!(headers) @@ -101,6 +106,8 @@ def valid_metadata_xml?(xml) end put ':id/packages/maven/*app_group/:app_name/:app_version/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do not_allowed! unless Gitlab.config.packages.enabled + unauthorized! unless can?(current_user, :write_package, user_project) + require_gitlab_workhorse! file_name, format = extract_format(params[:file_name]) diff --git a/ee/spec/factories/packages.rb b/ee/spec/factories/packages.rb index e7de98802836aa..f6244396a020b0 100644 --- a/ee/spec/factories/packages.rb +++ b/ee/spec/factories/packages.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true FactoryBot.define do factory :package, class: Packages::Package do project diff --git a/ee/spec/models/packages/maven_metadatum_spec.rb b/ee/spec/models/packages/maven_metadatum_spec.rb index 3a1c1154ed9502..99501f3bd875f1 100644 --- a/ee/spec/models/packages/maven_metadatum_spec.rb +++ b/ee/spec/models/packages/maven_metadatum_spec.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true require 'rails_helper' RSpec.describe Packages::MavenMetadatum, type: :model do diff --git a/ee/spec/models/packages/package_file_spec.rb b/ee/spec/models/packages/package_file_spec.rb index 619b005df53c04..a3167ef0b5ecc9 100644 --- a/ee/spec/models/packages/package_file_spec.rb +++ b/ee/spec/models/packages/package_file_spec.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true require 'rails_helper' RSpec.describe Packages::PackageFile, type: :model do diff --git a/ee/spec/models/packages/package_spec.rb b/ee/spec/models/packages/package_spec.rb index 4d76665b0ea6a2..258a2dcfd6e208 100644 --- a/ee/spec/models/packages/package_spec.rb +++ b/ee/spec/models/packages/package_spec.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true require 'rails_helper' RSpec.describe Packages::Package, type: :model do diff --git a/ee/spec/requests/api/maven_packages_spec.rb b/ee/spec/requests/api/maven_packages_spec.rb index 7d0db86c0633b5..e5eb57464c0f66 100644 --- a/ee/spec/requests/api/maven_packages_spec.rb +++ b/ee/spec/requests/api/maven_packages_spec.rb @@ -1,8 +1,9 @@ +# frozen_string_literal: true require 'spec_helper' describe API::MavenPackages do let(:user) { create(:user) } - let(:project) { create(:project) } + let(:project) { create(:project, :public) } let(:personal_access_token) { create(:personal_access_token, user: user) } let(:jwt_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } let(:headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => jwt_token } } @@ -35,7 +36,32 @@ end context 'private project' do - # Auth required, read permissions required + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end + + it 'returns the file' do + download_file_with_token(package_file_xml.file_name) + + expect(response).to have_gitlab_http_status(200) + expect(response.content_type.to_s).to eq('application/octet-stream') + end + + it 'denies download when not enough permissions' do + project.add_guest(user) + + download_file_with_token(package_file_xml.file_name) + + expect(response).to have_gitlab_http_status(400) + end + + it 'denies download when no private token' do + project.add_guest(user) + + download_file(package_file_xml.file_name) + + expect(response).to have_gitlab_http_status(400) + end end def download_file(file_name, params = {}, request_headers = headers) @@ -92,7 +118,7 @@ def authorize_upload_with_token(params = {}, request_headers = headers_with_toke end describe 'PUT /api/v4/projects/:id/packages/maven/*app_group/:app_name/:app_version/:file_name' do - let(:file_upload) { fixture_file_upload('spec/fixtures/maven/maven-metadata.xml') } + let(:file_upload) { fixture_file_upload('ee/spec/fixtures/maven/maven-metadata.xml') } before do # by configuring this path we allow to pass temp file from any path -- GitLab From e4164106588fd8a7ea710c26d6c60a19f7516d37 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 3 Aug 2018 18:00:32 +0300 Subject: [PATCH 28/55] Update policy and tests for maven package feature Signed-off-by: Dmitriy Zaporozhets --- ee/app/policies/ee/project_policy.rb | 10 ++++------ ee/lib/api/maven_packages.rb | 4 ++-- ee/spec/requests/api/maven_packages_spec.rb | 16 +++++++--------- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 54f85f70e9f2c1..5fed07ce4c95f2 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -84,24 +84,22 @@ module ProjectPolicy rule { can?(:read_issue) }.enable :read_issue_link - rule { can?(:public_access) }.policy do - enable :read_packages - end - rule { can?(:reporter_access) }.policy do enable :admin_board enable :read_deploy_board enable :admin_issue_link enable :admin_epic_issue - enable :read_packages + enable :read_package end rule { can?(:developer_access) }.policy do enable :admin_board enable :admin_vulnerability_feedback - enable :write_packages + enable :admin_package end + rule { can?(:public_access) }.enable :read_package + rule { can?(:developer_access) & security_reports_feature_available }.enable :read_project_security_dashboard rule { can?(:read_project) }.enable :read_vulnerability_feedback diff --git a/ee/lib/api/maven_packages.rb b/ee/lib/api/maven_packages.rb index da82406cdb2700..3a5288bba07054 100644 --- a/ee/lib/api/maven_packages.rb +++ b/ee/lib/api/maven_packages.rb @@ -80,7 +80,7 @@ def valid_metadata_xml?(xml) end put ':id/packages/maven/*app_group/:app_name/:app_version/:file_name/authorize', requirements: MAVEN_ENDPOINT_REQUIREMENTS do not_allowed! unless Gitlab.config.packages.enabled - unauthorized! unless can?(current_user, :write_package, user_project) + unauthorized! unless can?(current_user, :admin_package, user_project) require_gitlab_workhorse! Gitlab::Workhorse.verify_api_request!(headers) @@ -106,7 +106,7 @@ def valid_metadata_xml?(xml) end put ':id/packages/maven/*app_group/:app_name/:app_version/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do not_allowed! unless Gitlab.config.packages.enabled - unauthorized! unless can?(current_user, :write_package, user_project) + unauthorized! unless can?(current_user, :admin_package, user_project) require_gitlab_workhorse! diff --git a/ee/spec/requests/api/maven_packages_spec.rb b/ee/spec/requests/api/maven_packages_spec.rb index e5eb57464c0f66..cebb9cce8614e4 100644 --- a/ee/spec/requests/api/maven_packages_spec.rb +++ b/ee/spec/requests/api/maven_packages_spec.rb @@ -52,15 +52,13 @@ download_file_with_token(package_file_xml.file_name) - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(401) end it 'denies download when no private token' do - project.add_guest(user) - download_file(package_file_xml.file_name) - expect(response).to have_gitlab_http_status(400) + expect(response).to have_gitlab_http_status(404) end end @@ -70,8 +68,8 @@ def download_file(file_name, params = {}, request_headers = headers) "#{maven_metadatum.app_version}/#{file_name}"), params, request_headers end - def download_file_with_token(params = {}, request_headers = headers_with_token) - download_file(params, request_headers) + def download_file_with_token(file_name, params = {}, request_headers = headers_with_token) + download_file(file_name, params, request_headers) end end @@ -109,7 +107,7 @@ def download_file_with_token(params = {}, request_headers = headers_with_token) end def authorize_upload(params = {}, request_headers = headers) - put api("/projects/#{project.id}/packages/maven/com/example/my-app/1-0-SNAPSHOT/maven-metadata.xml/authorize"), params, request_headers + put api("/projects/#{project.id}/packages/maven/com/example/my-app/1.0-SNAPSHOT/maven-metadata.xml/authorize"), params, request_headers end def authorize_upload_with_token(params = {}, request_headers = headers_with_token) @@ -153,12 +151,12 @@ def authorize_upload_with_token(params = {}, request_headers = headers_with_toke .and change { Packages::PackageFile.count }.by(1) expect(response).to have_gitlab_http_status(200) - expect(package_file.original_filename).to eq(file_upload.original_filename) + expect(package_file.file_name).to eq(file_upload.original_filename) end end def upload_file(params = {}, request_headers = headers) - put api("/projects/#{project.id}/packages/maven/com/example/my-app/1-0-SNAPSHOT/maven-metadata.xml"), params, request_headers + put api("/projects/#{project.id}/packages/maven/com/example/my-app/1.0-SNAPSHOT/maven-metadata.xml"), params, request_headers end def upload_file_with_token(params = {}, request_headers = headers_with_token) -- GitLab From 5686de1e92f41ffc88637d92cf22340752a7a1e2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 6 Aug 2018 18:52:53 +0300 Subject: [PATCH 29/55] Refactor code related to maven packages Signed-off-by: Dmitriy Zaporozhets --- db/schema.rb | 5 +- .../finders/packages/maven_package_finder.rb | 23 ++++ .../packages/create_maven_package_service.rb | 26 +++-- .../packages/create_package_file_service.rb | 19 ++++ ...20180720120716_create_packages_packages.rb | 2 +- ...20121404_create_packages_maven_metadata.rb | 3 +- ee/lib/api/maven_packages.rb | 101 ++++++++---------- 7 files changed, 104 insertions(+), 75 deletions(-) create mode 100644 ee/app/finders/packages/maven_package_finder.rb create mode 100644 ee/app/services/packages/create_package_file_service.rb diff --git a/db/schema.rb b/db/schema.rb index 5a0e79622b005d..34ae5569fcc01c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1923,9 +1923,10 @@ create_table "packages_maven_metadata", force: :cascade do |t| t.integer "package_id", null: false + t.string "path", null: false t.string "app_group", null: false t.string "app_name", null: false - t.string "app_version", null: false + t.string "app_version" t.datetime "created_at", null: false t.datetime "updated_at", null: false end @@ -1949,7 +1950,7 @@ create_table "packages_packages", force: :cascade do |t| t.integer "project_id", null: false - t.string "name" + t.string "name", null: false t.string "version" t.datetime "created_at", null: false t.datetime "updated_at", null: false diff --git a/ee/app/finders/packages/maven_package_finder.rb b/ee/app/finders/packages/maven_package_finder.rb new file mode 100644 index 00000000000000..d81ef4530040d0 --- /dev/null +++ b/ee/app/finders/packages/maven_package_finder.rb @@ -0,0 +1,23 @@ +class Packages::MavenPackageFinder + attr_reader :project, :path + + def initialize(project, path) + @project = project + @path = path + end + + def execute + packages.last + end + + def execute! + packages.last! + end + + private + + def packages + project.packages.joins(:maven_metadatum) + .where(packages_maven_metadata: { path: path }) + end +end diff --git a/ee/app/services/packages/create_maven_package_service.rb b/ee/app/services/packages/create_maven_package_service.rb index 46bb5918a5d4b8..3697559f486ec3 100644 --- a/ee/app/services/packages/create_maven_package_service.rb +++ b/ee/app/services/packages/create_maven_package_service.rb @@ -2,24 +2,22 @@ module Packages class CreateMavenPackageService < BaseService def execute - package = Packages::Package.create!( - project: project, - name: full_app_name, - version: params[:app_version] + package = project.packages.create!( + name: params[:name], + version: params[:version] ) - Packages::MavenMetadatum.create!( - package: package, - app_group: params[:app_group], - app_name: params[:app_name], - app_version: params[:app_version] - ) - end + app_group, _, app_name = params[:name].rpartition('/') + app_group.tr!('/', '.') - private + package.create_maven_metadatum!( + path: params[:path], + app_group: app_group, + app_name: app_name, + app_version: params[:version] + ) - def full_app_name - params[:app_group] + '/' + params[:app_name] + package end end end diff --git a/ee/app/services/packages/create_package_file_service.rb b/ee/app/services/packages/create_package_file_service.rb new file mode 100644 index 00000000000000..a0e3df9d3f7b1e --- /dev/null +++ b/ee/app/services/packages/create_package_file_service.rb @@ -0,0 +1,19 @@ +class Packages::CreatePackageFileService + attr_reader :package, :params + + def initialize(package, params) + @package = package + @params = params + end + + def execute + package.package_files.create!( + file: parmas[:file], + size: params[:size], + file_name: params[:file_name], + file_type: params[:file_type], + file_sha1: params[:file_sha1], + file_md5: params[:file_md5] + ) + end +end diff --git a/ee/db/migrate/20180720120716_create_packages_packages.rb b/ee/db/migrate/20180720120716_create_packages_packages.rb index 8779e2f6d220d9..f9c89936b972a5 100644 --- a/ee/db/migrate/20180720120716_create_packages_packages.rb +++ b/ee/db/migrate/20180720120716_create_packages_packages.rb @@ -5,7 +5,7 @@ class CreatePackagesPackages < ActiveRecord::Migration def change create_table :packages_packages do |t| t.references :project, index: true, foreign_key: { on_delete: :cascade }, null: false - t.string :name + t.string :name, null: false t.string :version t.timestamps_with_timezone null: false diff --git a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb index c868b4b6cc1bc2..52e8993e5d1a5e 100644 --- a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb +++ b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb @@ -9,9 +9,10 @@ class CreatePackagesMavenMetadata < ActiveRecord::Migration def up create_table :packages_maven_metadata do |t| t.references :package, index: true, null: false + t.string :path, null: false t.string :app_group, null: false t.string :app_name, null: false - t.string :app_version, null: false + t.string :app_version t.timestamps_with_timezone null: false end diff --git a/ee/lib/api/maven_packages.rb b/ee/lib/api/maven_packages.rb index 3a5288bba07054..424a57b9532166 100644 --- a/ee/lib/api/maven_packages.rb +++ b/ee/lib/api/maven_packages.rb @@ -2,8 +2,6 @@ module API class MavenPackages < Grape::API MAVEN_ENDPOINT_REQUIREMENTS = { - app_name: API::NO_SLASH_URL_PART_REGEX, - app_version: API::NO_SLASH_URL_PART_REGEX, file_name: API::NO_SLASH_URL_PART_REGEX }.freeze @@ -25,14 +23,6 @@ def extract_format(file_name) [file_name, nil] end end - - def valid_metadata_xml?(xml) - version = Nokogiri::XML(xml).css('metadata:root > version').text - - # Skip handling top level maven-metadata.xml (one without the version) for now. - # Also make sure version in the metadata file is equal to one in the URL - version.present? && version == params[:app_version] - end end params do @@ -43,21 +33,18 @@ def valid_metadata_xml?(xml) detail 'This feature was introduced in GitLab 11.3' end params do - requires :app_group, type: String, desc: 'Package group id' - requires :app_name, type: String, desc: 'Package artifact id' - requires :app_version, type: String, desc: 'Package version' + requires :path, type: String, desc: 'Package path' requires :file_name, type: String, desc: 'Package file name' end - get ':id/packages/maven/*app_group/:app_name/:app_version/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do + get ':id/packages/maven/*path/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do unauthorized! unless can?(current_user, :read_package, user_project) file_name, format = extract_format(params[:file_name]) - metadata = ::Packages::MavenMetadatum.find_by!(app_group: params[:app_group], - app_name: params[:app_name], - app_version: params[:app_version]) + package = ::Packages::MavenPackageFinder + .new(user_project, params[:path]).execute! - package_file = metadata.package.package_files.recent.find_by!(file_name: file_name) + package_file = package.package_files.recent.find_by!(file_name: file_name) case format when 'md5' @@ -73,12 +60,10 @@ def valid_metadata_xml?(xml) detail 'This feature was introduced in GitLab 11.3' end params do - requires :app_group, type: String, desc: 'Package group id' - requires :app_name, type: String, desc: 'Package artifact id' - requires :app_version, type: String, desc: 'Package version' + requires :path, type: String, desc: 'Package path' requires :file_name, type: String, desc: 'Package file name' end - put ':id/packages/maven/*app_group/:app_name/:app_version/:file_name/authorize', requirements: MAVEN_ENDPOINT_REQUIREMENTS do + put ':id/packages/maven/*path/:file_name/authorize', requirements: MAVEN_ENDPOINT_REQUIREMENTS do not_allowed! unless Gitlab.config.packages.enabled unauthorized! unless can?(current_user, :admin_package, user_project) @@ -94,17 +79,17 @@ def valid_metadata_xml?(xml) detail 'This feature was introduced in GitLab 11.3' end params do - requires :app_group, type: String, desc: 'Package group id' - requires :app_name, type: String, desc: 'Package artifact id' - requires :app_version, type: String, desc: 'Package version' + requires :path, type: String, desc: 'Package path' requires :file_name, type: String, desc: 'Package file name' optional 'file.path', type: String, desc: %q(path to locally stored body (generated by Workhorse)) optional 'file.name', type: String, desc: %q(real filename as send in Content-Disposition (generated by Workhorse)) optional 'file.type', type: String, desc: %q(real content type as send in Content-Type (generated by Workhorse)) optional 'file.size', type: Integer, desc: %q(real size of file (generated by Workhorse)) + optional 'file.md5', type: String, desc: %q(md5 checksum of the file (generated by Workhorse)) + optional 'file.sha1', type: String, desc: %q(sha1 checksum of the file (generated by Workhorse)) optional 'file.sha256', type: String, desc: %q(sha256 checksum of the file (generated by Workhorse)) end - put ':id/packages/maven/*app_group/:app_name/:app_version/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do + put ':id/packages/maven/*path/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do not_allowed! unless Gitlab.config.packages.enabled unauthorized! unless can?(current_user, :admin_package, user_project) @@ -115,46 +100,48 @@ def valid_metadata_xml?(xml) uploaded_file = UploadedFile.from_params(params, :file, ::Packages::PackageFileUploader.workhorse_local_upload_path) bad_request!('Missing package file!') unless uploaded_file - metadata = ::Packages::MavenMetadatum.find_by(app_group: params[:app_group], - app_name: params[:app_name], - app_version: params[:app_version]) - - # TODO: Refactor. We don't need to read file for every request, only metadata and md5/sha1 - string_file = File.read(uploaded_file) + package = ::Packages::MavenPackageFinder + .new(user_project, params[:path]).execute - unless metadata + unless package if file_name == MAVEN_METADATA_FILE - return unless valid_metadata_xml?(string_file) + # Maven uploads several files during `mvn deploy` in next order: + # - my-company/my-app/1.0-SNAPSHOT/my-app.jar + # - my-company/my-app/1.0-SNAPSHOT/my-app.pom + # - my-company/my-app/1.0-SNAPSHOT/maven-metadata.xml + # - my-company/my-app/maven-metadata.xml + # + # The last xml file does not have VERSION in URL because it contains + # information about all versions. + package_name, version = params[:path], nil + else + package_name, _, version = params[:path].rpartition('/') end - # There is no metadata for this upload. We need to create a package - # record and corresponding maven metadata record - metadata = Packages::CreateMavenPackageService.new(user_project, current_user, params).execute + package_params = { + name: package_name, + path: params[:path], + version: version + } + + package = ::Packages::CreateMavenPackageService.new(user_project, current_user, package_params).execute end if format - # Maven tries to create a md5 and sha1 files for each package file. - # Instead, we update existing package file record with such data. - package_file = metadata.package.package_files.recent.find_by!(file_name: file_name) - - case format - when 'md5' - package_file.file_md5 = string_file - when 'sha1' - package_file.file_sha1 = string_file - end + # TODO: Validate our checksum with Maven. + no_content! else - # TODO: If maven-metadata.xml file is present for this package, we - # need to update it instead of creating a new one - package_file = metadata.package.package_files.new - package_file.file_name = file_name - package_file.file_type = file_name.rpartition('.').last - - # Convert string into CarrierWave compatible StringIO object - package_file.file = uploaded_file + file_params = { + file: uploaded_file, + size: params['file.size'], + file_name: file_name, + file_type: params['file.type'], + file_sha1: params['file.sha1'], + file_md5: params['file.md5'] + } + + ::Packages::CreatePackageFileService.new(package, file_params).execute end - - package_file.save! end end end -- GitLab From 4f027066c0912ef3090d7cd0eb5a7bd0984c3bd2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 6 Aug 2018 18:54:20 +0300 Subject: [PATCH 30/55] Fix var typo in create_package_file_service.rb Signed-off-by: Dmitriy Zaporozhets --- ee/app/services/packages/create_package_file_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/services/packages/create_package_file_service.rb b/ee/app/services/packages/create_package_file_service.rb index a0e3df9d3f7b1e..dd94172070357d 100644 --- a/ee/app/services/packages/create_package_file_service.rb +++ b/ee/app/services/packages/create_package_file_service.rb @@ -8,7 +8,7 @@ def initialize(package, params) def execute package.package_files.create!( - file: parmas[:file], + file: params[:file], size: params[:size], file_name: params[:file_name], file_type: params[:file_type], -- GitLab From 0d07bdfb4a624e0b98e23f601d5fcb877ef24fde Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 6 Aug 2018 19:03:44 +0300 Subject: [PATCH 31/55] Packages will not be included in project import/export feature for now Signed-off-by: Dmitriy Zaporozhets --- spec/lib/gitlab/import_export/all_models.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 54cf88b926110c..e10298cd106993 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -348,6 +348,7 @@ project: - software_license_policies - repository_languages - project_registry +- packages award_emoji: - awardable - user -- GitLab From ab8a23ed0f7b73ab8c1cdcead609e213781a502c Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 6 Aug 2018 19:15:25 +0300 Subject: [PATCH 32/55] Add more validations and update specs for maven package feature Signed-off-by: Dmitriy Zaporozhets --- ee/app/finders/packages/maven_package_finder.rb | 1 + ee/app/models/packages/maven_metadatum.rb | 3 +++ ee/app/models/packages/package.rb | 1 + ee/app/services/packages/create_package_file_service.rb | 1 + ee/spec/factories/packages.rb | 5 +++-- ee/spec/requests/api/maven_packages_spec.rb | 9 ++++----- 6 files changed, 13 insertions(+), 7 deletions(-) diff --git a/ee/app/finders/packages/maven_package_finder.rb b/ee/app/finders/packages/maven_package_finder.rb index d81ef4530040d0..f2cc5bf0af37b9 100644 --- a/ee/app/finders/packages/maven_package_finder.rb +++ b/ee/app/finders/packages/maven_package_finder.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class Packages::MavenPackageFinder attr_reader :project, :path diff --git a/ee/app/models/packages/maven_metadatum.rb b/ee/app/models/packages/maven_metadatum.rb index 58ba74d9debc5e..4b6a95377d150e 100644 --- a/ee/app/models/packages/maven_metadatum.rb +++ b/ee/app/models/packages/maven_metadatum.rb @@ -3,4 +3,7 @@ class Packages::MavenMetadatum < ActiveRecord::Base belongs_to :package validates :package, presence: true + validates :path, presence: true + validates :app_group, presence: true + validates :app_name, presence: true end diff --git a/ee/app/models/packages/package.rb b/ee/app/models/packages/package.rb index 8a655745132f25..55dca0fd10ff9c 100644 --- a/ee/app/models/packages/package.rb +++ b/ee/app/models/packages/package.rb @@ -5,4 +5,5 @@ class Packages::Package < ActiveRecord::Base has_one :maven_metadatum validates :project, presence: true + validates :name, presence: true end diff --git a/ee/app/services/packages/create_package_file_service.rb b/ee/app/services/packages/create_package_file_service.rb index dd94172070357d..7e2f40bdcfa3c9 100644 --- a/ee/app/services/packages/create_package_file_service.rb +++ b/ee/app/services/packages/create_package_file_service.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true class Packages::CreatePackageFileService attr_reader :package, :params diff --git a/ee/spec/factories/packages.rb b/ee/spec/factories/packages.rb index f6244396a020b0..c89cb13a42a941 100644 --- a/ee/spec/factories/packages.rb +++ b/ee/spec/factories/packages.rb @@ -43,8 +43,9 @@ factory :maven_metadatum, class: Packages::MavenMetadatum do package - app_group 'my/company/app' + path 'my/company/app/my-app/1.0-SNAPSHOT' + app_group 'my.company.app' app_name 'my-app' - app_version '1-0-SNAPSHOT' + app_version '1.0-SNAPSHOT' end end diff --git a/ee/spec/requests/api/maven_packages_spec.rb b/ee/spec/requests/api/maven_packages_spec.rb index cebb9cce8614e4..684c610fd214a9 100644 --- a/ee/spec/requests/api/maven_packages_spec.rb +++ b/ee/spec/requests/api/maven_packages_spec.rb @@ -13,7 +13,7 @@ project.add_developer(user) end - describe 'GET /api/v4/projects/:id/packages/maven/*app_group/:app_name/:app_version/:file_name' do + describe 'GET /api/v4/projects/:id/packages/maven/*path/:file_name' do let(:package) { create(:maven_package, project: project) } let(:maven_metadatum) { package.maven_metadatum } let(:package_file_xml) { package.package_files.find_by(file_type: 'xml') } @@ -64,8 +64,7 @@ def download_file(file_name, params = {}, request_headers = headers) get api("/projects/#{project.id}/packages/maven/" \ - "#{maven_metadatum.app_group}/#{maven_metadatum.app_name}/" \ - "#{maven_metadatum.app_version}/#{file_name}"), params, request_headers + "#{maven_metadatum.path}/#{file_name}"), params, request_headers end def download_file_with_token(file_name, params = {}, request_headers = headers_with_token) @@ -73,7 +72,7 @@ def download_file_with_token(file_name, params = {}, request_headers = headers_w end end - describe 'PUT /api/v4/projects/:id/packages/maven/*app_group/:app_name/:app_version/:file_name/authorize' do + describe 'PUT /api/v4/projects/:id/packages/maven/*path/:file_name/authorize' do it 'authorizes posting package with a valid token' do authorize_upload_with_token @@ -115,7 +114,7 @@ def authorize_upload_with_token(params = {}, request_headers = headers_with_toke end end - describe 'PUT /api/v4/projects/:id/packages/maven/*app_group/:app_name/:app_version/:file_name' do + describe 'PUT /api/v4/projects/:id/packages/maven/*path/:file_name' do let(:file_upload) { fixture_file_upload('ee/spec/fixtures/maven/maven-metadata.xml') } before do -- GitLab From ca9d5265d47cd0be64716b0b1d2b4b8ad75a75d2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 7 Aug 2018 18:07:59 +0300 Subject: [PATCH 33/55] Add all sort of checks on maven package api Signed-off-by: Dmitriy Zaporozhets --- ee/lib/api/maven_packages.rb | 30 ++++++++++++++++----- ee/spec/requests/api/maven_packages_spec.rb | 21 +++++++++++++-- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/ee/lib/api/maven_packages.rb b/ee/lib/api/maven_packages.rb index 424a57b9532166..aa5a7bf28aee66 100644 --- a/ee/lib/api/maven_packages.rb +++ b/ee/lib/api/maven_packages.rb @@ -11,9 +11,29 @@ class MavenPackages < Grape::API content_type :sha1, 'text/plain' content_type :binary, 'application/octet-stream' - before { authenticate_non_get! } + before do + require_packages_enabled! + authenticate_non_get! + authorize_packages_feature! + end helpers do + def require_packages_enabled! + not_found! unless Gitlab.config.packages.enabled + end + + def authorize_packages_feature! + forbidden! unless user_project.feature_available?(:packages) + end + + def authorize_can_read! + authorize!(:read_package, user_project) + end + + def authorize_can_admin! + authorize!(:admin_package, user_project) + end + def extract_format(file_name) name, _, format = file_name.rpartition('.') @@ -37,7 +57,7 @@ def extract_format(file_name) requires :file_name, type: String, desc: 'Package file name' end get ':id/packages/maven/*path/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do - unauthorized! unless can?(current_user, :read_package, user_project) + authorize_can_read! file_name, format = extract_format(params[:file_name]) @@ -64,8 +84,7 @@ def extract_format(file_name) requires :file_name, type: String, desc: 'Package file name' end put ':id/packages/maven/*path/:file_name/authorize', requirements: MAVEN_ENDPOINT_REQUIREMENTS do - not_allowed! unless Gitlab.config.packages.enabled - unauthorized! unless can?(current_user, :admin_package, user_project) + authorize_can_admin! require_gitlab_workhorse! Gitlab::Workhorse.verify_api_request!(headers) @@ -90,8 +109,7 @@ def extract_format(file_name) optional 'file.sha256', type: String, desc: %q(sha256 checksum of the file (generated by Workhorse)) end put ':id/packages/maven/*path/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do - not_allowed! unless Gitlab.config.packages.enabled - unauthorized! unless can?(current_user, :admin_package, user_project) + authorize_can_admin! require_gitlab_workhorse! diff --git a/ee/spec/requests/api/maven_packages_spec.rb b/ee/spec/requests/api/maven_packages_spec.rb index 684c610fd214a9..e7564404f82cd3 100644 --- a/ee/spec/requests/api/maven_packages_spec.rb +++ b/ee/spec/requests/api/maven_packages_spec.rb @@ -11,6 +11,7 @@ before do project.add_developer(user) + stub_licensed_features(packages: true) end describe 'GET /api/v4/projects/:id/packages/maven/*path/:file_name' do @@ -52,7 +53,7 @@ download_file_with_token(package_file_xml.file_name) - expect(response).to have_gitlab_http_status(401) + expect(response).to have_gitlab_http_status(403) end it 'denies download when no private token' do @@ -62,6 +63,14 @@ end end + it 'rejects request if feature is not in the license' do + stub_licensed_features(packages: false) + + download_file(package_file_xml.file_name) + + expect(response).to have_gitlab_http_status(403) + end + def download_file(file_name, params = {}, request_headers = headers) get api("/projects/#{project.id}/packages/maven/" \ "#{maven_metadatum.path}/#{file_name}"), params, request_headers @@ -94,7 +103,7 @@ def download_file_with_token(file_name, params = {}, request_headers = headers_w authorize_upload_with_token - expect(response).to have_gitlab_http_status(401) + expect(response).to have_gitlab_http_status(403) end it 'rejects requests that did not go through gitlab-workhorse' do @@ -134,6 +143,14 @@ def authorize_upload_with_token(params = {}, request_headers = headers_with_toke expect(response).to have_gitlab_http_status(401) end + it 'rejects request if feature is not in the license' do + stub_licensed_features(packages: false) + + upload_file_with_token + + expect(response).to have_gitlab_http_status(403) + end + context 'when params from workhorse are correct' do let(:package) { project.packages.reload.last } let(:package_file) { package.package_files.reload.last } -- GitLab From 43d1416a66902e04127c96b42a89f921aeedfd9f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 8 Aug 2018 10:44:32 +0300 Subject: [PATCH 34/55] Add changelog for maven packages feature [ci skip] Signed-off-by: Dmitriy Zaporozhets --- ...5811-add-maven-support-to-our-artifact-repository-mvc.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 ee/changelogs/unreleased/5811-add-maven-support-to-our-artifact-repository-mvc.yml diff --git a/ee/changelogs/unreleased/5811-add-maven-support-to-our-artifact-repository-mvc.yml b/ee/changelogs/unreleased/5811-add-maven-support-to-our-artifact-repository-mvc.yml new file mode 100644 index 00000000000000..e75e1855b99816 --- /dev/null +++ b/ee/changelogs/unreleased/5811-add-maven-support-to-our-artifact-repository-mvc.yml @@ -0,0 +1,5 @@ +--- +title: Add ability to upload and download maven packages from/to GitLab +merge_request: 6607 +author: +type: added -- GitLab From 60c64b7656fd1456a2a114d4de5ecf800f0cf142 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 8 Aug 2018 18:30:12 +0300 Subject: [PATCH 35/55] Add checksum for uploaded maven packages [ci skip] Signed-off-by: Dmitriy Zaporozhets --- ee/lib/api/maven_packages.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/ee/lib/api/maven_packages.rb b/ee/lib/api/maven_packages.rb index aa5a7bf28aee66..48d92c42aca7f9 100644 --- a/ee/lib/api/maven_packages.rb +++ b/ee/lib/api/maven_packages.rb @@ -146,8 +146,18 @@ def extract_format(file_name) end if format - # TODO: Validate our checksum with Maven. - no_content! + # TODO: Extract in separate method + if format == 'sha1' + package_file = package.package_files.recent.find_by!(file_name: file_name) + stored_sha1 = Digest::SHA256.hexdigest(package_file.file_sha1) + expected_sha1 = uploaded_file.sha256 + + if stored_sha1 == expected_sha1 + no_content! + else + conflict! + end + end else file_params = { file: uploaded_file, -- GitLab From 61e3269599b714758038b79f2d7a1d8bf6a2902c Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 9 Aug 2018 16:30:32 +0300 Subject: [PATCH 36/55] Refactor maven packages code and add more specs Signed-off-by: Dmitriy Zaporozhets --- .../finders/packages/package_file_finder.rb | 23 ++++++ ee/app/models/packages/package_file.rb | 4 +- .../packages/create_package_file_service.rb | 32 ++++---- ee/lib/api/maven_packages.rb | 41 ++++++---- .../packages/maven_package_finder_spec.rb | 21 ++++++ .../packages/package_file_finder_spec.rb | 21 ++++++ .../create_maven_package_service_spec.rb | 74 +++++++++++++++++++ .../create_package_file_service_spec.rb | 38 ++++++++++ 8 files changed, 221 insertions(+), 33 deletions(-) create mode 100644 ee/app/finders/packages/package_file_finder.rb create mode 100644 ee/spec/finders/packages/maven_package_finder_spec.rb create mode 100644 ee/spec/finders/packages/package_file_finder_spec.rb create mode 100644 ee/spec/services/packages/create_maven_package_service_spec.rb create mode 100644 ee/spec/services/packages/create_package_file_service_spec.rb diff --git a/ee/app/finders/packages/package_file_finder.rb b/ee/app/finders/packages/package_file_finder.rb new file mode 100644 index 00000000000000..74db44073d99e0 --- /dev/null +++ b/ee/app/finders/packages/package_file_finder.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true +class Packages::PackageFileFinder + attr_reader :package, :file_name + + def initialize(package, file_name) + @package = package + @file_name = file_name + end + + def execute + package_files.last + end + + def execute! + package_files.last! + end + + private + + def package_files + package.package_files.where(file_name: file_name) + end +end diff --git a/ee/app/models/packages/package_file.rb b/ee/app/models/packages/package_file.rb index abd709d5f29703..ab11dd611a3307 100644 --- a/ee/app/models/packages/package_file.rb +++ b/ee/app/models/packages/package_file.rb @@ -3,8 +3,8 @@ class Packages::PackageFile < ActiveRecord::Base belongs_to :package validates :package, presence: true + validates :file, presence: true + validates :file_name, presence: true mount_uploader :file, Packages::PackageFileUploader - - scope :recent, -> { reorder(id: :desc) } end diff --git a/ee/app/services/packages/create_package_file_service.rb b/ee/app/services/packages/create_package_file_service.rb index 7e2f40bdcfa3c9..58abed5bcd5846 100644 --- a/ee/app/services/packages/create_package_file_service.rb +++ b/ee/app/services/packages/create_package_file_service.rb @@ -1,20 +1,22 @@ # frozen_string_literal: true -class Packages::CreatePackageFileService - attr_reader :package, :params +module Packages + class CreatePackageFileService + attr_reader :package, :params - def initialize(package, params) - @package = package - @params = params - end + def initialize(package, params) + @package = package + @params = params + end - def execute - package.package_files.create!( - file: params[:file], - size: params[:size], - file_name: params[:file_name], - file_type: params[:file_type], - file_sha1: params[:file_sha1], - file_md5: params[:file_md5] - ) + def execute + package.package_files.create!( + file: params[:file], + size: params[:size], + file_name: params[:file_name], + file_type: params[:file_type], + file_sha1: params[:file_sha1], + file_md5: params[:file_md5] + ) + end end end diff --git a/ee/lib/api/maven_packages.rb b/ee/lib/api/maven_packages.rb index 48d92c42aca7f9..2710b7aab0d27b 100644 --- a/ee/lib/api/maven_packages.rb +++ b/ee/lib/api/maven_packages.rb @@ -43,6 +43,17 @@ def extract_format(file_name) [file_name, nil] end end + + def verify_package_file(package_file, uploaded_file) + stored_sha1 = Digest::SHA256.hexdigest(package_file.file_sha1) + expected_sha1 = uploaded_file.sha256 + + if stored_sha1 == expected_sha1 + no_content! + else + conflict! + end + end end params do @@ -64,7 +75,8 @@ def extract_format(file_name) package = ::Packages::MavenPackageFinder .new(user_project, params[:path]).execute! - package_file = package.package_files.recent.find_by!(file_name: file_name) + package_file = ::Packages::PackageFileFinder + .new(package, file_name).execute! case format when 'md5' @@ -142,23 +154,20 @@ def extract_format(file_name) version: version } - package = ::Packages::CreateMavenPackageService.new(user_project, current_user, package_params).execute + package = ::Packages::CreateMavenPackageService + .new(user_project, current_user, package_params).execute end - if format - # TODO: Extract in separate method - if format == 'sha1' - package_file = package.package_files.recent.find_by!(file_name: file_name) - stored_sha1 = Digest::SHA256.hexdigest(package_file.file_sha1) - expected_sha1 = uploaded_file.sha256 - - if stored_sha1 == expected_sha1 - no_content! - else - conflict! - end - end - else + case format + when 'sha1' + # After uploading a file, Maven tries to upload a sha1 and md5 version of it. + # Since we store md5/sha1 in database we simply need to validate our hash + # against one uploaded by Maven. We do this for `sha1` format. + package_file = ::Packages::PackageFileFinder + .new(package, file_name).execute! + + verify_package_file(package_file, uploaded_file) + when nil file_params = { file: uploaded_file, size: params['file.size'], diff --git a/ee/spec/finders/packages/maven_package_finder_spec.rb b/ee/spec/finders/packages/maven_package_finder_spec.rb new file mode 100644 index 00000000000000..eb198f527c2e48 --- /dev/null +++ b/ee/spec/finders/packages/maven_package_finder_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Packages::MavenPackageFinder do + let(:project) { create(:project) } + let(:package) { create(:maven_package, project: project) } + + describe '#execute!' do + it 'returns a package' do + finder = described_class.new(project, package.maven_metadatum.path) + + expect(finder.execute!).to eq(package) + end + + it 'raises an error' do + finder = described_class.new(project, 'com/example/my-app/1.0-SNAPSHOT') + + expect { finder.execute! }.to raise_error(ActiveRecord::RecordNotFound) + end + end +end diff --git a/ee/spec/finders/packages/package_file_finder_spec.rb b/ee/spec/finders/packages/package_file_finder_spec.rb new file mode 100644 index 00000000000000..9ee08026a64bdb --- /dev/null +++ b/ee/spec/finders/packages/package_file_finder_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Packages::PackageFileFinder do + let(:package) { create(:maven_package) } + let(:package_file) { package.package_files.first } + + describe '#execute!' do + it 'returns a package file' do + finder = described_class.new(package, package_file.file_name) + + expect(finder.execute!).to eq(package_file) + end + + it 'raises an error' do + finder = described_class.new(package, 'unknown.jpg') + + expect { finder.execute! }.to raise_error(ActiveRecord::RecordNotFound) + end + end +end diff --git a/ee/spec/services/packages/create_maven_package_service_spec.rb b/ee/spec/services/packages/create_maven_package_service_spec.rb new file mode 100644 index 00000000000000..45598085060276 --- /dev/null +++ b/ee/spec/services/packages/create_maven_package_service_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Packages::CreateMavenPackageService do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:app_name) { 'my-app'.freeze } + let(:version) { '1.0-SNAPSHOT'.freeze } + let(:path) { "my/company/app/#{app_name}" } + let(:path_with_version) { "#{path}/#{version}" } + + describe '#execute' do + context 'with version' do + let(:params) do + { + path: path_with_version, + name: path, + version: version + } + end + + it 'creates a new package with metadatum' do + package = described_class.new(project, user, params).execute + + expect(package).to be_valid + expect(package.name).to eq(path) + expect(package.version).to eq(version) + expect(package.maven_metadatum).to be_valid + expect(package.maven_metadatum.path).to eq(path_with_version) + expect(package.maven_metadatum.app_group).to eq('my.company.app') + expect(package.maven_metadatum.app_name).to eq(app_name) + expect(package.maven_metadatum.app_version).to eq(version) + end + end + + context 'without version' do + let(:params) do + { + path: path, + name: path, + version: nil + } + end + + it 'creates a new package with metadatum' do + package = described_class.new(project, user, params).execute + + expect(package).to be_valid + expect(package.name).to eq(path) + expect(package.version).to be nil + expect(package.maven_metadatum).to be_valid + expect(package.maven_metadatum.path).to eq(path) + expect(package.maven_metadatum.app_group).to eq('my.company.app') + expect(package.maven_metadatum.app_name).to eq(app_name) + expect(package.maven_metadatum.app_version).to be nil + end + end + + context 'path is missing' do + let(:params) do + { + name: path, + version: version + } + end + + it 'raises an error' do + service = described_class.new(project, user, params) + + expect { service.execute }.to raise_error(ActiveRecord::RecordInvalid) + end + end + end +end diff --git a/ee/spec/services/packages/create_package_file_service_spec.rb b/ee/spec/services/packages/create_package_file_service_spec.rb new file mode 100644 index 00000000000000..4cd880d3f40aa4 --- /dev/null +++ b/ee/spec/services/packages/create_package_file_service_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Packages::CreatePackageFileService do + let(:package) { create(:maven_package) } + + describe '#execute' do + context 'with valid params' do + let(:params) do + { + file: Tempfile.new, + file_name: 'foo.jar' + } + end + + it 'creates a new package file' do + package_file = described_class.new(package, params).execute + + expect(package_file).to be_valid + expect(package_file.file_name).to eq('foo.jar') + end + end + + context 'file is missing' do + let(:params) do + { + file_name: 'foo.jar' + } + end + + it 'raises an error' do + service = described_class.new(package, params) + + expect { service.execute }.to raise_error(ActiveRecord::RecordInvalid) + end + end + end +end -- GitLab From cbe6403e00eb51140cace04a86850c729b9230d7 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 10 Aug 2018 19:14:06 +0300 Subject: [PATCH 37/55] Improve database schema for packages feature [ci skip] Signed-off-by: Dmitriy Zaporozhets --- db/schema.rb | 22 +++++++++++-------- ...20180720120716_create_packages_packages.rb | 8 +++++-- ...720120726_create_packages_package_files.rb | 8 ++++--- ...20121404_create_packages_maven_metadata.rb | 4 ++-- ...0810155213_add_more_indices_to_packages.rb | 17 ++++++++++++++ ee/lib/api/maven_packages.rb | 10 ++++----- 6 files changed, 48 insertions(+), 21 deletions(-) create mode 100644 ee/db/migrate/20180810155213_add_more_indices_to_packages.rb diff --git a/db/schema.rb b/db/schema.rb index 34ae5569fcc01c..fc7e1c59026ef9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180807153545) do +ActiveRecord::Schema.define(version: 20180810155213) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1922,38 +1922,40 @@ end create_table "packages_maven_metadata", force: :cascade do |t| + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false t.integer "package_id", null: false t.string "path", null: false t.string "app_group", null: false t.string "app_name", null: false t.string "app_version" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false end add_index "packages_maven_metadata", ["package_id"], name: "index_packages_maven_metadata_on_package_id", using: :btree + add_index "packages_maven_metadata", ["path"], name: "index_packages_maven_metadata_on_path", using: :btree create_table "packages_package_files", force: :cascade do |t| + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false t.integer "package_id", null: false - t.string "file" - t.string "file_name", null: false t.integer "file_type" t.integer "file_store" t.integer "size" t.binary "file_md5" t.binary "file_sha1" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.string "file" + t.string "file_name", null: false end + add_index "packages_package_files", ["package_id", "file_name"], name: "index_packages_package_files_on_package_id_and_file_name", using: :btree add_index "packages_package_files", ["package_id"], name: "index_packages_package_files_on_package_id", using: :btree create_table "packages_packages", force: :cascade do |t| + t.datetime_with_timezone "created_at", null: false + t.datetime_with_timezone "updated_at", null: false t.integer "project_id", null: false t.string "name", null: false t.string "version" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false end add_index "packages_packages", ["project_id"], name: "index_packages_packages_on_project_id", using: :btree @@ -3119,6 +3121,8 @@ add_foreign_key "notes", "projects", name: "fk_99e097b079", on_delete: :cascade add_foreign_key "notification_settings", "users", name: "fk_0c95e91db7", on_delete: :cascade add_foreign_key "oauth_openid_requests", "oauth_access_grants", column: "access_grant_id", name: "fk_oauth_openid_requests_oauth_access_grants_access_grant_id" + add_foreign_key "packages_maven_metadata", "packages_packages", column: "package_id", name: "fk_be88aed360", on_delete: :cascade + add_foreign_key "packages_package_files", "packages_packages", column: "package_id", name: "fk_86f0f182f8", on_delete: :cascade add_foreign_key "pages_domains", "projects", name: "fk_ea2f6dfc6f", on_delete: :cascade add_foreign_key "path_locks", "projects", name: "fk_5265c98f24", on_delete: :cascade add_foreign_key "path_locks", "users" diff --git a/ee/db/migrate/20180720120716_create_packages_packages.rb b/ee/db/migrate/20180720120716_create_packages_packages.rb index f9c89936b972a5..01f229a843e38a 100644 --- a/ee/db/migrate/20180720120716_create_packages_packages.rb +++ b/ee/db/migrate/20180720120716_create_packages_packages.rb @@ -1,14 +1,18 @@ # frozen_string_literal: true class CreatePackagesPackages < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + DOWNTIME = false + disable_ddl_transaction! + def change create_table :packages_packages do |t| + t.timestamps_with_timezone null: false t.references :project, index: true, foreign_key: { on_delete: :cascade }, null: false + t.string :name, null: false t.string :version - - t.timestamps_with_timezone null: false end end end diff --git a/ee/db/migrate/20180720120726_create_packages_package_files.rb b/ee/db/migrate/20180720120726_create_packages_package_files.rb index 15696f29a34d34..d6e3292e334d90 100644 --- a/ee/db/migrate/20180720120726_create_packages_package_files.rb +++ b/ee/db/migrate/20180720120726_create_packages_package_files.rb @@ -8,16 +8,18 @@ class CreatePackagesPackageFiles < ActiveRecord::Migration def up create_table :packages_package_files do |t| + t.timestamps_with_timezone null: false t.references :package, index: true, null: false - t.string :file - t.string :file_name, null: false + t.integer :file_type t.integer :file_store t.integer :size + t.binary :file_md5 t.binary :file_sha1 - t.timestamps_with_timezone null: false + t.string :file + t.string :file_name, null: false end add_concurrent_foreign_key :packages_package_files, :packages_packages, diff --git a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb index 52e8993e5d1a5e..45fb03dfab35d1 100644 --- a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb +++ b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb @@ -8,13 +8,13 @@ class CreatePackagesMavenMetadata < ActiveRecord::Migration def up create_table :packages_maven_metadata do |t| + t.timestamps_with_timezone null: false t.references :package, index: true, null: false + t.string :path, null: false t.string :app_group, null: false t.string :app_name, null: false t.string :app_version - - t.timestamps_with_timezone null: false end add_concurrent_foreign_key :packages_maven_metadata, :packages_packages, diff --git a/ee/db/migrate/20180810155213_add_more_indices_to_packages.rb b/ee/db/migrate/20180810155213_add_more_indices_to_packages.rb new file mode 100644 index 00000000000000..b7406b8bfd830f --- /dev/null +++ b/ee/db/migrate/20180810155213_add_more_indices_to_packages.rb @@ -0,0 +1,17 @@ +class AddMoreIndicesToPackages < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :packages_package_files, [:package_id, :file_name] + add_concurrent_index :packages_maven_metadata, :path + end + + def down + remove_concurrent_index :packages_package_files, [:package_id, :file_name] + remove_concurrent_index :packages_maven_metadata, :path + end +end diff --git a/ee/lib/api/maven_packages.rb b/ee/lib/api/maven_packages.rb index 2710b7aab0d27b..127093ea4f6fc7 100644 --- a/ee/lib/api/maven_packages.rb +++ b/ee/lib/api/maven_packages.rb @@ -26,11 +26,11 @@ def authorize_packages_feature! forbidden! unless user_project.feature_available?(:packages) end - def authorize_can_read! + def authorize_download_package! authorize!(:read_package, user_project) end - def authorize_can_admin! + def authorize_create_package! authorize!(:admin_package, user_project) end @@ -68,7 +68,7 @@ def verify_package_file(package_file, uploaded_file) requires :file_name, type: String, desc: 'Package file name' end get ':id/packages/maven/*path/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do - authorize_can_read! + authorize_download_package! file_name, format = extract_format(params[:file_name]) @@ -96,7 +96,7 @@ def verify_package_file(package_file, uploaded_file) requires :file_name, type: String, desc: 'Package file name' end put ':id/packages/maven/*path/:file_name/authorize', requirements: MAVEN_ENDPOINT_REQUIREMENTS do - authorize_can_admin! + authorize_create_package! require_gitlab_workhorse! Gitlab::Workhorse.verify_api_request!(headers) @@ -121,7 +121,7 @@ def verify_package_file(package_file, uploaded_file) optional 'file.sha256', type: String, desc: %q(sha256 checksum of the file (generated by Workhorse)) end put ':id/packages/maven/*path/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do - authorize_can_admin! + authorize_create_package! require_gitlab_workhorse! -- GitLab From ffea496b137a0c6c4503b2fb071251df6575cec1 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 10 Aug 2018 19:32:15 +0300 Subject: [PATCH 38/55] Fix PackageFile for object storage [ci skip] Signed-off-by: Dmitriy Zaporozhets --- ee/app/models/packages/package_file.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ee/app/models/packages/package_file.rb b/ee/app/models/packages/package_file.rb index ab11dd611a3307..fb2697e7e8246b 100644 --- a/ee/app/models/packages/package_file.rb +++ b/ee/app/models/packages/package_file.rb @@ -7,4 +7,12 @@ class Packages::PackageFile < ActiveRecord::Base validates :file_name, presence: true mount_uploader :file, Packages::PackageFileUploader + + after_save :update_file_store, if: :file_changed? + + def update_file_store + # The file.object_store is set during `uploader.store!` + # which happens after object is inserted/updated + self.update_column(:file_store, file.object_store) + end end -- GitLab From 68237290aa1cbdac641f87d3693becb3c8f10660 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sat, 11 Aug 2018 13:21:16 +0300 Subject: [PATCH 39/55] Create maven package in one transaction Signed-off-by: Dmitriy Zaporozhets --- ee/app/models/packages/package.rb | 4 +++- .../packages/create_maven_package_service.rb | 21 ++++++++----------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/ee/app/models/packages/package.rb b/ee/app/models/packages/package.rb index 55dca0fd10ff9c..06ad48b8cf599d 100644 --- a/ee/app/models/packages/package.rb +++ b/ee/app/models/packages/package.rb @@ -2,7 +2,9 @@ class Packages::Package < ActiveRecord::Base belongs_to :project has_many :package_files - has_one :maven_metadatum + has_one :maven_metadatum, inverse_of: :package + + accepts_nested_attributes_for :maven_metadatum validates :project, presence: true validates :name, presence: true diff --git a/ee/app/services/packages/create_maven_package_service.rb b/ee/app/services/packages/create_maven_package_service.rb index 3697559f486ec3..672e628ca5ecad 100644 --- a/ee/app/services/packages/create_maven_package_service.rb +++ b/ee/app/services/packages/create_maven_package_service.rb @@ -2,22 +2,19 @@ module Packages class CreateMavenPackageService < BaseService def execute - package = project.packages.create!( - name: params[:name], - version: params[:version] - ) - app_group, _, app_name = params[:name].rpartition('/') app_group.tr!('/', '.') - package.create_maven_metadatum!( - path: params[:path], - app_group: app_group, - app_name: app_name, - app_version: params[:version] + project.packages.create!( + name: params[:name], + version: params[:version], + maven_metadatum_attributes: { + path: params[:path], + app_group: app_group, + app_name: app_name, + app_version: params[:version] + } ) - - package end end end -- GitLab From de0e4deca4e1d00c6ec3a5865aeb730f9c2b3bb2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sat, 11 Aug 2018 13:39:29 +0300 Subject: [PATCH 40/55] Refactor package creation in maven packages API Signed-off-by: Dmitriy Zaporozhets --- .../find_or_create_maven_package_service.rb | 38 +++++++++++++++++++ ee/lib/api/maven_packages.rb | 32 +--------------- 2 files changed, 40 insertions(+), 30 deletions(-) create mode 100644 ee/app/services/packages/find_or_create_maven_package_service.rb diff --git a/ee/app/services/packages/find_or_create_maven_package_service.rb b/ee/app/services/packages/find_or_create_maven_package_service.rb new file mode 100644 index 00000000000000..141f31a2169030 --- /dev/null +++ b/ee/app/services/packages/find_or_create_maven_package_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true +module Packages + class FindOrCreateMavenPackageService < BaseService + MAVEN_METADATA_FILE = 'maven-metadata.xml'.freeze + + def execute + package = ::Packages::MavenPackageFinder + .new(project, params[:path]).execute + + unless package + if params[:file_name] == MAVEN_METADATA_FILE + # Maven uploads several files during `mvn deploy` in next order: + # - my-company/my-app/1.0-SNAPSHOT/my-app.jar + # - my-company/my-app/1.0-SNAPSHOT/my-app.pom + # - my-company/my-app/1.0-SNAPSHOT/maven-metadata.xml + # - my-company/my-app/maven-metadata.xml + # + # The last xml file does not have VERSION in URL because it contains + # information about all versions. + package_name, version = params[:path], nil + else + package_name, _, version = params[:path].rpartition('/') + end + + package_params = { + name: package_name, + path: params[:path], + version: version + } + + package = ::Packages::CreateMavenPackageService + .new(project, current_user, package_params).execute + end + + package + end + end +end diff --git a/ee/lib/api/maven_packages.rb b/ee/lib/api/maven_packages.rb index 127093ea4f6fc7..951e52a62920bc 100644 --- a/ee/lib/api/maven_packages.rb +++ b/ee/lib/api/maven_packages.rb @@ -5,8 +5,6 @@ class MavenPackages < Grape::API file_name: API::NO_SLASH_URL_PART_REGEX }.freeze - MAVEN_METADATA_FILE = 'maven-metadata.xml'.freeze - content_type :md5, 'text/plain' content_type :sha1, 'text/plain' content_type :binary, 'application/octet-stream' @@ -122,7 +120,6 @@ def verify_package_file(package_file, uploaded_file) end put ':id/packages/maven/*path/:file_name', requirements: MAVEN_ENDPOINT_REQUIREMENTS do authorize_create_package! - require_gitlab_workhorse! file_name, format = extract_format(params[:file_name]) @@ -130,33 +127,8 @@ def verify_package_file(package_file, uploaded_file) uploaded_file = UploadedFile.from_params(params, :file, ::Packages::PackageFileUploader.workhorse_local_upload_path) bad_request!('Missing package file!') unless uploaded_file - package = ::Packages::MavenPackageFinder - .new(user_project, params[:path]).execute - - unless package - if file_name == MAVEN_METADATA_FILE - # Maven uploads several files during `mvn deploy` in next order: - # - my-company/my-app/1.0-SNAPSHOT/my-app.jar - # - my-company/my-app/1.0-SNAPSHOT/my-app.pom - # - my-company/my-app/1.0-SNAPSHOT/maven-metadata.xml - # - my-company/my-app/maven-metadata.xml - # - # The last xml file does not have VERSION in URL because it contains - # information about all versions. - package_name, version = params[:path], nil - else - package_name, _, version = params[:path].rpartition('/') - end - - package_params = { - name: package_name, - path: params[:path], - version: version - } - - package = ::Packages::CreateMavenPackageService - .new(user_project, current_user, package_params).execute - end + package = ::Packages::FindOrCreateMavenPackageService + .new(user_project, current_user, params).execute case format when 'sha1' -- GitLab From bc6cabf91befefbd98c0b08fef8692d81307f1c4 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 12 Aug 2018 18:58:56 +0300 Subject: [PATCH 41/55] Add missing add_foreign_key for packages Signed-off-by: Dmitriy Zaporozhets --- db/schema.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/db/schema.rb b/db/schema.rb index fc7e1c59026ef9..e64bd02138ffe9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -3123,6 +3123,7 @@ add_foreign_key "oauth_openid_requests", "oauth_access_grants", column: "access_grant_id", name: "fk_oauth_openid_requests_oauth_access_grants_access_grant_id" add_foreign_key "packages_maven_metadata", "packages_packages", column: "package_id", name: "fk_be88aed360", on_delete: :cascade add_foreign_key "packages_package_files", "packages_packages", column: "package_id", name: "fk_86f0f182f8", on_delete: :cascade + add_foreign_key "packages_packages", "projects", on_delete: :cascade add_foreign_key "pages_domains", "projects", name: "fk_ea2f6dfc6f", on_delete: :cascade add_foreign_key "path_locks", "projects", name: "fk_5265c98f24", on_delete: :cascade add_foreign_key "path_locks", "users" -- GitLab From b226460737b7448f2611f82bba30ffd84d875e67 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 13 Aug 2018 16:48:21 +0300 Subject: [PATCH 42/55] Update packages db schema after db review Signed-off-by: Dmitriy Zaporozhets --- db/schema.rb | 12 ++++++------ .../20180720120716_create_packages_packages.rb | 2 +- .../20180720120726_create_packages_package_files.rb | 10 +++++----- .../20180720121404_create_packages_maven_metadata.rb | 5 +++-- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index e64bd02138ffe9..966964ac99510e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1921,36 +1921,36 @@ t.string "nonce", null: false end - create_table "packages_maven_metadata", force: :cascade do |t| + create_table "packages_maven_metadata", id: :bigserial, force: :cascade do |t| t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false t.integer "package_id", null: false - t.string "path", null: false t.string "app_group", null: false t.string "app_name", null: false t.string "app_version" + t.text "path", null: false end add_index "packages_maven_metadata", ["package_id"], name: "index_packages_maven_metadata_on_package_id", using: :btree add_index "packages_maven_metadata", ["path"], name: "index_packages_maven_metadata_on_path", using: :btree - create_table "packages_package_files", force: :cascade do |t| + create_table "packages_package_files", id: :bigserial, force: :cascade do |t| t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false + t.integer "size", limit: 8 t.integer "package_id", null: false t.integer "file_type" t.integer "file_store" - t.integer "size" t.binary "file_md5" t.binary "file_sha1" - t.string "file" t.string "file_name", null: false + t.text "file", null: false end add_index "packages_package_files", ["package_id", "file_name"], name: "index_packages_package_files_on_package_id_and_file_name", using: :btree add_index "packages_package_files", ["package_id"], name: "index_packages_package_files_on_package_id", using: :btree - create_table "packages_packages", force: :cascade do |t| + create_table "packages_packages", id: :bigserial, force: :cascade do |t| t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false t.integer "project_id", null: false diff --git a/ee/db/migrate/20180720120716_create_packages_packages.rb b/ee/db/migrate/20180720120716_create_packages_packages.rb index 01f229a843e38a..72b98b3dd2a204 100644 --- a/ee/db/migrate/20180720120716_create_packages_packages.rb +++ b/ee/db/migrate/20180720120716_create_packages_packages.rb @@ -7,7 +7,7 @@ class CreatePackagesPackages < ActiveRecord::Migration disable_ddl_transaction! def change - create_table :packages_packages do |t| + create_table :packages_packages, id: :bigserial do |t| t.timestamps_with_timezone null: false t.references :project, index: true, foreign_key: { on_delete: :cascade }, null: false diff --git a/ee/db/migrate/20180720120726_create_packages_package_files.rb b/ee/db/migrate/20180720120726_create_packages_package_files.rb index d6e3292e334d90..558ca487521181 100644 --- a/ee/db/migrate/20180720120726_create_packages_package_files.rb +++ b/ee/db/migrate/20180720120726_create_packages_package_files.rb @@ -7,19 +7,19 @@ class CreatePackagesPackageFiles < ActiveRecord::Migration disable_ddl_transaction! def up - create_table :packages_package_files do |t| + create_table :packages_package_files, id: :bigserial do |t| t.timestamps_with_timezone null: false - t.references :package, index: true, null: false + t.bigint :size + + t.references :package, index: true, null: false t.integer :file_type t.integer :file_store - t.integer :size - t.binary :file_md5 t.binary :file_sha1 - t.string :file t.string :file_name, null: false + t.text :file, null: false end add_concurrent_foreign_key :packages_package_files, :packages_packages, diff --git a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb index 45fb03dfab35d1..d174c17fe6d34d 100644 --- a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb +++ b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb @@ -7,14 +7,15 @@ class CreatePackagesMavenMetadata < ActiveRecord::Migration disable_ddl_transaction! def up - create_table :packages_maven_metadata do |t| + create_table :packages_maven_metadata, id: :bigserial do |t| t.timestamps_with_timezone null: false t.references :package, index: true, null: false - t.string :path, null: false t.string :app_group, null: false t.string :app_name, null: false t.string :app_version + + t.text :path, null: false end add_concurrent_foreign_key :packages_maven_metadata, :packages_packages, -- GitLab From 292495368a2ba33b0dd10b687e637eccb9e2db94 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 13 Aug 2018 18:38:19 +0300 Subject: [PATCH 43/55] Fix package indices migration for MySQL Signed-off-by: Dmitriy Zaporozhets --- .../20180810155213_add_more_indices_to_packages.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ee/db/migrate/20180810155213_add_more_indices_to_packages.rb b/ee/db/migrate/20180810155213_add_more_indices_to_packages.rb index b7406b8bfd830f..ccf783c58abd2b 100644 --- a/ee/db/migrate/20180810155213_add_more_indices_to_packages.rb +++ b/ee/db/migrate/20180810155213_add_more_indices_to_packages.rb @@ -7,11 +7,17 @@ class AddMoreIndicesToPackages < ActiveRecord::Migration def up add_concurrent_index :packages_package_files, [:package_id, :file_name] - add_concurrent_index :packages_maven_metadata, :path + add_concurrent_index :packages_maven_metadata, :path, length: text_index_length end def down remove_concurrent_index :packages_package_files, [:package_id, :file_name] - remove_concurrent_index :packages_maven_metadata, :path + remove_concurrent_index :packages_maven_metadata, :path, length: text_index_length + end + + private + + def text_index_length + Gitlab::Database.mysql? ? 20 : nil end end -- GitLab From c77e4bbc41606d6d58abe17502ec0a12689b3c77 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 14 Aug 2018 13:57:51 +0300 Subject: [PATCH 44/55] Refactor packages migrations to make it work with both PG and MySQL [ci skip] Signed-off-by: Dmitriy Zaporozhets --- ... mysql_set_length_for_binary_and_text_indexes.rb} | 12 ++++++------ .../20180720120716_create_packages_packages.rb | 6 +++++- .../20180720120726_create_packages_package_files.rb | 4 ++-- .../20180720121404_create_packages_maven_metadata.rb | 3 ++- 4 files changed, 15 insertions(+), 10 deletions(-) rename config/initializers/{mysql_set_length_for_binary_indexes.rb => mysql_set_length_for_binary_and_text_indexes.rb} (57%) diff --git a/config/initializers/mysql_set_length_for_binary_indexes.rb b/config/initializers/mysql_set_length_for_binary_and_text_indexes.rb similarity index 57% rename from config/initializers/mysql_set_length_for_binary_indexes.rb rename to config/initializers/mysql_set_length_for_binary_and_text_indexes.rb index de0bc5322aac57..21359ab0bd4b6a 100644 --- a/config/initializers/mysql_set_length_for_binary_indexes.rb +++ b/config/initializers/mysql_set_length_for_binary_and_text_indexes.rb @@ -1,13 +1,13 @@ -# This patches ActiveRecord so indexes for binary columns created using the -# MySQL adapter apply a length of 20. Otherwise MySQL can't create an index on -# binary columns. +# This patches ActiveRecord so indexes for binary and text columns created +# using the MySQL adapter apply a length of 20. Otherwise MySQL can't create an +# index on binary and text columns. -module MysqlSetLengthForBinaryIndex +module MysqlSetLengthForBinaryAndTextIndex def add_index(table_name, column_names, options = {}) Array(column_names).each do |column_name| column = ActiveRecord::Base.connection.columns(table_name).find { |c| c.name == column_name } - if column&.type == :binary + if column&.type == :binary || column&.type == :text options[:length] = 20 end end @@ -17,5 +17,5 @@ def add_index(table_name, column_names, options = {}) end if defined?(ActiveRecord::ConnectionAdapters::Mysql2Adapter) - ActiveRecord::ConnectionAdapters::Mysql2Adapter.send(:prepend, MysqlSetLengthForBinaryIndex) + ActiveRecord::ConnectionAdapters::Mysql2Adapter.send(:prepend, MysqlSetLengthForBinaryAndTextIndex) end diff --git a/ee/db/migrate/20180720120716_create_packages_packages.rb b/ee/db/migrate/20180720120716_create_packages_packages.rb index 72b98b3dd2a204..847e5f86a249f7 100644 --- a/ee/db/migrate/20180720120716_create_packages_packages.rb +++ b/ee/db/migrate/20180720120716_create_packages_packages.rb @@ -8,8 +8,12 @@ class CreatePackagesPackages < ActiveRecord::Migration def change create_table :packages_packages, id: :bigserial do |t| + t.references :project, + index: true, + foreign_key: { on_delete: :cascade }, + null: false + t.timestamps_with_timezone null: false - t.references :project, index: true, foreign_key: { on_delete: :cascade }, null: false t.string :name, null: false t.string :version diff --git a/ee/db/migrate/20180720120726_create_packages_package_files.rb b/ee/db/migrate/20180720120726_create_packages_package_files.rb index 558ca487521181..20b01488de325e 100644 --- a/ee/db/migrate/20180720120726_create_packages_package_files.rb +++ b/ee/db/migrate/20180720120726_create_packages_package_files.rb @@ -8,11 +8,11 @@ class CreatePackagesPackageFiles < ActiveRecord::Migration def up create_table :packages_package_files, id: :bigserial do |t| + t.references :package, type: :bigint, index: true, null: false + t.timestamps_with_timezone null: false t.bigint :size - - t.references :package, index: true, null: false t.integer :file_type t.integer :file_store t.binary :file_md5 diff --git a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb index d174c17fe6d34d..f4ae91d7982543 100644 --- a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb +++ b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb @@ -8,8 +8,9 @@ class CreatePackagesMavenMetadata < ActiveRecord::Migration def up create_table :packages_maven_metadata, id: :bigserial do |t| + t.references :package, type: :bigint, index: true, null: false + t.timestamps_with_timezone null: false - t.references :package, index: true, null: false t.string :app_group, null: false t.string :app_name, null: false -- GitLab From 2bb221f0efe9068a4c08efa1a8ccd69fd22ab745 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 14 Aug 2018 14:03:16 +0300 Subject: [PATCH 45/55] Rebuild db/schema with changes in pockages migrations Signed-off-by: Dmitriy Zaporozhets --- db/schema.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 966964ac99510e..2a48d218eaf4a4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1922,9 +1922,9 @@ end create_table "packages_maven_metadata", id: :bigserial, force: :cascade do |t| + t.integer "package_id", limit: 8, null: false t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false - t.integer "package_id", null: false t.string "app_group", null: false t.string "app_name", null: false t.string "app_version" @@ -1935,10 +1935,10 @@ add_index "packages_maven_metadata", ["path"], name: "index_packages_maven_metadata_on_path", using: :btree create_table "packages_package_files", id: :bigserial, force: :cascade do |t| + t.integer "package_id", limit: 8, null: false t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false t.integer "size", limit: 8 - t.integer "package_id", null: false t.integer "file_type" t.integer "file_store" t.binary "file_md5" @@ -1951,9 +1951,9 @@ add_index "packages_package_files", ["package_id"], name: "index_packages_package_files_on_package_id", using: :btree create_table "packages_packages", id: :bigserial, force: :cascade do |t| + t.integer "project_id", null: false t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false - t.integer "project_id", null: false t.string "name", null: false t.string "version" end -- GitLab From c94b0a461368195e763a0b9fb5b5e4aefb17f6cb Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 14 Aug 2018 17:20:39 +0300 Subject: [PATCH 46/55] Revert changes to mysql initializer Signed-off-by: Dmitriy Zaporozhets --- ...dexes.rb => mysql_set_length_for_binary_indexes.rb} | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) rename config/initializers/{mysql_set_length_for_binary_and_text_indexes.rb => mysql_set_length_for_binary_indexes.rb} (67%) diff --git a/config/initializers/mysql_set_length_for_binary_and_text_indexes.rb b/config/initializers/mysql_set_length_for_binary_indexes.rb similarity index 67% rename from config/initializers/mysql_set_length_for_binary_and_text_indexes.rb rename to config/initializers/mysql_set_length_for_binary_indexes.rb index 21359ab0bd4b6a..fca8be89ce8f6d 100644 --- a/config/initializers/mysql_set_length_for_binary_and_text_indexes.rb +++ b/config/initializers/mysql_set_length_for_binary_indexes.rb @@ -1,13 +1,13 @@ -# This patches ActiveRecord so indexes for binary and text columns created +# This patches ActiveRecord so indexes for binary columns created # using the MySQL adapter apply a length of 20. Otherwise MySQL can't create an -# index on binary and text columns. +# index on binary columns. -module MysqlSetLengthForBinaryAndTextIndex +module MysqlSetLengthForBinaryIndex def add_index(table_name, column_names, options = {}) Array(column_names).each do |column_name| column = ActiveRecord::Base.connection.columns(table_name).find { |c| c.name == column_name } - if column&.type == :binary || column&.type == :text + if column&.type == :binary options[:length] = 20 end end @@ -17,5 +17,5 @@ def add_index(table_name, column_names, options = {}) end if defined?(ActiveRecord::ConnectionAdapters::Mysql2Adapter) - ActiveRecord::ConnectionAdapters::Mysql2Adapter.send(:prepend, MysqlSetLengthForBinaryAndTextIndex) + ActiveRecord::ConnectionAdapters::Mysql2Adapter.send(:prepend, MysqlSetLengthForBinaryIndex) end -- GitLab From 3604fd38631cb3a0278bfed785a12a4f9524c5eb Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 14 Aug 2018 18:12:07 +0300 Subject: [PATCH 47/55] Improve migrations for maven packages feature Signed-off-by: Dmitriy Zaporozhets --- db/schema.rb | 10 ++++------ .../20180720120726_create_packages_package_files.rb | 2 +- .../20180720121404_create_packages_maven_metadata.rb | 5 ++--- .../20180810155213_add_more_indices_to_packages.rb | 10 ++-------- 4 files changed, 9 insertions(+), 18 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 2a48d218eaf4a4..5cd039a6a95697 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1922,23 +1922,22 @@ end create_table "packages_maven_metadata", id: :bigserial, force: :cascade do |t| - t.integer "package_id", limit: 8, null: false t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false + t.integer "package_id", limit: 8, null: false t.string "app_group", null: false t.string "app_name", null: false t.string "app_version" t.text "path", null: false end - add_index "packages_maven_metadata", ["package_id"], name: "index_packages_maven_metadata_on_package_id", using: :btree - add_index "packages_maven_metadata", ["path"], name: "index_packages_maven_metadata_on_path", using: :btree + add_index "packages_maven_metadata", ["package_id", "path"], name: "index_packages_maven_metadata_on_package_id_and_path", using: :btree create_table "packages_package_files", id: :bigserial, force: :cascade do |t| - t.integer "package_id", limit: 8, null: false t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false t.integer "size", limit: 8 + t.integer "package_id", limit: 8, null: false t.integer "file_type" t.integer "file_store" t.binary "file_md5" @@ -1948,12 +1947,11 @@ end add_index "packages_package_files", ["package_id", "file_name"], name: "index_packages_package_files_on_package_id_and_file_name", using: :btree - add_index "packages_package_files", ["package_id"], name: "index_packages_package_files_on_package_id", using: :btree create_table "packages_packages", id: :bigserial, force: :cascade do |t| - t.integer "project_id", null: false t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false + t.integer "project_id", null: false t.string "name", null: false t.string "version" end diff --git a/ee/db/migrate/20180720120726_create_packages_package_files.rb b/ee/db/migrate/20180720120726_create_packages_package_files.rb index 20b01488de325e..28bfc90cab8337 100644 --- a/ee/db/migrate/20180720120726_create_packages_package_files.rb +++ b/ee/db/migrate/20180720120726_create_packages_package_files.rb @@ -8,7 +8,7 @@ class CreatePackagesPackageFiles < ActiveRecord::Migration def up create_table :packages_package_files, id: :bigserial do |t| - t.references :package, type: :bigint, index: true, null: false + t.references :package, type: :bigint, null: false t.timestamps_with_timezone null: false diff --git a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb index f4ae91d7982543..8ef677b960938c 100644 --- a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb +++ b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb @@ -8,15 +8,14 @@ class CreatePackagesMavenMetadata < ActiveRecord::Migration def up create_table :packages_maven_metadata, id: :bigserial do |t| - t.references :package, type: :bigint, index: true, null: false + t.references :package, type: :bigint, null: false t.timestamps_with_timezone null: false t.string :app_group, null: false t.string :app_name, null: false t.string :app_version - - t.text :path, null: false + t.string :path, limit: 1024, null: false end add_concurrent_foreign_key :packages_maven_metadata, :packages_packages, diff --git a/ee/db/migrate/20180810155213_add_more_indices_to_packages.rb b/ee/db/migrate/20180810155213_add_more_indices_to_packages.rb index ccf783c58abd2b..0c6bda7ccd6956 100644 --- a/ee/db/migrate/20180810155213_add_more_indices_to_packages.rb +++ b/ee/db/migrate/20180810155213_add_more_indices_to_packages.rb @@ -7,17 +7,11 @@ class AddMoreIndicesToPackages < ActiveRecord::Migration def up add_concurrent_index :packages_package_files, [:package_id, :file_name] - add_concurrent_index :packages_maven_metadata, :path, length: text_index_length + add_concurrent_index :packages_maven_metadata, [:package_id, :path] end def down remove_concurrent_index :packages_package_files, [:package_id, :file_name] - remove_concurrent_index :packages_maven_metadata, :path, length: text_index_length - end - - private - - def text_index_length - Gitlab::Database.mysql? ? 20 : nil + remove_concurrent_index :packages_maven_metadata, [:package_id, :path] end end -- GitLab From b71743c45c9b1815c2d341f4fcd6e768031aa26a Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 14 Aug 2018 17:12:38 +0300 Subject: [PATCH 48/55] Rename admin_package policy to create_package Signed-off-by: Dmitriy Zaporozhets --- ee/app/policies/ee/project_policy.rb | 2 +- ee/lib/api/maven_packages.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 5fed07ce4c95f2..8d20b973e1095b 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -95,7 +95,7 @@ module ProjectPolicy rule { can?(:developer_access) }.policy do enable :admin_board enable :admin_vulnerability_feedback - enable :admin_package + enable :create_package end rule { can?(:public_access) }.enable :read_package diff --git a/ee/lib/api/maven_packages.rb b/ee/lib/api/maven_packages.rb index 951e52a62920bc..7a66e860c5d6f8 100644 --- a/ee/lib/api/maven_packages.rb +++ b/ee/lib/api/maven_packages.rb @@ -29,7 +29,7 @@ def authorize_download_package! end def authorize_create_package! - authorize!(:admin_package, user_project) + authorize!(:create_package, user_project) end def extract_format(file_name) -- GitLab From ae7687f83befc349972643f72de220f92bbda250 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 14 Aug 2018 20:19:32 +0300 Subject: [PATCH 49/55] Fix maven packages path type in db/schema.rb Signed-off-by: Dmitriy Zaporozhets --- db/schema.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index 5cd039a6a95697..d20bb91c565625 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1928,7 +1928,7 @@ t.string "app_group", null: false t.string "app_name", null: false t.string "app_version" - t.text "path", null: false + t.string "path", limit: 1024, null: false end add_index "packages_maven_metadata", ["package_id", "path"], name: "index_packages_maven_metadata_on_package_id_and_path", using: :btree -- GitLab From 628741763aab49a8b4cbc66b49da1982e3a29d61 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 14 Aug 2018 20:31:41 +0300 Subject: [PATCH 50/55] Add format validation to package models Signed-off-by: Dmitriy Zaporozhets --- ee/app/models/packages/maven_metadatum.rb | 15 ++++++++++++--- ee/app/models/packages/package.rb | 5 ++++- ee/lib/ee/gitlab/regex.rb | 16 ++++++++++++++++ .../models/packages/maven_metadatum_spec.rb | 18 ++++++++++++++++++ ee/spec/models/packages/package_spec.rb | 6 ++++++ 5 files changed, 56 insertions(+), 4 deletions(-) diff --git a/ee/app/models/packages/maven_metadatum.rb b/ee/app/models/packages/maven_metadatum.rb index 4b6a95377d150e..e1f9d064987f1a 100644 --- a/ee/app/models/packages/maven_metadatum.rb +++ b/ee/app/models/packages/maven_metadatum.rb @@ -3,7 +3,16 @@ class Packages::MavenMetadatum < ActiveRecord::Base belongs_to :package validates :package, presence: true - validates :path, presence: true - validates :app_group, presence: true - validates :app_name, presence: true + + validates :path, + presence: true, + format: { with: Gitlab::Regex.maven_path_regex } + + validates :app_group, + presence: true, + format: { with: Gitlab::Regex.maven_app_group_regex } + + validates :app_name, + presence: true, + format: { with: Gitlab::Regex.maven_app_name_regex } end diff --git a/ee/app/models/packages/package.rb b/ee/app/models/packages/package.rb index 06ad48b8cf599d..b593f9546eb4fb 100644 --- a/ee/app/models/packages/package.rb +++ b/ee/app/models/packages/package.rb @@ -7,5 +7,8 @@ class Packages::Package < ActiveRecord::Base accepts_nested_attributes_for :maven_metadatum validates :project, presence: true - validates :name, presence: true + + validates :name, + presence: true, + format: { with: Gitlab::Regex.package_name_regex } end diff --git a/ee/lib/ee/gitlab/regex.rb b/ee/lib/ee/gitlab/regex.rb index 6747bf7ebe59fb..da37ee6015f68d 100644 --- a/ee/lib/ee/gitlab/regex.rb +++ b/ee/lib/ee/gitlab/regex.rb @@ -12,6 +12,22 @@ def environment_scope_regex def environment_scope_regex_message "can contain only letters, digits, '-', '_', '/', '$', '{', '}', '.', '*' and spaces" end + + def package_name_regex + @package_name_regex ||= /\A(([\w\-\.]*)\/)*([\w\-\.]*)\z/.freeze + end + + def maven_path_regex + package_name_regex + end + + def maven_app_name_regex + @maven_app_name_regex ||= /\A[\w\-\.]+\z/.freeze + end + + def maven_app_group_regex + maven_app_name_regex + end end end end diff --git a/ee/spec/models/packages/maven_metadatum_spec.rb b/ee/spec/models/packages/maven_metadatum_spec.rb index 99501f3bd875f1..c169a94beaa673 100644 --- a/ee/spec/models/packages/maven_metadatum_spec.rb +++ b/ee/spec/models/packages/maven_metadatum_spec.rb @@ -8,5 +8,23 @@ describe 'validations' do it { is_expected.to validate_presence_of(:package) } + + describe '#app_name' do + it { is_expected.to allow_value("my-app").for(:app_name) } + it { is_expected.to_not allow_value("my/app").for(:app_name) } + it { is_expected.to_not allow_value("my(app)").for(:app_name) } + end + + describe '#app_group' do + it { is_expected.to allow_value("my.domain.com").for(:app_group) } + it { is_expected.to_not allow_value("my/domain/com").for(:app_group) } + it { is_expected.to_not allow_value("my(domain)").for(:app_group) } + end + + describe '#path' do + it { is_expected.to allow_value("my/domain/com/my-app").for(:path) } + it { is_expected.to allow_value("my/domain/com/my-app/1.0-SNAPSHOT").for(:path) } + it { is_expected.to_not allow_value("my(domain)com.my-app").for(:path) } + end end end diff --git a/ee/spec/models/packages/package_spec.rb b/ee/spec/models/packages/package_spec.rb index 258a2dcfd6e208..23f58a55210a81 100644 --- a/ee/spec/models/packages/package_spec.rb +++ b/ee/spec/models/packages/package_spec.rb @@ -9,5 +9,11 @@ describe 'validations' do it { is_expected.to validate_presence_of(:project) } + + describe '#name' do + it { is_expected.to allow_value("my/domain/com/my-app").for(:name) } + it { is_expected.to allow_value("my.app-11.07.2018").for(:name) } + it { is_expected.to_not allow_value("my(dom$$$ain)com.my-app").for(:name) } + end end end -- GitLab From af4a4ef0f5224ac508be6eecb31c89db5a9e8c4d Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 15 Aug 2018 11:51:20 +0300 Subject: [PATCH 51/55] Reduce path to 512 chars to make id+path index to work with mysql Signed-off-by: Dmitriy Zaporozhets --- db/schema.rb | 2 +- ee/db/migrate/20180720121404_create_packages_maven_metadata.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index d20bb91c565625..65eec3c3fe3514 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1928,7 +1928,7 @@ t.string "app_group", null: false t.string "app_name", null: false t.string "app_version" - t.string "path", limit: 1024, null: false + t.string "path", limit: 512, null: false end add_index "packages_maven_metadata", ["package_id", "path"], name: "index_packages_maven_metadata_on_package_id_and_path", using: :btree diff --git a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb index 8ef677b960938c..7d422d951453f7 100644 --- a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb +++ b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb @@ -15,7 +15,7 @@ def up t.string :app_group, null: false t.string :app_name, null: false t.string :app_version - t.string :path, limit: 1024, null: false + t.string :path, limit: 512, null: false end add_concurrent_foreign_key :packages_maven_metadata, :packages_packages, -- GitLab From 9b9123626f6599261c45349215bd7a57f3a0c119 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 15 Aug 2018 12:25:31 +0300 Subject: [PATCH 52/55] Fix spec not_to syntax for package models Signed-off-by: Dmitriy Zaporozhets --- ee/spec/models/packages/maven_metadatum_spec.rb | 10 +++++----- ee/spec/models/packages/package_spec.rb | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ee/spec/models/packages/maven_metadatum_spec.rb b/ee/spec/models/packages/maven_metadatum_spec.rb index c169a94beaa673..b7c60bdd16084e 100644 --- a/ee/spec/models/packages/maven_metadatum_spec.rb +++ b/ee/spec/models/packages/maven_metadatum_spec.rb @@ -11,20 +11,20 @@ describe '#app_name' do it { is_expected.to allow_value("my-app").for(:app_name) } - it { is_expected.to_not allow_value("my/app").for(:app_name) } - it { is_expected.to_not allow_value("my(app)").for(:app_name) } + it { is_expected.not_to allow_value("my/app").for(:app_name) } + it { is_expected.not_to allow_value("my(app)").for(:app_name) } end describe '#app_group' do it { is_expected.to allow_value("my.domain.com").for(:app_group) } - it { is_expected.to_not allow_value("my/domain/com").for(:app_group) } - it { is_expected.to_not allow_value("my(domain)").for(:app_group) } + it { is_expected.not_to allow_value("my/domain/com").for(:app_group) } + it { is_expected.not_to allow_value("my(domain)").for(:app_group) } end describe '#path' do it { is_expected.to allow_value("my/domain/com/my-app").for(:path) } it { is_expected.to allow_value("my/domain/com/my-app/1.0-SNAPSHOT").for(:path) } - it { is_expected.to_not allow_value("my(domain)com.my-app").for(:path) } + it { is_expected.not_to allow_value("my(domain)com.my-app").for(:path) } end end end diff --git a/ee/spec/models/packages/package_spec.rb b/ee/spec/models/packages/package_spec.rb index 23f58a55210a81..eb9ade80cb989c 100644 --- a/ee/spec/models/packages/package_spec.rb +++ b/ee/spec/models/packages/package_spec.rb @@ -13,7 +13,7 @@ describe '#name' do it { is_expected.to allow_value("my/domain/com/my-app").for(:name) } it { is_expected.to allow_value("my.app-11.07.2018").for(:name) } - it { is_expected.to_not allow_value("my(dom$$$ain)com.my-app").for(:name) } + it { is_expected.not_to allow_value("my(dom$$$ain)com.my-app").for(:name) } end end end -- GitLab From f71943659ff2cb7f754bd2f427d023b7ab8d4c70 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 15 Aug 2018 12:39:30 +0300 Subject: [PATCH 53/55] Refactor migrations in a way it can work with MySQL properly Signed-off-by: Dmitriy Zaporozhets --- db/schema.rb | 2 +- ...80720120726_create_packages_package_files.rb | 6 ++++++ ...0720121404_create_packages_maven_metadata.rb | 6 ++++++ ...180810155213_add_more_indices_to_packages.rb | 17 ----------------- 4 files changed, 13 insertions(+), 18 deletions(-) delete mode 100644 ee/db/migrate/20180810155213_add_more_indices_to_packages.rb diff --git a/db/schema.rb b/db/schema.rb index 65eec3c3fe3514..d4bf2cac8a1d44 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20180810155213) do +ActiveRecord::Schema.define(version: 20180807153545) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/ee/db/migrate/20180720120726_create_packages_package_files.rb b/ee/db/migrate/20180720120726_create_packages_package_files.rb index 28bfc90cab8337..307646a0ce2583 100644 --- a/ee/db/migrate/20180720120726_create_packages_package_files.rb +++ b/ee/db/migrate/20180720120726_create_packages_package_files.rb @@ -22,6 +22,8 @@ def up t.text :file, null: false end + add_concurrent_index :packages_package_files, [:package_id, :file_name] + add_concurrent_foreign_key :packages_package_files, :packages_packages, column: :package_id, on_delete: :cascade @@ -32,6 +34,10 @@ def down remove_foreign_key :packages_package_files, column: :package_id end + if index_exists?(:packages_package_files, [:package_id, :file_name]) + remove_concurrent_index :packages_package_files, [:package_id, :file_name] + end + if table_exists?(:packages_package_files) drop_table :packages_package_files end diff --git a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb index 7d422d951453f7..f41d02bb65ff24 100644 --- a/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb +++ b/ee/db/migrate/20180720121404_create_packages_maven_metadata.rb @@ -18,6 +18,8 @@ def up t.string :path, limit: 512, null: false end + add_concurrent_index :packages_maven_metadata, [:package_id, :path] + add_concurrent_foreign_key :packages_maven_metadata, :packages_packages, column: :package_id, on_delete: :cascade @@ -28,6 +30,10 @@ def down remove_foreign_key :packages_maven_metadata, column: :package_id end + if index_exists?(:packages_maven_metadata, [:package_id, :path]) + remove_concurrent_index :packages_maven_metadata, [:package_id, :path] + end + if table_exists?(:packages_maven_metadata) drop_table :packages_maven_metadata end diff --git a/ee/db/migrate/20180810155213_add_more_indices_to_packages.rb b/ee/db/migrate/20180810155213_add_more_indices_to_packages.rb deleted file mode 100644 index 0c6bda7ccd6956..00000000000000 --- a/ee/db/migrate/20180810155213_add_more_indices_to_packages.rb +++ /dev/null @@ -1,17 +0,0 @@ -class AddMoreIndicesToPackages < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - disable_ddl_transaction! - - def up - add_concurrent_index :packages_package_files, [:package_id, :file_name] - add_concurrent_index :packages_maven_metadata, [:package_id, :path] - end - - def down - remove_concurrent_index :packages_package_files, [:package_id, :file_name] - remove_concurrent_index :packages_maven_metadata, [:package_id, :path] - end -end -- GitLab From 1efbbeb24a2f0a154e206bcf4e98fb8cf5b4143f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 15 Aug 2018 12:43:20 +0300 Subject: [PATCH 54/55] Fix rubocop complain around package_name_regex Signed-off-by: Dmitriy Zaporozhets --- ee/lib/ee/gitlab/regex.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/lib/ee/gitlab/regex.rb b/ee/lib/ee/gitlab/regex.rb index da37ee6015f68d..82e081bfac44d0 100644 --- a/ee/lib/ee/gitlab/regex.rb +++ b/ee/lib/ee/gitlab/regex.rb @@ -14,7 +14,7 @@ def environment_scope_regex_message end def package_name_regex - @package_name_regex ||= /\A(([\w\-\.]*)\/)*([\w\-\.]*)\z/.freeze + @package_name_regex ||= %r{\A(([\w\-\.]*)/)*([\w\-\.]*)\z}.freeze end def maven_path_regex -- GitLab From 3b0f7851c14045dbbc760aec3d432cbdc4895e67 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 15 Aug 2018 15:33:52 +0300 Subject: [PATCH 55/55] Update schema.rb to represent column order from packages migrations Signed-off-by: Dmitriy Zaporozhets --- db/schema.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index d4bf2cac8a1d44..e2d31702a0a62b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1922,9 +1922,9 @@ end create_table "packages_maven_metadata", id: :bigserial, force: :cascade do |t| + t.integer "package_id", limit: 8, null: false t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false - t.integer "package_id", limit: 8, null: false t.string "app_group", null: false t.string "app_name", null: false t.string "app_version" @@ -1934,10 +1934,10 @@ add_index "packages_maven_metadata", ["package_id", "path"], name: "index_packages_maven_metadata_on_package_id_and_path", using: :btree create_table "packages_package_files", id: :bigserial, force: :cascade do |t| + t.integer "package_id", limit: 8, null: false t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false t.integer "size", limit: 8 - t.integer "package_id", limit: 8, null: false t.integer "file_type" t.integer "file_store" t.binary "file_md5" @@ -1949,9 +1949,9 @@ add_index "packages_package_files", ["package_id", "file_name"], name: "index_packages_package_files_on_package_id_and_file_name", using: :btree create_table "packages_packages", id: :bigserial, force: :cascade do |t| + t.integer "project_id", null: false t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "updated_at", null: false - t.integer "project_id", null: false t.string "name", null: false t.string "version" end -- GitLab