Commit 1cd0fcfb authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Merge branch '276882-use-package-settings-maven' into 'master'

Use package settings to validate Maven packages

See merge request gitlab-org/gitlab!50691
parents ed30646b a14eeb09
...@@ -4,9 +4,25 @@ class Namespace::PackageSetting < ApplicationRecord ...@@ -4,9 +4,25 @@ class Namespace::PackageSetting < ApplicationRecord
self.primary_key = :namespace_id self.primary_key = :namespace_id
self.table_name = 'namespace_package_settings' self.table_name = 'namespace_package_settings'
PackageSettingNotImplemented = Class.new(StandardError)
PACKAGES_WITH_SETTINGS = %w[maven].freeze
belongs_to :namespace, inverse_of: :package_setting_relation belongs_to :namespace, inverse_of: :package_setting_relation
validates :namespace, presence: true validates :namespace, presence: true
validates :maven_duplicates_allowed, inclusion: { in: [true, false] } validates :maven_duplicates_allowed, inclusion: { in: [true, false] }
validates :maven_duplicate_exception_regex, untrusted_regexp: true, length: { maximum: 255 } validates :maven_duplicate_exception_regex, untrusted_regexp: true, length: { maximum: 255 }
class << self
def duplicates_allowed?(package)
return true unless package
raise PackageSettingNotImplemented unless PACKAGES_WITH_SETTINGS.include?(package.package_type)
duplicates_allowed = package.package_settings["#{package.package_type}_duplicates_allowed"]
regex = ::Gitlab::UntrustedRegexp.new("\\A#{package.package_settings["#{package.package_type}_duplicate_exception_regex"]}\\z")
duplicates_allowed || regex.match?(package.name)
end
end
end end
...@@ -200,6 +200,12 @@ class Packages::Package < ApplicationRecord ...@@ -200,6 +200,12 @@ class Packages::Package < ApplicationRecord
debian? && !version.nil? debian? && !version.nil?
end end
def package_settings
strong_memoize(:package_settings) do
project.namespace.package_settings
end
end
private private
def composer_tag_version? def composer_tag_version?
......
...@@ -10,6 +10,10 @@ module Packages ...@@ -10,6 +10,10 @@ module Packages
::Packages::Maven::PackageFinder.new(params[:path], current_user, project: project) ::Packages::Maven::PackageFinder.new(params[:path], current_user, project: project)
.execute .execute
unless Namespace::PackageSetting.duplicates_allowed?(package)
return ServiceResponse.error(message: 'Duplicate package is not allowed')
end
unless package unless package
# Maven uploads several files during `mvn deploy` in next order: # 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.jar
...@@ -48,7 +52,7 @@ module Packages ...@@ -48,7 +52,7 @@ module Packages
package.build_infos.safe_find_or_create_by!(pipeline: params[:build].pipeline) if params[:build].present? package.build_infos.safe_find_or_create_by!(pipeline: params[:build].pipeline) if params[:build].present?
package ServiceResponse.success(payload: { package: package })
end end
end end
end end
......
---
title: Check namespace package settings when creating Maven packages
merge_request: 50691
author:
type: changed
...@@ -220,9 +220,13 @@ module API ...@@ -220,9 +220,13 @@ module API
file_name, format = extract_format(params[:file_name]) file_name, format = extract_format(params[:file_name])
package = ::Packages::Maven::FindOrCreatePackageService result = ::Packages::Maven::FindOrCreatePackageService
.new(user_project, current_user, params.merge(build: current_authenticated_job)).execute .new(user_project, current_user, params.merge(build: current_authenticated_job)).execute
bad_request!(result.errors.first) if result.error?
package = result.payload[:package]
case format case format
when 'sha1' when 'sha1'
# After uploading a file, Maven tries to upload a sha1 and md5 version of it. # After uploading a file, Maven tries to upload a sha1 and md5 version of it.
......
...@@ -33,4 +33,49 @@ RSpec.describe Namespace::PackageSetting do ...@@ -33,4 +33,49 @@ RSpec.describe Namespace::PackageSetting do
end end
end end
end end
describe '#duplicates_allowed?' do
using RSpec::Parameterized::TableSyntax
subject { described_class.duplicates_allowed?(package) }
context 'package types with package_settings' do
# As more package types gain settings they will be added to this list
[:maven_package].each do |format|
let_it_be(:package) { create(format) } # rubocop:disable Rails/SaveBang
let_it_be(:package_type) { package.package_type }
let_it_be(:package_setting) { package.project.namespace.package_settings }
where(:duplicates_allowed, :duplicate_exception_regex, :result) do
true | '' | true
false | '' | false
false | '.*' | true
end
with_them do
context "for #{format}" do
before do
package_setting.update!(
"#{package_type}_duplicates_allowed" => duplicates_allowed,
"#{package_type}_duplicate_exception_regex" => duplicate_exception_regex
)
end
it { is_expected.to be(result) }
end
end
end
end
context 'package types without package_settings' do
[:npm_package, :conan_package, :nuget_package, :pypi_package, :composer_package, :generic_package, :golang_package, :debian_package].each do |format|
let_it_be(:package) { create(format) } # rubocop:disable Rails/SaveBang
let_it_be(:package_setting) { package.project.namespace.package_settings }
it 'raises an error' do
expect { subject }.to raise_error(Namespace::PackageSetting::PackageSettingNotImplemented)
end
end
end
end
end end
...@@ -745,4 +745,14 @@ RSpec.describe Packages::Package, type: :model do ...@@ -745,4 +745,14 @@ RSpec.describe Packages::Package, type: :model do
end end
end end
end end
describe '#package_settings' do
let_it_be(:group) { create(:group) }
let_it_be(:project) { create(:project, group: group) }
let_it_be(:package) { create(:maven_package, project: project) }
it 'returns the namespace package_settings' do
expect(package.package_settings).to eq(group.package_settings)
end
end
end end
...@@ -4,7 +4,8 @@ require 'spec_helper' ...@@ -4,7 +4,8 @@ require 'spec_helper'
RSpec.describe API::MavenPackages do RSpec.describe API::MavenPackages do
include WorkhorseHelpers include WorkhorseHelpers
let_it_be(:group) { create(:group) } let_it_be_with_refind(:package_settings) { create(:namespace_package_setting, :group) }
let_it_be(:group) { package_settings.namespace }
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:project, reload: true) { create(:project, :public, namespace: group) } let_it_be(:project, reload: true) { create(:project, :public, namespace: group) }
let_it_be(:package, reload: true) { create(:maven_package, project: project, name: project.full_path) } let_it_be(:package, reload: true) { create(:maven_package, project: project, name: project.full_path) }
...@@ -18,6 +19,7 @@ RSpec.describe API::MavenPackages do ...@@ -18,6 +19,7 @@ RSpec.describe API::MavenPackages do
let_it_be(:deploy_token_for_group) { create(:deploy_token, :group, read_package_registry: true, write_package_registry: true) } let_it_be(:deploy_token_for_group) { create(:deploy_token, :group, read_package_registry: true, write_package_registry: true) }
let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: deploy_token_for_group, group: group) } let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: deploy_token_for_group, group: group) }
let(:package_name) { 'com/example/my-app' }
let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') } let(:workhorse_token) { JWT.encode({ 'iss' => 'gitlab-workhorse' }, Gitlab::Workhorse.secret, 'HS256') }
let(:headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } } let(:headers) { { 'GitLab-Workhorse' => '1.0', Gitlab::Workhorse::INTERNAL_API_REQUEST_HEADER => workhorse_token } }
let(:headers_with_token) { headers.merge('Private-Token' => personal_access_token.token) } let(:headers_with_token) { headers.merge('Private-Token' => personal_access_token.token) }
...@@ -669,6 +671,35 @@ RSpec.describe API::MavenPackages do ...@@ -669,6 +671,35 @@ RSpec.describe API::MavenPackages do
end end
end end
context 'when package duplicates are not allowed' do
let(:package_name) { package.name }
let(:version) { package.version }
before do
package_settings.update!(maven_duplicates_allowed: false)
end
it 'rejects the request', :aggregate_failures do
expect { upload_file_with_token(params: params) }.not_to change { package.package_files.count }
expect(response).to have_gitlab_http_status(:bad_request)
expect(json_response['message']).to include('Duplicate package is not allowed')
end
context 'when the package name matches the exception regex' do
before do
package_settings.update!(maven_duplicate_exception_regex: '.*')
end
it 'stores the package file', :aggregate_failures do
expect { upload_file_with_token(params: params) }.to change { package.package_files.count }.by(1)
expect(response).to have_gitlab_http_status(:ok)
expect(jar_file.file_name).to eq(file_upload.original_filename)
end
end
end
context 'for sha1 file' do context 'for sha1 file' do
let(:dummy_package) { double(Packages::Package) } let(:dummy_package) { double(Packages::Package) }
...@@ -698,7 +729,7 @@ RSpec.describe API::MavenPackages do ...@@ -698,7 +729,7 @@ RSpec.describe API::MavenPackages do
end end
def upload_file(params: {}, request_headers: headers, file_extension: 'jar') def upload_file(params: {}, request_headers: headers, file_extension: 'jar')
url = "/projects/#{project.id}/packages/maven/com/example/my-app/#{version}/my-app-1.0-20180724.124855-1.#{file_extension}" url = "/projects/#{project.id}/packages/maven/#{package_name}/#{version}/my-app-1.0-20180724.124855-1.#{file_extension}"
workhorse_finalize( workhorse_finalize(
api(url), api(url),
method: :put, method: :put,
......
...@@ -11,29 +11,36 @@ RSpec.describe Packages::Maven::FindOrCreatePackageService do ...@@ -11,29 +11,36 @@ RSpec.describe Packages::Maven::FindOrCreatePackageService do
let(:file_name) { 'test.jar' } let(:file_name) { 'test.jar' }
let(:param_path) { "#{path}/#{version}" } let(:param_path) { "#{path}/#{version}" }
let(:params) { { path: param_path, file_name: file_name } } let(:params) { { path: param_path, file_name: file_name } }
let(:service) { described_class.new(project, user, params) }
describe '#execute' do describe '#execute' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
subject { described_class.new(project, user, params).execute } subject { service.execute }
RSpec.shared_examples 'reuse existing package' do shared_examples 'reuse existing package' do
it { expect { subject}.not_to change { Packages::Package.count } } it { expect { subject }.not_to change { Packages::Package.count } }
it { is_expected.to eq(existing_package) } it 'returns the existing package' do
expect(subject.payload).to eq(package: existing_package)
end
end end
RSpec.shared_examples 'create package' do shared_examples 'create package' do
it { expect { subject }.to change { Packages::Package.count }.by(1) } it { expect { subject }.to change { Packages::Package.count }.by(1) }
it 'sets the proper name and version' do it 'sets the proper name and version', :aggregate_failures do
pkg = subject pkg = subject.payload[:package]
expect(pkg.name).to eq(path) expect(pkg.name).to eq(path)
expect(pkg.version).to eq(version) expect(pkg.version).to eq(version)
end end
it_behaves_like 'assigns build to package' context 'with a build' do
subject { service.execute.payload[:package] }
it_behaves_like 'assigns build to package'
end
end end
context 'path with version' do context 'path with version' do
...@@ -90,5 +97,27 @@ RSpec.describe Packages::Maven::FindOrCreatePackageService do ...@@ -90,5 +97,27 @@ RSpec.describe Packages::Maven::FindOrCreatePackageService do
expect { subject }.to change { Packages::BuildInfo.count }.by(1) expect { subject }.to change { Packages::BuildInfo.count }.by(1)
end end
end end
context 'when package duplicates are not allowed' do
let_it_be_with_refind(:package_settings) { create(:namespace_package_setting, :group, maven_duplicates_allowed: false) }
let_it_be_with_refind(:group) { package_settings.namespace }
let_it_be_with_refind(:project) { create(:project, group: group) }
let!(:existing_package) { create(:maven_package, name: path, version: version, project: project) }
it { expect { subject }.not_to change { project.package_files.count } }
it 'returns an error', :aggregate_failures do
expect(subject.payload).to be_empty
expect(subject.errors).to include('Duplicate package is not allowed')
end
context 'when the package name matches the exception regex' do
before do
package_settings.update!(maven_duplicate_exception_regex: '.*')
end
it_behaves_like 'reuse existing package'
end
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment