diff --git a/config/feature_flags/development/npm_allow_packages_in_multiple_projects.yml b/config/feature_flags/development/npm_allow_packages_in_multiple_projects.yml new file mode 100644 index 0000000000000000000000000000000000000000..7541a0dd24e8d630b02cb4e11e52c17a535c9ed3 --- /dev/null +++ b/config/feature_flags/development/npm_allow_packages_in_multiple_projects.yml @@ -0,0 +1,8 @@ +--- +name: npm_allow_packages_in_multiple_projects +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/111775 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/391692 +milestone: '15.10' +type: development +group: group::package registry +default_enabled: false diff --git a/lib/api/concerns/packages/npm_endpoints.rb b/lib/api/concerns/packages/npm_endpoints.rb index f26b3a1d8c2fc894dc1060dcc573e4d9f639b5bf..ef4aec44cb521976a655d912a6ac518773948c52 100644 --- a/lib/api/concerns/packages/npm_endpoints.rb +++ b/lib/api/concerns/packages/npm_endpoints.rb @@ -163,8 +163,13 @@ def redirect_or_present_audit_report route_setting :authentication, job_token_allowed: true, deploy_token_allowed: true get '*package_name', format: false, requirements: ::API::Helpers::Packages::Npm::NPM_ENDPOINT_REQUIREMENTS do package_name = params[:package_name] - packages = ::Packages::Npm::PackageFinder.new(package_name, project: project_or_nil) - .execute + packages = + if Feature.enabled?(:npm_allow_packages_in_multiple_projects) + finder_for_endpoint_scope(package_name).execute + else + ::Packages::Npm::PackageFinder.new(package_name, project: project_or_nil) + .execute + end redirect_request = project_or_nil.blank? || packages.empty? diff --git a/lib/api/helpers/packages/npm.rb b/lib/api/helpers/packages/npm.rb index 352d77f472c86fc2b9b0c83793a5794ed48a205e..4eb6c39b7dce97aab4a055bd493a8c8f3e328303 100644 --- a/lib/api/helpers/packages/npm.rb +++ b/lib/api/helpers/packages/npm.rb @@ -33,6 +33,15 @@ def project end end + def finder_for_endpoint_scope(package_name) + case endpoint_scope + when :project + ::Packages::Npm::PackageFinder.new(package_name, project: project_or_nil) + when :instance + ::Packages::Npm::PackageFinder.new(package_name, namespace: top_namespace_from(package_name)) + end + end + def project_or_nil # mainly used by the metadata endpoint where we need to get a project # and return nil if not found (no errors should be raised) @@ -50,11 +59,17 @@ def project_id_or_nil params[:id] when :instance package_name = params[:package_name] - namespace_path = ::Packages::Npm.scope_of(package_name) - next unless namespace_path - namespace = Namespace.top_most - .by_path(namespace_path) + namespace = + if Feature.enabled?(:npm_allow_packages_in_multiple_projects) + top_namespace_from(package_name) + else + namespace_path = ::Packages::Npm.scope_of(package_name) + next unless namespace_path + + Namespace.top_most.by_path(namespace_path) + end + next unless namespace finder = ::Packages::Npm::PackageFinder.new( @@ -67,6 +82,15 @@ def project_id_or_nil end end end + + private + + def top_namespace_from(package_name) + namespace_path = ::Packages::Npm.scope_of(package_name) + return unless namespace_path + + Namespace.top_most.by_path(namespace_path) + end end end end diff --git a/spec/requests/api/npm_instance_packages_spec.rb b/spec/requests/api/npm_instance_packages_spec.rb index dcd2e4ae67735b8c184b0cff5e391a51dc275fa3..591a8ee68dccfa1f213c576880390c8fa57de6c0 100644 --- a/spec/requests/api/npm_instance_packages_spec.rb +++ b/spec/requests/api/npm_instance_packages_spec.rb @@ -11,8 +11,38 @@ include_context 'npm api setup' describe 'GET /api/v4/packages/npm/*package_name' do - it_behaves_like 'handling get metadata requests', scope: :instance do - let(:url) { api("/packages/npm/#{package_name}") } + let(:url) { api("/packages/npm/#{package_name}") } + + it_behaves_like 'handling get metadata requests', scope: :instance + + context 'with a duplicate package name in another project' do + subject { get(url) } + + let_it_be(:project2) { create(:project, :public, namespace: namespace) } + let_it_be(:package2) do + create(:npm_package, + project: project2, + name: "@#{group.path}/scoped_package", + version: '1.2.0') + end + + it 'includes all matching package versions in the response' do + subject + + expect(json_response['versions'].keys).to match_array([package.version, package2.version]) + end + + context 'with the feature flag disabled' do + before do + stub_feature_flags(npm_allow_packages_in_multiple_projects: false) + end + + it 'returns matching package versions from only one project' do + subject + + expect(json_response['versions'].keys).to match_array([package2.version]) + end + end end end diff --git a/spec/support_specs/helpers/packages/npm_spec.rb b/spec/support_specs/helpers/packages/npm_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..e1316a10fb1f5dd76c959ff282f4426a0926722e --- /dev/null +++ b/spec/support_specs/helpers/packages/npm_spec.rb @@ -0,0 +1,133 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::API::Helpers::Packages::Npm, feature_category: :package_registry do # rubocop: disable RSpec/FilePath + let(:object) { klass.new(params) } + let(:klass) do + Struct.new(:params) do + include ::API::Helpers + include ::API::Helpers::Packages::Npm + end + end + + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:namespace) { group } + let_it_be(:project) { create(:project, :public, namespace: namespace) } + let_it_be(:package) { create(:npm_package, project: project) } + + describe '#endpoint_scope' do + subject { object.endpoint_scope } + + context 'when params includes an id' do + let(:params) { { id: 42, package_name: 'foo' } } + + it { is_expected.to eq(:project) } + end + + context 'when params does not include an id' do + let(:params) { { package_name: 'foo' } } + + it { is_expected.to eq(:instance) } + end + end + + describe '#finder_for_endpoint_scope' do + subject { object.finder_for_endpoint_scope(package_name) } + + let(:package_name) { package.name } + + context 'when called with project scope' do + let(:params) { { id: project.id } } + + it 'returns a PackageFinder for project scope' do + expect(::Packages::Npm::PackageFinder).to receive(:new).with(package_name, project: project) + + subject + end + end + + context 'when called with instance scope' do + let(:params) { { package_name: package_name } } + + it 'returns a PackageFinder for namespace scope' do + expect(::Packages::Npm::PackageFinder).to receive(:new).with(package_name, namespace: group) + + subject + end + end + end + + describe '#project_id_or_nil' do + subject { object.project_id_or_nil } + + context 'when called with project scope' do + let(:params) { { id: project.id } } + + it { is_expected.to eq(project.id) } + end + + context 'when called with namespace scope' do + context 'when given an unscoped name' do + let(:params) { { package_name: 'foo' } } + + it { is_expected.to eq(nil) } + end + + context 'when given a scope that does not match a group name' do + let(:params) { { package_name: '@nonexistent-group/foo' } } + + it { is_expected.to eq(nil) } + end + + context 'when given a scope that matches a group name' do + let(:params) { { package_name: package.name } } + + it { is_expected.to eq(project.id) } + + context 'with another package with the same name, in another project in the namespace' do + let_it_be(:project2) { create(:project, :public, namespace: namespace) } + let_it_be(:package2) { create(:npm_package, name: package.name, project: project2) } + + it 'returns the project id for the newest matching package within the scope' do + expect(subject).to eq(project2.id) + end + end + end + + context 'with npm_allow_packages_in_multiple_projects disabled' do + before do + stub_feature_flags(npm_allow_packages_in_multiple_projects: false) + end + + context 'when given an unscoped name' do + let(:params) { { package_name: 'foo' } } + + it { is_expected.to eq(nil) } + end + + context 'when given a scope that does not match a group name' do + let(:params) { { package_name: '@nonexistent-group/foo' } } + + it { is_expected.to eq(nil) } + end + + context 'when given a scope that matches a group name' do + let(:params) { { package_name: package.name } } + + it { is_expected.to eq(project.id) } + + context 'with another package with the same name, in another project in the namespace' do + let_it_be(:project2) { create(:project, :public, namespace: namespace) } + let_it_be(:package2) { create(:npm_package, name: package.name, project: project2) } + + it 'returns the project id for the newest matching package within the scope' do + expect(subject).to eq(project2.id) + end + end + end + end + end + end +end