Merge branch '345055_conan_support_underscore_for_username_and_channel' into 'master'

Support blank values in Conan packages for username and channel

See merge request gitlab-org/gitlab!75106

app/models/packages/conan/metadatum.rb

 # frozen_string_literal: true
 
 class Packages::Conan::Metadatum < ApplicationRecord
+  NONE_VALUE = '_'
+
   belongs_to :package, -> { where(package_type: :conan) }, inverse_of: :conan_metadatum
 
   validates :package, presence: true
 
   validates :package_username,
-    presence: true,
-    format: { with: Gitlab::Regex.conan_recipe_component_regex }
+            :package_channel,
+            presence: true,
+            format: { with: Gitlab::Regex.conan_recipe_component_regex },
+            if: -> { Feature.disabled?(:packages_conan_allow_empty_username_channel) }
 
-  validates :package_channel,
-    presence: true,
-    format: { with: Gitlab::Regex.conan_recipe_component_regex }
+  validates :package_username,
+            :package_channel,
+            presence: true,
+            format: { with: Gitlab::Regex.conan_recipe_user_channel_regex },
+            if: -> { Feature.enabled?(:packages_conan_allow_empty_username_channel) }
 
   validate :conan_package_type
+  validate :username_channel_none_values, if: -> { Feature.enabled?(:packages_conan_allow_empty_username_channel) }
 
   def recipe
     "#{package.name}/#{package.version}@#{package_username}/#{package_channel}"
@@ -31,6 +38,15 @@ class Packages::Conan::Metadatum < ApplicationRecord
     package_username.tr('+', '/')
   end
 
+  def self.validate_username_and_channel(username, channel)
+    return if (username != NONE_VALUE && channel != NONE_VALUE) ||
+              (username == NONE_VALUE && channel == NONE_VALUE)
+
+    none_field = username == NONE_VALUE ? :username : :channel
+
+    yield(none_field)
+  end
+
   private
 
   def conan_package_type
@@ -38,4 +54,10 @@ class Packages::Conan::Metadatum < ApplicationRecord
       errors.add(:base, _('Package type must be Conan'))
     end
   end
+
+  def username_channel_none_values
+    self.class.validate_username_and_channel(package_username, package_channel) do |none_field|
+      errors.add("package_#{none_field}".to_sym, _("can't be solely blank"))
+    end
+  end
 end

config/feature_flags/development/packages_conan_allow_empty_username_channel.yml

+---
+name: packages_conan_allow_empty_username_channel
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/75106
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/346006
+milestone: '14.5'
+type: development
+group: group::package
+default_enabled: false

config/initializers/grape_validators.rb

@@ -11,3 +11,6 @@ Grape::Validations.register_validator(:untrusted_regexp, ::API::Validations::Val
 Grape::Validations.register_validator(:email_or_email_list, ::API::Validations::Validators::EmailOrEmailList)
 Grape::Validations.register_validator(:iteration_id, ::API::Validations::Validators::IntegerOrCustomValue)
 Grape::Validations.register_validator(:project_portable, ::API::Validations::Validators::ProjectPortable)
+# TODO Delete this validator along with the packages_conan_allow_empty_username_channel feature flag
+# # https://gitlab.com/gitlab-org/gitlab/-/issues/346006
+Grape::Validations.register_validator('packages_conan_user_channel', ::API::Validations::Validators::PackagesConanUserChannel)

doc/user/packages/conan_repository/index.md

@@ -103,6 +103,30 @@ A package with the recipe `Hello/0.1@mycompany/beta` is created.
 For more details about creating and managing Conan packages, see the
 [Conan documentation](https://docs.conan.io/en/latest/creating_packages.html).
 
+#### Package without a username and a channel
+
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/345055) in GitLab 14.6 [with a flag](../../../administration/feature_flags.md) named `packages_conan_allow_empty_username_channel`. Disabled by default.
+> - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/345055) in GitLab 14.6.
+
+Even though they are [recommended](https://docs.conan.io/en/latest/reference/conanfile/attributes.html#user-channel)
+to distinguish your package from a similarly named existing package,
+the username and channel are not mandatory fields for a Conan package.
+
+You can create a package without a username and channel by removing them from
+the `create` command:
+
+```shell
+conan create .
+```
+
+The username _and_ the channel must be blank. If only one of these fields is
+blank, the request is rejected.
+
+NOTE:
+Empty usernames and channels can only be used if you use a [project remote](#add-a-remote-for-your-project).
+If you use an [instance remote](#add-a-remote-for-your-instance), the username
+and the channel must be set.
+
 ## Add the Package Registry as a Conan remote
 
 To run `conan` commands, you must add the Package Registry as a Conan remote for

lib/api/concerns/packages/conan_endpoints.rb

@@ -105,10 +105,14 @@ module API
           params do
             requires :package_name, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package name'
             requires :package_version, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package version'
-            requires :package_username, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package username'
-            requires :package_channel, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package channel'
+            requires :package_username, type: String, packages_conan_user_channel: true, desc: 'Package username'
+            requires :package_channel, type: String, packages_conan_user_channel: true, desc: 'Package channel'
           end
           namespace 'conans/:package_name/:package_version/:package_username/:package_channel', requirements: PACKAGE_REQUIREMENTS do
+            after_validation do
+              check_username_channel if Feature.enabled?(:packages_conan_allow_empty_username_channel)
+            end
+
             # Get the snapshot
             #
             # the snapshot is a hash of { filename: md5 hash }
@@ -264,8 +268,8 @@ module API
           params do
             requires :package_name, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package name'
             requires :package_version, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package version'
-            requires :package_username, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package username'
-            requires :package_channel, type: String, regexp: PACKAGE_COMPONENT_REGEX, desc: 'Package channel'
+            requires :package_username, type: String, packages_conan_user_channel: true, desc: 'Package username'
+            requires :package_channel, type: String, packages_conan_user_channel: true, desc: 'Package channel'
             requires :recipe_revision, type: String, regexp: CONAN_REVISION_REGEX, desc: 'Conan Recipe Revision'
           end
           namespace 'files/:package_name/:package_version/:package_username/:package_channel/:recipe_revision', requirements: PACKAGE_REQUIREMENTS do
@@ -273,6 +277,10 @@ module API
               authenticate_non_get!
             end
 
+            after_validation do
+              check_username_channel if Feature.enabled?(:packages_conan_allow_empty_username_channel)
+            end
+
             params do
               requires :file_name, type: String, desc: 'Package file name', values: CONAN_FILES
             end

lib/api/helpers/packages/conan/api_helpers.rb

@@ -7,6 +7,21 @@ module API
         module ApiHelpers
           include Gitlab::Utils::StrongMemoize
 
+          def check_username_channel
+            username = declared(params)[:package_username]
+            channel = declared(params)[:package_channel]
+
+            if username == ::Packages::Conan::Metadatum::NONE_VALUE && package_scope == :instance
+              # at the instance level, username must not be empty (naming convention)
+              # don't try to process the empty username and eagerly return not found.
+              not_found!
+            end
+
+            ::Packages::Conan::Metadatum.validate_username_and_channel(username, channel) do |none_field|
+              bad_request!("#{none_field} can't be solely blank")
+            end
+          end
+
           def present_download_urls(entity)
             authorize!(:read_package, project)
 

lib/api/validations/validators/packages_conan_user_channel.rb

+# frozen_string_literal: true
+
+module API
+  module Validations
+    module Validators
+      # TODO Delete this validator along with the packages_conan_allow_empty_username_channel feature flag
+      # Use a regexp validator in its place: regexp: Gitlab::Regex.conan_recipe_user_channel_regex
+      # https://gitlab.com/gitlab-org/gitlab/-/issues/346006
+      class PackagesConanUserChannel < Grape::Validations::Base
+        def validate_param!(attr_name, params)
+          value = params[attr_name]
+
+          if Feature.enabled?(:packages_conan_allow_empty_username_channel)
+            unless Gitlab::Regex.conan_recipe_user_channel_regex.match?(value)
+              raise Grape::Exceptions::Validation.new(
+                params: [@scope.full_name(attr_name)],
+                message: 'is invalid'
+              )
+            end
+          else
+            unless Gitlab::Regex.conan_recipe_component_regex.match?(value)
+              raise Grape::Exceptions::Validation.new(
+                params: [@scope.full_name(attr_name)],
+                message: 'is invalid'
+              )
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/regex.rb

@@ -16,8 +16,13 @@ module Gitlab
         @conan_revision_regex ||= %r{\A0\z}.freeze
       end
 
+      def conan_recipe_user_channel_regex
+        %r{\A(_|#{conan_name_regex})\z}.freeze
+      end
+
       def conan_recipe_component_regex
-        @conan_recipe_component_regex ||= %r{\A[a-zA-Z0-9_][a-zA-Z0-9_\+\.-]{1,49}\z}.freeze
+        # https://docs.conan.io/en/latest/reference/conanfile/attributes.html#name
+        @conan_recipe_component_regex ||= %r{\A#{conan_name_regex}\z}.freeze
       end
 
       def composer_package_version_regex
@@ -211,6 +216,12 @@ module Gitlab
       def generic_package_file_name_regex
         generic_package_name_regex
       end
+
+      private
+
+      def conan_name_regex
+        @conan_name_regex ||= %r{[a-zA-Z0-9_][a-zA-Z0-9_\+\.-]{1,49}}.freeze
+      end
     end
 
     extend self

locale/gitlab.pot

@@ -40578,6 +40578,9 @@ msgstr ""
 msgid "can't be nil"
 msgstr ""
 
+msgid "can't be solely blank"
+msgstr ""
+
 msgid "can't be the same as the source project"
 msgstr ""
 

spec/lib/api/validations/validators/packages_conan_user_channel_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe API::Validations::Validators::PackagesConanUserChannel do
+  include ApiValidatorsHelpers
+
+  describe '#validate_param!' do
+    subject do
+      described_class.new(['test'], {}, false, scope.new)
+    end
+
+    shared_examples 'accepting valid values for conan user channels' do
+      let(:fifty_one_characters) { 'f_a' * 17}
+
+      it { expect_no_validation_error('test' => 'foobar') }
+      it { expect_no_validation_error('test' => 'foo_bar') }
+      it { expect_no_validation_error('test' => 'foo+bar') }
+      it { expect_no_validation_error('test' => '_foo+bar-baz+1.0') }
+      it { expect_no_validation_error('test' => '1.0.0') }
+      it { expect_validation_error('test' => '-foo_bar') }
+      it { expect_validation_error('test' => '+foo_bar') }
+      it { expect_validation_error('test' => '.foo_bar') }
+      it { expect_validation_error('test' => 'foo@bar') }
+      it { expect_validation_error('test' => 'foo/bar') }
+      it { expect_validation_error('test' => '!!()()') }
+      it { expect_validation_error('test' => fifty_one_characters) }
+    end
+
+    it_behaves_like 'accepting valid values for conan user channels'
+    it { expect_no_validation_error('test' => '_') }
+
+    context 'with packages_conan_allow_empty_username_channel disabled' do
+      before do
+        stub_feature_flags(packages_conan_allow_empty_username_channel: false)
+      end
+
+      it_behaves_like 'accepting valid values for conan user channels'
+      it { expect_validation_error('test' => '_') }
+    end
+  end
+end

spec/lib/gitlab/regex_spec.rb

@@ -264,23 +264,37 @@ RSpec.describe Gitlab::Regex do
     it { is_expected.not_to match('1.2.3') }
   end
 
-  describe '.conan_recipe_component_regex' do
-    subject { described_class.conan_recipe_component_regex }
-
-    let(:fifty_one_characters) { 'f_a' * 17}
-
-    it { is_expected.to match('foobar') }
-    it { is_expected.to match('foo_bar') }
-    it { is_expected.to match('foo+bar') }
-    it { is_expected.to match('_foo+bar-baz+1.0') }
-    it { is_expected.to match('1.0.0') }
-    it { is_expected.not_to match('-foo_bar') }
-    it { is_expected.not_to match('+foo_bar') }
-    it { is_expected.not_to match('.foo_bar') }
-    it { is_expected.not_to match('foo@bar') }
-    it { is_expected.not_to match('foo/bar') }
-    it { is_expected.not_to match('!!()()') }
-    it { is_expected.not_to match(fifty_one_characters) }
+  context 'conan recipe components' do
+    shared_examples 'accepting valid recipe components values' do
+      let(:fifty_one_characters) { 'f_a' * 17}
+
+      it { is_expected.to match('foobar') }
+      it { is_expected.to match('foo_bar') }
+      it { is_expected.to match('foo+bar') }
+      it { is_expected.to match('_foo+bar-baz+1.0') }
+      it { is_expected.to match('1.0.0') }
+      it { is_expected.not_to match('-foo_bar') }
+      it { is_expected.not_to match('+foo_bar') }
+      it { is_expected.not_to match('.foo_bar') }
+      it { is_expected.not_to match('foo@bar') }
+      it { is_expected.not_to match('foo/bar') }
+      it { is_expected.not_to match('!!()()') }
+      it { is_expected.not_to match(fifty_one_characters) }
+    end
+
+    describe '.conan_recipe_component_regex' do
+      subject { described_class.conan_recipe_component_regex }
+
+      it_behaves_like 'accepting valid recipe components values'
+      it { is_expected.not_to match('_') }
+    end
+
+    describe '.conan_recipe_user_channel_regex' do
+      subject { described_class.conan_recipe_user_channel_regex }
+
+      it_behaves_like 'accepting valid recipe components values'
+      it { is_expected.to match('_') }
+    end
   end
 
   describe '.package_name_regex' do

spec/models/packages/conan/metadatum_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Packages::Conan::Metadatum, type: :model do
+  using RSpec::Parameterized::TableSyntax
+
   describe 'relationships' do
     it { is_expected.to belong_to(:package) }
   end
@@ -45,6 +47,36 @@ RSpec.describe Packages::Conan::Metadatum, type: :model do
       it { is_expected.not_to allow_value("my@channel").for(:package_channel) }
     end
 
+    describe '#username_channel_none_values' do
+      let_it_be(:package) { create(:conan_package) }
+
+      let(:metadatum) { package.conan_metadatum }
+
+      subject { metadatum.valid? }
+
+      where(:username, :channel, :feature_flag, :valid) do
+        'username' | 'channel' | true  | true
+        'username' | '_'       | true  | false
+        '_'        | 'channel' | true  | false
+        '_'        | '_'       | true  | true
+
+        'username' | 'channel' | false | true
+        'username' | '_'       | false | false
+        '_'        | 'channel' | false | false
+        '_'        | '_'       | false | false
+      end
+
+      with_them do
+        before do
+          metadatum.package_username = username
+          metadatum.package_channel = channel
+          stub_feature_flags(packages_conan_allow_empty_username_channel: feature_flag)
+        end
+
+        it { is_expected.to eq(valid) }
+      end
+    end
+
     describe '#conan_package_type' do
       it 'will not allow a package with a different package_type' do
         package = build('package')
@@ -87,4 +119,27 @@ RSpec.describe Packages::Conan::Metadatum, type: :model do
       expect(described_class.full_path_from(package_username: username)).to eq('foo/bar/baz-buz')
     end
   end
+
+  describe '.validate_username_and_channel' do
+    where(:username, :channel, :error_field) do
+      'username' | 'channel' | nil
+      'username' | '_'       | :channel
+      '_'        | 'channel' | :username
+      '_'        | '_'       | nil
+    end
+
+    with_them do
+      if params[:error_field]
+        it 'yields the block when there is an error' do
+          described_class.validate_username_and_channel(username, channel) do |none_field|
+            expect(none_field).to eq(error_field)
+          end
+        end
+      else
+        it 'does not yield the block when there is no error' do
+          expect { |b| described_class.validate_username_and_channel(username, channel, &b) }.not_to yield_control
+        end
+      end
+    end
+  end
 end

spec/requests/api/conan_project_packages_spec.rb

 # frozen_string_literal: true
 require 'spec_helper'
 
-RSpec.describe API::ConanProjectPackages, quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/326194' do
+RSpec.describe API::ConanProjectPackages do
   include_context 'conan api setup'
 
   let(:project_id) { project.id }
+  let(:snowplow_standard_context_params) { { user: user, project: project, namespace: project.namespace } }
 
   describe 'GET /api/v4/projects/:id/packages/conan/v1/ping' do
     let(:url) { "/projects/#{project.id}/packages/conan/v1/ping" }
@@ -92,7 +93,7 @@ RSpec.describe API::ConanProjectPackages, quarantine: 'https://gitlab.com/gitlab
     end
   end
 
-  context 'file download endpoints' do
+  context 'file download endpoints', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/326194' do
     include_context 'conan file download endpoints'
 
     describe 'GET /api/v4/projects/:id/packages/conan/v1/files/:package_name/package_version/:package_username/:package_channel/

spec/support/shared_examples/requests/api/conan_packages_shared_examples.rb

@@ -178,6 +178,60 @@ RSpec.shared_examples 'rejects invalid recipe' do
   end
 end
 
+RSpec.shared_examples 'handling empty values for username and channel' do
+  using RSpec::Parameterized::TableSyntax
+
+  let(:recipe_path) { "#{package.name}/#{package.version}/#{package_username}/#{channel}" }
+
+  where(:username, :channel, :feature_flag, :status) do
+    'username' | 'channel' | true  | :ok
+    'username' | '_'       | true  | :bad_request
+    '_'        | 'channel' | true  | :bad_request_or_not_found
+    '_'        | '_'       | true  | :ok_or_not_found
+
+    'username' | 'channel' | false | :ok
+    'username' | '_'       | false | :bad_request
+    '_'        | 'channel' | false | :bad_request
+    '_'        | '_'       | false | :bad_request
+  end
+
+  with_them do
+    let(:package_username) do
+      if username == 'username'
+        package.conan_metadatum.package_username
+      else
+        username
+      end
+    end
+
+    before do
+      stub_feature_flags(packages_conan_allow_empty_username_channel: feature_flag)
+      project.add_maintainer(user) # avoid any permission issue
+    end
+
+    it 'returns the correct status code' do |example|
+      project_level = example.full_description.include?('api/v4/projects')
+
+      expected_status = case status
+                        when :ok_or_not_found
+                          project_level ? :ok : :not_found
+                        when :bad_request_or_not_found
+                          project_level ? :bad_request : :not_found
+                        else
+                          status
+                        end
+
+      if expected_status == :ok
+        package.conan_metadatum.update!(package_username: package_username, package_channel: channel)
+      end
+
+      subject
+
+      expect(response).to have_gitlab_http_status(expected_status)
+    end
+  end
+end
+
 RSpec.shared_examples 'rejects invalid file_name' do |invalid_file_name|
   let(:file_name) { invalid_file_name }
 
@@ -300,6 +354,7 @@ RSpec.shared_examples 'recipe snapshot endpoint' do
   it_behaves_like 'rejects invalid recipe'
   it_behaves_like 'rejects recipe for invalid project'
   it_behaves_like 'empty recipe for not found package'
+  it_behaves_like 'handling empty values for username and channel'
 
   context 'with existing package' do
     it 'returns a hash of files with their md5 hashes' do
@@ -324,6 +379,7 @@ RSpec.shared_examples 'package snapshot endpoint' do
   it_behaves_like 'rejects invalid recipe'
   it_behaves_like 'rejects recipe for invalid project'
   it_behaves_like 'empty recipe for not found package'
+  it_behaves_like 'handling empty values for username and channel'
 
   context 'with existing package' do
     it 'returns a hash of md5 values for the files' do
@@ -344,12 +400,14 @@ RSpec.shared_examples 'recipe download_urls endpoint' do
   it_behaves_like 'rejects invalid recipe'
   it_behaves_like 'rejects recipe for invalid project'
   it_behaves_like 'recipe download_urls'
+  it_behaves_like 'handling empty values for username and channel'
 end
 
 RSpec.shared_examples 'package download_urls endpoint' do
   it_behaves_like 'rejects invalid recipe'
   it_behaves_like 'rejects recipe for invalid project'
   it_behaves_like 'package download_urls'
+  it_behaves_like 'handling empty values for username and channel'
 end
 
 RSpec.shared_examples 'recipe upload_urls endpoint' do
@@ -362,6 +420,7 @@ RSpec.shared_examples 'recipe upload_urls endpoint' do
 
   it_behaves_like 'rejects invalid recipe'
   it_behaves_like 'rejects invalid upload_url params'
+  it_behaves_like 'handling empty values for username and channel'
 
   it 'returns a set of upload urls for the files requested' do
     subject
@@ -423,6 +482,7 @@ RSpec.shared_examples 'package upload_urls endpoint' do
 
   it_behaves_like 'rejects invalid recipe'
   it_behaves_like 'rejects invalid upload_url params'
+  it_behaves_like 'handling empty values for username and channel'
 
   it 'returns a set of upload urls for the files requested' do
     expected_response = {
@@ -458,6 +518,7 @@ RSpec.shared_examples 'delete package endpoint' do
   let(:recipe_path) { package.conan_recipe_path }
 
   it_behaves_like 'rejects invalid recipe'
+  it_behaves_like 'handling empty values for username and channel'
 
   it 'returns unauthorized for users without valid permission' do
     subject
@@ -568,12 +629,14 @@ RSpec.shared_examples 'recipe file download endpoint' do
   it_behaves_like 'a public project with packages'
   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'
 end
 
 RSpec.shared_examples 'package file download endpoint' do
   it_behaves_like 'a public project with packages'
   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'
 
   context 'tracking the conan_package.tgz download' do
     let(:package_file) { package.package_files.find_by(file_name: ::Packages::Conan::FileMetadatum::PACKAGE_BINARY) }
@@ -598,6 +661,7 @@ RSpec.shared_examples 'workhorse authorize endpoint' do
   it_behaves_like 'rejects invalid recipe'
   it_behaves_like 'rejects invalid file_name', 'conanfile.py.git%2fgit-upload-pack'
   it_behaves_like 'workhorse authorization'
+  it_behaves_like 'handling empty values for username and channel'
 end
 
 RSpec.shared_examples 'workhorse recipe file upload endpoint' do
@@ -619,6 +683,7 @@ RSpec.shared_examples 'workhorse recipe file upload endpoint' do
   it_behaves_like 'rejects invalid file_name', 'conanfile.py.git%2fgit-upload-pack'
   it_behaves_like 'uploads a package file'
   it_behaves_like 'creates build_info when there is a job'
+  it_behaves_like 'handling empty values for username and channel'
 end
 
 RSpec.shared_examples 'workhorse package file upload endpoint' do
@@ -640,6 +705,7 @@ RSpec.shared_examples 'workhorse package file upload endpoint' do
   it_behaves_like 'rejects invalid file_name', 'conaninfo.txttest'
   it_behaves_like 'uploads a package file'
   it_behaves_like 'creates build_info when there is a job'
+  it_behaves_like 'handling empty values for username and channel'
 
   context 'tracking the conan_package.tgz upload' do
     let(:file_name) { ::Packages::Conan::FileMetadatum::PACKAGE_BINARY }