From 7b8e814a10e0410dc9f26b83016a7f5086ced4d1 Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Thu, 14 Feb 2019 12:55:43 +1000 Subject: [PATCH] Ansible.Basic: make module options case insensitive with dep warning (#51583) * Ansible.Basic: make module options case insensitive with dep warning * Add porting guide info --- .../rst/porting_guides/porting_guide_2.8.rst | 4 ++ .../module_utils/csharp/Ansible.Basic.cs | 25 ++++++++++- .../library/ansible_basic_tests.ps1 | 45 +++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/docs/docsite/rst/porting_guides/porting_guide_2.8.rst b/docs/docsite/rst/porting_guides/porting_guide_2.8.rst index 266343a5b2a..091014a3586 100644 --- a/docs/docsite/rst/porting_guides/porting_guide_2.8.rst +++ b/docs/docsite/rst/porting_guides/porting_guide_2.8.rst @@ -112,6 +112,10 @@ add ``$ErrorActionPreference = "Continue"`` to the top of the module. This chang of the EAP that was accidentally removed in a previous release and ensure that modules are more resiliant to errors that may occur in execution. +PowerShell module options and option choices are currently case insensitive to what is defined in the module +specification. This behaviour is deprecated and a warning displayed to the user if a case insensitive match was found. +A future release of Ansible will make these checks case sensitive. + Modules removed --------------- diff --git a/lib/ansible/module_utils/csharp/Ansible.Basic.cs b/lib/ansible/module_utils/csharp/Ansible.Basic.cs index eee985e6a81..e5d99b2daa2 100644 --- a/lib/ansible/module_utils/csharp/Ansible.Basic.cs +++ b/lib/ansible/module_utils/csharp/Ansible.Basic.cs @@ -809,13 +809,18 @@ namespace Ansible.Basic private void CheckUnsupportedArguments(IDictionary param, List legalInputs) { HashSet unsupportedParameters = new HashSet(); + HashSet caseUnsupportedParameters = new HashSet(); List removedParameters = new List(); foreach (DictionaryEntry entry in param) { string paramKey = (string)entry.Key; - if (!legalInputs.Contains(paramKey)) + if (!legalInputs.Contains(paramKey, StringComparer.OrdinalIgnoreCase)) unsupportedParameters.Add(paramKey); + else if (!legalInputs.Contains(paramKey)) + // For backwards compatibility we do not care about the case but we need to warn the users as this will + // change in a future Ansible release. + caseUnsupportedParameters.Add(paramKey); else if (paramKey.StartsWith("_ansible_")) { removedParameters.Add(paramKey); @@ -852,6 +857,24 @@ namespace Ansible.Basic msg = String.Format("{0}. Supported parameters include: {1}", FormatOptionsContext(msg), String.Join(", ", legalInputs)); FailJson(msg); } + + if (caseUnsupportedParameters.Count > 0) + { + legalInputs.RemoveAll(x => passVars.Keys.Contains(x.Replace("_ansible_", ""))); + string msg = String.Format("Parameters for ({0}) was a case insensitive match: {1}", ModuleName, String.Join(", ", caseUnsupportedParameters)); + msg = String.Format("{0}. Module options will become case sensitive in a future Ansible release. Supported parameters include: {1}", + FormatOptionsContext(msg), String.Join(", ", legalInputs)); + Warn(msg); + } + + // Make sure we convert all the incorrect case params to the ones set by the module spec + foreach (string key in caseUnsupportedParameters) + { + string correctKey = legalInputs[legalInputs.FindIndex(s => s.Equals(key, StringComparison.OrdinalIgnoreCase))]; + object value = param[key]; + param.Remove(key); + param.Add(correctKey, value); + } } private void CheckMutuallyExclusive(IDictionary param, IList mutuallyExclusive) 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 4b87c1160fc..8118a0f7264 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 @@ -503,6 +503,51 @@ $tests = @{ $actual.invocation | Assert-DictionaryEquals -Expected @{module_args = $expected_module_args} } + "Parse module args with case insensitive input" = { + $spec = @{ + options = @{ + option1 = @{ type = "int"; required = $true } + } + } + $complex_args = @{ + _ansible_module_name = "win_test" + Option1 = "1" + } + + $m = [Ansible.Basic.AnsibleModule]::Create(@(), $spec) + # Verifies the case of the params key is set to the module spec not actual input + $m.Params.Keys | Assert-Equals -Expected @("option1") + $m.Params.option1 | Assert-Equals -Expected 1 + + # Verifies the type conversion happens even on a case insensitive match + $m.Params.option1.GetType().FullName | Assert-Equals -Expected "System.Int32" + + $failed = $false + try { + $m.ExitJson() + } catch [System.Management.Automation.RuntimeException] { + $failed = $true + $_.Exception.Message | Assert-Equals -Expected "exit: 0" + $actual = [Ansible.Basic.AnsibleModule]::FromJson($_test_out) + } + $failed | Assert-Equals -Expected $true + + $expected_warnings = "Parameters for (win_test) was a case insensitive match: Option1. " + $expected_warnings += "Module options will become case sensitive in a future Ansible release. " + $expected_warnings += "Supported parameters include: option1" + + $expected = @{ + changed = $false + invocation = @{ + module_args = @{ + option1 = 1 + } + } + warnings = @($expected_warnings) + } + $actual | Assert-DictionaryEquals -Expected $expected + } + "No log values" = { $spec = @{ options = @{