argspec - fix validating type for required options that are None (#79677) (#80542)

* Only bypass type validation for null parameters if the default is None. A default is mutually exclusive with required.

* Prevent coercing None to str type. Fail the type check instead.

(cherry picked from commit 694c11d5bdc7f5f7779d27315bec939dc9162ec6)

6 files changed, 140 insertions, 6 deletions

diff --git a/changelogs/fragments/79677-fix-argspec-type-check.yml b/changelogs/fragments/79677-fix-argspec-type-check.yml
new file mode 100644
index 0000000000..3fe8b0f420
--- /dev/null
+++ b/changelogs/fragments/79677-fix-argspec-type-check.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - module/role argument spec - validate the type for options that are None when the option is required or has a non-None default (https://github.com/ansible/ansible/issues/79656).
diff --git a/lib/ansible/module_utils/common/parameters.py b/lib/ansible/module_utils/common/parameters.py
index cc94889e4b..386eb875ce 100644
--- a/lib/ansible/module_utils/common/parameters.py
+++ b/lib/ansible/module_utils/common/parameters.py
@@ -609,7 +609,7 @@ def _validate_argument_types(argument_spec, parameters, prefix='', options_conte
             continue
 
         value = parameters[param]
-        if value is None:
+        if value is None and not spec.get('required') and spec.get('default') is None:
             continue
 
         wanted_type = spec.get('type')
diff --git a/lib/ansible/module_utils/common/validation.py b/lib/ansible/module_utils/common/validation.py
index 5a4cebbc20..fa494d4d61 100644
--- a/lib/ansible/module_utils/common/validation.py
+++ b/lib/ansible/module_utils/common/validation.py
@@ -381,7 +381,7 @@ def check_type_str(value, allow_conversion=True, param=None, prefix=''):
     if isinstance(value, string_types):
         return value
 
-    if allow_conversion:
+    if allow_conversion and value is not None:
         return to_native(value, errors='surrogate_or_strict')
 
     msg = "'{0!r}' is not a string and conversion is not allowed".format(value)
diff --git a/test/integration/targets/apt_repository/tasks/apt.yml b/test/integration/targets/apt_repository/tasks/apt.yml
index cb418c3436..28bb902913 100644
--- a/test/integration/targets/apt_repository/tasks/apt.yml
+++ b/test/integration/targets/apt_repository/tasks/apt.yml
@@ -240,7 +240,7 @@
 - assert:
     that:
       - result is failed
-      - result.msg == 'Please set argument \'repo\' to a non-empty value'
+      - result.msg.startswith("argument 'repo' is of type <class 'NoneType'> and we were unable to convert to str")
 
 - name: Test apt_repository with an empty value for repo
   apt_repository:
diff --git a/test/integration/targets/roles_arg_spec/roles/c/meta/main.yml b/test/integration/targets/roles_arg_spec/roles/c/meta/main.yml
index 1a1ccbe4a1..10dce6d21c 100644
--- a/test/integration/targets/roles_arg_spec/roles/c/meta/main.yml
+++ b/test/integration/targets/roles_arg_spec/roles/c/meta/main.yml
@@ -2,6 +2,15 @@ argument_specs:
     main:
         short_description: Main entry point for role C.
         options:
+            c_dict:
+                type: "dict"
+                required: true
             c_int:
                 type: "int"
                 required: true
+            c_list:
+                type: "list"
+                required: true
+            c_raw:
+                type: "raw"
+                required: true
diff --git a/test/integration/targets/roles_arg_spec/test.yml b/test/integration/targets/roles_arg_spec/test.yml
index b2b44c7ffd..b88d2e183d 100644
--- a/test/integration/targets/roles_arg_spec/test.yml
+++ b/test/integration/targets/roles_arg_spec/test.yml
@@ -145,7 +145,10 @@
   hosts: localhost
   gather_facts: false
   vars:
+    c_dict: {}
     c_int: 1
+    c_list: []
+    c_raw: ~
     a_str: "some string"
     a_int: 42
   tasks:
@@ -157,6 +160,125 @@
         include_role:
           name: c
 
+- name: "New play to reset vars: Test nested role including/importing role fails with null required options"
+  hosts: localhost
+  gather_facts: false
+  vars:
+    a_main_spec:
+      a_str:
+        required: true
+        type: "str"
+    c_main_spec:
+      c_int:
+        required: true
+        type: "int"
+      c_list:
+        required: true
+        type: "list"
+      c_dict:
+        required: true
+        type: "dict"
+      c_raw:
+        required: true
+        type: "raw"
+    # role c calls a's main and alternate entrypoints
+    a_str: ''
+    c_dict: {}
+    c_int: 0
+    c_list: []
+    c_raw: ~
+  tasks:
+    - name: test type coercion fails on None for required str
+      block:
+        - name: "Test import_role of role C (missing a_str)"
+          import_role:
+            name: c
+          vars:
+            a_str: ~
+        - fail:
+            msg: "Should not get here"
+      rescue:
+        - debug:
+            var: ansible_failed_result
+        - name: "Validate import_role failure"
+          assert:
+            that:
+              # NOTE: a bug here that prevents us from getting ansible_failed_task
+              - ansible_failed_result.argument_errors == [error]
+              - ansible_failed_result.argument_spec_data == a_main_spec
+          vars:
+            error: >-
+              argument 'a_str' is of type <class 'NoneType'> and we were unable to convert to str:
+              'None' is not a string and conversion is not allowed
+
+    - name: test type coercion fails on None for required int
+      block:
+        - name: "Test import_role of role C (missing c_int)"
+          import_role:
+            name: c
+          vars:
+            c_int: ~
+        - fail:
+            msg: "Should not get here"
+      rescue:
+        - debug:
+            var: ansible_failed_result
+        - name: "Validate import_role failure"
+          assert:
+            that:
+              # NOTE: a bug here that prevents us from getting ansible_failed_task
+              - ansible_failed_result.argument_errors == [error]
+              - ansible_failed_result.argument_spec_data == c_main_spec
+          vars:
+            error: >-
+              argument 'c_int' is of type <class 'NoneType'> and we were unable to convert to int:
+              <class 'NoneType'> cannot be converted to an int
+
+    - name: test type coercion fails on None for required list
+      block:
+        - name: "Test import_role of role C (missing c_list)"
+          import_role:
+            name: c
+          vars:
+            c_list: ~
+        - fail:
+            msg: "Should not get here"
+      rescue:
+        - debug:
+            var: ansible_failed_result
+        - name: "Validate import_role failure"
+          assert:
+            that:
+              # NOTE: a bug here that prevents us from getting ansible_failed_task
+              - ansible_failed_result.argument_errors == [error]
+              - ansible_failed_result.argument_spec_data == c_main_spec
+          vars:
+            error: >-
+              argument 'c_list' is of type <class 'NoneType'> and we were unable to convert to list:
+              <class 'NoneType'> cannot be converted to a list
+
+    - name: test type coercion fails on None for required dict
+      block:
+        - name: "Test import_role of role C (missing c_dict)"
+          import_role:
+            name: c
+          vars:
+            c_dict: ~
+        - fail:
+            msg: "Should not get here"
+      rescue:
+        - debug:
+            var: ansible_failed_result
+        - name: "Validate import_role failure"
+          assert:
+            that:
+              # NOTE: a bug here that prevents us from getting ansible_failed_task
+              - ansible_failed_result.argument_errors == [error]
+              - ansible_failed_result.argument_spec_data == c_main_spec
+          vars:
+            error: >-
+              argument 'c_dict' is of type <class 'NoneType'> and we were unable to convert to dict:
+              <class 'NoneType'> cannot be converted to a dict
 
 - name: "New play to reset vars: Test nested role including/importing role fails"
   hosts: localhost
@@ -171,13 +293,15 @@
         required: true
         type: "int"
 
+    c_int: 100
+    c_list: []
+    c_dict: {}
+    c_raw: ~
   tasks:
       - block:
         - name: "Test import_role of role C (missing a_str)"
           import_role:
             name: c
-          vars:
-            c_int: 100
 
         - fail:
             msg: "Should not get here"
@@ -202,7 +326,6 @@
           include_role:
             name: c
           vars:
-            c_int: 200
             a_str: "some string"
 
         - fail: