diff --git a/lib/ansible/module_utils/csharp/Ansible.Basic.cs b/lib/ansible/module_utils/csharp/Ansible.Basic.cs index 5266035973b..eee985e6a81 100644 --- a/lib/ansible/module_utils/csharp/Ansible.Basic.cs +++ b/lib/ansible/module_utils/csharp/Ansible.Basic.cs @@ -553,9 +553,7 @@ namespace Ansible.Basic { string msg = String.Format("argument spec entry contains an invalid key '{0}', valid keys: {1}", key, String.Join(", ", specDefaults.Keys)); - if (optionsContext.Count > 0) - msg += String.Format(" - found in {0}.", String.Join(" -> ", optionsContext)); - throw new ArgumentException(msg); + throw new ArgumentException(FormatOptionsContext(msg, " - ")); } // ensure the value is casted to the type we expect @@ -597,9 +595,7 @@ namespace Ansible.Basic { string msg = String.Format("argument spec for '{0}' did not match expected type {1}: actual type {2}", key, optionType.FullName, actualType.FullName); - if (optionsContext.Count > 0) - msg += String.Format(" - found in {0}.", String.Join(" -> ", optionsContext)); - throw new ArgumentException(msg); + throw new ArgumentException(FormatOptionsContext(msg, " - ")); } } @@ -626,18 +622,14 @@ namespace Ansible.Basic if (!optionTypes.ContainsKey(typeValue)) { string msg = String.Format("{0} '{1}' is unsupported", key, typeValue); - if (optionsContext.Count > 0) - msg += String.Format(" - found in {0}", String.Join(" -> ", optionsContext)); - msg += String.Format(". Valid types are: {0}", String.Join(", ", optionTypes.Keys)); + msg = String.Format("{0}. Valid types are: {1}", FormatOptionsContext(msg, " - "), String.Join(", ", optionTypes.Keys)); throw new ArgumentException(msg); } } else if (!(entry.Value is Delegate)) { string msg = String.Format("{0} must either be a string or delegate, was: {1}", key, valueType.FullName); - if (optionsContext.Count > 0) - msg += String.Format(" - found in {0}", String.Join(" -> ", optionsContext)); - throw new ArgumentException(msg); + throw new ArgumentException(FormatOptionsContext(msg, " - ")); } } } @@ -755,34 +747,43 @@ namespace Ansible.Basic { string msg = String.Format("argument for {0} is of type {1} and we were unable to convert to {2}: {3}", k, value.GetType(), type, e.InnerException.Message); - if (optionsContext.Count > 0) - msg += String.Format(" found in {0}", String.Join(" -> ", optionsContext)); - FailJson(msg); + FailJson(FormatOptionsContext(msg)); } // ensure it matches the choices if there are choices set - List choices = (List)v["choices"]; + List choices = ((List)v["choices"]).Select(x => x.ToString()).Cast().ToList(); if (choices.Count > 0) { - // allow one or more when type="list" param with choices + List values; + string choiceMsg; if (type == "list") { - var diffList = ((List)value).Except(choices).ToList(); - if (diffList.Count > 0) - { - string msg = String.Format("value of {0} must be one or more of: {1}. Got no match for: {2}", - k, String.Join(", ", choices), String.Join(", ", diffList)); - if (optionsContext.Count > 0) - msg += String.Format(" found in {0}", String.Join(" -> ", optionsContext)); - FailJson(msg); - } + values = ((List)value).Select(x => x.ToString()).Cast().ToList(); + choiceMsg = "one or more of"; } - else if (!choices.Contains(value)) + else { - string msg = String.Format("value of {0} must be one of: {1}, got: {2}", k, String.Join(", ", choices), value); - if (optionsContext.Count > 0) - msg += String.Format(" found in {0}", String.Join(" -> ", optionsContext)); - FailJson(msg); + values = new List() { value.ToString() }; + choiceMsg = "one of"; + } + + List diffList = values.Except(choices, StringComparer.OrdinalIgnoreCase).ToList(); + List caseDiffList = values.Except(choices).ToList(); + if (diffList.Count > 0) + { + string msg = String.Format("value of {0} must be {1}: {2}. Got no match for: {3}", + k, choiceMsg, String.Join(", ", choices), String.Join(", ", diffList)); + FailJson(FormatOptionsContext(msg)); + } + else if (caseDiffList.Count > 0) + { + // For backwards compatibility with Legacy.psm1 we need to be matching choices that are not case sensitive. + // We will warn the user it was case insensitive and tell them this will become case sensitive in the future. + string msg = String.Format( + "value of {0} was a case insensitive match of {1}: {2}. Checking of choices will be case sensitive in a future Ansible release. Case insensitive matches were: {3}", + k, choiceMsg, String.Join(", ", choices), String.Join(", ", caseDiffList.Select(x => RemoveNoLogValues(x, noLogValues))) + ); + Warn(FormatOptionsContext(msg)); } } } @@ -848,9 +849,7 @@ namespace Ansible.Basic { legalInputs.RemoveAll(x => passVars.Keys.Contains(x.Replace("_ansible_", ""))); string msg = String.Format("Unsupported parameters for ({0}) module: {1}", ModuleName, String.Join(", ", unsupportedParameters)); - if (optionsContext.Count > 0) - msg += String.Format(" found in {0}", String.Join(" -> ", optionsContext)); - msg += String.Format(". Supported parameters include: {0}", String.Join(", ", legalInputs)); + msg = String.Format("{0}. Supported parameters include: {1}", FormatOptionsContext(msg), String.Join(", ", legalInputs)); FailJson(msg); } } @@ -871,9 +870,7 @@ namespace Ansible.Basic if (count > 1) { string msg = String.Format("parameters are mutually exclusive: {0}", String.Join(", ", mutualCheck)); - if (optionsContext.Count > 0) - msg += String.Format(" found in {0}", String.Join(" -> ", optionsContext)); - FailJson(msg); + FailJson(FormatOptionsContext(msg)); } } } @@ -899,9 +896,7 @@ namespace Ansible.Basic if (missing.Count > 0) { string msg = String.Format("missing required arguments: {0}", String.Join(", ", missing)); - if (optionsContext.Count > 0) - msg += String.Format(" found in {0}", String.Join(" -> ", optionsContext)); - FailJson(msg); + FailJson(FormatOptionsContext(msg)); } } @@ -923,9 +918,7 @@ namespace Ansible.Basic if (found.Contains(true) && found.Contains(false)) { string msg = String.Format("parameters are required together: {0}", String.Join(", ", requiredCheck)); - if (optionsContext.Count > 0) - msg += String.Format(" found in {0}", String.Join(" -> ", optionsContext)); - FailJson(msg); + FailJson(FormatOptionsContext(msg)); } } } @@ -946,9 +939,7 @@ namespace Ansible.Basic if (count == 0) { string msg = String.Format("one of the following is required: {0}", String.Join(", ", requiredCheck)); - if (optionsContext.Count > 0) - msg += String.Format(" found in {0}", String.Join(" -> ", optionsContext)); - FailJson(msg); + FailJson(FormatOptionsContext(msg)); } } } @@ -993,9 +984,7 @@ namespace Ansible.Basic { string msg = String.Format("{0} is {1} but {2} of the following are missing: {3}", key, val.ToString(), term, String.Join(", ", missing)); - if (optionsContext.Count > 0) - msg += String.Format(" found in {0}", String.Join(" -> ", optionsContext)); - FailJson(msg); + FailJson(FormatOptionsContext(msg)); } } } @@ -1036,9 +1025,7 @@ namespace Ansible.Basic { string msg = String.Format("argument for list entry {0} is of type {1} and we were unable to convert to {2}: {3}", key, element.GetType(), elements, e.Message); - if (optionsContext.Count > 0) - msg += String.Format(" found in {0}", String.Join(" -> ", optionsContext)); - FailJson(msg); + FailJson(FormatOptionsContext(msg)); } } } @@ -1216,6 +1203,13 @@ namespace Ansible.Basic cleanupFiles = new List(); } + private string FormatOptionsContext(string msg, string prefix = " ") + { + if (optionsContext.Count > 0) + msg += String.Format("{0}found in {1}", prefix, String.Join(" -> ", optionsContext)); + return msg; + } + [DllImport("kernel32.dll")] private static extern IntPtr GetConsoleWindow(); diff --git a/test/integration/targets/win_certificate_store/tasks/test.yml b/test/integration/targets/win_certificate_store/tasks/test.yml index eac2039b833..8663424c064 100644 --- a/test/integration/targets/win_certificate_store/tasks/test.yml +++ b/test/integration/targets/win_certificate_store/tasks/test.yml @@ -5,7 +5,7 @@ path: '{{win_cert_dir}}\subj-cert.pem' store_location: FakeLocation register: fail_fake_location - failed_when: "fail_fake_location.msg != 'value of store_location must be one of: CurrentUser, LocalMachine, got: FakeLocation'" + failed_when: "fail_fake_location.msg != 'value of store_location must be one of: CurrentUser, LocalMachine. Got no match for: FakeLocation'" - name: fail with invalid store name win_certificate_store: @@ -13,7 +13,7 @@ path: '{{win_cert_dir}}\subj-cert.pem' store_name: FakeName register: fail_fake_name - failed_when: "fail_fake_name.msg != 'value of store_name must be one of: AddressBook, AuthRoot, CertificateAuthority, Disallowed, My, Root, TrustedPeople, TrustedPublisher, got: FakeName'" + failed_when: "fail_fake_name.msg != 'value of store_name must be one of: AddressBook, AuthRoot, CertificateAuthority, Disallowed, My, Root, TrustedPeople, TrustedPublisher. Got no match for: FakeName'" - name: fail when state=present and no path is set win_certificate_store: diff --git a/test/integration/targets/win_csharp_utils/library/ansible_basic_tests.ps1 b/test/integration/targets/win_csharp_utils/library/ansible_basic_tests.ps1 index cf06611989e..4b87c1160fc 100644 --- a/test/integration/targets/win_csharp_utils/library/ansible_basic_tests.ps1 +++ b/test/integration/targets/win_csharp_utils/library/ansible_basic_tests.ps1 @@ -1172,7 +1172,7 @@ test_no_log - Invoked with: $expected_msg = "internal error: argument spec entry contains an invalid key 'invalid', valid keys: apply_defaults, " $expected_msg += "aliases, choices, default, elements, mutually_exclusive, no_log, options, removed_in_version, " $expected_msg += "required, required_if, required_one_of, required_together, supports_check_mode, type - " - $expected_msg += "found in option_key -> sub_option_key." + $expected_msg += "found in option_key -> sub_option_key" $actual.Keys.Count | Assert-Equals -Expected 3 $actual.failed | Assert-Equals -Expected $true @@ -1446,6 +1446,114 @@ test_no_log - Invoked with: $actual.invocation | Assert-DictionaryEquals -Expected @{module_args = $complex_args} } + "Numeric choices" = { + $spec = @{ + options = @{ + option_key = @{ + choices = 1, 2, 3 + type = "int" + } + } + } + $complex_args = @{ + option_key = "2" + } + + $m = [Ansible.Basic.AnsibleModule]::Create(@(), $spec) + try { + $m.ExitJson() + } catch [System.Management.Automation.RuntimeException] { + $output = [Ansible.Basic.AnsibleModule]::FromJson($_test_out) + } + $output.Keys.Count | Assert-Equals -Expected 2 + $output.changed | Assert-Equals -Expected $false + $output.invocation | Assert-DictionaryEquals -Expected @{module_args = @{option_key = 2}} + } + + "Case insensitive choice" = { + $spec = @{ + options = @{ + option_key = @{ + choices = "abc", "def" + } + } + } + $complex_args = @{ + option_key = "ABC" + } + + $m = [Ansible.Basic.AnsibleModule]::Create(@(), $spec) + try { + $m.ExitJson() + } catch [System.Management.Automation.RuntimeException] { + $output = [Ansible.Basic.AnsibleModule]::FromJson($_test_out) + } + $expected_warning = "value of option_key was a case insensitive match of one of: abc, def. " + $expected_warning += "Checking of choices will be case sensitive in a future Ansible release. " + $expected_warning += "Case insensitive matches were: ABC" + + $output.invocation | Assert-DictionaryEquals -Expected @{module_args = @{option_key = "ABC"}} + $output.warnings.Count | Assert-Equals -Expected 1 + $output.warnings[0] | Assert-Equals -Expected $expected_warning + } + + "Case insensitive choice no_log" = { + $spec = @{ + options = @{ + option_key = @{ + choices = "abc", "def" + no_log = $true + } + } + } + $complex_args = @{ + option_key = "ABC" + } + + $m = [Ansible.Basic.AnsibleModule]::Create(@(), $spec) + try { + $m.ExitJson() + } catch [System.Management.Automation.RuntimeException] { + $output = [Ansible.Basic.AnsibleModule]::FromJson($_test_out) + } + $expected_warning = "value of option_key was a case insensitive match of one of: abc, def. " + $expected_warning += "Checking of choices will be case sensitive in a future Ansible release. " + $expected_warning += "Case insensitive matches were: VALUE_SPECIFIED_IN_NO_LOG_PARAMETER" + + $output.invocation | Assert-DictionaryEquals -Expected @{module_args = @{option_key = "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER"}} + $output.warnings.Count | Assert-Equals -Expected 1 + $output.warnings[0] | Assert-Equals -Expected $expected_warning + } + + "Case insentitive choice as list" = { + $spec = @{ + options = @{ + option_key = @{ + choices = "abc", "def", "ghi", "JKL" + type = "list" + elements = "str" + } + } + } + $complex_args = @{ + option_key = "AbC", "ghi", "jkl" + } + + $m = [Ansible.Basic.AnsibleModule]::Create(@(), $spec) + try { + $m.ExitJson() + } catch [System.Management.Automation.RuntimeException] { + $output = [Ansible.Basic.AnsibleModule]::FromJson($_test_out) + } + $expected_warning = "value of option_key was a case insensitive match of one or more of: abc, def, ghi, JKL. " + $expected_warning += "Checking of choices will be case sensitive in a future Ansible release. " + $expected_warning += "Case insensitive matches were: AbC, jkl" + + $output.invocation | Assert-DictionaryEquals -Expected @{module_args = $complex_args} + $output.warnings.Count | Assert-Equals -Expected 1 + $output.warnings[0] | Assert-Equals -Expected $expected_warning + } + "Invalid choice" = { $spec = @{ options = @{ @@ -1468,7 +1576,7 @@ test_no_log - Invoked with: } $failed | Assert-Equals -Expected $true - $expected_msg = "value of option_key must be one of: a, b, got: c" + $expected_msg = "value of option_key must be one of: a, b. Got no match for: c" $actual.Keys.Count | Assert-Equals -Expected 4 $actual.changed | Assert-Equals -Expected $false @@ -1500,7 +1608,7 @@ test_no_log - Invoked with: } $failed | Assert-Equals -Expected $true - $expected_msg = "value of option_key must be one of: a, b, got: ***" + $expected_msg = "value of option_key must be one of: a, b. Got no match for: ***" $actual.Keys.Count | Assert-Equals -Expected 4 $actual.changed | Assert-Equals -Expected $false