Merge branch '214471-add-validation-to-conan-recipe' into 'master'

Add validation to Conan recipe that conforms with Conan.io

Closes #214471

See merge request gitlab-org/gitlab!29739

ee/app/models/packages/package.rb

@@ -20,10 +20,9 @@ class Packages::Package < ApplicationRecord
   delegate :recipe, :recipe_path, to: :conan_metadatum, prefix: :conan
 
   validates :project, presence: true
+  validates :name, presence: true
 
-  validates :name,
-    presence: true,
-    format: { with: Gitlab::Regex.package_name_regex }
+  validates :name, format: { with: Gitlab::Regex.package_name_regex }, unless: :conan?
 
   validates :name,
     uniqueness: { scope: %i[project_id version package_type] }, unless: :conan?
@@ -32,6 +31,8 @@ class Packages::Package < ApplicationRecord
   validate :valid_npm_package_name, if: :npm?
   validate :package_already_taken, if: :npm?
   validates :version, format: { with: Gitlab::Regex.semver_regex }, if: :npm?
+  validates :name, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
+  validates :version, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
 
   enum package_type: { maven: 1, npm: 2, conan: 3, nuget: 4, pypi: 5, composer: 6 }
 

ee/changelogs/unreleased/214471-add-validation-to-conan-recipe.yml

+---
+title: Add validation to Conan recipe that conforms with Conan.io
+merge_request: 29739
+author: Bola Ahmed Buari
+type: fixed

ee/lib/ee/gitlab/regex.rb

@@ -20,7 +20,7 @@ module EE
         end
 
         def conan_recipe_component_regex
-          @conan_recipe_component_regex ||= %r{\A(\w[.+-]?)+\z}.freeze
+          @conan_recipe_component_regex ||= %r{\A[a-zA-Z0-9_][a-zA-Z0-9_\+\.-]{1,49}\z}.freeze
         end
 
         def package_name_regex

ee/spec/lib/gitlab/regex_spec.rb

@@ -33,13 +33,20 @@ describe Gitlab::Regex do
   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) }
   end
 
   describe '.feature_flag_regex' do

ee/spec/models/packages/conan_metadatum_spec.rb

@@ -8,12 +8,22 @@ RSpec.describe Packages::ConanMetadatum, type: :model do
   end
 
   describe 'validations' do
+    let(:fifty_one_characters) { 'f_a' * 17}
+
     it { is_expected.to validate_presence_of(:package) }
     it { is_expected.to validate_presence_of(:package_username) }
     it { is_expected.to validate_presence_of(:package_channel) }
 
     describe '#package_username' do
       it { is_expected.to allow_value("my-package+username").for(:package_username) }
+      it { is_expected.to allow_value("my_package.username").for(:package_username) }
+      it { is_expected.to allow_value("_my-package.username123").for(:package_username) }
+      it { is_expected.to allow_value("my").for(:package_username) }
+      it { is_expected.not_to allow_value('+my_package').for(:package_username) }
+      it { is_expected.not_to allow_value('.my_package').for(:package_username) }
+      it { is_expected.not_to allow_value('-my_package').for(:package_username) }
+      it { is_expected.not_to allow_value('m').for(:package_username) }
+      it { is_expected.not_to allow_value(fifty_one_characters).for(:package_username) }
       it { is_expected.not_to allow_value("my/package").for(:package_username) }
       it { is_expected.not_to allow_value("my(package)").for(:package_username) }
       it { is_expected.not_to allow_value("my@package").for(:package_username) }
@@ -22,6 +32,14 @@ RSpec.describe Packages::ConanMetadatum, type: :model do
     describe '#package_channel' do
       it { is_expected.to allow_value("beta").for(:package_channel) }
       it { is_expected.to allow_value("stable+1.0").for(:package_channel) }
+      it { is_expected.to allow_value("my").for(:package_channel) }
+      it { is_expected.to allow_value("my_channel.beta").for(:package_channel) }
+      it { is_expected.to allow_value("_my-channel.beta123").for(:package_channel) }
+      it { is_expected.not_to allow_value('+my_channel').for(:package_channel) }
+      it { is_expected.not_to allow_value('.my_channel').for(:package_channel) }
+      it { is_expected.not_to allow_value('-my_channel').for(:package_channel) }
+      it { is_expected.not_to allow_value('m').for(:package_channel) }
+      it { is_expected.not_to allow_value(fifty_one_characters).for(:package_channel) }
       it { is_expected.not_to allow_value("my/channel").for(:package_channel) }
       it { is_expected.not_to allow_value("my(channel)").for(:package_channel) }
       it { is_expected.not_to allow_value("my@channel").for(:package_channel) }

ee/spec/models/packages/package_spec.rb

@@ -78,6 +78,20 @@ RSpec.describe Packages::Package, type: :model do
       it { is_expected.to allow_value("my/domain/com/my-app").for(:name) }
       it { is_expected.to allow_value("my.app-11.07.2018").for(:name) }
       it { is_expected.not_to allow_value("my(dom$$$ain)com.my-app").for(:name) }
+
+      context 'conan package' do
+        subject { create(:conan_package) }
+
+        let(:fifty_one_characters) {'f_b' * 17}
+
+        it { is_expected.to allow_value('foo+bar').for(:name) }
+        it { is_expected.to allow_value('foo_bar').for(:name) }
+        it { is_expected.to allow_value('foo.bar').for(:name) }
+        it { is_expected.not_to allow_value(fifty_one_characters).for(:name) }
+        it { is_expected.not_to allow_value('+foobar').for(:name) }
+        it { is_expected.not_to allow_value('.foobar').for(:name) }
+        it { is_expected.not_to allow_value('%foo%bar').for(:name) }
+      end
     end
 
     describe '#version' do
@@ -93,6 +107,22 @@ RSpec.describe Packages::Package, type: :model do
         it { is_expected.not_to allow_value('../../../../../1.2.3').for(:version) }
         it { is_expected.not_to allow_value('%2e%2e%2f1.2.3').for(:version) }
       end
+
+      context 'conan package' do
+        subject { create(:conan_package) }
+
+        let(:fifty_one_characters) {'1.2' * 17}
+
+        it { is_expected.to allow_value('1.2').for(:version) }
+        it { is_expected.to allow_value('1.2.3-beta').for(:version) }
+        it { is_expected.to allow_value('1.2.3-pre1+build2').for(:version) }
+        it { is_expected.not_to allow_value('1').for(:version) }
+        it { is_expected.not_to allow_value(fifty_one_characters).for(:version) }
+        it { is_expected.not_to allow_value('1./2.3').for(:version) }
+        it { is_expected.not_to allow_value('.1.2.3').for(:version) }
+        it { is_expected.not_to allow_value('+1.2.3').for(:version) }
+        it { is_expected.not_to allow_value('%2e%2e%2f1.2.3').for(:version) }
+      end
     end
 
     describe '#package_already_taken' do

ee/spec/services/packages/npm/create_tag_service_spec.rb

@@ -31,7 +31,7 @@ describe Packages::Npm::CreateTagService do
       end
 
       context 'on different package type' do
-        let!(:package2) { create(:conan_package, project: package.project, name: package.name, version: package.version) }
+        let!(:package2) { create(:conan_package, project: package.project, name: 'conan_package_name', version: package.version) }
 
         it_behaves_like 'it creates the tag'
       end