Made changes based on review feedback

Add the optional 'v' in the version regex to allow version
that starts with v, for example, 'v1.2', example of this is added to the tests and documentation.
Also, reword the documentation with a more clear sentence

app/models/packages/package.rb

@@ -37,6 +37,7 @@ class Packages::Package < ApplicationRecord
   validates :name, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
   validates :version, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
   validates :version, format: { with: Gitlab::Regex.maven_version_regex }, if: -> { version? && maven? }
+  validates :version, format: { with: Gitlab::Regex.pypi_version_regex }, if: :pypi?
 
   enum package_type: { maven: 1, npm: 2, conan: 3, nuget: 4, pypi: 5, composer: 6 }
 

changelogs/unreleased/35080-add-validation-to-pypi-package-version.yml

+---
+title: Add validation to pypi package version
+merge_request: 35080
+author: Bola Ahmed Buari
+type: added

doc/user/packages/pypi_repository/index.md

@@ -207,6 +207,26 @@ When uploading packages, note that:
 - If you upload the same package with the same version multiple times, each consecutive upload
   is saved as a separate file. When installing a package, GitLab serves the most recent file.
 
+### Ensure your version string is valid
+
+If your version string (for example, `0.0.1`) is invalid, it will be rejected. GitLab uses the following regex to validate the version string.
+
+```ruby
+\A(?:
+    v?
+    (?:([0-9]+)!)?                                                 (?# epoch)
+    ([0-9]+(?:\.[0-9]+)*)                                          (?# release segment)
+    ([-_\.]?((a|b|c|rc|alpha|beta|pre|preview))[-_\.]?([0-9]+)?)?  (?# pre-release)
+    ((?:-([0-9]+))|(?:[-_\.]?(post|rev|r)[-_\.]?([0-9]+)?))?       (?# post release)
+    ([-_\.]?(dev)[-_\.]?([0-9]+)?)?                                (?# dev release)
+    (?:\+([a-z0-9]+(?:[-_\.][a-z0-9]+)*))?                         (?# local version)
+)\z}xi
+```
+
+You can play around with the regex and try your version strings on [this regular expression editor](https://rubular.com/r/FKM6d07ouoDaFV).
+
+For more details about the regex used, please check the [documentation here](https://www.python.org/dev/peps/pep-0440/#appendix-b-parsing-version-strings-with-regular-expressions))
+
 ### Upload packages with Twine
 
 If you were following the guide above, then the `MyPyPiPackage` package should

lib/gitlab/regex.rb

@@ -51,6 +51,21 @@ module Gitlab
         maven_app_name_regex
       end
 
+      def pypi_version_regex
+        # See the official regex: https://github.com/pypa/packaging/blob/16.7/packaging/version.py#L159
+
+        @pypi_version_regex ||= %r{
+          \A(?:
+            v?
+            (?:([0-9]+)!)?                                                 (?# epoch)
+            ([0-9]+(?:\.[0-9]+)*)                                          (?# release segment)
+            ([-_\.]?((a|b|c|rc|alpha|beta|pre|preview))[-_\.]?([0-9]+)?)?  (?# pre-release)
+            ((?:-([0-9]+))|(?:[-_\.]?(post|rev|r)[-_\.]?([0-9]+)?))?       (?# post release)
+            ([-_\.]?(dev)[-_\.]?([0-9]+)?)?                                (?# dev release)
+            (?:\+([a-z0-9]+(?:[-_\.][a-z0-9]+)*))?                         (?# local version)
+            )\z}xi.freeze
+      end
+
       def unbounded_semver_regex
         # See the official regex: https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string
 

spec/factories/packages.rb

@@ -56,14 +56,20 @@ FactoryBot.define do
     end
 
     factory :pypi_package do
-      pypi_metadatum
-
       sequence(:name) { |n| "pypi-package-#{n}"}
       sequence(:version) { |n| "1.0.#{n}" }
       package_type { :pypi }
 
-      after :create do |package|
+      transient do
+        without_loaded_metadatum { false }
+      end
+
+      after :create do |package, evaluator|
         create :package_file, :pypi, package: package, file_name: "#{package.name}-#{package.version}.tar.gz"
+
+        unless evaluator.without_loaded_metadatum
+          create :pypi_metadatum, package: package
+        end
       end
     end
 
@@ -297,7 +303,7 @@ FactoryBot.define do
   end
 
   factory :pypi_metadatum, class: 'Packages::Pypi::Metadatum' do
-    association :package, package_type: :pypi
+    package { create(:pypi_package, without_loaded_metadatum: true) }
     required_python { '>=2.7' }
   end
 

spec/lib/gitlab/regex_spec.rb

@@ -311,6 +311,73 @@ RSpec.describe Gitlab::Regex do
     it { is_expected.not_to match('%2e%2e%2f1.2.3') }
   end
 
+  describe '.pypi_version_regex' do
+    subject { described_class.pypi_version_regex }
+
+    it { is_expected.to match('0.1') }
+    it { is_expected.to match('2.0') }
+    it { is_expected.to match('1.2.0')}
+    it { is_expected.to match('0100!0.0') }
+    it { is_expected.to match('00!1.2') }
+    it { is_expected.to match('1.0a') }
+    it { is_expected.to match('1.0-a') }
+    it { is_expected.to match('1.0.a1') }
+    it { is_expected.to match('1.0a1') }
+    it { is_expected.to match('1.0-a1') }
+    it { is_expected.to match('1.0alpha1') }
+    it { is_expected.to match('1.0b1') }
+    it { is_expected.to match('1.0beta1') }
+    it { is_expected.to match('1.0rc1') }
+    it { is_expected.to match('1.0pre1') }
+    it { is_expected.to match('1.0preview1') }
+    it { is_expected.to match('1.0.dev1') }
+    it { is_expected.to match('1.0.DEV1') }
+    it { is_expected.to match('1.0.post1') }
+    it { is_expected.to match('1.0.rev1') }
+    it { is_expected.to match('1.0.r1') }
+    it { is_expected.to match('1.0c2') }
+    it { is_expected.to match('2012.15') }
+    it { is_expected.to match('1.0+5') }
+    it { is_expected.to match('1.0+abc.5') }
+    it { is_expected.to match('1!1.1') }
+    it { is_expected.to match('1.0c3') }
+    it { is_expected.to match('1.0rc2') }
+    it { is_expected.to match('1.0c1') }
+    it { is_expected.to match('1.0b2-346') }
+    it { is_expected.to match('1.0b2.post345') }
+    it { is_expected.to match('1.0b2.post345.dev456') }
+    it { is_expected.to match('1.2.rev33+123456') }
+    it { is_expected.to match('1.1.dev1') }
+    it { is_expected.to match('1.0b1.dev456') }
+    it { is_expected.to match('1.0a12.dev456') }
+    it { is_expected.to match('1.0b2') }
+    it { is_expected.to match('1.0.dev456') }
+    it { is_expected.to match('1.0c1.dev456') }
+    it { is_expected.to match('1.0.post456') }
+    it { is_expected.to match('1.0.post456.dev34') }
+    it { is_expected.to match('1.2+123abc') }
+    it { is_expected.to match('1.2+abc') }
+    it { is_expected.to match('1.2+abc123') }
+    it { is_expected.to match('1.2+abc123def') }
+    it { is_expected.to match('1.2+1234.abc') }
+    it { is_expected.to match('1.2+123456') }
+    it { is_expected.to match('1.2.r32+123456') }
+    it { is_expected.to match('1!1.2.rev33+123456') }
+    it { is_expected.to match('1.0a12') }
+    it { is_expected.to match('1.2.3-45+abcdefgh') }
+    it { is_expected.to match('v1.2.3') }
+    it { is_expected.not_to match('1.2.3-45-abcdefgh') }
+    it { is_expected.not_to match('..1.2.3') }
+    it { is_expected.not_to match('  1.2.3') }
+    it { is_expected.not_to match("1.2.3  \r\t") }
+    it { is_expected.not_to match("\r\t 1.2.3") }
+    it { is_expected.not_to match('1./2.3') }
+    it { is_expected.not_to match('1.2.3-4/../../') }
+    it { is_expected.not_to match('1.2.3-4%2e%2e%') }
+    it { is_expected.not_to match('../../../../../1.2.3') }
+    it { is_expected.not_to match('%2e%2e%2f1.2.3') }
+  end
+
   describe '.semver_regex' do
     subject { described_class.semver_regex }
 

spec/models/packages/package_spec.rb

@@ -169,6 +169,73 @@ RSpec.describe Packages::Package, type: :model do
         it { is_expected.not_to allow_value('%2e%2e%2f1.2.3').for(:version) }
       end
 
+      context 'pypi package' do
+        subject { create(:pypi_package) }
+
+        it { is_expected.to allow_value('0.1').for(:version) }
+        it { is_expected.to allow_value('2.0').for(:version) }
+        it { is_expected.to allow_value('1.2.0').for(:version) }
+        it { is_expected.to allow_value('0100!0.0').for(:version) }
+        it { is_expected.to allow_value('00!1.2').for(:version) }
+        it { is_expected.to allow_value('1.0a').for(:version) }
+        it { is_expected.to allow_value('1.0-a').for(:version) }
+        it { is_expected.to allow_value('1.0.a1').for(:version) }
+        it { is_expected.to allow_value('1.0a1').for(:version) }
+        it { is_expected.to allow_value('1.0-a1').for(:version) }
+        it { is_expected.to allow_value('1.0alpha1').for(:version) }
+        it { is_expected.to allow_value('1.0b1').for(:version) }
+        it { is_expected.to allow_value('1.0beta1').for(:version) }
+        it { is_expected.to allow_value('1.0rc1').for(:version) }
+        it { is_expected.to allow_value('1.0pre1').for(:version) }
+        it { is_expected.to allow_value('1.0preview1').for(:version) }
+        it { is_expected.to allow_value('1.0.dev1').for(:version) }
+        it { is_expected.to allow_value('1.0.DEV1').for(:version) }
+        it { is_expected.to allow_value('1.0.post1').for(:version) }
+        it { is_expected.to allow_value('1.0.rev1').for(:version) }
+        it { is_expected.to allow_value('1.0.r1').for(:version) }
+        it { is_expected.to allow_value('1.0c2').for(:version) }
+        it { is_expected.to allow_value('2012.15').for(:version) }
+        it { is_expected.to allow_value('1.0+5').for(:version) }
+        it { is_expected.to allow_value('1.0+abc.5').for(:version) }
+        it { is_expected.to allow_value('1!1.1').for(:version) }
+        it { is_expected.to allow_value('1.0c3').for(:version) }
+        it { is_expected.to allow_value('1.0rc2').for(:version) }
+        it { is_expected.to allow_value('1.0c1').for(:version) }
+        it { is_expected.to allow_value('1.0b2-346').for(:version) }
+        it { is_expected.to allow_value('1.0b2.post345').for(:version) }
+        it { is_expected.to allow_value('1.0b2.post345.dev456').for(:version) }
+        it { is_expected.to allow_value('1.2.rev33+123456').for(:version) }
+        it { is_expected.to allow_value('1.1.dev1').for(:version) }
+        it { is_expected.to allow_value('1.0b1.dev456').for(:version) }
+        it { is_expected.to allow_value('1.0a12.dev456').for(:version) }
+        it { is_expected.to allow_value('1.0b2').for(:version) }
+        it { is_expected.to allow_value('1.0.dev456').for(:version) }
+        it { is_expected.to allow_value('1.0c1.dev456').for(:version) }
+        it { is_expected.to allow_value('1.0.post456').for(:version) }
+        it { is_expected.to allow_value('1.0.post456.dev34').for(:version) }
+        it { is_expected.to allow_value('1.2+123abc').for(:version) }
+        it { is_expected.to allow_value('1.2+abc').for(:version) }
+        it { is_expected.to allow_value('1.2+abc123').for(:version) }
+        it { is_expected.to allow_value('1.2+abc123def').for(:version) }
+        it { is_expected.to allow_value('1.2+1234.abc').for(:version) }
+        it { is_expected.to allow_value('1.2+123456').for(:version) }
+        it { is_expected.to allow_value('1.2.r32+123456').for(:version) }
+        it { is_expected.to allow_value('1!1.2.rev33+123456').for(:version) }
+        it { is_expected.to allow_value('1.0a12').for(:version) }
+        it { is_expected.to allow_value('1.2.3-45+abcdefgh').for(:version) }
+        it { is_expected.to allow_value('v1.2.3').for(:version) }
+        it { is_expected.not_to allow_value('1.2.3-45-abcdefgh').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  \r\t").for(:version) }
+        it { is_expected.not_to allow_value("\r\t 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-4/../../').for(:version) }
+        it { is_expected.not_to allow_value('1.2.3-4%2e%2e%').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
+
       it_behaves_like 'validating version to be SemVer compliant for', :npm_package
       it_behaves_like 'validating version to be SemVer compliant for', :nuget_package
     end