diff --git a/app/models/packages/conan/recipe_revision.rb b/app/models/packages/conan/recipe_revision.rb index e13c7843eaa02cf8fe66766495f53f7978b883b9..dca2953fab0105383b889d9e46e7d32c23dd41dd 100644 --- a/app/models/packages/conan/recipe_revision.rb +++ b/app/models/packages/conan/recipe_revision.rb @@ -15,8 +15,8 @@ class RecipeRevision < ApplicationRecord has_many :file_metadata, inverse_of: :recipe_revision, class_name: 'Packages::Conan::FileMetadatum' validates :package, :project, presence: true - validates :revision, presence: true, - uniqueness: { scope: :package_id }, format: { with: ::Gitlab::Regex.conan_revision_regex_v2 } + validates :revision, presence: true, format: { with: ::Gitlab::Regex.conan_revision_regex_v2 } + validates :revision, uniqueness: { scope: :package_id }, on: %i[create update] end end end diff --git a/app/services/packages/conan/create_package_file_service.rb b/app/services/packages/conan/create_package_file_service.rb index 5e9f4cc216c8cf971d9f4f719b089edc74a438d8..e1c35b4e156a609fcef26bb8028a2073511374f9 100644 --- a/app/services/packages/conan/create_package_file_service.rb +++ b/app/services/packages/conan/create_package_file_service.rb @@ -3,6 +3,8 @@ module Packages module Conan class CreatePackageFileService + include Gitlab::Utils::StrongMemoize + def initialize(package, file, params) @package = package @file = file @@ -20,7 +22,8 @@ def execute file_md5: params['file.md5'], conan_file_metadatum_attributes: { conan_file_type: params[:conan_file_type], - package_reference_id: package_reference_id + package_reference_id: package_reference_id, + recipe_revision_id: recipe_revision_id } ) @@ -43,9 +46,19 @@ def execute def package_reference_id return unless params[:conan_package_reference].present? - package_reference_result = ::Packages::Conan::UpsertPackageReferenceService.new(package, params[:conan_package_reference]).execute! + package_reference_result = ::Packages::Conan::UpsertPackageReferenceService.new(package, params[:conan_package_reference], recipe_revision_id).execute! package_reference_result[:package_reference_id] end + + def recipe_revision_id + unless params[:recipe_revision].present? && params[:recipe_revision] != ::Packages::Conan::FileMetadatum::DEFAULT_REVISION + return + end + + recipe_reference_result = ::Packages::Conan::UpsertRecipeRevisionService.new(package, params[:recipe_revision]).execute! + recipe_reference_result[:recipe_revision_id] + end + strong_memoize_attr :recipe_revision_id end end end diff --git a/app/services/packages/conan/upsert_package_reference_service.rb b/app/services/packages/conan/upsert_package_reference_service.rb index 59f61ba31daf86cefc34934ca9b930da49a7a2a0..4df826ad098bde4329ae31d5cf8a62121efc6474 100644 --- a/app/services/packages/conan/upsert_package_reference_service.rb +++ b/app/services/packages/conan/upsert_package_reference_service.rb @@ -5,9 +5,13 @@ module Conan class UpsertPackageReferenceService include Gitlab::Utils::StrongMemoize - def initialize(package, package_reference_value) + UNIQUENESS_COLUMNS = %i[package_id reference].freeze + UNIQUENESS_COLUMNS_WITH_REVISION = %i[package_id recipe_revision_id reference].freeze + + def initialize(package, package_reference_value, recipe_revision_id = nil) @package = package @package_reference_value = package_reference_value + @recipe_revision_id = recipe_revision_id end def execute! @@ -22,13 +26,14 @@ def execute! private - attr_reader :package, :package_reference_value + attr_reader :package, :package_reference_value, :recipe_revision_id def package_reference ::Packages::Conan::PackageReference.new( package_id: package.id, reference: package_reference_value, - project_id: package.project_id + project_id: package.project_id, + recipe_revision_id: recipe_revision_id ) end strong_memoize_attr :package_reference @@ -36,10 +41,18 @@ def package_reference def upsert_package_reference ::Packages::Conan::PackageReference .upsert( - package_reference.attributes.slice('package_id', 'project_id', 'reference'), - unique_by: %i[package_id reference] + package_reference.attributes.slice('package_id', 'project_id', 'reference', 'recipe_revision_id'), + unique_by: uniqueness_constraint_columns ) end + + # Two uniqueness constraints are used: + # - (package_id, reference) when recipe_revision_id is NULL + # - (package_id, recipe_revision_id, reference) when recipe_revision_id is present + # This allows having the same package_id/reference pair with different recipe_revision_id values + def uniqueness_constraint_columns + recipe_revision_id.nil? ? UNIQUENESS_COLUMNS : UNIQUENESS_COLUMNS_WITH_REVISION + end end end end diff --git a/app/services/packages/conan/upsert_recipe_revision_service.rb b/app/services/packages/conan/upsert_recipe_revision_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..dc39bd63169d36018e83f157df180e939b463367 --- /dev/null +++ b/app/services/packages/conan/upsert_recipe_revision_service.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module Packages + module Conan + class UpsertRecipeRevisionService + include Gitlab::Utils::StrongMemoize + + UNIQUENESS_COLUMNS = %i[package_id revision].freeze + + def initialize(package, revision) + @package = package + @revision = revision + end + + def execute! + # We use a different validation context + # so that the uniqueness model validation on + # [revision, package_id] + # is skipped. + recipe_revision.validate!(:upsert) + + ServiceResponse.success(payload: { recipe_revision_id: upsert_recipe_revision[0]['id'] }) + end + + private + + attr_reader :package, :revision + + def recipe_revision + ::Packages::Conan::RecipeRevision.new( + package_id: package.id, + revision: revision, + project_id: package.project_id + ) + end + strong_memoize_attr :recipe_revision + + def upsert_recipe_revision + ::Packages::Conan::RecipeRevision + .upsert( + recipe_revision.attributes.slice('package_id', 'project_id', 'revision'), + unique_by: UNIQUENESS_COLUMNS + ) + end + end + end +end diff --git a/doc/ci/jobs/fine_grained_permissions.md b/doc/ci/jobs/fine_grained_permissions.md index 92d858e2c6e9f28f4cf8cb2872a404ffc64a9932..fcdaff362f58b5482010ca1bd6919be1d7ce5925 100644 --- a/doc/ci/jobs/fine_grained_permissions.md +++ b/doc/ci/jobs/fine_grained_permissions.md @@ -131,6 +131,8 @@ The following endpoints are available for CI/CD job tokens. | Packages: Read and write | `ADMIN_PACKAGES` | `PUT /projects/:id/packages/conan/v1/files/:package_name/:package_version/:package_username/:package_channel/:recipe_revision/export/:file_name` | Upload recipe package files | | Packages: Read and write | `ADMIN_PACKAGES` | `PUT /projects/:id/packages/conan/v1/files/:package_name/:package_version/:package_username/:package_channel/:recipe_revision/package/:conan_package_reference/:package_revision/:file_name/authorize` | Workhorse authorize the conan package file | | Packages: Read and write | `ADMIN_PACKAGES` | `PUT /projects/:id/packages/conan/v1/files/:package_name/:package_version/:package_username/:package_channel/:recipe_revision/package/:conan_package_reference/:package_revision/:file_name` | Upload package files | +| Packages: Read and write | `ADMIN_PACKAGES` | `PUT /projects/:id/packages/conan/v2/conans/:package_name/:package_version/:package_username/:package_channel/revisions/:recipe_revision/files/:file_name/authorize` | Workhorse authorize the conan recipe file | +| Packages: Read and write | `ADMIN_PACKAGES` | `PUT /projects/:id/packages/conan/v2/conans/:package_name/:package_version/:package_username/:package_channel/revisions/:recipe_revision/files/:file_name` | Upload recipe package files | | Packages: Read and write | `ADMIN_PACKAGES` | `PUT /projects/:id/packages/generic/:package_name/*package_version/(*path/):file_name/authorize` | Workhorse authorize generic package file | | Packages: Read and write | `ADMIN_PACKAGES` | `PUT /projects/:id/packages/generic/:package_name/*package_version/(*path/):file_name` | Upload package file | | Packages: Read and write | `ADMIN_PACKAGES` | `PUT /projects/:id/packages/maven/*path/:file_name/authorize` | Workhorse authorize the maven package file upload | diff --git a/lib/api/conan/v2/project_packages.rb b/lib/api/conan/v2/project_packages.rb index add60950f3dd23c96e1daf7b6a41adabdc0e40ef..7d83c552d75a66e83a2dacd054dcf900704f8502 100644 --- a/lib/api/conan/v2/project_packages.rb +++ b/lib/api/conan/v2/project_packages.rb @@ -64,6 +64,50 @@ class ProjectPackages < ::API::Base get urgency: :low do download_package_file(:recipe_file) end + + desc 'Upload recipe package files' do + detail 'This feature was introduced in GitLab 17.10' + success code: 200 + failure [ + { code: 400, message: 'Bad Request' }, + { code: 401, message: 'Unauthorized' }, + { code: 403, message: 'Forbidden' }, + { code: 404, message: 'Not Found' } + ] + tags %w[conan_packages] + end + + params do + requires :file, type: ::API::Validations::Types::WorkhorseFile, + desc: 'The package file to be published (generated by Multipart middleware)', + documentation: { type: 'file' } + end + + route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true + route_setting :authorization, job_token_policies: :admin_packages + + put urgency: :low do + upload_package_file(:recipe_file) + end + + desc 'Workhorse authorize the conan recipe file' do + detail 'This feature was introduced in GitLab 17.10' + success code: 200 + failure [ + { code: 400, message: 'Bad Request' }, + { code: 401, message: 'Unauthorized' }, + { code: 403, message: 'Forbidden' }, + { code: 404, message: 'Not Found' } + ] + tags %w[conan_packages] + end + + route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true + route_setting :authorization, job_token_policies: :admin_packages + + put 'authorize', urgency: :low do + authorize_workhorse!(subject: project, maximum_size: project.actual_limits.conan_max_file_size) + end end end end diff --git a/lib/api/concerns/packages/conan/shared_endpoints.rb b/lib/api/concerns/packages/conan/shared_endpoints.rb index c74e7388e33da50d2d78e30247c86fb2903456f9..f8dab7d61b3ff8f4fd4e1d991faff4374730d6a4 100644 --- a/lib/api/concerns/packages/conan/shared_endpoints.rb +++ b/lib/api/concerns/packages/conan/shared_endpoints.rb @@ -46,6 +46,7 @@ module SharedEndpoints before do not_found! if Gitlab::FIPS.enabled? require_packages_enabled! + authenticate_non_get! end namespace 'users' do diff --git a/lib/api/concerns/packages/conan/v1_endpoints.rb b/lib/api/concerns/packages/conan/v1_endpoints.rb index 6faf8f10b4e911b051c73087f24da1306fd40e04..b3f74c6daf8beb5bd56a9bd38b04481186c053f5 100644 --- a/lib/api/concerns/packages/conan/v1_endpoints.rb +++ b/lib/api/concerns/packages/conan/v1_endpoints.rb @@ -24,9 +24,6 @@ def x_conan_server_capabilities_header ['revisions'] end end - before do - authenticate_non_get! - end desc 'Ping the Conan API' do detail 'This feature was introduced in GitLab 12.2' diff --git a/spec/models/packages/conan/recipe_revision_spec.rb b/spec/models/packages/conan/recipe_revision_spec.rb index 1891871423a58add5c2a6b2515be3b2ba99cf6fa..1793c9f089b50c0f8ee22e028a9b9d621f677a8c 100644 --- a/spec/models/packages/conan/recipe_revision_spec.rb +++ b/spec/models/packages/conan/recipe_revision_spec.rb @@ -32,7 +32,7 @@ it 'has unique revision scoped to package_id' do # ignore case, same revision string with different case are converted to same hexa binary - is_expected.to validate_uniqueness_of(:revision).scoped_to(:package_id).case_insensitive + is_expected.to validate_uniqueness_of(:revision).scoped_to(:package_id).case_insensitive.on(%i[create update]) end context 'when validating hex format and length' do diff --git a/spec/requests/api/conan/v1/instance_packages_spec.rb b/spec/requests/api/conan/v1/instance_packages_spec.rb index 8520ee95b4f0976124022ee8ac8d4e968c5cc656..2db7b48d4de76f997ee8729be526044d5dfc4b02 100644 --- a/spec/requests/api/conan/v1/instance_packages_spec.rb +++ b/spec/requests/api/conan/v1/instance_packages_spec.rb @@ -156,6 +156,7 @@ context 'with file upload endpoints' do include_context 'for conan file upload endpoints' + let(:recipe_revision) { ::Packages::Conan::FileMetadatum::DEFAULT_REVISION } describe 'PUT /api/v4/packages/conan/v1/files/:package_name/:package_version/:package_username/:package_channel' \ '/:recipe_revision/export/:file_name/authorize' do @@ -182,14 +183,14 @@ describe 'PUT /api/v4/packages/conan/v1/files/:package_name/:package_version/:package_username/:package_channel' \ '/:recipe_revision/export/:file_name' do - let(:url) { "/api/v4/packages/conan/v1/files/#{recipe_path}/0/export/#{file_name}" } + let(:url) { "/packages/conan/v1/files/#{recipe_path}/0/export/#{file_name}" } it_behaves_like 'workhorse recipe file upload endpoint' end describe 'PUT /api/v4/packages/conan/v1/files/:package_name/:package_version/:package_username/:package_channel' \ '/:recipe_revision/export/:conan_package_reference/:package_revision/:file_name' do - let(:url) { "/api/v4/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/#{file_name}" } + let(:url) { "/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/#{file_name}" } it_behaves_like 'workhorse package file upload endpoint' end diff --git a/spec/requests/api/conan/v1/project_packages_spec.rb b/spec/requests/api/conan/v1/project_packages_spec.rb index 991f70ea7274169c98e4338a326601674eb26a2e..7709528c789aba0ca00d904575214fe86e6bf95d 100644 --- a/spec/requests/api/conan/v1/project_packages_spec.rb +++ b/spec/requests/api/conan/v1/project_packages_spec.rb @@ -182,6 +182,7 @@ context 'with file upload endpoints' do include_context 'for conan file upload endpoints' + let(:recipe_revision) { ::Packages::Conan::FileMetadatum::DEFAULT_REVISION } describe 'PUT /api/v4/projects/:id/packages/conan/v1/files/:package_name/:package_version/:package_username' \ '/:package_channel/:recipe_revision/export/:file_name/authorize' do @@ -210,7 +211,7 @@ describe 'PUT /api/v4/projects/:id/packages/conan/v1/files/:package_name/:package_version/:package_username' \ '/:package_channel/:recipe_revision/export/:file_name' do - let(:url) { "/api/v4/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}/0/export/#{file_name}" } + let(:url) { "/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}/0/export/#{file_name}" } it_behaves_like 'workhorse recipe file upload endpoint' end @@ -218,7 +219,7 @@ describe 'PUT /api/v4/projects/:id/packages/conan/v1/files/:package_name/:package_version/:package_username' \ '/:package_channel/:recipe_revision/export/:conan_package_reference/:package_revision/:file_name' do let(:url) do - "/api/v4/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/#{file_name}" + "/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}/0/package/123456789/0/#{file_name}" end it_behaves_like 'workhorse package file upload endpoint' diff --git a/spec/requests/api/conan/v2/project_packages_spec.rb b/spec/requests/api/conan/v2/project_packages_spec.rb index 707cce3c6664daafb945d3dd0631e5619728e65c..74e4842a42de4d25755cc849aafbeaaa924643fb 100644 --- a/spec/requests/api/conan/v2/project_packages_spec.rb +++ b/spec/requests/api/conan/v2/project_packages_spec.rb @@ -7,6 +7,24 @@ let_it_be_with_reload(:package) { create(:conan_package, project: project) } let(:project_id) { project.id } + let(:url) { "/projects/#{project_id}/packages/conan/v2/conans/#{url_suffix}" } + + shared_examples 'conan package revisions feature flag check' do + before do + stub_feature_flags(conan_package_revisions_support: false) + end + + it_behaves_like 'returning response status with message', status: :not_found, + message: "404 'conan_package_revisions_support' feature flag is disabled Not Found" + end + + shared_examples 'packages feature check' do + before do + stub_packages_setting(enabled: false) + end + + it_behaves_like 'returning response status', :not_found + end describe 'GET /api/v4/projects/:id/packages/conan/v2/users/check_credentials' do let(:url) { "/projects/#{project.id}/packages/conan/v2/users/check_credentials" } @@ -35,10 +53,11 @@ let(:file_name) { recipe_file.file_name } let(:recipe_revision) { recipe_file_metadata.recipe_revision_value } let(:url_suffix) { "#{recipe_path}/revisions/#{recipe_revision}/files/#{file_name}" } - let(:url) { "/projects/#{project_id}/packages/conan/v2/conans/#{url_suffix}" } subject(:request) { get api(url), headers: headers } + it_behaves_like 'conan package revisions feature flag check' + it_behaves_like 'packages feature check' it_behaves_like 'recipe file download endpoint' it_behaves_like 'accept get request on private project with access to package registry for everyone' it_behaves_like 'project not found by project id' @@ -48,40 +67,62 @@ let(:headers) { job_basic_auth_header(target_job) } end - context 'when feature flag is disabled' do - before do - stub_feature_flags(conan_package_revisions_support: false) - end - - it_behaves_like 'returning response status with message', status: :not_found, - message: "404 'conan_package_revisions_support' feature flag is disabled Not Found" - end - - context 'when packages feature disabled' do - before do - stub_packages_setting(enabled: false) - end - - it_behaves_like 'returning response status', :not_found + it_behaves_like 'enforcing job token policies', :read_packages, + allow_public_access_for_enabled_project_features: :package_registry do + let(:headers) { job_basic_auth_header(target_job) } end - describe 'parameter validation' do + describe 'parameter validation for recipe file endpoints' do using RSpec::Parameterized::TableSyntax let(:url_suffix) { "#{url_recipe_path}/revisions/#{url_recipe_revision}/files/#{url_file_name}" } + # rubocop:disable Layout/LineLength -- Avoid formatting to keep one-line table syntax where(:error, :url_recipe_path, :url_recipe_revision, :url_file_name) do - /package_name/ | 'pac$kage-1/1.0.0/namespace1+project-1/stable' | ref(:recipe_revision) | ref(:file_name) - /package_version/ | 'package-1/1.0.$/namespace1+project-1/stable' | ref(:recipe_revision) | ref(:file_name) - /package_username/ | 'package-1/1.0.0/name$pace1+project-1/stable' | ref(:recipe_revision) | ref(:file_name) - /package_channel/ | 'package-1/1.0.0/namespace1+project-1/$table' | ref(:recipe_revision) | ref(:file_name) - /recipe_revision/ | ref(:recipe_path) | 'invalid_revi$ion' | ref(:file_name) - /file_name/ | ref(:recipe_path) | ref(:recipe_revision) | 'invalid_file.txt' + /package_name/ | 'pac$kage-1/1.0.0/namespace1+project-1/stable' | ref(:recipe_revision) | ref(:file_name) + /package_version/ | 'package-1/1.0.$/namespace1+project-1/stable' | ref(:recipe_revision) | ref(:file_name) + /package_username/ | 'package-1/1.0.0/name$pace1+project-1/stable' | ref(:recipe_revision) | ref(:file_name) + /package_channel/ | 'package-1/1.0.0/namespace1+project-1/$table' | ref(:recipe_revision) | ref(:file_name) + /recipe_revision/ | ref(:recipe_path) | 'invalid_revi$ion' | ref(:file_name) + /recipe_revision/ | ref(:recipe_path) | Packages::Conan::FileMetadatum::DEFAULT_REVISION | ref(:file_name) + /file_name/ | ref(:recipe_path) | ref(:recipe_revision) | 'invalid_file.txt' end + # rubocop:enable Layout/LineLength with_them do it_behaves_like 'returning response status with error', status: :bad_request, error: params[:error] end end end + + context 'with file upload endpoints' do + include_context 'for conan file upload endpoints' + let(:file_name) { 'conanfile.py' } + let(:recipe_revision) { OpenSSL::Digest.hexdigest('MD5', 'valid_recipe_revision') } + + describe 'PUT /api/v4/projects/:id/packages/conan/v2/conans/:package_name/:package_version/:package_username/' \ + ':package_channel/revisions/:recipe_revision/files/:file_name' do + let(:url_suffix) { "#{recipe_path}/revisions/#{recipe_revision}/files/#{file_name}" } + + subject(:request) { put api(url), headers: headers_with_token } + + it_behaves_like 'conan package revisions feature flag check' + it_behaves_like 'packages feature check' + it_behaves_like 'workhorse recipe file upload endpoint', recipe_revision: true + end + + describe 'PUT /api/v4/projects/:id/packages/conan/v2/conans/:package_name/:package_version/:package_username/' \ + ':package_channel/revisions/:recipe_revision/files/:file_name/authorize' do + let(:url_suffix) { "#{recipe_path}/revisions/#{recipe_revision}/files/#{file_name}/authorize" } + + subject(:request) do + put api(url), + headers: headers_with_token + end + + it_behaves_like 'conan package revisions feature flag check' + it_behaves_like 'packages feature check' + it_behaves_like 'workhorse authorize endpoint' + end + end end diff --git a/spec/services/packages/conan/create_package_file_service_spec.rb b/spec/services/packages/conan/create_package_file_service_spec.rb index 8d19f406ead297737033dbd98a67e4dc0639e23e..fc19aaabe8aadd37a2832da6a7e39dea9b9e53b9 100644 --- a/spec/services/packages/conan/create_package_file_service_spec.rb +++ b/spec/services/packages/conan/create_package_file_service_spec.rb @@ -10,27 +10,12 @@ describe '#execute', :aggregate_failures do let(:file_name) { 'foo.tgz' } let(:conan_package_reference) { '1234567890abcdef1234567890abcdef12345678' } + let(:recipe_revision) { OpenSSL::Digest.hexdigest('MD5', 'recipe_revision') } subject(:response) { described_class.new(package, file, params).execute } - shared_examples 'a valid package_file' do - let(:params) do - { - file_name: file_name, - 'file.md5': '12345', - 'file.sha1': '54321', - 'file.size': '128', - 'file.type': 'txt', - recipe_revision: '0', - package_revision: '0', - conan_package_reference: conan_package_reference, - conan_file_type: :package_file - }.with_indifferent_access - end - - it 'creates a new package file' do - response = subject - + shared_examples 'creating package file' do + it 'creates a new package file with expected attributes' do expect(response).to be_success package_file = response[:package_file] @@ -39,13 +24,22 @@ expect(package_file.file_md5).to eq('12345') expect(package_file.size).to eq(128) expect(package_file.conan_file_metadatum).to be_valid - expect(package_file.conan_file_metadatum.recipe_revision_value).to eq('0') - expect(package_file.conan_file_metadatum.package_revision_value).to eq('0') - expect(package_file.conan_file_metadatum.recipe_revision_id).to be_nil - expect(package_file.conan_file_metadatum.package_revision_id).to be_nil - expect(package_file.conan_file_metadatum.package_reference.reference).to eq(conan_package_reference) - expect(package_file.conan_file_metadatum.conan_file_type).to eq('package_file') + expect(package_file.conan_file_metadatum.conan_file_type).to eq(conan_file_type) expect(package_file.file.read).to eq('content') + + expect(package_file.conan_file_metadatum.recipe_revision_value).to eq(params[:recipe_revision]) + expect(package_file.conan_file_metadatum.package_revision_value).to eq(params[:package_revision]) + expect(package_file.conan_file_metadatum.package_reference_value).to eq(params[:conan_package_reference]) + end + + context 'with default recipe revision' do + let(:recipe_revision) { ::Packages::Conan::FileMetadatum::DEFAULT_REVISION } + + it 'does not create recipe revision' do + package_file = response[:package_file] + + expect(package_file.conan_file_metadatum.recipe_revision).to be_nil + end end it_behaves_like 'assigns build to package file' do @@ -53,7 +47,8 @@ end end - shared_examples 'a valid recipe_file' do + shared_context 'with recipe file parameters' do + let(:conan_file_type) { 'recipe_file' } let(:params) do { file_name: file_name, @@ -61,33 +56,26 @@ 'file.sha1': '54321', 'file.size': '128', 'file.type': 'txt', - recipe_revision: '0', + recipe_revision: recipe_revision, conan_file_type: :recipe_file }.with_indifferent_access end + end - it 'creates a new recipe file' do - response = subject - - expect(response).to be_success - package_file = response[:package_file] - - expect(package_file).to be_valid - expect(package_file.file_name).to eq(file_name) - expect(package_file.file_md5).to eq('12345') - expect(package_file.size).to eq(128) - expect(package_file.conan_file_metadatum).to be_valid - expect(package_file.conan_file_metadatum.recipe_revision_value).to eq('0') - expect(package_file.conan_file_metadatum.package_revision_value).to be_nil - expect(package_file.conan_file_metadatum.recipe_revision_id).to be_nil - expect(package_file.conan_file_metadatum.package_revision_id).to be_nil - expect(package_file.conan_file_metadatum.package_reference).to be_nil - expect(package_file.conan_file_metadatum.conan_file_type).to eq('recipe_file') - expect(package_file.file.read).to eq('content') - end - - it_behaves_like 'assigns build to package file' do - subject(:package_file) { response[:package_file] } + shared_context 'with package file parameters' do + let(:conan_file_type) { 'package_file' } + let(:params) do + { + file_name: file_name, + 'file.md5': '12345', + 'file.sha1': '54321', + 'file.size': '128', + 'file.type': 'txt', + recipe_revision: recipe_revision, + package_revision: ::Packages::Conan::FileMetadatum::DEFAULT_REVISION, + conan_package_reference: conan_package_reference, + conan_file_type: :package_file + }.with_indifferent_access end end @@ -110,8 +98,15 @@ context 'with temp file' do include_context 'with temp file setup' - it_behaves_like 'a valid package_file' - it_behaves_like 'a valid recipe_file' + context 'with recipe file' do + include_context 'with recipe file parameters' + it_behaves_like 'creating package file' + end + + context 'with package file' do + include_context 'with package file parameters' + it_behaves_like 'creating package file' + end end context 'with remote file' do @@ -132,8 +127,15 @@ let(:file) { fog_to_uploaded_file(tmp_object) } - it_behaves_like 'a valid package_file' - it_behaves_like 'a valid recipe_file' + context 'with recipe file' do + include_context 'with recipe file parameters' + it_behaves_like 'creating package file' + end + + context 'with package file' do + include_context 'with package file parameters' + it_behaves_like 'creating package file' + end end context 'file is missing' do @@ -143,7 +145,7 @@ let(:params) do { file_name: file_name, - recipe_revision: '0', + recipe_revision: recipe_revision, conan_file_type: :recipe_file } end @@ -163,7 +165,8 @@ let(:params) do { file_name: file_name, - package_revision: '0', + package_revision: ::Packages::Conan::FileMetadatum::DEFAULT_REVISION, + recipe_revision: recipe_revision, conan_file_type: :package_file, conan_package_reference: conan_package_reference } @@ -192,8 +195,8 @@ 'file.sha1': '54321', 'file.size': '128', 'file.type': 'txt', - recipe_revision: '0', - package_revision: '0', + recipe_revision: recipe_revision, + package_revision: ::Packages::Conan::FileMetadatum::DEFAULT_REVISION, conan_package_reference: 'invalid_reference', conan_file_type: :package_file }.with_indifferent_access diff --git a/spec/services/packages/conan/upsert_package_reference_service_spec.rb b/spec/services/packages/conan/upsert_package_reference_service_spec.rb index 70ba0d37896cd7378d0b45e8a8965b1f962a7172..909e53f17e436e6e3899f8cf510b6025aa1d6779 100644 --- a/spec/services/packages/conan/upsert_package_reference_service_spec.rb +++ b/spec/services/packages/conan/upsert_package_reference_service_spec.rb @@ -5,18 +5,43 @@ RSpec.describe Packages::Conan::UpsertPackageReferenceService, feature_category: :package_registry do let_it_be(:package) { create(:conan_package, without_package_files: true) } let_it_be(:conan_package_reference) { '1234567890abcdef1234567890abcdef12345678' } + let_it_be(:recipe_revision) { create(:conan_recipe_revision, package: package) } + + let(:recipe_revision_id) { nil } + + shared_examples 'creates a new package reference' do + it 'creates a new package reference with correct attributes' do + expect { response }.to change { Packages::Conan::PackageReference.count }.by(1) + + package_reference = Packages::Conan::PackageReference.last + expect(package_reference.reference).to eq(conan_package_reference) + expect(package_reference.recipe_revision_id).to eq(recipe_revision_id) + expect(response).to be_success + expect(response[:package_reference_id]).to eq(package_reference.id) + end + end + + shared_examples 'returns existing package reference' do + it 'returns the existing package reference without creating new one' do + expect { response }.not_to change { Packages::Conan::PackageReference.count } + + expect(response).to be_success + expect(response[:package_reference_id]).to eq(existing_package_reference.id) + end + end describe '#execute!', :aggregate_failures do - subject(:response) { described_class.new(package, conan_package_reference).execute! } + subject(:response) { described_class.new(package, conan_package_reference, recipe_revision_id).execute! } context 'when the package reference doesn\'t exist' do - it 'creates the package reference' do - expect { response }.to change { Packages::Conan::PackageReference.count }.by(1) + context 'with no recipe revision' do + it_behaves_like 'creates a new package reference' + end + + context 'with recipe revision' do + let(:recipe_revision_id) { recipe_revision.id } - package_reference = Packages::Conan::PackageReference.last - expect(package_reference.reference).to eq(conan_package_reference) - expect(response).to be_success - expect(response[:package_reference_id]).to eq(package_reference.id) + it_behaves_like 'creates a new package reference' end context 'when the package reference is invalid' do @@ -29,15 +54,38 @@ end context 'when the package reference already exists' do - let_it_be(:package_reference) do - create(:conan_package_reference, package: package, reference: conan_package_reference, recipe_revision: nil) + context 'with no recipe revision' do + let_it_be(:existing_package_reference) do + create(:conan_package_reference, package: package, reference: conan_package_reference, + recipe_revision: nil) + end + + context 'when adding similar package reference with no recipe revision' do + it_behaves_like 'returns existing package reference' + end + + context 'when adding similar package reference with recipe revision' do + let_it_be(:recipe_revision_id) { recipe_revision.id } + + it_behaves_like 'creates a new package reference' + end end - it 'returns existing package reference' do - expect { response }.not_to change { Packages::Conan::PackageReference.count } + context 'with recipe revision' do + let_it_be(:existing_package_reference) do + create(:conan_package_reference, package: package, reference: conan_package_reference, + recipe_revision: recipe_revision) + end + + context 'when adding similar package reference with the same recipe revision' do + let_it_be(:recipe_revision_id) { recipe_revision.id } - expect(response).to be_success - expect(response[:package_reference_id]).to eq(package_reference.id) + it_behaves_like 'returns existing package reference' + end + + context 'when adding similar package reference with no recipe revision' do + it_behaves_like 'creates a new package reference' + end end end end diff --git a/spec/services/packages/conan/upsert_recipe_revision_service_spec.rb b/spec/services/packages/conan/upsert_recipe_revision_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..44755703d56a3ca739bc244b47bd56ce4eb95d42 --- /dev/null +++ b/spec/services/packages/conan/upsert_recipe_revision_service_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Conan::UpsertRecipeRevisionService, feature_category: :package_registry do + let_it_be(:package) { create(:conan_package, without_package_files: true) } + let_it_be(:recipe_revision_value) { OpenSSL::Digest.hexdigest('MD5', 'valid_recipe_revision') } + + describe '#execute!', :aggregate_failures do + subject(:response) { described_class.new(package, recipe_revision_value).execute! } + + context 'when the recipe revision doesn\'t exist' do + it 'creates the recipe revision' do + expect { response }.to change { Packages::Conan::RecipeRevision.count }.by(1) + + recipe_revision = Packages::Conan::RecipeRevision.last + expect(recipe_revision.revision).to eq(recipe_revision_value) + expect(response).to be_success + expect(response[:recipe_revision_id]).to eq(recipe_revision.id) + end + + context 'when the recipe revision is invalid' do + let(:recipe_revision_value) { nil } + + it 'raises the error' do + expect { response }.to raise_error(ActiveRecord::RecordInvalid) + end + end + end + + context 'when the recipe revision already exists' do + let_it_be(:recipe_revision) do + create(:conan_recipe_revision, package: package, revision: recipe_revision_value) + end + + it 'returns existing recipe revision' do + expect { response }.not_to change { Packages::Conan::RecipeRevision.count } + + expect(response).to be_success + expect(response[:recipe_revision_id]).to eq(recipe_revision.id) + end + end + end +end diff --git a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb index 2f9f30da2c527620b5b6ffcc37c39c11006c6051..aef9ea5ad1ce30c09b3a06cce7a227cf1ce35bc9 100644 --- a/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb @@ -906,13 +906,13 @@ end end -RSpec.shared_examples 'workhorse recipe file upload endpoint' do +RSpec.shared_examples 'workhorse recipe file upload endpoint' do |recipe_revision: false| let(:file_name) { 'conanfile.py' } let(:params) { { file: temp_file(file_name) } } subject(:request) do workhorse_finalize( - url, + api(url), method: :put, file_key: :file, params: params, @@ -930,6 +930,8 @@ it_behaves_like 'handling empty values for username and channel' it_behaves_like 'handling validation error for package' it_behaves_like 'protected package main example' + + it { expect { request }.to change { Packages::Conan::RecipeRevision.count }.by(1) } if recipe_revision end RSpec.shared_examples 'workhorse package file upload endpoint' do @@ -938,7 +940,7 @@ subject(:request) do workhorse_finalize( - url, + api(url), method: :put, file_key: :file, params: params, @@ -1025,6 +1027,7 @@ package_file = project.packages.last.package_files.reload.last expect(package_file.file_name).to eq(params[:file].original_filename) + expect(package_file.conan_file_metadatum.recipe_revision_value).to eq(recipe_revision) end context 'with existing package' do