From 71e33d25784ba9ba1c9f338faec64f2b346c937e Mon Sep 17 00:00:00 2001
From: Sam Doran <sdoran@redhat.com>
Date: Thu, 20 May 2021 11:55:30 -0400
Subject: [PATCH] version test - improve message when value is empty (#74754)

When an empty value is provided, no `version` attribute will exist on the `LooseVersion` or
`StrictVersion` object. We catch and handle this, but it's not immediatebly clear that an
AttributeError means an empty value was provided.

Specifically handle the case where value or version are empty and add more
helpful error messages.

Add integration tests.
---
 .../version_compare-error-on-empty.yml        |  2 ++
 lib/ansible/plugins/test/core.py              |  6 +++++
 .../targets/test_core/tasks/main.yml          | 26 ++++++++++++++++++-
 3 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/fragments/version_compare-error-on-empty.yml

diff --git a/changelogs/fragments/version_compare-error-on-empty.yml b/changelogs/fragments/version_compare-error-on-empty.yml
new file mode 100644
index 00000000000..2c26a8e8b22
--- /dev/null
+++ b/changelogs/fragments/version_compare-error-on-empty.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - version test - improve error message when an empty version is provided
diff --git a/lib/ansible/plugins/test/core.py b/lib/ansible/plugins/test/core.py
index e131dc01e10..8609afabbba 100644
--- a/lib/ansible/plugins/test/core.py
+++ b/lib/ansible/plugins/test/core.py
@@ -168,6 +168,12 @@ def version_compare(value, version, operator='eq', strict=None, version_type=Non
     if strict is not None and version_type is not None:
         raise errors.AnsibleFilterError("Cannot specify both 'strict' and 'version_type'")
 
+    if not value:
+        raise errors.AnsibleFilterError("Input version value cannot be empty")
+
+    if not version:
+        raise errors.AnsibleFilterError("Version parameter to compare against cannot be empty")
+
     Version = LooseVersion
     if strict:
         Version = StrictVersion
diff --git a/test/integration/targets/test_core/tasks/main.yml b/test/integration/targets/test_core/tasks/main.yml
index 0ae4da67354..9f753e3cb92 100644
--- a/test/integration/targets/test_core/tasks/main.yml
+++ b/test/integration/targets/test_core/tasks/main.yml
@@ -235,6 +235,24 @@
   ignore_errors: yes
   register: version_bad_semver
 
+- name: Try version with empty input value
+  debug:
+    msg: "{{ '' is version('1.0', '>') }}"
+  ignore_errors: yes
+  register: version_empty_input
+
+- name: Try version with empty comparison value
+  debug:
+    msg: "{{ '1.0' is version('', '>') }}"
+  ignore_errors: yes
+  register: version_empty_comparison
+
+- name: Try version with empty input and comparison values
+  debug:
+    msg: "{{ '' is version('', '>') }}"
+  ignore_errors: yes
+  register: version_empty_both
+
 - name: Assert version tests work
   assert:
     that:
@@ -258,6 +276,12 @@
       - version_strict_version_type is failed
       - version_bad_version_type is failed
       - version_bad_semver is failed
+      - version_empty_input is failed
+      - version_empty_input is search('version value cannot be empty')
+      - version_empty_comparison is failed
+      - version_empty_comparison is search('to compare against cannot be empty')
+      - version_empty_both is failed
+      - version_empty_both is search('version value cannot be empty')
 
 - name: Assert any tests work
   assert:
@@ -288,7 +312,7 @@
       - '"off" is not truthy(convert_bool=True)'
       - '"fred" is truthy(convert_bool=True)'
       - '{} is not truthy'
-      - '{"key": "value"} is truthy'      
+      - '{"key": "value"} is truthy'
 
 - name: Assert falsy tests work
   assert: