diff --git a/app/finders/packages/conan/package_file_finder.rb b/app/finders/packages/conan/package_file_finder.rb index a1ebf9f40fad2a857dd96c137a7937f792070e42..f2723c71633df8ce78f1bd471f9339a38b172cb8 100644 --- a/app/finders/packages/conan/package_file_finder.rb +++ b/app/finders/packages/conan/package_file_finder.rb @@ -8,6 +8,7 @@ class PackageFileFinder < ::Packages::PackageFileFinder def package_files files = super files = by_conan_file_type(files) + files = by_recipe_revision(files) by_conan_package_reference(files) end @@ -22,6 +23,16 @@ def by_conan_package_reference(files) files.with_conan_package_reference(params[:conan_package_reference]) end + + def by_recipe_revision(files) + return files unless params[:recipe_revision] + + if params[:recipe_revision] == Packages::Conan::FileMetadatum::DEFAULT_REVISION + files.without_conan_recipe_revision + else + files.with_conan_recipe_revision(params[:recipe_revision]) + end + end end end end diff --git a/app/models/packages/conan/file_metadatum.rb b/app/models/packages/conan/file_metadatum.rb index ccb873283cc8a9dc968a54ae44a9b46e4b77de4d..79aed19b185e7fb3fcd7c59e4e3bf6290b73029e 100644 --- a/app/models/packages/conan/file_metadatum.rb +++ b/app/models/packages/conan/file_metadatum.rb @@ -14,8 +14,7 @@ class Packages::Conan::FileMetadatum < ApplicationRecord validates :package_reference, absence: true, if: :recipe_file? validates :package_reference, presence: true, if: :package_file?, on: :create validate :conan_package_type - # recipe_revision and package_revision are not supported yet - validates :recipe_revision, absence: true + # package_revision is not supported yet validates :package_revision, absence: true enum conan_file_type: { recipe_file: 1, package_file: 2 } @@ -26,7 +25,7 @@ class Packages::Conan::FileMetadatum < ApplicationRecord CONAN_MANIFEST = 'conanmanifest.txt' def recipe_revision_value - DEFAULT_REVISION + recipe_revision&.revision || DEFAULT_REVISION end def package_revision_value diff --git a/app/models/packages/package_file.rb b/app/models/packages/package_file.rb index 5bab3309b386e8407fb7bdc937c6fe029c6085db..193ffe0534c9223ae2cf662a5f3e6a995b4975e6 100644 --- a/app/models/packages/package_file.rb +++ b/app/models/packages/package_file.rb @@ -99,6 +99,16 @@ class Packages::PackageFile < ApplicationRecord .where(packages_conan_file_metadata: { conan_package_reference: conan_package_reference }) end + scope :with_conan_recipe_revision, ->(recipe_revision) do + joins(conan_file_metadatum: :recipe_revision) + .where(packages_conan_recipe_revisions: { revision: recipe_revision }) + end + + scope :without_conan_recipe_revision, -> do + joins(:conan_file_metadatum) + .where(packages_conan_file_metadata: { recipe_revision_id: nil }) + end + def self.most_recent! recent.first! end diff --git a/db/post_migrate/20250131092512_add_idx_pkgs_conan_metadata_on_pkg_file_id_when_null_rec_rev.rb b/db/post_migrate/20250131092512_add_idx_pkgs_conan_metadata_on_pkg_file_id_when_null_rec_rev.rb new file mode 100644 index 0000000000000000000000000000000000000000..c8e0abf700c1f1e6a7cffc840565dbe825dfa8fc --- /dev/null +++ b/db/post_migrate/20250131092512_add_idx_pkgs_conan_metadata_on_pkg_file_id_when_null_rec_rev.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class AddIdxPkgsConanMetadataOnPkgFileIdWhenNullRecRev < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + milestone '17.9' + + INDEX_NAME = 'idx_pkgs_conan_metadata_on_pkg_file_id_when_null_rec_rev' + + def up + add_concurrent_index( + :packages_conan_file_metadata, + :package_file_id, + where: 'recipe_revision_id IS NULL', + name: INDEX_NAME + ) + end + + def down + remove_concurrent_index_by_name(:packages_conan_file_metadata, INDEX_NAME) + end +end diff --git a/db/post_migrate/20250131094134_add_idx_pkgs_conan_recipe_rev_on_id_and_revision.rb b/db/post_migrate/20250131094134_add_idx_pkgs_conan_recipe_rev_on_id_and_revision.rb new file mode 100644 index 0000000000000000000000000000000000000000..ce826761a54069c1918e248c903eba2f669179bf --- /dev/null +++ b/db/post_migrate/20250131094134_add_idx_pkgs_conan_recipe_rev_on_id_and_revision.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddIdxPkgsConanRecipeRevOnIdAndRevision < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + milestone '17.9' + + INDEX_NAME = 'idx_pkgs_conan_recipe_rev_on_id_and_revision' + + def up + add_concurrent_index( + :packages_conan_recipe_revisions, + [:id, :revision], + name: INDEX_NAME + ) + end + + def down + remove_concurrent_index_by_name(:packages_conan_recipe_revisions, INDEX_NAME) + end +end diff --git a/db/schema_migrations/20250131092512 b/db/schema_migrations/20250131092512 new file mode 100644 index 0000000000000000000000000000000000000000..d19bc1ae257a955b0498ef31a0d4c094b22306b0 --- /dev/null +++ b/db/schema_migrations/20250131092512 @@ -0,0 +1 @@ +0f1c02788064a0d3500787af973b0c0a26103115915ddd495cbd6c449cd29ee5 \ No newline at end of file diff --git a/db/schema_migrations/20250131094134 b/db/schema_migrations/20250131094134 new file mode 100644 index 0000000000000000000000000000000000000000..5c6d50186e23ce0f59141fec0bfda6c0d1ce833a --- /dev/null +++ b/db/schema_migrations/20250131094134 @@ -0,0 +1 @@ +cadce898a4a6407aea39f9462e18e6753e2f4a2d08117365ac70761b70b52d81 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index d711cdb9bdd47ab813c12b8861a8a2423b10d914..48f09a04ba00ce3019ca3d15ede2d864a5e8d778 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -30942,6 +30942,10 @@ CREATE INDEX idx_pipeline_execution_schedules_security_policy_id_and_id ON secur CREATE INDEX idx_pkgs_conan_file_metadata_on_pkg_file_id_when_recipe_file ON packages_conan_file_metadata USING btree (package_file_id) WHERE (conan_file_type = 1); +CREATE INDEX idx_pkgs_conan_metadata_on_pkg_file_id_when_null_rec_rev ON packages_conan_file_metadata USING btree (package_file_id) WHERE (recipe_revision_id IS NULL); + +CREATE INDEX idx_pkgs_conan_recipe_rev_on_id_and_revision ON packages_conan_recipe_revisions USING btree (id, revision); + CREATE INDEX idx_pkgs_debian_group_distribution_keys_on_distribution_id ON packages_debian_group_distribution_keys USING btree (distribution_id); CREATE INDEX idx_pkgs_debian_project_distribution_keys_on_distribution_id ON packages_debian_project_distribution_keys USING btree (distribution_id); diff --git a/ee/spec/requests/api/conan/v1/project_packages_spec.rb b/ee/spec/requests/api/conan/v1/project_packages_spec.rb index 05a3b7c7b7a53a87618adc9534e4941cb211d644..bc54f99e597e6bbb37cd12c0f9b7cbec4d4a199c 100644 --- a/ee/spec/requests/api/conan/v1/project_packages_spec.rb +++ b/ee/spec/requests/api/conan/v1/project_packages_spec.rb @@ -8,7 +8,7 @@ let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group, maintainers: user) } let_it_be(:project) { create(:project, group: group) } - let_it_be(:package) { create(:conan_package, project: project) } + let_it_be(:package) { create(:conan_package, project: project, without_recipe_revisions: true) } let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } let_it_be(:package_file) { package.package_files.find_by(file_name: 'conaninfo.txt') } let_it_be(:metadata) { package_file.conan_file_metadatum } diff --git a/lib/api/conan/v2/project_packages.rb b/lib/api/conan/v2/project_packages.rb index ca4d5e025873de9cb41d7d63f96e9da845682225..e1469bf50811d7117beabb005cfc7dee828d34b7 100644 --- a/lib/api/conan/v2/project_packages.rb +++ b/lib/api/conan/v2/project_packages.rb @@ -6,7 +6,7 @@ module V2 class ProjectPackages < ::API::Base before do if Feature.disabled?(:conan_package_revisions_support, Feature.current_request) - render_api_error!("'conan_package_revisions_support' feature flag is disabled", :not_found) + not_found!("'conan_package_revisions_support' feature flag is disabled") end end @@ -35,35 +35,37 @@ class ProjectPackages < ::API::Base check_username_channel end - desc 'Download recipe files' do - detail 'This feature was introduced in GitLab 17.8' - 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] - hidden true - end - - params do - with(type: String) do - requires :recipe_revision, regexp: Gitlab::Regex.conan_revision_regex_v2, + namespace 'revisions' do + params do + requires :recipe_revision, type: String, regexp: Gitlab::Regex.conan_revision_regex_v2, desc: 'Recipe revision', documentation: { example: 'df28fd816be3a119de5ce4d374436b25' } - requires :file_name, type: String, desc: 'Package file name', values: CONAN_FILES, - documentation: { example: 'conanfile.py' } end - end - - route_setting :authentication, job_token_allowed: true, basic_auth_personal_access_token: true - route_setting :authorization, job_token_policies: :read_packages - - get 'revisions/:recipe_revision/files/:file_name', requirements: FILE_NAME_REQUIREMENTS do - authorize_job_token_policies!(project) - - render_api_error!('Not supported', :not_found) + namespace ':recipe_revision' do + namespace 'files' do + params do + requires :file_name, type: String, desc: 'Package file name', values: CONAN_FILES, + documentation: { example: 'conanfile.py' } + end + namespace ':file_name', requirements: FILE_NAME_REQUIREMENTS do + desc 'Download recipe files' do + detail 'This feature was introduced in GitLab 17.8' + 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: :read_packages + get urgency: :low do + download_package_file(:recipe_file) + end + end + end + end end end end diff --git a/lib/api/helpers/packages/conan/api_helpers.rb b/lib/api/helpers/packages/conan/api_helpers.rb index e5e7442d22eb120ff711c7cdb7458339293c1eeb..b0e3538a513648ac2481e7114b04eed5e007ee72 100644 --- a/lib/api/helpers/packages/conan/api_helpers.rb +++ b/lib/api/helpers/packages/conan/api_helpers.rb @@ -29,7 +29,7 @@ def present_download_urls(entity) package, current_user, project, - conan_package_reference: params[:conan_package_reference], + conan_package_reference: declared(params)[:conan_package_reference], id: params[:id] ) @@ -68,7 +68,7 @@ def package_file?(file_name) def build_package_file_upload_url(file_name) options = url_options(file_name).merge( - conan_package_reference: params[:conan_package_reference], + conan_package_reference: declared(params)[:conan_package_reference], package_revision: ::Packages::Conan::FileMetadatum::DEFAULT_REVISION ) @@ -162,12 +162,15 @@ def token def download_package_file(file_type) authorize_read_package!(project) + not_found!('Package') unless package + package_file = ::Packages::Conan::PackageFileFinder .new( package, params[:file_name].to_s, conan_file_type: file_type, - conan_package_reference: params[:conan_package_reference] + conan_package_reference: declared(params)[:conan_package_reference], + recipe_revision: params[:recipe_revision] ).execute! track_package_event('pull_package', :conan, category: 'API::ConanPackages', project: project, namespace: project.namespace) if params[:file_name] == ::Packages::Conan::FileMetadatum::PACKAGE_BINARY diff --git a/spec/factories/packages/conan/packages.rb b/spec/factories/packages/conan/packages.rb index 1f9d5ed85eeadd5b9b7f3c58450818695655b5e9..4c5804f639a3a22a794794c5812034577b4e82a0 100644 --- a/spec/factories/packages/conan/packages.rb +++ b/spec/factories/packages/conan/packages.rb @@ -10,14 +10,17 @@ transient do without_package_files { false } - package_references { ['1234567890abcdef1234567890abcdef12345678'] } + without_recipe_revisions { false } + end + + conan_recipe_revisions do + next [] if without_recipe_revisions + + [association(:conan_recipe_revision, package: instance)] end conan_package_references do - package_references.map do |ref| - association(:conan_package_reference, - reference: ref, package: instance) - end + [association(:conan_package_reference, package: instance, recipe_revision: instance.conan_recipe_revisions.first)] end after :build do |package| @@ -28,16 +31,16 @@ after :create do |package, evaluator| unless evaluator.without_package_files - %i[conan_recipe_file conan_recipe_manifest].each do |file| - create :conan_package_file, file, package: package - end - + recipe_files = %i[conan_recipe_file conan_recipe_manifest] package_file_traits = %i[conan_package_info conan_package_manifest conan_package] - package.conan_package_references.each do |reference| - package_file_traits.each do |file| - create :conan_package_file, file, package: package, - conan_package_reference: reference - end + recipe_files.each do |file| + create :conan_package_file, file, package: package, + conan_recipe_revision: package.conan_recipe_revisions.first + end + package_file_traits.each do |file| + create :conan_package_file, file, package: package, + conan_package_reference: package.conan_package_references.first, + conan_recipe_revision: package.conan_recipe_revisions.first end end end diff --git a/spec/factories/packages/package_files.rb b/spec/factories/packages/package_files.rb index 3209c8987027a35987885201eb4df0d463cb8f58..7c178322681e67b7db15068210a4763159623ff3 100644 --- a/spec/factories/packages/package_files.rb +++ b/spec/factories/packages/package_files.rb @@ -26,12 +26,14 @@ transient do without_loaded_metadatum { false } conan_package_reference { package.conan_package_references.first } + conan_recipe_revision { package.conan_recipe_revisions.first } end trait(:conan_recipe_file) do after :create do |package_file, evaluator| unless evaluator.without_loaded_metadatum - create :conan_file_metadatum, :recipe_file, package_file: package_file + create :conan_file_metadatum, :recipe_file, + { package_file: package_file, recipe_revision: evaluator.conan_recipe_revision }.compact end end @@ -45,7 +47,8 @@ trait(:conan_recipe_manifest) do after :create do |package_file, evaluator| unless evaluator.without_loaded_metadatum - create :conan_file_metadatum, :recipe_file, package_file: package_file + create :conan_file_metadatum, :recipe_file, + { package_file: package_file, recipe_revision: evaluator.conan_recipe_revision }.compact end end @@ -60,7 +63,8 @@ after :create do |package_file, evaluator| unless evaluator.without_loaded_metadatum create :conan_file_metadatum, :package_file, - { package_file: package_file, package_reference: evaluator.conan_package_reference }.compact + { package_file: package_file, package_reference: evaluator.conan_package_reference, + recipe_revision: evaluator.conan_recipe_revision }.compact end end @@ -75,7 +79,8 @@ after :create do |package_file, evaluator| unless evaluator.without_loaded_metadatum create :conan_file_metadatum, :package_file, - { package_file: package_file, package_reference: evaluator.conan_package_reference }.compact + { package_file: package_file, package_reference: evaluator.conan_package_reference, + recipe_revision: evaluator.conan_recipe_revision }.compact end end @@ -90,7 +95,8 @@ after :create do |package_file, evaluator| unless evaluator.without_loaded_metadatum create :conan_file_metadatum, :package_file, - { package_file: package_file, package_reference: evaluator.conan_package_reference }.compact + { package_file: package_file, package_reference: evaluator.conan_package_reference, + recipe_revision: evaluator.conan_recipe_revision }.compact end end diff --git a/spec/finders/packages/conan/package_file_finder_spec.rb b/spec/finders/packages/conan/package_file_finder_spec.rb index 62906a7b0a922dc87c90270e7a9a9d925923a2e5..283a56696637202e03ac849bfeddb040a30925b5 100644 --- a/spec/finders/packages/conan/package_file_finder_spec.rb +++ b/spec/finders/packages/conan/package_file_finder_spec.rb @@ -20,10 +20,6 @@ end context 'with conan_package_reference' do - let_it_be(:other_package) { create(:conan_package) } - let_it_be(:package_file_name) { 'conan_package.tgz' } - let_it_be(:package_file) { package.package_files.find_by(file_name: package_file_name) } - let(:params) do { conan_package_reference: package_file.conan_file_metadatum.conan_package_reference } end @@ -37,6 +33,30 @@ it { is_expected.to eq(package_file) } end + + context 'with recipe_revision' do + let(:params) { { recipe_revision: recipe_revision_value } } + + context 'with default revision' do + let(:recipe_revision_value) { Packages::Conan::FileMetadatum::DEFAULT_REVISION } + let_it_be(:package_file_without_revision) do + create(:conan_package_file, :conan_recipe_file, + package: package, conan_recipe_revision: nil) + end + + it 'returns package files without recipe revision' do + expect(subject).to eq(package_file_without_revision) + end + end + + context 'with specific revision' do + let(:recipe_revision_value) { package_file.conan_file_metadatum.recipe_revision_value } + + it 'returns package files with matching recipe revision' do + expect(subject).to eq(package_file) + end + end + end end shared_examples 'not returning pending_destruction package files' do diff --git a/spec/models/packages/conan/file_metadatum_spec.rb b/spec/models/packages/conan/file_metadatum_spec.rb index 15d6f509e7ccbd7892685e9664836e1175c6812f..e882f9c6027e20e57150776e7891508317e031f9 100644 --- a/spec/models/packages/conan/file_metadatum_spec.rb +++ b/spec/models/packages/conan/file_metadatum_spec.rb @@ -26,7 +26,6 @@ describe 'validations' do it { is_expected.to validate_presence_of(:package_file) } - it { is_expected.to validate_absence_of(:recipe_revision) } it { is_expected.to validate_absence_of(:package_revision) } describe '#conan_package_reference' do @@ -138,11 +137,24 @@ end describe '#recipe_revision_value' do - let(:conan_file_metadatum) { build(:conan_file_metadatum, :recipe_file, package_file: package_file) } + context 'when recipe_revision is present' do + let(:revision) { 'some-revision-value' } - it 'returns the default recipe revision value' do - expect(conan_file_metadatum.recipe_revision_value).to eq( - Packages::Conan::FileMetadatum::DEFAULT_REVISION) + let(:file_metadatum) do + build(:conan_file_metadatum, recipe_revision: build(:conan_recipe_revision, revision: revision)) + end + + it 'returns the revision value' do + expect(file_metadatum.recipe_revision_value).to eq(revision) + end + end + + context 'when recipe_revision is nil' do + let(:file_metadatum) { build(:conan_file_metadatum) } + + it 'returns DEFAULT_REVISION' do + expect(file_metadatum.recipe_revision_value).to eq(described_class::DEFAULT_REVISION) + end end end diff --git a/spec/models/packages/conan/package_reference_spec.rb b/spec/models/packages/conan/package_reference_spec.rb index 3b8bff658068162835e2880b51d4241df9826d7d..9c42bb4c7a638eecf8268bb124ed284ecf0c3a19 100644 --- a/spec/models/packages/conan/package_reference_spec.rb +++ b/spec/models/packages/conan/package_reference_spec.rb @@ -35,7 +35,7 @@ describe 'uniqueness of reference' do let_it_be(:conan_package) { create(:conan_package, without_package_files: true) } - let_it_be(:existing_reference) { create(:conan_package_reference, package: conan_package) } + let_it_be(:existing_reference) { conan_package.conan_package_references.first } context 'when recipe_revision_id is not nil' do it 'validates uniqueness scoped to package_id and recipe_revision_id', :aggregate_failures do diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index f4bd693d719fe55668dadab99cdee20ccf580240..b858c6e3493604fdd00d5c180e652882dff88194 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -129,17 +129,6 @@ end end - describe '.with_conan_package_reference' do - let_it_be(:non_matching_package_file) { create(:package_file, :nuget) } - let_it_be(:metadatum) { create(:conan_file_metadatum, :package_file) } - let_it_be(:reference) { metadatum.conan_package_reference } - - it 'returns matching packages' do - expect(described_class.with_conan_package_reference(reference)) - .to eq([metadatum.package_file]) - end - end - describe '.for_rubygem_with_file_name' do let_it_be(:non_ruby_package) { create(:nuget_package, project: project, package_type: :nuget) } let_it_be(:ruby_package) { create(:rubygems_package, project: project, package_type: :rubygems) } @@ -154,6 +143,95 @@ end end + context 'Conan scopes' do + let_it_be(:package) { create(:conan_package, without_package_files: true) } + + describe '.with_conan_package_reference' do + let_it_be(:package_reference) { create(:conan_package_reference, package: package) } + let_it_be(:other_package_reference) { create(:conan_package_reference, package: package) } + + let_it_be(:matching_package_file) do + create(:conan_package_file, :conan_package, + package: package, + conan_package_reference: package_reference) + end + + let_it_be(:package_file_with_other_reference) do + create(:conan_package_file, :conan_package, + package: package, + conan_package_reference: other_package_reference) + end + + subject { described_class.with_conan_package_reference(reference) } + + context 'with existing reference' do + let(:reference) { matching_package_file.conan_file_metadatum.conan_package_reference } + + it 'returns package files with matching reference' do + expect(subject).to contain_exactly(matching_package_file) + end + end + + context 'when reference does not exist' do + let(:reference) { non_existing_record_id.to_s } + + it 'returns empty relation' do + expect(subject).to be_empty + end + end + end + + context 'recipe revision scopes' do + let_it_be(:recipe_revision) { package.conan_recipe_revisions.first } + let_it_be(:other_revision) { create(:conan_recipe_revision, package: package) } + + let_it_be(:package_file_with_revision) do + create(:conan_package_file, :conan_recipe_file, + package: package, + conan_recipe_revision: recipe_revision) + end + + let_it_be(:package_file_with_other_revision) do + create(:conan_package_file, :conan_recipe_file, + package: package, + conan_recipe_revision: other_revision) + end + + let_it_be(:package_file_without_revision) do + create(:conan_package_file, :conan_recipe_file, + package: package, conan_recipe_revision: nil) + end + + describe '.with_conan_recipe_revision' do + subject { described_class.with_conan_recipe_revision(revision) } + + context 'with existing revision' do + let(:revision) { recipe_revision.revision } + + it 'returns package files with matching recipe revision' do + expect(subject).to contain_exactly(package_file_with_revision) + end + end + + context 'when revision does not exist' do + let(:revision) { 'nonexistent' } + + it 'returns empty relation' do + expect(subject).to be_empty + end + end + end + + describe '.without_conan_recipe_revision' do + subject { described_class.without_conan_recipe_revision } + + it 'returns only package files without recipe revision' do + expect(subject).to contain_exactly(package_file_without_revision) + end + end + end + end + context 'Debian scopes' do let_it_be(:debian_changes) { debian_package.package_files.last } let_it_be(:debian_deb) { create(:debian_package_file, package: debian_package) } diff --git a/spec/presenters/packages/conan/package_presenter_spec.rb b/spec/presenters/packages/conan/package_presenter_spec.rb index 618f2014f16a07b4f871ecf1b748366467343ea8..ecca9e6e9260aaa12de606e215e82b1a39f728a1 100644 --- a/spec/presenters/packages/conan/package_presenter_spec.rb +++ b/spec/presenters/packages/conan/package_presenter_spec.rb @@ -4,15 +4,22 @@ RSpec.describe ::Packages::Conan::PackagePresenter, feature_category: :package_registry do let_it_be(:user) { create(:user) } - let_it_be(:conan_package_reference) { '1234567890abcdef1234567890abcdef12345678' } - let_it_be(:alternative_reference) { '1111111111111111111111111111111111111111' } - let_it_be(:package) { create(:conan_package, package_references: [conan_package_reference, alternative_reference]) } + let_it_be(:package) { create(:conan_package, without_recipe_revisions: true) } + let_it_be(:conan_package_reference) { package.conan_package_references.first } + let_it_be(:alternative_reference) { create(:conan_package_reference, package: package, recipe_revision: nil) } + let_it_be(:project) { package.project } let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, package: package) } - let(:params) { { package_scope: :instance } } let(:presenter) { described_class.new(package, user, project, params) } + before_all do + %i[conan_package_info conan_package_manifest conan_package].each do |file| + create(:conan_package_file, file, package: package, + conan_package_reference: alternative_reference, conan_recipe_revision: nil) + end + end + shared_examples 'no existing package' do context 'when package does not exist' do let(:package) { nil } @@ -94,7 +101,7 @@ end describe '#package_urls' do - let(:reference) { conan_package_reference } + let(:reference) { conan_package_reference.reference } let(:params) do { @@ -115,9 +122,9 @@ context 'existing package' do let(:expected_result) do { - "conaninfo.txt" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{conan_package_reference}/0/conaninfo.txt", - "conanmanifest.txt" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{conan_package_reference}/0/conanmanifest.txt", - "conan_package.tgz" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{conan_package_reference}/0/conan_package.tgz" + "conaninfo.txt" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{reference}/0/conaninfo.txt", + "conanmanifest.txt" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{reference}/0/conanmanifest.txt", + "conan_package.tgz" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{reference}/0/conan_package.tgz" } end @@ -134,9 +141,9 @@ let(:expected_result) do { - "conaninfo.txt" => "#{Settings.build_base_gitlab_url}/api/v4/projects/#{project.id}/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{conan_package_reference}/0/conaninfo.txt", - "conanmanifest.txt" => "#{Settings.build_base_gitlab_url}/api/v4/projects/#{project.id}/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{conan_package_reference}/0/conanmanifest.txt", - "conan_package.tgz" => "#{Settings.build_base_gitlab_url}/api/v4/projects/#{project.id}/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{conan_package_reference}/0/conan_package.tgz" + "conaninfo.txt" => "#{Settings.build_base_gitlab_url}/api/v4/projects/#{project.id}/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{reference}/0/conaninfo.txt", + "conanmanifest.txt" => "#{Settings.build_base_gitlab_url}/api/v4/projects/#{project.id}/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{reference}/0/conanmanifest.txt", + "conan_package.tgz" => "#{Settings.build_base_gitlab_url}/api/v4/projects/#{project.id}/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{reference}/0/conan_package.tgz" } end @@ -147,13 +154,13 @@ it { is_expected.to eq(expected_result) } context 'requesting the alternative reference' do - let(:reference) { alternative_reference } + let(:reference) { alternative_reference.reference } let(:expected_result) do { - "conaninfo.txt" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{alternative_reference}/0/conaninfo.txt", - "conanmanifest.txt" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{alternative_reference}/0/conanmanifest.txt", - "conan_package.tgz" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{alternative_reference}/0/conan_package.tgz" + "conaninfo.txt" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{reference}/0/conaninfo.txt", + "conanmanifest.txt" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{reference}/0/conanmanifest.txt", + "conan_package.tgz" => "#{Settings.build_base_gitlab_url}/api/v4/packages/conan/v1/files/#{package.conan_recipe_path}/0/package/#{reference}/0/conan_package.tgz" } end @@ -172,7 +179,7 @@ end describe '#package_snapshot' do - let(:reference) { conan_package_reference } + let(:reference) { conan_package_reference.reference } let(:params) { { conan_package_reference: reference } } subject { presenter.package_snapshot } diff --git a/spec/requests/api/conan/v1/instance_packages_spec.rb b/spec/requests/api/conan/v1/instance_packages_spec.rb index 955bedbed681d20c693758c5ed03d700ffeb14da..be1249514fa5b6345a6664f0d2114e8b12e24275 100644 --- a/spec/requests/api/conan/v1/instance_packages_spec.rb +++ b/spec/requests/api/conan/v1/instance_packages_spec.rb @@ -3,12 +3,13 @@ require 'spec_helper' RSpec.describe API::Conan::V1::InstancePackages, feature_category: :package_registry do + include_context 'conan api setup' + + let_it_be_with_reload(:package) { create(:conan_package, project: project, without_recipe_revisions: true) } let(:snowplow_gitlab_standard_context) do { user: user, project: project, namespace: project.namespace, property: 'i_package_conan_user' } end - include_context 'conan api setup' - describe 'GET /api/v4/packages/conan/v1/ping' do let_it_be(:url) { '/packages/conan/v1/ping' } @@ -130,7 +131,7 @@ describe 'GET /api/v4/packages/conan/v1/files/:package_name/:package_version/:package_username/:package_channel' \ '/:recipe_revision/export/:file_name' do subject(:request) do - get api("/packages/conan/v1/files/#{recipe_path}/#{metadata.recipe_revision_value}/export/" \ + get api("/packages/conan/v1/files/#{recipe_path}/#{recipe_file_metadata.recipe_revision_value}/export/" \ "#{recipe_file.file_name}"), headers: headers end @@ -142,8 +143,9 @@ describe 'GET /api/v4/packages/conan/v1/files/:package_name/:package_version/:package_username/:package_channel' \ '/:recipe_revision/package/:conan_package_reference/:package_revision/:file_name' do subject(:request) do - get api("/packages/conan/v1/files/#{recipe_path}/#{metadata.recipe_revision_value}/package" \ - "/#{metadata.conan_package_reference}/#{metadata.package_revision_value}/#{package_file.file_name}"), + get api("/packages/conan/v1/files/#{recipe_path}/#{package_file_metadata.recipe_revision_value}/package" \ + "/#{package_file_metadata.conan_package_reference}/#{package_file_metadata.package_revision_value}" \ + "/#{package_file.file_name}"), headers: headers end diff --git a/spec/requests/api/conan/v1/project_packages_spec.rb b/spec/requests/api/conan/v1/project_packages_spec.rb index 42fb639f91004f81412d346f40230a234480fc7d..fdaffe5b0f46444cf05b683b2f59a41f082474e6 100644 --- a/spec/requests/api/conan/v1/project_packages_spec.rb +++ b/spec/requests/api/conan/v1/project_packages_spec.rb @@ -5,19 +5,9 @@ RSpec.describe API::Conan::V1::ProjectPackages, feature_category: :package_registry do include_context 'conan api setup' + let_it_be_with_reload(:package) { create(:conan_package, project: project, without_recipe_revisions: true) } let(:project_id) { project.id } - shared_examples 'accept get request on private project with access to package registry for everyone' do - subject { get api(url) } - - before do - project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) - project.project_feature.update!(package_registry_access_level: ProjectFeature::PUBLIC) - end - - it_behaves_like 'returning response status', :ok - end - describe 'GET /api/v4/projects/:id/packages/conan/v1/ping' do let(:url) { "/projects/#{project.id}/packages/conan/v1/ping" } @@ -167,7 +157,7 @@ describe 'GET /api/v4/projects/:id/packages/conan/v1/files/:package_name/:package_version/:package_username' \ '/:package_channel/:recipe_revision/export/:file_name' do let(:url) do - "/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}/#{metadata.recipe_revision_value}" \ + "/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}/#{recipe_file_metadata.recipe_revision_value}" \ "/export/#{recipe_file.file_name}" end @@ -179,8 +169,9 @@ describe 'GET /api/v4/projects/:id/packages/conan/v1/files/:package_name/:package_version/:package_username' \ '/:package_channel/:recipe_revision/package/:conan_package_reference/:package_revision/:file_name' do let(:url) do - "/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}/#{metadata.recipe_revision_value}/package" \ - "/#{metadata.conan_package_reference}/#{metadata.package_revision_value}/#{package_file.file_name}" + "/projects/#{project_id}/packages/conan/v1/files/#{recipe_path}" \ + "/#{package_file_metadata.recipe_revision_value}/package/#{package_file_metadata.conan_package_reference}" \ + "/#{package_file_metadata.package_revision_value}/#{package_file.file_name}" end it_behaves_like 'package file download endpoint' diff --git a/spec/requests/api/conan/v2/project_packages_spec.rb b/spec/requests/api/conan/v2/project_packages_spec.rb index 06d5f08e53ac6201e7909e97ec6f787f7e196142..646cfd78b18347ab551093e98c9e099bdbd2ce3f 100644 --- a/spec/requests/api/conan/v2/project_packages_spec.rb +++ b/spec/requests/api/conan/v2/project_packages_spec.rb @@ -5,6 +5,9 @@ RSpec.describe API::Conan::V2::ProjectPackages, feature_category: :package_registry do include_context 'conan api setup' + let_it_be_with_reload(:package) { create(:conan_package, project: project) } + let(:project_id) { project.id } + describe 'GET /api/v4/projects/:id/packages/conan/v2/users/check_credentials' do let(:url) { "/projects/#{project.id}/packages/conan/v2/users/check_credentials" } @@ -29,32 +32,24 @@ ':package_channel/revisions/:recipe_revision/files/:file_name' do include_context 'conan file download endpoints' - let(:project_id) { project.id } - let(:recipe_revision) { OpenSSL::Digest.hexdigest('MD5', 'valid_recipe_revision') } 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(:get_request) { get api(url), headers: headers } - - # TODO: Endpoint is not implemented yet. See https://gitlab.com/gitlab-org/gitlab/-/issues/333033#note_2060136937. - it_behaves_like 'returning response status with message', status: :not_found, message: 'Not supported' + subject(:request) { get api(url), headers: headers } + 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' - # TODO remove expected_success_status: :not_found when endpoint is implemented - it_behaves_like 'enforcing job token policies', :read_packages, expected_success_status: :not_found do - let(:request) { get_request } - 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: "'conan_package_revisions_support' feature flag is disabled" + message: "404 'conan_package_revisions_support' feature flag is disabled Not Found" end context 'when packages feature disabled' do @@ -65,10 +60,6 @@ it_behaves_like 'returning response status', :not_found end - context 'in FIPS mode', :fips_mode do - it_behaves_like 'returning response status', :not_found - end - describe 'parameter validation' do using RSpec::Parameterized::TableSyntax 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 68e72868b2c4ea4c25782299e36c9674c60fd702..70ba0d37896cd7378d0b45e8a8965b1f962a7172 100644 --- a/spec/services/packages/conan/upsert_package_reference_service_spec.rb +++ b/spec/services/packages/conan/upsert_package_reference_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Packages::Conan::UpsertPackageReferenceService, feature_category: :package_registry do - let_it_be(:package) { create(:conan_package, without_package_files: true, package_references: []) } + let_it_be(:package) { create(:conan_package, without_package_files: true) } let_it_be(:conan_package_reference) { '1234567890abcdef1234567890abcdef12345678' } describe '#execute!', :aggregate_failures do diff --git a/spec/support/shared_contexts/requests/api/conan_packages_shared_context.rb b/spec/support/shared_contexts/requests/api/conan_packages_shared_context.rb index 4cc1588726d2c4175175e096f1901fb59fa6fd05..0c3e70e20ec5a872e642080e1957c2f14e02391d 100644 --- a/spec/support/shared_contexts/requests/api/conan_packages_shared_context.rb +++ b/spec/support/shared_contexts/requests/api/conan_packages_shared_context.rb @@ -18,10 +18,7 @@ let_it_be(:job, freeze: true) { create(:ci_build, :running, user: user, project: project) } - let_it_be(:conan_package_reference) { '1234567890abcdef1234567890abcdef12345678' } - let_it_be_with_reload(:package) do - create(:conan_package, project: project, package_references: [conan_package_reference]) - end + let(:conan_package_reference) { package.conan_package_references.first.reference } let(:job_token) { job.token } let(:auth_token) { personal_access_token.token } @@ -63,8 +60,9 @@ let(:headers) { build_token_auth_header(jwt.encoded) } let(:recipe_path) { package.conan_recipe_path } let(:package_file) { package.package_files.find_by(file_name: 'conaninfo.txt') } - let(:recipe_file) { package.package_files.find_by(file_name: 'conanfile.py') } - let(:metadata) { package_file.conan_file_metadatum } + let(:recipe_file) { package.package_files.find_by!(file_name: 'conanfile.py') } + let(:package_file_metadata) { package_file.conan_file_metadatum } + let(:recipe_file_metadata) { recipe_file.conan_file_metadatum } end RSpec.shared_context 'conan file upload endpoints' do 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 7aa6b7a61007b388bc8bedffabc94d31ba34256c..1c2e5fa2c6e5b5c5b57bd9ab0633f6f7d276dbdf 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 @@ -779,6 +779,7 @@ it_behaves_like 'an internal project with packages' it_behaves_like 'a private project with packages' it_behaves_like 'handling empty values for username and channel' + it_behaves_like 'package not found download' end RSpec.shared_examples 'package file download endpoint' do @@ -787,6 +788,7 @@ it_behaves_like 'an internal project with packages' it_behaves_like 'a private project with packages' it_behaves_like 'handling empty values for username and channel' + it_behaves_like 'package not found download' context 'tracking the conan_package.tgz download' do let(:package_file) { package.package_files.find_by(file_name: ::Packages::Conan::FileMetadatum::PACKAGE_BINARY) } @@ -1177,3 +1179,27 @@ let(:headers_with_token) { job_basic_auth_header(target_job).merge(workhorse_headers) } end end + +RSpec.shared_examples 'accept get request on private project with access to package registry for everyone' do + subject { get api(url) } + + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + project.project_feature.update!(package_registry_access_level: ProjectFeature::PUBLIC) + end + + it_behaves_like 'returning response status', :ok +end + +RSpec.shared_examples 'package not found download' do + context 'when package does not exist' do + let(:recipe_path) { "missing/0.1.0/#{project.full_path.tr('/', '+')}/stable" } + + it 'returns 404 not found' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Package Not Found') + end + end +end