From b51d643f4b0a3ff1c72a2600578b9a0f6ca3268d Mon Sep 17 00:00:00 2001
From: Massimiliano Adamo <maxadamo@gmail.com>
Date: Sun, 1 Aug 2021 15:39:20 +0200
Subject: [PATCH] simplify packages version detection

---
 manifests/pip.pp            | 17 +++++++++--------
 spec/acceptance/pip_spec.rb | 31 +++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/manifests/pip.pp b/manifests/pip.pp
index 25856fc7..eaada706 100644
--- a/manifests/pip.pp
+++ b/manifests/pip.pp
@@ -155,11 +155,13 @@
     $_ensure      = $ensure
   }
 
-  # Check if searching by explicit version.
-  if $_ensure =~ /^((19|20)[0-9][0-9]-(0[1-9]|1[1-2])-([0-2][1-9]|3[0-1])|[0-9]+\.\w+\+?\w*(\.\w+)*)$/ {
-    $grep_regex = "^${real_pkgname}[[:space:]]\\+(\\?${_ensure}\\()$\\|$\\|, \\|[[:space:]]\\)"
-  } else {
-    $grep_regex = "^${real_pkgname}[[:space:]].*$"
+  # We do not try to mimic a version scheme validation which is already implemented by the package manager.
+  # If it starts with a number it is probably a version.
+  # If it wasn't or if there is any error, the package manager will trigger a failure.
+  $grep_regex = $_ensure ? {
+    /^(present|absent|latest)$/ => "^${real_pkgname}[[:space:]].*$",
+    /^[0-9].*$/                 => "^${real_pkgname}[[:space:]]\\+(\\?${_ensure}\\()$\\|$\\|, \\|[[:space:]]\\)",
+    default                     => fail('ensure can be a version number or one of: present, absent, latest')
   }
 
   $extras_string = empty($extras) ? {
@@ -193,9 +195,8 @@
     }
   } else {
     case $_ensure {
-      /^((19|20)[0-9][0-9]-(0[1-9]|1[1-2])-([0-2][1-9]|3[0-1])|[0-9]+\.\w+\+?\w*(\.\w+)*)$/: {
-        # Version formats as per http://guide.python-distribute.org/specification.html#standard-versioning-schemes
-        # Explicit version.
+      /^[0-9].*$/: {
+        # Specific version
         $command        = "${pip_install} ${install_args} ${pip_common_args}==${_ensure}"
         $unless_command = "${pip_env} list | grep -i -e '${grep_regex}'"
       }
diff --git a/spec/acceptance/pip_spec.rb b/spec/acceptance/pip_spec.rb
index 9da96bef..5b64c6de 100644
--- a/spec/acceptance/pip_spec.rb
+++ b/spec/acceptance/pip_spec.rb
@@ -74,6 +74,37 @@ class { 'python':
     its(:stdout) { is_expected.not_to match %r{agent.* 0\.1\.2} }
   end
 
+  context 'fails to install package with wrong version' do
+    it 'throws an error' do
+      pp = <<-PUPPET
+      class { 'python':
+        version => '3',
+        dev     => 'present',
+      }
+
+      python::pyvenv { '/opt/test-venv':
+        ensure      => 'present',
+        systempkgs  => false,
+        mode        => '0755',
+        pip_version => '<= 20.3.4',
+      }
+
+      python::pip { 'agent package':
+        virtualenv => '/opt/test-venv',
+        pkgname    => 'agent',
+        ensure     => '0.1.33+2020-this_is_something-fun',
+      }
+      PUPPET
+
+      result = apply_manifest(pp, expect_failures: true)
+      expect(result.stderr).to contain(%r{returned 1 instead of one of})
+    end
+  end
+
+  describe command('/opt/test-venv/bin/pip show agent') do
+    its(:exit_status) { is_expected.to eq 1 }
+  end
+
   context 'install package via extra_index' do
     it 'works with no errors' do
       pp = <<-PUPPET